diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7b750545..3af78f0a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -106,6 +106,14 @@ Only the first is a real local scan. Note that the synthetic template is **below floor** (`names=7, estate=13, site_prefixes=1`) by design โ€” passing locally with it does not mean you would pass CI's gate, only that nothing structural was found. +It also prints a `token source:` line naming **where those counts came from** โ€” the +`MEFOR_FORBIDDEN_TOKENS` path, that variable carrying the list inline (named, never printed), or +`scripts/security/scan-tokens.local.txt` โ€” and says `OVERRIDDEN` when the environment won over the +file the run just installed. That variable takes precedence, so without the line `-Synthetic` could +install the template and then truthfully report the *real* set as configured, which reads as a +contradiction. The scanner's exit code is propagated too: a refusal is reported as `VERIFY FAILED`, +not as `CONFIGURED`. + ## Finding something to work on Browse issues labeled **`good first issue`** (small, self-contained) and **`help wanted`**. For diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index b180a1eb..3b1f886d 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -3054,7 +3054,7 @@ The distinction matters because these two paths do not look like the case AV cov ## 1000. Prove each required merge context can fail: negative controls for the gates that block merge -> ๐Ÿ”ข **Filed 2026-08-03 โ€” not started.** Value **7/10** ยท Difficulty **3/10** ยท _quick win_. Thirteen contexts are the entire merge gate and not one of them is proven able to go red โ€” a gate nobody has watched fail is an assumption wearing a green tick, and the class has now fired at least four times in this repo with no CI signal; the build is a negative-control fixture per context plus a job that fails if a context has none, no new dependency and no change to what the gates check. +> โœ… **SHIPPED (test-only; no branch-protection change, no new workflow).** All 13 required contexts now carry a registered negative control - the count READ from `.github/required-contexts.txt` at run time, never from memory. `tests/negative_controls.toml` records per context what the control plants, the pytest nodes that must go red without it, the shapes it deliberately does NOT break, and the nodes for that half; `tests/_negative_controls.py` + `tests/test_negative_controls.py` reconcile it against the LIVE required set and fail when a context has no control, when a control names a non-required context, when a node id resolves to no test, when the asymmetry half is empty, or when a `ci` control names a command no step invokes. The reconciliation runs inside the already-required `test` legs rather than in a new workflow, so it blocks today and adds NO new required context; two rows in `tests/test_ci_docs_only_detector.py` pin that the required-contexts file and the registry classify as CODE, because pytest is gated on `code == 'true'` and a docs-only classification would skip the gate on exactly the PR shape it exists for. `tests/test_merge_gate_controls.py` supplies the controls the contexts lacked, including the item's motivating instance RUN AGAINST THE PRE-FIX GATE: backlog-hygiene's own shell, lifted from the workflow, exits 1 through the shipped three-dot diff and 0 through the two-dot form on the identical synthetic PR. Watched fail, each restored in the same run: deleting `cancelled` from ci.yml's roll-up reddened 2 of 4 and left 2 green; widening one `.gitleaks.toml` allowlist entry reddened 2 of 4; adding `-ll` to the real bandit invocation reddened 1 of 4. **Residual, stated rather than closed:** the scanner binaries' own detection (bandit, gitleaks, npm-audit, pip-audit's audit half) runs in their own CI jobs, not in the pytest legs - what the legs hold is the property those jobs lose silently. Whether to add an ADVISORY workflow re-running the registered controls is an owner decision, deliberately not taken. **Cluster:** Security / CI gates. **Priority:** P1. **Verdict:** build. **Severity:** medium. @@ -4924,7 +4924,7 @@ The second step's arithmetic is measured: `GetFullPath('.git', )` r ## 1068. A `-Force` claim release leaves no record of who released whose claim -> ๐Ÿ”ข **Filed 2026-08-06 โ€” not started.** Value **5/10** ยท Difficulty **2/10** ยท _quick win_. `scripts/coord/claim.ps1:164` gates a non-holder release behind `-not $Force`, so `-Force` lets any session release a claim it does not hold. That escape hatch is **necessary and must stay** โ€” a claim whose holder's worktree is gone would otherwise be permanently stuck. The defect is that the release is `Remove-Item -LiteralPath $file -Force` and **nothing else**: no log, no audit line, no record of who released whose claim, when, or why. The guardrails are on the *advice*, not the *action*. +> โœ… **Shipped 2026-08-10.** Every `claim.ps1 -Release` now appends one JSON line to `/mefor-coord/claims/.history` โ€” the key, the releasing worktree and branch, the prior holder, its branch and its note, when the claim was taken, and `force` true/false. **`-Force` is KEPT**: a claim whose holder worktree is gone would otherwise be stuck forever, and the alternative people reach for is hand-deleting the file, which leaves less evidence still. The item was auditability, not prevention. **And it is not hypothetical** โ€” on 2026-08-10 a coordinator force-released claim #344 after establishing on evidence that the holder worktree was gone and that the work its note guarded had in fact merged as PR #153, while the note still read *"UNPUSHED, NO PR, GitHub finds NOTHING"*. That release was **correct**, and it left no trace, which is exactly the gap. Nothing is deployed, so on a first deployment there would be nobody to mislead (ยง0) โ€” but the claim registry is a shared coordination artifact **today**, and a stale note in it has already blocked a lane from claiming an item. **The record is written BEFORE the claim file is removed, and the release is refused if it cannot be written.** Both orders can lie once and only one lie is recoverable: recording after the removal reproduces this defect exactly (a completed release with nothing left to write the record from), whereas recording first can at worst claim a release that then failed โ€” and a failed removal appends a `release-failed` correction rather than leaving the first line false. Refusing costs nothing because a release is always retryable; the claim simply stays where it was. JSON Lines, LF-only, one line per release written in a SINGLE write to a handle opened `FileMode::Append` + `FileShare::Read`, so two worktrees releasing in the same instant cannot interleave a record. Safe inside the claims directory because every reader there keys on a file NAME (`-List` and `prune-merged.ps1` glob `*.json`, `claim_check.py` opens `.json`) and `ConvertTo-KeyFile` always appends `.json`, so no key can fold onto `.history`. `tests/test_coord_claim_release_history.py` (10 tests) was **red first, 8 of 10** against the unchanged script; the two that passed pre-fix are the guards against a WRONG fix โ€” a refused release and an unclaimed key must record NOTHING. Per #1000 it carries the negatives that a bare "somebody released something" record would sail through: a `-Force` takeover must name the PRIOR holder and the releaser as DIFFERENT paths, and `force` must record **false** when the flag was not passed. Also asserts the on-disk shape (LF, no CR, newline-terminated, every line parseable) and that `-List` still reports "No active claims." with the ledger present. **Not covered, deliberately:** `prune-merged.ps1` clears a stranded claim with its own `Remove-Item` and writes no `.history` line โ€” reported rather than folded in, because its releases are proven-own-worktree-only and already surface in its `claimsReleased` receipt, so the ledger is a complete record of `claim.ps1` releases and not of every release. **Cluster:** Coordination tooling / claim integrity. **Priority:** P3. **Verdict:** build (add a record; do **not** remove `-Force`). **Severity:** moderate โ€” nothing is silently mis-authorised, but a coordination primitive can be overridden by the party it constrains with no trace, which makes it a convention rather than a control. @@ -5211,7 +5211,7 @@ Both readings reach the same operational conclusion, which is the whole point of ## 1080. `setup-leak-gate.ps1` reports the LOADED token set as if it were the one it just installed -> ๐Ÿ”ข **Filed 2026-08-06 โ€” not started.** Value **3/10** ยท Difficulty **1/10** ยท _quick win_. `-Synthetic` prints *"Installed the SYNTHETIC template"* and then, three lines later, *"CONFIGURED with the real token set."* Both are individually true when `MEFOR_FORBIDDEN_TOKENS` is set: the script installed the synthetic file, and the scanner loaded the real list from the environment, which wins over the file. Nothing in the output says the env source **overrode** what was just installed, so the two lines read as a contradiction or, worse, as confirmation that the synthetic install produced a real-token gate. +> โœ… **Shipped 2026-08-10.** `setup-leak-gate.ps1`'s verify step now prints a `token source:` line naming what the scanner **actually loaded** โ€” `MEFOR_FORBIDDEN_TOKENS -> `, that variable carrying the list inline, or `scripts/security/scan-tokens.local.txt` โ€” plus an `OVERRIDDEN` banner when an install ran and the environment won over the file just written. Reproduced before the fix on a throwaway checkout, in exactly the filed shape: `-Synthetic` with `MEFOR_FORBIDDEN_TOKENS` pointing at another list printed *"Installed the SYNTHETIC template"* and then *"CONFIGURED with the real token set"*, with nothing between them to reconcile the two. **An inline value is named but never PRINTED** โ€” a non-path value IS the token list, and echoing it would publish what the gate protects into whatever log the operator was capturing. That is the load-bearing negative in the test file, because the obvious implementation of "name the resolved source" prints the variable's value and passes everything else. An explicitly **EMPTY** `MEFOR_FORBIDDEN_TOKENS` is now named as the CAUSE of `NOT CONFIGURED`, because that state means "no source" and does **not** fall back to the file, so the ordinary advice (`-From ` / `-Synthetic`) sends the operator round the same loop forever. **A second reporting defect in the same file is closed in the same commit:** the scanner's exit code was discarded entirely, so a refusal came back out as `CONFIGURED` and exit 0 โ€” contradicting this script's own header promise to *"exit non-zero if the sections are empty"*. Measured with an impossible `MEFOR_MIN_DETECTORS` floor: scanner exit **2**, script exit **0**. It now exits with the scanner's code under `VERIFY FAILED`, and deliberately does **not** reprint the scanner's output, since a hit line can quote matched content. `scan_forbidden.py` was **not** modified. Precedence is defined by its `_resolve_token_text` and this script necessarily re-expresses it to name the source, so `tests/test_setup_leak_gate_reports_source.py` (8 tests, **7 red first**; the one green is the paired positive control) pins the copy against the scanner's own `loaded names=` counts **in the same run**, and asserts the two sources yield **different** counts โ€” a fixture that could not tell them apart would pass while measuring nothing. Its token fixtures are DERIVED from the shipped example rather than hand-written, so no numeric site prefix is written into tracked test source and every floor section stays non-empty. Its child environment is pinned explicitly (`MEFOR_*` and `PYTHONIOENCODING`/`PYTHONUTF8` removed, `python` resolved through a shim), and that pinning is proven in both directions: the suite passes under a hostile ambient shell whose values make the same invocation exit 1 when inherited. **Related:** #1063 (same script, anchoring rather than reporting) โ€” this closes the reporting half of that pair. **Cluster:** Developer tooling / reporting accuracy. **Priority:** P4. **Verdict:** build (trivial). **Severity:** low, and the direction is safe โ€” the gate really is loaded with the real set, so the operator is better protected than the message implies, not worse. The cost is that a reader cannot tell which source armed the gate, which is the one question the script's own docstring says the verify step exists to answer. @@ -7415,7 +7415,7 @@ filing. ## 1208. no guard asserts that a credential factory PARAMETER maps to a SETTING name the redactor covers -> ๐Ÿ”ข **Filed 2026-08-09 - not started. THREE MEASURED INSTANCES of one shape, not a hypothesis.** Value **7/10** ยท Difficulty **4/10**. A connector factory takes a credential parameter and emits it under a DIFFERENT setting name. Every redaction control operates on the SETTING name. Nothing asserts the two agree, so a rename silently moves a credential outside the control's domain. +> โœ… **SHIPPED (test-only; `messagefoundry/config/wiring.py` unchanged).** `tests/test_credential_parameter_mapping.py` follows the VALUE across the rename boundary: a unique sentinel is injected into ONE factory parameter at a time, the factory is called, and the destination setting is read off the emitted settings - so no rename has to be taught to it, which is the fourth name list this item forbids. The real redactor is then asked about whatever key the sentinel was found under. A parameter that reaches no setting, or that cannot be built at all, is a FAILURE with a declared reason rather than a silent pass, and every declared exemption is asserted REACHED so the tables cannot go stale. Domain re-derived rather than inherited: 23 spec-returning factories (asserted `>= 23`), 58 credential- and URL-bearing parameters probed, 3 followed through an `env()`-only refusal. Watched fail: pre-#1106 reddens exactly the 2 renamed `with_signing` parameters and leaves 56 green; pre-#1207 reddens exactly the 10 URL-bearing ones and leaves 48 green; shipped code reddens none. It reaches surfaces the outcome-level sibling cannot: de-classifying `intake_api_key`, `intake_api_key_next`, `credential_password`, `ws_password` and `client_key_password` reddens this file on all five and `test_connection_factory_redaction_domain.py` on none, because that file drops a connector's credential arguments when the connector refuses to be built with them. `proxy` -> `proxy_url` is now covered BY DESIGN rather than by luck: URL-bearing parameters are selected by a suffix rule whose own coverage is checked against the destinations the redactor's URL rule owns. **Cluster:** Security / secret disclosure - prevention. **Priority:** P2. **Verdict:** build. **Severity:** no live defect at filing - the three known instances are closed. This is the guard that diff --git a/docs/SESSION-DRIFT-CONTROLS.md b/docs/SESSION-DRIFT-CONTROLS.md index b49ed080..2b759586 100644 --- a/docs/SESSION-DRIFT-CONTROLS.md +++ b/docs/SESSION-DRIFT-CONTROLS.md @@ -103,6 +103,13 @@ Frequently forgotten in discussions of "the gate", but it is the same problem cl `-Release` then `-Take` โ€” which drops the claim in between and re-opens the race it exists to close. The note is what `announce-session.ps1` broadcasts to every joining session *in preference to the worktree name*, so a note that cannot be corrected is announced as current intent indefinitely. + Every `-Release` โ€” **`-Force` included** โ€” appends one JSON line to `claims/.history` naming the key, + the releasing worktree and branch, the prior holder, its branch and note, and whether `-Force` was + used. The flag stays: a claim whose holder's worktree is gone would otherwise be stuck, and the + alternative people reach for is hand-deleting the file, which leaves less evidence still. What + changed is that the override is no longer invisible. The record is written *before* the claim file + is removed, and a release that cannot be recorded is refused rather than performed silently + (BACKLOG #1068). - **[`scripts/hooks/claim_check.py`](../scripts/hooks/claim_check.py)** โ€” `commit-msg` gate: a commit whose *subject* declares `BACKLOG #N` with a code-touching diff must hold a claim on N **for this worktree**. Motivated by a recorded incident: three sessions independently fixed one npm advisory; two PRs were diff --git a/scripts/coord/claim.ps1 b/scripts/coord/claim.ps1 index 308a2172..59337734 100644 --- a/scripts/coord/claim.ps1 +++ b/scripts/coord/claim.ps1 @@ -29,6 +29,12 @@ original signal and it was actively misleading: a 21h claim whose holder had committed two minutes earlier was labelled STALE and recommended for release. + EVERY RELEASE IS RECORDED, `-Force` included, as one JSON line appended to + /mefor-coord/claims/.history: the key, the releasing worktree and branch, the + prior holder, its branch and note, when the claim was taken, and whether -Force was used. The + record is written BEFORE the claim file is removed and the release is refused if it cannot be + written -- a release nobody can trace is the outcome this will not produce (BACKLOG #1068). + .EXAMPLE pwsh -NoProfile -File scripts\coord\claim.ps1 -Take 105 -Note "corepoint xml importer" pwsh -NoProfile -File scripts\coord\claim.ps1 -List @@ -65,6 +71,24 @@ $common = (& git -C $repo rev-parse --path-format=absolute --git-common-dir).Tri $claims = Join-Path $common "mefor-coord/claims" New-Item -ItemType Directory -Force -Path $claims | Out-Null +# THE RELEASE LEDGER (BACKLOG #1068). A release used to be `Remove-Item` and nothing else, so a +# `-Force` takeover -- one session releasing a claim held by another -- left no record of who released +# whose claim, when, or why. `-Force` has a real job and stays: a claim whose holder's worktree is +# gone would otherwise be stuck forever, and hand-deleting the file leaves even less evidence. The +# answer is a RECORD, not a refusal. +# +# Measured 2026-08-10. A coordinator force-released a claim after establishing on evidence that the +# holder's worktree was gone and that the work its note guarded had already merged, while the note +# still read "UNPUSHED, NO PR, GitHub finds NOTHING". That release was CORRECT and it left nothing +# behind to find, which is the whole gap: the registry is a shared coordination artifact, and a stale +# note in it has already blocked a lane from claiming an item. +# +# JSON Lines, LF-terminated, one object per release. It sits INSIDE the claims directory, which is +# safe because every reader of that directory keys on a file NAME -- `-List` and prune-merged.ps1 glob +# `*.json`, scripts/hooks/claim_check.py opens `.json` -- and ConvertTo-KeyFile always appends +# ".json", so no key can ever fold onto this name. +$history = Join-Path $claims ".history" + # A key is free text but becomes a FILENAME, so fold it to a safe, case-insensitive form. The original is # kept inside the json so `-List` can show what the human actually typed. function ConvertTo-KeyFile([string]$Key) { @@ -90,6 +114,32 @@ function Get-Mine([string]$Path) { [pscustomobject]@{ Claim = $c; IsMine = ($held -ieq $me) } } +# ONE record, appended exclusively, retried. FileMode::Append + FileShare::Read excludes a second +# WRITER -- two worktrees can release in the same instant -- while leaving readers alone, and the whole +# line goes out in a SINGLE Write to a handle already positioned at end-of-file, so a concurrent +# release cannot interleave half a record into another's. +# +# LF, not the platform newline: this is a machine-readable ledger read from PowerShell, python and +# git-bash on the same clone, and a mixed-newline JSONL file is one of those things nothing complains +# about until a parser splits differently from the writer. +# +# The catch is deliberately UNTYPED, for the reason written out at the claim-refresh Move below: +# PowerShell wraps an exception thrown by a .NET METHOD in a MethodInvocationException, so a typed +# `catch [System.IO.IOException]` here would never match and the failure would escape to +# $ErrorActionPreference = "Stop" instead of being retried. +function Add-HistoryLine([string]$Line) { + $bytes = [System.Text.Encoding]::UTF8.GetBytes($Line + "`n") + foreach ($attempt in 1..5) { + try { + $fs = [System.IO.File]::Open($history, [System.IO.FileMode]::Append, [System.IO.FileAccess]::Write, [System.IO.FileShare]::Read) + try { $fs.Write($bytes, 0, $bytes.Length) } finally { $fs.Dispose() } + return $true + } + catch { Start-Sleep -Milliseconds (20 * $attempt) } + } + return $false +} + # ONE liveness rule, three call sites (BACKLOG #345 Half B). # # `-List` learned this first, and that was the wrong half to fix alone: -List is where you BROWSE, and @@ -165,6 +215,12 @@ function Show-List { Write-Host "" } +# Resolved for BOTH paths, not just -Take. A release record that names who released the claim is only +# half an answer without the branch they were standing on -- the same question -Take records. +$branch = & git -C $repo branch --show-current +if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git -C $repo rev-parse --short HEAD) } +$branch = $branch.Trim() + if ($Release) { $file = Join-Path $claims ((ConvertTo-KeyFile $Release) + ".json") if (-not (Test-Path $file)) { Write-Host "No claim on '$Release' -- nothing to release."; exit 0 } @@ -199,8 +255,58 @@ if ($Release) { } exit 1 } - Remove-Item -LiteralPath $file -Force + # RECORD FIRST, then act. Both orders can lie once and only one lie is recoverable: removing first + # and recording after reproduces this item exactly (a completed release with no trace, and nothing + # left to write the record from), whereas recording first can at worst claim a release that then + # failed -- which the catch below corrects in the same ledger. Refusing when the record cannot be + # written is safe because a release is always retryable: the claim stays where it was. + # + # ConvertTo-Stamp on every field carried over from the claim file, not just the timestamp: + # ConvertFrom-Json date-coerces ANY ISO-8601-shaped string, and note/branch/worktree are free text. + $record = [ordered]@{ + ts = (Get-Date).ToString("o") + event = "release" + key = $Release + released_by = $repo + released_branch = $branch + prior_holder = ConvertTo-Stamp $info.Claim.worktree + prior_branch = ConvertTo-Stamp $info.Claim.branch + # The note is what a later reader judges the release BY -- it is the field that was stale and + # wrong in the incident above -- so the record keeps it rather than pointing at a deleted file. + prior_note = ConvertTo-Stamp $info.Claim.note + claimed = ConvertTo-Stamp $info.Claim.claimed + # The flag AS PASSED. Whether this was a takeover is already readable from prior_holder against + # released_by, and a record should not restate a fact it already carries. + force = [bool]$Force + } | ConvertTo-Json -Compress + if (-not (Add-HistoryLine $record)) { + Write-Host "" + Write-Host "REFUSING to release '$Release': the release record could not be written." -ForegroundColor Yellow + Write-Host " ledger : $history" + Write-Host " Your claim is UNCHANGED and still yours. Retry in a moment -- an untraceable" + Write-Host " release is the one outcome this refuses to produce." + exit 1 + } + try { + Remove-Item -LiteralPath $file -Force + } + catch { + # The line above says a release happened; it did not. Correct it in the same ledger rather than + # leave a record that is now false. + Add-HistoryLine ([ordered]@{ + ts = (Get-Date).ToString("o") + event = "release-failed" + key = $Release + released_by = $repo + reason = $_.Exception.Message + } | ConvertTo-Json -Compress) | Out-Null + throw + } Write-Host "Released claim on '$Release'." -ForegroundColor Green + if (-not $info.IsMine) { + Write-Host " TOOK OVER a claim held by $($info.Claim.worktree) [$($info.Claim.branch)]." -ForegroundColor Yellow + } + Write-Host " recorded in $history" exit 0 } @@ -210,10 +316,6 @@ if ($List) { Show-List; exit 0 } $safe = ConvertTo-KeyFile $Take $file = Join-Path $claims "$safe.json" -$branch = & git -C $repo branch --show-current -if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git -C $repo rev-parse --short HEAD) } -$branch = $branch.Trim() - try { # ATOMIC test-and-set -- identical to alloc.ps1. 'CreateNew' + FileShare::None throws IOException if a # sibling session got here first, and that throw IS the mutual exclusion. diff --git a/scripts/dev/setup-leak-gate.ps1 b/scripts/dev/setup-leak-gate.ps1 index 9747f843..879624cd 100644 --- a/scripts/dev/setup-leak-gate.ps1 +++ b/scripts/dev/setup-leak-gate.ps1 @@ -22,6 +22,15 @@ never ran, or ran with nothing loaded. So this script always finishes by invoking the scanner and printing the per-section detector counts, and exits non-zero if the sections are empty. + IT ALSO NAMES THE SOURCE THOSE COUNTS CAME FROM, and says so when the environment overrode the + file just written (BACKLOG #1080). MEFOR_FORBIDDEN_TOKENS wins over + scripts/security/scan-tokens.local.txt, so -Synthetic could install the template and then report + the REAL set as configured -- two true lines that together read as a contradiction. Counts without + a provenance leave exactly the ambiguity this step exists to close. Two consequences worth knowing: + an inline (non-path) MEFOR_FORBIDDEN_TOKENS value is named but never PRINTED, because that value + is the token list; and the scanner's exit code is now propagated, so a refusal is reported as + VERIFY FAILED instead of coming back out as CONFIGURED. + .EXAMPLE pwsh -NoProfile -File scripts/dev/setup-leak-gate.ps1 -From C:\path\to\tokens.txt .EXAMPLE @@ -83,27 +92,93 @@ if (Test-Path -LiteralPath $local) { } } +# --- which source will the scanner ACTUALLY load from? --------------------------------------------- +# The verify step below reports the LOADED token set. Reporting only that is what let -Synthetic print +# "Installed the SYNTHETIC template" and then, three lines later, "CONFIGURED with the real token set" +# (BACKLOG #1080). Both were true -- the file was installed, and MEFOR_FORBIDDEN_TOKENS won over it -- +# but nothing said the environment had OVERRIDDEN what had just been written, so the pair read as a +# contradiction or, worse, as confirmation that a synthetic install had produced a real-token gate. +# +# PRECEDENCE IS DEFINED BY scan_forbidden._resolve_token_text, NOT HERE. This is a second expression of +# it and would drift silently, so tests/test_setup_leak_gate_reports_source.py measures every branch +# below against the scanner's own detector counts in the SAME run rather than trusting either alone. +$envTokens = [Environment]::GetEnvironmentVariable('MEFOR_FORBIDDEN_TOKENS') +$envEmpty = $false +if ($null -ne $envTokens) { + $trimmed = $envTokens.Trim() + if (-not $trimmed) { + # An explicitly-EMPTY value means "no source" and does NOT fall back to the file. Worth naming, + # because in this one state installing a token list changes nothing and the ordinary advice + # ("-From " / "-Synthetic") sends the operator round the same loop indefinitely. + $envEmpty = $true + $source = 'NONE -- MEFOR_FORBIDDEN_TOKENS is set but EMPTY, which the scanner reads as "no source" (it does NOT fall back to the file)' + } + else { + $isFile = $false + try { $isFile = Test-Path -LiteralPath $trimmed -PathType Leaf } catch { $isFile = $false } + if ($isFile) { $source = "MEFOR_FORBIDDEN_TOKENS -> $trimmed" } + # NEVER echo the value. A non-path value IS the token list, carried inline; naming the variable + # is the whole diagnostic, and printing its content would publish exactly what this protects + # into whatever log the operator happened to be capturing. + else { $source = 'MEFOR_FORBIDDEN_TOKENS (inline token content -- value deliberately not printed)' } + } + $fromFile = $false +} +elseif (Test-Path -LiteralPath $local) { + $source = 'scripts/security/scan-tokens.local.txt' + $fromFile = $true +} +else { + $source = 'NONE -- no MEFOR_FORBIDDEN_TOKENS, and scripts/security/scan-tokens.local.txt is absent' + $fromFile = $false +} + # --- verify: what can the gate actually SEE? ------------------------------------------------------- $py = if (Test-Path (Join-Path $repo '.venv/Scripts/python.exe')) { Join-Path $repo '.venv/Scripts/python.exe' } else { 'python' } Write-Host '' Write-Host 'Verifying the gate can see:' -ForegroundColor Cyan $out = & $py $scanner --require-tokens 2>&1 +# Captured on the very next line, before any other native call can overwrite it. The scanner's verdict +# used to be discarded entirely, so a refusal came back out of here as CONFIGURED and exit 0 -- which +# contradicts this file's own header promise to "exit non-zero if the sections are empty". +$scanExit = $LASTEXITCODE $loaded = $out | Where-Object { $_ -match 'loaded names=' } | Select-Object -First 1 if (-not $loaded) { Write-Host ($out | Select-Object -Last 5); throw 'Scanner produced no detector-count line.' } Write-Host " $loaded" +Write-Host " token source: $source" +if ($PSCmdlet.ParameterSetName -ne 'Status' -and -not $fromFile) { + Write-Host ' OVERRIDDEN: the file this run just installed is NOT what the gate loaded -- MEFOR_FORBIDDEN_TOKENS takes precedence over scripts/security/scan-tokens.local.txt.' -ForegroundColor Yellow + Write-Host ' Unset that variable to use the file you just installed.' -ForegroundColor Yellow +} if ($loaded -match 'STRUCTURAL-ONLY') { Write-Host '' - Write-Host 'NOT CONFIGURED โ€” the pre-commit hook will fail closed on every commit.' -ForegroundColor Red - Write-Host ' Maintainers: -From Contributors: -Synthetic' + Write-Host 'NOT CONFIGURED -- the pre-commit hook will fail closed on every commit.' -ForegroundColor Red + if ($envEmpty) { + Write-Host ' CAUSE: MEFOR_FORBIDDEN_TOKENS is set to an EMPTY value, which the scanner reads as "no source".' -ForegroundColor Red + Write-Host ' Installing a token list will NOT fix this -- unset that variable first.' -ForegroundColor Red + } + else { + Write-Host ' Maintainers: -From Contributors: -Synthetic' + } exit 1 } +if ($scanExit -ne 0) { + Write-Host '' + Write-Host "VERIFY FAILED -- a token source is loaded but the scanner EXITED $scanExit." -ForegroundColor Red + Write-Host " token source: $source" + # Its output is NOT reprinted here: a hit line can quote matched content, and copying that into a + # terminal scrollback, a ticket or a CI log is the disclosure this whole gate exists to prevent. + Write-Host ' Re-run the scanner yourself to see why, and do not paste its output anywhere:' -ForegroundColor Red + Write-Host " $py scripts/security/scan_forbidden.py --require-tokens" + exit $scanExit +} if ($loaded -match 'SYNTHETIC') { Write-Host '' - Write-Host 'CONFIGURED (synthetic). Real customer tokens are NOT detected locally.' -ForegroundColor Yellow + Write-Host "CONFIGURED (synthetic), loaded from: $source. Real customer tokens are NOT detected locally." -ForegroundColor Yellow Write-Host ' It also flags the fictional customer/partner names this project uses in its own docs, so a hit here is not by itself a leak. CI is authoritative.' -ForegroundColor Yellow exit 0 } Write-Host '' -Write-Host 'CONFIGURED with the real token set.' -ForegroundColor Green +Write-Host "CONFIGURED with the real token set, loaded from: $source." -ForegroundColor Green exit 0 diff --git a/tests/_negative_controls.py b/tests/_negative_controls.py new file mode 100644 index 00000000..5a94df74 --- /dev/null +++ b/tests/_negative_controls.py @@ -0,0 +1,172 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Load and validate the negative-control registry (BACKLOG #1000). + +Shared rather than inlined for the same reason ``_workflow_contexts`` is: this reconciliation is run +from pytest (so it blocks merge through the existing ``test`` contexts, adding no new required context) +and can be run standalone against a checkout. Two copies of the rule set would be free to disagree, +which is the class the registry exists to catch. + +WHAT IT VALIDATES, and each rule is a decay mode that has actually happened somewhere in this repo: + +* **Every required context has a control.** Otherwise a context added to branch protection tomorrow + starts life unproven and nothing says so -- the decay mode that makes any one-off audit worthless. +* **Every registered context is required.** A control for a context nobody requires is effort spent + guarding nothing, and it inflates the coverage count. +* **Node ids resolve.** A control naming a test that does not exist is a control that does not exist. +* **`red` and `green` are both non-empty.** The asymmetry is mandatory: a control that only reddens + cannot tell you which layer does the work. +* **`ci` commands are wired into the job they claim.** A fixture asserter nobody invokes is a file. + +WHAT THE NODE-ID CHECK DOES AND DOES NOT PROVE, stated because the instrument has to answer the +question asked of it. It resolves each id by parsing the named file and looking for a test function of +that name, so it proves the test EXISTS and is spelled right. It does not prove the test is collected +or that it passes -- that is what running the suite does, and every registered id lives under ``tests/`` +precisely so the required ``test`` legs execute it. +""" + +from __future__ import annotations + +import ast +import tomllib +from dataclasses import dataclass +from pathlib import Path + +from tests._workflow_contexts import ROOT, jobs_of, required_contexts + +REGISTRY = Path(__file__).resolve().parent / "negative_controls.toml" + +#: Prose fields that must carry a real explanation. Short enough to admit a terse one, long enough that +#: "n/a" and "TODO" do not pass -- the two ways a mandatory field becomes decoration. +_MIN_PROSE = 40 + + +@dataclass(frozen=True) +class Control: + context: str + plants: str + holds: str + observed: str + red: tuple[str, ...] + green: tuple[str, ...] + ci: str | None + workflow: str | None + + +def load() -> list[Control]: + raw = tomllib.loads(REGISTRY.read_text(encoding="utf-8")) + controls = raw.get("control", []) + return [ + Control( + context=str(c.get("context", "")), + plants=str(c.get("plants", "")).strip(), + holds=str(c.get("holds", "")).strip(), + observed=str(c.get("observed", "")).strip(), + red=tuple(str(n) for n in c.get("red", [])), + green=tuple(str(n) for n in c.get("green", [])), + ci=str(c["ci"]) if "ci" in c else None, + workflow=str(c["workflow"]) if "workflow" in c else None, + ) + for c in controls + ] + + +def _test_names(path: Path) -> set[str]: + tree = ast.parse(path.read_text(encoding="utf-8")) + return { + node.name + for node in ast.walk(tree) + if isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef) + and node.name.startswith("test_") + } + + +def unresolved_nodes(controls: list[Control]) -> list[str]: + """Node ids that name no test function. A dangling id is a control that does not exist.""" + cache: dict[str, set[str]] = {} + missing: list[str] = [] + for control in controls: + for node in control.red + control.green: + rel, _, name = node.partition("::") + if not name: + missing.append(f"{node} (not a `file::test_name` node id)") + continue + path = ROOT / rel + if not path.is_file(): + missing.append(f"{node} (no such file)") + continue + if rel not in cache: + cache[rel] = _test_names(path) + if name not in cache[rel]: + missing.append(f"{node} (no test function of that name in {rel})") + return missing + + +def unwired_ci_commands(controls: list[Control]) -> list[str]: + """`ci` commands that do not appear in any step of the workflow they claim. + + A fixture asserter that nobody invokes is a file, not a control -- and it would still satisfy every + other rule here. + """ + unwired: list[str] = [] + for control in controls: + if control.ci is None: + continue + if control.workflow is None: + unwired.append(f"{control.ci} declares no `workflow`") + continue + body = "\n".join( + str(step.get("run", "")) + for job in jobs_of(control.workflow).values() + for step in job.get("steps", []) + ) + if control.ci not in body: + unwired.append(f"{control.ci!r} appears in no `run:` step of {control.workflow}") + return unwired + + +def reconcile() -> tuple[list[str], dict[str, int]]: + """Return (problems, {context: control count}) over the live required set.""" + controls = load() + required = required_contexts() + coverage = dict.fromkeys(required, 0) + problems: list[str] = [] + + for control in controls: + if control.context not in coverage: + problems.append( + f"control for {control.context!r} names a context that is NOT in " + ".github/required-contexts.txt -- either the context was removed from branch " + "protection or the string is a typo" + ) + continue + coverage[control.context] += 1 + for field, value in ( + ("plants", control.plants), + ("holds", control.holds), + ("observed", control.observed), + ): + if len(value) < _MIN_PROSE: + problems.append( + f"{control.context!r}: `{field}` is empty or a placeholder. A control without a " + "stated violation, a stated asymmetry and a stated observation is a claim." + ) + if not control.red: + problems.append(f"{control.context!r}: no `red` nodes -- nothing asserts it can fail") + if not control.green: + problems.append( + f"{control.context!r}: no `green` nodes. The asymmetry is mandatory: a control that " + "reddens uniformly cannot tell you which layer does the work, and cannot distinguish " + "'the other cases are safe by design' from 'safe by luck'." + ) + + for ctx, count in coverage.items(): + if count == 0: + problems.append( + f"required context {ctx!r} has NO negative control. It blocks merge and nobody has " + "watched it fail. Register one in tests/negative_controls.toml." + ) + + problems.extend(f"dangling control node: {m}" for m in unresolved_nodes(controls)) + problems.extend(f"unwired ci control: {m}" for m in unwired_ci_commands(controls)) + return problems, coverage diff --git a/tests/negative_controls.toml b/tests/negative_controls.toml new file mode 100644 index 00000000..902fb930 --- /dev/null +++ b/tests/negative_controls.toml @@ -0,0 +1,396 @@ +# NEGATIVE CONTROLS FOR THE REQUIRED MERGE CONTEXTS (BACKLOG #1000) +# +# WHY THIS FILE EXISTS. `.github/required-contexts.txt` names the contexts that block merge. It answers +# "is this check blocking?". It cannot answer the question underneath it -- "has anyone ever seen this +# check go red?" -- and a gate nobody has watched fail is an assumption wearing a green tick. The class +# has fired at least four times in this repository, each found by hand and none by CI: a required +# backlog gate computing a two-dot diff and crediting every PR with an older base; a required SAST gate +# scanning a two-directory allow-list while its sibling declared 56 more files in scope; a leak gate +# exiting 0 on content carrying a real site code; the same gate's home-path detector matching one of +# four spellings of the same Windows path. +# +# WHAT A CONTROL IS HERE. Not a re-test of what the gate checks -- the gates' own suites do that. One +# property per context: THIS GATE IS CAPABLE OF GOING RED, evidenced by a planted violation and an +# observed refusal. +# +# EVERY ENTRY IS ASYMMETRIC, and that field is mandatory. It is not enough that neutering a rule turns +# a control red: the control must fail for exactly the shapes that rule covers and KEEP PASSING for the +# shapes some other layer catches, or it cannot tell you which layer does the work. Measured 2026-08-05 +# on a different guard -- a fix believed to cover two NTFS alternate-data-stream spellings turned out to +# be load-bearing for exactly one, and only an eight-case control that reddened on one of them said so. +# A uniform red looks stronger and teaches nothing. So `holds` and `green` are as required as `plants` +# and `red`, and `tests/test_negative_controls.py` fails when either is missing. +# +# FORMAT. One `[[control]]` per context (a context may have several). +# context the exact string from .github/required-contexts.txt -- checked against it, both ways +# plants the violation the control introduces +# red pytest node ids that MUST fail if the gate stops catching that violation +# holds what the control deliberately does NOT break, and which layer covers it +# green pytest node ids for that half -- the shapes that must keep passing +# observed what was actually seen, and where. Prose only; nothing parses it. +# ci (optional) a command wired into the gate's OWN job, asserted present in that job +# workflow (with `ci`) the workflow file the command must appear in +# +# THIS FILE IS NOT BRANCH PROTECTION AND DOES NOT TOUCH IT. `.github/required-contexts.txt` is +# READ-ONLY here: it mirrors the live server, and a context added to it before the server is exactly +# the lie it exists to prevent. This file only reads it. +# +# WHAT THE SCANNER GATES CAN AND CANNOT BE CONTROLLED FOR, said once rather than repeated per entry. +# bandit, gitleaks, npm-audit, pip-audit and semgrep run third-party binaries the pytest legs do not +# install. Their DETECTION is exercised where the binary lives -- semgrep against annotated fixtures in +# its own CI job, pip-audit's slopsquat half by a stdlib script with its own unit suite. What the pytest +# legs can hold is the property those jobs lose SILENTLY: an enforcement flag removed, a severity floor +# added, an allowlist widened until it swallows the class, a scan re-narrowed to an allow-list. Those +# are the controls registered below, and the distinction is recorded rather than blurred. + +# --- ci.yml ---------------------------------------------------------------------------------------- + +[[control]] +context = "CI gate" +plants = """ +One gated leg reports `failure`, then `cancelled`, in a synthetic needs-results vector read through the +roll-up's own `if:` condition. `CI gate` is required BECAUSE the six legs behind it cannot be -- a +path-gated job does not report on a PR that touches none of its paths -- so this roll-up is the only +thing that turns a red sqlserver-store, postgres-store, load-test, load-test-sqlserver or +windows-service-smoke into a blocked merge. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_ci_gate_rollup_fires_on_a_failed_or_cancelled_leg", + "tests/test_merge_gate_controls.py::test_the_ci_gate_rollup_still_covers_every_leg_it_is_required_for", +] +holds = """ +An all-`skipped` vector must stay GREEN, and that is the case that actually happens: almost every PR +touches none of the six gated paths. A control that reddened on `skipped` would block every ordinary PR +while looking stronger. The reader itself is also controlled -- a condition with `cancelled` removed +must be reported as no longer firing on it, otherwise "the shipped condition is fine" and "the reader +matches nothing" are the same green. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_ci_gate_rollup_stays_green_when_every_gated_leg_skipped", + "tests/test_merge_gate_controls.py::test_the_rollup_reader_reports_a_condition_that_dropped_cancelled", +] +observed = """ +Live record, quoted in .github/required-contexts.txt: 18 success / 2 failure over 20 runs. One failure +was a nightly `schedule` run whose sqlserver load-test legs failed; one was a superseded re-push where +every job was CANCELLED -- both terminal states this control asserts are still named. Restored to +branch protection 2026-07-30 after checking exactly that record. + +RUN AGAINST A NEUTERED GATE, 2026-08-10: with `contains(needs.*.result, 'cancelled')` deleted from +ci.yml, 2 of the 4 roll-up controls went red and 2 stayed green -- the specific result, not a uniform +one. ci.yml was restored from the same run. +""" + +[[control]] +context = "test (ubuntu-latest, py3.14)" +plants = """ +A test that cannot pass, run by a child pytest outside this repository's rootdir. The assertion is on +the EXIT CODE alone. +""" +red = ["tests/test_merge_gate_controls.py::test_a_failing_test_makes_the_pytest_leg_exit_nonzero"] +holds = """ +A passing fixture must leave the runner at exit 0 -- a runner that failed unconditionally would satisfy +the planted case while blocking every PR, and nothing would say which of the two it was. The exit codes +are additionally re-measured with the child pinned to a HOSTILE encoding, because a control that only +ever ran under a favourable ambient value proves nothing about the Windows legs. +""" +green = [ + "tests/test_merge_gate_controls.py::test_a_passing_fixture_leaves_the_pytest_leg_green", + "tests/test_merge_gate_controls.py::test_the_pytest_exit_code_does_not_depend_on_the_ambient_encoding", +] +observed = """ +The planted failure exits non-zero and the passing fixture exits 0, unchanged under +PYTHONIOENCODING=ascii / PYTHONUTF8=0. This is not hypothetical plumbing: BACKLOG #1000 records the +measured case one layer over, where `pwsh -File script.ps1` returned 0 although the script inside died +at parameter binding, making every execution assertion built on that return code vacuously green. +""" + +[[control]] +context = "test (windows-2022, py3.14)" +plants = "As the ubuntu leg: a test that cannot pass, asserted on the child's exit code." +red = ["tests/test_merge_gate_controls.py::test_a_failing_test_makes_the_pytest_leg_exit_nonzero"] +holds = """ +Same asymmetry, and the encoding half is load-bearing HERE specifically: the Windows legs are where an +ambient code page differs from the author's shell, which is how a wave-2 lane shipped a test that +passed ubuntu and would have reddened these two contexts. +""" +green = [ + "tests/test_merge_gate_controls.py::test_a_passing_fixture_leaves_the_pytest_leg_green", + "tests/test_merge_gate_controls.py::test_the_pytest_exit_code_does_not_depend_on_the_ambient_encoding", +] +observed = "Run on this Windows worktree; exit codes as above, unchanged under a hostile encoding." + +[[control]] +context = "test (windows-2025, py3.14)" +plants = "As the ubuntu leg: a test that cannot pass, asserted on the child's exit code." +red = ["tests/test_merge_gate_controls.py::test_a_failing_test_makes_the_pytest_leg_exit_nonzero"] +holds = "Same asymmetry as the windows-2022 leg; the two differ only in runner image." +green = [ + "tests/test_merge_gate_controls.py::test_a_passing_fixture_leaves_the_pytest_leg_green", + "tests/test_merge_gate_controls.py::test_the_pytest_exit_code_does_not_depend_on_the_ambient_encoding", +] +observed = "Run on this Windows worktree; exit codes as above, unchanged under a hostile encoding." + +# --- security.yml ---------------------------------------------------------------------------------- + +[[control]] +context = "bandit (Python SAST)" +plants = """ +`bandit -ll` against the shipped invocation's muting detector. A severity floor keeps the scanner +running and the exit code intact while discarding a whole severity band -- so the required context goes +on reporting success, and a neutering scan looking for ADDED idioms (`|| true`, `--exit-zero`) cannot +see it. The sibling scope control plants the other half: a scan re-narrowed to an allow-list, which is +BACKLOG #334's shape. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_bandit_invocation_carries_no_severity_or_confidence_floor", + "tests/test_lint_scope_parity.py::test_ci_bandit_scans_the_repo_not_an_allow_list", +] +holds = """ +The reviewed `--skip B101,B110,B311,B404,B608` list must NOT be flagged. Each entry there is a per-check +exclusion with a stated reason, not a severity floor, and a detector that flagged it would be "fixed" by +deleting a correct annotation. The excludes must also keep matching semgrep's, so widening one gate's +blind spot cannot pass as tightening the other's. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_muting_detector_fires_on_a_synthetic_floor", + "tests/test_lint_scope_parity.py::test_semgrep_and_bandit_exclude_the_same_paths", +] +observed = """ +The detector fires on a synthetic `bandit -r . -ll --skip B101` and stays clean on the shipped command +in the same call. Detection by bandit itself runs in its own CI job against the pinned scanner lock. + +RUN AGAINST A NEUTERED GATE, 2026-08-10: adding `-ll` to security.yml's real bandit invocation turned +1 of the 4 selected controls red and left 3 green; security.yml was restored from the same run. +""" + +[[control]] +context = "pip-audit (dependency vulnerabilities)" +plants = """ +A hallucinated distribution name, a registered-but-empty project, and a freshly registered one, fed to +`scripts/security/new_dependency_check.py` -- the step in this job that answers what pip-audit +structurally cannot: a hallucinated name has no advisory, so it resolves, locks, hashes and installs +through every other DEP-1 control clean. +""" +red = [ + "tests/test_new_dependency_check.py::test_a_nonexistent_name_is_caught", + "tests/test_new_dependency_check.py::test_main_exits_1_on_a_hallucinated_name", + "tests/test_new_dependency_check.py::test_main_exits_2_when_the_sweep_examines_nothing", +] +holds = """ +A clean tree must produce no findings AND a non-zero examined count -- "found nothing" and "did not +look" are different answers and only one of them is clean. An allowlisted name must be skipped without +being counted, so the sweep's own liveness number cannot be inflated by entries it never checked. +""" +green = [ + "tests/test_new_dependency_check.py::test_a_clean_tree_produces_no_findings_and_a_nonzero_count", + "tests/test_new_dependency_check.py::test_the_allowlist_skips_a_name_and_does_not_count_it", +] +observed = """ +The planted names are refused and the clean tree passes with a non-zero count, in this suite. The +audit half (pip-audit over three locks) needs the network and the scanner lock and runs in its own job; +its documented blind spot -- a real package that is not the INTENDED one -- is recorded in the script. +""" + +[[control]] +context = "npm-audit (ide dependency vulnerabilities)" +plants = """ +`npm audit --audit-level=high` against the shipped invocation's muting detector. That flag exits 0 on +moderate advisories, which silently contradicts the job's own comment that the default level "fails on +ANY severity, matching pip-audit's strict posture". The second plant is the quieter one: a missing +`working-directory`, which leaves `--package-lock-only` auditing a tree with no lockfile. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_npm_audit_invocation_carries_no_severity_floor", + "tests/test_merge_gate_controls.py::test_the_npm_audit_target_lockfile_exists", +] +holds = """ +The shipped `npm audit --package-lock-only` must stay clean under the same detector, so the control is +not simply refusing every flag. `--package-lock-only` is itself an install-free NARROWING of what npm +does, and it is deliberate -- the control must distinguish it from a severity floor. +""" +green = ["tests/test_merge_gate_controls.py::test_the_muting_detector_fires_on_a_synthetic_floor"] +observed = """ +The detector fires on the synthetic floor and stays clean on the shipped command; ide/package-lock.json +resolves from the declared working-directory. Detection by npm itself runs in its own CI job. +""" + +[[control]] +context = "gitleaks (secret scan)" +plants = """ +Six credential shapes the default ruleset exists to catch -- an AWS access key id, a GitHub PAT, a Slack +bot token, a PEM private-key header, a 40-hex token and a URL with an inline database password -- all +ASSEMBLED AT RUNTIME, never committed as literals, and matched against every allowlist regex in +`.gitleaks.toml`. That allowlist is the one place this required context can be disabled without +touching a workflow at all. +""" +red = [ + "tests/test_merge_gate_controls.py::test_no_gitleaks_allowlist_regex_swallows_a_fabricated_secret", + "tests/test_merge_gate_controls.py::test_the_gitleaks_config_still_extends_the_default_ruleset", +] +holds = """ +The shipped allowlist must stay GREEN in the same call that flags two planted broad patterns. The +control is not demanding an empty allowlist -- the entries there are documented non-secret fixtures (a +CI container password, a SOAP placeholder class, a pinned audit-chain digest) and deleting them would +red the gate on correct content, which is how an allowlist gets widened in the first place. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_allowlist_narrowness_detector_fires_on_a_planted_broad_regex" +] +observed = """ +Planted `.{8,}` and `[A-Za-z0-9_/+-]{20,}` are both caught; the shipped entries match none of the six +shapes. This gate has also been seen red in ordinary use: the redaction suite's first JWT fixture was a +literal token and the gitleaks pre-commit hook rejected the commit, which is why that fixture is now +assembled from parts. + +RUN AGAINST A NEUTERED GATE, 2026-08-10: replacing one literal allowlist entry in the real +.gitleaks.toml with `[A-Za-z0-9_/+-]{16,}` turned 2 of the 4 selected controls red and left 2 green; +.gitleaks.toml was restored from the same run. +""" + +[[control]] +context = "semgrep (project SAST rules)" +plants = """ +Annotated handler fixtures carrying inter-statement PHI taint, an aliased import, an ambient subprocess +call, ambient deserialization, ambient eval/exec, an unparameterized db_lookup and a PHI-to-file write. +Each rule must fire EXACTLY as annotated. +""" +ci = "python scripts/ci/assert_semgrep_handler_taint.py" +workflow = "security.yml" +red = [ + "tests/test_lint_scope_parity.py::test_ci_semgrep_still_fails_the_build_on_a_finding", + "tests/test_lint_scope_parity.py::test_ci_semgrep_scans_the_repo_not_an_allow_list", +] +holds = """ +The `# ok` cases in the same fixture -- log.debug, msg.control_id, a parameterized db_lookup -- must +contribute ZERO findings, and the count equality is what asserts it. A rule set that fired on those +would be indistinguishable from one that fires on everything. On the workflow side, the excludes must +keep matching bandit's rather than growing independently. +""" +green = ["tests/test_lint_scope_parity.py::test_semgrep_and_bandit_exclude_the_same_paths"] +observed = """ +The fixture asserter is the gate's own step and fails the job on any count mismatch in either +direction. `--error` is separately asserted here because without it semgrep prints every match across +the whole scanned tree and exits 0 -- a required context that cannot fail, which is strictly worse than +the narrow scope it replaced. +""" + +[[control]] +context = "crypto-inventory (ASVS 11.1.3 discovery gate)" +plants = """ +A seam-only delegator with no inventory row, a Python file planted under the ide/ TypeScript tree, and +a provider row deleted from the documented inventory. Each must be reported. +""" +red = [ + "tests/test_crypto_inventory_scanner.py::test_seam_only_delegator_is_reported_undocumented", + "tests/test_crypto_inventory_scanner.py::test_ide_invariant_flags_a_planted_python_file", + "tests/test_crypto_inventory_doc.py::test_guard_detects_a_removed_provider_row", +] +holds = """ +A plain non-crypto store import must NOT be reported, and the real tree must stay clean -- a discovery +gate that flagged ordinary imports would be silenced by the first person it inconvenienced, which is a +slower version of switching it off. +""" +green = [ + "tests/test_crypto_inventory_scanner.py::test_a_plain_non_crypto_store_import_is_not_a_false_positive", + "tests/test_crypto_inventory_scanner.py::test_gate_is_clean_on_the_real_tree", +] +observed = "Each planted shape is reported and the real tree is clean, in this suite." + +[[control]] +context = "forbidden-content (customer/PHI leak guard)" +plants = """ +A customer name, a case-sensitive site code, a routable host IP, and a file under docs/security/ caught +by its PATH alone; plus a run that examines zero files, which must refuse rather than report clean. +""" +red = [ + "tests/test_scan_forbidden.py::test_scan_file_flags_customer_name", + "tests/test_scan_forbidden.py::test_scan_file_flags_site_code", + "tests/test_scan_forbidden.py::test_a_file_under_docs_security_is_flagged_by_its_path_alone", + "tests/test_scan_forbidden.py::test_zero_examined_still_refuses_rather_than_reporting_clean", +] +holds = """ +Private and documentation IP ranges, ordinary docs paths, and clean text must all pass. This is the +asymmetry with the most history behind it: BACKLOG #321 and #325 are both this gate failing to fire, +and a control that reddened on 10.0.0.1 or on docs/CI.md would have been relaxed rather than trusted. +""" +green = [ + "tests/test_scan_forbidden.py::test_scan_file_allows_private_and_doc_ips", + "tests/test_scan_forbidden.py::test_the_path_detector_does_not_flag_ordinary_paths", + "tests/test_scan_forbidden.py::test_scan_file_clean_text_has_no_hits", +] +observed = """ +Every planted shape is flagged and every benign one passes, in this suite. The gate additionally fails +CLOSED in CI on a per-section detector floor, because a partially-mangled token list once loaded as few +as 1 of 21 detectors and passed with a green tick. +""" + +# --- cla.yml --------------------------------------------------------------------------------------- + +[[control]] +context = "cla" +plants = """ +The context STRING, which is the only part of this gate the repository controls -- its detection lives +entirely in a third-party action. Planted historically rather than synthetically: docs/CI.md and cla.yml +both told a reader to require "CLA Assistant", the WORKFLOW name, which matches no status check. Adding +it to branch protection would have wedged every PR forever. A `name:` on the job would do the same +thing quietly, by moving the context off `cla`. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_cla_job_still_reports_under_the_job_key_and_not_the_workflow_name", + "tests/test_required_contexts.py::test_no_claim_file_names_the_cla_context_by_its_workflow_name", +] +holds = """ +The maintainer/bot allowlist must keep WORKING -- emptying it would demand a signature on every +maintainer push. What is asserted is its SHAPE: an enumeration, never a glob, because `bot*` would let +any human whose username begins with "bot" skip signing (review low-28). And the context must still +resolve to a real job, which is the opposite failure from the one planted above. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_cla_allowlist_is_an_enumeration_and_not_a_glob", + "tests/test_required_contexts.py::test_every_required_context_matches_a_real_job", +] +observed = """ +The job key is `cla`, it declares no `name:`, it runs on pull_request_target, and the allowlist is a +three-name enumeration. The wrong string was live in two files for months and is now refused by a test. +""" + +# --- backlog-hygiene.yml --------------------------------------------------------------------------- + +[[control]] +context = "a PR that implements BACKLOG #N must update BACKLOG.md" +plants = """ +A synthetic repository shaped like the PR this gate exists to police: a branch that claims +`BACKLOG #42` and changes engine code and nothing else, while `main` has separately moved +docs/BACKLOG.md since the branch point. The gate's OWN shell is lifted out of the workflow and run +against it -- not a re-implementation, which would be a second copy free to agree with itself. +""" +red = [ + "tests/test_merge_gate_controls.py::test_the_backlog_hygiene_gate_fails_a_code_pr_that_leaves_the_ledger_alone", + "tests/test_merge_gate_controls.py::test_the_two_dot_form_of_the_gate_passes_the_same_planted_pull_request", +] +holds = """ +Four benign shapes must stay green: no claim at all, a claim honoured in the live ledger, a claim +honoured in the ARCHIVE (an item retired between the claim and the PR), and a bare `#42`, which is a PR +number in this repo rather than an item. A gate that failed all four would satisfy the planted case and +block every pull request in the repository. +""" +green = [ + "tests/test_merge_gate_controls.py::test_the_backlog_hygiene_gate_leaves_the_benign_shapes_alone", + "tests/test_merge_gate_controls.py::test_the_bash_namespace_probe_rejects_an_interpreter_that_cannot_see_the_fixture", +] +observed = """ +RUN AGAINST THE PRE-FIX GATE, which is what makes this evidence rather than a claim. The identical +fixture goes RED (exit 1) through the shipped three-dot diff and GREEN (exit 0) with only the diff form +reverted to two-dot. That reproduces the recorded defect exactly: the two-dot form reports main-side +changes as reverse deltas, so any main-side edit to docs/BACKLOG.md credited every PR with an older +base, and the gate went green while enforcing nothing -- on precisely the population it polices. + +AND THE CONTROL ITSELF WAS CAUGHT DEPENDING ON ITS AMBIENT ENVIRONMENT, 2026-08-10, before it shipped. +Its first version resolved bash from PATH and passed an absolute Windows path; under a Git Bash parent +it passed, and from PowerShell -- where PATH resolves `bash` to the WSL launcher, a different +filesystem namespace -- all six hygiene assertions failed with exit 127. bash is now derived from +`git`, made to read a file this process wrote before any verdict is believed, and handed a RELATIVE +script path; 126/127 are a hard failure rather than a gate verdict. Re-measured green under a +deliberately hostile ambient (PYTHONIOENCODING=cp1252, PYTHONUTF8=0, LC_ALL=tr_TR.UTF-8, a global +core.autocrlf, and PATH ordered so the WSL bash comes first). +""" diff --git a/tests/test_ci_docs_only_detector.py b/tests/test_ci_docs_only_detector.py index 61218bbc..8edb0e1c 100644 --- a/tests/test_ci_docs_only_detector.py +++ b/tests/test_ci_docs_only_detector.py @@ -161,6 +161,13 @@ def test_extensionless_config_is_code_and_gitattributes_was_not_before( ".github/workflows/ci.yml", "pyproject.toml", ".gitignore", # BACKLOG #327 โ€” deliberately code, and it has no code-ish extension + # BACKLOG #1000. The negative-control reconciliation runs under pytest, which is gated on + # `code == 'true'` โ€” the same coupling that let a #320 banner merge without the suite + # compiling. Its decay mode is a context arriving in branch protection and being mirrored + # into `.github/required-contexts.txt`, so these two paths MUST classify as code or the gate + # is skipped on exactly the pull request shape it exists for. The first is extensionless. + ".github/required-contexts.txt", + "tests/negative_controls.toml", ], ) def test_code_paths_run_the_suite(path: str, pats: tuple[str, str, str]) -> None: diff --git a/tests/test_coord_claim_release_history.py b/tests/test_coord_claim_release_history.py new file mode 100644 index 00000000..0e41a498 --- /dev/null +++ b/tests/test_coord_claim_release_history.py @@ -0,0 +1,283 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Every release must leave a record -- including a ``-Force`` one (BACKLOG #1068). + +``claim.ps1 -Release`` was ``Remove-Item`` and nothing else. With ``-Force`` that meant any session +could release a claim it does not hold and leave no trace of who released whose claim, when, or why. +The escape hatch is NECESSARY -- a claim whose holder's worktree is gone would otherwise be stuck +forever, and the script prints that recipe itself -- so the fix is a RECORD, not a refusal. Every +test below therefore asserts a line was written; none asserts a release was blocked. + +**Measured 2026-08-10, which is why the record has to name the PRIOR holder.** A coordinator +force-released claim #344 after establishing on evidence that the holder's worktree was gone and that +the work the claim's note guarded had already merged, while that note still read "UNPUSHED, NO PR, +GitHub finds NOTHING". The release was correct and it left nothing behind to find. Nothing is +deployed and there is no user to mislead (CLAUDE.md 0), but the registry is a shared coordination +artifact today and a stale note in it has already blocked a lane from claiming an item. + +**The load-bearing negatives, per BACKLOG #1000.** A record that logged only "somebody released +something" satisfies "a line was appended" and answers none of the question, so: + +* ``test_a_force_takeover_names_the_PRIOR_holder_not_the_releaser`` -- the two paths must differ, and + the record must carry both. +* ``test_force_false_is_recorded_when_the_flag_was_not_passed`` -- a hardcoded ``force: true`` passes + the takeover test and fails this one. +* ``test_a_REFUSED_release_writes_nothing`` -- the record is written before the removal, so the case + that can produce a FALSE line (a release that never happened) is checked directly. +""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +CLAIM = ROOT / "scripts" / "coord" / "claim.ps1" +TIMEOUT = 60 + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None or os.name != "nt", + reason="claim.ps1 needs pwsh on Windows", +) + + +def git(repo: Path, *args: str) -> str: + proc = subprocess.run( + ["git", "-C", str(repo), *args], capture_output=True, text=True, timeout=TIMEOUT, check=True + ) + return proc.stdout + + +@pytest.fixture +def repo(tmp_path: Path) -> Path: + """A throwaway checkout carrying its OWN copy of claim.ps1. + + The copy is the sandbox: the script anchors on ``$PSScriptRoot`` (BACKLOG #1060), so it writes to + THIS repository's registry and never to the real one. Same fixture shape as + ``test_coord_claim_liveness.py`` for the same reason. + """ + r = tmp_path / "repo" + (r / "scripts" / "coord").mkdir(parents=True) + subprocess.run(["git", "init", "-q", "-b", "main", str(r)], check=True, capture_output=True) + git(r, "config", "user.email", "t@example.invalid") + git(r, "config", "user.name", "t") + shutil.copy2(CLAIM, r / "scripts" / "coord" / "claim.ps1") + (r / "f.txt").write_text("x", encoding="utf-8") + git(r, "add", "f.txt", "scripts/coord/claim.ps1") + git(r, "commit", "-qm", "base") + return r + + +def claim(cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: + """Run the checkout's OWN copy, with the child's environment PINNED. + + ``env`` is passed explicitly rather than inherited. claim.ps1 reads nothing from the environment + today, so nothing here is load-bearing for the assertions -- but a test that shells out and + inherits whatever the developer's shell exports is measuring the shell as much as the code, which + is how a lane's green survived locally and reddened at integration in wave 2. Pinning costs one + line and removes the whole class. + """ + env = dict(os.environ) + for var in ("PYTHONIOENCODING", "PYTHONUTF8"): + env.pop(var, None) + return subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(cwd / "scripts" / "coord" / "claim.ps1"), + *args, + ], + cwd=str(cwd), + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=TIMEOUT, + check=False, + env=env, + ) + + +def history_path(repo: Path) -> Path: + return repo / ".git" / "mefor-coord" / "claims" / ".history" + + +def records(repo: Path) -> list[dict[str, object]]: + """Every record in the ledger, parsed. + + Asserts the on-disk shape as it goes: JSON Lines, LF-terminated, no CR. A ledger that is only + ever read back by the code that wrote it can drift into any shape at all; this is the file an + operator greps months later, so the format is part of the contract. + """ + p = history_path(repo) + if not p.exists(): + return [] + text = p.read_text(encoding="utf-8") + assert "\r" not in text, f"the ledger must be LF-only JSON Lines, not CRLF: {text!r}" + assert text.endswith("\n"), f"every record must be newline-terminated: {text!r}" + return [json.loads(line) for line in text.splitlines() if line.strip()] + + +def peer_holding(repo: Path, tmp_path: Path, key: str, note: str = "the peer's work") -> Path: + """A second worktree that holds ``key``, sharing this repo's object store and registry.""" + peer = tmp_path / "peer-wt" + git(repo, "worktree", "add", "-q", "-b", "peer-branch", str(peer)) + assert claim(peer, "-Take", key, "-Note", note).returncode == 0 + return peer + + +def norm(p: object) -> str: + return str(p).replace("\\", "/").rstrip("/").casefold() + + +# -------------------------------------------------------------------------------------------------- +# the ordinary release +# -------------------------------------------------------------------------------------------------- + + +def test_releasing_your_own_claim_appends_a_record(repo: Path) -> None: + assert claim(repo, "-Take", "k", "-Note", "my work").returncode == 0 + proc = claim(repo, "-Release", "k") + + assert proc.returncode == 0, proc.stdout + proc.stderr + rows = records(repo) + assert len(rows) == 1, rows + row = rows[0] + assert row["event"] == "release" + assert row["key"] == "k" + assert norm(row["released_by"]) == norm(repo) + assert norm(row["prior_holder"]) == norm(repo) + assert row["prior_note"] == "my work" + assert row["force"] is False + assert row["ts"], "a record with no timestamp cannot be ordered against anything" + assert row["claimed"], "the record must carry when the claim was taken, not only when it ended" + + +def test_the_release_output_names_the_ledger_it_wrote_to(repo: Path) -> None: + """A record nobody can find is barely better than no record. + + The path is not guessable -- it lives beside the SHARED object store, which for a linked worktree + is a different directory from the one the operator is standing in. + """ + assert claim(repo, "-Take", "k", "-Note", "x").returncode == 0 + out = claim(repo, "-Release", "k").stdout + assert ".history" in out, out + + +# -------------------------------------------------------------------------------------------------- +# -Force: the case the item is about +# -------------------------------------------------------------------------------------------------- + + +def test_a_force_takeover_is_recorded(repo: Path, tmp_path: Path) -> None: + peer = peer_holding(repo, tmp_path, "k", note="the peer is mid-flight") + proc = claim(repo, "-Release", "k", "-Force") + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert not (repo / ".git" / "mefor-coord" / "claims" / "k.json").exists(), ( + "-Force must still release -- this item is about auditability, not prevention" + ) + rows = records(repo) + assert len(rows) == 1, rows + assert rows[0]["force"] is True + assert norm(rows[0]["prior_holder"]) == norm(peer) + + +def test_a_force_takeover_names_the_PRIOR_holder_not_the_releaser( + repo: Path, tmp_path: Path +) -> None: + """THE load-bearing case. "who released whose claim" needs both halves, and they differ here. + + A record carrying only the releasing worktree passes every other test in this file and answers + none of the question #1068 asks. + """ + peer = peer_holding(repo, tmp_path, "k", note="UNPUSHED, NO PR") + assert claim(repo, "-Release", "k", "-Force").returncode == 0 + + row = records(repo)[0] + assert norm(row["prior_holder"]) == norm(peer) + assert norm(row["released_by"]) == norm(repo) + assert norm(row["prior_holder"]) != norm(row["released_by"]), ( + "the fixture must make the two paths differ, or this test cannot fail" + ) + # The note is what a reader needs to judge whether the release was right -- it is the field that + # was stale and wrong in the 2026-08-10 incident. + assert row["prior_note"] == "UNPUSHED, NO PR" + assert row["prior_branch"] == "peer-branch" + + +def test_force_false_is_recorded_when_the_flag_was_not_passed(repo: Path) -> None: + """The negative that a hardcoded ``force: true`` cannot pass.""" + assert claim(repo, "-Take", "k", "-Note", "x").returncode == 0 + assert claim(repo, "-Release", "k").returncode == 0 + assert records(repo)[0]["force"] is False + + +def test_a_force_takeover_says_so_on_stdout_too(repo: Path, tmp_path: Path) -> None: + """The releasing session should be told it took someone else's key, not just that it succeeded.""" + peer_holding(repo, tmp_path, "k") + out = claim(repo, "-Release", "k", "-Force").stdout + assert "TOOK OVER" in out, out + assert "peer-branch" in out, out + + +# -------------------------------------------------------------------------------------------------- +# what must NOT be recorded +# -------------------------------------------------------------------------------------------------- + + +def test_a_REFUSED_release_writes_nothing(repo: Path, tmp_path: Path) -> None: + """The record is written BEFORE the removal, so a false line is the failure mode to check. + + A refused release did not happen. A ledger claiming it did is worse than the silence it replaced. + """ + peer_holding(repo, tmp_path, "k") + proc = claim(repo, "-Release", "k") # no -Force: refused + + assert proc.returncode == 1 + assert records(repo) == [], "a refusal is not a release and must not be recorded as one" + + +def test_releasing_an_unclaimed_key_writes_nothing(repo: Path) -> None: + proc = claim(repo, "-Release", "never-claimed") + assert proc.returncode == 0 + assert records(repo) == [] + + +# -------------------------------------------------------------------------------------------------- +# the ledger's own properties +# -------------------------------------------------------------------------------------------------- + + +def test_records_APPEND_rather_than_replace(repo: Path, tmp_path: Path) -> None: + """A ledger that keeps only the last release is a status field, not a history.""" + assert claim(repo, "-Take", "a", "-Note", "first").returncode == 0 + assert claim(repo, "-Release", "a").returncode == 0 + peer_holding(repo, tmp_path, "b", note="second") + assert claim(repo, "-Release", "b", "-Force").returncode == 0 + + rows = records(repo) + assert [r["key"] for r in rows] == ["a", "b"], rows + assert [r["force"] for r in rows] == [False, True], rows + + +def test_the_ledger_is_not_mistaken_for_a_claim(repo: Path) -> None: + """It lives INSIDE the claims directory, so the readers of that directory must ignore it. + + ``-List`` globs ``*.json`` and scripts/hooks/claim_check.py opens ``.json``; a dotfile named + ``.history`` is invisible to both. Asserted rather than argued, because "the glob will not match + it" is exactly the kind of premise that is true until someone widens the glob. + """ + assert claim(repo, "-Take", "k", "-Note", "x").returncode == 0 + assert claim(repo, "-Release", "k").returncode == 0 + assert history_path(repo).is_file(), "the ledger must exist for this test to mean anything" + + out = claim(repo, "-List").stdout + assert "No active claims." in out, out diff --git a/tests/test_credential_parameter_mapping.py b/tests/test_credential_parameter_mapping.py new file mode 100644 index 00000000..bcdb3c95 --- /dev/null +++ b/tests/test_credential_parameter_mapping.py @@ -0,0 +1,565 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""A credential factory PARAMETER must land on a SETTING name the redactor covers (BACKLOG #1208). + +THE BOUNDARY, and why it needed its own guard. A connector factory takes a credential parameter and +emits it under a DIFFERENT setting name. Every redaction control operates on the SETTING name. Nothing +asserted the two agree, so a rename silently moved a credential outside the control's domain. Three +measured instances of that one shape, all now closed: + +=================================== ========================== ============================ +factory PARAMETER (classified) emitted SETTING (was not) +=================================== ========================== ============================ +``with_signing`` ``private_key`` ``sign_private_key`` +``with_signing`` ``private_key_password`` ``sign_private_key_password`` +``Rest`` ``proxy`` ``proxy_url`` +=================================== ========================== ============================ + +``_is_secret_setting("private_key")`` was True and ``_is_secret_setting("sign_private_key")`` was +False: the parameter was covered and the setting it became was not. ``proxy`` -> ``proxy_url`` crosses +the same boundary and is harmless only because the URL-userinfo rule happens to cover the destination, +which is luck rather than design. THIS FILE MAKES IT DESIGN. + +WHY THE TWO EXISTING GUARDS CANNOT SEE IT. + +* ``test_every_credential_shaped_factory_param_is_classified`` reads PARAMETER names -- one abstraction + level away from where redaction operates. That is exactly how the ``with_signing`` rename walked + through it. +* ``tests/test_connection_factory_redaction_domain.py`` reads EMITTED settings end to end and proves the + OUTCOME per factory: nothing recognisably secret survives. It does not prove the MAPPING. A parameter + the factory drops, renames into a container, or folds into a composed string emits no sentinel at all, + and "no sentinel survived redaction" is then true for the least interesting reason. + +THIS IS NOT A FOURTH NAME LIST, and that is the whole point of the item. ``private_key`` and +``sign_private_key`` are different strings, so any name-to-name comparison has to be TAUGHT each rename +and therefore cannot catch the next one. **The mapping is discovered by following the VALUE**: inject a +unique sentinel into ONE parameter, call the factory, and search the emitted settings for it. The +destination key is whatever the sentinel is found under -- read off the running code, never declared +here. Only then is the real redactor asked about that destination. + +WHAT EACH PARAMETER MUST DO, one of: + +1. land in at least one setting whose name the redactor MASKS (the credential case), or +2. land in at least one setting whose name the URL-userinfo rule covers (the URL-bearing case), or +3. land in a destination declared in :data:`NON_MATERIAL_DESTINATIONS` -- a key that carries a NAME, a + PATH or an IDENTIFIER rather than material, WITH the reason, or +4. reach no setting at all, and be declared in :data:`NON_EMITTING` WITH the reason. + +A parameter that cannot be probed at all is a FAILURE, never a skip: an unbuildable factory is a hole +in the domain, and a hole that reports "clean" is the defect this family keeps recurring as. + +THE DECLARED TABLES CANNOT GO STALE. ``test_no_declared_exemption_is_stale`` fails when an entry in +either table is not actually reached by the probe. An allowlist nobody re-derives is the same defect one +level up -- a guard closing over an enumeration that stopped matching the code. + +MEASURED 2026-08-10, by reverting the shipped redaction to each pre-fix state in turn and watching what +this file does. 58 parameters probed across the spec-returning domain: + +=============================================== ================== ============================== +reverted to goes RED still GREEN +=============================================== ================== ============================== +shipped code (control) 0 58 +pre-#1106 (``sign_private_key`` unclassified) 2 56 +pre-#1207 (no URL-userinfo mask) 10 48 +shipped code again (control) 0 58 +=============================================== ================== ============================== + +**The red is SPECIFIC, not uniform, and that is the result worth having.** Pre-#1106 turns exactly the +two renamed ``with_signing`` parameters red and leaves 56 passing -- so this file locates the layer that +does the work rather than merely reacting to it. Pre-#1207 turns exactly the ten URL-bearing parameters +red. A control that reddened on all 58 in both cases would have looked stronger and taught nothing. + +WHAT IT ADDS OVER THE SIBLING, measured the same day rather than argued. De-classifying five credential +settings that reach connectors through an ``env()``-only refusal or a coupled argument -- +``intake_api_key``, ``intake_api_key_next``, ``credential_password``, ``ws_password``, +``client_key_password`` -- turns **this** file red on all five and +``test_connection_factory_redaction_domain.py`` red on **none**. That sibling drops a connector's +credential arguments when the connector refuses to be built with them, so those five surfaces were +inside its stated domain and outside what it could actually see. +""" + +from __future__ import annotations + +import importlib +import inspect +from typing import Any, NamedTuple + +import pytest + +import messagefoundry +from messagefoundry.config.wiring import redacted_settings +from tests.test_connection_factory_redaction_domain import ( + _factories_returning_a_spec, + _is_credential_param, +) + +# --------------------------------------------------------------------------------------------------- +# Sentinels. Fixed width with a terminator so no sentinel can be a PREFIX of another -- the first draft +# of this probe injected every parameter at once and reported `private_key` as landing in +# `sign_private_key_password`, because its sentinel was a prefix of the password parameter's. One +# parameter is injected per probe now, which removes the ambiguity at the source rather than papering +# over it, and the fixed width keeps that true if the probe is ever widened again. +# --------------------------------------------------------------------------------------------------- +_SENTINEL_N = 0 + + +def _next_sentinel() -> str: + global _SENTINEL_N + _SENTINEL_N += 1 + return f"MFMAP{_SENTINEL_N:04d}ZZ" + + +#: A SOAP body-secret placeholder must match ``^[A-Za-z0-9_.@-]{16,64}$``; anything shorter is refused. +def _placeholder_sentinel(sentinel: str) -> str: + return f"{sentinel}{'0' * (16 - len(sentinel))}" if len(sentinel) < 16 else sentinel + + +class Landing(NamedTuple): + """Where one parameter's sentinel was actually found.""" + + factory: str + param: str + kind: str # "credential" | "url" + form: str # "inline" | "env()" + keys: tuple[str, ...] + + +# --------------------------------------------------------------------------------------------------- +# Declared tables. Both are asserted REACHED, so neither can quietly stop matching the code. +# --------------------------------------------------------------------------------------------------- + +#: Destination setting keys that a credential-shaped PARAMETER legitimately lands on unredacted, +#: because the key carries a NAME, a PATH or an IDENTIFIER and not material. Each needs a reason. +NON_MATERIAL_DESTINATIONS: dict[str, str] = { + "odbc_password_key": ( + "the NAME of the ODBC keyword to put the password under (default 'PWD'), not the password -- " + "naming indirection. Masking it would hide which keyword an operator has to look at" + ), + "odbc_user_key": "the NAME of the ODBC user keyword (default 'UID'), not the user", + "signing_key": ( + "a PATH to the sender's PEM/DER key, not the material; transports/direct.py _read_file()s it. " + "The engine classifies signing_key_password and NOT signing_key deliberately" + ), + "intake_api_key_header": ( + "the NAME of the header the intake credential arrives in (default 'x-api-key'). The " + "credential itself is intake_api_key, which IS masked -- and is env()-only besides" + ), + "credential_domain": ( + "an AD domain name, documented non-secret in wiring.py: it names the directory the share " + "identity belongs to. credential_username and credential_password are both masked" + ), + "body_secret_tokens": ( + "SOAP body-secret PLACEHOLDER tokens (ADR 0015 amendment). Public BY CONTRACT -- the token " + "sits in committed Handler source and the transport swaps the real env() credential in at " + "send time. The paired body_secret_value_ IS masked" + ), + "proxy_no_proxy": ( + "hostname EXCLUSIONS -- the hosts to bypass the proxy for. Public by nature, and not a URL: " + "it is selected by the URL suffix rule only because the name ends in 'proxy'. Masking it " + "would hide routing configuration an operator has to audit" + ), +} + +#: Parameters whose sentinel reaches NO setting, with the reason. Empty today and asserted so: every +#: credential-shaped parameter on every factory currently lands somewhere. The table exists because +#: "reached nothing" is the interesting outcome the OUTCOME-level guard cannot distinguish from +#: "reached something safe", and it must have a stated home when it happens rather than passing quietly. +NON_EMITTING: dict[tuple[str, str], str] = {} + +#: Arguments a factory REQUIRES alongside the parameter under test, because it refuses the parameter +#: on its own. Connector-specific and therefore declared with a reason rather than inferred -- but the +#: alternative is dropping the parameter, and a probe that drops what it cannot build is a guard that +#: examined nothing. +ENABLING_ARGUMENTS: dict[tuple[str, str], dict[str, Any]] = { + ("Http", "intake_api_key"): {"intake_auth": "api_key"}, + ("Http", "intake_api_key_next"): { + "intake_auth": "api_key", + "intake_api_key": messagefoundry.env("probe_primary_key"), + }, + ("Http", "intake_api_key_header"): { + "intake_auth": "api_key", + "intake_api_key": messagefoundry.env("probe_primary_key"), + }, + ("File", "credential_username"): {"credential_password": messagefoundry.env("probe_share_pw")}, + ("File", "credential_password"): { + "credential_username": messagefoundry.env("probe_share_user") + }, + ("File", "credential_domain"): { + "credential_username": messagefoundry.env("probe_share_user"), + "credential_password": messagefoundry.env("probe_share_pw"), + }, + ("Soap", "basic_password"): {"basic_user": "probe-user"}, + ("Soap", "ws_password"): {"ws_username": "probe-user"}, + ("Soap", "proxy_password"): {"proxy": "http://proxy.invalid:8080", "proxy_user": "probe-user"}, + ("Rest", "proxy_password"): {"proxy": "http://proxy.invalid:8080", "proxy_user": "probe-user"}, + ("FHIR", "proxy_password"): {"proxy": "http://proxy.invalid:8080", "proxy_user": "probe-user"}, + ("DICOMweb", "proxy_password"): { + "proxy": "http://proxy.invalid:8080", + "proxy_user": "probe-user", + }, +} + +#: Parameters whose accepted SHAPE is not a bare string, so a plain sentinel cannot be injected. The +#: builder still follows the value; only the wrapper is declared. +SHAPED_INJECTION: dict[tuple[str, str], Any] = { + ("Soap", "body_secrets"): lambda s: {_placeholder_sentinel(s): messagefoundry.env(s.lower())}, +} + +#: Suffixes that make a parameter URL-BEARING: its value may carry ``user:password@`` userinfo, so the +#: destination must be a key the URL rule covers. A SUFFIX rule, not an enumeration -- and its coverage +#: is checked against the running code by +#: :func:`test_the_url_suffix_rule_selects_every_parameter_that_lands_in_a_url_setting`. +URL_BEARING_SUFFIXES = ("url", "uri", "endpoint", "proxy") + +#: What the redactor's own URL rule covers. Read here only to CHECK the suffix rule above against the +#: code; the mapping itself is still discovered by following the sentinel. +_URL_DESTINATION_SUFFIXES = ("url", "_uri", "endpoint", "_endpoint") + + +def _is_url_bearing_param(name: str) -> bool: + low = name.lower() + return low.endswith(URL_BEARING_SUFFIXES) and not _is_credential_param(name) + + +def _resolve(mod: str, name: str) -> Any: + fn = getattr(importlib.import_module(mod), name, None) + return fn if callable(fn) else None + + +def _takes_a_spec(fn: Any) -> bool: + params = list(inspect.signature(fn).parameters.values()) + return bool(params) and params[0].name == "spec" + + +def _filler(p: inspect.Parameter) -> Any: + """Any plausible value for a REQUIRED non-credential argument -- it keeps the call alive and is + never the thing under assertion.""" + ann = str(p.annotation) + if "url" in p.name.lower(): + return "https://example.invalid/token" + if "int" in ann and "str" not in ann: + return 2575 + if "statement" in p.name: + return "SELECT 1" + return "x" + + +def _usable_params(fn: Any) -> list[inspect.Parameter]: + params = list(inspect.signature(fn).parameters.values()) + rest = params[1:] if _takes_a_spec(fn) else params + return [p for p in rest if p.kind in (p.KEYWORD_ONLY, p.POSITIONAL_OR_KEYWORD)] + + +def _find_sentinel(settings: dict[str, Any], sentinel: str) -> tuple[str, ...]: + """Every settings key whose value carries the sentinel, at any depth. + + ``str(value)`` covers the nested cases the flat scan would miss -- a dict, a list, and an ``EnvRef`` + whose ``key`` IS the sentinel when the factory demanded ``env()``. + """ + low = sentinel.lower() + return tuple(k for k, v in settings.items() if low in str(v).lower()) + + +class ProbeResult(NamedTuple): + landing: Landing | None + refused_inline: bool + error: str | None + + +def _probe(factory: str, fn: Any, p: inspect.Parameter, kind: str) -> ProbeResult: + """Inject ONE sentinel into ONE parameter and report where it landed.""" + sentinel = _next_sentinel() + shaper = SHAPED_INJECTION.get((factory, p.name)) + if shaper is not None: + injected: Any = shaper(sentinel) + elif kind == "url": + injected = f"https://probeuser:{sentinel}@proxy.invalid:8080/path" + else: + injected = sentinel + + kwargs: dict[str, Any] = { + q.name: _filler(q) for q in _usable_params(fn) if q.default is inspect.Parameter.empty + } + kwargs.update(ENABLING_ARGUMENTS.get((factory, p.name), {})) + kwargs[p.name] = injected + base = messagefoundry.Rest(url="https://example.invalid/endpoint") + decorator = _takes_a_spec(fn) + + def build(kw: dict[str, Any]) -> Any: + return fn(base, **kw) if decorator else fn(**kw) + + refused = False + try: + spec = build(kwargs) + form = "inline" + except Exception as inline_exc: # noqa: BLE001 - the refusal message IS the signal here + # THE CONNECTOR REFUSES AN INLINE CREDENTIAL AND DEMANDS env(). That is the STRONGER control -- + # the value never resolves into settings, so no serializer can leak it. The MAPPING question + # survives it: the EnvRef's key is the sentinel, so the destination is still discoverable, and + # the destination is what this file asserts about. + refused = True + kwargs[p.name] = messagefoundry.env(sentinel.lower()) + try: + spec = build(kwargs) + form = "env()" + except Exception as env_exc: # noqa: BLE001 + return ProbeResult( + None, + refused, + f"inline: {type(inline_exc).__name__}: {inline_exc}; " + f"env(): {type(env_exc).__name__}: {env_exc}", + ) + + keys = _find_sentinel(dict(spec.settings), sentinel) + return ProbeResult(Landing(factory, p.name, kind, form, keys), refused, None) + + +def _collect() -> tuple[list[Landing], list[str], set[str]]: + """Probe every credential- and URL-bearing parameter of every spec-returning factory.""" + landings: list[Landing] = [] + errors: list[str] = [] + refused: set[str] = set() + for mod, name in _factories_returning_a_spec(): + fn = _resolve(mod, name) + if fn is None: # pragma: no cover - the sibling's domain test fails first if this happens + errors.append(f"{mod}.{name}: not importable") + continue + for p in _usable_params(fn): + kind = ( + "credential" + if _is_credential_param(p.name) + else "url" + if _is_url_bearing_param(p.name) + else None + ) + if kind is None: + continue + result = _probe(name, fn, p, kind) + if result.refused_inline: + refused.add(f"{name}.{p.name}") + if result.landing is None: + errors.append(f"{name}.{p.name} could not be built -- {result.error}") + continue + landings.append(result.landing) + return landings, errors, refused + + +LANDINGS, PROBE_ERRORS, REFUSED_INLINE = _collect() + +#: The credential-shaped landings, as pytest ids. +_IDS = [f"{ln.factory}.{ln.param}" for ln in LANDINGS] + + +def test_every_credential_and_url_parameter_was_probed() -> None: + """LIVENESS, and it reports WHAT it scanned rather than only a count. + + A probe that built nothing would make every parametrised assertion below vacuous, and an + unbuildable factory is a hole in the domain -- never a skip. + """ + print( + f"[1208] probed {len(LANDINGS)} parameters across {len({ln.factory for ln in LANDINGS})} " + f"factories; {len(REFUSED_INLINE)} refused an inline credential and were followed via env()" + ) + for ln in sorted(LANDINGS): + print( + f"[1208] {ln.factory}.{ln.param} ({ln.kind}, {ln.form}) -> {list(ln.keys) or 'NOTHING'}" + ) + assert not PROBE_ERRORS, ( + "these parameters could not be probed, so nothing here covers them:\n " + + "\n ".join(PROBE_ERRORS) + + "\nAdd the arguments the factory couples them to in ENABLING_ARGUMENTS (with the reason), or " + "the shape it demands in SHAPED_INJECTION. Do NOT drop the parameter: a probe that skips what " + "it cannot build is a guard that examined nothing." + ) + assert LANDINGS, "the probe found no credential- or URL-bearing parameters at all" + + +def test_the_domain_is_every_spec_returning_factory() -> None: + """The domain is DERIVED, and derived ONCE. + + It reuses ``_factories_returning_a_spec`` rather than re-walking the AST: a second derivation of + the same population is free to drift from the first, which is the defect one level up. 23 when + BACKLOG #1206 was fixed, and this asserts AT LEAST that -- a shrinking domain is how this class + recurs. + """ + discovered = {n for _, n in _factories_returning_a_spec()} + print(f"[1208] domain = {len(discovered)} spec-returning factories: {sorted(discovered)}") + assert len(discovered) >= 23, ( + f"the spec-returning domain shrank to {len(discovered)}; it was 23 when #1206 was fixed" + ) + probed = {ln.factory for ln in LANDINGS} + unprobed = discovered - probed + # A factory with no credential- or URL-bearing parameter legitimately contributes no landing. + for name in sorted(unprobed): + mod = next(m for m, n in _factories_returning_a_spec() if n == name) + fn = _resolve(mod, name) + assert fn is not None + interesting = [ + p.name + for p in _usable_params(fn) + if _is_credential_param(p.name) or _is_url_bearing_param(p.name) + ] + assert not interesting, ( + f"{name} has credential/URL parameters {interesting} but produced no landing -- the probe " + "silently dropped it" + ) + + +@pytest.mark.parametrize("landing", LANDINGS, ids=_IDS) +def test_a_credential_parameter_lands_on_a_setting_the_redactor_covers(landing: Landing) -> None: + """THE ASSERTION. The destination is read off the running code; the verdict comes from the real + redactor. + + Measured failure this catches, at engine ``64f6e178``: ``with_signing``'s ``private_key`` parameter + landed on ``sign_private_key``, and ``redacted_settings`` returned it VERBATIM through + ``GET /connections/{name}/metadata`` behind ``MONITORING_READ`` alone. + """ + if not landing.keys: + reason = NON_EMITTING.get((landing.factory, landing.param)) + assert reason, ( + f"{landing.factory}.{landing.param} reaches NO setting. It was consumed, composed into " + "another value, or dropped -- each of those needs a stated reason rather than silence, " + "because the outcome-level guard cannot tell them apart from 'safely redacted'. Declare it " + "in NON_EMITTING with the reason, or fix the factory." + ) + return + + probe = _next_sentinel() + unmasked: list[str] = [] + for key in landing.keys: + if key in NON_MATERIAL_DESTINATIONS: + continue + # Ask the REAL control about the destination the sentinel actually reached. For a credential + # parameter the whole value is secret; for a URL-bearing one only the userinfo is, and masking + # the whole URL would destroy the operator's view rather than protect it. + value = f"https://probeuser:{probe}@host.invalid/p" if landing.kind == "url" else probe + if probe in str(redacted_settings({key: value}).get(key)): + unmasked.append(key) + + assert not unmasked, ( + f"{landing.factory}.{landing.param} ({landing.kind}) lands on {unmasked}, and a credential " + f"placed under {'/'.join(unmasked)} survives redacted_settings. The PARAMETER may already be " + "classified under a DIFFERENT name -- that rename IS the defect this file exists for (BACKLOG " + "#1208). Classify the destination in config/wiring.py (_SECRET_SETTING_KEYS / " + "_is_secret_setting), or declare it in NON_MATERIAL_DESTINATIONS here WITH the reason it " + "carries a name, a path or an identifier rather than material." + ) + + +@pytest.mark.parametrize( + "landing", + [ln for ln in LANDINGS if ln.kind == "url"], + ids=[f"{ln.factory}.{ln.param}" for ln in LANDINGS if ln.kind == "url"], +) +def test_masking_a_url_destination_does_not_destroy_the_operator_view(landing: Landing) -> None: + """THE ASYMMETRY on the URL arm, and it is not decoration. + + A redactor that replaced every URL with ``***`` would satisfy the assertion above while silently + removing the account and host an operator needs to diagnose a connection -- a loss nothing would + report. The control must fail for the userinfo shape and KEEP PASSING for everything beside it. + """ + probe = _next_sentinel() + for key in landing.keys: + if key in NON_MATERIAL_DESTINATIONS: + continue + out = str( + redacted_settings({key: f"https://probeuser:{probe}@host.invalid/p?q=1"}).get(key) + ) + assert "probeuser" in out and "host.invalid/p?q=1" in out, ( + f"{key} lost the user, host or path: {out!r}. Only the secret is supposed to be removed." + ) + plain = str(redacted_settings({key: "https://plain.invalid/path?q=1"}).get(key)) + assert plain == "https://plain.invalid/path?q=1", ( + f"{key} rewrote a URL with no userinfo to {plain!r} -- that mangles ordinary configuration" + ) + + +def test_the_url_suffix_rule_selects_every_parameter_that_lands_in_a_url_setting() -> None: + """The suffix rule's OWN coverage, checked against the running code rather than asserted. + + ``URL_BEARING_SUFFIXES`` is a shape rule, and a shape rule can still miss a parameter. The oracle is + the destination: any parameter whose value lands on a setting the redactor's URL rule covers must + have been selected as URL-bearing, or its userinfo was never checked by anything. + """ + selected = {(ln.factory, ln.param) for ln in LANDINGS if ln.kind == "url"} + missed: list[str] = [] + for mod, name in _factories_returning_a_spec(): + fn = _resolve(mod, name) + if fn is None: # pragma: no cover + continue + for p in _usable_params(fn): + if (name, p.name) in selected or _is_credential_param(p.name): + continue + result = _probe(name, fn, p, "url") + if result.landing is None: + continue # the parameter refuses a URL outright; it cannot carry userinfo + for key in result.landing.keys: + if key.lower().endswith(_URL_DESTINATION_SUFFIXES): + missed.append(f"{name}.{p.name} -> {key}") + print(f"[1208] URL-bearing rule selected {len(selected)} parameters: {sorted(selected)}") + assert not missed, ( + f"these parameters land on a URL-shaped setting but the suffix rule did not select them, so " + f"nothing checked their userinfo: {missed}. Widen URL_BEARING_SUFFIXES." + ) + + +def test_at_least_one_connector_still_refuses_an_inline_credential() -> None: + """The env()-only refusal is a STRONGER control than redaction and it is asserted, not assumed. + + ``Http`` and ``File`` refuse an inline credential outright, so the value never resolves into + settings at all. If this set empties, either the refusals were removed (a real regression) or the + probe stopped detecting them -- and a green would prove neither. + """ + print(f"[1208] parameters that refuse an inline credential: {sorted(REFUSED_INLINE)}") + assert REFUSED_INLINE, ( + "no factory refused an inline credential. Either the env()-only refusals were removed, or the " + "probe stopped detecting them." + ) + + +def test_no_declared_exemption_is_stale() -> None: + """A guard that closes over an enumeration which stopped matching the code is this defect one level + up. Every declared exemption must be REACHED by the probe.""" + reached_keys = {key for ln in LANDINGS for key in ln.keys} + stale_destinations = sorted(set(NON_MATERIAL_DESTINATIONS) - reached_keys) + assert not stale_destinations, ( + f"NON_MATERIAL_DESTINATIONS declares {stale_destinations}, which no probed parameter reaches. " + "Either the factory changed and the entry is dead, or the probe stopped reaching it. Remove " + "the entry only after confirming which." + ) + empty_landings = {(ln.factory, ln.param) for ln in LANDINGS if not ln.keys} + stale_non_emitting = sorted(set(NON_EMITTING) - empty_landings) + assert not stale_non_emitting, ( + f"NON_EMITTING declares {stale_non_emitting}, which now DOES reach a setting. Delete the entry " + "-- the parameter is covered by the mapping assertion again." + ) + for reason in list(NON_MATERIAL_DESTINATIONS.values()) + list(NON_EMITTING.values()): + assert len(reason) > 20, "every exemption needs a real reason, not a placeholder" + + +def test_the_mapping_assertion_goes_red_on_a_renamed_destination() -> None: + """NEGATIVE CONTROL. A gate that has never been red is a claim, not a control. + + ``with_signing`` is rebuilt here in miniature: a factory that takes a classified credential + parameter and emits it under an UNclassified name. The probe must find the destination and the + redactor must be seen failing to mask it -- which is the exact shape of the three closed instances. + """ + sentinel = _next_sentinel() + + def _renaming_factory(*, private_key: str) -> Any: + spec = messagefoundry.Rest(url="https://example.invalid/x") + # A destination the redactor does not classify -- `sign_private_key` before BACKLOG #1106. + spec.settings["vendor_signing_material"] = private_key + return spec + + spec = _renaming_factory(private_key=sentinel) + keys = _find_sentinel(dict(spec.settings), sentinel) + assert keys == ("vendor_signing_material",), keys + # The verdict the parametrised assertion applies, run against this destination. + probe = _next_sentinel() + assert probe in str(redacted_settings({keys[0]: probe}).get(keys[0])), ( + "the control cannot see an unclassified destination, so its green above proves nothing" + ) + # ASYMMETRY: the SAME verdict on a classified destination must stay clean, or the control is just + # failing on everything and cannot tell you which layer does the work. + assert probe not in str(redacted_settings({"sign_private_key": probe}).get("sign_private_key")) diff --git a/tests/test_merge_gate_controls.py b/tests/test_merge_gate_controls.py new file mode 100644 index 00000000..8f7aee5e --- /dev/null +++ b/tests/test_merge_gate_controls.py @@ -0,0 +1,745 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Negative controls for the required merge contexts that had none (BACKLOG #1000). + +A GATE NOBODY HAS WATCHED FAIL IS AN ASSUMPTION WEARING A GREEN TICK. Thirteen contexts are the entire +merge gate, and several of them were guarded only by the property that they exist. This file supplies +the missing plant-and-observe controls; ``tests/negative_controls.toml`` records which control belongs +to which context and ``tests/test_negative_controls.py`` fails when a context has none. + +EVERY CONTROL HERE IS ASYMMETRIC, and that is the part most likely to be skipped. It is not enough that +neutering a rule turns a control red -- the control must fail for exactly the shapes that rule covers +and KEEP PASSING for the shapes some other layer catches, or it cannot tell you which layer does the +work. Measured 2026-08-05 on a different guard: a fix believed to cover two NTFS alternate-data-stream +spellings turned out to be load-bearing for exactly one, and the eight-case control that reddened on +only one of them is what said so. A uniform red would have flattered the code and taught nothing. + +So each control below is paired: a planted violation the gate must see, and a benign neighbour it must +leave alone. Where the gate's own detector is what is being checked, the detector is additionally run +against a synthetic NEUTERED form of the shipped command and observed firing -- otherwise "the shipped +command is clean" is indistinguishable from "the detector matches nothing". + +WHAT IS NOT HERE, said plainly. The scanner gates (bandit, gitleaks, npm-audit, pip-audit, semgrep) run +third-party binaries that this suite does not install, so what is asserted here is the property those +jobs can lose SILENTLY: an enforcement flag removed, a severity floor added, an allowlist widened until +it swallows the class. The scanners' detection itself is exercised inside their own CI jobs -- semgrep +by ``scripts/ci/assert_semgrep_handler_taint.py`` over annotated fixtures, pip-audit's slopsquat half by +``tests/test_new_dependency_check.py`` -- and the registry records which is which rather than letting +the two read as one. +""" + +from __future__ import annotations + +import os +import re +import shutil +import subprocess +import sys +import tomllib +from pathlib import Path +from typing import Any + +import pytest + +from tests._workflow_contexts import ROOT, jobs_of, load_workflow + +_TIMEOUT = 300 + + +# =================================================================================================== +# Child processes. PIN THE CHILD'S ENVIRONMENT -- do not inherit it. +# =================================================================================================== +def _child_env(**extra: str) -> dict[str, str]: + """A minimal, EXPLICIT environment for a child process. + + Measured in wave 2 of this backlog pass: a lane's new test passed in its author's shell and failed + at integration, because that shell happened to export ``PYTHONIOENCODING=utf-8`` while the test + pinned only the PARENT's decoding. It would have passed ubuntu and reddened the Windows legs. So + nothing is inherited here except what a child genuinely cannot run without, and the two variables + that incident turned on are set EXPLICITLY rather than passed through. + + ``LC_ALL=C`` and the ``GIT_CONFIG_*`` overrides exist for the same reason one level over: a global + ``core.autocrlf``, a global hooks path, or a locale that reorders ``grep`` output would otherwise + make the result a fact about this machine. + """ + env = { + "PATH": os.environ.get("PATH", ""), + "LC_ALL": "C", + "LANG": "C", + "PYTHONIOENCODING": "utf-8", + "PYTHONUTF8": "1", + "GIT_AUTHOR_NAME": "negative control", + "GIT_AUTHOR_EMAIL": "control@example.invalid", + "GIT_COMMITTER_NAME": "negative control", + "GIT_COMMITTER_EMAIL": "control@example.invalid", + "GIT_TERMINAL_PROMPT": "0", + } + # Windows: a child python cannot start without these, and they carry no behaviour of their own. + for name in ("SYSTEMROOT", "SystemRoot", "COMSPEC", "TEMP", "TMP", "WINDIR", "PATHEXT"): + if name in os.environ: + env[name] = os.environ[name] + env.update(extra) + return env + + +def _run(argv: list[str], cwd: Path, env: dict[str, str]) -> subprocess.CompletedProcess[bytes]: + """Run a child and return RAW BYTES. + + Decoding is done by the caller with ``errors="replace"``. A child whose output cannot be decoded + under the ambient code page must not be able to turn an assertion about an EXIT CODE into a + ``UnicodeDecodeError`` -- that failure mode is a property of the console, not of the gate. + """ + return subprocess.run( # noqa: S603 # nosec B603 - fixed argv, no shell, test-local paths + argv, cwd=str(cwd), env=env, capture_output=True, timeout=_TIMEOUT, check=False + ) + + +def _text(proc: subprocess.CompletedProcess[bytes]) -> str: + return (proc.stdout + proc.stderr).decode("utf-8", errors="replace") + + +# =================================================================================================== +# `CI gate` -- the roll-up that is the ONLY way six path-gated legs reach branch protection. +# =================================================================================================== +def _ci_gate_job() -> dict[str, Any]: + return jobs_of("ci.yml")["ci-gate"] + + +def _rollup_fail_condition() -> str: + for step in _ci_gate_job().get("steps", []): + if "if" in step and "needs" in str(step.get("if", "")): + return str(step["if"]) + raise AssertionError( + "ci.yml's `ci-gate` job has no step conditioned on `needs.*.result`. The roll-up is the only " + "path by which six path-gated legs reach branch protection; without that condition it reports " + "success unconditionally." + ) + + +def _terminal_states(condition: str) -> set[str]: + """The `needs.*.result` values the roll-up's condition fires on, read off the workflow.""" + return set(re.findall(r"contains\(\s*needs\.\*\.result\s*,\s*'([a-z]+)'\s*\)", condition)) + + +def _rollup_fires(condition: str, results: list[str]) -> bool: + """`contains(needs.*.result, X) || contains(needs.*.result, Y)` over a synthetic results vector.""" + return any(r in _terminal_states(condition) for r in results) + + +def test_the_ci_gate_rollup_fires_on_a_failed_or_cancelled_leg() -> None: + """PLANTED: one gated leg reports `failure`, then `cancelled`. The roll-up must fire on both. + + `CI gate` is required BECAUSE the six legs behind it cannot be: a path-gated job does not report on + a PR that touches none of its paths, which wedges every such PR forever. So the roll-up is the only + thing that turns a red sqlserver-store, postgres-store, load-test, load-test-sqlserver or + windows-service-smoke into a blocked merge. + """ + condition = _rollup_fail_condition() + states = _terminal_states(condition) + assert states == {"failure", "cancelled"}, ( + f"the roll-up fires on {sorted(states)}. `failure` alone lets a CANCELLED leg -- which is what a " + f"timed-out or infrastructure-killed run reports -- pass the merge gate. Condition: {condition!r}" + ) + for planted in ("failure", "cancelled"): + results = ["success", "success", "skipped", "success", "skipped", planted] + assert _rollup_fires(condition, results), ( + f"a gated leg reporting {planted!r} does not fire the roll-up: {condition!r}" + ) + step = next(s for s in _ci_gate_job()["steps"] if s.get("if") == condition) + assert "exit 1" in str(step.get("run", "")), ( + "the roll-up's failing step does not `exit 1`, so the condition fires and the job still " + "reports success" + ) + + +def test_the_ci_gate_rollup_stays_green_when_every_gated_leg_skipped() -> None: + """THE ASYMMETRY, and it is the case that actually happens. + + Almost every PR touches none of the six gated paths, so all six SKIP. A roll-up that failed on + `skipped` would block every ordinary PR -- .github/required-contexts.txt records the run where all + six skipped and `CI gate` still returned success, which is what made requiring it safe. A control + that reddened on everything would have destroyed that property while looking stronger. + """ + condition = _rollup_fail_condition() + assert not _rollup_fires(condition, ["skipped"] * 6) + assert not _rollup_fires(condition, ["success"] * 6) + assert not _rollup_fires(condition, ["success", "skipped", "skipped", "success", "skipped"]) + + +def test_the_rollup_reader_reports_a_condition_that_dropped_cancelled() -> None: + """NEGATIVE CONTROL OF THE CONTROL. Without it, "the shipped condition is fine" and "the reader + matches nothing" are the same green.""" + neutered = "contains(needs.*.result, 'failure')" + assert _terminal_states(neutered) == {"failure"} + assert not _rollup_fires(neutered, ["cancelled"]), ( + "the reader cannot see a dropped terminal state" + ) + assert _rollup_fires(_rollup_fail_condition(), ["cancelled"]), ( + "...and it does see the shipped one, so the assertion above is about the workflow rather than " + "about the reader" + ) + + +def test_the_ci_gate_rollup_still_covers_every_leg_it_is_required_for() -> None: + """A leg dropped from `needs:` leaves branch protection with no path to it at all -- the roll-up + keeps reporting success and the leg's failures stop mattering, silently.""" + needs = set(_ci_gate_job().get("needs", [])) + gated = { + "sqlserver-store", + "postgres-store", + "load-test", + "load-test-sqlserver", + "windows-service-smoke", + } + missing = sorted(gated - needs) + print(f"[#1000] ci-gate needs: {sorted(needs)}") + assert not missing, ( + f"these gated legs are no longer behind the roll-up: {missing}. They cannot be required " + "directly (a path-gated job does not report on a PR that misses its paths), so nothing on the " + "merge path would notice them going red." + ) + + +# =================================================================================================== +# `test (, py3.14)` -- can the test legs go red at all? +# =================================================================================================== +_FAILING_FIXTURE = "def test_planted_failure():\n assert 1 == 2, 'planted'\n" +_PASSING_FIXTURE = "def test_planted_pass():\n assert 1 == 1\n" + + +def _run_pytest_on(fixture: str, tmp_path: Path, **env_extra: str) -> int: + """Run a child pytest over ONE fixture file, outside this repository's rootdir. + + The assertion is on the EXIT CODE only. Exit codes are encoding-independent, which is the point: + the wave-2 incident this guards against turned on a child's stdout ENCODING, and an assertion that + reads the child's text is exactly the assertion that inherits it. + """ + work = tmp_path / f"probe_{abs(hash(fixture)) % 10000}" + work.mkdir() + (work / "test_probe.py").write_text(fixture, encoding="utf-8") + proc = _run( + [sys.executable, "-m", "pytest", "-q", "-p", "no:cacheprovider", str(work)], + cwd=work, + env=_child_env(**env_extra), + ) + print(f"[#1000] child pytest exit={proc.returncode} env_extra={env_extra}") + return proc.returncode + + +def test_a_failing_test_makes_the_pytest_leg_exit_nonzero(tmp_path: Path) -> None: + """PLANTED: a test that cannot pass. The runner must exit non-zero, or the three `test` contexts -- + the largest block of the merge gate -- are decoration. + + This is not hypothetical plumbing. BACKLOG #1000 records the measured case one layer over: running + an emitted command via `pwsh -File script.ps1` returns 0 even when the script inside died at + parameter binding, so every execution assertion built on that return code was vacuously green. It + was found only by writing a control that had to fail and watching it pass. + """ + assert _run_pytest_on(_FAILING_FIXTURE, tmp_path) != 0, ( + "a deliberately failing test did not make pytest exit non-zero" + ) + + +def test_a_passing_fixture_leaves_the_pytest_leg_green(tmp_path: Path) -> None: + """THE ASYMMETRY. A runner that exited non-zero unconditionally would satisfy the control above + while blocking every PR, and nothing would say which of the two it was.""" + assert _run_pytest_on(_PASSING_FIXTURE, tmp_path) == 0 + + +def test_the_pytest_exit_code_does_not_depend_on_the_ambient_encoding(tmp_path: Path) -> None: + """PROVEN UNDER A HOSTILE AMBIENT VALUE, not merely a favourable one. + + A control that only ever ran under `PYTHONIOENCODING=utf-8` proves nothing about the Windows legs, + where the ambient value is whatever the console code page says. Both probes are re-run with the + child pinned to a HOSTILE encoding; the exit codes must be identical, because they are the only + thing asserted. + """ + assert _run_pytest_on(_FAILING_FIXTURE, tmp_path, PYTHONIOENCODING="ascii", PYTHONUTF8="0") != 0 + assert _run_pytest_on(_PASSING_FIXTURE, tmp_path, PYTHONIOENCODING="ascii", PYTHONUTF8="0") == 0 + + +# =================================================================================================== +# `a PR that implements BACKLOG #N must update BACKLOG.md` -- the gate that went green enforcing +# nothing, run as the SHIPPED SHELL against a synthetic repository. +# =================================================================================================== +_HYGIENE_JOB = "banner-on-implementation" + + +def _hygiene_script() -> str: + steps = jobs_of("backlog-hygiene.yml")[_HYGIENE_JOB]["steps"] + script = next(str(s["run"]) for s in steps if "run" in s) + assert "BASE_SHA...$HEAD_SHA" in script or "$BASE_SHA...$HEAD_SHA" in script, ( + "backlog-hygiene.yml's diff is no longer three-dot. The two-dot form reports main-side changes " + "as reverse deltas, which credited every PR with an older base for a docs/BACKLOG.md edit it " + "never made -- the gate went green while enforcing nothing." + ) + return script + + +def _bash_candidates() -> list[Path]: + """Every plausible bash, GIT-DERIVED FIRST. + + ``shutil.which("bash")`` alone is a fact about PATH, and on Windows PATH order decides WHICH + OPERATING SYSTEM answers: ``C:\\Windows\\System32\\bash.exe`` is the WSL launcher, whose filesystem + namespace is not the one this process just wrote a fixture into. Git for Windows always ships bash + beside git, so git -- which every test here already requires -- is the deterministic anchor. + """ + found: list[Path] = [] + git = shutil.which("git") + if git: + # `/cmd/git.exe`, `/bin/git.exe` and `/mingw64/bin/git.exe` are all shipped + # layouts, so walk up and try both bash homes from each level. + for parent in Path(git).resolve().parents: + for rel in ("bin/bash.exe", "usr/bin/bash.exe", "bin/bash"): + found.append(parent / rel) + on_path = shutil.which("bash") + if on_path: + found.append(Path(on_path)) + return found + + +def _bash_sees(bash: Path, tmp_path: Path) -> bool: + """LIVE POSITIVE CONTROL for the namespace, not a guess from the path string. + + Rejecting ``system32`` by name would be a pattern match on a spelling. This writes a token into + the directory the fixture will live in and requires the candidate to read it back -- if it cannot, + it is looking at a different filesystem and every verdict it returns would be about nothing. + """ + probe = tmp_path / "mf_bash_probe.txt" + probe.write_text("MFPROBE-OK\n", encoding="utf-8") + try: + out = _run([str(bash), "-c", "cat mf_bash_probe.txt"], tmp_path, _child_env()) + except OSError: + return False + return out.returncode == 0 and b"MFPROBE-OK" in out.stdout + + +def _require_bash(tmp_path: Path) -> str: + """A bash that can see this process's files, or a loud failure -- never a skip. + + ci.yml sets ``defaults.run.shell: bash`` on every OS, so a leg without a usable bash could not run + the gate this control exercises, and a skip there would be a green that proves nothing. + """ + tried: list[str] = [] + for candidate in _bash_candidates(): + if not candidate.is_file(): + continue + tried.append(str(candidate)) + if _bash_sees(candidate, tmp_path): + print(f"[#1000] bash resolved to {candidate} (namespace probe passed)") + return str(candidate) + pytest.fail( + "no bash on this machine can read a file this process just wrote. Tried: " + f"{tried or '(none found)'}. On Windows, `bash` on PATH is often " + "C:\\Windows\\System32\\bash.exe -- the WSL launcher, which runs in a different filesystem " + "namespace, and a control that ran there would be measuring nothing." + ) + + +def _fixture_repo(tmp_path: Path, env: dict[str, str]) -> tuple[Path, str, str, str]: + """A repository shaped like the PR the gate exists to police. + + ``A`` is the merge base. ``B`` is the PR head: it changes engine code and NOTHING else. ``C`` is + main moving on AFTER the branch point, touching only ``docs/BACKLOG.md`` -- the shape the archive + move produced in bulk, and the shape the two-dot diff mis-credited. + """ + repo = tmp_path / "fixture" + (repo / "messagefoundry").mkdir(parents=True) + (repo / "docs").mkdir() + _run(["git", "init", "-b", "main", "."], repo, env) + (repo / "messagefoundry" / "engine.py").write_text("x = 1\n", encoding="utf-8") + (repo / "docs" / "BACKLOG.md").write_text("# ledger\n", encoding="utf-8") + _run(["git", "add", "."], repo, env) + _run(["git", "commit", "-m", "A"], repo, env) + base_a = _text(_run(["git", "rev-parse", "HEAD"], repo, env)).strip() + + _run(["git", "checkout", "-b", "pr"], repo, env) + (repo / "messagefoundry" / "engine.py").write_text("x = 2\n", encoding="utf-8") + _run(["git", "commit", "-am", "B: engine only"], repo, env) + head_b = _text(_run(["git", "rev-parse", "HEAD"], repo, env)).strip() + + _run(["git", "checkout", "main"], repo, env) + (repo / "docs" / "BACKLOG.md").write_text("# ledger\n\nmain moved on\n", encoding="utf-8") + _run(["git", "commit", "-am", "C: main-side ledger edit"], repo, env) + base_c = _text(_run(["git", "rev-parse", "HEAD"], repo, env)).strip() + _run(["git", "checkout", "pr"], repo, env) + assert base_a and head_b and base_c and len({base_a, head_b, base_c}) == 3 + return repo, base_c, head_b, base_a + + +def _run_hygiene( + bash: str, + script: str, + repo: Path, + env: dict[str, str], + *, + title: str, + body: str, + base: str, + head: str, +) -> tuple[int, str]: + """Run the workflow's own script, and VALIDATE THE SHAPE OF THE RESULT before returning it. + + The script path is passed RELATIVE to ``cwd``. An absolute Windows path is not portable across + bash builds -- backslashes are escape characters and a drive letter means nothing outside the + Windows namespace -- and the mangling presents as "no such file", i.e. as exit 127, which a caller + comparing `code != 0` would happily read as "the gate refused this PR". + + So 126/127 are a hard failure here rather than a verdict. That is the generalisable half of the + probe defect recorded in BACKLOG #1000: a probe must validate its own output rather than treating + a broken invocation as an answer. + """ + (repo / "gate.sh").write_text(script, encoding="utf-8", newline="\n") + proc = _run( + [bash, "gate.sh"], + repo, + {**env, "PR_TITLE": title, "PR_BODY": body, "BASE_SHA": base, "HEAD_SHA": head}, + ) + out = _text(proc) + assert proc.returncode not in (126, 127), ( + f"bash could not execute the gate script (exit {proc.returncode}): {out.strip()[:300]}. That " + "is not a gate verdict -- it is a broken invocation, and reading it as one would make every " + "assertion here vacuous." + ) + # Printed ASCII-safe: the gate's own error text carries a status glyph, and a Windows console + # under cp1252 would turn printing it into a UnicodeEncodeError -- an assertion about the gate + # lost to a property of the terminal. + return proc.returncode, out + + +@pytest.fixture +def hygiene(tmp_path: Path) -> tuple[str, str, Path, dict[str, str], str, str]: + env = _child_env( + HOME=str(tmp_path), + GIT_CONFIG_GLOBAL=str(tmp_path / "gitconfig"), + GIT_CONFIG_SYSTEM=str(tmp_path / "gitconfig"), + ) + (tmp_path / "gitconfig").write_text("", encoding="utf-8") + bash = _require_bash(tmp_path) + repo, base_c, head_b, _base_a = _fixture_repo(tmp_path, env) + return bash, _hygiene_script(), repo, env, base_c, head_b + + +def _ascii(text: str) -> str: + """Child output, safe to print under any console code page.""" + return text.encode("ascii", "backslashreplace").decode("ascii") + + +def test_the_bash_namespace_probe_rejects_an_interpreter_that_cannot_see_the_fixture( + tmp_path: Path, +) -> None: + """NEGATIVE CONTROL OF THE RESOLVER, and it is here because this file already shipped the bug once. + + MEASURED 2026-08-10. The first version resolved bash with ``shutil.which("bash")`` and passed it an + ABSOLUTE Windows path. Under a Git Bash parent it passed; run from PowerShell, where PATH resolves + ``bash`` to ``C:\\Windows\\System32\\bash.exe`` -- the WSL launcher, a different filesystem + namespace -- every hygiene control failed with exit 127 and the backslashes eaten. So the control's + verdict was a fact about PATH ORDER, which is precisely the ambient-environment green the wave-2 + incident warned about, one variable over. + + Two things fixed it and both are asserted here rather than described: the candidate is derived from + ``git`` (which ships bash beside it) and then made to READ A FILE this process wrote, and the + script is invoked by a RELATIVE path so no namespace conversion is involved at all. + + A candidate that cannot read the token must be rejected. ``sys.executable`` stands in for one: it + is a real, runnable program that is not a shell, so the probe must refuse it while accepting the + resolved bash in the same call. + """ + assert not _bash_sees(Path(sys.executable), tmp_path), ( + "the namespace probe accepted a non-shell interpreter, so it cannot reject a bash that is " + "looking at the wrong filesystem either" + ) + assert _bash_sees(Path(_require_bash(tmp_path)), tmp_path) + + +def test_the_backlog_hygiene_gate_fails_a_code_pr_that_leaves_the_ledger_alone( + hygiene: tuple[str, str, Path, dict[str, str], str, str], +) -> None: + """PLANTED: a PR that claims `BACKLOG #42`, changes engine code, and never touches the ledger -- + while main has separately moved docs/BACKLOG.md since the branch point. + + That last clause is the whole point. The gate computed its changed-file list with a two-dot + `git diff "$BASE_SHA" "$HEAD_SHA"`, which reports main-side changes as REVERSE deltas, so any + main-side edit to docs/BACKLOG.md credited every PR with an older base. It went green while + enforcing nothing, on exactly the population it exists to police. + """ + bash, script, repo, env, base, head = hygiene + code, out = _run_hygiene( + bash, + script, + repo, + env, + title="feat: something (BACKLOG #42)", + body="", + base=base, + head=head, + ) + print(f"[#1000] shipped three-dot gate exit={code}\n{_ascii(out)}") + assert code == 1, f"the gate passed a PR it exists to fail. exit={code}\n{_ascii(out)}" + assert "docs/BACKLOG.md" in out + + +def test_the_two_dot_form_of_the_gate_passes_the_same_planted_pull_request( + hygiene: tuple[str, str, Path, dict[str, str], str, str], +) -> None: + """RUN AGAINST THE PRE-FIX GATE, which is what makes the control above evidence rather than a + claim. The identical fixture, with only the diff form reverted, must go GREEN. + + If both forms failed, the fixture would be proving something else -- and the recorded defect would + be unreproduced. + """ + bash, script, repo, env, base, head = hygiene + pre_fix = script.replace('"$BASE_SHA...$HEAD_SHA"', '"$BASE_SHA" "$HEAD_SHA"') + assert pre_fix != script, ( + "the two-dot substitution matched nothing, so this test compares the shipped gate with itself" + ) + code, out = _run_hygiene( + bash, + pre_fix, + repo, + env, + title="feat: something (BACKLOG #42)", + body="", + base=base, + head=head, + ) + print(f"[#1000] pre-fix two-dot gate exit={code}\n{_ascii(out)}") + assert code == 0, ( + "the two-dot form no longer reproduces the recorded defect, so the three-dot assertion above " + f"is not measuring what it says. exit={code}\n{_ascii(out)}" + ) + + +@pytest.mark.parametrize( + ("case", "title", "body", "touch"), + [ + ("no claim at all", "chore: tidy", "", None), + ("claims and updates the ledger", "feat (BACKLOG #42)", "", "docs/BACKLOG.md"), + ( + "claims and updates an ARCHIVED item", + "feat (BACKLOG #42)", + "", + "docs/archive/backlog/x.md", + ), + ("a bare #42 is a PR number, not a claim", "fix for #42", "see #42", None), + ], +) +def test_the_backlog_hygiene_gate_leaves_the_benign_shapes_alone( + hygiene: tuple[str, str, Path, dict[str, str], str, str], + case: str, + title: str, + body: str, + touch: str | None, +) -> None: + """THE ASYMMETRY, four shapes wide. + + A gate that failed everything would satisfy the planted case and block every PR in the repo. Each + row is a shape the rule deliberately does NOT break: no claim, a claim honoured in the live ledger, + a claim honoured in the ARCHIVE (an item retired between the claim and the PR), and the `#42` + spelling that is a PR number in this repo rather than an item. + """ + bash, script, repo, env, base, head = hygiene + if touch: + target = repo / touch + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("banner\n", encoding="utf-8") + _run(["git", "add", "-A"], repo, env) + _run(["git", "commit", "-m", f"branch-side {touch}"], repo, env) + head = _text(_run(["git", "rev-parse", "HEAD"], repo, env)).strip() + code, out = _run_hygiene(bash, script, repo, env, title=title, body=body, base=base, head=head) + print(f"[#1000] benign case {case!r} exit={code}") + assert code == 0, f"the gate failed a benign PR shape ({case}). exit={code}\n{_ascii(out)}" + + +# =================================================================================================== +# `gitleaks (secret scan)` -- the allowlist is the neutering path a scope test cannot see. +# =================================================================================================== +_GITLEAKS = ROOT / ".gitleaks.toml" + + +def _gitleaks_allowlist_regexes() -> list[str]: + parsed = tomllib.loads(_GITLEAKS.read_text(encoding="utf-8")) + return [str(r) for r in (parsed.get("allowlist") or {}).get("regexes", [])] + + +def _fabricated_secrets() -> list[str]: + """Credential-shaped strings ASSEMBLED AT RUNTIME. + + Never committed as literals: gitleaks scans this repository and cannot tell a well-known test + vector from a live credential -- the sibling redaction suite already had a fixture rejected for + exactly that, correctly. Assembling from parts keeps the scanner useful on this file. + """ + hexish = "0123456789abcdef" * 3 + return [ + "AKIA" + "Q" * 16, + "ghp_" + "z" * 36, + "xoxb-" + "1" * 12 + "-" + "2" * 24, + "-----BEGIN " + "RSA PRIVATE KEY-----", + hexish[:40], + "postgres://svc:" + "P" * 24 + "@db.invalid:5432/x", + ] + + +def _swallowing(regexes: list[str], corpus: list[str]) -> list[str]: + return [r for r in regexes if any(re.search(r, s) for s in corpus)] + + +def test_no_gitleaks_allowlist_regex_swallows_a_fabricated_secret() -> None: + """PLANTED: six credential shapes gitleaks' default ruleset exists to catch. + + ``.gitleaks.toml`` is the one place this required context can be disabled without touching a + workflow: an allowlist entry broad enough to match a real credential turns the gate green while it + keeps scanning. Every shipped entry is deliberately a LITERAL or a tightly bounded pattern, and + this is the assertion that keeps it that way. + """ + regexes = _gitleaks_allowlist_regexes() + print( + f"[#1000] scanned {len(regexes)} gitleaks allowlist regexes against " + f"{len(_fabricated_secrets())} fabricated credential shapes" + ) + assert regexes, ( + ".gitleaks.toml parsed to ZERO allowlist regexes -- the format changed under this" + ) + swallowed = _swallowing(regexes, _fabricated_secrets()) + assert not swallowed, ( + f"these allowlist regexes match a fabricated credential: {swallowed}. NEVER allowlist a real " + "secret; a broad entry here neuters a required context with no workflow edit at all." + ) + + +def test_the_allowlist_narrowness_detector_fires_on_a_planted_broad_regex() -> None: + """NEGATIVE CONTROL OF THE CONTROL, and the ASYMMETRY. + + "The shipped allowlist is narrow" and "the detector matches nothing" are the same green. Two broad + patterns are planted and must be caught; the shipped entries must stay clean in the same call, so + the control is not merely demanding an empty allowlist -- which would delete a legitimate, + documented set of non-secret fixtures. + """ + planted = [r".{8,}", r"[A-Za-z0-9_/+-]{20,}"] + assert _swallowing(planted, _fabricated_secrets()) == planted, ( + "the detector cannot see a broad regex" + ) + assert not _swallowing(_gitleaks_allowlist_regexes(), _fabricated_secrets()) + + +def test_the_gitleaks_config_still_extends_the_default_ruleset() -> None: + """`useDefault = false` empties the ruleset and the job then scans for the project's own rules + only -- of which there are none. It reports success in seconds.""" + parsed = tomllib.loads(_GITLEAKS.read_text(encoding="utf-8")) + assert parsed.get("extend", {}).get("useDefault") is True, ( + ".gitleaks.toml no longer extends the default ruleset; the secret scan has nothing to match" + ) + + +# =================================================================================================== +# `bandit (Python SAST)` and `npm-audit` -- a severity floor mutes a gate without touching its scope. +# =================================================================================================== +def _step_run(workflow: str, job: str, name_fragment: str) -> str: + for step in jobs_of(workflow)[job].get("steps", []): + if name_fragment in str(step.get("name", "")): + return str(step.get("run", "")) + raise AssertionError(f"{workflow}:{job} has no step named like {name_fragment!r}") + + +#: Flags that keep a scanner running while discarding part of what it finds. NOT the `|| true` family +#: (tests/test_security_posture.py owns that) -- these leave the exit code intact and shrink the input +#: to it, which a neutering scan looking for added idioms cannot see. +_MUTING_FLAGS = { + "bandit": ( + r"(? list[str]: + body = "\n".join(line for line in command.splitlines() if not line.lstrip().startswith("#")) + return [p for p in _MUTING_FLAGS[family] if re.search(p, body)] + + +def test_the_bandit_invocation_carries_no_severity_or_confidence_floor() -> None: + """PLANTED via the detector: `bandit -ll` reports only MEDIUM and above and still exits non-zero + on what is left, so the job stays green-looking and blocking while it stops reporting a whole + severity band. The scope test next door asks WHAT it is pointed at; this asks what it keeps.""" + command = _step_run("security.yml", "bandit", "Scan source for insecure patterns") + found = _muted(command, "bandit") + print(f"[#1000] bandit invocation scanned for {len(_MUTING_FLAGS['bandit'])} muting flags") + assert not found, ( + f"the bandit invocation carries {found}, which discards findings while the required context " + f"keeps reporting success. Command: {command!r}" + ) + + +def test_the_npm_audit_invocation_carries_no_severity_floor() -> None: + """`npm audit --audit-level=high` exits 0 on moderate advisories. security.yml's own comment says + the default level "fails on ANY severity, matching pip-audit's strict posture" -- this is the + assertion that keeps that sentence true.""" + command = _step_run("security.yml", "npm-audit", "Audit the locked npm dependencies") + found = _muted(command, "npm") + assert not found, f"the npm audit invocation carries {found}: {command!r}" + + +def test_the_muting_detector_fires_on_a_synthetic_floor() -> None: + """NEGATIVE CONTROL OF THE CONTROL, plus the ASYMMETRY that matters here: the detector must NOT + fire on the reviewed `--skip B101,...` list, which is a per-check exclusion with a stated reason + for each entry -- not a severity floor. A detector that flagged it would be "fixed" by deleting a + correct annotation.""" + assert _muted("bandit -r . -ll --skip B101", "bandit") == [r"(? None: + """`npm audit --package-lock-only` reads the committed lockfile. Without it the job errors or + audits an empty tree, and `working-directory: ide` is the only thing pointing it at one.""" + workflow = load_workflow("security.yml") + job = workflow["jobs"]["npm-audit"] + workdir = str(((job.get("defaults") or {}).get("run") or {}).get("working-directory", "")) + assert workdir, "the npm-audit job lost its working-directory; it would audit the repo root" + lock = ROOT / workdir / "package-lock.json" + assert lock.is_file(), f"{lock} does not exist, so --package-lock-only has nothing to audit" + + +# =================================================================================================== +# `cla` -- the context string IS the control surface, and it has been wrong in this repo before. +# =================================================================================================== +def test_the_cla_job_still_reports_under_the_job_key_and_not_the_workflow_name() -> None: + """PLANTED historically, not synthetically: docs/CI.md and cla.yml both told a reader to require + "CLA Assistant" -- the WORKFLOW name, which matches no status check. Adding it to branch + protection would have wedged every PR forever. + + The detection this gate performs lives entirely in a third-party action, so what is controllable + here is whether the context can report at all: the job must declare no `name:` (making the context + its KEY, `cla`) and must run on a pull-request trigger. + """ + jobs = jobs_of("cla.yml") + assert "cla" in jobs, f"cla.yml's job key is no longer `cla`: {sorted(jobs)}" + assert "name" not in jobs["cla"], ( + "the cla job declared a `name:`, which changes its status-check context string. Branch " + "protection still requires `cla`, so the context would never report and every PR would wedge." + ) + workflow = load_workflow("cla.yml") + triggers = workflow.get("on") or workflow.get(True) or {} + assert "pull_request_target" in triggers or "pull_request" in triggers, ( + f"cla.yml has no pull-request trigger ({sorted(triggers)}), so the required `cla` context can " + "never report -- the required-but-absent trap" + ) + + +def test_the_cla_allowlist_is_an_enumeration_and_not_a_glob() -> None: + """THE ASYMMETRY, and a recorded finding rather than a hypothetical: a `bot*` glob would let any + human whose username begins with "bot" skip signing (review low-28). The allowlist is legitimate + and must keep working -- so this asserts its SHAPE, not its absence.""" + step = next(s for s in jobs_of("cla.yml")["cla"]["steps"] if "with" in s) + allowlist = str(step["with"]["allowlist"]) + assert allowlist.strip(), ( + "the CLA allowlist emptied; every maintainer push would need a signature" + ) + assert "*" not in allowlist, f"the CLA allowlist contains a glob: {allowlist!r}" diff --git a/tests/test_negative_controls.py b/tests/test_negative_controls.py new file mode 100644 index 00000000..34f49f46 --- /dev/null +++ b/tests/test_negative_controls.py @@ -0,0 +1,163 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Every required merge context must have a negative control (BACKLOG #1000). + +THE DEFECT THIS EXISTS FOR. Thirteen contexts are the entire merge gate and, before this, not one of +them was proven able to go red. The class has fired at least four times here with no CI signal, each +found by hand: a required backlog gate computing a two-dot diff and crediting every PR with an older +base; a required SAST gate scanning a two-directory allow-list; a leak gate exiting 0 on content +carrying a real site code; the same gate matching one of four spellings of a Windows path. Each was +filed as its own defect, which is right. None of them established the property that would have caught +all four -- a green run is evidence only if the gate has been shown it can go red on that class. + +THIS IS THE CI JOB THE ITEM ASKS FOR, and it deliberately is NOT a new workflow. Running the +reconciliation inside the ``test`` legs makes it BLOCKING today through contexts that are already +required, so it adds no new required context and needs no branch-protection change. A new advisory +workflow would have been weaker (advisory jobs do not stop auto-merge) and a new required one is an +owner decision that also has a live incident history: on 2026-07-29 protection was briefly cut to a +single required context with auto-merge armed and zero approvals, and PRs merged in that window. + +THE INSTRUMENT HAS TO ANSWER THE QUESTION ASKED OF IT, and "it runs in CI" is not the same sentence as +"it runs on the pull request shape it exists for". ``ci.yml`` gates the ``test`` legs' expensive steps +on ``needs.changes.outputs.code == 'true'``, and that coupling is exactly how a #320 banner merged +without the suite ever compiling. The decay mode this registry targets is a context arriving in branch +protection and being mirrored into ``.github/required-contexts.txt`` -- so both that file and this +registry must classify as CODE, not docs. Measured, and then pinned: they are now two rows in +``tests/test_ci_docs_only_detector.py::test_code_paths_run_the_suite`` rather than a sentence here. + +``.github/required-contexts.txt`` IS READ-ONLY TO THIS FILE. It mirrors the live server, and its own +header states the ordering rule -- branch protection first, then the file. Everything here only reads +it, so the registry can never be the reason a context looks required when it is not. + +NOT VACUOUS BY CONSTRUCTION, which is the failure mode of every coverage register. The reconciliation +is driven by the LIVE required set rather than by the registry, so a context added to branch protection +and mirrored into that file arrives here with zero controls and fails -- rather than simply not being +looked at. ``test_the_reconciliation_fails_when_a_context_loses_its_control`` proves the gate can say so +by removing a control and watching it. +""" + +from __future__ import annotations + +from tests import _negative_controls as reg +from tests._workflow_contexts import required_contexts + + +def test_the_registry_covers_every_required_context() -> None: + """THE GATE. It reports WHAT it scanned, not merely a count -- "no gaps" and "nothing was read" + are otherwise the same green, which is the shape of half the defects this registry indexes.""" + problems, coverage = reg.reconcile() + print(f"[#1000] reconciling {len(coverage)} required contexts against {reg.REGISTRY.name}") + for ctx in sorted(coverage): + print(f"[#1000] {coverage[ctx]} control(s): {ctx}") + assert coverage, ( + ".github/required-contexts.txt parsed to ZERO contexts, so this reconciliation compared " + "nothing. That is a broken check, not a clean sweep." + ) + assert not problems, "negative-control registry problems:\n " + "\n ".join(problems) + + +def test_every_required_context_is_covered_by_a_context_specific_control() -> None: + """A universal control (`no required job carries continue-on-error`) is real and valuable, and it + is NOT what this item asks for. It proves a gate cannot be switched off; it says nothing about + whether the gate can see the violation it exists for. Each context needs at least one control + planted at its own subject matter, which is what the per-context registry entries are.""" + _, coverage = reg.reconcile() + uncovered = sorted(ctx for ctx, n in coverage.items() if n < 1) + assert not uncovered, f"contexts with no control of their own: {uncovered}" + + +def test_the_registered_control_count_is_reported_and_has_not_collapsed() -> None: + """A liveness floor. The registry shrinking is how this decays -- not by anyone deciding a gate no + longer needs proving, but by a control being deleted alongside the test it names.""" + controls = reg.load() + red = sum(len(c.red) for c in controls) + green = sum(len(c.green) for c in controls) + print( + f"[#1000] {len(controls)} controls: {red} planted-violation nodes, {green} asymmetry nodes" + ) + assert len(controls) >= len(required_contexts()) + assert red >= 20 and green >= 15, ( + f"the registry collapsed to {red} red / {green} green nodes. Raise this floor when it " + "legitimately grows; never lower it to make the suite pass." + ) + + +# --- The gate's own negative controls. A gate that has never been red is a claim. ------------------- + + +def _problems_for(controls: list[reg.Control]) -> list[str]: + """Run the same rules over a synthetic control list, without touching the file on disk.""" + coverage = dict.fromkeys(required_contexts(), 0) + problems: list[str] = [] + for control in controls: + if control.context in coverage: + coverage[control.context] += 1 + problems.extend( + f"required context {ctx!r} has NO negative control" for ctx, n in coverage.items() if n == 0 + ) + problems.extend(f"dangling control node: {m}" for m in reg.unresolved_nodes(controls)) + return problems + + +def test_the_reconciliation_fails_when_a_context_loses_its_control() -> None: + """PLANTED: the `gitleaks (secret scan)` entry is dropped. The gate must name that context. + + Without this, "every context is covered" and "the reconciliation compares nothing" produce the + same green -- which is the defect this whole registry exists to make visible, occurring inside the + verification of its own fix. + """ + controls = [c for c in reg.load() if c.context != "gitleaks (secret scan)"] + problems = _problems_for(controls) + assert any("gitleaks (secret scan)" in p for p in problems), problems + print(f"[#1000] negative control: dropping one entry produced {len(problems)} problem(s)") + + +def test_the_reconciliation_stays_green_on_the_shipped_registry() -> None: + """THE ASYMMETRY. A reconciliation that reported a problem for everything would satisfy the test + above while being useless, and the two are indistinguishable from a red alone.""" + assert _problems_for(reg.load()) == [] + + +def test_the_node_resolver_reports_a_test_that_does_not_exist() -> None: + """PLANTED: a control naming a test function nobody wrote, and one naming a missing file. + + A registry of dangling node ids satisfies every count-based assertion above. Both spellings of the + failure are planted because they take different branches of the resolver, and a resolver that only + caught the missing FILE would pass a renamed TEST -- which is the likelier accident. + """ + ghost = reg.Control( + context="CI gate", + plants="x" * 50, + holds="x" * 50, + observed="x" * 50, + red=("tests/test_merge_gate_controls.py::test_this_function_does_not_exist",), + green=("tests/test_no_such_file_at_all.py::test_whatever",), + ci=None, + workflow=None, + ) + missing = reg.unresolved_nodes([ghost]) + assert len(missing) == 2, missing + assert any("no test function of that name" in m for m in missing), missing + assert any("no such file" in m for m in missing), missing + # ...and it stays silent on the real ones, so the assertion above is about the ghost. + assert reg.unresolved_nodes(reg.load()) == [] + + +def test_the_ci_wiring_check_reports_a_command_nobody_invokes() -> None: + """PLANTED: a `ci` control naming a command that appears in no step of the workflow it claims. + + This is the quiet way a fixture-based control dies: the asserter script stays in the tree, the + registry keeps pointing at it, and the step that ran it is deleted. Nothing else here would notice. + """ + orphan = reg.Control( + context="semgrep (project SAST rules)", + plants="x" * 50, + holds="x" * 50, + observed="x" * 50, + red=(), + green=(), + ci="python scripts/ci/this_asserter_is_not_wired.py", + workflow="security.yml", + ) + assert reg.unwired_ci_commands([orphan]), "the wiring check cannot see an uninvoked command" + assert reg.unwired_ci_commands(reg.load()) == [], "a shipped ci control is not actually wired" diff --git a/tests/test_setup_leak_gate_reports_source.py b/tests/test_setup_leak_gate_reports_source.py new file mode 100644 index 00000000..1a0a3745 --- /dev/null +++ b/tests/test_setup_leak_gate_reports_source.py @@ -0,0 +1,335 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The verify step must name the source it LOADED, not the file it installed (BACKLOG #1080). + +``setup-leak-gate.ps1 -Synthetic`` printed *"Installed the SYNTHETIC template"* and, three lines +later, *"CONFIGURED with the real token set."* Both were individually true whenever +``MEFOR_FORBIDDEN_TOKENS`` was set -- the script wrote the synthetic file, and the scanner loaded the +real list from the environment, which wins over the file -- but nothing said the environment had +OVERRIDDEN what had just been written. The pair reads as a contradiction or, worse, as confirmation +that a synthetic install produced a real-token gate. + +The script's own header is what makes that a defect rather than a nit: *"A green gate is evidence +only if you confirmed it can see the class it is meant to catch"*. Printing the detector counts +without printing WHERE they came from leaves exactly the ambiguity the step exists to close. + +**These tests pin a duplicated rule against its definition.** Precedence lives in +``scan_forbidden._resolve_token_text``; the script necessarily re-expresses it in order to name the +source, and a second expression drifts silently. So every case here asserts the script's claim about +the source AND the scanner's own ``loaded names=`` line from the SAME run, and the two-source case +asserts those lines DIFFER -- a fixture whose two sources were indistinguishable would pass while +measuring nothing. + +**The token fixtures are DERIVED from the shipped example, never hand-written.** Two reasons, both +load-bearing. Every floor section must be non-empty or the scanner refuses (see +``token_floor_failure``), and one of those sections is a numeric site prefix -- a value this repo's +own gate scans tracked files for. Deriving keeps the numeric prefix out of this file entirely and +keeps the fixture valid if the example's sections are ever renamed. + +**Assertions are ASCII-only, deliberately.** The output crosses two encoding boundaries (python's +stderr into pwsh, pwsh's stdout into pytest) and the em dash in the scanner's mode line arrives +mojibake on a stock Windows code page. Asserting on it would be testing the code pages. +""" + +from __future__ import annotations + +import os +import shutil +import subprocess +import sys +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] +_SETUP = _ROOT / "scripts" / "dev" / "setup-leak-gate.ps1" +_SECURITY = _ROOT / "scripts" / "security" +_EXAMPLE = _SECURITY / "scan-tokens.local.txt.example" +_TIMEOUT = 55 # under pyproject's per-test --timeout=60 + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None, reason="setup-leak-gate.ps1 needs pwsh (PowerShell 7)" +) + +#: Not echoed anywhere by a correct implementation -- see the inline-content test. +_INLINE_MARKER = "ZzInlineOnlyMarkerZz" +_EXTERNAL_MARKER = "ZzExternalCorpZz" + + +def _derived_tokens(marker: str) -> str: + """The shipped synthetic example plus one extra name detector. + + Real-SHAPED without being the example: ``is_synthetic_token_set()`` compares the parsed name + patterns, so one extra entry is enough for the scanner to stop labelling it SYNTHETIC, while the + detector counts move by exactly one -- which is what makes "which source loaded" observable. + """ + return ( + _EXAMPLE.read_text(encoding="utf-8") + + f"\n[names]\n\\b{marker}\\b | synthetic external token | i\n" + ) + + +def _git(*args: str, cwd: Path) -> None: + subprocess.run(["git", *args], cwd=str(cwd), check=True, capture_output=True) + + +def _checkout(path: Path) -> Path: + """A minimal checkout carrying the script and the three files it reaches for. + + Only those files are copied, NOT the whole of scripts/security: a maintainer running this suite + has the real ``scan-tokens.local.txt`` sitting in that directory, and a copytree would sweep the + private token list into a temp dir -- the one mistake the script itself refuses to make. + """ + (path / "scripts" / "dev").mkdir(parents=True) + (path / "scripts" / "security").mkdir(parents=True) + shutil.copy2(_SETUP, path / "scripts" / "dev" / "setup-leak-gate.ps1") + for name in ("scan_forbidden.py", "scan-allowlist.txt", "scan-tokens.local.txt.example"): + shutil.copy2(_SECURITY / name, path / "scripts" / "security" / name) + # check-ignore reads the working-tree .gitignore, so no commit is needed -- but the repo must + # exist, or the script's own not-git-ignored guard deletes what it wrote and throws. + (path / ".gitignore").write_text("scripts/security/scan-tokens.local.txt\n", encoding="utf-8") + _git("init", "-b", "main", ".", cwd=path) + return path + + +def _python_shim(path: Path) -> Path: + """A directory holding a ``python`` that forwards to THIS interpreter. + + The script resolves ``/.venv/Scripts/python.exe`` or falls back to bare ``python`` on PATH. + A temp checkout has no venv, so the verify step would otherwise depend on whichever ``python`` the + runner happens to expose -- which is why the neighbouring anchoring test declines to assert the + verify step at all. The shim makes the interpreter an INPUT of the test rather than an accident of + the leg, so the emitted strings and the exit code are assertable wherever pwsh runs. + """ + path.mkdir(parents=True, exist_ok=True) + if os.name == "nt": + (path / "python.cmd").write_text( + f'@echo off\r\n"{sys.executable}" %*\r\nexit /b %ERRORLEVEL%\r\n', encoding="ascii" + ) + else: + shim = path / "python" + shim.write_text(f'#!/bin/sh\nexec "{sys.executable}" "$@"\n', encoding="ascii") + shim.chmod(0o755) + return path + + +def _env(shim: Path, **over: str) -> dict[str, str]: + """The child's environment, PINNED -- never merely inherited. + + Every variable that steers the scanner is removed first, then set explicitly by the caller. A + maintainer with ``MEFOR_FORBIDDEN_TOKENS`` exported would otherwise run these tests against their + own real list and get greens that mean something else entirely. ``PYTHONIOENCODING`` / ``PYTHONUTF8`` + go too: they change the grandchild's stderr encoding, so leaving them ambient makes the result + depend on the developer's shell rather than on the code -- measured in this repo, on a test that + passed locally and reddened at integration for exactly that. + """ + env = dict(os.environ) + for var in ( + "MEFOR_FORBIDDEN_TOKENS", + "MEFOR_REQUIRE_TOKENS", + "MEFOR_MIN_DETECTORS", + "PYTHONIOENCODING", + "PYTHONUTF8", + ): + env.pop(var, None) + env["PATH"] = str(shim) + os.pathsep + env.get("PATH", "") + env.update(over) + return env + + +def _run(root: Path, *args: str, env: dict[str, str]) -> subprocess.CompletedProcess[str]: + return subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(root / "scripts" / "dev" / "setup-leak-gate.ps1"), + *args, + ], + cwd=str(root), + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=_TIMEOUT, + env=env, + ) + + +def _loaded(proc: subprocess.CompletedProcess[str]) -> str: + """The scanner's own detector-count line, as echoed by the script.""" + line = next((ln for ln in proc.stdout.splitlines() if "loaded names=" in ln), "") + assert line, f"no detector-count line in the output:\n{proc.stdout}\n{proc.stderr}" + return line.strip() + + +def _source_line(proc: subprocess.CompletedProcess[str]) -> str: + """The line that names the RESOLVED source, isolated from the rest of the run. + + Extracted rather than searched for as a substring of the whole output, and that is not fussiness. + ``-Synthetic`` already prints ``Installed the SYNTHETIC template -> scripts/security/ + scan-tokens.local.txt``, so a whole-output search for that path passes with the defect fully + present -- measured: the first draft of the file-source test below was green before the fix, for + exactly that reason. The claim under test is about the line that says what was LOADED. + """ + line = next((ln for ln in proc.stdout.splitlines() if "token source" in ln.lower()), "") + assert line, f"no line naming the resolved token source:\n{proc.stdout}\n{proc.stderr}" + return line.strip() + + +@pytest.fixture +def rig(tmp_path: Path) -> tuple[Path, Path, Path]: + """``(checkout, shim dir, an external token file)``.""" + root = _checkout(tmp_path / "repo") + shim = _python_shim(tmp_path / "shim") + ext = tmp_path / "external-tokens.txt" + ext.write_text(_derived_tokens(_EXTERNAL_MARKER), encoding="utf-8") + return root, shim, ext + + +# -------------------------------------------------------------------------------------------------- +# the source is named -- and the negative control that it is not named unconditionally +# -------------------------------------------------------------------------------------------------- + + +def test_the_installed_file_is_named_as_the_source_when_nothing_overrides_it( + rig: tuple[Path, Path, Path], +) -> None: + root, shim, _ = rig + proc = _run(root, "-Synthetic", env=_env(shim)) + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "scripts/security/scan-tokens.local.txt" in _source_line(proc) + assert "MEFOR_FORBIDDEN_TOKENS" not in _source_line(proc) + assert "SYNTHETIC" in _loaded(proc), "the scanner did not load the file that was just installed" + # THE NEGATIVE CONTROL. An override banner printed unconditionally would satisfy the override test + # below while telling the operator something false here. + assert "OVERRIDDEN" not in proc.stdout, proc.stdout + + +def test_an_env_file_override_is_named_and_announced(rig: tuple[Path, Path, Path]) -> None: + """The exact shape #1080 was filed for: install the synthetic file, load the environment's list.""" + root, shim, ext = rig + proc = _run(root, "-Synthetic", env=_env(shim, MEFOR_FORBIDDEN_TOKENS=str(ext))) + + assert proc.returncode == 0, proc.stdout + proc.stderr + # The two lines that used to read as a contradiction are both still there ... + assert "Installed the SYNTHETIC template" in proc.stdout + assert "CONFIGURED with the real token set" in proc.stdout + # ... and are now reconciled, by name. + assert "OVERRIDDEN" in proc.stdout, proc.stdout + assert "MEFOR_FORBIDDEN_TOKENS" in _source_line(proc) + assert str(ext) in _source_line(proc), ( + "the resolved path must be named, not merely the variable" + ) + + +def test_the_source_the_script_names_is_the_source_the_scanner_USED( + rig: tuple[Path, Path, Path], +) -> None: + """Pins the script's copy of the precedence rule against the scanner's actual behaviour. + + The script re-expresses ``scan_forbidden._resolve_token_text``; nothing but this stops the two + drifting. The detector-count lines must DIFFER between the two runs -- if the fixture's two token + sets were indistinguishable, every other assertion in this file would pass while measuring + nothing. + """ + root, shim, ext = rig + from_file = _run(root, "-Synthetic", env=_env(shim)) + from_env = _run(root, "-Synthetic", env=_env(shim, MEFOR_FORBIDDEN_TOKENS=str(ext))) + + assert _loaded(from_file) != _loaded(from_env), ( + f"the fixture cannot distinguish the two sources: {_loaded(from_file)!r}" + ) + assert "scripts/security/scan-tokens.local.txt" in _source_line(from_file) + assert str(ext) in _source_line(from_env) + # Direction, not merely difference: the env run must not be reported as the file it just wrote. + assert "SYNTHETIC" in _loaded(from_file) + assert "SYNTHETIC" not in _loaded(from_env) + + +def test_INLINE_token_content_is_named_but_never_echoed(rig: tuple[Path, Path, Path]) -> None: + """A non-path value IS the token list. Naming the variable is the diagnostic; printing it is a leak. + + The load-bearing negative for the whole change: the obvious way to "name the resolved source" is + to print the variable's value, which passes every other test here and publishes the private list + into whatever log the operator was capturing. + """ + root, shim, _ = rig + tokens = _derived_tokens(_INLINE_MARKER) + proc = _run(root, "-Synthetic", env=_env(shim, MEFOR_FORBIDDEN_TOKENS=tokens)) + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "OVERRIDDEN" in proc.stdout + assert "MEFOR_FORBIDDEN_TOKENS" in _source_line(proc) + assert "inline" in _source_line(proc).lower(), _source_line(proc) + assert _INLINE_MARKER not in proc.stdout, "the inline token content was echoed to stdout" + assert _INLINE_MARKER not in proc.stderr, "the inline token content was echoed to stderr" + + +def test_an_EMPTY_env_value_is_reported_as_the_cause_rather_than_a_missing_file( + rig: tuple[Path, Path, Path], +) -> None: + """The state where the ordinary advice is WRONG advice. + + An explicitly-empty ``MEFOR_FORBIDDEN_TOKENS`` means "no source" and does NOT fall back to the + file, so the script can install a token list, correctly report NOT CONFIGURED, and send the + operator round the same loop forever. Naming the cause is the difference between a diagnosis and a + restatement of the symptom. + """ + root, shim, _ = rig + proc = _run(root, "-Synthetic", env=_env(shim, MEFOR_FORBIDDEN_TOKENS="")) + + assert proc.returncode != 0, proc.stdout + assert "STRUCTURAL-ONLY" in _loaded(proc), "the scanner loaded a source after all" + assert "NOT CONFIGURED" in proc.stdout + assert "MEFOR_FORBIDDEN_TOKENS" in proc.stdout, proc.stdout + assert "EMPTY" in proc.stdout, proc.stdout + + +def test_status_mode_names_the_source_without_claiming_an_override( + rig: tuple[Path, Path, Path], +) -> None: + """No switch installs nothing, so nothing can have been overridden -- but the operator still needs + to know which source armed the gate, which is the one question the verify step exists to answer.""" + root, shim, _ = rig + assert _run(root, "-Synthetic", env=_env(shim)).returncode == 0 # arm it first + proc = _run(root, env=_env(shim)) + + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "Installed" not in proc.stdout, "status mode must not install anything" + assert "scripts/security/scan-tokens.local.txt" in _source_line(proc) + assert "OVERRIDDEN" not in proc.stdout + + +# -------------------------------------------------------------------------------------------------- +# the verdict must follow the scanner, not the happy path +# -------------------------------------------------------------------------------------------------- + + +def test_a_nonzero_scanner_exit_is_not_reported_as_CONFIGURED( + rig: tuple[Path, Path, Path], +) -> None: + """The script's header promises it "exits non-zero if the sections are empty"; it discarded the + scanner's exit code entirely, so a refusal came back out as CONFIGURED and exit 0. + + An impossible ``MEFOR_MIN_DETECTORS`` floor is the cheapest way to make the scanner refuse with a + source genuinely loaded -- the state the old code could not tell apart from success. + """ + root, shim, _ = rig + proc = _run(root, "-Synthetic", env=_env(shim, MEFOR_MIN_DETECTORS="99999")) + + assert proc.returncode != 0, ( + f"the scanner refused and the script reported success:\n{proc.stdout}" + ) + assert "VERIFY FAILED" in proc.stdout, proc.stdout + assert "CONFIGURED (synthetic)" not in proc.stdout, proc.stdout + + +def test_a_healthy_run_still_exits_zero(rig: tuple[Path, Path, Path]) -> None: + """The paired positive. Propagating an exit code is only a fix if the ordinary path stays green.""" + root, shim, _ = rig + proc = _run(root, "-Synthetic", env=_env(shim)) + assert proc.returncode == 0, proc.stdout + proc.stderr + assert "VERIFY FAILED" not in proc.stdout