Skip to content

fix: guard against EISDIR when formatting untracked directories #130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
TacoYakii wants to merge 1 commit into from
+38 −1
Closed

fix: guard against EISDIR when formatting untracked directories #130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion plugins/codex/scripts/lib/git.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,25 @@ function formatSection(title, body) {
return [`## ${title}`, "", body.trim() ? body.trim() : "(none)", ""].join("\n");
}

function listUntrackedFiles(cwd, relativePath) {
const files = gitChecked(cwd, ["ls-files", "--others", "--exclude-standard", relativePath]).stdout.trim().split("\n").filter(Boolean);
if (files.length === 1 && files[0] === relativePath) {
const absolutePath = path.join(cwd, relativePath);
return gitChecked(absolutePath, ["ls-files", "--others", "--exclude-standard"]).stdout.trim().split("\n").filter(Boolean).map((file) => path.join(relativePath, file));
Comment on lines +139 to +140
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep directory walk inside the current repository

When ls-files echoes a directory path, this code runs git with cwd set to path.join(cwd, relativePath), which follows symlinks. If an untracked entry is a symlinked directory pointing to another Git repo, the review context will enumerate and include files from that external repo (I reproduced this with linkrepo -> /tmp/extrepo, which surfaced /tmp/extrepo contents as linkrepo/...). This leaks data outside the target repository boundary and should be blocked by resolving real paths and rejecting symlink-directory traversal (or treating symlinks as files via lstat).

Useful? React with 👍 / 👎.

}
return files;
}

function formatUntrackedFile(cwd, relativePath) {
const absolutePath = path.join(cwd, relativePath);
const stat = fs.statSync(absolutePath);
const lstat = fs.lstatSync(absolutePath);
if (lstat.isSymbolicLink() && fs.statSync(absolutePath).isDirectory()) {
return `### ${relativePath}\n(skipped: symlinked directory)`;
}
if (lstat.isDirectory()) {
return listUntrackedFiles(cwd, relativePath).map((file) => formatUntrackedFile(cwd, file)).join("\n\n");
}
const stat = lstat.isSymbolicLink() ? fs.statSync(absolutePath) : lstat;
if (stat.size > MAX_UNTRACKED_BYTES) {
return `### ${relativePath}\n(skipped: ${stat.size} bytes exceeds ${MAX_UNTRACKED_BYTES} byte limit)`;
}
Expand Down
21 changes: 21 additions & 0 deletions tests/git.test.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,27 @@ test("resolveReviewTarget honors explicit base overrides", () => {
assert.equal(target.baseRef, "main");
});

test("collectReviewContext includes files inside untracked directories", () => {
const cwd = makeTempDir();
initGitRepo(cwd);
fs.writeFileSync(path.join(cwd, "app.js"), "console.log(1);\n");
run("git", ["add", "app.js"], { cwd });
run("git", ["commit", "-m", "init"], { cwd });

fs.mkdirSync(path.join(cwd, "newdir", "sub"), { recursive: true });
fs.writeFileSync(path.join(cwd, "newdir", "a.txt"), "hello a");
fs.writeFileSync(path.join(cwd, "newdir", "sub", "b.txt"), "hello b");

const target = resolveReviewTarget(cwd, {});
const ctx = collectReviewContext(cwd, target);

assert.equal(ctx.mode, "working-tree");
assert.match(ctx.content, /newdir\/a\.txt/);
assert.match(ctx.content, /newdir\/sub\/b\.txt/);
assert.match(ctx.content, /hello a/);
assert.match(ctx.content, /hello b/);
});

test("resolveReviewTarget requires an explicit base when no default branch can be inferred", () => {
const cwd = makeTempDir();
initGitRepo(cwd);
Expand Down