Merge branch 'fix/config-path-containment' into main

Closes two pre-existing holes in the Claude Code writer: a symlink at
(or above) a write target escaped the config root, and PUT/DELETE
/file honoured a caller-supplied ?cwd=. Both now go through the shared
guard in config-write.js.

server/__tests__/cc-config.test.js

@@ -551,16 +551,70 @@ describe("/api/cc-config", () => {
   });
 
   it("PUT /file edits memory CLAUDE.md without a name", async () => {
-    const { status, body } = await fetchJson(
-      `/api/cc-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`,
-      {
+    // chdir, not `?cwd=`: project-scope writes resolve against the SERVER's
+    // cwd on purpose, so a request cannot pick the destination. Passing
+    // `?cwd=` here would have written into the repo's own CLAUDE.md.
+    const prev = process.cwd();
+    process.chdir(FAKE_PROJECT);
+    try {
+      const { status, body } = await fetchJson("/api/cc-config/file", {
         method: "PUT",
         body: { scope: "project", type: "memory", content: "# new project memory" },
-      }
-    );
-    assert.equal(status, 200);
-    assert.ok(body.backupPath, "previous CLAUDE.md should be backed up");
-    assert.equal(fs.readFileSync(body.file, "utf8"), "# new project memory");
+      });
+      assert.equal(status, 200);
+      assert.ok(body.backupPath, "previous CLAUDE.md should be backed up");
+      assert.equal(fs.readFileSync(body.file, "utf8"), "# new project memory");
+      assert.equal(body.file, path.join(FAKE_PROJECT, "CLAUDE.md"));
+    } finally {
+      process.chdir(prev);
+    }
+  });
+
+  it("PUT /file ignores ?cwd= — a caller cannot choose where a write lands", async () => {
+    // The hole this closes: `?cwd=<anywhere>` with scope "project" dropped a
+    // file into any directory on disk. PATCH /settings already refused it;
+    // PUT/DELETE did not.
+    const victim = path.join(TMP, "victim-project");
+    fs.mkdirSync(victim, { recursive: true });
+    fs.writeFileSync(path.join(victim, "CLAUDE.md"), "UNTOUCHED");
+
+    const prev = process.cwd();
+    process.chdir(FAKE_PROJECT);
+    try {
+      const { status, body } = await fetchJson(
+        `/api/cc-config/file?cwd=${encodeURIComponent(victim)}`,
+        {
+          method: "PUT",
+          body: { scope: "project", type: "memory", content: "PWNED" },
+        }
+      );
+      assert.equal(status, 200);
+      assert.equal(body.file, path.join(FAKE_PROJECT, "CLAUDE.md"), "must ignore the query cwd");
+      assert.equal(fs.readFileSync(path.join(victim, "CLAUDE.md"), "utf8"), "UNTOUCHED");
+    } finally {
+      process.chdir(prev);
+    }
+  });
+
+  it("DELETE /file ignores ?cwd= too", async () => {
+    const victim = path.join(TMP, "victim-delete");
+    fs.mkdirSync(victim, { recursive: true });
+    fs.writeFileSync(path.join(victim, "CLAUDE.md"), "UNTOUCHED");
+    fs.writeFileSync(path.join(FAKE_PROJECT, "CLAUDE.md"), "# server-cwd memory");
+
+    const prev = process.cwd();
+    process.chdir(FAKE_PROJECT);
+    try {
+      const { status } = await fetchJson(`/api/cc-config/file?cwd=${encodeURIComponent(victim)}`, {
+        method: "DELETE",
+        body: { scope: "project", type: "memory" },
+      });
+      assert.equal(status, 200);
+      assert.equal(fs.existsSync(path.join(victim, "CLAUDE.md")), true, "victim must survive");
+      assert.equal(fs.existsSync(path.join(FAKE_PROJECT, "CLAUDE.md")), false);
+    } finally {
+      process.chdir(prev);
+    }
   });
 
   it("DELETE /file backs up and removes a single-file agent", async () => {

server/__tests__/cc-mutate-containment.test.js

@@ -0,0 +1,143 @@
+/**
+ * @file cc-mutate-containment.test.js
+ * @description Path-containment tests for cc-mutate.js's artifact writer.
+ *
+ * `isUnder` compares strings, which a symlink defeats: a link planted at
+ * `~/.claude/skills/<name>` — or at any ancestor of it — is lexically inside
+ * the root while the write lands wherever the link points. The Codex writer
+ * hit both variants (leaf first, then ancestor on a fresh create, where
+ * realpath fails with ENOENT and a naive check waves it through), and the
+ * backup DESTINATION is a third path with the same exposure.
+ *
+ * Every case here asserts the refusal AND that nothing appeared outside the
+ * root — a thrown error with the write already done is not a fix.
+ * @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
+ */
+
+const { describe, it, beforeEach, after } = require("node:test");
+const assert = require("node:assert/strict");
+const fs = require("node:fs");
+const path = require("node:path");
+const os = require("node:os");
+
+const TMP = fs.mkdtempSync(path.join(os.tmpdir(), "cc-mutate-containment-"));
+const FAKE_HOME = path.join(TMP, "home", ".claude");
+const OUTSIDE = path.join(TMP, "outside");
+
+fs.mkdirSync(FAKE_HOME, { recursive: true });
+fs.mkdirSync(OUTSIDE, { recursive: true });
+
+process.env.CLAUDE_HOME = FAKE_HOME;
+const ccMutate = require("../lib/cc-mutate");
+
+after(() => fs.rmSync(TMP, { recursive: true, force: true, maxRetries: 5 }));
+
+beforeEach(() => {
+  fs.rmSync(FAKE_HOME, { recursive: true, force: true });
+  fs.rmSync(OUTSIDE, { recursive: true, force: true });
+  fs.mkdirSync(FAKE_HOME, { recursive: true });
+  fs.mkdirSync(OUTSIDE, { recursive: true });
+});
+
+const write = (over = {}) =>
+  ccMutate.writeArtifact({
+    scope: "user",
+    type: "skills",
+    name: "victim",
+    content: "NEW",
+    ...over,
+  });
+
+describe("cc-mutate: symlink containment on the write path", () => {
+  it("refuses a leaf symlink pointing out of the root, and writes nothing there", () => {
+    const outsideDir = path.join(OUTSIDE, "target");
+    fs.mkdirSync(outsideDir, { recursive: true });
+    fs.writeFileSync(path.join(outsideDir, "SKILL.md"), "ORIGINAL");
+    fs.mkdirSync(path.join(FAKE_HOME, "skills"), { recursive: true });
+    fs.symlinkSync(outsideDir, path.join(FAKE_HOME, "skills", "victim"));
+
+    assert.throws(
+      () => write(),
+      (e) => e.code === "EOUTOFROOT"
+    );
+    assert.equal(fs.readFileSync(path.join(outsideDir, "SKILL.md"), "utf8"), "ORIGINAL");
+  });
+
+  it("refuses a symlinked ANCESTOR even when the artifact itself is new", () => {
+    // The case a leaf-only check misses: realpath on a not-yet-existing leaf
+    // throws ENOENT, so the walk has to climb to the symlinked parent.
+    fs.symlinkSync(OUTSIDE, path.join(FAKE_HOME, "skills"));
+
+    assert.throws(
+      () => write({ name: "fresh" }),
+      (e) => e.code === "EOUTOFROOT"
+    );
+    assert.equal(fs.existsSync(path.join(OUTSIDE, "fresh")), false);
+  });
+
+  it("refuses to back up through a symlinked backup root", () => {
+    // rmSync/unlink only remove the link, but the backup runs first and
+    // copies THROUGH it — so the destination needs the guard too.
+    fs.mkdirSync(path.join(FAKE_HOME, "skills", "victim"), { recursive: true });
+    fs.writeFileSync(path.join(FAKE_HOME, "skills", "victim", "SKILL.md"), "OLD");
+    fs.symlinkSync(OUTSIDE, path.join(FAKE_HOME, "cc-config-backups"));
+
+    assert.throws(
+      () => write(),
+      (e) => e.code === "EOUTOFROOT"
+    );
+    assert.deepEqual(fs.readdirSync(OUTSIDE), []);
+    // the original is untouched — the backup failing must not have written
+    assert.equal(
+      fs.readFileSync(path.join(FAKE_HOME, "skills", "victim", "SKILL.md"), "utf8"),
+      "OLD"
+    );
+  });
+
+  it("refuses a delete whose target is a symlink out of the root", () => {
+    const outsideFile = path.join(OUTSIDE, "SKILL.md");
+    fs.writeFileSync(outsideFile, "ORIGINAL");
+    fs.mkdirSync(path.join(FAKE_HOME, "skills"), { recursive: true });
+    fs.symlinkSync(OUTSIDE, path.join(FAKE_HOME, "skills", "victim"));
+
+    assert.throws(
+      () => ccMutate.deleteArtifact({ scope: "user", type: "skills", name: "victim" }),
+      (e) => e.code === "EOUTOFROOT"
+    );
+    assert.equal(fs.readFileSync(outsideFile, "utf8"), "ORIGINAL");
+  });
+});
+
+describe("cc-mutate: symlinks that stay inside the root still work", () => {
+  it("writes through a symlink whose target is inside the root", () => {
+    // Reorganising within ~/.claude is legitimate; the rule is where the path
+    // lands, not whether a link was involved.
+    const real = path.join(FAKE_HOME, "real-skills");
+    fs.mkdirSync(real, { recursive: true });
+    fs.symlinkSync(real, path.join(FAKE_HOME, "skills"));
+
+    const res = write({ name: "ok" });
+    assert.equal(res.created, true);
+    assert.equal(fs.readFileSync(path.join(real, "ok", "SKILL.md"), "utf8"), "NEW");
+  });
+
+  it("works when CLAUDE_HOME itself is a symlink — the dotfiles case", () => {
+    // A dotfiles-managed ~/.claude is ordinary. Comparing an unresolved root
+    // against a resolved target would reject every write on such a machine.
+    const realHome = path.join(TMP, "real-home");
+    fs.rmSync(realHome, { recursive: true, force: true });
+    fs.mkdirSync(realHome, { recursive: true });
+    const linkedHome = path.join(TMP, "linked-home");
+    fs.rmSync(linkedHome, { recursive: true, force: true });
+    fs.symlinkSync(realHome, linkedHome);
+
+    process.env.CLAUDE_HOME = linkedHome;
+    try {
+      const res = write({ name: "ok" });
+      assert.equal(res.created, true);
+      assert.equal(fs.readFileSync(path.join(realHome, "skills", "ok", "SKILL.md"), "utf8"), "NEW");
+    } finally {
+      process.env.CLAUDE_HOME = FAKE_HOME;
+    }
+  });
+});

server/lib/cc-mutate.js

@@ -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") {

server/lib/codex-mutate.js

@@ -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 ─────────────────────────────────────────────────────────

server/lib/config-write.js

@@ -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,
+};

server/routes/cc-config.js

@@ -178,7 +178,17 @@ router.put("/file", (req, res) => {
       .json({ error: { code: "EBADREQ", message: "scope and type are required" } });
   }
   try {
-    const result = ccMutate.writeArtifact({ scope, type, name, content, project, cwd: cwdOf(req) });
+    // process.cwd(), never ?cwd= — the same rule PATCH /settings already
+    // follows: a caller-chosen write target lets any request drop a file
+    // anywhere on disk. Reads keep the override; writes do not.
+    const result = ccMutate.writeArtifact({
+      scope,
+      type,
+      name,
+      content,
+      project,
+      cwd: process.cwd(),
+    });
     emitChanged({ action: "write", scope, type, name: name || null, project: project || null });
     res.json(result);
   } catch (err) {
@@ -194,7 +204,8 @@ router.delete("/file", (req, res) => {
       .json({ error: { code: "EBADREQ", message: "scope and type are required" } });
   }
   try {
-    const result = ccMutate.deleteArtifact({ scope, type, name, project, cwd: cwdOf(req) });
+    // Pinned for the same reason as PUT above.
+    const result = ccMutate.deleteArtifact({ scope, type, name, project, cwd: process.cwd() });
     emitChanged({ action: "delete", scope, type, name: name || null, project: project || null });
     res.json(result);
   } catch (err) {