diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c582cd294..bb517fdad 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1613,7 +1613,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/(__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" @@ -2109,6 +2109,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 @@ -2298,6 +2326,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 af3ba139e..5095b1e13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,45 @@ 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. 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 + `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)) + +### Changed +- **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 + 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/BACKLOG.md b/docs/BACKLOG.md index 2f59d930a..3bce5cc69 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -14614,6 +14614,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.)_ diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 379c23762..1d05280d1 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -106,6 +106,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`. **It covers the STORE hop and nothing else** — the check is a `StoreSettings` method, so the graph's own `Database` / `DatabasePoll` / `DatabaseLookup` / `DatabaseRef` hops are outside its reach by construction and each defaults to a static SQL login; `messagefoundry check`'s advisory `static-db-credentials` line names that set (BACKLOG #1182, [`CONNECTIONS.md`](CONNECTIONS.md) §*Static database credentials*) | +| `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..646eba356 100644 --- a/docs/DEPLOY-SERVER-DB.md +++ b/docs/DEPLOY-SERVER-DB.md @@ -130,6 +130,96 @@ 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 +``` + +> **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` +> 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. + +> **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 @@ -153,12 +243,12 @@ require_managed_identity = true # refuse a static SQL login on production PHI 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/docs/SECURITY-LOOSENING.md b/docs/SECURITY-LOOSENING.md index 4ced2cc0f..8312d7257 100644 --- a/docs/SECURITY-LOOSENING.md +++ b/docs/SECURITY-LOOSENING.md @@ -520,6 +520,49 @@ 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. +- **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 + +> 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) @@ -553,6 +596,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 81060fa90..a1d66adea 100644 --- a/messagefoundry/__main__.py +++ b/messagefoundry/__main__.py @@ -1833,6 +1833,10 @@ 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, @@ -1842,6 +1846,7 @@ def _serve(args: argparse.Namespace) -> int: (), (), (), + None, ) if _loosenings: _seclog = logging.getLogger(__name__) @@ -1851,7 +1856,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), ) @@ -5557,13 +5563,16 @@ 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, _rotation, (), (), ()) + for s, r in security_loosenings( + sec, _store, _auth, _alerts, _rotation, (), (), (), None + ) ] #: Emitted alongside every loosening list this subcommand prints, so a reader can never mistake a @@ -5576,7 +5585,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/_ui_seam.py b/messagefoundry/api/_ui_seam.py index 708da9626..38686d988 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 = "7077b317cd4ec8c5" +ENGINE_UI_SEAM: str = "350e0ad5502dadf7" @dataclass(frozen=True, slots=True) diff --git a/messagefoundry/api/app.py b/messagefoundry/api/app.py index e1f3999bd..1f79d6d27 100644 --- a/messagefoundry/api/app.py +++ b/messagefoundry/api/app.py @@ -82,6 +82,7 @@ render_metrics, ) from messagefoundry.api.models import ( + STORE_PRIVILEGE_NOT_PROBED, AiChatRequest, AiChatResponse, AiPolicy, @@ -164,6 +165,7 @@ StatsResetRequest, StatsResetResult, StatsResponse, + StorePrivilegeView, SystemStatus, UpdateInfo, UploadDeleteResult, @@ -324,6 +326,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 ( @@ -1713,6 +1716,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( @@ -1724,8 +1732,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 " @@ -1777,6 +1795,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) @@ -6121,6 +6140,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 6c51aea04..1d96ea783 100644 --- a/messagefoundry/api/models.py +++ b/messagefoundry/api/models.py @@ -1074,6 +1074,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 @@ -1117,6 +1143,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 9cdd9af12..e661b8086 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 @@ -178,6 +179,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" @@ -520,6 +551,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 @@ -4455,6 +4503,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)``. @@ -4464,9 +4513,10 @@ 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), ``[secret_rotation].enforce_store_key_expiry`` (#1004), 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), ``[secret_rotation].enforce_store_key_expiry`` (#1004), 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``, @@ -4490,7 +4540,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 @@ -4768,6 +4828,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 58a5aa037..8d2e062bb 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, @@ -1261,6 +1267,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..070b1fefc --- /dev/null +++ b/messagefoundry/store/privilege.py @@ -0,0 +1,412 @@ +# 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 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 + 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 _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], + 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`. + + **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. The role attributes are still listed — they say *how* the identity is + configured, which is what an operator has to change.""" + out: list[str] = [] + 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"), + ): + 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: + 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 3280fea69..81b9d8183 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, @@ -2942,6 +2949,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 2d5afc8a7..30733c03e 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__) @@ -4749,6 +4750,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/messagefoundry_webconsole/__init__.py b/messagefoundry_webconsole/__init__.py index 6987b89dc..bb1427b8a 100644 --- a/messagefoundry_webconsole/__init__.py +++ b/messagefoundry_webconsole/__init__.py @@ -44,7 +44,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({"7077b317cd4ec8c5"}) +SUPPORTED_ENGINE_SEAMS: frozenset[str] = frozenset({"350e0ad5502dadf7"}) #: 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 9c7120c41..eb63e4f95 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 -7077b317cd4ec8c5 +350e0ad5502dadf7 ## dataclass messagefoundry.api._ui_seam.UiDeps engine_seam @@ -216,10 +216,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, log_sinks, logs, pool, update messagefoundry.api.models.UpdateInfo: current_version, pinned_version, update_available messagefoundry.api.models.UploadResendRequest: index, to diff --git a/tests/test_alert_smtp_tls.py b/tests/test_alert_smtp_tls.py index 785cc621f..ed1a204d4 100644 --- a/tests/test_alert_smtp_tls.py +++ b/tests/test_alert_smtp_tls.py @@ -205,7 +205,7 @@ def _names(**kw: Any) -> list[str]: return [ n for n, _ in security_loosenings( - sec, StoreSettings(), AuthSettings(), alerts, SecretRotationSettings(), (), (), () + sec, StoreSettings(), AuthSettings(), alerts, SecretRotationSettings(), (), (), (), None ) ] @@ -250,6 +250,7 @@ def test_an_unconfigured_alert_transport_reports_no_hop_deviation() -> None: (), (), (), + 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 8dc143b2e..ab9800c62 100644 --- a/tests/test_client_network_allowlist.py +++ b/tests/test_client_network_allowlist.py @@ -52,7 +52,15 @@ def _loosenings(sec: SecuritySettings) -> list[tuple[str, str]]: 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(), SecretRotationSettings(), (), (), () + sec, + StoreSettings(), + AuthSettings(), + AlertsSettings(), + SecretRotationSettings(), + (), + (), + (), + None, ) diff --git a/tests/test_memory_encryption_readout.py b/tests/test_memory_encryption_readout.py index 17f0523fa..cad18fbdc 100644 --- a/tests/test_memory_encryption_readout.py +++ b/tests/test_memory_encryption_readout.py @@ -59,7 +59,15 @@ def _loosenings(sec: SecuritySettings) -> list[tuple[str, str]]: 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(), SecretRotationSettings(), (), (), () + sec, + StoreSettings(), + AuthSettings(), + AlertsSettings(), + SecretRotationSettings(), + (), + (), + (), + None, ) 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_security_config.py b/tests/test_security_config.py index e991350c7..c756c7805 100644 --- a/tests/test_security_config.py +++ b/tests/test_security_config.py @@ -36,7 +36,15 @@ def _loosenings(sec: SecuritySettings) -> list[tuple[str, str]]: 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(), SecretRotationSettings(), (), (), () + sec, + StoreSettings(), + AuthSettings(), + AlertsSettings(), + SecretRotationSettings(), + (), + (), + (), + None, ) diff --git a/tests/test_security_posture_defaults.py b/tests/test_security_posture_defaults.py index ff4b2c8af..151065eac 100644 --- a/tests/test_security_posture_defaults.py +++ b/tests/test_security_posture_defaults.py @@ -73,6 +73,7 @@ def _names( cleartext_hops, expiry_hops, db_hops, + None, ) ] @@ -107,6 +108,7 @@ def test_aad_bind_off_is_a_named_loosening() -> None: (), (), (), + None, ) ) assert "aad_bind" in named @@ -130,6 +132,7 @@ def test_aad_bind_loosening_names_its_no_op_caveat() -> None: (), (), (), + None, ) ) assert "no effect without a store key" in named["aad_bind"] @@ -150,6 +153,7 @@ def test_recheck_zero_with_ad_enabled_is_a_named_loosening() -> None: (), (), (), + None, ) ) assert "ad_session_recheck_seconds" in named @@ -442,6 +446,7 @@ def test_cleartext_accepted_is_a_named_loosening() -> None: ("OB_LEGACY", "OB_LAB"), (), (), + None, ) ) assert "cleartext_accepted" in named @@ -476,6 +481,7 @@ def test_expiry_relaxation_is_a_named_loosening() -> None: (), ("OB_PARTNER_ADT", "OB_LAB_ORU"), (), + None, ) ) assert "tls_allow_expired" in named @@ -502,6 +508,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 @@ -785,6 +792,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_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 new file mode 100644 index 000000000..6c05baf53 --- /dev/null +++ b/tests/test_store_privilege_preflight.py @@ -0,0 +1,995 @@ +# 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. + +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 + +import contextlib +import json +import logging +import os +import re +import secrets +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, + SecretRotationSettings, + 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")) + +#: 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 ------------------------ + + +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. + + 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), + ) + excess = postgres_excess( + roles=facts, owns_database=False, create_on_database=False, database="mefor" + ) + 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: + 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(), + SecretRotationSettings(), + (), + (), + (), + 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(), + SecretRotationSettings(), + (), + (), + (), + 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" + + +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) ------------- + + +@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 + + settings = load_settings(environ=os.environ).store + store = await SqlServerStore.open(settings) + 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 + # 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)") +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 = _throwaway_password() + 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 + + settings = load_settings(environ=os.environ).store + store = await PostgresStore.open(settings) + try: + report = await store.probe_principal_privileges() + finally: + await store.close() + 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, 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" + schema = "mefor_privprobe_test" + password = _throwaway_password() + admin = await PostgresStore.open(base) + try: + 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}'") + 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, "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}") + 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 + + # 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"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) + await admin.close()