diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 578ee64e..9ac07304 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -2872,7 +2872,7 @@ This is **wider than the disclosure describes.** [`CONFIGURATION.md:718`](CONFIG ## 329. Five `MEFOR_ALLOW_INSECURE_TLS` cells bypass the ADR 0092 clamp -> πŸ”’ **Filed 2026-08-01 β€” not started.** Value **6/10** Β· Difficulty **4/10** Β· _quick win_. The LDAPS bind (`ssl.CERT_NONE` on the authentication substrate for every AD identity), the SFTP host key, the webhook sink and the `[ai].api_key` still cross an enforcing production-PHI posture on one env var, and converting them is what collapses five per-site facts into one repo-wide invariant the ASVS scorecard's regex mechanism can actually express β€” bounded because setting the variable needs Administrator, who can already do worse; the cheap in-gate half shipped with #323, so what remains is threading an explicit posture into `AuthService`/`create_app`'s three out-of-gate constructors, where `_here()` would otherwise ship green and inert. +> βœ… **SHIPPED 2026-08-06 β€” the four out-of-gate insecure-TLS cells now route through the ADR-0092 clamp.** Value **6/10** Β· Difficulty **4/10** Β· _quick win_. LDAPS (`auth/ldap.py`), the SFTP host key (`transports/remotefile.py`), the webhook sink (`pipeline/alert_sinks.py`) and the AI-broker (`transports/ai_broker.py`) now gate the `MEFOR_ALLOW_INSECURE_TLS` escape through `weakened_tls_escape_permitted[_here]` β€” the instance posture threaded into `AuthService` / `create_app`'s out-of-gate constructors β€” so on an enforcing production-PHI instance the escape is inert and an unverified/cleartext hop stays refused. The fifth cell the heading names (Direct SMTP) was already clamped in #323, so this converted the remaining four. > ⚠️ **AMENDED 2026-08-03 β€” the census is FOUR, not five: #323 landed and took the Direct SMTP cell.** The heading, the evidence table (*"Confirmed at HEAD"*) and Proposed Β§1 all still name `transports/direct.py:170` as an unclamped cell, but that file now holds **no call to the raw predicate at all** β€” it imports only `weakened_tls_escape_permitted_here` (`messagefoundry/transports/direct.py:63`) and gates both arms on it (`:197` cleartext SMTP, `:215` `tls_verify=false`), with the #323 rationale β€” including its own warning that this absence is scoped to that file and never repo-wide β€” at `:182-196`; `:170` is now unrelated cert-loading. The Scope note called this in future tense and the 2026-08-03 banner already enumerates only four cells while still calling them *"five per-site facts"*, so read the table as **at least four** sites still reading the unclamped `insecure_tls_allowed()`: the SFTP host key (`messagefoundry/transports/remotefile.py:375`, feeding `AutoAddPolicy`/`RejectPolicy` at `:392-394`), LDAPS (`messagefoundry/auth/ldap.py:113`), the webhook alert sink (`messagefoundry/pipeline/alert_sinks.py:291` β€” the item cites `:290`) and the AI broker (`messagefoundry/transports/ai_broker.py:140`). > @@ -2927,7 +2927,7 @@ That distinction matters concretely for the ASVS record. The scorecard's absence ## 331. Anonymizer's fail-closed leak-check has no structural PHI detectors -> πŸ”’ **Filed 2026-08-01 β€” not started.** Value **6/10** Β· Difficulty **4/10** Β· _quick win_. The function that earns the right to share a de-identified dataset verifies a known-string denylist β€” `leak_check` is `scan_text` (FORBIDDEN patterns, one routable-IPv4 check, estate substrings; `scripts/security/scan_forbidden.py:772-795`) plus a field-anchored site code, and a real MRN is not a denylisted string β€” and on a token-less checkout it degrades to the IPv4 check alone over an HL7 body and still returns clean, a gap `f3c6d348` hit in practice with a hand overlay that was never committed; wiring `token_floor_failure()` into the bridge is small, but the unmapped-field report and detectors scoped to fields no rule matched cross the `anonymize` seam and must be mirrored into `tee/anon/leak.py` for `test_anon_parity`. +> βœ… **SHIPPED 2026-08-06 β€” structural PHI-shape detectors + unmapped-field coverage report + token-floor signal built.** Value **6/10** Β· Difficulty **4/10** Β· _quick win_. `leak_check`/`leak_report` now run high-precision structural detectors (dashed SSN, punctuated NANP phone, CX `MR`/`MRN`-typed identifier) over **the fields no rule matched**, record every present-but-unmapped field in a coverage report (`LeakReport.unmapped_fields`, carried into the `LeakError` on a refusal and exposed via the `on_report` hook), and record `token_floor_failure()` in every report, folding it into the fail-closed decision under the `require_live_denylist` **opt-in** lever β€” default off, so a token-less CI/OSS/fork load still passes with the structural detectors as the live backstop; a deployment that must refuse on an unloaded denylist sets the lever. The whole structural block is mirrored byte-identical into `tee/anon/leak.py` with a new engine/tee `leak_report` parity test; each detector was falsified. ADR 0030 Β§5/Β§7/Consequences amended (the "deferred" phrasing was stale). The aggressive/broad-shape tier (bare-digit DOB/SSN, name-like runs) stays deferred by owner call β€” it mass-false-positives on HL7 bodies dense with dates/order-numbers. **Cluster:** Security & Compliance. **Priority:** P2. **Verdict:** build. **Severity:** medium. @@ -3321,7 +3321,7 @@ What is NOT settled is the mechanism. Two independent passes reached different a ## 346. The sandbox import boundary is enforced only at runtime, under an off-by-default flag -> 🚧 **Status OPEN (filed 2026-08-02).** Value **4/10** Β· Difficulty **3/10** Β· _fill-in_. A type the sandbox child must **construct or receive** cannot live under a prefix on `DEFAULT_FORBIDDEN_MODULES` ([pipeline/sandbox.py](../messagefoundry/pipeline/sandbox.py)) β€” the child's import guard raises and the dispatch fails. That rule is real, it has already been violated once in shipped code, and **nothing enforces it**. `CapturedResponse` lived in `messagefoundry.store`; the child could not import it, which made `mode=subprocess` + ADR 0013 loopback re-ingress **DOA** until #339 relocated it to [config/response.py](../messagefoundry/config/response.py). The only guard runs **in the child, at dispatch time, and only when `[sandbox].mode=subprocess`** β€” which is not the default, so a re-violation is invisible to a green suite. +> βœ… **SHIPPED 2026-08-06 β€” a static `ast` import-boundary guard now pins it.** Value **4/10** Β· Difficulty **3/10** Β· _fill-in_. [tests/test_sandbox_import_boundary.py](../tests/test_sandbox_import_boundary.py) walks the `ast` import nodes of `_sandbox_codec.py` and `_sandbox_worker.py` and asserts none resolves under a `DEFAULT_FORBIDDEN_MODULES` prefix (imported from the runtime constant, never copied), with a committed positive control that each static import form the walker handles is seen and a negative control that benign `messagefoundry.*` imports are not flagged. Both files are clean today; the guard would red on first deployment if a future edit reintroduced a forbidden import, instead of failing silently only under `[sandbox].mode=subprocess`. **Cluster:** Correctness / test coverage. **Priority:** P2. **Verdict:** build (small). **Severity:** medium (blast radius: a feature is DOA for everyone who opted in), medium (likelihood: the codec's constructor set is precisely the surface that grows as the payload model does). @@ -3935,13 +3935,21 @@ record and the cell's current score live in the vault scorecard and are not rest ## 1006. A mutation that matches is not a mutation that bites: the absence-claim gate proves syntax, never behaviour -> πŸ”’ **Filed 2026-08-04 β€” not started. Scored 2026-08-04 β†’ P2.** Value **6/10** Β· Difficulty -> **3/10** Β· _quick win_. `check_absences` admits an ASVS absence claim on `re.search(a.pattern, -> a.mutation)` (`scripts/asvs/scorecard.py:395`) β€” one string field of a TOML row matched against -> another β€” so a well-formed reintroduction that would change nothing if applied passes all three -> of the gate's failure modes and certifies a non-control into the compliance record; the -> remainder is a required per-claim observable plus a mode that applies the mutation and requires -> that observable to go red, in one stdlib script and its fixture tests. +> βœ… **SHIPPED 2026-08-06 β€” a new opt-in mode can prove an absence claim BITES, which the pattern +> check structurally cannot.** Value **6/10** Β· Difficulty **3/10** Β· _quick win_. +> `scripts/asvs/scorecard.py` gains a `--prove-absences` mode: per claim it applies the `mutation` to +> a scratch copy of the tree and requires a named `observable` (a pytest node id) to go RED, failing +> closed on any exit code that is not an honest test failure (an already-red baseline, an +> uncollectable node, or a mutation that only breaks import is a PROVE-ERROR, never a proof). So a +> well-formed reintroduction that would change nothing if applied CAN be caught the moment its claim +> carries an `observable` β€” but the default `verify` path is byte-unchanged and no authored claim +> carries one yet, so nothing new is blocked by this alone today. Two optional `Absence` fields +> (`mutation_path`, `observable`) feed it, a coarse same-file static backstop screens claims that +> carry no observable, the scratch copy refuses secrets / the store / `docs/security` (defence for the +> eventual vault run), and fixture negative controls plus a CLI exit-code test prove the mode itself +> can go red. Public repo script + fixtures only; wiring the mode over the vault's ~81 existing +> absence claims (untouched) and backfilling their observables is the owner's follow-up +> (`scorecard.py:14-16`, ADR 0156 Β§7). **Cluster:** Security & Compliance. **Priority:** P2. **Verdict:** build. **Severity:** medium β€” the defect is in the instrument, not the engine, and a green instrument that cannot go red is the @@ -4727,7 +4735,7 @@ Retiring the tree costs the engine nothing operationally: **`tests/test_ech_egre ## 1014. connscale smoke test's fixed 24-port block is not parallel-safe across worktrees; the flaky marker hides the collision -> πŸ”’ **Filed 2026-08-04 β€” not started.** Value **5/10** Β· Difficulty **3/10** Β· _fill-in_. `test_connscale_smoke_end_to_end` hard-codes `base_port = 41000` and requires 24 **contiguous** inbound ports, so two checkouts running the suite at once contend for the same block. A `@pytest.mark.flaky` marker retries past the collision, so a determinate resource conflict wears a noise label. +> βœ… **SHIPPED 2026-08-06 β€” dynamic contiguous inbound-port allocation replaces the fixed 24-port block; the flaky marker is dropped.** Value **5/10** Β· Difficulty **3/10** Β· _fill-in_. `test_connscale_smoke_end_to_end` now reserves a random contiguous inbound-port block at runtime (`_free_contiguous_ports`), asserts contiguity at acquisition, and fails loudly if no free block is found, so a genuine cross-worktree collision surfaces as a red rather than a masked retry. **Cluster:** Testing / CI reliability. **Priority:** P3. **Verdict:** build (small). **Severity:** low β€” it costs retries and misdiagnosis, not correctness. @@ -5693,10 +5701,14 @@ The literal spellings are all caught, including the newline form. Only the indir ## 1060. `alloc.ps1` records the owning worktree from the current directory, so an absolute-path invocation misattributes it -> πŸ”’ **Filed 2026-08-05 β€” not started.** Value **5/10** Β· Difficulty **2/10** Β· _quick win_. `scripts/coord/alloc.ps1:51` takes the owner from `git rev-parse --show-toplevel`, which resolves against the **current directory** rather than the script's location. Invoke it by absolute `-File` path from a different worktree β€” which is how a session with several worktrees naturally calls it β€” and the allocation is recorded to the caller's worktree while the commit comes from another. The ledger gate then refuses that commit correctly, but far away from the cause and with a message about the wrong thing. +> βœ… **SHIPPED 2026-08-06 β€” every `git` call in both allocators is anchored to the script's own checkout, and the defect was larger than filed.** Value **5/10** Β· Difficulty **2/10** Β· _quick win_. `$repo` now comes from `git -C $PSScriptRoot rev-parse --path-format=absolute --show-toplevel`, and every subsequent call takes `-C $repo`. **`git -C`, not `Split-Path`:** the recorded `worktree` value is *compared* β€” by `ledger_check.py:227` and by `-List` β€” so its string form is part of a contract, and `--path-format=absolute` keeps writing the forward-slash form every claim already on disk carries. **The filing named one of four cwd-derived reads.** Also wrong, and measured on the pre-fix code: the `$branch` recorded with the claim was the *caller's* branch; the floor's boundary was parsed from the **caller's** `scripts/hooks/ledger_check.py`; and the floor's working-tree term β€” the one whose job is to catch a number written but committed **nowhere** β€” read the **caller's** `docs/BACKLOG.md`. That last one is not friction: a number drafted in the target worktree was invisible to the sweep and free to re-issue, which is the collision this script exists to prevent. **`scripts/coord/claim.ps1:54` carried the same construct and was never filed** β€” found by inspection while fixing this, fixed in the same commit; its enforcing hook `claim_check.py` reads the repo from cwd and is *right* to, because a commit hook's cwd **is** the committing worktree. Hook right, tool wrong, and only the tool can be invoked from elsewhere. The item's cheap second half β€” printing the recorded worktree at allocation time β€” was **already built** (`claimed by:`); what was missing is a note when the shell is standing somewhere else, which is now printed. Tested by the **divergence** with `-ShowFloor`, so no numbers were burned: two temp checkouts drafting different numbers, the allocator invoked by absolute path from the other one. The negative control was run, not assumed β€” reverted, the same test reports `floor : 7777`, `boundary : 1900` and a watermark under `Caller/.git/`, all three signals pointing at the wrong tree. Original filing follows. `scripts/coord/alloc.ps1:51` took the owner from `git rev-parse --show-toplevel`, which resolves against the **current directory** rather than the script's location. Invoke it by absolute `-File` path from a different worktree β€” which is how a session with several worktrees naturally calls it β€” and the allocation is recorded to the caller's worktree while the commit comes from another. The ledger gate then refuses that commit correctly, but far away from the cause and with a message about the wrong thing. **Cluster:** Session coordination / ledger integrity. **Priority:** P3. **Verdict:** build (small). **Severity:** no data loss and no security effect β€” the ledger gate **fails closed**, which is why this is a friction defect and not a correctness one. Nothing invalid lands; a valid commit is refused. +**SEVERITY CORRECTION, 2026-08-06, made while fixing it.** The paragraph above is right about the *recorded owner* and wrong about the item as a whole, because the filing looked at one line and the defect was in four. The ledger gate does fail closed on a misattributed claim β€” but the floor's **working-tree term** was cwd-derived too, and that term has no gate behind it. Its job is to see a number written to `docs/BACKLOG.md` and committed **nowhere**; reading the caller's tree makes a number drafted in the *target* worktree invisible, so the allocator hands it out as free and two items end up sharing it in the same tree. Both are then owned by that worktree, so `owns()` passes and the ledger gate never fires. That is the exact silent collision the script's own docstring says it exists to prevent, arrived at through the script. The window is narrow β€” anything committed on any ref is still caught by the all-refs term β€” but it is a correctness hole, not friction, and it was reproduced in the negative control rather than reasoned about. + +**AND THE FIX CONVERTED TWO SANDBOXED TESTS INTO WRITERS ON THE LIVE REGISTRY, which is the part worth remembering.** `tests/test_coord_claim_{refresh,liveness}.py` ran the **real** `scripts/coord/claim.ps1` with `cwd` set to a temp repo β€” scoped to a throwaway registry *purely by ambient cwd*, and one of them said so in a docstring: *"it scopes itself to the cwd's repo"*. The moment the script stopped consulting cwd, the passing half of the run wrote real claims into this clone's shared registry (two strays, removed by hand). So a cwd-dependence that looks like a defect in the tool can be load-bearing **isolation** in its tests, and removing it is a change to both. `tests/test_ledger_check.py` already staged a copy of `alloc.ps1` inside its fixture, which is exactly why it was the one file that did not break β€” the pattern existed and the two claim files were the outliers. Both now stage and commit the script into the fixture, so the sandbox is structural rather than ambient. Anyone fixing the remaining instances of this class (#1057, #1059) should check what their tests are isolated *by* before changing what the code reads. + **Reproduced 2026-08-05, twice, by accident.** A session ran `pwsh -NoProfile -File /scripts/coord/alloc.ps1 -Kind backlog` from worktree A while intending to commit from worktree B. `alloc/backlog/1058.json` recorded `"worktree": "<...>/trusting-wu-c2e6d5"`. The commit from `MessageFoundry-gate-deferrals` was then refused: *"BACKLOG item #1058 was not allocated to this worktree"* β€” true, unhelpful, and pointing at the allocator rather than at the invocation. Re-running with the shell actually inside the target worktree produced `1059.json` with the right owner and the commit went through. **#1058 is an abandoned hole**, which is the sanctioned outcome (`alloc.ps1`'s own docstring: *holes are free, collisions are not*). **The fix is small and there are two defensible shapes.** Either derive the repo from `$PSScriptRoot` so the allocator is anchored to the checkout it lives in β€” matching what `new.ps1` and `remove.ps1` already do β€” or keep the cwd behaviour and **say so at the point of use**, printing the recorded worktree in the `ALLOCATED` output so the mismatch is visible immediately rather than at commit time. The second is weaker but nearly free, and the two compose. Prefer anchoring: an allocator invoked by absolute path is being told which checkout to act on, and it should not then consult a different one. @@ -5769,7 +5781,7 @@ The second step's arithmetic is measured: `GetFullPath('.git', )` r ## 1063. `setup-leak-gate.ps1` picks the checkout from the current directory, so it can arm a worktree the operator did not name -> πŸ”’ **Filed 2026-08-06 β€” not started.** Value **3/10** Β· Difficulty **1/10** Β· _quick win_. `scripts/dev/setup-leak-gate.ps1:37` is `$repo = (& git rev-parse --show-toplevel 2>$null)` β€” no `-C`, no `-Repo` parameter, no `$PSScriptRoot` anchor. Invoked by absolute `-File` path from a different worktree, which is the ordinary shape on a clone with 40-plus of them, it installs the leak-gate token list into **the current directory's** checkout and prints `CONFIGURED` about that one, while the worktree the operator named keeps no token source. Its own directory siblings already do it correctly. +> βœ… **SHIPPED 2026-08-06 β€” the root is anchored to the script's own location, and the divergence is under test.** Value **3/10** Β· Difficulty **1/10** Β· _quick win_. `$repo` now comes from `Split-Path -Parent (Split-Path -Parent $PSScriptRoot)`, the form `postgres.ps1` and `sqlserver.ps1` in the same directory already use, plus an assert that the derived root actually carries `scripts/security/` β€” a wrong root should say so at the point of derivation rather than surface later as a confusing scanner failure. Tested by the **divergence**, not the happy path: two temp checkouts that both carry `scripts/security/`, the script invoked by absolute `-File` path while the shell stands in the other one, asserting the token list lands in the checkout holding the script and **not** in the caller's. The pre-fix behaviour was reproduced directly rather than inferred β€” reverted, the same test reports *"the named checkout was not armed"*. Only the three files the script reaches for are copied into the fixture, never the whole of `scripts/security/`, because a maintainer running the suite has the real token list sitting in that directory. Original filing follows. `scripts/dev/setup-leak-gate.ps1:37` was `$repo = (& git rev-parse --show-toplevel 2>$null)` β€” no `-C`, no `-Repo` parameter, no `$PSScriptRoot` anchor. Invoked by absolute `-File` path from a different worktree, which is the ordinary shape on a clone with 40-plus of them, it installs the leak-gate token list into **the current directory's** checkout and prints `CONFIGURED` about that one, while the worktree the operator named keeps no token source. Its own directory siblings already do it correctly. **Cluster:** Developer tooling / configuration anchoring. **Priority:** P4. **Verdict:** build (trivial). **Severity:** **low, and the low severity is load-bearing** β€” every failure direction here is loud or fail-closed, which is why this is filed at 3 rather than alongside its siblings. Nothing is silently ungated and no wrong authorisation is granted. @@ -5783,7 +5795,7 @@ $repo = Split-Path -Parent (Split-Path -Parent $PSScriptRoot) Anchoring on `$PSScriptRoot` binds the script to the checkout it lives in, which is what an absolute `-File` invocation is asking for. `-From` names the token **source**, not the checkout, so it does not already cover this. -**Same construct as #1060.** `alloc.ps1:51` is byte-equivalent (`git rev-parse --show-toplevel` with no anchor) and produces the same class of wrong answer β€” there, a misattributed ledger allocation; here, a token list installed into the wrong tree. Fixing them together is reasonable; filing them together was not, because their severities differ by two priority bands and folding this into #1060 would have inflated it. +**Same construct as #1060.** `alloc.ps1:51` is the same construct β€” `git rev-parse --show-toplevel` with no anchor β€” and produces the same class of wrong answer β€” there, a misattributed ledger allocation; here, a token list installed into the wrong tree. Fixing them together is reasonable; filing them together was not, because their severities differ by two priority bands and folding this into #1060 would have inflated it. (The filing said *byte-equivalent*. It is not: `alloc.ps1` carries `--path-format=absolute` and this script does not. The defect is identical; the bytes are not, and "byte-equivalent" is the kind of claim a later reader greps for and then trusts.) **How it was found, and why that matters more than the defect.** A repo-wide sweep for the cwd-as-identity shape assigned five surfaces and left `scripts/dev` and `scripts/service` in **no** surface at all β€” seven `.ps1` files in the seam. This was found only because the synthesising agent went outside its brief and swept the unassigned region. A measuring apparatus with a blind spot, hunting mechanisms with blind spots. Worth remembering when the next sweep is designed: **state the unassigned regions, or the result reads as completeness.** diff --git a/docs/CONNECTIONS.md b/docs/CONNECTIONS.md index cd4219f8..8eaca1b7 100644 --- a/docs/CONNECTIONS.md +++ b/docs/CONNECTIONS.md @@ -840,9 +840,11 @@ poll/write shape against a remote server, selected by an internal `protocol` set (`pip install 'messagefoundry[sftp]'`, lazily imported so an install that never uses SFTP skips it). **Host-key verification is ON by default** (the system host keys plus an optional extra `known_hosts`, paramiko `RejectPolicy`); an unknown key is **refused** unless `MEFOR_ALLOW_INSECURE_TLS` is set (and - loudly logged when it is). **This one cell reads the raw escape and is *not* clamped** β€” unlike the - `tls_verify` / `encrypt` cells elsewhere in this document, the variable still works here on a - production-PHI enforcing instance, so it is the SFTP setting to audit for rather than assume inert. + loudly logged when it is). **Since #329 this cell routes the escape through the clamped + `weakened_tls_escape_permitted_here()`** β€” like the `tls_verify` / `encrypt` cells elsewhere in this + document, so on a production-PHI enforcing instance the escape is inert and an unknown host key stays + refused (`RejectPolicy`) even with the variable set; it takes effect only on a non-enforcing / non-PHI + instance. - **`Ftp(...)`** β€” stdlib `ftplib`, **no extra**: `tls=False` is plain FTP, `tls=True` is **FTPS** (explicit TLS + `PROT P`, encrypting the control *and* data channels). FTPS **verifies the server certificate and hostname by default** (a verifying `SSLContext`, not ftplib's no-verify fallback). @@ -2433,7 +2435,7 @@ Legend: βœ… native Β· ~ partial / via extension / via another transport Β· ❌ n | **IBM MQ / MSMQ** | ~ | ❌ | βœ… | ❌ | not on roadmap | | **Kafka / streaming** | ~ | ❌ | βœ… | ❌ | not on roadmap | | **DICOM** (imaging) | βœ… | ~ | βœ… | βœ… | `DICOM-IN` C-STORE SCP (Phase 1) + `DICOM-OUT` C-STORE SCU/C-ECHO + `DICOMWEB-OUT` STOW-RS all shipped (ADR 0025); DICOMweb send exceeds both incumbents | -| **Serial (RS‑232)** + X/Y‑Modem/Kermit + **ASTM E1381/E1394/E1318** | ~ | ❌ | βœ… | ❌ | **declined-by-design (v0.2+)** β€” legacy/niche lab-instrument connectivity, no feed demand ([BACKLOG.md](BACKLOG.md) #27) | +| **Serial (RS‑232)** + X/Y‑Modem/Kermit + **ASTM E1381/E1394/E1318** | ~ | ❌ | βœ… | ❌ | **declined-by-design (v0.2+)** β€” legacy/niche lab-instrument connectivity, no feed demand ([BACKLOG #27](archive/backlog/BACKLOG-CLOSED.md#27-serial-rs-232--astm-e1381e1394e1318--decision-decline-unless-lab-analyzer-demand-no-build)) | | **FHIR** endpoint/client | βœ… | βœ… | βœ… | ~ | `FHIR-OUT` shipped (`FHIR()`, ADR 0022) + SMART Backend Services client auth (ADR 0024); the inbound **server facade** is deferred (BACKLOG #20) | | **Internal channel‑to‑channel** | βœ… | βœ… | βœ… | βœ… | the routing graph (wired by name) β€” plus two first-class internal inbounds: `Loopback()` (a captured reply) and `PassThrough()` (1:N internal re-ingress), ADR 0013 | | Printer / command‑line / screen‑scrape | ~ | ❌ | βœ… | ❌ | not on roadmap (niche) | diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index 13782a03..f28578b5 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -410,12 +410,12 @@ With it set, these otherwise-refused settings become permitted (each logs a loud - DATABASE destination / store: `Encrypt=false` or `TrustServerCertificate=true` (SQL Server), `[store].trust_server_certificate=true` / `[store].encrypt=false`. *(Clamped.)* - Plain-FTP credentials. *(Clamped.)* -- RemoteFile SFTP: accepting an unknown host key. *(Not clamped β€” the raw escape still applies.)* +- RemoteFile SFTP: accepting an unknown host key. *(Clamped since #329.)* - Cleartext SMTP submission on a **Direct** (S/MIME) destination. *(Not clamped; AUTH credentials over cleartext stay refused outright either way.)* - The non-connection cells that have nowhere to carry a per-hop declaration: the `[logging]` syslog/SIEM - forwarder and the API PHI-read serve hop *(both clamped)*, plus LDAPS, the webhook alert sink and the - AI-broker endpoint *(raw escape)*. + forwarder and the API PHI-read serve hop, plus LDAPS, the webhook alert sink and the + AI-broker endpoint. *(All clamped β€” LDAPS / the webhook sink / the AI broker since #329.)* **Two limits worth stating plainly.** *(a)* Since [ADR 0153](adr/0153-collapse-the-posture-gradient-no-data-label-may-allow-a-cleartext-hop.md) this variable has been **unhooked from the cleartext-hop authority** β€” that decision no longer reads it, @@ -426,9 +426,11 @@ cleartext HTTP family are now governed only by a per-connection `cleartext_accep factory parameter and no `connections.toml` key, so it is unreachable from config today. Refusal messages that suggest it are ahead of the code.) *(b)* Where it does still apply it is mostly **clamped** (ADR 0092 decision 2 / ADR 0148): it cannot relax a hop while `[security].enforcement = -enforce`, and for the MLLP/FTPS/plain-FTP and store-TLS cells the clamp additionally requires the instance -to be PHI β€” which is also the default. Either way, on the shipped posture those cells are inert; the -bullets marked *not clamped* are the exceptions that still honour the raw variable. +enforce`, and for the weakened-TLS / cleartext-escape cells that route through +`weakened_tls_escape_permitted` β€” at least the store-TLS, MLLP/FTPS and plain-FTP cells and, since #329, +LDAPS, the SFTP host key, the webhook sink and the AI broker β€” the clamp additionally requires the +instance to be PHI, which is also the default. Either way, on the shipped posture those cells are inert; +the bullets marked *not clamped* are the exceptions that still honour the raw variable. **Never set `MEFOR_ALLOW_INSECURE_TLS` in production.** Its presence is the single **environment-variable** switch that turns the remaining fail-closed verification checks into best-effort. diff --git a/docs/PHI.md b/docs/PHI.md index 245602ea..420383a6 100644 --- a/docs/PHI.md +++ b/docs/PHI.md @@ -970,7 +970,7 @@ with materially different PHI profiles, so they get their own rows; stream 4 is | **7. `connection_event` table β€” DEFAULT ON** (`[diagnostics].connection_events = true`) | transport/lifecycle events per connection: `established`, `closed` (reason `eof` or `idle_timeout` β€” no path produces any other), `idle_timeout`, `at_capacity`, `peer_not_allowlisted`, `frame_oversize`, `framing_error`, `peer_reset`, the inbound-HTTP intake-auth refusals `intake_auth_failed` / `auth_subject_denied` / `auth_rate_limited` (ADR 0154 D6 β€” peer address and mode only; **never** the credential, a prefix of it, or its length. Each of these also writes a tamper-evident audit-log row β€” the copy that survives an operator turning this diagnostics stream off), plus the runner's `connection_lost` / `connection_restored`. That is the whole vocabulary, asserted in CI against the literal emit call sites in `transports/` and the pipeline runner **and** cross-checked against the console's own filter tuple. The MLLP, raw-TCP and HTTP listeners emit these; the **DICOM inbound C-STORE SCP** and the **`ISA`/`IEA`-framed X12 inbound** emit none β€” the runner injects the sink onto **every** source (`wiring_runner.py`, over the base-class `on_connection_event` field), so both connectors *have* the wiring and simply never call it β€” so this stream covers those three listeners plus the runner's outbound-lane transitions β€” not literally every connection. An X12 feed's connects, allow-list refusals and at-capacity refusals are therefore **absent** from this stream | rows: `ts`, `connection`, `transport`, `direction`, `kind`, `peer_host`, `message_id` (correlation hint), `reason` | the store database, **all three backends** | Corepoint-style transport diagnostics β€” "did the sender connect, and why did it drop" | `GET /events` and `GET /connections/{name}/events` under **`monitoring:read`** (**not** a PHI permission) with per-channel RBAC β€” an out-of-scope `connection=` is 403'd *and* audited β€” server-clamped to ≀1000 rows | `[retention].connection_event_retention_hours` (its own **hours** window); 0 inherits `[retention].messages_days`; both 0 = keep forever. Plain age `DELETE` (metadata-only) | **`reason` is free text that can carry sensitive fragments.** Defended twice β€” `safe_exc()` at the source, `safe_text(reason)[:200]` at the store β€” then cipher-encrypted (AAD `("connection_event","reason",connection,ts,kind)`). Every other column is config metadata; the table is documented **metadata-only** β€” never a frame, body or HL7 field value. Writes are a pure side observer: a bounded in-memory queue drained by a background task outside any handoff transaction, so a flood can never block a listener or pin a message disposition | | **8. `alert_instance` table β€” default on wherever an `[alerts]` notifier exists** | resolvable operator alerts: `connection_stopped`, `queue_buildup`, `lane_stuck`, `message_stall`, `saturation`, `connection_error`, `content_match`, `storage_threshold`, `cert_expiry`, `secret_rotation`, `bootstrap_admin_expiring` (the UNCLAIMED first-run bootstrap admin nearing its auto-disable deadline β€” ASVS 6.4.5; its payload carries only the ISO deadline plus whole hours remaining, never the password or any secret), `integrity_drift`, `update_available`, `backup_failed`, `rcsi_off_degraded`, `leadership_acquired`, `dr_activated`, `gcm_invocations` (the per-key AES-GCM invocation bound crossing its 2^31 soft warn β€” ASVS 11.3.4; its payload carries a one-way `key_id` fingerprint plus counters, never key bytes) The three reachable **inverse** signals β€” `connection_restored`, `leadership_lost`, `dr_released` β€” are never rows here: `_record_state` routes an inverse through `_AUTO_RESOLVE` to `resolve_alert_instances_for`, never to `upsert_alert_instance`. (A fourth mapped key, `connection_started`, is emitted by no code path today.) | rows: `event_type`, `connection`, `severity`, `status`, `first_seen`, `last_seen`, `count`, `reason`, `acked_by`, `acked_at`, `resolved_at`, `suspended_until`, `escalation_tier` | the store database, **all three backends** | the operator alert list β€” acknowledge / resolve / suspend. Durable state is recorded **before** any suppression or throttle return, so a muted alert still leaves a record | `GET /alerts/active` under **`monitoring:diagnose`** (**not** a PHI permission) with the same per-channel scope; ack/resolve/suspend/**resume** are POSTs on the same tier, and the separate read-only `GET /alerts/rules` view sits on its own gate | shares the connection-event window; **only RESOLVED instances are DELETEd**, by `resolved_at` β€” an open or acknowledged condition is never aged out from under an operator | **`reason` is free text** taken from the event's `detail`/`reason`/`label`: `safe_text(reason)[:200]` then cipher-encrypted (AAD `("alert_instance","reason",event_type,connection)` β€” the de-dup grain, so one AAD covers both the INSERT and the re-fire UPDATE). `content_match` is **PHI-free by contract**: the sink method takes no value parameter, only the connection, an operator label and an optional rule id | | **9. `response` rows with `kind='ack_sent'` β€” DEFAULT ON** (`[diagnostics].response_sent = true`) | the ACK/NAK the engine returned to an inbound sender, under a sentinel destination `\x1fack:` | rows: `ack_code` (`AA`/`AE`/`AR`/`CA`/`CE`/`CR`), `ack_phase` (`decode`/`parse`/`strict`/`ingest`), `outcome`, `body`, `detail` | the store database | "what did we actually reply, and why" β€” the operator's answer to a sender disputing an ACK | `GET /messages/{id}/responses` under `messages:read` + `require_phi_read`; the `body` only for a caller who also holds `messages:view_raw`; every read writes a `response.read` audit row | `body`, `detail` and `resp_headers` are set to `NULL` in place by `purge_message_bodies` on the message-body window, on all three backends | **PHI fail-safe:** the ACK **body** is stored **only when the store cipher is active** β€” on a keyless store it is `NULL` rather than plaintext β€” and every NAK passes no body at all, so the offending field value is never persisted. The disposition metadata (`ack_code`/`ack_phase`/`outcome`) is non-PHI and always captured; `detail` is `safe_text`-scrubbed, 200-char bounded and encrypted | -| **10. `[alerts]` webhook transport** (off by default β€” `webhook_url` unset) | one HTTPS POST per alert, carrying every non-underscore event key as JSON | JSON | the operator's webhook endpoint (Slack/Teams/PagerDuty/custom) | operator notification | **`https` only** β€” a plaintext `http://` webhook URL is refused at construction unless the raw `MEFOR_ALLOW_INSECURE_TLS` escape is set (and then a warning is logged); note this path reads the **unclamped** escape, unlike the connectors. Redirects are refused; an optional `webhook_allowed_hosts` egress allowlist gates the host | the endpoint's | **carries the alert's `detail`/`reason` free text** (`safe_exc()`-scrubbed at the emit sites, but **not** re-run through `safe_text` on this path). Internal `_`-prefixed keys (per-rule recipients, rule id, cooldown) are stripped before send, so recipient addresses never cross the wire | +| **10. `[alerts]` webhook transport** (off by default β€” `webhook_url` unset) | one HTTPS POST per alert, carrying every non-underscore event key as JSON | JSON | the operator's webhook endpoint (Slack/Teams/PagerDuty/custom) | operator notification | **`https` only** β€” a plaintext `http://` webhook URL is refused at construction unless the `MEFOR_ALLOW_INSECURE_TLS` escape is set (and then a warning is logged); since #329 this path routes that escape through the clamped `weakened_tls_escape_permitted(posture)` (the instance posture threaded from the API lifespan), so on an enforcing-PHI instance the escape is inert and a cleartext webhook POST stays refused β€” the same clamp as the connectors, no longer the raw escape. Redirects are refused; an optional `webhook_allowed_hosts` egress allowlist gates the host | the endpoint's | **carries the alert's `detail`/`reason` free text** (`safe_exc()`-scrubbed at the emit sites, but **not** re-run through `safe_text` on this path). Internal `_`-prefixed keys (per-rule recipients, rule id, cooldown) are stripped before send, so recipient addresses never cross the wire | | **11. `[alerts]` SMTP transport β€” operator alert list** (off unless `email_smtp_host` + `email_from` + β‰₯1 `email_to`) | one email per alert; default subject `[MessageFoundry] β€” `, default body every non-underscore event key as `k: v` | plain text (always kept β€” never HTML-only); optional HTML alternative | the operators' mailboxes | operator notification | `smtp_allowed_hosts` egress allowlist; the SMTP password comes from `MEFOR_ALERTS_EMAIL_PASSWORD` or a `[secrets]` provider, never the config file; per-send timeout `email_timeout` | the mail system's | carries the same `detail`/`reason` free text as the webhook. #138 operator templates are constrained to a **closed non-PHI variable allowlist** validated fail-closed at config load. **Transport posture:** `send_plain_email` builds an explicit **verifying** context (chain + hostname + strict RFC 5280, TLS 1.2 floor) via `tls_policy.build_smtp_tls_context()` and passes it to `starttls()`, anchored to the OS roots, `[alerts].email_tls_ca_file`, or `[tls].internal_ca_file` β€” the same factory the EMAIL and DIRECT *message destinations* use, so all three SMTP cells now share one policy ([#323](BACKLOG.md), closed 2026-08-02). Before that this call passed **no** context and Python's stdlib default applied (`ssl._create_stdlib_context` **is** `ssl._create_unverified_context` β€” `CERT_NONE`, `check_hostname = False`), leaving the hop encrypted but unauthenticated. There is still **no hop gradient or attestation on this path** β€” unlike the connectors, this cell is constructed outside the `active_hop_posture` scope, so its deviations (`email_use_tls = false`, or `email_tls_verify = false`) are gated by a `[security].allow_unverified_alert_smtp_tls` **acknowledgment switch at the serve gate** rather than by the clamped escape: on an enforcing PHI instance `serve` refuses to start without it, and permits + `AUDIT`-logs the start with it. Both deviations are named by `security_loosenings()` and reported by `messagefoundry check`'s `alert-smtp-tls` advisory | | **12. Per-user security-event SMTP notifier** β€” **posture-mandatory on a PHI instance** | `account_locked`, `login_after_failures`, `password_changed`, `password_reset`, `email_changed`, `roles_changed`, `account_disabled`, `mfa_enabled`, `mfa_disabled`, `admin_action_new_ip` | plain-text email | the **affected user's own** mailbox | ASVS 6.3.5 / 6.3.7 out-of-band notification of security-relevant account changes | shares stream 11's SMTP transport and therefore its verifying context and its `[alerts].email_tls_*` knobs β€” note this is a **separate call site** (`pipeline/security_notify.py`), plumbed in its own right rather than inheriting by accident. On a PHI instance with auth enabled `serve` **refuses to start (exit 2) under `[security].enforcement = enforce`** when no effective channel exists; the explicit, **audited** opt-out is `[alerts].security_notifications_required = false` | the mail system's | the body carries the account username, a fixed description, optionally the failed-attempt count or the new email on file, and the source IP β€” **no message data, no secrets**. Dispatch is a bounded background queue; a failed send is logged, never raised (the event is still in `audit_log`) | | **13. `LoggingAlertSink` fallback** (when no `[alerts]` transport is configured) | every alert **this state-less sink implements**, at `WARNING` β€” `leadership_lost` / `dr_released` at `INFO`, and `connection_restored` is a **deliberate no-op** (a recovery needs no page and there is no instance to auto-resolve), so a lane recovery produces no record on this stream at all. `content_match` exists only on `NotifierAlertSink` and has no fallback-path record | β€” | folds into stream 1 | so alerts are never silent | inherits stream 1's | inherits stream 1's | includes the `detail`/`reason` free text, and therefore inherits stream 1's filters, ACL, forwarder and retention | diff --git a/docs/adr/0030-anonymization-test-harness-tee.md b/docs/adr/0030-anonymization-test-harness-tee.md index d1d76204..013dafee 100644 --- a/docs/adr/0030-anonymization-test-harness-tee.md +++ b/docs/adr/0030-anonymization-test-harness-tee.md @@ -15,6 +15,18 @@ 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/security/scan_forbidden.py`](../../scripts/security/scan_forbidden.py) (`FORBIDDEN`). +- **AMENDED 2026-08-06 (BACKLOG #331):** the leak-check's structural gap that Β§5 / Β§7 / Consequences + below called *scoped out / a deferred improvement* is now **built** β€” high-precision structural + PHI-shape detectors (dashed SSN, punctuated NANP phone, CX `MR`/`MRN`-typed identifier) over **the + fields no rule matched**, an unmapped-field **coverage report** (`LeakReport.unmapped_fields`; address + only, never a value, carried into the `LeakError` on a refusal and exposed via `on_report`), and the + `token_floor_failure` signal recorded in every report and folded into the fail-closed decision under + the `require_live_denylist` **opt-in** lever β€” default off, so a token-less CI/OSS/fork load still + passes with the structural detectors as the live backstop; a deployment that must refuse on an + unloaded denylist sets the lever. Mirrored behaviourally-identical into `tee/anon/leak.py`. The + *aggressive/broad* shape tier (bare-digit DOB/SSN, name-like runs) stays deferred β€” it + mass-false-positives on HL7 bodies (Β§5). This bullet is the single source of the change; the stale + "deferred" phrasings below point back here. - **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)** β€” @@ -266,8 +278,11 @@ silently misses a field is worse than none** β€” though note the limit in the ne PHI the rule map **missed** sails through the fail-closed gate *clean* unless that field happens to contain a denylisted token β€” a real MRN is not a denylisted string. **Rule-map completeness is therefore the primary control; the leak-check backstops known *strings*, not missed *fields*.** Adding structural detectors -(MRN/SSN/DOB/phone shape, NANP-reserved vs real) to the post-anon check as a *true* field-level backstop is a -candidate improvement, scoped out for this slice with the residual called out (Consequences). +(MRN/SSN/DOB/phone shape, NANP-reserved vs real) to the post-anon check as a *true* field-level backstop was a +candidate improvement β€” **built in BACKLOG #331** (AMENDED 2026-08-06; see the status banner): the +high-precision set (dashed SSN, punctuated NANP phone, CX `MR`/`MRN`-typed id) scoped to the UNMAPPED fields, +plus an unmapped-field coverage report (`LeakReport.unmapped_fields`) recorded on every pass and surfaced on +a refusal or via the `on_report` hook. The broad-shape tier stays scoped out (below). ### 6. Integration points @@ -294,7 +309,9 @@ candidate improvement, scoped out for this slice with the residual called out (C 0004](0004-payload-agnostic-ingress.md)) so X12 (`parsing/x12/`)/FHIR/raw plug in later β€” never HL7-parse a non-HL7 body. **Out:** statistical expert-determination de-id, free-text NLP scrubbing (NTE-3/OBX-5 narrative default to **blunt full-redaction**, Β§3 β€” not entity-level NLP), **per-component (bare-leaf) surrogates** (the -default encoders are field-level `^`-joined, Β§3), structural PHI detectors in the leak-check (Β§5), and any +default encoders are field-level `^`-joined, Β§3), an **aggressive/broad structural PHI search** (bare-digit +DOB/SSN, name-like alpha runs β€” the mass-false-positive tier; note the *high-precision* unmapped-field +detectors in the leak-check were **built in #331**, AMENDED 2026-08-06, Β§5), and any re-identification/linkage tooling. ### 8. Relationship to the planned de-id framework @@ -340,7 +357,9 @@ AI-assistant `deidentified` data-scope source (PHI.md Β§9 forward-links to it on not MRN/SSN/DOB/name *shapes*, so a field the rule map missed passes the "fail-closed" gate clean unless it contains a denylisted token. **Rule-map completeness is the primary control**; the leak-check is necessary, not sufficient. Free-text (OBX-5/NTE-3) is the **highest-risk residual** β€” hence its full-redact default - (Β§3). Structural detectors are a deferred improvement (Β§5/Β§7). + (Β§3). **AMENDED 2026-08-06 (#331):** high-precision structural detectors over the UNMAPPED fields plus an + unmapped-field coverage report are now **built** (that deferral is closed; the broad-shape tier stays + deferred, Β§7) β€” rule-map completeness remains the primary control. - **Vendored-copy drift β€” bytes *and* behaviour.** Two copies can diverge; the rule/surrogate/token *files* are mitigated by a CI **byte-parity** check (the existing tee discipline) under one authority. But the engine-side `anon/hl7.py` (delegating to `Message`) and the tee's **standalone stdlib re-encoder** are diff --git a/messagefoundry/anon/__init__.py b/messagefoundry/anon/__init__.py index 18ac3a94..a896602d 100644 --- a/messagefoundry/anon/__init__.py +++ b/messagefoundry/anon/__init__.py @@ -15,19 +15,22 @@ Public surface: * :func:`anonymize` β€” de-identify one HL7 message (raises nothing PHI-bearing). -* :func:`anonymize_checked` β€” :func:`anonymize` + a **fail-closed** :func:`leak_check`; raises - :class:`LeakError` (token categories only, never the value) if any known partner/site token - survives. This is how you *earn* the right to write a dataset to a shareable location. -* :func:`leak_check` β€” forbidden-token hits via the publish-guard authority (ADR 0030 Β§5). +* :func:`anonymize_checked` β€” :func:`anonymize` + a **fail-closed** :func:`leak_report`; raises + :class:`LeakError` (token categories + PHI shapes/addresses only, never a value) if any known + partner/site token survives **or** a structural PHI shape sits in a field no rule mapped. This is + how you *earn* the right to write a dataset to a shareable location. +* :func:`leak_check` / :func:`leak_report` β€” token hits + structural PHI-shape detection over the + unmapped fields + the unmapped-field coverage report (ADR 0030 Β§5, BACKLOG #331). """ from __future__ import annotations +from collections.abc import Callable from pathlib import Path from .hl7 import anonymize_message from .keying import Keyer -from .leak import LeakCheckUnavailable, leak_check +from .leak import LeakCheckUnavailable, LeakReport, coverage_clause, leak_check, leak_report from .rules import DEFAULT_RULES, AnonError, FieldRule, RuleError, SurrogateKind, load_rules __all__ = [ @@ -37,11 +40,13 @@ "Keyer", "LeakCheckUnavailable", "LeakError", + "LeakReport", "RuleError", "SurrogateKind", "anonymize", "anonymize_checked", "leak_check", + "leak_report", "load_rules", ] @@ -79,18 +84,37 @@ def anonymize_checked( salt: str, overlay: Path | None = None, rules: tuple[FieldRule, ...] | None = None, + require_live_denylist: bool = False, + on_report: Callable[[LeakReport], None] | None = None, ) -> str: - """:func:`anonymize`, then a fail-closed :func:`leak_check`; raise :class:`LeakError` on any hit. + """:func:`anonymize`, then a fail-closed :func:`leak_report`; raise :class:`LeakError` on any hit. Use this whenever the output may be persisted/shared β€” a silently-missed token is worse than no - anonymization (ADR 0030 Β§5). The raised error names token *categories* only, never the value. + anonymization (ADR 0030 Β§5). The verification is now two-layered (BACKLOG #331): the known-token + denylist **and** high-precision structural PHI-shape detectors over the fields no rule matched, + scoped by the same ``rules`` the anonymizer applied. The raised error names token *categories* and + field *shapes/addresses* only, never a value, and carries a coverage clause (the count + addresses + of the unmapped fields, whether the denylist tables were live) so a refusal is legible. + + ``require_live_denylist`` makes a non-live token source (``token_floor_reason`` set) a refusal + cause in its own right β€” the strict lever for a deployment that must not de-identify with the + customer denylist unloaded. It defaults **off**: the structural detectors are the live backstop, + and CI/OSS/fork runs legitimately have no token source. ``on_report`` receives the full + :class:`LeakReport` on both the clean and the refusing path (default: no emission). """ - output = anonymize(raw, salt=salt, overlay=overlay, rules=rules) - hits = leak_check(output) - if hits: + effective = rules if rules is not None else load_rules(overlay) + output = anonymize(raw, salt=salt, rules=effective) + report = leak_report(output, rules=effective) + if on_report is not None: + on_report(report) + causes = list(report.hits) + if require_live_denylist and report.token_floor_reason is not None: + causes.append(f"denylist not live: {report.token_floor_reason}") + if causes: raise LeakError( "anonymized output still carries forbidden token(s): " - + "; ".join(sorted(set(hits))) + + "; ".join(sorted(set(causes))) + " β€” refusing to emit (fail closed). Extend the rule map for the missed field(s)." + + coverage_clause(report) ) return output diff --git a/messagefoundry/anon/leak.py b/messagefoundry/anon/leak.py index df744318..24469f73 100644 --- a/messagefoundry/anon/leak.py +++ b/messagefoundry/anon/leak.py @@ -9,21 +9,39 @@ 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/``. -The check is the **fail-closed backstop**, not the primary control: it catches known *tokens*, not -structural PHI (a missed MRN field with no denylisted string sails through) β€” rule-map completeness -is the primary control (ADR 0030 Β§5). It is loaded lazily from the source checkout; an installed -wheel without ``scripts/`` raises a clear error (the anonymizer is a dev/migration tool, always run -from a checkout). +The token denylist alone is a **backstop that only catches known *strings***: a real MRN in a field +the rule map never mapped is not a denylisted token, so it would sail through clean. Two structural +controls close that (BACKLOG #331), scoped to the fields ``anonymize`` did **not** rewrite so an +already-pseudonymized field is never re-flagged: + +* an **unmapped-field coverage report** β€” every present-but-unmapped field is enumerated in the + :class:`LeakReport` (address only, never its value), so the check's reach is legible and a field + nobody thought to map is **recorded** (carried into the :class:`LeakError` on a refusal, and exposed + via ``on_report``) rather than passing unrecorded; and +* **high-precision structural PHI-shape detectors** over those unmapped values (dashed SSN, + punctuated NANP phone, CX ``MR``/``MRN``-typed identifier) β€” narrow by design to avoid the mass + false-positives a broad digit-run search would produce on HL7 bodies (ADR 0030 Β§5). + +The token-floor signal (``token_floor_failure``) is recorded in every report +(``token_floor_reason``/``token_tables_live``) and folded into the fail-closed decision when +``require_live_denylist`` is set (default off), so a token-less load is legible to any caller that +inspects the report or opts into refusing on it. + +Loaded lazily from the source checkout; an installed wheel without ``scripts/`` raises a clear error +(the anonymizer is a dev/migration tool, always run from a checkout). """ from __future__ import annotations import importlib.util +import re +from dataclasses import dataclass from functools import lru_cache from pathlib import Path from types import ModuleType -from .surrogates import message_has_site_code +from .rules import FieldRule +from .surrogates import Seps, message_has_site_code, read_message_seps class LeakCheckUnavailable(RuntimeError): @@ -47,16 +65,158 @@ def _scanner() -> ModuleType: ) -def leak_check(text: str) -> list[str]: - """Forbidden-token hits in ``text`` (empty list = clean), using the publish-guard's authority. +# --- structural PHI-shape detection over UNMAPPED fields (BACKLOG #331) ---------------------------- +# EVERYTHING from here to the end of this block is held BYTE-IDENTICAL with tee/anon/leak.py (the +# structural walk depends only on read_message_seps, which the parity test pins byte-for-byte). The +# detectors are deliberately high-precision β€” a broad digit-run search mass-false-positives on HL7 +# bodies dense with dates/order-numbers/set-ids (ADR 0030 Β§5), so the coverage report, not an +# aggressive heuristic, is the catch-all for shapes these cannot safely flag. + +#: A dashed US SSN ``NNN-NN-NNNN`` not embedded in a longer digit run. +_SSN_DASHED: re.Pattern[str] = re.compile(r"(? list[tuple[str, str]]: + """Every ``(address, value)`` in ``text`` whose whole-field ``SEG-i`` address is **not** in + ``mapped_paths`` and whose value is non-empty β€” the fields the rule map never touched. + + The MSH control header is skipped whole: its field indexing is off-by-one (MSH-N sits at + split-index N-1) and it carries routing/site data the field-anchored site-code pass already + covers, not patient PHI. ``mapped_paths`` is occurrence-agnostic (a rule applies to every + occurrence of its segment), so the address is the bare ``SEG-i``. Returns ``[]`` when the message + has no parseable MSH (there is no field separator to split on). """ - hits = [str(h) for h in _scanner().scan_text(text, include_estate=True)] - if message_has_site_code(text): - hits.append("site-code pattern") + parsed = read_message_seps(text) + if parsed is None: + return [] + _seps, field_sep = parsed + out: list[tuple[str, str]] = [] + for seg in text.replace("\r\n", "\r").replace("\n", "\r").split("\r"): + if not seg: + continue + fields = seg.split(field_sep) + if fields[0].upper() == "MSH": + continue + seg_id = fields[0] + for i in range(1, len(fields)): + value = fields[i] + if not value: + continue + address = f"{seg_id}-{i}" + if address in mapped_paths: + continue + out.append((address, value)) + return out + + +def _has_mrn_typed_identifier(value: str, seps: Seps) -> bool: + """True if any repetition of ``value`` is a CX with a non-empty id (component 1) and a whole + ``MR``/``MRN`` id-type component β€” an unmapped medical-record number by HL7 structure, far more + precise than a bare digit-run heuristic.""" + for rep in value.split(seps.repetition): + comps = rep.split(seps.component) + if comps[0] and any(comp.upper() in _MRN_TYPES for comp in comps): + return True + return False + + +def _structural_reasons(value: str, seps: Seps) -> list[str]: + """PHI-safe shape labels for one unmapped field value β€” the SHAPE only, never the value.""" + reasons: list[str] = [] + if _SSN_DASHED.search(value): + reasons.append("unmapped SSN-shaped value") + if _PHONE_DASHED.search(value) or _PHONE_PAREN.search(value): + reasons.append("unmapped phone-shaped value") + if _has_mrn_typed_identifier(value, seps): + reasons.append("unmapped MRN-typed identifier") + return reasons + + +def structural_phi_hits(text: str, mapped_paths: set[str]) -> list[str]: + """Structural PHI-shape hits over the fields no rule matched β€” reasons name the shape + field + ADDRESS only (e.g. ``"unmapped SSN-shaped value in GT1-16"``), never the offending value, so the + result is safe to raise/log. Empty when the message has no parseable MSH.""" + parsed = read_message_seps(text) + if parsed is None: + return [] + seps, _field_sep = parsed + hits: list[str] = [] + for address, value in unmapped_field_values(text, mapped_paths): + hits.extend(f"{reason} in {address}" for reason in _structural_reasons(value, seps)) return hits + + +@dataclass(frozen=True) +class LeakReport: + """The full result of a leak-check pass β€” the token hits that decide the fail-closed outcome plus + the coverage context that makes the check's reach legible (all PHI-safe: addresses and reasons, + never a field value). + + * ``hits`` β€” every leak reason (token/IP/site + structural); non-empty means refuse. + * ``unmapped_fields`` β€” the addresses present but matched by no rule (the coverage report). + * ``structural_hits`` β€” the subset of ``hits`` from the structural PHI-shape detectors. + * ``token_tables_live`` β€” whether the denylist tables loaded from a real token source. + * ``token_floor_reason`` β€” why the denylist is not trustworthy, or ``None`` if it is. + """ + + hits: list[str] + unmapped_fields: tuple[str, ...] + structural_hits: list[str] + token_tables_live: bool + token_floor_reason: str | None + + +def leak_report(text: str, *, rules: tuple[FieldRule, ...] | None = None) -> LeakReport: + """The full :class:`LeakReport` for ``text`` using the publish-guard's token authority. + + Token hits use the guard's substring + estate-token mode (ADR 0030 Β§5) plus a field-anchored + site-code check. Structural detection engages **only when ``rules`` is supplied** β€” the address + of every unmapped field is derived from ``{r.path for r in rules}`` and the high-precision + detectors run over those fields. With ``rules is None`` (a direct/legacy call over a bare string) + structural detection is skipped and the result is byte-for-byte the legacy token-only behaviour. + """ + scanner = _scanner() + token_hits = [str(h) for h in scanner.scan_text(text, include_estate=True)] + if message_has_site_code(text): + token_hits.append("site-code pattern") + if rules is None: + unmapped: tuple[str, ...] = () + structural: list[str] = [] + else: + mapped_paths = {r.path for r in rules} + unmapped = tuple(sorted({addr for addr, _ in unmapped_field_values(text, mapped_paths)})) + structural = structural_phi_hits(text, mapped_paths) + token_tables_live = bool(scanner.TOKENS_PRESENT) + token_floor_reason: str | None = scanner.token_floor_failure() + return LeakReport( + hits=token_hits + structural, + unmapped_fields=unmapped, + structural_hits=structural, + token_tables_live=token_tables_live, + token_floor_reason=token_floor_reason, + ) + + +def leak_check(text: str, *, rules: tuple[FieldRule, ...] | None = None) -> list[str]: + """Forbidden-token + structural PHI hits in ``text`` (empty list = clean) β€” a thin wrapper over + :func:`leak_report`. ``rules`` scopes the structural PHI-shape detectors to the fields no rule + matched; omitting it (a bare-string call) runs the legacy token-only check. + """ + return leak_report(text, rules=rules).hits + + +def coverage_clause(report: LeakReport) -> str: + """A PHI-safe suffix for a fail-closed message naming what the check reached β€” the count and + ADDRESSES of the unmapped fields (never their values) and whether the denylist tables were live.""" + live = "yes" if report.token_tables_live else "no" + fields = ", ".join(report.unmapped_fields) if report.unmapped_fields else "none" + return ( + f" (checked {len(report.unmapped_fields)} unmapped field(s): {fields}; " + f"denylist tables live: {live})" + ) diff --git a/messagefoundry/api/app.py b/messagefoundry/api/app.py index cc1a1833..e0fb7e24 100644 --- a/messagefoundry/api/app.py +++ b/messagefoundry/api/app.py @@ -1427,8 +1427,13 @@ async def ai_chat( ) # Build the broker from the SERVER's settings (never the request body). The SSRF endpoint-allowlist # + cleartext-credential checks run in the constructor; a misconfiguration is an operator error. + # #329: thread the derived instance posture (the same _phi_read_posture derived at create_app time + # from ai_settings, which == app.state.ai == `ai` on the managed path) so the broker's cleartext- + # http credential refusal is clamped on an enforcing-PHI instance β€” the escape can no longer put + # the api_key on the wire. Without this the route would build the broker with an unclamped escape + # (green and inert), the exact failure mode #329 exists to close. try: - broker = ai_broker_from_settings(ai) + broker = ai_broker_from_settings(ai, posture=_phi_read_posture) except AiBrokerError as exc: _log.warning("engine AI broker misconfigured: %s", exc) raise HTTPException(503, "engine-brokered AI assistance is not available") from exc @@ -5331,19 +5336,27 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: # WITHOUT changing capacity. A no-op returning None in production / every other test, so the # engine is byte-identical when the gate is unset. Stashed for /stats + shut down in finally. app.state.connscale_executor = maybe_install_executor_shim(asyncio.get_running_loop()) + # #329: the derived instance hop posture, computed ONCE here for the OUT-OF-GATE cells this + # lifespan builds β€” the alerts webhook sink (notifier_from_settings) and the LDAPS bind + # (AuthService β†’ LdapAuthenticator). Neither is built inside an active_hop_posture scope, so + # current_hop_posture() is None there and their weakened-TLS escape would ship UNCLAMPED (green + # and inert) without an explicit posture; threading this makes the ADR-0092 clamp apply on first + # deployment. None when the instance declares no [ai] (SQLite/test) β†’ the unclamped escape, + # byte-identical. Reuses the same hop_posture_from_ai derivation the store hop and the runner use. + _hop_posture = ( + hop_posture_from_ai( + ai_settings, enforcement=(security_settings or SecuritySettings()).enforcement + ) + if ai_settings is not None + else None + ) # #200 (ADR 0092 decision 2): thread the derived instance posture so the engine<->store weakened- # TLS refusal (connection_string / _build_ssl) clamps MEFOR_ALLOW_INSECURE_TLS β€” the escape can # never relax a production-PHI store hop. None when no [ai] (SQLite/test) β†’ unclamped, unchanged. store = await open_store( resolved, message_events=message_events, - posture=( - hop_posture_from_ai( - ai_settings, enforcement=(security_settings or SecuritySettings()).enforcement - ) - if ai_settings - else None - ), + posture=_hop_posture, ) # Offline uploaded-logs store (BACKLOG #125/#126, ADR 0134), on the LIVE store's cipher instance. # DISABLED (None) unless [store].uploads_dir is set, so no PHI-at-rest surface exists unless an @@ -5385,6 +5398,8 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: # #323 layer 3: the instance [tls] internal-CA policy reaches the alerts SMTP hop too, so # an estate on a private CA needs no per-alert CA path. trust_anchor_policy=tls_settings.policy() if tls_settings else None, + # #329: clamp the webhook sink's cleartext-http escape to the derived instance posture. + posture=_hop_posture, ) if alerts_settings is not None else None @@ -5633,6 +5648,10 @@ async def _audit_upload_prune(meta: UploadedFileMeta) -> None: # central run_anchor_preflight above) rather than refusing at enforce-only. Central # preflight already ran before any listener bound; this keeps the seam consistent. enforcing=trust_anchors_enforcing, + # #329: thread the derived instance posture to the LDAPS bind so its ad_tls_verify=false + # escape is clamped on an enforcing-PHI instance (LdapAuthenticator is built out of the + # connector-construction gate, so the clamp is inert unless the posture arrives here). + hop_posture=_hop_posture, ) bootstrap = await auth.initialize() app.state.auth = auth diff --git a/messagefoundry/auth/ldap.py b/messagefoundry/auth/ldap.py index 82e1b62e..66ae634d 100644 --- a/messagefoundry/auth/ldap.py +++ b/messagefoundry/auth/ldap.py @@ -27,8 +27,9 @@ from messagefoundry.config.settings import ( INSECURE_TLS_ESCAPE_ENV, AuthSettings, - insecure_tls_allowed, + weakened_tls_escape_permitted, ) +from messagefoundry.config.tls_policy import HopPosture logger = logging.getLogger(__name__) @@ -87,7 +88,11 @@ class LdapAuthenticator: """Binds against Active Directory over LDAPS and resolves a user's (nested) group membership.""" def __init__( - self, settings: AuthSettings, *, secret_provider: SecretProvider | None = None + self, + settings: AuthSettings, + *, + secret_provider: SecretProvider | None = None, + posture: HopPosture | None = None, ) -> None: if not settings.ad_server or not settings.ad_user_search_base: raise LdapError("AD is enabled but ad_server / ad_user_search_base are not configured") @@ -105,16 +110,26 @@ def __init__( if not settings.ad_bind_dn or not self._bind_password: raise LdapError("AD is enabled but the service-account bind is not configured") self._s = settings - # A disabled-cert-verification posture (ad_tls_verify=false over LDAPS) makes the service- - # account and user binds MITM-able, so it now REFUSES at startup unless the operator sets the - # explicit MEFOR_ALLOW_INSECURE_TLS dev escape β€” it can no longer be silently turned on in - # production (ASVS 12.3.2). With the escape set, we still warn loudly once at startup. + # #329: the instance hop posture (threaded by AuthService from create_app's derived posture). + # LDAPS is built OUT of the connector-construction gate (AuthService, not build_check_registry), + # so current_hop_posture() would be None here; the posture must be passed explicitly or the + # clamp below would be inert. None (a direct/test/embedding construction) falls back to the + # unclamped escape β€” byte-identical to the pre-#329 bare read. + self._posture = posture + # A disabled-cert-verification posture (ad_tls_verify=false over LDAPS) would make the service- + # account and user binds MITM-able on first deployment, so it REFUSES at startup unless the + # operator sets the explicit MEFOR_ALLOW_INSECURE_TLS dev escape (ASVS 12.3.2). #329 routes that + # escape through the ADR-0092 clamp (weakened_tls_escape_permitted): under an enforcing-PHI + # posture the escape is INERT, so it can never silence this refusal on such an instance β€” the + # blunt env var no longer buys verify-off there. With the escape permitted (non-enforcing/non-PHI + # or unstamped posture), we still warn loudly once at startup. if str(settings.ad_server).lower().startswith("ldaps") and not settings.ad_tls_verify: - if not insecure_tls_allowed(): + if not weakened_tls_escape_permitted(self._posture): raise LdapError( "ad_tls_verify=false disables LDAPS certificate verification (MITM risk). Use a " f"trusted CA via ad_tls_ca_cert_file, or set {INSECURE_TLS_ESCAPE_ENV}=1 to " - "explicitly allow it for a trusted-network dev/test bind." + "explicitly allow it for a trusted-network dev/test bind (refused on an enforcing " + "PHI instance even with that override set, #329)." ) logger.warning( "AD LDAPS certificate verification is DISABLED (ad_tls_verify=false, permitted by " diff --git a/messagefoundry/auth/service.py b/messagefoundry/auth/service.py index 4b1cb607..70f2c50e 100644 --- a/messagefoundry/auth/service.py +++ b/messagefoundry/auth/service.py @@ -61,6 +61,7 @@ from messagefoundry.config.models import SignatureAlgorithm from messagefoundry.config.secretprovider import SecretProvider, resolve_connector_secret from messagefoundry.config.settings import AuthSettings +from messagefoundry.config.tls_policy import HopPosture from messagefoundry.store.base import AdminStore from messagefoundry.store.store import SessionRecord, UserRecord, WebAuthnCredential @@ -239,6 +240,7 @@ def __init__( security_notifier: SecurityNotifier | None = None, secret_provider: SecretProvider | None = None, enforcing: bool = True, + hop_posture: HopPosture | None = None, ) -> None: self._store = store self._settings = settings @@ -271,8 +273,12 @@ def __init__( self._ldap: LdapAuthenticator | None = ldap elif settings.ad_enabled: # Thread the connector SecretProvider (ADR 0019 Β§5) so an ad_bind_password_secret reference - # resolves the bind password from the external backend (fail-closed) at construction. - self._ldap = LdapAuthenticator(settings, secret_provider=secret_provider) + # resolves the bind password from the external backend (fail-closed) at construction. #329: + # thread the instance hop posture too β€” LDAPS is built out of the connector-construction gate, + # so its ad_tls_verify=false escape clamp is inert unless the posture arrives explicitly here. + self._ldap = LdapAuthenticator( + settings, secret_provider=secret_provider, posture=hop_posture + ) else: self._ldap = None # Instance-scoped (one event loop per AuthService) so it never crosses loops in tests. diff --git a/messagefoundry/config/settings.py b/messagefoundry/config/settings.py index acd0e132..672b14ee 100644 --- a/messagefoundry/config/settings.py +++ b/messagefoundry/config/settings.py @@ -213,13 +213,18 @@ def weakened_tls_escape_permitted(posture: HopPosture | None = None) -> bool: """Whether ``MEFOR_ALLOW_INSECURE_TLS`` may permit a weakened / verify-off TLS hop under ``posture``, CLAMPED so an enforcing PHI hop is NEVER relaxed (#200, ADR 0092 decision 2). - The is_phi-blind **strict verify-off** cells β€” the engine<->store TLS gate + The is_phi-blind **weakened-TLS / cleartext-escape** cells route their global-escape check through + here so the blunt escape can no longer silence an **enforcing PHI** refusal (matching the + ``--allow-insecure-bind`` API-bind clamp). That is **at least** the engine<->store TLS gate (:func:`~messagefoundry.store.sqlserver.connection_string` / ``store.postgres._build_ssl``), the MLLP - and FTPS ``tls_verify=false`` contexts, and the credentialed plain-``ftp`` guard β€” route their global- - escape check through here so the blunt escape can no longer silence an **enforcing PHI** refusal - (matching the ``--allow-insecure-bind`` API-bind clamp). Pass the construction-time - :func:`~messagefoundry.config.tls_policy.current_hop_posture` (transport cells) or the store's threaded - posture. Semantics: the escape must be set at all, AND the hop must not be enforcing PHI. ``None`` + and FTPS ``tls_verify=false`` contexts and the credentialed plain-``ftp`` guard, **and β€” since #329 β€”** + the LDAPS ``ad_tls_verify=false`` bind (:mod:`messagefoundry.auth.ldap`), the SFTP unknown-host-key + acceptance (:mod:`messagefoundry.transports.remotefile`), and the webhook-alert-sink and AI-broker + cleartext-``http`` hops. Pass the construction-time + :func:`~messagefoundry.config.tls_policy.current_hop_posture` (in-gate transport cells, via + :func:`weakened_tls_escape_permitted_here`) or an explicitly-threaded posture (the store hop and the + out-of-gate #329 cells, whose construction never stamps the contextvar). Semantics: the escape must + be set at all, AND the hop must not be enforcing PHI. ``None`` (a backup utility / embedding / test outside the construction gate) falls back to the **unclamped** escape β€” byte-identical to pre-#200 β€” since the enforced serve/reload gate already vetted the real production posture, so this fallback never loosens the clamp.""" diff --git a/messagefoundry/pipeline/alert_sinks.py b/messagefoundry/pipeline/alert_sinks.py index 550a787d..79c310c7 100644 --- a/messagefoundry/pipeline/alert_sinks.py +++ b/messagefoundry/pipeline/alert_sinks.py @@ -43,9 +43,9 @@ AlertSeverity, AlertsSettings, EscalationTier, - insecure_tls_allowed, + weakened_tls_escape_permitted, ) -from messagefoundry.config.tls_policy import TrustAnchorPolicy, build_smtp_tls_context +from messagefoundry.config.tls_policy import HopPosture, TrustAnchorPolicy, build_smtp_tls_context __all__ = [ "AlertTransport", @@ -277,18 +277,26 @@ class WebhookTransport: """POST the event as JSON to a configured URL (fronts Slack/Teams/PagerDuty/custom webhooks).""" def __init__( - self, url: str, *, timeout: float = 10.0, allowed_hosts: tuple[str, ...] = () + self, + url: str, + *, + timeout: float = 10.0, + allowed_hosts: tuple[str, ...] = (), + posture: HopPosture | None = None, ) -> None: # Refuse a plaintext http:// webhook target unless the explicit dev escape is set: the alert # POST otherwise crosses the network in cleartext (ASVS 12.2.1 β€” no insecure fallback). https # is the only scheme accepted by default; http(s) remain the only schemes at all (see _post). - # Same refuse-unless-MEFOR_ALLOW_INSECURE_TLS pattern as LDAPS / SQL Server / MLLP β€” stricter - # than the credentialed-only http refusal on REST/SOAP, since a webhook has no PHI but should - # still never fall back to cleartext. + # Same refuse-unless-MEFOR_ALLOW_INSECURE_TLS pattern as LDAPS / SQL Server / MLLP. #329: read + # the escape through the ADR-0092 clamp (weakened_tls_escape_permitted) so on an enforcing-PHI + # instance the blunt env var can never re-permit a cleartext alert POST. The webhook sink is + # built out of the connector-construction gate (notifier_from_settings, in the app lifespan), so + # the posture is threaded explicitly; None (a direct/test construction) falls back to the + # unclamped escape β€” byte-identical to the pre-#329 bare read. scheme = urllib.parse.urlsplit(url).scheme.lower() if scheme not in ("http", "https"): raise ValueError(f"webhook url must be http or https, got scheme {scheme!r}") - if scheme == "http" and not insecure_tls_allowed(): + if scheme == "http" and not weakened_tls_escape_permitted(posture): raise ValueError( f"webhook url {url!r} uses plaintext http; refused unless " f"{INSECURE_TLS_ESCAPE_ENV} is set (dev/trusted-network only) β€” use https" @@ -1168,6 +1176,7 @@ def notifier_from_settings( *, secret_provider: SecretProvider | None = None, trust_anchor_policy: TrustAnchorPolicy | None = None, + posture: HopPosture | None = None, ) -> NotifierAlertSink | None: """Build a :class:`NotifierAlertSink` from ``[alerts]`` settings, or ``None`` when no transport is configured (the caller then leaves the engine on its default logging sink). @@ -1197,6 +1206,9 @@ def notifier_from_settings( alerts.webhook_url, timeout=alerts.webhook_timeout, allowed_hosts=tuple(alerts.webhook_allowed_hosts), + # #329: thread the derived instance posture so the cleartext-http refusal is clamped on + # an enforcing-PHI instance (the escape can no longer re-permit a plaintext alert POST). + posture=posture, ) ) if alerts.email_smtp_host and alerts.email_from and alerts.email_to: diff --git a/messagefoundry/transports/ai_broker.py b/messagefoundry/transports/ai_broker.py index 450aae66..834db029 100644 --- a/messagefoundry/transports/ai_broker.py +++ b/messagefoundry/transports/ai_broker.py @@ -41,7 +41,8 @@ import urllib.request from typing import TYPE_CHECKING -from messagefoundry.config.settings import INSECURE_TLS_ESCAPE_ENV, insecure_tls_allowed +from messagefoundry.config.settings import INSECURE_TLS_ESCAPE_ENV, weakened_tls_escape_permitted +from messagefoundry.config.tls_policy import HopPosture # Reuse rest.py's hardened, TLS-verifying, no-redirect opener + URL redaction (no new HTTP plumbing) β€” # exactly as smart.py / fhir.py / soap.py do. No import cycle: rest.py never imports this module. @@ -114,6 +115,7 @@ def __init__( model: str = "claude-opus-4-8", max_output_tokens: int = _DEFAULT_MAX_OUTPUT_TOKENS, timeout_seconds: float = _DEFAULT_TIMEOUT, + posture: HopPosture | None = None, ) -> None: if not endpoint: raise AiBrokerError( @@ -136,8 +138,13 @@ def __init__( "list it explicitly to permit engine-brokered AI egress (SSRF fail-closed)" ) # The api_key is a credential β€” refuse to send it over cleartext http (mirrors smart.py's token - # endpoint), unless the dev escape is set for a trusted-network dev/test box. - if scheme == "http" and not insecure_tls_allowed(): + # endpoint), unless the dev escape is set for a trusted-network dev/test box. #329: read the + # escape through the ADR-0092 clamp (weakened_tls_escape_permitted) so on an enforcing-PHI + # instance the blunt env var can never re-permit the key on the wire. The broker is built out of + # the connector-construction gate (the create_app ai_chat route), so the posture is threaded + # explicitly; None (a direct/test construction) falls back to the unclamped escape β€” byte- + # identical to the pre-#329 bare read. + if scheme == "http" and not weakened_tls_escape_permitted(posture): raise AiBrokerError( "[ai].endpoint over cleartext http would expose the api_key; refused unless " f"{INSECURE_TLS_ESCAPE_ENV} is set (dev/trusted-network only) β€” use https" @@ -239,14 +246,20 @@ def _extract_text(self, body: str) -> str: return text -def ai_broker_from_settings(ai: AiSettings) -> AiBroker: +def ai_broker_from_settings(ai: AiSettings, *, posture: HopPosture | None = None) -> AiBroker: """Build the :class:`AiBroker` from the loaded ``[ai]`` settings, or raise :class:`AiBrokerError` when the engine broker is not fully configured. Settings arrive already ``env()``-resolved (the API - lifespan stashes the resolved :class:`AiSettings` on ``app.state.ai``).""" + lifespan stashes the resolved :class:`AiSettings` on ``app.state.ai``). + + ``posture`` (#329) is the derived instance hop posture, threaded from the create_app ai_chat route so + the broker's cleartext-http credential refusal is clamped on an enforcing-PHI instance (the escape + can no longer put the ``api_key`` on the wire there). ``None`` = the unclamped escape, byte-identical + to before.""" return AiBroker( endpoint=ai.endpoint or "", api_key=ai.api_key or "", allowed_endpoints=list(ai.allowed_endpoints), provider=ai.provider, model=ai.model, + posture=posture, ) diff --git a/messagefoundry/transports/remotefile.py b/messagefoundry/transports/remotefile.py index ae737648..bf68157a 100644 --- a/messagefoundry/transports/remotefile.py +++ b/messagefoundry/transports/remotefile.py @@ -53,7 +53,6 @@ from messagefoundry.config.models import ConnectorType, ContentType, Destination, Source from messagefoundry.config.settings import ( INSECURE_TLS_ESCAPE_ENV, - insecure_tls_allowed, weakened_tls_escape_permitted_here, ) from messagefoundry.config.tls_policy import ( @@ -370,9 +369,13 @@ def __init__(self, settings: dict[str, Any]) -> None: self._known_hosts = settings.get("known_hosts") self._timeout = float(settings.get("connect_timeout", 30.0)) # Fail fast at construction (build_check time): an unknown-host-key posture without the escape - # must never silently weaken to auto-accept. The accept-unknown policy is gated here so the - # connector refuses to build rather than trust-on-first-use a man-in-the-middle. - self._accept_unknown = insecure_tls_allowed() + # must never silently weaken to auto-accept. #329: read the escape through the ADR-0092 clamp + # (weakened_tls_escape_permitted_here consults the active construction posture, exactly like the + # FTPS tls_verify=false sibling at :176 that this class is built alongside), so under an + # enforcing-PHI posture the escape is INERT β€” the accept-unknown policy then stays RejectPolicy + # and an unknown host key is refused at connect (:392-394), as today. Off the construction gate + # (posture unstamped) the escape is unclamped, byte-identical to the pre-#329 bare read. + self._accept_unknown = weakened_tls_escape_permitted_here() if self._accept_unknown: logger.warning( "REMOTEFILE sftp %s accepts UNKNOWN host keys (AutoAddPolicy) because %s is set " diff --git a/scripts/asvs/scorecard.py b/scripts/asvs/scorecard.py index 33bc1d52..2526fa01 100644 --- a/scripts/asvs/scorecard.py +++ b/scripts/asvs/scorecard.py @@ -19,8 +19,12 @@ from __future__ import annotations import argparse +import ast import re +import shutil +import subprocess import sys +import tempfile import tomllib from collections import Counter from dataclasses import dataclass, field @@ -102,11 +106,43 @@ class Absence: Do NOT derive ``mutation`` from ``pattern``. A value generated from the thing it validates satisfies the check by construction, which would make this the most authoritative-looking vacuous gate in the file β€” the same defect class it exists to close, arriving through the fix. + + ``mutation_path`` and ``observable`` feed the ``--prove-absences`` mode (:func:`prove_absences`), + which closes a mode the pattern check cannot see: a ``mutation`` that matches its pattern, whose + control speaks and whose corpus is quiet, yet **changes nothing observable when applied** β€” a + reintroduction raised into a swallowing handler, a field nobody reads, a flag nobody branches on. + ``re.search(pattern, mutation)`` proves the mutation is *well-formed*; it never proves it *bites*. + + - ``mutation_path`` β€” the file the reintroduction lands in, relative to ``root``. + - ``observable`` β€” the named artifact that must go red when the mutation is applied: a + ``tests/test_x.py::test_y`` pytest node id. When both fields are set the mode PROVES the claim + by execution β€” it runs the observable on a scratch copy of the tree (baseline must be green), + applies the mutation, and requires the observable to FAIL (and to fail as a test failure, not a + collection/usage error, which fails closed). When only ``mutation_path`` is set the mode falls + back to a coarse static backstop. + + Both fields default empty, and their absence means **"not yet proven by execution"** β€” never + "proven vacuous". They are opt-in per claim because a live proof spawns a pytest subprocess per + claim; a claim with neither field is reported as SKIPPED, not failed. + + Honest limits of the proving mode, stated so they are not overclaimed: + + - Application is **append-based**: the mutation text is appended to the scratch target, so it + breaks a fixture by *redefinition shadowing*. That faithfully reddens a well-formed + reintroduction in a fixture; it does not reproduce every in-function reintroduction a real claim + might describe. + - The static backstop is a **coarse same-file heuristic** β€” a ``raise`` in the mutation landing + lexically in a ``try`` whose every handler swallows (bare/``Exception``, log-only body). It + proves **nothing** in general: it cannot see a swallow in a *caller* rather than at the landing + site, so it would miss the very cross-file instance that motivated this item. It is a screen, + not a proof, and must not be written up as one. """ pattern: str positive_control: str mutation: str + mutation_path: str = "" + observable: str = "" @dataclass(frozen=True) @@ -143,6 +179,13 @@ class Findings: checked_anchors: int = 0 checked_absences: int = 0 skipped_anchors: int = 0 + #: Populated only by :func:`prove_absences`. ``proved_absences`` counts claims whose observable + #: went red under the applied mutation (a live proof); ``static_screened`` counts claims that took + #: the static backstop (a screen, not a proof); ``skipped_absences`` counts claims carrying no + #: ``mutation_path`` (nothing to apply). UNPROVEN and PROVE-ERROR outcomes go into ``problems``. + proved_absences: int = 0 + static_screened: int = 0 + skipped_absences: int = 0 @property def ok(self) -> bool: @@ -271,6 +314,12 @@ def load_scorecard(path: Path) -> list[Cell]: # No default. A missing mutation must be authored, not inferred β€” see the # Absence docstring on why deriving one from the pattern is worse than none. mutation=str(a["mutation"]), + # Optional, and deliberately NOT refused at load. A hard requirement here would + # void every already-authored absence claim (none carry these yet), and their + # re-authoring is out of this script's reach (ADR 0156 Β§7). Absent means "not + # yet proven by execution", surfaced by --prove-absences, not "proven vacuous". + mutation_path=str(a.get("mutation_path", "")), + observable=str(a.get("observable", "")), ) for a in raw.get("absence", []) ), @@ -431,6 +480,244 @@ def _grep_count(pattern: str, files: list[Path]) -> int: return sum(1 for f in files if rx.search(f.read_text(encoding="utf-8", errors="replace"))) +# --- proving an absence by mutation (--prove-absences) -------------------------------------------- +# +# check_absences proves a mutation is well-formed (its pattern fires on it). It cannot prove the +# mutation BITES: applied, does anything observable go red? A reintroduction raised into a swallowing +# handler passes every check in check_absences and changes nothing. This mode closes that hole by +# EXECUTING the claim β€” mutate a scratch copy of the tree, run the named observable, require it to go +# red β€” and it fails closed on every code that is not an honest test failure, so a typo'd node or an +# already-red observable can never masquerade as "the control bit". The whole pass runs inside a +# TemporaryDirectory scratch copy, so it never mutates `root` and never trips the committed-tree scan +# on itself. + + +def _scratch_ignore(dirpath: str, names: list[str]) -> set[str]: + """Names to skip when copying `root` into the scratch tree. Beyond the usual VCS/venv/cache noise + this refuses secrets and posture data β€” ``.env*``, ``*.db`` and its WAL sidecars (the local + store), and the vault's ``docs/security`` tree (ADR 0156 Β§7). The vault runs this module against + the REAL tree, so a scratch copy carrying those would spill them into a world-default temp dir, + which CLAUDE.md Β§9 forbids this module reading at all. Public-repo runs never see them (no + committed ``.env``/``*.db``, ``docs/security`` absent), so this is defence for the vault run.""" + ignored = set( + shutil.ignore_patterns( + ".git", + ".venv", + "__pycache__", + "node_modules", + ".env", + ".env.*", + "*.db", + "*.db-wal", + "*.db-shm", + )(dirpath, names) + ) + # `docs/security` is path-specific, not a basename glob: skip a `security` entry only directly + # under `docs`, leaving any unrelated `security` elsewhere in the tree copied. + if Path(dirpath).name == "docs" and "security" in names: + ignored.add("security") + return ignored + + +def _copy_scratch(root: Path, dest: Path) -> Path: + """Copy `root` into `dest`, skipping VCS/venv/cache dirs and β€” defensively, for the vault run β€” + secrets, the local store, and vault posture data (:func:`_scratch_ignore`). Never writes to + `root`.""" + shutil.copytree(root, dest, ignore=_scratch_ignore) + return dest + + +def _is_within_tree(rel: str) -> bool: + """True only for a repo-relative path with no anchor and no ``..`` component β€” one that cannot + escape the scratch copy when joined onto it. ``mutation_path`` comes from the authored scorecard, + so it is untrusted for this purpose: an absolute or ``..``-bearing value is refused, not resolved.""" + p = Path(rel) + if p.is_absolute() or p.anchor: + return False + return ".." not in p.parts + + +def _apply_mutation(scratch: Path, mutation_path: str, mutation: str) -> None: + """Append the reintroduction to the scratch target β€” redefinition shadowing is what makes a + well-formed reintroduction actually break an observable. Never called against `root`.""" + target = scratch / mutation_path + with target.open("a", encoding="utf-8") as fh: + fh.write("\n" + mutation + "\n") + + +def _run_node(scratch: Path, node_id: str, python: str, timeout: float) -> int: + """Run one pytest node inside the scratch copy and return its exit code. + + Invoked with ``--rootdir `` and ``cwd=scratch`` and ``-o addopts=`` so no repo + ``conftest``/``pyproject``/addopts leaks into the child run, and ``-p no:cacheprovider`` so it + writes nothing back. A timeout is treated as a non-{0,1} code β€” fail closed, never a proof. + """ + try: + proc = subprocess.run( # nosec B603 B607 - fixed argv, no shell; python is sys.executable, node id is scorecard-authored not shell-interpreted + [ + python, + "-m", + "pytest", + "-q", + "--rootdir", + str(scratch), + "-p", + "no:cacheprovider", + "-o", + "addopts=", + node_id, + ], + cwd=scratch, + capture_output=True, + text=True, + check=False, + timeout=timeout, + ) + except subprocess.TimeoutExpired: + return 124 # non-zero and non-1: fails closed as a PROVE-ERROR, never counted as a proof + return proc.returncode + + +def _handler_swallows(handler: ast.ExceptHandler) -> bool: + """A handler that catches broadly (bare or ``Exception``/``BaseException``) with a log-only/``pass`` + body and no re-raise β€” the shape that eats a reintroduced exception.""" + caught = handler.type + if not ( + caught is None + or (isinstance(caught, ast.Name) and caught.id in {"Exception", "BaseException"}) + ): + return False + if any(isinstance(n, ast.Raise) for stmt in handler.body for n in ast.walk(stmt)): + return False # a re-raise is not a swallow + return all(isinstance(stmt, (ast.Pass, ast.Expr)) for stmt in handler.body) + + +def _landing_swallows(source: str) -> bool: + """True if `source` contains a ``try`` whose EVERY handler swallows (see :func:`_handler_swallows`). + + A coarse same-file heuristic β€” it proves nothing in general and cannot see a swallow in a caller. + """ + try: + tree = ast.parse(source) + except SyntaxError: + return False + return any( + isinstance(node, ast.Try) + and bool(node.handlers) + and all(_handler_swallows(h) for h in node.handlers) + for node in ast.walk(tree) + ) + + +def _prove_one( + a: Absence, + cell_id: str, + root: Path, + scratch_dir: Path, + findings: Findings, + *, + python: str, + timeout: float, +) -> None: + if not a.mutation_path: + # Nothing to apply. Reported, not failed: opt-in per claim (a live proof spawns a subprocess). + findings.skipped_absences += 1 + return + if not _is_within_tree(a.mutation_path): + # mutation_path is authored data. An absolute path or a `..` escape would let _apply_mutation + # write outside the scratch copy (and the is_file probe below read outside `root`), defeating + # the 'never touches root' guarantee. Refuse it rather than resolve it. + findings.problems.append( + f"{cell_id}: absence claim PROVE-ERROR β€” mutation_path {a.mutation_path!r} is not a " + "repo-relative path inside the tree (it is absolute or contains '..'), so applying the " + "mutation could escape the scratch copy" + ) + return + target_in_root = root / a.mutation_path + if not target_in_root.is_file(): + findings.problems.append( + f"{cell_id}: absence claim PROVE-ERROR β€” mutation_path {a.mutation_path!r} is not a file " + "in the tree, so the mutation cannot be applied" + ) + return + + if a.observable: + scratch = _copy_scratch(root, scratch_dir) + baseline = _run_node(scratch, a.observable, python, timeout) + if baseline != 0: + # An already-red or uncollectable observable cannot attribute its red to the mutation. + findings.problems.append( + f"{cell_id}: absence claim PROVE-ERROR β€” observable {a.observable!r} is not green on " + f"the pristine tree (pytest exit {baseline}); a red or uncollectable baseline cannot " + "be attributed to the mutation" + ) + return + _apply_mutation(scratch, a.mutation_path, a.mutation) + mutated = _run_node(scratch, a.observable, python, timeout) + if mutated == 1: + findings.proved_absences += 1 # live proof: the control bit + elif mutated == 0: + findings.problems.append( + f"{cell_id}: absence claim UNPROVEN β€” applying the mutation to {a.mutation_path} left " + f"observable {a.observable!r} green (pytest exit 0); the mutation changes nothing the " + "control catches, so this claim is syntax without behaviour" + ) + else: + # exit 2/3/4/5/...: collection or usage error. NEVER a proof β€” a typo'd node or a mutation + # that merely breaks import must not masquerade as the control biting. + findings.problems.append( + f"{cell_id}: absence claim PROVE-ERROR β€” mutated run of {a.observable!r} errored " + f"(pytest exit {mutated}) rather than failing; a collection or usage error must not " + "count as the control biting" + ) + return + + # Static backstop: mutation_path but no observable. A screen, not a proof (see Absence docstring). + findings.static_screened += 1 + if re.search(r"\braise\b", a.mutation) and _landing_swallows( + target_in_root.read_text(encoding="utf-8", errors="replace") + ): + findings.problems.append( + f"{cell_id}: absence claim SUSPECT (static heuristic) β€” its reintroduction raises into " + f"{a.mutation_path}, which has a try/except that swallows (bare or Exception, log-only " + "body), so a live raise there may be caught and prove nothing. Supply an `observable` to " + "prove it by execution" + ) + + +def prove_absences( + cells: list[Cell], + root: Path, + *, + python: str = sys.executable, + timeout: float = 120.0, +) -> Findings: + """Prove each absence claim BITES: apply its mutation to a scratch copy and require its observable + to go red. Fails closed on anything that is not an honest baseline-green / mutated-fail pair. + + This is separate from :func:`verify` and opt-in (``--prove-absences``) because it spawns a pytest + subprocess per provable claim. It never touches `root`. + """ + findings = Findings() + resolved_root = root.resolve() + with tempfile.TemporaryDirectory(prefix="asvs_prove_") as td_base: + base = Path(td_base) + i = 0 + for c in cells: + for a in c.absence: + i += 1 + _prove_one( + a, + c.id, + resolved_root, + base / f"scratch_{i}", + findings, + python=python, + timeout=timeout, + ) + return findings + + def _sort_key(cell_id: str) -> tuple[int, ...]: try: return tuple(int(p) for p in cell_id.split(".")) @@ -596,20 +883,59 @@ def render_current(cells: list[Cell], *, anchor_sha: str) -> str: return chr(10).join(lines) + chr(10) +def _run_prove_absences(scorecard: Path, root: Path) -> int: + """The ``--prove-absences`` entry point: execute-prove every absence claim (see + :func:`prove_absences`). Needs no corpus β€” it applies mutations, it does not grep for patterns.""" + try: + cells = load_scorecard(scorecard) + except ScorecardError as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 # could not measure β€” never 0, never confused with "clean" + findings = prove_absences(cells, root) + print( + f"prove-absences: proved {findings.proved_absences} by mutation; " + f"{findings.static_screened} static-screened; {findings.skipped_absences} skipped; " + f"{len(findings.problems)} problem(s)" + ) + for p in findings.problems: + print(f" FAIL {p}", file=sys.stderr) + return 0 if findings.ok else 1 + + def main(argv: list[str] | None = None) -> int: ap = argparse.ArgumentParser(description="Verify or render the ASVS scorecard (ADR 0156).") ap.add_argument("--scorecard", type=Path, required=True) - ap.add_argument("--corpus", type=Path, required=True) + # Not required: --prove-absences applies mutations and never greps for patterns, so it needs no + # corpus. Verify mode still does; that is enforced after parsing, not by argparse. + ap.add_argument("--corpus", type=Path, required=False) ap.add_argument( "--root", type=Path, default=Path.cwd(), help="tree the evidence anchors point into" ) ap.add_argument("--render", type=Path, help="write the generated CURRENT.md here") + # A separate, opt-in mode: prove each absence claim BITES by applying its mutation to a scratch + # copy and requiring its observable to go red. Opt-in because it spawns a pytest subprocess per + # provable claim; kept out of the default verify path, which stays purely static. + ap.add_argument( + "--prove-absences", + action="store_true", + help="execute-prove absence claims (apply mutation to a scratch tree, require observable red)", + ) # NO --anchor-sha injected by CI. The anchor is the commit the EVIDENCE was read on β€” a property # of the assessment, recorded in [scorecard].anchor_commit. Passing ${{ github.sha }} made the # rendered file differ on every run, so the drift check could never pass: a gate that cannot go # green is as useless as one that cannot go red, and this one shipped that way. args = ap.parse_args(argv) + if args.prove_absences: + return _run_prove_absences(args.scorecard, args.root) + + if args.corpus is None: + print( + "error: --corpus is required to verify the scorecard (only --prove-absences may omit it)", + file=sys.stderr, + ) + return 2 + try: findings = verify(args.scorecard, args.corpus, args.root) except ScorecardError as exc: diff --git a/scripts/coord/alloc.ps1 b/scripts/coord/alloc.ps1 index 0daca930..fd5b3fbb 100644 --- a/scripts/coord/alloc.ps1 +++ b/scripts/coord/alloc.ps1 @@ -48,9 +48,27 @@ param( $ErrorActionPreference = "Stop" -$repo = (& git rev-parse --path-format=absolute --show-toplevel).Trim() -if (-not $repo) { throw "Not inside a git repository." } -$common = (& git rev-parse --path-format=absolute --git-common-dir).Trim() +# ANCHOR ON THE SCRIPT, NOT ON THE CURRENT DIRECTORY (BACKLOG #1060). Unanchored, every `git` call below +# resolved against wherever the shell happened to be standing, so an absolute `-File` invocation from +# worktree A while intending to commit from worktree B recorded the claim to A. The ledger gate then +# refused B's commit -- correctly, fails closed, nothing invalid lands -- but far from the cause and with +# a message about the wrong thing. It also cost a number: holes are free, collisions are not. +# +# `git -C $PSScriptRoot` rather than `Split-Path`, which is what the sibling scripts in scripts/dev use. +# The recorded `worktree` value is COMPARED, by ledger_check.py:227 and by -List below, so its string form +# is part of a contract: `--path-format=absolute` returns the forward-slash absolute form every claim +# already on disk carries, and `Split-Path` would start writing backslashes into the same field. Both +# comparisons normalise separators today, so this is not a live break -- it is a field whose format +# nothing pins, and changing it for no reason is how the next reader's grep stops matching. +# +# Every `git` call in this file is anchored the same way for the same reason. The floor's working-tree +# term (below) is the one where an unanchored read is more than misattribution: it looks for numbers +# written but committed NOWHERE, so reading the caller's tree makes a number drafted in the TARGET tree +# invisible and free to re-issue -- the collision this whole script exists to prevent. +$repo = (& git -C $PSScriptRoot rev-parse --path-format=absolute --show-toplevel 2>$null) +if (-not $repo) { throw "scripts/coord/ is not inside a git repository: $PSScriptRoot" } +$repo = $repo.Trim() +$common = (& git -C $repo rev-parse --path-format=absolute --git-common-dir).Trim() $allocRoot = Join-Path $common "mefor-coord/alloc" $alloc = Join-Path $allocRoot $Kind New-Item -ItemType Directory -Force -Path $alloc | Out-Null @@ -72,8 +90,8 @@ if (-not $Title -and -not $ShowFloor) { throw "-Title is required (it is recorde # `git branch --show-current` prints NOTHING on a detached HEAD, so `& git ...` yields $null (not "") # -- calling .Trim() on it here threw *before* the detached-HEAD fallback below could run. Null-check first. -$branch = & git branch --show-current -if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git rev-parse --short HEAD) } +$branch = & git -C $repo branch --show-current +if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git -C $repo rev-parse --short HEAD) } $branch = $branch.Trim() # FLOOR = max over (origin/main) U (every local + remote ref) U (existing allocations). @@ -88,9 +106,9 @@ function Get-Floor { $seen.Add(0) if ($Kind -eq "adr") { - $refs = @("origin/main") + @(& git for-each-ref --format='%(refname)' refs/heads refs/remotes) + $refs = @("origin/main") + @(& git -C $repo for-each-ref --format='%(refname)' refs/heads refs/remotes) foreach ($ref in ($refs | Select-Object -Unique)) { - $names = & git ls-tree --name-only $ref docs/adr/ 2>$null + $names = & git -C $repo ls-tree --name-only $ref docs/adr/ 2>$null foreach ($n in $names) { if ($n -match 'docs/adr/(\d{4})-') { $seen.Add([int]$Matches[1]) } } @@ -117,13 +135,13 @@ function Get-Floor { # files means adding each one here; an archive file not listed here is not policed. $backlogPaths = @("docs/BACKLOG.md", "docs/archive/backlog/BACKLOG-CLOSED.md") - $refs = @("origin/main", "HEAD") + @(& git for-each-ref --format='%(refname)' refs/heads refs/remotes) + $refs = @("origin/main", "HEAD") + @(& git -C $repo for-each-ref --format='%(refname)' refs/heads refs/remotes) $specs = foreach ($r in ($refs | Select-Object -Unique)) { foreach ($p in $backlogPaths) { "${r}:${p}" } } $oids = [System.Collections.Generic.HashSet[string]]::new() - foreach ($line in ($specs -join "`n" | & git cat-file --batch-check='%(objectname) %(objecttype)' 2>$null)) { + foreach ($line in ($specs -join "`n" | & git -C $repo cat-file --batch-check='%(objectname) %(objecttype)' 2>$null)) { $p = "$line".Split(' ') if ($p.Count -ge 2 -and $p[1] -eq 'blob') { [void]$oids.Add($p[0]) } } @@ -138,7 +156,7 @@ function Get-Floor { # that had been committed somewhere -- i.e. every case except the one this term is for. $rx = [regex]::new('^#{2,3} (\d+)\.', [System.Text.RegularExpressions.RegexOptions]::Multiline) if ($oids.Count -gt 0) { - foreach ($line in (($oids -join "`n") | & git cat-file --batch 2>$null)) { + foreach ($line in (($oids -join "`n") | & git -C $repo cat-file --batch 2>$null)) { $m = $rx.Match("$line") if ($m.Success) { $seen.Add([int]$m.Groups[1].Value) } } @@ -410,6 +428,23 @@ for ($i = $start; $i -lt $start + 500; $i++) { Write-Host " file : docs/BACKLOG.md" } Write-Host " claimed by: $repo [$branch]" + + # SAY IT AT THE POINT OF USE when the shell is standing somewhere else (BACKLOG #1060). Anchoring is + # now correct, but it is also SURPRISING: a caller who runs this by absolute path from worktree A gets + # a claim recorded to worktree B, and the only other place that fact surfaces is the ledger gate + # refusing a commit later, elsewhere, with a message about the wrong thing. One line here turns a + # deferred, misdirected refusal into an immediate, accurate note. Silent on the ordinary same-tree + # invocation, so it stays worth reading. + $cwdTop = (& git rev-parse --path-format=absolute --show-toplevel 2>$null) + if ($cwdTop) { + $a = ($cwdTop.Trim() -replace '\\', '/').TrimEnd('/') + $b = ($repo -replace '\\', '/').TrimEnd('/') + if ($a -ine $b) { + Write-Host " NOTE: your shell is in $a, but this allocator lives in $b, so the claim is recorded" -ForegroundColor Yellow + Write-Host " to $b. COMMIT FROM THERE -- the ledger gate keys entitlement on the worktree" -ForegroundColor Yellow + Write-Host " named above and will refuse the commit anywhere else." -ForegroundColor Yellow + } + } exit 0 } diff --git a/scripts/coord/claim.ps1 b/scripts/coord/claim.ps1 index b7b86057..308a2172 100644 --- a/scripts/coord/claim.ps1 +++ b/scripts/coord/claim.ps1 @@ -51,9 +51,17 @@ param( $ErrorActionPreference = "Stop" -$repo = (& git rev-parse --path-format=absolute --show-toplevel).Trim() -if (-not $repo) { throw "Not inside a git repository." } -$common = (& git rev-parse --path-format=absolute --git-common-dir).Trim() +# Anchored on the SCRIPT, not the current directory -- the reasoning is written out once, at the head of +# alloc.ps1 (BACKLOG #1060). It applies here identically and was found here by inspection rather than by +# a second reproduction: this file recorded `worktree = $repo` from the same unanchored call, so an +# absolute `-File` invocation from another worktree took the claim in the caller's name. A claim is +# advisory for free-text keys and ENFORCED for numbered ones by scripts/hooks/claim_check.py, which reads +# the repo from cwd correctly because it runs as a commit hook -- cwd IS the committing worktree there. +# Hook right, tool wrong, and only the tool can be invoked from somewhere else. +$repo = (& git -C $PSScriptRoot rev-parse --path-format=absolute --show-toplevel 2>$null) +if (-not $repo) { throw "scripts/coord/ is not inside a git repository: $PSScriptRoot" } +$repo = $repo.Trim() +$common = (& git -C $repo rev-parse --path-format=absolute --git-common-dir).Trim() $claims = Join-Path $common "mefor-coord/claims" New-Item -ItemType Directory -Force -Path $claims | Out-Null @@ -202,8 +210,8 @@ if ($List) { Show-List; exit 0 } $safe = ConvertTo-KeyFile $Take $file = Join-Path $claims "$safe.json" -$branch = & git branch --show-current -if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git rev-parse --short HEAD) } +$branch = & git -C $repo branch --show-current +if ([string]::IsNullOrWhiteSpace($branch)) { $branch = "detached@" + (& git -C $repo rev-parse --short HEAD) } $branch = $branch.Trim() try { @@ -345,4 +353,17 @@ Write-Host "CLAIMED '$Take'" -ForegroundColor Green Write-Host " by : $repo [$branch]" Write-Host " note : $(if ($Note) { $Note } else { '(no note)' })" Write-Host " release when done: pwsh -NoProfile -File scripts\coord\claim.ps1 -Release $Take" + +# Same note alloc.ps1 prints, for the same reason (BACKLOG #1060): anchoring is correct but surprising, +# and a claim recorded to a worktree the caller is not standing in otherwise surfaces only as a refused +# commit later. Silent on the ordinary same-tree invocation. +$cwdTop = (& git rev-parse --path-format=absolute --show-toplevel 2>$null) +if ($cwdTop) { + $a = ($cwdTop.Trim() -replace '\\', '/').TrimEnd('/') + $b = ($repo -replace '\\', '/').TrimEnd('/') + if ($a -ine $b) { + Write-Host " NOTE: your shell is in $a, but this script lives in $b, so the claim is" -ForegroundColor Yellow + Write-Host " recorded to $b. -Release must be run against that same worktree." -ForegroundColor Yellow + } +} exit 0 diff --git a/scripts/dev/setup-leak-gate.ps1 b/scripts/dev/setup-leak-gate.ps1 index 831eb833..9747f843 100644 --- a/scripts/dev/setup-leak-gate.ps1 +++ b/scripts/dev/setup-leak-gate.ps1 @@ -34,9 +34,23 @@ param( ) $ErrorActionPreference = 'Stop' -$repo = (& git rev-parse --show-toplevel 2>$null) -if (-not $repo) { throw 'Not inside a git checkout.' } + +# Repo root = parent of scripts\ = parent of this script's dir (scripts\dev). Same form postgres.ps1 and +# sqlserver.ps1 in this directory already use. +# +# NOT `git rev-parse --show-toplevel`: that resolves against the CURRENT DIRECTORY, not against the path +# this script was handed. Invoked by absolute -File path from another worktree -- the ordinary shape on a +# clone carrying dozens of them -- it armed the CALLER's checkout and printed CONFIGURED about that one, +# while the checkout the operator named kept no token source and went on failing closed (BACKLOG #1063). +# An absolute -File invocation is naming the checkout to act on; it must not then consult a different one. +# -From names the token SOURCE, not the checkout, so it never covered this. +$repo = Split-Path -Parent (Split-Path -Parent $PSScriptRoot) $secDir = Join-Path $repo 'scripts/security' +# ASSERT what the derivation only IMPLIES. Discovering a wrong root later, as a confusing scanner +# failure, is how this class stays invisible; new.ps1 takes the same posture after deriving its path. +if (-not (Test-Path -LiteralPath $secDir)) { + throw "Derived repo root has no scripts/security -- this script must live in /scripts/dev/: $repo" +} $local = Join-Path $secDir 'scan-tokens.local.txt' $example = Join-Path $secDir 'scan-tokens.local.txt.example' $scanner = Join-Path $secDir 'scan_forbidden.py' diff --git a/tee/anon/__init__.py b/tee/anon/__init__.py index 51bd77a4..df8cdb07 100644 --- a/tee/anon/__init__.py +++ b/tee/anon/__init__.py @@ -11,18 +11,21 @@ Public surface (same shape as the engine's): * :func:`anonymize` β€” de-identify one HL7 message. -* :func:`anonymize_checked` β€” :func:`anonymize` + a fail-closed :func:`leak_check`; raises - :class:`LeakError` (token categories only) on any surviving token. -* :func:`leak_check` β€” forbidden-token hits via the vendored token authority. +* :func:`anonymize_checked` β€” :func:`anonymize` + a fail-closed :func:`leak_report`; raises + :class:`LeakError` (token categories + PHI shapes/addresses only) on any surviving token or a + structural PHI shape in a field no rule mapped. +* :func:`leak_check` / :func:`leak_report` β€” token hits + structural PHI-shape detection over the + unmapped fields + the unmapped-field coverage report (vendored twin of the engine's; BACKLOG #331). """ from __future__ import annotations +from collections.abc import Callable from pathlib import Path from .hl7 import anonymize_message from .keying import Keyer -from .leak import leak_check +from .leak import LeakReport, coverage_clause, leak_check, leak_report from .rules import DEFAULT_RULES, AnonError, FieldRule, RuleError, SurrogateKind, load_rules __all__ = [ @@ -31,11 +34,13 @@ "FieldRule", "Keyer", "LeakError", + "LeakReport", "RuleError", "SurrogateKind", "anonymize", "anonymize_checked", "leak_check", + "leak_report", "load_rules", ] @@ -67,14 +72,29 @@ def anonymize_checked( salt: str, overlay: Path | None = None, rules: tuple[FieldRule, ...] | None = None, + require_live_denylist: bool = False, + on_report: Callable[[LeakReport], None] | None = None, ) -> str: - """:func:`anonymize`, then a fail-closed :func:`leak_check`; raise :class:`LeakError` on any hit.""" - output = anonymize(raw, salt=salt, overlay=overlay, rules=rules) - hits = leak_check(output) - if hits: + """:func:`anonymize`, then a fail-closed :func:`leak_report`; raise :class:`LeakError` on any hit. + + Two-layered like the engine's (BACKLOG #331): the known-token denylist plus high-precision + structural PHI-shape detectors over the fields no rule matched. ``require_live_denylist`` (default + off) makes a non-live token source a refusal cause; ``on_report`` receives the :class:`LeakReport` + on both paths. The error names token categories and field shapes/addresses only, never a value. + """ + effective = rules if rules is not None else load_rules(overlay) + output = anonymize(raw, salt=salt, rules=effective) + report = leak_report(output, rules=effective) + if on_report is not None: + on_report(report) + causes = list(report.hits) + if require_live_denylist and report.token_floor_reason is not None: + causes.append(f"denylist not live: {report.token_floor_reason}") + if causes: raise LeakError( "anonymized output still carries forbidden token(s): " - + "; ".join(sorted(set(hits))) + + "; ".join(sorted(set(causes))) + " β€” refusing to emit (fail closed). Extend the rule map for the missed field(s)." + + coverage_clause(report) ) return output diff --git a/tee/anon/leak.py b/tee/anon/leak.py index 0b7c5568..fc041a85 100644 --- a/tee/anon/leak.py +++ b/tee/anon/leak.py @@ -14,17 +14,21 @@ 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). +Returns **reasons only** (never the matched text), and the token denylist is the fail-closed +*backstop*, not the primary control: it catches known *tokens*, not structural PHI (ADR 0030 Β§5). The +structural PHI-shape detectors + unmapped-field coverage report below (BACKLOG #331) close that gap β€” +they are held byte-for-byte identical with the engine copy so the two agree on every input. """ from __future__ import annotations import importlib.util import re +from dataclasses import dataclass from pathlib import Path -from .surrogates import message_has_site_code +from .rules import FieldRule +from .surrogates import Seps, message_has_site_code, read_message_seps def _load_publish_guard(_start: Path | None = None) -> object | None: @@ -91,11 +95,165 @@ def scan_text(text: str, *, include_estate: bool = False) -> list[str]: return reasons -def leak_check(text: str) -> list[str]: - """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") +# --- structural PHI-shape detection over UNMAPPED fields (BACKLOG #331) ---------------------------- +# EVERYTHING from here to the end of this block is held BYTE-IDENTICAL with tee/anon/leak.py (the +# structural walk depends only on read_message_seps, which the parity test pins byte-for-byte). The +# detectors are deliberately high-precision β€” a broad digit-run search mass-false-positives on HL7 +# bodies dense with dates/order-numbers/set-ids (ADR 0030 Β§5), so the coverage report, not an +# aggressive heuristic, is the catch-all for shapes these cannot safely flag. + +#: A dashed US SSN ``NNN-NN-NNNN`` not embedded in a longer digit run. +_SSN_DASHED: re.Pattern[str] = re.compile(r"(? list[tuple[str, str]]: + """Every ``(address, value)`` in ``text`` whose whole-field ``SEG-i`` address is **not** in + ``mapped_paths`` and whose value is non-empty β€” the fields the rule map never touched. + + The MSH control header is skipped whole: its field indexing is off-by-one (MSH-N sits at + split-index N-1) and it carries routing/site data the field-anchored site-code pass already + covers, not patient PHI. ``mapped_paths`` is occurrence-agnostic (a rule applies to every + occurrence of its segment), so the address is the bare ``SEG-i``. Returns ``[]`` when the message + has no parseable MSH (there is no field separator to split on). + """ + parsed = read_message_seps(text) + if parsed is None: + return [] + _seps, field_sep = parsed + out: list[tuple[str, str]] = [] + for seg in text.replace("\r\n", "\r").replace("\n", "\r").split("\r"): + if not seg: + continue + fields = seg.split(field_sep) + if fields[0].upper() == "MSH": + continue + seg_id = fields[0] + for i in range(1, len(fields)): + value = fields[i] + if not value: + continue + address = f"{seg_id}-{i}" + if address in mapped_paths: + continue + out.append((address, value)) + return out + + +def _has_mrn_typed_identifier(value: str, seps: Seps) -> bool: + """True if any repetition of ``value`` is a CX with a non-empty id (component 1) and a whole + ``MR``/``MRN`` id-type component β€” an unmapped medical-record number by HL7 structure, far more + precise than a bare digit-run heuristic.""" + for rep in value.split(seps.repetition): + comps = rep.split(seps.component) + if comps[0] and any(comp.upper() in _MRN_TYPES for comp in comps): + return True + return False + + +def _structural_reasons(value: str, seps: Seps) -> list[str]: + """PHI-safe shape labels for one unmapped field value β€” the SHAPE only, never the value.""" + reasons: list[str] = [] + if _SSN_DASHED.search(value): + reasons.append("unmapped SSN-shaped value") + if _PHONE_DASHED.search(value) or _PHONE_PAREN.search(value): + reasons.append("unmapped phone-shaped value") + if _has_mrn_typed_identifier(value, seps): + reasons.append("unmapped MRN-typed identifier") + return reasons + + +def structural_phi_hits(text: str, mapped_paths: set[str]) -> list[str]: + """Structural PHI-shape hits over the fields no rule matched β€” reasons name the shape + field + ADDRESS only (e.g. ``"unmapped SSN-shaped value in GT1-16"``), never the offending value, so the + result is safe to raise/log. Empty when the message has no parseable MSH.""" + parsed = read_message_seps(text) + if parsed is None: + return [] + seps, _field_sep = parsed + hits: list[str] = [] + for address, value in unmapped_field_values(text, mapped_paths): + hits.extend(f"{reason} in {address}" for reason in _structural_reasons(value, seps)) return hits + + +@dataclass(frozen=True) +class LeakReport: + """The full result of a leak-check pass β€” the token hits that decide the fail-closed outcome plus + the coverage context that makes the check's reach legible (all PHI-safe: addresses and reasons, + never a field value). + + * ``hits`` β€” every leak reason (token/IP/site + structural); non-empty means refuse. + * ``unmapped_fields`` β€” the addresses present but matched by no rule (the coverage report). + * ``structural_hits`` β€” the subset of ``hits`` from the structural PHI-shape detectors. + * ``token_tables_live`` β€” whether the denylist tables loaded from a real token source. + * ``token_floor_reason`` β€” why the denylist is not trustworthy, or ``None`` if it is. + """ + + hits: list[str] + unmapped_fields: tuple[str, ...] + structural_hits: list[str] + token_tables_live: bool + token_floor_reason: str | None + + +def leak_report(text: str, *, rules: tuple[FieldRule, ...] | None = None) -> LeakReport: + """The full :class:`LeakReport` for ``text`` using the tee's vendored token authority. + + Behaviourally parallel to the engine's :func:`messagefoundry.anon.leak.leak_report`: the token + hits come from the tee's local :func:`scan_text` (and the field-anchored site-code check) rather + than the engine's ``_scanner()`` delegate, but the structural walk, coverage report, and + token-floor signal are the byte-identical shared logic above. Structural detection engages **only + when ``rules`` is supplied**; a bare-string call is the legacy token-only behaviour. + """ + token_hits = scan_text(text, include_estate=True) + if message_has_site_code(text): + token_hits.append("site-code pattern") + if rules is None: + unmapped: tuple[str, ...] = () + structural: list[str] = [] + else: + mapped_paths = {r.path for r in rules} + unmapped = tuple(sorted({addr for addr, _ in unmapped_field_values(text, mapped_paths)})) + structural = structural_phi_hits(text, mapped_paths) + token_tables_live: bool + token_floor_reason: str | None + if _GUARD is not None: + token_tables_live = bool(_GUARD.TOKENS_PRESENT) # type: ignore[attr-defined] + token_floor_reason = _GUARD.token_floor_failure() # type: ignore[attr-defined] + else: + token_tables_live = False + # nosec B105: a human-readable diagnostic string, not a credential β€” bandit's + # hardcoded-password heuristic fires only because the name contains "token". + token_floor_reason = "no publish guard reachable β€” refusing to run structural-only" # nosec B105 + return LeakReport( + hits=token_hits + structural, + unmapped_fields=unmapped, + structural_hits=structural, + token_tables_live=token_tables_live, + token_floor_reason=token_floor_reason, + ) + + +def leak_check(text: str, *, rules: tuple[FieldRule, ...] | None = None) -> list[str]: + """Forbidden-token + structural PHI hits in ``text`` (empty list = clean) β€” a thin wrapper over + :func:`leak_report`. ``rules`` scopes the structural PHI-shape detectors to the fields no rule + matched; omitting it (a bare-string call) runs the legacy token-only check. + """ + return leak_report(text, rules=rules).hits + + +def coverage_clause(report: LeakReport) -> str: + """A PHI-safe suffix for a fail-closed message naming what the check reached β€” the count and + ADDRESSES of the unmapped fields (never their values) and whether the denylist tables were live.""" + live = "yes" if report.token_tables_live else "no" + fields = ", ".join(report.unmapped_fields) if report.unmapped_fields else "none" + return ( + f" (checked {len(report.unmapped_fields)} unmapped field(s): {fields}; " + f"denylist tables live: {live})" + ) diff --git a/tests/test_anon_core.py b/tests/test_anon_core.py index 4a3a21e8..dc45e48f 100644 --- a/tests/test_anon_core.py +++ b/tests/test_anon_core.py @@ -22,6 +22,7 @@ anonymize_checked, leak, leak_check, + leak_report, load_rules, ) from messagefoundry.anon.surrogates import Seps, scrub_site_codes, surrogate_field @@ -256,6 +257,139 @@ def test_anonymize_checked_fails_closed_and_is_phi_safe(monkeypatch: pytest.Monk assert "DOE" not in message and "999" not in message # never echoes the body +# --- structural PHI detection on UNMAPPED fields (BACKLOG #331) ------------------------------------ +# The known-token denylist cannot see a real MRN/SSN in a field the rule map never mapped (a real MRN +# is not a denylisted string). These exercise the structural backstop over the UNMAPPED fields. All +# values are SYNTHETIC PHI SHAPES (fake, reserved-fictional, or component-structured) β€” never a real +# value β€” and each detector is falsified in the lane report. `DST` is a non-standard segment carrying +# no default rule, so DST-2/3 are the unmapped surface (the f3c6d348 blind-map case in miniature). + +_SSN_MSG = _msg( + r"MSH|^~\&|SAPP|SFAC|RAPP|RFAC|20260101120000||ADT^A01|M1|P|2.5.1", + "PID|1||1^^^H^MR||X^Y", + "DST|1|123-45-6789", # DST-2: unmapped field carrying a synthetic dashed SSN +) + + +@_NO_SCANNER +def test_leak_check_catches_unmapped_ssn() -> None: + """A synthetic dashed SSN in an unmapped field (DST-2) is caught and fails closed. + + Falsified: deleting `_SSN_DASHED` from leak.py's structural set made leak_check() return [] and + anonymize_checked() emit the dataset clean (RED), then restored. + """ + hits = leak_check(_SSN_MSG, rules=DEFAULT_RULES) + assert any("SSN" in h for h in hits), hits + with pytest.raises(LeakError): + anonymize_checked(_SSN_MSG, salt=_SALT) + + +@_NO_SCANNER +def test_leak_check_catches_unmapped_phone() -> None: + """Synthetic punctuated NANP numbers (reserved-fictional 555-01XX) in unmapped fields are caught, + both dashed and parenthesised. + + Falsified: removing the two phone detectors let the dataset slip through clean (RED), then restored. + """ + msg = _msg( + r"MSH|^~\&|A|B|C|D|20260101||ADT^A01|M1|P|2.5.1", + "PID|1||1^^^H^MR||X^Y", + "DST|1|202-555-0188|(202) 555-0188", # DST-2 dashed, DST-3 parenthesised + ) + hits = leak_check(msg, rules=DEFAULT_RULES) + assert any("phone" in h for h in hits), hits + assert any("DST-2" in h for h in hits) and any("DST-3" in h for h in hits), hits + + +@_NO_SCANNER +def test_leak_check_catches_unmapped_mrn() -> None: + """A CX id typed `MR` in an unmapped field (PID-2, absent from DEFAULT_RULES) is caught by HL7 + structure, and the raw id never surfaces in the reason or the LeakError (PHI-safe). + + Falsified: removing the MR/MRN component detector let the unmapped MRN pass clean (RED), then + restored β€” confirming the CX id-type signal, not a digit heuristic, is doing the work. + """ + msg = _msg( + r"MSH|^~\&|A|B|C|D|20260101||ADT^A01|M1|P|2.5.1", + "PID|1|98765^^^HOSP^MR||X^Y", # PID-2: unmapped CX, id-typed MR + ) + hits = leak_check(msg, rules=DEFAULT_RULES) + assert any("MRN" in h and "PID-2" in h for h in hits), hits + assert all("98765" not in h for h in hits) # names the shape + address, never the id + with pytest.raises(LeakError) as exc: + anonymize_checked(msg, salt=_SALT) + assert "98765" not in str(exc.value) + + +@_NO_SCANNER +def test_coverage_report_lists_unmapped_fields() -> None: + """The coverage report enumerates present-but-unmapped fields (address only) β€” the batch_18 + regression: a field nobody mapped is now visible, not silent. The fail-path LeakError carries the + value-free coverage clause. + + Falsified: stubbing `unmapped_field_values` to yield nothing emptied `.unmapped_fields` (RED), + then restored. + """ + benign = _msg( + r"MSH|^~\&|A|B|C|D|20260101120000||ADT^A01|M1|P|2.5.1", + "PID|1||1^^^H^MR||X^Y", + "DST|1|freeform", # DST-2: unmapped but benign β€” enumerated, not flagged + ) + report = leak_report(benign, rules=DEFAULT_RULES) + assert "DST-2" in report.unmapped_fields + assert report.structural_hits == [] # benign value β†’ enumerated only, no shape hit + with pytest.raises(LeakError) as exc: + anonymize_checked(_SSN_MSG, salt=_SALT) + text = str(exc.value) + assert "checked" in text and "unmapped field" in text and "DST-2" in text + + +@_NO_SCANNER +def test_false_positive_guard_benign_unmapped_fields() -> None: + """Unmapped fields dense with dates/coded-values/order-numbers (the mass-false-positive surface + ADR 0030 warns of) must NOT trip the check β€” why the bare-digit DOB/SSN heuristics were rejected. + + Falsified: broadening `_SSN_DASHED` to any 8+ digit run tripped the 14-digit EVN timestamp (RED), + then restored. + """ + benign = _msg( + r"MSH|^~\&|A|B|C|D|20260101120000||ADT^A01|M1|P|2.5.1", + "EVN|A01|20260101120000", # 14-digit timestamp + "OBX|1|NM|8480-6^Systolic^LN||128|mm[Hg]", # coded observation id + "ORC|NW|1000000042", # unmapped order-number run + "PID|1||1^^^H^MR||X^Y", + ) + assert leak_check(benign, rules=DEFAULT_RULES) == [] + assert anonymize_checked(benign, salt=_SALT) # clean β†’ returns, no raise + + +@_NO_SCANNER +def test_token_floor_surfaced_when_tables_empty(monkeypatch: pytest.MonkeyPatch) -> None: + """An empty token load is no longer a SILENT green (#331): the report records it and the strict + lever refuses on it β€” while the default keeps CI/OSS/fork runs (which have no token source) green, + the structural detectors being the live backstop. + + The empty-token state is forced deterministically (this dev checkout has a token source; CI does + not) by patching the loaded scanner's `TOKENS_PRESENT`. Falsified: stubbing `token_floor_failure` + to return None made `.token_floor_reason` None and the strict path stop refusing (RED), restored. + """ + monkeypatch.setattr(leak._scanner(), "TOKENS_PRESENT", False) + clean = _msg( + r"MSH|^~\&|A|B|C|D|20260101120000||ADT^A01|M1|P|2.5.1", + "PID|1||1^^^H^MR||X^Y", + ) + report = leak_report(clean, rules=DEFAULT_RULES) + assert report.token_tables_live is False + assert report.token_floor_reason is not None + # the DEFAULT decision does NOT refuse on empty tokens alone (structural detectors are the backstop) + assert anonymize_checked(clean, salt=_SALT) + # the strict lever DOES refuse, naming the floor reason but no field value + with pytest.raises(LeakError) as exc: + anonymize_checked(clean, salt=_SALT, require_live_denylist=True) + text = str(exc.value) + assert "denylist not live" in text and "fail closed" in text + + 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", diff --git a/tests/test_anon_parity.py b/tests/test_anon_parity.py index 0ddac6e7..d5d05c05 100644 --- a/tests/test_anon_parity.py +++ b/tests/test_anon_parity.py @@ -11,7 +11,9 @@ import pytest +from messagefoundry.anon import DEFAULT_RULES from messagefoundry.anon import anonymize as engine_anonymize +from messagefoundry.anon import leak as engine_leak from messagefoundry.generators import ( _core, _hl7data, @@ -111,6 +113,47 @@ def test_leak_tables_are_sourced_from_the_guard_when_present() -> None: assert tee_leak.FORBIDDEN and tee_leak.ESTATE_TOKENS # type: ignore[attr-defined] +# Synthetic-PHI-shape inputs (never a real value) whose UNMAPPED fields carry SSN/phone/MRN shapes, +# plus a benign case β€” the structural walk + coverage report + token-floor signal must agree between +# the engine (delegating to _scanner()) and the tee (reimplementing over _GUARD). The structural block +# is copied byte-for-byte between the two leak.py files; this is the divergence guard for it. +_LEAK_PARITY_INPUTS = [ + "MSH|^~\\&|A|B|C|D|20260101||ADT^A01|M1|P|2.5.1\rPID|1||1^^^H^MR||X^Y\rDST|1|123-45-6789", + "MSH|^~\\&|A|B|C|D|20260101||ADT^A01|M1|P|2.5.1\rPID|1||1^^^H^MR||X^Y" + "\rDST|1|202-555-0188|(202) 555-0188", + "MSH|^~\\&|A|B|C|D|20260101||ADT^A01|M1|P|2.5.1\rPID|1|98765^^^HOSP^MR||X^Y", + "MSH|^~\\&|A|B|C|D|20260101120000||ADT^A01|M1|P|2.5.1" + "\rEVN|A01|20260101120000\rOBX|1|NM|8480-6^Systolic^LN||128|mm[Hg]", +] + + +def _structural_fields(report: object) -> tuple[object, ...]: + """The STRUCTURAL/coverage fields of a LeakReport β€” the byte-copied #331 logic this guard pins. + + ``token_tables_live`` / ``token_floor_reason`` are intentionally excluded: they are derived from + the token authority, not the structural walk, and the engine's ``_scanner()`` (lazily lru_cached + on first call) and the tee's ``_GUARD`` (loaded at import) can snapshot the token source at + different times within a full-suite run β€” a fixture that patches ``MEFOR_FORBIDDEN_TOKENS`` before + the first ``_scanner()`` call poisons its token-floor view for the session. Those fields' cross- + copy agreement is already pinned by ``test_leak_token_table_matches_publish_guard``; here we guard + the detectors + coverage report, which are pure functions of (text, rules). + """ + return (report.hits, report.unmapped_fields, report.structural_hits) # type: ignore[attr-defined] + + +def test_leak_check_and_report_engine_equals_tee() -> None: + for msg in _LEAK_PARITY_INPUTS: + eng_hits = engine_leak.leak_check(msg, rules=DEFAULT_RULES) + tee_hits = tee_leak.leak_check(msg, rules=DEFAULT_RULES) + assert eng_hits == tee_hits, f"leak_check diverged on {msg!r}: {eng_hits!r} != {tee_hits!r}" + eng_report = _structural_fields(engine_leak.leak_report(msg, rules=DEFAULT_RULES)) + tee_report = _structural_fields(tee_leak.leak_report(msg, rules=DEFAULT_RULES)) + assert eng_report == tee_report, ( + f"leak_report structural fields diverged on {msg!r}:" + f"\n ENG {eng_report!r}\n TEE {tee_report!r}" + ) + + def test_adversarial_inputs_engine_output_equals_tee_output() -> None: for msg in _ADVERSARIAL: engine = engine_anonymize(msg, salt=_SALT) diff --git a/tests/test_asvs_scorecard.py b/tests/test_asvs_scorecard.py index 0c3b15fb..58fd0bc3 100644 --- a/tests/test_asvs_scorecard.py +++ b/tests/test_asvs_scorecard.py @@ -23,6 +23,7 @@ Cell, Findings, ScorecardError, + _copy_scratch, check_absences, check_anchors, check_completeness, @@ -31,6 +32,8 @@ count, load_corpus, load_scorecard, + main, + prove_absences, render_current, verify, ) @@ -675,3 +678,383 @@ def test_completeness_accepts_a_decided_cell_evidenced_only_by_an_absence_claim( Cell(id="2.1.1", level=3, verdict="unverified"), ] assert not [p for p in check_completeness(cells, CORPUS) if "carry NO anchor" in p] + + +# --- --prove-absences: a mutation that matches is not a mutation that BITES (#1006) --------------- +# +# check_absences proves a mutation's pattern fires on it; it never applies the mutation. So a +# well-formed reintroduction that would change nothing observable passes every check. prove_absences +# closes that hole by EXECUTING the claim: mutate a scratch copy, run the named observable, require it +# to go red -- and fail closed on any exit code that is not an honest test failure. Every fixture tree +# lives in tmp_path (never in the scanned packages), and the mutation is applied only to a scratch +# copy in a system TemporaryDirectory, so nothing here touches the committed corpus or tmp_path. + +_SCANNER = "def scan(p):\n return 'clean'\n" +_OBS_TEST = "from scanner import scan\n\n\ndef test_clean():\n assert scan('x') == 'clean'\n" + + +def _module(tmp_path: Path, name: str, body: str) -> Path: + """Write a code module fixture into tmp_path (the code the reintroduction lands in).""" + p = tmp_path / name + p.write_text(body, encoding="utf-8") + return p + + +def _obs_test(tmp_path: Path, name: str, body: str) -> Path: + """Write an observable pytest module fixture into tmp_path.""" + p = tmp_path / name + p.write_text(body, encoding="utf-8") + return p + + +def _live_claim(mutation: str, mutation_path: str, observable: str) -> Cell: + # pattern/positive_control are irrelevant to prove_absences (they drive check_absences); supply + # harmless values so the required fields are present. + return Cell( + id="1.1.1", + level=1, + verdict="fail", + absence=( + Absence( + pattern="x", + positive_control="y", + mutation=mutation, + mutation_path=mutation_path, + observable=observable, + ), + ), + ) + + +def test_prove_absences_proves_a_claim_when_the_mutation_reddens_its_observable( + tmp_path: Path, +) -> None: + """The positive half: a mutation that shadows `scan` reddens the observable, so the claim BITES. + + Falsified by making `_apply_mutation` a no-op: the observable stays green, the mode reports + UNPROVEN, and the `.ok`/`proved_absences == 1` assertions go RED. Restored. + """ + _module(tmp_path, "scanner.py", _SCANNER) + _obs_test(tmp_path, "test_scanner.py", _OBS_TEST) + claim = _live_claim( + 'def scan(p): return "infected"', "scanner.py", "test_scanner.py::test_clean" + ) + findings = prove_absences([claim], tmp_path) + assert findings.ok, findings.problems + assert findings.proved_absences == 1 + + +def test_prove_absences_fails_when_the_mutation_reddens_nothing(tmp_path: Path) -> None: + """The negative control the brief requires: a mutation to a file the observable never imports + reddens nothing, so the claim is UNPROVEN and the mode FAILS. + + Falsified by making the mode accept a mutated exit==0 as a pass (dropping the exit==1 + requirement): the non-biting claim then reports ok, and `not findings.ok` goes RED -- proving the + mode can actually fail. Restored. + """ + _module(tmp_path, "scanner.py", _SCANNER) + _module( + tmp_path, "unrelated.py", "VALUE = 1\n" + ) # exists, but the observable does not import it + _obs_test(tmp_path, "test_scanner.py", _OBS_TEST) + claim = _live_claim( + 'def scan(p): return "infected"', "unrelated.py", "test_scanner.py::test_clean" + ) + findings = prove_absences([claim], tmp_path) + assert not findings.ok + assert any("UNPROVEN" in p for p in findings.problems), findings.problems + assert findings.proved_absences == 0 + + +def test_prove_absences_fails_closed_when_the_observable_is_already_red(tmp_path: Path) -> None: + """An observable that fails on the pristine tree cannot attribute its red to the mutation. + + Falsified by removing the baseline-green check: the already-red observable stays red under the + mutation, is miscounted as `proved`, and this test's `not findings.ok` goes RED. Restored. + """ + _module(tmp_path, "scanner.py", _SCANNER) + _obs_test( + tmp_path, + "test_scanner.py", + "from scanner import scan\n\n\ndef test_clean():\n assert scan('x') == 'DIFFERENT'\n", + ) + claim = _live_claim( + 'def scan(p): return "infected"', "scanner.py", "test_scanner.py::test_clean" + ) + findings = prove_absences([claim], tmp_path) + assert not findings.ok + assert any("PROVE-ERROR" in p and "baseline" in p for p in findings.problems), findings.problems + assert findings.proved_absences == 0 + + +def test_prove_absences_fails_closed_when_the_mutation_errors_instead_of_failing( + tmp_path: Path, +) -> None: + """A mutation that breaks IMPORT of the observable's module errors at collection (pytest exit 4), + not a test failure (exit 1). A collection/usage error must NEVER count as the control biting -- + otherwise a typo'd node or an import-breaking mutation rebuilds the exact vacuity being fixed. + + Falsified by changing the mutated-run requirement from `exit == 1` to `exit != 0`: the exit-4 + collection error then masquerades as `proved`, and this test's `not findings.ok` goes RED. + Restored. + """ + _module(tmp_path, "scanner.py", _SCANNER) + _obs_test(tmp_path, "test_scanner.py", _OBS_TEST) + # Appended at module level, this raises when `scanner` is imported -> collection error, not a + # failing assertion. + claim = _live_claim( + 'raise RuntimeError("reintroduced")', "scanner.py", "test_scanner.py::test_clean" + ) + findings = prove_absences([claim], tmp_path) + assert not findings.ok + assert any("PROVE-ERROR" in p and "errored" in p for p in findings.problems), findings.problems + assert findings.proved_absences == 0 + + +def test_prove_absences_static_backstop_flags_a_raise_into_a_swallowing_file( + tmp_path: Path, +) -> None: + """With no observable, the static backstop flags a `raise` landing in a file whose try/except + swallows (bare/Exception, log-only body). A screen, not a proof -- but it fails the mode. + + Falsified by forcing `_landing_swallows` to return False: the swallow is not flagged, `findings.ok` + becomes True, and this test's `not findings.ok` goes RED. Restored. + """ + _module( + tmp_path, + "caller.py", + "import logging\n\nlog = logging.getLogger(__name__)\n\n\n" + "def reconcile():\n try:\n work()\n except Exception:\n" + " log.exception('reconcile failed')\n", + ) + claim = Cell( + id="13.3.4", + level=3, + verdict="fail", + absence=( + Absence( + pattern="x", + positive_control="y", + mutation='raise RuntimeError("reintroduced")', + mutation_path="caller.py", + observable="", # no observable -> static backstop + ), + ), + ) + findings = prove_absences([claim], tmp_path) + assert not findings.ok + assert any("SUSPECT" in p and "swallow" in p for p in findings.problems), findings.problems + assert findings.static_screened == 1 + + +def test_prove_absences_static_backstop_passes_a_non_swallowing_file(tmp_path: Path) -> None: + """REACH control: the static backstop must NOT flag a `raise` into a handler that re-raises -- + proving the heuristic reads the handler body, not merely the presence of a try/except. + + Falsified by forcing `_landing_swallows` to return True: the re-raising file is flagged, and this + test's `assert findings.ok` goes RED. Restored. + """ + _module( + tmp_path, + "plain.py", + "def reconcile():\n try:\n work()\n except Exception:\n raise\n", + ) + claim = Cell( + id="13.3.4", + level=3, + verdict="fail", + absence=( + Absence( + pattern="x", + positive_control="y", + mutation='raise RuntimeError("reintroduced")', + mutation_path="plain.py", + observable="", + ), + ), + ) + findings = prove_absences([claim], tmp_path) + assert findings.ok, findings.problems + assert findings.static_screened == 1 + assert not any("SUSPECT" in p for p in findings.problems) + + +def test_prove_absences_leaves_the_root_tree_untouched(tmp_path: Path) -> None: + """The mode must run OUT of the tracked tree: mutation lands only on the scratch copy. + + Falsified by pointing `_apply_mutation` at `root` instead of the scratch copy: root's scanner.py + changes, `after == before` goes RED (and the claim also drops to UNPROVEN). Restored. + """ + _module(tmp_path, "scanner.py", _SCANNER) + _obs_test(tmp_path, "test_scanner.py", _OBS_TEST) + before = {p: p.read_bytes() for p in sorted(tmp_path.rglob("*")) if p.is_file()} + claim = _live_claim( + 'def scan(p): return "infected"', "scanner.py", "test_scanner.py::test_clean" + ) + findings = prove_absences([claim], tmp_path) + assert findings.proved_absences == 1, findings.problems + after = {p: p.read_bytes() for p in sorted(tmp_path.rglob("*")) if p.is_file()} + assert after == before + + +def test_load_reads_optional_mutation_path_and_observable_and_omitting_them_still_loads( + tmp_path: Path, +) -> None: + """Round-trip: the two new fields load when present, and OMITTING them still loads (vault-safety -- + the ~81 existing absence claims carry neither and must stay loadable). + + Falsified by dropping the `.get` wiring in load_scorecard (hardcoding ``mutation_path=""``): half + (a) then reads "" and its assertion goes RED, while half (b) stays green -- proving the round-trip + is actually asserted. Restored. + """ + with_fields = tmp_path / "with.toml" + with_fields.write_text( + '[[cell]]\nid = "1.1.1"\nlevel = 1\nverdict = "fail"\n' + " [[cell.absence]]\n" + ' pattern = "clamd"\n' + ' positive_control = "ScanRejected"\n' + ' mutation = "import clamd"\n' + ' mutation_path = "messagefoundry/scan.py"\n' + ' observable = "tests/test_scan.py::test_rejects"\n', + encoding="utf-8", + ) + a = load_scorecard(with_fields)[0].absence[0] + assert a.mutation_path == "messagefoundry/scan.py" + assert a.observable == "tests/test_scan.py::test_rejects" + + without_fields = tmp_path / "without.toml" + without_fields.write_text( + '[[cell]]\nid = "1.1.2"\nlevel = 2\nverdict = "fail"\n' + " [[cell.absence]]\n" + ' pattern = "clamd"\n' + ' positive_control = "ScanRejected"\n' + ' mutation = "import clamd"\n', + encoding="utf-8", + ) + b = load_scorecard(without_fields)[0].absence[0] + assert b.mutation_path == "" and b.observable == "" + + +def test_prove_absences_refuses_a_mutation_path_that_escapes_the_scratch_tree( + tmp_path: Path, +) -> None: + """`mutation_path` is authored data. An absolute path or a `..` escape would let the mutation land + OUTSIDE the scratch copy (defeating the 'never touches root' guarantee), so the mode refuses it as + a PROVE-ERROR before applying anything -- it never counts as a proof. + + Falsified by making `_is_within_tree` return True unconditionally: the `..` path is no longer + refused, the run falls through to the is_file probe with a different message, and this test's + `any("repo-relative" in p ...)` assertion goes RED. Restored. + """ + _module(tmp_path, "scanner.py", _SCANNER) + _obs_test(tmp_path, "test_scanner.py", _OBS_TEST) + claim = _live_claim( + 'def scan(p): return "infected"', + "../escape.py", # a `..` that would climb out of the scratch copy + "test_scanner.py::test_clean", + ) + findings = prove_absences([claim], tmp_path) + assert not findings.ok + assert any("PROVE-ERROR" in p and "repo-relative" in p for p in findings.problems), ( + findings.problems + ) + assert findings.proved_absences == 0 + + +def test_copy_scratch_excludes_secrets_store_and_vault_posture(tmp_path: Path) -> None: + """The scratch copy the vault mutation-run reads must never carry secrets, the local store, or the + vault posture tree -- CLAUDE.md Β§9 forbids this module reading them at all. `_copy_scratch` skips + `.env*`, `*.db`(+WAL sidecars), and `docs/security`, while ordinary sources are still copied. + + Falsified by reverting `_scratch_ignore` to the bare VCS/venv/cache patterns: the `.env`, `*.db` + and `docs/security` fixtures are then copied into the scratch dir and every `not (dest/...).exists()` + assertion goes RED, while the `keep.py` assertion stays green. Restored. + """ + (tmp_path / ".env").write_text("EXAMPLE_PLACEHOLDER=not-a-secret\n", encoding="utf-8") + (tmp_path / "local.db").write_text("binary-store\n", encoding="utf-8") + (tmp_path / "local.db-wal").write_text("wal\n", encoding="utf-8") + (tmp_path / "docs" / "security").mkdir(parents=True) + (tmp_path / "docs" / "security" / "posture.toml").write_text("real = true\n", encoding="utf-8") + (tmp_path / "docs" / "PUBLIC.md").write_text("# public\n", encoding="utf-8") + (tmp_path / "keep.py").write_text("KEEP = 1\n", encoding="utf-8") + + dest = tmp_path.parent / "scratch_out" + _copy_scratch(tmp_path, dest) + + assert not (dest / ".env").exists() + assert not (dest / "local.db").exists() + assert not (dest / "local.db-wal").exists() + assert not (dest / "docs" / "security").exists() + # ordinary sources and other docs survive the copy + assert (dest / "keep.py").read_text(encoding="utf-8") == "KEEP = 1\n" + assert (dest / "docs" / "PUBLIC.md").exists() + + +def _biting_scorecard(sc: Path, mutation_path: str) -> None: + """Write a one-claim scorecard whose live absence claim points at `mutation_path`.""" + sc.write_text( + '[[cell]]\nid = "1.1.1"\nlevel = 1\nverdict = "fail"\n' + " [[cell.absence]]\n" + ' pattern = "x"\n' + ' positive_control = "y"\n' + " mutation = 'def scan(p): return \"infected\"'\n" + f' mutation_path = "{mutation_path}"\n' + ' observable = "test_scanner.py::test_clean"\n', + encoding="utf-8", + ) + + +def test_main_prove_absences_returns_0_on_a_biting_claim(tmp_path: Path) -> None: + """The CLI contract CI depends on: `--prove-absences` exits 0 when every claim's mutation reddens + its observable. Exercises `main` -> `_run_prove_absences` end to end, not just `prove_absences`. + + Falsified by changing `_run_prove_absences`'s `return 0 if findings.ok else 1` to `return 1`: this + test's `rc == 0` goes RED while the non-biting test below stays green. Restored. + """ + tree = tmp_path / "tree" + tree.mkdir() + _module(tree, "scanner.py", _SCANNER) + _obs_test(tree, "test_scanner.py", _OBS_TEST) + sc = tmp_path / "sc.toml" + _biting_scorecard(sc, "scanner.py") + rc = main(["--scorecard", str(sc), "--root", str(tree), "--prove-absences"]) + assert rc == 0 + + +def test_main_prove_absences_returns_1_on_a_nonbiting_claim(tmp_path: Path) -> None: + """The other half of the contract: `--prove-absences` exits 1 when a claim is UNPROVEN (its + mutation reddens nothing). Proves the CLI's non-zero failure path, not only the library's. + + Falsified by changing `_run_prove_absences`'s `return 0 if findings.ok else 1` to `return 0`: + this test's `rc == 1` goes RED while the biting test above stays green. Restored. + """ + tree = tmp_path / "tree" + tree.mkdir() + _module(tree, "scanner.py", _SCANNER) + _module(tree, "unrelated.py", "VALUE = 1\n") # present, but the observable never imports it + _obs_test(tree, "test_scanner.py", _OBS_TEST) + sc = tmp_path / "sc.toml" + _biting_scorecard(sc, "unrelated.py") + rc = main(["--scorecard", str(sc), "--root", str(tree), "--prove-absences"]) + assert rc == 1 + + +def test_main_verify_without_corpus_returns_exit_2(tmp_path: Path) -> None: + """Verify mode needs the corpus; omitting `--corpus` (without `--prove-absences`) must exit 2 -- + could-not-measure, never confused with a clean 0. Proves the argparse-independent guard in `main`. + + Falsified by deleting the `if args.corpus is None: ... return 2` branch in `main`: it then falls + through to `verify(...)` with `corpus=None`, raising instead of returning 2, and this test's + `rc == 2` goes RED. Restored. + """ + sc = tmp_path / "sc.toml" + sc.write_text( + '[[cell]]\nid = "1.1.1"\nlevel = 1\nverdict = "fail"\n' + " [[cell.absence]]\n" + ' pattern = "x"\n' + ' positive_control = "y"\n' + ' mutation = "import x"\n', + encoding="utf-8", + ) + rc = main(["--scorecard", str(sc), "--root", str(tmp_path)]) + assert rc == 2 diff --git a/tests/test_connscale_smoke.py b/tests/test_connscale_smoke.py index 38a6cbc5..cf18fd1e 100644 --- a/tests/test_connscale_smoke.py +++ b/tests/test_connscale_smoke.py @@ -18,6 +18,7 @@ from __future__ import annotations +import random import socket import sys @@ -28,6 +29,22 @@ pytestmark = pytest.mark.timeout(120) # the per-test 60s default is too tight for two engine spawns +# The connection-count sweep; its max sets the contiguous inbound-port width (BACKLOG #1014). +_SMOKE_COUNTS = (12, 24) + +# Contiguous inbound-port window for the random anchor (BACKLOG #1014). Both bounds are +# chosen to keep the block clear of ports OTHER tests bind, so a concurrent worktree's +# connscale block cannot land on a sibling's fixed listener: +# - LOWER bound sits ABOVE the sibling fixed-port MLLP band. Sibling tests bind fixed +# inbound ports in the 11xxx-19xxx range (e.g. 15099, 19601); anchoring at 20000+ keeps +# the connscale block entirely above them. [20000,30000) is empty of fixed test binds. +# - UPPER bound stays BELOW the OS ephemeral floors (Linux 32768+, Windows/macOS 49152+) +# so a kernel-assigned ephemeral port -- the sink/API ports from _free_port(), or any +# unrelated connection -- can never land inside the block after it is probed. +# The upper bound is exclusive. +_INBOUND_PORT_LO = 20000 +_INBOUND_PORT_HI = 30000 + def _free_port() -> int: s = socket.socket() @@ -39,12 +56,52 @@ def _free_port() -> int: s.close() +def _free_contiguous_ports(n: int, *, tries: int = 200) -> list[int]: + """Reserve ``n`` contiguous free inbound ports anchored at a RANDOM base. + + The random anchor is the concurrency fix (BACKLOG #1014): it de-correlates worktrees so + two suites rarely pick overlapping blocks. The old fixed ``base_port = 41000`` guaranteed + a collision whenever two checkouts ran the suite at once. Probe/bind-and-release only holds + the block momentarily, so it cannot truly reserve it against a concurrent engine -- the + random anchor over a wide window is the real defense, and a genuine future collision now + surfaces as a RED rather than a masked retry. + """ + if _INBOUND_PORT_HI - n <= _INBOUND_PORT_LO: + raise RuntimeError( + f"cannot reserve {n} contiguous ports in [{_INBOUND_PORT_LO},{_INBOUND_PORT_HI})" + ) + for _ in range(tries): + base = random.randint(_INBOUND_PORT_LO, _INBOUND_PORT_HI - n) + socks: list[socket.socket] = [] + try: + for i in range(n): + s = socket.socket() + # No SO_REUSEADDR on purpose: honest free-detection. A live listener must make + # bind FAIL here, unlike SO_REUSEADDR's Windows steal semantics. The block is + # released before the engine binds, so REUSEADDR would only add false-frees. + try: + s.bind(("127.0.0.1", base + i)) + except OSError: + s.close() + break + socks.append(s) + if len(socks) == n: + return list(range(base, base + n)) + finally: + for sock in socks: + sock.close() + raise RuntimeError( + f"could not reserve {n} contiguous free ports in " + f"[{_INBOUND_PORT_LO},{_INBOUND_PORT_HI}) after {tries} tries" + ) + + def _smoke_profile(base_port: int) -> object: # Small N (12 β†’ 24) + short holds so the smoke fits the pytest budget; both sweep modes by default. return load_connscale_profile_text(f""" [connscale] name = "smoke-it" -counts = [12, 24] +counts = {list(_SMOKE_COUNTS)} sweep_mode = "both" aggregate_rate = 24.0 per_conn_rate = 1.0 @@ -66,13 +123,18 @@ def _smoke_profile(base_port: int) -> object: """) -@pytest.mark.flaky( - reruns=2, reruns_delay=3 -) # CI runners are noisy (mf-ci-test-flakes): re-run clears async def test_connscale_smoke_end_to_end() -> None: - # Reserve a base inbound-port block that won't collide with the sink/API ports. The 24-conn max - # sweep needs 24 contiguous inbound ports; pick a high base well clear of the ephemeral churn. - base_port = 41000 + # Dynamically reserve a contiguous inbound-port block (BACKLOG #1014). The sweep's max + # connection count needs that many contiguous inbound ports, and the engine binds + # base_port + i for each. A RANDOM anchor de-correlates concurrent worktrees so they no + # longer contend for one fixed block; contiguity is asserted at the acquisition site, and + # the allocator fails loudly if no free block can be found (never a silent fixed fallback). + # The sink/API ports stay ephemeral (above the inbound window) and won't hit the block. + inbound_ports = _free_contiguous_ports(max(_SMOKE_COUNTS)) + assert inbound_ports == list(range(inbound_ports[0], inbound_ports[0] + max(_SMOKE_COUNTS))), ( + inbound_ports + ) + base_port = inbound_ports[0] sink_port = _free_port() api_port = _free_port() profile = _smoke_profile(base_port) @@ -148,3 +210,29 @@ def test_fd_sampler_reads_self() -> None: assert live is None or live > 0 # None only if the OS tool is unavailable on this runner dead = FdSampler(2**31 - 1).sample() # an implausible PID assert dead is None + + +def test_free_contiguous_ports_are_contiguous_and_in_window() -> None: + # The allocator returns exactly n ascending, contiguous ports inside the window. It + # deliberately does NOT re-bind to "prove free" -- that is TOCTOU-racy and would reintroduce + # the exact flake class BACKLOG #1014 removes. + ports = _free_contiguous_ports(8) + assert len(ports) == 8 + assert ports == list(range(ports[0], ports[0] + 8)) + assert ports[0] >= _INBOUND_PORT_LO + assert ports[-1] < _INBOUND_PORT_HI + + +def test_free_contiguous_ports_fails_loud_when_unsatisfiable() -> None: + # tries=0 hits the post-loop exhaustion branch deterministically (without occupying the + # whole window) and must raise -- never fall back silently to a fixed port (BACKLOG #1014). + with pytest.raises(RuntimeError, match="could not reserve"): + _free_contiguous_ports(8, tries=0) + + +def test_free_contiguous_ports_fails_loud_when_window_too_narrow() -> None: + # The width guard fires BEFORE any probing when the requested block cannot fit the window + # at all: asking for one more port than the window holds can never be satisfied, so it + # raises up front rather than looping (BACKLOG #1014 -- fail loud, never a silent fallback). + with pytest.raises(RuntimeError, match="cannot reserve"): + _free_contiguous_ports(_INBOUND_PORT_HI - _INBOUND_PORT_LO + 1) diff --git a/tests/test_coord_claim_liveness.py b/tests/test_coord_claim_liveness.py index 04d81ef2..9bb784c3 100644 --- a/tests/test_coord_claim_liveness.py +++ b/tests/test_coord_claim_liveness.py @@ -50,10 +50,14 @@ def git(repo: Path, *args: str) -> str: @pytest.fixture def repo(tmp_path: Path) -> Path: r = tmp_path / "repo" - r.mkdir() + (r / "scripts" / "coord").mkdir(parents=True) subprocess.run(["git", "init", "-q", "-b", "main", str(r)], check=True, capture_output=True) git(r, "config", "user.email", "t@example.invalid") git(r, "config", "user.name", "t") + # Staged and committed for the reason written out in test_coord_claim_refresh.py's fixture: + # claim.ps1 anchors on its own location now (BACKLOG #1060), so the copy IS the sandbox, and a + # linked worktree of this fixture carries its own -- which is what `peer_holding` relies on. + shutil.copy2(CLAIM, r / "scripts" / "coord" / "claim.ps1") (r / "f.txt").write_text("x", encoding="utf-8") git(r, "add", "-A") git(r, "commit", "-qm", "base") @@ -61,8 +65,16 @@ def repo(tmp_path: Path) -> Path: def claim(cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: + """Run the checkout's OWN copy -- it scopes itself to where it LIVES, not to the cwd.""" return subprocess.run( - ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(CLAIM), *args], + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(cwd / "scripts" / "coord" / "claim.ps1"), + *args, + ], cwd=str(cwd), capture_output=True, text=True, diff --git a/tests/test_coord_claim_refresh.py b/tests/test_coord_claim_refresh.py index 49ef8a1f..6f65cb76 100644 --- a/tests/test_coord_claim_refresh.py +++ b/tests/test_coord_claim_refresh.py @@ -48,10 +48,17 @@ def git(repo: Path, *args: str) -> str: @pytest.fixture def repo(tmp_path: Path) -> Path: r = tmp_path / "repo" - r.mkdir() + (r / "scripts" / "coord").mkdir(parents=True) subprocess.run(["git", "init", "-q", "-b", "main", str(r)], check=True, capture_output=True) git(r, "config", "user.email", "t@example.invalid") git(r, "config", "user.name", "t") + # STAGE THE SCRIPT INSIDE THE FIXTURE, and commit it, because claim.ps1 anchors on its own location + # rather than on the cwd (BACKLOG #1060). This is also what makes the sandbox STRUCTURAL: these + # tests used to be scoped to a temp registry only by ambient cwd, so the moment the script stopped + # consulting cwd they began writing REAL claims into this clone's shared registry -- measured, two + # strays landed there on the first run after the fix. Committing it means a linked worktree of this + # fixture carries its own copy, which is how `peer_holding` still gets a claim held by the peer. + shutil.copy2(CLAIM, r / "scripts" / "coord" / "claim.ps1") (r / "f.txt").write_text("x", encoding="utf-8") git(r, "add", "-A") git(r, "commit", "-qm", "base") @@ -59,9 +66,20 @@ def repo(tmp_path: Path) -> Path: def claim(cwd: Path, *args: str) -> subprocess.CompletedProcess[str]: - """Run the real script from ``cwd`` -- it scopes itself to the cwd's repo, and has no -Repo.""" + """Run the checkout's OWN copy -- the script scopes itself to where it LIVES, not to the cwd. + + ``cwd`` still names the worktree under test, but it does so by HOLDING that copy rather than by + being the ambient directory, which is what a session invoking `scripts\\coord\\claim.ps1` does. + """ return subprocess.run( - ["pwsh", "-NoProfile", "-NonInteractive", "-File", str(CLAIM), *args], + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(cwd / "scripts" / "coord" / "claim.ps1"), + *args, + ], cwd=str(cwd), capture_output=True, text=True, diff --git a/tests/test_hop_refusal_329.py b/tests/test_hop_refusal_329.py new file mode 100644 index 00000000..4b653764 --- /dev/null +++ b/tests/test_hop_refusal_329.py @@ -0,0 +1,281 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""#329 β€” route the four remaining ``MEFOR_ALLOW_INSECURE_TLS`` cells through the ADR-0092 clamp. + +Before this lane four cells still consulted the raw ``insecure_tls_allowed()`` predicate, so on first +deployment each WOULD permit weakened TLS regardless of the posture clamp: the SFTP unknown-host-key +acceptance (in-gate), and the LDAPS ``ad_tls_verify=false`` bind, the webhook alert sink and the +AI-broker cleartext-http credential hop (all out-of-gate). This suite proves each cell now REFUSES an +enforcing-PHI hop even with the escape set, that a non-enforcing / unstamped posture still crosses +(byte-identical to before), that the secure path is untouched, and β€” the load-bearing guard against the +item's headline "green and inert" risk β€” that the out-of-gate factories/constructors actually THREAD a +non-``None`` posture through the real ``create_app`` route and ``AuthService``/notifier seams. +""" + +from __future__ import annotations + +import logging +from collections.abc import AsyncIterator +from pathlib import Path + +import httpx +import pytest + +from messagefoundry.api import create_app +from messagefoundry.auth.ldap import LdapAuthenticator, LdapError +from messagefoundry.auth.service import AuthService +from messagefoundry.config.ai_policy import AiMode +from messagefoundry.config.settings import ( + INSECURE_TLS_ESCAPE_ENV, + AiSettings, + AlertsSettings, + AuthSettings, +) +from messagefoundry.config.tls_policy import HopPosture, active_hop_posture +from messagefoundry.pipeline import Engine +from messagefoundry.pipeline.alert_sinks import WebhookTransport, notifier_from_settings +from messagefoundry.store.store import MessageStore +from messagefoundry.transports import ai_broker as ai_broker_mod +from messagefoundry.transports.ai_broker import AiBroker, AiBrokerError, ai_broker_from_settings +from messagefoundry.transports.remotefile import _SftpClient + +# Mirror tests/test_hop_refusal_serve_clamp.py so the two suites decide against the same postures. +PROD_PHI = HopPosture(is_phi=True, enforcing=True) +STAGING_PHI = HopPosture(is_phi=True, enforcing=False) +SYNTHETIC = HopPosture(is_phi=False, enforcing=False) # dev / synthetic instance (no PHI) + + +@pytest.fixture +async def store(tmp_path: Path) -> AsyncIterator[MessageStore]: + s = await MessageStore.open(tmp_path / "hop329.db") + yield s + await s.close() + + +# --- Cell (2): SFTP host key (IN-GATE, reads current_hop_posture via _here) --------------------- + + +def test_sftp_host_key_clamped_prod_phi_even_with_escape(monkeypatch: pytest.MonkeyPatch) -> None: + # In-gate cell: with the escape set, the unknown-host-key auto-accept is CLAMPED under an enforcing + # PHI posture (accept_unknown stays False β†’ RejectPolicy), but a non-enforcing / synthetic posture + # still auto-accepts. Construction reads only the posture, so no paramiko fake is needed here. + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + s = {"host": "h", "port": 22, "remote_dir": "/in"} + with active_hop_posture(PROD_PHI): + assert _SftpClient(s)._accept_unknown is False # clamped: enforcing PHI refuses the escape + with active_hop_posture(STAGING_PHI): + assert ( + _SftpClient(s)._accept_unknown is True + ) # non-enforcing PHI still crosses with the escape + with active_hop_posture(SYNTHETIC): + assert _SftpClient(s)._accept_unknown is True # synthetic instance still crosses + + +def test_sftp_host_key_fail_closed_without_escape(monkeypatch: pytest.MonkeyPatch) -> None: + # Escape-off negative control: no posture crosses (unchanged fail-closed). Asserting True here would + # red immediately β€” proof the assertion binds something. + monkeypatch.delenv(INSECURE_TLS_ESCAPE_ENV, raising=False) + s = {"host": "h", "port": 22, "remote_dir": "/in"} + with active_hop_posture(PROD_PHI): + assert _SftpClient(s)._accept_unknown is False + # Unstamped (no active_hop_posture) with the escape off is also fail-closed. + assert _SftpClient(s)._accept_unknown is False + + +# --- Cell (1): LDAPS ad_tls_verify=false (OUT-OF-GATE, explicit posture) ------------------------ + + +def _ldaps_verifyoff() -> AuthSettings: + return AuthSettings( + ad_enabled=True, + ad_server="ldaps://dc.example.com", + ad_user_search_base="DC=example,DC=com", + ad_bind_dn="CN=svc,DC=example,DC=com", + ad_bind_password="synthetic-bind-secret", + ad_tls_verify=False, + ) + + +def test_ldaps_verifyoff_clamped_prod_phi_even_with_escape( + monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture +) -> None: + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + s = _ldaps_verifyoff() + # Enforcing PHI: refused even with the escape (the clamp bites). + with pytest.raises(LdapError, match="ad_tls_verify=false"): + LdapAuthenticator(s, posture=PROD_PHI) + # Non-enforcing PHI: crosses with the escape (warned loudly). + with caplog.at_level(logging.WARNING): + LdapAuthenticator(s, posture=STAGING_PHI) + assert any("DISABLED" in r.getMessage() for r in caplog.records) + # Unstamped posture (a direct/test construction) falls back to the unclamped escape β€” byte-identical. + LdapAuthenticator(s, posture=None) + + +async def test_ldaps_authservice_threads_posture( + store: MessageStore, monkeypatch: pytest.MonkeyPatch +) -> None: + # Seam: AuthService must thread hop_posture into the LdapAuthenticator it builds. With an enforcing + # PHI posture + the escape set, constructing the service raises LdapError β€” proof the posture reaches + # the cell. If the seam dropped it (posture=None), the escape would be unclamped and the build would + # succeed. + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + with pytest.raises(LdapError, match="ad_tls_verify=false"): + AuthService(store, _ldaps_verifyoff(), hop_posture=PROD_PHI) + # A non-enforcing posture builds the service without raising (crosses with the escape). + AuthService(store, _ldaps_verifyoff(), hop_posture=STAGING_PHI) + + +def test_ldaps_verifying_secure_path_unaffected(monkeypatch: pytest.MonkeyPatch) -> None: + # Secure-path negative control: ad_tls_verify=True (verifying LDAPS) + enforcing PHI + escape UNSET + # builds with no error β€” the guard only fires on the verify-OFF path, so #329 leaves the secure path + # byte-identical. + monkeypatch.delenv(INSECURE_TLS_ESCAPE_ENV, raising=False) + s = AuthSettings( + ad_enabled=True, + ad_server="ldaps://dc.example.com", + ad_user_search_base="DC=example,DC=com", + ad_bind_dn="CN=svc,DC=example,DC=com", + ad_bind_password="synthetic-bind-secret", + ad_tls_verify=True, + ) + LdapAuthenticator(s, posture=PROD_PHI) # no raise: verifying LDAPS is unaffected by the change + + +# --- Cell (3): webhook alert sink cleartext http (OUT-OF-GATE, explicit posture) ---------------- + + +def test_webhook_cleartext_clamped_prod_phi_even_with_escape( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + with pytest.raises(ValueError, match="plaintext http"): + WebhookTransport("http://hooks.example/x", posture=PROD_PHI) + # Non-enforcing PHI and unstamped both cross with the escape (byte-identical to before). + WebhookTransport("http://hooks.example/x", posture=STAGING_PHI) + WebhookTransport("http://hooks.example/x", posture=None) + + +def test_webhook_notifier_threads_posture(monkeypatch: pytest.MonkeyPatch) -> None: + # Seam: notifier_from_settings must thread posture into the WebhookTransport it builds. + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + with pytest.raises(ValueError, match="plaintext http"): + notifier_from_settings( + AlertsSettings(webhook_url="http://hooks.example/x"), posture=PROD_PHI + ) + # Non-enforcing posture builds the notifier (crosses with the escape). + notifier_from_settings( + AlertsSettings(webhook_url="http://hooks.example/x"), posture=STAGING_PHI + ) + + +def test_webhook_https_secure_path_unaffected(monkeypatch: pytest.MonkeyPatch) -> None: + # Negative control: an https webhook is never refused, whatever the posture. + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + WebhookTransport("https://hooks.example/x", posture=PROD_PHI) + + +# --- Cell (4): AI-broker cleartext http credential hop (OUT-OF-GATE, explicit posture) ---------- + +_HTTP_ENDPOINT = "http://ai.internal/v1/messages" +_HTTPS_ENDPOINT = "https://ai.internal/v1/messages" + + +def _managed_ai(**over: object) -> AiSettings: + kw: dict[str, object] = { + "mode": AiMode.MANAGED_ENDPOINT, + "environment": "prod", + "endpoint": _HTTP_ENDPOINT, + "api_key": "sk-synthetic-key", + "allowed_endpoints": ["ai.internal"], + } + kw.update(over) + return AiSettings(**kw) # type: ignore[arg-type] + + +def test_ai_broker_cleartext_clamped_prod_phi_even_with_escape( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + + def _build(posture: HopPosture | None) -> AiBroker: + return AiBroker( + endpoint=_HTTP_ENDPOINT, + api_key="k", + allowed_endpoints=["ai.internal"], + posture=posture, + ) + + with pytest.raises(AiBrokerError, match="cleartext http"): + _build(PROD_PHI) + _build(STAGING_PHI) # crosses with the escape on non-enforcing PHI + _build(None) # unstamped falls back to the unclamped escape β€” byte-identical + + +def test_ai_broker_factory_threads_posture(monkeypatch: pytest.MonkeyPatch) -> None: + # Seam: ai_broker_from_settings must thread posture into the AiBroker it builds. + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + with pytest.raises(AiBrokerError, match="cleartext http"): + ai_broker_from_settings(_managed_ai(), posture=PROD_PHI) + ai_broker_from_settings(_managed_ai(), posture=STAGING_PHI) # crosses on non-enforcing PHI + + +def test_ai_broker_https_secure_path_unaffected(monkeypatch: pytest.MonkeyPatch) -> None: + # Negative control: an https endpoint is never refused on the cleartext arm, whatever the posture. + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + AiBroker( + endpoint=_HTTPS_ENDPOINT, api_key="k", allowed_endpoints=["ai.internal"], posture=PROD_PHI + ) + + +# --- Anti-"green-and-inert" WIRING test: the real create_app ai_chat route threads the posture ----- + + +@pytest.fixture +async def engine(tmp_path: Path) -> AsyncIterator[Engine]: + eng = await Engine.create(tmp_path / "ai_wiring.db", poll_interval=0.02) + yield eng + await eng.stop() + + +def _client(app: object) -> httpx.AsyncClient: + transport = httpx.ASGITransport(app=app) # type: ignore[arg-type] + return httpx.AsyncClient(transport=transport, base_url="http://t") + + +@pytest.fixture +def stub_chat(monkeypatch: pytest.MonkeyPatch) -> None: + # If (and only if) the broker BUILDS, a canned reply keeps the route off the network β€” so a route + # that fails to thread the posture returns 200, not a network 502. That makes the 503-vs-not signal + # crisp for the falsification below. + def fake_chat(self: AiBroker, prompt: str) -> str: + return "def handle(msg): return Send('OB', msg)" + + monkeypatch.setattr(ai_broker_mod.AiBroker, "chat", fake_chat) + + +async def test_ai_chat_route_refuses_cleartext_under_enforcing_phi( + engine: Engine, monkeypatch: pytest.MonkeyPatch, stub_chat: None +) -> None: + # The load-bearing wiring guard: through the REAL route, a prod (enforcing-PHI) instance with the + # escape set must REFUSE to build the broker over a cleartext-http endpoint β†’ HTTP 503. This proves + # create_app actually threads a non-None posture into ai_broker_from_settings (the "green and inert" + # failure the item warns of). FALSIFY: drop `posture=_phi_read_posture` in the ai_chat route β†’ the + # broker builds over http and the route returns 200 (chat stubbed) instead of 503. + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + app = create_app(engine, ai_settings=_managed_ai(endpoint=_HTTP_ENDPOINT), allow_no_auth=True) + async with _client(app) as c: + r = await c.post("/ai/chat", json={"prompt": "hi"}) + assert r.status_code == 503 # broker refused by the clamp through the real route + + +async def test_ai_chat_route_allows_https_under_enforcing_phi( + engine: Engine, monkeypatch: pytest.MonkeyPatch, stub_chat: None +) -> None: + # Companion control: the SAME enforcing-PHI + escape setup over an HTTPS endpoint is NOT 503 β€” so the + # 503 above is the cleartext clamp, not a blanket refusal of the route. + monkeypatch.setenv(INSECURE_TLS_ESCAPE_ENV, "1") + app = create_app(engine, ai_settings=_managed_ai(endpoint=_HTTPS_ENDPOINT), allow_no_auth=True) + async with _client(app) as c: + r = await c.post("/ai/chat", json={"prompt": "hi"}) + assert r.status_code == 200 diff --git a/tests/test_sandbox_import_boundary.py b/tests/test_sandbox_import_boundary.py new file mode 100644 index 00000000..0b2abbf8 --- /dev/null +++ b/tests/test_sandbox_import_boundary.py @@ -0,0 +1,155 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Static import-boundary guard for the sandbox worker files (ADR 0087, BACKLOG #346). + +The sandbox draws its trust boundary at runtime: :class:`_ForbiddenImportFinder` (in +``_sandbox_worker``) refuses ``DEFAULT_FORBIDDEN_MODULES`` β€” ``socket``/``ssl``/``asyncio`` and the +I/O- and secret-bearing ``messagefoundry.*`` subpackages β€” but ONLY inside the ``[sandbox].mode= +subprocess`` child, which is not the default. So nothing statically pins that the two modules which +run *inside* that boundary (:mod:`messagefoundry.pipeline._sandbox_codec` and +:mod:`messagefoundry.pipeline._sandbox_worker`) do not themselves import a forbidden module. They are +clean today; a future edit that violated it would fail **only** on a deployment that turned the +sandbox on for security reasons, behind a green default-mode suite. + +This is defence-in-depth test coverage, not a code fix: the guard below walks the two files' own +``ast`` import nodes and asserts none resolves under a ``DEFAULT_FORBIDDEN_MODULES`` prefix. The +forbidden set is IMPORTED from the runtime constant, never copied, so the test tracks whatever the +sandbox actually forbids. Scope is those two files' **direct** imports β€” deliberately not a +transitive walk: importing the codec pulls asyncio/cryptography/store/transports/auth into +``sys.modules``, so a transitive walker would be red on clean shipped code and prove nothing. +""" + +from __future__ import annotations + +import ast +from pathlib import Path + +from messagefoundry.pipeline.sandbox import DEFAULT_FORBIDDEN_MODULES + +_PIPELINE = Path(__file__).resolve().parents[1] / "messagefoundry" / "pipeline" + +#: The two modules BACKLOG #346 scopes: the length-prefixed codec both ends of the pipe speak, and the +#: ``python -m`` worker child entrypoint. ``sandbox.py`` is out of that scope even though the worker +#: child also imports it (``_sandbox_worker.main`` pulls ``_read_frame_bytes``/``_write_frame`` from it, +#: before the runtime guard goes up) β€” it imports nothing forbidden today (stdlib + ``config.*`` + the +#: codec). A forbidden top-level import newly added to ``sandbox.py`` would evade THIS static pin (and +#: its already-bound reference would even survive the guard's ``sys.modules`` purge), so widening the +#: walk to it is a scope decision left to the owner, not a gap this two-file guard silently covers. +_TARGETS: tuple[tuple[str, Path], ...] = ( + ("messagefoundry.pipeline._sandbox_codec", _PIPELINE / "_sandbox_codec.py"), + ("messagefoundry.pipeline._sandbox_worker", _PIPELINE / "_sandbox_worker.py"), +) + + +def _is_forbidden(module: str) -> bool: + """True if ``module`` is a forbidden module or a submodule of one. + + Mirrors :meth:`_ForbiddenImportFinder.find_spec`'s prefix match by construction (``name == prefix + or name.startswith(prefix + ".")``), so the static check agrees exactly with the runtime one.""" + return any(module == p or module.startswith(p + ".") for p in DEFAULT_FORBIDDEN_MODULES) + + +def _walk(source: str, dotted: str) -> tuple[set[str], int]: + """Derive the imported dotted names from ``source`` (a module whose own name is ``dotted``). + + Returns ``(candidate module names, import-node count)``. ``ast.walk`` catches nested/function-level + imports for free. For ``from X import a, b`` the plain module ``X`` **and** ``X.a`` / ``X.b`` are + both candidates β€” the alias-append is what catches ``from messagefoundry import store`` (a + forbidden subpackage imported off the non-forbidden ``messagefoundry`` parent). Relative imports + are resolved to absolute against ``dotted`` so a future ``from ..store import x`` cannot walk + straight through the guard.""" + package = dotted.rsplit(".", 1)[0] + candidates: set[str] = set() + node_count = 0 + for node in ast.walk(ast.parse(source)): + if isinstance(node, ast.Import): + node_count += 1 + for alias in node.names: + candidates.add(alias.name) + elif isinstance(node, ast.ImportFrom): + node_count += 1 + if node.level == 0: + if node.module: + candidates.add(node.module) + for alias in node.names: + candidates.add(f"{node.module}.{alias.name}") + else: + # `from . import x` -> the package itself; `from ..store import x` -> one level up. + base_pkg = package.rsplit(".", node.level - 1)[0] + base = f"{base_pkg}.{node.module}" if node.module else base_pkg + candidates.add(base) + for alias in node.names: + candidates.add(f"{base}.{alias.name}") + return candidates, node_count + + +def test_sandbox_boundary_modules_import_nothing_forbidden() -> None: + """LIVE GUARD: the shipped codec + worker source imports nothing on the forbidden list. + + Green on the real source today; this pins it that way. A future ``import socket`` / ``from + messagefoundry.store import ...`` slipping into either file would make ``mode=subprocess`` DOA on + first deployment, and this reddens instead of the suite staying green.""" + # Anti-vacuity: the forbidden set is real and populated, so an empty scan cannot pass by default. + assert DEFAULT_FORBIDDEN_MODULES, "DEFAULT_FORBIDDEN_MODULES is empty" + assert "socket" in DEFAULT_FORBIDDEN_MODULES, DEFAULT_FORBIDDEN_MODULES + + violations: list[str] = [] + for dotted, path in _TARGETS: + assert path.exists(), f"sandbox boundary module missing: {path}" + candidates, node_count = _walk(path.read_text(encoding="utf-8"), dotted) + # Anti-vacuity: a walker that silently saw no imports must FAIL, not pass green. + assert node_count > 0, f"{path.name} yielded no import nodes" + for candidate in sorted(candidates): + if _is_forbidden(candidate): + violations.append(f"{path.name} imports {candidate}") + assert not violations, violations + + +def test_walker_flags_every_forbidden_import_form() -> None: + """POSITIVE CONTROL: prove the walker can SEE each import form, run in isolation per form. + + Each case is its own source string so the assertion depends only on that form's handling β€” e.g. + the alias-append case would still be masked by a sibling ``from messagefoundry.store import ...`` + if they shared one source, so they must not. Committed and always-on: it pins the alias-append, + relative-resolution and nested-import refinements in CI permanently.""" + ctx = "messagefoundry.pipeline._sandbox_worker" + cases: dict[str, tuple[str, str]] = { + "top-level import": ("import socket\n", "socket"), + "from-import of a forbidden submodule": ( + "from messagefoundry.store import base\n", + "messagefoundry.store", + ), + # The load-bearing case: `store` is forbidden but its parent `messagefoundry` is not, so only + # the alias-append candidate `messagefoundry.store` trips the guard here. + "from-parent import of a forbidden child (alias-append)": ( + "from messagefoundry import store\n", + "messagefoundry.store", + ), + "function-level import": ("def f():\n import ssl\n", "ssl"), + "relative import resolved to absolute": ( + "from ..auth import service\n", + "messagefoundry.auth", + ), + } + for label, (source, expected) in cases.items(): + candidates, node_count = _walk(source, ctx) + assert node_count > 0, f"{label}: no import nodes seen" + flagged = {c for c in candidates if _is_forbidden(c)} + assert expected in flagged, f"{label}: expected {expected!r} flagged, got {sorted(flagged)}" + + +def test_walker_does_not_flag_allowed_imports() -> None: + """NEGATIVE CONTROL: benign imports the sandbox child legitimately needs raise ZERO flags. + + Guards against an over-broad matcher (e.g. one that flagged the non-forbidden ``messagefoundry`` + parent), which would dead-letter a correct build β€” the flip side of a vacuous walker.""" + source = ( + "import json\n" + "from messagefoundry.config.wiring import Send\n" + "from messagefoundry.parsing.message import Message\n" + "from messagefoundry.pipeline import _sandbox_codec\n" + ) + candidates, node_count = _walk(source, "messagefoundry.pipeline._sandbox_worker") + assert node_count > 0 + flagged = {c for c in candidates if _is_forbidden(c)} + assert not flagged, sorted(flagged) diff --git a/tests/test_script_root_anchoring.py b/tests/test_script_root_anchoring.py new file mode 100644 index 00000000..5d51f4c6 --- /dev/null +++ b/tests/test_script_root_anchoring.py @@ -0,0 +1,233 @@ +"""Scripts invoked by absolute ``-File`` path must act on the checkout they LIVE in, not the cwd. + +The regression home for the "cwd is not the caller" family (BACKLOG #1057, #1059, #1060, #1062, #1063). +Three mechanisms in this repo silently assumed that where a command runs is where the caller is, and all +three failed in the benign-looking direction -- a wrong owner recorded, a gate armed in the wrong tree, an +occupancy of zero for a tree in active use. None of them raised. + +**Every test here asserts the DIVERGENCE, never the happy path.** A script run from inside its own +checkout passes with the bug still in: cwd and script root are the same directory, so the two candidate +answers are indistinguishable. The case that can tell them apart is an absolute ``-File`` invocation whose +cwd is a DIFFERENT checkout that also carries the file the script writes -- which is the ordinary shape on +a clone carrying dozens of worktrees, and is measured at 29% of writes on this repo. Per BACKLOG #1000 a +control needs the case that can distinguish; a test run from inside the target proves nothing. + +The static spelling guards below are deliberately paired with a behavioural test each. On their own they +would be exactly the green-because-it-cannot-see control this project keeps filing. +""" + +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +_ROOT = Path(__file__).resolve().parents[1] +_SETUP_LEAK_GATE = _ROOT / "scripts" / "dev" / "setup-leak-gate.ps1" +_SECURITY = _ROOT / "scripts" / "security" +_ALLOC = _ROOT / "scripts" / "coord" / "alloc.ps1" +_CLAIM = _ROOT / "scripts" / "coord" / "claim.ps1" + +pytestmark = pytest.mark.skipif( + shutil.which("pwsh") is None, reason="pwsh (PowerShell 7) not on PATH" +) + + +def _git(*args: str, cwd: Path) -> None: + subprocess.run(["git", *args], cwd=str(cwd), check=True, capture_output=True) + + +def _leak_gate_checkout(path: Path) -> Path: + """A minimal checkout carrying setup-leak-gate.ps1 and the files it reaches for. + + Only the three files the script needs are copied, NOT the whole of scripts/security. A maintainer + running this suite has the real ``scan-tokens.local.txt`` sitting in that directory, and a copytree + would sweep the private token list into a temp dir -- the one mistake the script itself refuses to + make (it deletes what it wrote rather than risk committing the list). + """ + (path / "scripts" / "dev").mkdir(parents=True) + (path / "scripts" / "security").mkdir(parents=True) + shutil.copy2(_SETUP_LEAK_GATE, path / "scripts" / "dev" / "setup-leak-gate.ps1") + for name in ("scan_forbidden.py", "scan-allowlist.txt", "scan-tokens.local.txt.example"): + shutil.copy2(_SECURITY / name, path / "scripts" / "security" / name) + # check-ignore reads the working-tree .gitignore, so no commit is needed -- but the repo must exist, + # or the script's own not-git-ignored guard deletes the file it just wrote and throws. + (path / ".gitignore").write_text("scripts/security/scan-tokens.local.txt\n", encoding="utf-8") + _git("init", "-b", "main", ".", cwd=path) + return path + + +def test_setup_leak_gate_arms_the_checkout_it_lives_in_not_the_cwd(tmp_path: Path) -> None: + """BACKLOG #1063. Reproduced by the divergence, which is the only shape that can fail. + + Pre-fix, ``$repo`` came from ``git rev-parse --show-toplevel`` -- the CURRENT directory -- so an + absolute ``-File`` invocation from another worktree installed the token list into the caller's tree + and printed CONFIGURED about it, while the tree the operator named kept no token source. Both + checkouts here carry ``scripts/security/``, so "which tree got the file" is the whole question and + neither answer is available by accident. + + The verify step's exit code is deliberately NOT asserted: it shells out to ``python``, which is not + guaranteed on PATH on every CI leg, and the copy has already happened by then. Placement is the fact + under test. + """ + named = _leak_gate_checkout(tmp_path / "Named") + caller = _leak_gate_checkout(tmp_path / "Caller") + + subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(named / "scripts" / "dev" / "setup-leak-gate.ps1"), + "-Synthetic", + ], + cwd=str(caller), # THE POINT: the shell is standing somewhere else entirely + capture_output=True, + text=True, + timeout=180, + ) + + installed = named / "scripts" / "security" / "scan-tokens.local.txt" + stray = caller / "scripts" / "security" / "scan-tokens.local.txt" + assert installed.is_file(), "the named checkout was not armed -- it read the cwd instead" + assert not stray.exists(), f"armed the CALLER's checkout instead: {stray}" + + +def _coord_checkout(path: Path, *, drafted: int, boundary: int) -> Path: + """A minimal checkout carrying scripts/coord/ and the two files the allocator reads from ``$repo``. + + ``drafted`` is a BACKLOG heading written to the working tree and committed NOWHERE -- the floor term + that exists to catch exactly that, and the one where reading the wrong tree is worse than + misattribution: a number invisible to the sweep is a number free to re-issue. + + ``boundary`` is the ``PUBLIC_BACKLOG_FLOOR`` the allocator parses out of ledger_check.py rather than + restating. A stub is faithful here because the allocator reads it with a regex over the raw text, and + it gives a second, independent signal for which checkout was consulted. + """ + (path / "scripts" / "coord").mkdir(parents=True) + (path / "scripts" / "hooks").mkdir(parents=True) + (path / "docs").mkdir(parents=True) + shutil.copy2(_ALLOC, path / "scripts" / "coord" / "alloc.ps1") + shutil.copy2(_CLAIM, path / "scripts" / "coord" / "claim.ps1") + (path / "scripts" / "hooks" / "ledger_check.py").write_text( + f"PUBLIC_BACKLOG_FLOOR = {boundary}\n", encoding="utf-8" + ) + (path / "docs" / "BACKLOG.md").write_text( + f"# Backlog\n\n## {drafted}. A number drafted here and committed nowhere\n", + encoding="utf-8", + ) + _git("init", "-b", "main", ".", cwd=path) + _git("config", "user.email", "t@e.com", cwd=path) + _git("config", "user.name", "t", cwd=path) + _git("add", "-A", cwd=path) + _git("commit", "-m", "fixture", "--no-verify", cwd=path) + return path + + +def test_alloc_reads_the_floor_from_its_own_checkout_not_the_cwd(tmp_path: Path) -> None: + """BACKLOG #1060, asserted by ``-ShowFloor`` so no numbers are burned. + + Allocation is a one-way door -- claims are never released, "holes are free, collisions are not" -- + so a test that allocated would leave permanent holes in the shared registry for every worktree of + this clone. ``-ShowFloor`` exists for exactly this and computes the same floor without advancing the + ratchet. + + The caller's drafted number is deliberately HIGHER than the target's. The floor is a maximum, so a + wrong read shows up as a floor that is too high; equal numbers, or a lower one in the caller, would + both pass with the bug still in. + """ + named = _coord_checkout(tmp_path / "Named", drafted=4242, boundary=1200) + caller = _coord_checkout(tmp_path / "Caller", drafted=7777, boundary=1900) + + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(named / "scripts" / "coord" / "alloc.ps1"), + "-ShowFloor", + "-Kind", + "backlog", + ], + cwd=str(caller), # THE POINT: the shell is standing in the other checkout + capture_output=True, + text=True, + timeout=180, + ) + assert proc.returncode == 0, proc.stderr or proc.stdout + out = proc.stdout + + assert "floor : 4242" in out, f"read the CALLER's working tree, not its own:\n{out}" + assert "7777" not in out, f"the caller's drafted number reached the floor:\n{out}" + # Independent second signal: the boundary comes from /scripts/hooks/ledger_check.py, so it + # answers "which checkout" without depending on the floor arithmetic at all. + assert "boundary : 1200" in out, f"parsed the CALLER's ledger_check.py:\n{out}" + # And the registry it would write to must live beside the named checkout's object store. + watermark = next(line for line in out.splitlines() if line.startswith("watermark:")) + assert str(named).replace("\\", "/").casefold() in watermark.replace("\\", "/").casefold(), ( + watermark + ) + + +def test_claim_records_the_checkout_it_lives_in_not_the_cwd(tmp_path: Path) -> None: + """The same construct in claim.ps1, which was NOT filed -- found by inspection while fixing #1060. + + A numbered claim is enforced by scripts/hooks/claim_check.py at commit time. That hook reads the repo + from cwd and is right to: cwd IS the committing worktree for a commit hook. Only the tool can be + invoked from somewhere else, so only the tool needed anchoring. + """ + named = _coord_checkout(tmp_path / "Named", drafted=4242, boundary=1200) + caller = _coord_checkout(tmp_path / "Caller", drafted=7777, boundary=1900) + + proc = subprocess.run( + [ + "pwsh", + "-NoProfile", + "-NonInteractive", + "-File", + str(named / "scripts" / "coord" / "claim.ps1"), + "-Take", + "anchoring-probe", + "-Note", + "divergence fixture", + ], + cwd=str(caller), + capture_output=True, + text=True, + timeout=180, + ) + assert proc.returncode == 0, proc.stderr or proc.stdout + + written = list((named / ".git" / "mefor-coord" / "claims").glob("*.json")) + assert written, f"no claim landed in the named checkout's registry:\n{proc.stdout}" + recorded = json.loads(written[0].read_text(encoding="utf-8"))["worktree"] + assert recorded.replace("\\", "/").casefold() == str(named).replace("\\", "/").casefold(), ( + f"claim recorded against the CALLER: {recorded}" + ) + assert not (caller / ".git" / "mefor-coord" / "claims").exists(), ( + "the caller's registry was written to" + ) + + +def test_setup_leak_gate_does_not_reintroduce_an_unanchored_toplevel(tmp_path: Path) -> None: + """A spelling guard for the regression, paired with the behavioural test above. + + Alone this would be worthless -- it cannot see the defect, only its most likely spelling. It earns + its place by naming the exact construct so a reader who reaches for ``--show-toplevel`` again gets + told why not, at the moment they do it. + """ + text = _SETUP_LEAK_GATE.read_text(encoding="utf-8") + assert "$PSScriptRoot" in text, "the repo root must be anchored to the script's own location" + for line in text.splitlines(): + stripped = line.strip() + if stripped.startswith("#") or "--show-toplevel" not in stripped: + continue + assert "-C " in stripped, ( + "an unanchored `git rev-parse --show-toplevel` resolves against the CURRENT directory, " + f"which is BACKLOG #1063: {stripped}" + )