feat(security)!: no data label may allow a cleartext hop (ADR 0153) + the shipped-posture default flips - #38
Merged
Merged
Conversation
ADR 0092's precedence arm 3 was `not is_phi -> ALLOW`: an instance whose environment file declared `data_class = "synthetic"` crossed EVERY cleartext transport hop, silently, with no warning and no audit record. `data_class` is authored in the same file as the hosts it governs, by the same hand — a typo in it was indistinguishable from a deliberate declaration, and its blast radius was every transport hop in the product. `insecure_hop_disposition` loses `is_phi`, and `audited_opt_out` goes with it. The precedence is now: loopback -> ALLOW, `hop_attested` -> ALLOW, `cleartext_accepted` -> WARN, not `enforcing` -> WARN, else REFUSE. Only an arm that returned ALLOW was deleted, so ADR 0092 decision 5 (no-loosen) holds BY CONSTRUCTION — asserted over the whole old input space, not merely claimed. The honest escape (decision 2) is a new `Destination` pair, `cleartext_accepted` + `cleartext_reason`, load-validated three ways and authored either code-first on `outbound()` or as TOP-LEVEL `connections.toml` keys. It is deliberately SEPARATE from `tls_hop_attested`, with the opposite claim — attestation says the hop IS secure by means the engine cannot see (ALLOW, silent); acceptance says it is NOT, and we accept that (WARN, logged + audited at every construction). Merging them would leave the audit trail unable to tell a proxy-terminated hop from plaintext PHI on a flat network. For `Tcp()`/`X12()`, which have no TLS support at all, the declaration is permanent and structural (BACKLOG #311). NO TLS default is flipped (decision 3): all four factories keep `tls = False`. Three implementation questions the ADR left open, resolved and recorded in its Build notes: * `rest._shipped_strict_disposition`'s no-loosen floor is re-keyed on `cleartext_accepted` rather than on the global escape. A naive port would have floored arm 3's WARN straight back to REFUSE, making the declaration INERT for REST/SOAP/FHIR/DICOMweb — the largest cleartext-egress family and the only one where it is a genuine escape. Side effect, intended: MEFOR_ALLOW_INSECURE_TLS can no longer relax a non-enforcing HTTP cleartext hop. That is a TIGHTENING, and it stops the blunt env var being alive on HTTP while dead on raw TCP. * the two out-of-scope DELEGATING callers — `phi_read_hop_disposition` and `settings.forward_hop_disposition` — restate the deleted `not is_phi -> ALLOW` arm explicitly instead of inheriting it. Both are non-connections with nowhere to carry a declaration; refusing them would create a deviation the loosening registry cannot express. The scope limit is now a written decision at the one place it applies, not an emergent property of a signature change. * the declaration reaches the CREDENTIAL hops (Digest, OAuth2/SMART token endpoints, the forward-proxy credential, SOAP WS-Security / body secrets), not only the body hops. Leaving credentials with no honest declaration would push an operator toward writing a FALSE `tls_hop_attested` — the precise defect this ADR exists to remove. SMTP AUTH over cleartext stays refused OUTRIGHT. Also here, because they cannot be separated from the above: * the flag-implies-reason rule is retro-fitted to `tls_hop_attested` (decision 2) and deliberately reaches `[logging].forward_hop_attested`, which shares the validator and whose reason docs/PHI.md already described as mandatory; * the attestation-audit branches drop their `posture.is_phi` conjunct — with the authority no longer reading the label, gating the audit on it would silence the record for exactly the hops that newly depend on an attestation to cross; * the acceptance audit marker is lower-case on purpose: the PHI redaction filter collapses adjacent ALL-CAPS tokens to `[redacted]`, which would have scrubbed the marker out of the very record it exists to make findable; * `security_loosenings()` gains a required `cleartext_hops` parameter and reports the declaring connections by name, alongside the in-flight posture lane's required `store`/`auth` parameters and its `aad_bind` / `ad_session_recheck_seconds` entries. `[store].aad_bind` now defaults true and `[auth].ad_session_recheck_seconds` 300 (ADR 0148 GIVEN 1 — the hardened path is the shipped path); the AD cross-field refusal is re-keyed on `model_fields_set` so an EXPLICIT value without `ad_enabled` still refuses while the inert shipped default does not fail startup on a non-AD box. * `messagefoundry check` gains a `cleartext-accepted` surface listing the WHOLE accepted set (the ADR's stated mitigation for broad declaration), and `security show` now emits its `loosenings_scope` marker rather than reporting a settings-only list as if it were the whole posture. `data_class`, `HopPosture.is_phi` and `MEFOR_ALLOW_INSECURE_TLS` are all retained; the cleartext-hop authority simply stops reading them.
…ture lane The precedence table is the anchor, so it is rewritten from a 2**5 product over the old inputs to a 2**4 one over the new arms — plus two guards the old suite had no equivalent of: * a SIGNATURE pin. Asserting the arms alone would not catch a re-added `is_phi=False -> ALLOW` behind a defaulted parameter, because every existing call site would keep passing. `inspect.signature` is the detector that CAN fail. * a STRICTLY-STRICTER property test over the whole OLD 32-row input space, against a verbatim copy of the pre-0153 precedence kept in the test file. ADR 0153's central claim — "removing an ALLOW arm can only turn a crossing into a WARN or a REFUSE" — was previously a sentence in a document; it is now a check. Every test that asserted the deleted arm is inverted rather than deleted, and says in its own body what it used to assert and why the answer moved: a synthetic instance now REFUSES on raw TCP/X12/MLLP/DIMSE/FTP, on the whole HTTP family, on the FhirLookup read path, at `messagefoundry check`, and at serve. The `allowed_with_escape` transport tests are re-keyed onto the per-connection declaration, because MEFOR_ALLOW_INSECURE_TLS no longer relaxes any cleartext hop. New coverage for the acceptance pair: the three load-validation rules on both halves; a Destination-ONLY pin (a later "symmetry" refactor must not add a dead inbound knob that reads like a working control); connections.toml/code-first desugar parity asserted on the DISPOSITION, not just the field; the WARN-plus- dedicated-audit-record at every construction; and a test that the acceptance and the attestation are DISTINGUISHABLE in the trail, which is the one distinction keeping them separate fields exists to preserve. Plus a pin that no transport factory flips its `tls` default (ADR 0153 decision 3's emphatic non-goal, which would otherwise rot silently). For the posture lane, tests/test_security_posture_defaults.py covers both flipped defaults and — deliberately — the REGISTRY around them: * the AD cross-field refusal is exercised through `load_settings`, not the constructor. Constructing AuthSettings(...) marks every passed field as set, so a constructor-only test cannot distinguish the shipped default from an explicitly-typed 300 — the entire distinction the guard turns on. The negative case uses the SAME value as the default, so it can only pass if the check keys on `model_fields_set`. * a COMPLETENESS FLOOR: every boolean `[security]` switch at its insecure value must be named by `security_loosenings()`, with an explicit exemption set that states why each exempt switch is not a loosening. Nothing asserted this before; under "one posture, loosen only" a registry with no floor is the shape that lets a later switch be added at an insecure value with nothing reporting it. * `GET /security/posture` on the real surface for all three deviations, including the connection-scoped one read live off the running graph.
…the deviations ADR 0153 flips to BUILT and gains a Build notes section carrying the three implementation questions it left open, the answers taken, and the visibility surfaces the owner required — including the two surfaces that CANNOT see a per-connection declaration (`security show`, which never loads the graph, and the serve-time warning, which fires before it is loaded) and what covers them instead. Recording those is the point: a scope limit nobody wrote down is indistinguishable from an oversight. ADR 0092 gains a prominent amendment banner at the top rather than an edit to its decision text. Its precedence is now historically wrong but its structure, its loopback carve-out, its attestation and its two-layer gating all still stand, so the record of what was decided in July stays intact and the banner says exactly which two arms moved. ADR 0019 (`aad_bind`) and ADR 0079 (`ad_session_recheck_seconds`) get dated amendments for their default flips, each stating why the flip is safe (dual-read and rotate-key upgrade for the first; the reconciler's own LDAP-client precondition for the second), what it costs (CRYPTO-1's byte-identity now describes the opt-out, not the default), and why the AD cross-field refusal had to be re-keyed rather than removed. ADR 0019's superseded amendment keeps its text and gains a pointer. docs/SECURITY-LOOSENING.md gains all three deviations in the register's existing shape (what you lose / when acceptable / compensating controls), plus a standards row each. The `cleartext_accepted` entry is written to be read by someone about to set it: it says plainly what crosses the wire, distinguishes it from `tls_hop_attested` in the operator's own terms, states that Tcp()/X12() have no migration target, and enumerates both where it IS reported and where it is not. The remaining drift is corrected where the code moved under it: CONFIGURATION.md's cleartext-egress note (which still described the synthetic carve-out), PHI.md's §2/§3/§8 cell-binding prose and its `[logging]` forwarder gate (which now explains why that cell still reads the data label when the transport cells do not), CONNECTIONS.md (a new section documenting the declaration, both authoring surfaces, the permanent-vs-transitional distinction, and the EMAIL row), SECURITY.md and ASVS-L2-PHASE0-CHANGES.md's quoted defaults, and the ADR index status line.
…TLS, and must be visible wherever it is honoured Four adversarial reviews of the ADR 0153 build converged on the same two defects. **`cleartext_accepted` had been threaded into `refuse_verify_off`.** A `verify_tls=false` hop is encrypted-but-unauthenticated, not cleartext, so ADR 0153 — scoped to "the cleartext transport-hop decision only" — does not govern it. It broke in both directions at once: an enforcing hop that REFUSED before now CROSSED on a declaration (a loosening ADR 0092 decision 5 forbids), while the clamped MEFOR_ALLOW_INSECURE_TLS escape silently stopped relaxing a non-enforcing one — a behaviour change to an escape that docs/SECURITY-LOOSENING.md asserts is unaffected. It also attached an operator's written "this peer cannot do TLS" reason to a peer that plainly does, and split the HTTP family from MLLP/FTPS on the same question. The cell now keeps its pre-0153 clamped escape (`weakened_tls=True` in `_shipped_strict_disposition`) and the declaration does not reach it; the only 0153 change that still does is the deleted `not is_phi` ALLOW arm, which can only tighten. **A `FhirLookup` read connection honoured the declaration with no way to author it and no way to see it.** `FhirLookupExecutor` read the pair off its spec settings, but it was not a `FhirLookup()` parameter and `accepted_cleartext_hops` walked only `registry.outbound`. So a cleartext PHI-READ hop could cross on a hand-written settings key that skipped the mandatory-reason rule and appeared in none of `messagefoundry check`, `security_loosenings()` or `GET /security/posture` — exactly the deviation-the-registry- cannot-see the governing rule forbids. The pair is now a factory parameter, coherence-checked at that one surface, and the shared reader walks `registry.fhir_lookups` too (entries named `fhir_lookup:<name>`, a separate namespace). Also: * the SMART token endpoint still read the RAW, unclamped `insecure_tls_allowed()`, so one process-wide env var put a signed `client_assertion` on cleartext http even on an enforcing PHI instance, invisible to every loosening surface. Routed through `refuse_cleartext_credential_hop` like its OAuth2 sibling — a tightening, and the last connection-scoped cleartext cell where the variable was still alive; * the acceptance record named no connection, though `cell` is a static family label and the HTTP-family message carries a host with no port — so with two destinations to one host it could not be traced back to the declaration that produced it. The name is now threaded to every cell that can know it, and mirrored (`cleartext_connection`) into the deep settings-driven seams alongside the pair; * `GET /security/posture` reported an empty cleartext list with NO marker when the engine has no loaded graph. It now carries `loosenings_scope`, the same discipline `security show` follows. Engine UI seam 14 -> 15 (v14 shipped in v0.3.2, so it is bumped rather than corrected in place); * `security show`'s bare `except Exception` narrowed to the four ways a settings file actually fails to resolve, and a MISSING file no longer reports itself as a degraded read — the shipped defaults ARE the effective posture there; * `security_loosenings()`'s docstring claimed "every security-relevant switch". It now states its real scope — every `[security]` switch plus three enumerated deviations outside it — and names the `[store]`/`[auth]` switches it does not yet reach.
…e wiring had no detector The scope corrections, each as a test that can fail: * `verify_tls=false` keeps the clamped global escape and is NOT relaxed by `cleartext_accepted` in any posture (the loosening this branch had introduced); * the three `..._allowed_with_escape` cleartext tests are renamed `..._allowed_when_accepted` — their bodies stopped exercising the escape, and a name that asserts a guarantee the body no longer checks is worse than no test; * the SMART token endpoint refuses under enforce WITH the env var set, and crosses on the per-connection declaration; `token_provider_from_settings` reads the mirrored pair; * a declared `FhirLookup` is named by `accepted_cleartext_hops`, and its factory enforces all three flag/reason coherence rules; * the acceptance record names the declaring connection (parametrised over every raw cell). Four surfaces had production wiring with no end-to-end proof, verified by mutation — each of these reds when its production line is disabled: * `create_managed_app`'s `app.state.auth_settings` stash. The existing route test sets the attribute by hand, so it could not fail if the stash regressed and the auth deviation went unreported. Driven through the real lifespan (mutation-checked: disabling the stash reds it); * `_dest_config`'s settings MIRROR — the only path a graph-authored declaration takes to the Digest / OAuth2 / SMART / forward-proxy seams. Its own tests hand the settings dict in, and the desugaring test asserts only the typed `Destination` fields; * `security show`'s `loosenings_scope` / `loosenings_partial` markers, which had zero test references while being the stated mitigation for that command's blindness; * rule 3 of the attestation retro-fit reaching `[logging].forward_hop_attested`, previously only claimed in a sibling test's docstring. `test_toml_rejects_the_pair_under_settings` now drives the LOADER instead of asserting a factory signature — the signature check would still pass if `_build_spec` ever grew a pass-through for unknown keys, which is the drift it exists to catch. The `[security]` completeness floor gains a `[store]`/`[auth]` sibling with an explicit, reasoned exemption set. It does not close the gap (extending the registry over those switches is separate work) but it makes the gap a written decision: a NEW bool in either section reds the test until it is either reported or exempted with a reason.
…escribing controls that do not exist
ADR 0153 said two different things about `rest._shipped_strict_disposition` — the
*Explicitly out of scope* table (a ratified scope decision) said "unchanged", Build note 1
seventy lines later said it had been re-keyed. The table now records what actually
happened, and Build note 1 is split: the floor IS re-keyed for the cleartext cells, and
the `verify_tls=false` cell it also serves is genuinely unchanged. Build note 3's claim
that the declaration reached "the OAuth2/SMART token endpoints" was false for SMART until
this branch made it true, and says so. A fourth build note records the `FhirLookup`
authoring surface. The Consequences bullet offering `[security].enforcement = warn` as the
alternative to a declaration is corrected: it works for the raw transports only — the HTTP
family's no-loosen floor turns that WARN back into a REFUSE.
`docs/CONNECTIONS.md` presented "exactly three ways such a hop crosses" and listed
`tls_hop_attested` as one. There is no authoring surface for it on a connection — no
factory takes it, it is not a `connections.toml` key — so an operator following that row
gets a hard load error, and the only row that loads is `cleartext_accepted`. They would
then declare "this hop is NOT secure" about a hop that is, inverting the one distinction
ADR 0153 says the audit trail exists to preserve. The table now lists the routes that
work, and the attestation is described as what it is: present in the engine, unreachable
from config, owed. Also: `Http()` removed from the "declare it here" transport list (it is
an inbound listener with no hop to declare), `Ftp()` qualified to the anonymous case, and
`FhirLookup()` added.
`docs/SECURITY-LOOSENING.md`: the `verify_tls=false` claim ("weakened-TLS refusals are
unaffected") is true again now the code matches it, and is made explicit; the audit-record
bullet says what is actually emitted (a WARNING log line naming the connection, cell, host
and reason — not a tamper-evident audit row, which pure `config/` code cannot write); the
`aad_bind` keyless bullet no longer claims the registry stays silent, because it does not;
the register gains a scope paragraph naming the `[store]`/`[auth]` switches it does not
cover, so the gap is visible rather than implied.
`docs/CONFIGURATION.md`: `[logging].forward_hop_attested_reason` is now MANDATORY with the
flag (the retro-fitted rule reaches that sibling), not "only valid with". ADR 0092's
amendment banner records that the escape survives for the weakened-TLS cells too, not only
the six non-connection ones.
PR #38 flipped [store].aad_bind on, making mfenc:v2 the default at-rest format. The v2 marker inserts an alg field ahead of the key id (mfenc:v2:<alg>:<key_id>:<b64> vs mfenc:v1:<key_id>:<b64>), so ten rotation assertions that hand-rolled `split(":", 3)[2] == active_key_id` were reading "a256gcm" where they expected the fingerprint. The product is correct: Cipher.active_marker_prefix already generalises the stores' rotation scans for v2. This is a test-only fix. Each site now calls that shipped accessor instead of re-deriving the layout beside it, which is version-agnostic (it survives a future v3) and strictly stronger than what it replaces: it pins marker + version + alg + key id + the trailing ':' together, where the old form checked one field of an arbitrary colon-split and never pinned the marker or the version at all. Two shapes, because the writer differs between the groups: - The two rotate-key CLI tests are the ones actually red in CI. Their writer is built inside open_store as build_store_cipher(settings), i.e. write_v2=aad_bind, and no object the test held had that posture. The local cipher now mirrors it via make_cipher(key_b, write_v2= settings.aad_bind), reused for the verify handle and the assertion. Reading the prefix off the plain make_cipher(key_b) would still have failed - that yields the v1 prefix. - The other eight sites construct AesGcmCipher(k2, retired_keys=[k1]) explicitly, so they bypass build_store_cipher, still write v1, and pass today. Hoisting that inline cipher into a local and reading its own prefix is a robustness/parity upgrade, not a red-to-green fix; it also makes them correct under the MEFOR_TEST_FORCE_AAD_BIND=1 sweep, where today they would read "a256gcm". Comments naming the v1 layout are updated so they no longer state a format the writer may not produce. No assertion was weakened or dropped: the neighbouring startswith(MARKER_PREFIX), the NULL-detail check, and the deliberate keyless-plaintext negatives are untouched. Verification: ruff check + ruff format --check clean over tests; both suites still collect (147 Postgres + 149 SQL Server); full suite 9117 passed, 816 skipped. Not verified locally, stated plainly: the Postgres and SQL Server legs skip without a server (296 skipped, zero executed here), so no leg is claimed to pass - CI is the real test. The marker arithmetic the fix turns on was proven server-free against the real cipher: a v2 blob's split(":", 3)[2] is "a256gcm" (reproducing the CI failure), it does not start with the write_v2=False prefix (the trap), it does start with the mirrored-posture prefix, and a v1 blob satisfies both the old and the new form (so the eight green sites do not flip).
… v1-layout site Review round on f9de129. Three findings survived verification against the code; the rest of the critique was checked and found mistaken or already handled. 1. Independence, not strength (the one substantive finding). Four of the ten converted sites drew their ONLY proof that rotation moved anything from `active_marker_prefix` — the very property the stores' rotation predicate is built from (`active_like = f"{cipher.active_marker_prefix}%"` in postgres.py, sqlserver.py and store.py). The old `split(":", 3)[2] == active_key_id` form drew its expectation from a DIFFERENT property, so the two cross-checked. Concretely: were `active_marker_prefix` to regress over-broad, `NOT LIKE` would select zero rows, rotation would silently no-op, and `startswith` would pass vacuously — while the neighbouring round-trips stay green because the handle still carries the retired key. Restored by pairing each prefix check with `_fingerprint(k1) not in <value>` — derived from the fingerprint, not the prefix, so it is independent again. Sites: test_response_rotation_and_purge, test_reencrypt_skips_null_response_ detail, test_reencrypt_rotates_summary_and_metadata, test_reencrypt_rotates_error_lasterror_detail. Deliberately NOT the reviewer's suggested `reencrypt_to_active() == N`: N is not derivable from the test's own setup at three of the four, it cannot be checked locally, and guessing it wrong turns green legs red. The other six sites already carry an independent proof (`== 6`, `== 2`, and the CLI tests' key_b-alone verify handle) and are left alone. 2. An over-claim in f9de129's own message and comments: the new form was described as pinning "marker + version + alg + key id". For the eight AesGcmCipher(...) sites the cipher is write_v2=False, so the prefix is `mfenc:v1:<kid>:` and there IS no alg segment. Comments reworded to say marker + version + key id, plus the alg on v2. 3. A stale section header: the H4 block still read "route through the SAME store cipher (mfenc:v1)". With [store].aad_bind defaulting True the engine's at-rest format is v2, so naming a version there misdescribes the product. The version is dropped rather than swapped — the format follows the setting. (test_postgres_store.py:1128 is correctly left naming v1: its `c2 = make_cipher(k2, [k1])` genuinely writes v1.) Plus an 11th instance of the original defect that the `split(":", 3)` grep could not match, because it hand-builds the same assumption by CONCATENATION instead of parsing it: tests/test_transform_state.py - new_id = make_cipher(new).active_key_id - assert at_rest.startswith(f"{PREFIX}{new_id}:") + assert at_rest.startswith(rotating.active_marker_prefix) `f"{PREFIX}{new_id}:"` IS the v1 branch of active_marker_prefix, open-coded, in a rotation test — the same intent as the nine rotation sites already converted. It is green in CI today only because its writer is an explicit v1 cipher. Unlike the SQL Server and Postgres legs, this one RUNS locally, so it is the only part of either round with direct execution evidence. Proved both ways: under MEFOR_TEST_FORCE_AAD_BIND=1 the old form fails with exactly the CI signature ("mfenc:v2:a256gcm:<kid>:..." vs "mfenc:v1:<kid>:") and the new form passes; both pass in the default posture. Verification: ruff check + ruff format --check clean over tests (531 files); collection unchanged (147 Postgres + 149 SQL Server = 296, 23 transform-state). The added assertions were proved server-free against the real cipher: across 4,800 fresh tokens per key pair, in BOTH v1 and v2, the retired fingerprint never appears in an active-key value (so the addition cannot flake), while an un-rotated value both fails the prefix check and still contains it — including under a simulated over-broad-prefix regression, where only the new check fires. Not verified locally, stated plainly: the SQL Server and Postgres legs skip without a server (296 collected, 0 executed). No claim is made that they pass; CI is the only real test for those nine sites. Known, pre-existing, out of scope: under the MEFOR_TEST_FORCE_AAD_BIND sweep test_state_value_encrypted_at_rest_and_read_back still fails on a v1-only `startswith(PREFIX)` "is it encrypted at all" check, as do ~14 similar siblings in the two server suites. Untouched here, and no CI job sets that flag.
wshallwshall
added a commit
that referenced
this pull request
Jul 29, 2026
…x, and pin the release toolchain (#48) * test(crypto): sweep the "encrypted at all" at-rest assertions off the frozen v1 prefix `[store].aad_bind` now defaults True (#38), so the default at-rest format is mfenc:v2. 41 test assertions anchored on `PREFIX` ("mfenc:v1:") whose actual claim is "this value is enciphered at rest at all" — a v1-only spelling that a v2 value silently fails. They move to the version-agnostic `MARKER_PREFIX` ("mfenc:"), which is the same anchor the store's own find-all/migration `mfenc:%` LIKE patterns and `is_encrypted()` already use. Evidence, not assertion. Under the ASVS correctness net `MEFOR_TEST_FORCE_AAD_BIND=1` (conftest forces every AesGcmCipher to the v2 writer), the swept files went 9 failed -> 1 failed. The one remaining failure is `test_store_aad_binding::test_v1_rows_still_read_under_aad_bind`, a site this commit deliberately does NOT touch: it fails on both branches because forcing v2 destroys its v1 premise, which is exactly what makes it the liveness receipt that the flag is live. DELIBERATELY LEFT as v1 (CRYPTO-1 / ADR 0019 coverage a sweep would DELETE), each now carrying a comment saying so — their absence is what made this dangerous to review: - the frozen-writer set (test_store_encryption: the _v1_blob oracle, the byte-identity gate, test_default_writer_is_v1_not_v2) - premise assertions whose point is "the row really IS v1" so a dual-read/no-rotation proof cannot go vacuous (test_v2_active_decrypts_v1_without_rotation, test_store_aad_binding, test_keyprovider x2, test_keyprovider_vault) - field-POSITION pins, where "mfenc:<fp>:" is a string no writer emits, so widening makes the positive unsatisfiable and the negative unfailable (test_key_id_is_a_fingerprint_not_zero, the legacy key_id='0' fixture) - undecryptable-payload injections (test_batch_claim_fifo, test_claim_fifo_heads): the marker version must be one the cipher DISPATCHES on, or the poison takes the unknown-version branch instead of the base64 failure the test is about Three checks got strictly stronger rather than merely wider: - the rotation assertion now anchors on `cipher.active_marker_prefix`, keeping the "re-encrypted under the ACTIVE key" half of the proof that a bare MARKER_PREFIX would have dropped - three NEGATIVE leak/plaintext checks (`not startswith` / `not in`) now exclude EVERY marker version; as v1-only they would have passed silently on a leaked or already-encrypted v2 value - test_ack_sent_store's manual decrypt now passes the cell AAD the store wrote with. Pre-existing and previously MASKED by the v1 assertion failing first; unmasked once the sweep let the test reach it. Harmless on v1 (that reader ignores the caller's aad by design). Comments naming "v1"/"the v1 marker" at swept sites were updated so they no longer lie. Verification (SQLite leg, from the worktree): 9135 passed, 816 skipped, 0 failed; ruff check + ruff format --check clean. Non-vacuity proven by injecting one regression (AesGcmCipher.encrypt returns plaintext): two swept assertions FAIL on a plaintext at-rest value and pass without it. * ci(supply-chain): pin the release-path toolchain + guard the pins against silent rot Five installs on the release path resolved whatever PyPI served at tag time (Scorecard PinnedDependenciesID; ADR 0034 §3). The `release` job holds contents/id-token/attestations: write and `release-harness` holds contents/id-token: write, so these ran with the OIDC identity that signs and publishes. The sharp one is `sigstore`: its step carries no `if:`, so it runs on every tag AND every dispatch, and the very next command signs the wheel, sdist, SBOM and VEX with that identity. release.yml sigstore -> ==4.4.0 release.yml pip + build (x2: engine and harness jobs) -> ==26.1.2 / ==1.5.0 release.yml cyclonedx-bom -> ~=7.3.1 release.yml packaging (x2) -> ==$PKG_PIN, derived from constraints.lock security.yml pip + cyclonedx-bom -> ==26.1.2 / ~=7.3.1 (keeps it the true SBOM twin) Three judgement calls, none of them the obvious choice: 1. sigstore 4.4.0, NOT the newer 4.5.0. .github/dependabot.yml sets a 5-day supply-chain cooldown whose stated purpose is dodging a package compromised shortly after publish; 4.5.0 is <48h old. Hard-pinning the SIGNING toolchain to an artifact fresher than the repo's own routine-update policy allows would invert that policy at the highest-privilege point in the pipeline. 4.4.0 has aged 23 days. Re-evaluate once 4.5.0 clears the window. 2. `packaging` is DERIVED from constraints.lock, not hardcoded. It IS a DEP-1 dependency (requirements.lock + constraints.lock both pin it at 26.2), so a literal would drift on the next Dependabot bump. Same run-time-read pattern as quality-advisory.yml's ruff pin, but FAIL-CLOSED instead of falling back to an unpinned fetch — a soft fallback on the release path defeats the pin exactly where it matters. Both installs also move ABOVE their `GITHUB_REF_TYPE = tag` guard so a workflow_dispatch dry-run exercises them; they were the only two of the five a dispatch could not reach, and an install that never runs before the tag cannot be validated before it. 3. ~=7.3.1 rather than ~=7.3 for cyclonedx-bom: the looser form floats the whole 7.x minor range, and a 7.4 could change the JSON shape scripts/security/sbom_finalize.py parses — which exits non-zero and FAILS the release. ~=7.3.1 still takes patch fixes. This is residual-risk reduction, NOT an alert closure: ADR 0034 §3 shows from this repo's own data that exactly-pinned tools (bandit==1.9.4, zizmor==1.5.2) are still flagged while --require-hashes installs are not. Pinning `sigstore` also pins the TOP only — its ~30 transitive deps still float at signing time. Closing it needs the hashed release-tools lock (ADR 0034 option B), which is an owner call and is deliberately NOT built here. GUARD: a pin nothing watches rots back to unpinned, and Dependabot has no updater for an inline `pip install X==Y` in a workflow (its `uv` ecosystem reads only pyproject.toml + uv.lock), so both a stale pin and a DELETED pin are invisible. tests/test_ci_venv_pinning.py gains two tests: a BLANKET scan of every pip-install target in release.yml (so a NEW unpinned install added tomorrow fails too — the case a fixed name list cannot see), plus a non-vacuity table asserting each tool is still installed and still pinned at EVERY occurrence. Guard proven by injecting one regression at a time rather than trusting a green run: un-pin sigstore -> 2 failed (blanket scan + the named backstop) DELETE the install -> 1 failed (only the backstop can see this, as designed) un-pin 1 of 2 `build` -> 3 failed (proves every occurrence is checked, not just the first) restored -> 12 passed Both workflows re-parsed as YAML. 192 passed across every test that reads .github/workflows. Line-neutral where ADR 0034 requires it: the scanner re-fires a dismissed alert under a NEW number when a line moves, so each pin was made in place. Not done: security.yml's `pip uv` (line 55) and `pip-audit` (81) stay unpinned — the DEP-1 job is contents: read and outside the release path. * test(crypto): finish the cell-AAD sweep and correct the rationale the sweep recorded Follow-up to 09767bf. The sweep widened the "encrypted at rest" assertions correctly, but review found five things it got wrong or left half-done. 1. Five more bare `decrypt()` calls on cell-bound columns (the defect the sweep fixed once, in test_ack_sent_store). Each fails closed on an mfenc:v2 value and was only invisible because the v1 prefix assertion above it failed first: - test_store_encryption x3, queue.last_error (store.py binds cell_aad("queue","last_error", <row id>)) - test_sqlserver_store x1, response.body (sqlserver.py:4326) - test_postgres_store x1, response.body (postgres.py:3668) The SS/PG pair skip without a live DB, so they surface only in CI. Both now read destination_name back from the row rather than rebuilding the "\x1fack:" sentinel. 2. Seven new comments asserted a causal link that does not exist: "the at-rest format follows [store].aad_bind (v2 by default), so a v1-only prefix would fail here". aad_bind is read in exactly one place -- base.py:1736, make_cipher(..., write_v2=settings.aad_bind), reachable only via build_cipher/open_store. Every one of these tests builds its own cipher, and make_cipher's write_v2 parameter still defaults False, so they observe mfenc:v1: in a normal run and the setting has no influence on them. Reworded to the true reason: the marker version is the cipher's business, not the assertion's. 3. Two poison-injection comments named the wrong failure mechanism. Measured: 'mfenc:v1:not-base64-$$$' -> ValueError: Nonce must be between 8 and 128 bytes 'mfenc:not-base64-$$$' -> CipherError: unknown at-rest marker version `_parse` splits on the second colon, which is absent, so the blob is empty and b64decode("") succeeds -- the raise comes out of AESGCM, not the base64 path. The decision to keep these two at v1 was right; only the stated mechanism was wrong. 4. test_store_encryption's M9 section claimed "WRITES v1 BY DEFAULT" / "the shipping default never emits a v2 marker" in the same file where the sweep added six "v2 by default" comments. ADR 0148 falsified the prose, not the tests: they pin make_cipher's PARAMETER default (still False), while the shipped store writes v2 via build_cipher. Both defaults now named separately. 5. test_bytes_per_message_amplification documented the at-rest shape as mfenc:v1:<key_id>:<b64>. Receipt (real numbers, run from this worktree). Default mode over the 11 edited modules: 166 passed, 351 skipped. Forced-v2 leg (MEFOR_TEST_FORCE_AAD_BIND=1) over all 60 test modules that import store.crypto -- the scanned set, not a 12-file window: 10 failed, 1159 passed, 359 skipped, down from 13 failed. The 3 that stopped failing are exactly the last_error sites in (1). All 10 remaining are deliberate-v1 sites that MUST red under a forced v2 writer: the four CRYPTO-1 frozen-writer pins, test_active_marker_prefix_v1_and_v2, test_key_id_is_a_fingerprint_not_zero, and the four v1-premise dual-read tests. No messagefoundry/ change. ruff check + ruff format --check clean over tests/. * ci(supply-chain): make the pin guard actually reject a non-pin, and finish the ADR 0034 rows Follow-up to 9bf7178. The pins were right; the guard protecting them was not, and two of the four ADR 0034 rows were only half-applied. THE GUARD ACCEPTED `>=` AS A PIN. `_VERSION_OPS` listed ("==", "~=", ">=", "<=", "!="), so `pip install "sigstore>=4.4.0"` — which resolves whatever PyPI serves at tag time, byte-for-byte the exposure ADR 0034:190 calls "the highest residual in the group … runs with the OIDC identity used to publish" — passed green under a test named test_release_toolchain_pin_is_present, and its own failure text never fired. Split into _PIN_OPS ("==", "~=") for the pin decision and _SPEC_OPS (every PEP 440 operator) for matching a token to a package NAME, so an unpinned `sigstore>=…` is reported as unpinned rather than as a missing step. Three more holes in the same scan: - `"/" not in target` exempted every URL and git+ target — the one class that cannot carry a pin at all. `pip install https://…/evil.whl` inserted before the Sigstore step scored ZERO failures. Remote is now tested FIRST and always reported; only local path shapes are exempt. - `line.split(" install ", 1)[1]` disagreed with the `\bpip\s+install\b` regex that selected the line, so `pip\tinstall` raised IndexError instead of asserting. Both now anchor on one regex, which also picks up `pip3 install` and `pip --quiet install`. - The vacuity floor said >= 6 while the file has 8 pip installs — two steps could be deleted before the scan noticed. Floor is now the real count. ADR 0034 ROWS FINISHED: - The harness `packaging` install got the pin but not the venv. ADR 0034:192 prescribes both, and the venv is the half carrying the risk: it landed in release-harness's MAIN interpreter, and the steps after it attach the wheel to the release and publish to PyPI. Now /tmp/harnesssmoke, mirroring the engine job's /tmp/relsmoke. - The two SBOM installs had drifted apart, which quietly retired ADR 0034:205's pre-tag validation route ("run security.yml's sbom job … the install command there is byte-identical"). Nothing in PR CI executes release.yml, so that dispatch was its only dry-run proxy. Both lines are now the same command and a test keeps them that way. The register itself said "Recommended hardening — identified, NOT done" over four rows this branch had already done, and called them "an owner decision, not a drive-by". Rewritten with per-row status and the reason the owner gate is retired for them (they are PR-visible now). What is NOT closed is stated plainly: these pin only the top of each install, sigstore's ~30 transitive deps still float, and §3's own data (bandit==1.9.4 pinned and still flagged) says no pin moves the Scorecard finding — option B remains the only thing that does, and remains an owner call. New guards: security.yml's unpinned installs are a REGISTERED set (pip/uv/pip-audit — contents:read, schedule-only, nothing anyone installs) so a new one there still reds; the SBOM twin-identity check; and a PR-time canary that constraints.lock still carries exactly one `packaging==` line, because release.yml derives that pin at run time and `exit 1`s without it — on the tag push. RECEIPTS. Guard mutation-tested against a TEMP COPY of the workflows (repo never modified), one regression at a time — control 0 failures, no crashes: sigstore == -> >= 2 blanket_scan + named_backstop build == -> >= (1 of the 2 installs) 2 blanket_scan + named_backstop[build] sigstore install deleted 2 blanket_scan + named_backstop new unpinned named install 1 blanket_scan new https://…whl install 1 blanket_scan (scored 0 before this commit) new git+ install 1 blanket_scan packaging pin -> bare 2 blanket_scan + named_backstop[packaging] TAB / pip3 / flag-before-subcommand 2 each (was an IndexError crash) SBOM twin drift 1 sbom_twin security.yml gains an unpinned target 1 security_registry --upgrade-deps / lost --require-hashes 2 / 1 constraints.lock loses packaging== 1 packaging_canary Both workflows re-parse as YAML; permissions, triggers and job lists are byte-identical to origin/main (checked, not assumed). 195 passed / 10 skipped across all 14 test modules that read .github/. No messagefoundry/ change. * ci(release): pin the console job's toolchain, which the new guard caught The console publish job landed while this branch was open and installs pip, build and packaging unpinned. The guard added earlier on this branch rejects exactly that, so updating the branch against main turned all three test legs red -- the guard catching a real regression written after it was, which is the best evidence it works that this branch could have produced. pip and build take the same explicit pins the sibling build jobs use. packaging is derived from constraints.lock the way relsmoke and harnesssmoke already do, so one lock bump moves every release-path packaging install together instead of letting them drift apart. Also adds the check the pin guard structurally cannot make. Getting here, an edit put a literal 0x01 byte in release.yml -- a sed backreference written into a non-raw Python string -- and the file became unparseable YAML that GitHub Actions could never have run. The pin guard passed green through it, because it greps lines and never parses the document. So the file is now verified by parsing it, and its permissions blocks and triggers are compared against origin/main as parsed structures rather than eyeballed: jobs release, release-webconsole, release-harness; permissions identical; triggers identical. Verification: pin guard 15 passed; full suite 9331 passed, 818 skipped, 1 failed. That failure is tests/test_gate_installed_parity.py, which is PRE-EXISTING and unrelated -- it reproduces identically on clean main, this branch touches neither the gate source nor that test, and the test is LOCAL-MACHINE only (it skips on CI, where no installed gate exists). It reports that this workstation's installed ~/.claude/hooks copy is older than the source that PR #36 merged; re-running install-gate.ps1 is the fix, and that is deliberately an owner action because it activates the EnterWorktree rule for every session on the box.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements ADR 0153 (Accepted 2026-07-25, previously
NOT yet built) and finishes the posture-default lane.ADR 0153
insecure_hop_dispositionno longer reads the data label at all — verified, its signature is now(enforcing, is_loopback_hop, hop_attested, cleartext_accepted). No data label can permit a cleartext hop. Per-destinationcleartext_accepted/cleartext_reasonland as WARN + audited, kept deliberately distinct fromtls_hop_attested.No TLS default was flipped — verified (
weakened_tls: bool = Falsethroughout), and pinned bytest_no_transport_factory_flips_its_tls_default. That was explicitly sized and rejected upstream because it hard-fails 14/14 inbound MLLP listeners.Beyond the ADR's letter, the build also closed
transports/smart.py— the last connection-scoped cell still reading the raw unclampedMEFOR_ALLOW_INSECURE_TLS— and extended the declaration toFhirLookupread connections, which the ADR did not enumerate.Posture defaults (one posture, loosen only)
[store].aad_bindFalse→True;[auth].ad_session_recheck_seconds0→300, with the cross-field refusal keyed onmodel_fields_setso the shipped default is inert without AD but an explicitly typed value still refuses — a naive flip would have failed startup on every non-AD deployment.security_loosenings()now takes(sec, store, auth, cleartext_hops), all required: an optional parameter is a detector that can silently fail to fire, and a deviation the registry cannot see is a second posture by the back door.[logging].forward_hop_attested = truewithout a reason now refuses. Clean in this repo (0 occurrences insamples/,harness/,ide/, docs) — but the estate config repo could not be grepped from this worktree. Check it before merging.SecurityPosture.loosenings_scope). The webconsole seam list and golden snapshot are updated here, but a console wheel built from an older tag will refuse to start against this engine — that is the handshake working as designed, and it makes this a lockstep release.Known gaps, stated rather than buried
tls_hop_attestedstill has no authoring surface on a connection — deliberately not wired, since that would add a silent-ALLOW per-connection loosening needing its own ratification. Docs that told operators to use it are corrected; recorded as owed.[store]/[auth]switches; they are enumerated as explicit exemptions in a floor test rather than left implicit.aad_bindflip changes whatopen_storewrites on every backend — those CI legs are the real test.Verification
ruff clean · ruff format clean · mypy 21 errors, exactly the pre-existing set, 0 new · pytest 9105 passed, 816 skipped, 1 failed.
That one failure is
test_anon_parity, which is not from this branch (git diff --name-only -- messagefoundry/anon teereturns 0 files) and was fixed independently in #35, now merged.