diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ddf65bc..4a2cb0a8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,50 @@ All notable changes to MessageFoundry are documented here. The format follows ## [Unreleased] +### Added +- **`messagefoundry audit-anchor`, and `audit-verify --expected-anchor` / `--expected-anchor-file` to + check one back.** The audit hash chain links each row to its predecessor, so deleting the *newest* + rows leaves a shorter chain that still walks cleanly — `audit-verify` on its own reports OK after a + tail-truncation, which is the shape an attacker hiding what they just did leaves behind. The store + could always compare against an external anchor; nothing exposed it, so the capability was + unreachable. `audit-anchor` prints `COUNT:HEAD` (a row count plus a digest — no PHI, no secret, safe + to hold in a ticket or an object store); passing it back reports `truncated or rewritten` when the + live chain differs. + **Know what it is before you build a job on it: an EXACT point-in-time seal**, comparing the count + *and* the head hash. The head half is not redundant — an attacker who cuts the newest rows and forges + the same number of replacements restores the count and leaves a chain that walks cleanly, so the head + is the only thing that differs. The cost of that detection is that a chain which merely **grew** also + reports `truncated or rewritten`. So it seals a chain **at rest across a gap in custody**: quiesce the + engine, anchor, hold the value off-box, re-verify while the chain is still quiesced — around a + maintenance window, a database move, a backup/restore, a hand-off. Anchoring and immediately + re-verifying compares a value to itself; re-checking a held anchor against a **running** engine alarms + on every ordinary boot. For continuous coverage of a live engine the off-box log forward / tee remains + the control, and `[integrity].audit_verify_on_start` is unchanged — it is a bare walk and stays blind + to a truncated tail. ([BACKLOG #328](docs/BACKLOG.md)) + ### Changed +- **A PHI instance reached through a declared reverse proxy with `[security].require_mfa` explicitly + off would refuse to start on first deployment, where it previously would not have.** The + MFA-at-exposure gate derived "is this instance exposed?" from `[api].serve_ui`, a field the ADR 0143 + console degrade arms rewrite **in place** earlier in the same startup. On the topology the runbooks + recommend — a loopback bind behind a declared TLS terminator, with the web console left at its + default — the auto-degrade cleared that flag first, so the gate evaluated "not exposed" and the + refusal was unreachable, while the ASVS 11.7.1 arm in the same startup classified the identical boot + as exposed. The gate now reads a single console-independent predicate (an off-loopback bind **or** + `[api].tls_terminated_upstream`), so it also fires when the console is auto-degraded, when + `serve_web_console = false` disables it outright, and when the console package is simply not + installed: the surface authenticating with one factor is the JSON operator API, which the proxy + serves either way. The `#189` dual-control advisory reads the same predicate and gains the same reach + (still warn-only). + **Who this would bite:** a deploying site that has explicitly set `require_mfa = false` on a + PHI-carrying environment behind a declared TLS terminator, under `enforcement = enforce`. **Two + remedies, both existing:** set `[security].require_mfa = true`, or set the already-shipped + acknowledgment `[security].allow_single_factor_admin_when_exposed = true`, which downgrades the + refusal to a loud audited warning. A plain loopback bind with nothing declared is **not** exposed and + is byte-identical. An **undeclared** proxy (`web_console_public_address` set, no + `tls_terminated_upstream`) deliberately still does not refuse — exposure there would be an inference — + but it no longer passes in silence: a new warning names single-factor admin directly on a PHI instance + with `require_mfa` off. ([BACKLOG #326](docs/BACKLOG.md), [ADR 0140](docs/adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md) amendment) - **BREAKING — an `[[alerts.rules]]` block that routes to an unconfigured transport now refuses at startup instead of being silently ignored.** `notifier_from_settings` returned early when **no** transport was configured, *before* the loop that cross-checks each rule's `transports` against the diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 8ecc7b56..ba77e15b 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -2866,7 +2866,7 @@ No test covers it. `tests/test_scan_tokens_source.py:559-583` (`test_absolute_ho ## 326. MFA-at-exposure refusal reads `serve_ui` after it is flipped off -> 🔢 **Filed 2026-08-01 — not started.** Value **6/10** · Difficulty **3/10** · _quick win_. ASVS 6.3.3's admin-MFA refusal and #189's dual-control warning are both inert in the topology the runbook recommends — the ADR 0143 auto-degrade sets `settings.api.serve_ui = False` in place before `ui_exposed` and `admin_exposed` are derived from it (`messagefoundry/__main__.py`, the flip and the two derivations in one ladder), so the engine calls one instance exposed for 11.7.1 and not exposed for 6.3.3 in a single boot — but `require_mfa` defaults on and `security_loosenings()` still names the explicit opt-out on every boot; re-key `admin_exposed` on the `instance_exposed` predicate already present in the file, fix two `exposure_desc` else-branches, and settle the refuse-on-upgrade fork against `docs/CONFIGURATION.md:1439`. +> ✅ **SHIPPED 2026-08-04.** `admin_exposed` is now `instance_exposed` — an off-loopback bind **or** `[api].tls_terminated_upstream` — defined ONCE above its first consumer, from two fields no earlier arm reassigns, and shared with the ASVS 11.7.1 arm that already used it. It reads no console flag, so the ADR 0143 in-place `serve_ui = False` degrades can no longer clear an exposure refusal: the MFA-at-exposure refusal and the #189 dual-control advisory now reach a declared-proxy instance whose console is auto-degraded, explicitly disabled, or absent (arms C/D in `tests/test_cli.py`, a real-gate row in `tests/test_checks_gate_parity.py`, a shape guard in `tests/test_security_doc_drift.py`). Both `exposure_desc` else-branches name the proxy instead of `[api].serve_ui`. **Built to REFUSE, per the owner ruling of 2026-08-04 — the WARN-FIRST blockquote below is SUPERSEDED** and is being amended by a separate session, so do not read it as the shipped behaviour: there is no warning-first phase, no dated flip and no new opt-in, the refusal rides the existing `[security].enforcement` split, and the pre-existing `allow_single_factor_admin_when_exposed` acknowledgment is unchanged (with more postures to act on). A plain loopback bind with nothing declared is byte-identical. The UNDECLARED-proxy residual is deliberately still not refused — nothing was declared, so exposure would be an inference — but it is no longer silent: the ADR 0068 §8 heuristic was **measured** not to cover it (it is about the /ui cookie, and the ADR 0143 auto-degrade suppresses it in the same posture), so a dedicated arm now warns, naming single-factor admin. **Two stale claims in the body below are corrected here rather than rewritten:** the arm table's arm-A string is now `admin interface reached through a declared reverse proxy ([api].tls_terminated_upstream)`, and the `docs/CONFIGURATION.md:1437`/`:1439` citations are wrong anchors — the opt-in scoping rule lives on the `require_memory_encryption_declaration` row and the `enforcement` refuse/warn split at `:88`/`:1020`. **Two residuals are left OPEN for the owner**, recorded in the [ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md) amendment: the `[auth] enabled = false` startup arm still keys on the bind alone (same two-answers-in-one-startup shape, one arm over, and it needs its own hoist plus its own adjudication), and the vault-only `OFF-LOOPBACK-DEPLOYMENT.md` runbook still carries blind-spot wording this fix invalidates. > **OWNER RULING 2026-08-04 — REFUSE OUTRIGHT. Supersedes an earlier ruling on this item that said > WARN-FIRST with a dated flip.** No warn-first, no dated flip, no opt-in flag. The corrected @@ -2993,7 +2993,7 @@ The two nearest-looking guards are neither: `tests/test_scaffold.py:51-52` asser ## 328. `audit-verify` cannot detect a truncated audit tail -> 🔢 **Filed 2026-08-01 — not started.** Value **6/10** · Difficulty **3/10** · _quick win_. Both shipped verification surfaces call `verify_audit_chain()` bare (`messagefoundry/__main__.py:3596`, `pipeline/engine.py:860`) and the `audit-verify` subparser declares only `--service-config` and `--db` (`__main__.py:571-578`), so a truncated keyed chain — the residue the anchor exists to catch — reports CLEAN with no way for an operator to supply one; the remainder is a new `audit-anchor` subcommand, an `--expected-anchor` flag into the already-present `expected_anchor=` keyword, and an `[integrity]` key for the startup path, with no change to the comparison logic and no store migration. _(was 5/10 · 3/10.)_ +> 🚧 **Status OPEN — Proposed 1-2 SHIPPED 2026-08-04, Proposed 3 DEFERRED.** `messagefoundry audit-anchor` (`--service-config` / `--db` / `--json`, with the same SQLite missing-DB refusal as its verify twin, so a typo'd path cannot mint an empty database and print an anchor OF NOTHING) prints `COUNT:HEAD`, and `audit-verify --expected-anchor COUNT:HEAD` / `--expected-anchor-file PATH` feeds it into the already-present `expected_anchor=` keyword — no comparison-logic change and no store migration, as filed. `docs/FEATURE-MAP.md`'s hand-maintained CLI count moved 30 to 31 with it. **Proposed 3 — the `[integrity]` startup-anchor key — is NOT built, which is why this stays OPEN.** The reason is measured, and pinned by `test_an_anchor_goes_stale_on_the_next_appended_row`: the shipped comparator is an EXACT point-in-time seal (row count *and* head hash), so a stored anchor consumed by the startup auto-verify would fire a false `integrity_drift` on essentially every restart, because any running instance writes audit rows. It needs a seal-on-stop / check-on-start design (or a monotonic-prefix comparator) before it is worth wiring, and the plumbing is a THREE-file edit — `config/settings.py`, `pipeline/engine.py`, and `api/app.py`'s `create_managed_app`, which is the only route an `[integrity]` key reaches the Engine by, and which the multi-session plan had scope-dropped. `[integrity].audit_verify_on_start` therefore remains a bare walk and still cannot see a truncated tail; that limit is now stated on its own `docs/CONFIGURATION.md` row and in ADR 0014 §16.4.2. The SQL Server and Postgres `audit_anchor` CLI tests are written and collect cleanly but have **never executed locally** (no Docker daemon) — they are CI-verified only. _(was 5/10 · 3/10.)_ > ⚠️ **AMENDED 2026-08-04 — `api/app.py` is IN scope; the multisession plan was wrong to drop it.** > `SCHEDULABLE-BACKLOG-MULTISESSION-PLAN.md` scoped this item as "CLI + settings" and said **DROP diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index c41c4efd..5e421d55 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -715,7 +715,7 @@ three built-in environment names (`dev`, `staging`, `prod`) derive PHI. The audi | `app_log_days` | int | `0` | past N days, **delete** application **log files** (`.log`/`.txt`, one level) from the configured `[logging].log_dir` (#120). The supervisor (NSSM `AppRotateBytes`) rotates the daily logs by **size** but never by **age**, so the log dir grows unbounded; this bounds it (by file mtime, so the currently-written file is never eligible). `0` = keep. **No-op unless `[logging].log_dir` is set.** Metadata only — file content is never read. While `app_log_compress_days` is on, the same window also ages out the `*.log.gz`/`*.txt.gz` archives that setting produces — so compressing a log doesn't make it immortal; with compression off the eligible set is exactly what it was | | `app_log_compress_days` | int | `0` | past N days, **gzip** application **log files** (`.log`/`.txt`, one level — the same selection as `app_log_days`, by mtime, so the currently-written file is never eligible) in `[logging].log_dir` to `.gz` (#119). The log stays readable (`gzip -d`) at a fraction of the disk, so a long-running box keeps far more history for the same footprint. Each file is **free-space prechecked** (`shutil.disk_usage` must show room for the source **plus** its archive plus a `max(10%, 1 MiB)` margin — short, and the file is **skipped and logged**, never attempted) and each written archive is **integrity-validated** — staged to an **exclusively created, randomly named** temp file beside it (`tempfile.mkstemp`: `O_CREAT\|O_EXCL`, so it never truncates an existing file, never follows a symlink, and never collides with a sibling engine shard compressing the same directory), `fsync`ed, re-read **off disk**, decompressed and compared **byte-for-byte** against the original, renamed into place, and then **validated again at `.gz` itself** — and it is that last check, on the bytes actually sitting where the log used to be, that authorizes removing the original. Any failure leaves the original **in place**, does not count it as compressed, and logs it; an existing `.gz` is never clobbered. The archive inherits the source's mtime, so `app_log_days` still ages it out. Files over 64 MiB are skipped (the codec is in-memory), and so is a file whose archive would not be **smaller** than it (an empty or already-compressed log — compressing must never *cost* disk). Names/counts/sizes are logged, **never file content**. `0` = never compress. **No-op unless `[logging].log_dir` is set.** Set it **shorter** than `app_log_days` — a longer window compresses nothing, since the delete sweep runs first | | `search_preset_days` | int | `0` | past N days, **delete** saved-search presets (ADR 0136) neither used nor edited since the cutoff. The stored `criteria` is the operator's own content/`field_value` needle — **PHI-shaped by construction**, encrypted at rest — so it needs a window like any other PHI tier (ASVS 14.2.7). The whole **row** is deleted, not blanked: a preset's entire payload *is* its criteria. **Keys on last-USED** (BACKLOG #306) — the cutoff is compared against the *later* of `updated_at` (written by a save) and `last_used_at` (written by a recall), so a preset you run daily but never re-save is **kept**. A preset last touched before the `last_used_at` column existed has it NULL and ages out on `updated_at` alone. `0` = keep forever (the default) | -| `audit_days` | int | `0` | **reserved / not enforced — keep-forever by design.** The rationale rests on the **audit-retention requirement** (45 CFR 164.316(b)(2)(i) six-year documentation retention; every framework floor is far below it — CIS Control 8.10 is 90 days, PCI DSS 4.0 §10.5.1 is 12 months, NIST SP 800-53 AU-11 defers to organizational policy), **not** on chain-breakage. *Corrected 2026-07-30:* this row used to argue "deleting rows would break the chain". That is true only of deleting the **oldest** rows, and it is exactly **inverted** for the threat that motivates audit retention — `verify_audit_chain`'s own docstring (`store/store.py:7385`) records that deleting the **newest** rows is *not* caught by the walk alone, because the surviving prefix still chains cleanly. An attacker hiding what they just did truncates the newest rows. **And the anchor that would catch it is not reachable from the shipped CLI** (`__main__.py:3418` calls `verify_audit_chain()` with no `expected_anchor`, and no flag supplies one), so `audit-verify` returns CLEAN after a tail-truncation; as shipped, truncation detection depends on the off-box tee, not on anything runnable locally. Do **not** restore the chain-breakage argument, and do **not** upgrade this to "the anchor detects it" without also shipping a way to pass one. Archive-first pruning is a tracked follow-up. **This row is the source of record for the chain-truncation reasoning**; `docs/PHI.md` §8 carries the retention-window inventory, and wherever its §8 still withholds pruning on the ground that "deleting rows would break the chain", that rationale is **superseded here** — do not read it as the authority | +| `audit_days` | int | `0` | **reserved / not enforced — keep-forever by design.** The rationale rests on the **audit-retention requirement** (45 CFR 164.316(b)(2)(i) six-year documentation retention; every framework floor is far below it — CIS Control 8.10 is 90 days, PCI DSS 4.0 §10.5.1 is 12 months, NIST SP 800-53 AU-11 defers to organizational policy), **not** on chain-breakage. *Corrected 2026-07-30:* this row used to argue "deleting rows would break the chain". That is true only of deleting the **oldest** rows, and it is exactly **inverted** for the threat that motivates audit retention — `MessageStore.verify_audit_chain`'s own docstring (`store/store.py:7535-7537`, and its `audit_anchor` sibling at `:7513-7518`) records that deleting the **newest** rows is *not* caught by the walk alone, because the surviving prefix still chains cleanly. An attacker hiding what they just did truncates the newest rows. *Updated 2026-08-04 ([BACKLOG #328](BACKLOG.md)):* **the anchor is now reachable from the CLI**, which is the condition this row set for changing this paragraph. `messagefoundry audit-anchor` prints `COUNT:HEAD`, and `messagefoundry audit-verify --expected-anchor COUNT:HEAD` (or `--expected-anchor-file PATH`) passes it back into `verify_audit_chain(expected_anchor=...)` (`_audit_verify`, `__main__.py:3727`). **A bare `audit-verify` is still clean after a tail-truncation** — that has not changed and is not a bug; the walk cannot see it, which is the entire reason the anchor exists. **Read the anchor's semantics before relying on it: it is an EXACT point-in-time seal, not a monotonic-prefix check.** It compares the row count *and* the head hash, so an anchor taken before any subsequent audit row reports `truncated or rewritten` on a chain that merely **grew**. It therefore seals a chain **at rest between two offline checks** — quiesce the engine, anchor, hold the value off-box, re-verify while the chain is still quiesced (across a maintenance window, a DB move, a backup/restore, a custodian hand-off) — and nothing else: anchoring and immediately re-verifying compares a value to itself, and re-checking a held anchor against a **running** engine alarms every boot. Do not write a compliance job that stores one anchor and re-checks it against a running engine; for a running engine the off-box tee is still the control. Do **not** restore the chain-breakage argument. Archive-first pruning is a tracked follow-up. **This row is the source of record for the chain-truncation reasoning**; `docs/PHI.md` §8 carries the retention-window inventory, and wherever its §8 still withholds pruning on the ground that "deleting rows would break the chain", that rationale is **superseded here** — do not read it as the authority | | `max_db_mb` | int | `0` | advisory only: warn (WARNING log + an `AlertSink` `storage_threshold` event) when the database exceeds this — measured as the **SQLite file + `-wal`/`-shm`**, `SUM(size)` over `sys.database_files` on **SQL Server**, and `pg_database_size()` on **Postgres**. Never auto-deletes. `0` = off | | `purge_interval_seconds` | float | `3600` | how often the purge/maintenance loop runs a pass | | `max_pass_seconds` | float | `0` | maximum wall-clock seconds **one maintenance pass** may spend (#121, [ADR 0137](adr/0137-time-boxed-retention-maintenance-pass-between-phase-cap.md)). A **between-phase soft cap**: `run_once` checks elapsed monotonic time before each phase and, once this is reached, **skips the remaining phases** (marking the pass `capped`) so a long pass can't run unbounded into the next maintenance window — the skipped tail re-runs next interval, and a skipped WAL-checkpoint/VACUUM does **not** advance its last-run marker. Checked only *between* phases, never inside one, so a running VACUUM is non-interruptible. `0` = off (the default — no cap); ~`14400` (4 h, the Corepoint off-peak ceiling) is the recommended value when enabled | @@ -1378,7 +1378,7 @@ an **editable** install (`pip install -e .` — no RECORD baseline) is a **no-op |---|---|---|---| | `enabled` | bool | `true` | run startup attestation at all. On by default (alert-only is harmless); a **no-op** off an editable install. Set `false` only to suppress the check entirely (e.g. an unusual packaging where RECORD is known-stale) — you then lose the in-place-tamper tripwire. | | `fail_closed_on_drift` | bool | `false` | when `true`, drift makes `serve` **refuse to start** (after recording the audit row + alerting). Default `false` = **alert-only**: a legitimate reviewed in-place security hotfix (the documented vendored-parser patch contingency) would itself trip a RECORD mismatch, so fail-closed-by-default would brick a legitimate patch. Opt in for hard enforcement on a locked-down instance. | -| `audit_verify_on_start` | bool | `false` | when `true`, the engine **re-walks the `audit_log` hash chain once at startup** (#190). **Alert-only by construction:** a broken chain logs a WARNING and fires the `AlertSink` but **never** crashes startup — a refuse-to-start on a tripped tamper alarm would be a self-inflicted DoS. Default `false` (opt in): on a very large `audit_log` the full re-walk adds startup latency, so it is not on by default. **Read the two limits below before citing this as tamper detection.** | +| `audit_verify_on_start` | bool | `false` | when `true`, the engine **re-walks the `audit_log` hash chain once at startup** (#190). **Alert-only by construction:** a broken chain logs a WARNING and fires the `AlertSink` but **never** crashes startup — a refuse-to-start on a tripped tamper alarm would be a self-inflicted DoS. Default `false` (opt in): on a very large `audit_log` the full re-walk adds startup latency, so it is not on by default. **It is a bare walk: it passes no anchor, so it is blind to a truncated tail** (below). Nothing here consumes an anchor — that is `messagefoundry audit-verify --expected-anchor`, run by an operator against a quiesced chain. **Read the two limits below before citing this as tamper detection.** | **The chain is *tamper-evident* only when the store is keyed.** With no store encryption key the cipher is `IdentityCipher`, whose `audit_mac_key()` returns `None` — "no DEK → no derived key → the audit chain @@ -1396,11 +1396,35 @@ deployment does get the keyed chain. The unkeyed chain is what a **synthetic** i you record "tamper-evident audit log" in a risk register. **And the walk does not catch a truncated tail.** `verify_audit_chain` detects modified or deleted -**older** rows, but deleting the **newest** rows leaves a prefix that still chains cleanly — and the -shipped `messagefoundry audit-verify` passes no `expected_anchor`, so it returns CLEAN after a -tail-truncation. An attacker hiding what they just did truncates the newest rows. Truncation detection -depends on the off-box log forward / tee, not on anything runnable locally. The full reasoning is the -[`[retention]`](#retention) `audit_days` row, which is the source of record for it. +**older** rows, but deleting the **newest** rows leaves a prefix that still chains cleanly, so a bare +walk returns CLEAN after a tail-truncation. An attacker hiding what they just did truncates the newest +rows. `audit_verify_on_start` is a bare walk and is therefore blind to exactly that. + +**What closes it is an anchor, and it is an operator command, not a startup setting** +([BACKLOG #328](BACKLOG.md)). `messagefoundry audit-anchor` prints `COUNT:HEAD`; passing it back as +`messagefoundry audit-verify --expected-anchor COUNT:HEAD` (or `--expected-anchor-file PATH`) compares +the live chain against it and reports `truncated or rewritten` when they differ. The anchor is a row +count plus a digest — no PHI, no secret — so it is safe to hold in a ticket or an object store, which +is what makes it an *external* witness. + +**The anchor is an EXACT point-in-time seal.** It compares the count **and** the head hash, so an +anchor taken before any subsequent audit row reports `truncated or rewritten` on a chain that merely +**grew**. The head half is not redundant with the count: an attacker who cuts the newest rows and +forges the same number of replacements restores the count *and* leaves a chain that walks cleanly, so +the head hash is the only thing that differs. The sharp edge and that detection are the same check. + +**So it seals a chain AT REST between two offline checks — that is the whole workflow, and it is the +only one with detection power.** Anchoring and immediately re-verifying compares a value to itself and +proves nothing; re-checking a held anchor against a **running** engine alarms on every ordinary boot, +because a running engine writes audit rows. What sits between those two useless readings is a real +control: **stop or quiesce the engine, take the anchor, hold it somewhere the engine's operator cannot +rewrite, and re-verify while the chain is still quiesced** — across a maintenance window, a database +move, a backup/restore, or a hand-off between custodians. Anything that happened to the DB in that gap +is what the anchor detects. Do not build a periodic job against a live engine on it. + +The full reasoning is the [`[retention]`](#retention) `audit_days` row, which is the source of record +for it. **For continuous coverage of a running engine, an off-box log forward / tee remains the answer** +— the anchor does not replace it and cannot, because the anchor is a seal on a stationary object. ### `[engine]` **Not implemented.** There is **no `EngineSettings` model**, so an `[engine]` block in @@ -1469,10 +1493,10 @@ and a PHI weakening under **strict enforcement** (`enforcement = enforce`, the d | `encrypt_stored_data` | bool | `true` | PHI encrypted at rest (key from the environment) | | `allow_unencrypted_phi` | bool | `false` | audited escape: start a PHI instance with **no** key | | `allow_unencrypted_phi_under_strict_enforcement` | bool | `false` | the **second acknowledgment** required to start a PHI instance keyless under strict enforcement ([ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md)). Under `enforcement = enforce`, `allow_unencrypted_phi = true` on its own is **not** enough — `serve` still refuses to start (exit 2) unless this is also set, so the highest-risk posture (real PHI + strict enforcement) is never one flag away from plaintext at rest. Under `enforcement = warn` the single `allow_unencrypted_phi` flag still governs. With both set the instance starts with PHI bodies, summary/metadata and the error columns **unencrypted at rest**, and the startup AUDIT line names **both** flags. A **loosening** — `security_loosenings()` reports it, so it is never silent | -| `allow_single_factor_admin_when_exposed` | bool | `false` | permit **single-factor admin on an exposed PHI instance** (ADR 0140). With `require_sign_in` on, `require_mfa` explicitly off, and the operator surface exposed — a **non-loopback bind**, or the console reached through a declared TLS-terminating proxy **with `serve_web_console` explicitly `true`** — a PHI instance under `enforcement = enforce` **refuses to start** (exit 2) — the Administrator role would authenticate with a single factor over the network. Setting this permits that start; it is recorded in a WARNING-level AUDIT line and the ordinary exposure warning still prints. A **loosening** — `security_loosenings()` reports it. **Blind spot — do not treat this refusal as your MFA control.** The proxy arm keys on the console actually being *served*, and a **default-on** console auto-degrades to JSON-only on an exposed instance (ADR 0143, flipped in place at startup), so the topology this document recommends — loopback bind behind a declared terminator, `serve_web_console` left at its default — does **not** trip the refusal even on a PHI instance under `enforce` with `require_mfa = false`. The JSON operator API is still reachable off-box through that proxy with single-factor admin. Set `require_mfa = true` there regardless; the gate will not catch you. **Prefer `require_mfa = true` — and know its scope.** Under the shipped `require_mfa_scope = "every_local_account"` it requires a second factor from **every** local account, *not* only Administrators, so a non-interactive **local** bearer-token service account becomes MFA-pending and cannot enrol unattended. **There are two remedies, not three.** Either make it a **directory (AD/Kerberos) principal** — those are out of scope under either value, their factor delegated to the directory — or set `require_mfa_scope = "administrators"` (itself reported as a loosening, and it leaves every local Administrator in scope) — see that row below. **mTLS is *not* the third.** A `[api].tls_client_cert_identities` mapping does grant a cert-identity that never meets the MFA gate, but that plane is admitted on exactly **one** route (`GET /service/identity`, `require_service_cert`) and carries no session, so an account "moved to mTLS" can read back its own identity and nothing else — it cannot replay, purge, poll status, or do any work a service account exists for. The `[api].tls_client_cert_identities` row above is the authority on that reach. Directory identities being out of scope also means an AD-only deployment is safe **for its AD users**; its local bootstrap admin and any local service accounts are still in scope | +| `allow_single_factor_admin_when_exposed` | bool | `false` | permit **single-factor admin on an exposed PHI instance** (ADR 0140). With `require_sign_in` on, `require_mfa` explicitly off, and the instance exposed — a **non-loopback bind**, **or** a declared TLS-terminating proxy (`[api].tls_terminated_upstream`) — a PHI instance under `enforcement = enforce` **refuses to start** (exit 2) — the Administrator role would authenticate with a single factor over the network. Setting this permits that start; it is recorded in a WARNING-level AUDIT line and the ordinary exposure warning still prints. A **loosening** — `security_loosenings()` reports it. **The exposure test does not consult the browser console** ([BACKLOG #326](BACKLOG.md); ADR 0140 amendment). It did, and that made the arm miss the topology this document recommends: a loopback bind behind a declared terminator with `serve_web_console` left at its default, where the ADR 0143 auto-degrade clears the console flag in place before the gate reads it. The exposed surface that authenticates with one factor is the **JSON operator API**, which the proxy serves whether or not `/ui` is mounted, so the predicate is the bind-and-proxy posture alone and the refusal fires on at least: an off-loopback bind; a declared proxy with the console left default-on; and a declared proxy with `serve_web_console = false`. **This refusal is the one exception to the "a new refusal fires only on a new opt-in" scoping rule** stated three rows below on `require_memory_encryption_declaration` — by owner ruling of 2026-08-04, recorded in the [ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md) amendment, which is the single source for why. Nothing new gates it. **One residual is deliberately left open:** an **undeclared** proxy — `web_console_public_address` set with no `tls_terminated_upstream` — does not count as exposed here, because nothing was declared, so exposure would be an *inference*, and an inference must not refuse. It **warns** instead, on its own dedicated arm: on a PHI instance with `require_mfa` explicitly off, startup prints that if that origin is served by an undeclared proxy the Administrator role is single-factor over the network and this refusal cannot see it. Do **not** read the ADR 0068 §8 undeclared-proxy warning as that control — it is about the `/ui` session cookie and HSTS, and it is suppressed entirely when the ADR 0143 auto-degrade clears the console flag, which the same `web_console_public_address` triggers. **Prefer `require_mfa = true` — and know its scope.** Under the shipped `require_mfa_scope = "every_local_account"` it requires a second factor from **every** local account, *not* only Administrators, so a non-interactive **local** bearer-token service account becomes MFA-pending and cannot enrol unattended. **There are two remedies, not three.** Either make it a **directory (AD/Kerberos) principal** — those are out of scope under either value, their factor delegated to the directory — or set `require_mfa_scope = "administrators"` (itself reported as a loosening, and it leaves every local Administrator in scope) — see that row below. **mTLS is *not* the third.** A `[api].tls_client_cert_identities` mapping does grant a cert-identity that never meets the MFA gate, but that plane is admitted on exactly **one** route (`GET /service/identity`, `require_service_cert`) and carries no session, so an account "moved to mTLS" can read back its own identity and nothing else — it cannot replay, purge, poll status, or do any work a service account exists for. The `[api].tls_client_cert_identities` row above is the authority on that reach. Directory identities being out of scope also means an AD-only deployment is safe **for its AD users**; its local bootstrap admin and any local service accounts are still in scope | | `allow_unverified_alert_smtp_tls` | bool | `false` | the **acknowledgment** required to start an enforcing PHI instance whose `[alerts]` SMTP hop does not authenticate the relay — i.e. `[alerts].email_use_tls = false` (cleartext) or `[alerts].email_tls_verify = false` (encrypted but accepts any certificate) ([#323](BACKLOG.md)). Covers BOTH shapes deliberately: cleartext is strictly worse than unauthenticated TLS, so gating only the second would hand an operator a bypass onto the worse posture. Without it `serve` refuses to start (exit 2); with it the start is permitted and named in a WARNING-level `AUDIT:` line. An **acknowledgment switch rather than the clamped `MEFOR_ALLOW_INSECURE_TLS` escape** the connectors use, because this cell is constructed outside the `active_hop_posture` scope where that clamp would be inert. A **loosening** — `security_loosenings()` reports it, so it is never silent | | `memory_encryption_operator_declared` | bool | `false` | **`[BUILT]` ([ADR 0152](adr/0152-in-use-data-protection-for-phi-platform-memory-encryption-attestation-asvs-11-7-1.md) rung 2, ASVS 11.7.1):** the operator's **declaration** that this host provides hardware memory encryption (AMD SEV-SNP / Intel TDX), so PHI is protected in RAM **while it is being processed**. The engine cannot verify it — a local CPU flag is emitted by the OS whose integrity the requirement protects against — so this records **who took responsibility**, the same discipline as `MEFOR_TLS_REVOCATION_ATTESTED`. It is deliberately **not** called "attested": in confidential computing that word means a CPU-signed quote verified against the silicon vendor's root PKI (ADR 0152 rung 3, **not built**). An **exposed** PHI instance without it **warns and starts** — on every environment, at both `enforcement` settings; it refuses only if `require_memory_encryption_declaration` is also set. A **positive platform read-out does not substitute for it** (a read-out must never relax a control). **Loopback and synthetic instances are byte-identical** (never consulted). If the platform read-out positively contradicts this, the contradiction is **warned at start and reported** as `memory_encryption_readout_contradicts_declaration` on `GET /security/posture` — but **never refused** (the read-out is a self-report, not evidence, and has known false negatives: driver not loaded, container without the device node mapped, Azure CVM paravisor). **Setting this does not make the instance ASVS 11.7.1-compliant** — see the read-out note below the table. Env: `MEFOR_SECURITY_MEMORY_ENCRYPTION_OPERATOR_DECLARED` | -| `require_memory_encryption_declaration` | bool | `false` | **`[BUILT]` (ADR 0152 rung 2):** turn the row-12 warning above into a **refusal** — an **exposed** PHI instance with no `memory_encryption_operator_declared` then **refuses to start** under `enforcement=enforce` (and still warns under `warn`). **Opt-in by design, and the default is load-bearing:** the property is a **host** property that no operator can satisfy on Windows (the read-out is always `null` there), and "exposed" includes the recommended loopback-behind-proxy topology, so a refusal by default would stop working dev/staging/prod deployments from booting on upgrade over something they cannot change. Same scoping rule as `[security].allowed_client_networks`' companion refusal (ADR 0151): a new refusal fires only on a new opt-in. Set it in an estate that has standardized on confidential-computing hosts and wants a missing declaration to be fatal. Env: `MEFOR_SECURITY_REQUIRE_MEMORY_ENCRYPTION_DECLARATION` | +| `require_memory_encryption_declaration` | bool | `false` | **`[BUILT]` (ADR 0152 rung 2):** turn the row-12 warning above into a **refusal** — an **exposed** PHI instance with no `memory_encryption_operator_declared` then **refuses to start** under `enforcement=enforce` (and still warns under `warn`). **Opt-in by design, and the default is load-bearing:** the property is a **host** property that no operator can satisfy on Windows (the read-out is always `null` there), and "exposed" includes the recommended loopback-behind-proxy topology, so a refusal by default would stop working dev/staging/prod deployments from booting on upgrade over something they cannot change. Same scoping rule as `[security].allowed_client_networks`' companion refusal (ADR 0151): a new refusal fires only on a new opt-in. **One exception exists, and it is recorded:** the `allow_single_factor_admin_when_exposed` refusal three rows above was corrected under BACKLOG #326 and fires with no new opt-in gating it — see that row and the [ADR 0140](adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md) amendment for the reasoning; do not generalise it. Set it in an estate that has standardized on confidential-computing hosts and wants a missing declaration to be fatal. Env: `MEFOR_SECURITY_REQUIRE_MEMORY_ENCRYPTION_DECLARATION` | | `organization_domains` | list[str] | `[]` | **`[BUILT]` (ASVS 3.7.3):** domains that count as **inside** your organization. The console interposes a "you are leaving this site" page, with a cancel, before any navigation to a destination **not** covered here. ASVS asks about destinations outside the application's **control**, and control is *organisational* rather than topological — your own AD FS is a different host, a different origin, and squarely yours — so this is a declared domain list, **not** a same-origin test. Matched on a **label boundary**: `hospital.example` covers `adfs.hospital.example` and **not** `evilhospital.example` (a bare suffix test would admit the lookalike, which is the failure that makes an interstitial worse than none). **Empty is the STRICT position, not the lax one:** with nothing declared, *every* absolute `http(s)` destination is treated as external and gets the page — including your own IdP. Declaring your domains here is the correct fix for that, **not** `external_link_allowlist`. Entries are bare domains: a URL, scheme or `*` wildcard is refused at config load, because all three look right and match nothing. Env: `MEFOR_SECURITY_ORGANIZATION_DOMAINS` | | `external_link_interstitial` | bool | `true` | **`[BUILT]` (ASVS 3.7.3):** show the "you are leaving this site" page at all. Setting it `false` means the console navigates off-site with **no notification and no cancel** — that is the control itself, so this is a posture decision rather than a convenience one, and `serve` prints a warning naming it at every start. The federated sign-in leg is affected: with the interstitial on and the IdP outside `organization_domains`, `GET /ui/oidc/start` renders the page and the flow is minted only on confirm (`POST`), which also closes the standing hole where any external page could begin a sign-in by linking to the start leg. Env: `MEFOR_SECURITY_EXTERNAL_LINK_INTERSTITIAL` | | `external_link_allowlist` | list[str] | `[]` | **`[BUILT]` (ASVS 3.7.3) — ⚠️ THE AUDITED ESCAPE, AND IT LOWERS SECURITY.** Destinations listed here are navigated to with **no notification and no cancel**, which is precisely what the requirement asks for. It exists for legitimate high-volume external destinations an operator does not want to declare as their own domain. Same label-boundary matching. Non-empty makes `serve` print a warning **naming every entry individually** — never a count, because "3 destinations exempted" is the shape of message that lets an entry nobody intended sit in a list for a year. **Prefer `organization_domains`**: declaring a domain you control is a statement about scope; allowlisting one you do not is a waiver. Env: `MEFOR_SECURITY_EXTERNAL_LINK_ALLOWLIST` | diff --git a/docs/EARLY-ADOPTER-GUIDE.md b/docs/EARLY-ADOPTER-GUIDE.md index fc61acf1..93afcd69 100644 --- a/docs/EARLY-ADOPTER-GUIDE.md +++ b/docs/EARLY-ADOPTER-GUIDE.md @@ -408,6 +408,19 @@ Full references: **[SECURITY.md](SECURITY.md)**, **[PHI.md](PHI.md)**, and **[DE surface in `last_error`/`detail`). - [ ] Run **`messagefoundry audit-verify`** periodically (the audit log is tamper-*evident*, not tamper-*proof*), and set `[retention]` windows — they are **off by default (kept forever)**. +- [ ] **Seal the audit DB across any gap in custody, with an anchor.** A bare `audit-verify` is clean + after the *newest* rows are deleted — the surviving prefix still chains — so on its own it is + blind to the attack it is run for. **`messagefoundry audit-anchor`** prints `COUNT:HEAD` (no PHI, + no secret) and **`messagefoundry audit-verify --expected-anchor COUNT:HEAD`** (or + `--expected-anchor-file`) checks it back. It is an **exact point-in-time seal**, and that fixes + how to use it: **stop or quiesce the engine, take the anchor, hold it somewhere the engine's + operator cannot rewrite, and re-verify while the chain is still quiesced** — around a maintenance + window, a database move, a backup/restore, or a hand-off between custodians. Whatever happened to + the DB in that gap is what it detects. **Two ways to get a useless answer:** taking the anchor and + verifying it in the same breath compares a value to itself, and re-checking a held anchor against + a **running** engine reports `truncated or rewritten` on every ordinary boot, because a running + engine writes audit rows. This is **not** a periodic control against a live engine — for that, an + off-box log forward / tee is still the answer, and it is the stronger one regardless. --- diff --git a/docs/FEATURE-MAP.md b/docs/FEATURE-MAP.md index 0fd101e5..471a6638 100644 --- a/docs/FEATURE-MAP.md +++ b/docs/FEATURE-MAP.md @@ -226,7 +226,7 @@ The PySide6 **desktop console was retired** and `messagefoundry/console/` delete | Environments + deferred `env()` values (`environments/.toml`) | ✅ | | | Env-aware promote (dry-run pre-flight) | ✅ | | | Config reload (`POST /config/reload`, allow-list-confined + audited) | ✅ | #85/#101 | -| CLI — 30 subcommands | ✅ | Run/author (`serve`, `supervise`, `init`, `import`, `validate`, `graph`, `dryrun`, `check`, `impact`, `connection`, `codeset`, `alert`, `security`, `generate`, `lens`, `hl7schema`, `hl7structures`, `adr-analyze`, `ai-policy`) + operate (`backup`, `restore-verify`, `rotate-key`, `rekey-audit`, `audit-verify`, `gen-key`, `protect-key`, `cert`, `verify`, `support-bundle`, `service`) — `_DISPATCH` in `__main__.py` is the registry | +| CLI — 31 subcommands | ✅ | Run/author (`serve`, `supervise`, `init`, `import`, `validate`, `graph`, `dryrun`, `check`, `impact`, `connection`, `codeset`, `alert`, `security`, `generate`, `lens`, `hl7schema`, `hl7structures`, `adr-analyze`, `ai-policy`) + operate (`backup`, `restore-verify`, `rotate-key`, `rekey-audit`, `audit-verify`, `audit-anchor`, `gen-key`, `protect-key`, `cert`, `verify`, `support-bundle`, `service`) — `_DISPATCH` in `__main__.py` is the registry | | Synthetic HL7 generators (ADT, …) | ✅ | `messagefoundry generate`; corpus git-ignored | | Windows service via NSSM | ✅ | docs/SERVICE.md | diff --git a/docs/REMOTE-CONSOLE.md b/docs/REMOTE-CONSOLE.md index eea33fc2..028b9566 100644 --- a/docs/REMOTE-CONSOLE.md +++ b/docs/REMOTE-CONSOLE.md @@ -179,11 +179,19 @@ Auth is on by default; remote users sign in with local accounts (± TOTP MFA) or `[security].allow_single_factor_admin_when_exposed` is not set ([`__main__.py`](../messagefoundry/__main__.py), the `admin_exposed` block). Either switch turns the refusal into a loud, audited warning that starts. A non-PHI instance is silent. - It also has a **blind spot**: the proxy arm keys on the console actually being *served*, and a - default-on console auto-degrades to JSON-only when exposed (§3) — so the recommended - loopback-behind-a-terminator topology does **not** trip it, while the JSON operator API is still - reachable off-box. Don't treat the refusal as your MFA control; see the - `allow_single_factor_admin_when_exposed` row in [`CONFIGURATION.md`](CONFIGURATION.md) and + **"Exposed" here is the bind-and-proxy posture, not the console**: an off-loopback bind, **or** + `[api].tls_terminated_upstream` — whether or not `/ui` ends up mounted. So the recommended + loopback-behind-a-terminator topology in §3 **does** trip it, including when the default-on console + auto-degrades to JSON-only, and when `serve_web_console = false` disables the console outright: the + single-factor surface being protected is the JSON operator API. (This is a correction — + [BACKLOG #326](BACKLOG.md); the arm used to read the console flag, which the §3 auto-degrade clears + first, and would have missed exactly that topology on first deployment.) An **undeclared** proxy — + a set `[security].web_console_public_address` with no `tls_terminated_upstream` — is outside the + predicate and does **not** refuse: nothing was declared, so exposure would be an inference. It gets + its own **warning** instead, naming single-factor admin explicitly, on a PHI instance with + `require_mfa` off. That is a distinct arm — **not** the ADR 0068 §8 undeclared-proxy warning, which + is about the `/ui` cookie and HSTS and is suppressed by §3's auto-degrade in the same posture. See + the `allow_single_factor_admin_when_exposed` row in [`CONFIGURATION.md`](CONFIGURATION.md) and [`SECURITY-LOOSENING.md`](SECURITY-LOOSENING.md). - Under the shipped `require_mfa_scope = "every_local_account"`, a non-interactive **local** bearer-token service account becomes MFA-pending and cannot enrol unattended. Settle this **before** diff --git a/docs/SECURITY-LOOSENING.md b/docs/SECURITY-LOOSENING.md index 444a78f6..c7a77878 100644 --- a/docs/SECURITY-LOOSENING.md +++ b/docs/SECURITY-LOOSENING.md @@ -223,6 +223,17 @@ trail. factor over the network. This ack **downgrades that refusal to a loud, audited warning** (the same warn-and-start `enforcement = warn` takes, but scoped to this one control), so the instance boots single-factor while staying at `enforce`. +- **Scope correction ([BACKLOG #326](BACKLOG.md)):** "a declared reverse proxy" above means exactly + `[api].tls_terminated_upstream` — the bind-and-proxy posture, **independent of the browser console**. The + shipped predicate additionally required the console to be *served*, which the ADR 0143 auto-degrade had + already turned off, so a loopback-behind-a-declared-proxy instance would not have reached this refusal at + all on first deployment and this ack would have had nothing to lift there. The wording in this section was + already the intended scope; the code now matches it, and the ack itself is unchanged. An **undeclared** + proxy (a set `web_console_public_address` with no `tls_terminated_upstream`) stays outside the predicate + — nothing was declared, so exposure there is an inference, and an inference must not refuse. It has its + own startup **warning**, which names single-factor admin directly on a PHI instance with `require_mfa` + off; read that arm, not the ADR 0068 §8 undeclared-proxy warning, as the control for this case (§8 is + about the `/ui` cookie and HSTS, and the ADR 0143 auto-degrade suppresses it in the same posture). - **When acceptable:** a production exposure where the second factor is supplied by a **compensating control outside MessageFoundry** — an authenticating reverse proxy / mTLS admin gateway, or AD/Kerberos MFA delegated to the directory (this flag gates only local Administrator accounts). diff --git a/docs/SECURITY.md b/docs/SECURITY.md index f408bcb6..c4ea2e95 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -1080,8 +1080,8 @@ one-to-one — that is why the bind/exposure posture occupies two rows and the A | PHI-read volume, per actor | `identity.user_id` | > 120 reads (`phi_read_rate_limit_per_actor`) per 60 s (`phi_read_rate_limit_window_seconds`); the global dimension `phi_read_rate_limit_global` defaults to `0` = **off** | **THROTTLE** 429 + `Retry-After: 10`, WARNING-logged, charged at **admission** before any store work | on, 120 / 60 s | `[auth].phi_read_rate_limit_enabled` | | Admin-write rate, per actor | `identity.user_id` × request method | **non-GET only**; > 12 writes (`admin_write_rate_limit_per_actor`) per 1.0 s (`admin_write_rate_limit_window_seconds`); no global dimension (`glob=0`) | **THROTTLE** 429 + `Retry-After: 1`, WARNING-logged. **JSON API only** — no `/ui` route charges it at this release | on, 12 writes / 1.0 s | `[auth].admin_write_rate_limit_enabled` | | Serve-hop security posture | declared data class (`[ai].data_class`, or derived from `[ai].environment`) × `[security].enforcement` × (`api.is_loopback` **or** `exposure_protected`), via `phi_read_hop_disposition` | disposition is REFUSE — a **PHI** instance under `enforcement = enforce` whose serve hop is neither loopback, nor in-process TLS, nor a declared TLS-terminating proxy. Setting `[security].enforcement = warn` turns the refusal into WARN-and-serve; a non-PHI declared data class removes it entirely | **DENY** 403 (PHI-free message) on every **JSON-API** PHI-read route (`require_phi_read`, plus the step-up bulk routes), **before** any identity work. **Not applied on the `/ui` browse routes** — `enforce_phi_read_hop` has no console call site, so those get the per-actor budget only (pinned by `test_the_ui_phi_browse_gap_is_disclosed`) | ALLOW on loopback | `[security].enforcement`, `[ai].data_class`/`environment`, `[api].tls_cert_file`, `tls_terminated_upstream` + `trusted_proxies` | -| Bind / exposure posture — refusing arms | `[api].host` loopback-ness, `tls_terminated_upstream`, `trusted_proxies`, `public_origin`, `serve_ui`; derived `ui_exposed`, `admin_exposed`; `[security].enforcement`; declared data class | auth off on a non-loopback bind; `/ui` exposed without the required origin/TLS declarations; `admin_exposed` + PHI + `enforcing` + `require_mfa` explicitly opted out | **DENY at startup** — `serve` prints an error and exits **2**. The refuse/warn dial is `[security].enforcement` (default `enforce`), **not** `production`: the auth-off and `/ui`-exposure arms refuse **unconditionally**, and the `require_mfa` arm refuses when the declared data class is PHI **and** enforcement is `enforce` — which includes the non-production `dev` and `staging` environments, both of which derive PHI — and warns otherwise. `[security].allow_single_factor_admin_when_exposed = true` downgrades that one arm to permitted-but-audited. The same attributes force the session cookie's `Secure` flag + HSTS, and permit WebAuthn `rp_id` derivation from the request URL **only** on a loopback bind with no proxy declared | loopback, nothing declared | `[api].*`, `[security].enforcement`, `[security].allow_single_factor_admin_when_exposed`, `[ai].data_class`/`environment` | -| Bind / exposure posture — dual-control arm | `admin_exposed` × `[approvals].enabled` × declared data class | `admin_exposed` **and** PHI **and** `[approvals].enabled` off — high-value actions complete on one caller's authority | **LOG** — a startup **WARNING only, on every instance including production**; `serve` does **not** refuse. The refuse arm is an explicit unresolved owner fork recorded in `__main__.py`, not a shipped control | approvals off | `[approvals].enabled` | +| Bind / exposure posture — refusing arms | `[api].host` loopback-ness, `tls_terminated_upstream`, `trusted_proxies`, `public_origin`; derived `instance_exposed` (loopback-ness **or** a declared terminator) and `admin_exposed`, plus `ui_exposed` for the `/ui` arms only; `[security].enforcement`; declared data class | auth off on a non-loopback bind; `/ui` exposed without the required origin/TLS declarations; `admin_exposed` + PHI + `enforcing` + `require_mfa` explicitly opted out | **DENY at startup** — `serve` prints an error and exits **2**. The refuse/warn dial is `[security].enforcement` (default `enforce`), **not** `production`: the auth-off and `/ui`-exposure arms refuse **unconditionally**, and the `require_mfa` arm refuses when the declared data class is PHI **and** enforcement is `enforce` — which includes the non-production `dev` and `staging` environments, both of which derive PHI — and warns otherwise. `[security].allow_single_factor_admin_when_exposed = true` downgrades that one arm to permitted-but-audited. **`admin_exposed` is `instance_exposed`, and reads no console flag** (BACKLOG #326): the ADR 0143 degrade arms rewrite `serve_ui` in place earlier in the same startup, so deriving an exposure decision from it made this arm and the dual-control arm below miss a declared-proxy instance whose console had been degraded or disabled — while the ASVS 11.7.1 arm called that same boot exposed. The same attributes force the session cookie's `Secure` flag + HSTS, and permit WebAuthn `rp_id` derivation from the request URL **only** on a loopback bind with no proxy declared | loopback, nothing declared | `[api].*`, `[security].enforcement`, `[security].allow_single_factor_admin_when_exposed`, `[ai].data_class`/`environment` | +| Bind / exposure posture — dual-control arm | `admin_exposed` (= `instance_exposed`: an off-loopback bind **or** a declared TLS terminator — never the console flag, BACKLOG #326) × `[approvals].enabled` × declared data class | `admin_exposed` **and** PHI **and** `[approvals].enabled` off — high-value actions complete on one caller's authority | **LOG** — a startup **WARNING only, on every instance including production**; `serve` does **not** refuse. The refuse arm is an explicit unresolved owner fork recorded in `__main__.py`, not a shipped control | approvals off | `[approvals].enabled` | | Pending federated-login flows, per client IP | the `client_ip` recorded on each staged flow | ≥ **16** pending flows from this address (`DEFAULT_PER_IP_CAP`, no knob), or ≥ `oidc_flow_cache_max` (**512**) engine-wide; 300 s TTL; **reject-when-full, never evict** (evict-oldest would turn a start-leg flood into a login DoS) | **DENY** the start leg — `FlowCacheFullError` → **303** to `/ui/login?e=rate_limited`, WARNING-logged, deliberately **never** audited so a flood cannot amplify into `audit_log` growth | 16 / 512 / 300 s | `[auth].oidc_flow_cache_max`, `oidc_flow_ttl_seconds` | | `Sec-Fetch-Mode` on the federated sign-in legs | the browser fetch-metadata header on `GET /ui/sso`, `POST /ui/oidc/start`, `GET /ui/oidc/callback` | header **present** and not `navigate` (absent = allowed, for non-browser clients). Distinct from the `Sec-Fetch-Site` row below: a different header, a different surface, and `assert_same_origin` deliberately does **not** run on the callback leg, whose `Sec-Fetch-Site` is legitimately cross-site | **DENY** — 303 → `/ui/login?e=sso_failed`\|`oidc_failed`, plus an **audited** `auth.login_failed` row carrying the closed-set slug `non_navigation_fetch`. Evaluated **after** the login limiter, so the audit write is itself rate-bounded | on | (no knob) | | Instance environment posture × claimed AI data scope | `[ai].derived_posture()` (from `[ai].environment` / `data_class` / `production`; an unresolved posture defaults to the **strictest** ceiling) re-resolved server-side through `resolve_effective_policy` on every `POST /ai/chat` | the effective mode is not `managed_endpoint`, or the request's `data_scope` exceeds the server-enforced ceiling (the engine-broker MVP enforces `code_only` regardless of what the caller claims) | **DENY** — **409** on the mode mismatch, **403** on scope excess; each audited `ai.assist` with PHI-safe metadata only | `mode = byo`, `data_scope = code_only` | `[ai].mode`, `[ai].data_scope`, `[ai].environment`/`data_class`/`production` | @@ -1623,7 +1623,18 @@ inherited from another caller. It is surfaced on `GET /audit` and in the `audit: **Tamper-evidence (AUDIT-INTEGRITY).** Each `audit_log` row carries a `row_hash` that chains the previous row's hash with this row's content (SHA-256), so deleting, editing, or reordering any row is -detectable. Verify the chain with `messagefoundry audit-verify` (exit 0 = intact). Rows written +detectable. Verify the chain with `messagefoundry audit-verify` — exit 0 means at least that no +surviving row was edited or reordered. It does **not** mean nothing was removed: deleting the *newest* +rows leaves a prefix that still chains cleanly, so a bare verify is clean after a tail-truncation. For +that, snapshot `messagefoundry audit-anchor` (`COUNT:HEAD`) and pass it back as `messagefoundry +audit-verify --expected-anchor`. It is an exact point-in-time seal, which fixes what it is for: it +seals a chain **at rest across a gap** — quiesce the engine, anchor, hold the value off-box, re-verify +while the chain is still quiesced (a maintenance window, a DB move, a backup/restore, a custodian +hand-off). Anchoring and re-verifying in one breath compares a value to itself, and a held anchor +re-checked against a **running** engine alarms on every boot, because a running engine writes audit +rows; for continuous coverage the off-box tee is still the control ([BACKLOG #328](BACKLOG.md); the +`[retention].audit_days` row in [`CONFIGURATION.md`](CONFIGURATION.md) is the source of record). Rows +written before the feature are chained on first start. The `client` address is folded **inside** the chained payload — deliberately, since attribution an attacker could rewrite without breaking tamper-evidence would be worse than none — as a **conditional trailing element**, appended only when non-`NULL`. A diff --git a/docs/adr/0014-alerting-rules-engine.md b/docs/adr/0014-alerting-rules-engine.md index 19374ebd..eb5ee67e 100644 --- a/docs/adr/0014-alerting-rules-engine.md +++ b/docs/adr/0014-alerting-rules-engine.md @@ -226,7 +226,11 @@ install. All three shipped defaults are unchanged and byte-identical. `[ai].data_class = "phi"` + a store key + the one-time `messagefoundry rekey_audit_chain` migration, after which keying is automatic. **Residual (accepted):** lever (B) depends on store encryption being on (off by default), so 16.4.2 is **Partial** until the WP #243 needle-mover turns encryption on — - tamper-evident today, forgery-evident once keyed. + tamper-evident today, forgery-evident once keyed. **A second residual on lever (A), added 2026-08-04 + ([BACKLOG #328](../BACKLOG.md)):** it is a *bare* walk — it passes no anchor — so it cannot see a + truncated tail (the surviving prefix still chains cleanly), whatever the keying state. The operator + lever for that is `messagefoundry audit-verify --expected-anchor`, run against a quiesced chain; there + is no startup setting that consumes an anchor. Do not read (A) as covering deletion of the newest rows. Each control is **A → Partial (accepted) / B → Pass** — none reaches an unconditional loopback (Posture-A) Pass; the exposed-deployment Pass is the instructed configuration above. This ADR does not diff --git a/docs/adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md b/docs/adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md index 56cc993e..09687316 100644 --- a/docs/adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md +++ b/docs/adr/0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md @@ -163,3 +163,82 @@ ack keeps its name). At the default (`enforce` × PHI) behaviour is **byte-ident non-production warn-and-start), so the two acks remain the **surgical stay-at-`enforce`, lift-exactly-one- control** alternative. The four floor items that stay hard-refused and the unconditional ePHI audit are unchanged. (This supersedes the original ack name in §Decision item 2 and the switch-2 discussion above.) + +## Amendment (2026-08-04) — the exposure predicate no longer reads the console flag (BACKLOG #326) + +The single-factor-admin-at-exposure carve-out above is keyed on `admin_exposed`. As shipped that was +`not settings.api.is_loopback or ui_exposed`, and `ui_exposed` is derived from `settings.api.serve_ui` +— a field the two [ADR 0143](0143-web-console-on-by-default-disableable-with-loopback-secure-context-browser-hardening.md) +degrade arms rewrite **in place** earlier in the same `serve` call (soft-degrade when the console wheel +is absent; auto-degrade when a default-on console meets an exposed bind). By the time this gate read it, +`serve_ui` answered "is `/ui` mounted?" — a presentation fact — rather than "is the admin interface +reachable from the network?", the exposure fact the carve-out is about. + +The consequence would have been visible on first deployment of the topology the runbooks **recommend**: +a loopback bind behind a declared TLS terminator, with `[security].serve_web_console` left at its +default. The auto-degrade clears the console flag, so `admin_exposed` evaluated `False`, so a production +PHI instance under `enforcement = enforce` with `[security].require_mfa = false` would have started +clean — with the JSON operator API reachable off-box through that proxy on a single admin factor, and +with `allow_single_factor_admin_when_exposed` having nothing to lift. The same boot was simultaneously +classified **exposed** by the ASVS 11.7.1 arm ([ADR 0152](0152-in-use-data-protection-for-phi-platform-memory-encryption-attestation-asvs-11-7-1.md)), +which already used the console-independent predicate. One startup, two contradictory answers to "is this +instance exposed?". + +**Corrected keying.** `instance_exposed = not settings.api.is_loopback or +settings.api.tls_terminated_upstream` is now defined **once**, above the first consumer, and both the +MFA-at-exposure arm (`admin_exposed = instance_exposed`) and the ASVS 11.7.1 arm read that one +definition. Neither reads `serve_ui`. `ui_exposed` survives, scoped to what it actually describes: the +`/ui`-specific origin/TLS refusals and the browser-console advisories. + +**What changes.** The refusal now fires on at least three postures it did not reach before: a declared +proxy with the console default-on (auto-degraded), a declared proxy with `serve_web_console = false`, +and a declared proxy where the console wheel is simply not installed. An off-loopback bind behaved +correctly before and is unchanged. A plain loopback bind with nothing declared is **not** exposed and +is byte-identical — the property the loopback-quiet tests pin. + +**This is a tightening, and it is deliberate.** A configuration that starts today would refuse on first +deployment of the corrected build. Per the owner ruling of 2026-08-04 it ships as a real refusal: no +warning-first phase, no dated flip, and **no new opt-in gating it**. There are no deployments to protect +from an upgrade, so the simple correct end state is what gets built. The refusal continues to ride the +existing `[security].enforcement` refuse/warn split like every sibling gate. + +**The acknowledgment is unchanged.** `[security].allow_single_factor_admin_when_exposed` still lifts this +one refusal to permitted-but-audited, with its WARNING-level `AUDIT:` line and its entry in +`security_loosenings()`. The ruling forbade a *new* opt-in gating the corrected refusal; it did not +retire this one. Its practical reach grows, because it now has the postures above to act on. + +**Residual, deliberately left open — an UNDECLARED proxy is still not "exposed", and it now has its own +warning.** A set `[api].public_origin` (`[security].web_console_public_address`) with no +`tls_terminated_upstream` on a loopback bind does not satisfy `instance_exposed`, so this refusal does +not fire there. That is a choice: nothing has been declared, so exposure would be an *inference*, and +the ruling that tightened this gate was about a **declared** proxy. Promoting an inference to a refusal +is a different decision, and was not authorised here. + +An earlier draft of this amendment justified leaving it open by asserting the case "still warns", +pointing at the ADR 0068 §8 undeclared-proxy heuristic. **That premise was false, and measurement is +what showed it**, so it is corrected here rather than quietly dropped. §8's warning is about the `/ui` +session cookie and HSTS — it says nothing about admin factors — and it is gated on +`settings.api.serve_ui`, which the ADR 0143 auto-degrade clears in place for exactly this input (a +default-on console plus a set `public_origin`). On the commonest shape of the posture it therefore did +not print at all: the residual was not "warned rather than refused", it was **silent**. A compensating +control must not rest on a false premise, so the control was built rather than the sentence softened — +a dedicated arm now warns, naming single-factor admin, when `public_origin` is set, the instance is not +`instance_exposed`, the declared data class is PHI, and `require_mfa` was explicitly opted out. A truly +signal-less undeclared proxy — no `public_origin` either — remains undetectable in-engine and +runbook-only, as ADR 0068 §8 records. + +**A second residual, NOT closed here — the auth-disabled refusal still keys on the bind alone.** The +much earlier startup arm for `[auth] enabled = false` reads `not settings.api.is_loopback`, several +hundred lines above where `instance_exposed` is defined, and this change does not touch it. So a PHI +instance with authentication entirely off, on a loopback bind behind a declared terminator, would still +start on first deployment — and the loosening text it prints still reads "loopback-only; a non-loopback +bind refuses", which the corrected exposure model contradicts. That is the same +two-answers-in-one-startup shape this amendment corrects, one arm over. It was left alone deliberately: +the owner ruling named `admin_exposed`, and re-keying the auth-off arm needs its own hoist (it runs +long before the current definition) plus its own adjudication, because it would convert that topology +from starting to refusing. **Recorded here as an open item for the owner** so it is not rediscovered as +a surprise. + +**Also amended:** [ADR 0143](0143-web-console-on-by-default-disableable-with-loopback-secure-context-browser-hardening.md) +carries a cross-reference noting that its in-place `serve_ui = False` flips no longer feed any exposure +predicate. diff --git a/docs/adr/0143-web-console-on-by-default-disableable-with-loopback-secure-context-browser-hardening.md b/docs/adr/0143-web-console-on-by-default-disableable-with-loopback-secure-context-browser-hardening.md index 023560c8..c8d714b3 100644 --- a/docs/adr/0143-web-console-on-by-default-disableable-with-loopback-secure-context-browser-hardening.md +++ b/docs/adr/0143-web-console-on-by-default-disableable-with-loopback-secure-context-browser-hardening.md @@ -172,3 +172,23 @@ Posture-A ASVS re-score is owner-gated and handled separately. - [x] `security_loosenings()` untouched (the switch is not a loosening); `docs/SECURITY-LOOSENING.md` reframed + switch-table default flipped. - [x] Docs (`CONFIGURATION.md`, `SECURITY.md`, `PHI.md`, `OFF-LOOPBACK-DEPLOYMENT.md`) + `ide/src/securityEditor.ts` flip "off by default" → on by default. - [ ] Posture-A ASVS re-score (3.3.1/3.3.3 stay Partial on loopback; auto-TLS deferred) — **owner-gated, separate**. + +## Cross-reference (2026-08-04) — the in-place `serve_ui = False` flips feed no exposure predicate (BACKLOG #326) + +Both degrade arms decided here — the package-absent soft-degrade and the exposed-bind auto-degrade — +rewrite `settings.api.serve_ui` **in place** during `serve`, deliberately, so the JSON-only decision +threads through the remaining gates and `create_managed_app`. That is unchanged. + +What changed is downstream: the MFA-at-exposure and #189 dual-control arms used to derive +`admin_exposed` from that already-rewritten field, so an auto-degrade silently cleared an exposure +refusal it was never meant to touch. Those arms now read a single console-independent +`instance_exposed` (an off-loopback bind **or** `[api].tls_terminated_upstream`). See the +[ADR 0140](0140-two-acknowledged-production-phi-no-loosen-carve-outs-single-factor-admin-at-exposure-keyless-phi-in-production.md) +amendment of the same date. + +Consequence for this ADR: an in-place flip decided here is now purely a **presentation** decision — it +determines whether `/ui` is mounted, and nothing else. `ui_exposed` remains, scoped to the +`/ui`-specific origin/TLS refusals and browser-console advisories that legitimately depend on the +console being served. This ADR mentioned neither `require_mfa` nor `admin_exposed`, which is how the +interaction came to exist without ever being adjudicated; recording it here is what puts it on the +record. diff --git a/messagefoundry/__main__.py b/messagefoundry/__main__.py index 0ff9950d..ae1b1ce0 100644 --- a/messagefoundry/__main__.py +++ b/messagefoundry/__main__.py @@ -576,6 +576,38 @@ def main(argv: list[str] | None = None) -> int: help="service settings TOML (default: ./messagefoundry.toml if present)", ) audit_verify.add_argument("--db", default=None, help="store path (overrides [store].path)") + # ONE mutually-exclusive group: the two flags carry the same value in two transports, and argparse + # refusing both is better than silently letting one win. + audit_verify_anchor = audit_verify.add_mutually_exclusive_group() + audit_verify_anchor.add_argument( + "--expected-anchor", + default=None, + metavar="COUNT:HEAD", + help="also compare against an anchor previously printed by 'audit-anchor'. The hash-chain " + "walk alone CANNOT see a truncated tail (the surviving prefix still chains cleanly); this " + "is what detects it", + ) + audit_verify_anchor.add_argument( + "--expected-anchor-file", + default=None, + metavar="PATH", + help="read the COUNT:HEAD anchor from a UTF-8 text file holding the output of " + "'messagefoundry audit-anchor' (on PowerShell 5.1 pipe to 'Set-Content -Encoding utf8' — " + "its '>' writes UTF-16, which is refused)", + ) + + audit_anchor = sub.add_parser( + "audit-anchor", + help="print the audit log's external anchor (COUNT:HEAD) to hold out-of-band and pass back " + "to 'audit-verify --expected-anchor'", + ) + audit_anchor.add_argument( + "--service-config", + default=None, + help="service settings TOML (default: ./messagefoundry.toml if present)", + ) + audit_anchor.add_argument("--db", default=None, help="store path (overrides [store].path)") + audit_anchor.add_argument("--json", action="store_true", help="emit JSON") rekey_audit = sub.add_parser( "rekey-audit", @@ -1859,6 +1891,31 @@ def _serve(args: argparse.Namespace) -> int: file=sys.stderr, ) + # THE SINGLE DEFINITION OF "this instance is exposed" (BACKLOG #326). Derived here, above the first + # consumer, from two fields no earlier arm reassigns — `is_loopback` and `tls_terminated_upstream` + # are read straight off the loaded config and are never mutated in place, unlike `serve_ui`. + # + # WHY IT CANNOT READ `settings.api.serve_ui`: that field is flipped to False IN PLACE twice above — + # the ADR 0143 soft-degrade when the console wheel is absent, and the ADR 0143 auto-degrade when a + # default-on console meets an exposed bind. By this line it answers "is /ui mounted?", a PRESENTATION + # fact, not "is the admin interface reachable from the network?", the EXPOSURE fact these gates need. + # Keying an exposure gate on it let one boot call the same instance exposed for ASVS 11.7.1 (the + # ADR 0152 arm below, which already used this predicate) and NOT exposed for ASVS 6.3.3 (the MFA arm + # here) — so the refusal was unreachable on the runbook's RECOMMENDED loopback-behind-proxy topology. + # The admin surface that authenticates with a single factor is the JSON API, which is served whether + # or not the browser console is. + # + # Deliberately NARROW: a set `public_origin` with no declared proxy is NOT exposure here. Widening + # it that far would convert a heuristic into a hard refusal, and the signal is genuinely weaker — + # nothing has been declared, so the engine is guessing. The residual that leaves is warned about + # EXPLICITLY, by the arm below the MFA gate. It must not lean on the ADR 0068 §8 undeclared-proxy + # warning at the top of this ladder: that one is about the /ui cookie and HSTS, it says nothing + # about admin factors, and it is gated on `serve_ui`, which the ADR 0143 auto-degrade has already + # cleared for exactly this input — a DEFAULT-on console plus a set `public_origin` — so on the + # commonest shape of this posture it does not print at all. Citing it as the compensating control + # would have rested that control on a premise measurement contradicts. + instance_exposed = not settings.api.is_loopback or settings.api.tls_terminated_upstream + # MFA-at-exposure posture (sec-mfa-on; WP-14, ASVS 6.3.3): an off-loopback bind serving local # accounts puts admin authentication on the network, where a single password factor is far weaker. # [security].require_mfa adds the native TOTP second factor for the Administrator role; with it off the @@ -1873,18 +1930,19 @@ def _serve(args: argparse.Namespace) -> int: # gates LOCAL Administrator accounts (the bootstrap admin is one) — it is safe to leave on even on # an AD-only deployment. # - # L5b review fix (ADR 0068 §8): the gate keys on the same EXPOSURE signal as the ladder above, - # not the bind host alone — the runbook's RECOMMENDED topology (loopback bind BEHIND a declared - # proxy, `ui_exposed`) puts the admin interface on the network exactly as an off-loopback bind - # does, so a production PHI console exposed through a declared proxy with require_mfa off is - # refused identically (extend-never-weaken). - admin_exposed = not settings.api.is_loopback or ui_exposed + # L5b review fix (ADR 0068 §8), corrected by BACKLOG #326: the gate keys on the same EXPOSURE signal + # as the ladder above, not the bind host alone — the runbook's RECOMMENDED topology (loopback bind + # BEHIND a declared proxy) puts the admin interface on the network exactly as an off-loopback bind + # does, so a production PHI instance reached through a declared proxy with require_mfa off is refused + # identically (extend-never-weaken). It reads `instance_exposed`, NOT the mutated console flag: the + # single-factor admin surface is the JSON API, so whether /ui happens to be mounted is irrelevant. + admin_exposed = instance_exposed if admin_exposed and settings.auth.enabled and not settings.auth.require_mfa: exposure_desc = ( f"API bound to non-loopback host {settings.api.host!r}" if not settings.api.is_loopback - else "browser console exposed through a declared reverse proxy " - "([api].serve_ui + tls_terminated_upstream)" + else "admin interface reached through a declared reverse proxy " + "([api].tls_terminated_upstream)" ) if data_class is DataClass.PHI: if enforcing and not settings.security.allow_single_factor_admin_when_exposed: @@ -1919,14 +1977,42 @@ def _serve(args: argparse.Namespace) -> int: file=sys.stderr, ) + # --- the UNDECLARED-proxy residual of the gate above, made visible (BACKLOG #326) --------------- + # `instance_exposed` is deliberately narrow, so a set `public_origin` on a loopback bind with no + # declared terminator does not refuse. That is the right call — nothing was declared, so the engine + # is inferring — but it must not be SILENT, and until this arm existed it was: the only other thing + # that could have spoken is the ADR 0068 §8 undeclared-proxy warning above, which is scoped to the + # /ui cookie and HSTS and is suppressed outright when the ADR 0143 auto-degrade clears `serve_ui` + # (which that same `public_origin` triggers). So the documented compensating control did not exist + # on the commonest shape of this posture. WARN, never refuse: the ruling that tightened the gate + # above was about a DECLARED proxy, and promoting an inference to a refusal is a different decision. + # Scoped as tightly as the refusal is: PHI only, and only where require_mfa was EXPLICITLY opted out. + if ( + not instance_exposed + and settings.api.public_origin + and settings.auth.enabled + and not settings.auth.require_mfa + and data_class is DataClass.PHI + ): + print( + "warning: [api].public_origin is set with no declared TLS terminator on a PHI instance " + f"({env_name!r}) with [security].require_mfa off — if that origin is served by an " + "UNDECLARED reverse proxy, the Administrator role is single-factor over the network and " + "the MFA-at-exposure refusal cannot see it (an undeclared proxy is not, and cannot be, an " + "exposure signal the engine can verify). Declare it with [api].tls_terminated_upstream + " + "trusted_proxies, or set [security].require_mfa=true.", + file=sys.stderr, + ) + # --- #189 dual-control-at-exposure posture (ASVS 2.3.5) ----------------------------------------- # High-value runtime actions (dead-letter replay, connection purge) complete on a SINGLE caller's # authority unless [approvals].enabled turns on maker-checker (a distinct second user holding # approvals:approve releases the request). On an off-box admin surface that concentration is the # weakest link: one compromised/coerced admin session can replay full-PHI dead-letters or purge a # connection with no second sign-off. Key on the SAME exposure signal as the MFA gate above - # (admin_exposed = off-loopback bind OR declared-proxy ui_exposed), so a loopback default is - # byte-identical (admin_exposed is False → this never trips) and a synthetic instance stays quiet + # (admin_exposed = instance_exposed = off-loopback bind OR a declared TLS-terminating proxy), so a + # plain loopback default is byte-identical (admin_exposed is False → this never trips, BACKLOG #326 + # preserved that property deliberately) and a synthetic instance stays quiet # (gated on data_class is PHI). This is WARN-ONLY by design (the reviewed default): dual-control is # off-by-default precisely so a genuine single-operator hospital deployment is never wedged, so # refusing to start on its absence would break a supported topology. @@ -1939,8 +2025,8 @@ def _serve(args: argparse.Namespace) -> int: approvals_exposure_desc = ( f"API bound to non-loopback host {settings.api.host!r}" if not settings.api.is_loopback - else "browser console exposed through a declared reverse proxy " - "([api].serve_ui + tls_terminated_upstream)" + else "admin interface reached through a declared reverse proxy " + "([api].tls_terminated_upstream)" ) print( f"warning: {approvals_exposure_desc} in a PHI-carrying environment ({env_name!r}) with " @@ -2364,8 +2450,12 @@ def _serve(args: argparse.Namespace) -> int: # read-out (or a bind-mounted device node) discharged the requirement with no human declaring # anything. The read-out now only softens the MESSAGE. What remains true is the asymmetry the # contradiction branch rests on: nothing here refuses on a read-out, in either direction. + # + # `instance_exposed` is NOT re-derived here. It is defined ONCE, above the MFA-at-exposure gate, and + # this arm shares that definition — the two must agree by construction. BACKLOG #326: a second copy + # is exactly how the ASVS 11.7.1 arm and the ASVS 6.3.3 arm came to disagree about whether the same + # boot was exposed. memory_declared = settings.security.memory_encryption_operator_declared - instance_exposed = not settings.api.is_loopback or settings.api.tls_terminated_upstream memory_undeclared_at_exposure = ( instance_exposed and data_class is DataClass.PHI and not memory_declared ) @@ -3562,6 +3652,127 @@ def _protect_key(args: argparse.Namespace) -> int: return 0 +_ANCHOR_FORM = ( + "expected COUNT:HEAD — the row count and the FULL head, copied verbatim from " + "'messagefoundry audit-anchor' (the 12-character head printed inside a FAIL message is a display " + "truncation, not an anchor); an empty log anchors as '0:'" +) + +#: Every hex character, both cases. The store only ever emits lowercase (``hexdigest()``); uppercase is +#: admitted and NORMALISED rather than rejected, because an operator who upper-cased the value in a +#: ticket must get a verify, not a tamper alarm. +_ANCHOR_HEX = frozenset("0123456789abcdefABCDEF") +#: ``hashlib.sha256``/``hmac.new(..., sha256)`` ``hexdigest()`` width — the only hex head length the +#: chain can produce, keyless or keyed (``store/store.py``, ``audit_row_hash``). +_ANCHOR_DIGEST_HEX_LEN = 64 +#: ADR 0138 ``vault_transit``: the row MAC is computed INSIDE Vault/OpenBao Transit +#: (``crypto_transit.TransitCipher.audit_hmac``), which returns its own opaque ``vault:v:`` +#: string — not hex, not 64 characters — and that string lands in ``row_hash`` verbatim. A future +#: isolated-module MAC provider with a different prefix MUST be added here, or a legitimate anchor from +#: that deployment is refused as malformed. +_ANCHOR_ISOLATED_MAC_PREFIX = "vault:v" + + +def _parse_anchor(text: str) -> tuple[int, str]: + """Parse a ``COUNT:HEAD`` audit anchor into the tuple ``verify_audit_chain`` expects. + + Raises ``ValueError`` naming the form. It must RAISE rather than fall back to an unanchored + verify: a silently-ignored anchor turns the whole control into a gate that reports green while + checking nothing, which is precisely the failure this subcommand exists to close. + + It must ALSO refuse rather than hand the comparator a head the store can never emit. + ``verify_audit_chain`` compares the head byte-exactly and reports *any* difference as + ``truncated or rewritten``, so an accepted-but-impossible head becomes a FALSE tamper alarm — a + red light on an intact chain, indistinguishable from a real detection. A control whose whole value + is that a FAIL means something cannot be allowed to manufacture FAILs out of its own input + handling — the inverse of the green-while-checking-nothing hole above, and it costs just as much. + + Two head shapes are legal, because exactly two are producible: + + * a **hex digest** — ``audit_row_hash``'s keyless SHA-256 or in-heap HMAC-SHA256 ``hexdigest()``, + always exactly 64 lowercase hex characters. Case is normalised, and the length is *required*: a + 12-character head pasted out of a FAIL message's display truncation is refused as malformed + input (rc 2) instead of being reported as tampering (rc 1). + * an **isolated-module MAC** — ADR 0138 ``vault_transit`` mode, whose ``vault:v1:…`` string is + passed through UNCHANGED. ``partition`` splits on the FIRST colon, so its internal colons + survive the ``COUNT:HEAD`` split. + + An EMPTY head is legal and load-bearing — ``audit_anchor()`` returns ``(0, "")`` for an empty log, + so ``0:`` must round-trip or a fresh instance is the one state that cannot be anchored. + """ + raw = text.strip() + count_text, sep, head = raw.partition(":") + if not sep: + raise ValueError(f"malformed audit anchor {text!r}: no ':' separator — {_ANCHOR_FORM}") + try: + count = int(count_text) + except ValueError: + raise ValueError( + f"malformed audit anchor {text!r}: row count {count_text!r} is not an integer — " + f"{_ANCHOR_FORM}" + ) from None + if count < 0: + raise ValueError( + f"malformed audit anchor {text!r}: row count {count} is negative — {_ANCHOR_FORM}" + ) + head = head.strip() + if not head: + return count, head + if all(c in _ANCHOR_HEX for c in head): + if len(head) != _ANCHOR_DIGEST_HEX_LEN: + raise ValueError( + f"malformed audit anchor {text!r}: head {head!r} is {len(head)} hex characters, not " + f"a full {_ANCHOR_DIGEST_HEX_LEN}-character digest — {_ANCHOR_FORM}" + ) + return count, head.lower() + if head.startswith(_ANCHOR_ISOLATED_MAC_PREFIX): + return count, head # opaque by construction; never normalise what we do not define + raise ValueError( + f"malformed audit anchor {text!r}: head {head!r} is neither a " + f"{_ANCHOR_DIGEST_HEX_LEN}-character hex digest nor an isolated-module " + f"{_ANCHOR_ISOLATED_MAC_PREFIX}… MAC (ADR 0138) — {_ANCHOR_FORM}" + ) + + +def _resolve_expected_anchor(args: argparse.Namespace) -> tuple[int, str] | None | int: + """The anchor for ``audit-verify``, or the exit code 2 if the flags are unusable. + + Returns ``None`` when neither flag was given (an unanchored verify, the historical behaviour). + Argparse's mutually-exclusive group has already refused both-at-once. + """ + from pathlib import Path + + raw: str | None + if args.expected_anchor_file is not None: + try: + # `utf-8-sig` absorbs a leading BOM: PowerShell 5.1's `Out-File`/`Set-Content -Encoding + # utf8` writes UTF-8 WITH one, and this product is deployed as a Windows service, so that + # is a first-class way an operator produces this file. + raw = Path(args.expected_anchor_file).read_text(encoding="utf-8-sig") + except (OSError, UnicodeDecodeError) as exc: + # `UnicodeDecodeError` subclasses `ValueError`, NOT `OSError` — catching only the latter + # let a mis-encoded file raise an unhandled traceback and exit 1, the SAME code + # `audit-verify` returns for a BROKEN CHAIN, so a compliance job keying on exit codes + # would have read a file-encoding problem as a detected tamper. PowerShell 5.1's `>` + # writes UTF-16LE, so this is the likely file, not an exotic one. + print( + f"error: cannot read --expected-anchor-file {args.expected_anchor_file!r}: {exc}. " + "It must be a UTF-8 text file holding the COUNT:HEAD line; PowerShell 5.1's '>' " + "writes UTF-16 — pipe to 'Set-Content -Encoding utf8' there.", + file=sys.stderr, + ) + return 2 + else: + raw = args.expected_anchor + if raw is None: + return None + try: + return _parse_anchor(raw) + except ValueError as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + + def _audit_verify(args: argparse.Namespace) -> int: import asyncio from pathlib import Path @@ -3571,6 +3782,13 @@ def _audit_verify(args: argparse.Namespace) -> int: from messagefoundry.config.settings import StoreBackend, load_settings from messagefoundry.store.base import open_store + # Resolve the anchor FIRST: it is a pure argv/file error, so it should not depend on a config load + # succeeding, and refusing it early keeps a typo from costing a store open. + resolved = _resolve_expected_anchor(args) + if isinstance(resolved, int): + return resolved + expected_anchor = resolved + cli: dict[str, dict[str, object]] = {} if args.db is not None: cli.setdefault("store", {})["path"] = args.db @@ -3593,7 +3811,7 @@ def _audit_verify(args: argparse.Namespace) -> int: async def run() -> tuple[bool, str | None]: store = await open_store(settings.store) try: - return await store.verify_audit_chain() + return await store.verify_audit_chain(expected_anchor=expected_anchor) finally: await store.close() @@ -3609,6 +3827,66 @@ async def run() -> tuple[bool, str | None]: return 0 if ok else 1 +def _audit_anchor(args: argparse.Namespace) -> int: + """Print ``COUNT:HEAD`` — the audit log's external anchor, to be held OUT-OF-BAND. + + The hash chain links each row to its predecessor, so deleting the NEWEST rows leaves a shorter + chain that still verifies: ``audit-verify`` alone reports OK on a truncated log. Comparing against + an anchor recorded elsewhere is what makes that visible, and this subcommand is how an operator + gets one. The anchor is a row count plus a digest — no PHI, no secret — so it is safe to store in + a ticket, an object store, or a compliance job's own database. + """ + import asyncio + from pathlib import Path + + from pydantic import ValidationError + + from messagefoundry.config.settings import StoreBackend, load_settings + from messagefoundry.store.base import open_store + + cli: dict[str, dict[str, object]] = {} + if args.db is not None: + cli.setdefault("store", {})["path"] = args.db + try: + settings = load_settings(config_path=args.service_config, cli=cli) + except (FileNotFoundError, ValueError, ValidationError) as exc: + print(f"error: {exc}", file=sys.stderr) + return 2 + + # The SAME M-31 guard as _audit_verify, and it matters MORE here: opening a SQLite store creates + # it, so a typo'd path would mint a fresh empty DB and print `0:` — an anchor OF NOTHING, which a + # later verify against the wrong database would then happily confirm. + if settings.store.backend == StoreBackend.SQLITE and not Path(settings.store.path).exists(): + print( + f"error: no audit database at {settings.store.path} — refusing to create one and print " + f"an anchor of an empty log (check --db / [store].path)", + file=sys.stderr, + ) + return 2 + + async def run() -> tuple[int, str]: + store = await open_store(settings.store) + try: + return await store.audit_anchor() + finally: + await store.close() + + count, head = asyncio.run(run()) + anchor = f"{count}:{head}" + if args.json: + _print_json({"count": count, "head": head, "anchor": anchor}, compact=True) + else: + print(anchor) + if count == 0: + # Same reasoning as the verify twin: an empty log on a real DB is legitimate, and at a glance + # indistinguishable from having anchored the wrong database (M-31). + print( + "warning: the audit log is empty — confirm this is the intended database.", + file=sys.stderr, + ) + return 0 + + def _rekey_audit(args: argparse.Namespace) -> int: """Enable HMAC keying of an EXISTING keyless audit chain (#190-D migration). @@ -4597,6 +4875,7 @@ def _emit_error(message: str, *, as_json: bool) -> int: "cert": _cert, "protect-key": _protect_key, "audit-verify": _audit_verify, + "audit-anchor": _audit_anchor, "rekey-audit": _rekey_audit, "rotate-key": _rotate_key, "backup": _backup, diff --git a/tests/test_audit_integrity.py b/tests/test_audit_integrity.py index a4687d6e..14a66e66 100644 --- a/tests/test_audit_integrity.py +++ b/tests/test_audit_integrity.py @@ -5,9 +5,11 @@ from __future__ import annotations import asyncio +import base64 import hashlib import hmac import json +import re from pathlib import Path import pytest @@ -352,6 +354,383 @@ def test_audit_verify_cli_refuses_missing_db( assert not missing.exists() # we refused before opening, so no empty DB was littered +# --- BACKLOG #328: `audit-anchor` + `audit-verify --expected-anchor` ----------------------------- +# +# The hash chain links each row to its predecessor, so deleting the NEWEST rows leaves a shorter chain +# that still walks cleanly: a bare `audit-verify` reports OK on a truncated log. `audit_anchor()` and +# `verify_audit_chain(expected_anchor=...)` already existed on the store protocol and all three +# backends; nothing exposed them to an operator, so the capability was unreachable from the CLI. +# +# The anchor is an EXACT point-in-time seal (measured, not assumed — see +# test_an_anchor_goes_stale_on_the_next_appended_row): it compares BOTH the row count and the head +# hash, so it is checked against a quiesced chain, not carried across normal operation. + + +def _seed_audit_rows(db: Path, n: int) -> None: + """Write ``n`` audit rows into a fresh store at ``db`` and close it.""" + + async def _run() -> None: + s = await MessageStore.open(db) + for i in range(n): + await s.record_audit(f"act{i}", actor="x") + await s.close() + + asyncio.run(_run()) + + +def _truncate_audit_tail(db: Path, keep: int) -> None: + """Delete every audit row past the first ``keep`` — the attack the chain walk cannot see.""" + + async def _run() -> None: + s = await MessageStore.open(db) + await s._db.execute("DELETE FROM audit_log WHERE id > ?", (keep,)) + await s._db.commit() + await s.close() + + asyncio.run(_run()) + + +def test_audit_anchor_cli_prints_count_and_head( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + db = tmp_path / "anchor.db" + _seed_audit_rows(db, 2) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + anchor = capsys.readouterr().out.strip() + assert re.fullmatch(r"2:[0-9a-f]{64}", anchor), anchor + + # The printed form must be exactly what the verify flag consumes — a round-trip, not two shapes + # that merely look alike. + assert main(["audit-verify", "--db", str(db), "--expected-anchor", anchor]) == 0 + assert "OK" in capsys.readouterr().out + + +def test_audit_anchor_cli_json(tmp_path: Path, capsys: pytest.CaptureFixture[str]) -> None: + db = tmp_path / "anchor_json.db" + _seed_audit_rows(db, 2) + + assert main(["audit-anchor", "--db", str(db), "--json"]) == 0 + payload = json.loads(capsys.readouterr().out) + assert payload["count"] == 2 + assert re.fullmatch(r"[0-9a-f]{64}", payload["head"]) + # `anchor` is the pre-joined COUNT:HEAD a job can hand straight back, so a caller never has to + # re-derive the separator convention from two fields. + assert payload["anchor"] == f"{payload['count']}:{payload['head']}" + + +def test_audit_anchor_cli_refuses_missing_db( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # M-31 parity with the verify twin, and it bites harder here: opening a SQLite store CREATES it, + # so a typo'd --db would mint an empty DB and print `0:` — an anchor OF NOTHING, which a later + # verify against that same wrong database would confirm forever. + missing = tmp_path / "typo.db" + assert main(["audit-anchor", "--db", str(missing)]) == 2 + assert "no audit database" in capsys.readouterr().err + assert not missing.exists() + + +def test_expected_anchor_detects_a_truncated_tail( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The load-bearing case: the blindness, and the thing that closes it, in one test. + + Half (a) pins the gap this feature exists for — a bare verify reports OK after the tail is cut. + If half (a) ever fails, the walk itself has changed and this whole subcommand needs re-reasoning. + Half (b) is the fix. Asserting only (b) would let someone "close" the finding by changing the + walk while nobody noticed the two halves had stopped describing the same system. + """ + db = tmp_path / "trunc.db" + _seed_audit_rows(db, 4) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + anchor = capsys.readouterr().out.strip() + + _truncate_audit_tail(db, keep=2) + + # (a) the surviving prefix still chains cleanly — the bare walk cannot see the deletion. + assert main(["audit-verify", "--db", str(db)]) == 0 + assert "OK" in capsys.readouterr().out + + # (b) the anchor sees it. + assert main(["audit-verify", "--db", str(db), "--expected-anchor", anchor]) == 1 + out = capsys.readouterr().out + assert "truncated or rewritten" in out and "FAIL" in out + + +def test_expected_anchor_rejects_a_malformed_value( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # A silently-ignored anchor is worse than no anchor: the command still exits 0, so the compliance + # job reports green while checking nothing. It must refuse loudly and name the form. + db = tmp_path / "malformed.db" + _seed_audit_rows(db, 2) + + assert main(["audit-verify", "--db", str(db), "--expected-anchor", "garbage"]) == 2 + err = capsys.readouterr().err + assert "COUNT:HEAD" in err and "malformed audit anchor" in err + + +def test_expected_anchor_accepts_the_empty_log_anchor( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # `audit_anchor()` returns (0, "") for an empty log, so `0:` must round-trip — otherwise a fresh + # instance is the one state that cannot be anchored, and the parser's strictness would have + # created a hole exactly where an operator starts. + db = tmp_path / "empty.db" + _seed_audit_rows(db, 0) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + assert capsys.readouterr().out.strip() == "0:" + assert main(["audit-verify", "--db", str(db), "--expected-anchor", "0:"]) == 0 + + +def test_expected_anchor_file_round_trips( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # The real compliance-job shape: `audit-anchor > anchor.txt` now, `--expected-anchor-file` later. + db = tmp_path / "file.db" + anchor_file = tmp_path / "anchor.txt" + _seed_audit_rows(db, 4) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + anchor_file.write_text(capsys.readouterr().out, encoding="utf-8") # trailing newline included + + assert main(["audit-verify", "--db", str(db), "--expected-anchor-file", str(anchor_file)]) == 0 + assert "OK" in capsys.readouterr().out + + _truncate_audit_tail(db, keep=1) + assert main(["audit-verify", "--db", str(db), "--expected-anchor-file", str(anchor_file)]) == 1 + assert "truncated or rewritten" in capsys.readouterr().out + + +def test_expected_anchor_file_refuses_an_unreadable_path( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # A missing anchor file must not degrade to an unanchored verify: an attacker who can cut the + # audit tail can also delete the file that would prove it. + db = tmp_path / "nofile.db" + _seed_audit_rows(db, 2) + + assert ( + main(["audit-verify", "--db", str(db), "--expected-anchor-file", str(tmp_path / "x")]) == 2 + ) + assert "cannot read --expected-anchor-file" in capsys.readouterr().err + + +def test_expected_anchor_file_refuses_a_utf16_file_without_crashing( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A mis-encoded anchor file must exit 2, not raise — and above all not exit 1. + + ``UnicodeDecodeError`` subclasses ``ValueError``, NOT ``OSError``, so a guard that catches only + ``OSError`` lets a decode error escape as an unhandled traceback whose exit code is **1** — the + same code ``audit-verify`` returns for a BROKEN CHAIN. A compliance job keying on exit codes would + read "your anchor file is UTF-16" as "the audit log was tampered with". + + Not an exotic input: this product deploys as a Windows service, and PowerShell 5.1's ``>`` + redirection writes UTF-16LE with a BOM. + """ + db = tmp_path / "utf16.db" + anchor_file = tmp_path / "anchor-utf16.txt" + _seed_audit_rows(db, 3) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + anchor = capsys.readouterr().out.strip() + anchor_file.write_bytes(anchor.encode("utf-16")) # BOM + UTF-16LE: what PS 5.1 `>` writes + assert anchor_file.read_bytes()[:2] == b"\xff\xfe", "fixture is not the UTF-16LE BOM shape" + + rc = main(["audit-verify", "--db", str(db), "--expected-anchor-file", str(anchor_file)]) + assert rc == 2, "a file-encoding problem must not share an exit code with a detected tamper" + err = capsys.readouterr().err + assert "cannot read --expected-anchor-file" in err + assert "UTF-8" in err # names the actual requirement, not just the exception + + +def test_expected_anchor_file_accepts_a_utf8_bom( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + # The other half of the same Windows reality: PS 5.1's `Out-File`/`Set-Content -Encoding utf8` + # writes UTF-8 WITH a BOM. Refusing that would leave a Windows operator with no working idiom, so + # the read is `utf-8-sig`, which absorbs it (and is a no-op on BOM-less UTF-8). + db = tmp_path / "utf8bom.db" + anchor_file = tmp_path / "anchor-bom.txt" + _seed_audit_rows(db, 3) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + anchor = capsys.readouterr().out.strip() + anchor_file.write_bytes(anchor.encode("utf-8-sig")) + assert anchor_file.read_bytes()[:3] == b"\xef\xbb\xbf", "fixture is not the UTF-8 BOM shape" + + assert main(["audit-verify", "--db", str(db), "--expected-anchor-file", str(anchor_file)]) == 0 + assert "OK" in capsys.readouterr().out + + +def test_expected_anchor_accepts_an_uppercased_head( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """An upper-cased head is the SAME anchor and must verify, not raise a tamper alarm. + + ``verify_audit_chain`` compares the head byte-exactly (``hmac.compare_digest`` over + ``audit_mac_bytes``), so a head differing only in case reports ``truncated or rewritten`` on a + chain nothing has touched. Anchors get copied through tickets, spreadsheets and change records, + which upper-case things; a control whose FAIL is supposed to mean something must not be able to + manufacture one out of its own input handling. The parser normalises instead. + """ + db = tmp_path / "upper.db" + _seed_audit_rows(db, 3) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + count, _, head = capsys.readouterr().out.strip().partition(":") + assert head.islower() and len(head) == 64 # the shape being normalised away + + rc = main(["audit-verify", "--db", str(db), "--expected-anchor", f"{count}:{head.upper()}"]) + assert rc == 0 + assert "OK" in capsys.readouterr().out + + +def test_expected_anchor_refuses_a_truncated_head_as_malformed_not_tampering( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """A short head is bad INPUT (rc 2), never a tamper detection (rc 1). + + The trap is baited by the product itself: the FAIL message prints both heads truncated to 12 + characters, so an operator retrying with the value they can see supplies a 12-character head. A + hex-only check accepts it, the byte-exact comparator cannot match it, and the operator gets + ``truncated or rewritten`` — with the message's own evidence line showing the two heads as + IDENTICAL, because it truncates the live one to the same 12 characters. Requiring the full digest + width turns that into an actionable input error. + """ + db = tmp_path / "shorthead.db" + _seed_audit_rows(db, 3) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + count, _, head = capsys.readouterr().out.strip().partition(":") + + rc = main(["audit-verify", "--db", str(db), "--expected-anchor", f"{count}:{head[:12]}"]) + assert rc == 2 + captured = capsys.readouterr() + assert "malformed audit anchor" in captured.err and "64-character digest" in captured.err + assert "truncated or rewritten" not in captured.out, ( + "a 12-character head is malformed input, but the chain was reported as truncated or " + "rewritten — a false tamper alarm costs this control as much as a missed one" + ) + + +def test_parse_anchor_passes_an_isolated_module_mac_through() -> None: + """An ADR 0138 ``vault_transit`` head is NOT hex, and must round-trip anyway. + + In that mode ``audit_row_hash`` delegates to ``TransitCipher.audit_hmac``, which returns Vault's + own opaque ``vault:v1:`` string; that string is what lands in ``audit_log.row_hash`` and + therefore what ``audit_anchor()`` prints. A hex-only head check refuses it — so the operator + control would have been unusable on the one store mode where the audit chain is keyed with no + in-heap key. Unit-level because reaching it end-to-end needs a live Transit backend. + """ + from messagefoundry.__main__ import _parse_anchor + + head = "vault:v1:" + base64.b64encode(b"\x01" * 32).decode("ascii") + assert not all(c in "0123456789abcdefABCDEF" for c in head) # the reason this case exists + # The count/head split must survive the MAC's own internal colons (partition on the FIRST only). + assert _parse_anchor(f"7:{head}") == (7, head) + + +def test_expected_anchor_and_file_are_mutually_exclusive(tmp_path: Path) -> None: + # Two transports for one value: argparse refuses both rather than letting one silently win. + db = tmp_path / "excl.db" + _seed_audit_rows(db, 1) + with pytest.raises(SystemExit) as exc: + main( + [ + "audit-verify", + "--db", + str(db), + "--expected-anchor", + "1:" + "0" * 64, + "--expected-anchor-file", + str(tmp_path / "a.txt"), + ] + ) + assert exc.value.code == 2 + + +def test_an_anchor_goes_stale_on_the_next_appended_row( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The anchor is an EXACT seal, not a monotonic-prefix check — pinned so nobody loosens it. + + Measured against the shipped comparator: ``count < exp_count or not head_ok``, where ``head_ok`` + compares the LAST row's hash. Appending one legitimate row therefore moves the head and the + anchor reports ``truncated or rewritten`` on a chain that merely GREW. + + That is a real ergonomic sharp edge, and the docs say so — anchor against a quiesced chain. It is + pinned here because the obvious "fix" (drop the head compare, keep ``count < exp_count``) would + take a real detection with it: see + ``test_expected_anchor_detects_a_same_count_tail_replacement``, which a count-only comparator + would pass. The false alarm and that detection are the SAME check. If this test ever needs to + change, the comparator semantics are being changed with it. + """ + db = tmp_path / "stale.db" + _seed_audit_rows(db, 4) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + anchor = capsys.readouterr().out.strip() + + async def _append_one() -> None: + s = await MessageStore.open(db) + await s.record_audit("legitimate", actor="x") + await s.close() + + asyncio.run(_append_one()) + + assert main(["audit-verify", "--db", str(db), "--expected-anchor", anchor]) == 1 + assert "truncated or rewritten" in capsys.readouterr().out + + +def test_expected_anchor_detects_a_same_count_tail_replacement( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The case that makes the head compare load-bearing rather than merely strict. + + An attacker who cuts the newest rows and forges the same number of replacements restores the row + COUNT and leaves a chain that walks cleanly (on a keyless store they can recompute it end to end). + Measured: the bare walk reports ``verified 4 audit row(s)``, and a hypothetical count-only + comparator would pass too, because ``4 < 4`` is false. Only the head hash differs. + + So the head compare is not redundant with the row count, and the false alarm pinned by + ``test_an_anchor_goes_stale_on_the_next_appended_row`` is the price of THIS detection — not an + independent wart that can be filed off on its own. + """ + db = tmp_path / "replaced.db" + _seed_audit_rows(db, 4) + + assert main(["audit-anchor", "--db", str(db)]) == 0 + anchor = capsys.readouterr().out.strip() + expected_count = int(anchor.split(":", 1)[0]) + + _truncate_audit_tail(db, keep=2) + + async def _forge_two() -> None: + s = await MessageStore.open(db) + for i in range(2): + await s.record_audit(f"forged{i}", actor="attacker") + await s.close() + + asyncio.run(_forge_two()) + + # The row count is back to where it started, so a count-only check has nothing to complain about. + assert main(["audit-anchor", "--db", str(db)]) == 0 + assert int(capsys.readouterr().out.strip().split(":", 1)[0]) == expected_count + + # And the chain itself walks cleanly — the forged rows chain correctly from their predecessor. + assert main(["audit-verify", "--db", str(db)]) == 0 + assert "OK" in capsys.readouterr().out + + # The head hash is what gives it away. + assert main(["audit-verify", "--db", str(db), "--expected-anchor", anchor]) == 1 + assert "truncated or rewritten" in capsys.readouterr().out + + def test_rekey_audit_cli( tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/test_checks_gate_parity.py b/tests/test_checks_gate_parity.py index 0cd84f0a..505255fc 100644 --- a/tests/test_checks_gate_parity.py +++ b/tests/test_checks_gate_parity.py @@ -202,6 +202,25 @@ def _serve( True, 0, ), + # BACKLOG #326: the SAME refusal on the runbook's RECOMMENDED topology — a LOOPBACK bind + # (local_access_only left true) behind a DECLARED TLS-terminating proxy. This row is the one the + # old `admin_exposed = not is_loopback or ui_exposed` keying could not produce: the ADR 0143 + # auto-degrade clears serve_ui first, so ui_exposed was False and a production PHI instance with + # single-factor admin on the network started clean. Driven through the REAL gate (`main(["serve", + # ...])`) because checks.py is not a second gate site for it — that mirror reads neither + # require_mfa nor is_loopback. + ( + "mfa-off-loopback-behind-proxy-prod-phi-refuses", + "security.require_mfa = false\nsecurity.block_unlisted_outbound = true\n" + "security.delete_message_bodies_after_days = 30\n" + + _MEMORY_ENCRYPTION + + _PROXY + + _RETENTION_DL + + _ALERTS, + "prod", + True, + 2, + ), # unbounded retention on a production PHI instance refuses. ( "unbounded-retention-prod-phi-refuses", diff --git a/tests/test_cli.py b/tests/test_cli.py index 84a4e27d..d2db6cb1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -987,6 +987,12 @@ def test_serve_loopback_prod_quiet_on_approvals( # Loopback byte-identity: admin_exposed is False on the loopback default, so even a production PHI # instance with approvals off never emits the advisory. Retention windows + SMTP satisfy the # #186a/#188 prod gates so the serve reaches rc 0 and only the approvals posture is under test. + # + # BACKLOG #326 re-keyed admin_exposed onto `instance_exposed` (off-loopback bind OR a declared + # TLS-terminating proxy). This case is unchanged BY CONSTRUCTION: `instance_exposed` is also False + # on a plain loopback bind with nothing declared, which is the property that keeps the loopback + # default quiet. test_plain_loopback_is_not_instance_exposed pins the same invariant against the + # MFA arm; both must hold or the re-key widened more than it was meant to. rc = _dualctl_serve( tmp_path, monkeypatch, @@ -1277,6 +1283,177 @@ def test_serve_ui_declared_proxy_warns_mfa_on_staging_phi( assert "declared reverse proxy" in err and "single-factor over the network" in err +# --- BACKLOG #326: the exposure predicate must not read the mutated console flag ------------------ +# +# `settings.api.serve_ui` is flipped False IN PLACE by both ADR 0143 degrade arms (console wheel +# absent; default-on console meeting an exposed bind) BEFORE the exposure gates read it. Keying +# `admin_exposed` on it therefore made the MFA-at-exposure refusal unreachable on the runbook's +# RECOMMENDED topology — a loopback bind BEHIND a declared reverse proxy with the console left at its +# default — while the ADR 0152 arm two hundred lines later called the very same boot exposed. +# +# These cases pin the corrected keying (`instance_exposed`) from OUTSIDE the engine: by exit code and +# by operator-visible text, never by reading the source. The two arms above already cover an +# EXPLICITLY-enabled console; what was missing was every arm where the console is not explicitly on. +# +# Each later production-PHI gate (bounded retention windows, the security-notification channel, the +# ADR 0152 in-use-data-protection declaration) is pre-satisfied, so exactly one gate decides each case. +_EXPOSURE_PRELUDE = ( + "security.local_access_only = true\n" # LOOPBACK bind — the whole point of these cases + "security.block_unlisted_outbound = true\n" + "security.delete_message_bodies_after_days = 30\n" + "security.memory_encryption_operator_declared = true\n" +) +_EXPOSURE_TAIL = ( + "[retention]\ndead_letter_days = 30\n" + '[alerts]\nemail_smtp_host = "smtp.example.org"\nemail_from = "sec@example.org"\n' +) +_DECLARED_PROXY = ( + '[api]\ntls_terminated_upstream = true\ntrusted_proxies = ["10.0.0.1"]\n' + 'proxy_intra_service_auth = "network"\nproxy_tls_min_version = "1.2"\n' +) + + +def test_serve_default_console_declared_proxy_still_requires_mfa_on_prod_phi( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + # ARM C — the load-bearing case. Loopback bind behind a DECLARED proxy with [security]. + # serve_web_console left at its DEFAULT: the ADR 0143 auto-degrade clears serve_ui, so the old + # `admin_exposed = not is_loopback or ui_exposed` was False and this production PHI instance + # started with single-factor admin on the network. It must refuse. + rc = _l5b_serve( + tmp_path, + monkeypatch, + # require_mfa = false opts out of the BACKLOG #187 secure default so the exposure gate fires. + _EXPOSURE_PRELUDE + "security.require_mfa = false\n" + _EXPOSURE_TAIL + _DECLARED_PROXY, + env="prod", + ) + assert rc == 2 + err = capsys.readouterr().err + assert "require_mfa off; refusing to start" in err + assert "declared reverse proxy" in err + + +def test_serve_console_explicitly_disabled_still_requires_mfa_on_prod_phi( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + # ARM D — an operator who EXPLICITLY turns the console off is in the identical exposure posture: + # the single-factor admin surface is the JSON API, which is served either way. Keying on the + # console flag made "I disabled the browser console" silently disable an MFA refusal too. + rc = _l5b_serve( + tmp_path, + monkeypatch, + _EXPOSURE_PRELUDE + + "security.require_mfa = false\nsecurity.serve_web_console = false\n" + + _EXPOSURE_TAIL + + _DECLARED_PROXY, + env="prod", + ) + assert rc == 2 + err = capsys.readouterr().err + assert "require_mfa off; refusing to start" in err + + +def test_serve_default_console_declared_proxy_warns_dual_control_on_phi( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + # The #189 dual-control arm reads the SAME `admin_exposed`, so it degraded with the same flag. + # require_mfa on pre-clears the sec-mfa-on gate, leaving only the approvals posture under test. + rc = _l5b_serve( + tmp_path, + monkeypatch, + 'security.enforcement = "warn"\n' + + _EXPOSURE_PRELUDE + + "security.require_mfa = true\n" + + _EXPOSURE_TAIL + + _DECLARED_PROXY, + env="prod", + ) + assert rc == 0 + err = capsys.readouterr().err + assert "[approvals].enabled off" in err and "single caller's authority" in err + + +def test_exposure_desc_names_the_proxy_not_the_console( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + # The refusal has to name a trigger the operator can act on. It used to read "([api].serve_ui + + # tls_terminated_upstream)" — grammatical, and false for every arm the fix newly catches, because + # serve_ui is False by the time the message is built. Pinned by BEHAVIOUR (the emitted text), not + # by a comment: on arm C the whole serve emits no other `serve_ui`, measured at HEAD. + rc = _l5b_serve( + tmp_path, + monkeypatch, + _EXPOSURE_PRELUDE + "security.require_mfa = false\n" + _EXPOSURE_TAIL + _DECLARED_PROXY, + env="prod", + ) + assert rc == 2 + err = capsys.readouterr().err + assert "serve_ui" not in err, ( + "the exposure description names [api].serve_ui as the trigger, but serve_ui is False on every " + "arm this refusal newly catches — the operator would go looking for a flag that is already off" + ) + assert "tls_terminated_upstream" in err + + +def test_plain_loopback_is_not_instance_exposed( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + # The byte-identity guard the re-key must not break, and the pin on the NARROW predicate. A plain + # loopback bind with nothing declared is not exposed, so neither the MFA refusal nor the #189 + # dual-control advisory may fire — even on a production PHI instance with both knobs off. This is + # why `console_exposed` (which also counts a bare `public_origin`) was NOT reused here: nothing has + # been declared in that case, so exposure is an INFERENCE, and an inference must not refuse. It + # must not be silent either — test_undeclared_proxy_warns_about_single_factor_admin covers that + # half. This case sets no `public_origin`, so it exercises neither arm: it is the floor, where the + # correct output is nothing at all. + rc = _l5b_serve( + tmp_path, + monkeypatch, + _EXPOSURE_PRELUDE + "security.require_mfa = false\n" + _EXPOSURE_TAIL, + env="prod", + ) + assert rc == 0 + err = capsys.readouterr().err + assert "require_mfa off" not in err + assert "approvals" not in err + + +def test_undeclared_proxy_warns_about_single_factor_admin( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """The residual the narrow predicate leaves open must be WARNED about, not merely documented. + + `instance_exposed` deliberately excludes a set `public_origin` with no declared terminator, and + the operator docs say that case "still only warns". Measured while this arm was written, it did + not: the only other candidate is the ADR 0068 §8 undeclared-proxy heuristic, which (a) is about + the /ui session cookie and HSTS and says nothing about admin factors, and (b) is gated on + `settings.api.serve_ui`, which the ADR 0143 auto-degrade clears in place for exactly this input — + a DEFAULT-on console plus a set `public_origin`. So on the commonest shape of this posture the + startup said nothing whatever about single-factor admin, and a compensating control cited in four + operator docs did not exist (CLAUDE.md §11: a compensating control must not rest on a false + premise). + + Both halves are asserted: the new warning is present, and the §8 heuristic is NOT — which is what + makes this arm load-bearing rather than duplicative. + """ + rc = _l5b_serve( + tmp_path, + monkeypatch, + _EXPOSURE_PRELUDE + + "security.require_mfa = false\n" + + 'security.web_console_public_address = "https://mefor.example.org"\n' + + _EXPOSURE_TAIL, + env="prod", + ) + assert rc == 0, "an UNDECLARED proxy is an inference — it must warn, never refuse" + err = capsys.readouterr().err + assert "UNDECLARED reverse proxy" in err and "single-factor over the network" in err + assert "session cookie ships WITHOUT Secure" not in err, ( + "the ADR 0068 §8 heuristic fired after all — re-check whether this arm is still needed, and " + "correct the docs either way" + ) + + # --- ADR 0143: the browser console is ON by default (loopback secure-context) --------------------- diff --git a/tests/test_postgres_store.py b/tests/test_postgres_store.py index 72f90ec9..421a2d84 100644 --- a/tests/test_postgres_store.py +++ b/tests/test_postgres_store.py @@ -3538,6 +3538,43 @@ async def test_audit_verify_cli_server(store, capsys) -> None: assert "OK:" in out and "verified 2" in out +async def test_audit_anchor_cli_server(store, capsys) -> None: + """BACKLOG #328 (Postgres mirror): ``audit-anchor`` + ``audit-verify --expected-anchor``. + + ``audit_anchor()`` is implemented separately per backend (``store.py`` / ``sqlserver.py`` / + ``postgres.py``), so a SQLite-only test is not evidence for this one. The ``expected_anchor`` + COMPARISON is byte-equivalent across all three, so what this case exercises is this backend's own + anchor SQL plus the CLI wrapper's connection handling. + + Same idiom as CLI-22 above: backend from ``MEFOR_STORE_*`` env, NO ``--db`` (the M-31 missing-DB + guard is SQLite-only and inert here), driven through ``asyncio.to_thread`` because ``_audit_anchor`` + calls ``asyncio.run`` internally. The anchor is captured at RUNTIME, never a literal.""" + from messagefoundry.__main__ import main + + await store.record_audit("message_view", actor="alice", detail="v1") + await store.record_audit("export", actor="bob", detail="e1") + + rc = await asyncio.to_thread(main, ["audit-anchor"]) # backend from env; NO --db + assert rc == 0 + anchor = capsys.readouterr().out.strip() + count_text, _, head = anchor.partition(":") + assert count_text == "2" and len(head) == 64 + + # Round-trips against the unchanged chain. + rc = await asyncio.to_thread(main, ["audit-verify", "--expected-anchor", anchor]) + assert rc == 0 + assert "OK:" in capsys.readouterr().out + + # Cut the NEWEST row: the walk still verifies the surviving prefix; only the anchor sees it. + await store._execute("DELETE FROM audit_log WHERE id = (SELECT MAX(id) FROM audit_log)") + rc = await asyncio.to_thread(main, ["audit-verify"]) + assert rc == 0 + assert "OK:" in capsys.readouterr().out + rc = await asyncio.to_thread(main, ["audit-verify", "--expected-anchor", anchor]) + assert rc == 1 + assert "truncated or rewritten" in capsys.readouterr().out + + async def test_rekey_audit_cli_server(store, capsys, monkeypatch) -> None: """CLI-23 (Postgres mirror): the ``rekey-audit`` CLI wrapper enables HMAC keying of an existing keyless chain (#190-D). It reaches the live Postgres store purely via ``MEFOR_STORE_*`` env (no diff --git a/tests/test_security_doc_drift.py b/tests/test_security_doc_drift.py index f7890811..131b29af 100644 --- a/tests/test_security_doc_drift.py +++ b/tests/test_security_doc_drift.py @@ -1642,6 +1642,47 @@ def test_contextual_prefixed_settings_force_a_documented_decision() -> None: ) +def test_admin_exposed_is_not_derived_from_the_mutated_console_flag() -> None: + """The exposure predicate must not read a field an earlier arm has already rewritten. + + ``settings.api.serve_ui`` is flipped to ``False`` IN PLACE by both ADR 0143 degrade arms before + the exposure gates run, so by the time ``admin_exposed`` is derived it answers "is /ui mounted?", + not "is the admin interface on the network?". Deriving the MFA-at-exposure and #189 dual-control + gates from it (BACKLOG #326) made both unreachable on the runbook's RECOMMENDED loopback-behind- + declared-proxy topology, while the ADR 0152 arm in the same function called that boot exposed. + + The behavioural pins live in ``tests/test_cli.py`` and ``tests/test_checks_gate_parity.py``. This + is the SHAPE guard: it exists so the defect cannot quietly return through a refactor that keeps + every current test green (re-introducing the console term only changes behaviour for configs no + case happens to cover). Kept here beside the dual-control slice because it uses the same idiom on + the same file, including its liveness receipt. + """ + source = (_ROOT / "messagefoundry" / "__main__.py").read_text(encoding="utf-8") + marker = "\n admin_exposed = " + # Liveness receipt FIRST: a rename or a reflow that makes the slice empty must red this test, not + # make it unfailable. `in` is checked explicitly so the failure names the cause rather than + # surfacing a bare ValueError from `index`. + assert marker in source, ( + "no top-level `admin_exposed = ` assignment found in messagefoundry/__main__.py. If it was " + "renamed, update this guard AND docs/SECURITY.md Table A AND the literal marker in " + "test_startup_dual_control_arm_is_documented_as_warn_only, which all name it." + ) + start = source.index(marker) + 1 + assignment = source[start : source.index("\n", start)] + assert assignment.strip().startswith("admin_exposed = ") and len(assignment.strip()) > len( + "admin_exposed = " + ), ( + f"the assignment slice looks wrong — the assertions below would pass vacuously: {assignment!r}" + ) + for banned in ("ui_exposed", "serve_ui"): + assert banned not in assignment, ( + f"`admin_exposed` is derived from `{banned}`, which the ADR 0143 degrade arms rewrite in " + f"place further up this same function (BACKLOG #326). Derive it from `instance_exposed` " + f"— the console being mounted is a presentation fact, not an exposure fact. Slice was: " + f"{assignment!r}" + ) + + def test_startup_dual_control_arm_is_documented_as_warn_only() -> None: """The bind/exposure inventory claimed a refusal the code does not implement. diff --git a/tests/test_sqlserver_store.py b/tests/test_sqlserver_store.py index 58ae589a..bac246fa 100644 --- a/tests/test_sqlserver_store.py +++ b/tests/test_sqlserver_store.py @@ -3567,6 +3567,44 @@ async def test_audit_verify_cli_server(store, capsys) -> None: assert "OK:" in out and "verified 2" in out +async def test_audit_anchor_cli_server(store, capsys) -> None: + """BACKLOG #328: ``audit-anchor`` + ``audit-verify --expected-anchor`` on the LIVE server store. + + ``audit_anchor()`` is implemented separately per backend (``store.py`` / ``sqlserver.py`` / + ``postgres.py``), so a SQLite-only test is not evidence for this one. The ``expected_anchor`` + COMPARISON is byte-equivalent across all three, so what this case actually exercises is this + backend's own anchor SQL plus the CLI wrapper's connection handling. + + Same idiom as CLI-22 above: backend from ``MEFOR_STORE_*`` env, NO ``--db`` (the M-31 missing-DB + guard is SQLite-only and inert here), driven through ``asyncio.to_thread`` because ``_audit_anchor`` + calls ``asyncio.run`` internally. The anchor is captured at RUNTIME and never written as a literal: + DELETE does not reseed SQL Server IDENTITY, so a hard-coded count or head would be wrong.""" + from messagefoundry.__main__ import main + + await store.record_audit("message_view", actor="alice", detail="v1") + await store.record_audit("export", actor="bob", detail="e1") + + rc = await asyncio.to_thread(main, ["audit-anchor"]) # backend from env; NO --db + assert rc == 0 + anchor = capsys.readouterr().out.strip() + count_text, _, head = anchor.partition(":") + assert count_text == "2" and len(head) == 64 + + # Round-trips against the unchanged chain. + rc = await asyncio.to_thread(main, ["audit-verify", "--expected-anchor", anchor]) + assert rc == 0 + assert "OK:" in capsys.readouterr().out + + # Cut the NEWEST row: the walk still verifies the surviving prefix; only the anchor sees it. + await store._execute("DELETE FROM audit_log WHERE id = (SELECT MAX(id) FROM audit_log)") + rc = await asyncio.to_thread(main, ["audit-verify"]) + assert rc == 0 + assert "OK:" in capsys.readouterr().out + rc = await asyncio.to_thread(main, ["audit-verify", "--expected-anchor", anchor]) + assert rc == 1 + assert "truncated or rewritten" in capsys.readouterr().out + + async def test_rekey_audit_cli_server(store, capsys, monkeypatch) -> None: """CLI-23: the ``rekey-audit`` CLI wrapper enables HMAC keying of an existing keyless chain (#190-D). It reaches the live SQL Server store purely via ``MEFOR_STORE_*`` env (no ``--db``; the