Security finding from the adversarial review of #12 (commit 1a9eb11). Partially reproduced.
Vulnerability
createWorktree (plugins/codex/scripts/lib/git.mjs) resolves the git dir without a boundary check:
const rawGitDir = gitChecked(repoRoot, ["rev-parse", "--git-dir"]).stdout.trim();
const gitDir = path.resolve(repoRoot, rawGitDir);
const excludePath = path.join(gitDir, "info", "exclude");
...
fs.appendFileSync(excludePath, `${...}.worktrees/\n`);
git rev-parse --git-dir returns an absolute path whenever the repo uses --separate-git-dir, is a linked worktree, or has a .git gitfile. When the value is absolute, path.resolve(repoRoot, "/abs/path") discards repoRoot and uses the absolute path verbatim. The subsequent appendFileSync then writes .worktrees/\n at that absolute location.
Attack
A crafted repo bundled as a tarball (not git clone — clone sanitizes) ships a .git gitfile pointing at an attacker-pre-staged valid git dir elsewhere on disk (e.g. ~/.cache/evil/.git). createWorktree then appends .worktrees/\n to ~/.cache/evil/.git/info/exclude. Content is benign (.worktrees/), but the destination is attacker-controlled and there is no boundary check that gitDir is inside repoRoot — a structural gap future code could turn worse.
Reproduced
Confirmed for legit linked worktrees (--git-dir returns /private/tmp/.../main/.git/worktrees/linked-wt). Could not reach a destructive payload: git refuses a gitfile pointing at a non-git dir, so the attacker must pre-stage a valid git dir at the target. Hence MEDIUM, not HIGH.
Severity
MEDIUM today (benign-content append anywhere a valid git dir can be pre-staged); structurally a missing-boundary check.
Fix
After path.resolve, assert gitDir === repoRoot || gitDir.startsWith(repoRoot + path.sep) (with realpath normalization on both sides) and bail otherwise. Treat any --git-dir outside repoRoot as a hard error.
Refs #12, #13 (umbrella), openai#135, openai#137.
Security finding from the adversarial review of #12 (commit 1a9eb11). Partially reproduced.
Vulnerability
createWorktree(plugins/codex/scripts/lib/git.mjs) resolves the git dir without a boundary check:git rev-parse --git-dirreturns an absolute path whenever the repo uses--separate-git-dir, is a linked worktree, or has a.gitgitfile. When the value is absolute,path.resolve(repoRoot, "/abs/path")discards repoRoot and uses the absolute path verbatim. The subsequentappendFileSyncthen writes.worktrees/\nat that absolute location.Attack
A crafted repo bundled as a tarball (not
git clone— clone sanitizes) ships a.gitgitfile pointing at an attacker-pre-staged valid git dir elsewhere on disk (e.g.~/.cache/evil/.git).createWorktreethen appends.worktrees/\nto~/.cache/evil/.git/info/exclude. Content is benign (.worktrees/), but the destination is attacker-controlled and there is no boundary check thatgitDiris inside repoRoot — a structural gap future code could turn worse.Reproduced
Confirmed for legit linked worktrees (
--git-dirreturns/private/tmp/.../main/.git/worktrees/linked-wt). Could not reach a destructive payload: git refuses a gitfile pointing at a non-git dir, so the attacker must pre-stage a valid git dir at the target. Hence MEDIUM, not HIGH.Severity
MEDIUM today (benign-content append anywhere a valid git dir can be pre-staged); structurally a missing-boundary check.
Fix
After
path.resolve, assertgitDir === repoRoot || gitDir.startsWith(repoRoot + path.sep)(with realpath normalization on both sides) and bail otherwise. Treat any--git-diroutside repoRoot as a hard error.Refs #12, #13 (umbrella), openai#135, openai#137.