Skip to content

Commit a6658d8

Browse files
axisrowclaude
andcommitted
fix(security): reject symlink-mode blobs in the worktree patch
Closes #15 (HIGH). applyWorktreePatch captures the worktree diff with `git diff --cached --binary --output=<file>` 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 `<repoRoot>/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) <[email protected]> Claude-Session: https://claude.ai/code/session_0189cGaohsYdpkB4otKZnpAt
1 parent 5a19596 commit a6658d8

2 files changed

Lines changed: 65 additions & 0 deletions

File tree

‎plugins/codex/scripts/lib/git.mjs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,21 @@ export function applyWorktreePatch(repoRoot, worktreePath, baseCommit) {
377377
if (!fs.existsSync(patchPath) || fs.statSync(patchPath).size === 0) {
378378
return { applied: false, detail: "No tracked changes to apply." };
379379
}
380+
// SECURITY (#15): a symlink created in the worktree (ln -s ~/.ssh/authorized_keys
381+
// ./steal) is captured as a mode-120000 diff entry with the target verbatim.
382+
// `git apply --index` would recreate the symlink in repoRoot pointing at the
383+
// attacker-chosen host path → exfil/append primitive. git apply rejects literal
384+
// "../" filename paths, but symlink-mode entries bypass that guard (the repo-side
385+
// filename is normal; only the symlink target points outside). Reject the patch
386+
// if any mode-120000 line appears. Leave-branch preserved: worktree stays for
387+
// manual inspection.
388+
const patchContent = fs.readFileSync(patchPath, "utf8");
389+
if (/^(?:new file|deleted file|old|new) mode 120000$/m.test(patchContent)) {
390+
return {
391+
applied: false,
392+
detail: `Worktree contains symlinks (mode 120000); patch not applied to prevent a host-file redirect via repoRoot. Inspect and copy them manually from ${worktreePath}.`
393+
};
394+
}
380395
const applyResult = git(repoRoot, ["apply", "--index", patchPath]);
381396
if (applyResult.status !== 0) {
382397
return { applied: false, detail: applyResult.stderr.trim() || "Patch apply failed (conflicts?)." };

‎tests/worktree.test.mjs‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,3 +333,53 @@ test("renderWorktreeTaskResult inspection command references baseCommit (staged-
333333
removeSession(session);
334334
}
335335
});
336+
337+
// SECURITY regression (#15): a symlink created in the worktree is captured as a
338+
// mode-120000 diff entry. Without the guard, `git apply --index` would recreate
339+
// the symlink in repoRoot pointing at an attacker-chosen host path → host-file
340+
// exfil/append. keep must detect mode 120000 in the patch and refuse to apply,
341+
// leaving the worktree in place.
342+
test("keep refuses to apply a patch containing a symlink (mode 120000) and preserves the worktree", () => {
343+
const { repoRoot } = createRepoWithInitialCommit();
344+
const session = createWorktreeSession(repoRoot);
345+
346+
try {
347+
// Plant a symlink in the worktree pointing at an absolute host path.
348+
fs.symlinkSync("/etc/passwd", path.join(session.worktreePath, "steal"));
349+
350+
const result = cleanupWorktreeSession(session, { keep: true });
351+
352+
assert.equal(result.applied, false);
353+
assert.match(result.detail, /symlinks/i);
354+
// The symlink must NOT have been recreated in repoRoot.
355+
assert.equal(fs.existsSync(path.join(repoRoot, "steal")), false, "symlink not replayed into repoRoot");
356+
// Worktree preserved (leave-branch).
357+
assert.equal(result.preserved, true);
358+
} finally {
359+
removeSession(session);
360+
}
361+
});
362+
363+
// SECURITY regression (#15, false-positive guard): a regular file whose CONTENT
364+
// happens to contain the literal "mode 120000" string (e.g. documentation of git
365+
// modes) must NOT trip the symlink guard. Only real diff mode headers
366+
// ("new file mode 120000" etc.) are symlinks. The regex matches line-anchored
367+
// mode headers, not arbitrary content.
368+
test("keep applies a regular file whose content literally mentions mode 120000 (no false positive)", () => {
369+
const { repoRoot } = createRepoWithInitialCommit();
370+
const session = createWorktreeSession(repoRoot);
371+
372+
try {
373+
fs.writeFileSync(
374+
path.join(session.worktreePath, "modes.md"),
375+
'Git emits "new file mode 120000" for symlinks.\n'
376+
);
377+
378+
const result = cleanupWorktreeSession(session, { keep: true });
379+
380+
assert.equal(result.applied, true, "regular file applied despite the literal mode string in content");
381+
assert.match(fs.readFileSync(path.join(repoRoot, "modes.md"), "utf8"), /mode 120000/);
382+
} finally {
383+
removeSession(session);
384+
}
385+
});

0 commit comments

Comments
 (0)