fix(logging): redact space-carrying origin-form targets in urllib3 retry lines (#5745)

urllib3 hands the raw Location field value on as the request target of the
recursive redirect frame, and that grammar admits interior spaces. The
Redirecting slots and the Incremented Retry url capture already tolerate
them; the per-request line and the two Retry:/Retrying tails bounded the
target at the first space, so a signed origin-form path survived all three.

Co-authored-by: sxh313 <sxh313@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>

backend/packages/harness/deerflow/logging_config.py

@@ -48,8 +48,12 @@ _URL_REDACT_RE = re.compile(r"(?P<scheme>[a-zA-Z][a-zA-Z0-9+.-]*://)(?P<userinfo
 # ``METHOD target HTTP/x.x`` line — rewritten to scheme + host with the
 # target collapsed to ``/<redacted>``. 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<scheme>[a-zA-Z][a-zA-Z0-9+.-]*://)(?P<userinfo>[^/?#\s"@]*@)?(?P<host>[^/?#\s"]+) "(?P<method>[A-Za-z]+) (?P<target>/[^"\s]*) (?P<version>HTTP/[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<scheme>[a-zA-Z][a-zA-Z0-9+.-]*://)(?P<userinfo>[^/?#\s"@]*@)?(?P<host>[^/?#\s"]+) "(?P<method>[A-Za-z]+) (?P<target>/[^"]*) (?P<version>HTTP/[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<scheme>[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<target>/\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<target>/\S.*)$")
 _URLLIB3_INCREMENT_RETRY_RE = re.compile(r"Incremented Retry for \(url='(?P<url>(?:[^']|'(?!\)))*)'\)")
-_URLLIB3_RETRYING_RE = re.compile(r"^(?P<head>Retrying \(.*\) after connection broken by .*'): (?P<target>/\S+)$")
+_URLLIB3_RETRYING_RE = re.compile(r"^(?P<head>Retrying \(.*\) after connection broken by .*'): (?P<target>/\S.*)$")
 
 # urllib3's ``Redirecting %s -> %s`` (poolmanager.py:500 at INFO,
 # connectionpool.py:922 at DEBUG) can carry an origin-form target in either

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: /<redacted>"
+
+    # 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}': /<redacted>"
+
+    # 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 /<redacted> 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: /<redacted>"