From aef7ec130ba6337aa7a59ad1fd9344651cb800b2 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 16:17:48 +0700 Subject: [PATCH 01/27] refactor(config): extract the safe-write primitives cc-mutate guards with --- server/__tests__/config-write.test.js | 63 ++++++++++++++++ server/lib/cc-mutate.js | 81 +++------------------ server/lib/config-write.js | 101 ++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 72 deletions(-) create mode 100644 server/__tests__/config-write.test.js create mode 100644 server/lib/config-write.js 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 }; From 57849d2f75134cc3ef9d6bbf7e7abcde6b63a538 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 16:24:17 +0700 Subject: [PATCH 02/27] refactor(codex): give the CODEX_HOME lookup its own module --- server/__tests__/codex-home.test.js | 41 ++++++++++++++++++++++++ server/lib/codex-home.js | 49 +++++++++++++++++++++++++++++ server/lib/codex-ingest.js | 7 ++--- 3 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 server/__tests__/codex-home.test.js create mode 100644 server/lib/codex-home.js diff --git a/server/__tests__/codex-home.test.js b/server/__tests__/codex-home.test.js new file mode 100644 index 00000000..15a3f9ab --- /dev/null +++ b/server/__tests__/codex-home.test.js @@ -0,0 +1,41 @@ +/** + * @file Tests for codex-home.js: CODEX_HOME resolution and related path + * resolutions for Codex project and surface directories. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const path = require("node:path"); +const os = require("node:os"); + +const { describe, it, beforeEach } = require("node:test"); +const assert = require("node:assert/strict"); + +describe("codex-home", () => { + beforeEach(() => { + // Clear the require cache so getCodexHome() reads the fresh env var + delete require.cache[require.resolve("../lib/codex-home")]; + }); + + it("honours CODEX_HOME over the home directory", () => { + process.env.CODEX_HOME = "/tmp/injected-codex"; + delete require.cache[require.resolve("../lib/codex-home")]; + assert.equal(require("../lib/codex-home").getCodexHome(), "/tmp/injected-codex"); + }); + + it("puts the project surfaces under /.codex", () => { + const { getProjectCodexDir } = require("../lib/codex-home"); + assert.equal(getProjectCodexDir("/repo"), path.join("/repo", ".codex")); + }); + + it("returns the cwd as the project root", () => { + const { getProjectRoot } = require("../lib/codex-home"); + assert.equal(getProjectRoot("/repo"), "/repo"); + }); + + it("defaults to ~/.codex when CODEX_HOME is not set", () => { + delete process.env.CODEX_HOME; + delete require.cache[require.resolve("../lib/codex-home")]; + const { getCodexHome } = require("../lib/codex-home"); + assert.equal(getCodexHome(), path.join(os.homedir(), ".codex")); + }); +}); diff --git a/server/lib/codex-home.js b/server/lib/codex-home.js new file mode 100644 index 00000000..448cffad --- /dev/null +++ b/server/lib/codex-home.js @@ -0,0 +1,49 @@ +/** + * @file Centralized Codex home directory path management. Resolves the Codex + * home directory, project-level Codex surfaces, and project roots. Supports a + * custom root via the CODEX_HOME environment variable so the dashboard can + * track non-standard Codex installations. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const path = require("node:path"); +const os = require("node:os"); + +/** + * The Codex home directory. Resolves to $CODEX_HOME if set, else + * ~/.codex. Always reads process.env at call time so tests can inject. + * + * @returns {string} Absolute path to the Codex home directory. + */ +function getCodexHome() { + return process.env.CODEX_HOME || path.join(os.homedir(), ".codex"); +} + +/** + * The directory where Codex stores per-project surfaces (rollout journals, + * SQLite mirrors, etc.) for a project at the given working directory. + * + * @param {string} cwd Absolute path to a project root. + * @returns {string} Absolute path to /.codex. + */ +function getProjectCodexDir(cwd) { + return path.join(cwd, ".codex"); +} + +/** + * The project root. For now, this is just an identity function — cwd is + * the project root. Exists as a contract for consistency with claude-home.js + * and to anchor the path-resolution hierarchy. + * + * @param {string} cwd Absolute path to a project root (where AGENTS.md lives). + * @returns {string} The same cwd. + */ +function getProjectRoot(cwd) { + return cwd; +} + +module.exports = { + getCodexHome, + getProjectCodexDir, + getProjectRoot, +}; diff --git a/server/lib/codex-ingest.js b/server/lib/codex-ingest.js index bcc0ddf6..c40257ae 100644 --- a/server/lib/codex-ingest.js +++ b/server/lib/codex-ingest.js @@ -33,6 +33,7 @@ const fs = require("node:fs"); const path = require("node:path"); const os = require("node:os"); +const { getCodexHome } = require("./codex-home"); // `*** Update File: ` lines inside an apply_patch heredoc. const PATCH_FILE_RE = /^\*\*\* (Add|Update|Delete) File: (.+)$/gm; @@ -98,10 +99,6 @@ function eventsFromPayload(payload) { return [{ tool_name: "Bash", tool_input: { command: commandFromPayload(payload, text) } }]; } -function codexHome() { - return process.env.CODEX_HOME || path.join(os.homedir(), ".codex"); -} - /** Every rollout journal under sessions/, newest mtime first, capped at MAX_FILES. */ function rolloutFiles() { const out = []; @@ -126,7 +123,7 @@ function rolloutFiles() { } } }; - walk(path.join(codexHome(), "sessions"), 0); + walk(path.join(getCodexHome(), "sessions"), 0); return out.sort((a, b) => b.mtime - a.mtime).slice(0, MAX_FILES); } From 761876686180e0222f2c8784d92916c70d59f64c Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 16:32:06 +0700 Subject: [PATCH 03/27] feat(codex): read the Codex config surfaces --- server/__tests__/codex-discovery.test.js | 135 +++++++++++ server/lib/codex-discovery.js | 272 +++++++++++++++++++++++ 2 files changed, 407 insertions(+) create mode 100644 server/__tests__/codex-discovery.test.js create mode 100644 server/lib/codex-discovery.js diff --git a/server/__tests__/codex-discovery.test.js b/server/__tests__/codex-discovery.test.js new file mode 100644 index 00000000..d5f606b5 --- /dev/null +++ b/server/__tests__/codex-discovery.test.js @@ -0,0 +1,135 @@ +/** + * @file Tests for codex-discovery.js: read-only discovery of Codex config + * surfaces (skills, agents, rules, memory, config) across the user (CODEX_HOME) + * and project (/.codex) scopes. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const fs = require("node:fs"); +const path = require("node:path"); +const os = require("node:os"); + +const { describe, it, before, after } = require("node:test"); +const assert = require("node:assert/strict"); + +function write(file, content) { + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, content); +} + +describe("codex-discovery", () => { + let TMP; + let REPO; + let prevCodexHome; + let cx; + + before(() => { + prevCodexHome = process.env.CODEX_HOME; + TMP = fs.mkdtempSync(path.join(os.tmpdir(), "codex-disc-")); + REPO = fs.mkdtempSync(path.join(os.tmpdir(), "codex-disc-repo-")); + process.env.CODEX_HOME = TMP; + delete require.cache[require.resolve("../lib/codex-home")]; + delete require.cache[require.resolve("../lib/codex-discovery")]; + cx = require("../lib/codex-discovery"); + }); + + after(() => { + if (prevCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prevCodexHome; + fs.rmSync(TMP, { recursive: true, force: true }); + fs.rmSync(REPO, { recursive: true, force: true }); + }); + + it("reads a user skill and a project skill into one scoped list", () => { + write( + path.join(TMP, "skills", "release-guard", "SKILL.md"), + "---\nname: release-guard\n---\nbody" + ); + write( + path.join(REPO, ".codex", "skills", "onboarding", "SKILL.md"), + "---\nname: onboarding\n---\nb" + ); + + const rows = cx.readSkills({ cwd: REPO }); + assert.deepEqual(rows.map((r) => [r.scope, r.name]).sort(), [ + ["project", "onboarding"], + ["user", "release-guard"], + ]); + assert.equal(rows.find((r) => r.name === "release-guard").frontmatter.name, "release-guard"); + }); + + it("pulls an agent's identity out of its TOML without a TOML parser", () => { + write( + path.join(REPO, ".codex", "agents", "reviewer.toml"), + 'name = "reviewer"\ndescription = "Read-only review agent."\nmodel = "gpt-5.4"\n' + + 'developer_instructions = """\nmulti\nline\n"""\n' + ); + + const [agent] = cx.readAgents({ cwd: REPO, scope: "project" }); + assert.equal(agent.meta.name, "reviewer"); + assert.equal(agent.meta.description, "Read-only review agent."); + assert.equal(agent.meta.model, "gpt-5.4"); + }); + + it("does not mistake a key inside a triple-quoted block for a top-level field", () => { + const meta = cx.readTomlMeta( + 'name = "implementer"\n' + + 'sandbox_mode = "workspace-write"\n' + + 'developer_instructions = """\n' + + 'model = "not-a-real-field"\n' + + '"""\n' + + 'model = "gpt-5.3-codex-spark"\n' + ); + assert.equal(meta.name, "implementer"); + assert.equal(meta.sandbox_mode, "workspace-write"); + assert.equal(meta.model, "gpt-5.3-codex-spark"); + }); + + it("reads a rule file's preview", () => { + write( + path.join(TMP, "rules", "no-force-push.rules"), + "never force-push main\nask before deleting" + ); + const [rule] = cx.readRules({ cwd: REPO, scope: "user" }); + assert.equal(rule.name, "no-force-push"); + assert.match(rule.preview, /never force-push main/); + }); + + it("reports a missing AGENTS.md as a row that does not exist, not a gap", () => { + const rows = cx.readMemory({ cwd: REPO }); + const user = rows.find((r) => r.scope === "user"); + assert.equal(user.exists, false, "the UI offers to create it, so the row must be present"); + }); + + it("reads project AGENTS.md from the repo root, not under .codex", () => { + write(path.join(REPO, "AGENTS.md"), "# Project instructions"); + const rows = cx.readMemory({ cwd: REPO }); + const project = rows.find((r) => r.scope === "project"); + assert.equal(project.exists, true); + assert.equal(project.file, path.join(REPO, "AGENTS.md")); + }); + + it("reads config.toml content for both scopes", () => { + write(path.join(TMP, "config.toml"), 'model = "gpt-5.3-codex-spark"\n'); + const rows = cx.readConfig({ cwd: REPO }); + const user = rows.find((r) => r.scope === "user"); + assert.equal(user.exists, true); + assert.match(user.content, /gpt-5\.3-codex-spark/); + const project = rows.find((r) => r.scope === "project"); + assert.equal(project.exists, false); + }); + + it("never walks outside the codex roots", () => { + write(path.join(TMP, "skills", "esc", "SKILL.md"), "x"); + for (const row of cx.readSkills({ cwd: REPO })) { + assert.ok(row.file.startsWith(TMP) || row.file.startsWith(REPO)); + } + }); + + it("summarizes counts and roots in the overview", () => { + const overview = cx.readOverview({ cwd: REPO }); + assert.equal(overview.roots.projectRoot, REPO); + assert.ok(overview.counts.skills.user >= 1); + assert.ok(overview.counts.agents.project >= 1); + }); +}); diff --git a/server/lib/codex-discovery.js b/server/lib/codex-discovery.js new file mode 100644 index 00000000..7fe21f3c --- /dev/null +++ b/server/lib/codex-discovery.js @@ -0,0 +1,272 @@ +/** + * @file codex-discovery.js + * @description Read-only discovery of Codex CLI configuration surfaces + * (skills, agents, rules, memory/AGENTS.md, config.toml). Powers the Codex + * Config panel. All operations are pure file reads — never writes. MCP + * servers and plugins are not covered here — they come from the `codex` CLI + * itself, read elsewhere. + * + * Path containment: every read resolves under getCodexHome(), + * getProjectCodexDir(), or getProjectRoot() (for AGENTS.md). Mirrors + * cc-discovery.js's scope handling and record shapes so the two panels feel + * like one system. + * @author Nguyễn Ngọc Trí Vĩ + */ +const fs = require("node:fs"); +const path = require("node:path"); +const { getCodexHome, getProjectCodexDir, getProjectRoot } = require("./codex-home"); +const { parseFrontmatter, isUnder, MAX_FILE_BYTES } = require("./cc-discovery"); + +function safeReadText(absPath) { + try { + const stat = fs.statSync(absPath); + if (!stat.isFile()) return null; + if (stat.size > MAX_FILE_BYTES) { + return { + truncated: true, + size: stat.size, + text: fs.readFileSync(absPath, "utf8").slice(0, MAX_FILE_BYTES), + mtime: stat.mtimeMs, + }; + } + return { + truncated: false, + size: stat.size, + text: fs.readFileSync(absPath, "utf8"), + mtime: stat.mtimeMs, + }; + } catch { + return null; + } +} + +function listDir(absPath) { + try { + return fs.readdirSync(absPath, { withFileTypes: true }); + } catch { + return []; + } +} + +// ── Skills ────────────────────────────────────────────────────────────── + +function readSkillsAt(scope, codexDir) { + const dir = path.join(codexDir, "skills"); + const skills = []; + for (const ent of listDir(dir)) { + const skillDir = path.join(dir, ent.name); + if (!ent.isDirectory() || !isUnder(codexDir, skillDir)) continue; + const skillFile = path.join(skillDir, "SKILL.md"); + const read = safeReadText(skillFile); + if (!read) continue; + const { frontmatter, body } = parseFrontmatter(read.text); + skills.push({ + scope, + name: ent.name, + path: skillDir, + file: skillFile, + size: read.size, + mtime: read.mtime, + truncated: read.truncated, + frontmatter: frontmatter || {}, + preview: body.slice(0, 320), + }); + } + return skills.sort((a, b) => a.name.localeCompare(b.name)); +} + +function readSkills(opts = {}) { + const out = []; + if (opts.scope !== "project") out.push(...readSkillsAt("user", getCodexHome())); + if (opts.scope !== "user") out.push(...readSkillsAt("project", getProjectCodexDir(opts.cwd))); + return out; +} + +// ── Agents (TOML) ────────────────────────────────────────────────────── + +/** + * Best-effort reader for the handful of top-level scalar fields the UI + * shows (`name`, `description`, `model`, `sandbox_mode`). Matches + * `key = "value"` / `key = 123` / `key = true` lines appearing before the + * first `[section]`, skipping over `"""..."""` blocks so a field name + * mentioned inside one (e.g. in `developer_instructions`) is never mistaken + * for a real top-level key. Anything richer than these scalars is the + * raw-text editor's job, not this reader's — no TOML parser here. + */ +function readTomlMeta(text) { + const meta = {}; + const lines = text.split("\n"); + let inTripleQuote = false; + for (const rawLine of lines) { + const line = rawLine.trim(); + if (inTripleQuote) { + if (line.includes('"""')) inTripleQuote = false; + continue; + } + if (/^\[/.test(line)) break; // first section ends the top-level scalars + const m = line.match(/^([A-Za-z0-9_-]+)\s*=\s*(.*)$/); + if (!m) continue; + let value = m[2]; + if (value.startsWith('"""')) { + if (!value.slice(3).includes('"""')) inTripleQuote = true; + continue; + } + if ( + (value.startsWith('"') && value.endsWith('"')) || + (value.startsWith("'") && value.endsWith("'")) + ) { + value = value.slice(1, -1); + } else if (value === "true" || value === "false") { + value = value === "true"; + } else if (/^-?\d+(\.\d+)?$/.test(value)) { + value = Number(value); + } else { + continue; // arrays, inline tables, etc. — not one of the shown fields + } + meta[m[1]] = value; + } + return meta; +} + +function readAgentsAt(scope, codexDir) { + const dir = path.join(codexDir, "agents"); + const agents = []; + for (const ent of listDir(dir)) { + if (!ent.name.endsWith(".toml")) continue; + const file = path.join(dir, ent.name); + if (!ent.isFile() || !isUnder(codexDir, file)) continue; + const read = safeReadText(file); + if (!read) continue; + agents.push({ + scope, + name: ent.name.replace(/\.toml$/, ""), + file, + size: read.size, + mtime: read.mtime, + meta: readTomlMeta(read.text), + preview: read.text.slice(0, 320), + }); + } + return agents.sort((a, b) => a.name.localeCompare(b.name)); +} + +function readAgents(opts = {}) { + const out = []; + if (opts.scope !== "project") out.push(...readAgentsAt("user", getCodexHome())); + if (opts.scope !== "user") out.push(...readAgentsAt("project", getProjectCodexDir(opts.cwd))); + return out; +} + +// ── Rules (plain text) ───────────────────────────────────────────────── + +function readRulesAt(scope, codexDir) { + const dir = path.join(codexDir, "rules"); + const rules = []; + for (const ent of listDir(dir)) { + if (!ent.name.endsWith(".rules")) continue; + const file = path.join(dir, ent.name); + if (!ent.isFile() || !isUnder(codexDir, file)) continue; + const read = safeReadText(file); + if (!read) continue; + rules.push({ + scope, + name: ent.name.replace(/\.rules$/, ""), + file, + size: read.size, + mtime: read.mtime, + preview: read.text.slice(0, 320), + }); + } + return rules.sort((a, b) => a.name.localeCompare(b.name)); +} + +function readRules(opts = {}) { + const out = []; + if (opts.scope !== "project") out.push(...readRulesAt("user", getCodexHome())); + if (opts.scope !== "user") out.push(...readRulesAt("project", getProjectCodexDir(opts.cwd))); + return out; +} + +// ── Memory (AGENTS.md) ───────────────────────────────────────────────── + +function readMemory(opts = {}) { + const sources = [ + { scope: "user", file: path.join(getCodexHome(), "AGENTS.md") }, + { scope: "project", file: path.join(getProjectRoot(opts.cwd), "AGENTS.md") }, + ]; + return sources.map(({ scope, file }) => { + const r = safeReadText(file); + if (!r) return { scope, file, exists: false }; + return { + scope, + file, + exists: true, + size: r.size, + mtime: r.mtime, + preview: r.text.slice(0, 480), + }; + }); +} + +// ── Config (config.toml) ─────────────────────────────────────────────── + +function readConfig(opts = {}) { + const sources = [ + { scope: "user", file: path.join(getCodexHome(), "config.toml") }, + { scope: "project", file: path.join(getProjectCodexDir(opts.cwd), "config.toml") }, + ]; + return sources.map(({ scope, file }) => { + const r = safeReadText(file); + if (!r) return { scope, file, exists: false }; + return { + scope, + file, + exists: true, + size: r.size, + mtime: r.mtime, + content: r.text, + truncated: r.truncated, + }; + }); +} + +// ── Overview (counts + roots) ────────────────────────────────────────── + +function readOverview(opts = {}) { + const skills = readSkills(opts); + const agents = readAgents(opts); + const rules = readRules(opts); + const memory = readMemory(opts); + const config = readConfig(opts); + + const countByScope = (arr) => ({ + user: arr.filter((x) => x.scope === "user").length, + project: arr.filter((x) => x.scope === "project").length, + }); + + return { + roots: { + codexHome: getCodexHome(), + projectCodexDir: getProjectCodexDir(opts.cwd), + projectRoot: getProjectRoot(opts.cwd), + }, + counts: { + skills: countByScope(skills), + agents: countByScope(agents), + rules: countByScope(rules), + memory: countByScope(memory.filter((m) => m.exists)), + config: countByScope(config.filter((c) => c.exists)), + }, + }; +} + +module.exports = { + readSkills, + readAgents, + readRules, + readMemory, + readConfig, + readOverview, + readTomlMeta, + isUnder, +}; From 98b1921ca671e941945d5f9139265621e5cac5b7 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 16:41:46 +0700 Subject: [PATCH 04/27] fix(codex): keep commented TOML fields, follow symlinked config surfaces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readTomlMeta was dropping any field with a trailing `# comment` (common in real agents/*.toml). readSkills/readAgents/readRules used bare isDirectory()/isFile(), which miss symlinked entries per cc-discovery.js's own documented Dirent quirk — reused its isDirLike/isFileLike instead (exporting isDirLike from cc-discovery.js, which wasn't previously public). Also swapped the "never walks outside" test for one that actually exercises a symlink escape, and dropped the unnecessary isUnder re-export. --- server/__tests__/codex-discovery.test.js | 23 ++++++++++++++++--- server/lib/cc-discovery.js | 1 + server/lib/codex-discovery.js | 29 ++++++++++++++++-------- 3 files changed, 41 insertions(+), 12 deletions(-) diff --git a/server/__tests__/codex-discovery.test.js b/server/__tests__/codex-discovery.test.js index d5f606b5..75643b9f 100644 --- a/server/__tests__/codex-discovery.test.js +++ b/server/__tests__/codex-discovery.test.js @@ -85,6 +85,14 @@ describe("codex-discovery", () => { assert.equal(meta.model, "gpt-5.3-codex-spark"); }); + it("keeps a field with a trailing inline comment, and keeps a '#' that is inside the string", () => { + const meta = cx.readTomlMeta( + 'model = "gpt-5.4" # default\n' + 'description = "uses # for headings"\n' + ); + assert.equal(meta.model, "gpt-5.4"); + assert.equal(meta.description, "uses # for headings"); + }); + it("reads a rule file's preview", () => { write( path.join(TMP, "rules", "no-force-push.rules"), @@ -119,11 +127,20 @@ describe("codex-discovery", () => { assert.equal(project.exists, false); }); - it("never walks outside the codex roots", () => { - write(path.join(TMP, "skills", "esc", "SKILL.md"), "x"); - for (const row of cx.readSkills({ cwd: REPO })) { + it("follows a symlinked skill dir (a supported install pattern) without ever reporting a path outside the codex roots", () => { + const outside = fs.mkdtempSync(path.join(os.tmpdir(), "codex-disc-outside-")); + write(path.join(outside, "SKILL.md"), "---\nname: linked\n---\nbody"); + fs.symlinkSync(outside, path.join(TMP, "skills", "linked"), "dir"); + + const rows = cx.readSkills({ cwd: REPO }); + const linked = rows.find((r) => r.name === "linked"); + assert.ok(linked, "a symlinked skill directory must still be discovered"); + assert.equal(linked.frontmatter.name, "linked"); + for (const row of rows) { assert.ok(row.file.startsWith(TMP) || row.file.startsWith(REPO)); + assert.ok(!row.file.startsWith(outside)); } + fs.rmSync(outside, { recursive: true, force: true }); }); it("summarizes counts and roots in the overview", () => { diff --git a/server/lib/cc-discovery.js b/server/lib/cc-discovery.js index efdc7a94..bfdf7514 100644 --- a/server/lib/cc-discovery.js +++ b/server/lib/cc-discovery.js @@ -925,6 +925,7 @@ module.exports = { parseFrontmatter, redactSettings, isUnder, + isDirLike, isFileLike, MAX_FILE_BYTES, HOOK_EVENT_TYPES, diff --git a/server/lib/codex-discovery.js b/server/lib/codex-discovery.js index 7fe21f3c..2e78428b 100644 --- a/server/lib/codex-discovery.js +++ b/server/lib/codex-discovery.js @@ -15,7 +15,13 @@ const fs = require("node:fs"); const path = require("node:path"); const { getCodexHome, getProjectCodexDir, getProjectRoot } = require("./codex-home"); -const { parseFrontmatter, isUnder, MAX_FILE_BYTES } = require("./cc-discovery"); +const { + parseFrontmatter, + isUnder, + isDirLike, + isFileLike, + MAX_FILE_BYTES, +} = require("./cc-discovery"); function safeReadText(absPath) { try { @@ -55,7 +61,7 @@ function readSkillsAt(scope, codexDir) { const skills = []; for (const ent of listDir(dir)) { const skillDir = path.join(dir, ent.name); - if (!ent.isDirectory() || !isUnder(codexDir, skillDir)) continue; + if (!isDirLike(ent, skillDir) || !isUnder(codexDir, skillDir)) continue; const skillFile = path.join(skillDir, "SKILL.md"); const read = safeReadText(skillFile); if (!read) continue; @@ -116,10 +122,16 @@ function readTomlMeta(text) { (value.startsWith("'") && value.endsWith("'")) ) { value = value.slice(1, -1); - } else if (value === "true" || value === "false") { - value = value === "true"; - } else if (/^-?\d+(\.\d+)?$/.test(value)) { - value = Number(value); + } else if (value.startsWith('"') || value.startsWith("'")) { + // A quoted value with a trailing `# comment` outside the string — + // e.g. model = "gpt-5.4" # default. Strip from the closing quote on. + const quote = value[0]; + const closeIdx = value.indexOf(quote, 1); + if (closeIdx > 0) value = value.slice(1, closeIdx); + else continue; + } else if (/^(true|false|-?\d+(\.\d+)?)\s*(#.*)?$/.test(value)) { + value = value.replace(/\s*#.*$/, ""); + value = value === "true" ? true : value === "false" ? false : Number(value); } else { continue; // arrays, inline tables, etc. — not one of the shown fields } @@ -134,7 +146,7 @@ function readAgentsAt(scope, codexDir) { for (const ent of listDir(dir)) { if (!ent.name.endsWith(".toml")) continue; const file = path.join(dir, ent.name); - if (!ent.isFile() || !isUnder(codexDir, file)) continue; + if (!isFileLike(ent, file) || !isUnder(codexDir, file)) continue; const read = safeReadText(file); if (!read) continue; agents.push({ @@ -165,7 +177,7 @@ function readRulesAt(scope, codexDir) { for (const ent of listDir(dir)) { if (!ent.name.endsWith(".rules")) continue; const file = path.join(dir, ent.name); - if (!ent.isFile() || !isUnder(codexDir, file)) continue; + if (!isFileLike(ent, file) || !isUnder(codexDir, file)) continue; const read = safeReadText(file); if (!read) continue; rules.push({ @@ -268,5 +280,4 @@ module.exports = { readConfig, readOverview, readTomlMeta, - isUnder, }; From 236e94760f7eec855331dea892950bed6f1b6ab8 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 16:48:45 +0700 Subject: [PATCH 05/27] fix(codex): escape-aware closing-quote scan, honest symlink coverage readTomlMeta's trailing-comment stripper found the first quote character regardless of escaping, so a description containing \"escaped quotes\" before a real comment got silently truncated instead of dropped. The scan now skips a quote preceded by a backslash. The "never walks outside" containment test passed by construction (row.file is always built off a fixed root) and would not fail if the isUnder guard were removed. Replaced it with two tests that actually pin the regression risk: a skill dir and an agent .toml symlinked in from outside CODEX_HOME, asserting the row is discovered and its content read through the link. isUnder stays as a defensive check (matches cc-discovery.js), now commented as such. --- server/__tests__/codex-discovery.test.js | 39 ++++++++++++++++++------ server/lib/codex-discovery.js | 17 +++++++++-- 2 files changed, 44 insertions(+), 12 deletions(-) diff --git a/server/__tests__/codex-discovery.test.js b/server/__tests__/codex-discovery.test.js index 75643b9f..b7cf0a0b 100644 --- a/server/__tests__/codex-discovery.test.js +++ b/server/__tests__/codex-discovery.test.js @@ -93,6 +93,11 @@ describe("codex-discovery", () => { assert.equal(meta.description, "uses # for headings"); }); + it("does not let an escaped quote before a real trailing comment truncate the value", () => { + const meta = cx.readTomlMeta('description = "say \\"hi\\"" # not a comment\n'); + assert.equal(meta.description, 'say \\"hi\\"'); + }); + it("reads a rule file's preview", () => { write( path.join(TMP, "rules", "no-force-push.rules"), @@ -127,19 +132,33 @@ describe("codex-discovery", () => { assert.equal(project.exists, false); }); - it("follows a symlinked skill dir (a supported install pattern) without ever reporting a path outside the codex roots", () => { + it("follows a skill directory symlinked in from outside CODEX_HOME — a supported install pattern", () => { const outside = fs.mkdtempSync(path.join(os.tmpdir(), "codex-disc-outside-")); write(path.join(outside, "SKILL.md"), "---\nname: linked\n---\nbody"); - fs.symlinkSync(outside, path.join(TMP, "skills", "linked"), "dir"); + const linkPath = path.join(TMP, "skills", "linked"); + fs.symlinkSync(outside, linkPath, "dir"); - const rows = cx.readSkills({ cwd: REPO }); - const linked = rows.find((r) => r.name === "linked"); - assert.ok(linked, "a symlinked skill directory must still be discovered"); - assert.equal(linked.frontmatter.name, "linked"); - for (const row of rows) { - assert.ok(row.file.startsWith(TMP) || row.file.startsWith(REPO)); - assert.ok(!row.file.startsWith(outside)); - } + const [linked] = cx.readSkills({ cwd: REPO, scope: "user" }); + assert.equal(linked.scope, "user"); + assert.equal(linked.name, "linked"); + assert.equal(linked.frontmatter.name, "linked", "frontmatter must be read through the link"); + assert.equal(linked.file, path.join(linkPath, "SKILL.md")); + fs.rmSync(outside, { recursive: true, force: true }); + }); + + it("follows an agent .toml symlinked in from outside CODEX_HOME", () => { + const outside = fs.mkdtempSync(path.join(os.tmpdir(), "codex-disc-outside-")); + const realFile = path.join(outside, "reviewer.toml"); + write(realFile, 'name = "reviewer"\n'); + const linkPath = path.join(TMP, "agents", "reviewer.toml"); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + fs.symlinkSync(realFile, linkPath, "file"); + + const [agent] = cx.readAgents({ cwd: REPO, scope: "user" }); + assert.equal(agent.scope, "user"); + assert.equal(agent.name, "reviewer"); + assert.equal(agent.meta.name, "reviewer", "meta must be read through the link"); + assert.equal(agent.file, linkPath); fs.rmSync(outside, { recursive: true, force: true }); }); diff --git a/server/lib/codex-discovery.js b/server/lib/codex-discovery.js index 2e78428b..82aed0a8 100644 --- a/server/lib/codex-discovery.js +++ b/server/lib/codex-discovery.js @@ -61,6 +61,8 @@ function readSkillsAt(scope, codexDir) { const skills = []; for (const ent of listDir(dir)) { const skillDir = path.join(dir, ent.name); + // isUnder here is defensive only — ent.name comes from readdirSync and + // can't carry a traversal segment, so this can never actually trip. if (!isDirLike(ent, skillDir) || !isUnder(codexDir, skillDir)) continue; const skillFile = path.join(skillDir, "SKILL.md"); const read = safeReadText(skillFile); @@ -124,9 +126,16 @@ function readTomlMeta(text) { value = value.slice(1, -1); } else if (value.startsWith('"') || value.startsWith("'")) { // A quoted value with a trailing `# comment` outside the string — - // e.g. model = "gpt-5.4" # default. Strip from the closing quote on. + // e.g. model = "gpt-5.4" # default. Find the real closing quote + // (skipping a `\"` escaped one) and strip everything after it. const quote = value[0]; - const closeIdx = value.indexOf(quote, 1); + let closeIdx = -1; + for (let i = 1; i < value.length; i++) { + if (value[i] === quote && value[i - 1] !== "\\") { + closeIdx = i; + break; + } + } if (closeIdx > 0) value = value.slice(1, closeIdx); else continue; } else if (/^(true|false|-?\d+(\.\d+)?)\s*(#.*)?$/.test(value)) { @@ -146,6 +155,8 @@ function readAgentsAt(scope, codexDir) { for (const ent of listDir(dir)) { if (!ent.name.endsWith(".toml")) continue; const file = path.join(dir, ent.name); + // isUnder here is defensive only — ent.name comes from readdirSync and + // can't carry a traversal segment, so this can never actually trip. if (!isFileLike(ent, file) || !isUnder(codexDir, file)) continue; const read = safeReadText(file); if (!read) continue; @@ -177,6 +188,8 @@ function readRulesAt(scope, codexDir) { for (const ent of listDir(dir)) { if (!ent.name.endsWith(".rules")) continue; const file = path.join(dir, ent.name); + // isUnder here is defensive only — ent.name comes from readdirSync and + // can't carry a traversal segment, so this can never actually trip. if (!isFileLike(ent, file) || !isUnder(codexDir, file)) continue; const read = safeReadText(file); if (!read) continue; From b0788eea80d91816d20c1cbf9d15fa1372f17a3b Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 16:57:02 +0700 Subject: [PATCH 06/27] fix(codex): backslash parity in the TOML closing-quote scan Round 2's escape check rejected a quote whenever a single backslash preceded it, so an escaped backslash before a real closing quote (e.g. path = "path\\" # c) was misread as an escaped quote and the field was dropped. Now counts the run of backslashes and only treats an ODD run as escaping the quote. Folded the old two-branch quote handling into one scan (it already finds the terminator correctly with or without a trailing comment) and decode \\ -> \ in the extracted value, since the scan already knows which backslash run is real. --- server/__tests__/codex-discovery.test.js | 10 ++++++++++ server/lib/codex-discovery.js | 25 ++++++++++++------------ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/server/__tests__/codex-discovery.test.js b/server/__tests__/codex-discovery.test.js index b7cf0a0b..6e325bff 100644 --- a/server/__tests__/codex-discovery.test.js +++ b/server/__tests__/codex-discovery.test.js @@ -98,6 +98,16 @@ describe("codex-discovery", () => { assert.equal(meta.description, 'say \\"hi\\"'); }); + it("recognises a quote preceded by an escaped backslash as a real terminator", () => { + const meta = cx.readTomlMeta('path = "path\\\\" # c\n'); + assert.equal(meta.path, "path\\"); + }); + + it("keeps counting backslash pairs correctly with four of them before the terminator", () => { + const meta = cx.readTomlMeta('key = "a\\\\\\\\" # c\n'); + assert.equal(meta.key, "a\\\\"); + }); + it("reads a rule file's preview", () => { write( path.join(TMP, "rules", "no-force-push.rules"), diff --git a/server/lib/codex-discovery.js b/server/lib/codex-discovery.js index 82aed0a8..d026dca0 100644 --- a/server/lib/codex-discovery.js +++ b/server/lib/codex-discovery.js @@ -119,25 +119,26 @@ function readTomlMeta(text) { if (!value.slice(3).includes('"""')) inTripleQuote = true; continue; } - if ( - (value.startsWith('"') && value.endsWith('"')) || - (value.startsWith("'") && value.endsWith("'")) - ) { - value = value.slice(1, -1); - } else if (value.startsWith('"') || value.startsWith("'")) { - // A quoted value with a trailing `# comment` outside the string — - // e.g. model = "gpt-5.4" # default. Find the real closing quote - // (skipping a `\"` escaped one) and strip everything after it. + if (value.startsWith('"') || value.startsWith("'")) { + // Find the real closing quote — one not preceded by an ODD run of + // backslashes (an even run, including zero, means it isn't escaped) — + // so a trailing `# comment` outside the string is stripped without + // being fooled by an escaped quote or an escaped backslash inside it. const quote = value[0]; let closeIdx = -1; for (let i = 1; i < value.length; i++) { - if (value[i] === quote && value[i - 1] !== "\\") { + if (value[i] !== quote) continue; + let backslashes = 0; + while (value[i - 1 - backslashes] === "\\") backslashes++; + if (backslashes % 2 === 0) { closeIdx = i; break; } } - if (closeIdx > 0) value = value.slice(1, closeIdx); - else continue; + if (closeIdx < 0) continue; + // \\ decodes to one backslash; other escapes (e.g. \") are left + // literal — this is a best-effort reader, not a full TOML unescaper. + value = value.slice(1, closeIdx).replace(/\\\\/g, "\\"); } else if (/^(true|false|-?\d+(\.\d+)?)\s*(#.*)?$/.test(value)) { value = value.replace(/\s*#.*$/, ""); value = value === "true" ? true : value === "false" ? false : Number(value); From ba796106975b56febafe1a28a114573074e6a8be Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 17:06:44 +0700 Subject: [PATCH 07/27] feat(codex): wrap the codex mcp/plugin CLI in argv-safe calls --- server/__tests__/codex-cli.test.js | 202 +++++++++++++++++++++++++++ server/lib/codex-cli.js | 212 +++++++++++++++++++++++++++++ 2 files changed, 414 insertions(+) create mode 100644 server/__tests__/codex-cli.test.js create mode 100644 server/lib/codex-cli.js diff --git a/server/__tests__/codex-cli.test.js b/server/__tests__/codex-cli.test.js new file mode 100644 index 00000000..03a6a09a --- /dev/null +++ b/server/__tests__/codex-cli.test.js @@ -0,0 +1,202 @@ +/** + * @file Tests for server/lib/codex-cli.js — the `codex mcp`/`codex plugin` + * wrapper that backs the Codex config panel's MCP-server and plugin + * management. Uses a fake `codex` script on PATH (the fixture style + * server/__tests__/cc-plugin-cli.test.js uses) rather than mocking + * child_process, and pins the exact argv each operation builds — that argv + * IS the security boundary. + * @author Nguyễn Ngọc Trí Vĩ + */ +const { describe, it, after } = require("node:test"); +const assert = require("node:assert/strict"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); + +const ROOT = fs.mkdtempSync(path.join(os.tmpdir(), "agenthub-codexcli-")); +const cli = require("../lib/codex-cli"); + +/** + * Put a fake `codex` on PATH. The default body echoes the argv, one element + * per line, so a test can assert the exact array that was spawned. + */ +function stubCodex(scriptBody = 'for a in "$@"; do echo "$a"; done') { + const bin = path.join(ROOT, `bin-${process.hrtime.bigint()}`); + fs.mkdirSync(bin, { recursive: true }); + const file = path.join(bin, "codex"); + fs.writeFileSync(file, `#!/usr/bin/env bash\n${scriptBody}\n`); + fs.chmodSync(file, 0o755); + const originalPath = process.env.PATH; + process.env.PATH = `${bin}${path.delimiter}${originalPath}`; + return () => { + process.env.PATH = originalPath; + }; +} + +/** PATH with no `codex` on it at all. */ +function stubMissingCodex() { + const empty = path.join(ROOT, `empty-${process.hrtime.bigint()}`); + fs.mkdirSync(empty, { recursive: true }); + const originalPath = process.env.PATH; + process.env.PATH = empty; + return () => { + process.env.PATH = originalPath; + }; +} + +const argvOf = (result) => result.stdout.trim().split("\n"); + +after(() => fs.rmSync(ROOT, { recursive: true, force: true, maxRetries: 5 })); + +describe("codex-cli argument validation", () => { + it("refuses a name that could be read as a flag or a path", async () => { + await assert.rejects(() => cli.mcpRemove("--force"), /name/i); + await assert.rejects(() => cli.mcpRemove("../../etc"), /name/i); + await assert.rejects(() => cli.pluginRemove("--force"), /name/i); + await assert.rejects(() => cli.marketplaceRemove("--force"), /name/i); + await assert.rejects(() => cli.mcpAdd({ name: "--force", command: "node" }), /name/i); + }); + + it("accepts a plain identifier as a name", () => { + assert.match("docs", cli.NAME_RE); + assert.doesNotMatch("--force", cli.NAME_RE); + assert.doesNotMatch("../../etc", cli.NAME_RE); + }); +}); + +describe("codex-cli argv construction", () => { + it("passes the server name as one argv entry, never through a shell", async () => { + const restore = stubCodex(); + try { + const result = await cli.mcpAdd({ name: "docs", command: "node", args: ["server.js"] }); + assert.deepEqual(argvOf(result), ["mcp", "add", "docs", "--", "node", "server.js"]); + } finally { + restore(); + } + }); + + it("builds mcp list / remove argvs", async () => { + const restore = stubCodex(); + try { + assert.deepEqual(argvOf(await cli.mcpList()), ["mcp", "list", "--json"]); + assert.deepEqual(argvOf(await cli.mcpRemove("docs")), ["mcp", "remove", "docs"]); + } finally { + restore(); + } + }); + + it("builds plugin list / add / remove argvs", async () => { + const restore = stubCodex(); + try { + assert.deepEqual(argvOf(await cli.pluginList()), ["plugin", "list", "--json"]); + assert.deepEqual(argvOf(await cli.pluginAdd("demo@sg")), ["plugin", "add", "demo@sg"]); + assert.deepEqual(argvOf(await cli.pluginRemove("demo")), ["plugin", "remove", "demo"]); + } finally { + restore(); + } + }); + + it("builds marketplace list / add / remove argvs, nested under plugin", async () => { + const restore = stubCodex(); + try { + assert.deepEqual(argvOf(await cli.marketplaceList()), [ + "plugin", + "marketplace", + "list", + "--json", + ]); + assert.deepEqual(argvOf(await cli.marketplaceAdd("https://example.com/mp.git")), [ + "plugin", + "marketplace", + "add", + "https://example.com/mp.git", + ]); + assert.deepEqual(argvOf(await cli.marketplaceRemove("sg")), [ + "plugin", + "marketplace", + "remove", + "sg", + ]); + } finally { + restore(); + } + }); + + it("never lets a rejected name reach a shell", async () => { + // If the value were interpolated into a shell string this would create + // the file; the NAME_RE guard rejects it before any spawn happens. + const canary = path.join(ROOT, "pwned"); + const restore = stubCodex(); + try { + await assert.rejects(() => cli.mcpRemove("docs; touch " + canary), /name/i); + assert.equal(fs.existsSync(canary), false); + } finally { + restore(); + } + }); +}); + +describe("codex-cli run outcomes", () => { + it("surfaces a non-zero exit as ok:false with stderr kept", async () => { + const restore = stubCodex('echo "no such marketplace" >&2\nexit 2'); + try { + const res = await cli.marketplaceRemove("nope"); + assert.equal(res.ok, false); + assert.equal(res.code, 2); + assert.match(res.stderr, /no such marketplace/); + } finally { + restore(); + } + }); + + it("throws a clear error when the codex CLI is not on PATH", async () => { + const restore = stubMissingCodex(); + try { + await assert.rejects( + () => cli.mcpList(), + (err) => { + assert.equal(err.code, "ECLINOTFOUND"); + assert.match(err.message, /not found on PATH/); + return true; + } + ); + } finally { + restore(); + } + }); + + it("parses JSON output into `data`", async () => { + const restore = stubCodex(`cat <<'JSON'\n{"servers":[{"name":"docs"}]}\nJSON`); + try { + const res = await cli.mcpList(); + assert.equal(res.ok, true); + assert.deepEqual(res.data, { servers: [{ name: "docs" }] }); + } finally { + restore(); + } + }); + + it("leaves `data` undefined when stdout is not JSON", async () => { + const restore = stubCodex('echo "removed docs"'); + try { + const res = await cli.mcpRemove("docs"); + assert.equal(res.ok, true); + assert.equal(res.data, undefined); + } finally { + restore(); + } + }); + + it("serializes mutating calls so two clicks cannot interleave", async () => { + const log = path.join(ROOT, `order-${process.hrtime.bigint()}.txt`); + const restore = stubCodex(`echo "start $2" >> ${log}\nsleep 0.2\necho "end $2" >> ${log}`); + try { + await Promise.all([cli.mcpRemove("first"), cli.mcpRemove("second")]); + const lines = fs.readFileSync(log, "utf8").trim().split("\n"); + assert.equal(lines[0].replace("start ", ""), lines[1].replace("end ", "")); + assert.equal(lines[2].replace("start ", ""), lines[3].replace("end ", "")); + } finally { + restore(); + } + }); +}); diff --git a/server/lib/codex-cli.js b/server/lib/codex-cli.js new file mode 100644 index 00000000..c2b7940a --- /dev/null +++ b/server/lib/codex-cli.js @@ -0,0 +1,212 @@ +/** + * @file codex-cli.js + * @description Drives the official `codex mcp …` / `codex plugin …` CLI so + * the Codex config panel can add/remove MCP servers, marketplaces and + * plugins without the dashboard touching Codex's TOML config directly. + * + * Why the CLI and not direct TOML edits: the CLI owns config.toml's schema + * (MCP server table, plugin/marketplace registration) and rewrites it in + * place. Reproducing that here would drift the moment the CLI's format + * moves — the same reasoning `cc-plugin-cli.js` follows for `claude plugin`. + * + * Hard constraints (do not relax without a follow-up review): + * - `execFile` with an explicit argv array. NEVER a shell string — a + * server/plugin/marketplace name comes straight from a user-typed field. + * - A name is validated against `NAME_RE` BEFORE spawning: no leading `-` + * (never a flag), no `/` (never a path). This is the same guard the + * HTTP routes re-check. + * - Mutating calls are serialized in-process; two dashboard clicks must + * not interleave writes to the same config.toml. + * - `CODEX_HOME` is pinned to the same home `codex-discovery` reads, so + * the CLI writes where the dashboard looks. + * + * A non-zero CLI exit resolves as `{ok: false}` (data, not an error) so a + * route can map it to HTTP 422. Only a rejected name or a missing binary + * throws — those are programming/environment errors, not CLI outcomes. + * + * @author Nguyễn Ngọc Trí Vĩ + */ + +const { execFile } = require("node:child_process"); +const { getCodexHome } = require("./codex-home"); + +const DEFAULT_TIMEOUT_MS = 120_000; +const MAX_OUTPUT_BYTES = 1024 * 1024; + +/** An MCP server / plugin / marketplace name. No slashes: never a path. */ +const NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; + +function makeError(code, message) { + const err = new Error(message); + err.code = code; + return err; +} + +/** + * Baseline check applied to every argv element we interpolate. A value that + * starts with `-` would be parsed as a flag by the CLI even inside an argv + * array, so it is rejected rather than escaped. + */ +function assertArg(value, label) { + if (typeof value !== "string" || !value.trim()) { + throw makeError("EBADARG", `${label} is required`); + } + if (value.length > 512) throw makeError("EBADARG", `${label} is too long`); + // eslint-disable-next-line no-control-regex + if (/[\u0000-\u001f\u007f]/.test(value)) { + throw makeError("EBADARG", `${label} contains control characters`); + } + if (value.startsWith("-")) throw makeError("EBADARG", `${label} may not start with "-"`); + return value; +} + +/** An MCP server / plugin / marketplace identifier — the routes' shared guard. */ +function assertName(value, label = "name") { + assertArg(value, label); + if (!NAME_RE.test(value)) { + throw makeError("EBADARG", `${label} is not a valid identifier`); + } + return value; +} + +// One in-process queue for every mutating CLI call — the same reasoning as +// cc-plugin-cli.js's `mutationQueue`: this dashboard is the only Node writer, +// so a promise chain is the whole locking story. It does NOT protect against +// a human running `codex …` in a terminal at the same moment. +let mutationQueue = Promise.resolve(); + +function serialize(fn) { + const run = mutationQueue.then(fn, fn); + mutationQueue = run.then( + () => undefined, + () => undefined + ); + return run; +} + +/** + * Run `codex ` and resolve with the outcome. A non-zero exit is a + * normal result (`ok:false`), not a rejection. When stdout parses as JSON + * the parsed value is attached as `data`. + * + * @param {string[]} args argv AFTER the binary name; every element pre-validated. + * @param {{timeoutMs?:number}} [opts] + * @returns {Promise<{ok:boolean, code:number|null, stdout:string, stderr:string, data?:object}>} + */ +function runCodex(args, opts = {}) { + const timeout = opts.timeoutMs || DEFAULT_TIMEOUT_MS; + return new Promise((resolve, reject) => { + const child = execFile( + "codex", + args, + { + timeout, + maxBuffer: MAX_OUTPUT_BYTES, + // Pin the CLI to the same home codex-discovery reads, so a write + // made here is a write the dashboard can see. + env: { ...process.env, CODEX_HOME: getCodexHome() }, + }, + (err, stdout, stderr) => { + const out = String(stdout || ""); + const errOut = String(stderr || ""); + if (err && (err.code === "ENOENT" || err.code === "EACCES")) { + return reject( + makeError( + "ECLINOTFOUND", + "the `codex` CLI was not found on PATH — install Codex or add it to the dashboard's PATH" + ) + ); + } + if (err && err.killed) { + return reject(makeError("ETIMEOUT", `codex ${args[0]} timed out after ${timeout}ms`)); + } + const code = err ? (typeof err.code === "number" ? err.code : 1) : 0; + const result = { ok: !err, code, stdout: out, stderr: errOut }; + try { + result.data = JSON.parse(out.trim()); + } catch { + /* not every command emits JSON */ + } + resolve(result); + } + ); + // Every command here is non-interactive. Closing stdin turns a would-be + // prompt into an immediate CLI error instead of a hang until `timeout`. + try { + child.stdin?.end(); + } catch { + /* the child may already have exited */ + } + }); +} + +// ── MCP servers ──────────────────────────────────────────────────────── + +async function mcpList() { + return runCodex(["mcp", "list", "--json"]); +} + +async function mcpAdd({ name, command, args = [], env }) { + const n = assertName(name); + const cmd = assertArg(command, "command"); + const cmdArgs = args.map((a, i) => assertArg(a, `args[${i}]`)); + const envFlags = Object.entries(env || {}).flatMap(([k, v]) => { + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(k)) { + throw makeError("EBADARG", `env key "${k}" is not a valid identifier`); + } + assertArg(String(v), `env.${k}`); + return ["--env", `${k}=${v}`]; + }); + return serialize(() => runCodex(["mcp", "add", n, ...envFlags, "--", cmd, ...cmdArgs])); +} + +async function mcpRemove(name) { + const n = assertName(name); + return serialize(() => runCodex(["mcp", "remove", n])); +} + +// ── Plugins ──────────────────────────────────────────────────────────── + +async function pluginList() { + return runCodex(["plugin", "list", "--json"]); +} + +async function pluginAdd(spec) { + const s = assertArg(spec, "plugin spec"); + return serialize(() => runCodex(["plugin", "add", s])); +} + +async function pluginRemove(name) { + const n = assertName(name); + return serialize(() => runCodex(["plugin", "remove", n])); +} + +// ── Marketplaces ─────────────────────────────────────────────────────── + +async function marketplaceList() { + return runCodex(["plugin", "marketplace", "list", "--json"]); +} + +async function marketplaceAdd(url) { + const u = assertArg(url, "marketplace url"); + return serialize(() => runCodex(["plugin", "marketplace", "add", u])); +} + +async function marketplaceRemove(name) { + const n = assertName(name); + return serialize(() => runCodex(["plugin", "marketplace", "remove", n])); +} + +module.exports = { + runCodex, + mcpList, + mcpAdd, + mcpRemove, + pluginList, + pluginAdd, + pluginRemove, + marketplaceList, + marketplaceAdd, + marketplaceRemove, + NAME_RE, +}; From dcd65f891c49e1c157dc51583be1f19c833c3691 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 17:15:24 +0700 Subject: [PATCH 08/27] fix(codex): support mcp add --url form, guard specs/URLs, cover --env --- server/__tests__/codex-cli.test.js | 87 ++++++++++++++++++++++++++++++ server/lib/codex-cli.js | 78 ++++++++++++++++++++++++--- 2 files changed, 157 insertions(+), 8 deletions(-) diff --git a/server/__tests__/codex-cli.test.js b/server/__tests__/codex-cli.test.js index 03a6a09a..e0802ef8 100644 --- a/server/__tests__/codex-cli.test.js +++ b/server/__tests__/codex-cli.test.js @@ -62,6 +62,45 @@ describe("codex-cli argument validation", () => { assert.doesNotMatch("--force", cli.NAME_RE); assert.doesNotMatch("../../etc", cli.NAME_RE); }); + + it("accepts and @ plugin specs only", () => { + assert.equal(cli.assertPluginSpec("agenthub"), "agenthub"); + assert.equal(cli.assertPluginSpec("agenthub@sg"), "agenthub@sg"); + for (const bad of ["../agenthub", "agenthub agenthub", "@sg", "-agenthub", "a/b"]) { + assert.throws(() => cli.assertPluginSpec(bad), undefined, `accepted ${bad}`); + } + }); + + it("accepts only http(s) URLs", () => { + assert.equal(cli.assertUrl("https://example.com/mp.git", "url"), "https://example.com/mp.git"); + for (const bad of ["ssh://example.com/mp.git", "not a url", "file:///tmp/x"]) { + assert.throws(() => cli.assertUrl(bad, "url"), /http\(s\)/, `accepted ${bad}`); + } + // rejected earlier, by the shared leading-dash guard, not the URL check + assert.throws(() => cli.assertUrl("--force", "url"), /may not start with/); + }); + + it("rejects mcpAdd with neither or both of url/command", async () => { + await assert.rejects(() => cli.mcpAdd({ name: "docs" }), /requires either/); + await assert.rejects( + () => cli.mcpAdd({ name: "docs", url: "https://x", command: "node" }), + /either url or command, not both/ + ); + }); + + it("rejects env together with url — --env is invalid on the url form", async () => { + await assert.rejects( + () => cli.mcpAdd({ name: "docs", url: "https://x", env: { A: "1" } }), + /stdio-only/ + ); + }); + + it("rejects an env key that is not a valid identifier", async () => { + await assert.rejects( + () => cli.mcpAdd({ name: "docs", command: "node", env: { "1bad": "1" } }), + /identifier/ + ); + }); }); describe("codex-cli argv construction", () => { @@ -75,6 +114,54 @@ describe("codex-cli argv construction", () => { } }); + it("puts --env flags between NAME and -- for a stdio server", async () => { + const restore = stubCodex(); + try { + const result = await cli.mcpAdd({ + name: "docs", + command: "node", + args: ["server.js"], + env: { API_KEY: "secret", MODE: "prod" }, + }); + assert.deepEqual(argvOf(result), [ + "mcp", + "add", + "docs", + "--env", + "API_KEY=secret", + "--env", + "MODE=prod", + "--", + "node", + "server.js", + ]); + } finally { + restore(); + } + }); + + it("builds the --url form with an optional --bearer-token-env-var", async () => { + const restore = stubCodex(); + try { + const result = await cli.mcpAdd({ + name: "docs", + url: "https://example.com/mcp", + bearerTokenEnvVar: "DOCS_TOKEN", + }); + assert.deepEqual(argvOf(result), [ + "mcp", + "add", + "docs", + "--bearer-token-env-var", + "DOCS_TOKEN", + "--url", + "https://example.com/mcp", + ]); + } finally { + restore(); + } + }); + it("builds mcp list / remove argvs", async () => { const restore = stubCodex(); try { diff --git a/server/lib/codex-cli.js b/server/lib/codex-cli.js index c2b7940a..ba00d271 100644 --- a/server/lib/codex-cli.js +++ b/server/lib/codex-cli.js @@ -14,7 +14,8 @@ * server/plugin/marketplace name comes straight from a user-typed field. * - A name is validated against `NAME_RE` BEFORE spawning: no leading `-` * (never a flag), no `/` (never a path). This is the same guard the - * HTTP routes re-check. + * HTTP routes re-check. Plugin specs and URLs get their own purpose-built + * guards (`assertPluginSpec`, `assertUrl`) — same idea, looser shape. * - Mutating calls are serialized in-process; two dashboard clicks must * not interleave writes to the same config.toml. * - `CODEX_HOME` is pinned to the same home `codex-discovery` reads, so @@ -35,6 +36,10 @@ const MAX_OUTPUT_BYTES = 1024 * 1024; /** An MCP server / plugin / marketplace name. No slashes: never a path. */ const NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*$/; +/** `name` or `name@marketplace` — same shape as cc-plugin-cli's PLUGIN_SPEC_RE. */ +const PLUGIN_SPEC_RE = /^[A-Za-z0-9][A-Za-z0-9._-]*(@[A-Za-z0-9][A-Za-z0-9._-]*)?$/; +/** An environment variable name — used for both `--env K=V` and `--bearer-token-env-var`. */ +const ENV_KEY_RE = /^[A-Za-z_][A-Za-z0-9_]*$/; function makeError(code, message) { const err = new Error(message); @@ -69,6 +74,39 @@ function assertName(value, label = "name") { return value; } +/** `` or `@`, as `codex plugin add` accepts. */ +function assertPluginSpec(value) { + assertArg(value, "plugin spec"); + if (!PLUGIN_SPEC_RE.test(value)) { + throw makeError("EBADARG", "plugin spec must be or @"); + } + return value; +} + +/** An environment variable NAME (never a literal secret). */ +function assertEnvKey(value, label) { + assertArg(value, label); + if (!ENV_KEY_RE.test(value)) { + throw makeError("EBADARG", `${label} is not a valid identifier`); + } + return value; +} + +/** An http(s) URL — `new URL()` both validates and normalizes the parse. */ +function assertUrl(value, label) { + assertArg(value, label); + let parsed; + try { + parsed = new URL(value); + } catch { + throw makeError("EBADARG", `${label} must be a valid http(s) URL`); + } + if (parsed.protocol !== "http:" && parsed.protocol !== "https:") { + throw makeError("EBADARG", `${label} must be a valid http(s) URL`); + } + return value; +} + // One in-process queue for every mutating CLI call — the same reasoning as // cc-plugin-cli.js's `mutationQueue`: this dashboard is the only Node writer, // so a promise chain is the whole locking story. It does NOT protect against @@ -146,16 +184,37 @@ async function mcpList() { return runCodex(["mcp", "list", "--json"]); } -async function mcpAdd({ name, command, args = [], env }) { +/** + * `codex mcp add` has two mutually exclusive forms: a streamable HTTP server + * (`--url`, optionally `--bearer-token-env-var`) or a stdio server + * (`-- `, optionally `--env K=V`). `--env` is invalid on + * the `--url` form per the CLI's own `--help`, so `{url, env}` together is + * rejected rather than silently dropping one of them. + */ +async function mcpAdd({ name, url, bearerTokenEnvVar, command, args = [], env }) { const n = assertName(name); + if (url && command) { + throw makeError("EBADARG", "mcpAdd accepts either url or command, not both"); + } + if (!url && !command) { + throw makeError("EBADARG", "mcpAdd requires either url or command"); + } + + if (url) { + if (env) throw makeError("EBADARG", "env is not valid together with url (stdio-only)"); + const u = assertUrl(url, "url"); + const flags = bearerTokenEnvVar + ? ["--bearer-token-env-var", assertEnvKey(bearerTokenEnvVar, "bearerTokenEnvVar")] + : []; + return serialize(() => runCodex(["mcp", "add", n, ...flags, "--url", u])); + } + const cmd = assertArg(command, "command"); const cmdArgs = args.map((a, i) => assertArg(a, `args[${i}]`)); const envFlags = Object.entries(env || {}).flatMap(([k, v]) => { - if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(k)) { - throw makeError("EBADARG", `env key "${k}" is not a valid identifier`); - } + const key = assertEnvKey(k, "env key"); assertArg(String(v), `env.${k}`); - return ["--env", `${k}=${v}`]; + return ["--env", `${key}=${v}`]; }); return serialize(() => runCodex(["mcp", "add", n, ...envFlags, "--", cmd, ...cmdArgs])); } @@ -172,7 +231,7 @@ async function pluginList() { } async function pluginAdd(spec) { - const s = assertArg(spec, "plugin spec"); + const s = assertPluginSpec(spec); return serialize(() => runCodex(["plugin", "add", s])); } @@ -188,7 +247,7 @@ async function marketplaceList() { } async function marketplaceAdd(url) { - const u = assertArg(url, "marketplace url"); + const u = assertUrl(url, "marketplace url"); return serialize(() => runCodex(["plugin", "marketplace", "add", u])); } @@ -209,4 +268,7 @@ module.exports = { marketplaceAdd, marketplaceRemove, NAME_RE, + PLUGIN_SPEC_RE, + assertPluginSpec, + assertUrl, }; From c9541dc509b128d5b6dd77adc5820307b3d433a3 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 17:22:31 +0700 Subject: [PATCH 09/27] feat(codex): write Codex artifacts with backup and atomic rename --- server/__tests__/codex-mutate.test.js | 237 ++++++++++++++++++++++++++ server/lib/codex-mutate.js | 231 +++++++++++++++++++++++++ 2 files changed, 468 insertions(+) create mode 100644 server/__tests__/codex-mutate.test.js create mode 100644 server/lib/codex-mutate.js diff --git a/server/__tests__/codex-mutate.test.js b/server/__tests__/codex-mutate.test.js new file mode 100644 index 00000000..0372dbf0 --- /dev/null +++ b/server/__tests__/codex-mutate.test.js @@ -0,0 +1,237 @@ +/** + * @file Tests for codex-mutate.js: create/edit/delete on Codex config + * surfaces (skills, agents, rules, memory, config) across the user + * (CODEX_HOME) and project (/.codex or for AGENTS.md) scopes. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const fs = require("node:fs"); +const path = require("node:path"); +const os = require("node:os"); + +const { describe, it, before, after } = require("node:test"); +const assert = require("node:assert/strict"); + +describe("codex-mutate", () => { + let TMP; + let REPO; + let prevCodexHome; + let cm; + + before(() => { + prevCodexHome = process.env.CODEX_HOME; + TMP = fs.mkdtempSync(path.join(os.tmpdir(), "codex-mut-")); + REPO = fs.mkdtempSync(path.join(os.tmpdir(), "codex-mut-repo-")); + process.env.CODEX_HOME = TMP; + delete require.cache[require.resolve("../lib/codex-home")]; + delete require.cache[require.resolve("../lib/codex-mutate")]; + cm = require("../lib/codex-mutate"); + }); + + after(() => { + if (prevCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = prevCodexHome; + fs.rmSync(TMP, { recursive: true, force: true }); + fs.rmSync(REPO, { recursive: true, force: true }); + }); + + it("creates a project skill as a directory with SKILL.md inside", () => { + const { file, created } = cm.writeArtifact({ + scope: "project", + type: "skills", + name: "release-guard", + content: "---\nname: x\n---\n", + cwd: REPO, + }); + assert.equal(created, true); + assert.equal(file, path.join(REPO, ".codex", "skills", "release-guard", "SKILL.md")); + }); + + it("backs the old body up before overwriting it", () => { + cm.writeArtifact({ scope: "user", type: "rules", name: "default", content: "old", cwd: REPO }); + const { backup } = cm.writeArtifact({ + scope: "user", + type: "rules", + name: "default", + content: "new", + cwd: REPO, + }); + assert.equal(fs.readFileSync(backup, "utf8"), "old"); + assert.equal(fs.readFileSync(path.join(TMP, "rules", "default.rules"), "utf8"), "new"); + }); + + it("refuses a name that climbs out of the root", () => { + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "agents", + name: "../../evil", + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADNAME" + ); + }); + + it("writes AGENTS.md at the repo root, not inside .codex", () => { + const { file } = cm.writeArtifact({ + scope: "project", + type: "memory", + content: "# rules", + cwd: REPO, + }); + assert.equal(file, path.join(REPO, "AGENTS.md")); + }); + + it("keeps a deleted skill's backup out of the skills tree", () => { + cm.writeArtifact({ scope: "user", type: "skills", name: "gone", content: "x", cwd: REPO }); + const { backup } = cm.deleteArtifact({ + scope: "user", + type: "skills", + name: "gone", + cwd: REPO, + }); + assert.ok( + !backup.includes(path.join(TMP, "skills", "gone")), + "a backup must never be discoverable as a skill" + ); + assert.equal(fs.existsSync(path.join(TMP, "skills", "gone")), false); + }); + + it("writes and reads back a project agent under .codex/agents/.toml", () => { + const { file } = cm.writeArtifact({ + scope: "project", + type: "agents", + name: "reviewer", + content: 'name = "reviewer"\n', + cwd: REPO, + }); + assert.equal(file, path.join(REPO, ".codex", "agents", "reviewer.toml")); + }); + + it("writes user config.toml under CODEX_HOME and project config.toml under /.codex", () => { + const u = cm.writeArtifact({ + scope: "user", + type: "config", + content: 'model = "x"\n', + cwd: REPO, + }); + assert.equal(u.file, path.join(TMP, "config.toml")); + const p = cm.writeArtifact({ + scope: "project", + type: "config", + content: 'model = "y"\n', + cwd: REPO, + }); + assert.equal(p.file, path.join(REPO, ".codex", "config.toml")); + }); + + it("rejects a scope that is neither user nor project", () => { + assert.throws( + () => + cm.writeArtifact({ + scope: "project-local", + type: "rules", + name: "x", + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADSCOPE" + ); + }); + + it("rejects a type outside TYPES", () => { + assert.throws( + () => + cm.writeArtifact({ scope: "user", type: "commands", name: "x", content: "x", cwd: REPO }), + (e) => e.code === "EBADTYPE" + ); + }); + + for (const badName of ["..", ".", "/etc/passwd", "a/b", "a\0b"]) { + it(`rejects the bad name ${JSON.stringify(badName)} with EBADNAME and writes nothing`, () => { + const target = path.join(TMP, "agents", `${badName.replace(/[/\0]/g, "_")}.toml`); + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "agents", + name: badName, + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADNAME" + ); + assert.equal(fs.existsSync(target), false); + }); + } + + it("rejects a name passed to memory", () => { + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "memory", + name: "AGENTS", + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADNAME" + ); + }); + + it("rejects a name passed to config", () => { + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "config", + name: "config", + content: "x", + cwd: REPO, + }), + (e) => e.code === "EBADNAME" + ); + }); + + it("rejects content over the size limit and leaves the existing file untouched", () => { + cm.writeArtifact({ scope: "user", type: "rules", name: "toobig", content: "small", cwd: REPO }); + const huge = "x".repeat(256 * 1024 + 1); + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "rules", + name: "toobig", + content: huge, + cwd: REPO, + }), + (e) => e.code === "ETOOLARGE" + ); + assert.equal(fs.readFileSync(path.join(TMP, "rules", "toobig.rules"), "utf8"), "small"); + }); + + it("refuses to delete something that does not exist, and creates no backup", () => { + const before = fs.existsSync(path.join(TMP, "codex-config-backups", "rules")) + ? fs.readdirSync(path.join(TMP, "codex-config-backups", "rules")).length + : 0; + assert.throws( + () => cm.deleteArtifact({ scope: "user", type: "rules", name: "never-existed", cwd: REPO }), + (e) => e.code === "ENOTFOUND" + ); + const after = fs.existsSync(path.join(TMP, "codex-config-backups", "rules")) + ? fs.readdirSync(path.join(TMP, "codex-config-backups", "rules")).length + : 0; + assert.equal(after, before, "no backup should be created for a missing target"); + }); + + it("lists backups for a given scope and type", () => { + cm.writeArtifact({ scope: "user", type: "rules", name: "listed", content: "v1", cwd: REPO }); + cm.writeArtifact({ scope: "user", type: "rules", name: "listed", content: "v2", cwd: REPO }); + const rows = cm.listBackups({ scope: "user", type: "rules" }); + assert.ok(rows.length >= 1); + assert.equal(rows[0].type, "rules"); + assert.equal(rows[0].name.startsWith("listed.rules."), true); + }); +}); diff --git a/server/lib/codex-mutate.js b/server/lib/codex-mutate.js new file mode 100644 index 00000000..cc95e691 --- /dev/null +++ b/server/lib/codex-mutate.js @@ -0,0 +1,231 @@ +/** + * @file codex-mutate.js + * @description Mutation helpers for the Codex Config panel. Handles + * create / overwrite / delete on Codex's text-file surfaces: skills, agents, + * rules, memory (AGENTS.md), and config (config.toml). Mirrors cc-mutate.js's + * structure (single `resolveTarget` chokepoint, regex-validated names, + * containment re-check, backup-before-mutation, atomic write) so the two + * panels share the same safety shape and error-code vocabulary. + * + * Hard constraints (do not relax without a follow-up review): + * - `memory` and `config` are fixed filenames (AGENTS.md / config.toml) — + * they take NO `name`. Passing one is a caller error (EBADNAME). + * - Every write/delete of an existing file creates a timestamped backup + * BEFORE the mutation. Backups land under + * /codex-config-backups//, a directory Codex never scans, so + * a deleted skill can never come back as a discoverable skill named + * after its backup. + * - Writes are atomic via temp file + fs.renameSync (see config-write.js). + * - Names are validated against a strict allowlist regex; resolved paths + * are double-checked to live under the expected containment root before + * any I/O. + * - Codex has no `project-local` scope and no auto-memory: only "user" and + * "project" are valid scopes. + * @author Nguyễn Ngọc Trí Vĩ + */ + +const fs = require("node:fs"); +const path = require("node:path"); +const { getCodexHome, getProjectCodexDir, getProjectRoot } = require("./codex-home"); +const { MAX_FILE_BYTES } = require("./cc-discovery"); +const { createBackup, atomicWriteFile, assertUnder } = require("./config-write"); + +const NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/; + +const TYPES = { + skills: { kind: "dir", subdir: "skills", filename: "SKILL.md" }, + agents: { kind: "file", subdir: "agents", ext: ".toml" }, + rules: { kind: "file", subdir: "rules", ext: ".rules" }, + memory: { kind: "fixed", file: "AGENTS.md" }, // AGENTS.md, no `name` + config: { kind: "fixed", file: "config.toml" }, // config.toml, no `name` +}; + +function makeError(code, message) { + const err = new Error(message); + err.code = code; + return err; +} + +function assertScope(scope) { + if (scope !== "user" && scope !== "project") { + throw makeError("EBADSCOPE", `unknown scope: ${scope}`); + } +} + +/** The a (scope) resolves under for skills/agents/rules subdirs. */ +function rootForScope(scope, opts = {}) { + return scope === "user" ? getCodexHome() : getProjectCodexDir(opts.cwd); +} + +/** Fixed-filename path for memory/config, per scope's own layout rule. */ +function fixedPathFor(type, scope, opts = {}) { + if (type === "memory") { + return scope === "user" + ? path.join(getCodexHome(), "AGENTS.md") + : path.join(getProjectRoot(opts.cwd), "AGENTS.md"); + } + // config + return scope === "user" + ? path.join(getCodexHome(), "config.toml") + : path.join(getProjectCodexDir(opts.cwd), "config.toml"); +} + +/** + * Resolve the on-disk target for a (scope, type, name) tuple AND the + * containment root used for path-traversal checks. + * + * Returns: { kind: "file" | "dir", target, filePath, containmentRoot } + */ +function resolveTarget(scope, type, name, opts = {}) { + const spec = TYPES[type]; + if (!spec) throw makeError("EBADTYPE", `unknown type: ${type}`); + assertScope(scope); + + if (spec.kind === "fixed") { + if (name !== undefined && name !== null) { + throw makeError("EBADNAME", `${type} is a fixed file and takes no name`); + } + const filePath = fixedPathFor(type, scope, opts); + return { kind: "file", target: filePath, filePath, containmentRoot: path.dirname(filePath) }; + } + + if (typeof name !== "string" || !NAME_RE.test(name)) { + throw makeError("EBADNAME", `name must match ${NAME_RE}`); + } + + const subdirAbs = path.join(rootForScope(scope, opts), spec.subdir); + + if (spec.kind === "dir") { + const target = path.join(subdirAbs, name); + return { + kind: "dir", + target, + filePath: path.join(target, spec.filename), + containmentRoot: subdirAbs, + }; + } + + const target = path.join(subdirAbs, name + spec.ext); + return { kind: "file", target, filePath: target, containmentRoot: subdirAbs }; +} + +/** /codex-config-backups//, always rooted the same way as the read side's . */ +function backupRootFor(scope, type, opts = {}) { + return path.join(rootForScope(scope, opts), "codex-config-backups", type); +} + +function rmTreeSync(p) { + fs.rmSync(p, { recursive: true, force: true }); +} + +// ── Public API ───────────────────────────────────────────────────────── + +/** + * Create or overwrite a single text artifact. `name` is required for + * skills/agents/rules and forbidden for memory/config. + * + * @param {{scope:string, type:string, name?:string, content:string, cwd?:string}} args + */ +function writeArtifact(args) { + const { scope, type, name, content, cwd } = args; + if (typeof content !== "string") throw makeError("EBADCONTENT", "content must be a string"); + if (Buffer.byteLength(content, "utf8") > MAX_FILE_BYTES) { + throw makeError("ETOOLARGE", `content exceeds ${MAX_FILE_BYTES} bytes`); + } + + const r = resolveTarget(scope, type, name, { cwd }); + assertUnder(r.containmentRoot, r.target); + + const existedBefore = fs.existsSync(r.filePath); + const backup = existedBefore + ? createBackup({ + source: r.kind === "dir" ? r.target : r.filePath, + backupRoot: backupRootFor(scope, type, { cwd }), + type, + name: path.basename(r.target), + }) + : null; + + if (r.kind === "dir") { + fs.mkdirSync(r.target, { recursive: true }); + } + atomicWriteFile(r.filePath, content); + + return { file: r.filePath, created: !existedBefore, backup }; +} + +/** + * Delete a single text artifact. Backup is mandatory and runs first; if + * the target does not exist, no backup is attempted. + */ +function deleteArtifact(args) { + const { scope, type, name, cwd } = args; + const r = resolveTarget(scope, type, name, { cwd }); + assertUnder(r.containmentRoot, r.target); + + if (!fs.existsSync(r.target)) { + throw makeError("ENOTFOUND", `${type}/${name || TYPES[type].file} does not exist`); + } + + const backup = createBackup({ + source: r.target, + backupRoot: backupRootFor(scope, type, { cwd }), + type, + name: path.basename(r.target), + }); + + if (r.kind === "dir") { + rmTreeSync(r.target); + } else { + fs.unlinkSync(r.target); + } + + return { file: r.filePath, backup }; +} + +/** + * List backups for either all types or a specific (scope, type) bucket. + * Returns [{ type, name, file, mtime, size }]. + */ +function listBackups(opts = {}) { + const out = []; + const scopes = opts.scope ? [opts.scope] : ["user", "project"]; + const types = opts.type ? [opts.type] : Object.keys(TYPES); + for (const scope of scopes) { + for (const type of types) { + const root = backupRootFor(scope, type, opts); + let entries = []; + try { + entries = fs.readdirSync(root, { withFileTypes: true }); + } catch { + continue; + } + for (const ent of entries) { + const full = path.join(root, ent.name); + let stat; + try { + stat = fs.statSync(full); + } catch { + continue; + } + out.push({ + type, + name: ent.name, + file: full, + mtime: stat.mtimeMs, + size: ent.isDirectory() ? null : stat.size, + }); + } + } + } + return out.sort((a, b) => b.mtime - a.mtime); +} + +module.exports = { + writeArtifact, + deleteArtifact, + listBackups, + resolveTarget, // exported for tests + TYPES, + NAME_RE, +}; From f3a2f41ed736dc9b4b1b79419cad76c4a21a5f7a Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 17:34:10 +0700 Subject: [PATCH 10/27] fix(codex): close symlink write escape and validate listBackups scope/type --- server/__tests__/codex-mutate.test.js | 86 +++++++++++++++++++++++++-- server/lib/codex-mutate.js | 47 ++++++++++++++- 2 files changed, 126 insertions(+), 7 deletions(-) diff --git a/server/__tests__/codex-mutate.test.js b/server/__tests__/codex-mutate.test.js index 0372dbf0..291aa7ad 100644 --- a/server/__tests__/codex-mutate.test.js +++ b/server/__tests__/codex-mutate.test.js @@ -127,7 +127,8 @@ describe("codex-mutate", () => { assert.equal(p.file, path.join(REPO, ".codex", "config.toml")); }); - it("rejects a scope that is neither user nor project", () => { + it("rejects a scope that is neither user nor project, and writes nothing", () => { + const target = path.join(TMP, "rules", "x.rules"); assert.throws( () => cm.writeArtifact({ @@ -139,14 +140,71 @@ describe("codex-mutate", () => { }), (e) => e.code === "EBADSCOPE" ); + assert.equal(fs.existsSync(target), false); }); - it("rejects a type outside TYPES", () => { + it("rejects a type outside TYPES, and writes nothing", () => { + const target = path.join(TMP, "commands", "x.md"); assert.throws( () => cm.writeArtifact({ scope: "user", type: "commands", name: "x", content: "x", cwd: REPO }), (e) => e.code === "EBADTYPE" ); + assert.equal(fs.existsSync(target), false); + }); + + it("refuses a symlinked skill dir that escapes both Codex roots, and writes nothing outside", () => { + const outside = fs.mkdtempSync(path.join(os.tmpdir(), "codex-mut-outside-")); + const linkPath = path.join(TMP, "skills", "escape-link"); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + fs.symlinkSync(outside, linkPath, "dir"); + + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "skills", + name: "escape-link", + content: "pwned", + cwd: REPO, + }), + (e) => e.code === "EOUTOFROOT" + ); + assert.equal(fs.existsSync(path.join(outside, "SKILL.md")), false); + fs.rmSync(outside, { recursive: true, force: true }); + }); + + it("follows a symlinked skill dir that points back inside the same root", () => { + const realDir = path.join(TMP, "skills", "real-dir"); + fs.mkdirSync(realDir, { recursive: true }); + const linkPath = path.join(TMP, "skills", "inside-link"); + fs.symlinkSync(realDir, linkPath, "dir"); + + const { file, created } = cm.writeArtifact({ + scope: "user", + type: "skills", + name: "inside-link", + content: "fine", + cwd: REPO, + }); + assert.equal(created, true); + assert.equal(fs.readFileSync(file, "utf8"), "fine"); + assert.equal(fs.readFileSync(path.join(realDir, "SKILL.md"), "utf8"), "fine"); + }); + + it("refuses to back up a symlinked artifact that escapes the roots when deleting it", () => { + const outside = fs.mkdtempSync(path.join(os.tmpdir(), "codex-mut-outside-")); + fs.writeFileSync(path.join(outside, "secret.rules"), "top secret"); + const linkPath = path.join(TMP, "rules", "escape-del.rules"); + fs.mkdirSync(path.dirname(linkPath), { recursive: true }); + fs.symlinkSync(path.join(outside, "secret.rules"), linkPath, "file"); + + assert.throws( + () => cm.deleteArtifact({ scope: "user", type: "rules", name: "escape-del", cwd: REPO }), + (e) => e.code === "EOUTOFROOT" + ); + assert.equal(fs.existsSync(path.join(outside, "secret.rules")), true); + fs.rmSync(outside, { recursive: true, force: true }); }); for (const badName of ["..", ".", "/etc/passwd", "a/b", "a\0b"]) { @@ -167,7 +225,8 @@ describe("codex-mutate", () => { }); } - it("rejects a name passed to memory", () => { + it("rejects a name passed to memory, and writes nothing", () => { + const target = path.join(TMP, "AGENTS.md"); assert.throws( () => cm.writeArtifact({ @@ -179,9 +238,12 @@ describe("codex-mutate", () => { }), (e) => e.code === "EBADNAME" ); + assert.equal(fs.existsSync(target), false); }); - it("rejects a name passed to config", () => { + it("rejects a name passed to config, and leaves any existing config.toml untouched", () => { + const target = path.join(TMP, "config.toml"); + const before = fs.existsSync(target) ? fs.readFileSync(target, "utf8") : null; assert.throws( () => cm.writeArtifact({ @@ -193,6 +255,8 @@ describe("codex-mutate", () => { }), (e) => e.code === "EBADNAME" ); + const after = fs.existsSync(target) ? fs.readFileSync(target, "utf8") : null; + assert.equal(after, before); }); it("rejects content over the size limit and leaves the existing file untouched", () => { @@ -234,4 +298,18 @@ describe("codex-mutate", () => { assert.equal(rows[0].type, "rules"); assert.equal(rows[0].name.startsWith("listed.rules."), true); }); + + it("refuses a listBackups scope that is neither user nor project", () => { + assert.throws( + () => cm.listBackups({ scope: "../../../../tmp" }), + (e) => e.code === "EBADSCOPE" + ); + }); + + it("refuses a listBackups type that tries to traverse outside the backups root", () => { + assert.throws( + () => cm.listBackups({ scope: "user", type: "../../../../tmp" }), + (e) => e.code === "EBADTYPE" + ); + }); }); diff --git a/server/lib/codex-mutate.js b/server/lib/codex-mutate.js index cc95e691..e4d96795 100644 --- a/server/lib/codex-mutate.js +++ b/server/lib/codex-mutate.js @@ -18,7 +18,11 @@ * - Writes are atomic via temp file + fs.renameSync (see config-write.js). * - Names are validated against a strict allowlist regex; resolved paths * are double-checked to live under the expected containment root before - * any I/O. + * any I/O. That string-only check does not see through a symlink, so + * any existing target is additionally realpath-resolved and required to + * land inside a real Codex root (see `assertNotSymlinkEscape`) — a + * symlink that merely reorganises files within the roots (a supported + * install pattern) keeps working; one pointing outside is refused. * - Codex has no `project-local` scope and no auto-memory: only "user" and * "project" are valid scopes. * @author Nguyễn Ngọc Trí Vĩ @@ -27,7 +31,7 @@ const fs = require("node:fs"); const path = require("node:path"); const { getCodexHome, getProjectCodexDir, getProjectRoot } = require("./codex-home"); -const { MAX_FILE_BYTES } = require("./cc-discovery"); +const { MAX_FILE_BYTES, isUnder } = require("./cc-discovery"); const { createBackup, atomicWriteFile, assertUnder } = require("./config-write"); const NAME_RE = /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/; @@ -118,6 +122,36 @@ function rmTreeSync(p) { fs.rmSync(p, { recursive: true, force: true }); } +/** realpath if it exists, else the plain resolved path — for roots that may not exist yet. */ +function safeRealpath(p) { + try { + return fs.realpathSync(p); + } catch { + return path.resolve(p); + } +} + +/** + * Symlinks are a supported install pattern (Task 3's reader follows them), + * so a symlink at `target` is not rejected outright — only one that + * dereferences outside both Codex roots. A missing/dangling symlink is a + * fresh create, not an error. Must run before anything reads through + * `target` (backup or write), since string-only containment checks never + * see through a link. + */ +function assertNotSymlinkEscape(target, cwd) { + let real; + try { + real = fs.realpathSync(target); + } catch { + return; + } + const roots = [safeRealpath(getCodexHome()), safeRealpath(getProjectCodexDir(cwd))]; + if (!roots.some((root) => isUnder(root, real))) { + throw makeError("EOUTOFROOT", `resolved path escapes the Codex roots: ${real}`); + } +} + // ── Public API ───────────────────────────────────────────────────────── /** @@ -135,6 +169,7 @@ function writeArtifact(args) { const r = resolveTarget(scope, type, name, { cwd }); assertUnder(r.containmentRoot, r.target); + assertNotSymlinkEscape(r.target, cwd); const existedBefore = fs.existsSync(r.filePath); const backup = existedBefore @@ -166,6 +201,7 @@ function deleteArtifact(args) { if (!fs.existsSync(r.target)) { throw makeError("ENOTFOUND", `${type}/${name || TYPES[type].file} does not exist`); } + assertNotSymlinkEscape(r.target, cwd); const backup = createBackup({ source: r.target, @@ -188,9 +224,14 @@ function deleteArtifact(args) { * Returns [{ type, name, file, mtime, size }]. */ function listBackups(opts = {}) { - const out = []; const scopes = opts.scope ? [opts.scope] : ["user", "project"]; const types = opts.type ? [opts.type] : Object.keys(TYPES); + for (const scope of scopes) assertScope(scope); + for (const type of types) { + if (!TYPES[type]) throw makeError("EBADTYPE", `unknown type: ${type}`); + } + + const out = []; for (const scope of scopes) { for (const type of types) { const root = backupRootFor(scope, type, opts); From 58623f37c0b8d72b76b93e7ade8beaebf5b778d4 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 17:43:09 +0700 Subject: [PATCH 11/27] fix(codex): close parent-dir symlink escape in the containment check --- server/__tests__/codex-mutate.test.js | 59 +++++++++++++++ server/lib/codex-mutate.js | 103 +++++++++++++++++--------- 2 files changed, 127 insertions(+), 35 deletions(-) diff --git a/server/__tests__/codex-mutate.test.js b/server/__tests__/codex-mutate.test.js index 291aa7ad..f4f7438f 100644 --- a/server/__tests__/codex-mutate.test.js +++ b/server/__tests__/codex-mutate.test.js @@ -207,6 +207,65 @@ describe("codex-mutate", () => { fs.rmSync(outside, { recursive: true, force: true }); }); + it("refuses a brand-new artifact whose subdir (not the leaf) is a symlink escaping the root", () => { + const outside = fs.mkdtempSync(path.join(os.tmpdir(), "codex-mut-outside-")); + const linkPath = path.join(TMP, "agents"); + // "agents" doesn't exist yet in this fresh suite of tests for this scope — + // remove it first if an earlier test already created it as a real dir. + fs.rmSync(linkPath, { recursive: true, force: true }); + fs.symlinkSync(outside, linkPath, "dir"); + + assert.throws( + () => + cm.writeArtifact({ + scope: "user", + type: "agents", + name: "brand-new", + content: "pwned", + cwd: REPO, + }), + (e) => e.code === "EOUTOFROOT" + ); + assert.equal(fs.existsSync(path.join(outside, "brand-new.toml")), false); + fs.rmSync(linkPath, { force: true }); + fs.rmSync(outside, { recursive: true, force: true }); + }); + + it("still writes correctly when $CODEX_HOME itself is a symlink (a dotfiles checkout)", () => { + const realHome = fs.mkdtempSync(path.join(os.tmpdir(), "codex-mut-realhome-")); + const linkHome = fs.mkdtempSync(path.join(os.tmpdir(), "codex-mut-linkhome-")); + fs.rmdirSync(linkHome); // symlinkSync needs the link path itself to not exist + fs.symlinkSync(realHome, linkHome, "dir"); + + const prev = process.env.CODEX_HOME; + process.env.CODEX_HOME = linkHome; + delete require.cache[require.resolve("../lib/codex-home")]; + delete require.cache[require.resolve("../lib/codex-mutate")]; + const linkedCm = require("../lib/codex-mutate"); + try { + const { file, created } = linkedCm.writeArtifact({ + scope: "user", + type: "rules", + name: "via-link", + content: "ok", + cwd: REPO, + }); + assert.equal(created, true); + assert.equal(fs.readFileSync(file, "utf8"), "ok"); + assert.equal( + fs.readFileSync(path.join(realHome, "rules", "via-link.rules"), "utf8"), + "ok", + "the write must land in the real directory the symlinked CODEX_HOME points to" + ); + } finally { + process.env.CODEX_HOME = prev; + delete require.cache[require.resolve("../lib/codex-home")]; + delete require.cache[require.resolve("../lib/codex-mutate")]; + fs.rmSync(linkHome, { force: true }); + fs.rmSync(realHome, { recursive: true, force: true }); + } + }); + for (const badName of ["..", ".", "/etc/passwd", "a/b", "a\0b"]) { it(`rejects the bad name ${JSON.stringify(badName)} with EBADNAME and writes nothing`, () => { const target = path.join(TMP, "agents", `${badName.replace(/[/\0]/g, "_")}.toml`); diff --git a/server/lib/codex-mutate.js b/server/lib/codex-mutate.js index e4d96795..29b644f6 100644 --- a/server/lib/codex-mutate.js +++ b/server/lib/codex-mutate.js @@ -18,11 +18,14 @@ * - Writes are atomic via temp file + fs.renameSync (see config-write.js). * - Names are validated against a strict allowlist regex; resolved paths * are double-checked to live under the expected containment root before - * any I/O. That string-only check does not see through a symlink, so - * any existing target is additionally realpath-resolved and required to - * land inside a real Codex root (see `assertNotSymlinkEscape`) — a - * symlink that merely reorganises files within the roots (a supported - * install pattern) keeps working; one pointing outside is refused. + * any I/O. That string-only check does not see through a symlink placed + * anywhere along the path — including the type's own subdir (e.g. + * `/skills`) — so the nearest existing ancestor of the target is + * additionally realpath-resolved and required to land inside the + * realpath'd scope root (see `assertNotSymlinkEscape`). A symlink that + * merely reorganises files within the root (a supported install + * pattern, and a symlinked CODEX_HOME itself) keeps working; one + * pointing outside is refused. * - Codex has no `project-local` scope and no auto-memory: only "user" and * "project" are valid scopes. * @author Nguyễn Ngọc Trí Vĩ @@ -75,10 +78,17 @@ function fixedPathFor(type, scope, opts = {}) { } /** - * Resolve the on-disk target for a (scope, type, name) tuple AND the - * containment root used for path-traversal checks. + * Resolve the on-disk target for a (scope, type, name) tuple, the + * containment root used for string-only path-traversal checks, and the + * trusted root used for the realpath-based symlink-escape check. * - * Returns: { kind: "file" | "dir", target, filePath, containmentRoot } + * `trustedRoot` is deliberately the OUTER root (CODEX_HOME / project .codex + * dir / project root), not `containmentRoot`: for skills/agents/rules, + * `containmentRoot` is `/` (e.g. `/skills`), and that + * subdir itself is exactly what an attacker could replace with a symlink — + * checking a target against its own compromised parent would always pass. + * + * Returns: { kind: "file" | "dir", target, filePath, containmentRoot, trustedRoot } */ function resolveTarget(scope, type, name, opts = {}) { const spec = TYPES[type]; @@ -90,14 +100,18 @@ function resolveTarget(scope, type, name, opts = {}) { throw makeError("EBADNAME", `${type} is a fixed file and takes no name`); } const filePath = fixedPathFor(type, scope, opts); - return { kind: "file", target: filePath, filePath, containmentRoot: path.dirname(filePath) }; + // No attacker-placeable segment between the trusted root and the fixed + // filename, so the containment root doubles as the trusted root. + const root = path.dirname(filePath); + return { kind: "file", target: filePath, filePath, containmentRoot: root, trustedRoot: root }; } if (typeof name !== "string" || !NAME_RE.test(name)) { throw makeError("EBADNAME", `name must match ${NAME_RE}`); } - const subdirAbs = path.join(rootForScope(scope, opts), spec.subdir); + const root = rootForScope(scope, opts); + const subdirAbs = path.join(root, spec.subdir); if (spec.kind === "dir") { const target = path.join(subdirAbs, name); @@ -106,11 +120,12 @@ function resolveTarget(scope, type, name, opts = {}) { target, filePath: path.join(target, spec.filename), containmentRoot: subdirAbs, + trustedRoot: root, }; } const target = path.join(subdirAbs, name + spec.ext); - return { kind: "file", target, filePath: target, containmentRoot: subdirAbs }; + return { kind: "file", target, filePath: target, containmentRoot: subdirAbs, trustedRoot: root }; } /** /codex-config-backups//, always rooted the same way as the read side's . */ @@ -122,33 +137,51 @@ function rmTreeSync(p) { fs.rmSync(p, { recursive: true, force: true }); } -/** realpath if it exists, else the plain resolved path — for roots that may not exist yet. */ -function safeRealpath(p) { - try { - return fs.realpathSync(p); - } catch { - return path.resolve(p); - } -} - /** * Symlinks are a supported install pattern (Task 3's reader follows them), - * so a symlink at `target` is not rejected outright — only one that - * dereferences outside both Codex roots. A missing/dangling symlink is a - * fresh create, not an error. Must run before anything reads through - * `target` (backup or write), since string-only containment checks never - * see through a link. + * so a symlink under `trustedRoot` is not rejected outright — only one that + * dereferences outside it. `trustedRoot` itself may be a symlink (a + * dotfiles-managed CODEX_HOME is a normal setup) — that's fine, it just + * means everything is compared against wherever it really points. + * + * `target` may not exist yet (a fresh create). Realpath can't resolve a + * path that isn't there, so this walks up to the nearest ancestor that DOES + * exist, realpath's that, and requires it to land inside the realpath'd + * root. Everything below that existing ancestor is plain path segments this + * module built itself from a NAME_RE-validated name or a fixed literal, so + * they can't smuggle in a symlink — only an EXISTING ancestor can. + * + * `trustedRoot` is created first (mkdirSync recursive — safe, it is never + * attacker-controlled) so the walk is guaranteed to bottom out at or below + * it, never wander past it into whatever happens to contain it. + * + * ponytail: this check and the write/delete that follows are not atomic — + * a symlink swapped in between them could still redirect one write. Closing + * that needs O_NOFOLLOW-style fd work with real platform variance; out of + * scope for this task (see task-5 fix round 2). */ -function assertNotSymlinkEscape(target, cwd) { +function assertNotSymlinkEscape(trustedRoot, target) { + fs.mkdirSync(trustedRoot, { recursive: true }); + const realRoot = fs.realpathSync(trustedRoot); + + let dir = target; let real; - try { - real = fs.realpathSync(target); - } catch { - return; + 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; + } } - const roots = [safeRealpath(getCodexHome()), safeRealpath(getProjectCodexDir(cwd))]; - if (!roots.some((root) => isUnder(root, real))) { - throw makeError("EOUTOFROOT", `resolved path escapes the Codex roots: ${real}`); + + if (!isUnder(realRoot, real)) { + throw makeError("EOUTOFROOT", `resolved path escapes the Codex root: ${real}`); } } @@ -169,7 +202,7 @@ function writeArtifact(args) { const r = resolveTarget(scope, type, name, { cwd }); assertUnder(r.containmentRoot, r.target); - assertNotSymlinkEscape(r.target, cwd); + assertNotSymlinkEscape(r.trustedRoot, r.target); const existedBefore = fs.existsSync(r.filePath); const backup = existedBefore @@ -201,7 +234,7 @@ function deleteArtifact(args) { if (!fs.existsSync(r.target)) { throw makeError("ENOTFOUND", `${type}/${name || TYPES[type].file} does not exist`); } - assertNotSymlinkEscape(r.target, cwd); + assertNotSymlinkEscape(r.trustedRoot, r.target); const backup = createBackup({ source: r.target, From 32839332f2ff5ab310bc8aa1c55eb25d9c9b41f1 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 17:54:36 +0700 Subject: [PATCH 12/27] feat(codex): expose the Codex config API --- openapi.yaml | 1276 +++++++++++++++++++++++++ server/__tests__/codex-config.test.js | 493 ++++++++++ server/index.js | 2 + server/openapi-extra.js | 2 + server/openapi-extra/codex-config.js | 744 ++++++++++++++ server/routes/codex-config.js | 366 +++++++ 6 files changed, 2883 insertions(+) create mode 100644 server/__tests__/codex-config.test.js create mode 100644 server/openapi-extra/codex-config.js create mode 100644 server/routes/codex-config.js diff --git a/openapi.yaml b/openapi.yaml index ae53000c..df8b830a 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -52,6 +52,8 @@ tags: description: OpenAPI/Swagger endpoints - name: CcConfig description: 'Claude Code configuration explorer: discover and (for low-risk artifacts) edit skills, agents, commands, output styles, MCP servers, hooks, settings, memory, plugins, marketplaces, keybindings, statusline' + - name: CodexConfig + description: 'Codex CLI configuration explorer: discover and (for low-risk artifacts) edit skills, agents, rules, memory (AGENTS.md), config.toml, MCP servers, plugins, and marketplaces.' - name: Run description: Spawn and supervise Claude Code runs from the dashboard (headless or interactive conversation), stream output over WebSocket, and manage run history - name: Lanes @@ -3005,6 +3007,339 @@ components: example: - /Users/son/.claude/.credentials.json - /Users/son/.claude.json + CodexConfigScope: + type: string + enum: + - user + - project + - all + description: Discovery scope. `user` = $CODEX_HOME (or ~/.codex); `project` = /.codex; `all` = both, merged. Defaults to `all` when omitted or unrecognized. + example: all + CodexConfigSkill: + type: object + description: 'A discovered skill: a directory containing a SKILL.md under /skills//.' + required: + - scope + - name + - path + - file + - size + - mtime + - truncated + - frontmatter + - preview + properties: + scope: + type: string + enum: + - user + - project + example: user + name: + type: string + example: code-reviewer + path: + type: string + example: /home/user/.codex/skills/code-reviewer + file: + type: string + example: /home/user/.codex/skills/code-reviewer/SKILL.md + size: + type: integer + example: 2048 + mtime: + type: number + example: 1718900000000 + truncated: + type: boolean + example: false + frontmatter: + type: object + additionalProperties: + type: string + example: + name: code-reviewer + description: Reviews diffs + preview: + type: string + example: Use this skill to review pull requests... + CodexConfigAgent: + type: object + description: A discovered TOML agent definition (/agents/.toml). + required: + - scope + - name + - file + - size + - mtime + - meta + - preview + properties: + scope: + type: string + enum: + - user + - project + example: user + name: + type: string + example: reviewer + file: + type: string + example: /home/user/.codex/agents/reviewer.toml + size: + type: integer + example: 512 + mtime: + type: number + example: 1718900000000 + meta: + type: object + additionalProperties: true + description: Best-effort top-level scalar fields (name, description, model, sandbox_mode). + example: + name: reviewer + model: gpt-5 + preview: + type: string + example: |- + name = "reviewer" + model = "gpt-5" + CodexConfigRule: + type: object + description: A discovered rules file (/rules/.rules). + required: + - scope + - name + - file + - size + - mtime + - preview + properties: + scope: + type: string + enum: + - user + - project + example: user + name: + type: string + example: default + file: + type: string + example: /home/user/.codex/rules/default.rules + size: + type: integer + example: 128 + mtime: + type: number + example: 1718900000000 + preview: + type: string + example: allow + CodexConfigMemoryEntry: + type: object + description: AGENTS.md at either the Codex home or the project root. + required: + - scope + - file + - exists + properties: + scope: + type: string + enum: + - user + - project + example: project + file: + type: string + example: /repo/AGENTS.md + exists: + type: boolean + example: true + size: + type: integer + example: 256 + mtime: + type: number + example: 1718900000000 + preview: + type: string + example: |- + # Project memory + Hello. + CodexConfigConfigEntry: + type: object + description: config.toml at either the Codex home or the project .codex dir. + required: + - scope + - file + - exists + properties: + scope: + type: string + enum: + - user + - project + example: user + file: + type: string + example: /home/user/.codex/config.toml + exists: + type: boolean + example: true + size: + type: integer + example: 640 + mtime: + type: number + example: 1718900000000 + content: + type: string + example: | + model = "gpt-5" + truncated: + type: boolean + example: false + CodexConfigOverviewResponse: + type: object + required: + - roots + - counts + properties: + roots: + type: object + properties: + codexHome: + type: string + example: /home/user/.codex + projectCodexDir: + type: string + example: /repo/.codex + projectRoot: + type: string + example: /repo + counts: + type: object + additionalProperties: + type: object + properties: + user: + type: integer + project: + type: integer + example: + skills: + user: 3 + project: 1 + agents: + user: 2 + project: 0 + rules: + user: 1 + project: 0 + memory: + user: 1 + project: 1 + config: + user: 1 + project: 0 + CodexConfigFileReadResponse: + type: object + required: + - ok + - file + - size + - mtime + - truncated + - text + properties: + ok: + type: boolean + example: true + file: + type: string + example: /home/user/.codex/rules/default.rules + size: + type: integer + example: 128 + mtime: + type: number + example: 1718900000000 + truncated: + type: boolean + example: false + text: + type: string + example: allow + CodexConfigFileWriteResponse: + type: object + required: + - file + - created + - backup + properties: + file: + type: string + example: /home/user/.codex/rules/default.rules + created: + type: boolean + example: true + backup: + type: object + nullable: true + additionalProperties: true + description: Backup metadata when an existing file was overwritten; null on first create. + CodexConfigFileDeleteResponse: + type: object + required: + - file + - backup + properties: + file: + type: string + example: /home/user/.codex/rules/default.rules + backup: + type: object + additionalProperties: true + CodexConfigBackupsResponse: + type: object + required: + - items + properties: + items: + type: array + items: + type: object + properties: + type: + type: string + example: rules + name: + type: string + example: default.rules.2026-06-25T12-00-00.000Z.bak + file: + type: string + mtime: + type: number + size: + type: integer + nullable: true + CodexConfigCliActionResponse: + type: object + description: Result of a successful mutating `codex` CLI call. + required: + - ok + - stdout + - stderr + properties: + ok: + type: boolean + example: true + stdout: + type: string + stderr: + type: string RunHandle: type: object description: Live, in-memory view of a spawned `claude` subprocess. Returned by GET /api/run (in `items`), POST /api/run (201), and GET /api/run/{id}. Backed by the spawner's handle map, which reaps each handle 5 minutes after the process exits — after that the run is only visible via GET /api/run/history. @@ -8426,6 +8761,947 @@ paths: application/json: schema: $ref: '#/components/schemas/ErrorResponse' + /api/codex-config/overview: + get: + tags: + - CodexConfig + summary: Codex config explorer overview + description: Read-only. Returns the three resolved filesystem roots plus per-scope counts across skills, agents, rules, memory, and config. + operationId: codexConfigGetOverview + parameters: + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + responses: + '200': + description: Overview of roots and counts. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigOverviewResponse' + /api/codex-config/skills: + get: + tags: + - CodexConfig + summary: List discovered skills + operationId: codexConfigGetSkills + parameters: + - name: scope + in: query + required: false + schema: + type: string + enum: + - user + - project + - all + default: all + description: 'Discovery scope: `user`, `project`, or `all` (default).' + example: all + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + responses: + '200': + description: List discovered skills. + content: + application/json: + schema: + type: object + properties: + skills: + type: array + items: + $ref: '#/components/schemas/CodexConfigSkill' + example: + skills: + - scope: user + name: code-reviewer + frontmatter: {} + preview: ... + /api/codex-config/agents: + get: + tags: + - CodexConfig + summary: List discovered agent definitions + operationId: codexConfigGetAgents + parameters: + - name: scope + in: query + required: false + schema: + type: string + enum: + - user + - project + - all + default: all + description: 'Discovery scope: `user`, `project`, or `all` (default).' + example: all + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + responses: + '200': + description: List discovered agent definitions. + content: + application/json: + schema: + type: object + properties: + agents: + type: array + items: + $ref: '#/components/schemas/CodexConfigAgent' + example: + agents: + - scope: user + name: reviewer + meta: + model: gpt-5 + /api/codex-config/rules: + get: + tags: + - CodexConfig + summary: List discovered rules files + operationId: codexConfigGetRules + parameters: + - name: scope + in: query + required: false + schema: + type: string + enum: + - user + - project + - all + default: all + description: 'Discovery scope: `user`, `project`, or `all` (default).' + example: all + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + responses: + '200': + description: List discovered rules files. + content: + application/json: + schema: + type: object + properties: + rules: + type: array + items: + $ref: '#/components/schemas/CodexConfigRule' + example: + rules: + - scope: user + name: default + preview: allow + /api/codex-config/memory: + get: + tags: + - CodexConfig + summary: Read AGENTS.md at both scopes + operationId: codexConfigGetMemory + parameters: + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + responses: + '200': + description: Read AGENTS.md at both scopes. + content: + application/json: + schema: + type: object + properties: + memory: + type: array + items: + $ref: '#/components/schemas/CodexConfigMemoryEntry' + example: + memory: + - scope: project + file: /repo/AGENTS.md + exists: true + /api/codex-config/config: + get: + tags: + - CodexConfig + summary: Read config.toml at both scopes + operationId: codexConfigGetConfig + parameters: + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + responses: + '200': + description: Read config.toml at both scopes. + content: + application/json: + schema: + type: object + properties: + config: + type: array + items: + $ref: '#/components/schemas/CodexConfigConfigEntry' + example: + config: + - scope: user + file: /home/user/.codex/config.toml + exists: true + /api/codex-config/file: + get: + tags: + - CodexConfig + summary: Read a single file body + description: 'Read-only. Returns the body of one file. Strict path containment: the resolved absolute path must live under CODEX_HOME, the project .codex dir, or (only for a file literally named AGENTS.md) the project root.' + operationId: codexConfigGetFile + parameters: + - name: path + in: query + required: true + schema: + type: string + example: /home/user/.codex/rules/default.rules + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + responses: + '200': + description: File body with metadata. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigFileReadResponse' + '400': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: READ_DENIED + message: path is outside allowed roots + put: + tags: + - CodexConfig + summary: Create or overwrite a text artifact + description: Writes skills/agents/rules/memory/config. `name` is required for skills/agents/rules and forbidden for memory/config (fixed filenames). Backs up any existing file first; broadcasts `codex_config_changed` on success. + operationId: codexConfigPutFile + parameters: + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - scope + - type + - content + properties: + scope: + type: string + enum: + - user + - project + type: + type: string + enum: + - skills + - agents + - rules + - memory + - config + name: + type: string + nullable: true + example: default + content: + type: string + example: + scope: user + type: rules + name: default + content: allow + responses: + '200': + description: File written. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigFileWriteResponse' + '400': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: EBADNAME + message: name must match ^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$ + '413': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: ETOOLARGE + message: content exceeds 262144 bytes + delete: + tags: + - CodexConfig + summary: Delete a text artifact + description: Deletes skills/agents/rules/memory/config. Backup is mandatory and runs first. Broadcasts `codex_config_changed` on success. + operationId: codexConfigDeleteFile + parameters: + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - scope + - type + properties: + scope: + type: string + enum: + - user + - project + type: + type: string + enum: + - skills + - agents + - rules + - memory + - config + name: + type: string + nullable: true + example: + scope: user + type: agents + name: reviewer + responses: + '200': + description: File deleted. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigFileDeleteResponse' + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: ENOTFOUND + message: agents/reviewer does not exist + /api/codex-config/backups: + get: + tags: + - CodexConfig + summary: List backups + description: Read-only. Lists timestamped backups, optionally filtered by scope and/or type. + operationId: codexConfigGetBackups + parameters: + - name: scope + in: query + required: false + schema: + type: string + enum: + - user + - project + - name: type + in: query + required: false + schema: + type: string + enum: + - skills + - agents + - rules + - memory + - config + - name: cwd + in: query + required: false + schema: + type: string + description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. + example: /repo + responses: + '200': + description: Backups list. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigBackupsResponse' + /api/codex-config/mcp: + get: + tags: + - CodexConfig + summary: List configured MCP servers + description: Reads live from `codex mcp list --json` — there is no on-disk reader for this surface. + operationId: codexConfigGetMcp + responses: + '200': + description: MCP servers as reported by the CLI. + content: + application/json: + schema: + type: object + properties: + mcp: {} + '422': + description: '`ECLIEXIT` — the `codex` CLI exited non-zero. stdout/stderr are preserved.' + content: + application/json: + schema: + type: object + properties: + error: + type: object + properties: + code: + type: string + message: + type: string + code: + type: integer + nullable: true + stdout: + type: string + stderr: + type: string + example: + error: + code: ECLIEXIT + message: marketplace "nope" is not registered + code: 1 + stdout: '' + stderr: | + ✘ marketplace "nope" is not registered + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: ECLINOTFOUND + message: the `codex` CLI was not found on PATH + /api/codex-config/plugins: + get: + tags: + - CodexConfig + summary: List installed plugins + description: Reads live from `codex plugin list --json`. + operationId: codexConfigGetPlugins + responses: + '200': + description: Plugins as reported by the CLI. + content: + application/json: + schema: + type: object + properties: + plugins: {} + '422': + description: '`ECLIEXIT` — the `codex` CLI exited non-zero. stdout/stderr are preserved.' + content: + application/json: + schema: + type: object + properties: + error: + type: object + properties: + code: + type: string + message: + type: string + code: + type: integer + nullable: true + stdout: + type: string + stderr: + type: string + example: + error: + code: ECLIEXIT + message: marketplace "nope" is not registered + code: 1 + stdout: '' + stderr: | + ✘ marketplace "nope" is not registered + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: ECLINOTFOUND + message: the `codex` CLI was not found on PATH + /api/codex-config/marketplaces: + get: + tags: + - CodexConfig + summary: List registered marketplaces + description: Reads live from `codex plugin marketplace list --json`. + operationId: codexConfigGetMarketplaces + responses: + '200': + description: Marketplaces as reported by the CLI. + content: + application/json: + schema: + type: object + properties: + marketplaces: {} + '422': + description: '`ECLIEXIT` — the `codex` CLI exited non-zero. stdout/stderr are preserved.' + content: + application/json: + schema: + type: object + properties: + error: + type: object + properties: + code: + type: string + message: + type: string + code: + type: integer + nullable: true + stdout: + type: string + stderr: + type: string + example: + error: + code: ECLIEXIT + message: marketplace "nope" is not registered + code: 1 + stdout: '' + stderr: | + ✘ marketplace "nope" is not registered + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: ECLINOTFOUND + message: the `codex` CLI was not found on PATH + /api/codex-config/mcp/add: + post: + tags: + - CodexConfig + summary: Add an MCP server + description: Runs `codex mcp add`. Accepts either a streamable-HTTP server (`url`, optional `bearerTokenEnvVar`) or a stdio server (`command`, `args`, optional `env`) — never both. + operationId: codexConfigPostMcpAdd + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - name + properties: + name: + type: string + url: + type: string + bearerTokenEnvVar: + type: string + command: + type: string + args: + type: array + items: + type: string + env: + type: object + additionalProperties: + type: string + example: + name: github + url: https://mcp.example.com/github + responses: + '200': + description: MCP server added. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigCliActionResponse' + '400': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: EBADARG + message: url must be a valid http(s) URL + '422': + description: '`ECLIEXIT` — the `codex` CLI exited non-zero. stdout/stderr are preserved.' + content: + application/json: + schema: + type: object + properties: + error: + type: object + properties: + code: + type: string + message: + type: string + code: + type: integer + nullable: true + stdout: + type: string + stderr: + type: string + example: + error: + code: ECLIEXIT + message: marketplace "nope" is not registered + code: 1 + stdout: '' + stderr: | + ✘ marketplace "nope" is not registered + /api/codex-config/mcp/remove: + post: + tags: + - CodexConfig + summary: Remove an MCP server + operationId: codexConfigPostMcpRemove + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - name + properties: + name: + type: string + example: + name: github + responses: + '200': + description: MCP server removed. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigCliActionResponse' + '422': + description: '`ECLIEXIT` — the `codex` CLI exited non-zero. stdout/stderr are preserved.' + content: + application/json: + schema: + type: object + properties: + error: + type: object + properties: + code: + type: string + message: + type: string + code: + type: integer + nullable: true + stdout: + type: string + stderr: + type: string + example: + error: + code: ECLIEXIT + message: marketplace "nope" is not registered + code: 1 + stdout: '' + stderr: | + ✘ marketplace "nope" is not registered + /api/codex-config/plugins/action: + post: + tags: + - CodexConfig + summary: Add or remove a plugin + description: Runs `codex plugin add ` or `codex plugin remove `. + operationId: codexConfigPostPluginsAction + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - spec + - action + properties: + spec: + type: string + action: + type: string + enum: + - add + - remove + example: + spec: demo@market + action: add + responses: + '200': + description: Plugin action applied. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigCliActionResponse' + '400': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: EBADACTION + message: action must be one of add, remove + '422': + description: '`ECLIEXIT` — the `codex` CLI exited non-zero. stdout/stderr are preserved.' + content: + application/json: + schema: + type: object + properties: + error: + type: object + properties: + code: + type: string + message: + type: string + code: + type: integer + nullable: true + stdout: + type: string + stderr: + type: string + example: + error: + code: ECLIEXIT + message: marketplace "nope" is not registered + code: 1 + stdout: '' + stderr: | + ✘ marketplace "nope" is not registered + /api/codex-config/marketplaces/add: + post: + tags: + - CodexConfig + summary: Register a marketplace + description: Runs `codex plugin marketplace add `. + operationId: codexConfigPostMarketplacesAdd + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - url + properties: + url: + type: string + example: + url: https://example.com/marketplace.json + responses: + '200': + description: Marketplace added. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigCliActionResponse' + '400': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: EBADARG + message: marketplace url must be a valid http(s) URL + '422': + description: '`ECLIEXIT` — the `codex` CLI exited non-zero. stdout/stderr are preserved.' + content: + application/json: + schema: + type: object + properties: + error: + type: object + properties: + code: + type: string + message: + type: string + code: + type: integer + nullable: true + stdout: + type: string + stderr: + type: string + example: + error: + code: ECLIEXIT + message: marketplace "nope" is not registered + code: 1 + stdout: '' + stderr: | + ✘ marketplace "nope" is not registered + /api/codex-config/marketplaces/action: + post: + tags: + - CodexConfig + summary: Remove a marketplace + description: Runs `codex plugin marketplace remove `. + operationId: codexConfigPostMarketplacesAction + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - name + - action + properties: + name: + type: string + action: + type: string + enum: + - remove + example: + name: demo-mp + action: remove + responses: + '200': + description: Marketplace action applied. + content: + application/json: + schema: + $ref: '#/components/schemas/CodexConfigCliActionResponse' + '400': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: EBADACTION + message: action must be one of remove + '422': + description: '`ECLIEXIT` — the `codex` CLI exited non-zero. stdout/stderr are preserved.' + content: + application/json: + schema: + type: object + properties: + error: + type: object + properties: + code: + type: string + message: + type: string + code: + type: integer + nullable: true + stdout: + type: string + stderr: + type: string + example: + error: + code: ECLIEXIT + message: marketplace "nope" is not registered + code: 1 + stdout: '' + stderr: | + ✘ marketplace "nope" is not registered + '503': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: ECLINOTFOUND + message: the `codex` CLI was not found on PATH /api/run: get: tags: diff --git a/server/__tests__/codex-config.test.js b/server/__tests__/codex-config.test.js new file mode 100644 index 00000000..797507d7 --- /dev/null +++ b/server/__tests__/codex-config.test.js @@ -0,0 +1,493 @@ +/** + * @file codex-config.test.js + * @description Tests for /api/codex-config — the Codex CLI configuration + * explorer. Builds a fake CODEX_HOME and project .codex/ in tmpdir, points + * the server at it, and exercises every read/write surface plus + * path-containment guards, backups, and the CLI-backed MCP/plugin/ + * marketplace routes (via a stubbed `codex` binary on PATH). + * @author Nguyễn Ngọc Trí Vĩ + */ + +const { describe, it, before, 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 http = require("node:http"); +const { WebSocket } = require("ws"); + +const TMP = fs.mkdtempSync(path.join(os.tmpdir(), "codex-config-test-")); +const FAKE_HOME = path.join(TMP, "home", ".codex"); +const FAKE_PROJECT = path.join(TMP, "project"); +const FAKE_PROJECT_CODEX = path.join(FAKE_PROJECT, ".codex"); + +fs.mkdirSync(path.join(FAKE_HOME, "skills", "demo-skill"), { recursive: true }); +fs.mkdirSync(path.join(FAKE_HOME, "agents"), { recursive: true }); +fs.mkdirSync(path.join(FAKE_HOME, "rules"), { recursive: true }); +fs.mkdirSync(path.join(FAKE_PROJECT_CODEX, "skills", "proj-skill"), { recursive: true }); + +fs.writeFileSync( + path.join(FAKE_HOME, "skills", "demo-skill", "SKILL.md"), + `---\nname: demo-skill\ndescription: A demo skill for tests\n---\n\nBody text here.` +); +fs.writeFileSync( + path.join(FAKE_PROJECT_CODEX, "skills", "proj-skill", "SKILL.md"), + `---\nname: proj-skill\n---\n\nProject skill.` +); +fs.writeFileSync( + path.join(FAKE_HOME, "agents", "demo-agent.toml"), + `name = "demo-agent"\nmodel = "gpt-5"\n` +); +fs.writeFileSync(path.join(FAKE_HOME, "rules", "default.rules"), "allow"); +fs.writeFileSync(path.join(FAKE_HOME, "config.toml"), 'model = "gpt-5"\n'); +fs.writeFileSync(path.join(FAKE_PROJECT, "AGENTS.md"), "# Project memory\nHello."); + +process.env.CODEX_HOME = FAKE_HOME; +const TEST_DB = path.join(TMP, "dashboard-test.db"); +process.env.DASHBOARD_DB_PATH = TEST_DB; +process.env.DASHBOARD_REMOTE_SYNC_MS = "0"; +process.env.DASHBOARD_LIVENESS_PROBE = "0"; + +const { createApp, startServer } = require("../index"); + +let server; +let BASE; + +function fetchJson(p, opts = {}) { + return new Promise((resolve, reject) => { + const url = new URL(p, BASE); + const headers = { ...(opts.headers || {}) }; + let bodyBuf; + if (opts.body !== undefined) { + bodyBuf = Buffer.from(JSON.stringify(opts.body)); + headers["Content-Type"] = "application/json"; + headers["Content-Length"] = bodyBuf.length; + } + const req = http.request( + { + hostname: url.hostname, + port: url.port, + path: url.pathname + url.search, + method: opts.method || "GET", + headers, + }, + (res) => { + const chunks = []; + res.on("data", (c) => chunks.push(c)); + res.on("end", () => { + const body = Buffer.concat(chunks).toString("utf8"); + let json; + try { + json = JSON.parse(body); + } catch { + json = body; + } + resolve({ status: res.statusCode, body: json }); + }); + } + ); + req.on("error", reject); + if (bodyBuf) req.write(bodyBuf); + req.end(); + }); +} + +function stubCodex(scriptBody) { + const bin = path.join(TMP, `bin-${process.hrtime.bigint()}`); + fs.mkdirSync(bin, { recursive: true }); + const file = path.join(bin, "codex"); + fs.writeFileSync(file, `#!/usr/bin/env bash\n${scriptBody}\n`); + fs.chmodSync(file, 0o755); + const originalPath = process.env.PATH; + process.env.PATH = `${bin}${path.delimiter}${originalPath}`; + return () => { + process.env.PATH = originalPath; + }; +} + +const ECHO_ARGV = 'for a in "$@"; do echo "$a"; done'; + +describe("/api/codex-config", () => { + before(async () => { + const app = createApp(); + server = await startServer(app, 0); + BASE = `http://127.0.0.1:${server.address().port}`; + }); + + after(async () => { + await new Promise((r) => server.close(r)); + try { + fs.rmSync(TMP, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } catch { + /* best-effort temp cleanup */ + } + }); + + it("overview reports counts and roots", async () => { + const { status, body } = await fetchJson( + `/api/codex-config/overview?cwd=${encodeURIComponent(FAKE_PROJECT)}` + ); + assert.equal(status, 200); + assert.equal(body.roots.codexHome, FAKE_HOME); + assert.equal(body.roots.projectCodexDir, FAKE_PROJECT_CODEX); + assert.equal(body.roots.projectRoot, FAKE_PROJECT); + assert.equal(body.counts.skills.user, 1); + assert.equal(body.counts.skills.project, 1); + assert.equal(body.counts.agents.user, 1); + assert.equal(body.counts.rules.user, 1); + }); + + it("lists skills for both scopes", async () => { + const { status, body } = await fetchJson( + `/api/codex-config/skills?cwd=${encodeURIComponent(FAKE_PROJECT)}` + ); + assert.equal(status, 200); + assert.ok(Array.isArray(body.skills)); + const demo = body.skills.find((s) => s.name === "demo-skill"); + assert.equal(demo.scope, "user"); + assert.equal(demo.frontmatter.name, "demo-skill"); + const proj = body.skills.find((s) => s.name === "proj-skill"); + assert.equal(proj.scope, "project"); + }); + + it("scope=user filters out project items", async () => { + const { body } = await fetchJson( + `/api/codex-config/skills?scope=user&cwd=${encodeURIComponent(FAKE_PROJECT)}` + ); + assert.ok(body.skills.every((s) => s.scope === "user")); + }); + + it("agents parses top-level TOML scalars", async () => { + const { body } = await fetchJson( + `/api/codex-config/agents?cwd=${encodeURIComponent(FAKE_PROJECT)}` + ); + const a = body.agents.find((x) => x.name === "demo-agent"); + assert.equal(a.meta.model, "gpt-5"); + }); + + it("rules returns preview text", async () => { + const { body } = await fetchJson( + `/api/codex-config/rules?cwd=${encodeURIComponent(FAKE_PROJECT)}` + ); + const r = body.rules.find((x) => x.name === "default"); + assert.match(r.preview, /allow/); + }); + + it("memory returns project AGENTS.md", async () => { + const { body } = await fetchJson( + `/api/codex-config/memory?cwd=${encodeURIComponent(FAKE_PROJECT)}` + ); + const proj = body.memory.find((x) => x.scope === "project"); + assert.equal(proj.exists, true); + assert.match(proj.preview, /Project memory/); + }); + + it("config returns user config.toml content", async () => { + const { body } = await fetchJson( + `/api/codex-config/config?cwd=${encodeURIComponent(FAKE_PROJECT)}` + ); + const user = body.config.find((x) => x.scope === "user"); + assert.equal(user.exists, true); + assert.match(user.content, /gpt-5/); + }); + + it("file endpoint reads inside CODEX_HOME", async () => { + const target = path.join(FAKE_HOME, "rules", "default.rules"); + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}&path=${encodeURIComponent(target)}` + ); + assert.equal(status, 200); + assert.equal(body.ok, true); + assert.equal(body.text, "allow"); + }); + + it("file endpoint blocks paths outside allowed roots", async () => { + const outside = path.join(TMP, "evil.rules"); + fs.writeFileSync(outside, "secret"); + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}&path=${encodeURIComponent(outside)}` + ); + assert.equal(status, 400); + assert.equal(body.error.code, "READ_DENIED"); + }); + + it("file endpoint requires a path", async () => { + const { status, body } = await fetchJson("/api/codex-config/file"); + assert.equal(status, 400); + assert.equal(body.error.code, "BAD_PATH"); + }); + + it("PUT /file creates a new rule (no backup, file did not exist)", async () => { + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`, + { + method: "PUT", + body: { scope: "user", type: "rules", name: "fresh", content: "deny" }, + } + ); + assert.equal(status, 200); + assert.equal(body.created, true); + assert.equal(body.backup, null); + assert.equal(fs.readFileSync(body.file, "utf8"), "deny"); + }); + + it("PUT /file overwrites an existing rule and creates a backup, and broadcasts codex_config_changed", async () => { + const wsUrl = `ws://127.0.0.1:${server.address().port}/ws`; + const ws = new WebSocket(wsUrl); + const messages = []; + await new Promise((resolve, reject) => { + ws.on("open", resolve); + ws.on("error", reject); + }); + ws.on("message", (msg) => { + try { + messages.push(JSON.parse(msg)); + } catch { + /* parse error */ + } + }); + + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`, + { + method: "PUT", + body: { scope: "user", type: "rules", name: "default", content: "deny-all" }, + } + ); + assert.equal(status, 200); + assert.equal(body.created, false); + assert.ok(body.backup, "backup metadata returned"); + assert.match(fs.readFileSync(body.file, "utf8"), /deny-all/); + + await new Promise((resolve) => setTimeout(resolve, 100)); + ws.close(); + assert.ok( + messages.some((m) => m.type === "codex_config_changed"), + "PUT /file should broadcast codex_config_changed" + ); + }); + + it("PUT /file rejects a write whose name escapes the root with 400, not 500", async () => { + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`, + { + method: "PUT", + body: { scope: "user", type: "agents", name: "../evil", content: "x" }, + } + ); + assert.equal(status, 400); + assert.equal(body.error.code, "EBADNAME"); + }); + + it("PUT /file rejects unknown type", async () => { + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`, + { + method: "PUT", + body: { scope: "user", type: "plugins", name: "x", content: "y" }, + } + ); + assert.equal(status, 400); + assert.equal(body.error.code, "EBADTYPE"); + }); + + it("PUT /file rejects a name on a fixed-filename type (memory)", async () => { + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`, + { + method: "PUT", + body: { scope: "project", type: "memory", name: "nope", content: "x" }, + } + ); + assert.equal(status, 400); + assert.equal(body.error.code, "EBADNAME"); + }); + + it("PUT /file rejects oversize content", async () => { + const huge = "x".repeat(256 * 1024 + 1); + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`, + { + method: "PUT", + body: { scope: "user", type: "rules", name: "huge", content: huge }, + } + ); + assert.equal(status, 413); + assert.equal(body.error.code, "ETOOLARGE"); + }); + + it("DELETE /file backs up and removes a rule", async () => { + fs.writeFileSync(path.join(FAKE_HOME, "rules", "to-delete.rules"), "bye"); + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`, + { + method: "DELETE", + body: { scope: "user", type: "rules", name: "to-delete" }, + } + ); + assert.equal(status, 200); + assert.ok(body.backup); + assert.equal(fs.existsSync(path.join(FAKE_HOME, "rules", "to-delete.rules")), false); + }); + + it("DELETE /file 404s on a missing item", async () => { + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`, + { + method: "DELETE", + body: { scope: "user", type: "rules", name: "never-existed" }, + } + ); + assert.equal(status, 404); + assert.equal(body.error.code, "ENOTFOUND"); + }); + + it("backups endpoint lists everything we just created", async () => { + const { status, body } = await fetchJson( + `/api/codex-config/backups?cwd=${encodeURIComponent(FAKE_PROJECT)}` + ); + assert.equal(status, 200); + assert.ok(Array.isArray(body.items)); + assert.ok(body.items.length >= 2, `expected >=2 backups, got ${body.items.length}`); + }); + + // ── CLI-backed routes (fake `codex` on PATH) ──────────────────────── + + it("GET /mcp returns the CLI's JSON output", async () => { + const restore = stubCodex(`cat <<'JSON'\n{"servers":[{"name":"docs"}]}\nJSON`); + try { + const { status, body } = await fetchJson("/api/codex-config/mcp"); + assert.equal(status, 200); + assert.deepEqual(body.mcp, { servers: [{ name: "docs" }] }); + } finally { + restore(); + } + }); + + it("POST /mcp/add runs the CLI with the given argv", async () => { + const restore = stubCodex(ECHO_ARGV); + try { + const { status, body } = await fetchJson("/api/codex-config/mcp/add", { + method: "POST", + body: { name: "docs", url: "https://example.com/mcp" }, + }); + assert.equal(status, 200); + assert.equal(body.ok, true); + assert.deepEqual(body.stdout.trim().split("\n"), [ + "mcp", + "add", + "docs", + "--url", + "https://example.com/mcp", + ]); + } finally { + restore(); + } + }); + + it("POST /mcp/remove runs the CLI", async () => { + const restore = stubCodex(ECHO_ARGV); + try { + const { status, body } = await fetchJson("/api/codex-config/mcp/remove", { + method: "POST", + body: { name: "docs" }, + }); + assert.equal(status, 200); + assert.deepEqual(body.stdout.trim().split("\n"), ["mcp", "remove", "docs"]); + } finally { + restore(); + } + }); + + it("POST /plugins/action validates the action name", async () => { + const { status, body } = await fetchJson("/api/codex-config/plugins/action", { + method: "POST", + body: { spec: "demo", action: "purge" }, + }); + assert.equal(status, 400); + assert.equal(body.error.code, "EBADACTION"); + }); + + it("POST /plugins/action adds through the CLI", async () => { + const restore = stubCodex(ECHO_ARGV); + try { + const { status, body } = await fetchJson("/api/codex-config/plugins/action", { + method: "POST", + body: { spec: "demo@market", action: "add" }, + }); + assert.equal(status, 200); + assert.deepEqual(body.stdout.trim().split("\n"), ["plugin", "add", "demo@market"]); + } finally { + restore(); + } + }); + + it("POST /marketplaces/add runs the CLI with the pasted url", async () => { + const restore = stubCodex(ECHO_ARGV); + try { + const { status, body } = await fetchJson("/api/codex-config/marketplaces/add", { + method: "POST", + body: { url: "https://example.com/mp.git" }, + }); + assert.equal(status, 200); + assert.deepEqual(body.stdout.trim().split("\n"), [ + "plugin", + "marketplace", + "add", + "https://example.com/mp.git", + ]); + } finally { + restore(); + } + }); + + it("POST /marketplaces/add rejects a non-http(s) url", async () => { + const { status, body } = await fetchJson("/api/codex-config/marketplaces/add", { + method: "POST", + body: { url: "not a url" }, + }); + assert.equal(status, 400); + assert.equal(body.error.code, "EBADARG"); + }); + + it("reports a failed CLI call as 422 and keeps the stderr", async () => { + const restore = stubCodex('echo "✘ marketplace \\"nope\\" is not registered" >&2\nexit 1'); + try { + const { status, body } = await fetchJson("/api/codex-config/marketplaces/action", { + method: "POST", + body: { name: "nope", action: "remove" }, + }); + assert.equal(status, 422); + assert.equal(body.error.code, "ECLIEXIT"); + assert.match(body.error.message, /not registered/); + } finally { + restore(); + } + }); + + it("POST /marketplaces/action validates the action name", async () => { + const { status, body } = await fetchJson("/api/codex-config/marketplaces/action", { + method: "POST", + body: { name: "demo-mp", action: "update" }, + }); + assert.equal(status, 400); + assert.equal(body.error.code, "EBADACTION"); + }); + + it("reports a missing `codex` binary as 503 rather than a crash", async () => { + const empty = path.join(TMP, `empty-${process.hrtime.bigint()}`); + fs.mkdirSync(empty, { recursive: true }); + const originalPath = process.env.PATH; + process.env.PATH = empty; + try { + const { status, body } = await fetchJson("/api/codex-config/marketplaces/action", { + method: "POST", + body: { name: "demo-mp", action: "remove" }, + }); + assert.equal(status, 503); + assert.equal(body.error.code, "ECLINOTFOUND"); + } finally { + process.env.PATH = originalPath; + } + }); +}); diff --git a/server/index.js b/server/index.js index 6066e2c9..77898157 100644 --- a/server/index.js +++ b/server/index.js @@ -72,6 +72,7 @@ const pushRouter = require("./routes/push"); const importRouter = require("./routes/import"); const updatesRouter = require("./routes/updates"); const ccConfigRouter = require("./routes/cc-config"); +const codexConfigRouter = require("./routes/codex-config"); const runRouter = require("./routes/run"); const alertsRouter = require("./routes/alerts"); const webhooksRouter = require("./routes/webhooks"); @@ -113,6 +114,7 @@ function createApp() { app.use("/api/import", importRouter); app.use("/api/updates", updatesRouter); app.use("/api/cc-config", ccConfigRouter); + app.use("/api/codex-config", codexConfigRouter); app.use("/api/run", runRouter); app.use("/api/lanes", lanesRouter); app.use("/api/locks", locksRouter); diff --git a/server/openapi-extra.js b/server/openapi-extra.js index a0d31a58..f016701a 100644 --- a/server/openapi-extra.js +++ b/server/openapi-extra.js @@ -11,6 +11,7 @@ // New endpoint groups (previously-undocumented gaps in the base spec). const ccConfig = require("./openapi-extra/cc-config"); +const codexConfig = require("./openapi-extra/codex-config"); const run = require("./openapi-extra/run"); const lanes = require("./openapi-extra/lanes"); const push = require("./openapi-extra/push"); @@ -44,6 +45,7 @@ function combine(...fragments) { module.exports = combine( ccConfig, + codexConfig, run, lanes, push, diff --git a/server/openapi-extra/codex-config.js b/server/openapi-extra/codex-config.js new file mode 100644 index 00000000..55618932 --- /dev/null +++ b/server/openapi-extra/codex-config.js @@ -0,0 +1,744 @@ +/** + * @file OpenAPI 3.0 fragment for the Codex Config router + * (`server/routes/codex-config.js`, mounted at `/api/codex-config`). + * Documents the read surfaces (skills, agents, rules, memory, config, plus + * MCP servers/plugins/marketplaces read live from the `codex` CLI), the + * low-risk mutation surface (PUT/DELETE /file), the backups listing, and the + * CLI-backed MCP/plugin/marketplace mutation endpoints. + * + * Mirrors `server/openapi-extra/cc-config.js`'s structure and level of + * detail is intentionally lighter — one shared response shape per surface + * kind rather than a bespoke schema per field — since this panel has far + * fewer fields per record. All schema names are prefixed with `CodexConfig`. + * The shared `{ error: { code, message } }` envelope reuses the base + * `ErrorResponse` schema (defined in server/openapi.js). + * + * @author Nguyễn Ngọc Trí Vĩ + */ + +const tags = [ + { + name: "CodexConfig", + description: + "Codex CLI configuration explorer: discover and (for low-risk artifacts) edit skills, agents, rules, memory (AGENTS.md), config.toml, MCP servers, plugins, and marketplaces.", + }, +]; + +// ── Shared sub-schemas ───────────────────────────────────────────────── + +const schemas = { + CodexConfigScope: { + type: "string", + enum: ["user", "project", "all"], + description: + "Discovery scope. `user` = $CODEX_HOME (or ~/.codex); `project` = /.codex; `all` = both, merged. Defaults to `all` when omitted or unrecognized.", + example: "all", + }, + + CodexConfigSkill: { + type: "object", + description: + "A discovered skill: a directory containing a SKILL.md under /skills//.", + required: [ + "scope", + "name", + "path", + "file", + "size", + "mtime", + "truncated", + "frontmatter", + "preview", + ], + properties: { + scope: { type: "string", enum: ["user", "project"], example: "user" }, + name: { type: "string", example: "code-reviewer" }, + path: { type: "string", example: "/home/user/.codex/skills/code-reviewer" }, + file: { type: "string", example: "/home/user/.codex/skills/code-reviewer/SKILL.md" }, + size: { type: "integer", example: 2048 }, + mtime: { type: "number", example: 1718900000000 }, + truncated: { type: "boolean", example: false }, + frontmatter: { + type: "object", + additionalProperties: { type: "string" }, + example: { name: "code-reviewer", description: "Reviews diffs" }, + }, + preview: { type: "string", example: "Use this skill to review pull requests..." }, + }, + }, + + CodexConfigAgent: { + type: "object", + description: "A discovered TOML agent definition (/agents/.toml).", + required: ["scope", "name", "file", "size", "mtime", "meta", "preview"], + properties: { + scope: { type: "string", enum: ["user", "project"], example: "user" }, + name: { type: "string", example: "reviewer" }, + file: { type: "string", example: "/home/user/.codex/agents/reviewer.toml" }, + size: { type: "integer", example: 512 }, + mtime: { type: "number", example: 1718900000000 }, + meta: { + type: "object", + additionalProperties: true, + description: + "Best-effort top-level scalar fields (name, description, model, sandbox_mode).", + example: { name: "reviewer", model: "gpt-5" }, + }, + preview: { type: "string", example: 'name = "reviewer"\nmodel = "gpt-5"' }, + }, + }, + + CodexConfigRule: { + type: "object", + description: "A discovered rules file (/rules/.rules).", + required: ["scope", "name", "file", "size", "mtime", "preview"], + properties: { + scope: { type: "string", enum: ["user", "project"], example: "user" }, + name: { type: "string", example: "default" }, + file: { type: "string", example: "/home/user/.codex/rules/default.rules" }, + size: { type: "integer", example: 128 }, + mtime: { type: "number", example: 1718900000000 }, + preview: { type: "string", example: "allow" }, + }, + }, + + CodexConfigMemoryEntry: { + type: "object", + description: "AGENTS.md at either the Codex home or the project root.", + required: ["scope", "file", "exists"], + properties: { + scope: { type: "string", enum: ["user", "project"], example: "project" }, + file: { type: "string", example: "/repo/AGENTS.md" }, + exists: { type: "boolean", example: true }, + size: { type: "integer", example: 256 }, + mtime: { type: "number", example: 1718900000000 }, + preview: { type: "string", example: "# Project memory\nHello." }, + }, + }, + + CodexConfigConfigEntry: { + type: "object", + description: "config.toml at either the Codex home or the project .codex dir.", + required: ["scope", "file", "exists"], + properties: { + scope: { type: "string", enum: ["user", "project"], example: "user" }, + file: { type: "string", example: "/home/user/.codex/config.toml" }, + exists: { type: "boolean", example: true }, + size: { type: "integer", example: 640 }, + mtime: { type: "number", example: 1718900000000 }, + content: { type: "string", example: 'model = "gpt-5"\n' }, + truncated: { type: "boolean", example: false }, + }, + }, + + CodexConfigOverviewResponse: { + type: "object", + required: ["roots", "counts"], + properties: { + roots: { + type: "object", + properties: { + codexHome: { type: "string", example: "/home/user/.codex" }, + projectCodexDir: { type: "string", example: "/repo/.codex" }, + projectRoot: { type: "string", example: "/repo" }, + }, + }, + counts: { + type: "object", + additionalProperties: { + type: "object", + properties: { user: { type: "integer" }, project: { type: "integer" } }, + }, + example: { + skills: { user: 3, project: 1 }, + agents: { user: 2, project: 0 }, + rules: { user: 1, project: 0 }, + memory: { user: 1, project: 1 }, + config: { user: 1, project: 0 }, + }, + }, + }, + }, + + CodexConfigFileReadResponse: { + type: "object", + required: ["ok", "file", "size", "mtime", "truncated", "text"], + properties: { + ok: { type: "boolean", example: true }, + file: { type: "string", example: "/home/user/.codex/rules/default.rules" }, + size: { type: "integer", example: 128 }, + mtime: { type: "number", example: 1718900000000 }, + truncated: { type: "boolean", example: false }, + text: { type: "string", example: "allow" }, + }, + }, + + CodexConfigFileWriteResponse: { + type: "object", + required: ["file", "created", "backup"], + properties: { + file: { type: "string", example: "/home/user/.codex/rules/default.rules" }, + created: { type: "boolean", example: true }, + backup: { + type: "object", + nullable: true, + additionalProperties: true, + description: "Backup metadata when an existing file was overwritten; null on first create.", + }, + }, + }, + + CodexConfigFileDeleteResponse: { + type: "object", + required: ["file", "backup"], + properties: { + file: { type: "string", example: "/home/user/.codex/rules/default.rules" }, + backup: { type: "object", additionalProperties: true }, + }, + }, + + CodexConfigBackupsResponse: { + type: "object", + required: ["items"], + properties: { + items: { + type: "array", + items: { + type: "object", + properties: { + type: { type: "string", example: "rules" }, + name: { type: "string", example: "default.rules.2026-06-25T12-00-00.000Z.bak" }, + file: { type: "string" }, + mtime: { type: "number" }, + size: { type: "integer", nullable: true }, + }, + }, + }, + }, + }, + + CodexConfigCliActionResponse: { + type: "object", + description: "Result of a successful mutating `codex` CLI call.", + required: ["ok", "stdout", "stderr"], + properties: { + ok: { type: "boolean", example: true }, + stdout: { type: "string" }, + stderr: { type: "string" }, + }, + }, +}; + +// ── Reusable parameters ────────────────────────────────────────────────── + +const scopeParam = { + name: "scope", + in: "query", + required: false, + schema: { type: "string", enum: ["user", "project", "all"], default: "all" }, + description: "Discovery scope: `user`, `project`, or `all` (default).", + example: "all", +}; + +const cwdParam = { + name: "cwd", + in: "query", + required: false, + schema: { type: "string" }, + description: + "Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd.", + example: "/repo", +}; + +const errorResponse = (code, message) => ({ + content: { + "application/json": { + schema: { $ref: "#/components/schemas/ErrorResponse" }, + example: { error: { code, message } }, + }, + }, +}); + +const cliExitResponse = { + description: "`ECLIEXIT` — the `codex` CLI exited non-zero. stdout/stderr are preserved.", + content: { + "application/json": { + schema: { + type: "object", + properties: { + error: { + type: "object", + properties: { code: { type: "string" }, message: { type: "string" } }, + }, + code: { type: "integer", nullable: true }, + stdout: { type: "string" }, + stderr: { type: "string" }, + }, + }, + example: { + error: { code: "ECLIEXIT", message: 'marketplace "nope" is not registered' }, + code: 1, + stdout: "", + stderr: '✘ marketplace "nope" is not registered\n', + }, + }, + }, +}; + +function readSurface(name, summary, schemaRef, exampleItems) { + return { + get: { + tags: ["CodexConfig"], + summary, + operationId: `codexConfigGet${name[0].toUpperCase()}${name.slice(1)}`, + parameters: name === "memory" || name === "config" ? [cwdParam] : [scopeParam, cwdParam], + responses: { + 200: { + description: `${summary}.`, + content: { + "application/json": { + schema: { + type: "object", + properties: { [name]: { type: "array", items: { $ref: schemaRef } } }, + }, + example: { [name]: exampleItems }, + }, + }, + }, + }, + }, + }; +} + +// ── Paths ───────────────────────────────────────────────────────────── + +const paths = { + "/api/codex-config/overview": { + get: { + tags: ["CodexConfig"], + summary: "Codex config explorer overview", + description: + "Read-only. Returns the three resolved filesystem roots plus per-scope counts across skills, agents, rules, memory, and config.", + operationId: "codexConfigGetOverview", + parameters: [cwdParam], + responses: { + 200: { + description: "Overview of roots and counts.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigOverviewResponse" }, + }, + }, + }, + }, + }, + }, + + "/api/codex-config/skills": readSurface( + "skills", + "List discovered skills", + "#/components/schemas/CodexConfigSkill", + [{ scope: "user", name: "code-reviewer", frontmatter: {}, preview: "..." }] + ), + "/api/codex-config/agents": readSurface( + "agents", + "List discovered agent definitions", + "#/components/schemas/CodexConfigAgent", + [{ scope: "user", name: "reviewer", meta: { model: "gpt-5" } }] + ), + "/api/codex-config/rules": readSurface( + "rules", + "List discovered rules files", + "#/components/schemas/CodexConfigRule", + [{ scope: "user", name: "default", preview: "allow" }] + ), + "/api/codex-config/memory": readSurface( + "memory", + "Read AGENTS.md at both scopes", + "#/components/schemas/CodexConfigMemoryEntry", + [{ scope: "project", file: "/repo/AGENTS.md", exists: true }] + ), + "/api/codex-config/config": readSurface( + "config", + "Read config.toml at both scopes", + "#/components/schemas/CodexConfigConfigEntry", + [{ scope: "user", file: "/home/user/.codex/config.toml", exists: true }] + ), + + "/api/codex-config/file": { + get: { + tags: ["CodexConfig"], + summary: "Read a single file body", + description: + "Read-only. Returns the body of one file. Strict path containment: the resolved absolute path must live under CODEX_HOME, the project .codex dir, or (only for a file literally named AGENTS.md) the project root.", + operationId: "codexConfigGetFile", + parameters: [ + { + name: "path", + in: "query", + required: true, + schema: { type: "string" }, + example: "/home/user/.codex/rules/default.rules", + }, + cwdParam, + ], + responses: { + 200: { + description: "File body with metadata.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigFileReadResponse" }, + }, + }, + }, + 400: errorResponse("READ_DENIED", "path is outside allowed roots"), + }, + }, + put: { + tags: ["CodexConfig"], + summary: "Create or overwrite a text artifact", + description: + "Writes skills/agents/rules/memory/config. `name` is required for skills/agents/rules and forbidden for memory/config (fixed filenames). Backs up any existing file first; broadcasts `codex_config_changed` on success.", + operationId: "codexConfigPutFile", + parameters: [cwdParam], + requestBody: { + required: true, + content: { + "application/json": { + schema: { + type: "object", + required: ["scope", "type", "content"], + properties: { + scope: { type: "string", enum: ["user", "project"] }, + type: { type: "string", enum: ["skills", "agents", "rules", "memory", "config"] }, + name: { type: "string", nullable: true, example: "default" }, + content: { type: "string" }, + }, + }, + example: { scope: "user", type: "rules", name: "default", content: "allow" }, + }, + }, + }, + responses: { + 200: { + description: "File written.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigFileWriteResponse" }, + }, + }, + }, + 400: errorResponse("EBADNAME", "name must match ^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$"), + 413: errorResponse("ETOOLARGE", "content exceeds 262144 bytes"), + }, + }, + delete: { + tags: ["CodexConfig"], + summary: "Delete a text artifact", + description: + "Deletes skills/agents/rules/memory/config. Backup is mandatory and runs first. Broadcasts `codex_config_changed` on success.", + operationId: "codexConfigDeleteFile", + parameters: [cwdParam], + requestBody: { + required: true, + content: { + "application/json": { + schema: { + type: "object", + required: ["scope", "type"], + properties: { + scope: { type: "string", enum: ["user", "project"] }, + type: { type: "string", enum: ["skills", "agents", "rules", "memory", "config"] }, + name: { type: "string", nullable: true }, + }, + }, + example: { scope: "user", type: "agents", name: "reviewer" }, + }, + }, + }, + responses: { + 200: { + description: "File deleted.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigFileDeleteResponse" }, + }, + }, + }, + 404: errorResponse("ENOTFOUND", "agents/reviewer does not exist"), + }, + }, + }, + + "/api/codex-config/backups": { + get: { + tags: ["CodexConfig"], + summary: "List backups", + description: + "Read-only. Lists timestamped backups, optionally filtered by scope and/or type.", + operationId: "codexConfigGetBackups", + parameters: [ + { + name: "scope", + in: "query", + required: false, + schema: { type: "string", enum: ["user", "project"] }, + }, + { + name: "type", + in: "query", + required: false, + schema: { type: "string", enum: ["skills", "agents", "rules", "memory", "config"] }, + }, + cwdParam, + ], + responses: { + 200: { + description: "Backups list.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigBackupsResponse" }, + }, + }, + }, + }, + }, + }, + + "/api/codex-config/mcp": { + get: { + tags: ["CodexConfig"], + summary: "List configured MCP servers", + description: + "Reads live from `codex mcp list --json` — there is no on-disk reader for this surface.", + operationId: "codexConfigGetMcp", + responses: { + 200: { + description: "MCP servers as reported by the CLI.", + content: { "application/json": { schema: { type: "object", properties: { mcp: {} } } } }, + }, + 422: cliExitResponse, + 503: errorResponse("ECLINOTFOUND", "the `codex` CLI was not found on PATH"), + }, + }, + }, + + "/api/codex-config/plugins": { + get: { + tags: ["CodexConfig"], + summary: "List installed plugins", + description: "Reads live from `codex plugin list --json`.", + operationId: "codexConfigGetPlugins", + responses: { + 200: { + description: "Plugins as reported by the CLI.", + content: { + "application/json": { schema: { type: "object", properties: { plugins: {} } } }, + }, + }, + 422: cliExitResponse, + 503: errorResponse("ECLINOTFOUND", "the `codex` CLI was not found on PATH"), + }, + }, + }, + + "/api/codex-config/marketplaces": { + get: { + tags: ["CodexConfig"], + summary: "List registered marketplaces", + description: "Reads live from `codex plugin marketplace list --json`.", + operationId: "codexConfigGetMarketplaces", + responses: { + 200: { + description: "Marketplaces as reported by the CLI.", + content: { + "application/json": { schema: { type: "object", properties: { marketplaces: {} } } }, + }, + }, + 422: cliExitResponse, + 503: errorResponse("ECLINOTFOUND", "the `codex` CLI was not found on PATH"), + }, + }, + }, + + "/api/codex-config/mcp/add": { + post: { + tags: ["CodexConfig"], + summary: "Add an MCP server", + description: + "Runs `codex mcp add`. Accepts either a streamable-HTTP server (`url`, optional `bearerTokenEnvVar`) or a stdio server (`command`, `args`, optional `env`) — never both.", + operationId: "codexConfigPostMcpAdd", + requestBody: { + required: true, + content: { + "application/json": { + schema: { + type: "object", + required: ["name"], + properties: { + name: { type: "string" }, + url: { type: "string" }, + bearerTokenEnvVar: { type: "string" }, + command: { type: "string" }, + args: { type: "array", items: { type: "string" } }, + env: { type: "object", additionalProperties: { type: "string" } }, + }, + }, + example: { name: "github", url: "https://mcp.example.com/github" }, + }, + }, + }, + responses: { + 200: { + description: "MCP server added.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigCliActionResponse" }, + }, + }, + }, + 400: errorResponse("EBADARG", "url must be a valid http(s) URL"), + 422: cliExitResponse, + }, + }, + }, + + "/api/codex-config/mcp/remove": { + post: { + tags: ["CodexConfig"], + summary: "Remove an MCP server", + operationId: "codexConfigPostMcpRemove", + requestBody: { + required: true, + content: { + "application/json": { + schema: { + type: "object", + required: ["name"], + properties: { name: { type: "string" } }, + }, + example: { name: "github" }, + }, + }, + }, + responses: { + 200: { + description: "MCP server removed.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigCliActionResponse" }, + }, + }, + }, + 422: cliExitResponse, + }, + }, + }, + + "/api/codex-config/plugins/action": { + post: { + tags: ["CodexConfig"], + summary: "Add or remove a plugin", + description: "Runs `codex plugin add ` or `codex plugin remove `.", + operationId: "codexConfigPostPluginsAction", + requestBody: { + required: true, + content: { + "application/json": { + schema: { + type: "object", + required: ["spec", "action"], + properties: { + spec: { type: "string" }, + action: { type: "string", enum: ["add", "remove"] }, + }, + }, + example: { spec: "demo@market", action: "add" }, + }, + }, + }, + responses: { + 200: { + description: "Plugin action applied.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigCliActionResponse" }, + }, + }, + }, + 400: errorResponse("EBADACTION", "action must be one of add, remove"), + 422: cliExitResponse, + }, + }, + }, + + "/api/codex-config/marketplaces/add": { + post: { + tags: ["CodexConfig"], + summary: "Register a marketplace", + description: "Runs `codex plugin marketplace add `.", + operationId: "codexConfigPostMarketplacesAdd", + requestBody: { + required: true, + content: { + "application/json": { + schema: { type: "object", required: ["url"], properties: { url: { type: "string" } } }, + example: { url: "https://example.com/marketplace.json" }, + }, + }, + }, + responses: { + 200: { + description: "Marketplace added.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigCliActionResponse" }, + }, + }, + }, + 400: errorResponse("EBADARG", "marketplace url must be a valid http(s) URL"), + 422: cliExitResponse, + }, + }, + }, + + "/api/codex-config/marketplaces/action": { + post: { + tags: ["CodexConfig"], + summary: "Remove a marketplace", + description: "Runs `codex plugin marketplace remove `.", + operationId: "codexConfigPostMarketplacesAction", + requestBody: { + required: true, + content: { + "application/json": { + schema: { + type: "object", + required: ["name", "action"], + properties: { + name: { type: "string" }, + action: { type: "string", enum: ["remove"] }, + }, + }, + example: { name: "demo-mp", action: "remove" }, + }, + }, + }, + responses: { + 200: { + description: "Marketplace action applied.", + content: { + "application/json": { + schema: { $ref: "#/components/schemas/CodexConfigCliActionResponse" }, + }, + }, + }, + 400: errorResponse("EBADACTION", "action must be one of remove"), + 422: cliExitResponse, + 503: errorResponse("ECLINOTFOUND", "the `codex` CLI was not found on PATH"), + }, + }, + }, +}; + +module.exports = { tags, schemas, paths }; diff --git a/server/routes/codex-config.js b/server/routes/codex-config.js new file mode 100644 index 00000000..bccf048e --- /dev/null +++ b/server/routes/codex-config.js @@ -0,0 +1,366 @@ +/** + * @file codex-config.js + * @description HTTP surface for inspecting and (carefully) mutating Codex + * CLI configuration: skills, agents, rules, memory (AGENTS.md), + * config.toml, MCP servers, plugins, and marketplaces. Powers the Codex + * Config panel — the Codex counterpart of cc-config.js. + * + * Read paths (skills/agents/rules/memory/config) come from codex-discovery.js + * (pure file reads). MCP servers, plugins, and marketplaces have no on-disk + * reader — codex-discovery.js deliberately leaves them to the `codex` CLI — + * so those three GET routes, plus every mutation, shell out through + * codex-cli.js. A non-zero CLI exit is reported as 422 with stdout/stderr + * preserved, never a 500. + * + * Write paths (skills/agents/rules/memory/config) go through codex-mutate.js: + * every write/delete is preceded by a timestamped backup, and every mutation + * error code is mapped explicitly below so an unmapped one can never fall + * through to a bare 500. + * + * @author Nguyễn Ngọc Trí Vĩ + */ + +const fs = require("node:fs"); +const path = require("node:path"); +const { Router } = require("express"); +const codexDiscovery = require("../lib/codex-discovery"); +const codexMutate = require("../lib/codex-mutate"); +const codexCli = require("../lib/codex-cli"); +const { getCodexHome, getProjectCodexDir, getProjectRoot } = require("../lib/codex-home"); +const { isUnder, MAX_FILE_BYTES } = require("../lib/cc-discovery"); +const { broadcast } = require("../websocket"); + +const router = Router(); + +function emitChanged(payload) { + try { + broadcast("codex_config_changed", { source: "dashboard", ...payload }); + } catch { + /* websocket may not be initialised in unit tests */ + } +} + +function scopeOf(req) { + const s = String(req.query.scope || "all"); + return s === "user" || s === "project" ? s : "all"; +} + +function cwdOf(req) { + // Mirrors cc-config's cwdOf: the server's own cwd is the natural + // "project", but ?cwd= lets the caller inspect another working dir. + const c = typeof req.query.cwd === "string" && req.query.cwd ? req.query.cwd : null; + return c || process.cwd(); +} + +// ── Mutation errors (codex-mutate.js) → HTTP status ──────────────────── +// Anything unmapped is a 500 — this map must cover every code codex-mutate +// can throw. +const ERR_TO_STATUS = { + EBADTYPE: 400, + EBADSCOPE: 400, + EBADNAME: 400, + EBADCONTENT: 400, + EOUTOFROOT: 400, + ETOOLARGE: 413, + ENOTFOUND: 404, +}; + +function mutateError(res, err) { + const status = ERR_TO_STATUS[err.code] || 500; + return res + .status(status) + .json({ error: { code: err.code || "EINTERNAL", message: err.message } }); +} + +// ── Discovery reads ───────────────────────────────────────────────────── + +router.get("/overview", (req, res) => { + res.json(codexDiscovery.readOverview({ cwd: cwdOf(req) })); +}); + +router.get("/skills", (req, res) => { + res.json({ skills: codexDiscovery.readSkills({ scope: scopeOf(req), cwd: cwdOf(req) }) }); +}); + +router.get("/agents", (req, res) => { + res.json({ agents: codexDiscovery.readAgents({ scope: scopeOf(req), cwd: cwdOf(req) }) }); +}); + +router.get("/rules", (req, res) => { + res.json({ rules: codexDiscovery.readRules({ scope: scopeOf(req), cwd: cwdOf(req) }) }); +}); + +router.get("/memory", (req, res) => { + res.json({ memory: codexDiscovery.readMemory({ cwd: cwdOf(req) }) }); +}); + +router.get("/config", (req, res) => { + res.json({ config: codexDiscovery.readConfig({ cwd: cwdOf(req) }) }); +}); + +// GET /api/codex-config/file?path=/abs/path — return body of a single file. +// Path must resolve under CODEX_HOME, the project .codex dir, or (only for a +// file literally named AGENTS.md) the project root. No dedicated reader for +// this exists in codex-discovery.js (it only reads whole surfaces), so this +// mirrors cc-discovery's readFileSafe against the Codex roots instead. +function readCodexFileSafe(absPath, opts = {}) { + const codexHome = getCodexHome(); + const projectCodexDir = getProjectCodexDir(opts.cwd); + const projectRoot = getProjectRoot(opts.cwd); + const resolved = path.resolve(absPath); + const inside = [codexHome, projectCodexDir, projectRoot].some((root) => isUnder(root, resolved)); + if (!inside) return { error: "path is outside allowed roots" }; + if ( + isUnder(projectRoot, resolved) && + !isUnder(projectCodexDir, resolved) && + path.basename(resolved) !== "AGENTS.md" + ) { + return { error: "only AGENTS.md is readable from project root" }; + } + let stat; + try { + stat = fs.statSync(resolved); + } catch { + return { error: "file not readable" }; + } + if (!stat.isFile()) return { error: "file not readable" }; + let text; + try { + text = fs.readFileSync(resolved, "utf8"); + } catch { + return { error: "file not readable" }; + } + const truncated = stat.size > MAX_FILE_BYTES; + return { + ok: true, + file: resolved, + size: stat.size, + mtime: stat.mtimeMs, + truncated, + text: truncated ? text.slice(0, MAX_FILE_BYTES) : text, + }; +} + +router.get("/file", (req, res) => { + const p = req.query.path; + if (typeof p !== "string" || !p) { + return res.status(400).json({ error: { code: "BAD_PATH", message: "path is required" } }); + } + const result = readCodexFileSafe(p, { cwd: cwdOf(req) }); + if (result.error) + return res.status(400).json({ error: { code: "READ_DENIED", message: result.error } }); + res.json(result); +}); + +// ── Mutation endpoints (codex-mutate.js) ──────────────────────────────── +// +// PUT /api/codex-config/file — create or overwrite. Body: { scope, type, name?, content } +// DELETE /api/codex-config/file — delete. Body: { scope, type, name? } +// GET /api/codex-config/backups[?scope=&type=] — list backups + +router.put("/file", (req, res) => { + const { scope, type, name, content } = req.body || {}; + if (typeof scope !== "string" || typeof type !== "string") { + return res + .status(400) + .json({ error: { code: "EBADREQ", message: "scope and type are required" } }); + } + try { + const result = codexMutate.writeArtifact({ scope, type, name, content, cwd: cwdOf(req) }); + emitChanged({ action: "write", scope, type, name: name || null }); + res.json(result); + } catch (err) { + return mutateError(res, err); + } +}); + +router.delete("/file", (req, res) => { + const { scope, type, name } = req.body || {}; + if (typeof scope !== "string" || typeof type !== "string") { + return res + .status(400) + .json({ error: { code: "EBADREQ", message: "scope and type are required" } }); + } + try { + const result = codexMutate.deleteArtifact({ scope, type, name, cwd: cwdOf(req) }); + emitChanged({ action: "delete", scope, type, name: name || null }); + res.json(result); + } catch (err) { + return mutateError(res, err); + } +}); + +router.get("/backups", (req, res) => { + const scope = + req.query.scope === "user" || req.query.scope === "project" ? req.query.scope : undefined; + const type = typeof req.query.type === "string" ? req.query.type : undefined; + try { + res.json({ items: codexMutate.listBackups({ scope, type, cwd: cwdOf(req) }) }); + } catch (err) { + return mutateError(res, err); + } +}); + +// ── MCP servers / plugins / marketplaces (via the `codex` CLI) ───────── +// +// Same reasoning as cc-config.js's plugin/marketplace routes: these shell +// out to `codex …` with an argv array — never a shell string — because the +// CLI owns config.toml's schema. Identifiers travel in the body, not the +// URL, for the same reason DELETE /file does. + +const CLI_ERR_TO_STATUS = { + EBADARG: 400, + ECLINOTFOUND: 503, + ETIMEOUT: 504, +}; + +function cliError(res, err) { + const status = CLI_ERR_TO_STATUS[err.code] || 500; + return res + .status(status) + .json({ error: { code: err.code || "EINTERNAL", message: err.message } }); +} + +/** + * Best-effort human message from a failed run. codex-cli.js does not export + * one (unlike cc-plugin-cli's cliMessage), so this route-local helper does + * the same "last non-empty line, prefer a marked one" heuristic. + */ +function cliMessage(result) { + const text = `${result.stderr || ""}\n${result.stdout || ""}`; + const lines = text + .split("\n") + .map((l) => l.trim()) + .filter(Boolean); + const marked = lines.find((l) => l.includes("✘") || l.toLowerCase().includes("error")); + const picked = marked || lines[lines.length - 1] || ""; + return picked.replace(/^✘\s*/, "").slice(0, 1000); +} + +/** A non-zero CLI exit is a 422, not a 500 — the caller can inspect stderr. */ +function cliFailure(res, result) { + return res.status(422).json({ + error: { + code: "ECLIEXIT", + message: cliMessage(result) || `codex exited with code ${result.code}`, + }, + code: result.code, + stdout: result.stdout, + stderr: result.stderr, + }); +} + +/** Read-through result for a GET route backed by the CLI. */ +function cliRead(res, result, key) { + if (!result.ok) return cliFailure(res, result); + res.json({ [key]: result.data !== undefined ? result.data : result.stdout }); +} + +/** Result for a mutating route: broadcast on success, 422 on CLI failure. */ +function cliResult(res, result, changed) { + if (!result.ok) return cliFailure(res, result); + emitChanged(changed); + res.json({ ok: true, stdout: result.stdout, stderr: result.stderr }); +} + +router.get("/mcp", async (_req, res) => { + try { + return cliRead(res, await codexCli.mcpList(), "mcp"); + } catch (err) { + return cliError(res, err); + } +}); + +router.get("/plugins", async (_req, res) => { + try { + return cliRead(res, await codexCli.pluginList(), "plugins"); + } catch (err) { + return cliError(res, err); + } +}); + +router.get("/marketplaces", async (_req, res) => { + try { + return cliRead(res, await codexCli.marketplaceList(), "marketplaces"); + } catch (err) { + return cliError(res, err); + } +}); + +router.post("/mcp/add", async (req, res) => { + try { + const result = await codexCli.mcpAdd(req.body || {}); + return cliResult(res, result, { action: "add", type: "mcp", name: (req.body || {}).name }); + } catch (err) { + return cliError(res, err); + } +}); + +router.post("/mcp/remove", async (req, res) => { + const { name } = req.body || {}; + try { + const result = await codexCli.mcpRemove(name); + return cliResult(res, result, { action: "remove", type: "mcp", name }); + } catch (err) { + return cliError(res, err); + } +}); + +const PLUGIN_ACTIONS = { + add: codexCli.pluginAdd, + remove: codexCli.pluginRemove, +}; + +router.post("/plugins/action", async (req, res) => { + const { spec, action } = req.body || {}; + const run = PLUGIN_ACTIONS[action]; + if (!run) { + return res.status(400).json({ + error: { + code: "EBADACTION", + message: `action must be one of ${Object.keys(PLUGIN_ACTIONS).join(", ")}`, + }, + }); + } + try { + const result = await run(spec); + return cliResult(res, result, { action, type: "plugin", name: spec }); + } catch (err) { + return cliError(res, err); + } +}); + +router.post("/marketplaces/add", async (req, res) => { + const { url } = req.body || {}; + try { + const result = await codexCli.marketplaceAdd(url); + return cliResult(res, result, { action: "add", type: "marketplace", name: url }); + } catch (err) { + return cliError(res, err); + } +}); + +const MARKETPLACE_ACTIONS = { + remove: codexCli.marketplaceRemove, +}; + +router.post("/marketplaces/action", async (req, res) => { + const { name, action } = req.body || {}; + const run = MARKETPLACE_ACTIONS[action]; + if (!run) { + return res.status(400).json({ + error: { + code: "EBADACTION", + message: `action must be one of ${Object.keys(MARKETPLACE_ACTIONS).join(", ")}`, + }, + }); + } + try { + const result = await run(name); + return cliResult(res, result, { action, type: "marketplace", name }); + } catch (err) { + return cliError(res, err); + } +}); + +module.exports = router; From 29ed86d6724b5586110087ccd44a5c6cb97e692a Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 20:07:57 +0700 Subject: [PATCH 13/27] fix(codex): close symlink escape on GET /file, pin project writes to server cwd --- openapi.yaml | 47 ++++++----- server/__tests__/codex-config.test.js | 111 ++++++++++++++++++++++++++ server/lib/codex-cli.js | 22 +++++ server/lib/codex-discovery.js | 48 +++++++++++ server/lib/codex-mutate.js | 1 + server/openapi-extra/codex-config.js | 9 ++- server/routes/codex-config.js | 97 +++++++--------------- 7 files changed, 246 insertions(+), 89 deletions(-) diff --git a/openapi.yaml b/openapi.yaml index df8b830a..d959224b 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -9017,16 +9017,8 @@ paths: tags: - CodexConfig summary: Create or overwrite a text artifact - description: Writes skills/agents/rules/memory/config. `name` is required for skills/agents/rules and forbidden for memory/config (fixed filenames). Backs up any existing file first; broadcasts `codex_config_changed` on success. + description: Writes skills/agents/rules/memory/config. `name` is required for skills/agents/rules and forbidden for memory/config (fixed filenames). `project` scope always resolves against the dashboard server's own cwd — there is no `?cwd=` override on writes, unlike the read routes, so a caller cannot point a write at an arbitrary directory. Backs up any existing file first; broadcasts `codex_config_changed` on success. operationId: codexConfigPutFile - parameters: - - name: cwd - in: query - required: false - schema: - type: string - description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. - example: /repo requestBody: required: true content: @@ -9091,16 +9083,8 @@ paths: tags: - CodexConfig summary: Delete a text artifact - description: Deletes skills/agents/rules/memory/config. Backup is mandatory and runs first. Broadcasts `codex_config_changed` on success. + description: Deletes skills/agents/rules/memory/config. `project` scope always resolves against the dashboard server's own cwd — no `?cwd=` override, same as PUT. Backup is mandatory and runs first. Broadcasts `codex_config_changed` on success. operationId: codexConfigDeleteFile - parameters: - - name: cwd - in: query - required: false - schema: - type: string - description: Override the working directory used to resolve the project root and project .codex dir. Defaults to the dashboard server's own cwd. - example: /repo requestBody: required: true content: @@ -9233,6 +9217,15 @@ paths: stdout: '' stderr: | ✘ marketplace "nope" is not registered + '502': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: ECLIBADOUTPUT + message: could not parse `codex` JSON output '503': content: application/json: @@ -9287,6 +9280,15 @@ paths: stdout: '' stderr: | ✘ marketplace "nope" is not registered + '502': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: ECLIBADOUTPUT + message: could not parse `codex` JSON output '503': content: application/json: @@ -9341,6 +9343,15 @@ paths: stdout: '' stderr: | ✘ marketplace "nope" is not registered + '502': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: ECLIBADOUTPUT + message: could not parse `codex` JSON output '503': content: application/json: diff --git a/server/__tests__/codex-config.test.js b/server/__tests__/codex-config.test.js index 797507d7..3b58ae0f 100644 --- a/server/__tests__/codex-config.test.js +++ b/server/__tests__/codex-config.test.js @@ -211,6 +211,42 @@ describe("/api/codex-config", () => { assert.equal(body.error.code, "READ_DENIED"); }); + it("file endpoint refuses a leaf symlink that escapes CODEX_HOME (no secret leaks)", async () => { + const outside = fs.mkdtempSync(path.join(os.tmpdir(), "codex-config-outside-")); + fs.writeFileSync(path.join(outside, "secret.rules"), "TOP SECRET LEAF"); + const linkPath = path.join(FAKE_HOME, "rules", "leaf-link.rules"); + fs.symlinkSync(path.join(outside, "secret.rules"), linkPath, "file"); + try { + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}&path=${encodeURIComponent(linkPath)}` + ); + assert.equal(status, 400); + assert.equal(body.error.code, "READ_DENIED"); + assert.doesNotMatch(JSON.stringify(body), /TOP SECRET LEAF/); + } finally { + fs.rmSync(outside, { recursive: true, force: true }); + } + }); + + it("file endpoint refuses an ancestor symlink that escapes CODEX_HOME (no secret leaks)", async () => { + const outside = fs.mkdtempSync(path.join(os.tmpdir(), "codex-config-outside-")); + fs.writeFileSync(path.join(outside, "secret.txt"), "TOP SECRET ANCESTOR"); + const linkDir = path.join(FAKE_HOME, "evil-dir"); + fs.symlinkSync(outside, linkDir, "dir"); + try { + const target = path.join(linkDir, "secret.txt"); + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}&path=${encodeURIComponent(target)}` + ); + assert.equal(status, 400); + assert.equal(body.error.code, "READ_DENIED"); + assert.doesNotMatch(JSON.stringify(body), /TOP SECRET ANCESTOR/); + } finally { + fs.rmSync(linkDir, { force: true }); + fs.rmSync(outside, { recursive: true, force: true }); + } + }); + it("file endpoint requires a path", async () => { const { status, body } = await fetchJson("/api/codex-config/file"); assert.equal(status, 400); @@ -267,6 +303,56 @@ describe("/api/codex-config", () => { ); }); + it("PUT /file ignores ?cwd= for project scope — writes to the server's own cwd, not a caller-chosen dir", async () => { + const victim = fs.mkdtempSync(path.join(os.tmpdir(), "codex-config-victim-")); + const originalCwd = process.cwd(); + const serverCwd = fs.mkdtempSync(path.join(os.tmpdir(), "codex-config-servercwd-")); + process.chdir(serverCwd); + try { + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(victim)}`, + { + method: "PUT", + body: { scope: "project", type: "memory", content: "PWNED" }, + } + ); + assert.equal(status, 200); + assert.equal(fs.existsSync(path.join(victim, "AGENTS.md")), false, "must not write to ?cwd="); + assert.equal(body.file, path.join(serverCwd, "AGENTS.md")); + assert.equal(fs.readFileSync(body.file, "utf8"), "PWNED"); + } finally { + process.chdir(originalCwd); + fs.rmSync(victim, { recursive: true, force: true }); + fs.rmSync(serverCwd, { recursive: true, force: true }); + } + }); + + it("DELETE /file ignores ?cwd= for project scope", async () => { + const victim = fs.mkdtempSync(path.join(os.tmpdir(), "codex-config-victim-")); + fs.writeFileSync(path.join(victim, ".codex-marker"), "x"); // proves this dir wasn't touched + const originalCwd = process.cwd(); + const serverCwd = fs.mkdtempSync(path.join(os.tmpdir(), "codex-config-servercwd-")); + fs.mkdirSync(path.join(serverCwd, ".codex", "rules"), { recursive: true }); + fs.writeFileSync(path.join(serverCwd, ".codex", "rules", "victim.rules"), "real"); + process.chdir(serverCwd); + try { + const { status } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(victim)}`, + { + method: "DELETE", + body: { scope: "project", type: "rules", name: "victim" }, + } + ); + assert.equal(status, 200); + assert.equal(fs.existsSync(path.join(victim, ".codex-marker")), true, "?cwd= dir left alone"); + assert.equal(fs.existsSync(path.join(serverCwd, ".codex", "rules", "victim.rules")), false); + } finally { + process.chdir(originalCwd); + fs.rmSync(victim, { recursive: true, force: true }); + fs.rmSync(serverCwd, { recursive: true, force: true }); + } + }); + it("PUT /file rejects a write whose name escapes the root with 400, not 500", async () => { const { status, body } = await fetchJson( `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}`, @@ -364,6 +450,31 @@ describe("/api/codex-config", () => { } }); + it("GET /mcp reports unparseable CLI JSON as 502, not a type-confused 200", async () => { + const restore = stubCodex('echo "not json"'); + try { + const { status, body } = await fetchJson("/api/codex-config/mcp"); + assert.equal(status, 502); + assert.equal(body.error.code, "ECLIBADOUTPUT"); + } finally { + restore(); + } + }); + + it("POST /mcp/add rejects a non-array args with 400, not a 500", async () => { + const restore = stubCodex(ECHO_ARGV); + try { + const { status, body } = await fetchJson("/api/codex-config/mcp/add", { + method: "POST", + body: { name: "docs", command: "node", args: "oops" }, + }); + assert.equal(status, 400); + assert.equal(body.error.code, "EBADARG"); + } finally { + restore(); + } + }); + it("POST /mcp/add runs the CLI with the given argv", async () => { const restore = stubCodex(ECHO_ARGV); try { diff --git a/server/lib/codex-cli.js b/server/lib/codex-cli.js index ba00d271..f1750233 100644 --- a/server/lib/codex-cli.js +++ b/server/lib/codex-cli.js @@ -199,6 +199,10 @@ async function mcpAdd({ name, url, bearerTokenEnvVar, command, args = [], env }) if (!url && !command) { throw makeError("EBADARG", "mcpAdd requires either url or command"); } + if (!Array.isArray(args)) throw makeError("EBADARG", "args must be an array"); + if (env !== undefined && (typeof env !== "object" || env === null || Array.isArray(env))) { + throw makeError("EBADARG", "env must be an object"); + } if (url) { if (env) throw makeError("EBADARG", "env is not valid together with url (stdio-only)"); @@ -256,8 +260,26 @@ async function marketplaceRemove(name) { return serialize(() => runCodex(["plugin", "marketplace", "remove", n])); } +/** + * Best-effort human message from a failed run. The CLI prints its errors with + * a `✘` marker on either stream; fall back to the last non-empty line. + * Unchanged copy of cc-plugin-cli.js's cliMessage — same CLI-output shape, + * same heuristic. + */ +function cliMessage(res) { + const text = `${res.stderr || ""}\n${res.stdout || ""}`; + const lines = text + .split("\n") + .map((l) => l.trim()) + .filter(Boolean); + const marked = lines.find((l) => l.includes("✘")); + const picked = marked || lines[lines.length - 1] || ""; + return picked.replace(/^✘\s*/, "").slice(0, 1000); +} + module.exports = { runCodex, + cliMessage, mcpList, mcpAdd, mcpRemove, diff --git a/server/lib/codex-discovery.js b/server/lib/codex-discovery.js index d026dca0..c3f8b0cd 100644 --- a/server/lib/codex-discovery.js +++ b/server/lib/codex-discovery.js @@ -22,6 +22,11 @@ const { isFileLike, MAX_FILE_BYTES, } = require("./cc-discovery"); +// Shared symlink-escape guard — the same check writeArtifact/deleteArtifact +// use, so a read and a write of the same path agree on what "inside the +// root" means. See codex-mutate.js's assertNotSymlinkEscape for the leaf + +// ancestor symlink reasoning. +const { assertNotSymlinkEscape } = require("./codex-mutate"); function safeReadText(absPath) { try { @@ -286,6 +291,48 @@ function readOverview(opts = {}) { }; } +// ── Single-file body endpoint (with strict path containment) ─────────── + +/** + * Read the body of one file living under CODEX_HOME, the project .codex + * dir, or (only for a file literally named AGENTS.md) the project root. + * Mirrors cc-discovery's readFileSafe, plus the same leaf/ancestor + * symlink-escape guard the write side (codex-mutate.js) uses — a string-only + * `isUnder` check alone would let a symlink dropped anywhere along the path + * read arbitrary files outside the root. + */ +function readFileSafe(absPath, opts = {}) { + const codexHome = getCodexHome(); + const projectCodexDir = getProjectCodexDir(opts.cwd); + const projectRoot = getProjectRoot(opts.cwd); + const resolved = path.resolve(absPath); + + const inside = [codexHome, projectCodexDir, projectRoot].some((root) => isUnder(root, resolved)); + if (!inside) return { error: "path is outside allowed roots" }; + if ( + isUnder(projectRoot, resolved) && + !isUnder(projectCodexDir, resolved) && + path.basename(resolved) !== "AGENTS.md" + ) { + return { error: "only AGENTS.md is readable from project root" }; + } + + const trustedRoot = isUnder(codexHome, resolved) + ? codexHome + : isUnder(projectCodexDir, resolved) + ? projectCodexDir + : projectRoot; + try { + assertNotSymlinkEscape(trustedRoot, resolved); + } catch (err) { + return { error: err.message }; + } + + const r = safeReadText(resolved); + if (!r) return { error: "file not readable" }; + return { ok: true, file: resolved, ...r }; +} + module.exports = { readSkills, readAgents, @@ -293,5 +340,6 @@ module.exports = { readMemory, readConfig, readOverview, + readFileSafe, readTomlMeta, }; diff --git a/server/lib/codex-mutate.js b/server/lib/codex-mutate.js index 29b644f6..c28862c8 100644 --- a/server/lib/codex-mutate.js +++ b/server/lib/codex-mutate.js @@ -300,6 +300,7 @@ module.exports = { deleteArtifact, listBackups, resolveTarget, // exported for tests + assertNotSymlinkEscape, // shared with codex-discovery.js's readFileSafe — one check, both verbs TYPES, NAME_RE, }; diff --git a/server/openapi-extra/codex-config.js b/server/openapi-extra/codex-config.js index 55618932..46de53ab 100644 --- a/server/openapi-extra/codex-config.js +++ b/server/openapi-extra/codex-config.js @@ -398,9 +398,8 @@ const paths = { tags: ["CodexConfig"], summary: "Create or overwrite a text artifact", description: - "Writes skills/agents/rules/memory/config. `name` is required for skills/agents/rules and forbidden for memory/config (fixed filenames). Backs up any existing file first; broadcasts `codex_config_changed` on success.", + "Writes skills/agents/rules/memory/config. `name` is required for skills/agents/rules and forbidden for memory/config (fixed filenames). `project` scope always resolves against the dashboard server's own cwd — there is no `?cwd=` override on writes, unlike the read routes, so a caller cannot point a write at an arbitrary directory. Backs up any existing file first; broadcasts `codex_config_changed` on success.", operationId: "codexConfigPutFile", - parameters: [cwdParam], requestBody: { required: true, content: { @@ -436,9 +435,8 @@ const paths = { tags: ["CodexConfig"], summary: "Delete a text artifact", description: - "Deletes skills/agents/rules/memory/config. Backup is mandatory and runs first. Broadcasts `codex_config_changed` on success.", + "Deletes skills/agents/rules/memory/config. `project` scope always resolves against the dashboard server's own cwd — no `?cwd=` override, same as PUT. Backup is mandatory and runs first. Broadcasts `codex_config_changed` on success.", operationId: "codexConfigDeleteFile", - parameters: [cwdParam], requestBody: { required: true, content: { @@ -518,6 +516,7 @@ const paths = { content: { "application/json": { schema: { type: "object", properties: { mcp: {} } } } }, }, 422: cliExitResponse, + 502: errorResponse("ECLIBADOUTPUT", "could not parse `codex` JSON output"), 503: errorResponse("ECLINOTFOUND", "the `codex` CLI was not found on PATH"), }, }, @@ -537,6 +536,7 @@ const paths = { }, }, 422: cliExitResponse, + 502: errorResponse("ECLIBADOUTPUT", "could not parse `codex` JSON output"), 503: errorResponse("ECLINOTFOUND", "the `codex` CLI was not found on PATH"), }, }, @@ -556,6 +556,7 @@ const paths = { }, }, 422: cliExitResponse, + 502: errorResponse("ECLIBADOUTPUT", "could not parse `codex` JSON output"), 503: errorResponse("ECLINOTFOUND", "the `codex` CLI was not found on PATH"), }, }, diff --git a/server/routes/codex-config.js b/server/routes/codex-config.js index bccf048e..ade362a3 100644 --- a/server/routes/codex-config.js +++ b/server/routes/codex-config.js @@ -20,14 +20,10 @@ * @author Nguyễn Ngọc Trí Vĩ */ -const fs = require("node:fs"); -const path = require("node:path"); const { Router } = require("express"); const codexDiscovery = require("../lib/codex-discovery"); const codexMutate = require("../lib/codex-mutate"); const codexCli = require("../lib/codex-cli"); -const { getCodexHome, getProjectCodexDir, getProjectRoot } = require("../lib/codex-home"); -const { isUnder, MAX_FILE_BYTES } = require("../lib/cc-discovery"); const { broadcast } = require("../websocket"); const router = Router(); @@ -100,53 +96,17 @@ router.get("/config", (req, res) => { // GET /api/codex-config/file?path=/abs/path — return body of a single file. // Path must resolve under CODEX_HOME, the project .codex dir, or (only for a -// file literally named AGENTS.md) the project root. No dedicated reader for -// this exists in codex-discovery.js (it only reads whole surfaces), so this -// mirrors cc-discovery's readFileSafe against the Codex roots instead. -function readCodexFileSafe(absPath, opts = {}) { - const codexHome = getCodexHome(); - const projectCodexDir = getProjectCodexDir(opts.cwd); - const projectRoot = getProjectRoot(opts.cwd); - const resolved = path.resolve(absPath); - const inside = [codexHome, projectCodexDir, projectRoot].some((root) => isUnder(root, resolved)); - if (!inside) return { error: "path is outside allowed roots" }; - if ( - isUnder(projectRoot, resolved) && - !isUnder(projectCodexDir, resolved) && - path.basename(resolved) !== "AGENTS.md" - ) { - return { error: "only AGENTS.md is readable from project root" }; - } - let stat; - try { - stat = fs.statSync(resolved); - } catch { - return { error: "file not readable" }; - } - if (!stat.isFile()) return { error: "file not readable" }; - let text; - try { - text = fs.readFileSync(resolved, "utf8"); - } catch { - return { error: "file not readable" }; - } - const truncated = stat.size > MAX_FILE_BYTES; - return { - ok: true, - file: resolved, - size: stat.size, - mtime: stat.mtimeMs, - truncated, - text: truncated ? text.slice(0, MAX_FILE_BYTES) : text, - }; -} - +// file literally named AGENTS.md) the project root. codexDiscovery.readFileSafe +// re-checks containment with the same leaf/ancestor symlink-escape guard the +// write side uses (codex-mutate.js's assertNotSymlinkEscape) — a plain +// string-only path check would let a symlink dropped under the root read +// arbitrary files outside it. router.get("/file", (req, res) => { const p = req.query.path; if (typeof p !== "string" || !p) { return res.status(400).json({ error: { code: "BAD_PATH", message: "path is required" } }); } - const result = readCodexFileSafe(p, { cwd: cwdOf(req) }); + const result = codexDiscovery.readFileSafe(p, { cwd: cwdOf(req) }); if (result.error) return res.status(400).json({ error: { code: "READ_DENIED", message: result.error } }); res.json(result); @@ -157,6 +117,11 @@ router.get("/file", (req, res) => { // PUT /api/codex-config/file — create or overwrite. Body: { scope, type, name?, content } // DELETE /api/codex-config/file — delete. Body: { scope, type, name? } // GET /api/codex-config/backups[?scope=&type=] — list backups +// +// Writes resolve "project" scope against the SERVER's cwd, never `?cwd=`: a +// write target chosen by the caller would let any request create a file +// anywhere on disk (same reasoning cc-config.js's PATCH /settings gives for +// refusing `?cwd=` there). Reads keep the override. router.put("/file", (req, res) => { const { scope, type, name, content } = req.body || {}; @@ -166,7 +131,7 @@ router.put("/file", (req, res) => { .json({ error: { code: "EBADREQ", message: "scope and type are required" } }); } try { - const result = codexMutate.writeArtifact({ scope, type, name, content, cwd: cwdOf(req) }); + const result = codexMutate.writeArtifact({ scope, type, name, content, cwd: process.cwd() }); emitChanged({ action: "write", scope, type, name: name || null }); res.json(result); } catch (err) { @@ -182,7 +147,7 @@ router.delete("/file", (req, res) => { .json({ error: { code: "EBADREQ", message: "scope and type are required" } }); } try { - const result = codexMutate.deleteArtifact({ scope, type, name, cwd: cwdOf(req) }); + const result = codexMutate.deleteArtifact({ scope, type, name, cwd: process.cwd() }); emitChanged({ action: "delete", scope, type, name: name || null }); res.json(result); } catch (err) { @@ -221,28 +186,12 @@ function cliError(res, err) { .json({ error: { code: err.code || "EINTERNAL", message: err.message } }); } -/** - * Best-effort human message from a failed run. codex-cli.js does not export - * one (unlike cc-plugin-cli's cliMessage), so this route-local helper does - * the same "last non-empty line, prefer a marked one" heuristic. - */ -function cliMessage(result) { - const text = `${result.stderr || ""}\n${result.stdout || ""}`; - const lines = text - .split("\n") - .map((l) => l.trim()) - .filter(Boolean); - const marked = lines.find((l) => l.includes("✘") || l.toLowerCase().includes("error")); - const picked = marked || lines[lines.length - 1] || ""; - return picked.replace(/^✘\s*/, "").slice(0, 1000); -} - /** A non-zero CLI exit is a 422, not a 500 — the caller can inspect stderr. */ function cliFailure(res, result) { return res.status(422).json({ error: { code: "ECLIEXIT", - message: cliMessage(result) || `codex exited with code ${result.code}`, + message: codexCli.cliMessage(result) || `codex exited with code ${result.code}`, }, code: result.code, stdout: result.stdout, @@ -250,10 +199,24 @@ function cliFailure(res, result) { }); } -/** Read-through result for a GET route backed by the CLI. */ +/** + * Read-through result for a GET route backed by `codex … --json`. `result.data` + * is only set when stdout parsed as JSON (see runCodex) — an ok:true result + * with unparseable stdout means the CLI's output shape changed under us, so + * this is a 502, not a 200 with a string where the OpenAPI fragment promises + * an array/object. + */ function cliRead(res, result, key) { if (!result.ok) return cliFailure(res, result); - res.json({ [key]: result.data !== undefined ? result.data : result.stdout }); + if (result.data === undefined) { + return res.status(502).json({ + error: { + code: "ECLIBADOUTPUT", + message: `could not parse \`codex\` JSON output for ${key}`, + }, + }); + } + res.json({ [key]: result.data }); } /** Result for a mutating route: broadcast on success, 422 on CLI failure. */ From aca8fd02eef2d4c789c1ee481dc2990908ac27f0 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 20:15:12 +0700 Subject: [PATCH 14/27] fix(codex): stop 502ing empty CLI output, GET /file no longer mkdirs or leaks paths --- openapi.yaml | 13 ++++++++-- server/__tests__/codex-config.test.js | 37 +++++++++++++++++++++++++++ server/lib/codex-discovery.js | 19 +++++++++++--- server/lib/codex-mutate.js | 10 +++++--- server/openapi-extra/codex-config.js | 5 ++-- server/routes/codex-config.js | 12 ++++++--- 6 files changed, 82 insertions(+), 14 deletions(-) diff --git a/openapi.yaml b/openapi.yaml index d959224b..c6a45277 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -8981,7 +8981,7 @@ paths: tags: - CodexConfig summary: Read a single file body - description: 'Read-only. Returns the body of one file. Strict path containment: the resolved absolute path must live under CODEX_HOME, the project .codex dir, or (only for a file literally named AGENTS.md) the project root.' + description: 'Read-only, no filesystem side effect. Returns the body of one file. Strict path containment: the resolved absolute path must live under CODEX_HOME, the project .codex dir, or (only for a file literally named AGENTS.md) the project root, and must not escape that root through a symlink. Error messages never reveal what does or does not exist outside the root.' operationId: codexConfigGetFile parameters: - name: path @@ -9012,7 +9012,16 @@ paths: example: error: code: READ_DENIED - message: path is outside allowed roots + message: path escapes the allowed root + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/ErrorResponse' + example: + error: + code: NOT_FOUND + message: file not found put: tags: - CodexConfig diff --git a/server/__tests__/codex-config.test.js b/server/__tests__/codex-config.test.js index 3b58ae0f..5593afef 100644 --- a/server/__tests__/codex-config.test.js +++ b/server/__tests__/codex-config.test.js @@ -247,6 +247,26 @@ describe("/api/codex-config", () => { } }); + it("file endpoint 404s on a missing file inside an existing root", async () => { + const target = path.join(FAKE_HOME, "rules", "does-not-exist.rules"); + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(FAKE_PROJECT)}&path=${encodeURIComponent(target)}` + ); + assert.equal(status, 404); + assert.equal(body.error.code, "NOT_FOUND"); + }); + + it("file endpoint 404s on a ?cwd= that doesn't exist yet, and creates nothing", async () => { + const missingCwd = path.join(TMP, `never-existed-${process.hrtime.bigint()}`); + const target = path.join(missingCwd, "AGENTS.md"); + const { status, body } = await fetchJson( + `/api/codex-config/file?cwd=${encodeURIComponent(missingCwd)}&path=${encodeURIComponent(target)}` + ); + assert.equal(status, 404); + assert.equal(body.error.code, "NOT_FOUND"); + assert.equal(fs.existsSync(missingCwd), false, "GET /file must not create the missing root"); + }); + it("file endpoint requires a path", async () => { const { status, body } = await fetchJson("/api/codex-config/file"); assert.equal(status, 400); @@ -450,6 +470,23 @@ describe("/api/codex-config", () => { } }); + it("GET /mcp|/plugins|/marketplaces treat empty stdout as an empty list (fresh-install case), not a 502", async () => { + const restore = stubCodex(""); // exits 0, prints nothing — the normal fresh-install state + try { + for (const [route, key] of [ + ["/api/codex-config/mcp", "mcp"], + ["/api/codex-config/plugins", "plugins"], + ["/api/codex-config/marketplaces", "marketplaces"], + ]) { + const { status, body } = await fetchJson(route); + assert.equal(status, 200, route); + assert.deepEqual(body[key], [], route); + } + } finally { + restore(); + } + }); + it("GET /mcp reports unparseable CLI JSON as 502, not a type-confused 200", async () => { const restore = stubCodex('echo "not json"'); try { diff --git a/server/lib/codex-discovery.js b/server/lib/codex-discovery.js index c3f8b0cd..b2b3a8f7 100644 --- a/server/lib/codex-discovery.js +++ b/server/lib/codex-discovery.js @@ -300,6 +300,16 @@ function readOverview(opts = {}) { * symlink-escape guard the write side (codex-mutate.js) uses — a string-only * `isUnder` check alone would let a symlink dropped anywhere along the path * read arbitrary files outside the root. + * + * A GET has no filesystem side effect: `assertNotSymlinkEscape` is called + * with `{create: false}` so it never mkdirs a caller-chosen root, and a + * missing root or file both come back as `{notFound: true}` rather than the + * write side's `EOUTOFROOT`. Failure messages never include the resolved + * path — a `GET /file` response is not the place to reveal what does or + * does not exist outside the root. + * + * @returns {{ok:true,file:string,size:number,mtime:number,truncated:boolean,text:string} + * | {notFound:true} | {error:string}} */ function readFileSafe(absPath, opts = {}) { const codexHome = getCodexHome(); @@ -323,13 +333,16 @@ function readFileSafe(absPath, opts = {}) { ? projectCodexDir : projectRoot; try { - assertNotSymlinkEscape(trustedRoot, resolved); + assertNotSymlinkEscape(trustedRoot, resolved, { create: false }); } catch (err) { - return { error: err.message }; + // A root that doesn't exist yet (ENOENT from realpathSync, since this + // call never mkdirs it) means there is nothing to find, not a refusal. + if (err.code === "ENOENT") return { notFound: true }; + return { error: "path escapes the allowed root" }; } const r = safeReadText(resolved); - if (!r) return { error: "file not readable" }; + if (!r) return { notFound: true }; return { ok: true, file: resolved, ...r }; } diff --git a/server/lib/codex-mutate.js b/server/lib/codex-mutate.js index c28862c8..2d593e9c 100644 --- a/server/lib/codex-mutate.js +++ b/server/lib/codex-mutate.js @@ -153,15 +153,19 @@ function rmTreeSync(p) { * * `trustedRoot` is created first (mkdirSync recursive — safe, it is never * attacker-controlled) so the walk is guaranteed to bottom out at or below - * it, never wander past it into whatever happens to contain it. + * it, never wander past it into whatever happens to contain it. Pass + * `{create: false}` for a read-only caller (codex-discovery.js's + * readFileSafe): a GET must have no filesystem side effect, so it skips the + * mkdir and instead lets a missing `trustedRoot` surface as the native + * `ENOENT` from `realpathSync` — the caller treats that as "not found". * * ponytail: this check and the write/delete that follows are not atomic — * a symlink swapped in between them could still redirect one write. Closing * that needs O_NOFOLLOW-style fd work with real platform variance; out of * scope for this task (see task-5 fix round 2). */ -function assertNotSymlinkEscape(trustedRoot, target) { - fs.mkdirSync(trustedRoot, { recursive: true }); +function assertNotSymlinkEscape(trustedRoot, target, opts = {}) { + if (opts.create !== false) fs.mkdirSync(trustedRoot, { recursive: true }); const realRoot = fs.realpathSync(trustedRoot); let dir = target; diff --git a/server/openapi-extra/codex-config.js b/server/openapi-extra/codex-config.js index 46de53ab..091c71ea 100644 --- a/server/openapi-extra/codex-config.js +++ b/server/openapi-extra/codex-config.js @@ -370,7 +370,7 @@ const paths = { tags: ["CodexConfig"], summary: "Read a single file body", description: - "Read-only. Returns the body of one file. Strict path containment: the resolved absolute path must live under CODEX_HOME, the project .codex dir, or (only for a file literally named AGENTS.md) the project root.", + "Read-only, no filesystem side effect. Returns the body of one file. Strict path containment: the resolved absolute path must live under CODEX_HOME, the project .codex dir, or (only for a file literally named AGENTS.md) the project root, and must not escape that root through a symlink. Error messages never reveal what does or does not exist outside the root.", operationId: "codexConfigGetFile", parameters: [ { @@ -391,7 +391,8 @@ const paths = { }, }, }, - 400: errorResponse("READ_DENIED", "path is outside allowed roots"), + 400: errorResponse("READ_DENIED", "path escapes the allowed root"), + 404: errorResponse("NOT_FOUND", "file not found"), }, }, put: { diff --git a/server/routes/codex-config.js b/server/routes/codex-config.js index ade362a3..896a9e7b 100644 --- a/server/routes/codex-config.js +++ b/server/routes/codex-config.js @@ -107,6 +107,8 @@ router.get("/file", (req, res) => { return res.status(400).json({ error: { code: "BAD_PATH", message: "path is required" } }); } const result = codexDiscovery.readFileSafe(p, { cwd: cwdOf(req) }); + if (result.notFound) + return res.status(404).json({ error: { code: "NOT_FOUND", message: "file not found" } }); if (result.error) return res.status(400).json({ error: { code: "READ_DENIED", message: result.error } }); res.json(result); @@ -201,14 +203,16 @@ function cliFailure(res, result) { /** * Read-through result for a GET route backed by `codex … --json`. `result.data` - * is only set when stdout parsed as JSON (see runCodex) — an ok:true result - * with unparseable stdout means the CLI's output shape changed under us, so - * this is a 502, not a 200 with a string where the OpenAPI fragment promises - * an array/object. + * is only set when stdout parsed as JSON (see runCodex). A fresh install with + * zero MCP servers/plugins/marketplaces is the normal case and prints nothing + * — that's an empty list, not a parse failure. Only stdout that has content + * and still doesn't parse (the CLI's output shape changed under us) is a 502, + * never a 200 with a string where the OpenAPI fragment promises an array/object. */ function cliRead(res, result, key) { if (!result.ok) return cliFailure(res, result); if (result.data === undefined) { + if (!result.stdout.trim()) return res.json({ [key]: [] }); return res.status(502).json({ error: { code: "ECLIBADOUTPUT", From 2ae9b06652ab91b8ceac8c1decbf9eb76aa849be Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 20:20:59 +0700 Subject: [PATCH 15/27] feat(codex): add the codexConfig API namespace Adds a typed client wrapper for the Codex config API alongside ccConfig, with methods for reading/writing skills, agents, rules, memory, config, MCP servers, plugins, and marketplaces. Mirrors ccConfig construction and error handling. All response shapes match the server's wire format. --- .../src/lib/__tests__/codexConfigApi.test.ts | 47 +++ client/src/lib/api.ts | 319 ++++++++++++++++++ 2 files changed, 366 insertions(+) create mode 100644 client/src/lib/__tests__/codexConfigApi.test.ts diff --git a/client/src/lib/__tests__/codexConfigApi.test.ts b/client/src/lib/__tests__/codexConfigApi.test.ts new file mode 100644 index 00000000..89bdf008 --- /dev/null +++ b/client/src/lib/__tests__/codexConfigApi.test.ts @@ -0,0 +1,47 @@ +/** + * @file Pins the codexConfig API routes and payloads. Tests that the client + * sends the right method, URL, and body shape to the Codex config backend. + * @author Nguyễn Ngọc Trí Vĩ + */ + +import { describe, it, expect, vi, afterEach } from "vitest"; +import { api } from "../api"; + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +function stubFetch(response: unknown) { + const spy = vi.fn().mockResolvedValue({ + ok: true, + status: 200, + headers: { get: () => "application/json" }, + json: async () => response, + text: async () => JSON.stringify(response), + }); + vi.stubGlobal("fetch", spy); + return spy; +} + +describe("api.codexConfig", () => { + it("sends a write as a single PUT with the artifact envelope", async () => { + const spy = stubFetch({ ok: true, file: "/x", created: true }); + + await api.codexConfig.write({ + scope: "user", + type: "rules", + name: "default", + content: "allow", + }); + + expect(spy).toHaveBeenCalledTimes(1); + const [, options] = spy.mock.calls[0] as [string, RequestInit | undefined]; + expect(options?.method).toBe("PUT"); + expect(JSON.parse(options?.body as string)).toEqual({ + scope: "user", + type: "rules", + name: "default", + content: "allow", + }); + }); +}); diff --git a/client/src/lib/api.ts b/client/src/lib/api.ts index 03b29cc9..ad6c5844 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -1609,6 +1609,162 @@ export const api = { }), }, + // ─────────────────────────────── Codex-Config API ────────────────────────────── + /** Read/write access to on-disk Codex CLI configuration - skills, agents, + * rules, memory (AGENTS.md), config.toml, MCP servers, plugins, and + * marketplaces - for the dashboard's "Codex Config" explorer/editor pages. */ + codexConfig: { + /** + * GET /api/codex-config/overview - counts of every artifact kind, for the + * explorer's landing page. + * @returns {@link CodexOverview} — filesystem roots plus per-kind counts. + */ + overview: () => request("/codex-config/overview"), + /** + * GET /api/codex-config/skills - user and/or project skill definitions. + * + * The optional `scope` is appended as `?scope=` only when provided; omitting + * it lets the server apply its default scope. + * + * @param scope Optional {@link CodexScope} ("user"|"project"|"all") filter. + * @returns `{ skills }` — the {@link CodexSkill} items for each skill. + */ + skills: (scope?: CodexScope) => + request<{ skills: CodexSkill[] }>(`/codex-config/skills${scope ? `?scope=${scope}` : ""}`), + /** + * GET /api/codex-config/agents - user and/or project agent definitions. + * @param scope Optional {@link CodexScope} filter. + * @returns `{ agents }` — {@link CodexAgent} items for each agent. + */ + agents: (scope?: CodexScope) => + request<{ agents: CodexAgent[] }>(`/codex-config/agents${scope ? `?scope=${scope}` : ""}`), + /** + * GET /api/codex-config/rules - user and/or project rules files. + * @param scope Optional {@link CodexScope} filter. + * @returns `{ rules }` — {@link CodexRule} items for each rule file. + */ + rules: (scope?: CodexScope) => + request<{ rules: CodexRule[] }>(`/codex-config/rules${scope ? `?scope=${scope}` : ""}`), + /** + * GET /api/codex-config/memory - AGENTS.md files at both scopes. + * @returns `{ memory }` — {@link CodexMemoryFile} items. + */ + memory: () => request<{ memory: CodexMemoryFile[] }>("/codex-config/memory"), + /** + * GET /api/codex-config/config - config.toml files at both scopes. + * @returns `{ config }` — {@link CodexConfigFile} items. + */ + config: () => request<{ config: CodexConfigFile[] }>("/codex-config/config"), + /** + * GET /api/codex-config/file - raw contents of one config file by absolute path. + * + * The absolute path is passed as a URL-encoded `path` query param (not a + * path segment) so arbitrary filesystem paths survive intact. + * + * @param path Absolute path of the config file to read. + * @returns {@link CodexFileResponse} — file text + metadata. + */ + file: (path: string) => + request(`/codex-config/file?path=${encodeURIComponent(path)}`), + /** + * PUT /api/codex-config/file - create/overwrite a config artifact; the + * server writes a backup of any previous content first. + * + * @param args {@link CodexWriteArgs} — scope/type/name/content. + * @returns {@link CodexMutationResult} — the written path and whether a new file was created. + */ + write: (args: CodexWriteArgs) => + request("/codex-config/file", { + method: "PUT", + body: JSON.stringify(args), + }), + /** + * DELETE /api/codex-config/file - remove a config artifact (also backed up). + * + * @param args {@link CodexDeleteArgs} — which artifact to delete. + * @returns {@link CodexMutationResult} — the deleted path and its backup path. + */ + remove: (args: CodexDeleteArgs) => + request("/codex-config/file", { + method: "DELETE", + body: JSON.stringify(args), + }), + /** + * GET /api/codex-config/backups - timestamped backups written by `write`/ + * `remove`, optionally filtered by scope/artifact type. + * + * @param params Optional `{ scope, type }` filter for the backup listing. + * @returns `{ items }` — the matching {@link CodexBackup} entries. + */ + backups: (params?: { scope?: "user" | "project"; type?: CodexArtifactType }) => + requestCodexBackupsHelper(params), + /** + * GET /api/codex-config/mcp - configured MCP servers from the codex CLI. + * @returns `{ mcp }` — {@link CodexMcpServer} items. + */ + mcp: () => request<{ mcp: CodexMcpServer[] }>("/codex-config/mcp"), + /** + * GET /api/codex-config/plugins - installed plugins from the codex CLI. + * @returns `{ plugins }` — {@link CodexPlugin} items. + */ + plugins: () => request<{ plugins: CodexPlugin[] }>("/codex-config/plugins"), + /** + * GET /api/codex-config/marketplaces - registered marketplaces from the codex CLI. + * @returns `{ marketplaces }` — {@link CodexMarketplace} items. + */ + marketplaces: () => request<{ marketplaces: CodexMarketplace[] }>("/codex-config/marketplaces"), + /** + * POST /api/codex-config/mcp/add - add an MCP server via the `codex` CLI. + * @param args Arguments for `codex mcp add`. + * @returns `{ ok, stdout, stderr }` — success and CLI output. + */ + mcpAdd: (args: unknown) => + request<{ ok: true; stdout: string; stderr: string }>("/codex-config/mcp/add", { + method: "POST", + body: JSON.stringify(args), + }), + /** + * POST /api/codex-config/mcp/remove - remove an MCP server via the `codex` CLI. + * @param name The MCP server name to remove. + * @returns `{ ok, stdout, stderr }` — success and CLI output. + */ + mcpRemove: (name: string) => + request<{ ok: true; stdout: string; stderr: string }>("/codex-config/mcp/remove", { + method: "POST", + body: JSON.stringify({ name }), + }), + /** + * POST /api/codex-config/plugins/action - add or remove a plugin via the `codex` CLI. + * @param args Arguments with `spec` and `action` fields. + * @returns `{ ok, stdout, stderr }` — success and CLI output. + */ + pluginAction: (args: unknown) => + request<{ ok: true; stdout: string; stderr: string }>("/codex-config/plugins/action", { + method: "POST", + body: JSON.stringify(args), + }), + /** + * POST /api/codex-config/marketplaces/add - register a marketplace via the `codex` CLI. + * @param url The marketplace URL to register. + * @returns `{ ok, stdout, stderr }` — success and CLI output. + */ + marketplaceAdd: (url: string) => + request<{ ok: true; stdout: string; stderr: string }>("/codex-config/marketplaces/add", { + method: "POST", + body: JSON.stringify({ url }), + }), + /** + * POST /api/codex-config/marketplaces/action - update or remove a marketplace via the `codex` CLI. + * @param args Arguments with `name` and `action` fields. + * @returns `{ ok, stdout, stderr }` — success and CLI output. + */ + marketplaceAction: (args: unknown) => + request<{ ok: true; stdout: string; stderr: string }>("/codex-config/marketplaces/action", { + method: "POST", + body: JSON.stringify(args), + }), + }, + // ────────────────────────────────── Run API ───────────────────────────────── /** Spawn/manage headless or conversational `claude` CLI child processes * launched from the dashboard's Run page, and stream their output. */ @@ -2230,6 +2386,23 @@ function requestBackupsHelper(params?: { scope?: "user" | "project"; type?: CcAr return request<{ items: CcBackup[] }>(`/cc-config/backups${q ? `?${q}` : ""}`); } +/** Helper for GET /api/codex-config/backups with optional `scope` and `type` filters. + * Builds an optional `?scope=&type=` query string (each part appended only when + * present) and calls {@link request} for GET /api/codex-config/backups. + * + * @param params Optional `{ scope, type }` filter for the backup listing. + * @returns `{ items }` — the matching {@link CodexBackup} entries. */ +function requestCodexBackupsHelper(params?: { + scope?: "user" | "project"; + type?: CodexArtifactType; +}) { + const qs = new URLSearchParams(); + if (params?.scope) qs.set("scope", params.scope); + if (params?.type) qs.set("type", params.type); + const q = qs.toString(); + return request<{ items: CodexBackup[] }>(`/codex-config/backups${q ? `?${q}` : ""}`); +} + // ───────────────────────────────────────────────────────────────────────────── // CC-Config types — request/response shapes for the "CC Config" explorer/editor. // These describe on-disk Claude Code configuration artifacts (skills, agents, @@ -2675,6 +2848,152 @@ export interface CcHookScripts { items: { name: string; file: string; size: number; mtime: number }[]; } +// ───────────────────────────────────────────────────────────────────────────── +// Codex-Config types — request/response shapes for the "Codex Config" explorer/editor. +// These describe on-disk Codex CLI configuration artifacts (skills, agents, rules, +// memory, config, MCP servers, plugins, marketplaces) as surfaced by the +// /api/codex-config/* routes. They live in this client because they are specific +// to the explorer UI. +// ───────────────────────────────────────────────────────────────────────────── + +/** Kind of Codex config artifact manageable via `api.codexConfig.write`/ + * `remove` - each maps to a distinct on-disk location. */ +export type CodexArtifactType = "skills" | "agents" | "rules" | "memory" | "config"; + +/** Scope filter accepted by most `codexConfig` list endpoints; "all" merges + * user + project scope in one response. */ +export type CodexScope = "user" | "project" | "all"; + +/** Body for PUT /api/codex-config/file - create or overwrite one artifact. */ +export interface CodexWriteArgs { + scope: "user" | "project"; + type: CodexArtifactType; + /** Artifact name (e.g. skill/agent/rule name); omitted for singleton + * artifacts like memory/config. */ + name?: string; + /** Full file contents to write. */ + content: string; +} + +/** Body for DELETE /api/codex-config/file - remove one artifact. */ +export interface CodexDeleteArgs { + scope: "user" | "project"; + type: CodexArtifactType; + name?: string; +} + +/** Response shape of a successful `codexConfig.write`/`remove` call. */ +export interface CodexMutationResult { + file: string; + created?: boolean; + backup?: { [key: string]: unknown } | null; +} + +/** One timestamped backup of a config artifact, from GET /api/codex-config/backups. */ +export interface CodexBackup { + type: string; + name: string; + file: string; + mtime: number; + size: number | null; +} + +/** A discovered skill: a directory containing a SKILL.md under skills//. */ +export interface CodexSkill { + scope: "user" | "project"; + name: string; + path: string; + file: string; + size: number; + mtime: number; + truncated: boolean; + frontmatter: Record; + preview: string; +} + +/** A discovered agent definition (TOML file). */ +export interface CodexAgent { + scope: "user" | "project"; + name: string; + file: string; + size: number; + mtime: number; + meta: Record; + preview: string; +} + +/** A discovered rules file. */ +export interface CodexRule { + scope: "user" | "project"; + name: string; + file: string; + size: number; + mtime: number; + preview: string; +} + +/** AGENTS.md file at either user or project scope. */ +export interface CodexMemoryFile { + scope: "user" | "project"; + file: string; + exists: boolean; + size?: number; + mtime?: number; + preview?: string; +} + +/** config.toml file at either user or project scope. */ +export interface CodexConfigFile { + scope: "user" | "project"; + file: string; + exists: boolean; + size?: number; + mtime?: number; + content?: string; + truncated?: boolean; +} + +/** Response shape of GET /api/codex-config/file - full contents of one config artifact. */ +export interface CodexFileResponse { + ok: true; + file: string; + size: number; + mtime: number; + truncated: boolean; + text: string; +} + +/** Response shape of GET /api/codex-config/overview. */ +export interface CodexOverview { + roots: { + codexHome: string; + projectCodexDir: string; + projectRoot: string; + }; + counts: { + skills: { user: number; project: number }; + agents: { user: number; project: number }; + rules: { user: number; project: number }; + memory: { user: number; project: number }; + config: { user: number; project: number }; + }; +} + +/** One configured MCP server, from GET /api/codex-config/mcp. */ +export interface CodexMcpServer { + [key: string]: unknown; +} + +/** One installed plugin, from GET /api/codex-config/plugins. */ +export interface CodexPlugin { + [key: string]: unknown; +} + +/** One registered marketplace, from GET /api/codex-config/marketplaces. */ +export interface CodexMarketplace { + [key: string]: unknown; +} + // ───────────────────────────────────────────────────────────────────────────── // Run types — request/response shapes for the Run page's `claude` process // spawning/management. `RunMode`/`RunStatus`/`PermissionMode`/`EffortLevel` From 8021e569f2986c218fe6883365ecb0785faf7a5f Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 20:27:52 +0700 Subject: [PATCH 16/27] fix(codex): add array coercion and typed arguments for CLI-backed routes - Coerce mcp/plugins/marketplaces GET responses to arrays (CLI may emit object), degrading to empty list rather than crash on unexpected shape - Add typed interfaces: CodexMcpAddArgs, CodexPluginActionArgs, CodexMarketplaceActionArgs with validated action literals - Mirrors ccConfig's type safety across the namespace --- client/src/lib/api.ts | 52 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 43 insertions(+), 9 deletions(-) diff --git a/client/src/lib/api.ts b/client/src/lib/api.ts index ad6c5844..592ab231 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -1700,25 +1700,37 @@ export const api = { requestCodexBackupsHelper(params), /** * GET /api/codex-config/mcp - configured MCP servers from the codex CLI. + * Coerces the CLI's JSON output (which may be an object) to an array. * @returns `{ mcp }` — {@link CodexMcpServer} items. */ - mcp: () => request<{ mcp: CodexMcpServer[] }>("/codex-config/mcp"), + mcp: async () => { + const res = await request<{ mcp: unknown }>("/codex-config/mcp"); + return { mcp: Array.isArray(res.mcp) ? res.mcp : [] }; + }, /** * GET /api/codex-config/plugins - installed plugins from the codex CLI. + * Coerces the CLI's JSON output (which may be an object) to an array. * @returns `{ plugins }` — {@link CodexPlugin} items. */ - plugins: () => request<{ plugins: CodexPlugin[] }>("/codex-config/plugins"), + plugins: async () => { + const res = await request<{ plugins: unknown }>("/codex-config/plugins"); + return { plugins: Array.isArray(res.plugins) ? res.plugins : [] }; + }, /** * GET /api/codex-config/marketplaces - registered marketplaces from the codex CLI. + * Coerces the CLI's JSON output (which may be an object) to an array. * @returns `{ marketplaces }` — {@link CodexMarketplace} items. */ - marketplaces: () => request<{ marketplaces: CodexMarketplace[] }>("/codex-config/marketplaces"), + marketplaces: async () => { + const res = await request<{ marketplaces: unknown }>("/codex-config/marketplaces"); + return { marketplaces: Array.isArray(res.marketplaces) ? res.marketplaces : [] }; + }, /** * POST /api/codex-config/mcp/add - add an MCP server via the `codex` CLI. - * @param args Arguments for `codex mcp add`. + * @param args {@link CodexMcpAddArgs} for `codex mcp add`. * @returns `{ ok, stdout, stderr }` — success and CLI output. */ - mcpAdd: (args: unknown) => + mcpAdd: (args: CodexMcpAddArgs) => request<{ ok: true; stdout: string; stderr: string }>("/codex-config/mcp/add", { method: "POST", body: JSON.stringify(args), @@ -1735,10 +1747,10 @@ export const api = { }), /** * POST /api/codex-config/plugins/action - add or remove a plugin via the `codex` CLI. - * @param args Arguments with `spec` and `action` fields. + * @param args {@link CodexPluginActionArgs} with `spec` and `action` fields. * @returns `{ ok, stdout, stderr }` — success and CLI output. */ - pluginAction: (args: unknown) => + pluginAction: (args: CodexPluginActionArgs) => request<{ ok: true; stdout: string; stderr: string }>("/codex-config/plugins/action", { method: "POST", body: JSON.stringify(args), @@ -1755,10 +1767,10 @@ export const api = { }), /** * POST /api/codex-config/marketplaces/action - update or remove a marketplace via the `codex` CLI. - * @param args Arguments with `name` and `action` fields. + * @param args {@link CodexMarketplaceActionArgs} with `name` and `action` fields. * @returns `{ ok, stdout, stderr }` — success and CLI output. */ - marketplaceAction: (args: unknown) => + marketplaceAction: (args: CodexMarketplaceActionArgs) => request<{ ok: true; stdout: string; stderr: string }>("/codex-config/marketplaces/action", { method: "POST", body: JSON.stringify(args), @@ -2994,6 +3006,28 @@ export interface CodexMarketplace { [key: string]: unknown; } +/** Arguments for POST /api/codex-config/mcp/add. */ +export interface CodexMcpAddArgs { + name: string; + url?: string; + bearerTokenEnvVar?: string; + command?: string; + args?: string[]; + env?: Record; +} + +/** Arguments for POST /api/codex-config/plugins/action. */ +export interface CodexPluginActionArgs { + spec: string; + action: "add" | "remove"; +} + +/** Arguments for POST /api/codex-config/marketplaces/action. */ +export interface CodexMarketplaceActionArgs { + name: string; + action: "remove"; +} + // ───────────────────────────────────────────────────────────────────────────── // Run types — request/response shapes for the Run page's `claude` process // spawning/management. `RunMode`/`RunStatus`/`PermissionMode`/`EffortLevel` From e38dc5555cf19be274c02b2e5b4fb09564b19d42 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 20:31:10 +0700 Subject: [PATCH 17/27] fix(codex): keep element types through the array coercion, model mcpAdd as a union --- client/src/lib/api.ts | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/client/src/lib/api.ts b/client/src/lib/api.ts index 592ab231..ac93133d 100644 --- a/client/src/lib/api.ts +++ b/client/src/lib/api.ts @@ -1703,27 +1703,31 @@ export const api = { * Coerces the CLI's JSON output (which may be an object) to an array. * @returns `{ mcp }` — {@link CodexMcpServer} items. */ - mcp: async () => { + mcp: async (): Promise<{ mcp: CodexMcpServer[] }> => { const res = await request<{ mcp: unknown }>("/codex-config/mcp"); - return { mcp: Array.isArray(res.mcp) ? res.mcp : [] }; + return { mcp: Array.isArray(res.mcp) ? (res.mcp as CodexMcpServer[]) : [] }; }, /** * GET /api/codex-config/plugins - installed plugins from the codex CLI. * Coerces the CLI's JSON output (which may be an object) to an array. * @returns `{ plugins }` — {@link CodexPlugin} items. */ - plugins: async () => { + plugins: async (): Promise<{ plugins: CodexPlugin[] }> => { const res = await request<{ plugins: unknown }>("/codex-config/plugins"); - return { plugins: Array.isArray(res.plugins) ? res.plugins : [] }; + return { plugins: Array.isArray(res.plugins) ? (res.plugins as CodexPlugin[]) : [] }; }, /** * GET /api/codex-config/marketplaces - registered marketplaces from the codex CLI. * Coerces the CLI's JSON output (which may be an object) to an array. * @returns `{ marketplaces }` — {@link CodexMarketplace} items. */ - marketplaces: async () => { + marketplaces: async (): Promise<{ marketplaces: CodexMarketplace[] }> => { const res = await request<{ marketplaces: unknown }>("/codex-config/marketplaces"); - return { marketplaces: Array.isArray(res.marketplaces) ? res.marketplaces : [] }; + return { + marketplaces: Array.isArray(res.marketplaces) + ? (res.marketplaces as CodexMarketplace[]) + : [], + }; }, /** * POST /api/codex-config/mcp/add - add an MCP server via the `codex` CLI. @@ -3006,15 +3010,15 @@ export interface CodexMarketplace { [key: string]: unknown; } -/** Arguments for POST /api/codex-config/mcp/add. */ -export interface CodexMcpAddArgs { - name: string; - url?: string; - bearerTokenEnvVar?: string; - command?: string; - args?: string[]; - env?: Record; -} +/** + * Arguments for POST /api/codex-config/mcp/add. A union, not one optional-heavy + * shape: `codex mcp add` takes either a URL or a command and rejects both or + * neither, and `env` is stdio-only. Modelling that here turns three of the + * server's `EBADARG` responses into compile errors. + */ +export type CodexMcpAddArgs = + | { name: string; url: string; bearerTokenEnvVar?: string } + | { name: string; command: string; args?: string[]; env?: Record }; /** Arguments for POST /api/codex-config/plugins/action. */ export interface CodexPluginActionArgs { From 865c6c1d1e44e0dca3dc234767c8c219de728172 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 20:40:47 +0700 Subject: [PATCH 18/27] refactor(config): lift the tool-agnostic config modals out of CcConfig Moves FileViewer, EditorModal, ConfirmDeleteModal, ToastNotice, MdItemList, ScopeBadge, and the shared atoms (Empty/SkeletonRows/CopyButton/formatBytes) into client/src/components/config/ so the upcoming Codex config panel can reuse them. EditorModal now takes a fetchContent callback instead of calling api.ccConfig.file itself; CcConfig.tsx supplies the Claude-specific fetcher. The moved EditorState/ConfirmDeleteState type field widens from CcArtifactType to string so non-Claude callers can use them too, with a cast at the two remaining Claude-specific call sites in CcConfig.tsx. --- .../components/config/ConfirmDeleteModal.tsx | 89 +++ client/src/components/config/EditorModal.tsx | 237 +++++++ client/src/components/config/FileViewer.tsx | 77 +++ client/src/components/config/MdItemList.tsx | 145 ++++ client/src/components/config/ScopeBadge.tsx | 32 + client/src/components/config/ToastNotice.tsx | 46 ++ .../config/__tests__/EditorModal.test.tsx | 48 ++ .../components/config/configPrimitives.tsx | 59 ++ client/src/pages/CcConfig.tsx | 640 +----------------- 9 files changed, 759 insertions(+), 614 deletions(-) create mode 100644 client/src/components/config/ConfirmDeleteModal.tsx create mode 100644 client/src/components/config/EditorModal.tsx create mode 100644 client/src/components/config/FileViewer.tsx create mode 100644 client/src/components/config/MdItemList.tsx create mode 100644 client/src/components/config/ScopeBadge.tsx create mode 100644 client/src/components/config/ToastNotice.tsx create mode 100644 client/src/components/config/__tests__/EditorModal.test.tsx create mode 100644 client/src/components/config/configPrimitives.tsx diff --git a/client/src/components/config/ConfirmDeleteModal.tsx b/client/src/components/config/ConfirmDeleteModal.tsx new file mode 100644 index 00000000..43786f62 --- /dev/null +++ b/client/src/components/config/ConfirmDeleteModal.tsx @@ -0,0 +1,89 @@ +/** + * @file ConfirmDeleteModal.tsx + * @description Confirmation modal for deleting a config artifact. Tool- + * agnostic — the caller supplies the async delete action; the modal only + * tracks the busy state and Escape-to-cancel behavior. + * @author Nguyễn Ngọc Trí Vĩ + */ + +import { useCallback, useEffect, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { ShieldAlert, Trash2 } from "lucide-react"; + +export type ConfirmDeleteState = { + type: string; + scope: "user" | "project" | "auto-memory"; + name?: string; + path: string; + project?: string; // set for type === "auto-memory" +} | null; + +interface ConfirmDeleteModalProps { + state: NonNullable; + onCancel: () => void; + onConfirm: () => Promise; +} + +export function ConfirmDeleteModal({ state, onCancel, onConfirm }: ConfirmDeleteModalProps) { + const { t } = useTranslation("ccConfig"); + const [busy, setBusy] = useState(false); + + useEffect(() => { + const onKey = (e: KeyboardEvent) => { + if (e.key === "Escape") onCancel(); + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [onCancel]); + + const handleConfirm = useCallback(async () => { + setBusy(true); + try { + await onConfirm(); + } finally { + setBusy(false); + } + }, [onConfirm]); + + return ( +
+
e.stopPropagation()} + > +
+ + + {t("edit.confirmDelete")} + +
+
+

