fix: prevent path traversal in proposeCommit (CWE-22)

[sebastiondev]

Summary

This PR fixes a path traversal vulnerability (CWE-22) in proposeCommit that allows an MCP client to write arbitrary files outside the project root.

Vulnerability

• File: src/tools/propose-commit.ts
• Function: proposeCommit
• CWE: CWE-22 (Improper Limitation of a Pathname to a Restricted Directory)
• Severity: High

The MCP tool propose_commit accepts file_path as a parameter directly from the MCP client. Before this fix, the implementation did:

const fullPath = resolve(options.rootDir, options.filePath);
// ... validation of content only ...
// later: fs.writeFile(fullPath, options.newContent)

path.resolve happily collapses .. segments and treats absolute paths as overrides. With no containment check, a file_path like ../../../.ssh/authorized_keys, ../../../etc/cron.d/x, or an absolute path such as /tmp/anywhere resolves outside rootDir and is then written to. There were no other guards on the path between the parameter and the write.

Data flow

1. MCP client (an LLM, possibly via prompt injection or a malicious tool-call context) invokes propose_commit with attacker-controlled file_path.
2. proposeCommit resolves the path against rootDir with no boundary check.
3. Content (also attacker-controlled via new_content) is written to the resolved location.

The MCP server's intended threat model treats rootDir as the sandbox boundary; this bug breaks that boundary.

Fix

Resolve rootDir to an absolute, canonical form, resolve the joined path, and reject anything that is not equal to rootDir or under rootDir + "/":

const rootDir = resolve(options.rootDir);
const fullPath = resolve(rootDir, options.filePath);

if (fullPath !== rootDir && !fullPath.startsWith(rootDir + "/")) {
  throw new Error(`Path traversal detected: resolved path escapes project root`);
}

This is the standard prefix-containment check after canonicalization, and it runs before any filesystem write, so the bad path never reaches writeFile.

Tests

Added test/main/propose-commit-traversal.test.mjs covering:

• Simple ../ traversal (e.g. ../escaped.txt) — rejected
• Deep traversal (../../../tmp/evil.txt) — rejected
• Nested traversal (subdir/../../escaped.txt) — rejected
• Absolute path outside rootDir — rejected
• Legitimate paths inside rootDir — still accepted
• Files in subdirectories of rootDir — still accepted

All new tests pass; existing tests are unaffected (only this function's preamble changed).

A note on portability: the check uses / as the separator, matching the existing POSIX-style usage in this file. On Windows the check would need path.sep; happy to follow up with that if you'd like it in the same PR.

Adversarial review

Before submitting we tried to disprove this. We checked whether anything upstream of proposeCommit already constrains file_path — the MCP tool schema accepts it as a free-form string, the handler passes it through unchanged, and there is no other validator in the call chain. We also considered whether the MCP server's process boundary alone is sufficient; it isn't, because the server typically runs with the developer's own filesystem permissions, so "stay inside the project root" has to be enforced in code. The exploitation precondition is just "the MCP client can be induced to call propose_commit with a crafted path," which is exactly the prompt-injection / malicious-context surface MCP tools are expected to defend against.

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.