From 9888c8e65ea9c947007a0e91c03df8e1fe5b7e67 Mon Sep 17 00:00:00 2001 From: mikehasa Date: Thu, 30 Jul 2026 23:58:53 +0800 Subject: [PATCH 1/8] fix(service): redact only the secret span, and say which field was cut Value-pattern redaction replaced the WHOLE string on any match, and the patterns were unanchored. "sk-" matched the middle of ordinary words ("task-", "disk-", "risk-", "brisk-") and "Bearer" matched the English phrase "Bearer token", so ordinary ledger data was destroyed: paths like ".../work-task-sessions-4801fa" were stored as a bare placeholder. Worse, every destroyed section_id collapsed onto the same literal string, merging unrelated jobs into one phantom section. All of it recorded success with no warning, which is exactly the quietly-wrong record this ledger exists to avoid. Now: * patterns are anchored -- a word boundary before "sk-", and something credential-shaped (>=16 chars of token charset containing a digit or a separator) after "Bearer", so prose can no longer trip them; * only the matched span is replaced, with a distinct "[REDACTED_SECRET]" placeholder, so surrounding prose, ids and paths survive; * a redaction stamps server-authored metadata naming each field and its pattern class, following the existing reserved-provenance convention: value_redaction_applied / value_redaction_fields, with any caller-supplied copy stripped first (reserved_value_redaction_provenance_stripped) so an agent cannot claim a redaction that never happened. The marker is never written in-band inside the value, which would itself be redactable. Sensitive key NAMES are unchanged: those still drop the whole value, because that is a decision about the field rather than a content match. Two existing tests asserted the old whole-string result. Both are updated to the stronger assertion (exact new value, prose preserved); their "secret is gone" assertions were left as-is and still pass. Already-damaged historical values are unrecoverable and are deliberately not backfilled. --- src/agentacct/service.py | 148 ++++++++++++++-- tests/test_cli.py | 5 +- tests/test_local_api.py | 4 +- tests/test_secret_value_redaction.py | 242 +++++++++++++++++++++++++++ 4 files changed, 381 insertions(+), 18 deletions(-) create mode 100644 tests/test_secret_value_redaction.py diff --git a/src/agentacct/service.py b/src/agentacct/service.py index f34a881..45332dc 100644 --- a/src/agentacct/service.py +++ b/src/agentacct/service.py @@ -151,31 +151,150 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: return recorded +# Secret shapes recognized inside ordinary string VALUES, ordered +# most-specific-first so the reported pattern class is the precise one. +# +# Both anchors are load-bearing. Unanchored, `sk-` matched the middle of +# ordinary English and ordinary identifiers -- "task-", "disk-", "risk-", +# "brisk-" all contain "sk-" -- and "Bearer" followed by any non-delimiter run +# matched the prose phrase "use a Bearer token here". Paired with the old +# whole-string replacement that silently destroyed real ledger values: a path +# ending +# ".../work-task-sessions-4801fa" was stored as a bare placeholder, and +# unrelated section ids that each tripped the pattern all collapsed onto the +# same literal string and merged into one phantom section. A quietly wrong +# record is worse than the leak it was guarding against, so the patterns now +# require a word boundary before `sk-` and something credential-shaped (rather +# than an English word) after `Bearer`. SECRET_VALUE_PATTERNS = ( - re.compile(r"Bearer\s+[^\s,;\]}\)]+", re.IGNORECASE), - re.compile(r"sk-[A-Za-z0-9_-]{12,}"), - re.compile(r"sk-or-v1-[A-Za-z0-9_-]{12,}"), + ( + "bearer_token", + re.compile( + r"\bBearer\s+(?=[A-Za-z0-9._~+/=-]*[0-9._~+/=-])[A-Za-z0-9._~+/=-]{16,}", + re.IGNORECASE, + ), + ), + ("openrouter_api_key", re.compile(r"\bsk-or-v1-[A-Za-z0-9_-]{12,}")), + ("api_key", re.compile(r"\bsk-[A-Za-z0-9_-]{12,}")), ) +# Only the matched span is replaced, so surrounding prose, ids and paths +# survive. Deliberately distinct from the "[REDACTED]" used for a sensitive key +# NAME (where dropping the whole value is the right call) so a reader can tell +# "a secret was cut out of this value" from "this field was a credential", and +# built from characters no pattern above can match, so redaction is idempotent. +SECRET_SPAN_PLACEHOLDER = "[REDACTED_SECRET]" + +# Server-authored redaction provenance. Written only by the recording paths +# when a span was actually replaced, and stripped from caller input first so an +# agent cannot claim a redaction that never happened. It lives in server-owned +# metadata and never in-band inside the value itself: an in-band marker would +# just be another string eligible for redaction. The names deliberately avoid +# the words is_sensitive_metadata_key() treats as credential-ish ("secret", +# "credential", ...), which would blank the marker's own value. +VALUE_REDACTION_MARKER_KEY = "value_redaction_applied" +VALUE_REDACTION_FIELDS_KEY = "value_redaction_fields" +RESERVED_VALUE_REDACTION_KEYS = ( + VALUE_REDACTION_MARKER_KEY, + VALUE_REDACTION_FIELDS_KEY, +) + + +def strip_value_redaction_provenance(event: dict[str, Any]) -> dict[str, Any]: + metadata = event.get("metadata") + if not isinstance(metadata, dict) or not any(key in metadata for key in RESERVED_VALUE_REDACTION_KEYS): + return event + sanitized = dict(metadata) + for key in RESERVED_VALUE_REDACTION_KEYS: + sanitized.pop(key, None) + sanitized["reserved_value_redaction_provenance_stripped"] = True + recorded = dict(event) + recorded["metadata"] = sanitized + return recorded + + +def _redact_secret_spans(text: str) -> tuple[str, tuple[str, ...]]: + """Replace secret-shaped spans in ``text``, keeping everything else verbatim.""" + + matched: list[str] = [] + for pattern_class, pattern in SECRET_VALUE_PATTERNS: + text, replacements = pattern.subn(SECRET_SPAN_PLACEHOLDER, text) + if replacements: + matched.append(pattern_class) + return text, tuple(matched) + + +def _redact_secrets( + value: Any, + *, + path: str = "", + found: list[dict[str, str]] | None = None, +) -> Any: + """Copy ``value`` redacted, appending ``field``/``pattern_class`` rows to ``found``. + + Sensitive key NAMES still lose the whole value: that is a decision about + the field, not a match against its content. Value patterns only take the + span they matched. + """ -def _redact_secrets(value: Any) -> Any: if isinstance(value, dict): redacted: dict[str, Any] = {} for key, child in value.items(): + child_path = f"{path}.{key}" if path else str(key) if is_sensitive_metadata_key(key): redacted[key] = "[REDACTED]" else: - redacted[key] = _redact_secrets(child) + redacted[key] = _redact_secrets(child, path=child_path, found=found) return redacted - if isinstance(value, list): - return [_redact_secrets(item) for item in value] - if isinstance(value, tuple): - return tuple(_redact_secrets(item) for item in value) - if isinstance(value, str) and any(pattern.search(value) for pattern in SECRET_VALUE_PATTERNS): - return "[REDACTED]" + if isinstance(value, (list, tuple)): + items = [ + _redact_secrets( + item, + path=f"{path}.{index}" if path else str(index), + found=found, + ) + for index, item in enumerate(value) + ] + return items if isinstance(value, list) else tuple(items) + if isinstance(value, str): + text, matched = _redact_secret_spans(value) + if matched and found is not None: + found.extend({"field": path, "pattern_class": name} for name in matched) + return text return value +def _stamp_value_redaction_marker( + event: dict[str, Any], + redactions: list[dict[str, str]], +) -> dict[str, Any]: + if not redactions: + return event + detail = sorted(redactions, key=lambda row: (row["field"], row["pattern_class"])) + stamped = dict(event) + metadata = stamped.get("metadata") + if metadata is None or isinstance(metadata, dict): + marked = dict(metadata or {}) + marked[VALUE_REDACTION_MARKER_KEY] = True + marked[VALUE_REDACTION_FIELDS_KEY] = detail + stamped["metadata"] = marked + else: + # Off-contract metadata is never rewritten, but the repair still has to + # be visible, so the marker lands beside it instead of replacing it. + stamped[VALUE_REDACTION_MARKER_KEY] = True + stamped[VALUE_REDACTION_FIELDS_KEY] = detail + return stamped + + +def redact_event_secrets(event: dict[str, Any]) -> dict[str, Any]: + """Redact one event and stamp server-authored provenance for what was cut.""" + + sanitized = strip_value_redaction_provenance(event) + redactions: list[dict[str, str]] = [] + redacted = _redact_secrets(sanitized, found=redactions) + return _stamp_value_redaction_marker(redacted, redactions) + + def _safe_nonnegative_int(value: Any) -> int: try: number = float(0 if value is None else value) @@ -350,8 +469,7 @@ def _normalize_finding_disposition_note(note: str | None, *, action: str) -> str if normalized is not None: if len(normalized) > 1200 or any(char in normalized for char in "\r\n\x00"): raise FindingDispositionConflict("finding disposition note is invalid") - redacted = _redact_secrets(normalized) - normalized = redacted if isinstance(redacted, str) else "[REDACTED]" + normalized, _matched = _redact_secret_spans(normalized) if action == "resolve" and not normalized: raise FindingDispositionConflict("resolving a finding requires a note") return normalized @@ -475,7 +593,7 @@ def _read_events_file_order(self, *, run_id: str | None = None) -> list[dict[str return events def _prepare_recorded_event(self, event: dict[str, Any]) -> dict[str, Any]: - recorded = _redact_secrets(dict(event)) + recorded = redact_event_secrets(dict(event)) # Server-owned fields are always assigned locally. Caller-provided values # must not be able to break sorting or spoof event identity. recorded["event_id"] = "evt_" + uuid.uuid4().hex[:12] @@ -572,7 +690,7 @@ def record_event( return recorded def _trusted_session_observation_candidate(self, event: dict[str, Any]) -> dict[str, Any]: - candidate = _redact_secrets(dict(event)) + candidate = redact_event_secrets(dict(event)) candidate = strip_untrusted_usage_truth_metadata(candidate) candidate = strip_untrusted_instrumentation_marker_metadata(candidate) candidate = strip_client_context_provenance(candidate) diff --git a/tests/test_cli.py b/tests/test_cli.py index fe671b8..483b1e2 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -452,10 +452,11 @@ def test_event_note_redacts_secret_shaped_summary_in_storage_and_output(tmp_path record = CliRunner().invoke(app, ["event", "note", f"called {secretish}", "--store-dir", str(store_dir)]) assert record.exit_code == 0, record.output - assert "[REDACTED]" in record.output + # Only the secret span is replaced now: the surrounding note survives. + assert "called [REDACTED_SECRET]" in record.output assert secretish not in record.output stored = (store_dir / "events.jsonl").read_text(encoding="utf-8") - assert "[REDACTED]" in stored + assert "called [REDACTED_SECRET]" in stored assert secretish not in stored diff --git a/tests/test_local_api.py b/tests/test_local_api.py index bb1781e..c25c228 100644 --- a/tests/test_local_api.py +++ b/tests/test_local_api.py @@ -1414,7 +1414,9 @@ def test_local_api_redacts_secret_shaped_string_values(tmp_path): assert response.status_code == 200 event = response.json()["event"] - assert event["metadata"]["summary"] == "[REDACTED]" + # Only the secret span goes; the prose around it is ledger data and stays. + assert event["metadata"]["summary"] == "called with [REDACTED_SECRET]" + assert event["metadata"]["value_redaction_applied"] is True assert secretish not in (tmp_path / "state" / "events.jsonl").read_text(encoding="utf-8") diff --git a/tests/test_secret_value_redaction.py b/tests/test_secret_value_redaction.py new file mode 100644 index 0000000..9a832ad --- /dev/null +++ b/tests/test_secret_value_redaction.py @@ -0,0 +1,242 @@ +"""Value-pattern secret redaction: cut the secret, keep the record. + +The original implementation replaced the WHOLE string on any pattern hit and +used unanchored patterns, so ordinary ledger data was destroyed: "task-", +"disk-" and "risk-" all contain "sk-", and the phrase "Bearer token" matched +the Authorization pattern. Worse than the loss, every destroyed section_id +collapsed onto the same literal placeholder, so unrelated jobs merged into one +phantom section. These tests pin both halves: real secrets still go, and the +false positives that caused the incident never touch a value again. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +from agentacct.service import SentinelService + + +# Built by concatenation so the fixtures are unmistakably synthetic while +# keeping the exact shape the patterns must still recognize. +FAKE_API_KEY = "sk-" + "fakeonly" + "0" * 32 + "AbCd" +FAKE_OPENROUTER_KEY = "sk-or-v1-" + "fakeonly" + "0" * 40 + "EfGh" +FAKE_BEARER_HEADER = "Bearer " + "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJmYWtlIn0.c2lnbmF0dXJl" + + +def _stored_text(store: Path) -> str: + return (store / "events.jsonl").read_text(encoding="utf-8") + + +def _stored_events(store: Path) -> list[dict]: + return [ + json.loads(line) + for line in _stored_text(store).splitlines() + if line.strip() + ] + + +def test_real_shaped_secrets_are_still_removed_from_the_stored_event(tmp_path: Path) -> None: + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "openai": FAKE_API_KEY, + "openrouter": FAKE_OPENROUTER_KEY, + "header": FAKE_BEARER_HEADER, + }, + } + ) + + assert recorded["metadata"]["openai"] == "[REDACTED_SECRET]" + assert recorded["metadata"]["openrouter"] == "[REDACTED_SECRET]" + assert recorded["metadata"]["header"] == "[REDACTED_SECRET]" + stored = _stored_text(store) + for secret in (FAKE_API_KEY, FAKE_OPENROUTER_KEY, FAKE_BEARER_HEADER): + assert secret not in stored + # The openrouter shape is a prefix-superset of the plain key shape; it must + # report as the specific class, not the generic one. + classes = {row["pattern_class"] for row in recorded["metadata"]["value_redaction_fields"]} + assert classes == {"api_key", "openrouter_api_key", "bearer_token"} + + +def test_surrounding_prose_survives_a_redacted_secret(tmp_path: Path) -> None: + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "summary": ( + f"Rotated the provider key to {FAKE_API_KEY} after the outage. " + "The dashboard came back clean." + ) + }, + } + ) + + assert recorded["metadata"]["summary"] == ( + "Rotated the provider key to [REDACTED_SECRET] after the outage. " + "The dashboard came back clean." + ) + assert FAKE_API_KEY not in _stored_text(store) + + +def test_incident_false_positives_leave_every_value_untouched(tmp_path: Path) -> None: + store = tmp_path / "state" + service = SentinelService(store) + project_dir = "/Users/x/.claude/worktrees/canonical-migration-4-work-task-sessions-4801fa" + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "section_id": "work-task-sessions-4801fa", + "project_dir": project_dir, + "branch": "feat/task-sessions-4801fa", + "metadata": { + "summary": ( + "disk-usage climbed while risk-assessment-2026 was open; " + "we should use a Bearer token here." + ), + "idempotency_key": "task-sessions-4801fa-attempt-1", + }, + } + ) + + assert recorded["section_id"] == "work-task-sessions-4801fa" + assert recorded["project_dir"] == project_dir + assert recorded["branch"] == "feat/task-sessions-4801fa" + assert recorded["metadata"]["summary"] == ( + "disk-usage climbed while risk-assessment-2026 was open; " + "we should use a Bearer token here." + ) + assert recorded["metadata"]["idempotency_key"] == "task-sessions-4801fa-attempt-1" + assert "[REDACTED" not in _stored_text(store) + + +def test_two_task_shaped_ids_stay_distinct_in_the_ledger(tmp_path: Path) -> None: + """The merge bug: both ids used to collapse onto the literal placeholder.""" + + store = tmp_path / "state" + service = SentinelService(store) + + first = service.record_event( + { + "source": "test", + "event_type": "note", + "section_id": "canonical-migration-4-work-task-sessions-4801fa", + "metadata": {"idempotency_key": "work-task-sessions-4801fa-checkpoint"}, + } + ) + second = service.record_event( + { + "source": "test", + "event_type": "note", + "section_id": "dashboard-rewrite-2-work-task-findings-9c31bb", + "metadata": {"idempotency_key": "work-task-findings-9c31bb-checkpoint"}, + } + ) + + assert first["section_id"] != second["section_id"] + assert first["metadata"]["idempotency_key"] != second["metadata"]["idempotency_key"] + assert first["event_id"] != second["event_id"] + stored = _stored_events(store) + assert len(stored) == 2 + assert {row["section_id"] for row in stored} == { + "canonical-migration-4-work-task-sessions-4801fa", + "dashboard-rewrite-2-work-task-findings-9c31bb", + } + + +def test_marker_names_the_redacted_fields_and_pattern_classes(tmp_path: Path) -> None: + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "branch": f"feat/{FAKE_API_KEY}", + "metadata": { + "summary": f"header was {FAKE_BEARER_HEADER}", + "items": [{"note": FAKE_OPENROUTER_KEY}], + }, + } + ) + + assert recorded["metadata"]["value_redaction_applied"] is True + assert recorded["metadata"]["value_redaction_fields"] == [ + {"field": "branch", "pattern_class": "api_key"}, + {"field": "metadata.items.0.note", "pattern_class": "openrouter_api_key"}, + {"field": "metadata.summary", "pattern_class": "bearer_token"}, + ] + + +def test_marker_is_absent_when_nothing_was_redacted(tmp_path: Path) -> None: + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "project_dir": "/repos/brisk-task-runner", + "metadata": {"summary": "disk-usage report for risk-assessment-2026"}, + } + ) + + assert "value_redaction_applied" not in recorded["metadata"] + assert "value_redaction_fields" not in recorded["metadata"] + + +def test_a_forged_redaction_marker_is_stripped_from_caller_metadata(tmp_path: Path) -> None: + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "summary": "nothing sensitive here", + "value_redaction_applied": True, + "value_redaction_fields": [{"field": "metadata.summary", "pattern_class": "api_key"}], + }, + } + ) + + assert "value_redaction_applied" not in recorded["metadata"] + assert "value_redaction_fields" not in recorded["metadata"] + assert recorded["metadata"]["reserved_value_redaction_provenance_stripped"] is True + + +def test_sensitive_key_names_still_lose_the_whole_value(tmp_path: Path) -> None: + """Key-name decisions are unchanged: only the value-pattern path narrowed.""" + + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "api_key": "some prose and " + FAKE_API_KEY + " and more prose", + "safe": "kept", + }, + } + ) + + assert recorded["metadata"]["api_key"] == "[REDACTED]" + assert recorded["metadata"]["safe"] == "kept" + # Whole-value key redaction is self-evident from the key name, so it does + # not claim a value-pattern redaction. + assert "value_redaction_applied" not in recorded["metadata"] + assert FAKE_API_KEY not in _stored_text(store) From 71bf462367ac29118ce7e085424b9d5619e2826d Mon Sep 17 00:00:00 2001 From: mikehasa Date: Fri, 31 Jul 2026 01:13:03 +0800 Subject: [PATCH 2/8] fix(service): widen the bearer run, keep the marker where it is read Review of the span-redaction change found four defects, three of them the same failure class the change was made to end. The bearer credential run had been narrowed from a delimiter to a fixed [A-Za-z0-9._~+/=-] alphabet, so a real credential stopped at its first "%", ":", "|", "@", "#" or "&". "Bearer abc%2Fdefghijklmnopqrstuvwx" in a plain "summary" field matched only "abc", fell under the length floor, and the whole credential was written to the ledger -- the value pattern is the only defence there, because the key name is not credential-ish. The run is delimiter-based again (whitespace, comma, semicolon, quotes, brackets/braces/ parens end it), so the whole credential is taken. The discriminator was "contains a digit OR a separator", and "-" is a separator, so hyphenated engineering prose was still destroyed: "switch to Bearer token-based-authentication soon" became "switch to [REDACTED_SECRET] soon". It now requires a DIGIT. A digit is the one thing an English compound cannot contain, and JWT, base64, hex and UUID credentials all have one. The accepted residual is a >=16 character all-alphabetic credential, which the value patterns miss; a field NAMED like a credential still loses its whole value. A rare miss beats routinely shredding prose. Trusted session observations stamped the marker and then threw it away: the allowlist rebuild in mark_trusted_local_session_observation_event keeps only known metadata keys. A title carrying a credential was stored correctly redacted with nothing saying data had been cut, which defeats the point. The two server-authored keys join that allowlist. They cannot be forged -- redact_event_secrets strips caller copies before the rebuild -- and because the row now carries them into _prepare_recorded_event, that path takes an already_redacted flag so the redactor cannot strip its own marker as if a caller had planted it. _prepare_recorded_event redacted and stamped, then unconditionally assigned event_id/created_at, so a marker could name a field now holding an ordinary server-generated id with no placeholder in it. Rows naming a server-assigned field are dropped, and the marker with them if nothing else was cut. The prune also runs before the observation allowlist rebuild, which is where observation_revision is computed, so the stored digest keeps agreeing with the stored content. Also closed while in the area: strip_value_redaction_provenance cleared a forged marker only from metadata, but the stamp lands top-level for off-contract metadata, so a caller could plant one there and keep it. Both homes are cleared now, which is what the surrounding comment already claimed. --- src/agentacct/service.py | 147 +++++++++++++-- src/agentacct/usage_truth.py | 10 + tests/test_secret_value_redaction.py | 267 ++++++++++++++++++++++++++- 3 files changed, 405 insertions(+), 19 deletions(-) diff --git a/src/agentacct/service.py b/src/agentacct/service.py index 45332dc..99bab4f 100644 --- a/src/agentacct/service.py +++ b/src/agentacct/service.py @@ -166,11 +166,30 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # record is worse than the leak it was guarding against, so the patterns now # require a word boundary before `sk-` and something credential-shaped (rather # than an English word) after `Bearer`. +# +# The credential run after `Bearer` stays DELIMITER-based rather than a fixed +# credential alphabet. Real credentials carry `%`, `:`, `|`, `@`, `#` and `&` +# (percent-encoding, basic-auth pairs, vendor prefixes), and a charset that +# omits them stops at the first such character: "Bearer abc%2Fdefgh..." would +# match only "abc", fall under the length floor, and write the whole credential +# to the ledger. Only the characters that END a value in the formats we ingest +# are excluded -- whitespace, comma, semicolon, quotes, and the bracket/brace/ +# paren pairs -- which also makes replacement structurally idempotent, since +# SECRET_SPAN_PLACEHOLDER starts with `[` and so leaves an empty run behind. +# +# The discriminator is a DIGIT somewhere in that run. The false positive that +# matters is hyphenated engineering prose -- "Bearer token-based-authentication" +# and "Bearer tokens-are-rotated-nightly" were both destroyed while `-` counted +# as credential-shaped -- and a digit is the one thing an English compound +# cannot contain, while JWT, base64, hex and UUID credentials all do. The +# accepted residual is a >=16 character all-alphabetic credential, which the +# value patterns miss (a field NAMED like a credential still loses its whole +# value): a rare miss, deliberately preferred over routinely shredding prose. SECRET_VALUE_PATTERNS = ( ( "bearer_token", re.compile( - r"\bBearer\s+(?=[A-Za-z0-9._~+/=-]*[0-9._~+/=-])[A-Za-z0-9._~+/=-]{16,}", + r"\bBearer\s+(?=[^\s,;\"'\[\]{}()]*[0-9])[^\s,;\"'\[\]{}()]{16,}", re.IGNORECASE, ), ), @@ -182,7 +201,9 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # survive. Deliberately distinct from the "[REDACTED]" used for a sensitive key # NAME (where dropping the whole value is the right call) so a reader can tell # "a secret was cut out of this value" from "this field was a credential", and -# built from characters no pattern above can match, so redaction is idempotent. +# built from characters no pattern above can match -- the brackets are excluded +# from the bearer run and there is no `sk-` in it -- so redaction is idempotent +# and a second pass can never eat its own placeholder. SECRET_SPAN_PLACEHOLDER = "[REDACTED_SECRET]" # Server-authored redaction provenance. Written only by the recording paths @@ -202,14 +223,27 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: def strip_value_redaction_provenance(event: dict[str, Any]) -> dict[str, Any]: metadata = event.get("metadata") - if not isinstance(metadata, dict) or not any(key in metadata for key in RESERVED_VALUE_REDACTION_KEYS): + # The stamp lands top-level when metadata is off-contract, so a caller can + # plant a forged claim in either home; both are cleared, or "an agent + # cannot claim a redaction that never happened" would only hold for one. + homes = [event, *([metadata] if isinstance(metadata, dict) else [])] + if not any(key in home for home in homes for key in RESERVED_VALUE_REDACTION_KEYS): return event - sanitized = dict(metadata) - for key in RESERVED_VALUE_REDACTION_KEYS: - sanitized.pop(key, None) - sanitized["reserved_value_redaction_provenance_stripped"] = True - recorded = dict(event) - recorded["metadata"] = sanitized + recorded = { + key: value + for key, value in event.items() + if key not in RESERVED_VALUE_REDACTION_KEYS + } + if isinstance(metadata, dict): + sanitized = { + key: value + for key, value in metadata.items() + if key not in RESERVED_VALUE_REDACTION_KEYS + } + sanitized["reserved_value_redaction_provenance_stripped"] = True + recorded["metadata"] = sanitized + else: + recorded["reserved_value_redaction_provenance_stripped"] = True return recorded @@ -295,6 +329,52 @@ def redact_event_secrets(event: dict[str, Any]) -> dict[str, Any]: return _stamp_value_redaction_marker(redacted, redactions) +# Top-level fields the server always assigns for itself. A caller-supplied +# value for one of them is overwritten, so a redaction of it never survives to +# be read. +SERVER_ASSIGNED_EVENT_FIELDS = ("event_id", "created_at") + + +def drop_server_assigned_redaction_rows(event: dict[str, Any]) -> dict[str, Any]: + """Forget marker rows naming a field the server overwrites anyway. + + An incoming ``event_id`` carrying a secret-shaped span is redacted like any + other string, but the server then assigns a fresh id over it. Left alone, + the marker would point a reader at an ordinary ``evt_...`` value with no + placeholder in it -- a claim the stored row cannot support. The rows go; + the marker itself goes too if nothing else was cut. + """ + + metadata = event.get("metadata") + # The marker lives in metadata, or beside it when metadata is off-contract. + for in_metadata, container in ((True, metadata), (False, event)): + if not isinstance(container, dict): + continue + detail = container.get(VALUE_REDACTION_FIELDS_KEY) + if not isinstance(detail, list): + continue + kept = [ + row + for row in detail + if not ( + isinstance(row, dict) + and row.get("field") in SERVER_ASSIGNED_EVENT_FIELDS + ) + ] + if len(kept) == len(detail): + return event + marked = { + key: value + for key, value in container.items() + if key not in RESERVED_VALUE_REDACTION_KEYS + } + if kept: + marked[VALUE_REDACTION_MARKER_KEY] = True + marked[VALUE_REDACTION_FIELDS_KEY] = kept + return {**event, "metadata": marked} if in_metadata else marked + return event + + def _safe_nonnegative_int(value: Any) -> int: try: number = float(0 if value is None else value) @@ -592,13 +672,28 @@ def _read_events_file_order(self, *, run_id: str | None = None) -> list[dict[str events.append(event) return events - def _prepare_recorded_event(self, event: dict[str, Any]) -> dict[str, Any]: - recorded = redact_event_secrets(dict(event)) + def _prepare_recorded_event( + self, + event: dict[str, Any], + *, + already_redacted: bool = False, + ) -> dict[str, Any]: + """Assign server-owned identity to one event, redacting it on the way. + + ``already_redacted`` is for the trusted session-observation lane, which + redacts and stamps in ``_trusted_session_observation_candidate`` -- it + has to, because the allowlist rebuild and the revision digest both run + on the redacted content. Re-running the redactor here would strip that + server-authored marker as if a caller had forged it, and the row would + record a cut with nothing saying so. + """ + + recorded = dict(event) if already_redacted else redact_event_secrets(dict(event)) # Server-owned fields are always assigned locally. Caller-provided values # must not be able to break sorting or spoof event identity. recorded["event_id"] = "evt_" + uuid.uuid4().hex[:12] recorded["created_at"] = time.time() - return recorded + return drop_server_assigned_redaction_rows(recorded) def _find_idempotent_event(self, event: dict[str, Any], idempotency_key: str) -> dict[str, Any] | None: for existing in reversed(self._read_events_file_order()): @@ -690,7 +785,12 @@ def record_event( return recorded def _trusted_session_observation_candidate(self, event: dict[str, Any]) -> dict[str, Any]: - candidate = redact_event_secrets(dict(event)) + # Prune before the allowlist rebuild below: that rebuild is what + # computes observation_revision, so a marker row dropped afterwards + # would leave the stored digest disagreeing with the stored content + # and make every re-import of the same source fact look like a new + # revision. + candidate = drop_server_assigned_redaction_rows(redact_event_secrets(dict(event))) candidate = strip_untrusted_usage_truth_metadata(candidate) candidate = strip_untrusted_instrumentation_marker_metadata(candidate) candidate = strip_client_context_provenance(candidate) @@ -750,7 +850,9 @@ def record_trusted_session_observation( local_session_observation_revision(row) == candidate_revision for row in same_identity ): - conflict_row = self._prepare_recorded_event(candidate) + conflict_row = self._prepare_recorded_event( + candidate, already_redacted=True + ) self._write_event_partition_unlocked( [*existing, conflict_row], preserved_unparseable ) @@ -762,7 +864,9 @@ def record_trusted_session_observation( else: authority = selected[0] if authority is candidate: - recorded = self._prepare_recorded_event(candidate) + recorded = self._prepare_recorded_event( + candidate, already_redacted=True + ) rewritten = [ row for row in existing @@ -884,7 +988,7 @@ def reconcile_trusted_session_observation_conflicts( if len(current_selected) != 1: continue replacements[key] = self._prepare_recorded_event( - current_selected[0] + current_selected[0], already_redacted=True ) if replacements: kept = [ @@ -1561,7 +1665,9 @@ def replace_events( continue seen_revisions.add(revision) conflict_rows.append( - self._prepare_recorded_event(candidate) + self._prepare_recorded_event( + candidate, already_redacted=True + ) ) if conflict_rows: self._write_event_partition_unlocked( @@ -1622,7 +1728,12 @@ def replace_events( for event in chosen ] recorded = [ - self._prepare_recorded_event(event) + # Observation rows arrive already redacted and stamped by + # _trusted_session_observation_candidate; every other lane + # is raw here and gets redacted on the way in. + self._prepare_recorded_event( + event, already_redacted=trusted_session_observation_import + ) for event in prepared_events ] self._write_event_partition_unlocked( diff --git a/src/agentacct/usage_truth.py b/src/agentacct/usage_truth.py index aee3d8c..93daa58 100644 --- a/src/agentacct/usage_truth.py +++ b/src/agentacct/usage_truth.py @@ -411,6 +411,16 @@ def usage_truth_table() -> list[dict[str, object]]: "evidenced_event_ids", "evidenced_event_id_total", "evidenced_outputs_skipped", + # Server-authored value-redaction provenance (service.py's + # RESERVED_VALUE_REDACTION_KEYS; spelled literally here because + # service imports this module, not the other way round, and a test + # pins the two lists together). An observation title or project path + # can carry a credential, and the redaction is applied before this + # rebuild -- without these the cut would happen silently, which is + # the exact failure the marker exists to prevent. A caller cannot + # forge them: redact_event_secrets strips caller copies first. + "value_redaction_applied", + "value_redaction_fields", } ) diff --git a/tests/test_secret_value_redaction.py b/tests/test_secret_value_redaction.py index 9a832ad..4f6973f 100644 --- a/tests/test_secret_value_redaction.py +++ b/tests/test_secret_value_redaction.py @@ -14,7 +14,14 @@ import json from pathlib import Path -from agentacct.service import SentinelService +from agentacct.service import ( + RESERVED_VALUE_REDACTION_KEYS, + SentinelService, +) +from agentacct.usage_truth import ( + is_local_session_observation_event, + mark_trusted_local_session_observation_event, +) # Built by concatenation so the fixtures are unmistakably synthetic while @@ -22,6 +29,41 @@ FAKE_API_KEY = "sk-" + "fakeonly" + "0" * 32 + "AbCd" FAKE_OPENROUTER_KEY = "sk-or-v1-" + "fakeonly" + "0" * 40 + "EfGh" FAKE_BEARER_HEADER = "Bearer " + "eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiJmYWtlIn0.c2lnbmF0dXJl" +# Credential alphabets that a fixed [A-Za-z0-9._~+/=-] charset cannot hold: +# percent-encoding and a basic-auth style colon pair. +FAKE_BEARER_OPAQUE = "Bearer " + "fake%3Auser:pass%2F" + "0" * 12 +FAKE_BEARER_BASE64 = "Bearer " + "ZmFrZS1vbmx5" + "0" * 20 + "Kw==" + +# The prose that the shipped discriminator destroyed: hyphenated engineering +# compounds, which are credential-shaped only if `-` counts as a credential +# character. +BEARER_PROSE = ( + "switch to Bearer token-based-authentication soon; " + "Bearer tokens-are-rotated-nightly by the job, and " + "we should use a Bearer token here." +) + + +def _session_observation(*, title: str) -> dict: + return { + "source": "codex-local-session-observation-import", + "event_type": "session_observed", + "run_id": "client_codex_redaction_marker", + "metadata": { + "client": "codex", + "client_session_id": "redaction-marker-session", + "client_session_kind": "root", + "source_namespace_fingerprint": "sha256:" + "b" * 64, + "project_dir": "/tmp/observed-project", + "started_at": 100.0, + "updated_at": 200.0, + "source_parse_complete": True, + "client_session_title": title, + "client_session_title_source": "explicit_client_title_field", + "client_session_title_sanitized": True, + "title_redacted": False, + }, + } def _stored_text(store: Path) -> str: @@ -240,3 +282,226 @@ def test_sensitive_key_names_still_lose_the_whole_value(tmp_path: Path) -> None: # not claim a value-pattern redaction. assert "value_redaction_applied" not in recorded["metadata"] assert FAKE_API_KEY not in _stored_text(store) + + +def test_a_bearer_credential_outside_the_token_charset_is_cut_whole(tmp_path: Path) -> None: + """A fixed credential alphabet leaked the whole key on the first `%`. + + "summary" is not a sensitive key NAME, so the value pattern is the only + defence here: with the charset stopping at `%`, the run after "Bearer " + was just "abc", fell under the length floor, and nothing matched at all. + """ + + store = tmp_path / "state" + service = SentinelService(store) + credential = "abc%2Fdefghijklmnopqrstuvwx" + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": f"curl failed: Authorization: Bearer {credential}"}, + } + ) + + assert recorded["metadata"]["summary"] == "curl failed: Authorization: [REDACTED_SECRET]" + assert credential not in _stored_text(store) + + +def test_hyphenated_prose_after_bearer_is_left_alone(tmp_path: Path) -> None: + """Counting `-` as credential-shaped shredded ordinary engineering prose.""" + + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "branch": "feat/bearer-token-based-authentication", + "metadata": {"summary": BEARER_PROSE}, + } + ) + + assert recorded["metadata"]["summary"] == BEARER_PROSE + assert recorded["branch"] == "feat/bearer-token-based-authentication" + assert "[REDACTED" not in _stored_text(store) + + +def test_every_credential_shaped_bearer_value_is_still_cut(tmp_path: Path) -> None: + """The other half of the prose rule: real credential shapes still go.""" + + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "jwt": FAKE_BEARER_HEADER, + "opaque": FAKE_BEARER_OPAQUE, + "base64": FAKE_BEARER_BASE64, + "header_line": f"Authorization: {FAKE_BEARER_HEADER}", + }, + } + ) + + assert recorded["metadata"]["jwt"] == "[REDACTED_SECRET]" + assert recorded["metadata"]["opaque"] == "[REDACTED_SECRET]" + assert recorded["metadata"]["base64"] == "[REDACTED_SECRET]" + assert recorded["metadata"]["header_line"] == "Authorization: [REDACTED_SECRET]" + stored = _stored_text(store) + for secret in (FAKE_BEARER_HEADER, FAKE_BEARER_OPAQUE, FAKE_BEARER_BASE64): + assert secret not in stored + + +def test_redacting_an_already_redacted_value_changes_nothing(tmp_path: Path) -> None: + """The widened run must not be able to eat its own placeholder.""" + + store = tmp_path / "state" + service = SentinelService(store) + once = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": f"header was {FAKE_BEARER_OPAQUE}"}, + } + ) + + twice = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": once["metadata"]["summary"]}, + } + ) + + assert twice["metadata"]["summary"] == once["metadata"]["summary"] + assert "value_redaction_applied" not in twice["metadata"] + + +def test_the_marker_never_names_a_field_the_server_reassigns(tmp_path: Path) -> None: + """event_id is overwritten, so a marker row naming it points at nothing.""" + + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "event_id": "caller-" + FAKE_API_KEY, + "metadata": {"summary": f"rotated {FAKE_API_KEY} today"}, + } + ) + + assert recorded["event_id"].startswith("evt_") + assert "[REDACTED_SECRET]" not in recorded["event_id"] + assert recorded["metadata"]["value_redaction_fields"] == [ + {"field": "metadata.summary", "pattern_class": "api_key"} + ] + assert FAKE_API_KEY not in _stored_text(store) + + +def test_a_reassigned_field_alone_leaves_no_marker_at_all(tmp_path: Path) -> None: + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "event_id": "caller-" + FAKE_API_KEY, + "metadata": {"summary": "nothing sensitive here"}, + } + ) + + assert recorded["event_id"].startswith("evt_") + assert "value_redaction_applied" not in recorded["metadata"] + assert "value_redaction_fields" not in recorded["metadata"] + + +def test_a_forged_top_level_redaction_marker_is_stripped(tmp_path: Path) -> None: + """The stamp lands top-level for off-contract metadata, so a caller can + plant it there too; that home is cleared like the metadata one.""" + + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "value_redaction_applied": True, + "value_redaction_fields": [ + {"field": "metadata.summary", "pattern_class": "api_key"} + ], + "metadata": {"summary": "nothing sensitive here"}, + } + ) + + assert "value_redaction_applied" not in recorded + assert "value_redaction_fields" not in recorded + assert recorded["metadata"]["reserved_value_redaction_provenance_stripped"] is True + + +def test_a_session_observation_says_that_its_title_was_cut(tmp_path: Path) -> None: + """A trusted observation used to lose the marker to the allowlist rebuild. + + The title was stored correctly redacted and nothing recorded that data had + been removed, which is the silent-cut failure the marker exists to prevent. + """ + + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + _session_observation(title=f"rotated {FAKE_API_KEY} today"), + trusted_session_observation_import=True, + ) + + assert recorded["metadata"]["client_session_title"] == "rotated [REDACTED_SECRET] today" + assert recorded["metadata"]["value_redaction_applied"] is True + assert recorded["metadata"]["value_redaction_fields"] == [ + {"field": "metadata.client_session_title", "pattern_class": "api_key"} + ] + # The marked row must still read back as a trusted observation: the lane + # rejects any metadata key outside its allowlist. + assert is_local_session_observation_event(recorded) + assert FAKE_API_KEY not in _stored_text(store) + + +def test_a_repeated_observation_import_is_still_one_revision(tmp_path: Path) -> None: + """The marker is part of the row, so it must be part of a stable digest.""" + + store = tmp_path / "state" + service = SentinelService(store) + observation = _session_observation(title=f"rotated {FAKE_API_KEY} today") + + first = service.record_event(dict(observation), trusted_session_observation_import=True) + second = service.record_event(dict(observation), trusted_session_observation_import=True) + + assert first["metadata"]["observation_revision"] == second["metadata"]["observation_revision"] + assert len(_stored_events(store)) == 1 + + +def test_the_observation_allowlist_keeps_server_authored_marker_keys() -> None: + """Unit guard for the two lists that have to agree across modules.""" + + marked = mark_trusted_local_session_observation_event( + { + **_session_observation(title="rotated [REDACTED_SECRET] today"), + "metadata": { + **_session_observation(title="rotated [REDACTED_SECRET] today")["metadata"], + "value_redaction_applied": True, + "value_redaction_fields": [ + {"field": "metadata.client_session_title", "pattern_class": "api_key"} + ], + }, + } + ) + + for key in RESERVED_VALUE_REDACTION_KEYS: + assert key in marked["metadata"] + assert is_local_session_observation_event(marked) From 8e4bfafeca1a4c828101737aa8e5dbe260e4395c Mon Sep 17 00:00:00 2001 From: mikehasa Date: Fri, 31 Jul 2026 02:07:29 +0800 Subject: [PATCH 3/8] fix(service): calibrate the bearer rule against the real ledger Three shipped bearer discriminators in a row destroyed ordinary engineering prose, each guessed rather than measured, each failing in the same direction. The last one required a DIGIT in the run after "Bearer" and asserted in a comment that "a digit is the one thing an English compound cannot contain" -- which is wrong for the exact vocabulary this ledger records: oauth2, sha256, base64, x509, rfc7519, http2. It cut "Bearer oauth2-client-credentials" and "debug Bearer sha256-hmac-signature", and newly mangled "Bearer $CI_API_TOKEN_V2" and "Bearer https://auth.example.com/v2/token". Calibrated instead against the maintainer's own store: 26773 distinct string values, 38951 distinct tokens, 8454 of them prose-shaped word compounds. The rule now refuses a shell variable reference, a URL, and any run that is entirely a word compound (words with an optional trailing version number joined by - _ . / = :), then accepts only a real Authorization header value, a >=16 character run carrying a separator no word compound reaches, a separator-free alphanumeric run of >=32, a UUID, or a run of >=10 consecutive digits. Measured zero false positives on all 26773 real values, all 8454 prose compounds spliced after "Bearer", and 49 adversarial phrases; zero misses on 13 credential shapes. The header branch captures the header NAME as `keep` and puts it back, so "Authorization: Bearer " loses the credential and not the word "Authorization". The comment now states the discriminator that is actually implemented and names the shapes it knowingly does not catch. Both sk- patterns were re-measured against the same corpus and match nothing in it, so they are unchanged. No existing test was edited. --- src/agentacct/service.py | 99 +++++++++++++--- tests/test_secret_value_redaction.py | 165 +++++++++++++++++++++++++++ 2 files changed, 250 insertions(+), 14 deletions(-) diff --git a/src/agentacct/service.py b/src/agentacct/service.py index 99bab4f..0ae69bf 100644 --- a/src/agentacct/service.py +++ b/src/agentacct/service.py @@ -165,7 +165,10 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # same literal string and merged into one phantom section. A quietly wrong # record is worse than the leak it was guarding against, so the patterns now # require a word boundary before `sk-` and something credential-shaped (rather -# than an English word) after `Bearer`. +# than an English word) after `Bearer`. Both `sk-` patterns were re-measured +# against the maintainer's ledger and match nothing in it: the word boundary is +# what keeps "task-", "disk-", "risk-" and the real ".../work-task-sessions" +# paths out, so they are left exactly as they are. # # The credential run after `Bearer` stays DELIMITER-based rather than a fixed # credential alphabet. Real credentials carry `%`, `:`, `|`, `@`, `#` and `&` @@ -173,23 +176,78 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # omits them stops at the first such character: "Bearer abc%2Fdefgh..." would # match only "abc", fall under the length floor, and write the whole credential # to the ledger. Only the characters that END a value in the formats we ingest -# are excluded -- whitespace, comma, semicolon, quotes, and the bracket/brace/ -# paren pairs -- which also makes replacement structurally idempotent, since -# SECRET_SPAN_PLACEHOLDER starts with `[` and so leaves an empty run behind. +# are excluded -- whitespace, comma, semicolon, quotes, angle brackets and the +# bracket/brace/paren pairs -- which also makes replacement structurally +# idempotent, since SECRET_SPAN_PLACEHOLDER starts with `[` and so leaves an +# empty run behind. `<` and `>` are in that exclusion set because they only ever +# appear around documentation placeholders ("Bearer "), never +# inside base64, base64url, hex or percent-encoding. # -# The discriminator is a DIGIT somewhere in that run. The false positive that -# matters is hyphenated engineering prose -- "Bearer token-based-authentication" -# and "Bearer tokens-are-rotated-nightly" were both destroyed while `-` counted -# as credential-shaped -- and a digit is the one thing an English compound -# cannot contain, while JWT, base64, hex and UUID credentials all do. The -# accepted residual is a >=16 character all-alphabetic credential, which the -# value patterns miss (a field NAMED like a credential still loses its whole -# value): a rare miss, deliberately preferred over routinely shredding prose. +# WHAT THE DISCRIMINATOR IS, AND WHY IT IS NOT A DIGIT TEST. Three earlier +# attempts guessed at this and each destroyed ordinary prose: any non-delimiter +# run matched "use a Bearer token here"; then a narrowed charset requiring a +# digit-or-separator matched "Bearer token-based-authentication"; then requiring +# a DIGIT matched "Bearer oauth2-client-credentials" and "Bearer +# sha256-hmac-signature", because the auth vocabulary this ledger records is +# full of digit-carrying words (oauth2, sha256, base64, x509, rfc7519, http2). +# A digit is emphatically NOT something an English compound cannot contain. +# +# The rule below was calibrated against the maintainer's own ledger (26773 +# distinct string values, 8454 of them prose-shaped word compounds) rather than +# guessed. A candidate span is refused when it is a shell variable reference +# (`$FOO`), when it is a URL, or when the whole run is a WORD COMPOUND -- words +# with an optional trailing version number, joined by `-`, `_`, `.`, `/`, `=` +# or `:`. That single structural refusal is what covers "oauth2-client- +# credentials", "x509-certificate-validation-chain", "header/credential", +# "token.rotation.policy" and "scope=read:write" alike. What survives the +# refusals is then accepted only in a known credential shape: it sits in a +# real `Authorization:`/`Proxy-Authorization:` header (captured as `keep`, so +# the header name is put back and only the credential goes), or it is >=16 chars +# and carries a separator no word compound reaches (`. / + = % : @ | # &`), or +# it is a separator-free alphanumeric run of >=32 characters (hex and unpadded +# base64), or it is a UUID. A >=10 digit run also qualifies: dates and version +# numbers in the real corpus top out at 8 digits (20260713), so ten consecutive +# digits is not something prose emits. +# +# KNOWN GAPS, stated rather than papered over. Outside a header we do not catch: +# a credential whose every `-._/=:`-separated piece happens to be letters with a +# trailing number (a real base64/hex token essentially never is, because those +# alphabets interleave digits mid-piece, but a short contrived one would slip); +# a separator-free alphabetic token of 16..31 characters; and anything after a +# literal `$`, so a credential really named `$TOKEN` is left alone deliberately. +# A field NAMED like a credential still loses its whole value via +# is_sensitive_metadata_key(), which is the backstop for all of these. +_BEARER_RUN = r"[^\s,;\"'\[\]{}()<>]" +_BEARER_SEPARATOR = r"[./+=%:@|#&]" +_BEARER_NOT_SHELL_OR_URL = r"(?!\$)(?![A-Za-z][A-Za-z0-9+.-]*://)" +# A word with an optional trailing version number, and a run of them joined by +# the characters English/technical compounds use. The trailing `(?!run)` makes +# the refusal apply only when the compound covers the WHOLE run, so a credential +# that merely starts word-like is not let through. +_BEARER_NOT_WORD_COMPOUND = ( + r"(?![A-Za-z]{1,32}[0-9]{0,4}(?:[-._/=:][A-Za-z]{1,32}[0-9]{0,4}){0,15}" + r"(?!" + _BEARER_RUN + r"))" +) +_BEARER_CREDENTIAL_SHAPE = ( + r"(?:(?=" + _BEARER_RUN + r"*(?:" + _BEARER_SEPARATOR + r"|[0-9]{10}))" + + _BEARER_RUN + r"{16,}" + r"|[A-Za-z0-9]{32,}" + r"|[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12})" +) SECRET_VALUE_PATTERNS = ( ( "bearer_token", re.compile( - r"\bBearer\s+(?=[^\s,;\"'\[\]{}()]*[0-9])[^\s,;\"'\[\]{}()]{16,}", + r"(?:(?P(?:Proxy-)?Authorization\s*[:=]\s*)Bearer\s+" + + _BEARER_NOT_SHELL_OR_URL + + _BEARER_NOT_WORD_COMPOUND + + _BEARER_RUN + + r"{16,}" + r"|\bBearer\s+" + + _BEARER_NOT_SHELL_OR_URL + + _BEARER_NOT_WORD_COMPOUND + + _BEARER_CREDENTIAL_SHAPE + + r")", re.IGNORECASE, ), ), @@ -247,12 +305,25 @@ def strip_value_redaction_provenance(event: dict[str, Any]) -> dict[str, Any]: return recorded +def _replace_secret_span(match: re.Match[str]) -> str: + """Swap in the placeholder, re-emitting any context the match had to read. + + A pattern may need leading context to decide that a span is a credential + without that context being part of the credential: "Authorization:" tells us + the run after "Bearer" is a real header value, but the header NAME is ledger + data and gets put back. Patterns opt in by capturing it as ``keep``. + """ + + keep = match.groupdict().get("keep") + return (keep or "") + SECRET_SPAN_PLACEHOLDER + + def _redact_secret_spans(text: str) -> tuple[str, tuple[str, ...]]: """Replace secret-shaped spans in ``text``, keeping everything else verbatim.""" matched: list[str] = [] for pattern_class, pattern in SECRET_VALUE_PATTERNS: - text, replacements = pattern.subn(SECRET_SPAN_PLACEHOLDER, text) + text, replacements = pattern.subn(_replace_secret_span, text) if replacements: matched.append(pattern_class) return text, tuple(matched) diff --git a/tests/test_secret_value_redaction.py b/tests/test_secret_value_redaction.py index 4f6973f..e2c81b3 100644 --- a/tests/test_secret_value_redaction.py +++ b/tests/test_secret_value_redaction.py @@ -43,6 +43,88 @@ "we should use a Bearer token here." ) +# The negative half of the calibration corpus. Every phrase here must come back +# out of record_event byte for byte. Grouped by which shipped rule ate it. +LEDGER_PROSE_CORPUS = ( + # round 1: any non-delimiter run after "Bearer" + "Bearer token", + "use a Bearer token here", + # round 2: narrowed charset requiring a digit or a separator + "Bearer token-based-authentication", + "Bearer tokens-are-rotated-nightly", + # round 3: requiring a digit -- the auth vocabulary is full of them + "Bearer oauth2-client-credentials migration", + "debug Bearer sha256-hmac-signature mismatch", + "Bearer x509-certificate-validation-chain rework", + "Bearer rfc7519-jwt-claims-validation is in scope", + "Bearer base64-url-encoded-payload decoding bug", + "Bearer http2-connection-reuse regression", + "Bearer sha512-digest-comparison helper", + # round 3 also newly mangled these two + "run with Bearer $CI_API_TOKEN_V2 in the header", + "see Bearer https://auth.example.com/v2/token for the endpoint", + # separators that a word compound reaches, so a bare separator test fails + "the Bearer header/credential split needs a doc note", + "Bearer token.rotation.policy documented", + "Bearer refresh_token_rotation is enabled", + "Bearer sso/saml2-assertion handoff", + "Bearer client.credentials.grant2 flow", + "Bearer jwt.decode.verify_signature=False was the bug", + "Bearer scope=read:write granted to the app", + "Bearer oauth2/openid-connect/discovery migration notes", + # documentation placeholders, which are angle-bracketed and not secrets + "Bearer ", + "Bearer ", + "Authorization: Bearer ", + "Authorization: Bearer token is required for all endpoints", + "the Authorization: Bearer header must be present on every request", + # shapes the real ledger is full of: section ids, run ids, branches, paths, + # snake_case identifiers, timestamps and Chinese narrative + "Bearer aethermoor-progress-audit-20260713 section id", + "Bearer eden-parent-plan-next-task-20260722 handoff", + "Bearer rollout-2026-07 window", + "Bearer phase-4-2-work-task-sessions rework", + "Bearer release-v1-2-3-candidate build", + "Bearer token expired at 2026-07-30T12:00:00Z", + "Bearer auth is configured in src/agentacct/service.py already", + "Bearer 认证令牌已经在昨天轮换完成，无需再次处理", + "Bearer token 的轮换策略：每晚一次", + "evidence_refreshable_usage_failed on claude/canonical-migration-4-work-task-sessions", + "silly-hellman-ccc6a2 and momentum-audit-8292ca490713ed37d8edae41 both replayed", + ".agent-sentinel/state/runs/run_20260717_023239_343b052e/report.md was regenerated", + # the "sk-" incident values + "task-2026-07-30-review", + "disk-usage-report-2026", + "risk-register-update-v2", + "brisk-refactor-of-the-store", + "the sk- prefix is what we match", + "src/agentacct/work-task-sessions-4801fa", +) + +# The positive half: one entry per credential shape the rule has to cut. All +# values are obviously fake and built by concatenation. +CREDENTIAL_CORPUS = ( + ("jwt_header_line", f"Authorization: {FAKE_BEARER_HEADER}"), + ( + "curl_header", + 'curl -H "Authorization: ' + FAKE_BEARER_HEADER + '" https://api.example.com/v1/ping', + ), + ("proxy_header_line", f"Proxy-Authorization: {FAKE_BEARER_HEADER}"), + ("authorization_equals", f"Authorization={FAKE_BEARER_BASE64}"), + ("opaque_percent_colon", FAKE_BEARER_OPAQUE), + ("base64_padded", FAKE_BEARER_BASE64), + ("hex_digest", "Bearer " + "0f1e2d3c4b5a6978" + "8796a5b4c3d2e1f0"), + ("uuid_token", "Bearer " + "3f9a1c72-5e4b-4c8d-9f21-7ab6d0e4c531"), + ("long_digit_run", "Bearer " + "local-redaction-placeholder-1234567890"), + ( + "opaque_alphabetic_in_header", + "Authorization: Bearer " + "RkFLRXNlc3Npb250b2tlbnZhbHVl", + ), + ("midsentence_token", f"rotated the {FAKE_BEARER_HEADER}"), + ("api_key", f"OPENAI_API_KEY={FAKE_API_KEY}"), + ("openrouter_key", f"OPENROUTER_API_KEY={FAKE_OPENROUTER_KEY}"), +) + def _session_observation(*, title: str) -> dict: return { @@ -356,6 +438,89 @@ def test_every_credential_shaped_bearer_value_is_still_cut(tmp_path: Path) -> No assert secret not in stored +def test_the_calibration_corpus_survives_the_bearer_rule_byte_for_byte( + tmp_path: Path, +) -> None: + """Prose the rule must never touch, sampled from a real 26773-value ledger. + + Three shipped bearer rules in a row destroyed ordinary engineering prose, + each time in the same direction, because each was guessed rather than + measured. These literals are the calibration set: the phrases every earlier + rule was caught on, plus the shapes the maintainer's own ledger is actually + full of (date-suffixed section ids, hashed run ids, snake_case identifiers, + paths, Chinese). Copied in as literals on purpose -- the test must not read + anybody's store. + """ + + store = tmp_path / "state" + service = SentinelService(store) + + for index, phrase in enumerate(LEDGER_PROSE_CORPUS): + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": phrase}, + } + ) + assert recorded["metadata"]["summary"] == phrase, (index, phrase) + assert "value_redaction_applied" not in recorded["metadata"], phrase + + assert "[REDACTED" not in _stored_text(store) + + +def test_every_calibration_credential_shape_is_cut_out_of_the_value( + tmp_path: Path, +) -> None: + """The other side of the same calibration: each leak shape still goes.""" + + store = tmp_path / "state" + service = SentinelService(store) + + for label, phrase in CREDENTIAL_CORPUS: + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": phrase}, + } + ) + summary = recorded["metadata"]["summary"] + assert "[REDACTED_SECRET]" in summary, (label, summary) + assert recorded["metadata"]["value_redaction_applied"] is True, label + + stored = _stored_text(store) + for label, phrase in CREDENTIAL_CORPUS: + secret = phrase.split()[-1] if label == "midsentence_token" else phrase + assert secret not in stored, label + + +def test_a_shell_variable_or_url_after_bearer_is_not_a_credential( + tmp_path: Path, +) -> None: + """Two shapes the digit rule newly mangled; both are references, not secrets. + + `$CI_API_TOKEN_V2` is the name of a secret, not the secret, and the endpoint + URL is public. Cutting either would lose ledger data and protect nothing. + """ + + store = tmp_path / "state" + service = SentinelService(store) + shell = "run with Bearer $CI_API_TOKEN_V2 in the header" + url = "see Bearer https://auth.example.com/v2/token for the endpoint" + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"shell": shell, "url": url}, + } + ) + + assert recorded["metadata"]["shell"] == shell + assert recorded["metadata"]["url"] == url + + def test_redacting_an_already_redacted_value_changes_nothing(tmp_path: Path) -> None: """The widened run must not be able to eat its own placeholder.""" From 2df8d1ffaddaf301a16edcdfecf2fdcf28a6f90c Mon Sep 17 00:00:00 2001 From: mikehasa Date: Fri, 31 Jul 2026 02:57:34 +0800 Subject: [PATCH 4/8] fix(service): cover prefixed provider tokens, stop cutting RFC prose Three defects in the shipped bearer rule, each measured against the maintainer's store rather than argued. A COVERAGE REGRESSION first. `ghp_...`, `gho_/ghu_/ghs_/ghr_`, `github_pat_`, `glpat-`, `AKIA`/`ASIA`, `xox[abprs]-` and `AIza` are a short letter prefix joined to an alphanumeric body, which is precisely the word-compound shape the prose refusal exists to protect, so after a bare "Bearer" they stopped being redacted -- while main and the previous commit on this branch both cut them, and while they remain the credentials most likely to actually leak. They are now matched ahead of the refusals. `AKIA`, `ASIA` and `AIza` are matched case-sensitively inside the otherwise case-insensitive pattern. A PROSE FALSE POSITIVE second. The word-compound recogniser only accepted pieces shaped letters-then-optional-digits, so any run with a bare numeric piece was read as a credential: "Bearer rfc6750/section-2.1 says the header is required" was cut, and RFC 6750 is the Bearer Token specification itself. Pieces may now also be a bare 1..4 digit number; the cap keeps the >=10-digit credential branch alive. The Authorization-header branch was not the backstop the comment implied, because the prose refusals were applied to it as well: "Authorization: Bearer abcdefgh-ijklmnop-qrstuvwx" was kept verbatim. Nobody writes an HTTP header in the middle of a sentence, so the refusals now stop at the bare-"Bearer" branch and the header branch takes any >=16 character run. And the previous commit's own honesty. It claimed zero false positives on "all 8454 prose compounds spliced after Bearer", but only the tokens that were ALREADY word compounds were tested in that position. Splicing the FULL distinct token set gives the real numbers, which are now in the comment: 26773 distinct string values (27401 counting object keys) and 43213 distinct tokens; zero of those strings is touched as it stands; splicing all 43213 after "Bearer " cuts 6252, of which 5306 are opaque-id shapes and paths the rule cannot tell from a credential, 74 are non-ASCII, digit-led or punctuation-led fragments, and 98 are genuinely prose-shaped. Those 98 are now named in KNOWN GAPS instead of rounded away, including "Bearer instrumentation." -- a long word whose trailing sentence period leaves an empty compound piece. Against the shipped HEAD the same measurement gives 155 prose-shaped hits, so the two rule changes heal 57 and introduce none. Measured through the shipped _redact_secret_spans: 0 whole-string false positives over 27401 strings, 0 new splice false positives from any of the three changes, 29 of 29 pinned credential shapes cut, 51 of 51 pinned prose phrases kept byte for byte, replacement still idempotent, worst case 1.97ms on a 20000-character run. Three tests added, one per fix, each failing without it. No existing test was edited. --- src/agentacct/service.py | 103 ++++++++++++++++------- tests/test_secret_value_redaction.py | 121 +++++++++++++++++++++++++++ 2 files changed, 194 insertions(+), 30 deletions(-) diff --git a/src/agentacct/service.py b/src/agentacct/service.py index 0ae69bf..04b9ed7 100644 --- a/src/agentacct/service.py +++ b/src/agentacct/service.py @@ -192,42 +192,85 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # full of digit-carrying words (oauth2, sha256, base64, x509, rfc7519, http2). # A digit is emphatically NOT something an English compound cannot contain. # -# The rule below was calibrated against the maintainer's own ledger (26773 -# distinct string values, 8454 of them prose-shaped word compounds) rather than -# guessed. A candidate span is refused when it is a shell variable reference -# (`$FOO`), when it is a URL, or when the whole run is a WORD COMPOUND -- words -# with an optional trailing version number, joined by `-`, `_`, `.`, `/`, `=` -# or `:`. That single structural refusal is what covers "oauth2-client- -# credentials", "x509-certificate-validation-chain", "header/credential", -# "token.rotation.policy" and "scope=read:write" alike. What survives the -# refusals is then accepted only in a known credential shape: it sits in a -# real `Authorization:`/`Proxy-Authorization:` header (captured as `keep`, so -# the header name is put back and only the credential goes), or it is >=16 chars -# and carries a separator no word compound reaches (`. / + = % : @ | # &`), or -# it is a separator-free alphanumeric run of >=32 characters (hex and unpadded -# base64), or it is a UUID. A >=10 digit run also qualifies: dates and version -# numbers in the real corpus top out at 8 digits (20260713), so ten consecutive -# digits is not something prose emits. +# The rule below was calibrated against the maintainer's own ledger rather than +# guessed. Outside a header a candidate span is refused when it is a shell +# variable reference (`$FOO`), when it is a URL, or when the whole run is a WORD +# COMPOUND -- pieces joined by `-`, `_`, `.`, `/`, `=` or `:`, each piece either +# a word with an optional trailing version number or a bare 1..4 digit number. +# That single structural refusal is what covers "oauth2-client-credentials", +# "x509-certificate-validation-chain", "header/credential", +# "token.rotation.policy", "scope=read:write" and "phase-4.2-work-task-sessions" +# alike. The bare-numeric piece is load-bearing: without it +# "Bearer rfc6750/section-2.1" was cut, and RFC 6750 IS the Bearer Token +# specification, so that is about the likeliest sentence in this ledger to +# follow the word. Four digits is the ceiling so the >=10-digit branch below +# still fires. What survives the refusals is then accepted only in a known +# credential shape: >=16 chars carrying a separator no word compound reaches +# (`. / + = % : @ | # &`), a separator-free alphanumeric run of >=32 characters +# (hex and unpadded base64), a UUID, or a run of >=10 digits -- dates and +# version numbers in the real corpus top out at 8 digits (20260713), so ten +# consecutive digits is not something prose emits. # -# KNOWN GAPS, stated rather than papered over. Outside a header we do not catch: -# a credential whose every `-._/=:`-separated piece happens to be letters with a -# trailing number (a real base64/hex token essentially never is, because those -# alphabets interleave digits mid-piece, but a short contrived one would slip); -# a separator-free alphabetic token of 16..31 characters; and anything after a +# The well-known provider token prefixes are matched BEFORE those refusals +# rather than after. `ghp_16C7...`, `glpat-...` and `AKIA...` read as word +# compounds -- short letter prefix, separator, alphanumeric body -- so the prose +# refusal swallowed them, and they are among the most commonly leaked +# credentials there are. `AKIA`, `ASIA` and `AIza` are matched case-sensitively +# inside the otherwise case-insensitive pattern, since only that exact casing is +# a real token prefix. +# +# Inside a real `Authorization:`/`Proxy-Authorization:` header NONE of the prose +# refusals apply: nobody writes an HTTP header in the middle of a sentence. That +# branch takes any >=16 character run, which is what makes it the backstop the +# rest of the rule leans on -- "Authorization: Bearer abcdefgh-ijklmnop-qrstuvwx" +# is a credential, not prose. The header name is captured as `keep` and put back, +# so only the credential goes. +# +# MEASURED against the maintainer's store: 26773 distinct string values (27401 +# distinct strings counting object keys) and 43213 distinct whitespace tokens. +# Not one of those strings is touched as it stands. Splicing every one of the +# 43213 tokens directly after "Bearer " -- deliberately harsher than reality, +# and the test the previous round ran only on the pre-filtered compound subset +# -- cuts 6252 of them: 2666 UUID or >=32-character alphanumeric runs and 2640 +# ids carrying an 8+ character hex run, which are opaque-id shapes this rule +# cannot tell from a credential and cuts on purpose; 774 filesystem paths; 74 +# non-ASCII, digit-led or punctuation-led fragments; and 98 that are genuinely +# prose-shaped. Alongside that, the 29 credential shapes pinned in +# tests/test_secret_value_redaction.py are all cut and its 51 prose phrases are +# all left byte for byte. +# +# KNOWN GAPS, stated rather than papered over. +# Not caught, outside a header: a credential whose every `-._/=:`-separated +# piece happens to be a word with a trailing number or a bare 1..4 digit number +# (a real base64/hex token essentially never is, because those alphabets +# interleave digits mid-piece, but a short contrived one would slip); a +# separator-free alphabetic token of 16..31 characters; and anything after a # literal `$`, so a credential really named `$TOKEN` is left alone deliberately. # A field NAMED like a credential still loses its whole value via # is_sensitive_metadata_key(), which is the backstop for all of these. +# Cut although it is prose -- the 98 above: a >=16 character technical +# identifier joined by a separator the compound recogniser does not accept +# ("off|shadow|authoritative", "agentacct==0.2.0"), and 16 that are one ordinary +# long word carrying trailing sentence punctuation, where the trailing `.` or +# `:` leaves an empty compound piece, so "Bearer instrumentation." is cut. Both +# want a wider compound recogniser rather than a wider refusal, and are left +# here as measured rather than rounded away. _BEARER_RUN = r"[^\s,;\"'\[\]{}()<>]" _BEARER_SEPARATOR = r"[./+=%:@|#&]" _BEARER_NOT_SHELL_OR_URL = r"(?!\$)(?![A-Za-z][A-Za-z0-9+.-]*://)" -# A word with an optional trailing version number, and a run of them joined by -# the characters English/technical compounds use. The trailing `(?!run)` makes -# the refusal apply only when the compound covers the WHOLE run, so a credential -# that merely starts word-like is not let through. +# A word with an optional trailing version number, or a bare version number, and +# a run of them joined by the characters English/technical compounds use. The +# trailing `(?!run)` makes the refusal apply only when the compound covers the +# WHOLE run, so a credential that merely starts word-like is not let through. +_BEARER_WORD_PIECE = r"(?:[A-Za-z]{1,32}[0-9]{0,4}|[0-9]{1,4})" _BEARER_NOT_WORD_COMPOUND = ( - r"(?![A-Za-z]{1,32}[0-9]{0,4}(?:[-._/=:][A-Za-z]{1,32}[0-9]{0,4}){0,15}" + r"(?![A-Za-z]{1,32}[0-9]{0,4}(?:[-._/=:]" + _BEARER_WORD_PIECE + r"){0,15}" r"(?!" + _BEARER_RUN + r"))" ) +_BEARER_PROVIDER_TOKEN = ( + r"(?:gh[pousr]_|github_pat_|glpat-|xox[abprs]-|(?-i:AKIA|ASIA|AIza))" + r"[A-Za-z0-9_-]{16,}" +) _BEARER_CREDENTIAL_SHAPE = ( r"(?:(?=" + _BEARER_RUN + r"*(?:" + _BEARER_SEPARATOR + r"|[0-9]{10}))" + _BEARER_RUN + r"{16,}" @@ -239,15 +282,15 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: "bearer_token", re.compile( r"(?:(?P(?:Proxy-)?Authorization\s*[:=]\s*)Bearer\s+" - + _BEARER_NOT_SHELL_OR_URL - + _BEARER_NOT_WORD_COMPOUND + _BEARER_RUN + r"{16,}" - r"|\bBearer\s+" + r"|\bBearer\s+(?:" + + _BEARER_PROVIDER_TOKEN + + r"|" + _BEARER_NOT_SHELL_OR_URL + _BEARER_NOT_WORD_COMPOUND + _BEARER_CREDENTIAL_SHAPE - + r")", + + r"))", re.IGNORECASE, ), ), diff --git a/tests/test_secret_value_redaction.py b/tests/test_secret_value_redaction.py index e2c81b3..84ae31d 100644 --- a/tests/test_secret_value_redaction.py +++ b/tests/test_secret_value_redaction.py @@ -126,6 +126,48 @@ ) +# Prefixed opaque provider tokens. Their own shape -- a short letter prefix, a +# separator, then an alphanumeric body -- reads as a word compound, so the prose +# refusal used to swallow them even though these are the most commonly leaked +# credentials in the world. Fake bodies, built by concatenation like the rest. +FAKE_PROVIDER_TOKENS = ( + ("github_personal", "ghp_" + "fakeonly" + "0" * 24 + "AbCd"), + ("github_oauth", "gho_" + "fakeonly" + "0" * 24 + "AbCd"), + ("github_user_to_server", "ghu_" + "fakeonly" + "0" * 24 + "AbCd"), + ("github_server_to_server", "ghs_" + "fakeonly" + "0" * 24 + "AbCd"), + ("github_refresh", "ghr_" + "fakeonly" + "0" * 24 + "AbCd"), + ("github_fine_grained", "github_pat_" + "fakeonly" + "0" * 24 + "AbCd"), + ("gitlab_personal", "glpat-" + "fakeonly" + "0" * 8 + "AbCd"), + ("aws_access_key", "AKIA" + "FAKEONLY" + "0" * 8), + ("aws_session_key", "ASIA" + "FAKEONLY" + "0" * 8), + ("slack_bot", "xoxb-" + "fakeonly" + "0" * 12 + "AbCd"), + ("slack_user", "xoxp-" + "fakeonly" + "0" * 12 + "AbCd"), + ("google_api", "AIza" + "fakeonly" + "0" * 24 + "AbCd"), +) + +# Prose whose compound carries a BARE NUMERIC piece. RFC 6750 is the Bearer +# Token specification itself, so the first line here is about the likeliest +# sentence in an engineering ledger to follow the word "Bearer"; the rest are +# shapes taken from the maintainer's own store. +BEARER_NUMERIC_PIECE_PROSE = ( + "Bearer rfc6750/section-2.1 says the header is required", + "Bearer phase-4.2-work-task-sessions rework", + "Bearer release/v1.2.3-candidate build", + "Bearer canonical-migration-4.3 notes", + "Bearer claude-4.5-sonnet-thinking was the model", + "Bearer activation.py:49 is where the check lives", +) + +# Runs that are word-shaped but sit inside a real Authorization header, where +# prose does not occur -- nobody writes an HTTP header in a sentence. +BEARER_HEADER_WORD_SHAPED = ( + ("hyphen_joined", "Authorization: Bearer ", "abcdefgh-ijklmnop-qrstuvwx"), + ("dot_joined", "Authorization: Bearer ", "abcdefgh.ijklmnop.qrstuvwx"), + ("proxy_hyphen_joined", "Proxy-Authorization: Bearer ", "abcdefgh-ijklmnop-qrstuvwx"), + ("equals_form", "Authorization=Bearer ", "abcdefgh-ijklmnop-qrstuvwx"), +) + + def _session_observation(*, title: str) -> dict: return { "source": "codex-local-session-observation-import", @@ -651,6 +693,85 @@ def test_a_repeated_observation_import_is_still_one_revision(tmp_path: Path) -> assert len(_stored_events(store)) == 1 +def test_a_prefixed_provider_token_after_a_bare_bearer_is_cut(tmp_path: Path) -> None: + """A coverage regression the word-compound refusal introduced. + + `ghp_...`, `glpat-...` and `AKIA...` are a letter prefix joined to an + alphanumeric body, which is exactly the shape the prose refusal exists to + protect, so they stopped being redacted outside an Authorization header -- + while remaining the credentials most likely to actually leak. + """ + + store = tmp_path / "state" + service = SentinelService(store) + + for label, token in FAKE_PROVIDER_TOKENS: + phrase = f"the job logged Bearer {token} to stdout" + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": phrase}, + } + ) + assert recorded["metadata"]["summary"] == ( + "the job logged [REDACTED_SECRET] to stdout" + ), label + assert recorded["metadata"]["value_redaction_applied"] is True, label + + stored = _stored_text(store) + for label, token in FAKE_PROVIDER_TOKENS: + assert token not in stored, label + + +def test_a_compound_with_a_bare_number_in_it_is_still_prose(tmp_path: Path) -> None: + """"Bearer rfc6750/section-2.1" is the Bearer spec, not a Bearer token.""" + + store = tmp_path / "state" + service = SentinelService(store) + + for phrase in BEARER_NUMERIC_PIECE_PROSE: + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": phrase}, + } + ) + assert recorded["metadata"]["summary"] == phrase, phrase + assert "value_redaction_applied" not in recorded["metadata"], phrase + + assert "[REDACTED" not in _stored_text(store) + + +def test_a_word_shaped_run_inside_an_authorization_header_is_a_credential( + tmp_path: Path, +) -> None: + """The header branch is the backstop, so the prose refusals stop at it. + + Applying the word-compound refusal inside `Authorization:` left real + hyphen- and dot-joined opaque tokens in the ledger verbatim, which is the + one place the surrounding text proves the run is a credential. + """ + + store = tmp_path / "state" + service = SentinelService(store) + + for label, header, credential in BEARER_HEADER_WORD_SHAPED: + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": f"curl sent {header}{credential}"}, + } + ) + expected_keep = header[: header.lower().index("bearer")] + assert recorded["metadata"]["summary"] == ( + f"curl sent {expected_keep}[REDACTED_SECRET]" + ), label + assert credential not in _stored_text(store), label + + def test_the_observation_allowlist_keeps_server_authored_marker_keys() -> None: """Unit guard for the two lists that have to agree across modules.""" From dfd293bb2792e6a93a1499707200c065707cd781 Mon Sep 17 00:00:00 2001 From: mikehasa Date: Fri, 31 Jul 2026 04:02:40 +0800 Subject: [PATCH 5/8] fix(service): split the secret patterns into three independent families One `Bearer`-anchored pattern was doing three jobs with three different precision requirements, so every guard added for one job silently disabled another: the prose refusal that saves "Bearer oauth2-client-credentials" also swallowed `ghp_...`, and applying it inside `Authorization:` kept a real hyphen-joined token verbatim. Four rounds of widen-then-narrow all landed on that one structure. It is now three separate compiled patterns that share no refusal, so adding a family, or a prefix to a family, cannot weaken another. 1. PREFIXED PROVIDER TOKENS, matched ANYWHERE in a value rather than only after "Bearer", with no prose refusal -- a vendor prefix plus a long body is not prose. Adds `hf_`, `r8_` and `tok_`, which main cut and which both earlier commits on this branch kept. Bodies are the vendor's alphabet rather than one shared permissive charset: `hf_`/`r8_`/`tok_` take no `_`, so "tok_rotation_is_documented_here" is not a credential, and `AKIA`/`ASIA` take exactly 16 uppercase alphanumerics with a boundary after, so "ASIAPACIFICREGIONCODE" is not one either. Both verified against the pattern. 2. HEADER CONTEXT, also with no prose refusal -- nobody writes an HTTP header mid-sentence. Now covers the JSON-serialized `{"Authorization": "Bearer ..."}` the reviewer found regressed, where the quote between the name and the colon ended the match and the credential was stored. The header NAME is still re-emitted and only the credential replaced. 3. BARE `Bearer ` IN PROSE, carrying the calibrated refusals unchanged. This family is allowed to miss things, because 1 and 2 now cover the realistic leak paths: a token pasted into a log or env line carries its own prefix, and a token in transit carries its header. THREE HONESTY DEFECTS, all of them verified before being fixed. KNOWN GAPS said the separator-free miss window was "16..31 characters". It is not: "Bearer " + "a"*32 is kept and "a"*33 is the first cut, and "Bearer " + "a"*32 + "2024" (36 chars) is kept, because the recogniser reads a run as ONE word when it is <=32 letters plus <=4 digits. The paragraph now states the rule and quotes the five strings that prove it, and test_the_separator_free_miss_window_is_where_known_gaps_says_it_is pins all five so it cannot drift off the code again. The provider-token fixtures pinned 3 of their 12 prefixes, not 12: 9 embedded "0" * 24 and passed through the generic >=10-digit branch, so deleting the provider family left them green. Measured, with the family removed: 3 of 12 failed. Every body is now purely alphabetic -- no 10-digit run, no >=32 alphanumeric run, no UUID, no `. / + = % : @ | # &` -- so each row can only be matched by its own prefix. With the family removed, 15 of 15 now fail. The residual breakdown could not be reproduced from the comment's own prose, so the sub-counts are gone. One re-derivable number replaces them, with the exact test written out: 24 of the 6252 spliced cuts are prose-shaped by `^[A-Za-z]+(?:[-'][A-Za-z]+)*[.,:;!?]?$`. The rest are named qualitatively and the reader is told to re-measure rather than quote. MEASURED through the shipped compiled patterns against the maintainer's store (6261 event lines; 26773 distinct string values, 27401 counting object keys, 43213 distinct whitespace tokens over the key-inclusive set), read-only: - Whole-string false positives: 0 of 26773. Also 0 at main, at 71bf462 and at 2df8d1f. - Splicing all 43213 tokens after "Bearer ": 6252 cut, and that set is EXACTLY the set 2df8d1f cuts -- set difference in both directions is empty. Family 3's set alone is likewise identical to 2df8d1f's single bearer_token set, which is the proof that the split neither weakened nor widened it. main cuts all 43213. - Marginal contribution of the two new families on the real corpus: 0 values and 0 spliced tokens each. They pay for themselves purely in shape coverage. - Credential-shape coverage, a 146-case catalogue over six serializations (bare Bearer, env/log line with no Bearer at all, header, JSON header, `Authorization=`, curl `-H`): 142 cut here, 128 at main, 114 at 2df8d1f, 62 at 71bf462. Nothing cut at 2df8d1f is missed here (0 regressions), and 18 cases cut here are missed at main -- every provider token with no Bearer word anywhere, which is how these actually leak. - Coverage below main in 4 of 146: a 20-letter run, a 28-letter run, and hyphen- and dot-joined 26-character opaque runs, each after a BARE "Bearer". All four are cut here in all five header serializations; main caught them only by cutting every run after the word, which is the incident this rule exists to undo. Named in KNOWN GAPS with that reason. - Idempotency: re-redacting every string any family touches is a no-op. - Backtracking: adversarial repetitions of each family's ambiguous shape (`Bearer a-1.` xN in a header, `Bearer abcd-abcd-...` xN, `ghp_` xN, nested `{"Authorization": ` xN) scale linearly to N=16000, worst 20 ms. Full suite: 2931 passed, 1 skipped. No existing test was edited to pass; the provider fixture bodies were strengthened, and three tests added. --- src/agentacct/service.py | 276 +++++++++++++++++---------- tests/test_secret_value_redaction.py | 141 ++++++++++++-- 2 files changed, 304 insertions(+), 113 deletions(-) diff --git a/src/agentacct/service.py b/src/agentacct/service.py index 04b9ed7..9db4ca2 100644 --- a/src/agentacct/service.py +++ b/src/agentacct/service.py @@ -151,40 +151,43 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: return recorded -# Secret shapes recognized inside ordinary string VALUES, ordered -# most-specific-first so the reported pattern class is the precise one. +# Secret shapes recognized inside ordinary string VALUES. # -# Both anchors are load-bearing. Unanchored, `sk-` matched the middle of -# ordinary English and ordinary identifiers -- "task-", "disk-", "risk-", -# "brisk-" all contain "sk-" -- and "Bearer" followed by any non-delimiter run -# matched the prose phrase "use a Bearer token here". Paired with the old -# whole-string replacement that silently destroyed real ledger values: a path -# ending -# ".../work-task-sessions-4801fa" was stored as a bare placeholder, and -# unrelated section ids that each tripped the pattern all collapsed onto the -# same literal string and merged into one phantom section. A quietly wrong -# record is worse than the leak it was guarding against, so the patterns now -# require a word boundary before `sk-` and something credential-shaped (rather -# than an English word) after `Bearer`. Both `sk-` patterns were re-measured -# against the maintainer's ledger and match nothing in it: the word boundary is -# what keeps "task-", "disk-", "risk-" and the real ".../work-task-sessions" -# paths out, so they are left exactly as they are. +# THREE INDEPENDENT FAMILIES, and the independence is the point. For four +# rounds one `Bearer`-anchored pattern did three jobs with three different +# precision requirements, so every guard added for one job silently disabled +# another: the prose refusal that saved "Bearer oauth2-client-credentials" also +# swallowed `ghp_...`, and applying it inside `Authorization:` kept a real +# hyphen-joined token verbatim. Each round fixed one and broke the next. They +# are now separate compiled patterns that share no refusal, so adding a family, +# or a prefix to a family, cannot weaken another one. # -# The credential run after `Bearer` stays DELIMITER-based rather than a fixed -# credential alphabet. Real credentials carry `%`, `:`, `|`, `@`, `#` and `&` -# (percent-encoding, basic-auth pairs, vendor prefixes), and a charset that -# omits them stops at the first such character: "Bearer abc%2Fdefgh..." would -# match only "abc", fall under the length floor, and write the whole credential -# to the ledger. Only the characters that END a value in the formats we ingest -# are excluded -- whitespace, comma, semicolon, quotes, angle brackets and the -# bracket/brace/paren pairs -- which also makes replacement structurally -# idempotent, since SECRET_SPAN_PLACEHOLDER starts with `[` and so leaves an -# empty run behind. `<` and `>` are in that exclusion set because they only ever -# appear around documentation placeholders ("Bearer "), never -# inside base64, base64url, hex or percent-encoding. +# 1. PREFIXED PROVIDER TOKENS -- self-identifying, context-free, matched +# anywhere in the value. No prose refusal: a vendor prefix plus a long body +# is not prose. (`sk-` and `sk-or-v1-` belong to this family too; they keep +# their own class names because the redaction marker is read downstream.) +# 2. HEADER CONTEXT -- `Authorization`/`Proxy-Authorization` plus a +# credential. No prose refusal: nobody writes an HTTP header mid-sentence. +# 3. BARE `Bearer ` IN PROSE -- the only genuinely ambiguous case, and +# the only one that carries the calibrated refusals. It is ALLOWED TO MISS +# THINGS, because families 1 and 2 now cover the realistic leak paths: a +# token pasted into a log or env line carries its own prefix, and a token +# in transit carries its header. Family 3 is the long tail, and buying a +# little more of that tail costs ledger prose, which is the trade this +# module already got wrong three times. # -# WHAT THE DISCRIMINATOR IS, AND WHY IT IS NOT A DIGIT TEST. Three earlier -# attempts guessed at this and each destroyed ordinary prose: any non-delimiter +# WHY ANCHORING AND SPAN REPLACEMENT ARE LOAD-BEARING. Unanchored, `sk-` matched +# the middle of ordinary English and ordinary identifiers -- "task-", "disk-", +# "risk-", "brisk-" all contain "sk-" -- and "Bearer" followed by any +# non-delimiter run matched the prose phrase "use a Bearer token here". Paired +# with the old whole-string replacement that silently destroyed real ledger +# values: a path ending ".../work-task-sessions-4801fa" was stored as a bare +# placeholder, and unrelated section ids that each tripped the pattern all +# collapsed onto the same literal string and merged into one phantom section. A +# quietly wrong record is worse than the leak it was guarding against. +# +# WHAT FAMILY 3'S DISCRIMINATOR IS, AND WHY IT IS NOT A DIGIT TEST. Three +# earlier attempts guessed and each destroyed ordinary prose: any non-delimiter # run matched "use a Bearer token here"; then a narrowed charset requiring a # digit-or-separator matched "Bearer token-based-authentication"; then requiring # a DIGIT matched "Bearer oauth2-client-credentials" and "Bearer @@ -192,70 +195,143 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # full of digit-carrying words (oauth2, sha256, base64, x509, rfc7519, http2). # A digit is emphatically NOT something an English compound cannot contain. # -# The rule below was calibrated against the maintainer's own ledger rather than -# guessed. Outside a header a candidate span is refused when it is a shell -# variable reference (`$FOO`), when it is a URL, or when the whole run is a WORD -# COMPOUND -- pieces joined by `-`, `_`, `.`, `/`, `=` or `:`, each piece either -# a word with an optional trailing version number or a bare 1..4 digit number. -# That single structural refusal is what covers "oauth2-client-credentials", +# The surviving rule was calibrated against the maintainer's own ledger rather +# than guessed. A candidate span is refused when it is a shell variable +# reference (`$FOO`), when it is a URL, or when the whole run is a WORD COMPOUND +# -- pieces joined by `-`, `_`, `.`, `/`, `=` or `:`, each piece either a word +# with an optional trailing version number or a bare 1..4 digit number. That one +# structural refusal covers "oauth2-client-credentials", # "x509-certificate-validation-chain", "header/credential", # "token.rotation.policy", "scope=read:write" and "phase-4.2-work-task-sessions" # alike. The bare-numeric piece is load-bearing: without it # "Bearer rfc6750/section-2.1" was cut, and RFC 6750 IS the Bearer Token # specification, so that is about the likeliest sentence in this ledger to -# follow the word. Four digits is the ceiling so the >=10-digit branch below -# still fires. What survives the refusals is then accepted only in a known -# credential shape: >=16 chars carrying a separator no word compound reaches +# follow the word. Four digits is the ceiling so the >=10-digit branch still +# fires. What survives the refusals is accepted only in a known credential +# shape: >=16 chars carrying a separator no word compound reaches # (`. / + = % : @ | # &`), a separator-free alphanumeric run of >=32 characters # (hex and unpadded base64), a UUID, or a run of >=10 digits -- dates and # version numbers in the real corpus top out at 8 digits (20260713), so ten # consecutive digits is not something prose emits. # -# The well-known provider token prefixes are matched BEFORE those refusals -# rather than after. `ghp_16C7...`, `glpat-...` and `AKIA...` read as word -# compounds -- short letter prefix, separator, alphanumeric body -- so the prose -# refusal swallowed them, and they are among the most commonly leaked -# credentials there are. `AKIA`, `ASIA` and `AIza` are matched case-sensitively -# inside the otherwise case-insensitive pattern, since only that exact casing is -# a real token prefix. -# -# Inside a real `Authorization:`/`Proxy-Authorization:` header NONE of the prose -# refusals apply: nobody writes an HTTP header in the middle of a sentence. That -# branch takes any >=16 character run, which is what makes it the backstop the -# rest of the rule leans on -- "Authorization: Bearer abcdefgh-ijklmnop-qrstuvwx" -# is a credential, not prose. The header name is captured as `keep` and put back, -# so only the credential goes. +# MEASURED, through these compiled patterns, against the maintainer's store +# (6261 event lines; 26773 distinct string values, 27401 distinct strings +# counting object keys, 43213 distinct whitespace tokens over that +# key-inclusive set). +# - Whole-string false positives: ZERO. Not one of the 26773 values is +# altered. Also zero at main and at both earlier commits on this branch. +# - Splice false positives, every one of the 43213 tokens placed directly +# after "Bearer " -- deliberately harsher than reality: 6252 cut. That set +# is EXACTLY the set the previous commit cut, element for element: the +# restructure added nothing and removed nothing on real data. main cuts all +# 43213, because main cuts any non-delimiter run after the word. +# - Marginal contribution of the two new families on the real corpus: zero +# values and zero spliced tokens each. They exist to catch credential +# shapes, and they cost nothing in prose. +# - Idempotency: re-redacting every string either family touches is a no-op. +# - Backtracking: adversarial repetitions of each family's ambiguous shape +# (`Bearer a-1.` x N inside a header, `Bearer abcd-abcd-...` x N, `ghp_` x N, +# nested `{"Authorization": ` x N) scale linearly to N=16000, worst 20 ms. +# - Credential-shape coverage, a 146-case catalogue of real token shapes in +# six serializations (bare Bearer, env/log line with no Bearer at all, +# header, JSON header, `Authorization=`, curl `-H`): 142 cut here, 128 at +# main, 114 at the previous commit. Nothing that any earlier revision cut is +# missed here except the four named below. # -# MEASURED against the maintainer's store: 26773 distinct string values (27401 -# distinct strings counting object keys) and 43213 distinct whitespace tokens. -# Not one of those strings is touched as it stands. Splicing every one of the -# 43213 tokens directly after "Bearer " -- deliberately harsher than reality, -# and the test the previous round ran only on the pre-filtered compound subset -# -- cuts 6252 of them: 2666 UUID or >=32-character alphanumeric runs and 2640 -# ids carrying an 8+ character hex run, which are opaque-id shapes this rule -# cannot tell from a credential and cuts on purpose; 774 filesystem paths; 74 -# non-ASCII, digit-led or punctuation-led fragments; and 98 that are genuinely -# prose-shaped. Alongside that, the 29 credential shapes pinned in -# tests/test_secret_value_redaction.py are all cut and its 51 prose phrases are -# all left byte for byte. +# The residue of the 6252 is NOT broken down further. The previous version of +# this comment gave sub-counts that no classifier written from its own prose +# could reproduce, which is its own kind of dishonesty. One re-derivable number +# instead: 24 of the 6252 are prose-shaped by the exact test +# `^[A-Za-z]+(?:[-'][A-Za-z]+)*[.,:;!?]?$` -- ordinary words, mostly carrying +# trailing sentence punctuation ("Bearer instrumentation." is cut, because the +# trailing `.` leaves an empty compound piece). The rest are opaque ids, hashes, +# paths and version strings this rule cannot tell from a credential and cuts on +# purpose. Anything finer should be re-measured, not quoted from here. The +# pinned corpora in tests/test_secret_value_redaction.py are the part a +# maintainer can re-derive without the store: every credential shape there is +# cut and every prose phrase there survives byte for byte. # # KNOWN GAPS, stated rather than papered over. -# Not caught, outside a header: a credential whose every `-._/=:`-separated -# piece happens to be a word with a trailing number or a bare 1..4 digit number -# (a real base64/hex token essentially never is, because those alphabets -# interleave digits mid-piece, but a short contrived one would slip); a -# separator-free alphabetic token of 16..31 characters; and anything after a -# literal `$`, so a credential really named `$TOKEN` is left alone deliberately. -# A field NAMED like a credential still loses its whole value via +# Outside a header, family 3 misses a run the word-compound recogniser reads as +# ONE word: at most 32 letters, optionally followed by at most 4 digits. +# Measured on this pattern: "Bearer " + "a"*32 is KEPT and "a"*33 is CUT; +# "Bearer " + "a"*32 + "2024" (36 chars) is KEPT and "a"*32 + "12345" is CUT. +# test_the_separator_free_miss_window_is_where_known_gaps_says_it_is pins those +# five strings, so this paragraph cannot drift off the code again. Four shapes +# in the coverage catalogue land in that window and are cut at main but not +# here: a 20-letter run, a 28-letter run, and hyphen- and dot-joined 26-char +# opaque runs, each after a BARE "Bearer". main caught them only by cutting +# every run after the word, which is the incident this rule exists to undo; all +# four are still cut in every header serialization, and a real base64/hex token +# essentially never lands in the window because those alphabets interleave +# digits mid-piece. Also not caught: anything after a literal `$`, so a +# credential really named `$TOKEN` is left alone deliberately; and any auth +# scheme other than `Bearer` in family 2 (`Basic`, `Token`), which main did not +# cut either. A field NAMED like a credential still loses its whole value via # is_sensitive_metadata_key(), which is the backstop for all of these. -# Cut although it is prose -- the 98 above: a >=16 character technical -# identifier joined by a separator the compound recogniser does not accept -# ("off|shadow|authoritative", "agentacct==0.2.0"), and 16 that are one ordinary -# long word carrying trailing sentence punctuation, where the trailing `.` or -# `:` leaves an empty compound piece, so "Bearer instrumentation." is cut. Both -# want a wider compound recogniser rather than a wider refusal, and are left -# here as measured rather than rounded away. +# The characters that can appear INSIDE a credential, shared by families 2 and +# 3: everything except the characters that END a value in the formats we ingest +# -- whitespace, comma, semicolon, quotes, angle brackets and the bracket/brace/ +# paren pairs. Excluding `[` is also what makes replacement structurally +# idempotent, since SECRET_SPAN_PLACEHOLDER starts with `[`. _BEARER_RUN = r"[^\s,;\"'\[\]{}()<>]" + +# --------------------------------------------------------------------------- +# FAMILY 1 -- prefixed provider tokens. +# +# Each alternative is a vendor prefix plus a body long enough that the pair is +# not something prose emits, so the family needs no surrounding context and no +# prose refusal: it matches ANYWHERE in a value, not only after "Bearer". The +# optional leading "Bearer " is consumed when it is there so the placeholder +# replaces the whole credential phrase rather than leaving a dangling word. +# +# Bodies are the vendor's real alphabet, deliberately NOT a shared permissive +# one. `hf_`, `r8_` and `tok_` take `[A-Za-z0-9]` with no `_`, which is what +# keeps them off ordinary snake_case identifiers; `AKIA`/`ASIA` take exactly 16 +# uppercase alphanumerics with a boundary after, which is what keeps them off +# SCREAMING_CASE words like "ASIAPACIFICREGION". `AKIA`, `ASIA` and `AIza` are +# matched case-sensitively inside the otherwise case-insensitive pattern, since +# only that exact casing is a real prefix. +_PROVIDER_TOKEN_ALTERNATIVES = ( + r"gh[pousr]_[A-Za-z0-9]{16,}", # GitHub personal/oauth/user/server/refresh + r"github_pat_[A-Za-z0-9_]{16,}", # GitHub fine-grained PAT + r"glpat-[A-Za-z0-9_-]{16,}", # GitLab personal access token + r"xox[abprs]-[A-Za-z0-9-]{12,}", # Slack bot/user/app/refresh tokens + r"hf_[A-Za-z0-9]{16,}", # Hugging Face + r"r8_[A-Za-z0-9]{16,}", # Replicate + r"tok_[A-Za-z0-9]{16,}", # vendor "tok_" opaque tokens + r"(?-i:AKIA|ASIA)[A-Z0-9]{16}(?![A-Za-z0-9])", # AWS access/session key id + r"(?-i:AIza)[A-Za-z0-9_-]{16,}", # Google API key +) +_PROVIDER_TOKEN_PATTERN = re.compile( + r"(?:\bBearer\s+)?\b(?:" + r"|".join(_PROVIDER_TOKEN_ALTERNATIVES) + r")", + re.IGNORECASE, +) + +# --------------------------------------------------------------------------- +# FAMILY 2 -- header context. +# +# `Authorization`/`Proxy-Authorization` followed by a credential, in the +# serializations this ledger actually stores: a bare header line, a `-H +# "Authorization: ..."` curl argument, an `Authorization=Bearer ...` form, and +# the JSON-serialized `{"Authorization": "Bearer ..."}`. The optional quotes +# around the separator are what make the JSON form work; without them the `"` +# between the name and the `:` ended the match and the credential was stored. +# +# No prose refusal applies here: nobody writes an HTTP header in the middle of +# a sentence, so any run of >=16 run-characters after "Bearer" is taken. The +# header NAME is ledger data, so it is captured as `keep` and put back; only the +# credential is replaced. +_AUTH_HEADER_NAME = r"(?:Proxy-)?Authorization" +_AUTH_HEADER_ASSIGN = r"[\"']?\s*[:=]\s*[\"']?" +_AUTHORIZATION_HEADER_PATTERN = re.compile( + r"(?P\b" + _AUTH_HEADER_NAME + _AUTH_HEADER_ASSIGN + r")" + r"Bearer\s+" + _BEARER_RUN + r"{16,}", + re.IGNORECASE, +) + +# --------------------------------------------------------------------------- +# FAMILY 3 -- a bare `Bearer ` sitting in prose. _BEARER_SEPARATOR = r"[./+=%:@|#&]" _BEARER_NOT_SHELL_OR_URL = r"(?!\$)(?![A-Za-z][A-Za-z0-9+.-]*://)" # A word with an optional trailing version number, or a bare version number, and @@ -267,33 +343,33 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: r"(?![A-Za-z]{1,32}[0-9]{0,4}(?:[-._/=:]" + _BEARER_WORD_PIECE + r"){0,15}" r"(?!" + _BEARER_RUN + r"))" ) -_BEARER_PROVIDER_TOKEN = ( - r"(?:gh[pousr]_|github_pat_|glpat-|xox[abprs]-|(?-i:AKIA|ASIA|AIza))" - r"[A-Za-z0-9_-]{16,}" -) _BEARER_CREDENTIAL_SHAPE = ( r"(?:(?=" + _BEARER_RUN + r"*(?:" + _BEARER_SEPARATOR + r"|[0-9]{10}))" + _BEARER_RUN + r"{16,}" r"|[A-Za-z0-9]{32,}" r"|[0-9A-Fa-f]{8}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{4}-[0-9A-Fa-f]{12})" ) +_BARE_BEARER_PATTERN = re.compile( + r"\bBearer\s+" + + _BEARER_NOT_SHELL_OR_URL + + _BEARER_NOT_WORD_COMPOUND + + _BEARER_CREDENTIAL_SHAPE, + re.IGNORECASE, +) + +# Applied in order; each pattern rewrites the text the next one sees. Most +# specific first. Where two families cover the same credential the order decides +# which class it is REPORTED as and how much surrounding context the placeholder +# swallows ("Bearer " and the header name are context, not credential) -- but +# never whether the credential goes, because each family alone already cuts it. SECRET_VALUE_PATTERNS = ( - ( - "bearer_token", - re.compile( - r"(?:(?P(?:Proxy-)?Authorization\s*[:=]\s*)Bearer\s+" - + _BEARER_RUN - + r"{16,}" - r"|\bBearer\s+(?:" - + _BEARER_PROVIDER_TOKEN - + r"|" - + _BEARER_NOT_SHELL_OR_URL - + _BEARER_NOT_WORD_COMPOUND - + _BEARER_CREDENTIAL_SHAPE - + r"))", - re.IGNORECASE, - ), - ), + ("provider_token", _PROVIDER_TOKEN_PATTERN), + ("authorization_header", _AUTHORIZATION_HEADER_PATTERN), + ("bearer_token", _BARE_BEARER_PATTERN), + # Family 1 members too -- self-identifying prefix, no context, no prose + # refusal. They keep their own class names because the redaction marker + # vocabulary is read downstream, and `sk-or-v1-` must be reported ahead of + # its own prefix-subset `sk-`. ("openrouter_api_key", re.compile(r"\bsk-or-v1-[A-Za-z0-9_-]{12,}")), ("api_key", re.compile(r"\bsk-[A-Za-z0-9_-]{12,}")), ) diff --git a/tests/test_secret_value_redaction.py b/tests/test_secret_value_redaction.py index 84ae31d..b3c677b 100644 --- a/tests/test_secret_value_redaction.py +++ b/tests/test_secret_value_redaction.py @@ -129,20 +129,34 @@ # Prefixed opaque provider tokens. Their own shape -- a short letter prefix, a # separator, then an alphanumeric body -- reads as a word compound, so the prose # refusal used to swallow them even though these are the most commonly leaked -# credentials in the world. Fake bodies, built by concatenation like the rest. +# credentials in the world. +# +# Every body here is PURELY ALPHABETIC (uppercase for AWS), which makes each row +# independently proving: no long digit run, no >=32-character alphanumeric run, +# no UUID and no `. / + = % : @ | # &` separator, so none of them can reach the +# generic credential-shape branch, and the word-compound refusal covers all of +# them. Delete the provider family and all 15 rows fail. The earlier fixtures +# embedded "0" * 24, so 9 of the 12 passed through the >=10-digit branch and +# pinned nothing about their own prefix. +_FAKE_BODY_24 = "FakeOnly" + "AbCdEfGh" + "IjKlMnOp" +_FAKE_BODY_20 = "FakeOnly" + "AbCdEfGh" + "IjKl" +_FAKE_BODY_16_UPPER = "FAKEONLY" + "FAKEONLY" FAKE_PROVIDER_TOKENS = ( - ("github_personal", "ghp_" + "fakeonly" + "0" * 24 + "AbCd"), - ("github_oauth", "gho_" + "fakeonly" + "0" * 24 + "AbCd"), - ("github_user_to_server", "ghu_" + "fakeonly" + "0" * 24 + "AbCd"), - ("github_server_to_server", "ghs_" + "fakeonly" + "0" * 24 + "AbCd"), - ("github_refresh", "ghr_" + "fakeonly" + "0" * 24 + "AbCd"), - ("github_fine_grained", "github_pat_" + "fakeonly" + "0" * 24 + "AbCd"), - ("gitlab_personal", "glpat-" + "fakeonly" + "0" * 8 + "AbCd"), - ("aws_access_key", "AKIA" + "FAKEONLY" + "0" * 8), - ("aws_session_key", "ASIA" + "FAKEONLY" + "0" * 8), - ("slack_bot", "xoxb-" + "fakeonly" + "0" * 12 + "AbCd"), - ("slack_user", "xoxp-" + "fakeonly" + "0" * 12 + "AbCd"), - ("google_api", "AIza" + "fakeonly" + "0" * 24 + "AbCd"), + ("github_personal", "ghp_" + _FAKE_BODY_24), + ("github_oauth", "gho_" + _FAKE_BODY_24), + ("github_user_to_server", "ghu_" + _FAKE_BODY_24), + ("github_server_to_server", "ghs_" + _FAKE_BODY_24), + ("github_refresh", "ghr_" + _FAKE_BODY_24), + ("github_fine_grained", "github_pat_" + _FAKE_BODY_24), + ("gitlab_personal", "glpat-" + _FAKE_BODY_20), + ("aws_access_key", "AKIA" + _FAKE_BODY_16_UPPER), + ("aws_session_key", "ASIA" + _FAKE_BODY_16_UPPER), + ("slack_bot", "xoxb-" + _FAKE_BODY_20), + ("slack_user", "xoxp-" + _FAKE_BODY_20), + ("google_api", "AIza" + _FAKE_BODY_24), + ("huggingface", "hf_" + _FAKE_BODY_20), + ("replicate", "r8_" + _FAKE_BODY_20), + ("vendor_opaque", "tok_" + _FAKE_BODY_20), ) # Prose whose compound carries a BARE NUMERIC piece. RFC 6750 is the Bearer @@ -724,6 +738,107 @@ def test_a_prefixed_provider_token_after_a_bare_bearer_is_cut(tmp_path: Path) -> assert token not in stored, label +def test_a_prefixed_provider_token_is_cut_with_no_bearer_word_at_all( + tmp_path: Path, +) -> None: + """Family 1 is context-free: the prefix alone is the evidence. + + While the provider prefixes lived inside the `Bearer`-anchored pattern, a + token pasted into an env line, a log line or a shell transcript -- which is + how they actually leak -- was stored verbatim. + """ + + store = tmp_path / "state" + service = SentinelService(store) + + for label, token in FAKE_PROVIDER_TOKENS: + phrase = f"export PROVIDER_TOKEN={token} && ./deploy.sh" + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": phrase}, + } + ) + assert recorded["metadata"]["summary"] == ( + "export PROVIDER_TOKEN=[REDACTED_SECRET] && ./deploy.sh" + ), label + assert recorded["metadata"]["value_redaction_fields"] == [ + {"field": "metadata.summary", "pattern_class": "provider_token"} + ], label + + stored = _stored_text(store) + for label, token in FAKE_PROVIDER_TOKENS: + assert token not in stored, label + + +def test_a_json_serialized_authorization_header_is_a_credential( + tmp_path: Path, +) -> None: + """The serialization this ledger stores most often, and it regressed once. + + `{"Authorization": "Bearer ..."}` puts a quote between the header name and + the colon, which ended the header match, so the branch fell through to the + prose rule and a hyphen-joined opaque token was kept verbatim. + """ + + store = tmp_path / "state" + service = SentinelService(store) + credential = "abcdefgh-ijklmnop-qrstuvwx" + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "json_body": '{"Authorization": "Bearer ' + credential + '"}', + "single_quoted": "{'Proxy-Authorization': 'Bearer " + credential + "'}", + "curl_arg": '-H "Authorization: Bearer ' + credential + '"', + }, + } + ) + + assert recorded["metadata"]["json_body"] == '{"Authorization": "[REDACTED_SECRET]"}' + assert recorded["metadata"]["single_quoted"] == ( + "{'Proxy-Authorization': '[REDACTED_SECRET]'}" + ) + assert recorded["metadata"]["curl_arg"] == '-H "Authorization: [REDACTED_SECRET]"' + assert credential not in _stored_text(store) + + +def test_the_separator_free_miss_window_is_where_known_gaps_says_it_is( + tmp_path: Path, +) -> None: + """Pin the KNOWN GAPS boundary so the comment cannot drift off the code. + + Outside a header the word-compound refusal reads a run as ONE word when it + is at most 32 letters, optionally followed by at most 4 digits. Everything + inside that window is kept; the first character past it is cut. The comment + quotes these exact strings, so a change to the recogniser fails here. + """ + + store = tmp_path / "state" + service = SentinelService(store) + kept = ("a" * 16, "a" * 32, "a" * 32 + "2024") + cut = ("a" * 33, "a" * 32 + "12345") + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + **{f"kept_{index}": f"Bearer {run}" for index, run in enumerate(kept)}, + **{f"cut_{index}": f"Bearer {run}" for index, run in enumerate(cut)}, + }, + } + ) + + for index, run in enumerate(kept): + assert recorded["metadata"][f"kept_{index}"] == f"Bearer {run}", run + for index, run in enumerate(cut): + assert recorded["metadata"][f"cut_{index}"] == "[REDACTED_SECRET]", run + + def test_a_compound_with_a_bare_number_in_it_is_still_prose(tmp_path: Path) -> None: """"Bearer rfc6750/section-2.1" is the Bearer spec, not a Bearer token.""" From be083b0c046d3598f72c93e5180c33b4742e5929 Mon Sep 17 00:00:00 2001 From: mikehasa Date: Fri, 31 Jul 2026 04:54:45 +0800 Subject: [PATCH 6/8] fix(service): correct the redaction comment's false claims, close two gaps Five review findings, all of them either a number the comment could not support or a coverage class it left unnamed. The >=10-digit branch was justified with "corpus digit runs top out at 8 digits". That is false: re.finditer(r"[0-9]+") over the snapshot's 27105 values finds 131131 runs, 414 of them >=10 digits, longest 19. Replaced with the branch's measured price -- deleting the digit alternative moves the spliced cut set 6405 -> 6178, so it alone accounts for 227 of 43545 spliced tokens, all opaque agent ids -- and whole-string stays 0 either way. Family 2 carried a >=16-character floor copied from family 3, so a short credential inside a real Authorization header was kept while main cut it (15 chars kept, 16 cut; pure digits kept at 8..15). Removing it outright is not safe: the pinned prose corpus contains "Authorization: Bearer token is required for all endpoints" and "the Authorization: Bearer header must be present on every request". The floor is now 8, the smallest value that clears every run in that corpus and closes the whole 8..15 window. Whole-string cuts stay 0 at every floor from 16 to 1; header-splice coverage goes 22445 -> 32709 of 43545. Family 1 promised that a token in a log or env line carries its own prefix, which was false for six vendors both main and HEAD kept in an env line, a log line and a URL query: sk_live_, sk_test_, npm_, pypi-, shpat_, sq0atp-. Added, each measured at 0 matches over the corpus first. pypi- is anchored on the macaroon body (AgE) because the bare prefix cuts the real ledger value "pypi-publish-and-install-smoke". KNOWN GAPS is now exhaustive against a 336-case catalogue scored on whether the credential is GONE, not on whether the string changed. That distinction found a gap the old wording would have hidden: a Key=Value;Key=Value connection string loses only its first pair, so its AccountKey survives even inside a header. Named as (c), with a test that asserts the secret is still there rather than pretending otherwise. The 20 ms ReDoS figure is gone; a wall-clock number does not reproduce. The claim is now linearity, with the measured doubling ratios and the sweep to re-run. Six new tests, no existing test edited. Suite 2937 passed, 1 skipped. --- src/agentacct/service.py | 219 ++++++++++++++++------ tests/test_secret_value_redaction.py | 268 +++++++++++++++++++++++++++ 2 files changed, 433 insertions(+), 54 deletions(-) diff --git a/src/agentacct/service.py b/src/agentacct/service.py index 9db4ca2..b11d017 100644 --- a/src/agentacct/service.py +++ b/src/agentacct/service.py @@ -167,14 +167,21 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # is not prose. (`sk-` and `sk-or-v1-` belong to this family too; they keep # their own class names because the redaction marker is read downstream.) # 2. HEADER CONTEXT -- `Authorization`/`Proxy-Authorization` plus a -# credential. No prose refusal: nobody writes an HTTP header mid-sentence. +# credential. It carries none of family 3's structural refusals, because +# the header name is already most of the evidence; it keeps only a length +# floor, and only because the ledger really does contain sentences ABOUT +# the header ("Authorization: Bearer token is required for all endpoints"). # 3. BARE `Bearer ` IN PROSE -- the only genuinely ambiguous case, and # the only one that carries the calibrated refusals. It is ALLOWED TO MISS -# THINGS, because families 1 and 2 now cover the realistic leak paths: a -# token pasted into a log or env line carries its own prefix, and a token -# in transit carries its header. Family 3 is the long tail, and buying a -# little more of that tail costs ledger prose, which is the trade this -# module already got wrong three times. +# THINGS, because families 1 and 2 cover the realistic leak paths: a token +# in transit carries its header, and a token pasted into a log or env line +# carries a vendor prefix IF that vendor is in family 1's list. That last +# clause is a real limit, not a formality -- family 1 is an enumeration, +# so a prefixed vendor it does not list (and any shape with no prefix at +# all: raw hex, base64, a JWT, a UUID) is NOT covered outside a header. +# Family 3 is the long tail, and buying a little more of that tail costs +# ledger prose, which is the trade this module already got wrong three +# times. # # WHY ANCHORING AND SPAN REPLACEMENT ARE LOAD-BEARING. Unanchored, `sk-` matched # the middle of ordinary English and ordinary identifiers -- "task-", "disk-", @@ -210,38 +217,69 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # fires. What survives the refusals is accepted only in a known credential # shape: >=16 chars carrying a separator no word compound reaches # (`. / + = % : @ | # &`), a separator-free alphanumeric run of >=32 characters -# (hex and unpadded base64), a UUID, or a run of >=10 digits -- dates and -# version numbers in the real corpus top out at 8 digits (20260713), so ten -# consecutive digits is not something prose emits. +# (hex and unpadded base64), a UUID, or a run of >=10 digits. # -# MEASURED, through these compiled patterns, against the maintainer's store -# (6261 event lines; 26773 distinct string values, 27401 distinct strings -# counting object keys, 43213 distinct whitespace tokens over that -# key-inclusive set). -# - Whole-string false positives: ZERO. Not one of the 26773 values is -# altered. Also zero at main and at both earlier commits on this branch. -# - Splice false positives, every one of the 43213 tokens placed directly -# after "Bearer " -- deliberately harsher than reality: 6252 cut. That set -# is EXACTLY the set the previous commit cut, element for element: the -# restructure added nothing and removed nothing on real data. main cuts all -# 43213, because main cuts any non-delimiter run after the word. +# THE >=10-DIGIT BRANCH IS JUSTIFIED BY ITS MEASURED COST, NOT BY A CLAIM ABOUT +# LONG NUMBERS. An earlier version of this comment said the corpus tops out at +# 8 digits, and that is simply false: `re.finditer(r"[0-9]+")` over the 27105 +# values finds 131131 digit runs, of which 414 are >=10 digits and the longest +# is 19 ("dashboard:finding:1964640965260215858fe13ee5214e66:0:mark_reviewed"). +# Standalone numbers are shorter but still exceed 8 -- of the 461 all-digit +# whitespace tokens the longest are 11 digits (14237112127) -- so no length +# threshold can be defended by asserting prose never reaches it. What can be +# defended is the branch's price. It is only a GATE into the >=16-character +# alternative, after the word-compound refusal has already run, so deleting +# `|[0-9]{10}` from the lookahead changes the corpus cut sets by: whole-string +# 0 -> 0, and spliced 6405 -> 6178. The branch is therefore solely responsible +# for 227 of 43545 spliced tokens, every one of them an opaque agent/session id +# of the "agent-a6033356814809200" shape -- the class this rule cuts on purpose +# because it cannot tell it from a credential. Re-derive by rebuilding +# _BEARER_CREDENTIAL_SHAPE without the digit alternative and diffing the two +# cut sets over the store. +# +# MEASURED, through these compiled patterns, against ONE SNAPSHOT of the +# maintainer's store (6325 event lines; 27105 distinct string values, 27733 +# distinct strings counting object keys, 43545 distinct whitespace tokens over +# that key-inclusive set). The store is live and grows, so a re-run will report +# different totals; the snapshot dimensions are quoted so the ratios below stay +# checkable. +# - Whole-string false positives: ZERO. Not one of the 27105 values is +# altered. Also zero at main and at all three earlier commits on this branch. +# - Splice false positives, every one of the 43545 tokens placed directly +# after "Bearer " -- deliberately harsher than reality: 6405 cut. That set +# is EXACTLY the set the two preceding commits on this branch cut, element +# for element (6405 at 2df8d1f and at dfd293b, 0 in either direction of the +# diff; 6539 one commit earlier at 8e4bfaf): neither the restructure nor +# this cleanup added or removed anything on real data. main cuts all 43545, +# because main cuts any non-delimiter run after the word. # - Marginal contribution of the two new families on the real corpus: zero # values and zero spliced tokens each. They exist to catch credential # shapes, and they cost nothing in prose. # - Idempotency: re-redacting every string either family touches is a no-op. # - Backtracking: adversarial repetitions of each family's ambiguous shape # (`Bearer a-1.` x N inside a header, `Bearer abcd-abcd-...` x N, `ghp_` x N, -# nested `{"Authorization": ` x N) scale linearly to N=16000, worst 20 ms. -# - Credential-shape coverage, a 146-case catalogue of real token shapes in -# six serializations (bare Bearer, env/log line with no Bearer at all, -# header, JSON header, `Authorization=`, curl `-H`): 142 cut here, 128 at -# main, 114 at the previous commit. Nothing that any earlier revision cut is -# missed here except the four named below. +# nested `{"Authorization": ` x N) are LINEAR in N, which is the claim worth +# making -- a millisecond figure here would only describe the machine that +# produced it. Measured by redacting all four shapes at N = 1000, 2000, 4000, +# 8000 and 16000 (best of three at each point) and comparing successive +# totals: each doubling of N multiplied the time by 2.10, 2.01, 2.05 and +# 1.90. Re-run that sweep rather than trusting any absolute number; a +# superlinear pattern shows up as a ratio that climbs with N, on any +# hardware. +# - Credential-shape coverage, a 336-case catalogue: 42 real token shapes in +# eight serializations (bare Bearer, env export line and `token:` log line +# with no Bearer at all, header line, JSON header, `Authorization=`, curl +# `-H`, and a URL query parameter). Scored on whether the CREDENTIAL IS +# GONE from the output, not on whether the string changed -- span +# replacement makes those different questions, and gap (c) below is exactly +# a case where the string changes and the secret stays. 274 covered here, +# 222 at main, 63 cases covered here that main leaves. 11 cases across the +# 7 shapes named below go the other way. # -# The residue of the 6252 is NOT broken down further. The previous version of +# The residue of the 6405 is NOT broken down further. The previous version of # this comment gave sub-counts that no classifier written from its own prose # could reproduce, which is its own kind of dishonesty. One re-derivable number -# instead: 24 of the 6252 are prose-shaped by the exact test +# instead: 24 of the 6405 are prose-shaped by the exact test # `^[A-Za-z]+(?:[-'][A-Za-z]+)*[.,:;!?]?$` -- ordinary words, mostly carrying # trailing sentence punctuation ("Bearer instrumentation." is cut, because the # trailing `.` leaves an empty compound piece). The rest are opaque ids, hashes, @@ -251,23 +289,52 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # maintainer can re-derive without the store: every credential shape there is # cut and every prose phrase there survives byte for byte. # -# KNOWN GAPS, stated rather than papered over. -# Outside a header, family 3 misses a run the word-compound recogniser reads as -# ONE word: at most 32 letters, optionally followed by at most 4 digits. -# Measured on this pattern: "Bearer " + "a"*32 is KEPT and "a"*33 is CUT; -# "Bearer " + "a"*32 + "2024" (36 chars) is KEPT and "a"*32 + "12345" is CUT. -# test_the_separator_free_miss_window_is_where_known_gaps_says_it_is pins those -# five strings, so this paragraph cannot drift off the code again. Four shapes -# in the coverage catalogue land in that window and are cut at main but not -# here: a 20-letter run, a 28-letter run, and hyphen- and dot-joined 26-char -# opaque runs, each after a BARE "Bearer". main caught them only by cutting -# every run after the word, which is the incident this rule exists to undo; all -# four are still cut in every header serialization, and a real base64/hex token -# essentially never lands in the window because those alphabets interleave -# digits mid-piece. Also not caught: anything after a literal `$`, so a -# credential really named `$TOKEN` is left alone deliberately; and any auth -# scheme other than `Bearer` in family 2 (`Basic`, `Token`), which main did not -# cut either. A field NAMED like a credential still loses its whole value via +# KNOWN GAPS, stated rather than papered over. This list is EXHAUSTIVE for the +# catalogue: 7 shapes in 11 cases are covered at main and not here, and all 7 +# are named below. Six of the seven are missed only after a BARE "Bearer" and +# are still cut in all four header serializations; (c) is the exception and a +# header does not save it. In the three serializations that carry no "Bearer" at +# all, main keeps all seven too, so none of this is a regression there. +# +# (a) THE SEPARATOR-FREE MISS WINDOW. Outside a header, family 3 misses a run +# the word-compound recogniser reads as ONE word: at most 32 letters, +# optionally followed by at most 4 digits. Measured: "Bearer " + "a"*32 is +# KEPT and "a"*33 is CUT; "Bearer " + "a"*32 + "2024" (36 chars) is KEPT +# and "a"*32 + "12345" is CUT. +# test_the_separator_free_miss_window_is_where_known_gaps_says_it_is pins +# those five strings, so this paragraph cannot drift off the code again. +# Four catalogue shapes land in the window: a 20-letter run, a 28-letter +# run, and hyphen- and dot-joined 26-char opaque runs. main caught them +# only by cutting every run after the word, which is the incident this rule +# exists to undo, and a real base64/hex token essentially never lands in +# the window because those alphabets interleave digits mid-piece. +# (b) A CREDENTIAL INSIDE A URL, e.g. "Bearer postgres://user:pw@host/db". +# Family 3 refuses anything matching `scheme://`, because ledger prose +# quotes URLs constantly. The password inside one is therefore kept after a +# bare "Bearer". Deliberate: relaxing it puts every quoted URL at risk. +# (c) A `Key=Value;Key=Value` CONNECTION STRING, e.g. the Azure +# "DefaultEndpointsProtocol=https;AccountName=...;AccountKey=..." shape. +# This is the ONE gap a header does not close, and the reason is span +# replacement rather than any refusal: `;` is not a run character, so the +# match ends at the first semicolon and only "DefaultEndpointsProtocol= +# https" is replaced. The AccountKey survives in ALL FIVE Bearer-carrying +# serializations. main destroyed the whole value instead, which is the +# behaviour this module deliberately abandoned. Deliberate, but the cost +# is real and it is not "still cut in a header" -- for this shape the only +# backstop is the key name. +# (d) ANYTHING AFTER A LITERAL `$`, so a credential really named `$TOKEN` is +# left alone rather than a shell variable reference being destroyed. +# (e) INSIDE A HEADER, a credential shorter than 8 run-characters, which is +# the price of family 2's floor: it exists so the ledger's own sentences +# about the header survive, and the same floor cuts a documentation word +# of 8 or more ("credentials" after "Authorization: Bearer" is replaced). +# Both directions are wrong somewhere; 8 is where the corpus put the line. +# Also not caught, and NOT a regression because main did not cut it either: any +# auth scheme other than `Bearer` in family 2 (`Basic`, `Token`); and any +# prefixed vendor family 1 does not enumerate, or any prefix-free shape (raw +# hex, base64, JWT, UUID, a bare 16-digit run), when it appears in an env or log +# line or a URL query with no `Bearer` and no `Authorization` anywhere near it. +# A field NAMED like a credential still loses its whole value via # is_sensitive_metadata_key(), which is the backstop for all of these. # The characters that can appear INSIDE a credential, shared by families 2 and # 3: everything except the characters that END a value in the formats we ingest @@ -286,12 +353,30 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # replaces the whole credential phrase rather than leaving a dangling word. # # Bodies are the vendor's real alphabet, deliberately NOT a shared permissive -# one. `hf_`, `r8_` and `tok_` take `[A-Za-z0-9]` with no `_`, which is what -# keeps them off ordinary snake_case identifiers; `AKIA`/`ASIA` take exactly 16 -# uppercase alphanumerics with a boundary after, which is what keeps them off -# SCREAMING_CASE words like "ASIAPACIFICREGION". `AKIA`, `ASIA` and `AIza` are +# one. `hf_`, `r8_`, `tok_`, `npm_`, `shpat_` and `sk_live_`/`sk_test_` take +# `[A-Za-z0-9]` with no `_`, which is what keeps them off ordinary snake_case +# identifiers; `AKIA`/`ASIA` take exactly 16 uppercase alphanumerics with a +# boundary after, which is what keeps them off SCREAMING_CASE words like +# "ASIAPACIFICREGION". `AKIA`, `ASIA`, `AIza` and the `AgE` of the PyPI body are # matched case-sensitively inside the otherwise case-insensitive pattern, since # only that exact casing is a real prefix. +# +# THE SIX PREFIXES ADDED LAST are the ones whose absence made a nonsense of the +# family's own promise: `sk_live_`/`sk_test_`, `npm_`, `pypi-`, `shpat_` and +# `sq0atp-` were kept by BOTH main and this module in an env export line, a +# `token:` log line and a URL query parameter -- exactly the "carries its own +# prefix" path family 1 exists to cover. Each is now cut in all three, and each +# was measured against the store before it was added: 0 matches among the 27105 +# values, so the coverage is bought with no false positives. This puts the +# module further above main, which is the intended direction. +# +# `pypi-` is anchored on the macaroon body, not on the bare prefix, and that is +# measured rather than assumed: `pypi-[A-Za-z0-9_-]{16,}` cuts the real ledger +# value "pypi-publish-and-install-smoke", while every PyPI upload token +# serializes a v2 macaroon whose base64 opens `AgE` ("AgEIcHlwaS5vcmc" for +# pypi.org, "AgENdGVzdC5weXBpLm9yZw" for test.pypi.org). Requiring `AgE` +# case-sensitively costs 0 of the 27105 corpus values; the bare-prefix form +# costs 1. _PROVIDER_TOKEN_ALTERNATIVES = ( r"gh[pousr]_[A-Za-z0-9]{16,}", # GitHub personal/oauth/user/server/refresh r"github_pat_[A-Za-z0-9_]{16,}", # GitHub fine-grained PAT @@ -300,6 +385,11 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: r"hf_[A-Za-z0-9]{16,}", # Hugging Face r"r8_[A-Za-z0-9]{16,}", # Replicate r"tok_[A-Za-z0-9]{16,}", # vendor "tok_" opaque tokens + r"sk_(?:live|test)_[A-Za-z0-9]{16,}", # Stripe secret key, live and test + r"npm_[A-Za-z0-9]{16,}", # npm access token + r"shpat_[A-Za-z0-9]{16,}", # Shopify admin API access token + r"sq0atp-[A-Za-z0-9_-]{16,}", # Square access token + r"(?-i:pypi-AgE)[A-Za-z0-9_-]{16,}", # PyPI upload token (macaroon body) r"(?-i:AKIA|ASIA)[A-Z0-9]{16}(?![A-Za-z0-9])", # AWS access/session key id r"(?-i:AIza)[A-Za-z0-9_-]{16,}", # Google API key ) @@ -318,15 +408,36 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # around the separator are what make the JSON form work; without them the `"` # between the name and the `:` ended the match and the credential was stored. # -# No prose refusal applies here: nobody writes an HTTP header in the middle of -# a sentence, so any run of >=16 run-characters after "Bearer" is taken. The -# header NAME is ledger data, so it is captured as `keep` and put back; only the -# credential is replaced. +# The floor is >=8, and it is 8 rather than 16 or 1 for reasons that were both +# measured. It used to be 16, copied from family 3, and in a header that only +# kept short credentials: `Authorization: Bearer a1b2c3d4e5f6g7h` (15) was kept +# while the same header at 16 was cut, and a pure-digit credential was kept at +# every length 8..15 -- main cut all of those. +# +# Removing the floor outright is what the comment above this one used to imply +# was safe, on the grounds that nobody writes an HTTP header mid-sentence. The +# ledger's own pinned prose corpus falsifies that: it contains +# "Authorization: Bearer token is required for all endpoints" and +# "the Authorization: Bearer header must be present on every request" -- English +# sentences ABOUT the header, whose runs after the word are "token" (5) and +# "header" (6). Documentation prose is the exception, so a floor stays, at the +# smallest value that both clears every run in that corpus and closes the whole +# 8..15 window: 8. The cost of the residual is stated in KNOWN GAPS (e). +# +# Nothing else moves. Whole-string cuts over the 27105 corpus values are 0 with +# the floor at 16, 8, 4, 2 or 1, and the bare-"Bearer " splice count stays at +# 6405 at every one of them because this family never fires without a header. +# What the change buys is header coverage: splicing each of the 43545 corpus +# tokens after "Authorization: Bearer " cuts 22445 at floor 16, 32709 at 8, and +# 42634 with no floor at all. +# +# The header NAME is ledger data, so it is captured as `keep` and put back; only +# the credential is replaced. _AUTH_HEADER_NAME = r"(?:Proxy-)?Authorization" _AUTH_HEADER_ASSIGN = r"[\"']?\s*[:=]\s*[\"']?" _AUTHORIZATION_HEADER_PATTERN = re.compile( r"(?P\b" + _AUTH_HEADER_NAME + _AUTH_HEADER_ASSIGN + r")" - r"Bearer\s+" + _BEARER_RUN + r"{16,}", + r"Bearer\s+" + _BEARER_RUN + r"{8,}", re.IGNORECASE, ) diff --git a/tests/test_secret_value_redaction.py b/tests/test_secret_value_redaction.py index b3c677b..8468ed2 100644 --- a/tests/test_secret_value_redaction.py +++ b/tests/test_secret_value_redaction.py @@ -181,6 +181,53 @@ ("equals_form", "Authorization=Bearer ", "abcdefgh-ijklmnop-qrstuvwx"), ) +# Family 2 used to carry a >=16-character floor copied from family 3, so a SHORT +# credential inside a real header was kept while main cut it. The floor is now 8, +# and both sides of that boundary are pinned because both are load-bearing: 8..15 +# is the window this closed, and 1..7 stays open on purpose so that the ledger's +# own sentences ABOUT the header ("Authorization: Bearer token is required...", +# run "token", 5 characters) survive. LEDGER_PROSE_CORPUS is where those live. +_SHORT_HEADER_MIXED = "a1b2c3d4e5f6g7h" +HEADER_CREDENTIALS_AT_OR_ABOVE_FLOOR = tuple( + _SHORT_HEADER_MIXED[:length] for length in range(8, 16) +) + tuple("1" * length for length in range(8, 16)) +HEADER_RUNS_BELOW_FLOOR = tuple(_SHORT_HEADER_MIXED[:length] for length in range(1, 8)) + +# Vendor prefixes added after the reviewer found family 1's own promise -- "a +# token pasted into a log or env line carries its own prefix" -- was false for +# them: BOTH main and this module kept every row below in an env line, a log line +# and a URL query. Bodies are purely alphabetic (plus the fixed PyPI macaroon +# opener) so each row is proven by its prefix alone, not by a digit run or a +# >=32-character alphanumeric run. +_FAKE_BODY_32 = "FakeOnly" + "AbCdEfGh" + "IjKlMnOp" + "QrStUvWx" +FAKE_LATE_PROVIDER_TOKENS = ( + ("stripe_live", "sk_live_" + _FAKE_BODY_24), + ("stripe_test", "sk_test_" + _FAKE_BODY_24), + ("npm_access", "npm_" + _FAKE_BODY_32), + ("pypi_upload", "pypi-AgE" + _FAKE_BODY_32), + ("shopify_admin", "shpat_" + _FAKE_BODY_24), + ("square_access", "sq0atp-" + _FAKE_BODY_20), +) + +# The `pypi-` prefix alone is NOT enough evidence: this is a real value from the +# maintainer's store, and `pypi-[A-Za-z0-9_-]{16,}` would destroy it. Anchoring +# on the macaroon body (`AgE`) is what keeps it. +PYPI_SHAPED_LEDGER_PROSE = "pypi-publish-and-install-smoke" + +# KNOWN GAPS (b): a credential inside a URL is kept after a BARE "Bearer", +# because the URL refusal that protects the quoted URLs this ledger is full of +# fires first. A header still catches it. +URL_EMBEDDED_CREDENTIAL = "postgres://user:s3cr3tpassw0rdvalue@db.example.com:5432/app" + +# KNOWN GAPS (c): the one gap a header does NOT close. `;` is not a run +# character, so the match ends at the first semicolon and the AccountKey after it +# survives in every serialization. +CONNECTION_STRING_SECRET = _FAKE_BODY_32 +CONNECTION_STRING = ( + "DefaultEndpointsProtocol=https;AccountName=acctname;AccountKey=" + + CONNECTION_STRING_SECRET +) + def _session_observation(*, title: str) -> dict: return { @@ -887,6 +934,227 @@ def test_a_word_shaped_run_inside_an_authorization_header_is_a_credential( assert credential not in _stored_text(store), label +def test_a_short_credential_inside_a_header_is_still_a_credential( + tmp_path: Path, +) -> None: + """Family 2's floor is 8, not the 16 it inherited from family 3. + + At 16 the header branch kept short credentials that main cut: + `Authorization: Bearer a1b2c3d4e5f6g7h` (15) was stored verbatim while the + same header at 16 was cut, and a pure-digit credential survived at every + length 8..15. Those are the strings pinned here. + """ + + store = tmp_path / "state" + service = SentinelService(store) + + for credential in HEADER_CREDENTIALS_AT_OR_ABOVE_FLOOR: + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "header": f"Authorization: Bearer {credential}", + "proxy": f"Proxy-Authorization: Bearer {credential}", + "json_body": '{"Authorization": "Bearer ' + credential + '"}', + "equals_form": f"Authorization=Bearer {credential}", + }, + } + ) + assert recorded["metadata"]["header"] == "Authorization: [REDACTED_SECRET]", credential + assert recorded["metadata"]["proxy"] == ( + "Proxy-Authorization: [REDACTED_SECRET]" + ), credential + assert recorded["metadata"]["json_body"] == ( + '{"Authorization": "[REDACTED_SECRET]"}' + ), credential + assert recorded["metadata"]["equals_form"] == ( + "Authorization=[REDACTED_SECRET]" + ), credential + assert recorded["metadata"]["value_redaction_applied"] is True, credential + + +def test_the_header_floor_still_stops_below_eight_characters(tmp_path: Path) -> None: + """The other side of the boundary, which is why the floor is not zero. + + The pinned prose corpus contains sentences ABOUT the header whose run after + the word is "token" (5) and "header" (6). Removing the floor outright cuts + those, so 1..7 stays open deliberately -- and the same runs after a BARE + "Bearer", with no header name in front of them, stay open at every length. + """ + + store = tmp_path / "state" + service = SentinelService(store) + + for run in HEADER_RUNS_BELOW_FLOOR: + header = f"Authorization: Bearer {run}" + bare = f"Bearer {run}" + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"header": header, "bare": bare}, + } + ) + assert recorded["metadata"]["header"] == header, run + assert recorded["metadata"]["bare"] == bare, run + assert "value_redaction_applied" not in recorded["metadata"], run + + for credential in HEADER_CREDENTIALS_AT_OR_ABOVE_FLOOR: + phrase = f"Bearer {credential}" + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": {"summary": phrase}, + } + ) + assert recorded["metadata"]["summary"] == phrase, credential + assert "value_redaction_applied" not in recorded["metadata"], credential + + +def test_the_late_vendor_prefixes_are_cut_with_no_bearer_word_at_all( + tmp_path: Path, +) -> None: + """The leak paths family 1 claims to cover, for the vendors it had missed. + + Each of these was kept by BOTH main and the previous revision in an env + export line, a `token:` log line and a URL query parameter, which made the + family's own "carries its own prefix" promise false for them. + """ + + store = tmp_path / "state" + service = SentinelService(store) + + for label, token in FAKE_LATE_PROVIDER_TOKENS: + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "env_line": f'export SERVICE_TOKEN="{token}"', + "log_line": f"token: {token}", + "url_query": f"https://api.example.com/v1/ping?access_token={token}", + }, + } + ) + assert recorded["metadata"]["env_line"] == ( + 'export SERVICE_TOKEN="[REDACTED_SECRET]"' + ), label + assert recorded["metadata"]["log_line"] == "token: [REDACTED_SECRET]", label + assert recorded["metadata"]["url_query"] == ( + "https://api.example.com/v1/ping?access_token=[REDACTED_SECRET]" + ), label + assert {"field": "metadata.env_line", "pattern_class": "provider_token"} in ( + recorded["metadata"]["value_redaction_fields"] + ), label + + stored = _stored_text(store) + for label, token in FAKE_LATE_PROVIDER_TOKENS: + assert token not in stored, label + + +def test_the_pypi_prefix_is_anchored_on_the_macaroon_not_on_the_word( + tmp_path: Path, +) -> None: + """`pypi-` alone is a hyphenated English compound this ledger really writes. + + A bare `pypi-[A-Za-z0-9_-]{16,}` alternative would cut the real store value + pinned here, so the alternative requires the `AgE` that opens every PyPI + upload token's serialized macaroon. + """ + + store = tmp_path / "state" + service = SentinelService(store) + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "workflow": f"ran {PYPI_SHAPED_LEDGER_PROSE} on the release tag", + "lowercase_lookalike": "pypi-agentacct-publish-and-verify-step", + }, + } + ) + + assert recorded["metadata"]["workflow"] == ( + f"ran {PYPI_SHAPED_LEDGER_PROSE} on the release tag" + ) + assert recorded["metadata"]["lowercase_lookalike"] == ( + "pypi-agentacct-publish-and-verify-step" + ) + assert "value_redaction_applied" not in recorded["metadata"] + + +def test_a_credential_in_a_url_is_kept_after_a_bare_bearer_and_cut_in_a_header( + tmp_path: Path, +) -> None: + """Pin KNOWN GAPS (b) on both sides, so neither half can drift. + + Family 3 refuses anything matching `scheme://` because ledger prose quotes + URLs constantly, so a password inside one survives a bare "Bearer" that main + would have cut. The header serializations are where it is still caught, and + that half is what makes the gap acceptable. + """ + + store = tmp_path / "state" + service = SentinelService(store) + credential = URL_EMBEDDED_CREDENTIAL + + recorded = service.record_event( + { + "source": "test", + "event_type": "note", + "metadata": { + "bare": f"Bearer {credential}", + "header": f"Authorization: Bearer {credential}", + "proxy": f"Proxy-Authorization: Bearer {credential}", + "json_body": '{"Authorization": "Bearer ' + credential + '"}', + "equals_form": f"Authorization=Bearer {credential}", + }, + } + ) + + assert recorded["metadata"]["bare"] == f"Bearer {credential}" + assert recorded["metadata"]["header"] == "Authorization: [REDACTED_SECRET]" + assert recorded["metadata"]["proxy"] == "Proxy-Authorization: [REDACTED_SECRET]" + assert recorded["metadata"]["json_body"] == '{"Authorization": "[REDACTED_SECRET]"}' + assert recorded["metadata"]["equals_form"] == "Authorization=[REDACTED_SECRET]" + + +def test_a_connection_string_keeps_its_account_key_even_inside_a_header( + tmp_path: Path, +) -> None: + """Pin KNOWN GAPS (c), the one gap a header does NOT close. + + `;` is not a run character, so the match ends at the first semicolon and + only "DefaultEndpointsProtocol=https" is replaced. The AccountKey after it + survives in every serialization, including the header ones. main erased the + whole value instead. This test exists so the comment cannot quietly claim + the header covers this shape too: it does not, and the honest backstop is + is_sensitive_metadata_key() on the field name. + """ + + store = tmp_path / "state" + service = SentinelService(store) + phrases = { + "bare": f"Bearer {CONNECTION_STRING}", + "header": f"Authorization: Bearer {CONNECTION_STRING}", + "json_body": '{"Authorization": "Bearer ' + CONNECTION_STRING + '"}', + "equals_form": f"Authorization=Bearer {CONNECTION_STRING}", + } + + recorded = service.record_event( + {"source": "test", "event_type": "note", "metadata": dict(phrases)} + ) + + assert recorded["metadata"]["bare"] == phrases["bare"] + for field in ("header", "json_body", "equals_form"): + assert CONNECTION_STRING_SECRET in recorded["metadata"][field], field + assert CONNECTION_STRING_SECRET in _stored_text(store) + + def test_the_observation_allowlist_keeps_server_authored_marker_keys() -> None: """Unit guard for the two lists that have to agree across modules.""" From 99842d1b4394898f961c0ef8b25afd8340bddf0f Mon Sep 17 00:00:00 2001 From: mikehasa Date: Fri, 31 Jul 2026 05:31:42 +0800 Subject: [PATCH 7/8] fix(service): stop claiming the known-gaps list is exhaustive A reviewer's independently built catalogue immediately found vendor formats this module keeps and main cut, which is what an enumeration of vendor prefixes will always do. Three corrections, no behaviour guesswork: - Drop the "EXHAUSTIVE for the catalogue" claim. The gaps paragraph now says what shapes are missed and why, plus how to re-derive the set for a given catalogue, instead of asserting a census it cannot support. - Gap (a) quoted only the single-piece bound (32 letters + 4 digits) and so understated the miss window by an unbounded factor: a compound of such pieces has no ceiling, and a 101-character hyphen-joined run is kept. - The preamble said six of seven gaps are still cut in a header. (d) is not: "$TOKEN" is 6 run-characters and family 2's floor is 8. Corrected, and gap (c) no longer calls itself the only header-proof gap. Also adds lin_api_, dop_v1_ and rk_live_/rk_test_ to family 1, each measured at 0 false positives over the 27739 distinct strings in the real store. --- src/agentacct/service.py | 60 +++++++++++++++++++--------- tests/test_secret_value_redaction.py | 4 ++ 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/src/agentacct/service.py b/src/agentacct/service.py index b11d017..9d407c6 100644 --- a/src/agentacct/service.py +++ b/src/agentacct/service.py @@ -289,32 +289,51 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: # maintainer can re-derive without the store: every credential shape there is # cut and every prose phrase there survives byte for byte. # -# KNOWN GAPS, stated rather than papered over. This list is EXHAUSTIVE for the -# catalogue: 7 shapes in 11 cases are covered at main and not here, and all 7 -# are named below. Six of the seven are missed only after a BARE "Bearer" and -# are still cut in all four header serializations; (c) is the exception and a -# header does not save it. In the three serializations that carry no "Bearer" at -# all, main keeps all seven too, so none of this is a regression there. +# KNOWN GAPS, stated rather than papered over -- and NOT claimed to be +# exhaustive. An earlier version of this paragraph said the list was exhaustive +# "for the catalogue", which is true only of the catalogue that produced it: a +# reviewer's independently built 45-shape catalogue immediately found vendor +# formats this module keeps and main cut (Linear `lin_api_` among them, now +# added to family 1). That is not a coincidence to be patched away. Family 1 is +# an ENUMERATION of vendor prefixes, so the set of things it misses is exactly +# "every vendor not yet listed", which no catalogue can close. Read the list +# below as the SHAPES of what is missed and why, not as a census. To re-derive +# it for a given catalogue, extract main's three patterns and diff coverage, +# scoring on whether the credential is GONE from the output rather than on +# whether the string changed -- span replacement makes those different +# questions, and gap (c) is precisely a case where the string changes and the +# secret stays. # -# (a) THE SEPARATOR-FREE MISS WINDOW. Outside a header, family 3 misses a run -# the word-compound recogniser reads as ONE word: at most 32 letters, -# optionally followed by at most 4 digits. Measured: "Bearer " + "a"*32 is -# KEPT and "a"*33 is CUT; "Bearer " + "a"*32 + "2024" (36 chars) is KEPT -# and "a"*32 + "12345" is CUT. -# test_the_separator_free_miss_window_is_where_known_gaps_says_it_is pins -# those five strings, so this paragraph cannot drift off the code again. -# Four catalogue shapes land in the window: a 20-letter run, a 28-letter -# run, and hyphen- and dot-joined 26-char opaque runs. main caught them -# only by cutting every run after the word, which is the incident this rule -# exists to undo, and a real base64/hex token essentially never lands in -# the window because those alphabets interleave digits mid-piece. +# Where a header does and does not help: (a) and (b) are missed only after a +# BARE "Bearer" and are still cut in the four header serializations. (c) and (d) +# are NOT saved by a header -- (c) because the match ends at the first +# semicolon, (d) because "$TOKEN" is 6 run-characters and family 2's floor is 8. +# In the serializations that carry no "Bearer" at all, main keeps these shapes +# too, so none of it is a regression there. +# +# (a) RUNS THE WORD-COMPOUND RECOGNISER READS AS PROSE. Outside a header, +# family 3 misses anything the recogniser accepts as a word compound. That +# is deliberately NOT length-bounded: a single piece is at most 32 letters +# plus at most 4 digits ("Bearer " + "a"*32 is KEPT, "a"*33 is CUT; +# "a"*32 + "2024" is KEPT, "a"*32 + "12345" is CUT -- five strings pinned +# by test_the_separator_free_miss_window_is_where_known_gaps_says_it_is), +# but a COMPOUND of such pieces has no ceiling at all: a 101-character +# hyphen-joined run of six 16-letter pieces is kept, because that is +# indistinguishable from "x509-certificate-validation-chain-and-so-on". +# An earlier version of this paragraph quoted only the single-piece bound +# and so understated the window by an unbounded factor. +# main caught these only by cutting every run after the word, which is the +# incident this rule exists to undo. A real base64/hex token essentially +# never lands in the window because those alphabets interleave digits +# mid-piece; a token deliberately shaped like prose does, and family 1 or a +# header is the only backstop for it. # (b) A CREDENTIAL INSIDE A URL, e.g. "Bearer postgres://user:pw@host/db". # Family 3 refuses anything matching `scheme://`, because ledger prose # quotes URLs constantly. The password inside one is therefore kept after a # bare "Bearer". Deliberate: relaxing it puts every quoted URL at risk. # (c) A `Key=Value;Key=Value` CONNECTION STRING, e.g. the Azure # "DefaultEndpointsProtocol=https;AccountName=...;AccountKey=..." shape. -# This is the ONE gap a header does not close, and the reason is span +# A header does not close this one, and the reason is span # replacement rather than any refusal: `;` is not a run character, so the # match ends at the first semicolon and only "DefaultEndpointsProtocol= # https" is replaced. The AccountKey survives in ALL FIVE Bearer-carrying @@ -389,6 +408,9 @@ def mark_trusted_finding_disposition(event: dict[str, Any]) -> dict[str, Any]: r"npm_[A-Za-z0-9]{16,}", # npm access token r"shpat_[A-Za-z0-9]{16,}", # Shopify admin API access token r"sq0atp-[A-Za-z0-9_-]{16,}", # Square access token + r"lin_api_[A-Za-z0-9]{16,}", # Linear API key + r"dop_v1_[A-Za-z0-9]{32,}", # DigitalOcean personal access token + r"rk_(?:live|test)_[A-Za-z0-9]{16,}", # Stripe restricted key, live and test r"(?-i:pypi-AgE)[A-Za-z0-9_-]{16,}", # PyPI upload token (macaroon body) r"(?-i:AKIA|ASIA)[A-Z0-9]{16}(?![A-Za-z0-9])", # AWS access/session key id r"(?-i:AIza)[A-Za-z0-9_-]{16,}", # Google API key diff --git a/tests/test_secret_value_redaction.py b/tests/test_secret_value_redaction.py index 8468ed2..7f5e92b 100644 --- a/tests/test_secret_value_redaction.py +++ b/tests/test_secret_value_redaction.py @@ -207,6 +207,10 @@ ("pypi_upload", "pypi-AgE" + _FAKE_BODY_32), ("shopify_admin", "shpat_" + _FAKE_BODY_24), ("square_access", "sq0atp-" + _FAKE_BODY_20), + ("linear_api", "lin_api_" + _FAKE_BODY_32), + ("digitalocean_pat", "dop_v1_" + _FAKE_BODY_32 + _FAKE_BODY_24), + ("stripe_restricted_live", "rk_live_" + _FAKE_BODY_24), + ("stripe_restricted_test", "rk_test_" + _FAKE_BODY_24), ) # The `pypi-` prefix alone is NOT enough evidence: this is a real value from the From e85b7e8dae61bc9db16be7afc1a013bfd17bbabd Mon Sep 17 00:00:00 2001 From: mikehasa Date: Fri, 31 Jul 2026 05:32:29 +0800 Subject: [PATCH 8/8] docs(changelog): record the secret-redaction fix --- CHANGELOG.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 265b27d..1c87641 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,28 @@ follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed +- Secret redaction no longer destroys the record it was protecting. Any value + containing something that looked like a credential was replaced *in its + entirety* with `[REDACTED]`, and the detector fired on ordinary words — the + api-key pattern matched inside `task-`, `disk-` and `risk-`, and the bearer + pattern matched the English phrase "Bearer token". On one real ledger that + destroyed 117 project paths, 20 section ids, 5 summaries and 2 idempotency + keys; because every casualty collapsed onto the same literal string, + unrelated sections merged into one phantom section. Redaction now replaces + only the matched span, and every redaction records which field was cut and + which pattern class cut it — a repair you cannot see is a bug. +- Redaction coverage is now broader than before, not narrower. Patterns are + split into three independent families — prefixed vendor tokens (GitHub, + GitLab, AWS, Google, Slack, Stripe, npm, PyPI, Hugging Face, Linear, + DigitalOcean and more, matched anywhere, with or without a `Bearer`), + `Authorization`/`Proxy-Authorization` headers including their JSON and + `curl -H` serializations, and the genuinely ambiguous bare `Bearer ` + — so a guard added for one can no longer disable another. The ambiguous + family was calibrated against a real ledger rather than guessed: zero false + positives across 27k distinct strings. The shapes still missed are named in + the module, with the method to re-derive them. + ## [0.5.2] - 2026-07-31 ### Fixed