Skip to content

fix: resolve dangling symlinks during MCP roots path validation#2373

Open
mohammadmseet-hue wants to merge 1 commit into
ChromeDevTools:mainfrom
mohammadmseet-hue:fix-roots-symlink-validation
Open

fix: resolve dangling symlinks during MCP roots path validation#2373
mohammadmseet-hue wants to merge 1 commit into
ChromeDevTools:mainfrom
mohammadmseet-hue:fix-roots-symlink-validation

Conversation

@mohammadmseet-hue

Copy link
Copy Markdown

What

Harden MCP roots path validation against dangling symlinks in resolveCanonicalPath() (src/utils/files.ts), used by McpContext.validatePath() to keep file-writing tools inside the configured workspace roots.

Problem

For a not-yet-existing output path, resolveCanonicalPath() walks up to the nearest existing ancestor, canonicalizes it with fs.realpath(), and re-appends the missing trailing segments.

fs.realpath() reports a dangling symlink (a symlink whose target does not exist yet) as ENOENT. The walk then treated the link's own name as an ordinary missing segment and appended it verbatim, so the returned canonical path stayed inside the root even though the link points outside it. saveFile() writes to the original unresolved path, the write follows the symlink, and the file lands outside every configured root. This gap remained after the earlier hardening in #2127 and #2269.

Fix

In the ENOENT branch, lstat() each candidate component. If it is itself a symlink, resolve its target with readlink and continue canonicalization from the target. The returned canonical path now reflects the real write destination, so validatePath() rejects out-of-root targets. A MAX_SYMLINK_DEPTH = 40 guard (mirrors SYMLOOP_MAX) bounds symlink cycles among not-yet-existing targets.

Tests

Adds two unit tests to tests/utils/files.test.ts:

  • a dangling symlink at the leaf resolves to its target, not its in-tree path;
  • a dangling intermediate directory symlink resolves the final path through the link target.

node --test build/tests/utils/files.test.js passes; prettier --check and eslint are clean on both files.

resolveCanonicalPath() walks to the nearest existing ancestor for a
not-yet-existing path, but fs.realpath() reports a dangling symlink as
ENOENT, so the link name was re-appended verbatim and the returned path
stayed inside the root even though the link points outside it. A write to
such a link then escapes every configured MCP root.

Resolve a component that is itself a symlink via readlink and continue
canonicalization from its target, so the path reflects the real write
destination and roots validation rejects out-of-root targets. A
MAX_SYMLINK_DEPTH guard bounds symlink cycles among not-yet-existing
targets. Adds unit tests for a dangling leaf symlink and a dangling
intermediate directory symlink.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant