Scope the read path by tenant (AUDIT-WP-0008-T04).
_read gated on may_read alone and never called permits_tenant, so any reader credential could read every tenant through /v1/events, /v1/events/<id>, /v1/dead-letters and /v1/secret-findings. Deployment bounded the exposure -- the only production sender holds may_read: false -- but the boundary was not in the code, which is the difference between E2 and E1 on the tenancy posture enforcement ladder. Two rules, because the surfaces divide cleanly. Event reads are filtered to the tenants the credential may act for. Surfaces with no tenant key to filter on -- stats, integrity, dead letters, secret findings -- require full tenant scope and are refused rather than served instance-wide facts to a scoped reader. A cross-tenant fetch returns 404 rather than 403. A distinguishable forbidden would confirm that an event id exists and which tenant holds it, turning the read surface into an existence oracle. Correlation lookup is filtered rather than refused, since a correlation id legitimately spans tenants. _readable_by fails closed: a record with no tenant is readable only at full scope. Three existing tests read instance-wide surfaces with a scoped credential, which this makes a 403; bound_app now carries an unrestricted operator identity and those reads use it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
26defd9bec
commit
aaa2b4c50b
6 changed files with 196 additions and 13 deletions
|
|
@ -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 |
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
|
|
|
|||
|
|
@ -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``.
|
||||
|
||||
|
|
|
|||
|
|
@ -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/<id>` | 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" \
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
```
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue