Skip to content

ci: close two auto-merge review gaps (codex on risk:standard, docs/legal carve-out) - #131

Merged
topcoder1 merged 1 commit into
mainfrom
claude/ci-gaps-codex-standard-legal-safepaths
Jul 27, 2026
Merged

ci: close two auto-merge review gaps (codex on risk:standard, docs/legal carve-out)#131
topcoder1 merged 1 commit into
mainfrom
claude/ci-gaps-codex-standard-legal-safepaths

Conversation

@topcoder1

Copy link
Copy Markdown
Owner

Two independent paths let changes reach main unreviewed.

  1. Codex never ran on risk:standard — simultaneously the fallback class
    for any unmatched path AND an auto-mergeable one. whois-api-llc/wxa_vpn#1270
    (+839 lines under src/**) passed CI clean; a manual codex run afterwards
    found four P1s including a forgeable-identity hole. #1273 (+786) same.

    Fixed with a SECOND job (review_standard), not a widened if:. The job
    id is load-bearing: the published check is " / Codex Review", and
    claude-author-automerge.yml bypasses its risk-tier gate on that check's
    SUCCESS — but a codex-gate.mjs skip still concludes success. Reusing job
    review would have let a SMALL auth/billing/migration change classified
    standard auto-merge with codex having read nothing. A distinct job id
    gives coverage with zero new bypass surface.

  2. docs/legal/** auto-merged via the docs safe-paths carve-out — wxa_vpn#1268,
    an AUP change asserting all bulk deliveries are watermarked (false for
    .mmdb and .parquet), merged in seconds unreviewed.

    Fixed in BOTH gates, because neither subsumes the other:

    • safe-paths-automerge.yml: built-in unsafe_overrides, checked before the
      safe globs. This workflow never consults the classifier, so a repo's
      risk-paths.yml sensitive: list cannot close it.
    • claude-author-automerge.yml: ^docs/legal/.* in the central risk regex.
      This is the DURABLE gate — it decides before arming, so unlike
      revocation it has no race. Unlike marketing trees, docs/legal/ is a
      fixed conventional path, so a central glob cannot over-block.

Both file scans now also match rename SOURCES (.previous_filename): the API
reports only the destination, so moving a legal doc out of docs/legal/ (or an
auth file out of src/auth/) escaped both gates.

Also hardens safe-paths' arm/revoke path, all found by codex review:

  • revoke an existing arm on override or truncated listing (GitHub preserves
    auto-merge across pushes, so recomputing all_safe=0 disarms nothing)
  • fail CLOSED on revoke: only a verified OFF succeeds, after 3 attempts
  • per-PR concurrency group + head-SHA guard before arming, so a stale run
    cannot arm a newer head it never classified
  • evaluate the override BEFORE the 3000-file truncation exit

KNOWN, NOT FIXED (documented inline): review_standard is ADVISORY — it is
in no required-checks ruleset, so a standard PR can merge before it finishes.
Making it blocking needs an always()-gated summary job promoted to each
caller's ruleset (a bare required context that cannot report bricks PRs on
"Expected" forever) — a 40+-repo operation and an operator decision. The same
skip-reads-as-success trust also still applies to sensitive on job review.

Codex pre-review: 10 rounds (protocol cap is 6 — exceeded deliberately, every
round through 9 surfaced a real defect; round 10's remainder is the documented
advisory-vs-durable-gate limitation above).

HUMAN_READABLE_SUMMARY: Codex now reviews ordinary risk:standard PRs, not
just risk:sensitive — in a separate job so a cost-skipped review can't be
mistaken for a passed one and bypass the manual-merge gate. And customer-facing
legal docs no longer sneak through the "docs are safe" auto-merge carve-out;
they're blocked in the central risk regex too, which is the gate that can't race.

Auto-merge rationale: MANUAL MERGE. Touches .github/workflows/** (risk:blocked)
and changes fleet-wide merge-gating semantics across 40+ consumer repos.

🤖 Generated with Claude Code

…gal carve-out)

Two independent paths let changes reach main unreviewed.

1. Codex never ran on risk:standard — simultaneously the fallback class
   for any unmatched path AND an auto-mergeable one. whois-api-llc/wxa_vpn#1270
   (+839 lines under src/**) passed CI clean; a manual codex run afterwards
   found four P1s including a forgeable-identity hole. #1273 (+786) same.

   Fixed with a SECOND job (`review_standard`), not a widened `if:`. The job
   id is load-bearing: the published check is "<job id> / Codex Review", and
   claude-author-automerge.yml bypasses its risk-tier gate on that check's
   SUCCESS — but a codex-gate.mjs skip still concludes success. Reusing job
   `review` would have let a SMALL auth/billing/migration change classified
   `standard` auto-merge with codex having read nothing. A distinct job id
   gives coverage with zero new bypass surface.

2. docs/legal/** auto-merged via the docs safe-paths carve-out — wxa_vpn#1268,
   an AUP change asserting all bulk deliveries are watermarked (false for
   .mmdb and .parquet), merged in seconds unreviewed.

   Fixed in BOTH gates, because neither subsumes the other:
   - safe-paths-automerge.yml: built-in unsafe_overrides, checked before the
     safe globs. This workflow never consults the classifier, so a repo's
     risk-paths.yml `sensitive:` list cannot close it.
   - claude-author-automerge.yml: ^docs/legal/.* in the central risk regex.
     This is the DURABLE gate — it decides before arming, so unlike
     revocation it has no race. Unlike marketing trees, docs/legal/ is a
     fixed conventional path, so a central glob cannot over-block.

Both file scans now also match rename SOURCES (.previous_filename): the API
reports only the destination, so moving a legal doc out of docs/legal/ (or an
auth file out of src/auth/) escaped both gates.

Also hardens safe-paths' arm/revoke path, all found by codex review:
- revoke an existing arm on override or truncated listing (GitHub preserves
  auto-merge across pushes, so recomputing all_safe=0 disarms nothing)
- fail CLOSED on revoke: only a verified OFF succeeds, after 3 attempts
- per-PR concurrency group + head-SHA guard before arming, so a stale run
  cannot arm a newer head it never classified
- evaluate the override BEFORE the 3000-file truncation exit

KNOWN, NOT FIXED (documented inline): `review_standard` is ADVISORY — it is
in no required-checks ruleset, so a standard PR can merge before it finishes.
Making it blocking needs an always()-gated summary job promoted to each
caller's ruleset (a bare required context that cannot report bricks PRs on
"Expected" forever) — a 40+-repo operation and an operator decision. The same
skip-reads-as-success trust also still applies to `sensitive` on job `review`.

Codex pre-review: 10 rounds (protocol cap is 6 — exceeded deliberately, every
round through 9 surfaced a real defect; round 10's remainder is the documented
advisory-vs-durable-gate limitation above).

HUMAN_READABLE_SUMMARY: Codex now reviews ordinary `risk:standard` PRs, not
just `risk:sensitive` — in a separate job so a cost-skipped review can't be
mistaken for a passed one and bypass the manual-merge gate. And customer-facing
legal docs no longer sneak through the "docs are safe" auto-merge carve-out;
they're blocked in the central risk regex too, which is the gate that can't race.

Auto-merge rationale: MANUAL MERGE. Touches .github/workflows/** (risk:blocked)
and changes fleet-wide merge-gating semantics across 40+ consumer repos.
@cursor

cursor Bot commented Jul 27, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@github-actions github-actions Bot added the risk:blocked Risk class: blocked label Jul 27, 2026
@github-actions

Copy link
Copy Markdown

Risk class: blocked — manual merge required.

This PR touches one of the blocked path categories from .github/risk-paths.yml (Dockerfiles, docker-compose, .github/workflows/**, **/.env*, **/secrets*, infra/, terraform/, k8s/, or the classifier config itself).

