From 7af60289f840ca010fb7de440a4c9276e0553a94 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 15:18:22 +0000 Subject: [PATCH 1/2] docs: require the security scans gate 2 lists but never required Gate 2's surface table names six security contexts -- `Security Scan - python`, `Security Scan - javascript`, `bandit`, `python-safety`, `npm-audit`, `trivy`. The unconditional required list contained none of them, and the conditional list named only `Security Scan - javascript`. Five appeared in the table and then in neither list, so the literal reading of v2 is that no security scan is required at all. v1 listed `Security` among its required six. The rewrite that set out to make gate 2 precise therefore dropped every security scan out of the required set -- a weakening in the one direction you would not want, introduced by the fix rather than present before it. Same defect class as the one the previous commit repaired: a list that does not say what it means. All six were verified against the 29 check runs on this documentation-only pull request's head (`27b2ecf`), not inferred: every one reports `success`. So the rationale given for holding `Security Scan - javascript` back -- "only when the diff touches JS/TS" -- is empirically false here, and it moves to the unconditional list with the rest. Also records that `trivy` and `Trivy` are two different check-runs on the same head, `success` and `neutral` respectively. Requiring the capitalised one would pin a check that never passes -- precisely the failure mode gate 2's own confirmation rule exists to catch. `Generate and Upload Coverage` (absent from all 29 checks) and `E2E Pipeline Tests` (skipped) stay conditional; both were re-confirmed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012z9PgTC5wJcu8UdfkJWJLq --- MERGE_POLICY.md | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/MERGE_POLICY.md b/MERGE_POLICY.md index 561ea289a..7445810ea 100644 --- a/MERGE_POLICY.md +++ b/MERGE_POLICY.md @@ -82,7 +82,21 @@ The real contexts, as observed on live pull requests: Required for every pull request: `validate`, `guards`, `lint-python`, `lint-frontend`, `build`, `test`, `CodeQL`, `gitleaks (working tree)`, -`dependency-review`, `PR Governance`, `Canonical issue and evidence`. +`dependency-review`, `PR Governance`, `Canonical issue and evidence`, +`Security Scan - python`, `Security Scan - javascript`, `bandit`, +`python-safety`, `npm-audit`, `trivy`. + +All six security contexts were verified to report `success` on a +documentation-only pull request (#1408, head `27b2ecf`), so none of them can +strand a change permanently pending. They are required unconditionally: v1 +listed `Security` among its required six, and dropping every security scan out +of the required set would be a weakening introduced by the rewrite that set out +to make this gate precise. + +> **`trivy` is lowercase.** Two distinct check-runs exist on the same head — +> `trivy` reports `success`, `Trivy` reports `neutral`. Selecting the +> capitalised one requires a check that never passes. This is exactly the trap +> the confirmation rule below exists to catch. **Conditionally required — never require these unconditionally:** @@ -91,7 +105,6 @@ Required for every pull request: `validate`, `guards`, `lint-python`, pending*, which blocks harder than failing. Required only when the diff touches `src/**`, `apps/**`, or `tests/**`. - `E2E Pipeline Tests` — passing or repository-skipped. -- `Security Scan - javascript` — only when the diff touches JS/TS. Before adding any check to branch protection, confirm the exact string appears in GitHub's required-checks picker, and confirm it reports on a From 81a0785e3d3bc3eee573b7464d97e640386282f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 17:16:08 +0000 Subject: [PATCH 2/2] docs: say what requiring the security checks actually buys The previous revision implied that moving the six security contexts into gate 2's required list restores security enforcement. It does not, and saying so would have been a worse version of the defect it was fixing -- a gate that names something real but cannot fail. Four of the six cannot fail on findings, by construction, in security.yml: `npm-audit` carries `continue-on-error: true`; `python-safety` carries both `|| true` and `continue-on-error: true`; `bandit` carries `|| true`; and both Trivy invocations set `exit-code: '0'`. `Security Scan - python` and `Security Scan - javascript` are CodeQL matrix jobs that report analysis completion and upload alerts -- whether an alert blocks is a code-scanning setting, not a workflow decision. So the guarantee is narrower than the previous wording claimed: gate 2 requires these checks to run and complete, which catches a scan that crashed, timed out, or was silently dropped from the pipeline. That is worth requiring on its own, and it is not vulnerability enforcement. Records the gap under "What is deliberately not here" rather than quietly widening scope. Removing those escapes would fail pull requests against the existing backlog of findings the moment it lands, which is a decision about tolerance for known vulnerabilities and belongs in its own pull request. Also notes the six were re-confirmed on this PR's own head (`7af6028`), a second documentation-only commit, and that the `trivy` / `Trivy` split reproduced there. Raised by the CodeRabbit review on #1410; verified against security.yml and codeql-analysis.yml rather than taken on report. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012z9PgTC5wJcu8UdfkJWJLq --- MERGE_POLICY.md | 49 ++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 9 deletions(-) diff --git a/MERGE_POLICY.md b/MERGE_POLICY.md index 7445810ea..a37a5a287 100644 --- a/MERGE_POLICY.md +++ b/MERGE_POLICY.md @@ -86,17 +86,38 @@ Required for every pull request: `validate`, `guards`, `lint-python`, `Security Scan - python`, `Security Scan - javascript`, `bandit`, `python-safety`, `npm-audit`, `trivy`. -All six security contexts were verified to report `success` on a -documentation-only pull request (#1408, head `27b2ecf`), so none of them can -strand a change permanently pending. They are required unconditionally: v1 -listed `Security` among its required six, and dropping every security scan out -of the required set would be a weakening introduced by the rewrite that set out -to make this gate precise. +All six were verified to report `success` on a documentation-only pull request +(#1408 head `27b2ecf`, and again on #1410 head `7af6028`), so none of them can +strand a change permanently pending. v1 listed `Security` among its required +six, and dropping every security scan out of the required set would be a +weakening introduced by the rewrite that set out to make this gate precise. + +> **What requiring these does and does not buy you.** Four of the six cannot +> fail on findings, by construction, in `.github/workflows/security.yml`: +> +> | Job | Why it always reports `success` | +> | --- | --- | +> | `npm-audit` | `continue-on-error: true` | +> | `python-safety` | `\|\| true` **and** `continue-on-error: true` | +> | `bandit` | `\|\| true` | +> | `trivy` | `exit-code: '0'` on both invocations | +> +> `Security Scan - python` and `Security Scan - javascript` are CodeQL matrix +> jobs; they report that analysis completed and upload alerts to the Security +> tab. Whether an alert blocks a merge is a code-scanning check-failure setting, +> not something the workflow decides. +> +> So gate 2 requires these checks to **run and complete** — which catches a scan +> that crashed, timed out, or was silently dropped from the pipeline. It is +> **not** vulnerability enforcement, and this policy should not be read as +> claiming it is. Making findings actually block is a change to +> `security.yml`, with its own diff and its own blast radius; see *What is +> deliberately not here*. > **`trivy` is lowercase.** Two distinct check-runs exist on the same head — -> `trivy` reports `success`, `Trivy` reports `neutral`. Selecting the -> capitalised one requires a check that never passes. This is exactly the trap -> the confirmation rule below exists to catch. +> `trivy` reports `success`, `Trivy` reports `neutral`, confirmed on both heads +> above. Selecting the capitalised one requires a check that never passes — +> precisely the trap the confirmation rule below exists to catch. **Conditionally required — never require these unconditionally:** @@ -191,6 +212,16 @@ laundering itself as enforcement. - **No evidence ledger requirement.** v1 mandated a receipt comment on #898 for every gate evaluation. Nothing consumed those receipts. Git history and check runs are the ledger. +- **No change to what the security scans enforce.** Gate 2 requires the six + security checks to run and complete; four of them cannot fail on findings, as + documented there. Turning `npm-audit`, `python-safety`, `bandit`, and `trivy` + into blocking gates means removing their `continue-on-error` / `|| true` / + `exit-code: '0'` escapes in `security.yml`, which will fail pull requests on + the existing backlog of findings the moment it lands. That is a deliberate + decision about tolerance for known vulnerabilities, not a documentation fix, + so it is left to its own pull request. Requiring the checks first is still + worth doing: it makes a scan that crashes or quietly disappears from the + pipeline visible, which is the failure this policy can actually close. ## Review clock