diff --git a/CHANGELOG.md b/CHANGELOG.md index 32ce27beb..b20b4737b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -290,6 +290,10 @@ breaking changes may land in a minor release. - Pin the run journal and `state.json` to the run dir's recorded identity: writes and re-reads refuse a run dir swapped for a link or another directory instead of acting outside the run, and Windows verify-stream logs refuse junctions. +- Catch more credential shapes in the `diagnose` / `probe-adapter` egress guard: Stripe + `sk_`/`rk_` keys, Slack `xapp-`, SendGrid `SG.` and PyPI tokens, including inside URL + paths, and `user:password@` URL credentials under any scheme, not only http(s). + `scrub_text` now redacts a URL's whole userinfo, before emails. ## [0.13.2] — 2026-10-09 diff --git a/src/bmad_loop/sanitize.py b/src/bmad_loop/sanitize.py index 88ec09396..597f56e10 100644 --- a/src/bmad_loop/sanitize.py +++ b/src/bmad_loop/sanitize.py @@ -86,7 +86,16 @@ _SECRET_PREFIX_RE = re.compile( r"^(?:" r"sk-|ghp_|gho_|ghu_|ghs_|ghr_|github_pat_|glpat-|gss_|" - r"xox[baprs]-|AKIA|ASIA|AIza|ya29\.|AGAPP|hf_|npm_|dop_v1_|sk-ant-" + r"xox[baprs]-|AKIA|ASIA|AIza|ya29\.|AGAPP|hf_|npm_|dop_v1_|sk-ant-|" + # Shapes the entropy arm below cannot see: each splits at `.`/`-`/`_` into + # runs shorter than `_SECRET_RUN_MIN`, so without a prefix they pass as plain + # identifiers. Stripe secret/restricted keys (legacy ones carry a 24-char + # tail), Slack app-level tokens (`xapp--…`, every segment short), SendGrid + # (`SG.<22>.<43>`, the 43 often split by `-`), and PyPI upload tokens, whose + # macaroon always opens `pypi-AgE`. Each arm is anchored on enough of the + # vendor format that an ordinary slug (`sk_util`, `pypi-mirror`, `SG.1`) + # does not trip it. + r"[sr]k_(?:live|test)_|xapp-\d+-|SG\.[A-Za-z0-9_-]{16,}\.|pypi-AgE" r")" ) _JWT_RE = re.compile(r"^eyJ[A-Za-z0-9_-]+\.eyJ[A-Za-z0-9_-]+") @@ -97,8 +106,26 @@ _SECRET_ENTROPY_MIN = 3.5 # bits/char; pure hex ~4.0, base64 ~6.0, prose/slug well below # Token shape used by assert_no_leak to re-scan rendered output for secrets. -_LEAK_TOKEN_RE = re.compile(r"[A-Za-z0-9._/+-]{6,}") -_URL_CRED_RE = re.compile(r"https?://[^/\s]*:[^/@\s]+@") +# `/` separates tokens, so an anchored prefix arm sees a key that follows a path +# segment; the entropy arm already splits on `/` via _ALNUM_RUN_RE. +_LEAK_TOKEN_RE = re.compile(r"[A-Za-z0-9._+-]{6,}") +# Any RFC 3986 scheme, not only http(s): connection strings (`postgres://`, +# `redis://`, `mongodb+srv://`, `amqp://`, `ssh://`) carry `user:password@` +# userinfo the same way and are as likely to reach a log tail or `--help` blob. +# Neither half crosses an authority delimiter (`/`, `?`, `#`), so a port plus a +# query that happens to hold an `@` (`redis://cache:6379?client=user@cluster`) is +# not mistaken for a password. The password half may hold `@` and runs to the +# LAST one in the authority, as URL parsers split it, so `user:p@ss@host` is +# matched — and redacted by scrub_text — whole rather than up to its first `@`. +# The scheme is capped at 32 characters and the user half stops at the first `:` +# (RFC 3986's user/password split). The pattern has no literal anchor, so an +# unbounded scheme restarts a scan at every position of a long alphanumeric run. +# That is quadratic on a base64 blob or a minified line, and scrub_text, +# _truncate_line and assert_no_leak all scan such lines. A longer scheme still +# matches, starting later; scrub_text keeps the scheme group either way. +_URL_CRED_RE = re.compile( + r"(?P[A-Za-z][A-Za-z0-9+.-]{0,31}://)(?P[^/?#\s:]*):[^/?#\s]+@" +) # The guard constructs whose match can straddle a truncation point AND whose # leading fragment would still be sensitive once the rest is clipped away. # `scrub_text`'s cut retracts out of these -- see `_truncate_line`. This sits @@ -145,6 +172,10 @@ _REDACTED_STR = "" _REDACTED_SECRET = "" # nosec B105 - redaction marker, not a credential _REDACTED_EMAIL = "" +# Replaces a URL's whole `user:password@`, the `@` included: a marker followed by +# `@` would itself read as `:@` to `_URL_CRED_RE` and make the +# guard refuse the very output that was just made safe. +_REDACTED_USERINFO = "" _REDACTED_DEPTH = "" @@ -293,8 +324,8 @@ def scrub_text(s: str, *, max_lines: int | None = None, max_chars: int | None = """Sanitize free text (a CLI's ``--help`` / ``--version`` / a log tail). Less aggressive than :func:`scrub_json` — help text is the CLI's own and - flag lines must survive — so we only redact the home dir and any emails, - then optionally cap the line count and each line's length. + flag lines must survive — so we only redact the home dir, URL credentials + and any emails, then optionally cap the line count and each line's length. ``max_chars`` bounds each line's **content** at ``max_chars``; a truncated line is emitted as at most ``max_chars`` characters plus the @@ -314,6 +345,10 @@ def scrub_text(s: str, *, max_lines: int | None = None, max_chars: int | None = of the redaction step. """ s = redact_home(s) + # URL credentials go BEFORE emails: the email rule would otherwise take the + # `x@example.com` tail of `redis://bob:hunter2!x@example.com`, leaving + # `hunter2!` behind with the `@` gone, so the guard's URL rule no longer sees it. + s = _URL_CRED_RE.sub(lambda m: m.group("scheme") + _REDACTED_USERINFO, s) s = _redact_emails(s) if max_lines is None and max_chars is None: return s diff --git a/tests/test_sanitize.py b/tests/test_sanitize.py index 95c5d7b98..f1652159f 100644 --- a/tests/test_sanitize.py +++ b/tests/test_sanitize.py @@ -360,6 +360,15 @@ def test_scrub_event_payload_is_scrub_json(home): "glpat-xxxxxxxxxxxxxxxxxxxx", # gitlab pat "AIzaSyA0000000000000000000000000000000", # google api key "a1b2c3d4e5f6a7b8c9d0e1f2a3b4c5d6e7f8a9b0", # 40-char high-entropy hex secret + # Shapes whose every alnum run is shorter than _SECRET_RUN_MIN, so only a + # prefix arm can catch them. Low-entropy canaries in the vendors' formats, + # not live keys. + "sk_live_CANARYxxxxxxxxxxxxxxxx", # stripe secret, legacy 24-char tail + "sk_test_CANARYxxxxxxxxxxxxxxxx", # stripe test secret + "rk_live_CANARYxxxxxxxxxxxxxxxx", # stripe restricted key + "xapp-1-A0CANARY01-0000000000-xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx", # slack app-level + "SG.CANARYxxxxxxxxxxxxxxx.CANARYxxxx-xxxxxxxxxxxxxxx_xxxxxxxxxxxxxxxx", # sendgrid + "pypi-AgEIcHlwaS5vcmcCANARY", # pypi upload token ], ) def test_looks_like_secret_catches_credentials(value): @@ -376,6 +385,12 @@ def test_looks_like_secret_catches_credentials(value): "01234567-89ab-cdef-0123-456789abcdef", # UUID: short runs at hyphens "DW-1", "1.2-add-logging", + # Near-misses for the vendor arms: each shares a prefix's first letters + # but not the format the arm is anchored on. + "sk_util", + "pypi-mirror", + "SG.1", + "xapp-name", ], ) def test_looks_like_secret_passes_safe_slugs(value): @@ -435,6 +450,12 @@ def test_assert_no_leak_clean_text(): [ ("contact me@example.com", "email"), ("see https://user:pass@host/x", "url-credentials"), + # Not only http(s): connection strings carry userinfo the same way. + ("dsn postgres://app:hunter2@localhost:5432/db", "url-credentials"), + ("REDIS_URL=redis://default:hunter2@cache:6379", "url-credentials"), + ("uri mongodb+srv://bob:hunter2@cluster0.example.net/x", "url-credentials"), + ("broker amqp://guest:guest@rabbit/vhost", "url-credentials"), + ("ssh://deploy:hunter2@host/repo.git", "url-credentials"), ("path /home/alice/x", "absolute-home-path"), # Every arm of _ABS_HOME_RE, not just the Linux one. The macOS and root # arms were correct but unpinned; so was the drive-letter-plus-FORWARD- @@ -476,6 +497,84 @@ def test_assert_no_leak_fires(text, rule): assert rule in sanitize.assert_no_leak(text) +@pytest.mark.parametrize( + "text", + [ + "ssh://git@github.com:22/org/repo.git", # user, no password + "git@github.com:org/repo.git", # scp-style, no scheme + "see https://host:8443/path", # a port is not a password + "redis://cache:6379?client=user@cluster", # `@` past the authority + "redis://cache:6379#user@cluster", + ], +) +def test_assert_no_leak_url_rule_needs_a_password(text): + assert "url-credentials" not in sanitize.assert_no_leak(text) + + +@pytest.mark.parametrize( + "text", + [ + # Each key follows a `/`. The prefix arms are `^`-anchored, so they see + # the key only when `/` ends the token before it. Low-entropy canaries in + # the vendors' formats, not live keys. + "https://host.test/sk_live_123456789012345678901234", # stripe secret in a URL path + "path/rk_live_abcdefABCDEF0123456789", # stripe restricted key + "./SG.CANARYxxxxxxxxxxxxxxx.CANARYxxxx-xxxx", # sendgrid + "a/sk-ant-xxxxxxxxxxxxxxxxxxxxx", # anthropic + ], +) +def test_assert_no_leak_finds_prefixed_keys_in_url_paths(text): + """Ablation: restoring `/` in ``_LEAK_TOKEN_RE`` fails every row.""" + assert "secret" in sanitize.assert_no_leak(text) + + +def test_url_cred_scheme_is_bounded(): + """The scheme is capped at 32 characters, one of the two properties that keep + the unanchored URL-credential scan linear on a long alphanumeric run. A longer + scheme still matches, starting later. + + Ablation: restoring the unbounded ``*`` scheme makes the match start at 0. + """ + text = "a" * 40 + "://u:p@h" + m = sanitize._URL_CRED_RE.search(text) + assert m is not None + assert m.start() == 8 + assert "url-credentials" in sanitize.assert_no_leak(text) + + +def test_url_cred_user_half_stops_at_first_colon(): + """The user half stops at the first ``:`` (RFC 3986's user/password split), + the other property that keeps the scan linear. The whole userinfo is still + redacted. + + Ablation: restoring ``[^/?#\\s]*`` for the user half yields ``"a:b"``. + """ + m = sanitize._URL_CRED_RE.search("redis://a:b:c@h") + assert m is not None + assert m.group("user") == "a" + assert sanitize.scrub_text("redis://a:b:c@h") == "redis://h" + + +@pytest.mark.parametrize( + "text", + [ + # Email-shaped tail: the email rule alone would leave `hunter2!` behind. + "redis://bob:hunter2!x@example.com", + "dsn postgres://app:hunter2@db.example.com:5432/db", + # `@` inside the password: redaction must run to the last `@`. + "redis://bob:p@ss@localhost:6379", + "amqp://bob:a@b@mq.example.com", + ], +) +def test_scrub_text_redacts_url_credentials_whole(text): + out = sanitize.scrub_text(text) + for fragment in ("hunter2", "p@ss", "a@b", "bob:", "app:"): + assert fragment not in out + assert "" in out + # The redacted output must not re-trip the guard on its own marker. + assert sanitize.assert_no_leak(out) == [] + + def test_assert_no_leak_sees_wsl_unc_through_the_json_render(): """#512's literal reproduction, asserted on BOTH encodings.