fix(redaction): make URL-userinfo redaction linear, not quadratic

_REDACT_URL_USERINFO could start a match at every letter of a run of
scheme characters, and each attempt read to the end of the run looking
for `://`. On a long unbroken run of letters or digits (a hex digest, an
ID, part of a response body) that is quadratic: 1.6s for 20k characters.

The display service redacts every message, stack trace and context value
it publishes in the error snapshot, holding the aggregator lock, and
re.sub holds the GIL for the whole call, so one such exception stalled
every thread, render loop included (~0.5s measured for 20k chars of hex).
It also made test_snapshot_stays_small the slowest test in the suite by
far: 142s of a 383s run, 139s of it in this one regex.

A match may now only start where a run of scheme characters starts
(negative lookbehind). Leading digits and `+.-` are captured in group 1
so the substitution restores them, and the scheme still has to start
with a letter, so what gets redacted is unchanged: old and new output
were identical on 300k fuzzed inputs. 20k chars now take ~0.5ms, 200k
~6ms, and test_snapshot_stays_small takes 0.8s.

test/test_redaction.py pins the exact output for schemes that begin after
digits or `+.-`, and bounds 50k-character runs at 1s; against the old
pattern those timing tests fail at 3-11s each.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KMXdS2S4NXTJ8ET96GymhK
This commit is contained in:
Claude
2026-09-24 15:28:50 +00:00
parent f3894916a9
commit eaaea93c77
3 changed files with 79 additions and 2 deletions
+6
View File
@@ -120,6 +120,12 @@ floor on the release that ships them):
when the count is only known to the display service.
- The Logs tab has a **Plugin errors** panel: per-plugin counts, repeating
errors and a Clear button.
- Redacting `user:password@` from URLs in exception text (`src/redaction.py`)
takes time proportional to the text, not its square. A plugin error quoting
a long unbroken run of letters or digits (a hex digest, an ID) used to stall
every thread of the display service for up to seconds each time the snapshot
was published: about 0.5s for 20k characters of hex. What gets redacted is
unchanged.
### Removed
+10 -2
View File
@@ -34,8 +34,16 @@ _REDACT_AUTH_HEADER = re.compile(
# Credentials embedded in a URL: https://user:password@host. requests quotes
# the full URL in its exceptions, so this is a realistic leak. The username is
# kept -- it identifies which account failed without being the secret.
_REDACT_URL_USERINFO = re.compile(r'([a-z][a-z0-9+.-]*://[^/\s:@]+:)([^/\s@]+)(@)',
re.IGNORECASE)
#
# A match may only start where a run of scheme characters starts. Unanchored,
# `[a-z][a-z0-9+.-]*://` was tried from every letter of a long run (a hex
# digest, an ID, a blob of response body), each attempt reading to the end of
# the run: quadratic, 1.6s for 20k characters, all of it holding the GIL.
# Leading digits and `+.-` sit inside group 1 so the substitution puts them
# back; the scheme proper still has to start with a letter.
_REDACT_URL_USERINFO = re.compile(
r'((?<![a-z0-9+.-])[0-9+.-]*[a-z][a-z0-9+.-]*://[^/\s:@]+:)([^/\s@]+)(@)',
re.IGNORECASE)
def redact_credentials(text: str) -> str:
+63
View File
@@ -0,0 +1,63 @@
"""redact_credentials must stay linear in the length of its input.
Regression under test: the URL-userinfo pattern (`scheme://user:password@`)
could start a match at every letter of a run of scheme characters, and each
attempt read to the end of the run looking for `://`. A 20k-character run took
1.6s; the display service redacts every message, stack trace and context value
it publishes in the error snapshot, and re.sub holds the GIL throughout, so an
exception quoting a hex digest or a long ID stalled the render loop with it.
test_error_snapshot_cross_process.py's snapshot-size test spent 140s here.
The anchored pattern has to redact exactly what the old one did, including a
scheme that begins after digits or `+.-` in the same run.
"""
import time
import pytest
from src.redaction import redact_credentials
class TestUrlUserinfo:
@pytest.mark.parametrize("text,expected", [
("401 for https://user:hunter2@example.com/api",
"401 for https://user:<redacted>@example.com/api"),
("HTTPS://USER:HUNTER2@EXAMPLE.COM",
"HTTPS://USER:<redacted>@EXAMPLE.COM"),
("git+ssh://deploy:hunter2@host/repo",
"git+ssh://deploy:<redacted>@host/repo"),
# The scheme starts after digits or +.- in the same run. Those
# characters must survive, and the password must still go.
("1http://user:hunter2@host", "1http://user:<redacted>@host"),
("+.-http://user:hunter2@host", "+.-http://user:<redacted>@host"),
("a1+http://user:hunter2@host", "a1+http://user:<redacted>@host"),
("see a://u:first@b and c://v:second@d",
"see a://u:<redacted>@b and c://v:<redacted>@d"),
])
def test_password_is_redacted_and_the_rest_kept(self, text, expected):
assert redact_credentials(text) == expected
def test_a_url_without_a_password_is_untouched(self):
text = "GET https://user@example.com/path failed"
assert redact_credentials(text) == text
class TestLinearTime:
# Each of these took seconds before the fix (letters ~10s at this size)
# and takes about a millisecond after it; the bound leaves CI plenty of
# headroom while still failing on a quadratic pattern.
@pytest.mark.parametrize("unit", ["x", "0123456789abcdef", "1a", "a+", "1"])
def test_long_scheme_character_runs(self, unit):
text = (unit * 50_000)[:50_000]
start = time.perf_counter()
assert redact_credentials(text) == text
elapsed = time.perf_counter() - start
assert elapsed < 1.0, f"{elapsed:.2f}s to redact {len(text)} chars of {unit!r}"
def test_a_credential_after_a_long_run_is_still_found(self):
run = "ab12" * 10_000
text = f"{run} https://user:hunter2@example.com"
start = time.perf_counter()
assert redact_credentials(text) == f"{run} https://user:<redacted>@example.com"
assert time.perf_counter() - start < 1.0