diff --git a/backend/packages/harness/deerflow/logging_config.py b/backend/packages/harness/deerflow/logging_config.py index bd29f5043c..79b3dfa4e9 100644 --- a/backend/packages/harness/deerflow/logging_config.py +++ b/backend/packages/harness/deerflow/logging_config.py @@ -48,8 +48,12 @@ _URL_REDACT_RE = re.compile(r"(?P[a-zA-Z][a-zA-Z0-9+.-]*://)(?P``. The method class is case-tolerant: # HTTP methods are case-sensitive tokens, and callers may pass lowercase -# custom methods through to urllib3. -_URLLIB3_REQUEST_LINE_RE = re.compile(r'(?P[a-zA-Z][a-zA-Z0-9+.-]*://)(?P[^/?#\s"@]*@)?(?P[^/?#\s"]+) "(?P[A-Za-z]+) (?P/[^"\s]*) (?PHTTP/[0-9.]+)"') +# custom methods through to urllib3. The target may contain spaces for the +# same reason ``Redirecting``'s slots may: on the recursive redirect frame it +# is the raw ``Location`` field value, whose grammar admits interior spaces, +# and a whitespace-strict target class let the signed path survive. Its scan +# still stops at the closing quote, so the pass stays linear-time. +_URLLIB3_REQUEST_LINE_RE = re.compile(r'(?P[a-zA-Z][a-zA-Z0-9+.-]*://)(?P[^/?#\s"@]*@)?(?P[^/?#\s"]+) "(?P[A-Za-z]+) (?P/[^"]*) (?PHTTP/[0-9.]+)"') # urllib3's retry sites log the request target with NO scheme and NO request- # line scaffolding, so neither pattern above can see it (installed 2.7.0): @@ -67,9 +71,18 @@ _URLLIB3_REQUEST_LINE_RE = re.compile(r'(?P[a-zA-Z][a-zA-Z0-9+.-]*://)(? # connectionpool.py:869, **WARNING**, so it passes the Gateway's INFO root # without DEBUG being enabled; the greedy prefix groups pin the split to # the final ``': `` so an error repr containing quotes cannot shift it. -_URLLIB3_RETRY_TARGET_RE = re.compile(r"^Retry: (?P/\S+)$") +# Both trailing-target shapes take a target that runs to the end of the +# message rather than to the first space, because ``url`` is the caller's raw +# request target and, on the recursive redirect frame, the raw ``Location`` +# field value (connectionpool.py:923-925) — whose grammar admits interior +# spaces, exactly as ``Redirecting``'s slots do (round 13). A whitespace-strict +# target left ``Retry: /download a file.pdf?sig=...`` and the same signed +# target on the WARNING line unredacted, while the two sibling passes that +# already take a loose tail redacted it. The target still has to start with +# ``/``, so prose that merely begins with ``Retry: `` keeps passing through. +_URLLIB3_RETRY_TARGET_RE = re.compile(r"^Retry: (?P/\S.*)$") _URLLIB3_INCREMENT_RETRY_RE = re.compile(r"Incremented Retry for \(url='(?P(?:[^']|'(?!\)))*)'\)") -_URLLIB3_RETRYING_RE = re.compile(r"^(?PRetrying \(.*\) after connection broken by .*'): (?P/\S+)$") +_URLLIB3_RETRYING_RE = re.compile(r"^(?PRetrying \(.*\) after connection broken by .*'): (?P/\S.*)$") # urllib3's ``Redirecting %s -> %s`` (poolmanager.py:500 at INFO, # connectionpool.py:922 at DEBUG) can carry an origin-form target in either diff --git a/backend/tests/test_logging_config.py b/backend/tests/test_logging_config.py index df3d5cb117..772f98d001 100644 --- a/backend/tests/test_logging_config.py +++ b/backend/tests/test_logging_config.py @@ -807,3 +807,50 @@ def test_url_redaction_filter_redirecting_covers_all_relative_ref_forms() -> Non assert record.getMessage() == expected, (t1, t2) assert "LeakedSig" not in record.getMessage() assert "token=QuerySecret" not in record.getMessage() + + +def test_url_redaction_filter_collapses_space_carrying_targets_in_every_retry_shape() -> None: + """Every urllib3 shape that carries a raw request target has to tolerate + interior spaces, not only ``Redirecting``. + + On the recursive redirect frame urllib3 hands the raw ``Location`` field + value on as ``url`` (connectionpool.py:923-925), and that field grammar + admits interior spaces — the same fact rounds 13 and 16 rest on. The three + shapes pinned here bounded their target at the first space, so a signed + path survived all of them while the two sibling passes that never did + (``Incremented Retry for (url='…')`` and ``Redirecting``) redacted it. + """ + from deerflow.logging_config import UrlRedactionFilter + + filt = UrlRedactionFilter() + + def _formatted(fmt: str, args: tuple) -> str: + record = logging.LogRecord("urllib3.connectionpool", logging.DEBUG, __file__, 1, fmt, args, None) + assert filt.filter(record) is True + return record.getMessage() + + signed = "/private/a b?sig=LeakedSig" + + # connectionpool.py:954 — bare target after the "Retry: " literal. + assert _formatted("Retry: %s", (signed,)) == "Retry: /" + + # connectionpool.py:869 — WARNING, so it clears the Gateway's INFO root. + retries_repr = "Retry(total=0, connect=None, read=None, redirect=None, status=None)" + error_repr = "ProtocolError('Connection aborted.', RemoteDisconnected('Remote end closed connection without response'))" + retrying = _formatted("Retrying (%r) after connection broken by '%r': %s", (retries_repr, error_repr, signed)) + assert retrying == f"Retrying ({retries_repr!r}) after connection broken by '{error_repr!r}': /" + + # connectionpool.py:545 — the quoted per-request line. + request_line = _formatted('%s://%s:%s "%s %s %s" %s %s', ("https", "cdn.example", 443, "GET", signed, "HTTP/1.1", 200, None)) + assert request_line == 'https://cdn.example:443 "GET / HTTP/1.1" 200 None' + + for formatted in (retrying, request_line, _formatted("Retry: %s", (signed,))): + assert "LeakedSig" not in formatted + assert "sig=" not in formatted + + # The looser tails trade reach for one class of prose: a line that opens + # with urllib3's own ``Retry: `` literal AND a slash-initial tail collapses + # whole even when it is not a request target. Prose after ``Retry: `` that + # does not start with ``/`` keeps passing through untouched, which + # test_url_redaction_filter_covers_urllib3_retry_lines already pins. + assert _formatted("Retry: /tmp/build.sock went away", ()) == "Retry: /"