diff --git a/backend/app/channels/AGENTS.md b/backend/app/channels/AGENTS.md index 3e6613b69..e72144ccf 100644 --- a/backend/app/channels/AGENTS.md +++ b/backend/app/channels/AGENTS.md @@ -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-.cos..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 ` 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 -> ` 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: ` DEBUG and `Retrying (…) after connection broken by '': ` **WARNING** — above the INFO root, so reachable at the default log level — via `urllib3.connectionpool`, and `Incremented Retry for (url='')` 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/...`, 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-.cos..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 ` 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 -> ` 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 the generic pass consumes it whole (`logging_config._url_pass_consumes_slot` asks that pass itself, so the kept set cannot drift from its `host`/`rest` stop conditions) — every other RFC 3986 relative-reference form (slash-less paths, query-only, fragments, network-path), non-hierarchical scheme, space-carrying slot, quote-at-boundary slot (`https://h/a')b?sig=…`, where `rest` stops at the closing mark), and empty-host absolute (`https:///path?sig=…`, which `host` never matches) collapses, so neither `download?sign=…` nor `other page?sig=…` leaks its signed query), the per-request `scheme://host:port "METHOD /path?query HTTP/x.x"` DEBUG line, `Retry: ` DEBUG and `Retrying (…) after connection broken by '': ` **WARNING** — above the INFO root, so reachable at the default log level — via `urllib3.connectionpool`, and `Incremented Retry for (url='')` 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/...`, 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=; ` (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 diff --git a/backend/packages/harness/deerflow/logging_config.py b/backend/packages/harness/deerflow/logging_config.py index 1b8bd3698..bd29f5043 100644 --- a/backend/packages/harness/deerflow/logging_config.py +++ b/backend/packages/harness/deerflow/logging_config.py @@ -76,7 +76,8 @@ _URLLIB3_RETRYING_RE = re.compile(r"^(?PRetrying \(.*\) after connection b # slot: connectionpool passes the origin-form request target, and the Location # 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 -# collapse to ``/`` here; absolute slots are left for that pass. +# collapse to ``/`` 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 # literal — because an ``-> /path`` arrow is not urllib3-owned shape: # non-URL logs render it too (sandbox mount mappings log @@ -86,17 +87,29 @@ _URLLIB3_RETRYING_RE = re.compile(r"^(?PRetrying \(.*\) after connection b # header string, and interior spaces are legal field syntax a misbehaving # 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 -# 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 # 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 # line was constructed left to right. _URLLIB3_REDIRECTING_ORIGIN_RE = re.compile(r"^Redirecting (?P\S.*?) -> (?P\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+.-]*://") + +# A Redirecting slot is kept only when the generic absolute-URL pass consumes +# it WHOLE, so the rule is asked of that pass itself rather than of an +# 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 # 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 retry lines that log a bare origin-form target (``Retry: ``, ``Incremented Retry for (url='')``, ``Retrying (…) after - connection broken by '…': ``), and every non-absolute slot of - ``Redirecting -> `` (kept whole only when a scheme - starts the slot, for the generic pass to rewrite). The record is rewritten in place + connection broken by '…': ``), and every ``Redirecting + -> `` slot the generic pass would not consume whole + (see _url_pass_consumes_slot). 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. @@ -207,18 +220,22 @@ class UrlRedactionFilter(logging.Filter): return match.group("head") + ": /" def _redact_redirecting_origin(match: re.Match[str]) -> str: - # 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). + # A slot stays verbatim ONLY when the generic absolute-URL pass + # — which runs after this one — consumes it whole; that pass is + # asked directly (see _url_pass_consumes_slot). + # 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). 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: - return target if _SLOT_ABSOLUTE_URL_RE.match(target) else "/" + return target if _url_pass_consumes_slot(target) else "/" return "Redirecting " + _slot(match.group("t1")) + " -> " + _slot(match.group("t2")) diff --git a/backend/tests/test_logging_config.py b/backend/tests/test_logging_config.py index 260e92436..df3d5cb11 100644 --- a/backend/tests/test_logging_config.py +++ b/backend/tests/test_logging_config.py @@ -711,8 +711,11 @@ def test_url_redaction_filter_redirecting_survives_spacey_location() -> None: assert spacey_t1.getMessage() == "Redirecting / -> /" assert "QuerySecret" not in spacey_t1.getMessage() - # An absolute Location with an interior space stays whole for the - # generic absolute-URL pass (which stops its rest at whitespace). + # An absolute Location with an interior space must NOT be handed to the + # 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( "urllib3.connectionpool", logging.DEBUG, @@ -723,8 +726,25 @@ def test_url_redaction_filter_redirecting_survives_spacey_location() -> None: None, ) assert filt.filter(spacey_absolute) is True - assert spacey_absolute.getMessage() == "Redirecting / -> https://mirror.example/ page?sig=OtherSecret" + assert spacey_absolute.getMessage() == "Redirecting / -> /" 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 / -> /" + assert "OtherSecret" not in spacey_absolute_t1.getMessage() # The sandbox arrow false positive stays excluded: the prefix anchor, # 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 — ``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 + needs a scheme) could see them. A slot is now kept ONLY when the generic + pass itself consumes it whole, so every relative-reference form collapses, + non-hierarchical schemes (``data:…``) collapse, a space-carrying + 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.""" 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 / -> /"), # 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 / -> https://mirror.example/"), + # ... 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 / -> /"), + ("https://mirror.example/other page?sig=OtherSecret", "/private/x", "Redirecting / -> /"), + ("/private/x", "https://mirror.example/a\tb?sig=OtherSecret", "Redirecting / -> /"), # any whitespace, not just a space + ("/private/x", "https://mirror.example/a%20b?sig=Ok", "Redirecting / -> https://mirror.example/"), # 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 / -> /"), + ("https://mirror.example/a')b?sig=LeakedSigQuote", "/private/x", "Redirecting / -> /"), + ("/private/x", "https:///path?sig=LeakedSigEmptyHost", "Redirecting / -> /"), + ("https:///path?sig=LeakedSigEmptyHost", "/private/x", "Redirecting / -> /"), + ("/private/x", 'https://mirror.example/a")b?sig=LeakedSigDQuote', "Redirecting / -> /"), + # 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 / -> https://mirror.example/"), ] for t1, t2, expected in cases: record = logging.LogRecord("urllib3.connectionpool", logging.DEBUG, __file__, 1, "Redirecting %s -> %s", (t1, t2), None)