feat(scripts): gate triage-triggered code/fix PRs behind write+ approval #523
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| #!/usr/bin/env bash | ||
| # write-approval-gate.lib.sh — Merge gate for triage-role-triggered code/fix runs. | ||
| # | ||
| # Source from post-code.src.sh / post-fix.src.sh (after post-failure-report.lib.sh | ||
| # for gha_echo): | ||
| # source "${SCRIPT_DIR}/lib/write-approval-gate.lib.sh" | ||
| # | ||
| # fullsend-ai/fullsend#5687: dispatch now accepts the GitHub `triage` role for | ||
| # /fs-code and /fs-fix. Triage-role users still get a bot-authored PR (the | ||
| # agent always held write-level credentials), but that PR must carry an | ||
| # explicit visible marker so reviewers and merge tooling know it needs a | ||
| # write+ collaborator's approval — the actual enforcement (requiring an | ||
| # approval from a currently write+ user, not just any reviewDecision) lives | ||
| # in skills/merge-queue/scripts/await-and-enqueue.sh, which checks this label. | ||
|
|
||
| # shellcheck shell=bash | ||
|
|
||
| [[ -n "${WRITE_APPROVAL_GATE_SH_LOADED:-}" ]] && return 0 | ||
| WRITE_APPROVAL_GATE_SH_LOADED=1 | ||
|
|
||
| # Emit a runner warning through gha_echo when available. | ||
| _write_approval_gate_warn() { | ||
| if declare -F gha_echo >/dev/null 2>&1; then | ||
| gha_echo warning "$*" | ||
| else | ||
| echo "warning: $*" >&2 | ||
| fi | ||
| } | ||
|
|
||
| # Normalize a TRIGGER_ROLE value: lowercase and trim surrounding whitespace. | ||
| _normalize_trigger_role() { | ||
| printf '%s' "${1:-}" | tr '[:upper:]' '[:lower:]' | xargs | ||
| } | ||
|
|
||
| # Apply the needs-write-approval label to a PR when TRIGGER_ROLE is "triage" | ||
| # (case-insensitive, surrounding whitespace ignored). No-op (and no gh calls) | ||
| # when TRIGGER_ROLE is unset or "write" — unset means the trigger was resolved | ||
| # at write+ (or a path that predates TRIGGER_ROLE, e.g. bot-triggered | ||
| # review->fix), so no gate is needed. Any other non-empty value is treated | ||
| # the same as "write" (no gate) but logged, since it likely indicates a bug | ||
| # in the caller rather than a legitimate write+ trigger. | ||
| # Requires REPO_FULL_NAME. Best-effort: never fails the calling script. | ||
| # Note: parameter is target_pr (not pr_number) to avoid SC2153 against | ||
| # PR_NUMBER from post-failure-report.lib.sh once both libs are bundled into | ||
|
There was a problem hiding this comment. [medium] fail-open The merge gate for triage-role PRs is best-effort only: if gh pr edit --add-label fails, the PR proceeds without the needs-write-approval label. The enforcement side (write_approval_ever_required) checks the issue-events timeline for a labeled event — a failed label application means the enforcement gate sees ever_required == false and returns 0, making the triage-triggered PR indistinguishable from a write-triggered PR. Suggested fix: Make the label application a hard failure (exit non-zero), or use a second enforcement signal so enforcement does not depend solely on the label. |
||
| # post-code.sh / post-fix.sh — same fix as maybe_assign_pr in pr-assignee.lib.sh. | ||
| apply_write_approval_gate_if_needed() { | ||
| local target_pr="$1" | ||
| local role | ||
| role="$(_normalize_trigger_role "${TRIGGER_ROLE:-}")" | ||
|
|
||
| if [[ -z "${role}" ]]; then | ||
| return 0 | ||
| fi | ||
| if [[ "${role}" != "triage" ]]; then | ||
| if [[ "${role}" != "write" ]]; then | ||
| _write_approval_gate_warn "Unrecognized TRIGGER_ROLE '${TRIGGER_ROLE}' — treating as write (no gate applied). Expected 'triage' or 'write'." | ||
| fi | ||
| return 0 | ||
| fi | ||
|
|
||
| echo "Trigger role is 'triage' — applying needs-write-approval gate to PR #${target_pr}" | ||
| gh label create "needs-write-approval" --repo "${REPO_FULL_NAME}" \ | ||
| --description "Triggered by a triage-role user; needs write+ approval before merge" \ | ||
| --color "B60205" 2>/dev/null || true | ||
| gh pr edit "${target_pr}" --repo "${REPO_FULL_NAME}" \ | ||
|
There was a problem hiding this comment. [medium] fail-open The merge gate for triage-role PRs is best-effort only: if gh pr edit --add-label fails (network error, token permission issue, API rate limit), the PR proceeds without the needs-write-approval label and has no merge gate. The 2>/dev/null || _write_approval_gate_warn pattern suppresses the error and continues. Since the label is the sole enforcement mechanism in this code, a transient gh failure means a triage-triggered PR becomes indistinguishable from a write-triggered PR — a silent fail-open on the authorization gate. Suggested fix: Consider making the label application non-best-effort for the triage path (exit with error or retry on failure), or implement complementary server-side enforcement so the label is defense-in-depth rather than the sole gate. |
||
| --add-label "needs-write-approval" 2>/dev/null || \ | ||
| _write_approval_gate_warn "Failed to apply needs-write-approval label to PR #${target_pr}" | ||
|
Comment on lines
+61
to
+67
There was a problem hiding this comment. 1. Gate label application fail-open When TRIGGER_ROLE=triage, apply_write_approval_gate_if_needed treats gh pr edit failures as warnings and returns success, so post-code/post-fix can continue and a triage-triggered PR may proceed without the required needs-write-approval merge gate label. This creates a fail-open authorization path where labeling errors (permissions/auth/network/API) can silently bypass the intended enforcement signal. Agent Prompt
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[low] authorization-bypass
Unrecognized TRIGGER_ROLE values are treated as write (no gate applied). If a new role is introduced and this script is not updated, PRs triggered by that role will bypass the write-approval gate.
Suggested fix: Invert the logic: only allow known safe roles to skip the gate.