fix(security): prevent tool call path traversal outside project root

[haoyu-haoyu]

Summary

Add safePath() utility to constrain all LLM-driven tool call paths within the project root, preventing prompt-injection attacks from reading or writing files outside the workspace.

The vulnerability

When an LLM is prompt-injected (e.g., via a malicious README), it can generate tool calls like Read({ path: "/etc/passwd" }) or Read({ path: "../../../.ssh/id_rsa" }). Since read-only tools (Read, Glob, Grep, LS) are auto-approved without user consent, the file contents are silently exfiltrated through the conversation.

The fix

New: server/utils/safePath.js

safePath(userPath, allowedRoot) → string | throws Error

• Absolute paths: Allowed only if they resolve inside the project root
• .. traversal: Resolved and verified to stay within root
• Symlink escapes: Resolved via realpathSync; for non-existent targets, walks up to the nearest existing ancestor to catch repo/link/missing/deep/file.txt where link -> /external/
• Clear error messages: "Path traversal blocked: ... resolves to ... which is outside the project root"

Modified: server/openrouter.js + server/cli-chat.js

Replace the unconstrained resolve() function in both executeTool() implementations:

// Before (vulnerable)
const resolve = (p) => path.isAbsolute(p) ? p : path.resolve(workingDir, p);

// After (safe)
const resolve = (p) => safePath(p, workingDir);

Tests: 9 cases covering

• Relative paths within root ✅
• Empty/null/undefined input ✅
• Absolute paths inside root (allowed) ✅
• Absolute paths outside root (blocked) ✅
• .. traversal above root (blocked) ✅
• Disguised .. traversal (blocked) ✅
• .. that stays within root (allowed) ✅
• Non-existent files and nested paths ✅

Codex review (3 rounds)

1. Initial review → found symlink parent escape and absolute path rejection too strict
2. After fix → found nested missing segments under symlink still escapable
3. After fix → ancestor walk confirmed correct

Note

This does NOT restrict Bash tool execution (which has its own permission model via permissionMode). It only constrains file-system tool calls (Read, Write, Edit, Glob, Grep, LS).

[Zhang-Henry]

Good fix for a real vulnerability. The symlink ancestor-walk logic is well thought out. Two issues to consider before merging.

[Zhang-Henry]

Issue: Legitimate symlinks inside the project that point outside the root are blocked

The realpathSync resolution means that if a project has an internal symlink like data -> /mnt/storage/data (common for large datasets, shared assets, etc.), then:

safePath('data/file.csv', root)
→ realpathSync resolves to /mnt/storage/data/file.csv
→ outside normalizedRoot
→ throws Error

This blocks Read, Write, Edit, Glob, Grep, and LS on any symlinked path that points outside the project root. This is a behavioral regression that could break legitimate workflows.

Suggested fix: Consider either:

1. An allowlist mechanism (e.g. SAFEPATH_ALLOWED_ROOTS env var or a .safepath.json config) for trusted external symlink targets.
2. Only check the logical resolved path (without realpathSync) for .. traversal and absolute-path escapes, and skip the symlink resolution — this blocks the ../../../etc/passwd attack without blocking legitimate symlinks. The symlink-parent-escape scenario (repo/link -> /external/) is much rarer and arguably less of a threat since it requires the user to have created the symlink themselves.

[Zhang-Henry]

Minor: Inconsistent return format for falsy input

When userPath is falsy, this returns allowedRoot as-is (the raw path). But for all other inputs, the function returns a realpathSync-resolved path via normalizedRoot.

If the project root itself is a symlink (e.g. /home/user/project -> /mnt/projects/foo), then:

• safePath('', root) → /home/user/project (symlink path)
• safePath('src/a.js', root) → /mnt/projects/foo/src/a.js (real path)

This could cause subtle path-comparison mismatches downstream. Consider normalizing the early return too:

if (!userPath) {
  try { return fs.realpathSync(allowedRoot); } catch { return allowedRoot; }
}

[haoyu-haoyu]

Thanks @Zhang-Henry — both issues addressed in 6eeb58f:

1. Legitimate symlinks blocked → fixed

Adopted your option 2: safePath now performs only a lexical check (path.resolve + prefix comparison), removing all realpathSync usage and the ancestor-walk logic. This means:

• data -> /mnt/storage/data symlinks inside the project work as expected
• ../../../etc/passwd is still blocked (path.resolve normalizes away ..)
• /etc/passwd is still blocked (absolute path outside root)

The trade-off (symlink-based escapes not caught) is acceptable since creating a malicious symlink requires the Bash tool which needs user approval.

Added a test that creates a symlink to os.tmpdir() and verifies it's allowed.

2. Inconsistent return for falsy input → fixed

Now returns path.resolve(allowedRoot) for falsy input, matching all other code paths. Added a test with a non-normalized root (ROOT/src/..) to verify normalization.

The implementation went from 86 lines to 52 lines — much simpler to audit. All 11 tests pass.