From ac87d2dc009b278a5cbfd27e68c56cbf9f2a1c91 Mon Sep 17 00:00:00 2001 From: axisrow Date: Mon, 20 Jul 2026 02:23:46 +0800 Subject: [PATCH] fix(security): refuse symlink at .worktrees before creating a worktree MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #14 (CRITICAL). createWorktree did fs.mkdirSync(/.worktrees, {recursive:true}) — Node's recursive mkdir follows a symlink at the final component. A crafted repo, a co-resident user, or a prior malicious run that did `ln -s ~/.config .worktrees` would redirect the subsequent `git worktree add` (the Codex workspace-write root) into ~/.ssh / ~/.config / ~/Library/LaunchAgents — a sandbox escape + persistence/credential primitive. Attack reproduced end-to-end before the fix: `.worktrees -> /tmp/attacker-dest` caused `git worktree add` to populate /tmp/attacker-dest/codex-/ with HEAD content. Fix: before mkdirSync, lstat .worktrees and throw if it is a symlink; after creation, verify the resolved realpath is contained within repoRoot. The leave-branch invariant is preserved (no new Destroy path — this guards creation). Regression test: plant a hostile symlink `.worktrees -> `, assert createWorktreeSession throws and the attacker destination stays empty. Verified: worktree 13/13; full npm test 156 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 | 17 +++++++++++++++++ tests/worktree.test.mjs | 25 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index 601202b8..2216ea7f 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -309,7 +309,24 @@ function buildAdversarialCollectionGuidance(options = {}) { export function createWorktree(repoRoot) { const ts = Date.now(); const worktreesDir = path.join(repoRoot, ".worktrees"); + // SECURITY (#14): if `.worktrees` already exists as a symlink, Node's recursive + // mkdirSync follows it and `git worktree add` would populate the symlink TARGET — + // a crafted repo (or a prior malicious run that did `ln -s ~/.config .worktrees`) + // would redirect the Codex workspace-write root into ~/.ssh, ~/.config, etc. + // Refuse any symlink at .worktrees, and verify the resolved path stays inside the repo. + if (fs.existsSync(worktreesDir) && fs.lstatSync(worktreesDir).isSymbolicLink()) { + throw new Error( + `Refusing to create worktree: ${worktreesDir} is a symlink (possible sandbox-escape attempt). Remove it manually if expected.` + ); + } fs.mkdirSync(worktreesDir, { recursive: true }); + const realRepoRoot = fs.realpathSync(repoRoot); + const realWorktreesDir = fs.realpathSync(worktreesDir); + if (realWorktreesDir !== realRepoRoot && !realWorktreesDir.startsWith(realRepoRoot + path.sep)) { + throw new Error( + `Refusing to create worktree: ${worktreesDir} resolves outside the repository (to ${realWorktreesDir}).` + ); + } // Ensure .worktrees/ is excluded from the target repo without modifying tracked files. // Use git rev-parse to resolve the real git dir (handles linked worktrees where .git is a file). diff --git a/tests/worktree.test.mjs b/tests/worktree.test.mjs index 51c56ccd..a9fb7dad 100644 --- a/tests/worktree.test.mjs +++ b/tests/worktree.test.mjs @@ -288,6 +288,31 @@ test("renderWorktreeTaskResult renders the manual-remove instructions (no destru } }); +// SECURITY regression (#14): a symlink at `.worktrees` must NOT be followed. +// Without the guard, recursive mkdirSync follows the symlink and `git worktree add` +// populates the symlink target — a crafted repo (or a prior malicious run) could +// redirect the Codex workspace-write root into ~/.ssh / ~/.config / etc. +test("createWorktreeSession refuses a symlink at .worktrees (no host-location redirect)", () => { + const { repoRoot } = createRepoWithInitialCommit(); + const attackerDest = makeTempDir("worktree-attack-dest-"); + + // Plant the hostile symlink: .worktrees -> attackerDest (outside repoRoot). + fs.symlinkSync(attackerDest, path.join(repoRoot, ".worktrees")); + + assert.throws( + () => createWorktreeSession(repoRoot), + /symlink/i, + "must refuse when .worktrees is a symlink" + ); + + // The attacker's destination must NOT have been populated. + assert.deepEqual( + fs.readdirSync(attackerDest), + [], + "attacker destination must remain empty (no worktree written through the symlink)" + ); +}); + // Regression: rescue changes are STAGED (getWorktreeDiff/applyWorktreePatch run // git add -A). The rendered inspection command must be HEAD/base-based so it // shows staged work — plain `git diff` shows only unstaged and could read empty,