diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 07780de..3847b90 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -39,20 +39,23 @@ permissions: {} jobs: release: - # Source-repo only. The public mirror carries a copy of this file, and the owner tags the mirror on - # its own snapshot sha — which fired this workflow there and red-X'd every tag. The mirror must never - # release: PyPI Trusted Publishing is bound to the SOURCE repo + `release.yml`, so a mirror upload is - # rejected on an OIDC claim mismatch, and a mirror `gh release create` would publish a release nobody - # asked for. + # Source-repo only — and since the MEFORORG cutover, MEFORORG/MessageFoundry IS the source repo. # - # The guard is INVERTED (`!=` the public slug) on purpose — DO NOT "normalize" it to - # `== 'MEFORORG/MessageFoundry'`. Historically scripts/publish/publish.ps1 (retired at the MEFORORG - # cutover) rewrote the private slug to the - # public one across *.yml when it materializes the mirror, so an `==` private-slug test is rewritten - # into `== 'MEFORORG/MessageFoundry'` and becomes TRUE exactly where it must be false. This form names - # only the public slug, so the rewrite cannot touch it: true on the source repo, false on the mirror. - # (Same rewrite-proof shape as ci.yml's `windows service smoke`, which pins itself ON the mirror.) - if: github.repository != 'MEFORORG/MessageFoundry' + # THIS GUARD WAS INVERTED UNTIL THE CUTOVER, AND THE INVERSION IS NOW WRONG. The old rationale: this + # repo was the published MIRROR, the mirror must never release, and scripts/publish/publish.ps1 + # rewrote the private slug to the public one across *.yml — so an `==` private-slug test would be + # rewritten into `== 'MEFORORG/MessageFoundry'` and become true exactly where it had to be false. + # Naming only the public slug made the test rewrite-proof. + # + # Both premises are gone: publish.ps1 was retired at the cutover (nothing rewrites anything), and + # this repo is the source, not the mirror. Left inverted, the job skips on the ONLY repo that can + # release — PyPI Trusted Publishing is bound to MEFORORG/MessageFoundry + `release.yml` (see the + # header), so the guard and the OIDC binding pointed in opposite directions and no tag could ever + # publish. Every other slug guard in .github/workflows (ci, codeql, quality-advisory, scorecard) + # was already flipped to `==` at the cutover; release.yml was missed. + # + # `==` is also the safe form now: a fork, or the retired private vault, still cannot release. + if: github.repository == 'MEFORORG/MessageFoundry' runs-on: ubuntu-latest permissions: contents: write # create the release + upload its assets @@ -270,10 +273,11 @@ jobs: # this job still BUILDS + version-checks the wheel and attaches it to the GitHub release, but does not # publish — so a missing harness PyPI project can never red a release. # - # Source-repo only, same rewrite-proof guard as `release` above (see the rationale there). `needs: - # release` already cascades the skip, so this is belt-and-braces: it keeps the harness release from - # firing on the mirror even if that dependency is ever dropped. - if: github.repository != 'MEFORORG/MessageFoundry' + # Source-repo only, same guard as `release` above (see the rationale there — it was inverted for the + # retired mirror topology and is flipped as of the cutover). `needs: release` already cascades the + # skip, so this is belt-and-braces: it keeps the harness release from firing anywhere that is not the + # source repo even if that dependency is ever dropped. + if: github.repository == 'MEFORORG/MessageFoundry' needs: release runs-on: ubuntu-latest permissions: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9b28d0e..6779ad5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -55,6 +55,46 @@ maintainer.) $env:QT_QPA_PLATFORM = "offscreen"; pytest -q ``` You can also run the project's own commit/CI gate: `python -m messagefoundry check`. +5. **Install the commit hooks — including the leak gate — before your first commit:** + ```powershell + pip install pre-commit + pre-commit install + pwsh -NoProfile -File scripts\dev\setup-leak-gate.ps1 -Synthetic + ``` + See *Leak gate* below for what that last line does and why it is not optional. + +### Leak gate (required, and it fails closed) + +One pre-commit hook — **forbidden-content** — scans for customer/PHI tokens. Its token list is +private and git-ignored, so it does **not** arrive with a clone or a `git worktree add`. Without a +token source the hook refuses to run and **every commit is blocked**: + +``` +scan_forbidden: no token source is configured ... refusing to run structural-only (fail closed). +``` + +That is deliberate. A gate that quietly loaded zero detectors would pass every commit green while +seeing nothing — the failure mode this project has hit more than once. Do not remove +`--require-tokens`, and do not reach for `--no-verify`. + +- **Outside contributors** cannot have the real list; it will never be distributable. Run + `setup-leak-gate.ps1 -Synthetic` to install the committed synthetic template. Your commits will + pass, and **CI runs the real detector set on your PR** — that is the authoritative check. +- **Maintainers** install the real list: `setup-leak-gate.ps1 -From `. + +The script always finishes by running the scanner and printing its per-section detector counts, +because a green gate is evidence only if you confirmed it can see. The scanner labels its mode on +every run, so a synthetic set can never be mistaken for a real one: + +``` +loaded names=7, estate=13, estate_file_scanned=12, site_prefixes=1 +loaded names=5, estate=3, ... [SYNTHETIC EXAMPLE TOKENS — blind to real customer tokens; CI is authoritative] +loaded names=0, estate=0, ... [STRUCTURAL-ONLY: no token source configured] +``` + +Only the first is a real local scan. Note that the synthetic template is **below CI's per-section +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. ## Finding something to work on diff --git a/scripts/dev/setup-leak-gate.ps1 b/scripts/dev/setup-leak-gate.ps1 new file mode 100644 index 0000000..8635f42 --- /dev/null +++ b/scripts/dev/setup-leak-gate.ps1 @@ -0,0 +1,87 @@ +<# +.SYNOPSIS + Configure the forbidden-content (customer/PHI leak) gate for THIS checkout, then prove it can see. + +.DESCRIPTION + The gate's token list is private and git-ignored, so it does NOT travel with a clone or a + `git worktree add`. Every fresh checkout therefore starts with no token source, and the + pre-commit hook -- which passes --require-tokens deliberately -- fails closed on every commit. + That is correct behaviour with an undocumented bootstrap; this script is the bootstrap. + + Choose ONE: + + -From Install the REAL token list (maintainers). The file is copied to the + git-ignored scripts/security/scan-tokens.local.txt. + -Synthetic Copy the committed synthetic template (outside contributors, who cannot have + the real list). The gate then runs, but is BLIND to real customer tokens -- + the scanner announces this on every run, and CI remains authoritative. + (no switch) Report the current state only; change nothing. + + WHY THE VERIFY STEP IS NOT OPTIONAL. A green gate is evidence only if you confirmed it can see + the class it is meant to catch. This repo has repeatedly hit gates that were green because they + 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. + +.EXAMPLE + pwsh -NoProfile -File scripts/dev/setup-leak-gate.ps1 -From C:\path\to\tokens.txt +.EXAMPLE + pwsh -NoProfile -File scripts/dev/setup-leak-gate.ps1 -Synthetic +#> +[CmdletBinding(DefaultParameterSetName = 'Status')] +param( + [Parameter(ParameterSetName = 'Real', Mandatory)][string]$From, + [Parameter(ParameterSetName = 'Synthetic', Mandatory)][switch]$Synthetic +) + +$ErrorActionPreference = 'Stop' +$repo = (& git rev-parse --show-toplevel 2>$null) +if (-not $repo) { throw 'Not inside a git checkout.' } +$secDir = Join-Path $repo 'scripts/security' +$local = Join-Path $secDir 'scan-tokens.local.txt' +$example = Join-Path $secDir 'scan-tokens.local.txt.example' +$scanner = Join-Path $secDir 'scan_forbidden.py' + +if ($PSCmdlet.ParameterSetName -eq 'Real') { + if (-not (Test-Path -LiteralPath $From)) { throw "No such token file: $From" } + Copy-Item -LiteralPath $From -Destination $local -Force + Write-Host "Installed the real token list -> scripts/security/scan-tokens.local.txt" -ForegroundColor Green +} +elseif ($PSCmdlet.ParameterSetName -eq 'Synthetic') { + Copy-Item -LiteralPath $example -Destination $local -Force + Write-Host "Installed the SYNTHETIC template -> scripts/security/scan-tokens.local.txt" -ForegroundColor Yellow + Write-Host " This gate cannot see real customer tokens. CI runs the real set on your PR." -ForegroundColor Yellow +} + +# The token file must never become committable. Verify rather than assume -- this is the one mistake +# that would publish the very list the gate protects. +if (Test-Path -LiteralPath $local) { + $ignored = & git -C $repo check-ignore -- 'scripts/security/scan-tokens.local.txt' 2>$null + if (-not $ignored) { + Remove-Item -LiteralPath $local -Force + throw 'scan-tokens.local.txt is NOT git-ignored in this checkout. Removed it rather than risk committing the token list.' + } +} + +# --- 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 +$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" + +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' + exit 1 +} +if ($loaded -match 'SYNTHETIC') { + Write-Host '' + Write-Host 'CONFIGURED (synthetic). Commits will pass; real customer tokens are NOT detected locally.' -ForegroundColor Yellow + exit 0 +} +Write-Host '' +Write-Host 'CONFIGURED with the real token set.' -ForegroundColor Green +exit 0 diff --git a/scripts/security/scan-tokens.local.txt.example b/scripts/security/scan-tokens.local.txt.example index af72cdb..5f4c7f2 100644 --- a/scripts/security/scan-tokens.local.txt.example +++ b/scripts/security/scan-tokens.local.txt.example @@ -16,6 +16,17 @@ # the name / estate / site-code detectors are empty (appropriate for a fork without the secret). # Set MEFOR_REQUIRE_TOKENS=1 to fail closed (exit 2) instead when the source is absent. # +# USING THIS FILE AS-IS IS A SUPPORTED CONTRIBUTOR SETUP, AND IS NOT THE SAME GATE AS CI. +# `scripts/dev/setup-leak-gate.ps1 -Synthetic` copies it to scan-tokens.local.txt so the pre-commit +# hook -- which passes --require-tokens and otherwise blocks EVERY commit -- can run. Two +# consequences, both intentional: +# * The scanner LABELS this set on every run: "[SYNTHETIC EXAMPLE TOKENS - blind to real customer +# tokens; CI is authoritative]". The placeholders below populate every section, so the count +# FLOOR alone cannot tell this apart from a real install -- the label is what does. +# * These counts (names=5, estate=3, site_prefix=1) are BELOW CI's per-section floor +# (MEFOR_MIN_DETECTORS=names=7,estate=13,site_prefixes=1 in .github/workflows/security.yml), so +# passing locally with this file does not predict CI's leak gate. +# # FORMAT (sectioned; `#` comments and blank lines ignored) # [names] one entry per line: REGEX | REASON | CASE # REASON optional (default "customer/vendor token"). diff --git a/scripts/security/scan_forbidden.py b/scripts/security/scan_forbidden.py index 5706ed0..71ceef8 100644 --- a/scripts/security/scan_forbidden.py +++ b/scripts/security/scan_forbidden.py @@ -455,6 +455,38 @@ def reload_tokens() -> None: _SITE_CODE_PATTERN_LITERAL = _NEVER +#: The committed synthetic template, used only to RECOGNISE itself (never as a token source). +EXAMPLE_TOKEN_FILE = Path(__file__).parent / "scan-tokens.local.txt.example" + + +def is_synthetic_token_set() -> bool: + """Is the loaded token set just the shipped synthetic example? + + Copying ``scan-tokens.local.txt.example`` is the documented way an OUTSIDE CONTRIBUTOR satisfies + the pre-commit hook: the real list is private and will never be distributable. That is a fine + contributor posture -- but it yields a gate that exits 0 while being BLIND to every real customer + token, which is indistinguishable from a genuinely clean run unless it is announced. A maintainer + who reaches for the example instead of installing the real list would get exactly the false-clean + this module exists to prevent, and the count floor cannot catch it: the synthetic set POPULATES + every section, so it satisfies "detectors that can fire" while firing on nothing real. + + Compared against the PARSED content, not the file bytes, so reformatting or re-commenting the copy + still reads as synthetic. + """ + if not TOKENS_PRESENT: + return False + try: + example = EXAMPLE_TOKEN_FILE.read_text(encoding="utf-8") + ex_names, ex_estate, _body, ex_prefixes = _parse_tokens(example) + except (OSError, ValueError): + return False + return ( + tuple(p.pattern for p, _ in ex_names) == tuple(p.pattern for p, _ in FORBIDDEN) + and tuple(ex_estate) == ESTATE_TOKENS + and tuple(ex_prefixes) == _SITE_PREFIXES + ) + + def loaded_token_counts() -> dict[str, int]: """Detector counts for the current token tables. @@ -810,10 +842,17 @@ def main(argv: list[str]) -> int: # alone cannot distinguish "scanned with the real tables and found nothing" from "loaded nothing # and had nothing to find" -- both are silent successes. counts = loaded_token_counts() + # Three distinguishable states, because "exit 0" collapses them: real tables (silent), the shipped + # synthetic example (populates every section and so passes the floor, but matches nothing real), + # and no source at all. Only the first is evidence. + if not TOKENS_PRESENT: + mode = " [STRUCTURAL-ONLY: no token source configured]" + elif is_synthetic_token_set(): + mode = " [SYNTHETIC EXAMPLE TOKENS — blind to real customer tokens; CI is authoritative]" + else: + mode = "" print( - "scan_forbidden: loaded " - + ", ".join(f"{k}={v}" for k, v in counts.items()) - + ("" if TOKENS_PRESENT else " [STRUCTURAL-ONLY: no token source configured]"), + "scan_forbidden: loaded " + ", ".join(f"{k}={v}" for k, v in counts.items()) + mode, file=sys.stderr, ) diff --git a/scripts/worktree/new.ps1 b/scripts/worktree/new.ps1 index 1dd71c4..748e947 100644 --- a/scripts/worktree/new.ps1 +++ b/scripts/worktree/new.ps1 @@ -85,6 +85,24 @@ try { if ($gitDir) { Set-Content -LiteralPath (Join-Path $gitDir 'mefor-home-branch') -Value $Name -Encoding utf8 } } catch { } +# The forbidden-content gate's token list is git-ignored, so `git worktree add` cannot deliver it and a +# fresh worktree starts unable to commit AT ALL -- the pre-commit hook passes --require-tokens and fails +# closed. Carry the source checkout's copy across if it has one; otherwise say so here, because the +# symptom is a hard commit block whose message does not mention worktrees. Best-effort: never block +# worktree creation. MEFOR_FORBIDDEN_TOKENS, if set, already covers every checkout on the machine. +try { + $tokenRel = 'scripts/security/scan-tokens.local.txt' + $srcToken = Join-Path $RepoRoot $tokenRel + if (Test-Path -LiteralPath $srcToken) { + Copy-Item -LiteralPath $srcToken -Destination (Join-Path $WorktreePath $tokenRel) -Force + Write-Host "Leak-gate token list copied into the worktree." -ForegroundColor DarkGray + } + elseif (-not $env:MEFOR_FORBIDDEN_TOKENS) { + Write-Host "NOTE: no leak-gate token source found - commits here will fail closed." -ForegroundColor Yellow + Write-Host " Fix once: pwsh -NoProfile -File scripts\dev\setup-leak-gate.ps1 -From " -ForegroundColor Yellow + } +} catch { } + function Show-NextSteps { Write-Host "" Write-Host "Worktree ready: $WorktreePath (branch '$Name')" -ForegroundColor Green diff --git a/tests/test_release_pipeline.py b/tests/test_release_pipeline.py index 13400ce..5e5e922 100644 --- a/tests/test_release_pipeline.py +++ b/tests/test_release_pipeline.py @@ -240,16 +240,28 @@ def test_release_publish_uses_trusted_publishing_no_token() -> None: # --- (6) the mirror must never release (rewrite-proof inverted guard) -------------------------------- -def test_release_mirror_guard_is_rewrite_proof() -> None: +def test_release_jobs_are_gated_ON_the_source_repo() -> None: + """Both release jobs must run on MEFORORG/MessageFoundry, and nowhere else. + + THIS ASSERTION USED TO BE THE EXACT OPPOSITE, and that is the point. Before the cutover this repo + was the published MIRROR, the mirror had to never release, and publish.ps1 rewrote the private slug + to the public one across *.yml — so the guard was written `!= public-slug` to be rewrite-proof, and + this test pinned that form. + + Both premises died at the cutover: publish.ps1 is retired, and MEFORORG is now the SOURCE. The old + assertion therefore kept the release pipeline gated OFF the only repo that can publish — PyPI + Trusted Publishing is bound to MEFORORG/MessageFoundry + release.yml — so no tag could ever ship, + and the test made that look intentional. A test can outlive the premise it encodes; this one did. + """ rel = _release() - # publish.ps1 rewrites the PRIVATE slug -> the PUBLIC slug across *.yml when it materializes the mirror. - # An `== private-slug` test would be rewritten into `== public-slug` and become TRUE on the mirror, so - # the guard is written as `!= public-slug` (which the rewrite cannot touch). Both release jobs use it. - assert rel.count("if: github.repository != 'MEFORORG/MessageFoundry'") >= 2, ( - "the rewrite-proof mirror guard (`!= 'MEFORORG/MessageFoundry'`) must gate BOTH the release and " - "release-harness jobs — a naive `== 'wshallwshall/MessageFoundry'` normalization is rewrite-VULNERABLE" + assert rel.count("if: github.repository == 'MEFORORG/MessageFoundry'") >= 2, ( + "both the `release` and `release-harness` jobs must be gated ON the source repo " + "(`== 'MEFORORG/MessageFoundry'`), or a pushed tag silently skips and nothing publishes" ) - # The vulnerable normalization must NOT be present as a job guard. - assert "if: github.repository == 'wshallwshall/MessageFoundry'" not in rel, ( - "the mirror guard was normalized to a rewrite-VULNERABLE `== private-slug` form" + # The pre-cutover inversion must never come back: it skips on the only repo that can release. + assert "if: github.repository != 'MEFORORG/MessageFoundry'" not in rel, ( + "the pre-cutover mirror guard (`!= 'MEFORORG/MessageFoundry'`) is back — that disables releases " + "entirely, because MEFORORG is the source repo now, not the mirror" ) + # The private vault must never be a release target either. + assert "wshallwshall" not in rel, "release.yml must not reference the retired private vault" diff --git a/tests/test_scan_forbidden.py b/tests/test_scan_forbidden.py index 090ea6a..4a5ad82 100644 --- a/tests/test_scan_forbidden.py +++ b/tests/test_scan_forbidden.py @@ -196,3 +196,60 @@ def test_scanner_no_longer_skips_its_own_token_bearing_tests(sf) -> None: assert sf._is_skipped("tests/test_scan_forbidden.py") is False assert sf._is_skipped("tests/test_anon_core.py") is False assert sf._is_skipped("messagefoundry/__init__.py") is False + + +# --- synthetic-vs-real token set --------------------------------------------------------------------- +# +# Copying scan-tokens.local.txt.example is the supported way an outside contributor satisfies the +# pre-commit hook (the real list is private and undistributable). The danger is that the example +# POPULATES every section, so it clears the count floor while matching nothing real — a maintainer who +# copied it instead of installing the real list would get a green, blind gate, and the floor could not +# tell. Distinguishing the two is therefore the only thing standing between "exit 0" and false clean. + +_EXAMPLE = _REPO_ROOT / "scripts" / "security" / "scan-tokens.local.txt.example" + + +def _scanner_with_tokens(monkeypatch: pytest.MonkeyPatch, text: str | None): + """A FRESH scanner module whose tokens came from ``text`` (inline content, or None for no source).""" + if text is None: + monkeypatch.setenv("MEFOR_FORBIDDEN_TOKENS", "") + else: + monkeypatch.setenv("MEFOR_FORBIDDEN_TOKENS", text) + return _load_scanner() + + +def test_the_shipped_example_is_recognised_as_synthetic(monkeypatch: pytest.MonkeyPatch) -> None: + mod = _scanner_with_tokens(monkeypatch, _EXAMPLE.read_text(encoding="utf-8")) + assert mod.TOKENS_PRESENT is True + assert mod.is_synthetic_token_set() is True + # The trap this guards: every floor section is non-empty, so counts alone look like a real install. + counts = mod.loaded_token_counts() + assert all(counts[s] > 0 for s in ("names", "estate", "site_prefixes")) + + +def test_a_reformatted_copy_of_the_example_still_reads_synthetic( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Compared on PARSED content, not bytes — otherwise adding one comment line would silently + reclassify a synthetic set as real, which is the direction that fails open.""" + noisy = "# a local note\n\n" + _EXAMPLE.read_text(encoding="utf-8") + "\n\n# trailing note\n" + mod = _scanner_with_tokens(monkeypatch, noisy) + assert mod.is_synthetic_token_set() is True + + +def test_a_real_shaped_token_set_is_NOT_flagged_synthetic(monkeypatch: pytest.MonkeyPatch) -> None: + """The control. Without this, a function that returned True unconditionally would look correct.""" + mod = _scanner_with_tokens( + monkeypatch, + "[names]\n\\bNOTREAL\\b | customer | i\n\n[estate]\nnotrealvendor\n\n[site_prefix]\n77\n", + ) + assert mod.TOKENS_PRESENT is True + assert mod.is_synthetic_token_set() is False + + +def test_no_token_source_is_absent_rather_than_synthetic(monkeypatch: pytest.MonkeyPatch) -> None: + """'Nothing loaded' and 'the example loaded' are different failures with different fixes, and the + run banner names them differently — so the predicate must not conflate them.""" + mod = _scanner_with_tokens(monkeypatch, None) + assert mod.TOKENS_PRESENT is False + assert mod.is_synthetic_token_set() is False