fix(cc-config): close the symlink write escape and the caller-chosen write target

The Claude writer's containment was lexical only: a symlink at
~/.claude/<type>/<name>, or at any ancestor of it, is a string inside the
root while the write lands wherever it points. The backup destination had
the same exposure, and delete copies through the link before unlinking it.

PUT/DELETE /file also honoured ?cwd=, so a project-scope write could be
aimed at any directory on disk. PATCH /settings already refused that, with
the reasoning in a comment; the two other write routes never adopted it.

The guard moves into config-write.js so both writers run one copy.
This commit is contained in:
2026-09-09 08:30:45 +07:00
parent 5720e8d5f8
commit 2ade30f670
6 changed files with 320 additions and 37 deletions
+30 -3
View File
@@ -31,7 +31,12 @@ const fs = require("node:fs");
const path = require("node:path");
const { getClaudeHome } = require("./claude-home");
const { isUnder, MAX_FILE_BYTES } = require("./cc-discovery");
const { timestamp, createBackup, atomicWriteFile } = require("./config-write");
const {
timestamp,
createBackup,
atomicWriteFile,
assertNoSymlinkEscape,
} = require("./config-write");
const NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/;
// Auto-memory files are arbitrary flat *.md filenames inside a project's
@@ -118,6 +123,7 @@ function resolveTarget(scope, type, name, opts = {}) {
target: filePath,
filePath,
containmentRoot: path.dirname(filePath),
trustedRoot: path.dirname(filePath),
};
}
@@ -127,7 +133,15 @@ function resolveTarget(scope, type, name, opts = {}) {
throw makeError("EBADNAME", `auto-memory name must be a flat *.md filename`);
}
const target = path.join(memDir, name);
return { kind: "file", target, filePath: target, containmentRoot: memDir };
// The projects root, not memDir: `<projects>/<slug>/memory` is itself
// replaceable with a link, so it cannot be its own comparison base.
return {
kind: "file",
target,
filePath: target,
containmentRoot: memDir,
trustedRoot: path.join(getClaudeHome(), "projects"),
};
}
if (typeof name !== "string" || !NAME_RE.test(name)) {
@@ -144,6 +158,7 @@ function resolveTarget(scope, type, name, opts = {}) {
target,
filePath: path.join(target, spec.filename),
containmentRoot: subdirAbs,
trustedRoot: root,
};
}
@@ -154,6 +169,7 @@ function resolveTarget(scope, type, name, opts = {}) {
target,
filePath: target,
containmentRoot: subdirAbs,
trustedRoot: root,
};
}
@@ -184,11 +200,14 @@ function rmTreeSync(p) {
* copies the whole tree. Returns the backup path (or null if there was
* nothing to back up — e.g. brand-new file).
*/
function backupTarget({ scope, type, target, opts }) {
function backupTarget({ scope, type, target, opts, trustedRoot }) {
let root;
if (type === "memory") root = memoryBackupRoot(scope, opts);
else if (type === "auto-memory") root = autoMemoryBackupRoot(path.dirname(target));
else root = backupRoot(scope, type, opts);
// The backup DESTINATION needs the same guard as the artifact: a link at
// `<root>/cc-config-backups` sends the copied contents outside the root.
if (trustedRoot) assertNoSymlinkEscape(trustedRoot, root, { label: "Claude" });
return createBackup({ source: target, backupRoot: root, type, name: path.basename(target) });
}
@@ -214,6 +233,9 @@ function writeArtifact(args) {
if (!isUnder(r.containmentRoot, r.target)) {
throw makeError("EOUTOFROOT", "resolved path is outside containment root");
}
// The lexical check above is defeated by a symlink at the target or at any
// ancestor of it — see config-write.js. Resolve before writing.
assertNoSymlinkEscape(r.trustedRoot, r.target, { label: "Claude" });
const existedBefore = fs.existsSync(r.filePath);
const backupPath = existedBefore
@@ -222,6 +244,7 @@ function writeArtifact(args) {
type,
target: r.kind === "dir" ? r.target : r.filePath,
opts: { cwd },
trustedRoot: r.trustedRoot,
})
: null;
@@ -250,6 +273,9 @@ function deleteArtifact(args) {
if (!isUnder(r.containmentRoot, r.target)) {
throw makeError("EOUTOFROOT", "resolved path is outside containment root");
}
// rmSync/unlinkSync only unlink the link itself, but the backup that runs
// first copies THROUGH it — so the same guard has to cover delete.
assertNoSymlinkEscape(r.trustedRoot, r.target, { label: "Claude" });
if (!fs.existsSync(r.target)) {
throw makeError("ENOTFOUND", `${type}/${name || "CLAUDE.md"} does not exist`);
@@ -260,6 +286,7 @@ function deleteArtifact(args) {
type,
target: r.target,
opts: { cwd },
trustedRoot: r.trustedRoot,
});
if (r.kind === "dir") {
+9 -23
View File
@@ -35,7 +35,12 @@ const fs = require("node:fs");
const path = require("node:path");
const { getCodexHome, getProjectCodexDir, getProjectRoot } = require("./codex-home");
const { MAX_FILE_BYTES, isUnder } = require("./cc-discovery");
const { createBackup, atomicWriteFile, assertUnder } = require("./config-write");
const {
createBackup,
atomicWriteFile,
assertUnder,
assertNoSymlinkEscape,
} = require("./config-write");
const NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/;
@@ -165,28 +170,9 @@ function rmTreeSync(p) {
* scope for this task (see task-5 fix round 2).
*/
function assertNotSymlinkEscape(trustedRoot, target, opts = {}) {
if (opts.create !== false) fs.mkdirSync(trustedRoot, { recursive: true });
const realRoot = fs.realpathSync(trustedRoot);
let dir = target;
let real;
for (;;) {
try {
real = fs.realpathSync(dir);
break;
} catch {
const parent = path.dirname(dir);
if (parent === dir) {
real = dir; // hit the filesystem root without finding anything real
break;
}
dir = parent;
}
}
if (!isUnder(realRoot, real)) {
throw makeError("EOUTOFROOT", `resolved path escapes the Codex root: ${real}`);
}
// Lives in config-write.js so the Claude writer runs the identical check —
// this guard cost four fix rounds to get right and must not exist twice.
assertNoSymlinkEscape(trustedRoot, target, { ...opts, label: "Codex" });
}
// ── Public API ─────────────────────────────────────────────────────────
+63 -1
View File
@@ -98,4 +98,66 @@ function assertUnder(root, target) {
}
}
module.exports = { timestamp, createBackup, atomicWriteFile, assertUnder };
/**
* The containment check for WRITE paths. `assertUnder` compares strings, which
* a symlink defeats: a link planted at `<root>/skills/<name>` — or at any
* ancestor of it — is lexically inside the root while the write lands wherever
* the link points.
*
* So resolve instead of compare. Walk up from `target` to the first component
* that actually exists, realpath THAT, and require the result inside the
* realpath'd root. Checking only `target` is not enough: on a create the leaf
* does not exist yet, `realpathSync` fails with ENOENT, and a symlinked
* ancestor sails through.
*
* A symlink that stays inside the root still works — the rule is where the
* path lands, not whether a link was involved.
*
* Read paths deliberately do NOT use this. Reading through a link shows you a
* file your own config directory points at, which is what the CLIs do;
* writing through one clobbers something unrelated.
*
* ponytail: check-then-use, so a local attacker who swaps a directory for a
* symlink between this call and the write still wins. Closing that needs
* O_NOFOLLOW-style fd work with real platform variance.
*
* @param {string} trustedRoot the outermost root the write must stay inside —
* never a subdirectory an attacker could replace with a link.
* @param {string} target the path about to be written, created or removed.
* @param {{create?: boolean, label?: string}} [opts] `create: false` skips
* materialising the root (a read-side caller must not have side effects).
*/
function assertNoSymlinkEscape(trustedRoot, target, opts = {}) {
if (opts.create !== false) fs.mkdirSync(trustedRoot, { recursive: true });
const realRoot = fs.realpathSync(trustedRoot);
let dir = target;
let real;
for (;;) {
try {
real = fs.realpathSync(dir);
break;
} catch {
const parent = path.dirname(dir);
if (parent === dir) {
real = dir; // hit the filesystem root without finding anything real
break;
}
dir = parent;
}
}
if (!isUnder(realRoot, real)) {
const err = new Error(`resolved path escapes the ${opts.label || "config"} root: ${real}`);
err.code = "EOUTOFROOT";
throw err;
}
}
module.exports = {
timestamp,
createBackup,
atomicWriteFile,
assertUnder,
assertNoSymlinkEscape,
};