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

fix(logging): collapse every non-absolute Redirecting slot, closes the #5225 round-15 residual (#5680)

Browse Source 
A Redirecting slot stayed verbatim unless it started with "/", but the
Location field-value grammar (RFC 3986 relative-part) also admits
slash-less relative references: 'Redirecting /private/x?token=Q ->
download?sign=LeakedSig' rendered the signed query verbatim, and neither
the slot rule nor the generic absolute-URL pass (which needs a scheme)
could see it. A slot is now kept only when a scheme matches at position
0 (left for the generic pass to rewrite); everything else collapses -
slash-less relative paths, query-only and fragment-only forms,
network-path references (collapsing any userinfo they carry), and
non-hierarchical schemes such as data: URIs. The ^Redirecting prefix
anchor is unchanged, so non-URL '-> /path' arrows (sandbox mount
mappings) keep passing through untouched.

Regression table covers all relative-reference forms plus the absolute
regression anchor; mutation-verified (reverting to startswith('/') goes
red). Aligned with the fix sketched in the #5225 round-15 review thread.
This commit is contained in:
hataa 2026-09-22 11:08:07 +08:00 committed by GitHub
parent e88599bb29
commit 4bc4241531
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 54 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -24,7 +24,7 @@ Bridges external messaging platforms (Feishu, Slack, Telegram, Discord, DingTalk
- For GitHub, the webhook router verifies the delivery then calls `fanout_event(bus, ...)`; matching agent bindings publish one `InboundMessage` each instead of a long-polling channel worker.
- Telegram photo/document updates use the largest photo size or document metadata, preserve `message.caption`, enforce the hosted Bot API's 20,000,000-byte download ceiling before and after download, and never expose the token-bearing Bot API file URL. Downloaded bytes cross the adapter/manager boundary only through `message_bus.INBOUND_FILE_CONTENT_KEY`; the manager consumes that transient field before persisting safe upload metadata.
- Feishu/Lark inbound image/file downloads read at most 20,000,001 bytes and reject anything above 20,000,000 bytes before persistence or sandbox sync. Oversize, unsafe-path, and path-resolution failures rewrite only that attachment placeholder to `Failed to obtain the [type]` so later attachments in the same message can still load.
- WeChat iLink inbound image/file downloads stream with an in-flight cap derived from `channels.wechat.max_inbound_image_bytes` / `max_inbound_file_bytes` (20 MB / 50 MB defaults) that aborts before the full read, and the payload-supplied `media.full_url` must pass a scheme + dot-boundary host-suffix allowlist (`channels.wechat.allowed_media_hosts`, defaulting to the `qq.com` family plus the configured `cdn_base_url` host) before anything is fetched. The limits are PLAINTEXT limits while the stream measures CIPHERTEXT, so the in-flight cap is the PKCS#7-padded size of exactly-limit plaintext (`WechatChannel._stream_cap_for`); the exact post-decryption check stays as the authority. There is deliberately **no URL re-fetch fallback**: `_read_wechat_inbound_file` (manager) only reads the locally staged `path` — the channel always stages one before publishing — so nothing fetches a WeChat URL outside the channel-side gate. WeCom media URLs (frame-supplied `url`, no per-channel size config) get the same style of host gate — `qq.com` suffixes plus the COS shape `ww-aibot-img-<APPID>.cos.<region>.myqcloud.com` WeCom serves signed links from, where the numeric suffix must be one of the verified WeCom-owned Tencent Cloud APPIDs (`_WECOM_MEDIA_COS_APPIDS`, currently only the `1258476243` observed in Tencent's published callback examples) because bucket names are user-chosen and any Tencent Cloud account can register a lookalike `ww-aibot-img-*` bucket; other accounts go through operator suffixes from `channels.wecom.allowed_media_hosts` resolved from the live channel at read time — and the manager-level `MAX_INBOUND_URL_FILE_BYTES` (50 MB) streaming cap. Both URL downloads and `_download_cdn_bytes` request `Accept-Encoding: identity`, refuse a response with any residual `Content-Encoding` before reading, and iterate `aiter_raw()`: httpx's transparent decoding would allocate the full decompressed body before the byte cap sees a chunk. No URL-based inbound-media log line may contain any part of the URL (signed links carry credentials in path and query): failure labels use the attachment filename or host only (`manager._inbound_file_label`), and reader failures log a sanitized exception summary — class name plus HTTP status (`manager._reader_error_summary`, `wechat._media_download_error_summary`), never the URL-bearing message or traceback. The rule extends to the HTTP client libraries' own records: httpx emits `HTTP Request: GET <full URL>` at INFO via the bare `httpx` logger, and urllib3 — enumerated against the installed source; every remaining emitter logs host:port only (connection establishment/reset) or one absolute URL (the header-parse warning) that the generic pattern rewrites — renders a request target in five shapes, each needing a dedicated redaction pattern because the generic absolute-URL pass cannot see a target with no scheme: `Redirecting <target> -> <target>` at INFO via `urllib3.poolmanager` and DEBUG via `urllib3.connectionpool` (either slot may be an origin-form relative Location, which pool/connection callers pass or the Location header carries; the shape keeps the `^Redirecting ` prefix anchor because `-> /path` arrows also appear in non-URL logs — sandbox mount mappings — and must pass through untouched, while both slots consume through end-of-line: the raw Location header may carry interior spaces, and the recursive urlopen frame can pass it on as the next request target), the per-request `scheme://host:port "METHOD /path?query HTTP/x.x"` DEBUG line, `Retry: <target>` DEBUG and `Retrying (…) after connection broken by '<error>': <target>` **WARNING** — above the INFO root, so reachable at the default log level — via `urllib3.connectionpool`, and `Incremented Retry for (url='<target>')` DEBUG via `urllib3.util.retry` (origin-form on the request path, absolute on the redirect path). The generic pass's `rest` treats a quote as a closing mark only at a boundary — followed by whitespace, a closing parenthesis, or end of string — so urllib3's `Incremented Retry for (url='…')` keeps its `')` scaffolding while a quote embedded in a URL stays consumed and redacted. Logger filters only see records emitted through that exact logger — child loggers neither inherit them nor trigger them on propagation — so `UrlRedactionFilter` is attached to the `httpx` logger AND to every root handler by `configure_logging` (handler-level filters see propagated records), which also keeps Telegram's token-bearing Bot API paths (`api.telegram.org/bot<token>/...`, emitted by python-telegram-bot's httpx client) out of the logs.
- WeChat iLink inbound image/file downloads stream with an in-flight cap derived from `channels.wechat.max_inbound_image_bytes` / `max_inbound_file_bytes` (20 MB / 50 MB defaults) that aborts before the full read, and the payload-supplied `media.full_url` must pass a scheme + dot-boundary host-suffix allowlist (`channels.wechat.allowed_media_hosts`, defaulting to the `qq.com` family plus the configured `cdn_base_url` host) before anything is fetched. The limits are PLAINTEXT limits while the stream measures CIPHERTEXT, so the in-flight cap is the PKCS#7-padded size of exactly-limit plaintext (`WechatChannel._stream_cap_for`); the exact post-decryption check stays as the authority. There is deliberately **no URL re-fetch fallback**: `_read_wechat_inbound_file` (manager) only reads the locally staged `path` — the channel always stages one before publishing — so nothing fetches a WeChat URL outside the channel-side gate. WeCom media URLs (frame-supplied `url`, no per-channel size config) get the same style of host gate — `qq.com` suffixes plus the COS shape `ww-aibot-img-<APPID>.cos.<region>.myqcloud.com` WeCom serves signed links from, where the numeric suffix must be one of the verified WeCom-owned Tencent Cloud APPIDs (`_WECOM_MEDIA_COS_APPIDS`, currently only the `1258476243` observed in Tencent's published callback examples) because bucket names are user-chosen and any Tencent Cloud account can register a lookalike `ww-aibot-img-*` bucket; other accounts go through operator suffixes from `channels.wecom.allowed_media_hosts` resolved from the live channel at read time — and the manager-level `MAX_INBOUND_URL_FILE_BYTES` (50 MB) streaming cap. Both URL downloads and `_download_cdn_bytes` request `Accept-Encoding: identity`, refuse a response with any residual `Content-Encoding` before reading, and iterate `aiter_raw()`: httpx's transparent decoding would allocate the full decompressed body before the byte cap sees a chunk. No URL-based inbound-media log line may contain any part of the URL (signed links carry credentials in path and query): failure labels use the attachment filename or host only (`manager._inbound_file_label`), and reader failures log a sanitized exception summary — class name plus HTTP status (`manager._reader_error_summary`, `wechat._media_download_error_summary`), never the URL-bearing message or traceback. The rule extends to the HTTP client libraries' own records: httpx emits `HTTP Request: GET <full URL>` at INFO via the bare `httpx` logger, and urllib3 — enumerated against the installed source; every remaining emitter logs host:port only (connection establishment/reset) or one absolute URL (the header-parse warning) that the generic pattern rewrites — renders a request target in five shapes, each needing a dedicated redaction pattern because the generic absolute-URL pass cannot see a target with no scheme: `Redirecting <target> -> <target>` at INFO via `urllib3.poolmanager` and DEBUG via `urllib3.connectionpool` (either slot may be an origin-form relative Location, which pool/connection callers pass or the Location header carries; the shape keeps the `^Redirecting ` prefix anchor because `-> /path` arrows also appear in non-URL logs — sandbox mount mappings — and must pass through untouched, while both slots consume through end-of-line: the raw Location header may carry interior spaces, and the recursive urlopen frame can pass it on as the next request target. A slot is kept verbatim only when it starts with an absolute URL (scheme at position 0) for the generic pass to rewrite — every other RFC 3986 relative-reference form (slash-less paths, query-only, fragments, network-path) and non-hierarchical schemes collapse, so a slash-less Location like `download?sign=…` cannot leak its signed query), the per-request `scheme://host:port "METHOD /path?query HTTP/x.x"` DEBUG line, `Retry: <target>` DEBUG and `Retrying (…) after connection broken by '<error>': <target>` **WARNING** — above the INFO root, so reachable at the default log level — via `urllib3.connectionpool`, and `Incremented Retry for (url='<target>')` DEBUG via `urllib3.util.retry` (origin-form on the request path, absolute on the redirect path). The generic pass's `rest` treats a quote as a closing mark only at a boundary — followed by whitespace, a closing parenthesis, or end of string — so urllib3's `Incremented Retry for (url='…')` keeps its `')` scaffolding while a quote embedded in a URL stays consumed and redacted. Logger filters only see records emitted through that exact logger — child loggers neither inherit them nor trigger them on propagation — so `UrlRedactionFilter` is attached to the `httpx` logger AND to every root handler by `configure_logging` (handler-level filters see propagated records), which also keeps Telegram's token-bearing Bot API paths (`api.telegram.org/bot<token>/...`, emitted by python-telegram-bot's httpx client) out of the logs.
2. `ChannelManager._dispatch_loop()` consumes from queue
3. For user-owned channel connections, incoming messages carry `connection_id`, `owner_user_id`, and `workspace_id`; `owner_user_id` becomes the DeerFlow run `user_id`, while the raw platform user id remains `channel_user_id`. The Gateway accepts `channel_user_id` only from an internally authenticated channel caller's top-level `body.context`, clears it from both free-form `body.config` sections, and writes it into runtime context only (never `configurable`, which is checkpointed). `bash_tool` exposes it to sandbox commands as the fixed env var `DEERFLOW_CHANNEL_USER_ID` — via a shell-quoted command-string prefix, NOT the `execute_command(env=...)` channel, which is reserved for request-scoped secrets and would switch `AioSandbox` onto the `bash.exec` path (image >= 1.9.3, fresh session per call). Per-call injection keeps group-chat identity correct (one thread/sandbox, many senders) **without depending on the AIO shell's session semantics**: every IM-channel command carries an explicit `export VAR=<id>; ` (valid id) or `unset VAR; ` (empty / non-str / over the 256-char cap). The AIO no-env path reuses a persistent shell session (the reason for the class lock, #1433), so a bare command could otherwise resolve a stale id an earlier sender exported; the `unset` closes the window the length/type guard would open (a dropped id would inherit the previous sender's value). Non-IM runs (no `channel_user_id` in context) are left untouched. Not injected on the Windows local sandbox (its PowerShell/cmd.exe fallback has no `export`/`unset`). Propagates across `task` delegation: `task_tool` captures the dispatching turn's id and the subagent executor forwards it into the subagent's runtime context, same as the guardrail attribution fields. The runtime-context value is authorization-grade at the Gateway/guardrail boundary, but the exported shell variable remains informational because any bash command can overwrite its own environment; skills must not treat the shell variable itself as authenticated identity. Tests: `tests/test_gateway_services.py`, `tests/test_channel_user_id_env.py`
4. For chat: look up/create thread through Gateway's LangGraph-compatible API

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

@ -93,6 +93,11 @@ _URLLIB3_RETRYING_RE = re.compile(r"^(?P<head>Retrying \(.*\) after connection b
# line was constructed left to right.
_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
# form, and non-hierarchical schemes) collapses — see _redact_redirecting_origin.
_SLOT_ABSOLUTE_URL_RE = re.compile(r"[a-zA-Z][a-zA-Z0-9+.-]*://")
# 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
# ``://`` makes each attempt walk to the end of the run, which is quadratic
@ -161,8 +166,9 @@ class UrlRedactionFilter(logging.Filter):
per-request ``scheme://host:port "METHOD target HTTP/x.x"`` line, the
retry lines that log a bare origin-form target (``Retry: <target>``,
``Incremented Retry for (url='<target>')``, ``Retrying (…) after
connection broken by '…': <target>``), and origin-form halves of
``Redirecting <target> -> <target>``. The record is rewritten in place
connection broken by '…': <target>``), and every non-absolute slot of
``Redirecting <target> -> <target>`` (kept whole only when a scheme
starts the slot, for the generic pass to rewrite). The record is rewritten in place
(``msg`` set to the redacted formatted message, ``args`` cleared) so
every downstream handler and formatter — text or JSON — sees the same
redacted line, while the method/status/error observability is preserved.
@ -201,10 +207,18 @@ class UrlRedactionFilter(logging.Filter):
return match.group("head") + ": /<redacted>"
def _redact_redirecting_origin(match: re.Match[str]) -> str:
# Origin-form slots collapse; absolute slots stay for the generic
# absolute-URL pass (which runs after this one).
# A slot stays verbatim ONLY when it is an absolute URL (a
# scheme at position 0), so the generic absolute-URL pass —
# which runs after this one — rewrites it. Everything else
# collapses: the Location field-value grammar (RFC 3986
# relative-part) also admits slash-less relative references
# (``download?sign=…``, ``?sign=…``, ``#frag``), network-path
# references (``//host/x``, whose userinfo collapses with it),
# and non-hierarchical schemes (``data:…``) — none of which
# either pass could otherwise see, and the slash-less forms
# kept their signed queries verbatim (round 15).
def _slot(target: str) -> str:
return "/<redacted>" if target.startswith("/") else target
return target if _SLOT_ABSOLUTE_URL_RE.match(target) else "/<redacted>"
return "Redirecting " + _slot(match.group("t1")) + " -> " + _slot(match.group("t2"))

34
backend/tests/test_logging_config.py
View File

@ -731,3 +731,37 @@ def test_url_redaction_filter_redirecting_survives_spacey_location() -> None:
sandbox = logging.LogRecord("deerflow.sandbox.local.local_sandbox_provider", logging.ERROR, "p.py", 1, "sandbox.mounts entry /srv/knowledge -> /mnt/knowledge ignored: missing", (), None)
assert filt.filter(sandbox) is True
assert sandbox.getMessage() == "sandbox.mounts entry /srv/knowledge -> /mnt/knowledge ignored: missing"
def test_url_redaction_filter_redirecting_covers_all_relative_ref_forms() -> None:
"""Round-15 residual: a Redirecting slot stayed verbatim unless it
started with "/", but the Location field-value grammar (RFC 3986
relative-part) also admits slash-less relative references —
``download?sign=…`` and ``?sign=…`` kept their signed queries verbatim,
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
with an absolute hierarchical URL (scheme at position 0), so every
relative-reference form collapses and non-hierarchical schemes
(``data:…``) collapse too; network-path references collapse with any
userinfo credentials they carry."""
from deerflow.logging_config import UrlRedactionFilter
filt = UrlRedactionFilter()
cases = [
# (t1, t2, expected t2 rendering after the generic pass runs)
("/private/BearerSecret?token=QuerySecret", "download?sign=LeakedSig", "Redirecting /<redacted> -> /<redacted>"), # round-15 repro
("/private/BearerSecret?token=QuerySecret", "?sign=LeakedSig", "Redirecting /<redacted> -> /<redacted>"), # query-only
("/private/x", "#frag", "Redirecting /<redacted> -> /<redacted>"), # fragment-only
("/private/x", "data:application/json;base64,SECRET", "Redirecting /<redacted> -> /<redacted>"), # non-hierarchical scheme
("/private/x", "//cdn.example/private/x?sig=OtherSecret", "Redirecting /<redacted> -> /<redacted>"), # network-path
("/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.
("/private/BearerSecret?token=QuerySecret", "https://mirror.example/other?sig=OtherSecret", "Redirecting /<redacted> -> https://mirror.example/<redacted>"),
]
for t1, t2, expected in cases:
record = logging.LogRecord("urllib3.connectionpool", logging.DEBUG, __file__, 1, "Redirecting %s -> %s", (t1, t2), None)
assert filt.filter(record) is True
assert record.getMessage() == expected, (t1, t2)
assert "LeakedSig" not in record.getMessage()
assert "token=QuerySecret" not in record.getMessage()