diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index 2216ea7f..bd302284 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -377,6 +377,21 @@ export function applyWorktreePatch(repoRoot, worktreePath, baseCommit) { if (!fs.existsSync(patchPath) || fs.statSync(patchPath).size === 0) { return { applied: false, detail: "No tracked changes to apply." }; } + // SECURITY (#15): a symlink created in the worktree (ln -s ~/.ssh/authorized_keys + // ./steal) is captured as a mode-120000 diff entry with the target verbatim. + // `git apply --index` would recreate the symlink in repoRoot pointing at the + // attacker-chosen host path → exfil/append primitive. git apply rejects literal + // "../" filename paths, but symlink-mode entries bypass that guard (the repo-side + // filename is normal; only the symlink target points outside). Reject the patch + // if any mode-120000 line appears. Leave-branch preserved: worktree stays for + // manual inspection. + const patchContent = fs.readFileSync(patchPath, "utf8"); + if (/^(?:new file|deleted file|old|new) mode 120000$/m.test(patchContent)) { + return { + applied: false, + detail: `Worktree contains symlinks (mode 120000); patch not applied to prevent a host-file redirect via repoRoot. Inspect and copy them manually from ${worktreePath}.` + }; + } const applyResult = git(repoRoot, ["apply", "--index", patchPath]); if (applyResult.status !== 0) { return { applied: false, detail: applyResult.stderr.trim() || "Patch apply failed (conflicts?)." }; diff --git a/tests/worktree.test.mjs b/tests/worktree.test.mjs index a9fb7dad..10dc9b05 100644 --- a/tests/worktree.test.mjs +++ b/tests/worktree.test.mjs @@ -333,3 +333,53 @@ test("renderWorktreeTaskResult inspection command references baseCommit (staged- removeSession(session); } }); + +// SECURITY regression (#15): a symlink created in the worktree is captured as a +// mode-120000 diff entry. Without the guard, `git apply --index` would recreate +// the symlink in repoRoot pointing at an attacker-chosen host path → host-file +// exfil/append. keep must detect mode 120000 in the patch and refuse to apply, +// leaving the worktree in place. +test("keep refuses to apply a patch containing a symlink (mode 120000) and preserves the worktree", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + // Plant a symlink in the worktree pointing at an absolute host path. + fs.symlinkSync("/etc/passwd", path.join(session.worktreePath, "steal")); + + const result = cleanupWorktreeSession(session, { keep: true }); + + assert.equal(result.applied, false); + assert.match(result.detail, /symlinks/i); + // The symlink must NOT have been recreated in repoRoot. + assert.equal(fs.existsSync(path.join(repoRoot, "steal")), false, "symlink not replayed into repoRoot"); + // Worktree preserved (leave-branch). + assert.equal(result.preserved, true); + } finally { + removeSession(session); + } +}); + +// SECURITY regression (#15, false-positive guard): a regular file whose CONTENT +// happens to contain the literal "mode 120000" string (e.g. documentation of git +// modes) must NOT trip the symlink guard. Only real diff mode headers +// ("new file mode 120000" etc.) are symlinks. The regex matches line-anchored +// mode headers, not arbitrary content. +test("keep applies a regular file whose content literally mentions mode 120000 (no false positive)", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const session = createWorktreeSession(repoRoot); + + try { + fs.writeFileSync( + path.join(session.worktreePath, "modes.md"), + 'Git emits "new file mode 120000" for symlinks.\n' + ); + + const result = cleanupWorktreeSession(session, { keep: true }); + + assert.equal(result.applied, true, "regular file applied despite the literal mode string in content"); + assert.match(fs.readFileSync(path.join(repoRoot, "modes.md"), "utf8"), /mode 120000/); + } finally { + removeSession(session); + } +});