fix(security): eliminate info/exclude write (closes #16)

plugins/codex/scripts/lib/git.mjs

@@ -328,15 +328,31 @@ export function createWorktree(repoRoot) {
     );
   }
 
-  // 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).
-  const rawGitDir = gitChecked(repoRoot, ["rev-parse", "--git-dir"]).stdout.trim();
-  const gitDir = path.resolve(repoRoot, rawGitDir);
-  const excludePath = path.join(gitDir, "info", "exclude");
-  const excludeContent = fs.existsSync(excludePath) ? fs.readFileSync(excludePath, "utf8") : "";
-  if (!excludeContent.includes(".worktrees")) {
-    fs.mkdirSync(path.dirname(excludePath), { recursive: true });
-    fs.appendFileSync(excludePath, `${excludeContent.endsWith("\n") || !excludeContent ? "" : "\n"}.worktrees/\n`);
+  // SECURITY (#16, redesign — eliminate, not detect): the original code wrote
+  // `.worktrees/` into <gitDir>/info/exclude to keep the worktree dir out of
+  // `git status`. That write was the entire #16 vulnerability: if gitDir resolved
+  // outside repoRoot (separate-git-dir, crafted .git gitfile, linked worktree),
+  // the exclude write landed at an attacker-controlled location. Detection-based
+  // fixes (boundary checks) broke legit linked worktrees and couldn't distinguish
+  // them from attack — git introspection can't authenticate external common-dirs.
+  //
+  // Eliminate the write entirely: git worktree add works WITHOUT the exclude
+  // (it was cosmetic, added in openai#137 commit 4bca062). Instead, keep
+  // `.worktrees/` out of git status via an in-repo `.worktrees/.gitignore`
+  // containing `*`, written inside the already-validated worktreesDir (#14 guard
+  // ensures it's inside repoRoot and not a symlink). No git-dir metadata write.
+  const dotGitignore = path.join(worktreesDir, ".gitignore");
+  // SECURITY: if .gitignore already exists as a symlink (crafted repo ships it as
+  // a dangling symlink to an external path), existsSync follows it and returns
+  // false, then writeFileSync would create a file at the symlink target outside
+  // repoRoot. lstat (not stat/existsSync) detects the symlink itself.
+  if (fs.existsSync(dotGitignore) && fs.lstatSync(dotGitignore).isSymbolicLink()) {
+    throw new Error(
+      `Refusing to create worktree: ${dotGitignore} is a symlink (possible redirect attempt). Remove it manually if expected.`
+    );
+  }
+  if (!fs.existsSync(dotGitignore)) {
+    fs.writeFileSync(dotGitignore, "*\n", "utf8");
   }
 
   const worktreePath = path.join(worktreesDir, `codex-${ts}`);

tests/worktree.test.mjs

@@ -415,3 +415,51 @@ test("keep applies a retargeted existing symlink as a regular file (no host redi
     removeSession(session);
   }
 });
+
+// SECURITY regression (#16, redesign — eliminate info/exclude write): createWorktree
+// no longer writes to <gitDir>/info/exclude. The #16 vulnerability (git-dir outside
+// repoRoot → exclude write at attacker location) is eliminated by not writing to
+// git-dir metadata. A separate-git-dir repo (previously the attack case) now works.
+test("createWorktreeSession works with separate-git-dir (no info/exclude write)", () => {
+  const repoRoot = makeTempDir();
+  const externalGitDir = makeTempDir("external-gitdir-");
+  run("git", ["init", "-q", "-b", "main", `--separate-git-dir=${externalGitDir}`, repoRoot]);
+  run("git", ["config", "user.name", "Test"], { cwd: repoRoot });
+  run("git", ["config", "user.email", "test@test"], { cwd: repoRoot });
+  fs.writeFileSync(path.join(repoRoot, "app.js"), "export const v = 1;\n");
+  run("git", ["add", "app.js"], { cwd: repoRoot });
+  run("git", ["commit", "-q", "-m", "init"], { cwd: repoRoot });
+
+  const session = createWorktreeSession(repoRoot);
+  try {
+    assert.ok(session.worktreePath);
+    assert.match(session.branch, /^codex\/\d+$/);
+    const excludePath = path.join(externalGitDir, "info", "exclude");
+    if (fs.existsSync(excludePath)) {
+      assert.doesNotMatch(
+        fs.readFileSync(excludePath, "utf8"),
+        /\.worktrees/,
+        "external info/exclude must not contain .worktrees (no git-dir write)"
+      );
+    }
+  } finally {
+    removeSession(session);
+  }
+});
+
+// Regression: linked worktree (gitdir in main .git/) works — no git-dir metadata
+// write means no boundary check needed, no legit-layout breakage.
+test("createWorktreeSession works from a linked worktree", () => {
+  const { repoRoot } = createRepoWithInitialCommit();
+  const linkedWt = makeTempDir("linked-wt-");
+  assert.equal(run("git", ["worktree", "add", linkedWt, "-b", "linked"], { cwd: repoRoot }).status, 0);
+
+  const session = createWorktreeSession(linkedWt);
+  try {
+    assert.ok(session.worktreePath);
+    assert.match(session.branch, /^codex\/\d+$/);
+  } finally {
+    removeSession(session);
+    run("git", ["worktree", "remove", "--force", linkedWt], { cwd: repoRoot });
+  }
+});