_rewrite_unique_bare_filenames handed the correlated /mnt/user-data virtual
path to Pattern.subn as a replacement template. That path is built from the
file's relative path, and a backslash is an ordinary character in a POSIX
filename, so a file written literally as "screenshots\q3.png" -- the shape a
model produces by passing a Windows-style path to a stdio server on a POSIX
host -- turned \q into an unknown template escape. Pattern.subn compiles the
template eagerly, so re.error escaped _convert_call_tool_result and failed the
whole tool call even though the server had already written the file, and the
agent never saw the path.
When the backslash does start a known escape (\r, \t, \b ...), the bare-filename
pass substituted that byte into the returned text instead, so
"screenshots\raw.png" came back as a path with a raw CR in the middle of it.
Insert the correlated path through a callable replacement, matching what
_rewrite_local_paths_in_text already does, so it is never parsed as a template.
* fix(mcp): resolve drive-qualified paths in file reference rewriting
urlparse reads a Windows drive prefix ("C:/...") as the URI scheme, so
_local_path_from_uri() returned None for every drive-qualified path and
MCP file references were never rewritten to /mnt/user-data/... virtual
paths on Windows hosts. file:// URIs were parsed with urlparse().path
alone, which also drops the drive qualifier.
- resolve file URIs through url2pathname so the /C:/... form keeps its
drive, and treat single-letter schemes as bare drive paths;
- match drive-qualified absolute paths in the free-text reference regex;
- build test URIs with Path.as_uri() and anchor absolute-path fixtures
at tmp_path so expectations are host-portable, and cover the
drive-prefix scheme quirk explicitly.
* fix(mcp): decode file URIs once and guard Windows path rejection
Review follow-up on #5242:
- url2pathname already percent-decodes on both platforms, so the extra
unquote() wrapper decoded references twice and broke filenames that
contain a literal '%'. Pass parsed.path straight through.
- On Windows, url2pathname raises OSError for paths containing a raw
'|' (e.g. file:///C:/tmp/a|b.png); catch it so one odd URI cannot
abort the whole best-effort rewrite pass.
- The relative-reference regex alternative now accepts backslash
separators, which is what Windows servers print for relative paths.
- Add Windows-only regressions driving the backslash free-text form
and a file:///C:/ URI end to end, plus the OSError rejection.
* fix(mcp): resolve file://C:/… URIs with a drive-qualified authority
Review follow-up on #5242 (two-slash Windows drive form):
- Some Windows tools emit file://C:/… without the third slash, which
puts the drive in the URI authority. Consult parsed.netloc: rebuild
the /C:/… URL path for a drive-qualified authority, keep the current
handling for empty and localhost authorities, and reject any other
host instead of silently treating its path as local.
- Extend the free-text regex so the two-slash form matches as one token
instead of the previous stray e://… mid-token match.
- Cover the two-slash form at the _local_path_from_uri unit, through
_rewrite_local_paths_in_text, and add a portable case asserting that
a remote-host file URI is ignored.
* fix(mcp): anchor the drive-qualified text alternative with a lookbehind
Review follow-up on #5242:
- [A-Za-z]:[\/] could steal a token at an earlier scan position:
for file:/tmp/… (single-slash form per RFC 8089 / Java File.toURI())
the match became e:/tmp/…, which resolves as a bare drive path and
left the reference unrewritten where /tmp/… was rewritten before.
Anchor the alternative with (?<![\w.-]) so word:/… shapes fall
through to the earlier alternatives.
- Add the missing coverage for the relative alternative's backslash
support (temp\page.yml through _rewrite_local_paths_in_text) and
a portable regression pinning the file:/… tokenization.
* fix(mcp): exclude internal temp files from workspace changes
* fix(mcp): address review — shared tmp subdir constant, any-depth docs, nested test
- Export MCP_TMP_SUBDIR from constants.py and import it in both stdio
launch paths (tools.py, task_tool_caller.py) so the "/tmp" suffix is
composed once.
- Document that the .mcp exclusion matches by directory name at any
depth (like .git/node_modules) in README.md and mcp/AGENTS.md —
subagent work dirs below the workspace root get their own .mcp/tmp.
- Pin the any-depth semantic in test_workspace_changes.py with a nested
workspace/project/.mcp assertion.
* docs(mcp): correct any-depth exclusion rationale; re-home tmp pinning comment
The previous commit justified the any-depth `.mcp` exclusion with subagent
work dirs sitting below the workspace root — a mechanism that doesn't
exist: subagents share the parent's thread_id and both stdio launch paths
resolve sandbox_work_dir(thread_id), so `.mcp/tmp` is always pinned at the
workspace root. Reword mcp/AGENTS.md and the test comment to the real
justification (consistency with the other reserved dir names, robustness
against a server creating a relative `.mcp` from another cwd).
Also move the orphaned "pinning the process temp dir" rationale from
tools.py to constants.py next to MCP_TMP_SUBDIR, where both importers see it.
* feat(skills): per-user skill isolation (#2905)
Implement user-scoped skill storage that isolates custom skills between
users while sharing public skills globally.
Key changes:
- Add UserScopedSkillStorage class for per-user custom skill directories
- Introduce get_or_new_user_skill_storage() factory with user_id context
- Auth middleware sets effective_user_id for request-scoped storage
- Agent/prompt/middleware now use user-scoped storage and prompt cache
- Sandbox mounts user-scoped skill directories for search/read tools
- Add validate_skill_file_path() to SkillStorage for path security
- Migration script supports --all-users bulk migration
- Frontend: add editable field to Skill type, error check in enableSkill
- All skill categories can be toggled (custom skills default to enabled)
- Update skill-creator SKILL.md with isolation-aware instructions
Tests:
- Add test_user_scoped_skill_storage.py (new)
- Update all existing skill tests for user-scoped storage
- Update sandbox, client, and router tests
* fix(skills): address second-round PR review feedback (#3889)
- P1-1: restrict legacy skill mount to users without custom skills
- P1-2: fail-closed for _is_disabled_skill_path (OSError → return True)
- P2-1: AND-merge global extensions_config skill disabled state
- P2-2: atomic write for _skill_states.json (mkstemp + replace)
- P2-3: normalize X-DeerFlow-Owner-User-Id in trusted boundary
- P2-4: LRU-bounded _enabled_skills_by_config_cache (OrderedDict, maxsize=256)
- P2-5: clear global prompt cache on PUBLIC skill toggle
- P2-6: invalidate skill caches on client.update_skill
* fix(tests): correct tool policy test after merge
* fix(skills): use DEFAULT_SKILLS_CONTAINER_PATH in UserScopedSkillStorage
The "/mnt/skills" literal in UserScopedSkillStorage.__init__ triggers
test_skill_container_path_defaults::test_mnt_skills_literal_is_owned_by_skill_constants_module
on CI. Migrate the default to the existing deerflow.constants constant,
matching the pattern already used by LocalSkillStorage, SkillStorage, and
the durable/tool_error middlewares.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix(mcp): migrate local MCP-produced files into sandbox outputs (#3597)
Stdio MCP servers (e.g. Playwright) write files to host paths that the
sandbox/artifact API cannot resolve, since it only serves paths under
/mnt/user-data. Copy local files referenced by ResourceLink results into
the thread's sandbox outputs dir and rewrite their URIs to
/mnt/user-data/outputs/... so they become readable.
Also scope pooled MCP sessions by user_id:thread_id instead of thread_id
alone, matching the per-(user_id, thread_id) filesystem isolation.
* fix(mcp): restrict file migration to trusted source roots (#3597)
Add a source-root allowlist to the MCP file-migration path so a
malicious or buggy MCP server cannot have us copy arbitrary host files
(e.g. /etc/passwd) into a thread's outputs directory, from where the
artifact API would serve them. Files are migrated only when located
under the OS temp dir (Playwright's default), the thread's own
user-data tree, or an operator-configured root via
DEERFLOW_MCP_MIGRATION_SOURCE_ROOTS.
Expand test coverage with allowlist/security cases (path escape refusal,
trusted-root acceptance), URL-encoded file:// paths, converter content
branches (image/embedded/error/structured), and copy/resolve failure
fallbacks.
* fix(mcp): harden local file migration into sandbox outputs
Address robustness and security gaps in the MCP ResourceLink file
migration:
- Set migrated files to 0o644 so a differently-UID sandbox container can
read them, instead of inheriting the source's (possibly 0o600) mode.
- Enforce the 100MB size cap during the copy (chunked, byte-counted)
rather than from a prior stat(), closing the grow-after-stat TOCTOU.
- Create the destination atomically with O_CREAT|O_EXCL to remove the
check-then-create name-collision race.
- Document the shared-$TMPDIR multi-tenant read surface and mitigation.
Add regression tests: symlink escape refusal, explicit $TMPDIR source
migration, 0o644 mode, and the outputs/user-data resolve() OSError
fallback branches.
* fix(mcp): migrate playwright text file outputs
* fix(mcp): translate MCP file outputs to virtual paths instead of copying (#3597)
Pin stdio MCP subprocess cwd and TMPDIR/TMP/TEMP under the thread workspace
so produced files always land in the mounted user-data tree, then rewrite
returned references via deterministic host->virtual path translation. Free
text is best-effort only: a reference is rewritten only when it resolves to
an existing file inside the thread's tree, and bare filenames are matched
against files created/modified by the same tool call. Replaces the previous
copy-into-outputs + regex approach (which missed cases like temp/page-*.yml).
* style(mcp): apply ruff format to mcp path translation tests
* perf(mcp): offload stdio FS work off event loop and gate on transport
Address review on #3600:
- Wrap the workspace dir prep, snapshot diff, and per-token path
resolution in asyncio.to_thread so they no longer block the event
loop (matches the repo's blocking-IO gate convention).
- Gate the cwd/temp pinning and snapshots on stdio transport only;
SSE/HTTP servers skip the filesystem work entirely.
- Skip the post-call snapshot diff when the result has no text content.
* test(mcp): cover stdio transport gating and text-content after-walk skip
Add unit/integration coverage for the new review-driven behavior:
- _prepare_stdio_workspace dir/temp/snapshot bundle
- _result_has_text_content detection (text, embedded text, image, empty)
- non-stdio transport skips cwd/temp pinning and touches no workspace dirs
- post-call snapshot diff is skipped without text content and runs with it
* fix(mcp): address stdio path rewrite review feedback
- Restrict the stdio MCP temp directory to 0700 instead of 0777.
- Preserve operator-provided stdio cwd values while keeping injected cwd values as strings.
- Add debug logging for deterministic path rewrites and bare-filename rewrite decisions.
- Document the stdio cwd/temp pinning, virtual-path translation, and user/thread session scope.
- Cover explicit cwd preservation and temp-dir permissions in session-pool tests.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
