Skip to content

fix(provisioner): keep k8s calls off event loop - #3

Open
AnoobFeng wants to merge 3 commits into
mainfrom
feature/provisioner-blocking-io
Open

fix(provisioner): keep k8s calls off event loop#3
AnoobFeng wants to merge 3 commits into
mainfrom
feature/provisioner-blocking-io

Conversation

@AnoobFeng

Copy link
Copy Markdown
Owner

test pro

AnoobFeng pushed a commit that referenced this pull request Jul 27, 2026
bytedance#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 bytedance#4174 — Follow-up issues: bytedance#4212, bytedance#4213, bytedance#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 bytedance#4174 — willem-bd 5th-round review items #1-bytedance#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 8d86dbf 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 bytedance#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 bytedance#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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant