diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a56378315..b1263acbb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -9,8 +9,8 @@ on: # `edited` re-evaluates the base-sensitive checks (main-base, # promotion-gate, paths, cleanup-paths) when a PR is retargeted. For the # required contexts a stale `skipped` run would satisfy the requirement - # outright; main-base is not required, so there the stale run instead - # leaves a mis-route unsignalled. + # outright; cleanup-paths is review-time defense rather than required, so + # there the stale run instead misleads the reviewer. types: [opened, synchronize, reopened, edited] push: branches: [main] @@ -167,20 +167,18 @@ jobs: # the staging→main promotion or a reviewed non-feature lane — the collect # run branches, the maintainer's cleanup sweep, and the metrics-refresh / # cert-backtest / salience-replay PRs. This job runs (and fails) only on a - # PR to `main` from - # any other head; every legitimate PR skips it. It is not among `main`'s - # required contexts (`gate`, `paths`, `promotion-gate`) and cannot be yet: a - # `pull_request` runs the workflow from the merge ref, and the legitimate - # lanes are cut from `main`, whose own ci.yml has no `main-base` job — the - # context would never report and an auto-merging collect PR would hang - # pending forever. Requireable once this definition promotes into `main`; - # until then it goes red on a mis-route without blocking the merge. Fork - # heads never match the allowlist — outside contributions route through - # `staging` too. The - # emergency escape hatch is a repo admin editing the ruleset: deliberate - # and auditable. Expression `==`/`startsWith` compare case-insensitively, - # so a write-access branch named `Staging` skips the jail — a hygiene - # gap, not a hole: the PR still needs the human merge. + # PR to `main` from any other head; every legitimate PR skips it, and the + # skipped run satisfies the requirement. It is among `main`'s required + # contexts (with `gate`, `paths`, `promotion-gate`): a `pull_request` runs + # the workflow from the merge ref, the legitimate lanes are all cut from + # `main`, and `main`'s own ci.yml carries this job, so the context reports + # on every lane — and a mis-routed feature PR runs it, fails, and cannot + # merge. Fork heads never match the allowlist — outside contributions + # route through `staging` too. The emergency escape hatch is a repo admin + # editing the ruleset: deliberate and auditable. Expression + # `==`/`startsWith` compare case-insensitively, so a write-access branch + # named `Staging` skips the jail — a hygiene gap, not a hole: the PR still + # needs the human merge. if: >- github.event_name == 'pull_request' && github.base_ref == 'main' && diff --git a/SECURITY.md b/SECURITY.md index 5ef8d5ebe..78a69fb22 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -108,8 +108,8 @@ runbook, [docs/security.md](docs/security.md). — only a maintainer-installed App can apply a label that fires a workflow at all. - **Branch protection and the deployment boundary.** `main` requires a PR - passing `gate`, `paths`, and `promotion-gate`; the **data App** is the sole - bypass actor, so the deterministic `run-pull` writers push corpus facts + passing `gate`, `paths`, `promotion-gate`, and `main-base`; the **data App** + is the sole bypass actor, so the deterministic `run-pull` writers push corpus facts straight to `main` while everything agentic goes through that PR — enforced by identity, since the agent workflows authenticate as a separate, non-bypass **dev App**. Both rulesets require **zero** approving reviews, so diff --git a/docs/pipeline.md b/docs/pipeline.md index 41df995f6..ad3df7550 100644 --- a/docs/pipeline.md +++ b/docs/pipeline.md @@ -296,25 +296,23 @@ below describes the damage). The promotion gates target exactly those two. The mechanics: -- **Feature PRs target `staging`** (AGENTS.md), and the routing rests on that - convention plus the maintainer's merge: `main`'s required checks are exactly - `gate`, `paths`, and `promotion-gate`. The **`main-base`** job signals a - mis-route — it runs, and fails, only on a PR to `main` whose head is not - `staging` or a reviewed non-feature lane (the collect run branches, the - maintainer's cleanup sweep, the metrics-refresh, cert-backtest, and - salience-replay PRs) — - but it is **not** a required context, so it goes red without being able to - block the merge. Its definition lives in `main`'s own ci.yml, so the context - reports on every lane into `main`; making it required is a pending ruleset - change that goes through the *Adding a required status check* procedure - below (docs/security.md inventories this). +- **Feature PRs target `staging`** (AGENTS.md), and the routing is enforced: + `main`'s required checks are exactly `gate`, `paths`, `promotion-gate`, and + **`main-base`**. `main-base` is the merge-routing jail — it runs, and fails, + only on a PR to `main` whose head is not `staging` or a reviewed non-feature + lane (the collect run branches, the maintainer's cleanup sweep, the + metrics-refresh, cert-backtest, and salience-replay PRs); on those + legitimate lanes it reports `skipped`, which satisfies the requirement. Its + definition lives in `main`'s own ci.yml, so the context reports on every + lane into `main` (docs/security.md inventories this). Rulesets cannot constrain a PR's source branch, which is why the routing lives as a check at all, and why the check deters mistakes while the human merge is what catches sabotage: a PR that edits ci.yml runs the edited definition. Dependabot targets `staging` for the same reason. The `staging` ruleset itself requires a pull request plus the status checks that can report - on a staging-targeted PR — `gate` and `paths`; `promotion-gate` keys on a - base of `main` and is always `skipped` here — with the **repository admin + on a staging-targeted PR — `gate` and `paths`; `main`'s other two, + `promotion-gate` and `main-base`, key on a base of `main` and are always + `skipped` here — with the **repository admin role as its sole bypass actor** — a required-checks rule blocks direct pushes of commits that carry no passing check runs, so the admin role is the only identity that can land the sync @@ -380,12 +378,12 @@ One-time setup (maintainer): create the branch from main (`git push origin main:staging`); add the `staging` ruleset — require a pull request plus the checks that can report on a staging-targeted PR (`gate` and `paths`), **repository admin role as the only bypass actor** (docs/security.md -inventories it); and add `promotion-gate` to `main`'s required checks -alongside `gate` and `paths` — it reports `skipped`, which satisfies the -requirement, on every PR that is not the promotion. `main-base` stays -**unrequired** until the *Adding a required status check* procedure below -clears it, because requiring a context before that procedure confirms its -producing job strands every collect auto-merge PR. +inventories it); and add `promotion-gate` — and, once the *Adding a required +status check* procedure below clears it, `main-base` — to `main`'s required +checks alongside `gate` and `paths`. Each reports `skipped`, which satisfies +the requirement, on every PR it does not gate; requiring a context before its +producing job reaches `main` strands every collect auto-merge PR, which is +what the procedure's ordering prevents. The `staging` *deployment environment* the freshness runs deploy to (deployment branches @@ -399,7 +397,7 @@ failing loudly: a context nothing on `main` produces leaves every PR into `main` pending forever, and the auto-merging collect PRs hang first. ```bash -scripts/promotion-gate.sh contexts # e.g. main-base +scripts/promotion-gate.sh contexts # the context you want to require ``` It reads `main: require PR`'s live required contexts and `main`'s own workflow @@ -412,9 +410,9 @@ report an advisory fact. Run it with your own token. 1. Land the job on `staging` and let it promote to `main` in an ordinary batch. 2. `scripts/promotion-gate.sh contexts ` — proceed only on *ready to require*. -3. Confirm a real PR of the kind you are gating reports the context. For - `main-base` that means watching one collect PR, since those auto-merge and - are what a mistake strands. +3. Confirm a real PR of the kind you are gating reports the context. For a + context that must report on the bot lanes, that means watching one of their + PRs, since those auto-merge and are what a mistake strands. 4. Add the context to the ruleset. Re-run step 2 afterwards: it now checks the context you just added. 5. Update the surfaces that record the required set — the pinned list in diff --git a/docs/security.md b/docs/security.md index f62593ac2..9acf159fa 100644 --- a/docs/security.md +++ b/docs/security.md @@ -81,18 +81,17 @@ pre-registration record's commit ids. **absent** from this bypass list. Required approvals are `0` — the maintainer reviews at merge time by convention, not by rule; set to `1` if a second reviewer exists. - - Required checks are exactly `gate`, `paths`, and `promotion-gate` (which - reports `skipped` — satisfying the requirement — on every PR that is not - the staging→main promotion). **`main-base` is not among them.** It is the - merge-routing jail: it runs — and fails — only on a PR to `main` whose head + - Required checks are exactly `gate`, `paths`, `promotion-gate`, and + `main-base` (the latter two report `skipped` — satisfying the requirement — + on every PR they do not gate). `main-base` is the merge-routing jail: it + runs — and fails — only on a PR to `main` whose head is not a same-repo `staging` or reviewed non-feature lane, so a feature PR cannot ride around the promotion path by mistake. Rulesets cannot constrain a PR's source branch, which is why it is a check rather than a rule. Its job definition lives in `main`'s own `ci.yml`, so the context reports on - every lane into `main`; making it required is a pending ruleset change - that goes through the *Adding a required status check* procedure in - [pipeline.md](pipeline.md) — until it lands, routing rests on the - promotion convention and the maintainer's merge. `cleanup-paths` is + every lane into `main`; adding a context like it goes through the *Adding + a required status check* procedure in + [pipeline.md](pipeline.md). `cleanup-paths` is deliberately **not** in the required list — a cleanup PR is never auto-merged, so it is review-time defense-in-depth. **Not** `zizmor` — it is path-filtered @@ -130,10 +129,10 @@ pre-registration record's commit ids. misbehaving writer that holds the data App's bypass token. - **`staging: require PR`** — the pre-merge branch every feature PR targets requires a pull request plus the required checks that can report on a - staging-targeted PR: `gate` and `paths`. (`main`'s third, `promotion-gate`, - is structurally always-`skipped` here — it keys on a base of `main` — so - requiring it would add no signal. The same is true of the `main-base` job, - which is not a required context anywhere.) **Bypass: the repository + staging-targeted PR: `gate` and `paths`. (`main`'s other two, + `promotion-gate` and `main-base`, are structurally always-`skipped` here — + each keys on a base of `main` — so requiring them would add no + signal.) **Bypass: the repository admin role only**, the escape hatch for a main→staging sync when the ordinary PR path is unavailable; its content is by construction already-gated `main` history merged with already-gated `staging` history. @@ -436,9 +435,9 @@ job-env export disabled) and they appear only pre-agent — in the composite's launch step, whose env the background `corpus-serve` process inherits, and in the deterministic provisioning steps' step-scoped env. A guard step fails the job if any `AWS_*` credential is visible in the job env when the agent steps begin, and this also levels the -engines: the Gemini sanitizer could never allowlist a credential, so corpus -retrieval used to be an accident of harness — now every engine queries the -same credential-free surface. What replaces the old residual: the sidecar is +engines: the Gemini sanitizer could never allowlist a credential, so every +engine queries the same credential-free surface rather than whichever one its +harness happens to let credentials reach. What replaces the old residual: the sidecar is an **unauthenticated localhost HTTP surface**, so any process on the runner — including the injected agent itself, which is the *intended* client — can query the corpus and spend ranged-read egress through it. That is the same diff --git a/src/fedcourtsai/required_checks.py b/src/fedcourtsai/required_checks.py index e159cb920..c19c3b1c7 100644 --- a/src/fedcourtsai/required_checks.py +++ b/src/fedcourtsai/required_checks.py @@ -3,7 +3,8 @@ A required context is satisfied only by a check run that *reports* on the PR, and the workflow that would report it has to exist on the branch whose workflows the PR runs. PRs into ``main`` from the bot lanes — the collect run -branches, ``cleanup/*``, ``metrics/refresh``, ``metrics/cert-backtest`` — are +branches, ``cleanup/*``, ``metrics/refresh``, ``metrics/cert-backtest``, +``metrics/salience-replay`` — are cut **from** ``main``, so they run ``main``'s own workflow files. Requiring a context that no workflow on that branch produces leaves every such PR pending forever, and the auto-merging collect PRs are the ones that hang first: data diff --git a/tests/test_required_checks.py b/tests/test_required_checks.py index 94f02b0ed..67df8475e 100644 --- a/tests/test_required_checks.py +++ b/tests/test_required_checks.py @@ -190,7 +190,7 @@ def test_main_currently_requires_only_contexts_it_can_produce() -> None: required may lack a producer. A promotion that renamed or deleted one of these jobs would hang every PR into `main`, and this fails first.""" workflows = Path(".github") / "workflows" - assert unproduced_contexts(["gate", "paths", "promotion-gate"], workflows) == [] + assert unproduced_contexts(["gate", "paths", "promotion-gate", "main-base"], workflows) == [] def test_odd_shapes_degrade_rather_than_raise(tmp_path: Path) -> None: