diff --git a/WORK-RECORDS.md b/WORK-RECORDS.md index 45f7701..4cba5f5 100644 --- a/WORK-RECORDS.md +++ b/WORK-RECORDS.md @@ -15,6 +15,7 @@ | workplan | AUDIT-WP-0005 | finished | — | workplans/AUDIT-WP-0005-postgres-store-and-production-deployment.md | | workplan | AUDIT-WP-0006 | finished | — | workplans/AUDIT-WP-0006-honest-custody-and-canon-join.md | | workplan | AUDIT-WP-0007 | finished | — | workplans/AUDIT-WP-0007-integrity-verification.md | +| workplan | AUDIT-WP-0008 | ready | — | workplans/AUDIT-WP-0008-tenancy-posture-alignment.md | | task | AUDIT-WP-0001-T01 | done | — | workplans/AUDIT-WP-0001-statehub-bootstrap.md | | task | AUDIT-WP-0001-T02 | done | — | workplans/AUDIT-WP-0001-statehub-bootstrap.md | | task | AUDIT-WP-0001-T03 | done | — | workplans/AUDIT-WP-0001-statehub-bootstrap.md | @@ -46,3 +47,9 @@ | task | AUDIT-WP-0007-T03 | done | — | workplans/AUDIT-WP-0007-integrity-verification.md | | task | AUDIT-WP-0007-T04 | done | — | workplans/AUDIT-WP-0007-integrity-verification.md | | task | AUDIT-WP-0007-T05 | done | — | workplans/AUDIT-WP-0007-integrity-verification.md | +| task | AUDIT-WP-0008-T01 | todo | — | workplans/AUDIT-WP-0008-tenancy-posture-alignment.md | +| task | AUDIT-WP-0008-T02 | todo | — | workplans/AUDIT-WP-0008-tenancy-posture-alignment.md | +| task | AUDIT-WP-0008-T03 | todo | — | workplans/AUDIT-WP-0008-tenancy-posture-alignment.md | +| task | AUDIT-WP-0008-T04 | todo | — | workplans/AUDIT-WP-0008-tenancy-posture-alignment.md | +| task | AUDIT-WP-0008-T05 | todo | — | workplans/AUDIT-WP-0008-tenancy-posture-alignment.md | +| task | AUDIT-WP-0008-T06 | todo | — | workplans/AUDIT-WP-0008-tenancy-posture-alignment.md | diff --git a/audit_core/ingestion.py b/audit_core/ingestion.py index 074ab62..b39d02a 100644 --- a/audit_core/ingestion.py +++ b/audit_core/ingestion.py @@ -224,16 +224,39 @@ class IngestionApplication: ) def _read(self, start_response, environ, path: str, identity): - """Operator read surface (AUDIT-WP-0004-T05). + """Operator read surface (AUDIT-WP-0004-T05, AUDIT-WP-0008-T04). Read is a distinct privilege from write: a sender credential must not be able to read the audit trail back. + + Read is also tenant-scoped. Until AUDIT-WP-0008-T04 this method gated + on ``may_read`` alone and never consulted ``permits_tenant``, so any + reader could read every tenant. Deployment bounded the exposure — the + production sender holds ``may_read: false`` — but the boundary was not + in the code, which is the difference between E2 and E1 in the tenancy + posture (framework §4.3). + + Two rules, because the surfaces divide cleanly: + + * Event reads are filtered to the tenants the credential may act for. + * Surfaces that are not tenant-keyed — chain verification, counters, + dead letters, secret findings — require full tenant scope. They + cannot be filtered, so a scoped reader is refused rather than served + instance-wide facts. """ if not identity.may_read: return self._json(start_response, HTTPStatus.FORBIDDEN, {"error": "read_forbidden"}) query = parse_qs(environ.get("QUERY_STRING", "")) try: + if path in _UNSCOPED_READ_PATHS and not identity.has_full_tenant_scope(): + # Refused rather than filtered: these carry no tenant key, so + # there is nothing to filter on and serving them to a scoped + # reader would leak across the boundary this method enforces. + return self._json( + start_response, HTTPStatus.FORBIDDEN, + {"error": "full_tenant_scope_required"}, + ) if path == "/v1/dead-letters": return self._json( start_response, HTTPStatus.OK, @@ -261,12 +284,22 @@ class IngestionApplication: start_response, HTTPStatus.BAD_REQUEST, {"error": "correlation_id_required"}, ) - return self._json( - start_response, HTTPStatus.OK, - {"events": self.backend.by_correlation(correlation, _limit(query))}, - ) + # A correlation id deliberately spans services, so a single + # correlation can carry events for more than one tenant. + # Filtering here rather than refusing keeps the surface useful + # to a scoped reader without widening what it sees. + events = [ + event for event in self.backend.by_correlation(correlation, _limit(query)) + if _readable_by(identity, event) + ] + return self._json(start_response, HTTPStatus.OK, {"events": events}) event_id = path[len("/v1/events/"):] record = self.backend.get(event_id) if event_id else None + if record is not None and not _readable_by(identity, record): + # 404, not 403. A distinguishable "forbidden" would confirm + # that an event id exists and which tenant holds it, turning + # the read surface into an existence oracle. + record = None if record is None: return self._json(start_response, HTTPStatus.NOT_FOUND, {"error": "not_found"}) return self._json(start_response, HTTPStatus.OK, record) @@ -431,6 +464,32 @@ def _normalize_timestamp(value: Any) -> str: return parsed.astimezone(timezone.utc).isoformat() +# Read surfaces with no tenant key to filter on. Serving these to a scoped +# reader would cross the boundary _read enforces, so they require full scope +# (AUDIT-WP-0008-T04). +_UNSCOPED_READ_PATHS = frozenset({ + "/v1/dead-letters", + "/v1/stats", + "/v1/secret-findings", + "/v1/integrity", +}) + + +def _readable_by(identity, record: dict) -> bool: + """Whether ``identity`` may read ``record``. + + Fails closed: a record carrying no tenant is readable only by an identity + with full scope. A stored event always has one — ``_validate`` requires it + — so reaching that branch means the row predates the requirement or was + written by something other than the ingest path, and neither is a case to + resolve in favour of the reader. + """ + if identity.has_full_tenant_scope(): + return True + tenant = record.get("tenant") if isinstance(record, dict) else None + return bool(tenant) and identity.permits_tenant(str(tenant)) + + def _limit(query: dict[str, list[str]], default: int = 100, ceiling: int = 1000) -> int: try: return max(1, min(int((query.get("limit") or [default])[0]), ceiling)) diff --git a/audit_core/senders.py b/audit_core/senders.py index a3fe186..26fc877 100644 --- a/audit_core/senders.py +++ b/audit_core/senders.py @@ -56,6 +56,16 @@ class SenderIdentity: def permits_source(self, source: str) -> bool: return WILDCARD in self.sources or source in self.sources + def has_full_tenant_scope(self) -> bool: + """Whether this identity is unrestricted across tenants. + + The read surfaces that are not tenant-keyed — chain verification, + counters, dead letters, secret findings — cannot be filtered, so they + are reserved for an identity that could read every tenant anyway. + Anything else would hand a scoped reader instance-wide facts. + """ + return WILDCARD in self.tenants + def permits_tenant(self, tenant: str) -> bool: """Whether this identity may write for ``tenant``. diff --git a/docs/operator-runbook.md b/docs/operator-runbook.md index 5f5c6d7..ae87b82 100644 --- a/docs/operator-runbook.md +++ b/docs/operator-runbook.md @@ -30,8 +30,8 @@ warden route show database-dynamic-credentials --json | --- | --- | | `GET /healthz` | Process is up. Liveness uses this. A database outage must **not** restart the pod. | | `GET /readyz` | Custody is reachable and `custody_class=operational`. Also reports `recoverable_days` and `tamper_evidence`. Readiness uses this; the pod leaves the Service rather than accept events it cannot store. | -| `GET /v1/stats` | In-process counters since start (`accepted`, `duplicate`, `conflict`, `rejected`, `unauthorized`, `forbidden`, `unavailable`, `error`). Resets on restart. Requires `may_read`. | -| `GET /v1/integrity` | Hash-chain walk: `{intact, events, head, first_break}`. No payloads. Requires `may_read`. A break is a custody defect, not a sender retry. | +| `GET /v1/stats` | In-process counters since start (`accepted`, `duplicate`, `conflict`, `rejected`, `unauthorized`, `forbidden`, `unavailable`, `error`). Resets on restart. Requires `may_read` and full tenant scope. | +| `GET /v1/integrity` | Hash-chain walk: `{intact, events, head, first_break}`. No payloads. Requires `may_read` and full tenant scope. A break is a custody defect, not a sender retry. | A missing `AUDIT_CORE_DATABASE_URL` / credential directory is a startup failure (`AUDIT_CORE_REQUIRE_CUSTODY_CLASS=operational`), not a silent @@ -61,6 +61,20 @@ again. Reboot unreadiness is unrelated. All read routes require a sender identity with `may_read: true`. A write credential must not be able to read the trail back. +**Read is also tenant-scoped** (AUDIT-WP-0008-T04). A reader sees only the +tenants its identity may act for, and the two rules differ by surface: + +| Surface | Behaviour for a scoped reader | +|---|---| +| `GET /v1/events/` | Another tenant's event returns **404**, identical to an id that does not exist. A distinguishable 403 would confirm the event exists and whose it is. | +| `GET /v1/events?correlation_id=` | Filtered to the readable slice. A correlation id legitimately spans services and tenants, so this is filtered rather than refused. | +| `/v1/stats`, `/v1/integrity`, `/v1/dead-letters`, `/v1/secret-findings` | **403 `full_tenant_scope_required`.** These carry no tenant key, so there is nothing to filter on; they are reserved for an identity with `tenants: ["*"]`. | + +An operator identity holding `tenants: ["*"]` is unaffected and sees everything, +which is what `$OPERATOR_TOKEN` below assumes. A reader scoped to one tenant is +the case this protects: before T04, `may_read` was the only gate and any reader +could read every tenant. + ```bash # One event curl -sS -H "Authorization: Bearer $OPERATOR_TOKEN" \ diff --git a/tests/test_ingestion.py b/tests/test_ingestion.py index b3ec072..753e62d 100644 --- a/tests/test_ingestion.py +++ b/tests/test_ingestion.py @@ -240,8 +240,21 @@ def bound_app(tmp_path, **kw): may_read=kw.get("may_read", False), secret_policy=kw.get("secret_policy", "redact"), ) + # An unrestricted operator sits alongside the scoped sender. The + # instance-wide read surfaces — stats, dead letters, secret findings, + # integrity — carry no tenant key and so require full scope + # (AUDIT-WP-0008-T04); reading them as the scoped sender is what that task + # made a 403. + operator = SenderIdentity( + name="operator", + tokens=("operator",), + sources=frozenset({"user-engine"}), + tenants=frozenset({"*"}), + may_write=False, + may_read=True, + ) backend = SQLiteAuditBackend(str(tmp_path / "bound.db")) - return IngestionApplication(backend, SenderRegistry([identity])), backend + return IngestionApplication(backend, SenderRegistry([identity, operator])), backend def test_credential_may_not_claim_another_tenant(tmp_path): @@ -332,7 +345,7 @@ def test_secret_rejection_withholds_the_payload(tmp_path): material would write that material into the audit store.""" app, _ = bound_app(tmp_path, secret_policy="reject", may_read=True) assert invoke(app, event(data={"password": "hunter2"}))[0].startswith("400") - _, body = invoke(app, None, path="/v1/dead-letters", method="GET", body=b"") + _, body = invoke(app, None, path="/v1/dead-letters", method="GET", body=b"", token="operator") entry = body["dead_letters"][0] assert entry["reason"] == "secret_shaped_field" assert entry["payload_withheld"] is True @@ -385,7 +398,7 @@ def test_findings_are_counted_by_path_for_both_outcomes(tmp_path): strict, _ = bound_app(tmp_path, secret_policy="reject") invoke(strict, event(id="r1", data={"auth_token": "x"}), key="r1") - _, body = invoke(app, None, path="/v1/secret-findings", method="GET", body=b"") + _, body = invoke(app, None, path="/v1/secret-findings", method="GET", body=b"", token="operator") rows = {(r["outcome"], r["field_path"]): r for r in body["secret_findings"]} assert rows[("redacted", "data.auth_token")]["occurrences"] == 3 assert rows[("redacted", "data.auth_token")]["action"] == "membership.added" @@ -485,7 +498,7 @@ def test_counters_track_each_outcome(tmp_path): invoke(app, event(id="e2", tenant="tenant:coulomb"), key="e2") # rejected invoke(app, event(), token="nope") # unauthorized - _, body = invoke(app, None, path="/v1/stats", method="GET", body=b"") + _, body = invoke(app, None, path="/v1/stats", method="GET", body=b"", token="operator") counts = body["counts"] assert counts["accepted"] == 1 assert counts["duplicate"] == 1 @@ -499,3 +512,83 @@ def test_stats_require_the_read_privilege(tmp_path): app, _ = bound_app(tmp_path, may_read=False) status, _ = invoke(app, None, path="/v1/stats", method="GET", body=b"") assert status.startswith("403") + + +# --- read is tenant-scoped (AUDIT-WP-0008-T04) ------------------------------ + +def two_tenant_app(tmp_path): + """A seeded store, plus a reader scoped to one of the two tenants. + + The writer is unrestricted so both tenants exist; the reader is bound to + ``binky`` only. Both applications share one backend, which is the point — + the boundary has to hold in the read path, not in the store. + """ + backend = SQLiteAuditBackend(str(tmp_path / "scoped.db")) + writer = SenderIdentity( + name="seeder", tokens=("seed",), sources=frozenset({"user-engine"}), + tenants=frozenset({"*"}), may_read=True, + ) + reader = SenderIdentity( + name="scoped-reader", tokens=("scoped",), sources=frozenset({"user-engine"}), + tenants=frozenset({"tenant:friendly:binky"}), may_read=True, + ) + seed = IngestionApplication(backend, SenderRegistry([writer])) + assert invoke(seed, event(), token="seed")[0].startswith("202") + assert invoke( + seed, event(id="evt-2", tenant="tenant:coulomb"), key="evt-2", token="seed" + )[0].startswith("202") + return IngestionApplication(backend, SenderRegistry([reader])), seed + + +def test_scoped_reader_cannot_fetch_another_tenants_event(tmp_path): + """The defect AUDIT-WP-0008 found: may_read was the only gate.""" + scoped, _ = two_tenant_app(tmp_path) + + status, body = invoke(scoped, None, path="/v1/events/evt-1", method="GET", + body=b"", token="scoped") + assert status.startswith("200") + assert body["tenant"] == "tenant:friendly:binky" + + status, body = invoke(scoped, None, path="/v1/events/evt-2", method="GET", + body=b"", token="scoped") + assert status.startswith("404") + assert body["error"] == "not_found" + + +def test_cross_tenant_refusal_is_indistinguishable_from_absence(tmp_path): + """403 here would confirm the event exists and is someone else's.""" + scoped, _ = two_tenant_app(tmp_path) + present = invoke(scoped, None, path="/v1/events/evt-2", method="GET", + body=b"", token="scoped") + absent = invoke(scoped, None, path="/v1/events/evt-nope", method="GET", + body=b"", token="scoped") + assert present == absent + + +def test_correlation_lookup_is_filtered_not_refused(tmp_path): + """One correlation legitimately spans tenants; serve the readable slice.""" + scoped, seed = two_tenant_app(tmp_path) + + status, body = invoke_query(scoped, "correlation_id=corr-1", token="scoped") + assert status.startswith("200") + assert {e["event_id"] for e in body["events"]} == {"evt-1"} + + status, body = invoke_query(seed, "correlation_id=corr-1", token="seed") + assert {e["event_id"] for e in body["events"]} == {"evt-1", "evt-2"} + + +@pytest.mark.parametrize( + "path", ["/v1/dead-letters", "/v1/stats", "/v1/secret-findings", "/v1/integrity"] +) +def test_unscoped_surfaces_require_full_tenant_scope(tmp_path, path): + """Not tenant-keyed, so they cannot be filtered — refuse instead.""" + scoped, seed = two_tenant_app(tmp_path) + + status, body = invoke(scoped, None, path=path, method="GET", body=b"", + token="scoped", key=None) + assert status.startswith("403") + assert body["error"] == "full_tenant_scope_required" + + status, _ = invoke(seed, None, path=path, method="GET", body=b"", + token="seed", key=None) + assert status.startswith("200") diff --git a/workplans/AUDIT-WP-0008-tenancy-posture-alignment.md b/workplans/AUDIT-WP-0008-tenancy-posture-alignment.md index f987a0b..2d0945a 100644 --- a/workplans/AUDIT-WP-0008-tenancy-posture-alignment.md +++ b/workplans/AUDIT-WP-0008-tenancy-posture-alignment.md @@ -266,7 +266,7 @@ surface alongside `recoverable_days`. ```task id: AUDIT-WP-0008-T02 -status: todo +status: done priority: high state_hub_task_id: "224a12c0-d646-4c7f-9215-120e1ede4691" ``` @@ -303,7 +303,7 @@ with the dependency open; it is not publishable with the dependency hidden. ```task id: AUDIT-WP-0008-T04 -status: todo +status: done priority: high state_hub_task_id: "e4440db3-6eb0-49f6-9aa9-930db8be9d18" ```