2 Commits

Author	SHA1	Message	Date
Aari	7b330101d2	fix(tools): exclude injected runtime from list_uploaded_files schema (#4375) (#4376) ... Declaring the injected runtime arg as `Annotated[Runtime, InjectedToolArg] | None` made the top-level annotation a Union, so LangChain no longer treated it as injected. It leaked into the model-facing schema and pydantic raised PydanticInvalidForJsonSchema on the ToolRuntime dataclass the moment the tool was bound to a model. The tool is bound by default for the lead agent, so any default run on an OpenAI-compatible provider failed at tool-bind time. Declare runtime as a bare Runtime first param, matching every other built-in tool (present_files, view_image, task, ...), which LangChain auto-injects and auto-excludes from the schema. Add a schema regression test that binds the tool.	2026-07-23 08:22:15 +08:00
Lee minjing	e225ad57d7	feat(uploads): lazy-load historical files via list_uploaded_files tool (#4174) ... * feat(uploads): lazy-load historical files via list_uploaded_files tool Replace per-turn injection of all historical upload metadata with on-demand discovery via a new `list_uploaded_files` built-in tool, following the same deferred-discovery pattern used by skills. - Rename <uploaded_files> block to <current_uploads> (current-run files only) - Add list_uploaded_files tool with include_outline: bool|list[str] - Extract outline helpers to shared deerflow/utils/file_outline.py - Update system prompt to reflect lazy-loading behaviour - Historical file scan removed from UploadsMiddleware.before_agent() Co-Authored-By: Claude <noreply@anthropic.com> * fix(uploads): clear uploaded_files state when no new files in current turn When before_agent() returns None on empty turns, the LastValue uploaded_files field retains the previous turn's filenames. list_uploaded_files then incorrectly excludes those files as "current-run" files, making them invisible until the next upload. Fix: return {"uploaded_files": []} instead of None to explicitly clear state. Add two-turn regression test covering the exact scenario from review feedback. Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve CI lint errors and stale test assertion from merge - Split long prompt line to fit 240-char limit - Add missing `Any` import in list_uploaded_files_tool - Remove unused `re` import in file_conversion (outline code moved) - Remove unused `os` import in middleware test - Fix test assertion: <uploaded_files> → <current_uploads> after main merge Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve CI lint errors and stale test assertion from merge - Split long prompt line to fit 240-char limit - Add missing `Any` import in list_uploaded_files_tool - Remove unused `re` import in file_conversion (outline code moved) - Remove unused `os` import in middleware test - Fix test assertion: <uploaded_files> → <current_uploads> after main merge Co-Authored-By: Claude <noreply@anthropic.com> * fix: add current_uploads to input sanitization exempt tags The lazy-loading PR renamed <uploaded_files> to <current_uploads>. The anti-drift guard scans all framework XML blocks and requires each to be either blocked or explicitly exempted. current_uploads wraps trusted server-generated file metadata, not user input, so it belongs in the exempt set. Co-Authored-By: Claude <noreply@anthropic.com> * test: regenerate replay golden after uploaded_files state change before_agent now returns {"uploaded_files": []} instead of None, adding uploaded_files to SSE values events. Regenerated via DEERFLOW_WRITE_GOLDEN=1. Co-Authored-By: Claude <noreply@anthropic.com> * fix: review feedback — memory pipeline, stale tags, state clearing, nits - Match both tags in memory stripping pipeline (uploaded_files|current_uploads) - Remove stale uploaded_files from _BLOCKED_TAG_NAMES - Clear uploaded_files on all before_agent early-return paths - Fix ponytail: stray word in file_conversion re-export comment - Remove dead total_omitted branch in _format_omitted_summary - ruff format fixes Co-Authored-By: Claude <noreply@anthropic.com> * fix: block current_uploads, sanitize only original user content Per review feedback: instead of exempting <current_uploads> (which allows user forgery), move it to _BLOCKED_TAG_NAMES and change InputSanitizationMiddleware._process_request to scan only the original user content (ORIGINAL_USER_CONTENT_KEY) when available. Server-injected trusted blocks are no longer checked against the blocked-tag denylist. Co-Authored-By: Claude <noreply@anthropic.com> * docs: clarify fallback reason in input sanitization comment Co-Authored-By: Claude <noreply@anthropic.com> * @ fix: third-round review feedback — state visibility, sanitization, regex, nits - list_uploaded_files_tool: logger.warning instead of silent try/except on runtime.state read failure (High) - input_sanitization_middleware: _extract_text_from_content skips empty text blocks to match message_content_to_text behaviour; rfind fallback path logs warning for observability (Medium) - memory pipeline regexes: backreference (?P<tag>)(?P=tag) in message_processing.py and prompt.py (Low) - file_conversion.py: re-export moved to top of file (Low) - Tests: middleware→tool state bridge test; integrated forged-tag + multimodal sanitization tests PR #4174 — Follow-up issues: #4212, #4213, #4214 Co-Authored-By: Claude <noreply@anthropic.com> @ * @ fix: 4th-round review — denylist, sanitization, scandir, nits - Add "uploaded_files" back to _BLOCKED_TAG_NAMES (old tag still processed by deermem; user forgery must be escaped) (consistency) - Fix inaccurate rfind-fallback comment: UploadsMiddleware keeps string as string, fallback is unreachable for strings (doc fix) - Distinguish "empty string key" (upload without text) from "non-string key" (caller forgery) so empty-text uploads never escape the server block (edge) - Merge dual os.scandir(uploads_dir) calls into one list re-use (minor) - Add comment on .md sibling skip known limitation: user-uploaded .md files whose stem collides with a converted doc are hidden (boundary, no code change) Co-Authored-By: Claude <noreply@anthropic.com> @ * @ fix: tighten rfind-failure fallback — distinguish server blocks from user blocks When _extract_text_from_content and message_content_to_text disagree on multimodal list content and rfind fails, use content[0] (server-injected <current_uploads> block) vs content[1:] (user blocks) to sanitize only user blocks. Raw strings and non-standard dict blocks that _extract_text_from_content misses are now also sanitized. Non-distinguishable paths (< 2 text blocks, non-list content) still degrade to full sanitization (safe — server block may be escaped but user forgery never leaks). All fallback paths log via logger.warning. Decision 18 / willem-bd 4th-round comment #3 Co-Authored-By: Claude <noreply@anthropic.com> @ * @ fix: correct comments referencing text_blocks → content in rfind fallback Co-Authored-By: Claude <noreply@anthropic.com> @ * fix: 5th-round review — dead code, subagent gating, integration test, perf, consistency - Delete unreachable ORIGINAL_USER_CONTENT_KEY guard in rfind fallback branch (original_user_content guaranteed non-empty str at that point) - Remove list_uploaded_files from BUILTIN_TOOLS; add include_upload_tool param to get_available_tools(), default True; task_tool.py passes False so subagents no longer receive a tool whose state exclusion is broken - Add integration test exercising real create_agent graph (not mocked runtime.state) to verify LangGraph propagates before_agent state writes into ToolRuntime.state during same-turn tool calls - Cache DirEntry.stat() st_size in candidates tuple to avoid second per-file syscall in the rendering loop - Make the upload-tag pre-check case-insensitive (content_str.lower()) to match _UPLOAD_BLOCK_RE re.IGNORECASE PR #4174 — willem-bd 5th-round review items #1-#5 Co-Authored-By: Claude <noreply@anthropic.com> * fix(channels): pass files metadata through _human_input_message() for IM uploads _human_input_message() was not passing additional_kwargs.files to the downstream message. UploadsMiddleware read no files, wrote uploaded_files=[], and list_uploaded_files reported same-run IM attachments as historical files (fancyboi999 repro). Fix: add files parameter to _human_input_message(), call site passes files=uploaded. Regression test locks the contract. Co-Authored-By: Claude <noreply@anthropic.com> * fix(channels): remove legacy <uploaded_files> manual prepend to fix double-injection regression Commit 8d86dbf6 added files= pass-through to UploadsMiddleware but left the manual _format_uploaded_files_block() prepend in place. Every IM attachment reached the model twice — once via the legacy <uploaded_files> block and once via <current_uploads>. This commit removes the manual prepend and the now-dead _format_uploaded_files_block() function. UploadsMiddleware is the sole upload-context producer for both IM and web paths. Reported-by: fancyboi999 (PR review) Co-Authored-By: Claude <noreply@anthropic.com> * docs: update #4212 issue body to reflect completed fixes and narrowed remaining scope * chore: remove temporary scratch file * fix(middleware): neutralize user-derived values inside <current_uploads> block Upload-derived filenames, paths, outline titles, and preview text are interpolated verbatim inside the trusted <current_uploads> wrapper, which InputSanitizationMiddleware exempts from sanitization. A crafted filename or document heading containing blocked authority tags would bypass the guardrail and enter model context as trusted framework data. Fix: call neutralize_untrusted_tags() on all four user-derived values inside _format_file_entry(), preserving the outer <current_uploads> wrapper untouched. Reported-by: fancyboi999 (P1 security review) Co-Authored-By: Claude <noreply@anthropic.com> * fix(middleware): neutralize extension labels in omitted-file summary Files exceeding the 10-item context cap bypass _format_file_entry(). Their extensions, derived from user-controlled filenames via _extension_label(), were interpolated verbatim into the trusted <current_uploads> wrapper — another path for blocked authority tags to escape the guardrail. Fix: neutralize extension values inside _extension_label(), the single extraction point for all extension labels. Reported-by: fancyboi999 (P1 security review) Co-Authored-By: Claude <noreply@anthropic.com> * fix(tools): neutralize user-derived values in list_uploaded_files tool result Apply neutralize_untrusted_tags() to every model-visible user-derived value returned by list_uploaded_files: filename, virtual path, extension, outline titles, outline preview lines, and omitted-file extension summary. This closes the last remaining injection bypass in the upload lazy-loading path - the <current_uploads> block and its omitted summary were already neutralized (previous commits), but the list_uploaded_files tool produced a second exit for the same attacker-controlled metadata that ToolResultSanitizationMiddleware did not cover. Co-Authored-By: Claude <noreply@anthropic.com> * fix(tests): add missing include_upload_tool=False to task_tool mock assertions PR #4174 added include_upload_tool parameter to get_available_tools(). task_tool.py correctly passes include_upload_tool=False for subagents but 5 existing tests' assert_called_once_with expectations were not updated, causing CI failures. Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-22 14:02:56 +08:00