From 535ea1754626f3ae376c6f24eb5e96baa437477f Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 3 Sep 2026 15:17:49 -0500 Subject: [PATCH 1/7] feat(store): carry the store-principal privilege preflight onto current main Carried from w3-store-privilege-preflight (11b0a8aa), a branch that never had a PR and sits 530 commits behind main. BACKLOG #1234 names a defect in this code, but no Builder could act on it because the subject did not exist on main. This makes it exist. The preflight reads the store principal's effective privileges at startup and warns when they exceed what the engine needs. Observation and refusal are separate: [store].require_least_privilege gates refusal and stays False, the default the branch shipped. The message is rewritten on carry. The original read RESCUE COMMIT and DO NOT MERGE AS-IS and recorded a --no-verify commit, so carrying it onto main would import a false instruction. The content is unchanged. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 25 + docs/CONFIGURATION.md | 1 + docs/DEPLOY-SERVER-DB.md | 70 +++ docs/SECURITY-LOOSENING.md | 38 ++ messagefoundry/__main__.py | 21 +- messagefoundry/api/app.py | 42 ++ messagefoundry/api/models.py | 33 + messagefoundry/config/settings.py | 93 ++- messagefoundry/store/postgres.py | 75 +++ messagefoundry/store/privilege.py | 389 ++++++++++++ messagefoundry/store/sqlserver.py | 85 +++ messagefoundry/store/store.py | 22 +- tests/test_alert_smtp_tls.py | 7 +- tests/test_client_network_allowlist.py | 4 +- tests/test_memory_encryption_readout.py | 4 +- tests/test_security_config.py | 4 +- tests/test_security_posture_defaults.py | 15 +- tests/test_store_privilege_preflight.py | 774 ++++++++++++++++++++++++ 18 files changed, 1685 insertions(+), 17 deletions(-) create mode 100644 messagefoundry/store/privilege.py create mode 100644 tests/test_store_privilege_preflight.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 84072e47b..9766d33fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,31 @@ All notable changes to MessageFoundry are documented here. The format follows ## [Unreleased] ### Added +- **A startup preflight that reads the store principal's *effective* privileges, so the least-privilege + grant the runbooks prescribe stops being a claim the engine cannot check.** + [`DEPLOY-SERVER-DB.md`](docs/DEPLOY-SERVER-DB.md) told operators exactly which grant the engine's + database login needs, and the engine had no way to see what it had actually been given: no + fixed-server-role probe and no database-role probe existed anywhere, and + `[store].require_managed_identity` constrains the credential's *kind* rather than its privilege — a + `sysadmin` gMSA satisfies it clean. On a first deployment an over-granted store principal would + therefore have gone unobserved. `serve` now reads fixed **server**-role and **database**-role + membership plus `CONTROL SERVER` / database `CONTROL` on SQL Server, and role attributes + (`SUPERUSER`, `CREATEROLE`, `CREATEDB`, `REPLICATION`, `BYPASSRLS`), assumable predefined roles and + database ownership on PostgreSQL — before any listener binds. + **It observes and warns; it does not refuse by default** — refusing on an over-grant could block a + legitimate deployment mid-setup, and the engine does not own the grant. Every start logs what it saw, + writes a `store_privilege_preflight` audit row, and names each excess grant in + `security_loosenings()` and `GET /security/posture`. Set `[store].require_least_privilege = true` to + turn the warning into a refusal (refuse/warn splits on `[security].enforcement`, exactly like + `require_managed_identity`). + **It does not fail open, and that is the part to know before reading its output.** A probe that + cannot run — permission denied, a driver error, a store handle with no probe — reports + `unobservable`, which is a *different* result from "observed, and it is fine" in the log line, in the + audit row and in the posture response, and which a declared `require_least_privilege` also refuses. + SQLite reports `not_applicable` and says why: a local file has no server principal, and the control + there is the filesystem ACL. The PostgreSQL least-privilege grant is now documented + ([`DEPLOY-SERVER-DB.md`](docs/DEPLOY-SERVER-DB.md) §1.2), which it previously was not. + ([BACKLOG #1008](docs/BACKLOG.md)) - **`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 diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 7bfb54468..9e0c7e542 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -101,6 +101,7 @@ backend-limited. | `username` | str | — | server DBs (required when `auth = sql`) | | `password` | secret | — | **env only** (`MEFOR_STORE_PASSWORD`) | | `require_managed_identity` | bool | `false` | delegated-identity precondition (#203, ASVS 13.2.1/13.3.2): when `true`, `serve` **refuses to start (exit 2)** unless the store authenticates via a managed identity — SQL Server `auth = integrated`/`entra`. SQLite is exempt; Postgres cannot satisfy it. Off by default. **The refuse/warn split is `[security].enforcement`, not the production tier** — the gate reads `enforcing` ([`__main__.py`](../messagefoundry/__main__.py), the `managed_identity_precondition` block), and `enforce` is the shipped default on `dev` and `staging` as much as on `prod`, so a staging box that turns this on and leaves `auth = "sql"` is **refused**, not warned. It downgrades to a warning only under `enforcement = warn` | +| `require_least_privilege` | bool | `false` | least-**privilege** precondition on the store principal (#1008, ASVS 13.2.2) — the privilege sibling of `require_managed_identity` above, which constrains the credential's *kind* and never what it may do (a `sysadmin` gMSA satisfies that one clean; never grant one — [`DEPLOY-SERVER-DB.md` §1.1](DEPLOY-SERVER-DB.md)). **The probe itself is NOT gated by this setting:** it runs at every start regardless, logs what it observed, writes a `store_privilege_preflight` audit row, and reports any excess grant in `security_loosenings()` / `GET /security/posture`. This flag adds the **refusal**: when `true`, `serve` refuses to start if the principal holds more than the grant [`DEPLOY-SERVER-DB.md` §1.1/§1.2](DEPLOY-SERVER-DB.md) prescribes — **and also if the probe could not run at all**, since a declared refusal that passed an unobservable principal would be the fail-open shape it exists to prevent. Off by default so it can never block a legitimate deployment mid-setup. Refuse/warn splits on `[security].enforcement` exactly like `require_managed_identity`. SQLite is exempt (a local file has no server principal). | | `encrypt`, `trust_server_certificate` | bool | `true`/`false` | TLS to the DB | | `ssl_root_cert` | path | — | server DBs — pin the DB server's certificate by **file** so a private/self-signed DB CA verifies **without** a machine-wide trust import, on the **secure** posture only (`encrypt = true`, `trust_server_certificate = false`) — it never disables verification. **Postgres:** an asyncpg `SSLContext` CA-bundle (chain + hostname still checked). **SQL Server:** the ODBC Driver **18.1+** `ServerCertificate` keyword (a leaf/exact-cert match; needs driver ≥ 18.1). Rejected for SQLite (no TLS); a missing file fails loud at load. A path, not a secret — may live in the file. See [`DEPLOY-SERVER-DB.md` §5](DEPLOY-SERVER-DB.md). | | `multi_subnet_failover` | bool | `false` | **SQL Server only** — emit the ODBC `MultiSubnetFailover=Yes` keyword so a client connecting to an Always On Availability Group **listener** reaches the current primary promptly across subnets, instead of serially waiting out each replica subnet's DNS/TCP timeout on failover. A no-op for Postgres/SQLite (they never see the ODBC string). Off by default — only a multi-subnet AOAG needs it. | diff --git a/docs/DEPLOY-SERVER-DB.md b/docs/DEPLOY-SERVER-DB.md index 3e8008704..d3acac14f 100644 --- a/docs/DEPLOY-SERVER-DB.md +++ b/docs/DEPLOY-SERVER-DB.md @@ -130,6 +130,76 @@ require_managed_identity = true # refuse a static SQL login on production PHI > moved: that start issues the DDL batch and **fails outright** without it. Pre-creating the schema > and never granting it is the other supported posture, and it takes the same upgrade discipline. +### 1.2 PostgreSQL — the least-privilege role + +The SQL Server set in §1.1 does **not** transfer: Postgres has no fixed **database** roles, the schema +uses `BIGSERIAL` sequences rather than `IDENTITY`, and there is no stored-procedure path +(`fifo_claim_proc` is SQL Server only). The Postgres equivalent is a role with **no attributes**, plus +object grants. Two supported postures — pick one: + +**Posture A — the engine role owns its own schema** (simplest; the engine bootstraps and upgrades +itself, §2): + +```sql +CREATE ROLE mefor LOGIN PASSWORD :'pw'; -- NOSUPERUSER NOCREATEDB NOCREATEROLE are the defaults +GRANT CONNECT ON DATABASE messagefoundry TO mefor; +CREATE SCHEMA mefor AUTHORIZATION mefor; -- run by a DBA; the role owns only this schema +-- then set [store].db_schema = "mefor" so the pool's search_path lands there +``` + +**Posture B — a DBA pre-creates the objects; the engine role holds only row CRUD** (the analogue of +"pre-create the schema and never grant `db_ddladmin`"; it carries the same upgrade discipline — the +first start of any build whose schema moved must be run by a principal that may issue DDL): + +```sql +CREATE ROLE mefor LOGIN PASSWORD :'pw'; +GRANT CONNECT ON DATABASE messagefoundry TO mefor; +GRANT USAGE ON SCHEMA mefor TO mefor; -- USAGE, not CREATE +GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA mefor TO mefor; +GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA mefor TO mefor; -- the BIGSERIAL sequences +``` + +> **Why nothing wider, derived from the store rather than asserted.** The Postgres store issues +> `CREATE TABLE IF NOT EXISTS` / `CREATE INDEX IF NOT EXISTS` inside its own schema, then only +> `SELECT` / `INSERT` / `UPDATE` / `DELETE`. Its concurrency primitives are `pg_advisory_xact_lock` +> and `SET LOCAL statement_timeout`, both available to any role. It installs no extension, creates no +> function, and never uses `LISTEN`/`NOTIFY` or `COPY`. +> +> **Never `SUPERUSER`**, and never `CREATEROLE` / `CREATEDB` / `REPLICATION` / `BYPASSRLS`. Do **not** +> make the engine role the **owner of the database** — ownership carries `CREATE` on the database and +> the right to drop it, neither of which the engine uses. Do not grant `pg_read_all_data`, +> `pg_write_all_data`, `pg_read_server_files`, `pg_write_server_files`, `pg_execute_server_program`, +> `pg_signal_backend`, `pg_checkpoint`, `pg_maintain` or `pg_create_subscription`, nor a managed-cloud +> umbrella role (`rds_superuser`, `cloudsqlsuperuser`, `azure_pg_admin`). +> +> **No managed identity.** Postgres has no managed-identity auth mode, so `[store].auth` is a static +> role+password (supply it via `MEFOR_STORE_PASSWORD`, never the file) and +> `[store].require_managed_identity` is unsatisfiable on this backend — see +> [`CONFIGURATION.md`](CONFIGURATION.md). `[store].require_least_privilege` is the orthogonal control +> and **does** apply here (§1.3). + +### 1.3 The startup privilege preflight (`[store].require_least_privilege`) + +The grants in §1.1 and §1.2 used to be prescriptions the engine could not check. They are now +**observed at every start**, before any listener binds: + +| Backend | What the probe reads | On SQLite | +|---|---|---| +| SQL Server | fixed **server**-role and **database**-role membership by name (`IS_SRVROLEMEMBER` / `IS_ROLEMEMBER` — authoritative and independent of catalog visibility), plus `CONTROL SERVER` / database `CONTROL`, plus any user-defined database role the catalog exposes | n/a | +| PostgreSQL | every role the principal may assume and the **attributes** each carries (`SUPERUSER`, `CREATEROLE`, `CREATEDB`, `REPLICATION`, `BYPASSRLS`), plus database ownership and `CREATE` on the database | n/a | +| SQLite | — | reported **not applicable**: a local file has no server principal; access is the filesystem ACL on the `.db` and its `-wal`/`-shm` sidecars | + +- **The WARN arm ships on and cannot block an install.** Every start logs what was observed, writes a + `store_privilege_preflight` audit row, and — when the principal holds more than the documented set — + names each extra grant in `security_loosenings()` and in `GET /security/posture`. +- **Refusal is opt-in:** set `[store].require_least_privilege = true` to refuse to start on an + over-grant. Like `require_managed_identity`, the refuse/warn split reads `[security].enforcement`, + not the deployment tier. +- **It does not fail open.** If the probe cannot run — permission denied, a driver error, a backend + with no probe — the status is `unobservable`, which is reported as its own loud condition and is + **never** rendered as a clean result. Under `require_least_privilege` an unobservable probe refuses, + because a declared refusal that passed a principal it could not read would be a control in name only. + --- ## 2. Schema bootstrap & evolution diff --git a/docs/SECURITY-LOOSENING.md b/docs/SECURITY-LOOSENING.md index ea9fcec27..e8bef4c0b 100644 --- a/docs/SECURITY-LOOSENING.md +++ b/docs/SECURITY-LOOSENING.md @@ -495,6 +495,43 @@ the call to the Console on 2026-09-02; the Console decided ([ADR 0118](adr/0118- `security_loosenings()` / `GET /security/posture`. Inbound names are prefixed `inbound:`. - **What it cannot do:** it is advisory only, on every posture, in both directions. Nothing refuses it. +### `store_principal_over_granted` — the engine's database credential holds more than its runbook allows + +> **An OBSERVATION, not a switch.** Nobody sets this; the serve-time preflight reads the store +> principal's *effective* privileges and compares them against the grant +> [`DEPLOY-SERVER-DB.md` §1.1/§1.2](DEPLOY-SERVER-DB.md) prescribes. BACKLOG #1008, ASVS 13.2.2. +- **What you lose:** the store credential can reach data and administrative operations the engine + never uses. A `sysadmin` / `db_owner` login can read and alter every database on the instance; a + Postgres `SUPERUSER`, database owner, or member of `pg_read_all_data` / `pg_execute_server_program` + can do the equivalent. Any code path that reaches the store — an injection, a compromised process, + a mistaken statement — inherits that reach, so the blast radius of every other store defect widens. +- **Why the engine cannot simply refuse:** it does not own the grant. Refusing by default would block a + legitimate deployment mid-setup on a posture only a DBA can change, so the shipped arm **warns**. +- **When acceptable:** during bring-up, while a DBA reduces the grant. Not as a steady state. +- **It is never silent:** a WARN at every start naming each excess grant; a `store_privilege_preflight` + audit row; a `store_principal_over_granted` entry here and in `GET /security/posture`, whose + `store_privilege` field carries the full observation. +- **How to refuse:** set `[store].require_least_privilege = true` + ([`CONFIGURATION.md`](CONFIGURATION.md)). The refuse/warn split is `[security].enforcement`. +- **`require_managed_identity` does NOT cover this.** It constrains the credential's *kind* — a + `sysadmin` gMSA satisfies it clean. The two are orthogonal and a site needs both. + +### `store_principal_privileges_unobserved` — the privilege posture could not be read + +> The complement of the entry above, and it is reported **separately** on purpose: an over-grant and an +> un-run probe demand different operator actions, and merging them would let "nobody looked" render as +> a finding about what was seen. +- **What you lose:** nothing is asserted about the store principal, in either direction. The + least-privilege grant both runbooks prescribe is **unverified** on this instance, so an over-granted + credential would not be detected here. This is the *absence* of a clean result, not one. +- **How it happens:** the principal is denied the privilege query, the driver errors, or the store + handle implements no probe. +- **On SQLite this entry never fires.** A local file has no server principal, so the probe reports + `not_applicable` — a third, distinct status — and reports nothing here. Treating SQLite as + "unobserved" would put a permanent, unactionable entry on every single-node install, and a + permanently-true warning is read as noise. +- **How to refuse:** the same `[store].require_least_privilege = true` refuses on this condition too. + --- ## Standards mapping (ASVS v5.0 · NIST SP 800-53r5 · HIPAA §164.312) @@ -528,6 +565,7 @@ carried from that drive-to-pass, not re-derived here.** | `cleartext_accepted` (per-connection declared cleartext hop) | V12 Secure Communication | **SC-8** Transmission Confidentiality and Integrity · **SC-8(1)** Cryptographic Protection | §164.312(e)(1) Transmission Security · §164.312(e)(2)(ii) Encryption | | `tls_allow_expired` (per-connection expiry-only relaxation) | V12 Secure Communication | **SC-8(1)** Cryptographic Protection · **SC-12** Cryptographic Key Establishment and Management | §164.312(e)(1) Transmission Security · §164.312(e)(2)(ii) Encryption | | generic-ODBC `DATABASE` TLS unenforced (per-connection, driver-owned) | V12 Secure Communication | **SC-8** Transmission Confidentiality and Integrity · **SC-8(1)** Cryptographic Protection | §164.312(e)(1) Transmission Security · §164.312(e)(2)(ii) Encryption | +| `store_principal_over_granted` / `store_principal_privileges_unobserved` (observed store-principal privilege) | V13 Configuration (backend component accounts, 13.2.2) | **AC-6(5)** Privileged Accounts · **AC-6(9)** Log Use of Privileged Functions · **CM-7(5)** Authorized Software / least functionality | §164.312(a)(1) Access Control · §164.308(a)(4) Information Access Management | > The synthetic-vs-PHI relaxation (a synthetic instance keeps the PHI-only gates relaxed) is **risk-based > tailoring** keyed on `handles_real_patient_data`: an instance carrying no ePHI is out of scope for the diff --git a/messagefoundry/__main__.py b/messagefoundry/__main__.py index 04475589d..0f64dc672 100644 --- a/messagefoundry/__main__.py +++ b/messagefoundry/__main__.py @@ -1563,8 +1563,12 @@ def _serve(args: argparse.Namespace) -> int: # later — per connection — by the connector's own construction-time WARN (the ADR 0153 acceptance # with its reason and an audit record; the #333 generic-ODBC TLS reminder naming the connection), # and completely by `messagefoundry check` and GET /security/posture, which both have the graph. + # The store is NOT open yet either, so the #1008 store-principal privilege OBSERVATION is passed as + # None for the same reason and with the same discipline: it is reported moments later by the + # preflight's own log line + audit row once the lifespan opens the store, and completely by + # GET /security/posture. None here is "not yet observed", never "observed and clean". _loosenings = security_loosenings( - settings.security, settings.store, settings.auth, settings.alerts, (), (), () + settings.security, settings.store, settings.auth, settings.alerts, (), (), (), None ) if _loosenings: _seclog = logging.getLogger(__name__) @@ -1574,7 +1578,8 @@ def _serve(args: argparse.Namespace) -> int: "Per-connection cleartext_accepted (ADR 0153), tls_allow_expired and generic-ODBC " "DATABASE TLS declarations are NOT in this list — the graph is not loaded yet; they are " "reported by the connector construction gate, `messagefoundry check` and " - "GET /security/posture.", + "GET /security/posture. Nor is the store-principal privilege observation (#1008) — the " + "store is not open yet; the startup preflight logs and audits it moments from now.", len(_loosenings), "; ".join(f"{name} ({risk})" for name, risk in _loosenings), ) @@ -5003,13 +5008,14 @@ def _security(args: argparse.Namespace) -> int: _loosenings_partial = True def _loosenings(sec: SecuritySettings) -> list[dict[str, str]]: - # This CLI reads a SETTINGS file and never loads the connection graph, so it cannot see ANY of - # the three per-connection declarations — it passes empty lists and declares the gap in - # `loosenings_scope` below, instead of reporting a settings-only view as if it were the whole + # This CLI reads a SETTINGS file and never loads the connection graph — nor does it open the + # store — so it can see NEITHER the three per-connection declarations NOR the #1008 + # store-principal privilege observation. It passes empty lists and None and declares BOTH gaps + # in `loosenings_scope` below, instead of reporting a settings-only view as if it were the whole # posture. `messagefoundry check` and GET /security/posture are the complete surfaces. return [ {"switch": s, "risk": r} - for s, r in security_loosenings(sec, _store, _auth, _alerts, (), (), ()) + for s, r in security_loosenings(sec, _store, _auth, _alerts, (), (), (), None) ] #: Emitted alongside every loosening list this subcommand prints, so a reader can never mistake a @@ -5022,7 +5028,8 @@ def _loosenings(sec: SecuritySettings) -> list[dict[str, str]]: "loosenings_scope": ( "settings only ([security]/[store]/[auth]/[alerts]); the per-connection " "cleartext_accepted, tls_allow_expired and generic-ODBC DATABASE TLS declarations are NOT " - "included — see `messagefoundry check` or GET /security/posture" + "included, and neither is the store-principal privilege observation (#1008 — this command " + "opens no store) — see `messagefoundry check` or GET /security/posture" ), } diff --git a/messagefoundry/api/app.py b/messagefoundry/api/app.py index d419cafd9..6fabfcc9e 100644 --- a/messagefoundry/api/app.py +++ b/messagefoundry/api/app.py @@ -80,6 +80,7 @@ render_metrics, ) from messagefoundry.api.models import ( + STORE_PRIVILEGE_NOT_PROBED, AiChatRequest, AiChatResponse, AiPolicy, @@ -161,6 +162,7 @@ StatsResetRequest, StatsResetResult, StatsResponse, + StorePrivilegeView, SystemStatus, UpdateInfo, UploadDeleteResult, @@ -307,6 +309,7 @@ make_spec, ) from messagefoundry.store.metadata import user_metadata +from messagefoundry.store.privilege import run_store_privilege_preflight from messagefoundry.store.store import _secure_file from messagefoundry.transports.ai_broker import AiBrokerError, ai_broker_from_settings from messagefoundry.transports.base import ( @@ -1609,6 +1612,11 @@ async def security_posture( "NOT included (see `messagefoundry check`)" ) ) + # #1008: the store-principal privilege OBSERVATION the serve lifespan stashed. `None` means no + # preflight ran in this process (an embedding, or an app built without the managed lifespan) — + # the registry then reports nothing for it and `store_privilege` below renders the explicit + # `not_probed` status, so silence never reads as a clean observation. + store_privilege = getattr(request.app.state, "store_privilege", None) loosenings = [ SecurityLoosening(switch=name, risk=risk) for name, risk in security_loosenings( @@ -1619,8 +1627,18 @@ async def security_posture( cleartext_hops, expired_hops, db_hops, + store_privilege, ) ] + store_privilege_view = ( + StorePrivilegeView(status=STORE_PRIVILEGE_NOT_PROBED) + if store_privilege is None + else StorePrivilegeView( + status=store_privilege.status.value, + excess=list(store_privilege.excess), + detail=store_privilege.detail, + ) + ) synthetic_relaxation = ( "strict PHI-only controls (at-rest-encryption refusal, deny-by-default egress, bounded " "retention) are relaxed: this instance is marked synthetic " @@ -1672,6 +1690,7 @@ async def security_posture( security=security.model_dump(), loosenings=loosenings, loosenings_scope=loosenings_scope, + store_privilege=store_privilege_view, synthetic_relaxation=synthetic_relaxation, fips_mode=fips_mode, # interpreter ssl/_hashlib OpenSSL FIPS-provider state; None=undeterminable openssl_version=openssl_version, # that OpenSSL's version string (public metadata) @@ -6000,6 +6019,29 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: await notifier.aclose() await store.close() raise + # #1008 (ASVS 13.2.2): read the store principal's EFFECTIVE privileges and report them, BEFORE + # any listener binds — the same seam and the same teardown discipline as the two preflights + # above. It ALWAYS runs: the WARN arm is the shipped behaviour and cannot block an install (a + # log line, an audit row, a GET /security/posture entry), so there is nothing to gate. Only the + # REFUSE arm is gated, on [store].require_least_privilege AND [security].enforcement=enforce, + # and it refuses on an UNOBSERVABLE probe as well as an over-grant — a declared refusal that + # passed a principal it could not read would be the fail-open shape the setting exists to close. + # SQLite reports NOT_APPLICABLE (no server principal), so the default single-node path is a log + # line and nothing else. + try: + app.state.store_privilege = ( + await run_store_privilege_preflight( + store, + require_least_privilege=resolved.require_least_privilege, + enforcing=(security_enforcement or SecurityEnforcement.ENFORCE) + is SecurityEnforcement.ENFORCE, + ) + ).posture() + except BaseException: + if notifier is not None: + await notifier.aclose() + await store.close() + raise # Cluster coordinator (Track B Step 3) — built from the opened store so a Postgres-backed # store can reach its pool. Returns the no-op NullCoordinator unless [cluster].enabled on a # Postgres store, so single-node is byte-identical. The Engine owns its lifecycle (start/stop diff --git a/messagefoundry/api/models.py b/messagefoundry/api/models.py index 4f8f7a929..b2cf9a2b0 100644 --- a/messagefoundry/api/models.py +++ b/messagefoundry/api/models.py @@ -1011,6 +1011,32 @@ class SecurityLoosening(BaseModel): risk: str +#: The value ``StorePrivilegeView.status`` carries when no preflight ran in THIS process (an +#: embedding, or an app built without the managed lifespan). It is deliberately its own value: the +#: three real statuses are what the probe found, and "nobody looked" must never render as one of them. +STORE_PRIVILEGE_NOT_PROBED = "not_probed" + + +class StorePrivilegeView(BaseModel): + """What the startup preflight OBSERVED about the store principal's effective privileges (#1008, + ASVS 13.2.2). + + ``status`` is the load-bearing field and has four values, three of which come straight from + ``StorePrivilegeStatus``: ``observed`` (the probe read the principal — ``excess`` then tells you + whether it is clean), ``not_applicable`` (SQLite: a local file, no server principal exists), + ``unobservable`` (the probe could NOT run) and ``not_probed`` (no preflight ran in this process). + Only ``observed`` with an empty ``excess`` is a clean bill of health; the other three are each a + DIFFERENT reason there isn't one, and none of them may be read as a pass.""" + + status: str + #: Privileges the principal holds beyond the documented least-privilege grant, named individually. + #: A count would say "3 grants are excessive" without saying which, which is the shape that lets a + #: grant nobody intended survive a posture review. + excess: list[str] = Field(default_factory=list) + #: Why the probe could not observe, or what it observed against. Never secret material. + detail: str = "" + + class SecurityPosture(BaseModel): """The instance's **effective** PHI-at-rest security posture (M5), behind the authenticated, permission-gated ``GET /security/posture`` route. Surfaces what protection is *actually* in effect @@ -1054,6 +1080,13 @@ class SecurityPosture(BaseModel): # settings-only subset with no marker would understate the posture, which is the one thing this # route must not do; ``messagefoundry security show`` carries the same marker for the same reason. loosenings_scope: str | None = None + # #1008 (ASVS 13.2.2): the store principal's OBSERVED effective privileges. Always present — the + # default is the explicit ``not_probed`` status, never an empty/clean-looking value, so an app that + # never ran the preflight says so rather than reading as observed-and-fine. An over-grant and an + # unobservable probe ALSO appear in `loosenings` above; this field is the full observation. + store_privilege: StorePrivilegeView = Field( + default_factory=lambda: StorePrivilegeView(status=STORE_PRIVILEGE_NOT_PROBED) + ) # Set WHERE handles_real_patient_data=false: the strict PHI-only controls (at-rest-encryption refusal, # deny-by-default egress, bounded retention) are relaxed because the instance carries no ePHI (AC-6). synthetic_relaxation: str | None = None diff --git a/messagefoundry/config/settings.py b/messagefoundry/config/settings.py index 0214873f9..bfb61d1d9 100644 --- a/messagefoundry/config/settings.py +++ b/messagefoundry/config/settings.py @@ -39,6 +39,7 @@ import string import tomllib from collections.abc import Mapping, Sequence +from dataclasses import dataclass from datetime import date from enum import Enum from pathlib import Path @@ -177,6 +178,36 @@ class StoreBackend(str, Enum): # noqa: UP042 ) +class StorePrivilegeStatus(str, Enum): # noqa: UP042 + """Whether the store principal's effective privileges were actually READ (#1008, ASVS 13.2.2). + + Three values, and the third is the point. ``OBSERVED`` with an empty excess list is a clean bill + of health; ``UNOBSERVABLE`` is the ABSENCE of one. A two-valued version of this enum would let a + probe that never ran report as a pass, which is the fail-open shape the preflight exists to close. + """ + + OBSERVED = "observed" # the probe ran and read the principal's effective privileges + NOT_APPLICABLE = "not_applicable" # SQLite: a local file, no server principal exists to probe + UNOBSERVABLE = "unobservable" # the probe could NOT run — permission denied, no probe, an error + + +@dataclass(frozen=True, slots=True) +class StorePrivilegePosture: + """The store-privilege preflight's finding in the plain data shape :func:`security_loosenings` + consumes. + + A plain dataclass rather than the store package's richer report for the same reason the + connection-scoped deviations arrive there as plain NAMES: ``config.settings`` must never import + the store package (``store/*`` imports THIS module, so the reverse direction is a cycle).""" + + status: StorePrivilegeStatus + #: What the principal holds beyond the documented least-privilege grant; empty when it holds + #: nothing extra, and always empty when ``status`` is not ``OBSERVED``. + excess: tuple[str, ...] = () + #: Why the probe could not observe (``UNOBSERVABLE``), or what it observed against (otherwise). + detail: str = "" + + class SqliteSync(str, Enum): # noqa: UP042 NORMAL = "normal" # crash-safe under WAL, no per-commit fsync (default) FULL = "full" @@ -519,6 +550,23 @@ class StoreSettings(_Section): # credential) is exempt; Postgres has no managed-identity auth mode, so it cannot satisfy it. Admin # device posture + AD/SMTP managed identity stay deployment-delegated (see docs/SECURITY.md). require_managed_identity: bool = False + # Least-PRIVILEGE precondition on the store principal (#1008, ASVS 13.2.2) — the privilege sibling + # of require_managed_identity above, which constrains the credential's KIND and never what it may + # do (a `sysadmin` gMSA satisfies that one clean). The serve-time probe + # (store/privilege.py) reads the principal's EFFECTIVE fixed-server-role / database-role membership + # on SQL Server and its role attributes / grants on Postgres, and compares them against the grant + # docs/DEPLOY-SERVER-DB.md §1.1/§1.2 prescribes. + # + # OFF BY DEFAULT, and this default governs the REFUSE arm only. The WARN arm ships ON: the probe + # always runs, always logs, always audits, and always feeds security_loosenings() — it cannot block + # an install, so nothing is gated behind this. Refusal is what is gated, because a preflight that + # refused on over-grant by default could block a legitimate deployment mid-setup, which is not this + # control's job. When TRUE, `serve` refuses to start on an observed over-grant — AND on a probe that + # could NOT RUN, because a declared refusal that passes an unobservable principal is exactly the + # fail-open shape the operator turned it on to prevent. Like require_managed_identity the split + # reads [security].enforcement, NOT the deployment tier, so enforcement='warn' downgrades the + # refusal to a warning. SQLite is exempt (a local file has no server principal to probe). + require_least_privilege: bool = False encrypt: bool = True trust_server_certificate: bool = False # Optional certificate file to verify the DB server certificate against a PRIVATE / self-signed CA (the @@ -4306,6 +4354,7 @@ def security_loosenings( cleartext_hops: Sequence[str], expiry_relaxed_hops: Sequence[str], unverified_db_hops: Sequence[str], + store_privilege: StorePrivilegePosture | None, ) -> list[tuple[str, str]]: """The ``[security]`` switches at their INSECURE value, plus the enumerated deviations outside that section, as ``(switch, plain-language risk)``. @@ -4315,8 +4364,9 @@ def security_loosenings( that iterates ``SecuritySettings.model_fields`` and fails on an unreported, unexempted one — plus an ENUMERATED set of deviations that live elsewhere: ``[store].aad_bind``, ``[auth].ad_session_recheck_seconds``, ``[alerts].email_use_tls``/``email_tls_verify`` (#323 - layer 3), and three per-connection deviations — ``cleartext_accepted``, ``tls_allow_expired``, and a - generic-ODBC ``DATABASE`` hop with TLS unenforced (#333). It is NOT yet + layer 3), three per-connection deviations — ``cleartext_accepted``, ``tls_allow_expired``, and a + generic-ODBC ``DATABASE`` hop with TLS unenforced (#333) — and the store principal's OBSERVED + privilege posture (#1008). It is NOT yet an exhaustive registry of every security-relevant switch in every section; ``[store]``/``[auth]`` carry others (``encrypt``, ``trust_server_certificate``, ``enabled``, ``require_mfa``, ``ad_tls_verify``, ``ad_allow_insecure_ldap``, ``oidc_require_mfa_claim``, @@ -4340,7 +4390,17 @@ def security_loosenings( posture by the back door. An optional parameter is a detector that silently fails to fire; a required one makes omission a type error at every call site. - The last three parameters are the CONNECTION-scoped deviations, each a list of connection NAMES: + ``store_privilege`` is the store-principal privilege OBSERVATION (#1008, ASVS 13.2.2), for the same + reason and in the same plain shape: it is what the principal actually holds, produced by the + serve-time probe in ``store/privilege.py`` and passed in as a + :class:`StorePrivilegePosture` so this module never imports the store package. ``None`` means THIS + CALL SITE has no probe result (no store is open — ``messagefoundry security show``, or a posture + read on an engine that never ran the preflight), and the caller SAYS SO in its own output. It is not + a clean result and this registry never renders it as one. Note the switch that acts on the finding + — ``[store].require_least_privilege`` — is a HARDENING, so it is not itself reported here; the + DEVIATION is what the observation found, exactly as with the three connection-scoped entries. + + The three sequence parameters are the CONNECTION-scoped deviations, each a list of connection NAMES: ``cleartext_hops`` declares ``cleartext_accepted`` (ADR 0153), ``expiry_relaxed_hops`` declares ``tls_allow_expired`` (#129 / ADR 0094), and ``unverified_db_hops`` is a generic-ODBC ``DATABASE`` connection whose ``odbc_params`` leave TLS unenforced (#66 / ADR 0092's amendment). They arrive as @@ -4603,6 +4663,33 @@ def security_loosenings( "rows, and the DSN credential, may cross in plaintext", ) ) + # --- the STORE PRINCIPAL's observed privilege posture (#1008, ASVS 13.2.2). An OBSERVATION, like + # the three connection-scoped entries above and unlike every switch: the deviation is what the + # engine's own database credential turns out to hold, which no [store] flag declares. Both arms are + # reported and they read DIFFERENTLY on purpose — "could not observe" is the ABSENCE of a clean + # result, and collapsing it into silence would make this registry assert a posture nobody checked. + if store_privilege is not None: + if store_privilege.status is StorePrivilegeStatus.UNOBSERVABLE: + out.append( + ( + "store_principal_privileges_unobserved", + "the store principal's EFFECTIVE privileges could not be read " + f"({store_privilege.detail}) — the least-privilege grant both server-DB runbooks " + "prescribe is UNVERIFIED on this instance, so an over-granted database credential " + "would not be detected here; this is the absence of a clean result, not one", + ) + ) + elif store_privilege.excess: + named = ", ".join(store_privilege.excess) + out.append( + ( + "store_principal_over_granted", + f"the store principal holds {len(store_privilege.excess)} privilege(s) BEYOND the " + f"least-privilege grant docs/DEPLOY-SERVER-DB.md prescribes ({named}) — the " + "engine's own database credential can reach data and administrative operations " + "its runbook says it must not", + ) + ) return out diff --git a/messagefoundry/store/postgres.py b/messagefoundry/store/postgres.py index 825060d08..559e41ea2 100644 --- a/messagefoundry/store/postgres.py +++ b/messagefoundry/store/postgres.py @@ -66,6 +66,7 @@ from messagefoundry.config.settings import ( INSECURE_TLS_ESCAPE_ENV, StoreBackend, + StorePrivilegeStatus, StoreSettings, weakened_tls_escape_permitted, ) @@ -110,6 +111,11 @@ merge_user_metadata, ) from messagefoundry.store.pool_metrics import AcquireWaitHistogram, PoolStatus +from messagefoundry.store.privilege import ( + PostgresRoleFacts, + StorePrivilegeReport, + postgres_excess, +) from messagefoundry.store.store import ( MESSAGE_EVENT_KINDS, NOT_DEPLOYED_EVENT, @@ -1236,6 +1242,75 @@ async def require_rcsi_for_pooled(self) -> None: # Server's optional READ_COMMITTED_SNAPSHOT (ADR 0066 §3.3). return None + async def probe_principal_privileges(self) -> StorePrivilegeReport: + """Read this role's EFFECTIVE privileges (#1008, ASVS 13.2.2) and report them against the grant + ``docs/DEPLOY-SERVER-DB.md`` §1.2 prescribes (a plain ``LOGIN`` role with **no** attributes, + ``CONNECT`` on the database and ownership of its own schema — nothing wider). + + Postgres has no fixed-server-role / fixed-database-role split to mirror SQL Server's. The + equivalents are role ATTRIBUTES (``SUPERUSER`` and friends), membership in the predefined + ``pg_*`` roles, and ownership, so all three are read. + + **Attributes are read per assumable role, not only for the principal itself.** ``pg_has_role(…, + 'MEMBER')`` is what the principal can exercise (by inheritance or ``SET ROLE``), so a + user-defined wrapper role that is itself ``SUPERUSER`` is caught by what it GRANTS rather than + by whether its name is on a list — a name denylist cannot see that. ``pg_roles`` is + world-readable, so unlike SQL Server's catalogs this read needs no special permission.""" + rows = await self._fetchall( + "SELECT r.rolname, (r.rolname = current_user) AS is_self, r.rolsuper," + " r.rolcreaterole, r.rolcreatedb, r.rolreplication, r.rolbypassrls" + " FROM pg_catalog.pg_roles r" + " WHERE pg_catalog.pg_has_role(current_user, r.oid, 'MEMBER')" + " ORDER BY r.rolname" + ) + scalar = await self._fetchone( + "SELECT current_user AS principal, current_catalog AS db_name," + " pg_catalog.pg_has_role(current_user, d.datdba, 'MEMBER') AS owns_database," + " pg_catalog.has_database_privilege(current_user, d.oid, 'CREATE') AS create_on_database" + " FROM pg_catalog.pg_database d WHERE d.datname = current_catalog" + ) + if scalar is None: + return StorePrivilegeReport( + backend=self.backend, + status=StorePrivilegeStatus.UNOBSERVABLE, + detail="the privilege query returned no row for current_catalog", + ) + facts = tuple( + PostgresRoleFacts( + name=str(r["rolname"]), + is_self=bool(r["is_self"]), + superuser=bool(r["rolsuper"]), + createrole=bool(r["rolcreaterole"]), + createdb=bool(r["rolcreatedb"]), + replication=bool(r["rolreplication"]), + bypassrls=bool(r["rolbypassrls"]), + ) + for r in rows + ) + database = str(scalar["db_name"] or self._settings.database or "") + return StorePrivilegeReport( + backend=self.backend, + status=StorePrivilegeStatus.OBSERVED, + principal=str(scalar["principal"] or ""), + database=database, + # Postgres has no server-role tier; its server-level equivalents are role ATTRIBUTES, which + # surface in `excess` rather than as role names. Left empty rather than faked with a + # SQL-Server-shaped value that would not mean the same thing. + server_roles=(), + database_roles=tuple(f.name for f in facts if not f.is_self), + excess=postgres_excess( + roles=facts, + owns_database=bool(scalar["owns_database"]), + create_on_database=bool(scalar["create_on_database"]), + database=database, + ), + detail=( + "roles are every role this principal may assume (pg_has_role MEMBER, so inherited and " + "SET ROLE alike); role ATTRIBUTES (SUPERUSER/CREATEROLE/CREATEDB/REPLICATION/BYPASSRLS) " + "are Postgres's server-level equivalent and are reported as excess, not as role names" + ), + ) + # --- PHI-at-rest cipher seam for nullable text columns (WP-5) ------------- # Cell-bound AAD (ASVS 11.3.3, ADR 0019): `aad` is REQUIRED so mypy-strict flags any un-threaded diff --git a/messagefoundry/store/privilege.py b/messagefoundry/store/privilege.py new file mode 100644 index 000000000..c4822ff3c --- /dev/null +++ b/messagefoundry/store/privilege.py @@ -0,0 +1,389 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""Startup preflight on the **effective privileges of the store principal** (BACKLOG #1008, ASVS 13.2.2). + +Both server-DB runbooks prescribe a least-privilege grant for the engine's database principal +(``docs/DEPLOY-SERVER-DB.md`` §1.1 for SQL Server, §1.2 for Postgres). Until this module the engine +could not **observe** whether the principal it actually connects as matches that prescription: no +fixed-server-role probe and no database-role-membership probe existed anywhere in the four packages +the ASVS scorecard scans, and ``[store].require_managed_identity`` constrains the credential's *kind* +(Windows Integrated / Entra vs a static SQL login), never its privilege — a ``sysadmin`` gMSA +satisfies it clean. So on a first deployment an over-granted store principal **would** go unobserved. +Nothing is over-granted today: MessageFoundry is a not-deployed beta with zero instances. + +**The shape, and it is the whole design: OBSERVE AND WARN LOUDLY FIRST.** + +* The **WARN** arm ships **ON**. It is a log line plus an audit row plus a + :func:`~messagefoundry.config.settings.security_loosenings` entry; it cannot block any install. +* The **REFUSE** arm is gated behind operator-declared ``[store].require_least_privilege`` (default + ``False``) — a preflight that refused on over-grant by default could block a legitimate deployment + mid-setup, which is not this control's job. Like the ``require_managed_identity`` gate it copies, + the declared refusal downgrades to a warning under ``[security].enforcement = warn``. +* It must not fail **open** either. A probe that cannot run — permission denied, an unsupported + backend, a store handle with no probe at all — reports :attr:`StorePrivilegeStatus.UNOBSERVABLE`, + which is a distinct, named, loud condition everywhere it surfaces: a different log line, a + different audit ``status``, and its own posture entry. *"Could not observe"* and *"observed, and + it is fine"* are never the same output. Under a declared ``require_least_privilege`` an + unobservable probe **refuses**, because a control that cannot see is exactly the fail-open shape + the setting was turned on to prevent. +* SQLite reports :attr:`StorePrivilegeStatus.NOT_APPLICABLE` and says what it did instead of + pretending it ran: a local file has no server principal, and its access control is the filesystem's. + +**This module names ``IS_SRVROLEMEMBER`` / ``IS_ROLEMEMBER`` / ``db_owner`` / ``sysadmin`` on +purpose, and that flips ASVS cell 13.2.2's absence claim** (``pattern = +"IS_SRVROLEMEMBER|IS_ROLEMEMBER|db_owner|sysadmin"``, scanned over ``messagefoundry``, +``messagefoundry_webconsole``, ``harness`` and ``scripts`` — see +``tests/test_docs_db_grants.py``). That is the scorecard correctly noticing the code changed, not a +lint failure to be worked around by obfuscating the SQL. The paired vault-side re-score is a separate, +deliberate act; do not hide these tokens to keep an absence claim green that is no longer true. +""" + +from __future__ import annotations + +import json +import logging +from collections.abc import Sequence +from dataclasses import dataclass +from typing import TYPE_CHECKING, Protocol, runtime_checkable + +from messagefoundry.config.settings import ( + StoreBackend, + StorePrivilegePosture, + StorePrivilegeStatus, +) +from messagefoundry.support.redact import redact_log_line + +if TYPE_CHECKING: # pragma: no cover - typing only + from messagefoundry.store.base import Store + +log = logging.getLogger(__name__) + +__all__ = [ + "POSTGRES_EXCESSIVE_ROLES", + "SQLSERVER_DOCUMENTED_DATABASE_ROLES", + "SQLSERVER_FIXED_DATABASE_ROLES", + "SQLSERVER_FIXED_SERVER_ROLES", + "PostgresRoleFacts", + "PrivilegeProbeStore", + "StorePrivilegeError", + "StorePrivilegeReport", + "postgres_excess", + "probe_failure", + "run_store_privilege_preflight", + "sqlserver_excess", +] + + +class StorePrivilegeError(RuntimeError): + """Raised by the preflight when ``[store].require_least_privilege`` is declared and the principal + is over-granted (or could not be observed) — the caller refuses to start before any listener binds.""" + + +# --- SQL Server ------------------------------------------------------------------------------- +#: The closed set of SQL Server FIXED SERVER roles, probed by name rather than enumerated from +#: ``sys.server_principals``: catalog visibility is permission-filtered, so an enumeration that comes +#: back empty is indistinguishable from "a member of nothing" — the false-clean this control exists to +#: prevent. ``IS_SRVROLEMEMBER`` answers for the CURRENT login and needs no catalog permission. +SQLSERVER_FIXED_SERVER_ROLES: tuple[str, ...] = ( + "sysadmin", + "securityadmin", + "serveradmin", + "setupadmin", + "processadmin", + "diskadmin", + "dbcreator", + "bulkadmin", +) + +#: The closed set of SQL Server FIXED DATABASE roles, probed by name for the same reason. +SQLSERVER_FIXED_DATABASE_ROLES: tuple[str, ...] = ( + "db_owner", + "db_securityadmin", + "db_accessadmin", + "db_backupoperator", + "db_ddladmin", + "db_datareader", + "db_datawriter", + "db_denydatareader", + "db_denydatawriter", +) + +#: The grant both runbooks prescribe (``docs/DEPLOY-SERVER-DB.md`` §1.1) and the engine's store +#: actually needs: row CRUD plus the schema DDL the ADR 0064 bootstrap issues on a moved schema. +#: The documented SERVER-role set is EMPTY — the engine needs no fixed server role at all. +SQLSERVER_DOCUMENTED_DATABASE_ROLES: frozenset[str] = frozenset( + {"db_datareader", "db_datawriter", "db_ddladmin"} +) + +#: ``db_deny*`` memberships REMOVE access. They are reported as observed but are never "excess" — a +#: control that flagged a restriction as an over-grant would train an operator to ignore it. +_SQLSERVER_DENY_ROLES: frozenset[str] = frozenset({"db_denydatareader", "db_denydatawriter"}) + + +def sqlserver_excess( + *, + server_roles: Sequence[str], + database_roles: Sequence[str], + control_server: bool, + control_database: bool, + database: str, +) -> tuple[str, ...]: + """What an observed SQL Server principal holds BEYOND the documented least-privilege grant. + + Pure — no I/O — so both directions (over-granted and correctly-granted) are unit-testable without + a database, and the live server legs assert the same function against a real login. + + A membership that IMPLIES a permission suppresses the implied one, so the list reads as a set of + distinct grants rather than one grant restated: ``sysadmin`` already carries ``CONTROL SERVER``, + and ``db_owner`` already carries ``CONTROL`` on the database.""" + out: list[str] = [] + for role in server_roles: + out.append(f"server role {role}") + for role in database_roles: + if role in SQLSERVER_DOCUMENTED_DATABASE_ROLES or role in _SQLSERVER_DENY_ROLES: + continue + out.append(f"database role {role}") + if control_server and "sysadmin" not in server_roles: + out.append("CONTROL SERVER") + if control_database and "db_owner" not in database_roles: + out.append(f"CONTROL on database {database}") + return tuple(out) + + +# --- Postgres --------------------------------------------------------------------------------- +@dataclass(frozen=True, slots=True) +class PostgresRoleFacts: + """One role the store principal can assume, with the ATTRIBUTES that role carries. + + Attributes are read per role rather than only for the principal itself so a user-defined wrapper + role that is itself ``SUPERUSER`` is caught by what it grants, not by whether its name happens to + be on a list. A denylist of role NAMES cannot see that; this can.""" + + name: str + is_self: bool + superuser: bool + createrole: bool + createdb: bool + replication: bool + bypassrls: bool + + +#: Predefined Postgres roles that reach beyond the engine's own schema — cross-schema data access, +#: host file/program reach, or administrative control. Membership in any is more than the documented +#: grant. Cloud-managed superuser umbrella roles are included because they are superuser in all but +#: the ``rolsuper`` bit (which managed providers withhold), so the attribute check alone misses them. +POSTGRES_EXCESSIVE_ROLES: frozenset[str] = frozenset( + { + "pg_read_all_data", + "pg_write_all_data", + "pg_read_server_files", + "pg_write_server_files", + "pg_execute_server_program", + "pg_signal_backend", + "pg_checkpoint", + "pg_maintain", + "pg_create_subscription", + "rds_superuser", + "cloudsqlsuperuser", + "azure_pg_admin", + } +) + + +def postgres_excess( + *, + roles: Sequence[PostgresRoleFacts], + owns_database: bool, + create_on_database: bool, + database: str, +) -> tuple[str, ...]: + """What an observed Postgres principal holds BEYOND the documented least-privilege grant. + + Pure, like :func:`sqlserver_excess`. + + ``SUPERUSER`` short-circuits the rest: a superuser is implicitly a member of every role and holds + every database privilege, so enumerating them would bury the one finding that matters under a + dozen restatements of it. Its own role attributes are still listed — they say *how* the identity + is configured, which is what an operator has to change.""" + out: list[str] = [] + superuser = any(r.superuser for r in roles) + if superuser: + out.append("SUPERUSER") + self_rows = [r for r in roles if r.is_self] + for attr, label in ( + ("createrole", "CREATEROLE"), + ("createdb", "CREATEDB"), + ("replication", "REPLICATION"), + ("bypassrls", "BYPASSRLS"), + ): + if any(getattr(r, attr) for r in self_rows): + out.append(label) + if superuser: + return tuple(out) + for role in roles: + if role.name in POSTGRES_EXCESSIVE_ROLES: + out.append(f"role {role.name}") + if owns_database: + out.append(f"OWNER of database {database}") + elif create_on_database: + # Only when it is NOT the owner: ownership already carries CREATE, so reporting both would + # restate one grant as two. + out.append(f"CREATE on database {database}") + return tuple(out) + + +# --- the report ------------------------------------------------------------------------------- +@dataclass(frozen=True, slots=True) +class StorePrivilegeReport: + """What the probe ACTUALLY observed — never what it assumes. + + ``status`` is the load-bearing field: :attr:`StorePrivilegeStatus.OBSERVED` with an empty + :attr:`excess` is a clean bill of health, and it is a DIFFERENT value from + :attr:`StorePrivilegeStatus.UNOBSERVABLE`, which is the absence of one.""" + + backend: StoreBackend + status: StorePrivilegeStatus + principal: str = "" + database: str = "" + server_roles: tuple[str, ...] = () + database_roles: tuple[str, ...] = () + excess: tuple[str, ...] = () + detail: str = "" + + def posture(self) -> StorePrivilegePosture: + """The settings-layer view :func:`security_loosenings` consumes (a plain data type, so + ``config.settings`` never has to know the store package — the same reason the connection-scoped + deviations arrive there as plain names).""" + return StorePrivilegePosture(status=self.status, excess=self.excess, detail=self.detail) + + def summary(self) -> str: + """One operator-readable line. The three statuses read differently ON PURPOSE.""" + if self.status is StorePrivilegeStatus.NOT_APPLICABLE: + return ( + f"store privilege preflight: NOT APPLICABLE on {self.backend.value} — {self.detail}" + ) + if self.status is StorePrivilegeStatus.UNOBSERVABLE: + return ( + f"store privilege preflight: COULD NOT OBSERVE the {self.backend.value} store " + f"principal's effective privileges — {self.detail}. This is NOT a clean result: the " + "documented least-privilege grant is UNVERIFIED on this instance" + ) + if self.excess: + return ( + f"store privilege preflight: the {self.backend.value} store principal " + f"{self.principal!r} on {self.database!r} holds {len(self.excess)} privilege(s) " + f"BEYOND the documented least-privilege grant: {', '.join(self.excess)}" + ) + return ( + f"store privilege preflight: OBSERVED the {self.backend.value} store principal " + f"{self.principal!r} on {self.database!r} — no privilege beyond the documented grant " + f"(server roles: {', '.join(self.server_roles) or 'none'}; database roles: " + f"{', '.join(self.database_roles) or 'none'})" + ) + + def audit_detail(self) -> dict[str, object]: + """The non-secret, PHI-free audit payload: a status, principal/role NAMES and the excess list.""" + return { + "backend": self.backend.value, + "status": self.status.value, + "principal": self.principal, + "database": self.database, + "server_roles": list(self.server_roles), + "database_roles": list(self.database_roles), + "excess": list(self.excess), + "detail": self.detail, + } + + +@runtime_checkable +class PrivilegeProbeStore(Protocol): + """The narrow store slice this preflight uses. A SEPARATE protocol (not part of + :class:`~messagefoundry.store.base.Store`) so a backend opts in structurally — the SQLite + ``MessageStore``, ``SqlServerStore`` and ``PostgresStore`` all implement it. A handle that does + not is reported UNOBSERVABLE rather than skipped: an unimplemented probe is a thing the engine + could not observe, which is precisely what that status means.""" + + async def probe_principal_privileges(self) -> StorePrivilegeReport: ... + + +def probe_failure(backend: StoreBackend, exc: BaseException) -> StorePrivilegeReport: + """An UNOBSERVABLE report built from a probe exception, with the driver text redacted. + + The message goes through the shared secret/PHI redactor before it reaches a log or an audit row: + a driver diagnostic can echo connection parameters, and this text lands in a durable audit table + an operator reads. Over-redaction is the correct direction here — the NAMED condition ("could not + observe") is the load-bearing part, not the driver's wording.""" + return StorePrivilegeReport( + backend=backend, + status=StorePrivilegeStatus.UNOBSERVABLE, + detail=f"{type(exc).__name__}: {redact_log_line(str(exc))[:300]}", + ) + + +async def run_store_privilege_preflight( + store: Store, + *, + require_least_privilege: bool, + enforcing: bool, +) -> StorePrivilegeReport: + """Probe the store principal's effective privileges, report what was observed, and — only under a + declared ``[store].require_least_privilege`` on an enforcing instance — refuse. + + Wire it into serve startup **after** the store opens and **before** any listener binds (the ADR + 0041 attestation / ASVS 6.7.1 trust-anchor preflights sit in the same place, for the same reason). + The audit write is best-effort and never masks the finding. + + Raises :class:`StorePrivilegeError` when the operator declared ``require_least_privilege``, + ``[security].enforcement`` is ``enforce``, and the principal is either over-granted or + UNOBSERVABLE.""" + backend = getattr(store, "backend", StoreBackend.SQLITE) + if isinstance(store, PrivilegeProbeStore): + try: + report = await store.probe_principal_privileges() + except Exception as exc: # noqa: BLE001 — any probe failure is UNOBSERVABLE, never a silent pass + report = probe_failure(backend, exc) + else: + report = StorePrivilegeReport( + backend=backend, + status=StorePrivilegeStatus.UNOBSERVABLE, + detail=( + f"this {backend.value} store handle ({type(store).__name__}) implements no " + "privilege probe, so the principal's effective privileges were never read" + ), + ) + + # NOT_APPLICABLE is not "unclean": SQLite genuinely has no principal to over-grant, so folding it + # into the warning arm would put a permanent, unactionable warning on every single-node install — + # and a permanently-true warning is read as noise, which costs this control its readers. + unclean = report.status is StorePrivilegeStatus.UNOBSERVABLE or bool(report.excess) + refusing = unclean and require_least_privilege and enforcing + if not unclean: + log.info("%s", report.summary()) + else: + log.warning( + "%s — [store].require_least_privilege=%s, enforcing=%s%s", + report.summary(), + require_least_privilege, + enforcing, + "; REFUSING to start" if refusing else "", + ) + + # NOT_APPLICABLE writes no audit row: SQLite has no principal, so there is no observation to + # record and a row saying so on every start would be noise that dilutes the ones that matter. + if report.status is not StorePrivilegeStatus.NOT_APPLICABLE: + detail = report.audit_detail() + detail["require_least_privilege"] = require_least_privilege + detail["refused"] = refusing + try: + await store.record_audit( + "store_privilege_preflight", actor=None, detail=json.dumps(detail) + ) + except Exception: # noqa: BLE001 — auditing is best-effort; never mask the finding + log.exception("store privilege preflight: failed to record the audit row") + + if refusing: + raise StorePrivilegeError( + f"{report.summary()} — [store].require_least_privilege is set and " + "[security].enforcement is 'enforce'; refusing to start" + ) + return report diff --git a/messagefoundry/store/sqlserver.py b/messagefoundry/store/sqlserver.py index be5f53003..5200345b2 100644 --- a/messagefoundry/store/sqlserver.py +++ b/messagefoundry/store/sqlserver.py @@ -59,6 +59,7 @@ INSECURE_TLS_ESCAPE_ENV, SqlAuth, StoreBackend, + StorePrivilegeStatus, StoreSettings, weakened_tls_escape_permitted, ) @@ -95,6 +96,12 @@ merge_user_metadata, ) from messagefoundry.store.pool_metrics import AcquireWaitHistogram, ClaimPoolStatus, PoolStatus +from messagefoundry.store.privilege import ( + SQLSERVER_FIXED_DATABASE_ROLES, + SQLSERVER_FIXED_SERVER_ROLES, + StorePrivilegeReport, + sqlserver_excess, +) from messagefoundry.store.store import ( MESSAGE_EVENT_KINDS, NOT_DEPLOYED_EVENT, @@ -2907,6 +2914,84 @@ async def require_rcsi_for_pooled(self) -> None: " WITH ROLLBACK IMMEDIATE — refusing to start pooled claimers (fail closed)" ) + async def probe_principal_privileges(self) -> StorePrivilegeReport: + """Read this login's EFFECTIVE fixed-server-role and database-role membership (#1008, + ASVS 13.2.2) and report what was observed against the grant ``docs/DEPLOY-SERVER-DB.md`` §1.1 + prescribes (``db_datareader`` + ``db_datawriter`` + ``db_ddladmin``, and **no** server role). + + **Both closed role sets are probed BY NAME, not enumerated from the catalog, and that is the + load-bearing choice.** ``sys.server_principals`` / ``sys.database_principals`` are + permission-filtered: an enumeration that comes back empty is indistinguishable from *"a member + of nothing"*, so a visibility restriction would render as a clean bill of health — the exact + false-clean this preflight exists to prevent. ``IS_SRVROLEMEMBER`` / ``IS_ROLEMEMBER`` answer + for the CURRENT principal and need no catalog permission, so they are authoritative. The + catalog enumeration runs too, but only to ADD user-defined database roles the closed set cannot + name, and it is best-effort: it can only widen the finding, never narrow it. + + ``HAS_PERMS_BY_NAME`` covers the grants that are permissions rather than roles (``CONTROL + SERVER``, ``CONTROL`` on the database), so a principal over-granted by direct ``GRANT`` rather + than by role membership is still seen.""" + server_cols = [ + f"IS_SRVROLEMEMBER(?) AS srv_{i}" for i in range(len(SQLSERVER_FIXED_SERVER_ROLES)) + ] + db_cols = [ + f"IS_ROLEMEMBER(?) AS dbr_{i}" for i in range(len(SQLSERVER_FIXED_DATABASE_ROLES)) + ] + row = await self._fetchone( + "SELECT SUSER_SNAME() AS login_name, USER_NAME() AS db_user, DB_NAME() AS db_name," + " HAS_PERMS_BY_NAME(NULL, NULL, 'CONTROL SERVER') AS control_server," + " HAS_PERMS_BY_NAME(DB_NAME(), 'DATABASE', 'CONTROL') AS control_db, " + + ", ".join(server_cols + db_cols), + SQLSERVER_FIXED_SERVER_ROLES + SQLSERVER_FIXED_DATABASE_ROLES, + ) + if row is None: + return StorePrivilegeReport( + backend=self.backend, + status=StorePrivilegeStatus.UNOBSERVABLE, + detail="the privilege query returned no row", + ) + server_roles = tuple( + name for i, name in enumerate(SQLSERVER_FIXED_SERVER_ROLES) if row[f"srv_{i}"] == 1 + ) + database_roles = tuple( + name for i, name in enumerate(SQLSERVER_FIXED_DATABASE_ROLES) if row[f"dbr_{i}"] == 1 + ) + note = ( + "fixed server + database role membership probed BY NAME (authoritative, catalog-visibility" + " independent); user-defined database roles added best-effort from sys.database_principals" + ) + try: + extra = await self._fetchall( + "SELECT p.name AS role_name FROM sys.database_principals p" + " WHERE p.type = 'R' AND p.name <> 'public' AND IS_ROLEMEMBER(p.name) = 1" + ) + except Exception as exc: # noqa: BLE001 — additive only; the closed-set probe already answered + note += ( + f" (that enumeration failed: {type(exc).__name__} — user-defined roles NOT listed)" + ) + else: + # dict.fromkeys preserves order while folding the fixed roles the enumeration repeats. + database_roles = tuple( + dict.fromkeys(database_roles + tuple(str(r["role_name"]) for r in extra)) + ) + database = str(row["db_name"] or self._settings.database or "") + return StorePrivilegeReport( + backend=self.backend, + status=StorePrivilegeStatus.OBSERVED, + principal=str(row["login_name"] or ""), + database=database, + server_roles=server_roles, + database_roles=database_roles, + excess=sqlserver_excess( + server_roles=server_roles, + database_roles=database_roles, + control_server=row["control_server"] == 1, + control_database=row["control_db"] == 1, + database=database, + ), + detail=f"database user {str(row['db_user'] or '')!r}; {note}", + ) + async def _ensure_schema(self) -> bool: """Apply the shipped DDL batch, or skip it entirely when the ``schema_meta`` marker already records this exact batch (ADR 0064). Returns ``True`` iff the batch ran.""" diff --git a/messagefoundry/store/store.py b/messagefoundry/store/store.py index 57200dd15..3b6573425 100644 --- a/messagefoundry/store/store.py +++ b/messagefoundry/store/store.py @@ -69,7 +69,7 @@ # may not import `messagefoundry.store` — can rebuild the SAME class the engine publishes. Re-exported # here so every existing `from messagefoundry.store.store import CapturedResponse` keeps working. from messagefoundry.config.response import CapturedResponse as CapturedResponse # re-export -from messagefoundry.config.settings import StoreBackend +from messagefoundry.config.settings import StoreBackend, StorePrivilegeStatus from messagefoundry.parsing.binary import strip_documents as _strip_documents from messagefoundry.redaction import safe_text from messagefoundry.store.audit_tee import emit_audit_tee @@ -95,6 +95,7 @@ merge_user_metadata, ) from messagefoundry.store.pool_metrics import PoolStatus +from messagefoundry.store.privilege import StorePrivilegeReport log = logging.getLogger(__name__) @@ -4668,6 +4669,25 @@ async def require_rcsi_for_pooled(self) -> None: # there is nothing to verify (ADR 0066 §3.5). return None + async def probe_principal_privileges(self) -> StorePrivilegeReport: + """NOT_APPLICABLE, and it says what it did instead of pretending it ran (#1008, ASVS 13.2.2). + + SQLite has no login, no fixed-server-role tier and no database-role tier: this process opens a + file. Returning ``OBSERVED`` with an empty excess list would be a clean bill of health for a + check that never happened, which is the one thing this preflight must never emit — so the + status is its own value, and the detail names the control that DOES govern access here.""" + return StorePrivilegeReport( + backend=self.backend, + status=StorePrivilegeStatus.NOT_APPLICABLE, + database=self.path, + detail=( + "the SQLite store is a local file this process opens directly — there is no server " + "principal, no fixed-server-role tier and no database-role tier to read. Access to the " + "store is governed by the filesystem ACL on the database file and its -wal/-shm " + "sidecars, which is an OS-level control the engine does not probe" + ), + ) + async def claim_next_fifo( self, name: str, diff --git a/tests/test_alert_smtp_tls.py b/tests/test_alert_smtp_tls.py index 49d582102..4b92b84ce 100644 --- a/tests/test_alert_smtp_tls.py +++ b/tests/test_alert_smtp_tls.py @@ -202,7 +202,10 @@ def _names(**kw: Any) -> list[str]: ) sec = SecuritySettings(**kw.pop("security", {})) return [ - n for n, _ in security_loosenings(sec, StoreSettings(), AuthSettings(), alerts, (), (), ()) + n + for n, _ in security_loosenings( + sec, StoreSettings(), AuthSettings(), alerts, (), (), (), None + ) ] @@ -238,7 +241,7 @@ def test_an_unconfigured_alert_transport_reports_no_hop_deviation() -> None: names = [ n for n, _ in security_loosenings( - SecuritySettings(), StoreSettings(), AuthSettings(), bare, (), (), () + SecuritySettings(), StoreSettings(), AuthSettings(), bare, (), (), (), None ) ] assert "email_use_tls" not in names diff --git a/tests/test_client_network_allowlist.py b/tests/test_client_network_allowlist.py index 8cd47d63b..1e2fcfe87 100644 --- a/tests/test_client_network_allowlist.py +++ b/tests/test_client_network_allowlist.py @@ -50,7 +50,9 @@ def _loosenings(sec: SecuritySettings) -> list[tuple[str, str]]: The registry takes all four inputs as REQUIRED arguments deliberately (ADR 0148: one posture, and a deviation the registry cannot see is a second posture by the back door). The tests below are about the ``[security]`` switches specifically, so the other three are pinned at shipped values here.""" - return security_loosenings(sec, StoreSettings(), AuthSettings(), AlertsSettings(), (), (), ()) + return security_loosenings( + sec, StoreSettings(), AuthSettings(), AlertsSettings(), (), (), (), None + ) PW = "a-strong-test-passphrase" # >=15, no app/vendor terms — satisfies the ASVS policy diff --git a/tests/test_memory_encryption_readout.py b/tests/test_memory_encryption_readout.py index 32cffbfec..09835f0ac 100644 --- a/tests/test_memory_encryption_readout.py +++ b/tests/test_memory_encryption_readout.py @@ -57,7 +57,9 @@ def _loosenings(sec: SecuritySettings) -> list[tuple[str, str]]: The registry takes all four inputs as REQUIRED arguments deliberately (ADR 0148: one posture, and a deviation the registry cannot see is a second posture by the back door). The tests below are about the ``[security]`` switches specifically, so the other three are pinned at shipped values here.""" - return security_loosenings(sec, StoreSettings(), AuthSettings(), AlertsSettings(), (), (), ()) + return security_loosenings( + sec, StoreSettings(), AuthSettings(), AlertsSettings(), (), (), (), None + ) SAMPLES_CONFIG = Path(__file__).resolve().parents[1] / "samples" / "config" diff --git a/tests/test_security_config.py b/tests/test_security_config.py index 9ad66726b..3190d6c80 100644 --- a/tests/test_security_config.py +++ b/tests/test_security_config.py @@ -34,7 +34,9 @@ def _loosenings(sec: SecuritySettings) -> list[tuple[str, str]]: The registry takes all four inputs as REQUIRED arguments deliberately (ADR 0148: one posture, and a deviation the registry cannot see is a second posture by the back door). The tests below are about the ``[security]`` switches specifically, so the other three are pinned at shipped values here.""" - return security_loosenings(sec, StoreSettings(), AuthSettings(), AlertsSettings(), (), (), ()) + return security_loosenings( + sec, StoreSettings(), AuthSettings(), AlertsSettings(), (), (), (), None + ) SAMPLES_CONFIG = Path(__file__).resolve().parents[1] / "samples" / "config" diff --git a/tests/test_security_posture_defaults.py b/tests/test_security_posture_defaults.py index 5a17beaee..a98456c6a 100644 --- a/tests/test_security_posture_defaults.py +++ b/tests/test_security_posture_defaults.py @@ -70,6 +70,7 @@ def _names( cleartext_hops, expiry_hops, db_hops, + None, ) ] @@ -103,6 +104,7 @@ def test_aad_bind_off_is_a_named_loosening() -> None: (), (), (), + None, ) ) assert "aad_bind" in named @@ -125,6 +127,7 @@ def test_aad_bind_loosening_names_its_no_op_caveat() -> None: (), (), (), + None, ) ) assert "no effect without a store key" in named["aad_bind"] @@ -136,7 +139,9 @@ def test_aad_bind_loosening_names_its_no_op_caveat() -> None: def test_recheck_zero_with_ad_enabled_is_a_named_loosening() -> None: auth = _ad(ad_session_recheck_seconds=0) named = dict( - security_loosenings(SecuritySettings(), StoreSettings(), auth, AlertsSettings(), (), (), ()) + security_loosenings( + SecuritySettings(), StoreSettings(), auth, AlertsSettings(), (), (), (), None + ) ) assert "ad_session_recheck_seconds" in named assert "revocation" in named["ad_session_recheck_seconds"] @@ -427,6 +432,7 @@ def test_cleartext_accepted_is_a_named_loosening() -> None: ("OB_LEGACY", "OB_LAB"), (), (), + None, ) ) assert "cleartext_accepted" in named @@ -460,6 +466,7 @@ def test_expiry_relaxation_is_a_named_loosening() -> None: (), ("OB_PARTNER_ADT", "OB_LAB_ORU"), (), + None, ) ) assert "tls_allow_expired" in named @@ -485,6 +492,7 @@ def test_generic_odbc_unenforced_tls_is_a_named_loosening() -> None: (), (), ("OB_PG_RESULTS", "inbound:IB_PG_ORDERS"), + None, ) ) assert "generic_odbc_tls_unenforced" in named @@ -768,6 +776,11 @@ def test_every_store_and_auth_bool_is_reported_or_exempt() -> None: # HARDENINGS at their non-default value (turning them ON tightens), so a flip is not a loosening. "require_encryption", "require_managed_identity", + # #1008: turning it ON adds a REFUSAL on an over-granted / unobservable store principal. The + # deviation it acts on IS reported — as the OBSERVATION passed in `store_privilege`, not as + # this switch — so it stays visible either way, and reporting the switch would make a + # hardening read as a weakening. + "require_least_privilege", # Security-relevant and gated ELSEWHERE, not by this registry. Extending it over them is real # work with its own SECURITY-LOOSENING.md entries — recorded as owed, not done silently. "encrypt", # the keyless-PHI serve gate refuses it in its own right diff --git a/tests/test_store_privilege_preflight.py b/tests/test_store_privilege_preflight.py new file mode 100644 index 000000000..883532e5d --- /dev/null +++ b/tests/test_store_privilege_preflight.py @@ -0,0 +1,774 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""BACKLOG #1008 (ASVS 13.2.2) — the startup preflight on the store principal's EFFECTIVE privileges. + +The defect, in the conditional this repo requires (MessageFoundry is a not-deployed beta, zero +instances): the engine DOCUMENTED a least-privilege store grant it could never observe, so on a first +deployment an over-granted store principal WOULD go unobserved. ``[store].require_managed_identity`` +does not close it — it constrains the credential's KIND, and a ``sysadmin`` gMSA satisfies it clean. + +Three properties are pinned here, and the third is the one a careless refactor breaks: + +1. **It sees an over-grant.** Both pure comparators name every privilege beyond the documented set. +2. **It does not false-alarm.** A correctly-granted principal produces an EMPTY excess list. A control + that only ever fires is indistinguishable from one that always fires. +3. **"Could not observe" is never "observed and fine".** A probe that raises, a store handle with no + probe at all, and SQLite's genuine non-applicability each produce a DISTINCT status with its own + wording — and under a declared ``require_least_privilege`` the unobservable case REFUSES, because a + declared refusal that passes a principal it could not read is the fail-open shape the setting was + turned on to prevent. + +**The live legs run against real servers** (``MEFOR_TEST_SQLSERVER`` / ``MEFOR_TEST_POSTGRES``) and +carry directions 1 and 2 end to end: they create a purpose-made least-privilege principal, connect AS +it, and assert an empty excess list — then over-grant it and assert the probe names the grant. A local +``pytest`` silently skips both legs, so a green local run proves the policy and the wiring, never the +SQL. CI's own store legs connect as ``sa`` / ``postgres``, which are over-granted by construction, so +the CI leg is itself a standing positive control that the probe fires on a real superuser. +""" + +from __future__ import annotations + +import contextlib +import json +import logging +import os +from pathlib import Path +from typing import Any + +import httpx +import pytest + +from messagefoundry.api import create_app +from messagefoundry.config.settings import ( + AlertsSettings, + AuthSettings, + SecuritySettings, + SqlAuth, + StoreBackend, + StorePrivilegePosture, + StorePrivilegeStatus, + StoreSettings, + security_loosenings, +) +from messagefoundry.pipeline import Engine +from messagefoundry.store import open_store, sqlite_settings +from messagefoundry.store.privilege import ( + SQLSERVER_DOCUMENTED_DATABASE_ROLES, + PostgresRoleFacts, + StorePrivilegeError, + StorePrivilegeReport, + postgres_excess, + run_store_privilege_preflight, + sqlserver_excess, +) + +_SQLSERVER_ON = bool(os.getenv("MEFOR_TEST_SQLSERVER")) +_POSTGRES_ON = bool(os.getenv("MEFOR_TEST_POSTGRES")) + +# The exact grant docs/DEPLOY-SERVER-DB.md §1.1 prescribes, restated here so a change to either side +# has to be a deliberate two-file edit rather than a silent drift in one. +_DOCUMENTED_SQLSERVER = ("db_datareader", "db_datawriter", "db_ddladmin") + + +# --- direction 2 first: the correctly-granted principal must be SILENT ------------------------ + + +def test_sqlserver_documented_grant_is_not_flagged() -> None: + """The three prescribed database roles, no server role, no direct CONTROL: nothing to report.""" + assert ( + sqlserver_excess( + server_roles=(), + database_roles=_DOCUMENTED_SQLSERVER, + control_server=False, + control_database=False, + database="MessageFoundry", + ) + == () + ) + + +def test_the_comparator_and_the_runbook_name_the_same_three_roles() -> None: + """The documented set is a CONSTANT the comparator reads, not a list retyped in two places.""" + assert set(_DOCUMENTED_SQLSERVER) == set(SQLSERVER_DOCUMENTED_DATABASE_ROLES) + + +def test_sqlserver_deny_roles_are_reported_but_never_called_excess() -> None: + """``db_deny*`` REMOVES access. Flagging a restriction as an over-grant would train an operator to + ignore the list, which is the one failure a posture control cannot afford.""" + assert ( + sqlserver_excess( + server_roles=(), + database_roles=(*_DOCUMENTED_SQLSERVER, "db_denydatawriter"), + control_server=False, + control_database=False, + database="MessageFoundry", + ) + == () + ) + + +def test_postgres_documented_grant_is_not_flagged() -> None: + """A plain LOGIN role with no attributes, no extra role membership, owning no database.""" + facts = (PostgresRoleFacts("mefor", True, False, False, False, False, False),) + assert ( + postgres_excess( + roles=facts, owns_database=False, create_on_database=False, database="mefor" + ) + == () + ) + + +# --- direction 1: an over-granted principal must be NAMED ------------------------------------- + + +def test_sqlserver_db_owner_is_named() -> None: + excess = sqlserver_excess( + server_roles=(), + database_roles=("db_owner",), + control_server=False, + control_database=True, + database="MessageFoundry", + ) + assert excess == ("database role db_owner",) + + +def test_sqlserver_sysadmin_is_named_and_the_implied_permission_is_not_restated() -> None: + """``sysadmin`` carries ``CONTROL SERVER`` and ``db_owner`` carries database ``CONTROL``. Listing + the implied permissions too would restate one grant as several and inflate the count an operator + reads.""" + excess = sqlserver_excess( + server_roles=("sysadmin",), + database_roles=("db_owner", *_DOCUMENTED_SQLSERVER), + control_server=True, + control_database=True, + database="MessageFoundry", + ) + assert excess == ("server role sysadmin", "database role db_owner") + + +def test_sqlserver_direct_control_grants_are_named_without_a_role() -> None: + """An over-grant made by ``GRANT CONTROL`` rather than by role membership is still seen — role + membership alone would miss it entirely.""" + excess = sqlserver_excess( + server_roles=(), + database_roles=_DOCUMENTED_SQLSERVER, + control_server=True, + control_database=True, + database="MessageFoundry", + ) + assert excess == ("CONTROL SERVER", "CONTROL on database MessageFoundry") + + +def test_sqlserver_user_defined_role_is_named() -> None: + """The closed fixed-role set cannot name a site's own role; the catalog enumeration adds it.""" + excess = sqlserver_excess( + server_roles=(), + database_roles=(*_DOCUMENTED_SQLSERVER, "app_admins"), + control_server=False, + control_database=False, + database="MessageFoundry", + ) + assert excess == ("database role app_admins",) + + +def test_postgres_superuser_is_named() -> None: + facts = (PostgresRoleFacts("postgres", True, True, True, True, True, True),) + excess = postgres_excess( + roles=facts, owns_database=True, create_on_database=True, database="mefor" + ) + assert excess[0] == "SUPERUSER" + assert "CREATEROLE" in excess + + +def test_postgres_superuser_does_not_enumerate_every_implied_role() -> None: + """A superuser is implicitly a member of every predefined role. Listing them would bury the one + finding that matters under a dozen restatements of it.""" + facts = ( + PostgresRoleFacts("postgres", True, True, False, False, False, False), + PostgresRoleFacts("pg_read_all_data", False, False, False, False, False, False), + PostgresRoleFacts("pg_execute_server_program", False, False, False, False, False, False), + ) + excess = postgres_excess( + roles=facts, owns_database=True, create_on_database=True, database="mefor" + ) + assert excess == ("SUPERUSER",) + + +def test_postgres_superuser_via_an_assumable_wrapper_role_is_caught() -> None: + """The principal itself has no attributes; a role it may assume is SUPERUSER. A denylist of role + NAMES cannot see this — reading attributes per assumable role can.""" + facts = ( + PostgresRoleFacts("mefor", True, False, False, False, False, False), + PostgresRoleFacts("site_dba", False, True, False, False, False, False), + ) + excess = postgres_excess( + roles=facts, owns_database=False, create_on_database=False, database="mefor" + ) + assert excess == ("SUPERUSER",) + + +def test_postgres_dangerous_predefined_roles_are_named() -> None: + facts = ( + PostgresRoleFacts("mefor", True, False, False, False, False, False), + PostgresRoleFacts("pg_write_all_data", False, False, False, False, False, False), + ) + excess = postgres_excess( + roles=facts, owns_database=False, create_on_database=False, database="mefor" + ) + assert excess == ("role pg_write_all_data",) + + +def test_postgres_database_ownership_is_named_and_suppresses_the_implied_create() -> None: + facts = (PostgresRoleFacts("mefor", True, False, False, False, False, False),) + assert postgres_excess( + roles=facts, owns_database=True, create_on_database=True, database="mefor" + ) == ("OWNER of database mefor",) + assert postgres_excess( + roles=facts, owns_database=False, create_on_database=True, database="mefor" + ) == ("CREATE on database mefor",) + + +# --- direction 3: the three non-observations must read differently ---------------------------- + + +class _FakeStore: + """A store handle exposing only what the preflight touches: a backend, a probe and an audit sink.""" + + def __init__(self, report: StorePrivilegeReport | None, *, raises: Exception | None = None): + self.backend = StoreBackend.SQLSERVER + self._report = report + self._raises = raises + self.audits: list[tuple[str, str | None]] = [] + + async def probe_principal_privileges(self) -> StorePrivilegeReport: + if self._raises is not None: + raise self._raises + assert self._report is not None + return self._report + + async def record_audit(self, action: str, *, actor: str | None, detail: str | None) -> None: + self.audits.append((action, detail)) + + +class _ProbelessStore: + """A store handle that implements NO privilege probe — structurally outside the protocol.""" + + def __init__(self) -> None: + self.backend = StoreBackend.POSTGRES + self.audits: list[tuple[str, str | None]] = [] + + async def record_audit(self, action: str, *, actor: str | None, detail: str | None) -> None: + self.audits.append((action, detail)) + + +def _clean(excess: tuple[str, ...] = ()) -> StorePrivilegeReport: + return StorePrivilegeReport( + backend=StoreBackend.SQLSERVER, + status=StorePrivilegeStatus.OBSERVED, + principal="CORP\\mefor-svc$", + database="MessageFoundry", + database_roles=_DOCUMENTED_SQLSERVER, + excess=excess, + ) + + +async def test_a_probe_that_raises_is_unobservable_not_clean() -> None: + store = _FakeStore(None, raises=RuntimeError("permission denied on sys.database_principals")) + report = await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=False, + enforcing=True, + ) + assert report.status is StorePrivilegeStatus.UNOBSERVABLE + assert report.excess == () + # The wording, not just the enum: this string is what an operator actually reads. + assert "COULD NOT OBSERVE" in report.summary() + assert "UNVERIFIED" in report.summary() + + +async def test_a_store_with_no_probe_is_unobservable_not_skipped() -> None: + """An unimplemented probe is a thing the engine could not observe. Narrowing it away silently + would make a whole backend invisible to this control while every output still read clean.""" + store = _ProbelessStore() + report = await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=False, + enforcing=True, + ) + assert report.status is StorePrivilegeStatus.UNOBSERVABLE + assert "implements no privilege probe" in report.detail + + +async def test_the_probe_secret_redacts_the_driver_message() -> None: + """A driver diagnostic can echo connection parameters, and this text lands in a durable audit row.""" + store = _FakeStore( + None, raises=RuntimeError("login failed; MEFOR_STORE_PASSWORD=hunter2swordfish") + ) + report = await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=False, + enforcing=False, + ) + assert "hunter2swordfish" not in report.detail + assert "hunter2swordfish" not in json.dumps(report.audit_detail()) + + +async def test_sqlite_is_not_applicable_and_says_what_it_did(tmp_path: Path) -> None: + """SQLite must not report OBSERVED-clean: that is a clean bill of health for a check that never + happened. It reports its own status and names the control that DOES govern access here.""" + store = await open_store(sqlite_settings(tmp_path / "p.db")) + try: + report = await run_store_privilege_preflight( + store, require_least_privilege=True, enforcing=True + ) + finally: + await store.close() + assert report.status is StorePrivilegeStatus.NOT_APPLICABLE + assert "filesystem ACL" in report.detail + assert "NOT APPLICABLE" in report.summary() + + +async def test_sqlite_never_refuses_even_under_a_declared_requirement(tmp_path: Path) -> None: + """There is genuinely no principal to over-grant, so refusing would block every single-node + install for a condition that cannot exist. Pinned so a later 'fail closed everywhere' edit reds.""" + store = await open_store(sqlite_settings(tmp_path / "p2.db")) + try: + await run_store_privilege_preflight(store, require_least_privilege=True, enforcing=True) + finally: + await store.close() + + +# --- the WARN arm ships ON; only the REFUSE arm is gated -------------------------------------- + + +async def test_an_over_grant_warns_loudly_on_the_shipped_defaults( + caplog: pytest.LogCaptureFixture, +) -> None: + """The shipped default must not be silent: default-off warning would leave the exact blind spot + this item exists to close.""" + store = _FakeStore(_clean(excess=("server role sysadmin",))) + with caplog.at_level(logging.WARNING, logger="messagefoundry.store.privilege"): + report = await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=False, + enforcing=True, + ) + assert report.excess == ("server role sysadmin",) + assert any("BEYOND the documented least-privilege grant" in r.message for r in caplog.records) + + +async def test_a_clean_observation_is_not_a_warning(caplog: pytest.LogCaptureFixture) -> None: + """The complementary arm — a control that warns on a correct grant becomes noise and stops being + read, which is indistinguishable from not shipping it.""" + store = _FakeStore(_clean()) + with caplog.at_level(logging.WARNING, logger="messagefoundry.store.privilege"): + await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=True, + enforcing=True, + ) + assert [r for r in caplog.records if r.levelno >= logging.WARNING] == [] + + +async def test_an_over_grant_does_not_refuse_on_the_shipped_defaults() -> None: + """Refusal is gated BY DESIGN: a preflight that refused by default could block a legitimate + deployment mid-setup, which is not this control's job.""" + store = _FakeStore(_clean(excess=("database role db_owner",))) + await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=False, + enforcing=True, + ) + + +async def test_an_over_grant_refuses_under_a_declared_requirement() -> None: + store = _FakeStore(_clean(excess=("database role db_owner",))) + with pytest.raises(StorePrivilegeError, match="db_owner"): + await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=True, + enforcing=True, + ) + + +async def test_the_declared_refusal_downgrades_under_enforcement_warn() -> None: + """The split reads [security].enforcement, exactly like the require_managed_identity gate it copies.""" + store = _FakeStore(_clean(excess=("database role db_owner",))) + report = await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=True, + enforcing=False, + ) + assert report.excess == ("database role db_owner",) + + +async def test_an_unobservable_probe_refuses_under_a_declared_requirement() -> None: + """THE fail-open test. An operator who declared require_least_privilege asked for a control; one + that passes a principal it could not read is not a control, it is a log line pretending to be one.""" + store = _FakeStore(None, raises=RuntimeError("permission denied")) + with pytest.raises(StorePrivilegeError, match="COULD NOT OBSERVE"): + await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=True, + enforcing=True, + ) + + +# --- the durable record ----------------------------------------------------------------------- + + +async def test_an_observation_writes_an_audit_row_carrying_the_status() -> None: + store = _FakeStore(_clean(excess=("server role sysadmin",))) + await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=False, + enforcing=True, + ) + assert len(store.audits) == 1 + action, detail = store.audits[0] + assert action == "store_privilege_preflight" + assert detail is not None + payload = json.loads(detail) + assert payload["status"] == "observed" + assert payload["excess"] == ["server role sysadmin"] + assert payload["refused"] is False + + +async def test_an_unobservable_probe_also_writes_the_row() -> None: + """A missing row would make the loudest condition the least durable one.""" + store = _FakeStore(None, raises=RuntimeError("permission denied")) + await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=False, + enforcing=True, + ) + assert json.loads(store.audits[0][1] or "{}")["status"] == "unobservable" + + +async def test_an_audit_failure_never_masks_the_refusal() -> None: + """Auditing is best-effort; the finding is not.""" + + class _AuditFails(_FakeStore): + async def record_audit(self, action: str, *, actor: str | None, detail: str | None) -> None: + raise RuntimeError("audit table unavailable") + + store = _AuditFails(_clean(excess=("database role db_owner",))) + with pytest.raises(StorePrivilegeError): + await run_store_privilege_preflight( + store, # type: ignore[arg-type] + require_least_privilege=True, + enforcing=True, + ) + + +# --- the posture registry --------------------------------------------------------------------- + + +def _names(store_privilege: StorePrivilegePosture | None) -> dict[str, str]: + return dict( + security_loosenings( + SecuritySettings(), + StoreSettings(), + AuthSettings(), + AlertsSettings(), + (), + (), + (), + store_privilege, + ) + ) + + +def test_registry_names_an_over_granted_principal() -> None: + named = _names( + StorePrivilegePosture( + status=StorePrivilegeStatus.OBSERVED, excess=("server role sysadmin",) + ) + ) + assert "store_principal_over_granted" in named + # The names, individually — a count would say "1 grant is excessive" without saying which, which + # is the shape that lets a grant nobody intended survive a posture review. + assert "server role sysadmin" in named["store_principal_over_granted"] + + +def test_registry_names_an_unobservable_probe_separately() -> None: + """Two entries, not one: an over-grant and an un-run probe demand different operator actions.""" + named = _names( + StorePrivilegePosture(status=StorePrivilegeStatus.UNOBSERVABLE, detail="permission denied") + ) + assert "store_principal_privileges_unobserved" in named + assert "store_principal_over_granted" not in named + assert "UNVERIFIED" in named["store_principal_privileges_unobserved"] + + +def test_registry_is_silent_on_a_clean_observation() -> None: + assert _names(StorePrivilegePosture(status=StorePrivilegeStatus.OBSERVED)) == {} + + +def test_registry_is_silent_on_sqlite() -> None: + assert _names(StorePrivilegePosture(status=StorePrivilegeStatus.NOT_APPLICABLE)) == {} + + +def test_registry_is_silent_when_no_probe_result_reached_it() -> None: + """`None` is "this call site has no probe result", and the CALLER declares that gap in its own + output (`security show`'s loosenings_scope, the posture route's `store_privilege` field). Rendering + it as an entry here would fire a finding on every graphless read.""" + assert _names(None) == {} + + +def test_the_refusal_switch_is_a_hardening_and_is_not_itself_a_loosening() -> None: + """``require_least_privilege`` at its non-default value TIGHTENS, so it must not appear — the + registry reports switches at their INSECURE value, and reporting a hardening would invert it. It + is exempted in tests/test_security_posture_defaults.py's [store] floor for exactly this reason.""" + assert ( + dict( + security_loosenings( + SecuritySettings(), + StoreSettings(require_least_privilege=True), + AuthSettings(), + AlertsSettings(), + (), + (), + (), + None, + ) + ) + == {} + ) + + +# --- the API read-out ------------------------------------------------------------------------- + + +@pytest.fixture +async def engine(tmp_path: Path): # type: ignore[no-untyped-def] + eng = await Engine.create(tmp_path / "priv.db", poll_interval=0.02) + yield eng + await eng.stop() + + +async def _posture(engine: Engine, **state: object) -> dict[str, Any]: + app = create_app(engine, allow_no_auth=True) + for key, value in state.items(): + setattr(app.state, key, value) + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://t") as client: + resp = await client.get("/security/posture") + assert resp.status_code == 200 + body: dict[str, Any] = resp.json() + return body + + +async def test_posture_route_says_not_probed_when_no_preflight_ran(engine: Engine) -> None: + """An app built without the managed lifespan never ran the probe. It must SAY so rather than omit + the field or render an empty/clean-looking value — silence here is the fail-open shape.""" + body = await _posture(engine) + assert body["store_privilege"]["status"] == "not_probed" + + +async def test_posture_route_reports_an_observed_over_grant(engine: Engine) -> None: + body = await _posture( + engine, + store_privilege=StorePrivilegePosture( + status=StorePrivilegeStatus.OBSERVED, excess=("server role sysadmin",) + ), + ) + assert body["store_privilege"]["status"] == "observed" + assert body["store_privilege"]["excess"] == ["server role sysadmin"] + switches = [entry["switch"] for entry in body["loosenings"]] + assert "store_principal_over_granted" in switches + + +async def test_posture_route_reports_an_unobservable_probe(engine: Engine) -> None: + body = await _posture( + engine, + store_privilege=StorePrivilegePosture( + status=StorePrivilegeStatus.UNOBSERVABLE, detail="permission denied" + ), + ) + assert body["store_privilege"]["status"] == "unobservable" + switches = [entry["switch"] for entry in body["loosenings"]] + assert "store_principal_privileges_unobserved" in switches + + +async def test_posture_route_distinguishes_clean_from_unobserved(engine: Engine) -> None: + """The property the whole design turns on, asserted at the surface an operator reads.""" + clean = await _posture( + engine, store_privilege=StorePrivilegePosture(status=StorePrivilegeStatus.OBSERVED) + ) + blind = await _posture( + engine, + store_privilege=StorePrivilegePosture( + status=StorePrivilegeStatus.UNOBSERVABLE, detail="permission denied" + ), + ) + assert clean["store_privilege"]["status"] != blind["store_privilege"]["status"] + assert clean["loosenings"] != blind["loosenings"] + + +def test_serve_lifespan_runs_the_preflight_and_stashes_a_real_observation(tmp_path: Path) -> None: + """The wiring, not the policy: a managed app must reach the route with the probe's OWN result, or + the field sits at `not_probed` forever and the whole preflight is invisible in the console. + + ``TestClient`` as a context manager is what drives the lifespan (the same idiom the rest of the + suite uses), and the lifespan is where the preflight runs.""" + from starlette.testclient import TestClient + + from messagefoundry.api.app import create_managed_app + + app = create_managed_app(store_settings=sqlite_settings(tmp_path / "managed.db")) + with TestClient(app) as tc: + resp = tc.get("/security/posture") + assert resp.status_code == 200 + # SQLite: NOT_APPLICABLE — the real probe result, provably not the `not_probed` default. + assert resp.json()["store_privilege"]["status"] == "not_applicable" + + +# --- live server legs (skipped locally; CI's store legs are the standing coverage) ------------- + + +@pytest.mark.skipif(not _SQLSERVER_ON, reason="set MEFOR_TEST_SQLSERVER=1 (+ MEFOR_STORE_* env)") +async def test_live_sqlserver_probe_observes_the_configured_principal() -> None: + """The probe runs against a real SQL Server and OBSERVES — never UNOBSERVABLE on a working store.""" + from messagefoundry.config.settings import load_settings + from messagefoundry.store.sqlserver import SqlServerStore + + store = await SqlServerStore.open(load_settings(environ=os.environ).store) + try: + report = await store.probe_principal_privileges() + finally: + await store.close() + assert report.status is StorePrivilegeStatus.OBSERVED + assert report.principal, "the probe must name the login it observed" + assert report.database + + +@pytest.mark.skipif(not _SQLSERVER_ON, reason="set MEFOR_TEST_SQLSERVER=1 (+ MEFOR_STORE_* env)") +async def test_live_sqlserver_probe_sees_both_directions_on_a_purpose_made_principal() -> None: + """Create a least-privilege login, connect AS it, assert an EMPTY excess list — then add + ``db_owner`` and assert the probe names it. Both directions against a real server, or neither. + + Skips (never fails) when the configured principal cannot create logins: the assertion is about the + probe, and a store credential without ``ALTER ANY LOGIN`` cannot set the fixture up. CI connects as + ``sa``, so the leg that matters always runs it.""" + from messagefoundry.config.settings import load_settings + from messagefoundry.store.sqlserver import SqlServerStore + + base = load_settings(environ=os.environ).store + login = "mefor_privprobe_test" + password = "Pr0be_Synth_2026!x" # synthetic, throwaway, dropped below — never a real credential + admin = await SqlServerStore.open(base) + try: + try: + await admin._execute( + f"IF SUSER_ID('{login}') IS NULL CREATE LOGIN {login} WITH PASSWORD='{password}'," + " CHECK_POLICY=OFF" + ) + await admin._execute( + f"IF USER_ID('{login}') IS NULL CREATE USER {login} FOR LOGIN {login}" + ) + for role in _DOCUMENTED_SQLSERVER: + await admin._execute(f"ALTER ROLE {role} ADD MEMBER {login}") + except Exception as exc: # noqa: BLE001 — a fixture-setup limit, not a probe failure + pytest.skip(f"cannot create a test login with this store principal: {exc}") + + least = base.model_copy( + update={"auth": SqlAuth.SQL, "username": login, "password": password} + ) + store = await SqlServerStore.open(least) + try: + clean = await store.probe_principal_privileges() + finally: + await store.close() + assert clean.status is StorePrivilegeStatus.OBSERVED + assert clean.excess == (), f"a correctly-granted login must be silent, got {clean.excess}" + assert set(_DOCUMENTED_SQLSERVER) <= set(clean.database_roles) + + await admin._execute(f"ALTER ROLE db_owner ADD MEMBER {login}") + store = await SqlServerStore.open(least) + try: + over = await store.probe_principal_privileges() + finally: + await store.close() + assert "database role db_owner" in over.excess + finally: + for stmt in ( + f"IF USER_ID('{login}') IS NOT NULL DROP USER {login}", + f"IF SUSER_ID('{login}') IS NOT NULL DROP LOGIN {login}", + ): + with contextlib.suppress(Exception): # teardown is best-effort + await admin._execute(stmt) + await admin.close() + + +@pytest.mark.skipif(not _POSTGRES_ON, reason="set MEFOR_TEST_POSTGRES=1 (+ MEFOR_STORE_* env)") +async def test_live_postgres_probe_observes_the_configured_principal() -> None: + from messagefoundry.config.settings import load_settings + from messagefoundry.store.postgres import PostgresStore + + store = await PostgresStore.open(load_settings(environ=os.environ).store) + try: + report = await store.probe_principal_privileges() + finally: + await store.close() + assert report.status is StorePrivilegeStatus.OBSERVED + assert report.principal + assert report.database + + +@pytest.mark.skipif(not _POSTGRES_ON, reason="set MEFOR_TEST_POSTGRES=1 (+ MEFOR_STORE_* env)") +async def test_live_postgres_probe_sees_both_directions_on_a_purpose_made_role() -> None: + """The Postgres twin: a plain LOGIN role must be silent, and one granted ``pg_read_all_data`` must + be named. Skips (never fails) when the configured role cannot CREATE ROLE.""" + from messagefoundry.config.settings import load_settings + from messagefoundry.store.postgres import PostgresStore + + base = load_settings(environ=os.environ).store + role = "mefor_privprobe_test" + password = "Pr0be_Synth_2026!x" # synthetic, throwaway, dropped below + admin = await PostgresStore.open(base) + try: + schema = base.db_schema or "public" + try: + await admin._execute(f"DROP ROLE IF EXISTS {role}") + await admin._execute(f"CREATE ROLE {role} LOGIN PASSWORD '{password}'") + # docs/DEPLOY-SERVER-DB.md §1.2 posture B (pre-created objects, engine role holds CRUD): + # CONNECT + USAGE + row CRUD + sequence USAGE, and nothing wider. The role must be able to + # OPEN the store, or the negative direction would skip for the wrong reason. + for stmt in ( + f"GRANT CONNECT ON DATABASE {base.database} TO {role}", + f"GRANT USAGE ON SCHEMA {schema} TO {role}", + f"GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA {schema} TO {role}", + f"GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA {schema} TO {role}", + ): + await admin._execute(stmt) + except Exception as exc: # noqa: BLE001 — a fixture-setup limit, not a probe failure + pytest.skip(f"cannot create a test role with this store principal: {exc}") + + least = base.model_copy(update={"username": role, "password": password}) + store = await PostgresStore.open(least) + try: + clean = await store.probe_principal_privileges() + finally: + await store.close() + assert clean.status is StorePrivilegeStatus.OBSERVED + assert clean.excess == (), f"a correctly-granted role must be silent, got {clean.excess}" + + await admin._execute(f"GRANT pg_read_all_data TO {role}") + store = await PostgresStore.open(least) + try: + over = await store.probe_principal_privileges() + finally: + await store.close() + assert "role pg_read_all_data" in over.excess + finally: + for stmt in ( + f"REVOKE ALL ON ALL SEQUENCES IN SCHEMA {base.db_schema or 'public'} FROM {role}", + f"REVOKE ALL ON ALL TABLES IN SCHEMA {base.db_schema or 'public'} FROM {role}", + f"REVOKE ALL ON SCHEMA {base.db_schema or 'public'} FROM {role}", + f"REVOKE ALL ON DATABASE {base.database} FROM {role}", + f"DROP ROLE IF EXISTS {role}", + ): + with contextlib.suppress(Exception): # teardown is best-effort + await admin._execute(stmt) + await admin.close() From 0e9fd86b8680daef8c25b5bdf04828498216db62 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 3 Sep 2026 15:22:58 -0500 Subject: [PATCH 2/7] feat(store): carry the preflight corrections and its live server-DB CI legs Carried from w3-store-privilege-preflight (d12b28de). Three things: the postgres_excess fix that names the WRAPPER role an attribute sits on, a dedicated live-DB pytest step per backend in ci.yml, and the seam bump. Four conflicts against 530 commits of drift, all resolved toward main: ci.yml path gate. Main's regex is far broader than the branch's. Kept main's and added the branch's one real addition, store_privilege, to the tests alternation. The store/ prefix already pulls the server-DB legs for privilege.py; the test-file arm matters when only the test changes. The seam trio (_ui_seam.py, webconsole __init__, golden snapshot). The branch hand-bumped ENGINE_UI_SEAM to 19. Main retired the hand-chosen number for a discovered digest under BACKLOG #1220, and its own seam comment names this branch as half of the reason. Took main's scheme and regenerated: the digest moves 93ba1f10b9dccfc8 to b93f38d097f97a45, the golden gains store_privilege on SecurityPosture plus a StorePrivilegeView row, and the console pin is set by hand as the tool requires. The branch's v19 seam comment is dropped; its content already lives on the model in api/models.py. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 51 ++++- CHANGELOG.md | 15 +- docs/DEPLOY-SERVER-DB.md | 32 ++- messagefoundry/api/_ui_seam.py | 2 +- messagefoundry/store/privilege.py | 47 +++-- messagefoundry_webconsole/__init__.py | 2 +- tests/golden/webconsole_seam.snapshot | 5 +- tests/test_store_privilege_preflight.py | 268 +++++++++++++++++++++--- 8 files changed, 373 insertions(+), 49 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ad60a6bdc..fe881665f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1494,7 +1494,7 @@ jobs: # are declared there (ADR 0013), and SQL Server / Postgres both declare supports_response_capture # + supports_pt_reingress True — so a regression in that surface is a SERVER-DB regression and must # pull these legs, not just the SQLite suite. - if echo "$changed" | grep -qE '^(messagefoundry/store/|messagefoundry/__main__|messagefoundry/api/app|messagefoundry/parsing/(__init__|binary|message|peek|x12/)|messagefoundry/pipeline/(__init__|alerts|cluster|config_convergence|dr|leader_tasks|phase_timing|sharding|stage_dispatcher|wiring_runner)|messagefoundry/config/(models|response|settings|wiring)|messagefoundry/transports/(__init__|base|database|dicomweb|fhir|http_auth|mllp|rest|soap|tcp|x12)|tests/test_(sqlserver|postgres|cluster|database_connector|database_source|pooled|stage_dispatcher|batch_claim|claim_fifo|inline_fast_path|seq_only_fifo|fifo_index|per_lane_wake|response_capture|reingress|x12_rte|shard_recovery|shard_cert|adr0071|adr0075|adr0114|adr0157|dr_server_seed_gate|dr7_server_config_only_backup|backup_runner_server_db|connscale|load_failover|load_runner)|\.github/workflows/ci\.yml)'; then + if echo "$changed" | grep -qE '^(messagefoundry/store/|messagefoundry/__main__|messagefoundry/api/app|messagefoundry/parsing/(__init__|binary|message|peek|x12/)|messagefoundry/pipeline/(__init__|alerts|cluster|config_convergence|dr|leader_tasks|phase_timing|sharding|stage_dispatcher|wiring_runner)|messagefoundry/config/(models|response|settings|wiring)|messagefoundry/transports/(__init__|base|database|dicomweb|fhir|http_auth|mllp|rest|soap|tcp|x12)|tests/test_(sqlserver|postgres|cluster|database_connector|database_source|pooled|stage_dispatcher|batch_claim|claim_fifo|inline_fast_path|seq_only_fifo|fifo_index|per_lane_wake|response_capture|reingress|x12_rte|shard_recovery|shard_cert|adr0071|adr0075|adr0114|adr0157|dr_server_seed_gate|dr7_server_config_only_backup|backup_runner_server_db|connscale|load_failover|load_runner|store_privilege)|\.github/workflows/ci\.yml)'; then echo "serverdb=true" >> "$GITHUB_OUTPUT" else echo "serverdb=false" >> "$GITHUB_OUTPUT" @@ -1990,6 +1990,34 @@ jobs: tests/test_backup_runner_server_db_sqlserver.py tests/test_dr7_server_config_only_backup_sqlserver.py + - name: Run the store-principal privilege preflight on real SQL Server + env: + MEFOR_TEST_SQLSERVER: "1" + MEFOR_STORE_BACKEND: sqlserver + MEFOR_STORE_SERVER: localhost + MEFOR_STORE_PORT: "1433" + MEFOR_STORE_DATABASE: MessageFoundry + MEFOR_STORE_AUTH: sql + MEFOR_STORE_USERNAME: sa + MEFOR_STORE_PASSWORD: "Str0ng_P@ssw0rd!" + MEFOR_STORE_TRUST_SERVER_CERTIFICATE: "true" + MEFOR_ALLOW_INSECURE_TLS: "1" + PYTHONFAULTHANDLER: "1" + # BACKLOG #1008 (ASVS 13.2.2). The preflight's POLICY is backend-free and runs on every leg; + # its SQL is not, and `IS_SRVROLEMEMBER` / `IS_ROLEMEMBER` / `HAS_PERMS_BY_NAME` execute + # against a real server or nowhere. This file gates those legs PER TEST rather than at module + # level (its SQLite cases must still run everywhere), so tests/test_serverdb_ci_coverage.py + # deliberately does not see it and would not have flagged its absence here — the file's own + # test_the_live_legs_of_this_file_are_run_by_a_server_db_ci_step does, and it was failed on + # purpose against this workflow before this step existed. + # This leg connects as `sa`, which is `sysadmin` by construction, so it is also the standing + # POSITIVE control: a probe that stopped seeing an over-grant reds here. + # Same pyodbc 5.3.0 + py3.14 native-crash retry as the steps above (upstream pyodbc#1459). + run: >- + bash scripts/ci/retry-native-crash.sh + pytest -v + tests/test_store_privilege_preflight.py + # Postgres store backend (Track B): run the gated store suite against a real PostgreSQL service # container (Linux, so 1x minutes). Runs NIGHTLY + on-demand (workflow_dispatch — use # `gh workflow run ci.yml --ref ` to exercise it on a feature branch) + on PRs that touch @@ -2179,6 +2207,27 @@ jobs: tests/test_backup_runner_server_db_postgres.py tests/test_dr7_server_config_only_backup_postgres.py + - name: Run the store-principal privilege preflight on real Postgres + env: + MEFOR_TEST_POSTGRES: "1" + MEFOR_STORE_BACKEND: postgres + MEFOR_STORE_SERVER: localhost + MEFOR_STORE_PORT: "5432" + MEFOR_STORE_DATABASE: messagefoundry + MEFOR_STORE_USERNAME: postgres + MEFOR_STORE_PASSWORD: mefor + MEFOR_STORE_ENCRYPT: "false" + MEFOR_ALLOW_INSECURE_TLS: "1" + # BACKLOG #1008 (ASVS 13.2.2), the Postgres twin of the SQL Server step — see its comment for + # why this file needs naming here even though tests/test_serverdb_ci_coverage.py does not flag + # it. `pg_has_role` / `rolsuper` / `has_database_privilege` execute against a real server or + # nowhere. This leg connects as `postgres`, a SUPERUSER owning the database, so it is the + # standing POSITIVE control; the both-directions case additionally creates a purpose-made + # least-privilege role and asserts the probe stays SILENT on it. + run: >- + pytest -v + tests/test_store_privilege_preflight.py + # Headless load test (Track B / throughput): serve the synthetic high-fan-out load config (auth # off, small fan-out) and drive the smoke profile through the real `python -m harness --load` CLI, # asserting zero message loss + all SLOs (exit 0) and uploading the JSON/CSV report. Skipped on PRs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9766d33fb..a9c17bdd2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,10 @@ All notable changes to MessageFoundry are documented here. The format follows therefore have gone unobserved. `serve` now reads fixed **server**-role and **database**-role membership plus `CONTROL SERVER` / database `CONTROL` on SQL Server, and role attributes (`SUPERUSER`, `CREATEROLE`, `CREATEDB`, `REPLICATION`, `BYPASSRLS`), assumable predefined roles and - database ownership on PostgreSQL — before any listener binds. + database ownership on PostgreSQL — before any listener binds. The PostgreSQL attributes are read + across **every role the principal may assume**, not only its own row: attributes are never + inherited, but a member may `SET ROLE` to the holder and exercise them, so a wrapper role carrying + `CREATEROLE` is named (`CREATEROLE via role site_ops`) instead of reading clean. **It observes and warns; it does not refuse by default** — refusing on an over-grant could block a legitimate deployment mid-setup, and the engine does not own the grant. Every start logs what it saw, writes a `store_privilege_preflight` audit row, and names each excess grant in @@ -32,6 +35,16 @@ All notable changes to MessageFoundry are documented here. The format follows there is the filesystem ACL. The PostgreSQL least-privilege grant is now documented ([`DEPLOY-SERVER-DB.md`](docs/DEPLOY-SERVER-DB.md) §1.2), which it previously was not. ([BACKLOG #1008](docs/BACKLOG.md)) + +### Changed +- **Web console engine UI seam `18` -> `19`.** `SecurityPosture` gained the additive `store_privilege` + object above. Additive with a default, so an older console ignores it; the seam still bumps because + the golden seam contract introspects that model's field set. +- **`DEPLOY-SERVER-DB.md` §1.2 posture B now states its prerequisite.** "A DBA pre-creates the objects" + is not sufficient on its own: the engine skips its DDL batch only when the `schema_meta` marker + records the current batch, and on PostgreSQL `CREATE TABLE IF NOT EXISTS` against an existing table + is still refused for a role holding only `USAGE` (the schema ACL is checked before the existence + skip, measured on 16.14). Bootstrap once with a DDL-capable principal, then hand over. - **`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 diff --git a/docs/DEPLOY-SERVER-DB.md b/docs/DEPLOY-SERVER-DB.md index d3acac14f..646eba356 100644 --- a/docs/DEPLOY-SERVER-DB.md +++ b/docs/DEPLOY-SERVER-DB.md @@ -159,6 +159,16 @@ GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA mefor TO mefor; GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA mefor TO mefor; -- the BIGSERIAL sequences ``` +> **Posture B has a prerequisite that "pre-create the objects" does not spell out: the `schema_meta` +> marker must already record the current DDL batch.** The engine skips its batch only on that marker +> (§2); with the marker absent or stale it runs `CREATE TABLE IF NOT EXISTS`, and on PostgreSQL that +> is **refused for a role holding only `USAGE`** — measured on 16.14, `CREATE TABLE IF NOT EXISTS` +> against an already-existing table fails with *permission denied for schema*, because the schema ACL +> is checked **before** the existence skip. `IF NOT EXISTS` does not rescue it. So hand-creating the +> tables is not enough: bootstrap by running the engine once as a DDL-capable principal (which writes +> the marker), then hand over to the `USAGE`-only role — and re-grant for the first start of any build +> whose schema moved, exactly as §2 says. If that sequencing is awkward, use posture A. + > **Why nothing wider, derived from the store rather than asserted.** The Postgres store issues > `CREATE TABLE IF NOT EXISTS` / `CREATE INDEX IF NOT EXISTS` inside its own schema, then only > `SELECT` / `INSERT` / `UPDATE` / `DELETE`. Its concurrency primitives are `pg_advisory_xact_lock` @@ -200,6 +210,16 @@ The grants in §1.1 and §1.2 used to be prescriptions the engine could not chec **never** rendered as a clean result. Under `require_least_privilege` an unobservable probe refuses, because a declared refusal that passed a principal it could not read would be a control in name only. +> **Two things it reports that a site may not expect, both by construction.** On SQL Server, a +> **user-defined database role** is named as excess even when it wraps exactly the three prescribed +> ones: the probe reads *membership*, not a role's contents, and it cannot expand a site role without +> catalog permission it deliberately does not depend on. Grant the three fixed roles directly, or +> accept the entry. On PostgreSQL, a role **attribute** is reported when it sits on any role the +> principal may assume, not only on the principal itself — attributes are never inherited, but a +> member may `SET ROLE` to the holder and exercise them, so the wrapper is named alongside the +> attribute (`CREATEROLE via role site_ops`). Neither is a false positive; both are grants beyond what +> §1.1/§1.2 prescribe. + --- ## 2. Schema bootstrap & evolution @@ -223,12 +243,12 @@ The grants in §1.1 and §1.2 used to be prescriptions the engine could not chec The engine login's grants are §1.1 — `db_datareader` + `db_datawriter` + `db_ddladmin`, and never `db_owner` or `sysadmin`. -> _Filled by staging:_ the **PostgreSQL** bootstrap role grants + the pre-create DDL per backend. The -> SQL Server set is settled in §1.1 and is **not** transferable: Postgres has no equivalent of SQL Server's fixed **database** roles -> (`db_datareader`/`db_datawriter`/`db_ddladmin`), -> the schema uses `BIGSERIAL` sequences rather than `IDENTITY`, and there is no stored-procedure path -> (`fifo_claim_proc` is SQL Server only), so the Postgres grants are a role/schema-ownership question -> this doc does not yet answer. +> **The PostgreSQL role grants are now answered in §1.2** — the two supported postures, why nothing +> wider, and posture B's marker prerequisite. The SQL Server set (§1.1) never transferred and does not +> now: Postgres has no equivalent of SQL Server's fixed **database** roles +> (`db_datareader`/`db_datawriter`/`db_ddladmin`), the schema uses `BIGSERIAL` sequences rather than +> `IDENTITY`, and there is no stored-procedure path (`fifo_claim_proc` is SQL Server only), which is +> why §1.2 answers it as a role-attribute / schema-ownership question instead. --- diff --git a/messagefoundry/api/_ui_seam.py b/messagefoundry/api/_ui_seam.py index c93585371..6cd141af4 100644 --- a/messagefoundry/api/_ui_seam.py +++ b/messagefoundry/api/_ui_seam.py @@ -134,7 +134,7 @@ #: proof is that commit 40a4d5d9 added a REQUIRED ``UploadedFileList.scope`` field the console renders #: unconditionally while touching no seam file at all. Regenerate with #: ``python scripts/webconsole_seam_snapshot.py --write``; never hand-edit it to silence a gate. -ENGINE_UI_SEAM: str = "93ba1f10b9dccfc8" +ENGINE_UI_SEAM: str = "b93f38d097f97a45" @dataclass(frozen=True, slots=True) diff --git a/messagefoundry/store/privilege.py b/messagefoundry/store/privilege.py index c4822ff3c..070b1fefc 100644 --- a/messagefoundry/store/privilege.py +++ b/messagefoundry/store/privilege.py @@ -155,9 +155,15 @@ def sqlserver_excess( class PostgresRoleFacts: """One role the store principal can assume, with the ATTRIBUTES that role carries. - Attributes are read per role rather than only for the principal itself so a user-defined wrapper - role that is itself ``SUPERUSER`` is caught by what it grants, not by whether its name happens to - be on a list. A denylist of role NAMES cannot see that; this can.""" + Attributes are read per role rather than only for the principal itself, so a user-defined wrapper + role is caught by what it grants rather than by whether its name happens to be on a list. A + denylist of role NAMES cannot see that; this can. + + **Why a role the principal is merely a MEMBER of still counts.** Measured on PostgreSQL 16.14: a + member of a ``CREATEROLE`` role is refused ``CREATE ROLE`` outright — attributes are never + inherited — and succeeds immediately after ``SET ROLE`` to that role. ``pg_has_role(current_user, + oid, 'MEMBER')``, the predicate the probe reads with, is exactly "may ``SET ROLE`` to it", so an + attribute on any assumable role is one the principal can exercise at will.""" name: str is_self: bool @@ -190,6 +196,16 @@ class PostgresRoleFacts: ) +def _attribute_finding(label: str, holders: Sequence[PostgresRoleFacts]) -> str: + """``LABEL`` when the principal carries the attribute on its own row, else ``LABEL via role x``. + + The wrapper is NAMED because it is the object an operator has to change: a bare ``CREATEROLE`` + against a principal whose own attributes are all clean sends them looking in the wrong place.""" + if any(r.is_self for r in holders): + return label + return f"{label} via role {', '.join(sorted(r.name for r in holders))}" + + def postgres_excess( *, roles: Sequence[PostgresRoleFacts], @@ -201,24 +217,31 @@ def postgres_excess( Pure, like :func:`sqlserver_excess`. + **Every attribute is read across every assumable role, not only the principal's own row** — see + :class:`PostgresRoleFacts` for the measurement that settles why membership is enough. Reading the + four non-superuser attributes on the principal alone let a wrapper role carrying ``CREATEROLE`` / + ``CREATEDB`` / ``REPLICATION`` / ``BYPASSRLS`` read as a clean least-privilege role. Mere + membership is still silent: a wrapper with no attribute and no ``pg_*`` name produces nothing, or + every site that groups its grants behind a role would carry a finding it cannot act on. + ``SUPERUSER`` short-circuits the rest: a superuser is implicitly a member of every role and holds every database privilege, so enumerating them would bury the one finding that matters under a - dozen restatements of it. Its own role attributes are still listed — they say *how* the identity - is configured, which is what an operator has to change.""" + dozen restatements of it. The role attributes are still listed — they say *how* the identity is + configured, which is what an operator has to change.""" out: list[str] = [] - superuser = any(r.superuser for r in roles) - if superuser: - out.append("SUPERUSER") - self_rows = [r for r in roles if r.is_self] + superusers = [r for r in roles if r.superuser] + if superusers: + out.append(_attribute_finding("SUPERUSER", superusers)) for attr, label in ( ("createrole", "CREATEROLE"), ("createdb", "CREATEDB"), ("replication", "REPLICATION"), ("bypassrls", "BYPASSRLS"), ): - if any(getattr(r, attr) for r in self_rows): - out.append(label) - if superuser: + holders = [r for r in roles if getattr(r, attr)] + if holders: + out.append(_attribute_finding(label, holders)) + if superusers: return tuple(out) for role in roles: if role.name in POSTGRES_EXCESSIVE_ROLES: diff --git a/messagefoundry_webconsole/__init__.py b/messagefoundry_webconsole/__init__.py index fc3c2b29f..cd747ac7f 100644 --- a/messagefoundry_webconsole/__init__.py +++ b/messagefoundry_webconsole/__init__.py @@ -45,7 +45,7 @@ # If cross-seam support is ever genuinely wanted, re-widen this set AND add the CI matrix that # installs the MIN and MAX supported engine builds — the claim and its test land together, or not # at all. -SUPPORTED_ENGINE_SEAMS: frozenset[str] = frozenset({"93ba1f10b9dccfc8"}) +SUPPORTED_ENGINE_SEAMS: frozenset[str] = frozenset({"b93f38d097f97a45"}) #: The vendored static assets shipped in THIS wheel (mounted at /ui/static by :func:`mount_ui`). STATIC_DIR = Path(__file__).parent / "static" diff --git a/tests/golden/webconsole_seam.snapshot b/tests/golden/webconsole_seam.snapshot index bc470218b..afb8aab9e 100644 --- a/tests/golden/webconsole_seam.snapshot +++ b/tests/golden/webconsole_seam.snapshot @@ -7,7 +7,7 @@ # by hand (BACKLOG #1220) - so a newly rendered DTO is covered with nobody editing a list. ## ENGINE_UI_SEAM -93ba1f10b9dccfc8 +b93f38d097f97a45 ## dataclass messagefoundry.api._ui_seam.UiDeps engine_seam @@ -214,10 +214,11 @@ messagefoundry.api.models.ReloadResult: dry_run, handlers, inbound, outbound, ro messagefoundry.api.models.SearchPresetCreateRequest: criteria, name messagefoundry.api.models.SearchPresetCriteria: channel_id, content, control_id, field_path, field_value, limit, message_type, status, target messagefoundry.api.models.SecurityLoosening: risk, switch -messagefoundry.api.models.SecurityPosture: allow_unencrypted_phi, backend, client_address_monoculture, client_denied_last, client_network_denials, data_class, encryption_enabled, enforcement, environment, fips_mode, kex_groups, key_id, key_source, loosenings, loosenings_scope, memory_encryption_note, memory_encryption_operator_declared, memory_encryption_readout_contradicts_declaration, memory_encryption_readout_source, memory_encryption_self_reported_active, memory_encryption_self_reported_capability, memory_encryption_self_reported_mechanism, openssl_version, plaintext_columns, production, require_encryption, security, synthetic_relaxation +messagefoundry.api.models.SecurityPosture: allow_unencrypted_phi, backend, client_address_monoculture, client_denied_last, client_network_denials, data_class, encryption_enabled, enforcement, environment, fips_mode, kex_groups, key_id, key_source, loosenings, loosenings_scope, memory_encryption_note, memory_encryption_operator_declared, memory_encryption_readout_contradicts_declaration, memory_encryption_readout_source, memory_encryption_self_reported_active, memory_encryption_self_reported_capability, memory_encryption_self_reported_mechanism, openssl_version, plaintext_columns, production, require_encryption, security, store_privilege, synthetic_relaxation messagefoundry.api.models.ServiceStatusInfo: enabled, service_name, state messagefoundry.api.models.StatsResetRequest: all, targets messagefoundry.api.models.StatsResetTarget: channel_id, destination, role +messagefoundry.api.models.StorePrivilegeView: detail, excess, status messagefoundry.api.models.SystemStatus: claim_proc, db, engine, kpis, logs, pool, update messagefoundry.api.models.UpdateInfo: current_version, pinned_version, update_available messagefoundry.api.models.UploadResendRequest: index, to diff --git a/tests/test_store_privilege_preflight.py b/tests/test_store_privilege_preflight.py index 883532e5d..2ed917094 100644 --- a/tests/test_store_privilege_preflight.py +++ b/tests/test_store_privilege_preflight.py @@ -22,8 +22,14 @@ carry directions 1 and 2 end to end: they create a purpose-made least-privilege principal, connect AS it, and assert an empty excess list — then over-grant it and assert the probe names the grant. A local ``pytest`` silently skips both legs, so a green local run proves the policy and the wiring, never the -SQL. CI's own store legs connect as ``sa`` / ``postgres``, which are over-granted by construction, so -the CI leg is itself a standing positive control that the probe fires on a real superuser. +SQL. + +Those legs connect as ``sa`` / ``postgres``, which are over-granted by construction, so each is also a +standing POSITIVE control that the probe still fires on a real superuser — but that is only true while +a workflow step actually runs this file, and per-test ``skipif`` gating puts it outside the scope +``tests/test_serverdb_ci_coverage.py`` polices. So the wiring is asserted here, by +``test_the_live_legs_of_this_file_are_run_by_a_server_db_ci_step``, which was failed on purpose +against the unedited workflow before it was trusted. """ from __future__ import annotations @@ -32,6 +38,8 @@ import json import logging import os +import re +import secrets from pathlib import Path from typing import Any @@ -65,11 +73,24 @@ _SQLSERVER_ON = bool(os.getenv("MEFOR_TEST_SQLSERVER")) _POSTGRES_ON = bool(os.getenv("MEFOR_TEST_POSTGRES")) +#: This file's own path as a ci.yml step spells it — read by the wiring guards further down. +_THIS_FILE = "tests/test_store_privilege_preflight.py" + # The exact grant docs/DEPLOY-SERVER-DB.md §1.1 prescribes, restated here so a change to either side # has to be a deliberate two-file edit rather than a silent drift in one. _DOCUMENTED_SQLSERVER = ("db_datareader", "db_datawriter", "db_ddladmin") +def _throwaway_password() -> str: + """A per-run password for the purpose-made live-leg principal, GENERATED rather than written down. + + Not a literal, on purpose. A hardcoded one would be a credential-shaped string in a public repo — + it would need a `.gitleaks.toml` allowlist entry, and every allowlist entry is a rule the scanner + stops applying. `token_urlsafe` is `[A-Za-z0-9_-]` only, so it never needs quoting inside the + fixture's SQL string literals, and the prefix keeps it complex enough for a password policy.""" + return "Px9_" + secrets.token_urlsafe(24) + + # --- direction 2 first: the correctly-granted principal must be SILENT ------------------------ @@ -196,7 +217,10 @@ def test_postgres_superuser_does_not_enumerate_every_implied_role() -> None: def test_postgres_superuser_via_an_assumable_wrapper_role_is_caught() -> None: """The principal itself has no attributes; a role it may assume is SUPERUSER. A denylist of role - NAMES cannot see this — reading attributes per assumable role can.""" + NAMES cannot see this — reading attributes per assumable role can. + + The wrapper is NAMED, because that role is the object an operator has to change; ``SUPERUSER`` + alone would send them looking at a principal whose own attributes are all clean.""" facts = ( PostgresRoleFacts("mefor", True, False, False, False, False, False), PostgresRoleFacts("site_dba", False, True, False, False, False, False), @@ -204,7 +228,44 @@ def test_postgres_superuser_via_an_assumable_wrapper_role_is_caught() -> None: excess = postgres_excess( roles=facts, owns_database=False, create_on_database=False, database="mefor" ) - assert excess == ("SUPERUSER",) + assert excess == ("SUPERUSER via role site_dba",) + + +def test_postgres_createrole_via_an_assumable_wrapper_role_is_caught() -> None: + """Every role ATTRIBUTE is reachable through membership, not only ``SUPERUSER`` — so every one of + them is read across the assumable roles, not just on the principal's own row. + + Measured on PostgreSQL 16.14 rather than assumed: a member of a ``CREATEROLE`` role is refused + ``CREATE ROLE`` outright (attributes are never inherited) and succeeds immediately after + ``SET ROLE`` to it. ``pg_has_role(current_user, oid, 'MEMBER')`` — the predicate the probe reads + with — is exactly "may SET ROLE to it", so a reachable attribute is a held one. Checking these four + on the principal's own row alone let a wrapper carrying ``CREATEROLE`` / ``CREATEDB`` / + ``REPLICATION`` / ``BYPASSRLS`` read as a clean least-privilege role.""" + facts = ( + PostgresRoleFacts("mefor", True, False, False, False, False, False), + PostgresRoleFacts("site_ops", False, False, True, True, False, False), + ) + excess = postgres_excess( + roles=facts, owns_database=False, create_on_database=False, database="mefor" + ) + assert excess == ("CREATEROLE via role site_ops", "CREATEDB via role site_ops") + + +def test_postgres_membership_in_a_plain_role_is_not_an_attribute_finding() -> None: + """The complementary arm of the test above: reading attributes across every assumable role must + not turn mere MEMBERSHIP into an attribute finding. A wrapper that carries no attribute and is not + a predefined ``pg_*`` role is silent — otherwise every site that groups grants behind a role would + get a permanent finding it cannot act on.""" + facts = ( + PostgresRoleFacts("mefor", True, False, False, False, False, False), + PostgresRoleFacts("app_readers", False, False, False, False, False, False), + ) + assert ( + postgres_excess( + roles=facts, owns_database=False, create_on_database=False, database="mefor" + ) + == () + ) def test_postgres_dangerous_predefined_roles_are_named() -> None: @@ -624,6 +685,117 @@ def test_serve_lifespan_runs_the_preflight_and_stashes_a_real_observation(tmp_pa assert resp.json()["store_privilege"]["status"] == "not_applicable" +def test_the_preflight_reads_the_PASSED_store_settings_not_the_ambient_environment( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A green that depends on the ambient environment is not a green. + + Every non-live test in this file asserts against a SQLite store it constructs explicitly, which is + only meaningful if a hostile ``MEFOR_STORE_*`` in the environment cannot reach in and change which + backend is probed. This runs the whole managed lifespan with the environment set to a SQL Server + the box cannot reach and ``require_least_privilege`` declared, and still expects the SQLite + ``not_applicable`` result — an ambient leak would instead try to open ``db.invalid`` and fail, or + refuse on an unobservable probe. + + **The second half is what stops this being vacuous.** It asserts the same environment WOULD have + produced a SQL Server store through ``load_settings``, so the hostile values are demonstrably ones + the code reads. A hostile-ambient test built on a variable nothing consults proves nothing, and + that is the shape it is guarding against.""" + from messagefoundry.api.app import create_managed_app + from messagefoundry.config.settings import load_settings + + hostile = { + "MEFOR_STORE_BACKEND": "sqlserver", + "MEFOR_STORE_SERVER": "db.invalid", + "MEFOR_STORE_DATABASE": "Hostile", + "MEFOR_STORE_USERNAME": "hostile", + "MEFOR_STORE_PASSWORD": "unused-by-this-test", + "MEFOR_STORE_REQUIRE_LEAST_PRIVILEGE": "true", + } + # The pin is load-bearing: these exact keys resolve to a SQL Server store with the refusal armed. + would_be = load_settings(environ=hostile).store + assert would_be.backend is StoreBackend.SQLSERVER + assert would_be.require_least_privilege is True + + from starlette.testclient import TestClient + + for key, value in hostile.items(): + monkeypatch.setenv(key, value) + app = create_managed_app(store_settings=sqlite_settings(tmp_path / "pinned.db")) + with TestClient(app) as tc: + resp = tc.get("/security/posture") + assert resp.status_code == 200 + assert resp.json()["store_privilege"]["status"] == "not_applicable" + + +# --- the live legs must actually be RUN somewhere, or they are decoration ---------------------- + + +def _ci_yml() -> str: + return (Path(__file__).resolve().parents[1] / ".github" / "workflows" / "ci.yml").read_text( + encoding="utf-8" + ) + + +def _steps_running_this_file(gate: str) -> list[str]: + """ci.yml step names that EXPORT ``gate`` and whose executable lines run this test file. + + Comment lines are stripped first, for the reason ``tests/test_serverdb_ci_coverage.py`` states: + a step's prose may name a file it does not run.""" + out: list[str] = [] + for block in re.split(r"\n - name:", _ci_yml()): + if f"{gate}: " not in block: + continue + executable = "\n".join( + line for line in block.splitlines() if not line.lstrip().startswith("#") + ) + if _THIS_FILE in executable: + out.append(block.splitlines()[0].strip()) + return out + + +@pytest.mark.parametrize("gate", ["MEFOR_TEST_SQLSERVER", "MEFOR_TEST_POSTGRES"]) +def test_the_live_legs_of_this_file_are_run_by_a_server_db_ci_step(gate: str) -> None: + """The live probe SQL executes NOWHERE unless a ci.yml step under ``gate`` names this file. + + ``tests/test_serverdb_ci_coverage.py`` does not cover this file and says so: it asserts the sharp + MODULE-gated invariant only, and this file gates its live legs PER TEST so its SQLite cases still + run. That leaves the exact hole it was built to close, one class over — the file collects on every + plain leg, its live legs report `skipped`, and `skipped` reads identical to `passed` at a glance. + Since the probe's SQL is the one thing no local run and no SQLite leg can exercise, a file nobody + wires means the statements in ``store/sqlserver.py`` and ``store/postgres.py`` are never once + executed. + + Falsified on purpose before it was trusted: with ci.yml unedited this parametrization failed for + BOTH gates, naming the file it scanned for. Scope is deliberately this one file — a general + version needs an allow-list, which is what keeps the sibling guard sharp.""" + steps = _steps_running_this_file(gate) + assert steps, ( + f"no ci.yml step exporting {gate} runs {_THIS_FILE}, so its live legs execute nowhere — " + f"scanned {len(_ci_yml().splitlines())} lines of .github/workflows/ci.yml. Add the file to " + "the sqlserver-store / postgres-store job and extend the `serverdb` change-detection " + "alternation so editing it pulls the leg that proves it." + ) + + +def test_editing_this_file_pulls_the_server_db_legs_that_prove_it() -> None: + """A file a leg runs but the change-detection alternation does not match is covered only by the + nightly cron — editing it does not pull the leg. Same invariant + ``test_serverdb_path_gate_admits_every_file_those_legs_run`` enforces, asserted here because that + test derives its file set from module-gated suites and never sees this one.""" + for line in _ci_yml().splitlines(): + if "grep -qE" in line and "tests/test_(" in line: + match = re.search(r"tests/test_\(([^)]*)\)", line) + assert match is not None + alternation = re.compile(rf"^test_({match.group(1)})") + assert alternation.match(Path(_THIS_FILE).stem), ( + f"the `serverdb` alternation does not match {_THIS_FILE}; it reads: " + f"tests/test_({match.group(1)})" + ) + return + pytest.fail("could not locate the `serverdb` change-detection alternation in ci.yml") + + # --- live server legs (skipped locally; CI's store legs are the standing coverage) ------------- @@ -633,7 +805,8 @@ async def test_live_sqlserver_probe_observes_the_configured_principal() -> None: from messagefoundry.config.settings import load_settings from messagefoundry.store.sqlserver import SqlServerStore - store = await SqlServerStore.open(load_settings(environ=os.environ).store) + settings = load_settings(environ=os.environ).store + store = await SqlServerStore.open(settings) try: report = await store.probe_principal_privileges() finally: @@ -641,6 +814,18 @@ async def test_live_sqlserver_probe_observes_the_configured_principal() -> None: assert report.status is StorePrivilegeStatus.OBSERVED assert report.principal, "the probe must name the login it observed" assert report.database + # THE STANDING POSITIVE CONTROL, and the reason this leg is worth its runtime. CI connects as + # `sa`, which is `sysadmin` by construction, so a probe that has quietly stopped SEEING an + # over-grant — a mis-bound parameter, a renamed column, a driver returning True/False where the + # comparison expects 1 — reds here instead of reporting a clean bill of health for a superuser. + # Gated on the CONFIGURED login name, which comes from the environment and not from the probe, so + # a site pointing this leg at a correctly least-privileged login is not failed for being correct. + if (settings.username or "").lower() == "sa": + assert "server role sysadmin" in report.excess, ( + "this leg is configured as `sa`, a sysadmin login, so the probe MUST name it; a clean " + f"result here means the probe is not observing what it claims to (server roles: " + f"{report.server_roles})" + ) @pytest.mark.skipif(not _SQLSERVER_ON, reason="set MEFOR_TEST_SQLSERVER=1 (+ MEFOR_STORE_* env)") @@ -656,7 +841,7 @@ async def test_live_sqlserver_probe_sees_both_directions_on_a_purpose_made_princ base = load_settings(environ=os.environ).store login = "mefor_privprobe_test" - password = "Pr0be_Synth_2026!x" # synthetic, throwaway, dropped below — never a real credential + password = _throwaway_password() admin = await SqlServerStore.open(base) try: try: @@ -706,7 +891,8 @@ async def test_live_postgres_probe_observes_the_configured_principal() -> None: from messagefoundry.config.settings import load_settings from messagefoundry.store.postgres import PostgresStore - store = await PostgresStore.open(load_settings(environ=os.environ).store) + settings = load_settings(environ=os.environ).store + store = await PostgresStore.open(settings) try: report = await store.probe_principal_privileges() finally: @@ -714,44 +900,65 @@ async def test_live_postgres_probe_observes_the_configured_principal() -> None: assert report.status is StorePrivilegeStatus.OBSERVED assert report.principal assert report.database + # The Postgres half of the standing POSITIVE control (see the SQL Server twin). This leg is + # configured as `postgres`, a SUPERUSER that also owns the database, so the probe must say so; + # gated on the configured role name, which comes from the environment and not from the probe. + if (settings.username or "").lower() == "postgres": + assert "SUPERUSER" in report.excess, ( + "this leg is configured as the `postgres` superuser, so the probe MUST name it; a clean " + f"result means it is not observing what it claims to (roles read: {report.database_roles})" + ) @pytest.mark.skipif(not _POSTGRES_ON, reason="set MEFOR_TEST_POSTGRES=1 (+ MEFOR_STORE_* env)") async def test_live_postgres_probe_sees_both_directions_on_a_purpose_made_role() -> None: - """The Postgres twin: a plain LOGIN role must be silent, and one granted ``pg_read_all_data`` must - be named. Skips (never fails) when the configured role cannot CREATE ROLE.""" + """The Postgres twin, and the one leg that carries BOTH directions end to end: a correctly-granted + role must be SILENT, and the same role granted ``pg_read_all_data`` must be NAMED. + + The fixture builds ``docs/DEPLOY-SERVER-DB.md`` §1.2 **posture A** — the engine role owns its own + schema — and that choice is a measurement, not a preference. Posture B (``USAGE`` on a pre-created + schema, no ``CREATE``) cannot open the store on a database whose ``schema_meta`` marker is absent + or stale: measured on PostgreSQL 16.14, ``CREATE TABLE IF NOT EXISTS`` on an ALREADY-EXISTING table + is refused with *permission denied for schema* — the schema ACL is checked BEFORE the existence + skip, so ``IF NOT EXISTS`` does not save it. A posture-B fixture would therefore have failed inside + ``PostgresStore.open`` and reported as a probe defect. Posture A is self-contained: the role + bootstraps its own schema, so this leg does not depend on any earlier CI step having seeded one. + + Schema ownership is deliberately NOT something ``postgres_excess`` looks at — it is the documented + posture, so an empty excess list here is the assertion that matters. + + Skips (never fails) when the configured role cannot CREATE ROLE: that is a limit of the fixture's + credential, not a fact about the probe.""" from messagefoundry.config.settings import load_settings from messagefoundry.store.postgres import PostgresStore base = load_settings(environ=os.environ).store role = "mefor_privprobe_test" - password = "Pr0be_Synth_2026!x" # synthetic, throwaway, dropped below + schema = "mefor_privprobe_test" + password = _throwaway_password() admin = await PostgresStore.open(base) try: - schema = base.db_schema or "public" try: + await admin._execute(f"DROP SCHEMA IF EXISTS {schema} CASCADE") await admin._execute(f"DROP ROLE IF EXISTS {role}") await admin._execute(f"CREATE ROLE {role} LOGIN PASSWORD '{password}'") - # docs/DEPLOY-SERVER-DB.md §1.2 posture B (pre-created objects, engine role holds CRUD): - # CONNECT + USAGE + row CRUD + sequence USAGE, and nothing wider. The role must be able to - # OPEN the store, or the negative direction would skip for the wrong reason. - for stmt in ( - f"GRANT CONNECT ON DATABASE {base.database} TO {role}", - f"GRANT USAGE ON SCHEMA {schema} TO {role}", - f"GRANT SELECT, INSERT, UPDATE, DELETE ON ALL TABLES IN SCHEMA {schema} TO {role}", - f"GRANT USAGE, SELECT ON ALL SEQUENCES IN SCHEMA {schema} TO {role}", - ): - await admin._execute(stmt) + await admin._execute(f"GRANT CONNECT ON DATABASE {base.database} TO {role}") + # Posture A, and NOTHING wider: no role attribute, no predefined pg_* membership, no + # database ownership. `CREATE SCHEMA ... AUTHORIZATION` is the runbook's own statement. + await admin._execute(f"CREATE SCHEMA {schema} AUTHORIZATION {role}") except Exception as exc: # noqa: BLE001 — a fixture-setup limit, not a probe failure pytest.skip(f"cannot create a test role with this store principal: {exc}") - least = base.model_copy(update={"username": role, "password": password}) + least = base.model_copy( + update={"username": role, "password": password, "db_schema": schema} + ) store = await PostgresStore.open(least) try: clean = await store.probe_principal_privileges() finally: await store.close() assert clean.status is StorePrivilegeStatus.OBSERVED + assert clean.principal == role assert clean.excess == (), f"a correctly-granted role must be silent, got {clean.excess}" await admin._execute(f"GRANT pg_read_all_data TO {role}") @@ -761,13 +968,24 @@ async def test_live_postgres_probe_sees_both_directions_on_a_purpose_made_role() finally: await store.close() assert "role pg_read_all_data" in over.excess + + # The attribute arm, on the SAME role: an attribute carried by a role the principal may merely + # assume is reachable via SET ROLE, so the probe must name it AND name the wrapper. This is the + # direction that read clean before the comparator was corrected. + await admin._execute(f"CREATE ROLE {role}_wrap CREATEROLE NOLOGIN") + await admin._execute(f"GRANT {role}_wrap TO {role}") + store = await PostgresStore.open(least) + try: + wrapped = await store.probe_principal_privileges() + finally: + await store.close() + assert f"CREATEROLE via role {role}_wrap" in wrapped.excess finally: for stmt in ( - f"REVOKE ALL ON ALL SEQUENCES IN SCHEMA {base.db_schema or 'public'} FROM {role}", - f"REVOKE ALL ON ALL TABLES IN SCHEMA {base.db_schema or 'public'} FROM {role}", - f"REVOKE ALL ON SCHEMA {base.db_schema or 'public'} FROM {role}", + f"DROP SCHEMA IF EXISTS {schema} CASCADE", f"REVOKE ALL ON DATABASE {base.database} FROM {role}", f"DROP ROLE IF EXISTS {role}", + f"DROP ROLE IF EXISTS {role}_wrap", ): with contextlib.suppress(Exception): # teardown is best-effort await admin._execute(stmt) From 682c09780e82dc433c161433637aef647cfd1e5d Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 3 Sep 2026 15:23:48 -0500 Subject: [PATCH 3/7] docs(store): link the two privilege findings that read like probe misfires Carried unchanged from w3-store-privilege-preflight (ffe1afee). A site reading store_principal_over_granted had nowhere to learn why a role it believes is correct was named, and the two cases where that happens are the ones a careful operator is most likely to hit. On SQL Server a user-defined database role is reported even when it wraps exactly the three prescribed fixed roles, because the probe reads membership rather than a role's contents. On PostgreSQL a role attribute is reported when it sits on any role the principal may assume, not only on its own row. Without the link the honest operator conclusion is that the probe is broken, which is how a posture control loses its readers. DEPLOY-SERVER-DB.md section 1.3 already carries the explanation, so this is the pointer to it and not a restatement. The original commit message carried a verification record taken against a 2026-08-11 baseline, 530 commits behind. It is dropped rather than repeated here, because re-asserting a stale run as if it covered this tree would be false. What ran on this carry is recorded in the PR body. Co-Authored-By: Claude Opus 5 --- docs/SECURITY-LOOSENING.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/SECURITY-LOOSENING.md b/docs/SECURITY-LOOSENING.md index e8bef4c0b..6233d629d 100644 --- a/docs/SECURITY-LOOSENING.md +++ b/docs/SECURITY-LOOSENING.md @@ -515,6 +515,12 @@ the call to the Console on 2026-09-02; the Console decided ([ADR 0118](adr/0118- ([`CONFIGURATION.md`](CONFIGURATION.md)). The refuse/warn split is `[security].enforcement`. - **`require_managed_identity` does NOT cover this.** It constrains the credential's *kind* — a `sysadmin` gMSA satisfies it clean. The two are orthogonal and a site needs both. +- **Before concluding it has misfired**, read [`DEPLOY-SERVER-DB.md` §1.3](DEPLOY-SERVER-DB.md): two + entries surprise sites that are trying to do the right thing. A SQL Server **user-defined** database + role is named even when it wraps exactly the three prescribed ones (the probe reads membership, not + a role's contents), and a PostgreSQL role **attribute** is named when it sits on any role the + principal may assume rather than on the principal itself (`CREATEROLE via role site_ops`) — reachable + by `SET ROLE`, so held in practice. Both are real deviations from the prescribed grant, not noise. ### `store_principal_privileges_unobserved` — the privilege posture could not be read From 0e90a20608099a4936bb8e861563149e6c274a21 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 3 Sep 2026 15:31:03 -0500 Subject: [PATCH 4/7] docs(backlog): #1234 is startable now that its subject exists on main (PR #764) The 2026-08-23 amendment named the clearing condition in its own words: that branch is rebased onto main with a PR opened. PR #764 does that, so the item moves from unstartable to startable. It does NOT close. The defect it names is unfixed and PR #764 is a carry rather than a fix, deliberately, because the two-arm test the item specifies is its own reviewable change. Three corrections to the record while I was in there: The 328-behind figure was right when written and is now 530. The branch kept drifting for the eleven days the row sat in the pool, which is the cost that amendment was warning about. The absence measurement is re-run with a stronger positive control. The 2026-08-23 reading established only that the instrument could find the word store; require_managed_identity across four .py files is the control that could actually have disagreed. The defect's HAS_PERMS_BY_NAME limb is named for the first time. The item's original scope covered the role columns only, so a fix reading just that scope would leave a direct GRANT CONTROL SERVER mis-read exactly as before. Co-Authored-By: Claude Opus 5 --- docs/BACKLOG.md | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index 603526405..5e9655c1c 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -11885,6 +11885,48 @@ preflight is re-implemented against current `main` as its own item. **Neither is **AND THE SCREEN THAT MISSED IT CANNOT SEE THIS BY CONSTRUCTION.** Verdict, closing-act, claim state, build commits and retirement markers all read the LEDGER. ***"Does the subject exist on main" is the only check that reads the CODE, and it is the one that decides startability.*** + +**AMENDMENT 2026-09-03, builder seat. THE CLEARING CONDITION IS MET -- THIS ITEM IS STARTABLE. IT IS +NOT CLOSED.** The amendment above named the exact condition: *"that branch is rebased onto `main` +with a PR opened"*. That has now happened, on the owner's ruling of 2026-09-03. **PR #764** carries +the preflight onto current `origin/main`, so the subject this item reports a defect in now exists +where a builder would branch from. + +***THE DEFECT IS UNCHANGED AND UNFIXED, WHICH IS WHY THIS ROW STAYS OPEN.*** PR #764 is a carry, not +a fix, and deliberately so: the two-arm test this item specifies is its own reviewable change, and +folding a behaviour fix into a 530-commit catch-up would make both unreviewable. **What moved is +startability, and only that.** + +**WHERE THE DEFECT SITS ON THAT BRANCH,** so the next builder does not re-find it. In +`messagefoundry/store/sqlserver.py`, inside `probe_principal_privileges` at `:2917`: `:2980` returns +`status=StorePrivilegeStatus.OBSERVED` unconditionally once the row is non-NULL, and `:2954`/`:2957` +fold a NULL role result to False via `== 1`, which reads as *"not a member"* and therefore as clean. +`:2988` and `:2989` fold `HAS_PERMS_BY_NAME` the same way -- **a limb this item's original scope did +not name**, so a fix that repairs only the role columns would leave a direct `GRANT CONTROL SERVER` +mis-read exactly as before. Sibling arms for comparison: `postgres.py:1245`, `store.py:4672`. +`[store].require_least_privilege` remains `False` at `config/settings.py:569`, so the WARN-is-the- +only-reachable-arm reading above still holds unchanged. + +**THE 328-BEHIND FIGURE ABOVE IS CORRECTED TO 530, AND THE DRIFT IS THE POINT.** Measured 2026-09-03 +at `origin/main`: `w3-store-privilege-preflight`, tip `94cb72e6`, dated 2026-08-11, is 4 ahead and +**530 behind**. It was 328 behind when the amendment above was written and kept moving for the +eleven days this row sat in the pool, which is the cost that amendment was warning about. The +absence measurement was re-run and still held at carry time: `require_least_privilege` and +`least_privilege` both return **zero** hits in any `.py` on `origin/main`. **Positive control, +same instrument and ref: `require_managed_identity` returns hits across four `.py` files** -- a +stronger control than the 2026-08-23 reading used, which established only that the instrument could +find the word `store`. + +**THE ORIGINAL BRANCH WAS NOT REBASED IN PLACE AND WAS NOT FORCE-PUSHED.** PR #764 cherry-picks onto +a NEW branch cut from current `main`. Three of the four commits were carried; the fourth is an empty +coordination record correcting a false claim in the second, and that correction was applied to the +carried commit message rather than carried as a commit of its own. +`w3-store-privilege-preflight` is untouched and remains the only copy of the pre-carry history. + +**PROVENANCE.** Written by a builder, which `CLAUDE.md` section 5 assigns: the Builder owns *"the PR +carrying the `BACKLOG.md` update"*. That supersedes the 2026-08-13 ruling cited at **#1235** that +builders may not author ledger content -- the pre-2026-09-01 method it belonged to was replaced. + ## 1235. a citation to an unallocated backlog number is a trap that arms itself the day the number is issued > ✅ **SHIPPED -- verified on main 2026-08-25.** The wiring limb landed at `b47c9fd9` (PR #560, sibling gate `backlog_citation_check.py` run in CI on documentation-only PRs). The header-falsification that kept this row must-not-close (2026-08-24 marker, below) is now corrected: `scripts/docs/dangling_citation_check.py`'s docstring on main states plainly, in its own words, "THIS SCRIPT IS RUN BY NO WORKFLOW AND NO PRE-COMMIT HOOK," distinguishes itself from the CI-wired sibling by name, and carries the wiring measurement with a positive control (this script: zero hits across `.github/` and `.pre-commit-config.yaml`; `backlog_status_check`/`ledger_check`/`scan_forbidden`: 3/2/3). That content is on main by a non-ancestor commit (`06427921`) whose CONTENT matches exactly -- confirmed by direct read, not cited as an ancestor. The marker's stated condition ("the closing PR falsified the detector's own header and left it uncorrected") has cleared; retiring it below rather than leaving a cleared condition standing as a live blocker. 🚧 **Re-scored 2026-08-20 -> P3.** Value **4/10** · Difficulty **2/10** · _fill-in_. The rule and both coverage residuals landed, leaving one gap: the detector runs only inside pytest, which is skipped on documentation-only pull requests, and a citation is introduced by editing prose. Wiring it as a workflow step with a paired must-trip and must-not-trip arm is a small additive change on an existing gate seam, and the coverage bound stands regardless since the detector cannot see the private companion repository where the filed instances live. _(was 6/10 · 2/10.)_ From 4cde805ce27f4bf05c36d86e027bd1071596d8a5 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 3 Sep 2026 15:33:06 -0500 Subject: [PATCH 5/7] docs(changelog): state the seam move as the digest main actually uses The carried entry claimed a seam bump 18 to 19. That was true on the 2026-08-11 branch and is false here: main retired the hand-chosen integer for a discovered digest under BACKLOG #1220, so this carry moves 93ba1f10b9dccfc8 to b93f38d097f97a45 and adds StorePrivilegeView to the discovered surface. A changelog line naming a version number that does not exist is the kind of false record a reader has no way to catch. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9c17bdd2..811e96702 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,9 +37,10 @@ All notable changes to MessageFoundry are documented here. The format follows ([BACKLOG #1008](docs/BACKLOG.md)) ### Changed -- **Web console engine UI seam `18` -> `19`.** `SecurityPosture` gained the additive `store_privilege` - object above. Additive with a default, so an older console ignores it; the seam still bumps because - the golden seam contract introspects that model's field set. +- **Web console engine UI seam `93ba1f10b9dccfc8` -> `b93f38d097f97a45`.** `SecurityPosture` gained the + additive `store_privilege` object above, and `StorePrivilegeView` joins the discovered surface. + Additive with a default, so an older console ignores it; the seam still moves because the golden seam + contract introspects that model's field set. - **`DEPLOY-SERVER-DB.md` §1.2 posture B now states its prerequisite.** "A DBA pre-creates the objects" is not sufficient on its own: the engine skips its DDL batch only when the `schema_meta` marker records the current batch, and on PostgreSQL `CREATE TABLE IF NOT EXISTS` against an existing table From a4225b0174e82ad84cfff1cf96dd5e343281a594 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Thu, 3 Sep 2026 15:36:17 -0500 Subject: [PATCH 6/7] ci: admit messagefoundry/api/__init__ to the server-DB path gate tests/test_serverdb_ci_coverage.py went red on this branch and it was right to. The carried test file is a server-DB suite and imports messagefoundry.api, and main's regex covered messagefoundry/api/app but not the package __init__. So editing that file would not have pulled the legs that prove it. Widened messagefoundry/api/app to messagefoundry/api/(__init__|app). The gate binds the ASSERTED-ON file rather than the asserting one, which is why a new suite can open a hole in it without touching the source that hole is about. Co-Authored-By: Claude Opus 5 --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fe881665f..ee4074890 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1494,7 +1494,7 @@ jobs: # are declared there (ADR 0013), and SQL Server / Postgres both declare supports_response_capture # + supports_pt_reingress True — so a regression in that surface is a SERVER-DB regression and must # pull these legs, not just the SQLite suite. - if echo "$changed" | grep -qE '^(messagefoundry/store/|messagefoundry/__main__|messagefoundry/api/app|messagefoundry/parsing/(__init__|binary|message|peek|x12/)|messagefoundry/pipeline/(__init__|alerts|cluster|config_convergence|dr|leader_tasks|phase_timing|sharding|stage_dispatcher|wiring_runner)|messagefoundry/config/(models|response|settings|wiring)|messagefoundry/transports/(__init__|base|database|dicomweb|fhir|http_auth|mllp|rest|soap|tcp|x12)|tests/test_(sqlserver|postgres|cluster|database_connector|database_source|pooled|stage_dispatcher|batch_claim|claim_fifo|inline_fast_path|seq_only_fifo|fifo_index|per_lane_wake|response_capture|reingress|x12_rte|shard_recovery|shard_cert|adr0071|adr0075|adr0114|adr0157|dr_server_seed_gate|dr7_server_config_only_backup|backup_runner_server_db|connscale|load_failover|load_runner|store_privilege)|\.github/workflows/ci\.yml)'; then + if echo "$changed" | grep -qE '^(messagefoundry/store/|messagefoundry/__main__|messagefoundry/api/(__init__|app)|messagefoundry/parsing/(__init__|binary|message|peek|x12/)|messagefoundry/pipeline/(__init__|alerts|cluster|config_convergence|dr|leader_tasks|phase_timing|sharding|stage_dispatcher|wiring_runner)|messagefoundry/config/(models|response|settings|wiring)|messagefoundry/transports/(__init__|base|database|dicomweb|fhir|http_auth|mllp|rest|soap|tcp|x12)|tests/test_(sqlserver|postgres|cluster|database_connector|database_source|pooled|stage_dispatcher|batch_claim|claim_fifo|inline_fast_path|seq_only_fifo|fifo_index|per_lane_wake|response_capture|reingress|x12_rte|shard_recovery|shard_cert|adr0071|adr0075|adr0114|adr0157|dr_server_seed_gate|dr7_server_config_only_backup|backup_runner_server_db|connscale|load_failover|load_runner|store_privilege)|\.github/workflows/ci\.yml)'; then echo "serverdb=true" >> "$GITHUB_OUTPUT" else echo "serverdb=false" >> "$GITHUB_OUTPUT" From 67202ea636630197ad84392752a7c429e73d3bd1 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Wed, 9 Sep 2026 09:02:01 -0500 Subject: [PATCH 7/7] fix(store): call security_loosenings with its real signature, and classify the posture detail Four blockers. Three are hand-fixed here; the fourth is cleared by merging main. security_loosenings takes nine positional parameters and secret_rotation is the fifth. Two test files called it with eight and no secret_rotation, so store_privilege bound to the unverified_db_hops slot and the call raised before it ran. tests/test_store_privilege_preflight.py now passes SecretRotationSettings() at slot five in both of its call sites. tests/test_store_key_calendar_expiry.py, written against the eight-parameter signature, gets the ninth argument None -- "no preflight ran", which this branch's own test_registry_is_silent_when_no_probe_result_reached_it pins as contributing nothing, so neither calendar-expiry assertion changes meaning. NOT by giving store_privilege a default. That was offered on the PR thread and it is wrong: with a default, secret_rotation binds to the () that follows it, and settings.py then raises AttributeError reading .enforce_store_key_expiry off a tuple. The shortcut moves the error rather than removing it. The deny-by-default PHI guard needed ('StorePrivilegeView', 'detail') classified. Traced rather than assumed: app.py copies the value from the in-memory StorePrivilegePosture the serve lifespan stashes, and all seven StorePrivilegeReport construction sites build detail from a fixed literal or from live process state -- an exception class name, a backend name, a driver message through redact_log_line capped at 300 characters. It projects no store column, so it is bound None, which that register defines as a claim about PROVENANCE and not a ruling on sensitivity. Merging origin/main clears the install-gate leg on its own: tests/test_install_gate_allowlist_merge.py now resolves to main's fixed blob 912143ca1 (#992), and its 32 tests pass. Co-Authored-By: Claude Opus 5 --- tests/test_no_store_phi_coverage.py | 6 ++++++ tests/test_store_key_calendar_expiry.py | 2 ++ tests/test_store_privilege_preflight.py | 3 +++ 3 files changed, 11 insertions(+) diff --git a/tests/test_no_store_phi_coverage.py b/tests/test_no_store_phi_coverage.py index dee506b28..172867bc1 100644 --- a/tests/test_no_store_phi_coverage.py +++ b/tests/test_no_store_phi_coverage.py @@ -152,6 +152,12 @@ def _classified_columns() -> dict[str, str]: ("PendingApprovalResponse", "detail"): None, # why the action is held for a second approver ("AlertTestEmailResult", "detail"): None, # a safe_exc-scrubbed SMTP send failure ("ConnectionTestResult", "detail"): None, # a reachability-probe outcome + # The store-privilege preflight's own outcome. app.py copies it from the in-memory + # StorePrivilegePosture the serve lifespan stashed, so the route reads no store row: per + # backend the string is a fixed literal, or a driver exception's class name plus str(exc) + # through redact_log_line and cut to 300 characters. Its PERSISTED twin is audit_log.detail + # (PL-4), not either of the PL-2 `detail` columns this field NAME collides with. + ("StorePrivilegeView", "detail"): None, ("AiPolicy", "reason"): None, # why the AI policy clamped, derived from config ("ConnectionMetadata", "metadata"): None, # the operator's own connections.toml label table # OPEN QUESTION, recorded on BACKLOG #1185 and deliberately NOT ruled here. These two carry a diff --git a/tests/test_store_key_calendar_expiry.py b/tests/test_store_key_calendar_expiry.py index 166e762bc..0bb81a3b1 100644 --- a/tests/test_store_key_calendar_expiry.py +++ b/tests/test_store_key_calendar_expiry.py @@ -238,6 +238,7 @@ def test_the_opt_out_is_a_NAMED_security_loosening() -> None: (), (), (), + None, ) ) assert "enforce_store_key_expiry" in named @@ -260,6 +261,7 @@ def test_the_shipped_default_is_not_reported_as_a_loosening() -> None: (), (), (), + None, ) ] assert named == [] diff --git a/tests/test_store_privilege_preflight.py b/tests/test_store_privilege_preflight.py index 2ed917094..6c05baf53 100644 --- a/tests/test_store_privilege_preflight.py +++ b/tests/test_store_privilege_preflight.py @@ -50,6 +50,7 @@ from messagefoundry.config.settings import ( AlertsSettings, AuthSettings, + SecretRotationSettings, SecuritySettings, SqlAuth, StoreBackend, @@ -532,6 +533,7 @@ def _names(store_privilege: StorePrivilegePosture | None) -> dict[str, str]: StoreSettings(), AuthSettings(), AlertsSettings(), + SecretRotationSettings(), (), (), (), @@ -588,6 +590,7 @@ def test_the_refusal_switch_is_a_hardening_and_is_not_itself_a_loosening() -> No StoreSettings(require_least_privilege=True), AuthSettings(), AlertsSettings(), + SecretRotationSettings(), (), (), (),