{t("edit.confirmDeleteBody")}

+
+ {t("edit.confirmDeletePath", { path: state.path })} +
+
+
+ + +
+
+
+ ); +} diff --git a/client/src/components/config/EditorModal.tsx b/client/src/components/config/EditorModal.tsx new file mode 100644 index 00000000..7ea3305c --- /dev/null +++ b/client/src/components/config/EditorModal.tsx @@ -0,0 +1,237 @@ +/** + * @file EditorModal.tsx + * @description Create/edit modal for text-file config artifacts (skills, + * agents, commands, output styles, memory). Tool-agnostic: it takes a + * `fetchContent` callback instead of reaching for any particular tool's API, + * so the same modal serves both the Claude Code and Codex config pages. + * @author Nguyễn Ngọc Trí Vĩ + */ + +import { useCallback, useEffect, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { Pencil, X, Save, AlertCircle } from "lucide-react"; + +export type EditorState = + | { + mode: "create"; + type: string; + defaultScope: "user" | "project"; + template: string; + project?: string; // set for type === "auto-memory" + } + | { + mode: "edit"; + type: string; + scope: "user" | "project" | "auto-memory"; + name: string; + filePath: string; + project?: string; // set for type === "auto-memory" + } + | null; + +interface EditorModalProps { + state: NonNullable; + fetchContent: (filePath: string) => Promise; + onClose: () => void; + onSave: (args: { + type: string; + targetScope: "user" | "project" | "auto-memory"; + name: string | undefined; + content: string; + project?: string; + }) => Promise; +} + +export function EditorModal({ state, fetchContent, onClose, onSave }: EditorModalProps) { + const { t } = useTranslation("ccConfig"); + const isCreate = state.mode === "create"; + const isAutoMemory = state.type === "auto-memory"; + const [content, setContent] = useState(isCreate ? state.template : ""); + const [name, setName] = useState(""); + const [targetScope, setTargetScope] = useState<"user" | "project">( + isCreate ? state.defaultScope : state.scope === "auto-memory" ? "user" : state.scope + ); + const [loading, setLoading] = useState(!isCreate); + const [saving, setSaving] = useState(false); + const [error, setError] = useState(null); + + // For edit mode, fetch the actual file content + useEffect(() => { + if (state.mode === "edit") { + setLoading(true); + fetchContent(state.filePath) + .then((text) => { + setContent(text); + setLoading(false); + }) + .catch((err: unknown) => { + const msg = err instanceof Error ? err.message : "unknown"; + setError(msg); + setLoading(false); + }); + } + }, [state, fetchContent]); + + // Esc to close + useEffect(() => { + const onKey = (e: KeyboardEvent) => { + if (e.key === "Escape") onClose(); + }; + window.addEventListener("keydown", onKey); + return () => window.removeEventListener("keydown", onKey); + }, [onClose]); + + const handleSave = useCallback(async () => { + setSaving(true); + setError(null); + try { + if (state.type !== "memory" && state.mode === "create" && !name) { + setError(t("edit.nameLabel")); + setSaving(false); + return; + } + // For auto-memory creates, append a .md extension when the user omits it. + const createName = isAutoMemory && !/\.md$/i.test(name) ? `${name}.md` : name; + const effectiveName = + state.mode === "edit" ? state.name : state.type === "memory" ? undefined : createName; + await onSave({ + type: state.type, + targetScope: isAutoMemory ? "auto-memory" : targetScope, + name: effectiveName, + content, + project: state.project, + }); + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : "unknown"; + setError(t("edit.writeError", { message: msg })); + } finally { + setSaving(false); + } + }, [state, targetScope, name, content, isAutoMemory, onSave, t]); + + const titleText = isCreate + ? isAutoMemory + ? t("memory.newFileTitle", { project: state.project ?? "" }) + : t("edit.newTitle", { type: state.type }) + : t("edit.editTitle", { name: state.mode === "edit" ? state.name : "" }); + + return ( +
+
e.stopPropagation()} + > +
+ + {titleText} + +
+ +
+ {isCreate && state.type !== "memory" && ( +
+
+ + setName(e.target.value)} + placeholder={ + isAutoMemory ? t("memory.namePlaceholder") : t("edit.namePlaceholder") + } + pattern={isAutoMemory ? undefined : "[A-Za-z0-9][A-Za-z0-9._-]{0,63}"} + className="w-full bg-surface-2 border border-border rounded-md px-3 py-1.5 text-sm font-mono text-fg-primary placeholder:text-fg-muted focus:outline-none focus:border-accent/50" + /> +

+ {isAutoMemory ? t("memory.nameHelp") : t("edit.nameHelp")} +

+
+ {isAutoMemory ? ( +
+ +
+ {state.project} +
+

{t("memory.projectHelp")}

+
+ ) : ( +
+ +
+ {(["user", "project"] as const).map((s) => ( + + ))} +
+
+ )} +
+ )} + +
+ + {loading ? ( +
+ ) : ( +