diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 2e4965b..29aedb5 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -14,6 +14,6 @@ # Security, licensing, and the public-mirror publish/scan tooling. /docs/SECURITY.md @wshallwshall /docs/Secure_Development_Standards.md @wshallwshall -/scripts/publish/ @wshallwshall +/scripts/security/ @wshallwshall /CLA.md @wshallwshall /LICENSE @wshallwshall diff --git a/.github/dependabot.yml b/.github/dependabot.yml new file mode 100644 index 0000000..2449496 --- /dev/null +++ b/.github/dependabot.yml @@ -0,0 +1,91 @@ +# Dependabot: surface vulnerable / outdated dependencies and CI actions as PRs (CI-1 / DEP-1). +# Until a committed lockfile lands, this is the primary signal for a known-CVE dependency. +version: 2 +updates: + # Python deps via the native "uv" ecosystem (was "pip"). The uv ecosystem resolves against + # pyproject.toml + uv.lock and REGENERATES uv.lock IN its PRs (version updates GA 2025-03, + # security updates GA 2025-12) — the old "pip" ecosystem updated requirements/pyproject but NOT + # uv.lock. It still does not re-derive the EXPORTED locks (requirements.lock + docker/locks/* + + # constraints.lock), which the DEP-1 gate in security.yml byte-diffs; + # .github/workflows/dependabot-lock-resync.yml re-exports those on the Dependabot branch so the + # gate stays green. + - package-ecosystem: "uv" + directory: "/" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + # Supply-chain cooldown: age a fresh release before opening a ROUTINE update PR, to dodge a + # package compromised shortly after publish. Security updates ignore cooldown (Dependabot + # behavior), so a real advisory fix still arrives immediately. Pairs with the auto-merge + # workflow: routine patches auto-merge AFTER aging; security patches auto-merge now. + cooldown: + # ~5-day aging window (DEPENDENCY-POSTURE-REVIEW.md) lengthens the malicious-fresh-publish + # dodge on the routine VERSION track; the SECURITY track still bypasses cooldown (Dependabot + # design), now backstopped by the published-GHSA gate in dependabot-auto-merge.yml (SEC-007 #2). + default-days: 5 + semver-major-days: 7 + groups: + # Version-update grouping (applies-to defaults to version-updates). + python-deps: + patterns: ["*"] + # Security-update grouping: collapse multiple vulnerable-dep fixes into ONE PR so a single + # review/merge clears them — faster remediation when several CVEs land together. + python-security: + applies-to: security-updates + patterns: ["*"] + + - package-ecosystem: "github-actions" + directory: "/" + schedule: + interval: "weekly" + groups: + # One weekly PR for all action bumps instead of one PR per action (CI cost: every extra PR + # buys a full required-check pass + a push-to-main run when it auto-merges). Mirrors the + # python-deps/ide-deps grouping above. A patch batched with a major rides the same PR — + # latency-only, auto-merge still gates on the full CI pass. Security updates keep their own + # group so an advisory fix is never held behind a routine batch. + actions-deps: + patterns: ["*"] + actions-security: + applies-to: security-updates + patterns: ["*"] + + # The VS Code extension's npm tree (build-time toolchain — the shipped artifact is the esbuild + # bundle). Surfaces a vulnerable/outdated npm dep as a PR, the same DEP-1 surveillance the uv + # ecosystem gets; the blocking npm-audit job in security.yml is the fail-closed companion. + - package-ecosystem: "npm" + directory: "/ide" + schedule: + interval: "weekly" + open-pull-requests-limit: 10 + cooldown: + default-days: 3 + semver-major-days: 7 + # Three dev-deps are pinned to the extension's runtime contract and must NOT float to the newest + # published version (grouping them into a routine bump broke `ide build` in PR #649 and PR #877): + # * @types/node tracks the Node runtime inside VS Code's Electron, not the latest stubs. The + # 20 -> 26 jump made the Node globals/builtins unresolvable at typecheck (TS2591/TS2304). A + # major is a deliberate moduleResolution (node16/nodenext) migration on its own PR, not an + # auto-bump — so hold the 20.x line (patches still flow). + # * @types/vscode must never exceed the engines.vscode floor (^1.95). Typing against a newer + # API surface lets the extension compile against methods absent from the editors it claims to + # support, failing at runtime on an older VS Code rather than in CI. Raise engines.vscode and + # this pin together, as one deliberate change, when dropping support for older VS Code. + # * typescript majors are a compiler migration, not a routine bump. The 5 -> 6 jump broke the + # typecheck outright (TS2591/TS2304/TS2694 — @types/node globals and the NodeJS namespace no + # longer resolve under the new lib/moduleResolution defaults). #875 deliberately left it + # un-ignored so CI could adjudicate the major on its own PR; CI failed it (#877), so hold the + # 5.x line and adopt TS 6 as its own deliberate migration (tsconfig lib/module review). + ignore: + - dependency-name: "@types/node" + versions: [">=21.0.0"] + - dependency-name: "@types/vscode" + versions: [">=1.96.0"] + - dependency-name: "typescript" + versions: [">=6.0.0"] + groups: + ide-deps: + patterns: ["*"] + ide-security: + applies-to: security-updates + patterns: ["*"] diff --git a/.github/workflows/backlog-hygiene.yml b/.github/workflows/backlog-hygiene.yml deleted file mode 100644 index f6b2b2a..0000000 --- a/.github/workflows/backlog-hygiene.yml +++ /dev/null @@ -1,99 +0,0 @@ -# Backlog status hygiene — stop `docs/BACKLOG.md` from lying about build state. -# -# WHY. Work ships, the item's banner is never updated, and the backlog goes on describing finished -# work as open. A 2026-07-09 audit found 11 items misfiled as open — including #60 (turnkey DR), -# which shipped with ADR 0049 and a working `backup` / `restore-verify` CLI while its banner still -# read "PRE-RESERVED, owner-gated". That stale banner was then repeated as fact in a merged PR. The -# same rot left the Corepoint gap analysis ~22% obsolete. A doc that lies about build state is worse -# than no doc: it silently misdirects planning. -# -# TWO HALVES. -# * The STRUCTURAL half ("every item declares exactly one status") is enforced by pytest — -# `tests/test_backlog_status_check.py` runs the checker against the real file on every PR, so it -# rides the existing test matrix and needs no job here. -# * The BEHAVIOURAL half is here, because only the PR context can see it: if a PR claims to -# implement a backlog item (`BACKLOG #N` in its title or body) and touches engine/IDE code, then -# it must also update `docs/BACKLOG.md`. That is the step whose omission caused #60. -# -# Read-only. No secrets. Workflow expressions are hoisted into `env` and never interpolated into a -# `run:` body (zizmor: a PR title/body is attacker-controlled on a fork PR and must arrive as data). -name: backlog-hygiene - -on: - pull_request: - branches: [main] - -permissions: - contents: read - -concurrency: - group: backlog-hygiene-${{ github.event.pull_request.number }} - cancel-in-progress: true - -jobs: - banner-on-implementation: - # QUOTED: an unquoted YAML scalar ends at " #" (comment start), which silently truncated this to - # "a PR that implements BACKLOG". The job name is the required-status-check *context* string, so - # a truncated name is what you would have to add to branch protection. - name: "a PR that implements BACKLOG #N must update BACKLOG.md" - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - fetch-depth: 0 - persist-credentials: false - - - name: Require a backlog update when a PR claims to implement an item - env: - # Hoisted, never interpolated into the script body (zizmor: template injection). - PR_TITLE: ${{ github.event.pull_request.title }} - PR_BODY: ${{ github.event.pull_request.body }} - BASE_SHA: ${{ github.event.pull_request.base.sha }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - run: | - set -euo pipefail - - # Does this PR *claim* to implement a backlog item? Only the explicit `BACKLOG #N` token - # counts — a bare `#123` is ambiguous in this repo (it is usually a PR number). - claim="$(printf '%s\n%s\n' "$PR_TITLE" "$PR_BODY" | grep -oiE 'BACKLOG #[0-9]+' | head -1 || true)" - if [ -z "$claim" ]; then - echo "No 'BACKLOG #N' claim in this PR — nothing to enforce." - echo "(If this PR completes a backlog item, say so with 'BACKLOG #N' and update its banner.)" - exit 0 - fi - - changed="$(git diff --name-only "$BASE_SHA" "$HEAD_SHA")" - touches_code=false - case "$changed" in - *messagefoundry/*|*ide/*|*messagefoundry_webconsole/*) touches_code=true ;; - esac - - if [ "$touches_code" != true ]; then - echo "PR claims '$claim' but changes no engine/IDE code — no banner update required." - exit 0 - fi - - if printf '%s\n' "$changed" | grep -qx 'docs/BACKLOG.md'; then - echo "OK — PR claims '$claim', touches code, and updates docs/BACKLOG.md." - exit 0 - fi - - n="$(printf '%s' "$claim" | grep -oE '[0-9]+')" - cat >&2 < ✅ **SHIPPED in ().** - - and remove its '🔢 Re-scored' banner (an item must declare exactly one status). - - This gate exists because #60 shipped while its banner still said "PRE-RESERVED", and that - stale banner was later repeated as fact. A doc that lies about build state silently - misdirects planning. - - If this PR only *partially* implements the item, keep the open banner and say so in the - item body — then drop the 'BACKLOG #$n' token from this PR's title/body. - EOF - exit 1 diff --git a/.github/workflows/release-sync-check.yml b/.github/workflows/release-sync-check.yml deleted file mode 100644 index 5a74c09..0000000 --- a/.github/workflows/release-sync-check.yml +++ /dev/null @@ -1,44 +0,0 @@ -# Release-sync tripwire: assert the git tag, the PyPI wheel, and the public mirror agree on one -# version (scripts/publish/check_release_sync.py). release.yml already locks tag == PyPI and -# publish.ps1's version guard locks tag == mirror at publish time; this catches drift that appears -# AFTER publish (a mirror republished from the wrong ref, a PyPI yank, a deleted tag). Read-only: -# two HTTPS GETs (PyPI JSON + the mirror's raw __init__.py) + `git tag`. No secrets, no writes. -# -# A real desync (PyPI version with no source tag; mirror ahead of PyPI) fails the build (exit 1); -# tolerable states (mirror lagging, a release in flight, pre-launch with nothing on PyPI) print a -# WARN and stay green. -name: release-sync - -on: - schedule: - # Weekly — version sync only changes at release time, so a daily cadence would be noise. The - # release flow can also run the script directly as a post-publish step for immediate confirmation. - - cron: "0 7 * * 1" - workflow_dispatch: - -permissions: - contents: read - -jobs: - release-sync: - name: tag / PyPI / mirror version agreement - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - with: - fetch-depth: 0 # need the vX.Y.Z tags (the private-tag side of the check) - - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 - with: - python-version: "3.14" - - name: Check tag / PyPI / mirror are in sync - run: | - # The script lives under the deny-listed scripts/publish/, so it is ABSENT on the OSS mirror; - # the sync check only runs on the private repo (the publish source). Skip where it's absent. - if [ ! -f scripts/publish/check_release_sync.py ]; then - echo "scripts/publish/check_release_sync.py absent (OSS mirror) — skipping the release-sync check." - exit 0 - fi - # `packaging` gives canonical version normalization (so a "0.1.0-rc1" tag and PyPI's - # "0.1.0rc1" compare equal); the script falls back to a naive form if it's absent. - python -m pip install --upgrade pip packaging - python scripts/publish/check_release_sync.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6f79511..07780de 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -46,7 +46,8 @@ jobs: # asked for. # # The guard is INVERTED (`!=` the public slug) on purpose — DO NOT "normalize" it to - # `== 'MEFORORG/MessageFoundry'`. scripts/publish/publish.ps1 rewrites the private slug to the + # `== '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. @@ -75,7 +76,7 @@ jobs: # badges — but anonymous fetches of a private repo's Actions badge SVG 404 (broken badge # images), and every body link 404s on click. The public mirror (MEFORORG/MessageFoundry) # runs the same workflows and is anonymously reachable, so rewrite the slug here (the same - # rewrite scripts/publish/publish.ps1 applies to the mirror) before `python -m build` embeds + # rewrite the retired scripts/publish/publish.ps1 used to apply) before `python -m build` embeds # the README. Ephemeral runner edit only — never committed. sed -i 's#MEFORORG/MessageFoundry#MEFORORG/MessageFoundry#g' README.md if grep -q 'MEFORORG/MessageFoundry' README.md; then diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index af0e649..5afd61e 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -16,6 +16,11 @@ name: Security # against an unchanged main. On-demand full scans: workflow_dispatch. on: pull_request: + # Post-merge re-scan (main only). A fork PR is scanned STRUCTURAL-ONLY by design -- the secret is + # unavailable to it -- so without this arm no fully-loaded scan ever sees fork-contributed content. + # Scoped to main so branch pushes do not double-run alongside their own PR. + push: + branches: [main] schedule: # Daily, so a CVE freshly disclosed against an UNCHANGED pinned dep (no push/PR to trigger a scan) # is caught within ~24h instead of up to 7 days — a weekly cadence can't meet a 72h-class remediation @@ -344,24 +349,48 @@ jobs: forbidden-content: name: forbidden-content (customer/PHI leak guard) runs-on: ubuntu-latest - # BLOCKING: scan the PUBLISHABLE subset — every tracked file MINUS scripts/publish/publish-denylist.txt - # (the same deny-list publish.ps1 removes) — for customer/PHI-adjacent strings: partner/site names, the - # real production estate, routable host IPs. These are not *secrets* (gitleaks won't flag them) but must - # never reach the open-source mirror. This is the same scanner publish.ps1 runs as its fail-closed gate, - # brought forward to PR/push-to-main time so a token can't ride a squash-merge onto main and sit there - # until the next publish catches it. See docs/security/PUBLISHING.md. + # BLOCKING (required context): scan the WHOLE tracked tree for customer/PHI-adjacent strings -- + # partner/site names, the real production estate, routable host IPs, internal worktree slugs and + # absolute home paths. These are not *secrets* (gitleaks will not flag them) but must never land on + # this public repo. The committed scanner ships only STRUCTURAL detectors plus a synthetic + # .example; the real token list is NEVER committed -- it arrives from the MEFOR_FORBIDDEN_TOKENS + # secret here, and locally from a git-ignored scripts/security/scan-tokens.local.txt. steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 with: python-version: "3.14" - - name: Scan the publishable subset for forbidden content - # Stdlib-only scanner (no install). Exits non-zero — failing the build — on any hit. The scanner - # lives under the deny-listed scripts/publish/, so it is ABSENT on the OSS mirror — there's - # nothing to gate there (everything is already the published subset); skip rather than error. + - name: Scan the tracked tree for forbidden content + # Stdlib-only scanner (no install). + # + # MEFOR_MIN_DETECTORS is what makes "fail-closed" mean anything. Requiring a token SOURCE only + # proves one arrived: a partially-mangled secret still loaded as few as 1 of 21 detectors and + # passed with a green tick, and losing just the final line silently disabled every site-code + # detector. The floor is PER-SECTION, not a bare total -- a total is a SUM, so growth in a cheap + # section masks collapse in an expensive one (names 7->1 alongside estate 13->19 still totals 21). + # It is a FLOOR: adding tokens needs no CI change, losing them fails the build. + # + # zizmor: the secret is never interpolated into this run: body. It arrives as the step-level env + # var (an opaque value, not an Actions-expression sink). + env: + MEFOR_FORBIDDEN_TOKENS: ${{ secrets.MEFOR_FORBIDDEN_TOKENS }} + # Whether this run CANNOT legitimately see the secret. Empty on pushes and same-repo PRs. + IS_FORK_PR: ${{ github.event.pull_request.head.repo.fork }} run: | - if [ -f scripts/publish/scan_forbidden.py ]; then - python scripts/publish/scan_forbidden.py --published + if [ -n "$MEFOR_FORBIDDEN_TOKENS" ]; then + # Deliberately NOT written to scripts/security/scan-tokens.local.txt: the scanner resolves + # the env var first, so that file would never be read -- it would only drop the full real + # token list into the job workspace for every later step to see. + export MEFOR_REQUIRE_TOKENS=1 + export MEFOR_MIN_DETECTORS=names=7,estate=13,site_prefixes=1 + echo "token list loaded from the MEFOR_FORBIDDEN_TOKENS secret (fail-closed, per-section floor)." + elif [ "$IS_FORK_PR" = "true" ]; then + echo "fork PR -- the secret is unavailable BY DESIGN; structural-only scan." else - echo "scripts/publish/scan_forbidden.py absent (OSS mirror) — skipping the publish leak-gate." + # Degrading here would be SILENT and PERMANENT: a renamed, deleted, environment-scoped or + # rotated secret takes the fork branch forever, scanning nothing while the REQUIRED context + # stays green. Absent-on-a-non-fork is a broken gate, not a mode. + echo "::error::MEFOR_FORBIDDEN_TOKENS is absent on a non-fork run. The customer-leak gate cannot run. Check the repository secret (repo-scoped, not environment-scoped)." >&2 + exit 2 fi + python scripts/security/scan_forbidden.py --path . diff --git a/.github/workflows/selfhosted-win2025-sql.yml b/.github/workflows/selfhosted-win2025-sql.yml index 99eda85..24147c0 100644 --- a/.github/workflows/selfhosted-win2025-sql.yml +++ b/.github/workflows/selfhosted-win2025-sql.yml @@ -4,9 +4,11 @@ # (they exercise SQL Server only on a Linux service container). It is the companion to ci.yml's # `sql server (store + connector)` legs — the SAME suites, run on the real OS + driver stack. # -# SECURITY (a self-hosted runner executes repository code): -# * Gated to `workflow_dispatch` ONLY — never `pull_request`, never a fork, no schedule. The repo is -# private, so untrusted fork code never reaches this runner. Run it ON DEMAND when the VM is up. +# SECURITY (a self-hosted runner executes repository code — critical on a PUBLIC repo, where a fork PR +# could otherwise run attacker code on the runner host): +# * Gated to `workflow_dispatch` ONLY — never `pull_request`, never a fork, no schedule. Dispatch is +# restricted to users with write access, so untrusted fork code never reaches this runner. That +# dispatch-only posture (NOT repo privacy) is the control. Run it ON DEMAND when the VM is up. # * NOT a required check, and there is NO scheduled trigger — so the VM never has to be on. Nothing # queues or fails while it is off; a manual run just waits for the runner (and expires after ~24h if # the VM never comes up). It never blocks a PR or a merge. diff --git a/.gitignore b/.gitignore index da4eb67..8973596 100644 --- a/.gitignore +++ b/.gitignore @@ -108,3 +108,23 @@ Thumbs.db # this file before the tooling that references it exists, so ignoring the path up front # removes any window in which a real token list sits un-ignored in a working tree. scripts/security/scan-tokens.local.txt + +# --- Private-only paths (formerly enforced by scripts/publish/publish-denylist.txt) --------------- +# The cutover RETIRES the publish deny-list. On the private repo these were TRACKED and stripped from +# the mirror at publish time; with the deny-list gone and development happening directly on the public +# repo, a gitignore rule is now the ONLY thing keeping them out of a commit -- and the cutover runbook +# runs `git add -A`. Same failure shape as the leak-scanner token file, different files. +# +# DELIBERATELY NOT LISTED: tests/test_scan_forbidden.py, tests/test_anon_core.py and +# .github/dependabot.yml. Those were deny-listed too, but this change set publishes synthetic/public +# versions of them -- ignoring them here would silently drop content that is meant to ship. +/CLAUDE.md +/.claude/ +/TRANSCRIPTS.md +/docs/security/ +/docs/reviews/ +/docs/marketing/ +/docs/BACKLOG.md +/docs/WORKTREES.md +/docs/CI-TOPOLOGY.md +/docs/Secure_Development_Standards.md diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 8f35562..1ec6d77 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -18,6 +18,23 @@ repos: entry: ruff check --fix language: system types: [python] + # Leak guard (always on): keep customer/PHI-adjacent strings -- partner names, site data, + # routable host IPs, internal worktree slugs and absolute home paths -- out of the TRACKED tree + # so they can never reach this public repo. Mirrors the CI forbidden-content gate. + # + # `--require-tokens` makes this FAIL CLOSED. The real token list lives in a git-ignored + # scripts/security/scan-tokens.local.txt that does NOT travel with a clone or a new worktree; + # without the flag the hook would load zero customer detectors there and pass every commit + # green. pre-commit can pass ARGS to a hook but has no per-hook `env:`, so the flag is the only + # mechanism. Run `pre-commit run --all-files` once after creating the token file to confirm. + - id: forbidden-content + name: forbidden-content (customer/PHI leak guard) + entry: python scripts/security/scan_forbidden.py + args: [--require-tokens] + language: system + pass_filenames: true + types: [text] + exclude: ^scripts/security/scan-tokens\.local\.txt$ # Secret scanning — blocks a commit that would introduce a credential/token/key. # (rev is verified by `pre-commit autoupdate` on first install; a wrong tag fails loudly there, diff --git a/docs/CI.md b/docs/CI.md new file mode 100644 index 0000000..29cffe2 --- /dev/null +++ b/docs/CI.md @@ -0,0 +1,71 @@ +# CI overview + +**Audience:** contributors and maintainers working on MessageFoundry. This page describes what +Continuous Integration runs on a pull request, which checks must pass before a change can merge, and a +few gotchas that have cost real debugging time. Branch protection on `main` is the source of truth for +which checks are *required* — this page describes the intended layout. + +## Workflows + +| Workflow | What it does | +|---|---| +| `ci.yml` | Lint (`ruff check` + `ruff format --check`), types (`mypy --strict`, plus a `--platform win32` pass on Linux so Windows type-branches are checked), and the `pytest` suite across **ubuntu-latest**, **windows-2022**, and **windows-2025** (Python 3.14). Also builds the VS Code extension (`ide/`). A `CI gate` job rolls the legs up. | +| `security.yml` | Static and supply-chain security: `bandit` (Python SAST), `semgrep`, `pip-audit` and `npm-audit` against the hash-locked tree, `gitleaks` (secret scan), `forbidden-content` (customer/PHI leak guard), a crypto-inventory check, an SBOM build, and a `trivy` scan. A **daily cron** re-runs the dependency audits so a CVE filed against an unchanged pin is caught within ~24h. | +| `codeql.yml` | GitHub CodeQL analysis (python / javascript-typescript). | +| `scorecard.yml` | OpenSSF Scorecard analysis. | +| `cla.yml` | CLA Assistant — records the Contributor License Agreement signature on each PR. | +| `zizmor.yml` | Lints the workflow files themselves for insecure patterns (template injection, over-broad tokens). **Blocking.** | + +Several heavier legs (server-DB store tests, load/throughput, service-smoke, DICOM/FHIR breadth) run +**nightly on a schedule** and/or only when a PR touches their paths, so an ordinary PR does not pay for +them. Because they do not run on every PR, they are **not** required to merge (a job that never reports +would otherwise wedge the PR — see the gotcha below). + +## Checks required to merge + +The stable contexts required on `main` are: + +- `CI gate` +- `test (ubuntu-latest, py3.14)` +- `test (windows-2022, py3.14)` +- `test (windows-2025, py3.14)` +- `bandit (Python SAST)` +- `pip-audit (dependency vulnerabilities)` +- `forbidden-content (customer/PHI leak guard)` +- `CLA Assistant` + +CodeQL and Scorecard run on PRs but are **advisory** (not in the required set) — their SARIF upload needs +`security-events: write`, which fork-PR tokens do not have, so requiring them would block PRs from forks. +Nightly / path-gated legs (service-smoke, load, SQL/Postgres store) are deliberately **not** required. + +### The `CI gate` roll-up + +`CI gate` `needs:` the individual legs, runs with `if: always()`, and fails **only** on a `failure` or +`cancelled` leg. A **`skipped`** leg counts as a **pass** — that is what lets a path-gated leg stay off +an unrelated PR without turning the gate red. + +> ### The required-but-absent trap +> A **required** status check that never reports **blocks every PR forever**. So before you stop a job +> from running on PRs (path-gate it, or make it schedule-only), make sure it is **not** in the required +> list first. Add a job to the required list only once you have seen it report on a real PR. + +## Gotchas + +- **Run `actionlint` on every `ci.yml` edit.** GitHub interpolates `${{ }}` expressions *anywhere* in a + `run:` script — comments included — before the shell sees it, so a stray/invalid expression aborts + workflow compilation: **no jobs are created**, the run is attributed to a phantom event, and required + contexts silently never appear (the PR just looks stuck). `zizmor` does not catch this; `actionlint` + does. +- **Pass matrix/expression values through `env:`, don't inline them in `run:`.** A dynamic + `matrix: ${{ fromJSON(...) }}` defeats zizmor's static analysis, which then flags its expansion inside + `run:` as template injection. The fix is to route the value through `env:` — the remedy endorsed in + `.github/zizmor.yml` — not to suppress the rule. The same applies to any secret used in a `run:` step: + write it to a file via an intermediate `env:` var rather than inlining `${{ secrets.* }}`. +- **A SQL-Server test leg can die with a native segfault** (exit 139, in the DB driver). It hits `main` + too — it is not a regression in your PR. Clear it with `gh run rerun --failed`. +- **`prod` is a fail-closed PHI environment.** `serve --env prod` refuses to start without a store + encryption key, without an egress allow-list, and without bounded retention windows + (`[retention].messages_days` **and** `dead_letter_days` must be `> 0`). Any prod-like CI job must + supply all three or the service crash-loops and never serves `/health`. +- **Git-Bash mangles `git show :`** (the colon). Use + `MSYS_NO_PATHCONV=1 git show "origin/main:.github/workflows/ci.yml"`. diff --git a/docs/CONTRIBUTOR-PROGRAM-PLAN.md b/docs/CONTRIBUTOR-PROGRAM-PLAN.md index 9386f76..fca4e4e 100644 --- a/docs/CONTRIBUTOR-PROGRAM-PLAN.md +++ b/docs/CONTRIBUTOR-PROGRAM-PLAN.md @@ -1,12 +1,12 @@ -# Contributor Program Plan — from solo developer to an open contributor community +# Contributor Program — decision record & community plan -> **Status: Draft for owner review.** This is a *plan*, not an implementation. No governance -> files, workflow changes, or repo settings are created until the owner says "go". Authored in the -> `planning-contributors` worktree (branch `planning-contributors`). -> -> **As-of:** authored against `main` @ `9866a97` (2026-06-14); `main` has since advanced. Contributor -> go-live is sequenced against the **v0.1 "enterprise-ready" milestone** (see the -> [v0.1 release plan](releases/v0.1-PLAN.md) and [`EARLY-ADOPTER-GUIDE.md`](EARLY-ADOPTER-GUIDE.md)). +> **Status: Decision made (2026-07).** `MEFORORG/MessageFoundry` is the **single active development +> repository**. The former one-way publish pipeline is **retired**, and the previously-private +> `wshallwshall/MessageFoundry` is retained only as an **inactive, read-only archive** (never deleted; +> it holds the full history and internal-only material). This document records that decision and the +> community/governance plan that follows from it. Community go-live is still sequenced against the +> **v0.1 "enterprise-ready" milestone** (see the [v0.1 release plan](releases/v0.1-PLAN.md) and +> [`EARLY-ADOPTER-GUIDE.md`](EARLY-ADOPTER-GUIDE.md)). --- @@ -18,19 +18,34 @@ Three forces pull against each other and shape every decision below: -1. **PHI/healthcare safety.** This engine carries PHI. The private repo was made private on - 2026-06-10 after customer data once leaked into a public repo. Outside contributors must - *never* be able to see real PHI or customer connection data. +1. **PHI/healthcare safety.** This engine carries PHI. Outside contributors must *never* be able to see + real PHI or customer connection data. The repository is PHI-clean **by construction** — real + migration/customer data lives in a *separate, git-ignored* location and is never tracked here. 2. **Architectural integrity.** The "no channel object", code-first Router/Handler model, the reliability invariants, and the PHI guardrails are easy to erode with well-meaning PRs. Contribution - has to be gated on understanding, not just green CI. + is gated on understanding, not just green CI. 3. **Solo-maintainer bandwidth.** One person cannot absorb unbounded triage, review, and community - management. Every process below is designed to be runnable by **one** maintainer and to *scale down* - gracefully, then add a second maintainer deliberately. + management. Every process below is runnable by **one** maintainer and *scales down* gracefully, then + adds a second maintainer deliberately. --- -## 1. Where we are today (inventory) +## 1. The repository model (decided) + +Development happens **in the open** on `MEFORORG/MessageFoundry`. Outside PRs are ordinary GitHub PRs +against `main`; there is no snapshot/mirror boundary to bridge and no SHA divergence. The whole PHI +air-gap therefore rests on the always-on leak gate holding **100% of the time**: the +`forbidden-content` scan (with `gitleaks`) runs as a **required PR check** and on `main`, fail-closed, +so an inbound PR carrying a secret, routable IP, or customer string fails *before* a human reads it. +Treat any bypass of that gate as a Sev-1. + +This is the only model with a *standard, sustainable* contribution flow — full Issues / PRs / +Discussions / Projects on one repo, no per-PR replay cost. The tradeoff (no snapshot "air gap") is +accepted and mitigated by the required leak gate above and the PHI rules in §5. + +--- + +## 2. Where we are today (inventory) **Already built — reuse, don't recreate:** @@ -42,94 +57,41 @@ Three forces pull against each other and shape every decision below: | CLA enforcement | ✅ CLA Assistant bot wired | `.github/workflows/cla.yml` | | Security policy | ✅ private disclosure + remediation SLAs | `.github/SECURITY.md` | | Issue templates | ✅ bug / feature / config | `.github/ISSUE_TEMPLATE/` | -| CI / security gates | ✅ quartet (ruff/format/mypy/pytest) + bandit/gitleaks | `.github/workflows/{ci,security}.yml` | +| CI / security gates | ✅ tests/lint/types + bandit/semgrep/gitleaks + required leak gate | `.github/workflows/{ci,security}.yml`, [`docs/CI.md`](CI.md) | | Dependabot | ✅ | `.github/dependabot.yml` | -| Public OSS mirror | ✅ one-way curated snapshot, fail-closed scan gate | `scripts/publish/` | +| Code of Conduct | ✅ Contributor Covenant | [`CODE_OF_CONDUCT.md`](../CODE_OF_CONDUCT.md) | -**Missing — the governance & community layer this plan adds:** +**Missing — the governance & community layer this plan tracks:** -- ❌ `CODE_OF_CONDUCT.md` (table stakes for any public project; GitHub surfaces its absence) -- ❌ `GOVERNANCE.md` — who decides, how, and the maintainer ladder -- ❌ `MAINTAINERS.md` / `CODEOWNERS` — who owns which subsystem; bus-factor plan -- ❌ `.github/PULL_REQUEST_TEMPLATE.md` — the contributor's pre-merge checklist -- ❌ A public **roadmap** + curated **`good first issue` / `help wanted`** on-ramp -- ❌ A **support/discussion** channel (GitHub Discussions) distinct from the issue tracker -- ❌ A **triage cadence** and labeling scheme -- ❌ Contributor **recognition** (CONTRIBUTORS file / all-contributors) -- ❌ **The defined inbound path** for outside PRs (see §2 — the crux) +- `GOVERNANCE.md` — who decides, how, and the maintainer ladder (§3). +- `MAINTAINERS.md` / `CODEOWNERS` — who owns which subsystem; bus-factor plan. +- `.github/PULL_REQUEST_TEMPLATE.md` — the contributor's pre-merge checklist. +- A public **roadmap** + curated **`good first issue` / `help wanted`** on-ramp. +- A **support/discussion** channel (GitHub Discussions) distinct from the issue tracker. +- A **triage cadence** and labeling scheme. +- Contributor **recognition** (CONTRIBUTORS file / all-contributors). --- -## 2. The crux: the repo model (owner decision #1) - -This is the **single most important decision** and everything downstream depends on it. - -Today the topology is **private-primary + one-way public mirror**: - -- `MEFORORG/MessageFoundry` (**private**) is the source of truth. -- `MEFORORG/MessageFoundry` (**public**) is a *history-free, curated snapshot* produced by - `scripts/publish/publish.ps1` — it shares **no SHAs** with the private repo and is **publish-only**. - -**The problem:** there is **no inbound path**. A contributor can only see and PR the *public mirror*, -but the mirror is regenerated wholesale on each publish — an outside PR against it has nowhere to go -and would be obliterated on the next snapshot. You cannot run a contributor program on a publish-only -mirror. One of three models must be chosen: - -### Option A — Flip to public-primary *(recommended, at the v0.1 tag)* -Make `MEFORORG/MessageFoundry` the **real development repo**. The owner develops in the open; outside -PRs are ordinary GitHub PRs. Retire the one-way publish flow for OSS dev; keep a *private* repo only -for commercial/customer-specific artifacts if any exist. - -- **Why it works here:** the repo is already **PHI-clean by construction** — real migration/customer - data lives in the *separate* git-ignored `migration-local/` repo, never in the engine repo. The - `publish.ps1` curation (deny-list, slug rewrite) is a small, one-time fold-in. -- **Pro:** the only model with a *standard, sustainable* contribution flow. No bridge to maintain, no - SHA divergence, full issue/PR/Discussions/Projects on one repo. -- **Con:** loses the snapshot "air gap"; discipline (the `scan_forbidden.py` + gitleaks pre-push gate, - now run as required CI) must hold permanently. Mitigated — see §5. -- **Recommended timing:** flip **at the v0.1 GA tag**, not before (see §7). Pre-v0.1, solo velocity - matters more than openness. - -### Option B — Keep dual-repo, build an inbound bridge -Contributors PR the public mirror; a defined process replays accepted public PRs onto private `main`, -then re-publishes. -- **Pro:** preserves the snapshot air gap. -- **Con:** every accepted PR is hand-replayed across a SHA boundary (contributor loses authorship - continuity, `git blame` fractures, attribution is manual). **High per-PR maintainer cost** — the - opposite of what a solo maintainer needs. Not recommended. - -### Option C — Stay private-primary, closed to outside code (status quo+) -Accept *issues and discussions* publicly but keep code contribution closed until later. -- **Pro:** zero new risk; lets you build community/feedback before opening code. -- **Con:** not actually a contributor program for *code*; defers the real decision. -- **Use as:** the **interim** state between now and the Option-A flip (see §7, Phase 0–1). - -> **Recommendation:** **C now → A at v0.1.** Open *issues/discussions* immediately (low risk, builds -> signal), and **flip to public-primary at the v0.1 tag** so code contribution opens on a sustainable -> topology. Reject Option B — the bridge cost is unsustainable for a solo maintainer. - ---- - -## 3. Governance model (owner decision #2) +## 3. Governance model A solo project needs an *honest* governance doc, not a pretend committee. Proposed `GOVERNANCE.md`: - **Model: BDFL / single steward, explicitly.** The owner is the sole maintainer and final - decision-maker today. Say so plainly — pretending otherwise erodes trust faster than honesty. + decision-maker today. Say so plainly. - **Decision-making:** *lazy consensus* on issues/PRs (silence = assent after a stated window); the - steward breaks ties. **Architectural changes go through an ADR** (the project already uses - `docs/adr/` — make ADR-or-it-didn't-happen the rule for anything touching the invariants). + steward breaks ties. **Architectural changes go through an ADR** (`docs/adr/`) — ADR-or-it-didn't- + happen for anything touching the invariants. - **The maintainer ladder** (how trust is earned, so the bus factor can grow deliberately): 1. **Contributor** — anyone with a merged PR. - 2. **Triager** — issue/PR labeling + triage rights. Earned by sustained, accurate triage help. - 3. **Maintainer (committer)** — merge rights to a subsystem via `CODEOWNERS`. Earned by a track - record of high-quality PRs *and* good review judgment. Requires the steward's invitation. + 2. **Triager** — issue/PR labeling + triage rights, earned by sustained accurate triage help. + 3. **Maintainer (committer)** — merge rights to a subsystem via `CODEOWNERS`, earned by a track + record of high-quality PRs *and* good review judgment; requires the steward's invitation. 4. **Steward** — the owner; holds admin, release, security-advisory, and tie-break authority. -- **Bus factor is a named risk.** A single steward means a single point of failure for security - advisories and releases. Goal: recruit **one** trusted second maintainer before contribution volume - makes solo review the bottleneck — *and* before relying on the security-advisory process (a private - advisory team of one is fragile). Track this as an explicit milestone, not a someday. -- **Scope boundaries for contributions** (set expectations up front to avoid heartbreak PRs): +- **Bus factor is a named risk.** A single steward is a single point of failure for security advisories + and releases. Goal: recruit **one** trusted second maintainer before review volume makes solo review + the bottleneck. Track this as an explicit milestone. +- **Scope boundaries for contributions:** - **Welcome:** bug fixes w/ tests, new **Connections/transports** (registry-pluggable by design), docs, example Routers/Handlers, generators, test coverage, perf with benchmarks. - **Discuss-first (ADR/issue before code):** anything touching the reliability invariants, the @@ -141,31 +103,30 @@ A solo project needs an *honest* governance doc, not a pretend committee. Propos ## 4. The contributor experience (artifacts to create) -On "go", create these (each is small; none is engine code): - -1. **`CODE_OF_CONDUCT.md`** — adopt **Contributor Covenant v2.1** verbatim; enforcement contact = - the steward's security/abuse email. (Lowest-effort, highest-signal community artifact.) -2. **`GOVERNANCE.md`** — §3 above. -3. **`MAINTAINERS.md` + `.github/CODEOWNERS`** — start with the steward owning everything; pre-mark - the *sensitive* paths (`messagefoundry/auth/`, `store/`, `transports/`, `api/security*`, - `docs/SECURITY.md`, `docs/Secure_Development_Standards.md`, `scripts/publish/`) so that when a 2nd - maintainer joins, sensitive review still routes to the steward. -4. **`.github/PULL_REQUEST_TEMPLATE.md`** — checklist: *tests added; quartet green - (`messagefoundry check`); no real PHI; CLA agreed; docs/ADR updated if behavior/architecture - changed; uses Connection/Router/Handler vocabulary; no new declarative-channel/GUI-in-engine/Black*. -5. **Labels + on-ramp** — `good first issue`, `help wanted`, `needs triage`, `discuss-first`, - `area:*` (transport/store/api/console/parsing/auth/docs). Curate **5–10 genuinely small** - first issues before announcing — an empty on-ramp kills momentum. -6. **Public roadmap** — largely in place already (the README roadmap, [`FEATURE-MAP.md`](FEATURE-MAP.md), - and the built-vs-experimental map in [`EARLY-ADOPTER-GUIDE.md`](EARLY-ADOPTER-GUIDE.md)); optionally - add a GitHub Projects board so contributors can see what's actively being worked on. -7. **GitHub Discussions** — enable as the Q&A/design forum, distinct from Issues (bugs/features) and - Security advisories (vulns). Keep everything on GitHub for auditability; defer chat (Discord/Slack). -8. **Contributor recognition** — a `CONTRIBUTORS` file or the all-contributors bot; credit security - reporters per the existing SECURITY.md. -9. **Refresh `CONTRIBUTING.md`** — add: the `python -m messagefoundry check` gate (already mentioned), - the worktree workflow ([`WORKTREES.md`](WORKTREES.md)) for parallel work, the triage/label legend, - a pointer to GOVERNANCE/Code of Conduct, and the "discuss-first" scope boundaries from §3. +Each is small; none is engine code: + +1. **`GOVERNANCE.md`** — §3 above. +2. **`MAINTAINERS.md` + `.github/CODEOWNERS`** — start with the steward owning everything; pre-mark the + *sensitive* paths (`messagefoundry/auth/`, `store/`, `transports/`, `api/security*`, the security + policy, and `scripts/security/`) so that when a 2nd maintainer joins, sensitive review still routes + to the steward. +3. **`.github/PULL_REQUEST_TEMPLATE.md`** — checklist: *tests added; gates green + (`python -m messagefoundry check`); no real PHI; CLA agreed; docs/ADR updated if + behavior/architecture changed; uses Connection/Router/Handler vocabulary; no new + declarative-channel / GUI-in-engine / Black.* +4. **Labels + on-ramp** — `good first issue`, `help wanted`, `needs triage`, `discuss-first`, + `area:*` (transport/store/api/console/parsing/auth/docs). Curate **5–10 genuinely small** first + issues before announcing — an empty on-ramp kills momentum. +5. **Public roadmap** — largely in place (the README roadmap, [`FEATURE-MAP.md`](FEATURE-MAP.md), and + the built-vs-experimental map in [`EARLY-ADOPTER-GUIDE.md`](EARLY-ADOPTER-GUIDE.md)); optionally add + a GitHub Projects board. +6. **GitHub Discussions** — enable as the Q&A/design forum, distinct from Issues (bugs/features) and + Security advisories (vulns). Keep everything on GitHub for auditability; defer chat. +7. **Contributor recognition** — a `CONTRIBUTORS` file or the all-contributors bot; credit security + reporters per the existing security policy. +8. **Refresh `CONTRIBUTING.md`** — the `python -m messagefoundry check` gate, the triage/label legend, + a pointer to GOVERNANCE / Code of Conduct / [`docs/CI.md`](CI.md), and the "discuss-first" scope + boundaries from §3. --- @@ -173,110 +134,85 @@ On "go", create these (each is small; none is engine code): This is a healthcare engine; these are hard gates, not nice-to-haves. -- **Contributors only ever touch the PHI-clean repo.** Under Option A the public repo contains zero - PHI/customer data *by construction* (migration artifacts stay in the separate git-ignored repo). - Reaffirm this boundary in CONTRIBUTING + Code of Conduct: **no real PHI or customer data in issues, - PRs, tests, fixtures, or screenshots — synthetic HL7 only** (`messagefoundry generate`). -- **The forbidden-content scan becomes required CI on inbound PRs.** Today `scan_forbidden.py` + - gitleaks run as a *pre-publish* gate. Re-wire them as a **required PR check** (and on `main`) so an - outside PR carrying a secret/IP/customer string fails closed *before* a human reads it. Run untrusted - PRs with `pull_request_target` hardening / minimal token scope so fork PRs can't exfiltrate secrets. +- **Contributors only ever touch the PHI-clean repo.** The repository contains zero PHI/customer data + *by construction* (migration artifacts stay in a separate git-ignored location). Reaffirm this in + CONTRIBUTING + Code of Conduct: **no real PHI or customer data in issues, PRs, tests, fixtures, or + screenshots — synthetic HL7 only** (`messagefoundry generate`). +- **The forbidden-content scan is required CI on inbound PRs.** `scan_forbidden.py` + `gitleaks` run as + **required PR checks** (and on `main`), fail-closed, so an outside PR carrying a secret/IP/customer + string fails *before* a human reads it. Fork PRs run with minimal token scope so they can't exfiltrate + secrets; the real token list is never committed (it loads from a git-ignored local file plus an + Actions secret). - **Executed-Python config is a trust boundary.** Routers/Handlers are *code that runs in-process*. Example-config contributions get read with that in mind; never auto-execute untrusted contributed config in CI without sandboxing. Document this in the PR review checklist. -- **Security disclosure stays private** (existing SECURITY.md). Do **not** route vulns through public - issues/Discussions. The advisory team must reach **≥2 people** once a second maintainer exists - (today: steward only — note it as a known single-point risk). -- **CLA before first merge** (already enforced by the bot). **Gate:** the CLA + the "MessageFoundry - Organization" entity it names should get a **lawyer review** before the program is announced (the - CLA.md template already flags this). Tie this to the open-core/commercial intent. +- **Security disclosure stays private** (existing `.github/SECURITY.md`). Do **not** route vulns through + public issues/Discussions. The advisory team must reach **≥2 people** once a second maintainer exists + (today: steward only — a known single-point risk). +- **CLA before first merge** (already enforced by the bot). **Gate:** the CLA and the entity it names + should get a **lawyer review** before the program is announced. - **Branch protection, contributor-mode.** Today protections are tuned for a solo dev (CI-gated, no required human review — a solo dev can't self-approve). When contributions open: require **≥1 maintainer approval on external PRs**, keep all CI checks + CLA required, require **CODEOWNERS review - on sensitive paths**, keep "no direct push to `main`", and retain a **logged** admin-bypass for - solo emergencies. (See [[mf-solo-developer]] memory — this supersedes the "no required reviews" - guidance *once there is someone other than the owner to review*.) + on sensitive paths**, keep "no direct push to `main`", and retain a **logged** admin-bypass for solo + emergencies. --- ## 6. Triage & sustainability (so one person can run this) - **Weekly triage pass**, time-boxed: label new issues, close stale/dupes, tag `good first issue`. -- **Response SLA you can actually keep:** acknowledge new issues/PRs within ~1 week; be explicit in - CONTRIBUTING that this is a small project so reviews may take time. Under-promise. +- **Response SLA you can actually keep:** acknowledge new issues/PRs within ~1 week; be explicit that + this is a small project so reviews may take time. Under-promise. - **Bots do the toil:** CLA Assistant (have), Dependabot (have); add **stale-bot** for abandoned - issues/PRs and optionally a triage/label automation. Every bot added is maintainer time returned. + issues/PRs and optionally triage/label automation. - **"Discuss-first" deflects expensive PRs early** — the label + scope boundaries in §3 stop a - contributor from spending a weekend on something that will be declined on principle. -- **ADR discipline scales review:** if the architecture rationale is written down, you re-explain it - by linking, not retyping. + contributor spending a weekend on something that will be declined on principle. +- **ADR discipline scales review:** if the rationale is written down, you re-explain it by linking, not + retyping. --- -## 7. Phased rollout (sequenced against v0.1) - -Each phase has an explicit entry gate. **Nothing starts until the owner says "go" on Phase 0.** +## 7. Community rollout (sequenced against v0.1) -**Phase 0 — Foundation (pre-v0.1, low risk, while still private-primary).** -Create the *paper* governance layer with no topology change: `CODE_OF_CONDUCT.md`, `GOVERNANCE.md`, -`MAINTAINERS.md`, `CODEOWNERS`, `PULL_REQUEST_TEMPLATE.md`, refreshed `CONTRIBUTING.md`. Get the **CLA -lawyer-reviewed**. These can land on private `main` and ride the next publish to the mirror. +**Phase 0 — Foundation.** Create the *paper* governance layer: `GOVERNANCE.md`, `MAINTAINERS.md`, +`CODEOWNERS`, `PULL_REQUEST_TEMPLATE.md`, refreshed `CONTRIBUTING.md`. Get the **CLA lawyer-reviewed**. *Exit gate:* docs merged; CLA legally cleared. -**Phase 1 — Open the front door (issues/discussions only; Option C).** -Enable **Discussions** + public **issue** intake on the mirror; publish the roadmap; curate the first -`good first issue` set. *Still no outside code merges yet* (the mirror is publish-only). This builds -signal and surfaces a possible second maintainer at near-zero risk. -*Exit gate:* v0.1 GA tagged; the four v0.1 hard gates (PHI log redaction, no "experimental" backends, +**Phase 1 — Open the front door (issues/discussions).** Enable **Discussions** + public **issue** +intake; publish the roadmap; curate the first `good first issue` set. This builds signal and surfaces a +possible second maintainer at near-zero risk. +*Exit gate:* v0.1 GA tagged; the v0.1 hard gates (PHI log redaction, no "experimental" backends, published throughput baseline, off-loopback/TLS) met per the [v0.1 release plan](releases/v0.1-PLAN.md). -**Phase 2 — Flip to public-primary (Option A) and open code contribution.** -Fold `publish.ps1`'s curation into the public repo as required CI; make `MEFORORG/MessageFoundry` the -dev repo; apply contributor-mode branch protection (§5); announce "open for contributions". +**Phase 2 — Open code contribution.** Apply contributor-mode branch protection (§5); announce "open for +contributions". *Exit gate:* `scan_forbidden`/gitleaks green as required PR checks; contributor-mode protections live; first external PR merged end-to-end as a dry run. -**Phase 3 — Grow the bus factor.** -Identify and invite a **second maintainer**; populate `CODEOWNERS`/`MAINTAINERS.md`; bring them onto -the security-advisory team. Revisit governance (BDFL → small maintainer team) only if/when volume -warrants — not before. +**Phase 3 — Grow the bus factor.** Identify and invite a **second maintainer**; populate +`CODEOWNERS`/`MAINTAINERS.md`; bring them onto the security-advisory team. Revisit governance (BDFL → +small maintainer team) only if/when volume warrants. *Exit gate:* a second maintainer has merge rights and advisory access. --- -## 8. Owner decisions needed before Phase 0 starts +## 8. Owner decisions still open -1. **Repo model & timing** (§2) — confirm **C-now → A-at-v0.1**, or choose B / a different timing. -2. **Governance model** (§3) — confirm **BDFL/single-steward, documented honestly**, with the ladder. -3. **CLA path** (§5) — ✅ **DECIDED 2026-06-14: keep the open-core CLA** (not DCO), preserving the - dual-license/commercial-edition path. Remaining action (owner): lawyer-review the - [`CLA.md`](../CLA.md) template and confirm the "MessageFoundry Organization" entity it names, - **before** the program is publicly announced. Not a blocker for merging the Phase 0 governance docs. -4. **Code of Conduct** — Contributor Covenant v2.1 (recommended) vs. another. -5. **Communication surface** — GitHub Discussions only (recommended) vs. add chat now. -6. **The "MessageFoundry Organization"** — is this a formed legal entity, or does the CLA wording need - adjusting to the actual owner/entity? (Affects the CLA lawyer review.) +1. **CLA lawyer review** — clear the [`CLA.md`](../CLA.md) template and confirm the entity it names, + **before** the program is publicly announced. (Not a blocker for merging the Phase 0 governance docs.) +2. **Communication surface** — GitHub Discussions only (recommended) vs. add chat now. +3. **Second maintainer** — identify a candidate to begin Phase 3. --- ## 9. Open questions / risks -- **AGPL + open-core friction.** The AGPL §13 + relicensing CLA is a deliberate open-core posture, but - some contributors decline CLAs on principle. Accept a (likely small) contributor-pool cost; this is - a values call the owner has already leaned into. +- **AGPL + open-core friction.** The AGPL §13 + relicensing CLA is a deliberate open-core posture; some + contributors decline CLAs on principle. Accept a (likely small) contributor-pool cost. - **Single security-advisory contact** is a real single point of failure until Phase 3. -- **Mirror-flip discipline** (Option A) puts the whole PHI air-gap on the CI scan gate holding 100% of - the time. The gate is fail-closed today; treat any bypass as a Sev-1. -- **Contributions vs. the fork-based component SDK vision** ([[mf-vision-and-plan]]): the long-term - model is a read-only SDK users *fork to customize*. Clarify for contributors what belongs **upstream** - (core engine, transports, fixes) vs. what is a **downstream fork** (their site-specific - Routers/Handlers) — so the contribution funnel points at the right target. - ---- - -## 10. What happens on "go" - -On an explicit "go" for **Phase 0**, the work is: author the six governance/community files in §4 (1–4, -9) on this `planning-contributors` branch, open a PR, and run the quartet. Phases 1–3 each get their own -"go" gated on the exits above. No repo settings, no announcements, and no CLA reliance happen without -the corresponding decision in §8 being made first. +- **The PHI air-gap rests entirely on the CI leak gate** holding 100% of the time. The gate is + fail-closed; treat any bypass as a Sev-1. +- **Contributions vs. the fork-based component SDK vision:** the long-term model is a read-only SDK + users *fork to customize*. Clarify for contributors what belongs **upstream** (core engine, + transports, fixes) vs. what is a **downstream fork** (their site-specific Routers/Handlers). diff --git a/docs/PHI.md b/docs/PHI.md index fd6b42b..e0f0f2c 100644 --- a/docs/PHI.md +++ b/docs/PHI.md @@ -1062,7 +1062,7 @@ Properties of the anonymizer: `AnonError`) — it never emits an un-scrubbed body. Surfaces: the **`messagefoundry tee anonymize-captures`** subcommand and the test-harness -`CaptureSink`/corpus hooks. [`scripts/publish/scan_forbidden.py`](../scripts/publish/scan_forbidden.py) +`CaptureSink`/corpus hooks. [`scripts/security/scan_forbidden.py`](../scripts/security/scan_forbidden.py) is now the **single leak-token source-of-truth** (a fail-closed leak gate). HL7 v2 is supported first; X12/FHIR seams come later. diff --git a/docs/Secure_AI_Development_Standards.md b/docs/Secure_AI_Development_Standards.md index 8ded944..e864f86 100644 --- a/docs/Secure_AI_Development_Standards.md +++ b/docs/Secure_AI_Development_Standards.md @@ -166,7 +166,7 @@ Ordered rules, mirroring [AI.md](AI.md)'s `resolve_effective_policy()` clamp ord | Floor control | Why T0 still has it | |---|---| | **No real PHI to the AI**, and **none in any commit, test fixture, published artifact, shared memory, screenshot, or CI log** — by default, every tier. **One narrow exception** (PHI *to the AI* only): a signed **BAA + zero-data-retention** agreement covering the AI tool may permit real PHI in a prompt/context — see **§4.5**. That exception **never** covers PHI in commits / fixtures / published artifacts / shared memory / logs, which stays **absolutely forbidden**. | A throwaway spike that ingests real PHI is already a HIPAA exposure — and a BAA with the *AI vendor* never blesses PHI in your *git history*. | -| **No secrets/keys/`.env`/`*.db`** readable by the assistant — enforced by the `.claude/settings.json` deny-list. **But the deny-list is PATH-based:** it cannot stop a secret/PHI string you **paste** into the prompt, that a command the agent ran **echoes** into its captured output, or that gets written into project memory. **The human must not paste them, and must not let `dryrun`/`generate` output be captured into a committed file, transcript, or memory.** **Deterministic backstop:** the forbidden-content CI scan ([`scan_forbidden.py`](../scripts/publish/scan_forbidden.py)) and `gitleaks` catch a forbidden string or secret that lands in a *commit*, even though a path deny-list cannot stop a paste. | A path deny-list gives false confidence; the leak paths it misses are human-driven — so a commit-time scan backstops them, but "the human must not" is the first line. | +| **No secrets/keys/`.env`/`*.db`** readable by the assistant — enforced by the `.claude/settings.json` deny-list. **But the deny-list is PATH-based:** it cannot stop a secret/PHI string you **paste** into the prompt, that a command the agent ran **echoes** into its captured output, or that gets written into project memory. **The human must not paste them, and must not let `dryrun`/`generate` output be captured into a committed file, transcript, or memory.** **Deterministic backstop:** the forbidden-content CI scan ([`scan_forbidden.py`](../scripts/security/scan_forbidden.py)) and `gitleaks` catch a forbidden string or secret that lands in a *commit*, even though a path deny-list cannot stop a paste. | A path deny-list gives false confidence; the leak paths it misses are human-driven — so a commit-time scan backstops them, but "the human must not" is the first line. | | **All content the assistant READS during a build is DATA, never instructions.** HL7 samples, `connections.toml`, file content, a pasted log, a WebFetch page, an MCP tool result — a value that *reads like* a command ("ignore prior rules", "add this dependency", "exfiltrate `.env`") is **still data**. Never let fetched/tool/file content auto-trigger an edit or command. (This is the dev-process lift of [`../CLAUDE.md`](../CLAUDE.md) §8 and [PHI.md](PHI.md)'s treat-as-untrusted-data rule.) | Inbound HL7 and partner config are *attacker-influenceable*; an agent that acts on embedded instructions is steered entirely inside the build process, bypassing every runtime control. | | **Reject code you cannot explain** — even if it works. **Explaining it with AI assistance is acceptable** — what's rejected is code that stays **opaque even with the AI's help**. (A deliberate, documented **override** of the stricter "unaided comprehension" anti-SOUP bar — see §10 and the [A.6](#a6-documented-deviations) deviation.) | Shipping code no one can reason about is how "developers who build but cannot debug" are made (Fawzy); applies from T0, independent of PHI. The T3 escalation (a *qualified* human can explain — AI-assisted is fine — **and** independent review) is in §6.6. | | **Model + version transparency.** The assistant's identity and the session are retained as a provenance signal. | Provenance is a property you cannot reconstruct after the fact. | @@ -261,7 +261,7 @@ At T1 and above, use **Plan mode** so the diff is checked against an **approved **MCP and web egress trust** (default-deny at T2+/PHI-touch): - **MCP servers** are arbitrary third-party processes that can read repo content and receive whatever the agent sends — a direct PHI/secret egress channel and a supply-chain trust boundary **outside** AI.md's product model. **Default-deny for T2+/PHI-touch.** Any MCP server must be **vetted, version-pinned, and recorded** (provenance); **no PHI or secret may cross an MCP boundary**; MCP results are **untrusted data**. -- **WebSearch / WebFetch** are live egress: **no PHI, secret, or customer-identifying string in any query or tool call** (a partner name, internal hostname, or config fragment). The repo's [`scan_forbidden.py`](../scripts/publish/scan_forbidden.py) backstops forbidden strings that **land in a commit / the published mirror** — it is **not** a live interceptor of a WebSearch/WebFetch/MCP query, so runtime egress discipline is on the human. **At T3, web egress is off or reviewed.** +- **WebSearch / WebFetch** are live egress: **no PHI, secret, or customer-identifying string in any query or tool call** (a partner name, internal hostname, or config fragment). The repo's [`scan_forbidden.py`](../scripts/security/scan_forbidden.py) backstops forbidden strings that **land in a commit / the published mirror** — it is **not** a live interceptor of a WebSearch/WebFetch/MCP query, so runtime egress discipline is on the human. **At T3, web egress is off or reviewed.** *Maps to:* Principle 2 (context engineering) · SSDF **PO.3, PO.5** · ASVS V14 (config), V8. @@ -433,7 +433,7 @@ The repo's tiered-honesty taxonomy, applied to the **dev-process tooling itself* - PreToolUse [`block-blanket-git-stage.ps1`](../scripts/hooks/block-blanket-git-stage.ps1) (Bash + PowerShell, fail-open). - [`.claude/settings.json`](../.claude/settings.json) secrets/keys/`*.db` **path-based** deny-list + destructive-command denies. -- Blocking security CI: bandit, semgrep ([`.semgrep/messagefoundry.yml`](../.semgrep/messagefoundry.yml)), pip-audit, gitleaks, crypto-inventory, forbidden-content ([`scripts/publish/scan_forbidden.py`](../scripts/publish/scan_forbidden.py)). The CycloneDX **SBOM** job is **advisory** (`continue-on-error`), not blocking. +- Blocking security CI: bandit, semgrep ([`.semgrep/messagefoundry.yml`](../.semgrep/messagefoundry.yml)), pip-audit, gitleaks, crypto-inventory, forbidden-content ([`scripts/security/scan_forbidden.py`](../scripts/security/scan_forbidden.py)). The CycloneDX **SBOM** job is **advisory** (`continue-on-error`), not blocking. - **Dependency-CVE fast response (SSDF RV.2 evidence):** dependency-vulnerability metrics ([`vuln-metrics.yml`](../.github/workflows/vuln-metrics.yml)), scoped Dependabot auto-merge + supply-chain cooldown ([`dependabot-auto-merge.yml`](../.github/workflows/dependabot-auto-merge.yml)), auto lock-resync ([`dependabot-lock-resync.yml`](../.github/workflows/dependabot-lock-resync.yml)), and the adopter vulnerable-pin CI tripwire. The **`CI gate` roll-up** required check gates the conditional/matrix legs in [`ci.yml`](../.github/workflows/ci.yml). *(This is the **audit/known-CVE** posture — distinct from the still-deferred hallucinated/typosquatted **new-dependency-introduction** check below.)* - [`messagefoundry check`](../messagefoundry/checks.py) exit-coded validate + dryrun gate. - SessionStart worktree-context hook; worktree scripts ([`new.ps1`/`remove.ps1`](../scripts/worktree/)); shared AI project memory (facts only); synthetic generators; the dependency-boundary test ([`tests/test_dependency_boundaries.py`](../tests/test_dependency_boundaries.py)). @@ -523,7 +523,7 @@ MEFOR is an open-source HL7 v2.x integration engine (Python; FastAPI; SQLite/WAL | Lint/format/type/test + cross-DB store suites + the `CI gate` roll-up required check | [`.github/workflows/ci.yml`](../.github/workflows/ci.yml) | Built | | Dependency-CVE fast response (RV.2): vuln metrics, scoped Dependabot auto-merge + cooldown, auto lock-resync, adopter vulnerable-pin tripwire | [`vuln-metrics.yml`](../.github/workflows/vuln-metrics.yml), [`dependabot-auto-merge.yml`](../.github/workflows/dependabot-auto-merge.yml), [`dependabot-lock-resync.yml`](../.github/workflows/dependabot-lock-resync.yml) | Built | | Project SAST rules | [`.semgrep/messagefoundry.yml`](../.semgrep/messagefoundry.yml) | Built | -| Customer/PHI leak guard | [`scripts/publish/scan_forbidden.py`](../scripts/publish/scan_forbidden.py) | Built | +| Customer/PHI leak guard | [`scripts/security/scan_forbidden.py`](../scripts/security/scan_forbidden.py) | Built | | Human-gate stack | [PR template](../.github/PULL_REQUEST_TEMPLATE.md), [CODEOWNERS](../.github/CODEOWNERS), [`cla.yml`](../.github/workflows/cla.yml) | Built | | Engine-boundary import test (depends-on integrity) | [`tests/test_dependency_boundaries.py`](../tests/test_dependency_boundaries.py) | Built | | Listener cleartext-bind + source-IP **parity** guard (one check over all LISTEN connectors) | [`messagefoundry/pipeline/wiring_runner.py`](../messagefoundry/pipeline/wiring_runner.py) | Built | diff --git a/docs/Secure_Build_Scorecard_MEFOR.md b/docs/Secure_Build_Scorecard_MEFOR.md index ee5c03d..b1b7820 100644 --- a/docs/Secure_Build_Scorecard_MEFOR.md +++ b/docs/Secure_Build_Scorecard_MEFOR.md @@ -55,7 +55,7 @@ Status tags use the built / designed-but-deferred / aspirational taxonomy (SDS | 2 | Secure coding practices enforced | **Built — Strong** *(grade holds; asserted evidence overstated)* | `parsing/peek.py` + `parsing/validate.py` (ingress validation), `store/store.py` (bound-parameter SQL; f-strings interpolate only literal column names per STORE-4), `api/security.py` (deny-by-default 401/403), `store/crypto.py` (AES-GCM) + `auth/passwords.py` (argon2id). Overstated: "PW.5 all Pass" is false. The cited `SDS-CONFORMANCE-REVIEW-2026-06-12.md` marks REST/SOAP (:74), file-handler (:75) and TLS-in-transit (:76) Partial, and that review is an explicit self-review, not independent. The four specific items credited (ingress / param-SQL / fail-closed authz / vetted crypto) are Pass. | | 3 | Blocking security gates in CI, red-on-regression | **Built — Strong** | `.github/workflows/security.yml`: 7 jobs, no `continue-on-error`, none `if:`-gated, so all run on `pull_request`: pip-audit, npm-audit, bandit, gitleaks (`fetch-depth: 0`), `semgrep --error` (→ `.semgrep/messagefoundry.yml`, 5 dangerous-sink ERROR rules), crypto-inventory, forbidden-content. Honest residual verified: `continue-on-error: true` appears only on `sbom` (L104) + `trivy` (L137), both additionally cron/dispatch-only, never PR-gating. Nit: `.semgrep` header comment is stale, still calls semgrep "advisory". | | 4 | Dependency & supply-chain integrity | **Built — Strong** (3 sub-items open) | `security.yml` pip-audit runs DEP-1 (`uv lock --check` + byte-diff + `pip install --require-hashes` + `pip-audit -r requirements.lock`), daily SCA cron; `dependabot.yml` (uv/actions/npm) + fail-closed published-GHSA auto-merge gate (`dependabot-auto-merge.yml`). `release.yml` tag-time blocking CycloneDX SBOM + Sigstore keyless + SLSA `attest-build-provenance` + PyPI Trusted Publishing/PEP 740. Open (`SDS-REMEDIATION-PLAN.md`): CI SBOM blocking (4.1/:63), signed-commit/DCO (4.3/:65), PS.3 build-input archival (:64). | -| 5 | Secrets hygiene | **Built — Strong** | `security.yml` gitleaks full-history + forbidden-content, both blocking; `scripts/publish/scan_forbidden.py` fail-closed (exit 1 on hit, exit 2 on vacuous scan; customer-name / 54xxxx site-code / routable-IPv4 patterns) shared across `publish.ps1` GateScanner + `.pre-commit-config.yaml` + the CI job (`--published`); `.claude/settings.json` deny-list on `.env*`/`secrets/**`/`*.key`/`*.pem`/`*.pfx`/`*.db`. Secrets are `MEFOR_*`-env-only (`config/settings.py`). | +| 5 | Secrets hygiene | **Built — Strong** | `security.yml` gitleaks full-history + forbidden-content, both blocking; `scripts/security/scan_forbidden.py` fail-closed (exit 1 on hit, exit 2 on a vacuous or under-loaded scan; customer-name / site-code / routable-IPv4 / worktree-slug / home-path detectors) shared by `.pre-commit-config.yaml` and the CI job, both scanning the WHOLE tracked tree. The token list is externalized -- never committed -- and CI asserts a per-section detector floor so a partial or mangled token source fails the build rather than passing green; `.claude/settings.json` deny-list on `.env*`/`secrets/**`/`*.key`/`*.pem`/`*.pfx`/`*.db`. Secrets are `MEFOR_*`-env-only (`config/settings.py`). | | 6 | Secure-by-default, fail-closed configuration | **Built — Strong** (at-rest fail-closed for every PHI posture) | `messagefoundry/__main__.py` loopback-only no-auth refusal (:914-920), `--allow-insecure-bind` that cannot relax the no-auth or Posture-B refusals (:104-111), off-loopback TLS-revocation refusal (:1285-1338), keyless-PHI refusal in every env (:989-1004); `config/settings.py` `require_mfa=True` (:1391). At-rest mechanism (verified 2026-07-14): `serve` requires an env (`__main__.py:938`) and `require_posture()` (`settings.py:1625`) refuses a keyless start for any unresolved/custom-env posture; `_KNOWN_ENV_POSTURE` (`settings.py:1560`) is `dev→SYNTHETIC`, `staging→PHI`, `prod→PHI`, so the keyless-PHI gate refuses every PHI posture. No PHI posture runs cleartext. Keyless `IdentityCipher` cleartext (`store/crypto.py:454`) is reachable only on a synthetic posture or the audited `allow_unencrypted_phi` opt-out; `require_encryption=False` (:301) is a stricter-still guard, not the switch. Residual (risk-accepted, register standing note): an operator could misuse a synthetic-declared env for real PHI. Like the `allow_unencrypted_phi` opt-out, that is an operator responsibility, not an engine gap. | | 7 | Interface & transport authentication | **Built — Strong** (revocation/ECH deferred-by-design) | `transports/smart.py:180` (SMART `private_key_jwt`), `transports/mllp.py:468-556` (MLLP-over-TLS + mTLS, `CERT_REQUIRED`), `api/tls.py` + `api/tls_client_cert.py` (HTTPS/WSS + verified peer-cert, ADR 0083), `transports/soap.py` WS-Security + `parsing/xml/signature.py` enveloped XML-DSig (ADR 0015), `auth/totp.py` + `auth/webauthn.py` MFA; `config/tls_policy.py` `VERIFY_X509_STRICT`. Deferred-by-design: OCSP/CRL delegated to org PKI (ADR 0078; 12.1.4 canonical Fail — the register's Partial was reconciled to Fail 2026-07-14; start-time revocation refusal built); ECH absent (register 12.1.5 Fail). | | 8 | Audit & tamper-evident logging | **Built — Strong** | `store/store.py` `audit_row_hash()` (:803, SHA-256 or HMAC-SHA256 folding prior-row hash), `record_audit()` (:6113, INSERT-only), `verify_audit_chain()` (:6276) + `audit_anchor()` tail-truncation catch; `auth/service.py:1953` per-actor `_audit()`; PHI-redacted off-box tee `store/audit_tee.py` (default OFF). Tested: `tests/test_audit_integrity.py` + `tests/test_audit_offbox_tee.py`. Conservative note: TLS-syslog is actually built (`settings.py` `SyslogProtocol.TLS`, RFC 5425), not plan-only as an earlier pass hedged. | @@ -132,7 +132,7 @@ The canonical verdicts-of-record live in the cited `docs/security/` set. This sc **Enforcement artifacts (verify by file, not by prose):** - `.github/workflows/security.yml` — 7 blocking + 2 advisory gates (signal 3); `release.yml` — release-time SBOM/Sigstore/SLSA provenance (signals 4/12); `dependabot.yml`, `dependabot-auto-merge.yml`, `vuln-metrics.yml`. -- [`.github/SECURITY.md`](../.github/SECURITY.md) — RV.2 SLA windows + private disclosure channel (signal 9); `scripts/publish/scan_forbidden.py` — fail-closed forbidden-content, shared by publish/pre-commit/CI (signal 5); `.claude/settings.json` — secret/key/db deny-list. +- [`.github/SECURITY.md`](../.github/SECURITY.md) — RV.2 SLA windows + private disclosure channel (signal 9); `scripts/security/scan_forbidden.py` — fail-closed forbidden-content, shared by pre-commit and CI (signal 5); `.claude/settings.json` — secret/key/db deny-list. - `messagefoundry/__main__.py` — loopback bind + `--allow-insecure-bind` guard + keyless-PHI refusal (signal 6); `config/settings.py` — posture derivation + `require_mfa=True`; `store/crypto.py` + `store/keyprovider.py` — AES-GCM + unbuilt external HSM/KMS seams; `api/tls.py`, `transports/mllp.py`, `transports/smart.py`, `transports/fhir.py`, `auth/totp.py`, `auth/webauthn.py` — interface & transport auth (signal 7). **Reading notes.** (1) Several source-review Fail/Deferred findings are stale against HEAD (2026-07-14): SMART/FHIR connectors, TOTP + WebAuthn MFA, `release.yml`, and 19 git tags now exist versus the review's "absent/empty" verdicts. (2) Some line-number citations have drifted from moved code — for example approvals settings cited at `:1594/:807` versus actual `:2210/:1113`; the values match, the pinpoints do not. Verify against current line numbers. diff --git a/docs/adr/0030-anonymization-test-harness-tee.md b/docs/adr/0030-anonymization-test-harness-tee.md index 9b81cde..d1d7620 100644 --- a/docs/adr/0030-anonymization-test-harness-tee.md +++ b/docs/adr/0030-anonymization-test-harness-tee.md @@ -14,7 +14,7 @@ the harness capture sink ([`harness/reconcile/capture.py`](../../harness/reconcile/capture.py)) and file-replay loader ([`harness/reconcile/compare.py`](../../harness/reconcile/compare.py), [`harness/load/corpus.py`](../../harness/load/corpus.py)); and the publish leak-gate - [`scripts/publish/scan_forbidden.py`](../../scripts/publish/scan_forbidden.py) (`FORBIDDEN`). + [`scripts/security/scan_forbidden.py`](../../scripts/security/scan_forbidden.py) (`FORBIDDEN`). - **Decision in one line:** ship a **pure-stdlib, dependency-free `anon` package** that turns real, messy HL7 v2 into structurally-faithful **PHI-free** datasets via a **two-layer rule model — a declarative field-*selection* map (data) over a code registry of pure surrogate *functions* (logic)** — @@ -237,7 +237,7 @@ Every anonymized dataset is run through the **reconciled forbidden-token set** * blocks the whole output). The mechanism, spelled out so the SoT claim is honest: 1. **One authoritative token list.** The `FORBIDDEN` set in - [`scan_forbidden.py`](../../scripts/publish/scan_forbidden.py) becomes an **importable module-level + [`scan_forbidden.py`](../../scripts/security/scan_forbidden.py) becomes an **importable module-level constant/function** (today it is a CLI script with two callers — `publish.ps1` and the pre-commit hook; making it importable is net-new work, not mere "exposure"). 2. **Fold in the drifted copy.** `tests/test_load_config.py`'s `_FORBIDDEN_SUBSTRINGS` (adds the diff --git a/docs/adr/0149-multi-ecosystem-sbom-vex-and-sbom-quality-gate.md b/docs/adr/0149-multi-ecosystem-sbom-vex-and-sbom-quality-gate.md index ed95f5e..9611331 100644 --- a/docs/adr/0149-multi-ecosystem-sbom-vex-and-sbom-quality-gate.md +++ b/docs/adr/0149-multi-ecosystem-sbom-vex-and-sbom-quality-gate.md @@ -37,7 +37,7 @@ This must not weaken any existing invariant. In particular it must not touch the invariant](../../CLAUDE.md)** (staged queue / at-least-once) or the **count-and-log invariant** — these are build/release-pipeline and documentation changes only, with one new **stdlib-only, side-effect-free** helper script. It must also respect the publishing rule that **security-posture docs stay private** -([`scripts/publish/publish-denylist.txt`](../../scripts/publish/publish-denylist.txt) covers +(the retired publish deny-list covered `docs/security`); the SBOM/VEX and the operator guide are **transparency artifacts meant to be public**, so they live outside the deny-listed paths. diff --git a/docs/benchmarks/THROUGHPUT-EXECUTION-PLAN.md b/docs/benchmarks/THROUGHPUT-EXECUTION-PLAN.md index 61e13b4..c085e04 100644 --- a/docs/benchmarks/THROUGHPUT-EXECUTION-PLAN.md +++ b/docs/benchmarks/THROUGHPUT-EXECUTION-PLAN.md @@ -448,8 +448,8 @@ points to ~6.4 vCPU; if not, ~23 shards and no box helps — Phases C/F decide w (publish at ≤50% of the measured ceiling), with the **PROVES / does NOT prove** discipline from E2 carried forward — explicitly disclaiming the flat-vs-month-start-2.89×-peak, the adopter-SAN-vs-our-NVMe disk, multi-hour stability, and real-PHI byte sizes. Ratios only; no customer/site/partner/host/IP. Run - `python scripts/publish/scan_forbidden.py --published` first. - *Falsifier:* `scan_forbidden.py --published` flags content ⇒ a denylisted term leaked. — **🔍 FABLE REVIEW** + `python scripts/security/scan_forbidden.py --path .` first. + *Falsifier:* `scan_forbidden.py --path .` flags content ⇒ a denylisted term leaked. — **🔍 FABLE REVIEW** (a doc; the scan gate + review catch leaks cheaply). **Exit criteria.** A published, un-fabricated capability number at ≤50% of the measured ceiling, with its diff --git a/docs/releases/BACKLOG-EXECUTION-PLAN-2026-07-24.md b/docs/releases/BACKLOG-EXECUTION-PLAN-2026-07-24.md index 833dd7c..07182bc 100644 --- a/docs/releases/BACKLOG-EXECUTION-PLAN-2026-07-24.md +++ b/docs/releases/BACKLOG-EXECUTION-PLAN-2026-07-24.md @@ -180,7 +180,7 @@ allocated and written **when that item is actually built**, with the real design - **secret-rotation inventory** (any new `MEFOR_*` name ending `_PASSWORD`/`_TOKEN`/`_SECRET`/`_KEY`) → `CRITICAL_SECRETS` **and** the rotation-schedule table. - **route doc-drift** (any new API route) → `docs/SECURITY.md` route map + the count constants. - - **forbidden-content** — runs locally: `python scripts/publish/scan_forbidden.py --published`. ⚠️ It flags any + - **forbidden-content** — runs locally: `python scripts/security/scan_forbidden.py --path .`. ⚠️ It flags any dotted-decimal string as a routable IP, so a bare **X.509 OID literal** (e.g. the subjectAltName OID) trips it — use the symbolic constant (`x509.SubjectAlternativeName.oid`) instead. It also blocks the **customer name** anywhere outside the allowlisted `docs/security/*` — say "the ASVS drive-to-green cluster", never the diff --git a/docs/releases/HANDOFF-105-corepoint-verb-coverage.md b/docs/releases/HANDOFF-105-corepoint-verb-coverage.md index 0dca09e..76a0651 100644 --- a/docs/releases/HANDOFF-105-corepoint-verb-coverage.md +++ b/docs/releases/HANDOFF-105-corepoint-verb-coverage.md @@ -82,7 +82,7 @@ fixture `tests/fixtures/corepoint/acme_adt_package.xml` is the pattern to follow Verify before every commit: ```powershell -python scripts\publish\scan_forbidden.py --published # must exit 0 +python scripts\security\scan_forbidden.py --path . # must exit 0 ``` --- @@ -168,7 +168,7 @@ python -m ruff format ; python -m ruff check python -m mypy messagefoundry python -m pytest tests/test_corepoint_import.py -q python scripts\docs\backlog_status_check.py -python scripts\publish\scan_forbidden.py --published +python scripts\security\scan_forbidden.py --path . ``` ⚠️ `scan_forbidden` also blocks the **spaced** two-word form of *action-list* — the repo convention is the diff --git a/docs/releases/v0.1-EXECUTION-PLAN.md b/docs/releases/v0.1-EXECUTION-PLAN.md index 833d224..330d5e4 100644 --- a/docs/releases/v0.1-EXECUTION-PLAN.md +++ b/docs/releases/v0.1-EXECUTION-PLAN.md @@ -312,7 +312,7 @@ the engine's first partner-facing inbound socket beyond MLLP — needs auth/TLS/ deliveries). SQLite-store-specific (PG/SQL Server use pools). 2. **`in_pipeline` drain gauge** — folded into Gate #3 (WS-D) for credibility; 3-backend change. 3. **Single-source the publish-guard token list** — fold the additional real-estate tokens the harness - denylist test enumerates into `scripts/publish/scan_forbidden.py` (the documented single source of + denylist test enumerates into `scripts/security/scan_forbidden.py` (the documented single source of truth) and have the test import them. **Owner-managed** — touches publish-guard config and risks flagging innocent tracked content; needs a deliberate owner false-positive pass. diff --git a/docs/testing/FEATURE-COVERAGE-PLAN.md b/docs/testing/FEATURE-COVERAGE-PLAN.md index 43205a2..6c7a213 100644 --- a/docs/testing/FEATURE-COVERAGE-PLAN.md +++ b/docs/testing/FEATURE-COVERAGE-PLAN.md @@ -1353,7 +1353,7 @@ Recommended tests to close gaps: | ANON-3 | Structure-preserving surrogates (name/address/mrn/id/ssn/phone/dob/provider) | 0030 | test_anon_core.py::test_surrogate_field_*, ::test_alphanumeric_*, ::test_partial_dob_*; test_anon_parity.py::test_surrogate_pools_carry_no_hl7_delimiter | partial | func: ADDRESS/PHONE/PROVIDER have no direct datatype-shape assertions (only indirect 'PHI gone') | med | S | | ANON-4 | FREETEXT blunt full-redact + OBX-5 textual-only gating (OBX-2), NTE-3 | 0030 | test_anon_core.py::test_freetext_is_blunt_redacted, ::test_obx5_freetext_only_when_value_type_textual | covered | phi: opt-into-surrogation residual-leak path untested (default redact is) | high | S | | ANON-5 | HL7 adapter: MSH-separator whole-field write, fail-closed refusal, MLLP strip, custom seps | 0030 | test_anon_core.py::test_anonymize_scrubs_phi_keeps_structure, ::test_reads_custom_separators, ::test_no_msh_refused, ::test_mllp_framed, ::test_a40_merge_linkage | covered | none material | high | S | -| ANON-6 | Site-code (54xxxx) field-anchored scrub + detector | 0030 | test_anon_core.py::test_site_code_scrub_is_field_anchored, ::test_leak_check_clean_and_dirty | covered | parity: no 54xxxx input in the engine/tee adversarial corpus | med | S | +| ANON-6 | Site-code (externalized prefix) field-anchored scrub + detector | 0030 | test_anon_core.py::test_site_code_scrub_is_field_anchored, ::test_leak_check_clean_and_dirty | covered | parity: no site-code input in the engine/tee adversarial corpus | med | S | | ANON-7 | Fail-closed leak-check bridge (single scan_forbidden authority) + `anonymize_checked` PHI-safe LeakError | 0030 | test_anon_core.py::test_leak_check_clean_and_dirty, ::test_anonymize_checked_fails_closed (both skipif no scanner) | covered | func: tests SKIPPED on OSS mirror (scanner private); engine LeakCheckUnavailable raise untested | high | S | | ANON-8 | Engine↔tee parity: byte-identical files, vendored pool/token lockstep, golden+adversarial re-encoder equivalence | 0030 | test_anon_parity.py (byte-identical, vendored _hl7data, token table, adversarial+golden equality, fail-closed both sides, empty-table-on-mirror) | covered | none material | med | S | | ANON-9 | tee `anonymize-captures` (env salt, requires captures, fail-closed, round-trip corpus) | 0030 | test_anon_integration.py::test_tee_anonymize_captures_end_to_end, ::_refuses_no_msh_body, ::_requires_salt, ::_errors_when_no_captures | partial | phi: no capsys/caplog assertion bodies stay off stdout/stderr; func: CLI-level LeakError->rc1 (token in KEPT field) untested | high | S | @@ -1369,10 +1369,10 @@ Recommended tests to close gaps: - ANON-9 (highest value): add a `capsys`/`caplog` assertion to the tee `anonymize-captures` tests that neither a raw capture body nor any pre-anon field value ever appears on stdout/stderr; and a CLI-level fail-closed case where a capture carries a forbidden token in a KEPT field (MSH-3-style) → exit 1, no file written. - ANON-3: direct structural assertions for the ADDRESS (XAD `Street^^City^State^Zip^USA`), PHONE (NANP-reserved `NXX-555-01XX`), and PROVIDER (XCN `Id^Family^Given`) surrogates, mirroring the existing name/mrn/dob assertions. - ANON-1: assert the salt is never emitted — capture logs during an `anonymize`/`anonymize_checked` run and confirm the salt string never appears in any record or raised error. -- ANON-7: add a 54xxxx/forbidden-token input to the engine↔tee adversarial parity corpus, and assert engine-side `leak_check` raises `LeakCheckUnavailable` when `scripts/publish/` is absent. +- ANON-7: add a site-code/forbidden-token input to the engine↔tee adversarial parity corpus, and assert engine-side `leak_check` raises `LeakCheckUnavailable` when the scanner is absent. - ANON-14: a settings-validator test that `protocol="tls"` with verification on and no `forward_tls_ca_file` fails closed (rejects the silent-public-CA path). -> **Auditor notes:** Scope boundaries: ANON-11/ANON-12 (redaction + logging guardrails) overlap the logging/settings sibling subsystem but are the PHI-leak surface, so audited here. ANON-14/ANON-15 (TLS syslog, default-on, time-sync from ADR 0080) sit in logging_setup.py/settings.py and could also be claimed by a logging subsystem; retained per explicit scope. ANON-16 (AI-assist clamp) is engine-side policy only — the send-time enforcement is in the out-of-scope VS Code IDE extension. No web-console surface in this subsystem beyond the GET /ai/policy endpoint (covered). No PySide6-desktop-console ANON surface exists. The leak-check tests (ANON-7) rely on the private-only scripts/publish/scan_forbidden.py and are skipped on the public OSS mirror — real evidence exists but only on the private CI leg. Cross_backend applies only to ANON-13 (audit tee) and is genuinely covered on SQLite + live SQL Server + Postgres. +> **Auditor notes:** Scope boundaries: ANON-11/ANON-12 (redaction + logging guardrails) overlap the logging/settings sibling subsystem but are the PHI-leak surface, so audited here. ANON-14/ANON-15 (TLS syslog, default-on, time-sync from ADR 0080) sit in logging_setup.py/settings.py and could also be claimed by a logging subsystem; retained per explicit scope. ANON-16 (AI-assist clamp) is engine-side policy only — the send-time enforcement is in the out-of-scope VS Code IDE extension. No web-console surface in this subsystem beyond the GET /ai/policy endpoint (covered). No PySide6-desktop-console ANON surface exists. The leak-check tests (ANON-7) rely on the private-only scripts/security/scan_forbidden.py and are skipped on the public OSS mirror — real evidence exists but only on the private CI leg. Cross_backend applies only to ANON-13 (audit tee) and is genuinely covered on SQLite + live SQL Server + Postgres. ## 21. Config, wiring, connections.toml, environments, code-sets, lifecycle `[CFG]` diff --git a/messagefoundry/anon/leak.py b/messagefoundry/anon/leak.py index d00730c..df74431 100644 --- a/messagefoundry/anon/leak.py +++ b/messagefoundry/anon/leak.py @@ -5,7 +5,7 @@ A de-identified dataset is only safe to commit/share once it is **proven** free of known customer/partner/site tokens — and that token set must be the SINGLE source of truth, not a third copy that drifts (the very drift that already bit ``tests/test_load_config.py``). So the engine-side -leak-check delegates to ``scripts/publish/scan_forbidden.py`` — the owner-managed publish guard — via +leak-check delegates to ``scripts/security/scan_forbidden.py`` — the owner-managed guard — via its importable :func:`scan_text`. The standalone ``tee/anon/leak.py`` vendors the same token data (held identical by the parity test), since the tee cannot reach ``scripts/``. @@ -34,7 +34,7 @@ class LeakCheckUnavailable(RuntimeError): def _scanner() -> ModuleType: here = Path(__file__).resolve() for parent in here.parents: - candidate = parent / "scripts" / "publish" / "scan_forbidden.py" + candidate = parent / "scripts" / "security" / "scan_forbidden.py" if candidate.is_file(): spec = importlib.util.spec_from_file_location("mefor_anon_scan_forbidden", candidate) if spec is not None and spec.loader is not None: @@ -42,7 +42,7 @@ def _scanner() -> ModuleType: spec.loader.exec_module(module) return module raise LeakCheckUnavailable( - "could not locate scripts/publish/scan_forbidden.py — the anonymizer leak-check requires " + "could not locate scripts/security/scan_forbidden.py — the anonymizer leak-check requires " "the source checkout (it is a dev/migration tool, not an installed-wheel runtime)" ) @@ -52,11 +52,11 @@ def leak_check(text: str) -> list[str]: Substring + estate-token mode (ADR 0030 §5): catches a partner/site token anywhere in a message body, including inside a field, where the publish-prose word-boundary form would miss it. The - ``54xxxx`` site code is checked **field-anchored** (``message_has_site_code``), matching the - replace path, so a scrub *miss* is caught without false-positiving on a value that merely contains - a ``54xxxx`` run (a timestamp, a fabricated 1954/2054 date). + site code is checked **field-anchored** (``message_has_site_code``), matching the replace path, so + a scrub *miss* is caught without false-positiving on a value that merely contains a site-code run + (a timestamp, a fabricated date). """ hits = [str(h) for h in _scanner().scan_text(text, include_estate=True)] if message_has_site_code(text): - hits.append("site-code pattern (54xxxx)") + hits.append("site-code pattern") return hits diff --git a/messagefoundry/anon/surrogates.py b/messagefoundry/anon/surrogates.py index 18bf003..4295f81 100644 --- a/messagefoundry/anon/surrogates.py +++ b/messagefoundry/anon/surrogates.py @@ -19,19 +19,103 @@ from __future__ import annotations +import os import random import re from collections.abc import Callable from dataclasses import dataclass +from pathlib import Path from . import _pools from .keying import Keyer from .rules import SurrogateKind -#: A field/component value that *is exactly* a ``54xxxx`` site code (ADR 0030 §5, owner decision -#: 2026-06-20: **field-anchored** — ``fullmatch`` on a whole component, so a coincidental ``54xxxx`` -#: inside a longer timestamp/order number is left alone, unlike the publish gate's broad catch-all). -SITE_CODE_RE = re.compile(r"54\d{4}") +#: A never-matching pattern — the site-code detector when no prefix is configured (a token-less +#: public build). ``re`` has no built-in "match nothing", so encode one explicitly. +_NEVER: re.Pattern[str] = re.compile(r"(?!x)x") + + +def _resolve_token_text() -> str | None: + """The active token content, or ``None`` if no source is configured — the SAME source + ``scripts/security/scan_forbidden.py`` reads, so the site-code authority is externalized in one + place (never committed). ``MEFOR_FORBIDDEN_TOKENS`` wins: an existing file path is read; any other + non-empty value is inline content; an explicitly-empty value means "no source". Otherwise the + git-ignored ``scripts/security/scan-tokens.local.txt`` is located by walking up from this file + (a dev checkout); an installed wheel without it degrades to no-op. + """ + env = os.environ.get("MEFOR_FORBIDDEN_TOKENS") + if env is not None: + env = env.strip() + if not env: + return None + try: + p = Path(env) + if p.is_file(): + return p.read_text(encoding="utf-8") + except OSError: + pass + return env + for parent in Path(__file__).resolve().parents: + candidate = parent / "scripts" / "security" / "scan-tokens.local.txt" + try: + if candidate.is_file(): + return candidate.read_text(encoding="utf-8") + except OSError: + pass + return None + + +def _load_site_prefixes() -> tuple[str, ...]: + """The estate's site-code numeric prefixes, EXTERNALIZED out of source (they identify a real + customer estate; ADR 0030 §5, owner decision 2026-06-20). Only the ``[site_prefix]`` section of + the shared token file is read here, two-digit prefixes only (the site-code model is a two-digit + prefix + four digits). An absent source yields ``()`` — the site-code safety pass then no-ops, + correct for a token-less public build where the rule map is the primary control. + """ + text = _resolve_token_text() + if text is None: + return () + prefixes: list[str] = [] + section: str | None = None + for raw in text.splitlines(): + line = raw.strip() + if not line or line.startswith("#"): + continue + if line.startswith("[") and line.endswith("]"): + section = line[1:-1].strip().lower() + continue + if section == "site_prefix" and line.isdigit() and len(line) == 2: + prefixes.append(line) + return tuple(dict.fromkeys(prefixes)) + + +#: Field-anchored site-code detector (``fullmatch`` on a whole component, so a coincidental site-code +#: run inside a longer timestamp/order number is left alone, unlike the publish gate's broad catch-all). +#: Built from the externalized prefixes; ``_NEVER`` when none are configured. Recomputed by +#: :func:`reload_site_prefixes`; read as a stable public contract by the leak-check bridge. +_SITE_PREFIXES: tuple[str, ...] = () +SITE_CODE_RE: re.Pattern[str] = _NEVER +#: Two-digit prefixes that are NOT site-code prefixes — a fabricated replacement code draws its lead +#: from here so it can never itself be a site code. +_NON_SITE_PREFIXES: tuple[str, ...] = () + + +def reload_site_prefixes() -> None: + """Recompute the externalized site-code globals from the current environment / local token file. + Called once at import and by tests after adjusting the source. Never raises.""" + global _SITE_PREFIXES, SITE_CODE_RE, _NON_SITE_PREFIXES + _SITE_PREFIXES = _load_site_prefixes() + if _SITE_PREFIXES: + alt = "|".join(re.escape(p) for p in _SITE_PREFIXES) + SITE_CODE_RE = re.compile(rf"(?:{alt})\d{{4}}") + else: + SITE_CODE_RE = _NEVER + _NON_SITE_PREFIXES = tuple( + f"{n:02d}" for n in range(10, 100) if f"{n:02d}" not in _SITE_PREFIXES + ) + + +reload_site_prefixes() #: What :class:`SurrogateKind.FREETEXT` collapses a narrative field to. Free text routinely embeds #: identifiers (a name/MRN/DOB in prose) that field-level surrogation cannot reach and the leak-check @@ -194,19 +278,23 @@ def surrogate_field(kind: SurrogateKind, value: str, keyer: Keyer, seps: Seps) - def scrub_site_codes(value: str, keyer: Keyer, seps: Seps) -> str: - """Field-anchored ``54xxxx`` safety pass (ADR 0030 §5): replace any **whole component** that is - exactly a site code with a fabricated 6-digit non-``54`` code, leaving coincidental ``54xxxx`` - inside a longer value (timestamps, order numbers) untouched. Applied to every field as a net - under the rule map, since a site code can lurk in an unexpected field. + """Field-anchored site-code safety pass (ADR 0030 §5): replace any **whole component** that is + exactly a site code with a fabricated non-site code of the same width, leaving a coincidental + site-code run inside a longer value (timestamps, order numbers) untouched. Applied to every field + as a net under the rule map, since a site code can lurk in an unexpected field. A no-op when no + site-code prefix is configured (a token-less build; the rule map remains the primary control). """ - if not value or "54" not in value: + if not value or not _SITE_PREFIXES: + return value + if not any(p in value for p in _SITE_PREFIXES): return value def _one(component: str) -> str: if not SITE_CODE_RE.fullmatch(component): return component rng = keyer.rng("sitecode", component) - return f"{rng.randrange(10, 54)}{_fake_digits(rng, 4)}" + prefix = rng.choice(_NON_SITE_PREFIXES) + return f"{prefix}{_fake_digits(rng, len(component) - len(prefix))}" reps = value.split(seps.repetition) out_reps = [] @@ -245,11 +333,12 @@ def read_message_seps(text: str) -> tuple[Seps, str] | None: def message_has_site_code(text: str) -> bool: - """True if any whole field/component/subcomponent **is exactly** a ``54xxxx`` site code — the - field-anchored leak-check that matches the scrub (ADR 0030 §5). Using ``fullmatch`` per component - (not a broad substring search) means a value that merely *contains* a ``54xxxx`` run — a timestamp, - a fabricated 1954/2054 date, a long order number — is not falsely flagged, while a genuine scrub - *miss* still is. Falls back to a broad search only for unstructured (no-MSH) text.""" + """True if any whole field/component/subcomponent **is exactly** a site code — the field-anchored + leak-check that matches the scrub (ADR 0030 §5). Using ``fullmatch`` per component (not a broad + substring search) means a value that merely *contains* a site-code run — a timestamp, a fabricated + date, a long order number — is not falsely flagged, while a genuine scrub *miss* still is. Falls + back to a broad search only for unstructured (no-MSH) text. Always False when no site-code prefix + is configured.""" parsed = read_message_seps(text) if parsed is None: return SITE_CODE_RE.search(text) is not None @@ -264,13 +353,13 @@ def message_has_site_code(text: str) -> bool: def scrub_message_site_codes(text: str, keyer: Keyer) -> str: - """Field-anchored ``54xxxx`` safety pass over a whole ``\\r``-delimited message (ADR 0030 §5). + """Field-anchored site-code safety pass over a whole ``\\r``-delimited message (ADR 0030 §5). Splits on the message's own separators and replaces any whole **component** that is exactly a site code, anywhere in the message — the net under the rule map for a site code lurking in an unexpected field. MSH-1 (field separator) and MSH-2 (encoding characters) are left untouched so the header stays parseable. The broad, unanchored catch-all stays in the publish leak-gate as - defense-in-depth; here we never over-redact a coincidental ``54xxxx`` inside a longer value. + defense-in-depth; here we never over-redact a coincidental site-code run inside a longer value. """ parsed = read_message_seps(text) if parsed is None: diff --git a/packaging/messagefoundry-webconsole/RELEASE.md b/packaging/messagefoundry-webconsole/RELEASE.md index 82bf432..94728ee 100644 --- a/packaging/messagefoundry-webconsole/RELEASE.md +++ b/packaging/messagefoundry-webconsole/RELEASE.md @@ -62,8 +62,8 @@ second-wheel release job — mirror it (with the console's **own** version, sinc - [ ] Ensure the **SBOM** job covers the second wheel. - [ ] Add the wheel to the **publish/mirror** wiring as needed - ([`scripts/publish/`](../../scripts/publish/)); extend - `scripts/publish/check_release_sync.py` if the mirror should track the console version too. + (retired at the MEFORORG cutover -- development is now direct on the public repo, so there + is no mirror to keep in sync and no release-sync checker). ## 5. CI build + release job @@ -88,7 +88,8 @@ second-wheel release job — mirror it (with the console's **own** version, sinc ## 7. Post-publish verification -- [ ] Run `python scripts/publish/check_release_sync.py` (tag == PyPI == mirror). +- [ ] Confirm the tag matches the PyPI version. (The tag == PyPI == mirror checker was retired + with the publish machinery at the MEFORORG cutover; there is no mirror to compare against.) - [ ] `pip install "messagefoundry[webconsole]"` resolves the pair inside the compat range. - [ ] `serve_ui=true` boots end-to-end on the CLI/service path with the console installed, and the engine still **boots + refuses `serve_ui` cleanly** with the console **absent** (return 2 / diff --git a/scripts/security/crypto_inventory_check.py b/scripts/security/crypto_inventory_check.py index 1d6da72..185ef8e 100644 --- a/scripts/security/crypto_inventory_check.py +++ b/scripts/security/crypto_inventory_check.py @@ -24,7 +24,7 @@ The walk-set (:data:`WALK_ROOTS`) is byte-identical to ``tests/test_security_static.py``'s ``_CRYPTO_ROOTS`` (BACKLOG #283 owns that pin; this gate consumes it). -Stdlib only (no install), like ``scripts/publish/scan_forbidden.py`` — runnable as a CI step and a +Stdlib only (no install), like ``scripts/security/scan_forbidden.py`` — runnable as a CI step and a pytest. Usage:: python scripts/security/crypto_inventory_check.py # scan the five real roots diff --git a/scripts/security/scan-allowlist.txt b/scripts/security/scan-allowlist.txt new file mode 100644 index 0000000..213ce9f --- /dev/null +++ b/scripts/security/scan-allowlist.txt @@ -0,0 +1,19 @@ +# Vetted false-positive allowlist for scan_forbidden.py (one line-regex per line; '#' comments). +# +# NEVER allowlist real customer data -- only genuinely-innocent lines a pattern over-matches. +# Each entry MUST be narrow enough that a real customer/host string in the same file still trips. +# See the scanner header in scripts/security/scan_forbidden.py. + +# sys.dm_os_wait_stats soak rows in the benchmark artifacts (docs/benchmarks/results/.../storedmv_soak.txt) +# print " ms tasks". A task count that lands in the +# site-code range (e.g. WRITELOG ... 993496 tasks) collides with the site-code guard (99\d{4}) despite +# being a pure engine perf counter with no customer-data path. This row shape is unambiguous DMV output, +# so a real site code embedded in a path/HL7 field/prose (PT_990700_ADT, ^993496^) does NOT match it. +^\s*[A-Z0-9_]+\s+\d+ ms\s+\d+ tasks\s*$ + +# OIDC Core spec-section citations in the federated-SSO claim ladder (ADR 0142) read as dotted +# quads: 'OIDC core 3.1.3.7/2' is section 3.1.3.7, rule 2 -- not a host. The citation is worth +# keeping verbatim in security-critical verification code, so the line is allowlisted rather than +# reworded. Anchored to the literal 'OIDC core
/' shape, so a real routable IP +# elsewhere in claims.py (or anywhere else) still trips the scan. +OIDC core \d+(?:\.\d+)+/\d+ diff --git a/scripts/security/scan-tokens.local.txt.example b/scripts/security/scan-tokens.local.txt.example new file mode 100644 index 0000000..af72cdb --- /dev/null +++ b/scripts/security/scan-tokens.local.txt.example @@ -0,0 +1,55 @@ +# scan-tokens.local.txt.example — SYNTHETIC template for the forbidden-content token list. +# +# WHAT THIS IS +# The customer/vendor token list that powers the forbidden-content gate is PRIVATE and is NEVER +# committed. This *.example file is the committed, synthetic template. To use it: +# 1. Copy it to `scan-tokens.local.txt` in this same directory (that name is git-ignored). +# 2. Replace the SYNTHETIC placeholders below with the REAL token set, which the owner migrates +# from the private vault (the former inline FORBIDDEN / ESTATE_TOKENS / site-code prefix that +# lived in the retired scripts/publish/scan_forbidden.py). +# 3. `pre-commit install` — the gate now loads real tokens locally. +# +# In CI the same content is supplied via the `MEFOR_FORBIDDEN_TOKENS` Actions secret, whose value is +# EITHER a path to a file in this format OR the file's contents inline (newline-separated). +# +# With NO source present the scanner runs STRUCTURAL-ONLY: only the routable-IPv4 detector fires; +# 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. +# +# FORMAT (sectioned; `#` comments and blank lines ignored) +# [names] one entry per line: REGEX | REASON | CASE +# REASON optional (default "customer/vendor token"). +# CASE optional: "i" case-insensitive (default) or "s" case-sensitive. +# The field delimiter is a space-pipe-space ( | ); a regex alternation a|b is fine. +# [estate] one substring token per line (matched case-insensitively). Applied to anonymizer +# BODIES, and — unlike the [names] patterns, which are word-boundary anchored and so +# cannot see a token butted against "_" — also to tracked FILES, which is what +# catches a customer token buried in an identifier like OB__ORU. +# [estate_body_only] +# a SUBSET of [estate] (list the token in both): tokens that are ordinary words, or a +# vendor product name used as an example identifier, which would mass-false-positive +# over tracked source. These stay BODY-ONLY and are excluded from the file scan. +# [site_prefix] one numeric prefix per line. Two detectors are built: PREFIX followed by four digits, +# and the PATTERN written out (the prefix followed by a regex quantifier expression +# or an x-run) — the prefix itself is the secret, so spelling the pattern leaks it. +# +# Everything below is SYNTHETIC (ACME / EXAMPLE, and a NON-REAL 99xxxx site prefix). It exercises the +# mechanism and MUST be replaced for real use. Do NOT commit the filled-in scan-tokens.local.txt. + +[names] +\bacme\b | customer name (ACME) | i +\bexampleco\b | partner name (EXAMPLE) | i +example\s+estate | reference to the example estate | i +\bwidget\s+manifests?\b | migration artifact (EXAMPLE widget manifest) | i +\bACMECORP\b | customer adopter repo (ACMECORP) | s + +[estate] +acme +exampleco +examplevendor + +[estate_body_only] +examplevendor + +[site_prefix] +99 diff --git a/scripts/security/scan_forbidden.py b/scripts/security/scan_forbidden.py new file mode 100644 index 0000000..5706ed0 --- /dev/null +++ b/scripts/security/scan_forbidden.py @@ -0,0 +1,883 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Forbidden-content scanner: keep customer/PHI-adjacent strings out of the public repo. + +Two callers share this one module so their detection can never drift: + * ``.pre-commit-config.yaml`` runs it over staged files before every commit. + * ``.github/workflows/security.yml`` runs it over the whole tracked tree in CI. + +Why a custom scanner in addition to gitleaks: gitleaks finds *secrets* (keys/tokens). This finds +*customer-identifying* strings -- a partner/vendor name, a real site-estate's site-code prefix, a +routable host IP -- which are not credentials but must never reach the open-source repo. + +Token authority is EXTERNALIZED. The committed source carries only STRUCTURAL detectors (a routable- +IPv4 detector and the generic site-code *shape*); the real customer/vendor name patterns, estate +substrings, and the site-code numeric prefix are loaded at runtime from, in order of precedence: + + 1. ``MEFOR_FORBIDDEN_TOKENS`` -- either a path to a token file OR the token content inline + (newline-separated, same format). Used in CI via the Actions secret of the same name. + 2. ``scripts/security/scan-tokens.local.txt`` -- a git-ignored local file (pre-commit). A synthetic + template ships as ``scan-tokens.local.txt.example``. + +With no source present the scanner degrades to STRUCTURAL-ONLY (routable-IPv4 only); the name / +estate / site-code detectors are simply empty -- appropriate for a fork with no access to the secret. +Set ``MEFOR_REQUIRE_TOKENS=1`` to fail closed (exit 2) instead when the source is absent, so a +misconfigured owner/CI run refuses rather than silently under-scanning. + +PRESENCE IS NOT SUFFICIENCY. A source that loads only PART of its tokens is the dangerous case: it +satisfies "tokens present", prints no structural-only warning, and passes a gate that calls itself +fail-closed. So requiring tokens also requires every floor section (``names`` / ``estate`` / +``site_prefixes``) to be non-empty, and ``MEFOR_MIN_DETECTORS=N`` (or ``--require-tokens=N``) asserts +a minimum TOTAL, which is what catches loss *within* a section. It is a floor, not an equality: +adding tokens is free, losing them fails. The expected total comes from OUTSIDE the token source -- +a count carried inside the file would be destroyed by the same mangling it is meant to detect. + +``--require-tokens[=N]`` exists because pre-commit can pass args to a hook but cannot set env for +one, so it is the only way to make the commit-time gate fail closed on a checkout with no token file. + +Token-file format (sectioned; ``#`` comments and blank lines ignored): + [names] REGEX | REASON | CASE -- REASON optional (default "customer/vendor token"); + CASE optional: "i" case-insensitive (default) or "s" case-sensitive. The field + delimiter is a space-pipe-space (`` | ``); a regex alternation ``a|b`` is unaffected. + [estate] one substring token per line (matched case-insensitively, anywhere in a body). + [site_prefix] one numeric prefix per line; the detector matches PREFIX + four digits. + +Usage: + scan_forbidden.py [FILE ...] # scan the given files (how pre-commit invokes it) + scan_forbidden.py --path DIR # scan every text file under DIR + scan_forbidden.py --show-context # also print the matched line/value (NEVER used in CI -- a hit + # means the content already sits in a tracked file, so echoing + # it would copy the leak into public CI logs; default is + # reasons-only: location + category, never the matched text) + scan_forbidden.py # scan all git-tracked files in the current repo + +Exit: 0 clean, 1 forbidden content found (fail closed), 2 usage error / required-tokens-missing. +""" + +from __future__ import annotations + +import os +import re +import subprocess +import sys +from pathlib import Path + +# -------------------------------------------------------------------------------------------------- +# Structural detectors (NO customer data -- safe to commit). These run regardless of the token source. +# -------------------------------------------------------------------------------------------------- + +# Routable-IPv4 detector. The look-arounds keep dotted OIDs (1.3.6.1..., 2.16.840...) and version +# strings embedded in longer dotted sequences from matching; only a free-standing IPv4 does. +_OCTET = r"(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)" +_IPV4 = re.compile(rf"(?, $HOME, %USERPROFILE%, {home}), the well-known shared and CI +# accounts, and the DOCUMENTATION placeholder names this repo already uses in examples (me, svc, you, +# user, username, example). Everything else looks like a real account and fires. That list is the whole +# judgement call here: "is this a real person's login" is not decidable by shape, so the pattern trusts +# a small, explicit set of conventional stand-ins and treats anything else as a disclosure. +_HOME_PATH = re.compile( + r"(?:[A-Za-z]:[\\/]Users|/home|/Users)[\\/]" + r"(?!<|\$|%|\{" + r"|(?:Public|Default|All|ContainerAdministrator|runner|vsts" + r"|me|svc|you|user|username|example)[\\/\s\"'`]" + r")" + r"[A-Za-z][A-Za-z0-9._-]*" +) + +# A pattern that can never match -- the "detector off" sentinel used for the site-code regexes when no +# numeric prefix is loaded (structural-only / fork context). ``(?!)`` is an always-failing assertion, +# so ``.search``/``.finditer``/``.fullmatch`` never fire and ``.pattern`` stays a valid string. +_NEVER: re.Pattern[str] = re.compile(r"(?!)") + +# Skip routable-IP detection (only) where dotted numbers are package versions, not hosts. +_IP_SKIP_SUFFIXES = {".lock"} +_IP_SKIP_NAMES = {"requirements.lock", "uv.lock", "package-lock.json"} + +# Lock/SVG/password-list files are dense with incidental standalone digit runs -> skip the site-code +# file scan (only) on them to avoid a false-positive storm. +_SITE_SKIP_SUFFIXES = {".lock", ".svg"} +_SITE_SKIP_NAMES = { + "requirements.lock", + "uv.lock", + "package-lock.json", + "common_passwords.txt", +} + +SKIP_DIRS = { + ".git", + ".venv", + "venv", + "node_modules", + "__pycache__", + ".mypy_cache", + ".ruff_cache", + ".pytest_cache", + "build", + "dist", +} + +# The ONLY skipped path: the git-ignored real-token file itself. It is never committed (so it is +# absent from a tracked scan), but a whole-worktree ``--path .`` scan on the owner's machine would +# otherwise walk it and self-trip on every real token it exists to hold. Nothing that reaches the +# public repo is skipped -- no blanket public directory, and NOT ``.gitignore`` (which is scanned). +SKIP_PATHS = ("scripts/security/scan-tokens.local.txt",) + + +def _is_skipped(posix: str) -> bool: + # ``posix`` is ALWAYS relative to the scan root (main() derives it that way in --path mode too), so + # this is an EXACT path match, never a suffix match. A suffix test would skip a same-named file at a + # different path, which must still be scanned. + return posix in SKIP_PATHS + + +# -------------------------------------------------------------------------------------------------- +# Token authority (EXTERNALIZED -- never committed). These module globals are (re)computed by +# ``reload_tokens()`` and read by the anonymizer leak-check bridges (messagefoundry/anon/leak.py and +# tee/anon/leak.py) as the single source of truth, so their names/shapes are a stable public contract. +# -------------------------------------------------------------------------------------------------- + +#: Location of the git-ignored local token file (pre-commit source). Overridable in tests. +LOCAL_TOKEN_FILE = Path(__file__).parent / "scan-tokens.local.txt" + +FORBIDDEN: list[tuple[re.Pattern[str], str]] = [] +ESTATE_TOKENS: tuple[str, ...] = () +SITE_CODE_RE: re.Pattern[str] = _NEVER +#: Boundary-aware site-code FILE detector: fires on a code delimited by non-alphanumerics +#: (``PT_990123_ADT`` -> matches) but NOT on the prefix embedded in a longer alphanumeric run +#: (a SHA, a DOB, a dotted version), so the broad-substring false-positive storm does not happen on +#: source files. ``.`` is a boundary exclusion too (a real site code is never dot-adjacent). +_SITE_CODE_FILE: re.Pattern[str] = _NEVER +#: The site-code detectors above match a prefix followed by four LITERAL digits. But the SECRET IS THE +#: PREFIX, not any particular code, so a doc or comment that writes the PATTERN itself -- the prefix +#: followed by a regex quantifier expression, or by an x-run -- discloses exactly the same thing while +#: matching neither. Not hypothetical: ADR 0030's de-identification pass certified a file clean by +#: grepping one form and walked past three occurrences of the other, one on the ADJACENT line. A +#: form-blind sweep reads as evidence of absence while being incapable of finding what it looks for. +_SITE_CODE_PATTERN_LITERAL: re.Pattern[str] = _NEVER +#: Estate tokens are substring-matched, so a few are file-scanned and the rest are body-only. Listed in +#: the token source under ``[estate_body_only]``: tokens that are ordinary words or a vendor product +#: name used as an example identifier, which mass-false-positive over tracked source. Everything in +#: ``[estate]`` and NOT here is compiled into ``_ESTATE_FILE_RES`` and applied by ``scan_file``. +_ESTATE_BODY_ONLY: frozenset[str] = frozenset() +#: (token, boundary-aware pattern) for every file-scanned estate token. The boundary requires non-LETTER +#: flanks: it must fire on ``OB__ORU`` (underscore is a word char, so the ``\b`` name patterns +#: cannot see it -- the exact hole that let a customer org name sit in a tracked test and be reported +#: clean by every gate) while NOT firing on a token that is merely a run of letters inside an unrelated +#: identifier, e.g. the WebAuthn exception name ``InvalidCBORData``. +_ESTATE_FILE_RES: tuple[tuple[str, re.Pattern[str]], ...] = () +#: True when a token source was found AND it actually yielded detectors. Both halves matter: see +#: ``reload_tokens``. +TOKENS_PRESENT: bool = False +#: The parsed site prefixes themselves, kept so ``loaded_token_counts`` can report how many loaded. +#: Deriving that number from ``SITE_CODE_RE is not _NEVER`` made it a 0-or-1 presence BIT, so N loaded +#: prefixes and 1 printed identically -- and the counts line is the ONE diagnostic that distinguishes a +#: real scan from a vacuous one, so a floor built on it would have inherited the same blindness. +_SITE_PREFIXES: tuple[str, ...] = () + + +def _resolve_token_text() -> str | None: + """The active token content, or ``None`` if no source is configured. + + ``MEFOR_FORBIDDEN_TOKENS`` wins: an existing file path is read; any other non-empty value is taken + as inline content; an explicitly-empty value means "no source". Otherwise the git-ignored local + file is read if present. + """ + env = os.environ.get("MEFOR_FORBIDDEN_TOKENS") + if env is not None: + env = env.strip() + if not env: + return None + try: + p = Path(env) + if p.is_file(): + return p.read_text(encoding="utf-8") + except OSError: + pass + return env # inline newline-separated token content + try: + if LOCAL_TOKEN_FILE.is_file(): + return LOCAL_TOKEN_FILE.read_text(encoding="utf-8") + except OSError: + pass + return None + + +#: Zero-width, bidi-control, BOM and C0/C1 control codepoints. These are the corruption class the +#: COUNT FLOOR cannot see. A zero-width space is not ``str.isspace()`` and survives ``str.strip()`` +#: (unlike NBSP, which strips to empty), so one injected INSIDE a token by a paste through a rendering +#: surface leaves an entry that parses, counts toward the floor, and silently never matches: measured +#: identical counts with detection flipped off. Rejecting them at parse time is what makes the floor +#: count detectors that can FIRE rather than entries that merely PARSE. +def _is_invisible(ch: str) -> bool: + """True for a codepoint that carries no glyph: control, zero-width, BOM or bidi mark. + + ``str.isprintable()`` is False for exactly the categories that matter here -- Cc controls and + Cf formats, which covers U+200B ZERO WIDTH SPACE, U+FEFF and the bidi overrides -- and True for + ordinary letters, digits and the plain space. Expressed via isprintable() rather than a + character-class literal on purpose: a literal needs escape sequences that are themselves easy to + mangle, in a check whose entire job is catching mangling. + """ + return not ch.isprintable() + + +#: An empty negative lookahead matches nothing anywhere -- the module's own ``_NEVER`` sentinel. +_NEVER_MATCH_RE = re.compile(r"\(\?\!\)") + + +def _reject_entry(kind: str, value: str) -> bool: + """True (and warns) when an entry carries an invisible codepoint, so it must be dropped. + + This is the corruption class the COUNT FLOOR cannot see. A zero-width space is not + ``str.isspace()`` and survives ``str.strip()`` (unlike NBSP, which strips to empty), so one + injected INSIDE a token by a paste through a rendering surface leaves an entry that parses, + counts toward the floor, and silently never matches -- measured: identical counts, detection + flipped off. Rejecting these makes the floor count detectors that can FIRE, not entries that + merely PARSE. + + The warning deliberately does NOT echo the value: this runs in a world-readable Actions log on + a public repo and the value is a customer/vendor token. Codepoint and offset are enough to fix. + """ + for offset, ch in enumerate(value): + if _is_invisible(ch): + print( + f"scan_forbidden: ignoring a {kind} containing an invisible/control codepoint " + f"U+{ord(ch):04X} at offset {offset} - it would count toward the detector floor " + "but never match. Re-set the token source from the file rather than pasting it.", + file=sys.stderr, + ) + return True + return False + + +#: UTF-8 BOM. Built with chr() rather than written literally: an escape or a raw +#: codepoint here is exactly the kind of thing that gets mangled in transit, in the one +#: place whose job is to survive mangling. +_BOM = chr(0xFEFF) + + +def _parse_tokens( + text: str, +) -> tuple[list[tuple[re.Pattern[str], str]], tuple[str, ...], tuple[str, ...], list[str]]: + """Parse sectioned token content. + + Returns (name patterns, estate substrings, body-only estate substrings, site prefixes). + ``[estate_body_only]`` is a SUBSET marker, not a separate token set -- a token listed there should + also appear in ``[estate]``; it simply stays out of the file scan. + """ + section: str | None = None + names: list[tuple[re.Pattern[str], str]] = [] + estate: list[str] = [] + body_only: list[str] = [] + prefixes: list[str] = [] + known = {"names", "estate", "estate_body_only", "site_prefix"} + for lineno, raw in enumerate(text.splitlines(), 1): + # Strip a BOM before anything else: a UTF-8 BOM immediately ahead of the first section header + # defeats the ``startswith("[")`` test below, silently voiding that entire section. + line = raw.lstrip(_BOM).strip() + if not line or line.startswith("#"): + continue + if line.startswith("[") and line.endswith("]"): + section = line[1:-1].strip().lower() + if section not in known: + # Silently discarding an unrecognised section is how a typo'd header dropped up to 13 + # tokens with no diagnostic at all. Name it. + print( + f"scan_forbidden: line {lineno}: unknown section header — its entries will be " + f"IGNORED (expected one of: {', '.join(sorted(known))})", + file=sys.stderr, + ) + continue + if section is None: + print( + f"scan_forbidden: line {lineno}: content before the first section header — IGNORED", + file=sys.stderr, + ) + continue + if section == "names": + parts = [p.strip() for p in line.split(" | ")] + if len(parts) > 3: + # The field delimiter is a space-pipe-space, so a regex with a SPACED alternation + # (``a | b``) splits into extra fields and parts[0] silently truncates to its first + # branch -- still compiling, so the re.error handler never fires and the count is + # unchanged. Ambiguous input is refused rather than half-honoured. + print( + f"scan_forbidden: line {lineno}: [names] entry has {len(parts)} fields, expected " + "at most 3 (PATTERN | REASON | CASE) — a spaced regex alternation would be " + "truncated, so this entry is IGNORED (content withheld)", + file=sys.stderr, + ) + continue + pattern = parts[0] + reason = parts[1] if len(parts) > 1 and parts[1] else "customer/vendor token" + case = parts[2].lower() if len(parts) > 2 and parts[2] else "i" + if case not in {"i", "s"}: + # Warn, but KEEP the entry. Dropping it would lose a detector, and under-detection is + # the dangerous direction; the ``else re.I`` below already falls back to + # case-INSENSITIVE, which is strictly broader than case-sensitive and so can only + # over-match, never under-match. Fail toward more detection. + print( + f"scan_forbidden: line {lineno}: [names] CASE field is not 'i' or 's' — defaulting " + "to case-INSENSITIVE (content withheld)", + file=sys.stderr, + ) + # Only an explicit "s" is case-sensitive; anything else (including a malformed flag) is + # case-insensitive. There is deliberately no separate assignment implementing that + # fallback -- one would be dead code, indistinguishable from this line under mutation. + flags = 0 if case == "s" else re.I + if _reject_entry("name pattern", pattern): + continue + if _NEVER_MATCH_RE.search(pattern): + # ``(?!)`` is this module's own "detector off" sentinel. Accepted from a token source + # it would compile cleanly, count toward the floor, and never fire. + print( + "scan_forbidden: ignoring a name pattern that can never match " + "(contains the empty-negative-lookahead sentinel)", + file=sys.stderr, + ) + continue + try: + compiled = re.compile(pattern, flags) + if compiled.search(reason): + # A REASON is printed verbatim on every hit, into a world-readable Actions log on + # the public repo. A reason that names its own token therefore publishes the token + # on the first match -- the same defect as echoing it in a parse warning, just on + # the success path. Neutralise the LABEL, keep the DETECTOR: dropping the entry + # would trade a disclosure for under-detection, which is the worse failure. + print( + f"scan_forbidden: line {lineno}: [names] REASON matches its own pattern and " + "would echo the token on every hit — using the generic reason instead", + file=sys.stderr, + ) + reason = "customer/vendor token" + names.append((compiled, reason)) + except re.error: + # NEVER echo the pattern. This warning lands in a world-readable Actions log on the + # PUBLIC repo and the pattern IS the customer/vendor token, so a malformed line -- + # exactly the case where the source was mishandled -- would leak the thing the gate + # exists to protect. Position only. + print( + f"scan_forbidden: ignoring an uncompilable [names] regex at entry " + f"{len(names) + 1} (content withheld)", + file=sys.stderr, + ) + elif section == "estate": + if not _reject_entry("estate token", line): + estate.append(line.lower()) + elif section == "estate_body_only": + if not _reject_entry("estate_body_only token", line): + body_only.append(line.lower()) + elif section == "site_prefix": + # ASCII digits only: str.isdigit() is True for non-ASCII digits (e.g. Arabic-Indic), which + # would compile into the site-code alternation and never match an ASCII site code. + if line.isdigit() and line.isascii(): + prefixes.append(line) + else: + # Position only -- see the [names] branch above. A site prefix is customer data. + print( + f"scan_forbidden: ignoring a non-ASCII-numeric [site_prefix] entry at position " + f"{len(prefixes) + 1} (content withheld)", + file=sys.stderr, + ) + # DEDUPE before the caller counts. The floor is a count, so a double-pasted section would + # otherwise inflate it: 13 estate tokens pasted twice reads as 26 and masks the loss of 13 real + # ones. Duplicates also add no detection. Order is preserved so behaviour is deterministic. + estate = list(dict.fromkeys(estate)) + body_only = list(dict.fromkeys(body_only)) + prefixes = list(dict.fromkeys(prefixes)) + seen_names: dict[tuple[str, int], None] = {} + deduped_names: list[tuple[re.Pattern[str], str]] = [] + for compiled, why in names: + key = (compiled.pattern, compiled.flags) + if key not in seen_names: + seen_names[key] = None + deduped_names.append((compiled, why)) + return deduped_names, tuple(estate), tuple(body_only), prefixes + + +def reload_tokens() -> None: + """Recompute the externalized token globals from the current environment / local file. + + Called once at import and again at the top of :func:`main` (so a fresh CLI process always reflects + its environment). Tests call it after adjusting the source. Never raises: an absent source simply + yields empty tables (structural-only). + """ + global FORBIDDEN, ESTATE_TOKENS, SITE_CODE_RE, _SITE_CODE_FILE, TOKENS_PRESENT + global _SITE_CODE_PATTERN_LITERAL, _ESTATE_BODY_ONLY, _ESTATE_FILE_RES, _SITE_PREFIXES + text = _resolve_token_text() + names, estate, body_only, prefixes = _parse_tokens(text or "") + # A source being PRESENT is not the same as a source being USABLE. Deriving TOKENS_PRESENT from + # ``text is not None`` alone meant a mangled secret -- headers lost, comments only, a UTF-8 BOM ahead + # of the first section header -- produced ZERO detectors while still reporting "tokens present", so + # MEFOR_REQUIRE_TOKENS=1 passed and the gate ran structural-only with a green tick. The cutover + # runbook has the owner paste a whole sectioned file into a GitHub secret box, which is exactly the + # mangling case. Require that parsing actually yielded something. + TOKENS_PRESENT = text is not None and bool(names or estate or prefixes) + FORBIDDEN = names + ESTATE_TOKENS = estate + _ESTATE_BODY_ONLY = frozenset(body_only) + _ESTATE_FILE_RES = tuple( + (token, re.compile(rf"(? dict[str, int]: + """Detector counts for the current token tables. + + ``main`` prints these on every run. "The gate exited 0" is NOT evidence it scanned anything -- these + counts are what distinguishes a real scan from a vacuous one, and they are what + ``MEFOR_MIN_DETECTORS`` / ``--require-tokens N`` assert against. + + ``estate`` and ``estate_file_scanned`` legitimately differ: tokens listed under + ``[estate_body_only]`` are ordinary words or vendor product names that mass-false-positive over + tracked source, so they are applied to message BODIES but held out of the file scan. A gap between + the two is expected, not a defect -- only the FLOOR counts (``names`` / ``estate`` / + ``site_prefixes``) feed the gate. + """ + return { + "names": len(FORBIDDEN), + "estate": len(ESTATE_TOKENS), + "estate_file_scanned": len(_ESTATE_FILE_RES), + "site_prefixes": len(_SITE_PREFIXES), + } + + +#: The sections a fully-loaded token source must ALL populate. ``TOKENS_PRESENT`` is deliberately an OR +#: (any section proves a source exists at all); the gate below is the AND. +_FLOOR_SECTIONS: tuple[str, ...] = ("names", "estate", "site_prefixes") + + +def parse_min_spec(value: str) -> int | dict[str, int]: + """Parse a floor spec: either a bare total (``21``) or per-section (``names=7,estate=13``). + + Per-section is STRICTLY STRONGER and is what CI should use. A bare total is a SUM, so growth in a + cheap section masks collapse in an expensive one -- names 7->1 alongside estate 13->19 still + totals 21 and passes, with 6 of 7 customer-name detectors silently off. + """ + value = value.strip() + if value.isdigit() and value.isascii(): + return int(value) + spec: dict[str, int] = {} + for part in value.split(","): + part = part.strip() + if not part: + continue + if "=" not in part: + raise ValueError(f"floor spec {part!r} is neither an integer nor section=N") + name, _, num = part.partition("=") + name, num = name.strip(), num.strip() + if name not in _FLOOR_SECTIONS: + raise ValueError( + f"unknown floor section {name!r} (expected one of {', '.join(_FLOOR_SECTIONS)})" + ) + if not (num.isdigit() and num.isascii()): + raise ValueError(f"floor for {name!r} must be an integer, got {num!r}") + spec[name] = int(num) + if not spec: + raise ValueError("empty floor spec") + return spec + + +def token_floor_failure(min_detectors: int | dict[str, int] | None = None) -> str | None: + """Why the loaded tables are not trustworthy, or ``None`` if they are. + + Presence is not sufficiency. ``TOKENS_PRESENT`` is satisfied by ANY ONE section surviving, so a + partially-mangled source -- a clipboard paste that lost a section, a typo'd header, a BOM ahead of + the first header -- loaded as few as 1 of 21 detectors and still passed a "fail-closed" gate with a + green tick. Losing only the FINAL line, the likeliest paste error, silently disabled every + site-code detector while printing "fail-closed". This closes that in two independent ways: + + * every floor section must be non-empty (catches whole-section loss), and + * the total must meet ``min_detectors`` when given (catches loss WITHIN a section, e.g. 7 names + down to 3, which the section check alone cannot see). + + The expected total is deliberately supplied from OUTSIDE the token source (CI env / CLI arg). An + ``[expect]`` count carried inside the file would be destroyed by the very mangling it is meant to + detect -- self-referential, and vacuous in exactly the way this function exists to prevent. + """ + if not TOKENS_PRESENT: + return ( + "no token source is configured (set MEFOR_FORBIDDEN_TOKENS or place " + "scripts/security/scan-tokens.local.txt) — refusing to run structural-only" + ) + counts = loaded_token_counts() + empty = [s for s in _FLOOR_SECTIONS if not counts[s]] + if empty: + return ( + f"the token source loaded but section(s) {', '.join(empty)} are EMPTY — a partial or " + f"mangled source (loaded {', '.join(f'{s}={counts[s]}' for s in _FLOOR_SECTIONS)}). " + "Re-set the source from the file rather than pasting it" + ) + if isinstance(min_detectors, dict): + short = [f"{s} {counts[s]}<{need}" for s, need in min_detectors.items() if counts[s] < need] + if short: + return ( + f"section detector counts below their floor ({', '.join(short)}) — the token source " + "is incomplete. Per-section floors catch the case a total cannot: growth in one " + "section masking collapse in another" + ) + elif min_detectors is not None: + total = sum(counts[s] for s in _FLOOR_SECTIONS) + if total < min_detectors: + return ( + f"only {total} detectors loaded, below the required floor of {min_detectors} " + f"({', '.join(f'{s}={counts[s]}' for s in _FLOOR_SECTIONS)}) — the token source is " + "incomplete. This is a FLOOR: adding tokens is free, losing them is not" + ) + return None + + +#: Benign strings an allowlist entry must NOT match. An entry that matches any of these is broad +#: enough to veto ordinary source lines, and therefore broad enough to switch the gate off wholesale. +#: The empty string is included so a pattern that can match nothing-in-particular is caught too. +_ALLOWLIST_CANARIES: tuple[str, ...] = ( + "", + "the quick brown fox jumps over the lazy dog", + "def handler(message: Message) -> list[Send]:", + "# a perfectly ordinary comment", + "0123456789", +) + + +#: Identifier separators neutralised before the second name-pattern pass. Only ``_`` today: it is +#: the one separator that is also a WORD character, so it is the only one that defeats . Kept as +#: a pattern (not str.replace) so a future separator is a one-character edit. +_IDENT_SEP = re.compile(r"_") + + +def _takes_ident_pass(pat: re.Pattern[str]) -> bool: + r"""Whether a name pattern is eligible for the identifier (underscore-neutralised) pass. + + Only SINGLE-TOKEN patterns are. The gap being closed is a lone token buried in an identifier -- + ``OB_TOKEN_ORU`` -- which a \b-anchored pattern cannot see because ``_`` is a word character. + + A MULTI-WORD pattern must be excluded, and this is not hypothetical: neutralising ``_`` for a + two-word phrase makes it match ordinary snake_case. Measured on the real tree, a phrase pattern + matched 9 occurrences of a perfectly generic Python identifier across two files -- every one a + false positive, and enough to block the cutover PR on its own required gate. Whitespace in the + pattern (literal or ``\s``) is the signal. + """ + src = pat.pattern + return " " not in src and r"\s" not in src + + +def _load_allowlist() -> list[re.Pattern[str]]: + """Optional line-regex allowlist for vetted false positives (one regex per line, '#' comments). + + NEVER allowlist real customer data -- only genuinely-innocent lines a structural pattern + over-matches. + """ + f = Path(__file__).parent / "scan-allowlist.txt" + if not f.exists(): + return [] + out: list[re.Pattern[str]] = [] + for lineno, raw in enumerate(f.read_text(encoding="utf-8").splitlines(), 1): + line = raw.strip() + if not line or line.startswith("#"): + continue + try: + pat = re.compile(line) + except re.error: + print( + f"scan_forbidden: scan-allowlist.txt line {lineno}: uncompilable regex — IGNORED", + file=sys.stderr, + ) + continue + # An allowlist entry is a per-line VETO applied BEFORE any detector runs, so one over-broad + # entry disables the ENTIRE gate -- every detector, every file -- while the "loaded ..." + # diagnostic still reports full counts, making the log indistinguishable from a healthy run. + # This file is committed and public, so the entry that does it need not be malicious: a + # hastily-written `.` or `.*` for one false positive is enough. Reject anything that matches + # ordinary prose or the empty string. + if any(pat.search(probe) is not None for probe in _ALLOWLIST_CANARIES): + print( + f"scan_forbidden: scan-allowlist.txt line {lineno}: pattern matches ordinary text — " + "it would veto every line and disable the whole gate. IGNORED. Anchor it or make it " + "specific to the false positive you are excusing.", + file=sys.stderr, + ) + continue + out.append(pat) + return out + + +ALLOWLIST = _load_allowlist() +reload_tokens() + + +# -------------------------------------------------------------------------------------------------- +# Scanning +# -------------------------------------------------------------------------------------------------- + + +def _git_tracked() -> list[str]: + # Fixed literal argv ["git", "ls-files"]: no shell, no user input. + return subprocess.run( # nosec B603 B607 + ["git", "ls-files"], capture_output=True, text=True, check=True + ).stdout.splitlines() + + +def _candidate_files(argv: list[str]) -> list[tuple[Path, str]]: + # Returns (openable path, scan-root-RELATIVE posix). The relative posix is what all skip decisions + # and hit reports key on, so the logic is identical whether we scan tracked files (already + # repo-relative) or a --path snapshot in an arbitrary directory. + if argv and argv[0] == "--path": + if len(argv) < 2: + print("scan_forbidden: --path requires a directory", file=sys.stderr) + sys.exit(2) + root = Path(argv[1]) + return [(p, p.relative_to(root).as_posix()) for p in root.rglob("*") if p.is_file()] + if argv: + return [(Path(a), Path(a).as_posix()) for a in argv] + return [(Path(p), Path(p).as_posix()) for p in _git_tracked()] + + +def _read_text(path: Path) -> str | None: + try: + data = path.read_bytes() + except OSError: + return None + if b"\x00" in data[:4096]: # binary file -- nothing to scan + return None + return data.decode("utf-8", errors="replace") + + +def scan_file(path: Path, rel_posix: str | None = None, *, show_context: bool = False) -> list[str]: + """Forbidden-content hits in a file, as ``path:line: reason`` strings. + + Reasons-only by default (location + category, never the matched value or line) so a hit report is + safe to print in public CI logs. ``show_context`` additionally appends the matched value and the + trimmed line -- for local triage only, never CI. + """ + posix = rel_posix if rel_posix is not None else path.as_posix() + if _is_skipped(posix): + return [] + text = _read_text(path) + if text is None: + return [] + ip_scan = path.suffix not in _IP_SKIP_SUFFIXES and path.name not in _IP_SKIP_NAMES + site_scan = path.suffix not in _SITE_SKIP_SUFFIXES and path.name not in _SITE_SKIP_NAMES + hits: list[str] = [] + for lineno, line in enumerate(text.splitlines(), 1): + if any(a.search(line) for a in ALLOWLIST): + continue + ctx = f": {line.strip()[:120]}" if show_context else "" + before = len(hits) + # ``_`` is a WORD character, so a \b-anchored name pattern cannot see its token inside an + # identifier: ``\bTOKEN\b`` does not match ``OB_TOKEN_ORU``. That blindness has hidden real + # leaks in this repo more than once, and the estate set only covers it for tokens that happen + # to appear in BOTH lists. Scanning a shadow copy with identifier separators neutralised closes + # it for EVERY name pattern rather than for the instances someone remembered to duplicate. + # Substitution preserves length, so offsets and ``show_context`` stay meaningful. + unwrapped = _IDENT_SEP.sub(" ", line) + for pat, reason in FORBIDDEN: + if pat.search(line) or (_takes_ident_pass(pat) and pat.search(unwrapped)): + hits.append(f"{posix}:{lineno}: {reason}{ctx}") + if site_scan: + for m in _SITE_CODE_FILE.finditer(line): + value = f" ({m.group(0)})" if show_context else "" + hits.append(f"{posix}:{lineno}: site code{value}{ctx}") + if _SITE_CODE_PATTERN_LITERAL.search(line): + hits.append(f"{posix}:{lineno}: site-code pattern written out{ctx}") + if ip_scan: + for m in _IPV4.finditer(line): + ip = m.group(0) + if not _ALLOWED_IP.match(ip): + value = f" ({ip})" if show_context else "" + hits.append(f"{posix}:{lineno}: routable IP address{value}{ctx}") + # Internal-environment disclosure. Reason-only, never the value: the slug or account name IS + # the disclosure, so echoing it into a public CI log would publish what the hit is reporting. + if _WORKTREE_SLUG.search(line): + hits.append(f"{posix}:{lineno}: worktree/branch slug (internal project name)") + if _HOME_PATH.search(line): + hits.append(f"{posix}:{lineno}: absolute user-home path (OS account name)") + # Estate substrings run LAST and only on a line nothing else flagged: the sets overlap (the + # customer name is typically in [names] AND [estate]), and double-reporting one line adds noise + # rather than information. What this adds is the case no other detector can reach -- a token + # butted against word characters, e.g. ``OB__ORU``. + if len(hits) == before: + for _token, estate_pat in _ESTATE_FILE_RES: + if estate_pat.search(line): + hits.append(f"{posix}:{lineno}: estate token{ctx}") + break + return hits + + +def scan_text(text: str, *, include_estate: bool = False) -> list[str]: + """Forbidden-token **reasons** in an in-memory string -- the importable single source of truth for + body scanning (the anonymizer leak-check, ADR 0030 §5). + + Always runs the FORBIDDEN name patterns + a routable-IP check. With ``include_estate`` (a message + body, where a token can sit *inside* a field the word-boundary form would miss) it also applies the + ESTATE_TOKENS substring set. The ``site_prefix`` code is deliberately NOT checked here: a broad + substring search false-positives on every value that merely contains such a run (timestamps, + fabricated dates), so the anonymizer checks it **field-anchored** instead + (``surrogates.message_has_site_code``). Returns REASONS ONLY -- never the matched text -- so a + caller may raise/log the result without leaking PHI. + """ + reasons: list[str] = [] + for pat, reason in FORBIDDEN: + if pat.search(text): + reasons.append(reason) + for m in _IPV4.finditer(text): + if not _ALLOWED_IP.match(m.group(0)): + reasons.append("routable IP address") + break + if include_estate: + lowered = text.lower() + reasons.extend(f"estate token ({token})" for token in ESTATE_TOKENS if token in lowered) + return reasons + + +def _parse_require_flag(argv: list[str]) -> tuple[bool, int | dict[str, int] | None, list[str]]: + """Pull ``--require-tokens[=N]`` out of argv, returning (require, floor, remaining args). + + pre-commit can pass ARGS to a hook but has no per-hook ``env:`` key, so this flag is the only way + to make the COMMIT-time gate fail closed. Without it a fresh clone or a new worktree -- neither of + which carries the git-ignored token file -- runs every commit with zero name/estate/site detectors + and reports "Passed". CI keeps using the environment variables. + """ + require = False + minimum: int | dict[str, int] | None = None + rest: list[str] = [] + for arg in argv: + if arg == "--require-tokens": + require = True + elif arg.startswith("--require-tokens="): + value = arg.split("=", 1)[1] + require, minimum = True, parse_min_spec(value) + elif arg.startswith("--") and arg != "--path": + # Refuse unknown flags rather than treating them as filenames. Falling through put an + # unrecognised token at rest[0], which defeated the ``rest[0] == "--path"`` test: the run + # silently became a file-list scan of a nonexistent file, examined ZERO files, and exited + # 0. A typo'd flag must fail loudly, not quietly scan nothing. + raise ValueError(f"unknown option {arg!r}") + else: + rest.append(arg) + return require, minimum, rest + + +#: Values accepted as "on" for MEFOR_REQUIRE_TOKENS. An exact ``== "1"`` compare meant a well-meant +#: ``true``/``yes`` silently disabled the gate while looking configured. +_TRUTHY = frozenset({"1", "true", "yes", "on"}) + + +def main(argv: list[str]) -> int: + reload_tokens() + show_context = "--show-context" in argv + rest = [a for a in argv if a != "--show-context"] + try: + flag_require, minimum, rest = _parse_require_flag(rest) + except ValueError as exc: + print(f"scan_forbidden: {exc}", file=sys.stderr) + return 2 + + # Announce what loaded BEFORE any refusal, so a failure shows the counts that CAUSED it. Exit 0 + # 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() + print( + "scan_forbidden: loaded " + + ", ".join(f"{k}={v}" for k, v in counts.items()) + + ("" if TOKENS_PRESENT else " [STRUCTURAL-ONLY: no token source configured]"), + file=sys.stderr, + ) + + env_require = os.environ.get("MEFOR_REQUIRE_TOKENS", "").strip() + if env_require and env_require.lower() not in _TRUTHY: + print( + f"scan_forbidden: MEFOR_REQUIRE_TOKENS={env_require!r} is not a recognised value " + f"({'/'.join(sorted(_TRUTHY))}) — refusing rather than silently running unguarded.", + file=sys.stderr, + ) + return 2 + require = flag_require or env_require.lower() in _TRUTHY + if minimum is None: + env_min = os.environ.get("MEFOR_MIN_DETECTORS", "").strip() + if env_min: + try: + minimum = parse_min_spec(env_min) + except ValueError as exc: + print(f"scan_forbidden: MEFOR_MIN_DETECTORS: {exc}", file=sys.stderr) + return 2 + # Asking for a floor implies requiring tokens; otherwise a floor set without the require + # flag would be silently inert -- another gate that cannot fail. + require = True + + if require: + why = token_floor_failure(minimum) + if why is not None: + print(f"scan_forbidden: {why} (fail closed).", file=sys.stderr) + return 2 + + path_mode = bool(rest) and rest[0] == "--path" + hits: list[str] = [] + scanned = 0 + for abs_path, rel in _candidate_files(rest): + # SKIP_DIRS is evaluated on the RELATIVE parts, so a scan-root path component that happens to be + # named build/venv/dist/… can no longer short-circuit the scan of every file. + if not abs_path.is_file() or set(Path(rel).parts) & SKIP_DIRS or _is_skipped(rel): + continue + scanned += 1 + hits.extend(scan_file(abs_path, rel, show_context=show_context)) + + # A --path scan that examined ZERO files is not a pass: it means the whole tree was skipped (a + # SKIP_DIRS component in the path, or an empty/wrong tree). Refuse rather than report a vacuous clean. + if path_mode and scanned == 0: + print( + "scan_forbidden: --path examined ZERO files (all skipped, or empty tree) — refusing " + "(fail closed). Check the scan path for a SKIP_DIRS component (build/venv/dist/…).", + file=sys.stderr, + ) + return 2 + + if hits: + print("FORBIDDEN CONTENT -- commit blocked (fail closed):", file=sys.stderr) + for h in hits: + print(f" {h}", file=sys.stderr) + print( + f"\n{len(hits)} hit(s). A real customer/host string must be removed, not allowlisted. " + "For a genuine false positive, narrow the pattern or add a vetted line to " + "scripts/security/scan-allowlist.txt.", + file=sys.stderr, + ) + return 1 + return 0 + + +if __name__ == "__main__": + raise SystemExit(main(sys.argv[1:])) diff --git a/tee/anon/leak.py b/tee/anon/leak.py index b83a340..0b7c556 100644 --- a/tee/anon/leak.py +++ b/tee/anon/leak.py @@ -3,15 +3,16 @@ """Leak-check for the standalone tee (ADR 0030 §5) — a thin front-end over the publish-guard authority. The token tables (customer/partner names + estate-vendor tokens) are the owner-managed -``scripts/publish/scan_forbidden.py`` set; this file **loads them from that guard by path at import** +``scripts/security/scan_forbidden.py`` set; this file **loads them from that guard by path at import** (the same way ``messagefoundry/anon/leak.py`` does) rather than vendoring a copy — so **no literal or fragmented customer/vendor token appears in this tracked, published file**, and there is nothing to drift (``test_anon_parity`` still pins the two equal). Loading by path keeps the tee ``messagefoundry``-free. -``scripts/publish/`` is deny-listed on the OSS mirror, so the guard is **absent** there: the name + -estate tables then load **empty** and the leak-check degrades to a no-op for those (a public checkout -has no customer estate to leak). The generic site-code / IP detectors keep a literal default so the -anonymizer's structural checks still function without the guard. +The real tokens themselves are EXTERNALIZED out of the guard (a git-ignored token file / +``MEFOR_FORBIDDEN_TOKENS``), so a token-less checkout (a fork, or an installed wheel with no +``scripts/``) loads the name + estate + site-code tables **empty** and the leak-check degrades to a +no-op for those (a public checkout has no customer estate to leak). The generic IP detector keeps a +literal default so the anonymizer's structural IP check still functions without a token source. Returns **reasons only** (never the matched text), and is the fail-closed backstop, not the primary control: it catches known *tokens*, not structural PHI (ADR 0030 §5). @@ -27,13 +28,13 @@ def _load_publish_guard(_start: Path | None = None) -> object | None: - """Load the owner-managed publish guard (``scripts/publish/scan_forbidden.py``) by path, walking up - from this file. It is the SINGLE source for the token tables, so none live literally here. Absent on - the OSS mirror (``scripts/publish/`` is deny-listed) → returns ``None`` and the tables load empty. - ``_start`` overrides the search origin for tests.""" + """Load the owner-managed guard (``scripts/security/scan_forbidden.py``) by path, walking up from + this file. It is the SINGLE source for the token tables, so none live literally here. Absent from an + installed wheel with no ``scripts/`` → returns ``None`` and the tables load empty. ``_start`` + overrides the search origin for tests.""" origin = (_start if _start is not None else Path(__file__)).resolve() for parent in origin.parents: - candidate = parent / "scripts" / "publish" / "scan_forbidden.py" + candidate = parent / "scripts" / "security" / "scan_forbidden.py" if candidate.exists(): spec = importlib.util.spec_from_file_location("tee_anon_publish_guard", candidate) if spec is not None and spec.loader is not None: @@ -52,9 +53,11 @@ def _load_publish_guard(_start: Path | None = None) -> object | None: ESTATE_TOKENS: tuple[str, ...] = tuple(_GUARD.ESTATE_TOKENS) if _GUARD else () # type: ignore[attr-defined] # Generic structural detectors (NOT customer data): loaded from the guard when present (parity), with a -# literal default so a public checkout's site-code / IP checks still work without the guard. +# literal default so a public checkout's IP check still works without the guard. The site-code detector +# is EXTERNALIZED (no literal prefix in source) — empty (never-match) without a token source. +_NEVER = re.compile(r"(?!x)x") _OCTET = r"(?:25[0-5]|2[0-4]\d|1\d\d|[1-9]?\d)" -SITE_CODE_RE = _GUARD.SITE_CODE_RE if _GUARD else re.compile(r"54\d{4}") # type: ignore[attr-defined] +SITE_CODE_RE = _GUARD.SITE_CODE_RE if _GUARD else _NEVER # type: ignore[attr-defined] _IPV4 = _GUARD._IPV4 if _GUARD else re.compile(rf"(? object | None: def scan_text(text: str, *, include_estate: bool = False) -> list[str]: """Forbidden-token **reasons** in ``text`` (no matched text) — vendored twin of - ``scan_forbidden.scan_text``. The ``54xxxx`` site code is checked field-anchored by - :func:`leak_check`, not here (see the engine docstring).""" + ``scan_forbidden.scan_text``. The site code is checked field-anchored by :func:`leak_check`, not + here (see the engine docstring).""" reasons: list[str] = [] for pat, reason in FORBIDDEN: if pat.search(text): @@ -89,10 +92,10 @@ def scan_text(text: str, *, include_estate: bool = False) -> list[str]: def leak_check(text: str) -> list[str]: - """Forbidden-token hits in ``text`` (empty = clean) — the tee's fail-closed leak gate. The - ``54xxxx`` site code is checked **field-anchored** (matching the replace path), so a scrub miss is - caught without false-positiving on a value that merely contains a ``54xxxx`` run.""" + """Forbidden-token hits in ``text`` (empty = clean) — the tee's fail-closed leak gate. The site + code is checked **field-anchored** (matching the replace path), so a scrub miss is caught without + false-positiving on a value that merely contains a site-code run.""" hits = scan_text(text, include_estate=True) if message_has_site_code(text): - hits.append("site-code pattern (54xxxx)") + hits.append("site-code pattern") return hits diff --git a/tee/anon/surrogates.py b/tee/anon/surrogates.py index 18bf003..4295f81 100644 --- a/tee/anon/surrogates.py +++ b/tee/anon/surrogates.py @@ -19,19 +19,103 @@ from __future__ import annotations +import os import random import re from collections.abc import Callable from dataclasses import dataclass +from pathlib import Path from . import _pools from .keying import Keyer from .rules import SurrogateKind -#: A field/component value that *is exactly* a ``54xxxx`` site code (ADR 0030 §5, owner decision -#: 2026-06-20: **field-anchored** — ``fullmatch`` on a whole component, so a coincidental ``54xxxx`` -#: inside a longer timestamp/order number is left alone, unlike the publish gate's broad catch-all). -SITE_CODE_RE = re.compile(r"54\d{4}") +#: A never-matching pattern — the site-code detector when no prefix is configured (a token-less +#: public build). ``re`` has no built-in "match nothing", so encode one explicitly. +_NEVER: re.Pattern[str] = re.compile(r"(?!x)x") + + +def _resolve_token_text() -> str | None: + """The active token content, or ``None`` if no source is configured — the SAME source + ``scripts/security/scan_forbidden.py`` reads, so the site-code authority is externalized in one + place (never committed). ``MEFOR_FORBIDDEN_TOKENS`` wins: an existing file path is read; any other + non-empty value is inline content; an explicitly-empty value means "no source". Otherwise the + git-ignored ``scripts/security/scan-tokens.local.txt`` is located by walking up from this file + (a dev checkout); an installed wheel without it degrades to no-op. + """ + env = os.environ.get("MEFOR_FORBIDDEN_TOKENS") + if env is not None: + env = env.strip() + if not env: + return None + try: + p = Path(env) + if p.is_file(): + return p.read_text(encoding="utf-8") + except OSError: + pass + return env + for parent in Path(__file__).resolve().parents: + candidate = parent / "scripts" / "security" / "scan-tokens.local.txt" + try: + if candidate.is_file(): + return candidate.read_text(encoding="utf-8") + except OSError: + pass + return None + + +def _load_site_prefixes() -> tuple[str, ...]: + """The estate's site-code numeric prefixes, EXTERNALIZED out of source (they identify a real + customer estate; ADR 0030 §5, owner decision 2026-06-20). Only the ``[site_prefix]`` section of + the shared token file is read here, two-digit prefixes only (the site-code model is a two-digit + prefix + four digits). An absent source yields ``()`` — the site-code safety pass then no-ops, + correct for a token-less public build where the rule map is the primary control. + """ + text = _resolve_token_text() + if text is None: + return () + prefixes: list[str] = [] + section: str | None = None + for raw in text.splitlines(): + line = raw.strip() + if not line or line.startswith("#"): + continue + if line.startswith("[") and line.endswith("]"): + section = line[1:-1].strip().lower() + continue + if section == "site_prefix" and line.isdigit() and len(line) == 2: + prefixes.append(line) + return tuple(dict.fromkeys(prefixes)) + + +#: Field-anchored site-code detector (``fullmatch`` on a whole component, so a coincidental site-code +#: run inside a longer timestamp/order number is left alone, unlike the publish gate's broad catch-all). +#: Built from the externalized prefixes; ``_NEVER`` when none are configured. Recomputed by +#: :func:`reload_site_prefixes`; read as a stable public contract by the leak-check bridge. +_SITE_PREFIXES: tuple[str, ...] = () +SITE_CODE_RE: re.Pattern[str] = _NEVER +#: Two-digit prefixes that are NOT site-code prefixes — a fabricated replacement code draws its lead +#: from here so it can never itself be a site code. +_NON_SITE_PREFIXES: tuple[str, ...] = () + + +def reload_site_prefixes() -> None: + """Recompute the externalized site-code globals from the current environment / local token file. + Called once at import and by tests after adjusting the source. Never raises.""" + global _SITE_PREFIXES, SITE_CODE_RE, _NON_SITE_PREFIXES + _SITE_PREFIXES = _load_site_prefixes() + if _SITE_PREFIXES: + alt = "|".join(re.escape(p) for p in _SITE_PREFIXES) + SITE_CODE_RE = re.compile(rf"(?:{alt})\d{{4}}") + else: + SITE_CODE_RE = _NEVER + _NON_SITE_PREFIXES = tuple( + f"{n:02d}" for n in range(10, 100) if f"{n:02d}" not in _SITE_PREFIXES + ) + + +reload_site_prefixes() #: What :class:`SurrogateKind.FREETEXT` collapses a narrative field to. Free text routinely embeds #: identifiers (a name/MRN/DOB in prose) that field-level surrogation cannot reach and the leak-check @@ -194,19 +278,23 @@ def surrogate_field(kind: SurrogateKind, value: str, keyer: Keyer, seps: Seps) - def scrub_site_codes(value: str, keyer: Keyer, seps: Seps) -> str: - """Field-anchored ``54xxxx`` safety pass (ADR 0030 §5): replace any **whole component** that is - exactly a site code with a fabricated 6-digit non-``54`` code, leaving coincidental ``54xxxx`` - inside a longer value (timestamps, order numbers) untouched. Applied to every field as a net - under the rule map, since a site code can lurk in an unexpected field. + """Field-anchored site-code safety pass (ADR 0030 §5): replace any **whole component** that is + exactly a site code with a fabricated non-site code of the same width, leaving a coincidental + site-code run inside a longer value (timestamps, order numbers) untouched. Applied to every field + as a net under the rule map, since a site code can lurk in an unexpected field. A no-op when no + site-code prefix is configured (a token-less build; the rule map remains the primary control). """ - if not value or "54" not in value: + if not value or not _SITE_PREFIXES: + return value + if not any(p in value for p in _SITE_PREFIXES): return value def _one(component: str) -> str: if not SITE_CODE_RE.fullmatch(component): return component rng = keyer.rng("sitecode", component) - return f"{rng.randrange(10, 54)}{_fake_digits(rng, 4)}" + prefix = rng.choice(_NON_SITE_PREFIXES) + return f"{prefix}{_fake_digits(rng, len(component) - len(prefix))}" reps = value.split(seps.repetition) out_reps = [] @@ -245,11 +333,12 @@ def read_message_seps(text: str) -> tuple[Seps, str] | None: def message_has_site_code(text: str) -> bool: - """True if any whole field/component/subcomponent **is exactly** a ``54xxxx`` site code — the - field-anchored leak-check that matches the scrub (ADR 0030 §5). Using ``fullmatch`` per component - (not a broad substring search) means a value that merely *contains* a ``54xxxx`` run — a timestamp, - a fabricated 1954/2054 date, a long order number — is not falsely flagged, while a genuine scrub - *miss* still is. Falls back to a broad search only for unstructured (no-MSH) text.""" + """True if any whole field/component/subcomponent **is exactly** a site code — the field-anchored + leak-check that matches the scrub (ADR 0030 §5). Using ``fullmatch`` per component (not a broad + substring search) means a value that merely *contains* a site-code run — a timestamp, a fabricated + date, a long order number — is not falsely flagged, while a genuine scrub *miss* still is. Falls + back to a broad search only for unstructured (no-MSH) text. Always False when no site-code prefix + is configured.""" parsed = read_message_seps(text) if parsed is None: return SITE_CODE_RE.search(text) is not None @@ -264,13 +353,13 @@ def message_has_site_code(text: str) -> bool: def scrub_message_site_codes(text: str, keyer: Keyer) -> str: - """Field-anchored ``54xxxx`` safety pass over a whole ``\\r``-delimited message (ADR 0030 §5). + """Field-anchored site-code safety pass over a whole ``\\r``-delimited message (ADR 0030 §5). Splits on the message's own separators and replaces any whole **component** that is exactly a site code, anywhere in the message — the net under the rule map for a site code lurking in an unexpected field. MSH-1 (field separator) and MSH-2 (encoding characters) are left untouched so the header stays parseable. The broad, unanchored catch-all stays in the publish leak-gate as - defense-in-depth; here we never over-redact a coincidental ``54xxxx`` inside a longer value. + defense-in-depth; here we never over-redact a coincidental site-code run inside a longer value. """ parsed = read_message_seps(text) if parsed is None: diff --git a/tests/test_anon_core.py b/tests/test_anon_core.py new file mode 100644 index 0000000..74b7c1b --- /dev/null +++ b/tests/test_anon_core.py @@ -0,0 +1,293 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Core anonymizer behaviour (ADR 0030): keying, the rule model, surrogates, the HL7 adapter, and the +fail-closed leak-check — engine side.""" + +from __future__ import annotations + +from collections.abc import Iterator +from pathlib import Path + +import pytest + +from messagefoundry.anon import ( + DEFAULT_RULES, + AnonError, + FieldRule, + Keyer, + LeakError, + RuleError, + SurrogateKind, + anonymize, + anonymize_checked, + leak, + leak_check, + load_rules, +) +from messagefoundry.anon.surrogates import Seps, scrub_site_codes, surrogate_field + +# The leak-check delegates to scripts/security/scan_forbidden.py (the relocated forbidden-content +# scanner). It ships on the public mirror but loads its real customer/vendor token list from a +# git-ignored local file / Actions secret, so on a fork checkout the token tables are EMPTY. The two +# leak tests below therefore INJECT synthetic tokens into the loaded scanner (rather than relying on the +# real list), so they exercise the mechanism identically with or without the secret and carry no real +# token. The scanner is still absent from an installed wheel (no scripts/), where the engine raises +# LeakCheckUnavailable by design — skip the two that need it there. +_LEAK_SCANNER = Path(__file__).resolve().parents[1] / "scripts" / "security" / "scan_forbidden.py" +_NO_SCANNER = pytest.mark.skipif( + not _LEAK_SCANNER.exists(), + reason="leak-check needs scripts/security/scan_forbidden.py (absent on an installed wheel)", +) + +_SALT = "unit-salt-0123456789abcdef" +_SEPS = Seps() + + +@pytest.fixture +def synthetic_site_prefix(monkeypatch: pytest.MonkeyPatch) -> Iterator[str]: + """Inject a SYNTHETIC two-digit site-code prefix into the externalized detector, so the site-code + tests exercise the real mechanism without any real prefix living in this (now-scanned) file.""" + from messagefoundry.anon import surrogates + + monkeypatch.setenv("MEFOR_FORBIDDEN_TOKENS", "[site_prefix]\n99\n") + surrogates.reload_site_prefixes() + yield "99" + monkeypatch.delenv("MEFOR_FORBIDDEN_TOKENS", raising=False) + surrogates.reload_site_prefixes() + + +def _msg(*segments: str) -> str: + return "\r".join(segments) + + +_SAMPLE = _msg( + r"MSH|^~\&|SAPP|SFAC|RAPP|RFAC|20260101120000||ADT^A01|MSGCTRL|P|2.5.1", + "EVN|A01|20260101120000", + r"PID|1||12345^^^HOSP^MR~67890^^^OTH^MR||DOE^JOHN^Q||19800101|M|||9 REAL ST^^TOWN^CA^90210||5551234567", + "NK1|1|DOE^JANE|SPO|9 REAL ST^^TOWN^CA^90210|5559998888", + "OBX|1|NM|8480-6^Systolic^LN||128|mm[Hg]", + "OBX|2|TX|NOTE^Note^LN||Patient JOHN DOE seen", + "NTE|1||free text note", +) + + +# --- keying --------------------------------------------------------------------------------------- + + +def test_keyer_deterministic_and_salt_sensitive() -> None: + a, b = Keyer("salt-aaaaaaaaaaaaaaaa"), Keyer("salt-aaaaaaaaaaaaaaaa") + assert a.seed("mrn", "12345") == b.seed("mrn", "12345") + assert a.seed("mrn", "12345") != a.seed("mrn", "54321") + assert a.seed("mrn", "12345") != a.seed("name", "12345") # kind is part of the key + assert Keyer("other-saltttttttttt").seed("mrn", "12345") != a.seed("mrn", "12345") + + +def test_keyer_rejects_weak_salt() -> None: + with pytest.raises(ValueError, match="at least"): + Keyer("short") + with pytest.raises(ValueError): + Keyer("") + + +# --- rule model ----------------------------------------------------------------------------------- + + +def test_default_rules_loaded_without_overlay() -> None: + assert load_rules(None) == DEFAULT_RULES + paths = {r.path for r in DEFAULT_RULES} + assert {"PID-3", "PID-5", "PID-7", "MRG-1", "MRG-4", "OBX-5", "NTE-3"} <= paths + # MSH / coded fields are NOT scrubbed (kept by omission) + assert not any(r.path.startswith("MSH-") for r in DEFAULT_RULES) + + +def test_overlay_adds_retargets_keeps_drops(tmp_path) -> None: + overlay = tmp_path / "anon.toml" + overlay.write_text( + '[hl7.fields]\n"ZPD-2" = "mrn"\n"PID-5" = "drop"\n\n[hl7]\nkeep = ["PID-13"]\n', + encoding="utf-8", + ) + rules = {r.path: r.kind for r in load_rules(overlay)} + assert rules["ZPD-2"] is SurrogateKind.MRN # added + assert rules["PID-5"] is SurrogateKind.DROP # retargeted + assert "PID-13" not in rules # keep cancels the default scrub + + +@pytest.mark.parametrize( + "body", + [ + '[hl7.fields]\n"PID-5.1" = "name"\n', # component path rejected + '[hl7.fields]\n"PID-5" = "scramble"\n', # unknown kind rejected + "[oops]\nx = 1\n", # unknown top-level table rejected + "[hl7]\nwat = 1\n", # unknown [hl7] key rejected + ], +) +def test_overlay_schema_is_enforced(tmp_path, body: str) -> None: + overlay = tmp_path / "anon.toml" + overlay.write_text(body, encoding="utf-8") + with pytest.raises(RuleError): + load_rules(overlay) + + +# --- surrogates ----------------------------------------------------------------------------------- + + +def test_surrogate_field_maps_each_repetition_and_preserves_authority() -> None: + keyer = Keyer(_SALT) + out = surrogate_field(SurrogateKind.MRN, "12345^^^HOSP^MR~67890^^^OTH^MR", keyer, _SEPS) + reps = out.split("~") + assert len(reps) == 2 + assert reps[0].endswith("^^^HOSP^MR") and reps[1].endswith("^^^OTH^MR") # authority kept + assert "12345" not in out and "67890" not in out # ids fabricated + + +def test_freetext_is_blunt_redacted() -> None: + assert ( + surrogate_field(SurrogateKind.FREETEXT, "anything at all", Keyer(_SALT), _SEPS) + == "[REDACTED]" + ) + + +def test_drop_blanks_and_empty_stays_empty() -> None: + assert surrogate_field(SurrogateKind.DROP, "x", Keyer(_SALT), _SEPS) == "" + assert surrogate_field(SurrogateKind.NAME, "", Keyer(_SALT), _SEPS) == "" + + +def test_site_code_scrub_is_field_anchored(synthetic_site_prefix: str) -> None: + keyer = Keyer(_SALT) + # The site-code prefix is externalized; the fixture injects a synthetic one, so no real site code + # sits in this (now-scanned) file. + code = synthetic_site_prefix + "0088" # a whole (synthetic) site code + # a whole component that IS a site code is replaced ... + assert code not in scrub_site_codes(f"WARD^{code}^A", keyer, _SEPS) + # ... but the same run INSIDE a longer value (timestamp) is left alone (field-anchored) + ts = f"2026{code}100" + assert scrub_site_codes(ts, keyer, _SEPS) == ts + + +# --- HL7 adapter ---------------------------------------------------------------------------------- + + +def test_anonymize_scrubs_phi_keeps_structure_and_routing() -> None: + out = anonymize(_SAMPLE, salt=_SALT) + # PHI gone + for phi in ("DOE", "JOHN", "12345", "67890", "19800101", "9 REAL ST", "5551234567"): + assert phi not in out, f"PHI {phi!r} leaked" + # structure/routing kept + assert "MSGCTRL" in out # MSH-10 control id preserved (correlation) + assert "ADT^A01" in out # message type preserved (routing) + assert "8480-6" in out and "128" in out # numeric OBX result preserved + assert out.count("\r") == _SAMPLE.count("\r") # same segment count + # two PID-3 repetitions survive as two repetitions + pid = next(line for line in out.split("\r") if line.startswith("PID")) + assert pid.split("|")[3].count("~") == 1 + + +def test_obx5_freetext_only_when_value_type_textual() -> None: + out = anonymize(_SAMPLE, salt=_SALT) + obx = [line for line in out.split("\r") if line.startswith("OBX")] + assert "128" in obx[0] # NM result kept + assert "[REDACTED]" in obx[1] # TX note redacted + assert "[REDACTED]" in next(line for line in out.split("\r") if line.startswith("NTE")) + + +def test_anonymize_is_deterministic_and_salt_sensitive() -> None: + assert anonymize(_SAMPLE, salt=_SALT) == anonymize(_SAMPLE, salt=_SALT) + assert anonymize(_SAMPLE, salt=_SALT) != anonymize(_SAMPLE, salt="different-saltttttttt") + + +def test_a40_merge_keeps_pid3_mrg1_linkage() -> None: + msg = _msg( + r"MSH|^~\&|A|B|C|D|20260101||ADT^A40|M1|P|2.5.1", + "PID|1||55501^^^H^MR||SMITH^ANN||19700101|F", + "MRG|55501^^^H^MR", + ) + out = anonymize(msg, salt=_SALT) + pid3 = next(line for line in out.split("\r") if line.startswith("PID")).split("|")[3] + mrg1 = next(line for line in out.split("\r") if line.startswith("MRG")).split("|")[1] + assert "55501" not in pid3 and pid3 == mrg1 # same surrogate => merge linkage survives + + +def test_anonymize_reads_custom_separators_from_msh() -> None: + msg = "MSH!*~\\&!A!B!C!D!20260101!!ADT^A01!M1!P!2.5.1\rPID!1!!13579*x*x*H*MR!!POE*MARY!!19900101!F" + out = anonymize(msg, salt=_SALT) + assert ( + "13579" not in out and "POE" not in out + ) # scrubbed despite '!' field / '*' component seps + + +# --- leak-check ----------------------------------------------------------------------------------- + + +@_NO_SCANNER +def test_leak_check_clean_and_dirty( + monkeypatch: pytest.MonkeyPatch, synthetic_site_prefix: str +) -> None: + # Inject a SYNTHETIC estate token so the check is exercised whether or not the real list is loaded. + monkeypatch.setattr(leak._scanner(), "ESTATE_TOKENS", ("acmecorp",)) + assert leak_check(anonymize(_SAMPLE, salt=_SALT)) == [] + site = ( + synthetic_site_prefix + "0088" + ) # a synthetic site code — no real prefix in this scanned file + hits = leak_check(f"note mentioning ACMECORP and {site}") + assert any("acmecorp" in h.lower() for h in hits) # estate token named + assert any("site-code" in h.lower() for h in hits) # field-anchored site-code pattern caught + + +@_NO_SCANNER +def test_anonymize_checked_fails_closed_and_is_phi_safe(monkeypatch: pytest.MonkeyPatch) -> None: + # synthetic estate token in a KEPT field (MSH-3) survives surrogation -> must raise + monkeypatch.setattr(leak._scanner(), "ESTATE_TOKENS", ("acmecorp",)) + dirty = _msg( + r"MSH|^~\&|ACMECORP|B|C|D|20260101||ADT^A01|M1|P|2.5.1", + "PID|1||999^^^H^MR||DOE^JOHN||19800101|M", + ) + with pytest.raises(LeakError) as exc: + anonymize_checked(dirty, salt=_SALT) + message = str(exc.value) + assert "acmecorp" in message.lower() # names the token category + assert "DOE" not in message and "999" not in message # never echoes the body + + +def test_alphanumeric_identifier_preserves_width_and_shape() -> None: + msg = _msg( + r"MSH|^~\&|A|B|C|D|20260101||ADT^A01|M1|P|2.5.1", + "PID|1||AB0049^^^H^MR||X^Y", + ) + out = anonymize(msg, salt=_SALT) + id_part = next(line for line in out.split("\r") if line.startswith("PID")).split("|")[3] + id_part = id_part.split("^")[0] + assert "AB0049" not in out + assert len(id_part) == 6 # width preserved (not shrunk to a digit count) + assert id_part[:2].isalpha() and id_part[2:].isdigit() # shape preserved: 2 letters + 4 digits + + +@pytest.mark.parametrize(("original", "width"), [("1980", 4), ("198001", 6), ("19800101", 8)]) +def test_partial_dob_preserves_precision(original: str, width: int) -> None: + msg = _msg( + r"MSH|^~\&|A|B|C|D|20260101||ADT^A01|M1|P|2.5.1", + f"PID|1||9^^^H^MR||X^Y||{original}", + ) + out = anonymize(msg, salt=_SALT) + dob = next(line for line in out.split("\r") if line.startswith("PID")).split("|")[7] + assert ( + len(dob) == width and dob.isdigit() and dob != original + ) # precision/width kept, value fake + + +def test_no_msh_message_is_refused_fail_closed() -> None: + with pytest.raises(AnonError): + anonymize("PID|1||9^^^H^MR||DOE^JOHN||19800101|M", salt=_SALT) + + +def test_mllp_framed_message_is_anonymized() -> None: + framed = "\x0b" + _SAMPLE + "\x1c\r" # VT … FS CR framing + out = anonymize(framed, salt=_SALT) + assert "DOE" not in out and out.startswith("MSH") # framing stripped, body scrubbed + + +def test_anonymize_with_explicit_rules_only_touches_those_fields() -> None: + out = anonymize(_SAMPLE, salt=_SALT, rules=(FieldRule("PID-5", SurrogateKind.NAME),)) + pid = next(line for line in out.split("\r") if line.startswith("PID")) + assert "DOE" not in pid.split("|")[5] # PID-5 scrubbed + assert "12345" in pid # PID-3 left intact (not in the explicit rule set) + assert "DOE^JANE" in out # NK1-2 untouched (only PID-5 was in scope) diff --git a/tests/test_anon_integration.py b/tests/test_anon_integration.py index e1727be..6d7b7e1 100644 --- a/tests/test_anon_integration.py +++ b/tests/test_anon_integration.py @@ -197,7 +197,7 @@ def test_corpus_from_file_rejects_empty_and_malformed(tmp_path) -> None: # mirror too. # # The IP is ASSEMBLED at runtime (never a source literal): the anon leak-check and the CI publish guard -# share scripts/publish/scan_forbidden.py, so a literal routable IP in this file trips the repo +# share scripts/security/scan_forbidden.py, so a literal routable IP in this file trips the repo # forbidden-content scan even though it is synthetic (public DNS, not a customer host). _LEAK_IP = ".".join(["8"] * 4) _LEAKY_RAW = ( diff --git a/tests/test_anon_parity.py b/tests/test_anon_parity.py index f9709dd..0ddac6e 100644 --- a/tests/test_anon_parity.py +++ b/tests/test_anon_parity.py @@ -40,12 +40,13 @@ def _load_scan_forbidden() -> object: - path = _ROOT / "scripts" / "publish" / "scan_forbidden.py" + # scripts/security/, not the retired scripts/publish/. The scanner is COMMITTED now, so this skip + # should effectively never fire in a source checkout — it remains only for an installed wheel with + # no scripts/ tree above it. Leaving the old path here silently skipped the parity assertions + # below, which are the only thing keeping tee/anon/leak.py's tables identical to the guard's. + path = _ROOT / "scripts" / "security" / "scan_forbidden.py" if not path.exists(): - # Private-only (scripts/publish/ is deny-listed in the OSS mirror); skip where it's absent. - pytest.skip( - "scan_forbidden.py is private-only (OSS-mirror deny-list)", allow_module_level=True - ) + pytest.skip("scan_forbidden.py not found above this tree", allow_module_level=True) spec = importlib.util.spec_from_file_location("scan_forbidden", path) assert spec is not None and spec.loader is not None mod = importlib.util.module_from_spec(spec) @@ -81,17 +82,32 @@ def test_leak_token_table_matches_publish_guard() -> None: def test_leak_tables_load_empty_without_the_publish_guard(tmp_path: Path) -> None: - # On the OSS mirror scripts/publish/ is deny-listed, so the guard is ABSENT. The token tables must - # then load EMPTY (never a stale/fragmented copy), so no customer/vendor token ships in the tee. - # Exercise the loader against a tree that has no scripts/publish above it. + # Where no guard is reachable (an installed wheel with no scripts/ above it) the token tables + # must load EMPTY -- never a stale or fragmented copy -- so no customer/vendor token ships in + # the tee. Exercise the loader against a tree that has no scripts/security above it. assert tee_leak._load_publish_guard(tmp_path / "no-guard-here" / "leak.py") is None # type: ignore[attr-defined] def test_leak_tables_are_sourced_from_the_guard_when_present() -> None: - # In the private source tree the guard IS present -> the tables are populated *from it* (not hard- - # coded here). Skipped on the mirror, where the guard is absent and the tables are legitimately empty. - if tee_leak._load_publish_guard() is None: # type: ignore[attr-defined] - pytest.skip("publish guard is private-only (absent on the OSS mirror)") + """The tee's tables are populated FROM the guard, never hard-coded in the published file. + + The gate here used to be "is the guard present", because the guard lived under the deny-listed + scripts/publish/ AND carried its tokens as literals -- so present implied populated. Neither half + holds now: the guard is committed at scripts/security/, and its tokens are EXTERNALIZED to a + secret / git-ignored file. Guard-present therefore no longer implies tables-populated, and this + test failed in CI on exactly that (present guard, no token source, empty tables) while passing + locally only because the developer's checkout has the token file. + + Gate on the TOKEN SOURCE instead -- the condition that actually determines whether there is + anything to source. The assertion still bites: blanking the bridge in tee/anon/leak.py reds it. + """ + guard = tee_leak._load_publish_guard() # type: ignore[attr-defined] + if guard is None: + pytest.skip("guard absent (e.g. an installed wheel with no scripts/ above it)") + if not getattr(guard, "TOKENS_PRESENT", False): + pytest.skip( + "no token source configured (fork CI / fresh clone) — tables legitimately empty" + ) assert tee_leak.FORBIDDEN and tee_leak.ESTATE_TOKENS # type: ignore[attr-defined] diff --git a/tests/test_feature_map_claims.py b/tests/test_feature_map_claims.py index a9561a4..d226748 100644 --- a/tests/test_feature_map_claims.py +++ b/tests/test_feature_map_claims.py @@ -2,7 +2,7 @@ # Copyright (C) 2026 MessageFoundry Organization and contributors """Guards on `docs/FEATURE-MAP.md`, the **public** capability catalog. -FEATURE-MAP.md is not in `scripts/publish/publish-denylist.txt`, so it reaches the public +FEATURE-MAP.md is not git-ignored as a private-only path, so it reaches the public mirror. In July 2026 it carried a hardcoded ASVS score (`214 / 0 / 0 / 131`) copied from `docs/security/ASVS-L3-ASSESSMENT.md` — a document that had since been banner-marked "⛔ SUPERSEDED … the scoring in this document is not reliable". Three defects at once: a diff --git a/tests/test_load_config.py b/tests/test_load_config.py index 9fd6fda..9e6d047 100644 --- a/tests/test_load_config.py +++ b/tests/test_load_config.py @@ -156,24 +156,25 @@ def test_invalid_transform_mode_fails_loud(monkeypatch: pytest.MonkeyPatch) -> N # Real estate tokens that must never appear in the shipped load graph or profiles. The site-code -# pattern catches 6-digit 54xxxx codes; the rest are partner/product/customer substrings drawn from -# the (git-ignored) real migration estate. NOTE: the generic product name "Corepoint" is deliberately -# NOT here — it's the competitor named all over the repo's own docs; the guard targets real customer/ -# partner/site identifiers, not the product name. +# pattern catches externalized 6-digit site codes; the rest are partner/product/customer substrings +# drawn from the (git-ignored) real migration estate. NOTE: the generic product name "Corepoint" is +# deliberately NOT here — it's the competitor named all over the repo's own docs; the guard targets +# real customer/partner/site identifiers, not the product name. # -# These estate tokens + the site-code pattern now live in the publish guard -# (scripts/publish/scan_forbidden.py) as the SINGLE source of truth (ADR 0030 §5), shared with the +# These estate tokens + the site-code pattern now live in the guard +# (scripts/security/scan_forbidden.py) as the SINGLE source of truth (ADR 0030 §5), shared with the # anonymizer's leak-check; this test imports them instead of keeping a divergent copy (the drift # BACKLOG #36 recorded). The scanner lives under scripts/ (not an installed package), so it is loaded # by path — mirroring tests/test_scan_forbidden.py. def _load_scan_forbidden() -> object: - path = Path(__file__).resolve().parents[1] / "scripts" / "publish" / "scan_forbidden.py" + path = Path(__file__).resolve().parents[1] / "scripts" / "security" / "scan_forbidden.py" if not path.exists(): - # Private-only: scripts/publish/ is deny-listed in the OSS mirror, so the estate-token - # assertions this module feeds don't apply there. Skip the whole module rather than error at - # collection (the scanner is the single source of truth only where it exists). + # The scanner lives under scripts/ (not an installed package); on an installed wheel it is + # absent, so the estate-token assertions this module feeds don't apply. Skip the whole module + # rather than error at collection. pytest.skip( - "scan_forbidden.py is private-only (OSS-mirror deny-list)", allow_module_level=True + "scan_forbidden.py needs the source checkout (absent on an installed wheel)", + allow_module_level=True, ) spec = importlib.util.spec_from_file_location("scan_forbidden", path) assert spec is not None and spec.loader is not None @@ -204,7 +205,7 @@ def test_no_forbidden_tokens_in_shipped_load_artifacts() -> None: if token in text: offenders.append(f"{path}: {token!r}") if _SITE_CODE.search(text): - offenders.append(f"{path}: site-code pattern 54xxxx") + offenders.append(f"{path}: site-code pattern") assert not offenders, "real estate tokens leaked into shipped load artifacts: " + "; ".join( offenders ) diff --git a/tests/test_scan_forbidden.py b/tests/test_scan_forbidden.py new file mode 100644 index 0000000..090ea6a --- /dev/null +++ b/tests/test_scan_forbidden.py @@ -0,0 +1,198 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The forbidden-content scanner (scripts/security/scan_forbidden.py). + +Covers the detection mechanisms — customer/partner/vendor names (word-boundary, with a case-sensitive +bare code), the routable-IP check with its private/documentation allow-set, and the field-scan site-code +detector — that the pre-commit hook and the CI leak-gate (.github/workflows/security.yml) run. + +The scanner's real customer/vendor token list is *externalized* (a git-ignored local file for pre-commit +and the ``MEFOR_FORBIDDEN_TOKENS`` Actions secret for CI), so it loads EMPTY on a fork/no-secret checkout +and this module carries **no real token**. Every test here injects **synthetic** placeholder patterns via +the ``sf`` fixture (below) instead of relying on the real list — so the detection mechanism is exercised +identically with or without the secret, and this file is itself safe to scan (it is no longer skipped). + +The scanner lives under scripts/ (not an installed package), so it is loaded by path.""" + +from __future__ import annotations + +import importlib.util +import re +from pathlib import Path + +import pytest + +_REPO_ROOT = Path(__file__).resolve().parents[1] +_SCANNER_PATH = _REPO_ROOT / "scripts" / "security" / "scan_forbidden.py" + + +def _load_scanner(): + spec = importlib.util.spec_from_file_location("scan_forbidden", _SCANNER_PATH) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +# --- synthetic token set (never a real customer/vendor token) ---------------------------------------- +# Mirrors the SHAPE of the real externalized list so the same code paths are exercised: a word-boundary +# case-insensitive name, a partner/vendor name, a case-SENSITIVE bare code (an uppercase code is flagged, +# a lowercase same-spelling schema/identifier is intentionally NOT), and an any-case adopter-repo token. +_SYNTH_FORBIDDEN = [ + (re.compile(r"\bACME\b", re.I), "customer name (ACME)"), + (re.compile(r"\bWIDGETCO\b", re.I), "partner name (WIDGETCO)"), + (re.compile(r"\bXMPL\b"), "customer (Example Medical Center / XMPL)"), + (re.compile(r"\bXMPLREPO\b", re.I), "customer adopter repo (XMPLREPO)"), +] +_SYNTH_ESTATE = ("acmecorp", "widgetco", "examplevendor") + +# A non-real site-code prefix (99xxxx) stands in for the real one, injected into both the field-scan +# detector and the in-memory SITE_CODE_RE so no real site code appears in this now-scanned file. +_SYNTH_SITE_CODE_RE = re.compile(r"99\d{4}") +_SYNTH_SITE_CODE_FILE = re.compile(r"(? str: + # A routable IP is any address OUTSIDE the scanner's allow-set (loopback/RFC1918/RFC5737/multicast). + # The probe IP is a globally shared public resolver -- never a customer host -- and is assembled + # from octets at runtime so this comment carries no literal dotted-quad of its own. + # at runtime so no literal routable dotted-quad ever sits in this (now-scanned) source file. The + # documentation ranges (192.0.2.x etc.) are ALLOWED and so cannot exercise the "flag routable" path. + return ".".join(("8", "8", "8", "8")) + + +# --- forbidden-content detection --------------------------------------------------------------------- + + +def test_scan_file_flags_customer_name(sf, tmp_path: Path) -> None: + p = tmp_path / "doc.md" + p.write_text("This note mentions ACME here.\n", encoding="utf-8") + hits = sf.scan_file(p) + assert len(hits) == 1 + assert "customer name (ACME)" in hits[0] + + +def test_scan_file_flags_case_sensitive_code(sf, tmp_path: Path) -> None: + # Uppercase "XMPL" (the customer code) and "XMPLREPO" (adopter repo) are caught; a lowercase "xmpl" + # schema/identifier is intentionally NOT flagged (case-sensitive bare code — avoids identifier churn). + p = tmp_path / "doc.md" + p.write_text( + "Deploy notes for XMPL, from the XMPLREPO repo.\n" + "The xmpl.Provider_GetNpi proc is a fine lowercase schema.\n", + encoding="utf-8", + ) + hits = sf.scan_file(p) + reasons = " ".join(hits) + assert "Example Medical Center / XMPL" in reasons # uppercase code caught + assert "XMPLREPO" in reasons # adopter repo caught + assert not any("xmpl.Provider_GetNpi" in h for h in hits) # lowercase schema not flagged + + +def test_scan_file_flags_routable_ip_address(sf, tmp_path: Path) -> None: + ip = _routable_ip() + p = tmp_path / "hosts.txt" + p.write_text(f"forward to {ip} on the wire\n", encoding="utf-8") + # show_context=True: scan_file is REASON-ONLY by default, so a hit report is safe to print in a + # public CI log. This test is specifically about naming the offending VALUE, so it opts in — which + # also keeps the show_context path covered. Without it the assertion cannot pass. + hits = sf.scan_file(p, show_context=True) + assert any(f"routable IP address ({ip})" in h for h in hits) + + +def test_scan_file_allows_private_and_doc_ips(sf, tmp_path: Path) -> None: + # Loopback / RFC1918 / TEST-NET (RFC5737) addresses never identify a real host — must not trip. These + # literals are in the allow-set, so they are also safe against the real scanner reading this file. + p = tmp_path / "net.txt" + p.write_text("127.0.0.1 10.0.0.4 192.168.1.9 172.16.5.5 192.0.2.10\n", encoding="utf-8") + assert sf.scan_file(p) == [] + + +def test_scan_file_flags_site_code(sf, tmp_path: Path) -> None: + # A synthetic 99xxxx site code left in source/docs — the shape of the gap that let a real site-coded + # connection name reach the mirror. The ``_``-delimited code is caught even though a bare ``\b`` + # boundary would miss it. + p = tmp_path / "wiring.py" + p.write_text("re-routes deeper (e.g. ``PT_990210_ADT_2``)\n", encoding="utf-8") + hits = sf.scan_file(p, show_context=True) # reason-only by default; see the IP test above + assert any("site code (990210)" in h for h in hits) + + +def test_scan_file_site_code_ignores_embedded_digit_runs(sf, tmp_path: Path) -> None: + # A 99xxxx run embedded in a longer alphanumeric/decimal token is NOT a site code: a hex hash, a + # digit-prefixed run, a password, a decimal fraction, and a dotted version must all pass (the + # boundary excludes alphanumerics AND ``.``), or the check would false-positive across the whole tree. + p = tmp_path / "noise.txt" + p.write_text( + "hash aff07c990210ff\n" # hex-embedded + "dob 20990210\n" # digit-prefixed + "pw 9876990210\n" # digit-embedded + "ratio 75.990210\n" # decimal fraction + "ver 1.990210.2\n", # dotted version + encoding="utf-8", + ) + assert sf.scan_file(p) == [] + + +def test_scan_file_site_code_skips_noisy_file_types(sf, tmp_path: Path) -> None: + # Lock / SVG / password-list files are dense with incidental standalone 6-digit runs — skipped by file + # type so a benign coordinate or hash line does not fail closed (a standalone 990210 WOULD otherwise + # match; the skip, not a boundary, is what saves these). + for name in ("requirements.lock", "art.svg", "common_passwords.txt"): + p = tmp_path / name + p.write_text("standalone 990210 here\n", encoding="utf-8") + assert sf.scan_file(p) == [], name + + +def test_scan_file_skips_binary(sf, tmp_path: Path) -> None: + # NUL in the head => treated as binary, not scanned. The ACME token (which the fixture WOULD flag as + # text) and the address are ignored. The address is a TEST-NET (allowed) literal, so this source line + # is clean even under the real scanner. + p = tmp_path / "blob.bin" + p.write_bytes(b"\x00\x01\x02 ACME 203.0.113.9") + assert sf.scan_file(p) == [] + + +def test_scan_file_clean_text_has_no_hits(sf, tmp_path: Path) -> None: + p = tmp_path / "ok.md" + p.write_text("A generic engine doc about HL7 routing. Corepoint is fine in prose.\n", "utf-8") + assert sf.scan_file(p) == [] # bare competitor name is intentionally allowed + + +# --- reasons-only in-memory scan (the importable scan_text contract) --------------------------------- + + +def test_scan_text_returns_reasons_without_matched_context(sf) -> None: + # scan_text is the importable single source of truth the anonymizer leak-check reuses; it returns + # REASONS ONLY (never the matched text), so a caller may raise/log the result without leaking content. + reasons = sf.scan_text(f"mentions ACME and {_routable_ip()}") + assert "customer name (ACME)" in reasons + assert "routable IP address" in reasons + assert not any("ACME and" in r for r in reasons) # the matched line is never echoed + + +# --- scan coverage invariant (lockstep with the SKIP_PATHS narrowing) -------------------------------- + + +def test_scanner_no_longer_skips_its_own_token_bearing_tests(sf) -> None: + # These two tests were rewritten to SYNTHETIC tokens so they can be scanned on the public tree — they + # are no longer in SKIP_PATHS. If a real token were ever added here it MUST be caught, not silently + # shipped. This pins that invariant (lockstep with the SKIP_PATHS narrowing in scan_forbidden.py). + 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 diff --git a/tests/test_scan_tokens_source.py b/tests/test_scan_tokens_source.py new file mode 100644 index 0000000..baef6a7 --- /dev/null +++ b/tests/test_scan_tokens_source.py @@ -0,0 +1,812 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Self-tests for the externalized forbidden-content token source. + +These assert the two things the cutover depends on for scripts/security/scan_forbidden.py: + + 1. the COMMITTED source carries ZERO real tokens -- with no external source it degrades to + structural-only (empty name/estate/site tables) yet still flags a routable IP; and scanning the + committed scanner + its .example finds no routable IP of its own; and + + 2. the committed scan-tokens.local.txt.example is SYNTHETIC -- it parses to exactly the ACME/EXAMPLE + placeholders and the non-real ``99`` site prefix, so any real token slipping into it fails here. + +The scanner is loaded by path (scripts/ is not a package). Every routable test IP is assembled from +octets at runtime so THIS test file contains no literal routable dotted-quad of its own to trip the +gate that scans it. +""" + +from __future__ import annotations + +import importlib.util +import itertools +from pathlib import Path +from types import ModuleType + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] +_SCANNER = _ROOT / "scripts" / "security" / "scan_forbidden.py" +_EXAMPLE = _ROOT / "scripts" / "security" / "scan-tokens.local.txt.example" + +# A routable probe IP, built from parts so no literal dotted-quad appears in this tracked file -- +# including in this comment, which is scanned like any other line. +_ROUTABLE_IP = ".".join(["8", "8", "8", "8"]) +# Non-routable examples (RFC1918 + RFC5737) that must NOT be flagged. +_ALLOWED_IPS = (".".join(["10", "0", "0", "5"]), ".".join(["192", "0", "2", "9"])) + +_counter = itertools.count() + + +def _load(source: str | Path | None, monkeypatch: pytest.MonkeyPatch) -> ModuleType: + """Load a FRESH scanner instance and force a deterministic token source. + + ``source=None`` -> structural-only (env cleared, local file pointed at a nonexistent path). + ``source=`` -> that token file via MEFOR_FORBIDDEN_TOKENS. + """ + spec = importlib.util.spec_from_file_location(f"sf_{next(_counter)}", _SCANNER) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) # runs reload_tokens() once against the ambient env + if source is None: + monkeypatch.delenv("MEFOR_FORBIDDEN_TOKENS", raising=False) + mod.LOCAL_TOKEN_FILE = _ROOT / "does-not-exist-scan-tokens.local.txt" # type: ignore[attr-defined] + else: + monkeypatch.setenv("MEFOR_FORBIDDEN_TOKENS", str(source)) + mod.reload_tokens() # type: ignore[attr-defined] + return mod + + +def test_committed_files_exist() -> None: + assert _SCANNER.is_file(), "relocated scanner missing at scripts/security/" + assert _EXAMPLE.is_file(), "synthetic token template missing" + + +def test_structural_only_has_no_baked_tokens(monkeypatch: pytest.MonkeyPatch) -> None: + mod = _load(None, monkeypatch) + assert mod.TOKENS_PRESENT is False # type: ignore[attr-defined] + # No customer/vendor tokens are compiled into the committed source. + assert mod.FORBIDDEN == [] # type: ignore[attr-defined] + assert mod.ESTATE_TOKENS == () # type: ignore[attr-defined] + # The site-code detector is OFF with no prefix loaded (would-be code does not match). + assert mod.SITE_CODE_RE.search("990123") is None # type: ignore[attr-defined] + + +def test_structural_only_still_flags_routable_ip( + monkeypatch: pytest.MonkeyPatch, +) -> None: + mod = _load(None, monkeypatch) + assert "routable IP address" in mod.scan_text(f"host {_ROUTABLE_IP}") # type: ignore[attr-defined] + # Private / documentation IPs are not flagged. + for ip in _ALLOWED_IPS: + assert mod.scan_text(f"host {ip}") == [] # type: ignore[attr-defined] + + +def test_structural_only_scan_file_flags_routable_ip( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + mod = _load(None, monkeypatch) + f = tmp_path / "cfg.txt" + f.write_text(f"server = {_ROUTABLE_IP}\nprivate = {_ALLOWED_IPS[0]}\n", encoding="utf-8") + hits = mod.scan_file(f) # type: ignore[attr-defined] + assert any("routable IP address" in h for h in hits) + # Reasons-only by default: the matched value is NOT echoed. + assert all(_ROUTABLE_IP not in h for h in hits) + # ...but --show-context (local only) does include it. + ctx = mod.scan_file(f, show_context=True) # type: ignore[attr-defined] + assert any(_ROUTABLE_IP in h for h in ctx) + + +def test_committed_files_contain_no_routable_ip( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # Scan the committed scanner + its .example with the structural detector: any routable IP baked + # into them would surface here. (Their only IPs are RFC5737/RFC1918 allow-listed prefixes.) + mod = _load(None, monkeypatch) + for path in (_SCANNER, _EXAMPLE): + assert mod.scan_file(path) == [], f"{path.name} carries structural forbidden content" # type: ignore[attr-defined] + + +def test_example_is_synthetic(monkeypatch: pytest.MonkeyPatch) -> None: + mod = _load(_EXAMPLE, monkeypatch) + assert mod.TOKENS_PRESENT is True # type: ignore[attr-defined] + # Estate tokens are exactly the synthetic placeholders -- a real token here fails this equality. + assert set(mod.ESTATE_TOKENS) == {"acme", "exampleco", "examplevendor"} # type: ignore[attr-defined] + assert mod.FORBIDDEN, "example [names] section should compile at least one pattern" # type: ignore[attr-defined] + # The site prefix is the NON-REAL 99xxxx. + assert mod.SITE_CODE_RE.search("990123") is not None # type: ignore[attr-defined] + + +def test_example_tokens_match_as_specified(monkeypatch: pytest.MonkeyPatch) -> None: + mod = _load(_EXAMPLE, monkeypatch) + # Case-insensitive name. + assert mod.scan_text("welcome to ACME today") # type: ignore[attr-defined] + assert mod.scan_text("acme corp") # type: ignore[attr-defined] + # Case-sensitive adopter repo: matches uppercase, not lowercase. + assert mod.scan_text("repo ACMECORP") # type: ignore[attr-defined] + assert mod.scan_text("repo acmecorp") == [] # type: ignore[attr-defined] + # Estate substring inside a field-like body. + assert any("estate token" in r for r in mod.scan_text("PID|exampleco|x", include_estate=True)) # type: ignore[attr-defined] + # Boundary-aware site-code file detector. + assert mod._SITE_CODE_FILE.search("PT_990123_ADT") is not None # type: ignore[attr-defined] + assert mod._SITE_CODE_FILE.search("ab990123cd") is None # type: ignore[attr-defined] + + +def test_estate_token_butted_against_word_characters_is_file_scanned( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The hole that let a real customer org name sit in a tracked test and pass every gate. + + ``[names]`` patterns are word-boundary anchored and ``_`` is a word character, so a token inside + ``OB__ORU`` is invisible to them — and the estate set used to be BODY-only, so nothing else + looked either. The scanner reported the repo clean on a real leak. + """ + mod = _load(_EXAMPLE, monkeypatch) + p = tmp_path / "test_lanes.py" + p.write_text('lanes = {"OB_ACME_ORU"}\n', encoding="utf-8") + hits = mod.scan_file(p, "tests/test_lanes.py") # type: ignore[attr-defined] + assert hits, "the identifier form must be caught" + # Reason-only by default: this gate fails into a world-readable Actions log on the public repo. + assert not any("ACME" in h for h in hits) + + # NB: WHICH detector catches this changed. The token is in BOTH [names] and [estate], so the + # single-token identifier pass now fires the NAME detector directly and the estate file scan + # (gated on "nothing else flagged this line") is correctly suppressed. The estate path is covered + # on its own by test_estate_only_token_still_caught_by_the_file_scan, which builds a source where + # a token is in [estate] and NOT in [names] -- this fixture contains no such token. + + +def test_estate_token_inside_a_longer_word_is_not_flagged( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Substring matching's false-positive class — a token that is merely a run of letters inside an + unrelated identifier (real case: the WebAuthn exception name ``InvalidCBORData``).""" + mod = _load(_EXAMPLE, monkeypatch) + p = tmp_path / "webauthn.py" + p.write_text('raise InvalidACMEData("bad attestation")\n', encoding="utf-8") + assert mod.scan_file(p, "messagefoundry/webauthn.py") == [] # type: ignore[attr-defined] + + +def test_body_only_estate_tokens_are_excluded_from_the_file_scan( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """``[estate_body_only]`` keeps dictionary-ish tokens out of the FILE scan while leaving them in + the body scan, where a fail-closed false positive is the safer error.""" + mod = _load(_EXAMPLE, monkeypatch) + body_only = next(iter(mod._ESTATE_BODY_ONLY)) # type: ignore[attr-defined] + p = tmp_path / "doc.md" + p.write_text(f'row = db_lookup("{body_only}", "SELECT 1")\n', encoding="utf-8") + assert mod.scan_file(p, "docs/doc.md") == [] # type: ignore[attr-defined] + assert mod.scan_text(f"a {body_only} value", include_estate=True) # type: ignore[attr-defined] + + +def test_site_code_pattern_written_out_is_flagged( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The PREFIX is the secret, so writing the PATTERN leaks it as surely as writing a code. + + ADR 0030's de-identification pass certified a file clean by grepping one form while three + occurrences of the other survived — one on the line adjacent to a line it had just fixed. + """ + mod = _load(_EXAMPLE, monkeypatch) + for body in (f"a bare `99{chr(92)}d{{4}}` substring", "the `99xxxx` pattern"): + p = tmp_path / "NEW.md" + p.write_text(body + "\n", encoding="utf-8") + hits = mod.scan_file(p, "docs/NEW.md") # type: ignore[attr-defined] + assert any("site-code pattern" in h for h in hits), body + # ...but an incidental digit run that merely starts with the prefix is not a disclosure. + p = tmp_path / "d.md" + p.write_text("dob = 19990123\nratio = 75.995512\n", encoding="utf-8") + assert mod.scan_file(p, "docs/d.md") == [] # type: ignore[attr-defined] + + +@pytest.mark.parametrize( + "mangled", + [ + pytest.param("[na", id="truncated-paste"), + pytest.param("# only comments\n# no sections\n", id="comments-only"), + pytest.param("acme\nexampleco\n", id="section-headers-lost"), + ], +) +def test_present_but_unusable_token_source_fails_closed( + mangled: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """TOKENS_PRESENT must mean USABLE, not merely non-None. + + Derived from ``text is not None`` alone, a mangled secret yielded ZERO detectors while still + reporting "tokens present" — so MEFOR_REQUIRE_TOKENS=1 passed and the gate ran structural-only + behind a green required check. Runbook step C.2 has the owner paste a whole sectioned file into a + GitHub secret box, which is precisely the mangling case. + """ + monkeypatch.setenv("MEFOR_FORBIDDEN_TOKENS", mangled) + spec = importlib.util.spec_from_file_location(f"sf_{next(_counter)}", _SCANNER) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + mod.reload_tokens() # type: ignore[attr-defined] + assert mod.TOKENS_PRESENT is False # type: ignore[attr-defined] + assert sum(mod.loaded_token_counts().values()) == 0 # type: ignore[attr-defined] + + +def test_inline_env_token_content(monkeypatch: pytest.MonkeyPatch) -> None: + # MEFOR_FORBIDDEN_TOKENS may carry the content inline (CI secret form), not only a path. + inline = "[names]\n\\bwidgetco\\b | vendor (WIDGET) | i\n[site_prefix]\n88\n" + monkeypatch.setenv("MEFOR_FORBIDDEN_TOKENS", inline) + spec = importlib.util.spec_from_file_location(f"sf_{next(_counter)}", _SCANNER) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + mod.reload_tokens() # type: ignore[attr-defined] + assert mod.TOKENS_PRESENT is True # type: ignore[attr-defined] + assert mod.scan_text("a widgetco b") # type: ignore[attr-defined] + assert mod.SITE_CODE_RE.search("880001") is not None # type: ignore[attr-defined] + + +def test_require_tokens_fails_closed(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + # With MEFOR_REQUIRE_TOKENS=1 and no source, the CLI refuses (exit 2) rather than under-scanning. + spec = importlib.util.spec_from_file_location(f"sf_{next(_counter)}", _SCANNER) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + mod.LOCAL_TOKEN_FILE = tmp_path / "absent.txt" # type: ignore[attr-defined] + monkeypatch.delenv("MEFOR_FORBIDDEN_TOKENS", raising=False) + monkeypatch.setenv("MEFOR_REQUIRE_TOKENS", "1") + assert mod.main([str(tmp_path)]) == 2 # type: ignore[attr-defined] + + +# -------------------------------------------------------------------------------------------------- +# Partial-load floor. TOKENS_PRESENT is an OR across sections, so a source that lost SOME of its +# tokens satisfied "fail-closed" while loading as few as 1 of 21 detectors -- printing no +# structural-only warning and passing with a green tick. Every token below is SYNTHETIC. +# -------------------------------------------------------------------------------------------------- + +_SYNTH_NAMES = ( + "[names]\n" + + r"ACME\s+Health | customer organisation name | i" + + "\nEXAMPLECORP | partner name | i\n" +) +_SYNTH_ESTATE = "[estate]\nacmelab\nexamplenet\n" +_SYNTH_PREFIX = "[site_prefix]\n99\n" +# names=2 + estate=2 + site_prefixes=1 == 5 floor detectors +_SYNTH_FULL = f"{_SYNTH_NAMES}\n{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" +_SYNTH_TOTAL = 5 + + +def _write(tmp_path: Path, content: str) -> Path: + p = tmp_path / "tokens.txt" + p.write_text(content, encoding="utf-8") + return p + + +def _tree(tmp_path: Path) -> Path: + """A scannable directory holding one innocuous file. + + It must NOT be empty: the scanner separately refuses a ``--path`` that examined zero files, which + would make every "this passes" assertion below succeed for entirely the wrong reason -- the same + can't-fail trap the floor itself exists to close. + """ + d = tmp_path / "tree" + d.mkdir(exist_ok=True) + (d / "note.md").write_text("nothing to see here\n", encoding="utf-8") + return d + + +@pytest.mark.parametrize( + ("label", "content"), + [ + # The likeliest paste error is losing the TAIL -- and [site_prefix] is the LAST section, so + # dropping one line silently disables every site-code detector. + ("lost the trailing site_prefix", f"{_SYNTH_NAMES}\n{_SYNTH_ESTATE}"), + ("only site_prefix survived", _SYNTH_PREFIX), + ("only names survived", _SYNTH_NAMES), + ("only estate survived", _SYNTH_ESTATE), + # A typo'd header drops that section into the section=None void with no diagnostic at all. + ( + "names header typo'd", + f"[naems]\nACME | x | i\n\n{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}", + ), + ], +) +def test_partially_loaded_source_fails_closed( + label: str, content: str, monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A source that loads only SOME sections must refuse, not pass. + + Before the floor these all exited 0: TOKENS_PRESENT was satisfied by any single surviving + section, so the run reported 'fail-closed' while most detectors were off. + """ + scan_dir = _tree(tmp_path) + mod = _load(_write(tmp_path, content), monkeypatch) + assert mod.TOKENS_PRESENT is True, f"{label}: precondition -- a source WAS found" # type: ignore[attr-defined] + monkeypatch.setenv("MEFOR_REQUIRE_TOKENS", "1") + assert mod.main(["--path", str(scan_dir)]) == 2, f"{label}: partial load must fail closed" # type: ignore[attr-defined] + + +def test_fully_loaded_source_passes(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """Control for the parametrised case above: a COMPLETE source must still pass. + + Without this, a floor that rejected everything would look identical to a working one. + """ + scan_dir = _tree(tmp_path) + mod = _load(_write(tmp_path, _SYNTH_FULL), monkeypatch) + monkeypatch.setenv("MEFOR_REQUIRE_TOKENS", "1") + # Clear any ambient floor: inheriting one from the developer's shell would turn this control RED + # for a reason unrelated to what it asserts. + monkeypatch.delenv("MEFOR_MIN_DETECTORS", raising=False) + assert mod.main(["--path", str(scan_dir)]) == 0 # type: ignore[attr-defined] + + +def test_min_detectors_catches_loss_within_a_section( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The section check cannot see 2 names becoming 1; the numeric floor can.""" + scan_dir = _tree(tmp_path) + monkeypatch.setenv("MEFOR_REQUIRE_TOKENS", "1") + + mod = _load(_write(tmp_path, _SYNTH_FULL), monkeypatch) + monkeypatch.setenv("MEFOR_MIN_DETECTORS", str(_SYNTH_TOTAL)) + assert mod.main(["--path", str(scan_dir)]) == 0, "the floor must admit an intact source" # type: ignore[attr-defined] + + # One name lost: every section is still non-empty, so ONLY the total can catch it. + thinned = ( + "[names]\n" + + r"ACME\s+Health | customer organisation name | i" + + f"\n\n{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" + ) + mod2 = _load(_write(tmp_path, thinned), monkeypatch) + assert all(mod2.loaded_token_counts()[s] for s in ("names", "estate", "site_prefixes")) # type: ignore[attr-defined] + assert mod2.main(["--path", str(scan_dir)]) == 2, "below-floor total must fail closed" # type: ignore[attr-defined] + + +def test_floor_permits_growth(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """It is a FLOOR, not an equality -- adding a token must not break CI.""" + scan_dir = _tree(tmp_path) + grown = f"{_SYNTH_NAMES}THIRDCO | another partner | i\n\n{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" + mod = _load(_write(tmp_path, grown), monkeypatch) + monkeypatch.setenv("MEFOR_REQUIRE_TOKENS", "1") + monkeypatch.setenv("MEFOR_MIN_DETECTORS", str(_SYNTH_TOTAL)) + counts = mod.loaded_token_counts() # type: ignore[attr-defined] + total = counts["names"] + counts["estate"] + counts["site_prefixes"] + # Without this precondition the test cannot tell a FLOOR from an EQUALITY: if the "grown" source + # happened to total exactly the configured number it would pass under either semantics. + assert total > _SYNTH_TOTAL, "precondition: the source really did grow past the floor" + assert mod.main(["--path", str(scan_dir)]) == 0 # type: ignore[attr-defined] + # ...and the floor still bites once raised above the grown total, so "passes" is not unconditional. + monkeypatch.setenv("MEFOR_MIN_DETECTORS", str(total + 1)) + assert mod.main(["--path", str(scan_dir)]) == 2 # type: ignore[attr-defined] + + +def test_site_prefix_count_is_a_count_not_a_presence_bit( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """loaded_token_counts reported 0-or-1, so losing one of two prefixes was invisible. + + Any floor built on that number would have inherited the blindness. + """ + two = f"{_SYNTH_NAMES}\n{_SYNTH_ESTATE}\n[site_prefix]\n99\n88\n" + mod = _load(_write(tmp_path, two), monkeypatch) + assert mod.loaded_token_counts()["site_prefixes"] == 2 # type: ignore[attr-defined] + + +def test_require_tokens_cli_flag_fails_closed_without_env( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """pre-commit can pass ARGS but cannot set env for a hook, so the flag is the local gate.""" + scan_dir = _tree(tmp_path) + monkeypatch.delenv("MEFOR_REQUIRE_TOKENS", raising=False) + monkeypatch.delenv("MEFOR_MIN_DETECTORS", raising=False) + + mod = _load(_write(tmp_path, _SYNTH_PREFIX), monkeypatch) + assert mod.main(["--require-tokens", "--path", str(scan_dir)]) == 2 # type: ignore[attr-defined] + + mod2 = _load(_write(tmp_path, _SYNTH_FULL), monkeypatch) + assert mod2.main(["--require-tokens", "--path", str(scan_dir)]) == 0 # type: ignore[attr-defined] + assert mod2.main([f"--require-tokens={_SYNTH_TOTAL + 1}", "--path", str(scan_dir)]) == 2 # type: ignore[attr-defined] + + +def test_min_detectors_implies_require(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + """A floor set WITHOUT the require flag must not be silently inert.""" + scan_dir = _tree(tmp_path) + monkeypatch.delenv("MEFOR_REQUIRE_TOKENS", raising=False) + mod = _load(_write(tmp_path, _SYNTH_PREFIX), monkeypatch) + monkeypatch.setenv("MEFOR_MIN_DETECTORS", str(_SYNTH_TOTAL)) + assert mod.main(["--path", str(scan_dir)]) == 2 # type: ignore[attr-defined] + # Exit 2 arrives from several branches (usage error, zero files examined, absent source), so pin + # the REASON as well -- otherwise this passes for the wrong cause and stops being evidence. + why = mod.token_floor_failure(_SYNTH_TOTAL) # type: ignore[attr-defined] + assert why is not None and "EMPTY" in why, why + + +# -------------------------------------------------------------------------------------------------- +# Corruption-in-place. The count floor above defeats LINE LOSS, but it counts entries that PARSE, not +# detectors that can FIRE -- so a token corrupted in place kept the counts identical while detection +# silently flipped off. Measured before the fix: one zero-width space inside a token left TOTAL +# unchanged and stopped the token matching. Zero-width chars are not str.isspace() and survive +# str.strip() (unlike NBSP, which strips to empty), which is exactly what a paste through a rendering +# surface produces at the runbook's C.2 step. +# -------------------------------------------------------------------------------------------------- + +_ZWSP = "\u200b" + + +def test_zero_width_char_inside_a_token_is_rejected( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The entry must be DROPPED, so the floor can see the loss it previously could not.""" + honest = _load(_write(tmp_path, _SYNTH_FULL), monkeypatch) + baseline = honest.loaded_token_counts()["estate"] # type: ignore[attr-defined] + + corrupt = _SYNTH_FULL.replace("acmelab", f"acme{_ZWSP}lab") + mod = _load(_write(tmp_path, corrupt), monkeypatch) + assert mod.loaded_token_counts()["estate"] == baseline - 1 # type: ignore[attr-defined] + # ...and with the floor set to the honest total the run now refuses. + assert mod.token_floor_failure(_SYNTH_TOTAL) is not None # type: ignore[attr-defined] + + +def test_clean_token_containing_a_space_is_still_accepted( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """Control: the invisible-char check must not reject ordinary printable content. + + A plain space IS printable; without this, a rule that rejected everything would look identical + to a working one. + """ + src = f"{_SYNTH_NAMES}\n[estate]\nacme lab\nexamplenet\n\n{_SYNTH_PREFIX}" + mod = _load(_write(tmp_path, src), monkeypatch) + assert mod.loaded_token_counts()["estate"] == 2 # type: ignore[attr-defined] + assert mod.token_floor_failure(_SYNTH_TOTAL) is None # type: ignore[attr-defined] + + +def test_never_matching_pattern_cannot_pad_the_count( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """``(?!)`` is the module's own detector-off sentinel; accepted as a token it would count but + never fire, letting filler reach the floor while real detection stayed degraded.""" + thinned_padded = ( + "[names]\n" + + r"ACME\s+Health | customer organisation name | i" + + "\n(?!) | pad | i\n(?!) | pad | i\n\n" + + f"{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" + ) + mod = _load(_write(tmp_path, thinned_padded), monkeypatch) + assert mod.loaded_token_counts()["names"] == 1, "padding must not inflate the count" # type: ignore[attr-defined] + assert mod.token_floor_failure(_SYNTH_TOTAL) is not None # type: ignore[attr-defined] + + +def test_non_ascii_digit_site_prefix_is_rejected( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """str.isdigit() is True for non-ASCII digits, which would compile in and never match a site code.""" + src = f"{_SYNTH_NAMES}\n{_SYNTH_ESTATE}\n[site_prefix]\n\u0669\u0669\n" + mod = _load(_write(tmp_path, src), monkeypatch) + assert mod.loaded_token_counts()["site_prefixes"] == 0 # type: ignore[attr-defined] + assert mod.token_floor_failure() is not None, "an empty floor section must refuse" # type: ignore[attr-defined] + + +def test_duplicate_entries_do_not_inflate_the_count( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A double-pasted section would otherwise read as twice the detectors and mask real loss.""" + doubled = f"{_SYNTH_NAMES}\n{_SYNTH_ESTATE}\n{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" + mod = _load(_write(tmp_path, doubled), monkeypatch) + assert mod.loaded_token_counts()["estate"] == 2, "duplicates must not count twice" # type: ignore[attr-defined] + + +def test_unknown_flag_is_refused_not_treated_as_a_filename( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """An unrecognised flag used to land at rest[0], defeating the --path test and scanning nothing.""" + scan_dir = _tree(tmp_path) + mod = _load(_write(tmp_path, _SYNTH_FULL), monkeypatch) + monkeypatch.setenv("MEFOR_REQUIRE_TOKENS", "1") + assert mod.main(["--reqire-tokens", "--path", str(scan_dir)]) == 2 # type: ignore[attr-defined] + + +def test_unrecognised_require_value_refuses( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """An exact '=="1"' compare meant MEFOR_REQUIRE_TOKENS=true silently disabled the gate.""" + scan_dir = _tree(tmp_path) + mod = _load(_write(tmp_path, _SYNTH_PREFIX), monkeypatch) + monkeypatch.setenv("MEFOR_REQUIRE_TOKENS", "yes") + assert mod.main(["--path", str(scan_dir)]) == 2, "'yes' must engage the gate, not bypass it" # type: ignore[attr-defined] + monkeypatch.setenv("MEFOR_REQUIRE_TOKENS", "maybe") + assert mod.main(["--path", str(scan_dir)]) == 2, "an unrecognised value must refuse" # type: ignore[attr-defined] + + +def test_parse_warnings_never_echo_token_content( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """These warnings land in a world-readable Actions log on the PUBLIC repo.""" + secret = "SUPERSECRETCUSTOMERNAME" + src = f"[names]\n{secret}( | bad regex | i\n\n{_SYNTH_ESTATE}\n[site_prefix]\n{secret}\n" + _load(_write(tmp_path, src), monkeypatch) + err = capsys.readouterr().err + assert "ignoring an uncompilable" in err, "precondition: the warning fired" + assert secret not in err, "the warning must NOT echo the token" + + +# -------------------------------------------------------------------------------------------------- +# Internal-environment disclosure detectors. These are SHAPE-based, not token-based, so no token list +# can catch them and they must stay live even in structural-only / fork runs. Both were present in the +# retired scanner and absent from the relocated one -- a silent loss of coverage at cutover (10 real +# hit locations across 5 files on the tree at the time). +# -------------------------------------------------------------------------------------------------- + +_BS = chr(92) # a literal backslash, kept out of string literals to avoid escape confusion + + +def test_worktree_slug_is_flagged_without_any_token_source( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A branch/worktree slug names the TASK, which can name a prospect, engagement or competitor. + + Unbounded by nature, so there is no list to add it to -- shape is the only control that scales, + and it must work with the token tables empty. + """ + mod = _load(None, monkeypatch) + assert mod.TOKENS_PRESENT is False, "precondition: structural-only" # type: ignore[attr-defined] + f = tmp_path / "notes.md" + # Split so no single SOURCE line here is itself a full match: this file is scanned by the gate it + # tests, and a literal probe makes the suite trip its own detector. + f.write_text("see .claude/work" + "trees/some-task-name-a1b2c3 for details\n", encoding="utf-8") + hits = mod.scan_file(f, "docs/notes.md") # type: ignore[attr-defined] + assert any("worktree/branch slug" in h for h in hits) + # Reason-only: the slug IS the disclosure, so it must not be echoed into a public CI log. + assert not any("some-task-name" in h for h in hits) + + +def test_absolute_home_path_is_flagged_but_placeholders_are_not( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """An absolute home path carries the OS account name; documented stand-ins must not fire.""" + mod = _load(None, monkeypatch) + real = tmp_path / "real.md" + # The POSIX arm is split for the same reason as the slug probe above -- the Windows arm already + # avoids a literal via _BS. + real.write_text( + f"C:{_BS}Users{_BS}Alice{_BS}Code{_BS}thing\n" + "/ho" + "me/bob/src\n", encoding="utf-8" + ) + hits = mod.scan_file(real, "docs/real.md") # type: ignore[attr-defined] + assert sum("absolute user-home path" in h for h in hits) == 2 + assert not any("Alice" in h or "bob" in h for h in hits), "must not echo the account name" + + ok = tmp_path / "ok.md" + ok.write_text( + f"C:{_BS}Users{_BS}{_BS}Code\n/home/runner/work/repo\n/Users/user/project\n" + f"/home/me/notes\nC:{_BS}Users{_BS}Public{_BS}Shared\n", + encoding="utf-8", + ) + assert not any( + "absolute user-home path" in h + for h in mod.scan_file(ok, "docs/ok.md") # type: ignore[attr-defined] + ), "placeholders / CI / shared accounts must not fire" + + +# -------------------------------------------------------------------------------------------------- +# Round-3 hardening: parser diagnostics, allowlist breadth, and per-section floors. +# -------------------------------------------------------------------------------------------------- + + +def test_unknown_section_header_warns_instead_of_silently_dropping( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A typo'd header dropped a whole section with NO diagnostic at all.""" + src = f"[naems]\nwidgetco | n | i\n\n{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" + mod = _load(_write(tmp_path, src), monkeypatch) + assert mod.loaded_token_counts()["names"] == 0 # type: ignore[attr-defined] + assert "unknown section header" in capsys.readouterr().err + + +def test_bom_before_first_header_no_longer_voids_the_section( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A UTF-8 BOM defeated the startswith('[') test, silently voiding that section.""" + mod = _load(_write(tmp_path, chr(0xFEFF) + _SYNTH_FULL), monkeypatch) + assert mod.loaded_token_counts()["names"] == 2, "BOM must be stripped, not swallow the section" # type: ignore[attr-defined] + + +def test_spaced_alternation_is_refused_not_silently_truncated( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The delimiter is a space-pipe-space, so `a | b` inside a pattern truncated it to `a`. + + The truncated pattern still compiles, so the re.error handler never fired and the count was + unchanged -- a silent narrowing. Ambiguity is now refused. + """ + src = f"[names]\nfoo | bar | baz | i\n\n{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" + mod = _load(_write(tmp_path, src), monkeypatch) + assert mod.loaded_token_counts()["names"] == 0 # type: ignore[attr-defined] + err = capsys.readouterr().err + assert "expected at most 3" in err + assert "foo" not in err and "bar" not in err, "must not echo the entry" + + +def test_overbroad_allowlist_entry_is_rejected(tmp_path: Path) -> None: + """One degenerate allowlist line vetoes every line BEFORE any detector runs, disabling the whole + gate -- while the loaded-counts diagnostic still reports full tables, so the log looks healthy.""" + import importlib.util + import shutil + + src = _ROOT / "scripts" / "security" + dst = tmp_path / "security" + shutil.copytree(src, dst) + for degenerate in (".", ".*", "^", "[0-9]*"): + (dst / "scan-allowlist.txt").write_text(degenerate + "\n", encoding="utf-8") + spec = importlib.util.spec_from_file_location( + f"al_{next(_counter)}", dst / "scan_forbidden.py" + ) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + assert mod.ALLOWLIST == [], f"{degenerate!r} must be rejected" # type: ignore[attr-defined] + + +def test_shipped_allowlist_survives_its_own_validator() -> None: + """Control: the committed entries must still LOAD. + + Without this, a validator that rejected everything would look identical to a working one. + """ + import importlib.util + + spec = importlib.util.spec_from_file_location(f"al_{next(_counter)}", _SCANNER) + assert spec is not None and spec.loader is not None + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + committed = [ + ln.strip() + for ln in (_ROOT / "scripts" / "security" / "scan-allowlist.txt") + .read_text(encoding="utf-8") + .splitlines() + if ln.strip() and not ln.strip().startswith("#") + ] + assert len(mod.ALLOWLIST) == len(committed) # type: ignore[attr-defined] + + +def test_per_section_floor_catches_what_a_total_cannot( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """A bare total is a SUM: growth in one section masks collapse in another.""" + masked = ( + "[names]\nwidgetco | n | i\n\n[estate]\nacmelab\nzorpnet\nthirdnet\n\n[site_prefix]\n99\n" + ) + mod = _load(_write(tmp_path, masked), monkeypatch) + c = mod.loaded_token_counts() # type: ignore[attr-defined] + total = c["names"] + c["estate"] + c["site_prefixes"] + assert total == 5, "precondition: the total is unchanged" + assert mod.token_floor_failure(5) is None, "a total floor cannot see this" # type: ignore[attr-defined] + assert mod.token_floor_failure({"names": 2, "estate": 2, "site_prefixes": 1}) is not None # type: ignore[attr-defined] + + +def test_min_spec_parsing_rejects_nonsense(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + mod = _load(_write(tmp_path, _SYNTH_FULL), monkeypatch) + assert mod.parse_min_spec("21") == 21 # type: ignore[attr-defined] + assert mod.parse_min_spec("names=7,estate=13") == {"names": 7, "estate": 13} # type: ignore[attr-defined] + for bad in ("names", "bogus=3", "names=x", ""): + with pytest.raises(ValueError): + mod.parse_min_spec(bad) # type: ignore[attr-defined] + + +def test_bad_case_field_keeps_the_detector_and_widens_it( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """An invalid CASE flag must NOT drop the entry. + + Dropping loses a detector, and under-detection is the dangerous direction. Case-insensitive is + strictly broader than case-sensitive, so falling back can only over-match -- never under-match. + """ + src = f"[names]\nWIDGETCO | n | X\n\n{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" + mod = _load(_write(tmp_path, src), monkeypatch) + assert mod.loaded_token_counts()["names"] == 1, "the detector must survive a bad CASE flag" # type: ignore[attr-defined] + err = capsys.readouterr().err + assert "defaulting to case-INSENSITIVE" in err + assert "WIDGETCO" not in err, "must not echo the entry" + # And it really is case-insensitive now. + assert mod.scan_text("we use widgetco here") # type: ignore[attr-defined] + + +# -------------------------------------------------------------------------------------------------- +# Identifier pass. `_` is a WORD character, so a \b-anchored name pattern cannot see its token inside +# `OB_TOKEN_ORU`. A second pass over the line with `_` neutralised closes that for every name pattern +# rather than only for tokens someone remembered to duplicate into [estate]. +# -------------------------------------------------------------------------------------------------- + +_B = chr(92) + "b" # a literal \b, built so no shell/heredoc can collapse it to a backspace + + +def test_single_token_pattern_sees_the_identifier_form( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The G19 shape: a \b-anchored pattern whose token has NO [estate] counterpart.""" + src = ( + "[names]\n" + _B + "widgetco" + _B + " | partner name | i\n\n" + "[estate]\nsomethingelse\n\n[site_prefix]\n99\n" + ) + mod = _load(_write(tmp_path, src), monkeypatch) + assert mod.loaded_token_counts()["names"] == 1, "precondition: the pattern compiled" # type: ignore[attr-defined] + + f = tmp_path / "feed.py" + for content, expected in ( + ("we work with widgetco daily\n", True), + ('lanes = {"OB_WIDGETCO_ORU"}\n', True), + ("conn = ib_widgetco_adt\n", True), + ("widgetcofactory is a different word\n", False), + ("nothing to see here\n", False), + ): + f.write_text(content, encoding="utf-8") + hits = mod.scan_file(f, "tests/feed.py") # type: ignore[attr-defined] + assert bool(hits) is expected, content + assert not any("idgetco" in h for h in hits), "reason-only: must not echo the token" + + +def test_multi_word_pattern_does_not_match_snake_case( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The false positive that made the first version of this unusable. + + Neutralising `_` for a PHRASE pattern makes it match ordinary snake_case. Measured on the real + tree, a two-word pattern hit 9 occurrences of a generic Python identifier across two files -- + every one a false positive, and enough to block the cutover PR on its own required gate. Only + single-token patterns get the second pass. + """ + src = ( + "[names]\n" + _B + "action" + chr(92) + "s+list" + _B + " | source-system artifact | i\n\n" + f"{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" + ) + mod = _load(_write(tmp_path, src), monkeypatch) + assert mod.loaded_token_counts()["names"] == 1 # type: ignore[attr-defined] + + f = tmp_path / "mod.py" + f.write_text("for i, action_list in enumerate(lists):\n", encoding="utf-8") + assert mod.scan_file(f, "messagefoundry/mod.py") == [], ( # type: ignore[attr-defined] + "an ordinary snake_case identifier must NOT trip a multi-word pattern" + ) + # ...but the real phrase in prose still does. Split for the same reason as the probes above: the + # phrase is itself a live detector, so a literal here would trip the gate on this very file. + f.write_text("exported from the " + "action" + " list\n", encoding="utf-8") + assert mod.scan_file(f, "docs/x.md") # type: ignore[attr-defined] + + +def test_reason_that_names_its_own_token_is_neutralised( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A REASON is printed verbatim on every hit, into a world-readable Actions log. + + A reason naming its own token therefore publishes the token on the first match -- the same + defect as echoing it in a parse warning, but on the success path. The DETECTOR must survive: + dropping the entry would trade a disclosure for under-detection, which is worse. + """ + src = f"[names]\nwidgetco | the widgetco partner | i\n\n{_SYNTH_ESTATE}\n{_SYNTH_PREFIX}" + mod = _load(_write(tmp_path, src), monkeypatch) + assert mod.loaded_token_counts()["names"] == 1, "the detector must survive" # type: ignore[attr-defined] + assert "would echo the token" in capsys.readouterr().err + + f = tmp_path / "x.md" + f.write_text("we use widgetco here\n", encoding="utf-8") + hits = mod.scan_file(f, "docs/x.md") # type: ignore[attr-defined] + assert hits, "still detected" + assert not any("widgetco" in h for h in hits), "the hit must not carry the token" + + +def test_estate_only_token_still_caught_by_the_file_scan( + monkeypatch: pytest.MonkeyPatch, tmp_path: Path +) -> None: + """The estate file-scan path must keep working independently of the identifier pass. + + For a token in BOTH [names] and [estate] the name detector now fires first and suppresses the + estate scan, so that path needs its own coverage: a token in [estate] and NOT in [names]. + Without this, the original test's coverage would have silently migrated to a different mechanism. + """ + src = f"{_SYNTH_NAMES}\n[estate]\nzorpnet\n\n{_SYNTH_PREFIX}" + mod = _load(_write(tmp_path, src), monkeypatch) + assert not any(pat.search("zorpnet") for pat, _ in mod.FORBIDDEN), ( # type: ignore[attr-defined] + "precondition: the token is estate-only" + ) + assert "zorpnet" in [tok for tok, _ in mod._ESTATE_FILE_RES], ( # type: ignore[attr-defined] + "precondition: it is file-scanned, not body-only" + ) + f = tmp_path / "lanes.py" + f.write_text('lane = "OB_ZORPNET_ORU"\n', encoding="utf-8") + hits = mod.scan_file(f, "tests/lanes.py") # type: ignore[attr-defined] + assert any("estate token" in h for h in hits) + assert not any("zorpnet" in h.lower() for h in hits), "reason-only" diff --git a/tests/test_security_static.py b/tests/test_security_static.py index f338cf8..d491717 100644 --- a/tests/test_security_static.py +++ b/tests/test_security_static.py @@ -100,7 +100,7 @@ # exclusion is a fact about the tree rather than a filter. # # ``scripts/`` is walked by neither the ReDoS nor the single-JSON/URL-parser clause: it is -# build/release tooling not reachable from untrusted input, and ``scripts/publish/check_release_sync.py`` +# build/release tooling not reachable from untrusted input, and the retired release-sync checker # carries a release-version parser (``(\d+(?:\.\d+)*)(.*)$``) that matches the nested-quantifier shape # while backtracking linearly. The scanner has no suppression mechanism, so walking it there would red # the build on a measured false positive instead of finding a real defect. @@ -509,6 +509,14 @@ def _py_files(*roots: Path) -> list[Path]: r"""'(?i)\\b(' + '|'.join(_CREDENTIAL_QUERY_KEYS) + ')=[^&\\s\\"\']+'""", ), "messagefoundry/parsing/_builtin_hl7.py": ("f'{e}\\\\.({prefixes})(?!{e})'",), # ASVS 1.3.3 + # ADR 0030 §4h: the site-code prefix is no longer a literal in the anonymizer — it is EXTERNALIZED + # and loaded at runtime, so the detector is composed from the loaded value rather than written out. + # That is the POINT of §4h (the prefix is customer data and must not sit in tracked source), and it + # necessarily makes the expression unresolvable to this static scanner. The shape is bounded and + # non-catastrophic by inspection: an alternation of digit literals followed by a fixed ``\d{4}`` — + # no nested quantifier, no overlapping alternation. + "messagefoundry/anon/surrogates.py": ("f'(?:{alt})\\\\d{{4}}'",), + "tee/anon/surrogates.py": ("f'(?:{alt})\\\\d{{4}}'",), # a wrapper's parameter. NOTE: register_ui_action's own re.compile(pattern) stays here BY # CONSTRUCTION — its argument is the function's parameter — but every one of its 25 call sites is # now resolved through _PATTERN_WRAPPERS, so no console route pattern is unscanned. consistency.py