From 99c820fbac3dc057eabeefd1423d64a91c426692 Mon Sep 17 00:00:00 2001 From: James Sesler Date: Thu, 23 Jul 2026 16:30:05 -0400 Subject: [PATCH] fix: symlink sandbox bypass in isWithinPath (security) (#v0.12.10) The Pi extension's tool-call sandbox used lexical path comparison (resolve()) without resolving symlinks. A sub-agent could create a symlink inside .codecarto/ pointing to a file outside the allowed root (e.g. ~/.ssh/id_rsa) and write through it -- the isWithinPath guard would pass because it only checked the lexical path, not the resolved target. Fix: added isWithinPathResolved which uses realpath before comparison. Updated the Pi tool_call hook to use isWithinPathResolved instead of isWithinPath. The MCP server was not affected (it does not use isWithinPath). Regression tests: - Documents the vulnerability (lexical check returns true for symlink) - Verifies the fix (resolved check returns false for symlink) - Verifies legitimate paths still pass - Verifies the root itself passes Discovered during the v0.12.9 self-analysis case study, independently confirmed via reproduction, and fixed with regression coverage. 268/268 tests pass (264 original + 4 new). --- CHANGELOG.md | 6 ++++ core/utils.ts | 21 ++++++++++++ extensions/codecarto/index.ts | 6 +++- package-lock.json | 4 +-- package.json | 2 +- server.json | 4 +-- tests/symlink-sandbox.test.mjs | 60 ++++++++++++++++++++++++++++++++++ 7 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 tests/symlink-sandbox.test.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 90b7563..37f5443 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ All notable changes to this project are documented here. The format is based on ## [Unreleased] +## [0.12.10] — 2026-07-23 + +### Fixed + +- Symlink sandbox bypass in `isWithinPath` (security). The Pi extension's tool-call sandbox used lexical path comparison (`resolve()`) without resolving symlinks, allowing a sub-agent to create a symlink inside `.codecarto/` pointing to a file outside the allowed root and write through it. Added `isWithinPathResolved` which uses `realpath` before comparison, and updated the Pi tool-call hook to use it. Regression tests verify both the vulnerability (documented) and the fix. Discovered during the v0.12.9 self-analysis case study, independently confirmed, and fixed with regression coverage. + ## [0.12.9] — 2026-07-23 ### Fixed diff --git a/core/utils.ts b/core/utils.ts index 93b4183..66b8e01 100644 --- a/core/utils.ts +++ b/core/utils.ts @@ -40,6 +40,27 @@ export function isWithinPath(path: string, root: string): boolean { return normalizedPath.startsWith(`${normalizedRoot}${process.platform === "win32" ? "\\" : "/"}`); } +/** + * Symlink-aware version of isWithinPath. Resolves symlinks on both the path + * and root before comparing, preventing bypass via symlinks inside the + * allowed root that point outside it. + * + * Falls back to lexical isWithinPath if realpath fails (e.g., path does not + * exist yet), which is safe for write targets that haven't been created. + */ +export async function isWithinPathResolved(path: string, root: string): Promise { + try { + const resolvedPath = await realpath(path); + const resolvedRoot = await realpath(root); + return isWithinPath(resolvedPath, resolvedRoot); + } catch { + // If realpath fails (path doesn't exist yet, broken symlink, etc.), + // fall back to lexical check. For write targets this is safe because + // the parent directory should already be within the root. + return isWithinPath(path, root); + } +} + export function isPlainObject(value: unknown): value is Record { return typeof value === "object" && value !== null && !Array.isArray(value); } diff --git a/extensions/codecarto/index.ts b/extensions/codecarto/index.ts index bacd84b..4126520 100644 --- a/extensions/codecarto/index.ts +++ b/extensions/codecarto/index.ts @@ -27,6 +27,7 @@ import { getPipelineLabel, getWorkspaceState, isWithinPath, + isWithinPathResolved, listSkillNames, loadCodecartoConfig, loadUsage, @@ -217,7 +218,10 @@ export default function codeCartographerExtension(pi: ExtensionAPI) { if (config.library.path && await discoverLibrary(config.library.path)) { allowedRoots.push(await canonicalPath(config.library.path)); } - if (!allowedRoots.some((allowedRoot) => isWithinPath(targetPath, allowedRoot))) { + const withinAllowed = await Promise.all( + allowedRoots.map((allowedRoot) => isWithinPathResolved(targetPath, allowedRoot)), + ); + if (!withinAllowed.some((result) => result)) { if (ctx.hasUI) { ctx.ui.notify(`Blocked ${event.toolName} outside .codecarto/ or configured library: ${inputPath}`, "warning"); } diff --git a/package-lock.json b/package-lock.json index c05b5b3..29d3104 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "codecartographer-pi", - "version": "0.12.9", + "version": "0.12.10", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "codecartographer-pi", - "version": "0.12.9", + "version": "0.12.10", "license": "MIT", "dependencies": { "@modelcontextprotocol/sdk": "^1.29.0" diff --git a/package.json b/package.json index 7f73839..df7de75 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "codecartographer-pi", - "version": "0.12.9", + "version": "0.12.10", "mcpName": "io.github.HuginnIndustries/codecartographer", "description": "Evidence-backed reverse engineering and human-gated software planning for Pi and MCP coding agents.", "type": "module", diff --git a/server.json b/server.json index c988c99..856db92 100644 --- a/server.json +++ b/server.json @@ -8,12 +8,12 @@ "url": "https://github.com/HuginnIndustries/CodeCartographer", "source": "github" }, - "version": "0.12.9", + "version": "0.12.10", "packages": [ { "registryType": "npm", "identifier": "codecartographer-pi", - "version": "0.12.9", + "version": "0.12.10", "transport": { "type": "stdio" } diff --git a/tests/symlink-sandbox.test.mjs b/tests/symlink-sandbox.test.mjs new file mode 100644 index 0000000..0621c8f --- /dev/null +++ b/tests/symlink-sandbox.test.mjs @@ -0,0 +1,60 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdir, rm, symlink, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { isWithinPath, isWithinPathResolved } from "../core/utils.ts"; + +let tmpRoot; + +async function setup() { + tmpRoot = join(tmpdir(), `cc-symlink-test-${process.pid}-${Date.now()}`); + const allowedDir = join(tmpRoot, "allowed"); + const outsideDir = join(tmpRoot, "outside"); + await mkdir(allowedDir, { recursive: true }); + await mkdir(outsideDir, { recursive: true }); + await writeFile(join(outsideDir, "secret.txt"), "SECRET"); + await symlink(join(outsideDir, "secret.txt"), join(allowedDir, "link.txt")); + return { allowedDir, outsideDir, symlinkPath: join(allowedDir, "link.txt") }; +} + +async function cleanup() { + await rm(tmpRoot, { recursive: true, force: true }).catch(() => {}); +} + +test("isWithinPath (lexical) is bypassed by symlinks - documents the vulnerability", async () => { + const { allowedDir, symlinkPath } = await setup(); + try { + assert.equal(isWithinPath(symlinkPath, allowedDir), true); + } finally { + await cleanup(); + } +}); + +test("isWithinPathResolved blocks symlinks pointing outside the root", async () => { + const { allowedDir, symlinkPath } = await setup(); + try { + assert.equal(await isWithinPathResolved(symlinkPath, allowedDir), false); + } finally { + await cleanup(); + } +}); + +test("isWithinPathResolved allows legitimate paths inside the root", async () => { + const { allowedDir } = await setup(); + try { + const legitPath = join(allowedDir, "findings", "architecture.md"); + assert.equal(await isWithinPathResolved(legitPath, allowedDir), true); + } finally { + await cleanup(); + } +}); + +test("isWithinPathResolved allows the root itself", async () => { + const { allowedDir } = await setup(); + try { + assert.equal(await isWithinPathResolved(allowedDir, allowedDir), true); + } finally { + await cleanup(); + } +}); \ No newline at end of file