Redact secret-shaped fields by default, countable per field path
AUDIT-WP-0004-T04, closing the workplan. Decision (Bernd): default to redaction, allow rejection per sender. Losing an audit record over one field is worse than storing it masked, but a higher-assurance channel must be able to refuse rather than mask. secret_policy is set per sender identity in AUDIT_CORE_SENDERS and defaults to redact. Detection now covers the whole payload at any depth, including lists, rather than only the top level of data. Under redaction the value is masked and the key is preserved: dropping the key would hide that the sender transmitted the field at all, which is exactly what an operator needs in order to stop it. The stored record carries details.redaction with policy and affected paths, so a reader never has to infer whether what they see is what was sent. Idempotency is unaffected - the payload hash is taken over the original request body, so redaction is deterministic and a resubmission still reconciles as a duplicate. Both outcomes are counted durably by sender, source, action and field path, exposed at GET /v1/secret-findings. Per-path aggregation is the point: the actionable unit is "stop emitting data.auth.token on membership.added", not "there were 47 redactions". Counters survive restart because the fix they drive lives in another service. Contract doc updated to match. Tests 46 -> 50. WP-0004 is finished. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
0ad526c2d8
commit
576caa2665
8 changed files with 441 additions and 29 deletions
|
|
@ -23,10 +23,10 @@
|
|||
| task | AUDIT-WP-0003-T04 | cancel | — | workplans/AUDIT-WP-0003-user-engine-event-ingestion-service.md |
|
||||
| task | AUDIT-WP-0004-T01 | done | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0004-T02 | done | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0004-T03 | todo | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0004-T03 | done | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0004-T04 | todo | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0004-T05 | todo | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0004-T06 | todo | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0004-T05 | done | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0004-T06 | done | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0004-T07 | done | — | workplans/AUDIT-WP-0004-receiver-correctness-and-hardening.md |
|
||||
| task | AUDIT-WP-0005-T01 | todo | — | workplans/AUDIT-WP-0005-postgres-store-and-production-deployment.md |
|
||||
| task | AUDIT-WP-0005-T02 | todo | — | workplans/AUDIT-WP-0005-postgres-store-and-production-deployment.md |
|
||||
|
|
|
|||
|
|
@ -38,11 +38,16 @@ from audit_core.interface import (
|
|||
EventValidationError,
|
||||
IdempotentAuditBackend,
|
||||
)
|
||||
from audit_core.redaction import (
|
||||
POLICY_REDACT,
|
||||
SecretFieldRejection,
|
||||
apply_policy,
|
||||
finding_from_path,
|
||||
)
|
||||
from audit_core.senders import SenderIdentity, SenderRegistry, development_registry
|
||||
from audit_core.sqlite_backend import SQLiteAuditBackend
|
||||
|
||||
MAX_BODY_BYTES = 256 * 1024
|
||||
_SECRET_FRAGMENTS = ("password", "secret", "token", "credential", "private_key")
|
||||
|
||||
log = logging.getLogger("audit_core.ingestion")
|
||||
|
||||
|
|
@ -99,7 +104,10 @@ class IngestionApplication:
|
|||
start_response, HTTPStatus.UNAUTHORIZED, {"error": "unauthorized"}
|
||||
)
|
||||
|
||||
if method == "GET" and (path.startswith("/v1/events") or path == "/v1/dead-letters"):
|
||||
if method == "GET" and (
|
||||
path.startswith("/v1/events")
|
||||
or path in ("/v1/dead-letters", "/v1/secret-findings")
|
||||
):
|
||||
return self._read(start_response, environ, path, identity)
|
||||
|
||||
if path != "/v1/events" or method != "POST":
|
||||
|
|
@ -109,15 +117,26 @@ class IngestionApplication:
|
|||
return self._json(start_response, HTTPStatus.FORBIDDEN, {"error": "write_forbidden"})
|
||||
|
||||
raw = b""
|
||||
payload: Any = {}
|
||||
try:
|
||||
raw = self._read_body(environ)
|
||||
event = normalize(
|
||||
json.loads(raw), environ.get("HTTP_IDEMPOTENCY_KEY"), identity
|
||||
)
|
||||
payload = json.loads(raw)
|
||||
event = normalize(payload, environ.get("HTTP_IDEMPOTENCY_KEY"), identity)
|
||||
except SecretFieldRejection as exc:
|
||||
self._count_secrets(payload, identity, "rejected", exc.findings)
|
||||
self._dead_letter(raw, str(exc), identity)
|
||||
return self._json(start_response, HTTPStatus.BAD_REQUEST, {"error": str(exc)})
|
||||
except (ValueError, TypeError, KeyError, json.JSONDecodeError) as exc:
|
||||
self._dead_letter(raw, str(exc), identity)
|
||||
return self._json(start_response, HTTPStatus.BAD_REQUEST, {"error": str(exc)})
|
||||
|
||||
redaction = event.details.get("redaction")
|
||||
if redaction:
|
||||
self._count_secrets(
|
||||
payload, identity, "redacted",
|
||||
[finding_from_path(p) for p in redaction["paths"]],
|
||||
)
|
||||
|
||||
try:
|
||||
result = self.backend.accept(event, hashlib.sha256(raw).hexdigest())
|
||||
except EventConflictError as exc:
|
||||
|
|
@ -156,6 +175,11 @@ class IngestionApplication:
|
|||
start_response, HTTPStatus.OK,
|
||||
{"dead_letters": self.backend.dead_letters(_limit(query))},
|
||||
)
|
||||
if path == "/v1/secret-findings":
|
||||
return self._json(
|
||||
start_response, HTTPStatus.OK,
|
||||
{"secret_findings": self.backend.secret_findings(_limit(query))},
|
||||
)
|
||||
if path == "/v1/events":
|
||||
correlation = (query.get("correlation_id") or [""])[0]
|
||||
if not correlation:
|
||||
|
|
@ -178,6 +202,28 @@ class IngestionApplication:
|
|||
start_response, HTTPStatus.SERVICE_UNAVAILABLE, {"error": "backend_unavailable"}
|
||||
)
|
||||
|
||||
def _count_secrets(self, payload: Any, identity, outcome: str, findings) -> None:
|
||||
"""Count secret-shaped fields by path, sender, source and action.
|
||||
|
||||
Counted so the sending service can be fixed. Aggregation is by field
|
||||
path rather than by event, because the actionable unit is "stop
|
||||
emitting ``data.auth.token`` on ``membership.added``", not "there were
|
||||
47 redactions".
|
||||
"""
|
||||
counter = getattr(self.backend, "count_secret_findings", None)
|
||||
if not callable(counter) or not findings:
|
||||
return
|
||||
source = str(payload.get("source") or "") if isinstance(payload, dict) else ""
|
||||
action = str(payload.get("type") or "") if isinstance(payload, dict) else ""
|
||||
try:
|
||||
counter(
|
||||
sender=identity.name, source=source, action=action,
|
||||
outcome=outcome, findings=findings,
|
||||
)
|
||||
except BackendUnavailableError as exc:
|
||||
# A missed counter must never change the event's outcome.
|
||||
log.error("could not count secret findings: %s", exc)
|
||||
|
||||
def _dead_letter(self, raw: bytes, reason: str, identity) -> None:
|
||||
"""Record a rejection so an operator can see what the sender dropped."""
|
||||
recorder = getattr(self.backend, "record_rejection", None)
|
||||
|
|
@ -268,8 +314,21 @@ def normalize(
|
|||
elif source != "user-engine":
|
||||
raise ValueError("source_not_allowed")
|
||||
observed_at = _normalize_timestamp(payload["occurred_at"])
|
||||
if _contains_secret(payload["data"]):
|
||||
raise ValueError("secret_shaped_field")
|
||||
policy = identity.secret_policy if identity is not None else POLICY_REDACT
|
||||
# Raises SecretFieldRejection under the reject policy; that exception
|
||||
# carries the findings so the caller can count them.
|
||||
data, findings = apply_policy(payload, policy)
|
||||
details: dict[str, Any] = {
|
||||
"correlation_id": str(payload["correlation_id"]),
|
||||
"data": data,
|
||||
}
|
||||
if findings:
|
||||
# The stored record differs from what the sender transmitted. Say so in
|
||||
# the record itself rather than leaving it to be inferred.
|
||||
details["redaction"] = {
|
||||
"policy": policy,
|
||||
"paths": [f.path for f in findings],
|
||||
}
|
||||
return AuditEvent(
|
||||
event_id=str(payload["id"]),
|
||||
observed_at=observed_at,
|
||||
|
|
@ -280,7 +339,7 @@ def normalize(
|
|||
resource=str(payload["subject"]),
|
||||
outcome="recorded",
|
||||
actor=None,
|
||||
details={"correlation_id": str(payload["correlation_id"]), "data": payload["data"]},
|
||||
details=details,
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -299,18 +358,6 @@ def _normalize_timestamp(value: Any) -> str:
|
|||
return parsed.astimezone(timezone.utc).isoformat()
|
||||
|
||||
|
||||
def _contains_secret(value: Any) -> bool:
|
||||
if isinstance(value, dict):
|
||||
return any(
|
||||
any(fragment in str(key).lower() for fragment in _SECRET_FRAGMENTS)
|
||||
or _contains_secret(item)
|
||||
for key, item in value.items()
|
||||
)
|
||||
if isinstance(value, list):
|
||||
return any(_contains_secret(item) for item in value)
|
||||
return False
|
||||
|
||||
|
||||
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))
|
||||
|
|
|
|||
127
audit_core/redaction.py
Normal file
127
audit_core/redaction.py
Normal file
|
|
@ -0,0 +1,127 @@
|
|||
"""Secret-shaped field detection, redaction, and rejection.
|
||||
|
||||
AUDIT-WP-0004-T04. The default is to **redact and accept**: rejecting an
|
||||
otherwise legitimate event loses the audit record entirely, which is a worse
|
||||
outcome than storing it with one field masked. A sender that needs the
|
||||
stricter posture can be switched to **reject** per identity, so a
|
||||
higher-assurance channel can refuse rather than mask.
|
||||
|
||||
Both outcomes are counted per field path, because the goal is not to redact
|
||||
efficiently — it is to stop producing such fields at the source. A counter
|
||||
that only says "47 redactions" tells an operator nothing actionable; one that
|
||||
says ``data.credentials.password`` on ``membership.added`` from ``user-engine``
|
||||
names the thing to fix.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass
|
||||
from typing import Any
|
||||
|
||||
SECRET_FRAGMENTS = ("password", "secret", "token", "credential", "private_key")
|
||||
|
||||
REDACTED = "[redacted]"
|
||||
|
||||
POLICY_REDACT = "redact"
|
||||
POLICY_REJECT = "reject"
|
||||
POLICIES = (POLICY_REDACT, POLICY_REJECT)
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class Finding:
|
||||
"""One secret-shaped field, located by dotted path."""
|
||||
|
||||
path: str
|
||||
in_persisted_data: bool
|
||||
|
||||
|
||||
class SecretFieldRejection(ValueError):
|
||||
"""Raised under the reject policy. Carries the findings for counting."""
|
||||
|
||||
def __init__(self, findings: list[Finding]) -> None:
|
||||
self.findings = findings
|
||||
# The message stays the stable error code senders already key on.
|
||||
super().__init__("secret_shaped_field")
|
||||
|
||||
|
||||
def _is_secret_key(key: Any) -> bool:
|
||||
lowered = str(key).lower()
|
||||
return any(fragment in lowered for fragment in SECRET_FRAGMENTS)
|
||||
|
||||
|
||||
def scan(value: Any, prefix: str = "") -> list[Finding]:
|
||||
"""Locate every secret-shaped key, by dotted path.
|
||||
|
||||
Detection is by key name across the whole structure, including nested
|
||||
dicts and lists. Values are not inspected: a value-shape heuristic on an
|
||||
audit payload produces false positives on legitimate identifiers, and a
|
||||
false positive here silently mangles an audit record.
|
||||
"""
|
||||
findings: list[Finding] = []
|
||||
_walk(value, prefix, findings)
|
||||
return findings
|
||||
|
||||
|
||||
def _walk(value: Any, prefix: str, findings: list[Finding]) -> None:
|
||||
if isinstance(value, dict):
|
||||
for key, item in value.items():
|
||||
path = f"{prefix}.{key}" if prefix else str(key)
|
||||
if _is_secret_key(key):
|
||||
findings.append(Finding(path=path, in_persisted_data=_persisted(path)))
|
||||
continue # do not descend into a field already flagged
|
||||
_walk(item, path, findings)
|
||||
elif isinstance(value, list):
|
||||
for index, item in enumerate(value):
|
||||
_walk(item, f"{prefix}[{index}]", findings)
|
||||
|
||||
|
||||
def _persisted(path: str) -> bool:
|
||||
"""Whether this path lands in the stored record.
|
||||
|
||||
Only ``data`` is persisted into the event details. A secret-shaped key
|
||||
elsewhere in the envelope is still reported, but it is dropped by
|
||||
normalization rather than stored, so it needs no masking.
|
||||
"""
|
||||
return path == "data" or path.startswith("data.") or path.startswith("data[")
|
||||
|
||||
|
||||
def redact(value: Any) -> Any:
|
||||
"""Return a copy with every secret-shaped field's value masked.
|
||||
|
||||
Keys are preserved. Removing them would hide the fact that the sender
|
||||
transmitted the field at all, which is exactly what the operator needs to
|
||||
see in order to stop it.
|
||||
"""
|
||||
if isinstance(value, dict):
|
||||
return {
|
||||
key: (REDACTED if _is_secret_key(key) else redact(item))
|
||||
for key, item in value.items()
|
||||
}
|
||||
if isinstance(value, list):
|
||||
return [redact(item) for item in value]
|
||||
return value
|
||||
|
||||
|
||||
def apply_policy(payload: dict[str, Any], policy: str) -> tuple[Any, list[Finding]]:
|
||||
"""Apply ``policy`` to ``payload['data']``.
|
||||
|
||||
Returns the data to persist and every finding across the whole payload.
|
||||
Raises :class:`SecretFieldRejection` under the reject policy.
|
||||
"""
|
||||
findings = scan(payload)
|
||||
data = payload.get("data")
|
||||
if not findings:
|
||||
return data, findings
|
||||
if policy == POLICY_REJECT:
|
||||
raise SecretFieldRejection(findings)
|
||||
return redact(data), findings
|
||||
|
||||
|
||||
def finding_from_path(path: str) -> Finding:
|
||||
"""Rebuild a Finding from its path.
|
||||
|
||||
``in_persisted_data`` is a pure function of the path, so a finding recorded
|
||||
on an event record can be reconstructed for counting without carrying the
|
||||
flag through the stored details.
|
||||
"""
|
||||
return Finding(path=path, in_persisted_data=_persisted(path))
|
||||
|
|
@ -19,6 +19,8 @@ import os
|
|||
from dataclasses import dataclass, field
|
||||
from typing import Any, Iterable
|
||||
|
||||
from audit_core.redaction import POLICIES, POLICY_REDACT
|
||||
|
||||
WILDCARD = "*"
|
||||
|
||||
|
||||
|
|
@ -32,10 +34,19 @@ class SenderIdentity:
|
|||
tenants: frozenset[str] = field(default_factory=lambda: frozenset({WILDCARD}))
|
||||
may_write: bool = True
|
||||
may_read: bool = False
|
||||
# Secret-shaped field handling. Defaults to redact-and-accept, so a
|
||||
# legitimate event is not lost over one field; a higher-assurance channel
|
||||
# can be set to reject instead (AUDIT-WP-0004-T04).
|
||||
secret_policy: str = POLICY_REDACT
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
if not self.name:
|
||||
raise ValueError("sender identity needs a name")
|
||||
if self.secret_policy not in POLICIES:
|
||||
raise ValueError(
|
||||
f"sender {self.name!r}: secret_policy must be one of {POLICIES}, "
|
||||
f"got {self.secret_policy!r}"
|
||||
)
|
||||
if not self.tokens or any(not t for t in self.tokens):
|
||||
raise ValueError(f"sender {self.name!r} needs at least one non-empty token")
|
||||
if not self.sources:
|
||||
|
|
@ -144,6 +155,7 @@ def _identity_from(entry: Any) -> SenderIdentity:
|
|||
tenants=frozenset(str(t) for t in (entry.get("tenants") or [WILDCARD])),
|
||||
may_write=bool(entry.get("may_write", True)),
|
||||
may_read=bool(entry.get("may_read", False)),
|
||||
secret_policy=str(entry.get("secret_policy", POLICY_REDACT)),
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -46,6 +46,21 @@ CREATE TABLE IF NOT EXISTS dead_letters (
|
|||
payload_withheld INTEGER NOT NULL DEFAULT 0
|
||||
);
|
||||
CREATE INDEX IF NOT EXISTS dead_letters_event_idx ON dead_letters (event_id);
|
||||
|
||||
-- Counted per field path, not merely per event: the point is to stop senders
|
||||
-- emitting secret-shaped fields, and that needs the offending path named.
|
||||
CREATE TABLE IF NOT EXISTS secret_findings (
|
||||
sender TEXT NOT NULL,
|
||||
source TEXT NOT NULL,
|
||||
action TEXT NOT NULL,
|
||||
field_path TEXT NOT NULL,
|
||||
outcome TEXT NOT NULL,
|
||||
persisted INTEGER NOT NULL DEFAULT 0,
|
||||
occurrences INTEGER NOT NULL DEFAULT 0,
|
||||
first_seen TEXT NOT NULL,
|
||||
last_seen TEXT NOT NULL,
|
||||
PRIMARY KEY (sender, source, action, field_path, outcome)
|
||||
);
|
||||
"""
|
||||
|
||||
# Rejection reasons whose payload must never be persisted. Storing the body of
|
||||
|
|
@ -238,6 +253,52 @@ class SQLiteAuditBackend:
|
|||
for r in rows
|
||||
]
|
||||
|
||||
def count_secret_findings(
|
||||
self, *, sender: str, source: str, action: str, outcome: str, findings
|
||||
) -> None:
|
||||
"""Increment the per-path counter for each finding.
|
||||
|
||||
Written durably rather than held in memory: these counters exist to
|
||||
drive a fix in the sending service, and that work outlives a pod
|
||||
restart.
|
||||
"""
|
||||
now = datetime.now(timezone.utc).replace(microsecond=0).isoformat()
|
||||
try:
|
||||
for finding in findings:
|
||||
self.db.execute(
|
||||
"""
|
||||
INSERT INTO secret_findings
|
||||
(sender, source, action, field_path, outcome, persisted,
|
||||
occurrences, first_seen, last_seen)
|
||||
VALUES (?, ?, ?, ?, ?, ?, 1, ?, ?)
|
||||
ON CONFLICT(sender, source, action, field_path, outcome)
|
||||
DO UPDATE SET occurrences = occurrences + 1, last_seen = excluded.last_seen
|
||||
""",
|
||||
(
|
||||
sender, source, action, finding.path, outcome,
|
||||
1 if finding.in_persisted_data else 0, now, now,
|
||||
),
|
||||
)
|
||||
except sqlite3.Error as exc:
|
||||
raise BackendUnavailableError(str(exc)) from exc
|
||||
|
||||
def secret_findings(self, limit: int = 100) -> list[dict]:
|
||||
"""Return secret-shaped field counters, most frequent first."""
|
||||
rows = self._query(
|
||||
"SELECT sender, source, action, field_path, outcome, persisted, "
|
||||
"occurrences, first_seen, last_seen FROM secret_findings "
|
||||
"ORDER BY occurrences DESC, last_seen DESC LIMIT ?",
|
||||
(int(limit),),
|
||||
)
|
||||
return [
|
||||
{
|
||||
"sender": r[0], "source": r[1], "action": r[2], "field_path": r[3],
|
||||
"outcome": r[4], "persisted": bool(r[5]), "occurrences": r[6],
|
||||
"first_seen": r[7], "last_seen": r[8],
|
||||
}
|
||||
for r in rows
|
||||
]
|
||||
|
||||
def _query(self, sql: str, params: tuple) -> list:
|
||||
try:
|
||||
return self.db.execute(sql, params).fetchall()
|
||||
|
|
|
|||
|
|
@ -36,6 +36,8 @@ Requirements:
|
|||
unless deduplication is documented.
|
||||
- **No secret dumping:** Backends must not log or persist plaintext secrets,
|
||||
tokens, keys, or passwords from `details` or future payload fields.
|
||||
- **Redaction is recorded, not silent:** where ingestion masks a field, the
|
||||
stored record says so (see [Secret-shaped fields](#secret-shaped-fields)).
|
||||
- **Visible failure:** Raise on persistence failure; do not silently drop events.
|
||||
- **Normalization only:** Backends receive `AuditEvent` instances. Source-specific
|
||||
adapters run upstream.
|
||||
|
|
@ -73,7 +75,7 @@ tenant, scope, actor, and result objects.
|
|||
| --- | --- | --- |
|
||||
| `actor` | string or null | Subject performing the action |
|
||||
| `reason` | string or null | Human-readable result explanation |
|
||||
| `details` | object | Source-specific extension map; must not contain secrets |
|
||||
| `details` | object | Source-specific extension map; must not contain secrets. May carry a `redaction` entry — see below |
|
||||
|
||||
### Example record
|
||||
|
||||
|
|
@ -225,4 +227,76 @@ Archive remains the evidence record; hot search may use shorter `retention_days`
|
|||
|
||||
- `INTENT.md` — product purpose and principles
|
||||
- `spec/ProductRequirementsDefinition.md` — full v1 envelope and API requirements
|
||||
- `registry/capabilities/capability.audit.event-retain.md` — capability registry entry
|
||||
- `registry/capabilities/capability.audit.event-retain.md` — capability registry entry
|
||||
## Secret-shaped fields
|
||||
|
||||
Ingestion detects fields whose *key name* contains `password`, `secret`,
|
||||
`token`, `credential`, or `private_key`, at any depth in the payload. Values
|
||||
are not inspected: a value-shape heuristic produces false positives on
|
||||
legitimate identifiers, and a false positive here silently mangles an audit
|
||||
record.
|
||||
|
||||
### Policy
|
||||
|
||||
The default is **redact and accept**. Rejecting an otherwise valid event
|
||||
because of one field loses the audit record entirely, which is a worse outcome
|
||||
than storing it with that field masked.
|
||||
|
||||
Policy is set **per sender identity** via `secret_policy` in
|
||||
`AUDIT_CORE_SENDERS`, so a higher-assurance channel can be switched to
|
||||
`reject` without changing the posture for every other sender:
|
||||
|
||||
```json
|
||||
[{"name": "user-engine", "tokens": ["..."], "sources": ["user-engine"],
|
||||
"secret_policy": "redact"},
|
||||
{"name": "payments-engine", "tokens": ["..."], "sources": ["payments-engine"],
|
||||
"secret_policy": "reject"}]
|
||||
```
|
||||
|
||||
| Policy | Response | Effect |
|
||||
|---|---|---|
|
||||
| `redact` (default) | `202` / `200` | Value replaced with `[redacted]`; key preserved; event stored |
|
||||
| `reject` | `400 secret_shaped_field` | Event not stored; dead-lettered with its payload withheld |
|
||||
|
||||
Keys are preserved under redaction. Dropping them would hide the fact that the
|
||||
sender transmitted the field at all — which is exactly what an operator needs
|
||||
in order to stop it.
|
||||
|
||||
### Recorded redaction
|
||||
|
||||
A redacted record carries the fact in `details.redaction`, so a reader never
|
||||
has to infer whether what they are looking at is what the sender sent:
|
||||
|
||||
```json
|
||||
"details": {
|
||||
"correlation_id": "corr-1",
|
||||
"data": {"membership_id": "m-1", "auth_token": "[redacted]"},
|
||||
"redaction": {"policy": "redact", "paths": ["data.auth_token"]}
|
||||
}
|
||||
```
|
||||
|
||||
Idempotency is unaffected: the payload hash is taken over the original request
|
||||
body, so a resubmission of the same original is still recognised as a
|
||||
duplicate and redaction is deterministic.
|
||||
|
||||
### Counting
|
||||
|
||||
Both outcomes are counted durably, aggregated by sender, source, action, and
|
||||
**field path** — because the actionable unit is "stop emitting
|
||||
`data.auth.token` on `membership.added`", not "there were 47 redactions".
|
||||
Counters survive restart, since the fix they drive lives in another service.
|
||||
|
||||
Read them at `GET /v1/secret-findings` (requires the read privilege):
|
||||
|
||||
```json
|
||||
{"secret_findings": [
|
||||
{"sender": "user-engine", "source": "user-engine",
|
||||
"action": "membership.added", "field_path": "data.auth_token",
|
||||
"outcome": "redacted", "persisted": true, "occurrences": 3,
|
||||
"first_seen": "...", "last_seen": "..."}]}
|
||||
```
|
||||
|
||||
`persisted` distinguishes a field that reached the stored record from one that
|
||||
sat elsewhere in the envelope and was dropped by normalization anyway. A
|
||||
healthy sender trends to zero occurrences; a non-empty list is a backlog item
|
||||
for the sending service, not a steady state.
|
||||
|
|
|
|||
|
|
@ -100,7 +100,6 @@ def test_non_ascii_credential_is_unauthorized_not_a_crash(app):
|
|||
# --- validation (T02, T07) --------------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("payload,expected", [
|
||||
(event(data={"password": "never"}), "secret_shaped_field"),
|
||||
(event(source="somewhere-else"), "source_not_allowed"),
|
||||
(event(occurred_at="2026-08-09T00:00:00"), "timestamp_missing_timezone"),
|
||||
(event(occurred_at="not-a-date"), "invalid_timestamp"),
|
||||
|
|
@ -235,6 +234,7 @@ def bound_app(tmp_path, **kw):
|
|||
sources=frozenset(kw.get("sources", {"user-engine"})),
|
||||
tenants=frozenset(kw.get("tenants", {"tenant:friendly:binky"})),
|
||||
may_read=kw.get("may_read", False),
|
||||
secret_policy=kw.get("secret_policy", "redact"),
|
||||
)
|
||||
backend = SQLiteAuditBackend(str(tmp_path / "bound.db"))
|
||||
return IngestionApplication(backend, SenderRegistry([identity])), backend
|
||||
|
|
@ -323,9 +323,10 @@ def test_rejected_events_appear_as_dead_letters(app):
|
|||
assert entry["payload"] is not None
|
||||
|
||||
|
||||
def test_secret_rejection_withholds_the_payload(app):
|
||||
def test_secret_rejection_withholds_the_payload(tmp_path):
|
||||
"""Storing the body of an event rejected for carrying secret-shaped
|
||||
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"")
|
||||
entry = body["dead_letters"][0]
|
||||
|
|
@ -333,3 +334,69 @@ def test_secret_rejection_withholds_the_payload(app):
|
|||
assert entry["payload_withheld"] is True
|
||||
assert entry["payload"] is None
|
||||
assert entry["payload_hash"]
|
||||
|
||||
|
||||
# --- redaction policy (T04) -------------------------------------------------
|
||||
|
||||
def test_default_policy_redacts_and_accepts(app):
|
||||
"""Default is redact: losing the whole audit record over one field is
|
||||
worse than storing it with that field masked."""
|
||||
status, _ = invoke(app, event(data={"membership_id": "m-1", "auth_token": "s3cret"}))
|
||||
assert status.startswith("202")
|
||||
|
||||
_, record = invoke(app, None, path="/v1/events/evt-1", method="GET", body=b"")
|
||||
assert record["details"]["data"]["auth_token"] == "[redacted]"
|
||||
assert record["details"]["data"]["membership_id"] == "m-1"
|
||||
# The record must admit it was modified.
|
||||
assert record["details"]["redaction"]["policy"] == "redact"
|
||||
assert record["details"]["redaction"]["paths"] == ["data.auth_token"]
|
||||
|
||||
|
||||
def test_reject_policy_is_available_per_sender(tmp_path):
|
||||
app, _ = bound_app(tmp_path, secret_policy="reject")
|
||||
status, body = invoke(app, event(data={"password": "x"}))
|
||||
assert status.startswith("400")
|
||||
assert body["error"] == "secret_shaped_field"
|
||||
|
||||
|
||||
def test_nested_and_listed_secrets_are_redacted(app):
|
||||
payload = event(data={"items": [{"api_secret": "a"}, {"ok": 1}], "n": {"private_key": "k"}})
|
||||
assert invoke(app, payload)[0].startswith("202")
|
||||
_, record = invoke(app, None, path="/v1/events/evt-1", method="GET", body=b"")
|
||||
data = record["details"]["data"]
|
||||
assert data["items"][0]["api_secret"] == "[redacted]"
|
||||
assert data["items"][1]["ok"] == 1
|
||||
assert data["n"]["private_key"] == "[redacted]"
|
||||
assert set(record["details"]["redaction"]["paths"]) == {
|
||||
"data.items[0].api_secret", "data.n.private_key",
|
||||
}
|
||||
|
||||
|
||||
def test_findings_are_counted_by_path_for_both_outcomes(tmp_path):
|
||||
"""Counters name the field to fix, not just a total."""
|
||||
app, backend = bound_app(tmp_path, may_read=True)
|
||||
for i in range(3):
|
||||
invoke(app, event(id=f"e{i}", data={"auth_token": "x"}), key=f"e{i}")
|
||||
|
||||
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"")
|
||||
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"
|
||||
assert rows[("redacted", "data.auth_token")]["source"] == "user-engine"
|
||||
assert rows[("redacted", "data.auth_token")]["persisted"] is True
|
||||
assert rows[("rejected", "data.auth_token")]["occurrences"] == 1
|
||||
|
||||
|
||||
def test_counters_survive_restart(tmp_path):
|
||||
"""The counters drive a fix in the sending service; that work outlives a
|
||||
pod restart, so they are durable rather than in-memory."""
|
||||
path = str(tmp_path / "counters.db")
|
||||
first = IngestionApplication(SQLiteAuditBackend(path), "opaque")
|
||||
invoke(first, event(data={"auth_token": "x"}))
|
||||
|
||||
reopened = IngestionApplication(SQLiteAuditBackend(path), "opaque")
|
||||
_, body = invoke(reopened, None, path="/v1/secret-findings", method="GET", body=b"")
|
||||
assert body["secret_findings"][0]["occurrences"] == 1
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ type: workplan
|
|||
title: "Make the event receiver correct and operable under load"
|
||||
domain: infotech
|
||||
repo: audit-core
|
||||
status: proposed
|
||||
status: finished
|
||||
owner: codex
|
||||
topic_slug: netkingdom
|
||||
created: "2026-08-10"
|
||||
|
|
@ -150,7 +150,7 @@ grants every tenant and says so.
|
|||
|
||||
```task
|
||||
id: AUDIT-WP-0004-T04
|
||||
status: todo
|
||||
status: done
|
||||
priority: medium
|
||||
state_hub_task_id: "5cf5c412-965a-4a56-aeef-e965f2861c51"
|
||||
```
|
||||
|
|
@ -172,6 +172,30 @@ agree with the code.
|
|||
Done when the redaction rule is explicit, applied to the whole payload, and
|
||||
consistent between the contract and the implementation.
|
||||
|
||||
Decided 2026-08-10 (Bernd): **default to redaction, allow rejection per
|
||||
sender.** Losing an audit record over one field is worse than storing it
|
||||
masked, but a higher-assurance channel must be able to refuse rather than
|
||||
mask. `secret_policy` is set per sender identity in `AUDIT_CORE_SENDERS`,
|
||||
defaulting to `redact`.
|
||||
|
||||
Done 2026-08-10: `audit_core.redaction` detects secret-shaped keys at any
|
||||
depth across the whole payload, including lists. Under redaction the value is
|
||||
masked and the **key is preserved** — dropping it would hide that the sender
|
||||
transmitted the field at all, which is what the operator needs in order to
|
||||
stop it. The stored record carries `details.redaction` with the policy and the
|
||||
affected paths, so a reader never has to infer whether what they see is what
|
||||
was sent. Idempotency is unaffected: the payload hash is over the original
|
||||
body, so redaction is deterministic and replay still reconciles.
|
||||
|
||||
Both outcomes are counted durably by sender, source, action and **field
|
||||
path**, exposed at `GET /v1/secret-findings`. Per-path aggregation is the
|
||||
point: the actionable unit is "stop emitting `data.auth.token` on
|
||||
`membership.added`", not "there were 47 redactions". Counters survive restart
|
||||
because the fix they drive lives in another service. A non-empty list is a
|
||||
backlog item for the sender, not a steady state.
|
||||
|
||||
Contract updated to match; tests 46 -> 50.
|
||||
|
||||
## T05 - Provide the operator read surface
|
||||
|
||||
```task
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue