fix(auto-merge): harden trusted actor enforcement - #670
Conversation
![botantler-1[bot]](https://avatars.githubusercontent.com/in/1025824?s=60&v=4){kind=small}
Code Coverage OverviewLanguages: Go, C# Go / code-coverage/testThe overall coverage remains at 50%, unchanged from the C# / code-coverage/dotnetThe overall coverage remains at 100%, unchanged from the Updated |
✅MegaLinter analysis: Success
Notices📣 MegaLinter 9.5.0 is out! Discover the new features and security recommendations in the release announcement. (Skip this info by defining See detailed reports in MegaLinter artifacts Your project could benefit from a custom flavor, which would allow you to run only the linters you need, and thus improve runtime performances. (Skip this info by defining
|
|
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 15abdec. Configure here.
devantler
changed the title
Refreshed the existing draft onto current main at cbafde3 and addressed the unresolved Bugbot finding. The actor allowlist now applies only to synchronize events; reopened and ready-for-review events remain eligible for trusted bot PRs when a maintainer triggers them. Local proof at cbafde3:
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
|
Warning Review limit reached
Next review available in: 24 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe auto-merge workflow adds opt-in actor-trust enforcement, actor-aware eligibility classification, and a disarm job that revokes rejected auto-merge or merge-queue state. Freshness and concurrency checks prevent stale events from arming merges. Fixtures, shell tests, CI aggregation, and README documentation cover the new behavior. Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/tests/test-enable-auto-merge-author-gate.sh:
- Around line 72-78: Update the test setup around allowlist_json to define the
trusted PR-author list explicitly, rather than deriving it from the .eligible
fixture entries. Keep the existing reviewers_json pattern and ensure
expected_condition continues using the explicit trusted-author JSON for all
author checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6db6ba78-3837-44ce-b003-353a25410494
📒 Files selected for processing (3)
.github/tests/enable-auto-merge-authors.json.github/tests/test-enable-auto-merge-author-gate.sh.github/workflows/enable-auto-merge.yaml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
.github/workflows/*.yaml: Keep all GitHub Actions workflows under.github/workflows/.
Reusable workflows must use theworkflow_calltrigger.
Pin every remote action reference to a full commit SHA with a version comment; do not use remote self-references.
Includestep-security/harden-runneras the first step of every reusable-workflow job, withegress-policy: audit.
Set top-level workflow permissions to{}and grant permissions per job.
Setpersist-credentials: falseonactions/checkoutunless the job must push.
Workflows used as organization-level rulesets must includepull_requestandmerge_grouptriggers in addition toworkflow_call.
For reusable workflows referencing a sibling action, check out the workflow repository at${{ job.workflow_sha }}into.devantler-tech-actions, then invoke the action locally; remove the checkout before workspace-wide scans or commits.
New reusable-workflow jobs, steps, or behaviors must be behind a default-off boolean opt-in input and guarded withif: ${{ inputs.<enable-x> }}.
When a workflow supports bothworkflow_dispatchandworkflow_call, normalize boolean inputs withinputs.<enable-x> == true || inputs.<enable-x> == 'true'.
Test both enabled and disabled states of every feature flag with CI test jobs.
Gating reusable workflows must have both a passing self-test and a failing-input self-test that verifies the expected finding; non-gating workflows require happy-path coverage.
Preserve tested consumer contracts, such asvalidate-go-project.yamlhonoring.govulncheck-allow.txt; update the corresponding self-tests whenever the implementation changes.
Files:
.github/workflows/enable-auto-merge.yaml
.github/tests/**
📄 CodeRabbit inference engine (AGENTS.md)
Place deliberately bad fixtures for gating-workflow tests outside the gate's normal scan scope.
Files:
.github/tests/test-enable-auto-merge-author-gate.sh.github/tests/enable-auto-merge-authors.json
🔇 Additional comments (3)
.github/workflows/enable-auto-merge.yaml (1)
65-82: LGTM!.github/tests/enable-auto-merge-authors.json (1)
7-109: LGTM!.github/tests/test-enable-auto-merge-author-gate.sh (1)
89-99: LGTM!
Addressed the current-head CodeRabbit finding at 9586391: the test now pins the trusted-author policy explicitly instead of deriving it from expected fixture outcomes. Relevant gate suites and local structure checks remain green; CI is rerunning on this head. |
@coderabbitai review |
|
✅ Action performedReview finished.
|
@codex review for auto-merge actor trust and regression coverage |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 958639185b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Addressed the current-head Codex finding at c6cb321. The rejected synchronize path now disarms stale classic auto-merge and merge-queue state in a separate GITHUB_TOKEN-only job; APP_PRIVATE_KEY is structurally excluded. RED/GREEN author-gate coverage, pentad-gate fixtures, shellcheck, YAML/JSON parsing, and diff hygiene pass locally. CI is rerunning on the new head. |
@coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='1791e38acca872f2a34487dc21a84fb9f94218c0'
echo '== checked-out HEAD =='
git rev-parse HEAD
echo '== requested commit =='
git rev-parse --verify "${target}^{commit}"
git show --no-ext-diff --format=fuller --no-patch "$target"
echo '== delta from first parent =='
git diff-tree --no-commit-id --name-status -r "$target"
git diff --no-ext-diff --check "${target}^" "$target"
git diff --no-ext-diff --unified=100 "${target}^" "$target"
echo '== relevant paths =='
fd -HI -t f . | grep -E '(^|/)(disarm-auto-merge\.sh|test-disarm-auto-merge\.sh|enable-auto-merge\.yaml|ci\.yaml|test-enable-auto-merge-author-gate\.sh|enable-auto-merge-authors\.json)$' || true
echo '== relevant source and test files =='
for f in \
.scripts/disarm-auto-merge.sh \
.github/tests/test-disarm-auto-merge.sh \
.github/tests/disarm-auto-merge-mock-bin/gh \
.github/workflows/enable-auto-merge.yaml \
.github/workflows/ci.yaml
do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f"
fi
done
echo '== references to the disarm script/tests =='
rg -n -C 4 --hidden --glob '!.git/**' \
'(disarm-auto-merge|mergeAutoMerge|enqueuePullRequest|disablePullRequestAutoMerge)' \
.github .scripts README.md 2>/dev/null || trueLength of output: 212690 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== author-gate permission and disarm assertions =='
sed -n '120,205p' .github/tests/test-enable-auto-merge-author-gate.sh
echo '== all disarm-job permission references =='
rg -n -C 5 --hidden --glob '!.git/**' \
'(disarm-untrusted-update|contents:[[:space:]]*(read|write)|pull-requests:[[:space:]]*write)' \
.github/tests .github/workflows/enable-auto-merge.yaml
echo '== parent revision of the disarm permissions =='
git show --no-ext-diff '1791e38^:.github/workflows/enable-auto-merge.yaml' | \
sed -n '154,195p'Length of output: 25496
No new findings. The split fixtures correctly isolate classic auto-merge disarming, merge-queue dequeueing, and the read-only no-op path; each also rejects the other state’s side effect. The surrounding structural checks continue to enforce the token-only disarm boundary, trusted script checkout, and newest-event-wins concurrency behavior. ✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1791e38acc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
@coderabbitai review Fresh substantive review requested for exact head |
@codex review Fresh substantive review requested for exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='61cdc93efdab64186b5f63f981ebb1e440a3427e'
echo '== requested and checked-out commits =='
git rev-parse --verify "${target}^{commit}"
git rev-parse HEAD
git show --no-ext-diff --format=fuller --no-patch "$target"
echo '== delta from first parent =='
git diff-tree --no-commit-id --name-status -r "$target"
git diff --no-ext-diff --check "${target}^" "$target"
git diff --no-ext-diff --unified=100 "${target}^" "$target"
echo '== relevant files =='
fd -HI -t f . | grep -E '(^|/)(enable-auto-merge\.yaml|disarm-auto-merge\.sh|test-disarm-auto-merge\.sh|test-enable-auto-merge-author-gate\.sh|enable-auto-merge-authors\.json|ci\.yaml)$' || true
echo '== workflow: concurrency, eligibility, and disarm sections =='
if [ -f .github/workflows/enable-auto-merge.yaml ]; then
nl -ba .github/workflows/enable-auto-merge.yaml | sed -n '1,360p'
fi
echo '== disarm script and dedicated test =='
for f in .scripts/disarm-auto-merge.sh .github/tests/test-disarm-auto-merge.sh .github/tests/test-enable-auto-merge-author-gate.sh; do
if [ -f "$f" ]; then
echo "--- $f ---"
cat -n "$f"
fi
done
echo '== stale-ordering and fallback references =='
rg -n -C 8 --hidden --glob '!.git/**' \
'(stale|fallback|concurren|cancel-in-progress|synchronize|disarm|updatedAt|head\.sha|workflow_run|event.*order)' \
.github .scripts README.md 2>/dev/null || true
echo '== CI invocation context =='
if [ -f .github/workflows/ci.yaml ]; then
rg -n -C 8 '(test-disarm-auto-merge|test-enable-auto-merge-author-gate)' .github/workflows/ci.yaml || true
fiLength of output: 57950 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='61cdc93efdab64186b5f63f981ebb1e440a3427e'
echo '== complete disarm job through auto-merge resolution =='
git show "${target}:.github/workflows/enable-auto-merge.yaml" | \
awk '/^ disarm-untrusted-update:/{p=1} /^ auto-merge:/{if (p) q=1} {if (p) print} q && /^ - name: 🛂 Verify review and pre-merge gates/{exit}'
echo '== current dedicated disarm regression test =='
git show "${target}:.github/tests/test-disarm-auto-merge.sh"
echo '== current gh mock(s) used by disarm test =='
git ls-tree -r --name-only "$target" | grep -E 'disarm-auto-merge.*(mock|bin)|mock.*disarm-auto-merge' || true
while IFS= read -r path; do
echo "--- $path ---"
git show "${target}:${path}"
done < <(git ls-tree -r --name-only "$target" | grep -E 'disarm-auto-merge.*(mock|bin)|mock.*disarm-auto-merge' || true)
echo '== parent/current test delta for stale-order behavior =='
git diff --no-ext-diff --unified=120 "${target}^" "$target" -- \
.github/tests/test-disarm-auto-merge.sh \
.github/tests/test-enable-auto-merge-author-gate.sh
echo '== all tests and workflow references to ordering-proof inputs =='
git grep -n -E 'LATEST_ACTOR|RUN_ID|EVENT_UPDATED_AT|actions/runs|headRefOid|updatedAt|stale|supersed' "$target" -- \
.github .scripts README.md || true
echo '== GitHub workflow-run API response handling in repository =='
git grep -n -E 'actions/runs|workflow_runs|run_attempt|workflow_id' "$target" -- \
.github .scripts || trueLength of output: 17393 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='61cdc93efdab64186b5f63f981ebb1e440a3427e'
echo '== gh mock used by disarm-script fixtures =='
git show "${target}:.github/tests/disarm-auto-merge-mock-bin/gh"
echo '== author-gate assertions after the ordering section =='
git show "${target}:.github/tests/test-enable-auto-merge-author-gate.sh" | \
awk '/disarm_step_env=/{p=1} p {print} /^exit "\$status"/{exit}'
echo '== all test references to the run-history endpoint and workflow identity =='
git grep -n -E '(actions/runs|workflow_runs|workflow_id|path|LATEST_ACTOR|supersed)' "$target" -- .github/tests || true
echo '== workflow identity fields available in the queried response are not used =='
git show "${target}:.github/workflows/enable-auto-merge.yaml" | \
awk '/run_pages=\$\(gh api/{p=1} p {print} /fi$/{if (++n == 2) exit}'Length of output: 12027
Please restrict the candidate runs to this workflow (a stable workflow ID/path/name from the queried run metadata), and add fixtures for both a later trusted run of this workflow (skip is valid) and a later trusted run of another workflow (must still disarm). The current tests only assert that the query/actor strings exist, and the dedicated script test does not exercise this wrapper-level decision. ✅ Action performedReview finished.
|
Fixed the workflow-identity gap at exact head @coderabbitai review this exact head. |
@codex review Fresh substantive review requested for exact head |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='50926c0f48a078a94297a385d46dfd31520e25ac'
echo '== requested commit and checkout =='
git rev-parse --verify "${target}^{commit}"
git rev-parse HEAD
git show --no-ext-diff --format=fuller --no-patch "$target"
echo '== commit delta =='
git diff-tree --no-commit-id --name-status -r "$target"
git diff --no-ext-diff --check "${target}^" "$target"
git diff --no-ext-diff --unified=120 "${target}^" "$target"
echo '== candidate paths =='
fd -HI -t f . | grep -E '(^|/)(enable-auto-merge\.yaml|disarm-auto-merge\.sh|test-disarm-auto-merge\.sh|test-enable-auto-merge-author-gate\.sh|enable-auto-merge-authors\.json|ci\.yaml)$' || true
echo '== workflow outline =='
ast-grep outline .github/workflows/enable-auto-merge.yaml --items all 2>/dev/null || true
echo '== workflow decision path =='
nl -ba .github/workflows/enable-auto-merge.yaml | sed -n '1,360p'
echo '== affected test files =='
for file in \
.github/tests/test-disarm-auto-merge.sh \
.github/tests/test-enable-auto-merge-author-gate.sh \
.github/tests/enable-auto-merge-authors.json \
.github/tests/disarm-auto-merge-mock-bin/gh
do
if [ -f "$file" ]; then
echo "--- $file ---"
cat -n "$file"
fi
done
echo '== workflow-run identity/query references =='
rg -n -C 10 --hidden --glob '!.git/**' \
'(workflow_id|actions/runs|same.workflow|reauthor|stale|supersed|updated_at|pull_request)' \
.github .scripts README.md 2>/dev/null || trueLength of output: 59446 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='50926c0f48a078a94297a385d46dfd31520e25ac'
echo '== disarm workflow block =='
git show "${target}:.github/workflows/enable-auto-merge.yaml" |
sed -n '/^ disarm-untrusted-update:/,/^ auto-merge:/p'
echo '== selector and regression test =='
for path in \
.scripts/find-trusted-superseding-pr-run.sh \
.github/tests/test-find-trusted-superseding-pr-run.sh \
.github/tests/superseding-pr-runs.json
do
echo "--- ${path} ---"
git show "${target}:${path}"
done
echo '== all relevant fixture and mock assets =='
git ls-tree -r --name-only "$target" |
grep -E '(superseding|disarm-auto-merge.*mock|mock.*disarm-auto-merge|test-disarm-auto-merge)' |
while IFS= read -r path; do
echo "--- ${path} ---"
git show "${target}:${path}"
done
echo '== API error handling and fallback references =='
git grep -n -E -C 8 \
'(gh api|WORKFLOW_ID|run_pages|find-trusted-superseding|LATEST_ACTOR|fail.closed|fail-closed)' \
"$target" -- .github/workflows/enable-auto-merge.yaml .github/tests .scripts README.md || trueLength of output: 82666
Line 212 — the new Please handle metadata/list lookup failure as “no reauthorization proven”: emit a warning and continue to the disarm script. Keep the only skip path limited to a positively identified, newer trusted run of the same workflow and PR. Add a regression fixture or wrapper-level mock asserting that lookup failure invokes revocation rather than aborting first. |
Fixed the fail-closed lookup gap at exact head @coderabbitai review this exact head. |
@codex review Fresh substantive review requested for exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='578314f091b5c409b8c40243d222d38fa1c59b23'
echo '== requested commit and checkout =='
git rev-parse --verify "${target}^{commit}"
git rev-parse HEAD
git show --no-ext-diff --format=fuller --no-patch "$target"
echo '== commit delta and whitespace =='
git diff-tree --no-commit-id --name-status -r "$target"
git diff --no-ext-diff --check "${target}^" "$target"
git diff --no-ext-diff --unified=100 "${target}^" "$target"
echo '== relevant candidate files =='
fd -HI -t f . | grep -E '(^|/)(enable-auto-merge\.yaml|disarm-auto-merge\.sh|find-trusted-superseding-pr-run\.sh|handle.*stale.*\.sh|test-.*(disarm|superseding|stale).*\.(sh|json)|ci\.yaml)$' || true
echo '== workflow disarm section =='
sed -n '/^ disarm-untrusted-update:/,/^ auto-merge:/p' .github/workflows/enable-auto-merge.yaml
echo '== scripts that participate in stale-event/revocation decision =='
for path in \
.scripts/find-trusted-superseding-pr-run.sh \
.scripts/disarm-auto-merge.sh \
.scripts/handle-stale-auto-merge-rejection.sh \
.scripts/disarm-stale-auto-merge.sh
do
if [ -f "$path" ]; then
echo "--- $path ---"
cat -n "$path"
fi
done
echo '== focused test and fixture files =='
for path in \
.github/tests/test-find-trusted-superseding-pr-run.sh \
.github/tests/test-disarm-auto-merge.sh \
.github/tests/test-handle-stale-auto-merge-rejection.sh \
.github/tests/superseding-pr-runs.json \
.github/tests/stale-auto-merge-rejection.json
do
if [ -f "$path" ]; then
echo "--- $path ---"
cat -n "$path"
fi
done
echo '== references and call sites =='
rg -n -C 6 --hidden --glob '!.git/**' \
'(find-trusted-superseding-pr-run|disarm-auto-merge|WORKFLOW_ID|actions/runs|workflow_id|reauthor|supersed|stale)' \
.github .scripts README.md 2>/dev/null || trueLength of output: 150690
No new findings. The stale-event decision is now correctly fail-closed: live-state, workflow-identity, run-list, and selector failures all reach the real revocation script. The sole non-revocation path requires a positively selected later run with the same workflow ID and PR number from a trusted actor. The wrapper-level mocks and selector fixtures cover the important positive and negative ordering cases, while the isolated token-only disarm boundary remains intact. ✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 578314f091
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
@coderabbitai review Fresh substantive review requested for exact head |
@codex review Fresh substantive review requested for exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='20a7fb6b9f8b4f2967b087491e21ebffcbb58920'
echo '== requested commit and checkout =='
git rev-parse --verify "${target}^{commit}"
git rev-parse HEAD
git show --no-ext-diff --format=fuller --no-patch "$target"
echo '== commit delta and whitespace =='
git diff-tree --no-commit-id --name-status -r "$target"
git diff --no-ext-diff --check "${target}^" "$target"
git diff --no-ext-diff --unified=120 "${target}^" "$target"
echo '== relevant candidate paths =='
fd -HI -t f . | grep -E \
'(^|/)(enable-auto-merge\.yaml|find-trusted-superseding-pr-run\.sh|handle-stale-auto-merge-rejection\.sh|disarm-auto-merge\.sh|test-.*(superseding|stale|disarm).*\.(sh|json)|.*mock.*/gh)$' \
|| true
echo '== workflow stale-rejection call site =='
if [ -f .github/workflows/enable-auto-merge.yaml ]; then
nl -ba .github/workflows/enable-auto-merge.yaml |
sed -n '/^ disarm-untrusted-update:/,/^ auto-merge:/p'
fi
echo '== ordering scripts and static test/fixture assets =='
for path in \
.scripts/find-trusted-superseding-pr-run.sh \
.scripts/handle-stale-auto-merge-rejection.sh \
.scripts/disarm-auto-merge.sh \
.github/tests/test-find-trusted-superseding-pr-run.sh \
.github/tests/test-handle-stale-auto-merge-rejection.sh \
.github/tests/superseding-pr-runs.json \
.github/tests/stale-auto-merge-rejection.json \
.github/tests/disarm-auto-merge-mock-bin/gh
do
if [ -f "$path" ]; then
echo "--- $path ---"
cat -n "$path"
fi
done
echo '== all creation/update ordering references =='
rg -n -C 8 --hidden --glob '!.git/**' \
'(created_at|createdAt|updated_at|updatedAt|LIVE_PR|EVENT_UPDATED|candidate|supersed|reauthor|WORKFLOW_ID|workflow_id)' \
.github .scripts README.md 2>/dev/null || trueLength of output: 17478 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='20a7fb6b9f8b4f2967b087491e21ebffcbb58920'
echo '== workflow disarm job and wrapper call-site =='
git show "${target}:.github/workflows/enable-auto-merge.yaml" |
sed -n '/^ disarm-untrusted-update:/,/^ auto-merge:/p'
echo '== stale wrapper test, complete =='
git show "${target}:.github/tests/test-disarm-untrusted-event.sh"
echo '== gh mock, complete =='
git show "${target}:.github/tests/disarm-auto-merge-mock-bin/gh"
echo '== selector test and fixtures, complete =='
git show "${target}:.github/tests/test-find-trusted-superseding-pr-run.sh"
git show "${target}:.github/tests/superseding-pr-runs.json"
echo '== committed references and invocation arguments =='
git grep -n -E -C 6 \
'(disarm-untrusted-event\.sh|find-trusted-superseding-pr-run\.sh|live_updated_at|event_updated_at|created_at|created=")' \
"$target" -- .github .scripts README.md || true
echo '== shell static syntax validation (without executing repository scripts) =='
for path in \
.scripts/disarm-untrusted-event.sh \
.scripts/find-trusted-superseding-pr-run.sh \
.github/tests/test-disarm-untrusted-event.sh \
.github/tests/test-find-trusted-superseding-pr-run.sh \
.github/tests/disarm-auto-merge-mock-bin/gh
do
git show "${target}:${path}" | bash -n
echo "syntax OK: ${path}"
doneLength of output: 144649
No new findings. The selector now requires a same-workflow, same-PR, trusted candidate run to have been created no earlier than the observed live PR update, so a trusted run from before the newer update cannot suppress revocation. The wrapper retains fail-closed behavior for live-state, workflow-identity, run-list, and selector failures. The new negative ordering fixture covers the B-before-C case, and static Bash syntax validation passed for the modified scripts, tests, and mock. ✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20a7fb6b9f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
@coderabbitai review Please perform a fresh substantive review of exact head |
@codex review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
target='af9ce77a86ef6cfcb0fc714cfb43a0172b01568e'
echo '== requested commit and checked-out HEAD =='
git rev-parse --verify "${target}^{commit}"
git rev-parse HEAD
git show --no-ext-diff --format=fuller --no-patch "$target"
echo '== first-parent delta and whitespace check =='
git diff-tree --no-commit-id --name-status -r "$target"
git diff --no-ext-diff --check "${target}^" "$target"
git diff --no-ext-diff --unified=120 "${target}^" "$target"
echo '== candidate files =='
git ls-tree -r --name-only "$target" | grep -E \
'(^|/)(enable-auto-merge\.yaml|find-trusted-superseding-pr-run\.sh|disarm-untrusted-event\.sh|disarm-auto-merge\.sh|test-.*(superseding|disarm|stale).*\.(sh|json)|.*mock.*/gh)$' \
|| true
echo '== workflow stale-event decision call site =='
git show "${target}:.github/workflows/enable-auto-merge.yaml" | \
sed -n '/^ disarm-untrusted-update:/,/^ auto-merge:/p'
echo '== ordering and trust scripts =='
for path in \
.scripts/find-trusted-superseding-pr-run.sh \
.scripts/disarm-untrusted-event.sh \
.scripts/disarm-auto-merge.sh
do
if git cat-file -e "${target}:${path}" 2>/dev/null; then
echo "--- ${path} ---"
git show "${target}:${path}" | cat -n
fi
done
echo '== relevant tests, fixtures, and GitHub CLI mocks =='
git ls-tree -r --name-only "$target" | grep -E \
'(^\.github/tests/)(test-.*(superseding|disarm|stale).*\.(sh|json)|.*mock.*/gh|.*(superseding|stale).*\.(json|yaml))$' \
| while IFS= read -r path; do
echo "--- ${path} ---"
git show "${target}:${path}" | cat -n
done
echo '== scoped references to ordering and actor metadata =='
git grep -n -E -C 8 \
'(created_at|updated_at|run_started_at|triggering_actor|actor|rerun|run_attempt|workflow_id|workflow_run|supersed|reauthor|LIVE_PR)' \
"$target" -- .github .scripts README.md || trueLength of output: 202231 🏁 Script executed: #!/bin/bash
set -euo pipefail
target='af9ce77a86ef6cfcb0fc714cfb43a0172b01568e'
echo '== exact first-parent patch =='
git diff --no-ext-diff --unified=80 "${target}^" "$target" -- \
.github/workflows/enable-auto-merge.yaml \
.scripts/find-trusted-superseding-pr-run.sh \
.scripts/disarm-untrusted-event.sh \
.github/tests/superseding-pr-runs.json \
.github/tests/test-find-trusted-superseding-pr-run.sh \
.github/tests/enable-auto-merge-authors.json \
.github/tests/test-enable-auto-merge-author-gate.sh \
.github/tests/test-disarm-untrusted-event.sh \
.github/tests/disarm-auto-merge-mock-bin/gh \
README.md
echo '== selector: complete =='
git show "${target}:.scripts/find-trusted-superseding-pr-run.sh" | cat -n
echo '== selector fixture names and candidate identity/timestamps =='
git show "${target}:.github/tests/superseding-pr-runs.json" |
jq -r '.[] |
.name as $name |
[$name, .current_run_id, .workflow_id, .pr_number, .live_updated_at,
(.runs[]?.workflow_runs[]? |
"id=\(.id), wf=\(.workflow_id), created=\(.created_at), actor=\(.actor.login // ""), rerun=\(.triggering_actor.login // ""), prs=\([.pull_requests[]?.number] | join(","))")
] | `@tsv`'
echo '== actor-trust fixtures that distinguish original and rerun initiators =='
git show "${target}:.github/tests/enable-auto-merge-authors.json" |
jq -c '.[] | select(has("triggering_actor") or (.name | test("rerun|actor"; "i")))'
echo '== workflow classifier and disarm predicates, with source lines =='
git show "${target}:.github/workflows/enable-auto-merge.yaml" |
nl -ba | sed -n '80,220p'
echo '== mock API routing relevant to workflow run metadata and pagination =='
git show "${target}:.github/tests/disarm-auto-merge-mock-bin/gh" | cat -n
echo '== focused test registrations =='
git show "${target}:.github/workflows/ci.yaml" |
nl -ba | sed -n '2105,2130p'Length of output: 62365
No new findings. The selector now requires a candidate run to be created strictly after the observed live PR update, so equal-second ordering does not authorize a skip. It also requires both the original run actor and The focused fixtures cover same-second ambiguity, pre-update candidates, untrusted rerun initiators, untrusted original actors, unrelated workflows, and different PRs. The only non-revocation path remains a positively proven newer trusted run of the same workflow and PR. ✅ Action performedReview finished.
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af9ce77a86
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Why
An allowlisted automation PR can receive a later pull-request event from a different actor. When actor-trust enforcement is enabled, the privileged auto-merge path must reject every untrusted head, lifecycle trigger, or rerun initiator, revoke merge state left by an earlier trusted event, and preserve event order even when GitHub schedules jobs out of order.
What
enforce-actor-trustinput for reusable callers andENFORCE_ACTOR_TRUSTvariable support for direct/required-workflow runs.pull_requesttrigger on an allowlisted bot-authored PR to have both an allowlisted originalgithub.actorand currentgithub.triggering_actor; an untrusted collaborator cannot replay a trusted event.GITHUB_TOKENdisarm job with the exact write scopes needed to revoke both classic auto-merge and merge-queue entries; it never receives the App private key.updated_at. If live state changed, a stale rejection stands down only when the newest later run of the same caller workflow and PR was created strictly after the observed live update and has both original and rerun-triggering actors allowlisted. Same-second timestamps, unrelated workflows, earlier trusted runs, comments, reviews, and later untrusted events still revoke fail-closed.Rollout
Legacy trusted-author behavior remains unchanged by default. Consumers opt in only after granting the reusable caller
actions: read,contents: write, andpull-requests: write; missing ordering proof or revoke authority fails closed by revoking and/or leaving the required workflow red.Fixes
Closes the actor-trust, lifecycle re-arm, untrusted-rerun, stale-head, stale-lifecycle, scheduling-inversion, and disarm-ordering gaps; no separate issue is linked.