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.
This commit is contained in:
2026-09-08 16:48:45 +07:00
parent 98b1921ca6
commit 236e94760f
2 changed files with 44 additions and 12 deletions
+29 -10
View File
@@ -93,6 +93,11 @@ describe("codex-discovery", () => {
assert.equal(meta.description, "uses # for headings"); 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", () => { it("reads a rule file's preview", () => {
write( write(
path.join(TMP, "rules", "no-force-push.rules"), path.join(TMP, "rules", "no-force-push.rules"),
@@ -127,19 +132,33 @@ describe("codex-discovery", () => {
assert.equal(project.exists, false); 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-")); const outside = fs.mkdtempSync(path.join(os.tmpdir(), "codex-disc-outside-"));
write(path.join(outside, "SKILL.md"), "---\nname: linked\n---\nbody"); 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] = cx.readSkills({ cwd: REPO, scope: "user" });
const linked = rows.find((r) => r.name === "linked"); assert.equal(linked.scope, "user");
assert.ok(linked, "a symlinked skill directory must still be discovered"); assert.equal(linked.name, "linked");
assert.equal(linked.frontmatter.name, "linked"); assert.equal(linked.frontmatter.name, "linked", "frontmatter must be read through the link");
for (const row of rows) { assert.equal(linked.file, path.join(linkPath, "SKILL.md"));
assert.ok(row.file.startsWith(TMP) || row.file.startsWith(REPO)); fs.rmSync(outside, { recursive: true, force: true });
assert.ok(!row.file.startsWith(outside)); });
}
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 }); fs.rmSync(outside, { recursive: true, force: true });
}); });
+15 -2
View File
@@ -61,6 +61,8 @@ function readSkillsAt(scope, codexDir) {
const skills = []; const skills = [];
for (const ent of listDir(dir)) { for (const ent of listDir(dir)) {
const skillDir = path.join(dir, ent.name); 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; if (!isDirLike(ent, skillDir) || !isUnder(codexDir, skillDir)) continue;
const skillFile = path.join(skillDir, "SKILL.md"); const skillFile = path.join(skillDir, "SKILL.md");
const read = safeReadText(skillFile); const read = safeReadText(skillFile);
@@ -124,9 +126,16 @@ function readTomlMeta(text) {
value = value.slice(1, -1); value = value.slice(1, -1);
} else if (value.startsWith('"') || value.startsWith("'")) { } else if (value.startsWith('"') || value.startsWith("'")) {
// A quoted value with a trailing `# comment` outside the string — // 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 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); if (closeIdx > 0) value = value.slice(1, closeIdx);
else continue; else continue;
} else if (/^(true|false|-?\d+(\.\d+)?)\s*(#.*)?$/.test(value)) { } else if (/^(true|false|-?\d+(\.\d+)?)\s*(#.*)?$/.test(value)) {
@@ -146,6 +155,8 @@ function readAgentsAt(scope, codexDir) {
for (const ent of listDir(dir)) { for (const ent of listDir(dir)) {
if (!ent.name.endsWith(".toml")) continue; if (!ent.name.endsWith(".toml")) continue;
const file = path.join(dir, ent.name); 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; if (!isFileLike(ent, file) || !isUnder(codexDir, file)) continue;
const read = safeReadText(file); const read = safeReadText(file);
if (!read) continue; if (!read) continue;
@@ -177,6 +188,8 @@ function readRulesAt(scope, codexDir) {
for (const ent of listDir(dir)) { for (const ent of listDir(dir)) {
if (!ent.name.endsWith(".rules")) continue; if (!ent.name.endsWith(".rules")) continue;
const file = path.join(dir, ent.name); 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; if (!isFileLike(ent, file) || !isUnder(codexDir, file)) continue;
const read = safeReadText(file); const read = safeReadText(file);
if (!read) continue; if (!read) continue;