diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7ae0c4cb..fad928d8 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -96,7 +96,12 @@ jobs: - name: Build sdist + wheel run: | - python -m pip install --upgrade pip build + # PINNED (Scorecard PinnedDependenciesID; ADR 0034 §3). This runs in the job holding + # contents/id-token/attestations: write, and `build` produces the artifact that is signed and + # published — an unpinned resolve here picks whatever PyPI serves at tag time. Neither tool is + # in any DEP-1 lock, so `==` is drift-free (and Dependabot cannot bump an inline workflow + # install: re-check these by hand when bumping, tests/test_ci_venv_pinning.py keeps them pinned). + python -m pip install "pip==26.1.2" "build==1.5.0" python -m build ls -l dist/ @@ -134,9 +139,21 @@ jobs: # the module attribute and the wheel filename could not all be canonical at once. Version() # normalises both sides, so canonical "0.3.0rc1" in __init__.py matches tag v0.3.0-rc1 and the # check still fails loudly on a genuine mismatch. + # `packaging` pin DERIVED from constraints.lock, never hardcoded — it IS a DEP-1 dependency + # (requirements.lock + constraints.lock both pin it), so a literal here would drift silently on + # the next Dependabot bump. Same run-time-read pattern as quality-advisory.yml's ruff pin, but + # FAIL-CLOSED rather than falling back to an unpinned fetch: this is the release path. Installed + # OUTSIDE the tag guard so a workflow_dispatch dry-run exercises the pinned install — the guard + # below is what stays tag-only, and an install this step never reaches cannot be validated + # before the tag that depends on it. + PKG_PIN="$(sed -n 's/^packaging==\([^ ;]*\).*/\1/p' constraints.lock | head -1)" + if [ -z "$PKG_PIN" ]; then + echo "::error::no packaging== pin in constraints.lock — refusing an unpinned install on the release path"; exit 1 + fi + echo "packaging pin from constraints.lock: $PKG_PIN" + /tmp/relsmoke/bin/pip install --quiet "packaging==$PKG_PIN" if [ "${GITHUB_REF_TYPE:-}" = "tag" ]; then want="${GITHUB_REF_NAME#v}" - /tmp/relsmoke/bin/pip install --quiet packaging /tmp/relsmoke/bin/python - "$built" "$want" <<'PYVER' import sys from packaging.version import InvalidVersion, Version @@ -180,7 +197,14 @@ jobs: # The core lock is the honest closure a `pip install messagefoundry` pulls (the all-extras # requirements.lock drags PySide6/dev tooling the wheel never requires); pip-audit still audits # the all-extras set. See docs/SUPPLY-CHAIN.md + ADR 0149. - python -m pip install --upgrade "cyclonedx-bom~=7.3" + # ~=7.3.1 (not ~=7.3): the looser form floats the whole 7.x minor range, and a 7.4 could change + # the CycloneDX JSON shape sbom_finalize.py parses — which exits non-zero and FAILS the release. + # ~=7.3.1 still takes patch fixes and keeps the lxml-6.x/cp314 floor rationale above intact. + # BYTE-IDENTICAL to security.yml's SBOM install, and kept that way by a test. Nothing in PR CI + # executes release.yml (tag push only), so ADR 0034's documented pre-tag check is to dispatch + # security.yml's sbom job and read ITS log — which only proves anything while the two install + # commands are the same command. + python -m pip install "pip==26.1.2" "cyclonedx-bom~=7.3.1" # No unpinned pip bootstrap: --require-hashes resolves nothing (tests/test_ci_venv_pinning.py). python -m venv /tmp/sbomenv /tmp/sbomenv/bin/pip install --require-hashes -r docker/locks/requirements-core.lock @@ -219,7 +243,16 @@ jobs: - name: Sign artifacts with Sigstore (keyless, GitHub OIDC) run: | - python -m pip install sigstore + # PINNED — the sharpest of these installs: this step is unconditional (every tag AND every + # dispatch) and the very next command signs the release artifacts with the job's OIDC identity, + # the same identity that publishes to PyPI below. 4.4.0, not the newer 4.5.0: .github/ + # dependabot.yml sets a 5-day supply-chain cooldown to dodge a package compromised shortly + # after publish, and 4.5.0 is <48h old — hard-pinning the SIGNING toolchain to a fresher + # artifact than the repo's own routine-update policy allows inverts that policy at the highest- + # privilege point in the pipeline. Re-evaluate to 4.5.0 once it has aged past the window. + # NOTE: this pins the TOP only; sigstore's ~30 transitive deps still float at signing time. + # Closing the Scorecard alert outright needs the hashed release-tools lock (ADR 0034 option B). + python -m pip install "sigstore==4.4.0" # Sign the wheel + sdist AND the SBOM + VEX, so an operator can verify the provenance of the # bill-of-materials and the exploitability assessment too — not just the code artifacts (ADR 0149). python -m sigstore sign dist/*.tar.gz dist/*.whl \ @@ -346,7 +379,10 @@ jobs: - name: Build the console wheel (wheel-only — the package tree is force-included from the repo root) run: | - python -m pip install --upgrade pip build + # Pinned like every other build job on this path: this workflow's jobs hold contents/id-token + # write and publish the release artifacts, so an unpinned resolve takes whatever PyPI serves at + # tag time (Scorecard PinnedDependenciesID, ADR 0034 §3). Guarded by tests/test_ci_venv_pinning.py. + python -m pip install "pip==26.1.2" "build==1.5.0" # Wheel-only for the same reason as the harness: messagefoundry_webconsole/ lives OUTSIDE this # project dir and is pulled in via force-include, so an sdist would not be self-contained. python -m build --wheel ./packaging/messagefoundry-webconsole --outdir webconsole-dist @@ -361,7 +397,13 @@ jobs: # (tag webconsole-v0.3.0-rc1 -> 0.3.0rc1), so a string compare could never match a pre-release. if [ "${GITHUB_REF_TYPE:-}" = "tag" ]; then want="${GITHUB_REF_NAME#webconsole-v}" - python -m pip install --quiet packaging + # Derived from the lock, exactly as the relsmoke/harnesssmoke jobs do, so one bump moves + # every release-path packaging install together instead of drifting apart. + PKG_PIN="$(sed -n 's/^packaging==\([^ ;]*\).*/\1/p' constraints.lock | head -1)" + if [ -z "$PKG_PIN" ]; then + echo "::error::no packaging== pin in constraints.lock — refusing an unpinned install on the release path"; exit 1 + fi + python -m pip install --quiet "packaging==$PKG_PIN\" python - "$built" "$want" <<'PYVER' import sys from packaging.version import InvalidVersion, Version @@ -464,7 +506,9 @@ jobs: - name: Build the harness wheel (wheel-only — harness/ is force-included from the repo root) run: | - python -m pip install --upgrade pip build + # PINNED, same rationale as the engine's build step: this job also holds contents/id-token: + # write, and it publishes the harness wheel when PUBLISH_HARNESS is set. + python -m pip install "pip==26.1.2" "build==1.5.0" # Wheel-only on purpose: the harness source (harness/) lives OUTSIDE this project dir (it is # force-included from ../../harness), so an sdist would not be self-contained. Pure-Python, so a # wheel suffices. Version is read from messagefoundry/__init__.py (lockstep with the engine). @@ -481,10 +525,26 @@ jobs: # and a string compare could NEVER match — this job failed on every pre-release tag by # construction, whatever __version__ said. With PUBLISH_HARNESS=true it also runs after the # engine has already uploaded, so the failure would land half-published. + # Pin DERIVED from constraints.lock + installed outside the tag guard — see the engine job's + # wheel smoke above for the full rationale. (`build` already pulls packaging>=24.0 into this + # same interpreter, so the install is near-redundant; pinning it beats deleting it, which would + # leave the dependency implicit and unpinned via build's own resolve.) + # + # …and into a THROWAWAY VENV, not this job's interpreter, which is the other half of ADR 0034's + # recommendation. This job holds contents: write + id-token: write and the steps AFTER this one + # attach the wheel to the release and publish it to PyPI, so an install resolved into the main + # interpreter here sits inside the publishing identity. The engine job already does it this way + # (/tmp/relsmoke); the compare script needs nothing but packaging.version + stdlib. + PKG_PIN="$(sed -n 's/^packaging==\([^ ;]*\).*/\1/p' constraints.lock | head -1)" + if [ -z "$PKG_PIN" ]; then + echo "::error::no packaging== pin in constraints.lock — refusing an unpinned install on the release path"; exit 1 + fi + echo "packaging pin from constraints.lock: $PKG_PIN" + python -m venv /tmp/harnesssmoke + /tmp/harnesssmoke/bin/pip install --quiet "packaging==$PKG_PIN" if [ "${GITHUB_REF_TYPE:-}" = "tag" ]; then want="${GITHUB_REF_NAME#v}" - python -m pip install --quiet packaging - python - "$built" "$want" <<'PYVER' + /tmp/harnesssmoke/bin/python - "$built" "$want" <<'PYVER' import sys from packaging.version import InvalidVersion, Version diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index b7328c43..c56e696a 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -134,8 +134,13 @@ jobs: # `requirements ` parser has no metadata and emits license-less components. The core lock # (docker/locks/requirements-core.lock) is the honest runtime closure `pip install messagefoundry` # pulls; the all-extras requirements.lock stays covered by the pip-audit job above. cyclonedx-bom - # ~=7.3 → lxml 6.x (cp314 wheels) so the 3.14 runner doesn't source-build lxml. ADR 0149. - python -m pip install --upgrade pip "cyclonedx-bom~=7.3" + # ~=7.3.1 → lxml 6.x (cp314 wheels) so the 3.14 runner doesn't source-build lxml. ADR 0149. + # PINNED and BYTE-IDENTICAL to release.yml's SBOM install (enforced by + # tests/test_ci_venv_pinning.py). ~=7.3.1 rather than ~=7.3 so a 7.4 cannot change the JSON + # shape sbom_finalize.py parses. This job is ADR 0034's pre-tag dry-run for the release SBOM + # step, so the two must stay the SAME command — if they drift, dispatching this one proves + # nothing about the release. + python -m pip install "pip==26.1.2" "cyclonedx-bom~=7.3.1" # No unpinned pip bootstrap: --require-hashes resolves nothing (tests/test_ci_venv_pinning.py). python -m venv /tmp/sbomenv /tmp/sbomenv/bin/pip install --require-hashes -r docker/locks/requirements-core.lock diff --git a/docs/adr/0034-static-analysis-triage-policy-accepted-risk-register.md b/docs/adr/0034-static-analysis-triage-policy-accepted-risk-register.md index 9b5e9f47..8333f3e3 100644 --- a/docs/adr/0034-static-analysis-triage-policy-accepted-risk-register.md +++ b/docs/adr/0034-static-analysis-triage-policy-accepted-risk-register.md @@ -180,21 +180,36 @@ practical, or expect to re-dismiss every anchor below it. The two workflow fixes deliberately made line-neutral — one line deleted, one comment line added — which is why their rationale lives in `tests/test_ci_venv_pinning.py`'s module docstring rather than in the workflow. -### Recommended hardening — identified, NOT done +### Recommended hardening Recorded here because a `won't fix` dismissal makes an item invisible, and these were found *while* -justifying those dismissals. None of them closes its alert; each reduces residual risk. - -| Where | Recommendation | Why it matters | -|---|---|---| -| `release.yml` `pip install sigstore` | Pin `sigstore==` | The **highest residual in the group**: a completely unpinned install inside the job holding `contents: write` + `id-token: write` + `attestations: write`, resolved immediately before it signs the wheel, sdist, SBOM and VEX. A malicious release fetched at that moment runs with the OIDC identity used to publish. | -| `release.yml` `pip install --upgrade pip build` | Pin `build==` | Unpinned PEP 517 frontend that produces the published wheel/sdist. | -| `release.yml` `pip install --quiet packaging` (harness job) | Pin `packaging==`; install into a throwaway venv as the engine job already does | Resolved into the **publishing** job's main interpreter rather than a scratch venv. | -| `release.yml` `pip install --quiet packaging` (`/tmp/relsmoke`) | Pin `packaging==` | Contained (disposable venv, version-compare only), but free to pin. | -| `dependabot-auto-merge.yml` `security-events: read` | Remove the scope | Dead. Its comment claims it reads Dependabot alerts, but the gate calls the **global** `/advisories` endpoint, which is repo-scope-independent. Verified; least-privilege hygiene only. | - -`sigstore`/`build`/`packaging` pins touch the **release critical path**, which no PR CI leg executes — -see below — so they are an owner decision, not a drive-by. +justifying those dismissals. **None of them closes its alert** (see §3 — a version pin does not satisfy +`PinnedDependenciesID`); each reduces residual risk. + +**Status update 2026-07-29 — the four `release.yml` rows below are DONE.** They were built together +with the guard that keeps them, and the "owner decision, not a drive-by" note that used to close this +section is retired for them: it argued the pins are unvalidatable before a tag, and the answer was to +make them PR-visible instead. The `dependabot-auto-merge.yml` scope row is still open. + +| Where | Recommendation | Status | Why it matters | +|---|---|---|---| +| `release.yml` `pip install sigstore` | Pin `sigstore==` | **Done** — `sigstore==4.4.0`. Deliberately *not* the newer 4.5.0: `.github/dependabot.yml` sets `cooldown.default-days: 5`, 4.5.0 was <48 h old, and pinning the *signing* toolchain fresher than the repo's own update policy allows would invert that policy at the highest-privilege point. Re-evaluate once it ages out. | The **highest residual in the group**: a completely unpinned install inside the job holding `contents: write` + `id-token: write` + `attestations: write`, resolved immediately before it signs the wheel, sdist, SBOM and VEX. A malicious release fetched at that moment runs with the OIDC identity used to publish. | +| `release.yml` `pip install --upgrade pip build` | Pin `build==` | **Done** — `pip==26.1.2 build==1.5.0`, in **both** the engine and harness build steps. | Unpinned PEP 517 frontend that produces the published wheel/sdist. | +| `release.yml` `pip install --quiet packaging` (harness job) | Pin `packaging==`; install into a throwaway venv as the engine job already does | **Done, both halves** — pin *derived from `constraints.lock`* (it is a DEP-1 transitive, so a literal would rot), and moved into `/tmp/harnesssmoke` mirroring `/tmp/relsmoke`. | Resolved into the **publishing** job's main interpreter rather than a scratch venv. | +| `release.yml` `pip install --quiet packaging` (`/tmp/relsmoke`) | Pin `packaging==` | **Done** — same `constraints.lock`-derived pin. | Contained (disposable venv, version-compare only), but free to pin. | +| `dependabot-auto-merge.yml` `security-events: read` | Remove the scope | **Open** | Dead. Its comment claims it reads Dependabot alerts, but the gate calls the **global** `/advisories` endpoint, which is repo-scope-independent. Verified; least-privilege hygiene only. | + +Two things the pins deliberately do **not** do. They pin only the **top** of each install — +`sigstore`'s ~30 transitive dependencies still float at signing time — and, per §3, they move the +Scorecard finding not at all. **Option B (a PEP 735 `release-tools` group flowing into `uv.lock` and a +fifth hashed export) remains the only thing that closes the alert**, and remains an owner decision +because it adds a lock artifact to the DEP-1 machinery. + +The `packaging` pins are **fail-closed on a tag**: `release.yml` `sed`s the version out of +`constraints.lock` and `exit 1`s if the line is gone. `packaging` is not a declared dependency — it +survives in that lock only as a `pytest` transitive — so +`tests/test_ci_venv_pinning.py::test_constraints_lock_still_carries_the_packaging_pin` is the PR-time +canary for a check that would otherwise first fire during a release. ### What no test can see @@ -203,4 +218,6 @@ Both workflow fixes land on paths **no PR CI leg runs**: `security.yml`'s SBOM j swallowed), and `release.yml` runs only on a tag push. So the first real execution of either edit is a nightly or **a release**. `tests/test_ci_venv_pinning.py` is a text guard over the workflow source, not an execution. Before the next tag, run `security.yml`'s sbom job via `workflow_dispatch` and read its -log — the install command there is byte-identical to `release.yml`'s. +log — the install command there is byte-identical to `release.yml`'s, and +`test_sbom_install_is_byte_identical_in_release_and_security` now enforces that identity, because the +dry-run is evidence about the release step only for as long as the two commands are the same command. diff --git a/tests/test_ack_sent_store.py b/tests/test_ack_sent_store.py index 87d8b80e..2e265841 100644 --- a/tests/test_ack_sent_store.py +++ b/tests/test_ack_sent_store.py @@ -12,7 +12,7 @@ import sqlite3 from pathlib import Path -from messagefoundry.store.crypto import PREFIX, generate_key, make_cipher +from messagefoundry.store.crypto import MARKER_PREFIX, cell_aad, generate_key, make_cipher from messagefoundry.store.store import MessageStore ADT = "MSH|^~\\&|S|F|R|RF|20260101||ADT^A01|MSG1|P|2.5.1\rPID|1||100^^^H^MR||DOE^JANE\r" @@ -22,8 +22,11 @@ def _response_rows(db_path: Path) -> list[tuple]: con = sqlite3.connect(db_path) try: + # message_id + response_seq trail the display columns so existing positional indices hold; + # they are the row half of the cell AAD the body is bound under (ADR 0019). return con.execute( - "SELECT destination_name, kind, ack_code, ack_phase, body FROM response" + "SELECT destination_name, kind, ack_code, ack_phase, body, message_id, response_seq " + "FROM response" ).fetchall() finally: con.close() @@ -47,14 +50,23 @@ async def test_aa_body_encrypted_when_store_encrypted(tmp_path: Path) -> None: ack = next(r for r in rows if r.kind == "ack_sent") assert ack.ack_code == "AA" and ack.ack_phase == "ingest" assert ack.body == AA # decrypted round-trip - # On disk the body is ciphertext, never the plaintext ACK: it carries the v1 encrypted marker, - # is not the plaintext frame, and decrypts back to exactly it. Assert the decrypt round-trip - # (deterministic) rather than `"MSA" not in ` — that old substring check flaked - # because a base64 ciphertext randomly contains that 3-char run (base64 alphabet includes M/S/A). + # On disk the body is ciphertext, never the plaintext ACK: it carries the mfenc: encrypted + # marker, is not the plaintext frame, and decrypts back to exactly it. Assert the decrypt + # round-trip (deterministic) rather than `"MSA" not in ` — that old substring check + # flaked because a base64 ciphertext randomly contains that 3-char run (alphabet includes M/S/A). + # The claim is encryptedness, so anchor on the VERSION-AGNOSTIC marker. WHICH mfenc format gets + # written is the cipher's business, not this assertion's: v1 here (make_cipher's writer default + # is still the frozen v1 one), v2 wherever the store builds its cipher through build_cipher + # (write_v2=[store].aad_bind, now on) or under the MEFOR_TEST_FORCE_AAD_BIND leg. Format pinning + # is owned by the CRYPTO-1 tests in test_store_encryption.py, not here. disk = next(r for r in _response_rows(db) if r[1] == "ack_sent") - assert disk[4].startswith(PREFIX) # stored under the encrypted marker, not in the clear + assert disk[4].startswith(MARKER_PREFIX) # under the encrypted marker, not in the clear assert disk[4] != AA - assert cipher.decrypt(disk[4]) == AA # and it genuinely encrypts the AA frame + # Decrypt under the SAME cell AAD the store wrote with (ASVS 11.3.3 / ADR 0019): a v2 value is + # bound to its (table, column, row) cell, so a bare decrypt fails closed on one. Harmless on a + # v1 value — that reader ignores the caller's aad by design (dual-read). + aad = cell_aad("response", "body", disk[5], disk[0], disk[6]) + assert cipher.decrypt(disk[4], aad=aad) == AA # and it genuinely encrypts the AA frame finally: await store.close() diff --git a/tests/test_alert_state.py b/tests/test_alert_state.py index 8c7720ac..629ef473 100644 --- a/tests/test_alert_state.py +++ b/tests/test_alert_state.py @@ -17,7 +17,7 @@ from messagefoundry.config.settings import AlertRule, AlertSeverity from messagefoundry.pipeline.alert_sinks import NotifierAlertSink -from messagefoundry.store.crypto import PREFIX, generate_key, make_cipher +from messagefoundry.store.crypto import MARKER_PREFIX, generate_key, make_cipher from messagefoundry.store.store import MessageStore # --- store lifecycle --------------------------------------------------------- @@ -193,7 +193,10 @@ async def test_reason_encrypted_at_rest(tmp_path: Path) -> None: con = sqlite3.connect(db) try: raw = con.execute("SELECT reason FROM alert_instance").fetchone()[0] - assert isinstance(raw, str) and raw.startswith(PREFIX) # ciphertext on disk + # Version-agnostic marker: the claim is "enciphered at rest", not which mfenc format wrote it. + # That belongs to the cipher — v1 from make_cipher's default here, v2 wherever the store builds + # its cipher via build_cipher (write_v2=[store].aad_bind) or under MEFOR_TEST_FORCE_AAD_BIND. + assert isinstance(raw, str) and raw.startswith(MARKER_PREFIX) # ciphertext on disk assert "refused" not in raw finally: con.close() diff --git a/tests/test_batch_claim_fifo.py b/tests/test_batch_claim_fifo.py index 044d8a37..8da17274 100644 --- a/tests/test_batch_claim_fifo.py +++ b/tests/test_batch_claim_fifo.py @@ -311,6 +311,11 @@ async def test_t8_undecryptable_interior_dead_lettered_tail_survives( channel = "IB_T8" mids = await _seed_ingress(enc, channel, [100.0, 101.0, 102.0]) # Corrupt row2's payload to an undecryptable blob (keep the marker so it routes through decrypt). + # The v1 marker is DELIBERATE (do not "sweep" it to a bare mfenc:): the version must be one the + # cipher DISPATCHES on. Measured: "mfenc:v1:not-base64-$$$" parses to key_id="not-base64-$$$" with + # an EMPTY blob (no second colon), so base64 decoding succeeds and AESGCM raises ValueError("Nonce + # must be between 8 and 128 bytes") — the poison path this test exercises. A bare "mfenc:" never + # reaches it: _parse rejects the version first with CipherError, a different fail-closed branch. await enc._db.execute( "UPDATE queue SET payload=? WHERE message_id=? AND stage=?", ("mfenc:v1:not-base64-$$$", mids[1], Stage.INGRESS.value), diff --git a/tests/test_binary_carriage.py b/tests/test_binary_carriage.py index 44cc66b8..1a455acf 100644 --- a/tests/test_binary_carriage.py +++ b/tests/test_binary_carriage.py @@ -33,7 +33,7 @@ ) from messagefoundry.pipeline.dryrun import route_only, transform_one from messagefoundry.pipeline.wiring_runner import RegistryRunner -from messagefoundry.store.crypto import PREFIX, generate_key, make_cipher +from messagefoundry.store.crypto import MARKER_PREFIX, generate_key, make_cipher from messagefoundry.store.store import MessageStore # Byte fixtures that break a latin-1/TEXT round-trip: every value 0x00–0xFF (incl. NUL + high bytes), @@ -186,7 +186,7 @@ async def test_binary_carriage_survives_encrypted_store(tmp_path: Path) -> None: record = await store.get_message(mid) finally: await store.close() - assert _raw_at_rest(db).startswith(PREFIX) # outer layer: encrypted on disk + assert _raw_at_rest(db).startswith(MARKER_PREFIX) # outer layer: encrypted on disk assert record is not None and record["raw"] == carried # decrypts to the inner mfb64: form assert RawMessage(record["raw"], "dicom").raw_bytes == DICOM_LIKE diff --git a/tests/test_bytes_per_message_amplification.py b/tests/test_bytes_per_message_amplification.py index 146c9817..f66060e6 100644 --- a/tests/test_bytes_per_message_amplification.py +++ b/tests/test_bytes_per_message_amplification.py @@ -39,7 +39,9 @@ 1. **Character width.** `queue.payload` / `messages.raw` are `NVARCHAR(MAX)` on SQL Server with no UTF-8 collation, i.e. UTF-16: **2 bytes per ASCII character.** SQLite `TEXT` is UTF-8: 1. 2. **Cipher expansion.** With `MEFOR_STORE_ENCRYPTION_KEY` set, each copy becomes - `mfenc:v1::` — roughly `4/3 * raw + ~64` bytes. Default is identity. + `mfenc:v2:::` — the shipped at-rest format since ADR 0148 + defaulted `[store].aad_bind` on; `aad_bind=false` selects the frozen `mfenc:v1::` + writer. Either way roughly `4/3 * raw + ~64` bytes. Default cipher is identity (no key set). 3. **Everything the database writes that is not the body**: row and page overhead, indexes, and above all the **transaction log**, which durably records each of the `3 + 2H + 2N` transactions. diff --git a/tests/test_ci_venv_pinning.py b/tests/test_ci_venv_pinning.py index 480b4daf..b4176f89 100644 --- a/tests/test_ci_venv_pinning.py +++ b/tests/test_ci_venv_pinning.py @@ -22,9 +22,26 @@ exactly as unpinned but is invisible to the scanner. That is ADR 0034's rejected option 3 — a visible dismissal-with-reason beats an invisible filter. -Deliberately scoped to the LOCK-ONLY venvs. `/tmp/relsmoke` (`release.yml`) legitimately installs -unpinned `packaging` — it exists to prove the freshly built wheel's own declared closure resolves, so -feeding it a lock would defeat its purpose — and is dismissed separately. Pure text checks, no network. +The lock-only venvs are one half. The other half is every OTHER `pip install` on the release path — +`build`, `sigstore`, `cyclonedx-bom`, `packaging`, and the `pip` bootstraps — which resolved whatever +PyPI served at tag time. `sigstore` is the sharp one: its step is unconditional and the very next +command signs the release artifacts with the job's OIDC identity, the same identity that publishes to +PyPI. Those are now version-pinned, and the second half of this module keeps them that way — nothing +else can see the regression, because Dependabot has no updater for an inline `pip install X==Y` in a +workflow (its `uv` ecosystem only reads pyproject.toml + uv.lock), so a stale pin rots invisibly and a +DELETED pin is invisible twice over. + +`/tmp/relsmoke` (`release.yml`) stays out of the hash-verified rule — it exists to prove the freshly +built wheel's own declared closure resolves, so feeding it a lock would defeat its purpose — but its +`packaging` install is covered by the version-pin rule below. Pure text checks, no network. + +SCOPE, stated so it is a boundary rather than an oversight: the version-pin rule is the RELEASE path. +`security.yml` keeps four `--upgrade pip` bootstraps plus unpinned `uv` and `pip-audit`; those jobs are +`contents: read`, schedule/dispatch-only, and produce nothing anyone installs. They are registered in +`SECURITY_YML_ACCEPTED_UNPINNED` instead of pinned, so a NEW unpinned install there still fails — the +exception is enumerated, not open-ended. The one `security.yml` install held to the release rule is the +SBOM step, because ADR 0034 makes it the pre-tag dry-run for `release.yml`'s and the two must stay the +same command. """ from __future__ import annotations @@ -95,3 +112,235 @@ def test_scratch_venvs_do_not_hide_an_unpinned_pip_fetch(workflow: str) -> None: f"unpinned exactly like the deleted `pip install --upgrade pip`, but the scanner cannot see it. " f"ADR 0034 requires a visible dismissal over an invisible filter." ) + + +# --- the release path: every named package must carry a version ------------------------------------ + +#: Any `pip install`, in any spelling that reaches a shell: `pip install`, `pip3 install`, +#: `python -m pip install`, `/bin/pip install`, and with flags BEFORE the subcommand +#: (`pip --quiet install X`). Matching only `pip install` would let any of the others through, and a +#: line this regex does not match is a line the scan below never examines — a silent hole, not a +#: failure. +_PIP_INSTALL = re.compile(r"\bpip3?\s+(?:-\S+\s+)*install\b") + +#: A PIN. `==` fixes the version; `~=X.Y.Z` fixes everything but the patch. `$PKG_PIN` counts — it is +#: read out of constraints.lock at run time (the quality-advisory.yml ruff-pin pattern), which is MORE +#: current than a literal, not less. +#: +#: `>=`, `<=` and `!=` are deliberately NOT here. They are FLOORS, not pins: `pip install +#: "sigstore>=4.4.0"` resolves whatever PyPI serves at tag time, which is exactly the exposure this +#: module exists to prevent (ADR 0034 §3 calls it "the highest residual in the group … runs with the +#: OIDC identity used to publish"). Accepting them would have let that regression pass green under a +#: test named `test_release_toolchain_pin_is_present`. If a genuine range is ever wanted here, exempt +#: it by name — do not widen this tuple. +_PIN_OPS = ("==", "~=") + +#: Every PEP 508/440 operator that can bind a version to a NAME. Used only to recognise that a token +#: like `sigstore>=4.4.0` is still a `sigstore` install — so an unpinned one is reported as unpinned +#: rather than as a missing step. +_SPEC_OPS = ("===", "==", "~=", ">=", "<=", "!=", "<", ">", "@", "[") + +#: Install targets that legitimately name no version: a path (the artifact under test) and the +#: hash-verified lock installs, where every version is pinned INSIDE the lock. +_EXEMPT_TARGETS = frozenset({"."}) + +#: VCS/URL scheme prefixes. A target pip fetches over the network by URL is unpinnable BY CONSTRUCTION +#: — there is no version specifier to add — so it is always reported, never shape-exempted. +_REMOTE_SCHEMES = ("git+", "hg+", "svn+", "bzr+") + +#: Tools whose release-path pin must EXIST — the non-vacuity backstop for the scan above. Deleting a +#: step would otherwise make the scan pass by finding nothing left to check. +RELEASE_PINNED_TOOLS = ( + ("release.yml", "sigstore"), + ("release.yml", "build"), + ("release.yml", "pip"), + ("release.yml", "cyclonedx-bom"), + ("release.yml", "packaging"), + ("security.yml", "cyclonedx-bom"), +) + +#: `security.yml`'s OWN unpinned installs, registered rather than pinned. That file's jobs run on a +#: schedule/dispatch with `contents: read`, no publishing identity and no artifact anyone consumes, so +#: they are off the release-path rule by decision — but registering them means a NEW unpinned install +#: added to that file still reds `test_security_yml_unpinned_installs_are_registered`. The scope call +#: is recorded here instead of being invisible. (`pip` also appears PINNED in that file's SBOM step, +#: which must stay byte-identical to release.yml's — see the twin test below.) +SECURITY_YML_ACCEPTED_UNPINNED = frozenset({"pip", "uv", "pip-audit"}) + + +def _install_targets(line: str) -> list[str]: + """The package tokens a ``pip install`` line names — flags, and the arguments of flags that take + one, removed. Everything left is something pip will resolve. + + Anchored on the same regex that selected the line, rather than splitting on a literal ``" + install "``: the literal disagrees with the regex on `pip\tinstall` and on flags placed before the + subcommand, and disagreeing means an IndexError instead of a readable failure. + """ + match = _PIP_INSTALL.search(line) + if match is None: # pragma: no cover - callers filter on the same regex + return [] + targets: list[str] = [] + skip_next = False + for tok in line[match.end() :].split(): + if skip_next: + skip_next = False + continue + if tok in ("-r", "--requirement", "-c", "--constraint", "--index-url", "--extra-index-url"): + skip_next = True + continue + if tok.startswith("-"): + continue + targets.append(tok.strip("\"'")) + return targets + + +def _is_remote(target: str) -> bool: + """A URL / VCS target. Checked BEFORE any path shape test: a remote wheel URL ends in ``.whl`` and + contains ``/`` exactly like the local artifact under test, so a shape test alone would exempt the + one class of target that can never carry a pin.""" + return "://" in target or target.startswith(_REMOTE_SCHEMES) + + +def _needs_a_pin(target: str) -> bool: + """True when pip resolves this target from an index (so it must name a version) or fetches it from + the network (so it can never be pinned and is always reported).""" + if target in _EXEMPT_TARGETS: + return False + if _is_remote(target): + return True + return "/" not in target and not any(op in target for op in _PIN_OPS) + + +def test_release_path_pip_installs_name_a_version() -> None: + """EVERY package `release.yml` installs must carry a version specifier. + + A blanket scan, not a name list, so a NEW unpinned install added tomorrow fails too — the failure + mode a fixed table cannot see. Local path installs (``dist/*.whl``) and ``-r `` installs are + exempt: the first is the artifact under test, the second is pinned inside the lock. A URL or + ``git+`` target is NOT exempt — it is unpinnable and therefore always reported. + """ + lines = [ln for ln in _code_lines(_WORKFLOWS / "release.yml") if _PIP_INSTALL.search(ln)] + # Non-vacuity: this file HAS a toolchain to pin, and the floor is the ACTUAL count, not a slack + # one — at `>= 6` two whole install steps could be deleted before the check noticed. Consolidating + # installs is fine; re-point this number in the same commit so the decision stays deliberate. + assert len(lines) >= 8, ( + f"release.yml now has only {len(lines)} pip installs — either the scan has stopped matching " + f"them or steps were removed; re-point this floor rather than letting it pass on a shrunken " + f"set.\n" + "\n".join(lines) + ) + + unpinned = [ + (ln, target) for ln in lines for target in _install_targets(ln) if _needs_a_pin(target) + ] + assert not unpinned, ( + f"release.yml installs these WITHOUT a pin: {unpinned}. This workflow's jobs hold " + f"contents/id-token/attestations: write and sign + publish the release artifacts, so an " + f"unpinned resolve here takes whatever PyPI serves at tag time (Scorecard " + f"PinnedDependenciesID; ADR 0034 §3). Pin it with `==`/`~=`, or derive the pin from " + f"constraints.lock the way the `packaging` installs do. `>=` is NOT a pin. A URL/git+ target " + f"cannot be pinned at all — install it from an index instead." + ) + + +@pytest.mark.parametrize(("workflow", "package"), RELEASE_PINNED_TOOLS) +def test_release_toolchain_pin_is_present(workflow: str, package: str) -> None: + """Each release-path tool is still installed, and still pinned wherever it is installed. + + Guards the direction the blanket scan cannot: a pin that vanishes with its step. Every occurrence + is checked, not just the first — pinning one of the two `build` installs (engine + harness) would + move the exposure rather than remove it. + """ + lines = [ln for ln in _code_lines(_WORKFLOWS / workflow) if _PIP_INSTALL.search(ln)] + hits = [ + (ln, target) + for ln in lines + for target in _install_targets(ln) + # _SPEC_OPS, not _PIN_OPS, on purpose: `sigstore>=4.4.0` must be recognised AS a sigstore + # install so the pin check below reports it. Matching on _PIN_OPS alone would read it as "the + # step is gone" — a different, misleading failure. + if target == package or target.startswith(tuple(f"{package}{op}" for op in _SPEC_OPS)) + ] + assert hits, ( + f"{workflow} no longer installs {package!r} — if the step was removed on purpose, drop it " + f"from RELEASE_PINNED_TOOLS in the same commit; otherwise this guard just went blind." + ) + unpinned = [ln for ln, target in hits if not any(op in target for op in _PIN_OPS)] + assert not unpinned, ( + f"{workflow} installs {package!r} without a `==`/`~=` pin at: {unpinned}. Dependabot cannot " + f"bump an inline `pip install` in a workflow, so an unpinned one here is never even noticed; " + f"a `>=` floor resolves to whatever PyPI serves at tag time and is not a pin." + ) + + +def test_security_yml_unpinned_installs_are_registered() -> None: + """`security.yml` is deliberately NOT held to the release-path rule — but its exceptions are a + registered set, so a new unpinned install there still fails. + + The disclosure matters: `security.yml` keeps `--upgrade pip` in four places and installs `uv` and + `pip-audit` unpinned. Those jobs are `contents: read`, scheduled/dispatch-only, and produce no + artifact anyone installs, which is why they were left alone. Recording that decision here is the + difference between a scope boundary and an oversight. + """ + lines = [ln for ln in _code_lines(_WORKFLOWS / "security.yml") if _PIP_INSTALL.search(ln)] + assert lines, "security.yml has no pip installs — this guard is no longer looking at anything" + unregistered = sorted( + { + target + for ln in lines + for target in _install_targets(ln) + if _needs_a_pin(target) and target not in SECURITY_YML_ACCEPTED_UNPINNED + } + ) + assert not unregistered, ( + f"security.yml gained unpinned install target(s) {unregistered}. Pin them, or add them to " + f"SECURITY_YML_ACCEPTED_UNPINNED with the reason — the point of the registry is that the " + f"exception is a decision someone made, not a gap nobody noticed." + ) + + +def test_sbom_install_is_byte_identical_in_release_and_security() -> None: + """The two CycloneDX installs must be the SAME command. + + Nothing in PR CI executes `release.yml` (tag push only, ADR 0034 "What no test can see"), so the + documented way to validate its SBOM step before cutting a tag is to dispatch `security.yml`'s sbom + job and read that log. That check is only evidence while the two commands are identical — the + moment they drift, the dry-run proves something about a command the release does not run. + """ + installs = {} + for workflow in ("release.yml", "security.yml"): + matches = [ + ln + for ln in _code_lines(_WORKFLOWS / workflow) + if _PIP_INSTALL.search(ln) and "cyclonedx-bom" in ln + ] + assert len(matches) == 1, ( + f"{workflow} has {len(matches)} cyclonedx-bom install lines, expected exactly 1 — " + f"re-point this twin check rather than letting it compare the wrong pair.\n{matches}" + ) + installs[workflow] = matches[0] + assert installs["release.yml"] == installs["security.yml"], ( + "the SBOM install commands have drifted:\n" + f" release.yml : {installs['release.yml']}\n" + f" security.yml: {installs['security.yml']}\n" + "ADR 0034 makes security.yml's sbom job the pre-tag dry-run for release.yml's. Keep both " + "lines identical, or replace that dry-run route with one that actually covers the release." + ) + + +def test_constraints_lock_still_carries_the_packaging_pin() -> None: + """`release.yml` derives its `packaging` pin from this line — and `exit 1`s without it, ON A TAG. + + `packaging` is not a declared dependency anywhere in `pyproject.toml`; it survives in + `constraints.lock` only as a transitive of the dev extra's test tooling (`pytest`, + `pytest-rerunfailures`). A routine Dependabot bump that drops that edge would take the line with + it, and the first thing to notice would be the tag push itself — the single most expensive moment + to discover it (release.yml's own header documents this repo's half-published v0.3.1 incident). + This is the PR-time canary for a fail-closed check that otherwise fires only during a release. + """ + body = (_REPO / "constraints.lock").read_text(encoding="utf-8") + pins = re.findall(r"^packaging==\S+", body, re.MULTILINE) + assert len(pins) == 1, ( + f"expected exactly one `packaging==` line in constraints.lock, found {pins}. release.yml " + f"resolves its pin with `sed … | head -1`, so zero lines hard-fail the next tag push and " + f"two would silently pick the first." + ) diff --git a/tests/test_claim_fifo_heads.py b/tests/test_claim_fifo_heads.py index 16ec56ce..8fd0850d 100644 --- a/tests/test_claim_fifo_heads.py +++ b/tests/test_claim_fifo_heads.py @@ -575,6 +575,12 @@ async def test_poison_rows_dead_lettered_dropped_and_lane_rearmed( key = base64.b64encode(b"\x11" * 32).decode("ascii") enc = await MessageStore.open(tmp_path / "heads.db", cipher=make_cipher(key, [])) try: + # The injected "mfenc:v1:not-base64-$$$" payloads below are undecryptable ON PURPOSE, and the + # v1 marker is DELIBERATE (do not "sweep" it to a bare mfenc:): the version must be one the + # cipher DISPATCHES on. Measured: it parses to key_id="not-base64-$$$" with an EMPTY blob (no + # second colon), so base64 decoding succeeds and AESGCM raises ValueError("Nonce must be between + # 8 and 128 bytes"). A bare "mfenc:" never reaches that — _parse rejects the version first with + # CipherError, a different fail-closed branch. # Lane 1: a single poison HEAD → dropped + DEAD + the lane re-arms. a = await _seed_ingress(enc, "IB_HP1", [100.0]) await enc._db.execute( diff --git a/tests/test_connection_event_store.py b/tests/test_connection_event_store.py index 5629067c..7e239015 100644 --- a/tests/test_connection_event_store.py +++ b/tests/test_connection_event_store.py @@ -12,7 +12,7 @@ import sqlite3 from pathlib import Path -from messagefoundry.store.crypto import PREFIX, generate_key, make_cipher +from messagefoundry.store.crypto import MARKER_PREFIX, generate_key, make_cipher from messagefoundry.store.store import MessageStore ADT = "MSH|^~\\&|S|F|R|RF|20260101||ADT^A01|MSG1|P|2.5.1\rPID|1||100^^^H^MR||DOE^JANE\r" @@ -90,7 +90,10 @@ async def test_reason_encrypted_at_rest(tmp_path: Path) -> None: assert _col_at_rest(db, "kind") == "framing_error" assert _col_at_rest(db, "connection") == "IB" reason_disk = _col_at_rest(db, "reason") - assert isinstance(reason_disk, str) and reason_disk.startswith(PREFIX) + # The version-agnostic marker: which columns are enciphered is the claim, not which mfenc + # format the writer emits. That is the cipher's choice — v1 from make_cipher's default here, + # v2 via build_cipher (write_v2=[store].aad_bind) or under MEFOR_TEST_FORCE_AAD_BIND. + assert isinstance(reason_disk, str) and reason_disk.startswith(MARKER_PREFIX) assert "boom" not in reason_disk # …and the read path decrypts it back events = await store.list_connection_events() diff --git a/tests/test_content_search.py b/tests/test_content_search.py index a8184197..490d34e5 100644 --- a/tests/test_content_search.py +++ b/tests/test_content_search.py @@ -31,7 +31,7 @@ make_spec, row_matches, ) -from messagefoundry.store.crypto import PREFIX, generate_key, make_cipher +from messagefoundry.store.crypto import MARKER_PREFIX, generate_key, make_cipher from messagefoundry.store.store import MessageStore PW = "a-strong-test-passphrase" # ≥15, satisfies the ASVS policy @@ -120,7 +120,7 @@ async def test_content_match_on_encrypted_store(tmp_path: Path) -> None: at_rest = [str(r[0]) for r in con.execute("SELECT raw FROM messages").fetchall()] finally: con.close() - assert all(v.startswith(PREFIX) and "JANE" not in v for v in at_rest) + assert all(v.startswith(MARKER_PREFIX) and "JANE" not in v for v in at_rest) spec = make_spec(content="JANE", field_path=None, field_value=None) result = await store.search_messages(spec) diff --git a/tests/test_ed_documents_e2e.py b/tests/test_ed_documents_e2e.py index aef6a381..77edf98a 100644 --- a/tests/test_ed_documents_e2e.py +++ b/tests/test_ed_documents_e2e.py @@ -37,7 +37,7 @@ from messagefoundry.parsing.message import Message from messagefoundry.pipeline.wiring_runner import RegistryRunner from messagefoundry.store import MessageStatus, MessageStore -from messagefoundry.store.crypto import PREFIX, generate_key, make_cipher +from messagefoundry.store.crypto import MARKER_PREFIX, generate_key, make_cipher from messagefoundry.transports import DeliveryError from messagefoundry.transports.mllp import MLLPDestination, MLLPSource, build_ack @@ -170,11 +170,13 @@ async def test_base64_pdf_encrypted_at_rest(tmp_path: Path) -> None: finally: await store.close() - # On disk the PDF base64 is AES-256-GCM ciphertext, never plaintext. + # On disk the PDF base64 is AES-256-GCM ciphertext, never plaintext. Anchor on the version-agnostic + # mfenc: marker — the claim is encryptedness, and the marker format is the cipher's choice (v1 from + # make_cipher's default here, v2 via build_cipher/[store].aad_bind or MEFOR_TEST_FORCE_AAD_BIND). at_rest_raw = _raw_at_rest(db, column="raw", table="messages") at_rest_payload = _raw_at_rest(db, column="payload", table="queue") - assert at_rest_raw.startswith(PREFIX) - assert at_rest_payload.startswith(PREFIX) + assert at_rest_raw.startswith(MARKER_PREFIX) + assert at_rest_payload.startswith(MARKER_PREFIX) assert source_b64 not in at_rest_raw # the document never hits disk in the clear assert source_b64 not in at_rest_payload diff --git a/tests/test_keyprovider.py b/tests/test_keyprovider.py index 4bb276f2..8733313d 100644 --- a/tests/test_keyprovider.py +++ b/tests/test_keyprovider.py @@ -247,6 +247,9 @@ def retired_keys(self) -> list[str]: return self._retired # A row written by today's cipher under KEY_A... + # DELIBERATELY v1 (do not sweep to MARKER_PREFIX): the acceptance criterion this test is named for + # is "decrypts an EXISTING mfenc:v1 row with NO rotation", so establishing that the fixture really + # is v1 is the premise, not incidental. make_cipher's default writer is the frozen v1 one. token = make_cipher(KEY_A).encrypt(ADT) assert token.startswith(PREFIX) diff --git a/tests/test_keyprovider_vault.py b/tests/test_keyprovider_vault.py index 88e70306..d7bf5b97 100644 --- a/tests/test_keyprovider_vault.py +++ b/tests/test_keyprovider_vault.py @@ -79,6 +79,8 @@ def test_vault_active_key_unwraps_and_decrypts_without_rotation( assert transit.calls == [(_TRANSIT_KEY, _WRAPPED_DEK)] # A row written under KEY_A decrypts with the provider's key — NO re-encryption, mfenc:v1 unchanged. + # DELIBERATELY v1 (do not sweep to MARKER_PREFIX): "an existing v1 row survives a KeyProvider swap + # byte-for-byte" is the claim, so the fixture being v1 is the premise of the test. token = make_cipher(KEY_A).encrypt(ADT) assert token.startswith(PREFIX) cipher = make_cipher(active, provider.retired_keys()) diff --git a/tests/test_message_export.py b/tests/test_message_export.py index 99cfc8e4..e59dfd25 100644 --- a/tests/test_message_export.py +++ b/tests/test_message_export.py @@ -25,7 +25,7 @@ from messagefoundry.auth.tokens import hash_token from messagefoundry.config.settings import AuthSettings from messagefoundry.pipeline import Engine -from messagefoundry.store.crypto import PREFIX, generate_key, make_cipher +from messagefoundry.store.crypto import MARKER_PREFIX, generate_key, make_cipher from messagefoundry.store.store import MessageStore PW = "a-strong-test-passphrase" # ≥15, satisfies the ASVS policy @@ -174,7 +174,10 @@ async def test_export_save_all_streams_decrypted_bodies(engine: Engine) -> None: raws = {row["control_id"]: row["raw"] for row in rows} assert "DOE^JANE" in str(raws["MSGA"]) assert str(raws["MSGA"]).startswith("MSH") # not the mfenc: ciphertext - assert PREFIX not in str(raws["MSGA"]) + # A NEGATIVE leak check, so it must exclude EVERY at-rest marker version: a v1-only spelling + # would pass silently on a leaked v2 blob instead of failing. Non-vacuous — the synthetic ADT + # plaintext contains no "mfenc:" at all, and the startswith("MSH") above pins the shape. + assert MARKER_PREFIX not in str(raws["MSGA"]) async def test_export_save_selected_by_ids(engine: Engine) -> None: diff --git a/tests/test_postgres_store.py b/tests/test_postgres_store.py index f43a2b79..72f90ec9 100644 --- a/tests/test_postgres_store.py +++ b/tests/test_postgres_store.py @@ -264,11 +264,10 @@ async def test_complete_with_response_parity(store) -> None: async def test_record_ack_sent_aa_body_encrypted_at_rest_pg(store) -> None: # (1) An AA ack_body is persisted only on an ENCRYPTED store, and on disk it is ciphertext: it - # carries the v1 marker, is not the plaintext AA frame, and decrypts back to exactly it. A second, - # ciphered handle is needed because the fixture store is the identity cipher (unencrypted) — mirrors - # the existing at-rest encryption tests in this file. + # carries the mfenc: marker, is not the plaintext AA frame, and decrypts back to exactly it. A + # second, ciphered handle is needed because the fixture store is the identity cipher (unencrypted) + # — mirrors the existing at-rest encryption tests in this file. from messagefoundry.config.settings import load_settings - from messagefoundry.store.crypto import PREFIX from messagefoundry.store.postgres import PostgresStore settings = load_settings(environ=os.environ).store @@ -290,14 +289,22 @@ async def test_record_ack_sent_aa_body_encrypted_at_rest_pg(store) -> None: assert ack.body == _ACK_AA # decrypted round-trip # Raw column read (the ciphered handle's _fetchone does NOT decrypt) → ciphertext on disk. Assert # the deterministic decrypt round-trip, NOT `"MSA" not in ` (base64 can contain that run). - disk = ( - await s._fetchone( - "SELECT body FROM response WHERE message_id=$1 AND kind='ack_sent'", mid - ) - )["body"] - assert disk.startswith(PREFIX) # stored under the encrypted marker, not in the clear + # destination_name + response_seq come back alongside the body: they are the row half of the + # cell AAD the store binds it under (postgres.py record_ack_sent), and destination_name is a + # SENTINEL ("\x1fack:" + inbound_name), so read it rather than reconstructing it here. + row = await s._fetchone( + "SELECT body, destination_name, response_seq FROM response" + " WHERE message_id=$1 AND kind='ack_sent'", + mid, + ) + disk = row["body"] + assert disk.startswith(MARKER_PREFIX) # stored under the encrypted marker, not in the clear assert disk != _ACK_AA - assert cipher.decrypt(disk) == _ACK_AA # and it genuinely encrypts the AA frame + # Decrypt under the SAME cell AAD the store wrote with (ASVS 11.3.3 / ADR 0019): a v2 value is + # bound to its (table, column, row) cell, so a bare decrypt fails closed on one. Harmless on a + # v1 value — that reader ignores the caller's aad by design (dual-read). + aad = cell_aad("response", "body", mid, row["destination_name"], row["response_seq"]) + assert cipher.decrypt(disk, aad=aad) == _ACK_AA # and it genuinely encrypts the AA frame finally: await s.close() @@ -2141,7 +2148,7 @@ async def test_summary_metadata_encrypted_at_rest_and_decrypt(store) -> None: """EF-3: summary/metadata (direct MRN + patient name) are ciphered at rest on Postgres and decrypt on the detail + tracking-list read paths — parity with the SQLite suite.""" from messagefoundry.config.settings import load_settings - from messagefoundry.store.crypto import PREFIX, AesGcmCipher + from messagefoundry.store.crypto import AesGcmCipher from messagefoundry.store.postgres import PostgresStore settings = load_settings(environ=os.environ).store @@ -2153,8 +2160,8 @@ async def test_summary_metadata_encrypted_at_rest_and_decrypt(store) -> None: ) # at rest: ciphertext, with no MRN/name/site visible in the blob. row = await s._fetchone("SELECT summary, metadata FROM messages WHERE id=$1", mid) - assert row["summary"].startswith(PREFIX) and "999001" not in row["summary"] - assert row["metadata"].startswith(PREFIX) and "WESTWING" not in row["metadata"] + assert row["summary"].startswith(MARKER_PREFIX) and "999001" not in row["summary"] + assert row["metadata"].startswith(MARKER_PREFIX) and "WESTWING" not in row["metadata"] # decrypt on the read paths. rec = await s.get_message(mid) assert rec["summary"] == summary and rec["metadata"] == metadata diff --git a/tests/test_sqlserver_store.py b/tests/test_sqlserver_store.py index fe85a5d1..6c337eac 100644 --- a/tests/test_sqlserver_store.py +++ b/tests/test_sqlserver_store.py @@ -1146,11 +1146,10 @@ async def test_correlate_orders_by_seq_and_decrypts(store) -> None: async def test_record_ack_sent_aa_body_encrypted_at_rest_ss(store) -> None: # (1) An AA ack_body is persisted only on an ENCRYPTED store, and on disk it is ciphertext: it - # carries the v1 marker, is not the plaintext AA frame, and decrypts back to exactly it. A second, - # ciphered handle is needed because the fixture store is the identity cipher (unencrypted) — mirrors - # the existing at-rest encryption tests in this file. + # carries the mfenc: marker, is not the plaintext AA frame, and decrypts back to exactly it. A + # second, ciphered handle is needed because the fixture store is the identity cipher (unencrypted) + # — mirrors the existing at-rest encryption tests in this file. from messagefoundry.config.settings import load_settings - from messagefoundry.store.crypto import PREFIX from messagefoundry.store.sqlserver import SqlServerStore settings = load_settings(environ=os.environ).store @@ -1172,14 +1171,22 @@ async def test_record_ack_sent_aa_body_encrypted_at_rest_ss(store) -> None: assert ack.body == _ACK_AA # decrypted round-trip # Raw column read (the ciphered handle's _fetchone does NOT decrypt) → ciphertext on disk. Assert # the deterministic decrypt round-trip, NOT `"MSA" not in ` (base64 can contain that run). - disk = ( - await s._fetchone( - "SELECT body FROM response WHERE message_id=? AND kind=?", (mid, "ack_sent") - ) - )["body"] - assert disk.startswith(PREFIX) # stored under the encrypted marker, not in the clear + # destination_name + response_seq come back alongside the body: they are the row half of the + # cell AAD the store binds it under (sqlserver.py record_ack_sent), and destination_name is a + # SENTINEL ("\x1fack:" + inbound_name), so read it rather than reconstructing it here. + row = await s._fetchone( + "SELECT body, destination_name, response_seq FROM response" + " WHERE message_id=? AND kind=?", + (mid, "ack_sent"), + ) + disk = row["body"] + assert disk.startswith(MARKER_PREFIX) # stored under the encrypted marker, not in the clear assert disk != _ACK_AA - assert cipher.decrypt(disk) == _ACK_AA # and it genuinely encrypts the AA frame + # Decrypt under the SAME cell AAD the store wrote with (ASVS 11.3.3 / ADR 0019): a v2 value is + # bound to its (table, column, row) cell, so a bare decrypt fails closed on one. Harmless on a + # v1 value — that reader ignores the caller's aad by design (dual-read). + aad = cell_aad("response", "body", mid, row["destination_name"], row["response_seq"]) + assert cipher.decrypt(disk, aad=aad) == _ACK_AA # and it genuinely encrypts the AA frame finally: await s.close() @@ -1474,7 +1481,7 @@ async def test_summary_metadata_encrypted_at_rest_and_decrypt(store) -> None: """EF-3: summary/metadata (direct MRN + patient name) ciphered at rest on SQL Server and decrypt on the detail + tracking-list read paths — parity with the SQLite/PG suites.""" from messagefoundry.config.settings import load_settings - from messagefoundry.store.crypto import PREFIX, AesGcmCipher + from messagefoundry.store.crypto import AesGcmCipher from messagefoundry.store.sqlserver import SqlServerStore settings = load_settings(environ=os.environ).store @@ -1486,8 +1493,8 @@ async def test_summary_metadata_encrypted_at_rest_and_decrypt(store) -> None: ) # at rest: ciphertext, with no MRN/name/site visible in the blob. row = (await s._fetchall("SELECT summary, metadata FROM messages WHERE id=?", (mid,)))[0] - assert row["summary"].startswith(PREFIX) and "999001" not in row["summary"] - assert row["metadata"].startswith(PREFIX) and "WESTWING" not in row["metadata"] + assert row["summary"].startswith(MARKER_PREFIX) and "999001" not in row["summary"] + assert row["metadata"].startswith(MARKER_PREFIX) and "WESTWING" not in row["metadata"] # decrypt on the read paths. rec = await s.get_message(mid) assert rec["summary"] == summary and rec["metadata"] == metadata @@ -1539,9 +1546,11 @@ async def test_reencrypt_rotates_summary_and_metadata(store) -> None: # --- H4: error / last_error / message_events.detail encrypted at rest ---------- # SQL Server parity with SQLite/Postgres: the three nullable disposition-text columns route through the # SAME store cipher — at-rest ciphertext in whichever mfenc format that cipher writes, decrypt-on-read, -# rotated on rekey, and legacy plaintext migrated on open. (Naming a version here would be wrong: the -# format follows [store].aad_bind, so it is v2 by default.) The prior "SQL Server keeps these -# plaintext" residual is retired. +# rotated on rekey, and legacy plaintext migrated on open. (Naming a version here would be wrong: these +# tests hand the store a make_cipher() handle, whose writer default is still the frozen v1; the SHIPPED +# store builds its cipher via build_cipher with write_v2=[store].aad_bind, now on. The claim is which +# columns are enciphered, which holds either way.) The prior "SQL Server keeps these plaintext" +# residual is retired. async def test_error_lasterror_detail_encrypted_at_rest_and_decrypt(store) -> None: @@ -1549,7 +1558,7 @@ async def test_error_lasterror_detail_encrypted_at_rest_and_decrypt(store) -> No Server and decrypt on every read path — parity with the SQLite/PG suites. Error strings are plain (no HL7 delimiters) so safe_text leaves them intact and the round-trip is exact.""" from messagefoundry.config.settings import load_settings - from messagefoundry.store.crypto import PREFIX, AesGcmCipher + from messagefoundry.store.crypto import AesGcmCipher from messagefoundry.store.sqlserver import SqlServerStore settings = load_settings(environ=os.environ).store @@ -1566,17 +1575,19 @@ async def test_error_lasterror_detail_encrypted_at_rest_and_decrypt(store) -> No fail = "delivery refused by partner endpoint" await s.mark_failed(item.id, fail, RetryPolicy(max_attempts=1), now=110.0) # -> DEAD - # AT REST: every value is mfenc:v1:... ciphertext — the cleartext phrase never appears in the col. + # AT REST: every value is mfenc:... ciphertext — the cleartext phrase never appears in the col. + # (Version-agnostic per the section header: the marker version belongs to the cipher, not to + # this claim — v1 here, v2 via build_cipher/[store].aad_bind or MEFOR_TEST_FORCE_AAD_BIND.) erow = (await s._fetchall("SELECT error FROM messages WHERE id=?", (eid,)))[0] - assert erow["error"].startswith(PREFIX) and "bad parse" not in erow["error"] + assert erow["error"].startswith(MARKER_PREFIX) and "bad parse" not in erow["error"] qrow = (await s._fetchall("SELECT last_error FROM queue WHERE message_id=?", (mid,)))[0] - assert qrow["last_error"].startswith(PREFIX) and "refused" not in qrow["last_error"] + assert qrow["last_error"].startswith(MARKER_PREFIX) and "refused" not in qrow["last_error"] drows = await s._fetchall( "SELECT detail FROM message_events WHERE detail IS NOT NULL ORDER BY id" ) assert drows, "expected at least one event with a detail" for d in drows: - assert d["detail"].startswith(PREFIX) # no plaintext detail at rest + assert d["detail"].startswith(MARKER_PREFIX) # no plaintext detail at rest assert "bad parse" not in d["detail"] and "refused" not in d["detail"] # DECRYPT ON READ: every read path returns the cleartext. @@ -1641,7 +1652,7 @@ async def test_legacy_plaintext_error_detail_migrated_on_open(store) -> None: _encrypt_existing_rows (the message_events.detail pass is keyed on the INT IDENTITY id). After the keyed open, the at-rest columns are ciphertext and reads still return the original cleartext.""" from messagefoundry.config.settings import load_settings - from messagefoundry.store.crypto import PREFIX, AesGcmCipher + from messagefoundry.store.crypto import AesGcmCipher from messagefoundry.store.sqlserver import SqlServerStore settings = load_settings(environ=os.environ).store @@ -1657,9 +1668,10 @@ async def test_legacy_plaintext_error_detail_migrated_on_open(store) -> None: ) item = (await plain.claim_ready(now=100.0))[0] await plain.mark_failed(item.id, fail, RetryPolicy(max_attempts=1), now=110.0) - # sanity: stored plaintext (no cipher prefix) before the migration runs. + # sanity: stored plaintext (no cipher prefix) before the migration runs. A NEGATIVE assert, so + # it must exclude EVERY marker version — a v1-only spelling would pass on an encrypted v2 value. erow = (await plain._fetchall("SELECT error FROM messages WHERE id=?", (eid,)))[0] - assert erow["error"] == err and not erow["error"].startswith(PREFIX) + assert erow["error"] == err and not erow["error"].startswith(MARKER_PREFIX) finally: await plain.close() @@ -1667,11 +1679,11 @@ async def test_legacy_plaintext_error_detail_migrated_on_open(store) -> None: keyed = await SqlServerStore.open(settings, cipher=AesGcmCipher(b"k" * 32)) try: erow = (await keyed._fetchall("SELECT error FROM messages WHERE id=?", (eid,)))[0] - assert erow["error"].startswith(PREFIX) and "bad parse" not in erow["error"] + assert erow["error"].startswith(MARKER_PREFIX) and "bad parse" not in erow["error"] qrow = (await keyed._fetchall("SELECT last_error FROM queue WHERE message_id=?", (mid,)))[0] - assert qrow["last_error"].startswith(PREFIX) + assert qrow["last_error"].startswith(MARKER_PREFIX) drows = await keyed._fetchall("SELECT detail FROM message_events WHERE detail IS NOT NULL") - assert drows and all(d["detail"].startswith(PREFIX) for d in drows) + assert drows and all(d["detail"].startswith(MARKER_PREFIX) for d in drows) # reads still return the original cleartext after the in-place migration. assert (await keyed.get_message(eid))["error"] == err assert (await keyed.list_dead())[0]["last_error"] == fail @@ -1894,7 +1906,7 @@ async def test_reference_snapshot_encrypted_at_rest(store) -> None: """Reference values (may carry PHI for patient-keyed sets) are mfenc ciphertext at rest while reference_view() serves plaintext — SQLite/PG parity (test_reference_sets.py analog).""" from messagefoundry.config.settings import load_settings - from messagefoundry.store.crypto import PREFIX, AesGcmCipher + from messagefoundry.store.crypto import AesGcmCipher from messagefoundry.store.sqlserver import SqlServerStore settings = load_settings(environ=os.environ).store @@ -1905,7 +1917,7 @@ async def test_reference_snapshot_encrypted_at_rest(store) -> None: assert s.reference_view()["codes"]["MRN"] == "SECRET999" # cache is plaintext # The value column at rest is ciphertext (no PHI visible in the blob). row = (await s._fetchall("SELECT value FROM reference"))[0] - assert row["value"].startswith(PREFIX) and "SECRET999" not in row["value"] + assert row["value"].startswith(MARKER_PREFIX) and "SECRET999" not in row["value"] finally: await s.close() # Reopening with the same cipher decrypts back into the cache. @@ -2012,14 +2024,15 @@ async def test_reference_plaintext_migrated_on_keyed_reopen(store) -> None: IdentityCipher writes plaintext JSON, and the first keyed open's _encrypt_existing_rows reference pass encrypts them in place (the no-key -> key transition).""" from messagefoundry.config.settings import load_settings - from messagefoundry.store.crypto import PREFIX, AesGcmCipher + from messagefoundry.store.crypto import AesGcmCipher from messagefoundry.store.sqlserver import SqlServerStore try: - # (1) The keyless fixture handle writes plaintext JSON at rest. + # (1) The keyless fixture handle writes plaintext JSON at rest. A NEGATIVE assert, so it must + # exclude EVERY marker version — a v1-only spelling would pass on an encrypted v2 value. await store.write_reference_snapshot(name="codes", version="v1", rows={"MRN": "SECRET999"}) row = (await store._fetchall("SELECT value FROM reference"))[0] - assert row["value"] == '"SECRET999"' and not row["value"].startswith(PREFIX) + assert row["value"] == '"SECRET999"' and not row["value"].startswith(MARKER_PREFIX) # (2) Re-open WITH a key: open() runs the _encrypt_existing_rows reference pass and # migrates it. @@ -2028,7 +2041,7 @@ async def test_reference_plaintext_migrated_on_keyed_reopen(store) -> None: ) try: row = (await keyed._fetchall("SELECT value FROM reference"))[0] - assert row["value"].startswith(PREFIX) and "SECRET999" not in row["value"] + assert row["value"].startswith(MARKER_PREFIX) and "SECRET999" not in row["value"] assert keyed.reference_view()["codes"]["MRN"] == "SECRET999" # decrypts on cache load finally: await keyed.close() diff --git a/tests/test_staged_pipeline.py b/tests/test_staged_pipeline.py index 13db85ae..27448691 100644 --- a/tests/test_staged_pipeline.py +++ b/tests/test_staged_pipeline.py @@ -28,7 +28,7 @@ WiringError, inbound, ) -from messagefoundry.store.crypto import PREFIX, make_cipher +from messagefoundry.store.crypto import MARKER_PREFIX, make_cipher from messagefoundry.store.store import MessageStatus, MessageStore, OutboxStatus, Stage RAW = "MSH|^~\\&|S|F|R|RF|20260101||ADT^A01|MSG1|P|2.5.1\rPID|1||100||DOE^JANE\r" @@ -841,7 +841,7 @@ async def test_legacy_outbox_migrates_to_queue_with_encryption(tmp_path: Path) - item = await store.claim_next_fifo("OB_A") assert item is not None and item.payload == "PLAINPAYLOAD" on_disk = sqlite3.connect(path).execute("SELECT payload FROM queue").fetchone()[0] - assert str(on_disk).startswith(PREFIX) # encrypted at rest after migration + assert str(on_disk).startswith(MARKER_PREFIX) # encrypted at rest after migration finally: await store.close() diff --git a/tests/test_store_encryption.py b/tests/test_store_encryption.py index 00241346..fcfc5258 100644 --- a/tests/test_store_encryption.py +++ b/tests/test_store_encryption.py @@ -46,7 +46,9 @@ def _raw_at_rest(db_path: Path, column: str = "raw", table: str = "messages") -> def test_cipher_round_trip_and_hides_plaintext() -> None: cipher = make_cipher(generate_key()) token = cipher.encrypt(ADT) - assert token.startswith(PREFIX) + # "Enciphered at all" — the version-agnostic marker. Format pinning is owned separately by the M9 + # section below (test_default_writer_is_v1_not_v2 / test_v1_writer_is_byte_identical). + assert token.startswith(MARKER_PREFIX) # PHI-hidden, asserted deterministically: the whole plaintext can never appear in the token # (it contains non-base64 bytes like '|' and '\r'), and the round-trip proves real encryption. # NEVER assert short-substring absence ("MSH"/"DOE") — a random base64 body contains any given @@ -90,8 +92,8 @@ async def test_bodies_encrypted_at_rest(tmp_path: Path) -> None: await store.close() raw = _raw_at_rest(db) payload = _raw_at_rest(db, column="payload", table="queue") - assert raw.startswith(PREFIX) and "DOE" not in raw # body is ciphertext on disk - assert payload.startswith(PREFIX) + assert raw.startswith(MARKER_PREFIX) and "DOE" not in raw # body is ciphertext on disk + assert payload.startswith(MARKER_PREFIX) async def test_reads_and_delivery_decrypt(tmp_path: Path) -> None: @@ -139,8 +141,12 @@ async def test_claim_ready_dead_letters_undecryptable_row(tmp_path: Path) -> Non cur = await store._db.execute("SELECT status, last_error FROM queue WHERE id=?", (bad_id,)) row = await cur.fetchone() assert row["status"] == OutboxStatus.DEAD.value # poison row dead-lettered, not stranded - # last_error is itself ciphered now (WP-5), so decrypt it before checking the reason. - assert "undecryptable" in store._cipher.decrypt(row["last_error"] or "") + # last_error is itself ciphered (WP-5) AND cell-bound (ASVS 11.3.3 / ADR 0019), so decrypt it + # under the SAME (table, column, row) AAD the store wrote with — a bare decrypt fails closed on + # a v2 value. Harmless on v1: that reader ignores the caller's aad by design (dual-read). + assert "undecryptable" in store._cipher.decrypt( + row["last_error"] or "", aad=cell_aad("queue", "last_error", bad_id) + ) finally: await store.close() @@ -169,8 +175,11 @@ async def test_claim_ingress_dead_letters_undecryptable_row(tmp_path: Path) -> N ) row = await cur.fetchone() assert row["status"] == OutboxStatus.DEAD.value # poison row dead-lettered, not stranded - # last_error is itself ciphered now (WP-5), so decrypt it before checking the reason. - assert "undecryptable" in store._cipher.decrypt(row["last_error"] or "") + # Ciphered (WP-5) and cell-bound (ADR 0019) — decrypt under the row's own AAD, see the outbound + # poison-row test above. + assert "undecryptable" in store._cipher.decrypt( + row["last_error"] or "", aad=cell_aad("queue", "last_error", ingress_id) + ) # Dead ingress row with no outbound rows → the message is finalized to ERROR. assert (await store.get_message(mid))["status"] == MessageStatus.ERROR.value finally: @@ -189,8 +198,9 @@ async def test_migration_encrypts_existing_rows(tmp_path: Path) -> None: key = generate_key() encrypted = await MessageStore.open(db, cipher=make_cipher(key)) # reopen with a key → migrate try: - assert _raw_at_rest(db).startswith(PREFIX) # existing row now encrypted on disk - assert _raw_at_rest(db, column="payload", table="queue").startswith(PREFIX) + # The migration's own guard is the version-agnostic `mfenc:%` anchor, so assert the same shape. + assert _raw_at_rest(db).startswith(MARKER_PREFIX) # existing row now encrypted on disk + assert _raw_at_rest(db, column="payload", table="queue").startswith(MARKER_PREFIX) record = await encrypted.get_message(mid) assert record is not None and record["raw"] == ADT # still readable finally: @@ -219,8 +229,8 @@ async def test_error_and_event_detail_encrypted_at_rest_and_decrypt(tmp_path: Pa # messages.error and the message_events.detail copy are both ciphertext on disk... err_at_rest = _raw_at_rest(db, column="error") det_at_rest = _raw_at_rest(db, column="detail", table="message_events") - assert err_at_rest.startswith(PREFIX) and "SECRET" not in err_at_rest - assert det_at_rest.startswith(PREFIX) and "SECRET" not in det_at_rest + assert err_at_rest.startswith(MARKER_PREFIX) and "SECRET" not in err_at_rest + assert det_at_rest.startswith(MARKER_PREFIX) and "SECRET" not in det_at_rest # ...and decrypt on every read path. assert (await store.get_message(mid))["error"] == PHI_ERR assert any(m["error"] == PHI_ERR for m in await store.list_messages()) @@ -238,7 +248,7 @@ async def test_last_error_encrypted_at_rest_and_decrypts(tmp_path: Path) -> None await store.claim_ready() await store.dead_letter_now(row["id"], PHI_ERR) at_rest = _raw_at_rest(db, column="last_error", table="queue") - assert at_rest.startswith(PREFIX) and "SECRET" not in at_rest + assert at_rest.startswith(MARKER_PREFIX) and "SECRET" not in at_rest dead = await store.list_dead() assert dead and dead[0]["last_error"] == PHI_ERR # dead-letter view decrypts assert (await store.outbox_for(mid))[0]["last_error"] == PHI_ERR # detail view decrypts @@ -290,8 +300,8 @@ async def test_summary_and_metadata_encrypted_at_rest_and_decrypt(tmp_path: Path # ...ciphertext on disk (no MRN/name/site visible)... sm = _raw_at_rest(db, column="summary") md = _raw_at_rest(db, column="metadata") - assert sm.startswith(PREFIX) and "999001" not in sm and "DOE" not in sm - assert md.startswith(PREFIX) and "WESTWING" not in md + assert sm.startswith(MARKER_PREFIX) and "999001" not in sm and "DOE" not in sm + assert md.startswith(MARKER_PREFIX) and "WESTWING" not in md # ...and decrypt on the detail + tracking-list read paths. rec = await store.get_message(mid) assert rec is not None and rec["summary"] == EF3_SUMMARY and rec["metadata"] == EF3_METADATA @@ -330,8 +340,8 @@ async def test_migration_encrypts_existing_summary_metadata(tmp_path: Path) -> N encrypted = await MessageStore.open(db, cipher=make_cipher(generate_key())) # reopen → migrate try: - assert _raw_at_rest(db, column="summary").startswith(PREFIX) # migrated on disk - assert _raw_at_rest(db, column="metadata").startswith(PREFIX) + assert _raw_at_rest(db, column="summary").startswith(MARKER_PREFIX) # migrated on disk + assert _raw_at_rest(db, column="metadata").startswith(MARKER_PREFIX) [m] = await encrypted.list_messages() assert m["summary"] == EF3_SUMMARY and m["metadata"] == EF3_METADATA # still readable finally: @@ -365,12 +375,19 @@ def test_key_id_is_a_fingerprint_not_zero() -> None: key_b64 = generate_key() token = make_cipher(key_b64).encrypt("x") fp = _fingerprint(base64.b64decode(key_b64)) + # DELIBERATELY v1 (do not sweep to MARKER_PREFIX): these pin the key_id's POSITION in the marker, + # and `mfenc:v1::` is a v1-only field order (v2 is `mfenc:v2:::`). A version- + # agnostic `mfenc::` is a string no writer ever emits, so the positive assert could never pass + # and the negative one could never fail — strictly weaker, not wider. The v2 layout is pinned + # separately by test_active_marker_prefix_v1_and_v2. assert token.startswith(f"{PREFIX}{fp}:") # self-identifying key_id assert not token.startswith(f"{PREFIX}0:") # not the old hardcoded "0" def test_legacy_key_id_zero_decrypts_via_fallback() -> None: # A pre-WP-5 row was tagged key_id '0'. The keyring's try-all fallback still decrypts it. + # DELIBERATELY v1 (do not sweep): a legacy row IS v1 by definition — v2 postdates WP-5 — and a + # version-agnostic `mfenc:0:` would be rejected by _parse as an unknown marker version instead. import base64 import os @@ -396,7 +413,9 @@ async def test_rotation_reencrypts_and_retired_key_bridges(tmp_path: Path) -> No # Reopen with B active + A retired: existing A-rows still read (decrypt via the retired key), # then rotate them to B. - rotating = await MessageStore.open(db, cipher=make_cipher(key_b, [key_a])) + rotating_cipher = make_cipher(key_b, [key_a]) + assert isinstance(rotating_cipher, AesGcmCipher) + rotating = await MessageStore.open(db, cipher=rotating_cipher) try: assert (await rotating.get_message(mid))["raw"] == ADT assert await rotating.reencrypt_to_active() >= 2 # raw + the outbound payload @@ -404,7 +423,12 @@ async def test_rotation_reencrypts_and_retired_key_bridges(tmp_path: Path) -> No finally: await rotating.close() raw_b = _raw_at_rest(db) - assert raw_b.startswith(PREFIX) and raw_b != raw_a # re-encrypted under the new key + # Re-encrypted under the ACTIVE (new) key — not merely "still ciphertext". Take the expected marker + # from the rotating cipher itself: active_marker_prefix carries key B's fingerprint in the right + # position for whichever format that cipher writes (v1 here, v2 when the store is handed a + # build_cipher handle or run under MEFOR_TEST_FORCE_AAD_BIND — the field order differs between the + # two). A bare MARKER_PREFIX would drop the under-the-new-key half of the proof. + assert raw_b.startswith(rotating_cipher.active_marker_prefix) and raw_b != raw_a # B alone (no retired key) now reads everything — the bridge key is no longer needed. final = await MessageStore.open(db, cipher=make_cipher(key_b)) @@ -436,10 +460,16 @@ async def test_rotation_without_prior_key_raises(tmp_path: Path) -> None: # # The hard constraint is CRYPTO-1: the mfenc:v1 WRITER is frozen — existing v1 ciphertext and new v1 # writes stay byte-identical. M9 adds *agility infrastructure only*: a version/alg-dispatching cipher -# that is DECODE-CAPABLE of mfenc:v2 and CAN write it (opt-in), but WRITES v1 BY DEFAULT. AES-256-GCM -# stays the only registered algorithm. These tests pin: (1) v1 byte-identical (frozen fixture); (2) v2 -# round-trip; (3) a v2-active cipher reads v1 with no rotation; (4) mixed v1+v2 rows; (5) fail-closed -# CipherError on an unknown marker version AND an unknown alg id. +# that is DECODE-CAPABLE of mfenc:v2 and CAN write it (opt-in). AES-256-GCM stays the only registered +# algorithm. These tests pin: (1) v1 byte-identical (frozen fixture); (2) v2 round-trip; (3) a v2-active +# cipher reads v1 with no rotation; (4) mixed v1+v2 rows; (5) fail-closed CipherError on an unknown +# marker version AND an unknown alg id. +# +# TWO DIFFERENT "defaults" — do not collapse them. `make_cipher`'s `write_v2` PARAMETER still defaults +# False (the frozen v1 writer), which is what the assertions below pin and what keeps CRYPTO-1 testable +# from a bare `make_cipher(key)`. The SHIPPED at-rest format is no longer v1: `build_cipher` passes +# `write_v2=[store].aad_bind`, and ADR 0148 flipped that setting's default to True, so a store opened +# the normal way writes mfenc:v2. Everything in this section is about the former. # A FROZEN FIXTURE: a v1 blob written by the pre-M9 writer for plaintext "LEGACY-V1" under the key below # (nonce fixed to 12 zero bytes). Hardcoded so a regression in the v1 reader is caught against a value @@ -483,7 +513,9 @@ def test_v1_writer_is_byte_identical(monkeypatch: pytest.MonkeyPatch) -> None: def test_default_writer_is_v1_not_v2() -> None: - # The shipping default never emits a v2 marker — no at-rest format change ships with M9. + # `make_cipher`'s write_v2 PARAMETER default — the frozen v1 writer, CRYPTO-1. NOT a claim about the + # shipped store: that builds its cipher through build_cipher(write_v2=[store].aad_bind), which ADR + # 0148 defaulted to True, so a normally-opened store writes v2. This pins the library default only. token = make_cipher(generate_key()).encrypt("x") assert token.startswith(PREFIX) # mfenc:v1: assert not token.startswith("mfenc:v2:") @@ -507,6 +539,9 @@ def test_v2_active_decrypts_v1_without_rotation() -> None: # forced migration). Same key, so the v2-active cipher decrypts the v1 blob it did not write. key = generate_key() v1_token = make_cipher(key).encrypt(ADT) # written by a v1 cipher + # DELIBERATELY v1 (do not sweep): this establishes the PREMISE that the row really is v1. Under a + # version-agnostic prefix a v2 token would satisfy it and the dual-read proof below collapses into + # a vacuous v2-reads-v2 tautology. assert v1_token.startswith(PREFIX) v2_cipher = make_cipher(key, write_v2=True) assert v2_cipher.decrypt(v1_token) == ADT # decoded with no rotation @@ -836,8 +871,11 @@ async def test_foreign_key_at_runtime_dead_letters_rather_than_degrading(tmp_pat ) dead = await cur.fetchone() assert dead["status"] == OutboxStatus.DEAD.value # poison row dead-lettered, not stranded - # last_error is ciphered (WP-5) under the ACTIVE key B, so the reopened store decrypts it. - assert "undecryptable" in store._cipher.decrypt(dead["last_error"] or "") + # last_error is ciphered (WP-5) under the ACTIVE key B, so the reopened store decrypts it — + # under the row's own cell AAD (ADR 0019), which a v2 value requires and a v1 value ignores. + assert "undecryptable" in store._cipher.decrypt( + dead["last_error"] or "", aad=cell_aad("queue", "last_error", row["id"]) + ) finally: await store.close() @@ -1019,7 +1057,9 @@ async def test_migration_encrypts_existing_state_value(tmp_path: Path) -> None: store = await MessageStore.open(db, cipher=make_cipher(generate_key())) # reopen → migrate try: at_rest = _state_at_rest(db) - assert at_rest.startswith(PREFIX) and "SECRETSTATEMRN" not in at_rest # sealed on disk + # sealed on disk — version-agnostic, because the marker version is whatever cipher the store was + # handed writes (v1 here, v2 via build_cipher/[store].aad_bind or MEFOR_TEST_FORCE_AAD_BIND) + assert at_rest.startswith(MARKER_PREFIX) and "SECRETSTATEMRN" not in at_rest assert store.state_view()[("ns", "k")] == {"mrn": "SECRETSTATEMRN"} # decrypts on read finally: await store.close() @@ -1043,7 +1083,7 @@ async def test_state_migration_skips_already_encrypted_values(tmp_path: Path) -> first = await MessageStore.open(db, cipher=make_cipher(key)) # migrate plaintext -> ciphertext await first.close() sealed = _state_at_rest(db) - assert sealed.startswith(PREFIX) + assert sealed.startswith(MARKER_PREFIX) # same shape as the `mfenc:%` guard under test second = await MessageStore.open(db, cipher=make_cipher(key)) # re-run the migration try: diff --git a/tests/test_transform_state.py b/tests/test_transform_state.py index 991611d6..bc2df27e 100644 --- a/tests/test_transform_state.py +++ b/tests/test_transform_state.py @@ -33,7 +33,7 @@ from messagefoundry.parsing.message import Message from messagefoundry.pipeline.dryrun import dry_run, transform_one from messagefoundry.pipeline.retention import RetentionRunner -from messagefoundry.store.crypto import PREFIX, generate_key, make_cipher +from messagefoundry.store.crypto import MARKER_PREFIX, generate_key, make_cipher from messagefoundry.store.store import MessageStatus, MessageStore, Stage RAW = "MSH|^~\\&|S|F|R|RF|20260101||ADT^A01|MSG1|P|2.5.1\rPID|1||100||DOE^JANE\r" @@ -273,9 +273,11 @@ async def test_state_value_encrypted_at_rest_and_read_back(tmp_path: Path) -> No ) finally: await store.close() - # On disk: ciphertext (prefix present, plaintext value not visible). + # On disk: ciphertext (version-agnostic marker present, plaintext value not visible). Same rule as + # the rotation test below — the marker version is the cipher's choice (v1 from make_cipher's default + # here, v2 via build_cipher/[store].aad_bind or MEFOR_TEST_FORCE_AAD_BIND), not this test's claim. at_rest = _state_at_rest(db, "patient_anon", "MRN-DOE") - assert at_rest.startswith(PREFIX) + assert at_rest.startswith(MARKER_PREFIX) assert "ANON-XYZ" not in at_rest @@ -324,7 +326,7 @@ async def test_key_rotation_reencrypts_state_and_reads_still_work(tmp_path: Path rotated = await store2.reencrypt_to_active() assert rotated >= 1 # the state value (among others) re-encrypted # On disk it is now under the NEW key id, and reads still resolve. Take the expected marker - # from the rotating cipher itself instead of hand-building f"{PREFIX}{new_id}:" — that spelling + # from the rotating cipher itself instead of hand-building f"mfenc:v1:{new_id}:" — that spelling # bakes in the v1 field order and misreads a v2 value (mfenc:v2:::), which is what # [store].aad_bind makes the default at rest. `old_id not in` keeps the independent proof that # rotation actually moved the value, derived from active_key_id rather than the prefix.