From a6658d834a19aee5ad57d30e79db618edf56ffbd Mon Sep 17 00:00:00 2001 From: axisrow Date: Mon, 20 Jul 2026 02:46:44 +0800 Subject: [PATCH] fix(security): reject symlink-mode blobs in the worktree patch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #15 (HIGH). applyWorktreePatch captures the worktree diff with `git diff --cached --binary --output=` and replays it with `git apply --index` in repoRoot. A symlink created in the (attacker/Codex- controlled) worktree — `ln -s ~/.ssh/authorized_keys ./steal` — is captured as a mode-120000 diff entry with the target path verbatim. `git apply --index` then recreates the symlink in repoRoot pointing at the attacker-chosen absolute host path. Subsequent cat/git-show/editor-open exfiltrates the file; >> appends. The byte-preserving `--output` transfer does not help — the symlink target is legitimately part of the diff. `git apply` rejects literal "../" filename paths, but symlink-mode entries bypass that guard: the repo-side filename is a normal path; only the symlink target points outside. Attack reproduced before the fix: `ln -s /etc/passwd ./steal` in the worktree produced a patch with `new file mode 120000`, and `git apply --index` recreated `/steal -> /etc/passwd`. Fix: after writing the patch file, scan it for a `mode 120000` line. If found, do NOT apply — return {applied:false} with a message directing the user to inspect/copy the symlinks manually from the worktree. The leave-branch invariant is preserved (the worktree stays; nothing is destroyed). Regression test: plant a symlink in the worktree, call keep, assert applied === false and the symlink is NOT recreated in repoRoot. Verified: worktree 14/14; full npm test 157 pass / 4 pre-existing (unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_0189cGaohsYdpkB4otKZnpAt --- plugins/codex/scripts/lib/git.mjs | 15 ++++++++++ tests/worktree.test.mjs | 50 +++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) 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); + } +});