From 236e94760f7eec855331dea892950bed6f1b6ab8 Mon Sep 17 00:00:00 2001 From: nntrivi2001 Date: Tue, 8 Sep 2026 16:48:45 +0700 Subject: [PATCH] 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;