fix: prevent path traversal in walkDirectory targetPath (CWE-22)

[sebastiondev]

Summary

walkDirectory() in src/core/walker.ts accepts a caller-supplied targetPath that is joined to rootDir via resolve() without any containment check. Because resolve() happily collapses .. segments and follows symlinks, an MCP client (or a prompt-injected agent driving one) can pass values like "../../.." or a symlink that points outside the project root and have the walker enumerate arbitrary directories on the host. The resulting file list — including header/symbol extracts produced by downstream tools — is returned to the MCP client, giving an attacker arbitrary filesystem read/enumeration scoped only by the OS permissions of the server process.

• CWE: CWE-22 (Path Traversal)
• Affected function: walkDirectory in src/core/walker.ts
• Reachable sink: the context_tree MCP tool propagates the user-controlled target_path argument into walkDirectory({ rootDir, targetPath }). Other callers pass targetPath: undefined and are unaffected, but a single fix at the chokepoint covers them all.

Data flow

MCP client args.target_path
  → context_tree tool handler
  → walkDirectory({ rootDir, targetPath })
  → resolve(rootDir, targetPath)         // ← no containment check
  → readdir() recursion                  // ← arbitrary FS enumeration
  → results returned to MCP client

Fix

In walkDirectory:

1. realpath() both rootDir and the resolved startDir so symlink targets are evaluated, not the link names.
2. Compute relative(rootRealPath, startRealPath) and reject if the result is absolute or starts with .. (i.e. anything outside the root).
3. On rejection, throw rather than silently returning [], so the MCP client gets a clear error instead of an empty/misleading result.
4. Pass the realpath-resolved root to loadIgnoreRules and walkRecursive so ignore matching is consistent with the canonicalised tree.

The check is implemented as a small helper:

function isWithinRoot(rootDir: string, targetPath: string): boolean {
  const relPath = relative(rootDir, targetPath);
  return relPath === "" || (!relPath.startsWith("..") && !isAbsolute(relPath));
}

This is the standard Node pattern for path containment and correctly handles the three classes of bypass we tested (.. traversal, sibling directories sharing a name prefix, and symlinks pointing outside the root).

Tests

Added three regression tests in test/main/walker.test.mjs:

• rejects targetPath traversal outside root — targetPath: ".."
• rejects targetPath traversal to sibling paths with shared prefixes — guards against a naïve startsWith(rootDir) check (e.g. /tmp/fixtures vs /tmp/fixtures-sibling).
• rejects symlink escapes passed as targetPath — creates a symlink inside the fixture pointing outside, confirms realpath resolution catches it.

Full test suite: 218 tests passing, including the three new ones. No existing behaviour changed for legitimate in-root targetPath values (verified by the existing walker tests).

Why this is exploitable

Preconditions are minimal:

• Attacker can influence MCP tool arguments. In agent setups this happens via prompt injection of the model that drives the MCP client — a well-documented threat model for MCP servers.
• The server process has filesystem read access outside ROOT_DIR. This is true by default: nothing in the project sandboxes the Node process, so anything the user account can read is reachable.

There is no auth layer between the MCP transport and the tool handler, no allowlist on target_path, and no canonicalisation prior to this fix. The pre-fix stat(startDir) only checks existence; it does not constrain location.

Adversarial review

Before submitting we tried to talk ourselves out of this finding. The two plausible mitigations would be (a) framework-level path scoping in the MCP SDK, or (b) the OS limiting what the process can read. Neither applies: the MCP SDK does not inspect tool arguments for filesystem semantics, and the server is intended to run as the developer's user, which is exactly the account with access to interesting files (SSH keys, env files, source trees of other projects). We also confirmed context_tree is the only caller that forwards user-controlled targetPath, so this isn't shadowed by a sibling sink that would make the fix moot.

Note: src/tools/static-analysis.ts has a separate targetPath sink that flows into a linter subprocess. That's a distinct issue (different sink, different CWE class) and is intentionally out of scope here — this PR is deliberately surgical.

cc @lewiswigmore

[vercel]

@sebastiondev is attempting to deploy a commit to the ForLoopCodes' projects Team on Vercel.

A member of the Team first needs to authorize it.