github/deer-flow
1
0
Fork 0
mirror of https://github.com/bytedance/deer-flow.git synced 2026-09-25 14:06:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(logging): collapse space-carrying Redirecting slots (#5225 round 16) (#5687)

Browse Source 
* fix(logging): collapse space-carrying Redirecting slots (#5225 round 16)

A Redirecting slot was kept verbatim whenever a scheme matched at
position 0, but the generic absolute-URL pass it is handed to stops
both its `host` and `rest` groups at whitespace. A Location header
value carrying an interior space (legal field syntax a misbehaving
server can emit, and the shape round 13 deliberately kept matching)
therefore survived in the clear after the first space:
'Redirecting /private/x?token=Q -> https://cdn.example/other page?sig=LeakedSig'
redacted to '... -> https://cdn.example/<redacted> page?sig=LeakedSig'.
The slot is now kept only when a whitespace-free absolute URL fills it,
so the pass is guaranteed to consume it whole; space-carrying slots
collapse like every other shape neither pass could cover.

* fix(logging): ask the URL pass whether it consumes a Redirecting slot whole (#5225 round 16 review)

The kept-set regex ([a-zA-Z...]*://\S+) was a stand-in for the invariant
"the generic absolute-URL pass consumes the slot whole", and two
whitespace-free absolute shapes satisfied it while the pass actually
stopped early: a quote that reads as a closing mark (https://h/a')b?sig=…)
and an empty host before the first /?# (https:///path?sig=…), which its
host group never matches. Both left the signed tail in the log.

Replace the stand-in with the check itself: run the pass's own pattern
against the slot and keep it only when the match spans it end to end, so
the rule cannot drift from the pattern's stop conditions again.

---------

Co-authored-by: sxh313 <sxh313@users.noreply.github.com>
This commit is contained in:
sxh 2026-09-23 09:22:31 +08:00 committed by GitHub
parent 31bfd40f38
commit 1451e0a2e0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 87 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
backend/app/channels/AGENTS.md
View File

File diff suppressed because one or more lines are too long

57
backend/packages/harness/deerflow/logging_config.py
View File

@ -76,7 +76,8 @@ _URLLIB3_RETRYING_RE = re.compile(r"^(?P<head>Retrying \(.*\) after connection b
# slot: connectionpool passes the origin-form request target, and the Location # slot: connectionpool passes the origin-form request target, and the Location
# header may itself be a relative reference (RFC 9110 allows it). The generic # header may itself be a relative reference (RFC 9110 allows it). The generic
# absolute-URL pass only sees scheme-bearing halves, so origin-form slots # absolute-URL pass only sees scheme-bearing halves, so origin-form slots
# collapse to ``/<redacted>`` here; absolute slots are left for that pass. # collapse to ``/<redacted>`` here; a slot is left for that pass only when
# the pass consumes it whole (see _url_pass_consumes_slot).
# The pattern keeps the ``^Redirecting `` prefix anchor — the urllib3-owned # The pattern keeps the ``^Redirecting `` prefix anchor — the urllib3-owned
# literal — because an ``-> /path`` arrow is not urllib3-owned shape: # literal — because an ``-> /path`` arrow is not urllib3-owned shape:
# non-URL logs render it too (sandbox mount mappings log # non-URL logs render it too (sandbox mount mappings log
@ -86,17 +87,29 @@ _URLLIB3_RETRYING_RE = re.compile(r"^(?P<head>Retrying \(.*\) after connection b
# header string, and interior spaces are legal field syntax a misbehaving # header string, and interior spaces are legal field syntax a misbehaving
# server can emit — a whitespace-strict tail would void the pass entirely # server can emit — a whitespace-strict tail would void the pass entirely
# and leak the origin-form request target in the first slot (round 13). A # and leak the origin-form request target in the first slot (round 13). A
# space-carrying second slot collapses whole when it starts with ``/``. The # space-carrying slot collapses whole whether or not it starts with ``/``:
# the generic pass stops its ``rest`` at whitespace, so an absolute
# space-carrying slot kept its signed tail (round 16). The
# first slot gets the same grammar treatment: the recursive urlopen frame # first slot gets the same grammar treatment: the recursive urlopen frame
# passes the previous raw Location as its url, so t1 can carry interior # passes the previous raw Location as its url, so t1 can carry interior
# spaces too — it is lazy, splitting at the FIRST `` -> `` the way the # spaces too — it is lazy, splitting at the FIRST `` -> `` the way the
# line was constructed left to right. # line was constructed left to right.
_URLLIB3_REDIRECTING_ORIGIN_RE = re.compile(r"^Redirecting (?P<t1>\S.*?) -> (?P<t2>\S.*)$") _URLLIB3_REDIRECTING_ORIGIN_RE = re.compile(r"^Redirecting (?P<t1>\S.*?) -> (?P<t2>\S.*)$")
# A Redirecting slot is kept only when it starts with an absolute
# hierarchical URL; everything else (every RFC 3986 relative-reference # A Redirecting slot is kept only when the generic absolute-URL pass consumes
# form, and non-hierarchical schemes) collapses — see _redact_redirecting_origin. # it WHOLE, so the rule is asked of that pass itself rather than of an
_SLOT_ABSOLUTE_URL_RE = re.compile(r"[a-zA-Z][a-zA-Z0-9+.-]*://") # approximation that can drift from it. The pass leaves a tail in the clear
# whenever its match stops early: ``rest`` halts at whitespace and at a quote
# that reads as a closing mark (``https://h/a')b?sig=…`` keeps ``')b?sig=…``),
# and an empty host before the first ``/?#`` (``https:///path?sig=…``) matches
# nothing at all because ``host`` needs one character. Both are legal absolute
# URLs, so a hand-written "is it absolute and whitespace-free" test cannot see
# them. Everything the pass does not consume whole collapses.
def _url_pass_consumes_slot(slot: str) -> bool:
match = _URL_REDACT_RE.match(slot)
return match is not None and match.end() == len(slot)
# The two scheme-bearing patterns start with a character class, so re.sub # The two scheme-bearing patterns start with a character class, so re.sub
# retries the match at every position of a long token — a letter run with no # retries the match at every position of a long token — a letter run with no
@ -166,9 +179,9 @@ class UrlRedactionFilter(logging.Filter):
per-request ``scheme://host:port "METHOD target HTTP/x.x"`` line, the per-request ``scheme://host:port "METHOD target HTTP/x.x"`` line, the
retry lines that log a bare origin-form target (``Retry: <target>``, retry lines that log a bare origin-form target (``Retry: <target>``,
``Incremented Retry for (url='<target>')``, ``Retrying (…) after ``Incremented Retry for (url='<target>')``, ``Retrying (…) after
connection broken by '…': <target>``), and every non-absolute slot of connection broken by '…': <target>``), and every ``Redirecting <target>
``Redirecting <target> -> <target>`` (kept whole only when a scheme -> <target>`` slot the generic pass would not consume whole
starts the slot, for the generic pass to rewrite). The record is rewritten in place (see _url_pass_consumes_slot). The record is rewritten in place
(``msg`` set to the redacted formatted message, ``args`` cleared) so (``msg`` set to the redacted formatted message, ``args`` cleared) so
every downstream handler and formatter — text or JSON — sees the same every downstream handler and formatter — text or JSON — sees the same
redacted line, while the method/status/error observability is preserved. redacted line, while the method/status/error observability is preserved.
@ -207,18 +220,22 @@ class UrlRedactionFilter(logging.Filter):
return match.group("head") + ": /<redacted>" return match.group("head") + ": /<redacted>"
def _redact_redirecting_origin(match: re.Match[str]) -> str: def _redact_redirecting_origin(match: re.Match[str]) -> str:
# A slot stays verbatim ONLY when it is an absolute URL (a # A slot stays verbatim ONLY when the generic absolute-URL pass
# scheme at position 0), so the generic absolute-URL pass — # — which runs after this one — consumes it whole; that pass is
# which runs after this one — rewrites it. Everything else # asked directly (see _url_pass_consumes_slot).
# collapses: the Location field-value grammar (RFC 3986 # Everything else collapses: the Location field-value grammar
# relative-part) also admits slash-less relative references # (RFC 3986 relative-part) also admits slash-less relative
# (``download?sign=…``, ``?sign=…``, ``#frag``), network-path # references (``download?sign=…``, ``?sign=…``, ``#frag``),
# references (``//host/x``, whose userinfo collapses with it), # network-path references (``//host/x``, whose userinfo
# and non-hierarchical schemes (``data:…``) — none of which # collapses with it), and non-hierarchical schemes
# either pass could otherwise see, and the slash-less forms # (``data:…``) — none of which either pass could otherwise see,
# kept their signed queries verbatim (round 15). # and the slash-less forms kept their signed queries verbatim
# (round 15). A space-carrying slot is legal Location syntax
# too, and the generic pass stops its ``rest`` at whitespace,
# so the signed tail after the first space survived the same
# way (round 16).
def _slot(target: str) -> str: def _slot(target: str) -> str:
return target if _SLOT_ABSOLUTE_URL_RE.match(target) else "/<redacted>" return target if _url_pass_consumes_slot(target) else "/<redacted>"
return "Redirecting " + _slot(match.group("t1")) + " -> " + _slot(match.group("t2")) return "Redirecting " + _slot(match.group("t1")) + " -> " + _slot(match.group("t2"))

56
backend/tests/test_logging_config.py
View File

@ -711,8 +711,11 @@ def test_url_redaction_filter_redirecting_survives_spacey_location() -> None:
assert spacey_t1.getMessage() == "Redirecting /<redacted> -> /<redacted>" assert spacey_t1.getMessage() == "Redirecting /<redacted> -> /<redacted>"
assert "QuerySecret" not in spacey_t1.getMessage() assert "QuerySecret" not in spacey_t1.getMessage()
# An absolute Location with an interior space stays whole for the # An absolute Location with an interior space must NOT be handed to the
# generic absolute-URL pass (which stops its rest at whitespace). # generic absolute-URL pass: that pass stops its ``rest`` at whitespace,
# so the signed tail after the first space used to survive (round 16).
# The slot collapses whole instead, like any other non-whole-coverable
# slot shape.
spacey_absolute = logging.LogRecord( spacey_absolute = logging.LogRecord(
"urllib3.connectionpool", "urllib3.connectionpool",
logging.DEBUG, logging.DEBUG,
@ -723,8 +726,25 @@ def test_url_redaction_filter_redirecting_survives_spacey_location() -> None:
None, None,
) )
assert filt.filter(spacey_absolute) is True assert filt.filter(spacey_absolute) is True
assert spacey_absolute.getMessage() == "Redirecting /<redacted> -> https://mirror.example/<redacted> page?sig=OtherSecret" assert spacey_absolute.getMessage() == "Redirecting /<redacted> -> /<redacted>"
assert "BearerSecret" not in spacey_absolute.getMessage() assert "BearerSecret" not in spacey_absolute.getMessage()
assert "OtherSecret" not in spacey_absolute.getMessage()
assert "QuerySecret" not in spacey_absolute.getMessage()
# Same in the first slot, where the recursive urlopen frame carries the
# previous raw Location.
spacey_absolute_t1 = logging.LogRecord(
"urllib3.connectionpool",
logging.DEBUG,
__file__,
1,
"Redirecting %s -> %s",
("https://mirror.example/other page?sig=OtherSecret", "/private/x"),
None,
)
assert filt.filter(spacey_absolute_t1) is True
assert spacey_absolute_t1.getMessage() == "Redirecting /<redacted> -> /<redacted>"
assert "OtherSecret" not in spacey_absolute_t1.getMessage()
# The sandbox arrow false positive stays excluded: the prefix anchor, # The sandbox arrow false positive stays excluded: the prefix anchor,
# not a strict tail, is what keeps non-Redirecting messages untouched. # not a strict tail, is what keeps non-Redirecting messages untouched.
@ -739,10 +759,13 @@ def test_url_redaction_filter_redirecting_covers_all_relative_ref_forms() -> Non
relative-part) also admits slash-less relative references — relative-part) also admits slash-less relative references —
``download?sign=…`` and ``?sign=…`` kept their signed queries verbatim, ``download?sign=…`` and ``?sign=…`` kept their signed queries verbatim,
and neither the slot rule nor the generic absolute-URL pass (which and neither the slot rule nor the generic absolute-URL pass (which
needs a scheme) could see them. A slot is now kept ONLY when it starts needs a scheme) could see them. A slot is now kept ONLY when the generic
with an absolute hierarchical URL (scheme at position 0), so every pass itself consumes it whole, so every relative-reference form collapses,
relative-reference form collapses and non-hierarchical schemes non-hierarchical schemes (``data:…``) collapse, a space-carrying
(``data:…``) collapse too; network-path references collapse with any absolute slot collapses instead of leaking its signed tail (round 16),
and so does a slot the pass stops early on — a quote that reads as a
closing mark, or an empty host the ``host`` group never matches;
network-path references collapse with any
userinfo credentials they carry.""" userinfo credentials they carry."""
from deerflow.logging_config import UrlRedactionFilter from deerflow.logging_config import UrlRedactionFilter
@ -758,6 +781,25 @@ def test_url_redaction_filter_redirecting_covers_all_relative_ref_forms() -> Non
("/private/x", "//user:tok@cdn.example/private/x?sig=OtherSecret", "Redirecting /<redacted> -> /<redacted>"), # network-path + userinfo ("/private/x", "//user:tok@cdn.example/private/x?sig=OtherSecret", "Redirecting /<redacted> -> /<redacted>"), # network-path + userinfo
# Absolute URLs are still kept whole for the generic absolute-URL pass. # Absolute URLs are still kept whole for the generic absolute-URL pass.
("/private/BearerSecret?token=QuerySecret", "https://mirror.example/other?sig=OtherSecret", "Redirecting /<redacted> -> https://mirror.example/<redacted>"), ("/private/BearerSecret?token=QuerySecret", "https://mirror.example/other?sig=OtherSecret", "Redirecting /<redacted> -> https://mirror.example/<redacted>"),
# ... but only when the generic pass consumes the slot WHOLE. Its
# ``host``/``rest`` groups stop at whitespace, so a space-carrying
# absolute slot leaks its signed tail if it is handed over (round 16).
("/private/BearerSecret?token=QuerySecret", "https://mirror.example/other page?sig=OtherSecret", "Redirecting /<redacted> -> /<redacted>"),
("https://mirror.example/other page?sig=OtherSecret", "/private/x", "Redirecting /<redacted> -> /<redacted>"),
("/private/x", "https://mirror.example/a\tb?sig=OtherSecret", "Redirecting /<redacted> -> /<redacted>"), # any whitespace, not just a space
("/private/x", "https://mirror.example/a%20b?sig=Ok", "Redirecting /<redacted> -> https://mirror.example/<redacted>"), # percent-encoded space stays absolute
# The pass also stops early INSIDE a whitespace-free absolute slot, so
# the "is it absolute" test alone was still not sufficient (review of
# #5687): a quote that reads as a closing mark ends ``rest`` there,
# and an empty host before the first ``/?#`` matches nowhere at all.
("/private/x", "https://mirror.example/a')b?sig=LeakedSigQuote", "Redirecting /<redacted> -> /<redacted>"),
("https://mirror.example/a')b?sig=LeakedSigQuote", "/private/x", "Redirecting /<redacted> -> /<redacted>"),
("/private/x", "https:///path?sig=LeakedSigEmptyHost", "Redirecting /<redacted> -> /<redacted>"),
("https:///path?sig=LeakedSigEmptyHost", "/private/x", "Redirecting /<redacted> -> /<redacted>"),
("/private/x", 'https://mirror.example/a")b?sig=LeakedSigDQuote', "Redirecting /<redacted> -> /<redacted>"),
# A quote embedded mid-path is NOT a closing mark, so that slot is
# still consumed whole and keeps its host for debuggability.
("/private/x", "https://mirror.example/a'b?sig=Ok", "Redirecting /<redacted> -> https://mirror.example/<redacted>"),
] ]
for t1, t2, expected in cases: for t1, t2, expected in cases:
record = logging.LogRecord("urllib3.connectionpool", logging.DEBUG, __file__, 1, "Redirecting %s -> %s", (t1, t2), None) record = logging.LogRecord("urllib3.connectionpool", logging.DEBUG, __file__, 1, "Redirecting %s -> %s", (t1, t2), None)