Auto-finish: agent/codex/skip-unstarted-github-billing-checks-saf-2026-08-31-10-06 - #743
Conversation
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
2 finding(s) — 🟠 2 high
⛔ Merge gate: blocked — 2 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | src/pr.js:98 |
The billing-waiver detector trusts an annotation shape that pull-request-controlled workflow code can reproduce exactly. |
| 🟠 high | src/finish/review-gate.js:207-212 |
The UNSTABLE bypass accepts every skipped check merely because at least one unrelated check has a billing waiver. |
Provider codex · commit 3747838
- HIGH src/pr.js:98 The billing-waiver detector trusts an annotation shape that pull-request-controlled workflow code can reproduce exactly. A workflow can emit the same path, line, level, blank fields, and message, causing a genuine failed check to be waived and leaving security dependent on a preflight script that the same PR may modify; the waiver must rely on trusted GitHub job metadata that workflow output cannot forge. - HIGH src/finish/review-gate.js:212 The UNSTABLE bypass accepts every skipped check merely because at least one unrelated check has a billing waiver. Consequently, a skipped required security, deployment, or test check can be silently treated as a billing companion and the PR can pass the merge gate without that check being audited.
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | src/finish/review-gate.js:236 |
The no-verdict fallback treats every skipped check as a billing companion whenever any billing waiver exists. |
Provider codex · commit 2d75b5a
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | src/finish/review-gate.js:236-240 |
Waived checks can pass without corresponding audited waiver names when GitHub supplies no merge-state verdict. |
Provider codex · commit 22e836b
- HIGH src/finish/review-gate.js:240 Waived checks can pass without corresponding audited waiver names when GitHub supplies no merge-state verdict. In that path `allAccountedFor` trusts the numeric waived count alone, so a malformed or incomplete snapshot can return green without propagating `billingWaivedNames`, bypassing both CI and the mandatory local preflight.
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟡 1 medium
✅ Merge gate: pass — no blocking findings (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟡 medium | src/pr.js:313-315 |
Billing classification repeats two synchronous GitHub API requests for every failed check on every CI poll. |
Provider codex · commit c931b00
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | templates/scripts/agent-branch-finish.sh:181-183 |
The mandatory fallback executes a preflight script from the untrusted PR worktree. |
Provider codex · commit c931b00
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | src/pr.js:102 |
The annotations request incorrectly uses an Actions job ID as a check-run ID. |
Provider codex · commit d7003bb
- HIGH src/pr.js:102 The annotations request incorrectly uses an Actions job ID as a check-run ID. The `/actions/jobs/{job_id}` and `/check-runs/{check_run_id}/annotations` endpoints use different identifiers, so real GitHub requests will normally fail and billing-blocked checks will never be waived; the test masks this by assigning both fake endpoints the same ID.
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
2 finding(s) — 🟡 2 medium
✅ Merge gate: pass — no blocking findings (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟡 medium | src/pr.js:98 |
The billing-waiver check treats a missing or null runner_id as zero. |
| 🟡 medium | src/pr.js:318 |
Billing detection performs two synchronous GitHub API requests for every failed check on every polling snapshot. |
Provider codex · commit 394bcda
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟡 1 medium
✅ Merge gate: pass — no blocking findings (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟡 medium | src/pr.js:100 |
A malformed or missing check_run_url throws instead of failing closed as a non-waivable check. |
Provider codex · commit 394bcda
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
2 finding(s) — 🟠 1 high · 🟡 1 medium
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | src/pr.js:72 |
The billing waiver is not bound to the CheckRun being classified. |
| 🟡 medium | templates/scripts/agent-branch-finish.sh:160 |
Materializing the trusted preflight with git show breaks valid preflight paths stored as symbolic links. |
Provider codex · commit fa5db7b
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | src/pr.js:120 |
GitHub Actions job IDs are incorrectly required to equal check-run IDs. |
Provider codex · commit 830ab5c
- HIGH src/pr.js:120 GitHub Actions job IDs are incorrectly required to equal check-run IDs. These are distinct GitHub entities and generally have different IDs, so legitimate billing-blocked jobs will never be waived; the later check_run_url validation already safely associates the job with the expected check run.
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | templates/scripts/agent-branch-finish.sh:160-164 |
The mandatory preflight is copied to an unrelated temporary path before execution. |
Provider codex · commit 6ac5af3
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | templates/scripts/agent-branch-finish.sh:215 |
The mandatory “trusted” preflight still executes with the untrusted PR worktree as its current directory. |
Provider codex · commit 687003e
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | templates/scripts/agent-branch-finish.sh:229 |
The mandatory preflight runs with the trusted base archive as its working directory, so existing preflight scripts that test . will |
Provider codex · commit b0f6e59
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | src/finish/review-gate.js:242-244 |
The trusted merge-state branches bypass waiver-accounting validation. |
Provider codex · commit d3705eb
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | templates/scripts/agent-branch-finish.sh:170-172 |
The trusted preflight path can escape the extracted base archive through a symlink. |
Provider codex · commit df76842
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
2 finding(s) — 🟠 1 high · 🟡 1 medium
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | templates/scripts/agent-branch-finish.sh:170-171 |
The mandatory-preflight target-awareness check can be satisfied by a comment or unused string. |
| 🟡 medium | src/pr.js:370 |
Billing-waiver detection performs synchronous GitHub API requests once per failing check during every status poll. |
Provider codex · commit cc3d47c
- HIGH templates/scripts/agent-branch-finish.sh:171 The mandatory-preflight target-awareness check can be satisfied by a comment or unused string. `grep` proves only that the variable name occurs somewhere, so a script that still tests the archived base tree can pass while the billing-waived PR worktree remains untested, defeating the fail-closed guarantee.
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | templates/scripts/agent-branch-finish.sh:176-177 |
The mandatory-preflight target-awareness check is bypassable because it only searches for the environment variable's name. |
Provider codex · commit 5ff49f0
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
✅ No findings. Nothing worth an inline comment in this diff.
Provider codex · commit b2072c7
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | templates/scripts/agent-branch-finish.sh:161 |
Mandatory preflight trust is bypassed for absolute script paths. |
Provider codex · commit b2072c7
Constraint: Preserve optional absolute preflight support while forcing billing-waived runs through the trusted base archive.\nTested: bash -n templates/scripts/agent-branch-finish.sh; node --test test/finish-preflight-flag.test.js
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟠 1 high
⛔ Merge gate: blocked — 1 blocking finding(s) (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟠 high | templates/scripts/agent-branch-finish.sh:157-158 |
Mandatory preflight resolution accidentally accepts absolute configured paths. |
Provider codex · commit 3c114fa
- HIGH templates/scripts/agent-branch-finish.sh:158 Mandatory preflight resolution accidentally accepts absolute configured paths. Concatenating an absolute value such as `/scripts/agent-preflight.sh` with `$trusted_tree/` can resolve to the archived script, but `run_preflight` later classifies it as absolute and skips the capability check, trusted working directory, and target-worktree argument protocol.
NagyVikt
left a comment
NagyVikt
left a comment
There was a problem hiding this comment.
🛡️ GitGuardex code-assist
1 finding(s) — 🟡 1 medium
✅ Merge gate: pass — no blocking findings (blocks on high/critical).
| Severity | Location | Finding |
|---|---|---|
| 🟡 medium | templates/scripts/agent-branch-finish.sh:222 |
The trusted-tree check rejects valid preflight paths after realpath canonicalizes them. |
Provider codex · commit e545f5f
1 participant
Summary
Test plan