fix: guard against EISDIR when formatting untracked directories#130
fix: guard against EISDIR when formatting untracked directories#130TacoYakii wants to merge 1 commit intoopenai:mainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2638bebb81
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
plugins/codex/scripts/lib/git.mjs
Outdated
| const entries = fs.readdirSync(absolutePath, { recursive: true }); | ||
| return entries | ||
| .filter((entry) => fs.statSync(path.join(absolutePath, String(entry))).isFile()) | ||
| .map((entry) => formatUntrackedFile(cwd, path.join(relativePath, String(entry)))) |
There was a problem hiding this comment.
.git internals in untracked dirs
When git ls-files --others --exclude-standard yields a directory entry like child/ (common for an embedded Git repo), this new recursive walk descends into child/.git and formats its internal files. That defeats Git’s filtering boundary, can massively bloat review context, and may expose sensitive metadata (e.g., remotes/config) that should not be part of untracked file content. The directory expansion should explicitly skip .git (and ideally honor Git ignore semantics) instead of blindly recursing all entries.
Useful? React with 👍 / 👎.
TacoYakii
force-pushed
the
fix-eisdir-untracked-directory
branch
from
2638beb to
1405bf7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1405bf7608
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
plugins/codex/scripts/lib/git.mjs
Outdated
| return files | ||
| .map((file) => formatUntrackedFile(cwd, file)) |
There was a problem hiding this comment.
When relativePath is an untracked nested Git repo (for example child/), git ls-files --others --exclude-standard child/ can return child/ again, so formatUntrackedFile calls itself with the same argument indefinitely and eventually crashes with RangeError: Maximum call stack size exceeded. This keeps the original review-flow failure for embedded repos and can also hang first due to repeated Git invocations; guard against self-referential results (or skip directory entries like nested .git repos) before recursing. Fresh evidence: in a local repro, git ls-files --others --exclude-standard child/ returned child/ exactly.
Useful? React with 👍 / 👎.
TacoYakii
force-pushed
the
fix-eisdir-untracked-directory
branch
from
1405bf7 to
c0b6468
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c0b6468440
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| 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)); |
There was a problem hiding this comment.
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 👍 / 👎.
TacoYakii
force-pushed
the
fix-eisdir-untracked-directory
branch
from
c0b6468 to
746daaa
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 746daaa1e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
plugins/codex/scripts/lib/git.mjs
Outdated
| const stat = fs.lstatSync(absolutePath); | ||
| if (stat.isDirectory()) { | ||
| return listUntrackedFiles(cwd, relativePath).map((file) => formatUntrackedFile(cwd, file)).join("\n\n"); | ||
| } | ||
| if (stat.size > MAX_UNTRACKED_BYTES) { |
There was a problem hiding this comment.
Using fs.lstatSync here makes stat.size the symlink entry length, but fs.readFileSync still follows the link and reads the target file. In a repo with an untracked symlink to a large file (including one outside the repo), the MAX_UNTRACKED_BYTES gate is bypassed and the full target content is injected into review context, which can balloon prompt size and expose unintended data. This regression was introduced by switching from statSync to lstatSync for the size check.
Useful? React with 👍 / 👎.
`git ls-files --others` can return directory paths. Without a directory check, `fs.readFileSync` throws EISDIR and crashes the review flow. Add `stat.isDirectory()` guard that delegates back to git to recursively list files inside untracked directories to avoid the EISDIR error, honoring .gitignore rules and avoiding symlink loops. For nested git repos, detect self-referential results and re-run git ls-files from inside the nested repo to include its files. Use lstatSync to detect symlinks. Symlinked directories are skipped to prevent traversal outside the repository boundary. For symlink files, resolve via statSync to enforce the MAX_UNTRACKED_BYTES limit against the actual target size.
TacoYakii
force-pushed
the
fix-eisdir-untracked-directory
branch
from
746daaa to
bd64198
Compare
|
🦀 Assigned! I will submit a PR for this issue. Looking at the fix now. |
|
Thanks for contributing. Closing as duplicate |
3 participants
Summary
git ls-files --others --exclude-standardreturns directory names (e.g.dir/) instead of individual files when a directory is entirely untrackedformatUntrackedFilepasses these directory paths tofs.readFileSync, which throwsEISDIRand crashes the review flowstat.isDirectory()guard that recursively reads files inside untracked directories to avoid the EISDIR error