diff --git a/server/__tests__/config-write.test.js b/server/__tests__/config-write.test.js new file mode 100644 index 00000000..d4307473 --- /dev/null +++ b/server/__tests__/config-write.test.js @@ -0,0 +1,63 @@ +/** + * @file config-write.test.js + * @description Tests for the shared safe-write primitives (atomicWriteFile, + * createBackup, assertUnder) that cc-mutate.js and the Codex config writer + * both depend on. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const { describe, it } = 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 { atomicWriteFile, createBackup, assertUnder } = require("../lib/config-write"); + +describe("atomicWriteFile", () => { + it("leaves no temp file behind after a successful write", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cw-")); + atomicWriteFile(path.join(dir, "a.md"), "body"); + assert.equal(fs.readFileSync(path.join(dir, "a.md"), "utf8"), "body"); + assert.deepEqual(fs.readdirSync(dir), ["a.md"]); + }); +}); + +describe("createBackup", () => { + it("backs a directory up as a tree, not a single file", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cw-")); + fs.mkdirSync(path.join(dir, "src", "sk"), { recursive: true }); + fs.writeFileSync(path.join(dir, "src", "sk", "SKILL.md"), "x"); + const out = createBackup({ + source: path.join(dir, "src", "sk"), + backupRoot: path.join(dir, "bak"), + type: "skills", + name: "sk", + }); + assert.ok(fs.existsSync(path.join(out, "SKILL.md"))); + }); + + it("returns null when the source does not exist", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "cw-")); + const out = createBackup({ + source: path.join(dir, "missing"), + backupRoot: path.join(dir, "bak"), + type: "agents", + name: "missing", + }); + assert.equal(out, null); + }); +}); + +describe("assertUnder", () => { + it("rejects a target outside the root", () => { + assert.throws( + () => assertUnder("/a/b", "/a/c/x"), + (e) => e.code === "EOUTOFROOT" + ); + }); + + it("allows a target under the root", () => { + assert.doesNotThrow(() => assertUnder("/a/b", "/a/b/c")); + }); +}); diff --git a/server/lib/cc-mutate.js b/server/lib/cc-mutate.js index a63a2e4c..aa7d2af2 100644 --- a/server/lib/cc-mutate.js +++ b/server/lib/cc-mutate.js @@ -31,6 +31,7 @@ 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 NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/; // Auto-memory files are arbitrary flat *.md filenames inside a project's @@ -173,84 +174,22 @@ function autoMemoryBackupRoot(memDir) { return path.join(memDir, ".cc-config-backups", "auto-memory"); } -function timestamp() { - return new Date().toISOString().replace(/[:]/g, "-"); -} - -function copyDirSync(src, dst) { - fs.mkdirSync(dst, { recursive: true }); - for (const ent of fs.readdirSync(src, { withFileTypes: true })) { - const s = path.join(src, ent.name); - const d = path.join(dst, ent.name); - if (ent.isDirectory()) copyDirSync(s, d); - else if (ent.isFile()) fs.copyFileSync(s, d); - // symlinks/sockets/etc skipped intentionally — these surfaces are - // text-file-only by spec - } -} - function rmTreeSync(p) { fs.rmSync(p, { recursive: true, force: true }); } /** - * Always-on backup. For files, copies to /..bak. For - * dirs (skills), copies the whole tree. Returns the backup path (or null - * if there was nothing to back up — e.g. brand-new file). + * Always-on backup, routed to the bucket the (scope, type) pair backs up + * into. For files, copies to /..bak. For dirs (skills), + * copies the whole tree. Returns the backup path (or null if there was + * nothing to back up — e.g. brand-new file). */ -function createBackup({ scope, type, target, kind, opts }) { - if (!fs.existsSync(target)) return null; +function backupTarget({ scope, type, target, opts }) { 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); - fs.mkdirSync(root, { recursive: true }); - const base = path.basename(target); - const stamp = timestamp(); - if (kind === "dir") { - const dst = path.join(root, `${base}.${stamp}.bak`); - copyDirSync(target, dst); - return dst; - } - // file - const dst = path.join(root, `${base}.${stamp}.bak`); - fs.copyFileSync(target, dst); - return dst; -} - -/** - * Atomic write: tmp file → fsync (best-effort) → rename. Tmp is unlinked - * on any failure path. Caller is responsible for ensuring parent dir exists. - */ -function atomicWriteFile(filePath, content) { - const dir = path.dirname(filePath); - fs.mkdirSync(dir, { recursive: true }); - const tmp = path.join(dir, `.${path.basename(filePath)}.${process.pid}.${Date.now()}.tmp`); - let fd; - try { - fd = fs.openSync(tmp, "wx"); - fs.writeSync(fd, content); - try { - fs.fsyncSync(fd); - } catch { - // fsync may fail on some filesystems / tmpfs — non-fatal - } - fs.closeSync(fd); - fd = null; - fs.renameSync(tmp, filePath); - } catch (err) { - try { - if (fd != null) fs.closeSync(fd); - } catch { - /* ignore */ - } - try { - if (fs.existsSync(tmp)) fs.unlinkSync(tmp); - } catch { - /* ignore */ - } - throw err; - } + return createBackup({ source: target, backupRoot: root, type, name: path.basename(target) }); } // ── Public API ───────────────────────────────────────────────────────── @@ -278,11 +217,10 @@ function writeArtifact(args) { const existedBefore = fs.existsSync(r.filePath); const backupPath = existedBefore - ? createBackup({ + ? backupTarget({ scope, type, target: r.kind === "dir" ? r.target : r.filePath, - kind: r.kind, opts: { cwd }, }) : null; @@ -317,11 +255,10 @@ function deleteArtifact(args) { throw makeError("ENOTFOUND", `${type}/${name || "CLAUDE.md"} does not exist`); } - const backupPath = createBackup({ + const backupPath = backupTarget({ scope, type, target: r.target, - kind: r.kind === "memoryFile" ? "file" : r.kind, opts: { cwd }, }); diff --git a/server/lib/config-write.js b/server/lib/config-write.js new file mode 100644 index 00000000..00e5bbf2 --- /dev/null +++ b/server/lib/config-write.js @@ -0,0 +1,101 @@ +/** + * @file config-write.js + * @description Shared safe-write primitives for config writers (cc-mutate.js + * for Claude Code, and any Codex equivalent): path containment, timestamped + * backup-before-mutate, and atomic file write. Extracted out of cc-mutate.js + * so both writers share one copy of this security-sensitive code instead of + * diverging copies. + * + * Hard constraints (do not relax without a follow-up review): + * - Backups happen BEFORE the mutation and land outside the tree being + * backed up (caller passes `backupRoot`). + * - Writes are atomic via temp file + fs.renameSync. Tmp is removed on + * any failure path. + * - `assertUnder` is the single containment check callers must run on any + * resolved path before touching disk. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const fs = require("node:fs"); +const path = require("node:path"); +const { isUnder } = require("./cc-discovery"); + +function timestamp() { + return new Date().toISOString().replace(/[:]/g, "-"); +} + +function copyDirSync(src, dst) { + fs.mkdirSync(dst, { recursive: true }); + for (const ent of fs.readdirSync(src, { withFileTypes: true })) { + const s = path.join(src, ent.name); + const d = path.join(dst, ent.name); + if (ent.isDirectory()) copyDirSync(s, d); + else if (ent.isFile()) fs.copyFileSync(s, d); + // symlinks/sockets/etc skipped intentionally — these surfaces are + // text-file-only by spec + } +} + +/** + * Always-on backup. Copies `source` (file or directory tree) into + * `backupRoot` as `..bak`. Returns the backup path, or null if + * `source` does not exist (e.g. brand-new file/skill). + */ +function createBackup({ source, backupRoot, type, name }) { + if (!fs.existsSync(source)) return null; + fs.mkdirSync(backupRoot, { recursive: true }); + const stamp = timestamp(); + const dst = path.join(backupRoot, `${name}.${stamp}.bak`); + if (fs.statSync(source).isDirectory()) { + copyDirSync(source, dst); + } else { + fs.copyFileSync(source, dst); + } + return dst; +} + +/** + * Atomic write: tmp file → fsync (best-effort) → rename. Tmp is unlinked + * on any failure path. Caller is responsible for ensuring parent dir exists. + */ +function atomicWriteFile(filePath, content) { + const dir = path.dirname(filePath); + fs.mkdirSync(dir, { recursive: true }); + const tmp = path.join(dir, `.${path.basename(filePath)}.${process.pid}.${Date.now()}.tmp`); + let fd; + try { + fd = fs.openSync(tmp, "wx"); + fs.writeSync(fd, content); + try { + fs.fsyncSync(fd); + } catch { + // fsync may fail on some filesystems / tmpfs — non-fatal + } + fs.closeSync(fd); + fd = null; + fs.renameSync(tmp, filePath); + } catch (err) { + try { + if (fd != null) fs.closeSync(fd); + } catch { + /* ignore */ + } + try { + if (fs.existsSync(tmp)) fs.unlinkSync(tmp); + } catch { + /* ignore */ + } + throw err; + } +} + +/** Throw EOUTOFROOT unless `target` resolves to `root` or a descendant. */ +function assertUnder(root, target) { + if (!isUnder(root, target)) { + const err = new Error("resolved path is outside containment root"); + err.code = "EOUTOFROOT"; + throw err; + } +} + +module.exports = { timestamp, createBackup, atomicWriteFile, assertUnder };