Auto-merge is refused by claude-author-automerge.yml. A maintainer should review the diff and click "Squash and merge" themselves.

(This is a policy notice, not a code-quality failure. The classify job itself does not fail — required CI checks remain authoritative for "is the code green.")

@github-actions

Copy link
Copy Markdown

Coverage Floor — mode: enforce

metric value
measured 100.0%
floor (current) 99.0%
target 100.0%
last bumped 2026-05-12

@claude

claude Bot commented Jul 27, 2026

Copy link
Copy Markdown

No issues found. Traced the safe-paths override/revoke/enable branches, the rename-source scans, and the distinct review_standard job id — all fail closed toward manual merge and match the pinned tests.

@topcoder1
topcoder1 merged commit f1e8c9f into main Jul 27, 2026
15 checks passed
@topcoder1
topcoder1 deleted the claude/ci-gaps-codex-standard-legal-safepaths branch July 27, 2026 18:08
topcoder1 added a commit to whois-api-llc/ProfessionalServices that referenced this pull request Jul 27, 2026
…ane (#22)

Adds a Codex review lane for `risk:standard` PRs, which previously got
**no** second-model review.

`standard` is simultaneously the fallback class for any path matching no
rule in `.github/risk-paths.yml` AND a class
`claude-author-automerge.yml` will auto-merge — so it was the largest
body of code reaching `main` unreviewed. Precedent:
whois-api-llc/wxa_vpn#1270 (+839 lines under `src/**`) and #1273 (+786)
were both `standard`, both passed CI clean, and a manual Codex run on
#1270 afterwards found four P1s including a forgeable-identity security
hole.

**The job id is a security boundary — `review_standard`, not a widened
`if:` on `review`.** The published check is `<job id> / Codex Review`,
and `claude-author-automerge.yml`'s `codex_check_name` (default `review
/ Codex Review`, verified not overridden in this repo) names the check
whose SUCCESS bypasses the risk-tier manual-merge gate. That bypass
trusts a *conclusion*, not a review: when `codex-gate.mjs` skips (small
diff, or docs/tests-only) the review steps skip but the job still
concludes `success`. Folding `standard` into `review` would let a SMALL
change to an auth/billing/migration path — classified `standard` because
it is not in this repo's `sensitive:` list, yet flagged `risky=1` by the
central regex — auto-merge with Codex having read nothing.

**Advisory, not a merge gate.** Not in any required-checks ruleset, so a
standard PR can merge before this finishes; findings land as follow-up
signal. Spend is bounded by `codex-gate.mjs` (small-diff and
docs/tests-only skips), so this is not "Codex on every PR".

Mechanical fleet rollout: 41 repos, identical block, generated and
validated by script (each patch asserts existing jobs are byte-identical
and exactly one job was added; all 41 actionlint-clean). Template
already merged as topcoder1/ci-workflows#131; reference implementation
whois-api-llc/wxa_vpn#1282.

HUMAN_READABLE_SUMMARY: Codex now also reviews ordinary PRs in this
repo, not just high-risk ones. It runs as its own job so a Codex run
that was skipped for being cheap can never be mistaken for one that
passed and let a risky change auto-merge.

Auto-merge rationale: MANUAL MERGE — touches `.github/workflows/**`
(risk:blocked).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:blocked Risk class: blocked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant