diff --git a/alembic/versions/0099_scrub_secrets_from_retrieval_logs.py b/alembic/versions/0099_scrub_secrets_from_retrieval_logs.py index c643d21..d2c5f9f 100644 --- a/alembic/versions/0099_scrub_secrets_from_retrieval_logs.py +++ b/alembic/versions/0099_scrub_secrets_from_retrieval_logs.py @@ -50,14 +50,34 @@ branch_labels = None depends_on = None # Vendor-prefixed credentials — the prefix IS the tell, so no entropy guessing. +# +# `\m` IS LOAD-BEARING AND IS NOT `\b`. It anchors the prefix to the START OF +# A WORD, which the Python twin spells `\b`. Two separate mistakes were made +# porting this and they compounded: +# +# 1. The boundary was dropped entirely, so `sk-` matched inside any word +# containing it. `` — a string that appears in +# thousands of these rows — became ``, because +# `sk-` + `notification` is a prefix followed by twelve word characters. +# 2. Writing `\b` would not have fixed it. In Postgres ARE `\b` is a +# BACKSPACE character, not a word boundary; `\m` (start of word) and +# `\y` (either edge) are the spellings that mean what Python's `\b` +# means. +# +# Both were live for one run of this migration, on one install, and the cost +# is recorded rather than papered over: the mangled rows cannot be restored, +# because the original text is what the UPDATE overwrote. The live scrubber in +# services/retrieval_telemetry.py was never affected — its `\b` is Python's +# and behaves correctly, which is why rows written after the deploy are intact +# and only migration-rewritten ones were damaged. _TOKEN = ( - r"(fmcp_|flt_|ghp_|gho_|ghs_|ghu_|github_pat_|glpat-|xox[abprs]-" + r"\m(fmcp_|flt_|ghp_|gho_|ghs_|ghu_|github_pat_|glpat-|xox[abprs]-" r"|sk-[A-Za-z0-9]*-?|AKIA|ASIA)[A-Za-z0-9_\-]{12,}" ) # A value handed to a secret-NAMED variable, in shell, env, YAML, JSON or a # query string. The NAME identifies it, so the value can be anything. _ASSIGNED = ( - r"([A-Za-z0-9_]*(token|secret|password|passwd|api[_-]?key|access[_-]?key)" + r"\m([A-Za-z0-9_]*(token|secret|password|passwd|api[_-]?key|access[_-]?key)" r"[A-Za-z0-9_]*)(\s*[:=]\s*[\"']?)([^\s\"'&]{8,})" ) _AUTH_HEADER = r"(authorization\s*:\s*(bearer|basic|token)\s+)(\S+)" diff --git a/tests/test_retrieval_query_scrubbing.py b/tests/test_retrieval_query_scrubbing.py index c4a13ed..d1aa18d 100644 --- a/tests/test_retrieval_query_scrubbing.py +++ b/tests/test_retrieval_query_scrubbing.py @@ -133,3 +133,42 @@ def test_the_write_path_scrubs_rather_than_the_read_path(): assert "[redacted" in payload["query"] # The rest of the command survives, or the row stops being evidence. assert "git push" in payload["query"] + + +# ── the SQL twin has its own word-boundary spelling (#3925) ───────────── +# +# Migration 0099 carries an inlined copy of these patterns, deliberately: a +# migration is a frozen record of what already ran, and importing the live +# ones would mean it quietly did something different next year. +# +# Frozen is not the same as correct, and the first cut was neither. The +# boundary was dropped in the port, so `sk-` matched inside any word +# containing it — `` became `` across +# thousands of rows on the one install that ran it. And writing `\b` would not +# have saved it: in Postgres ARE `\b` is a BACKSPACE, not a word boundary. +# `\m` (start of word) is the spelling that means what Python's `\b` means. +# +# So this pins the property no reader can eyeball, and it is a PRESENCE check +# on a token that must appear rather than an absence check on prose (#3352). + +def test_the_migrations_patterns_anchor_to_a_word_start_the_postgres_way(): + """`\\m`, never `\\b` — the two are unrelated in Postgres.""" + import pathlib + + src = (pathlib.Path(__file__).resolve().parents[1] + / "alembic" / "versions" + / "0099_scrub_secrets_from_retrieval_logs.py").read_text() + + for name in ("_TOKEN", "_ASSIGNED"): + line = src.split(f"{name} = (")[1].split(")")[0] + assert r"\m" in line, ( + f"migration 0099's {name} no longer anchors to a word start. " + f"Without it a vendor prefix matches INSIDE a word — `sk-` in " + f"`task-notification` is the case that actually happened — and " + f"the UPDATE overwrites the only copy of the text it mangles." + ) + assert r"\b" not in line, ( + f"migration 0099's {name} uses `\\b`, which is a BACKSPACE in " + f"Postgres ARE rather than a word boundary. Python's `\\b` and " + f"Postgres's `\\m` look interchangeable and are not." + )