fix: refuse unexpected gitlinks staged by git add - #761
Conversation
Detect new mode-160000 entries after add via git diff --cached --raw so nested .git directories cannot be silently committed as attacker-chosen gitlinks. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Warning Review limit reached
Next review available in: 3 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a safety check to prevent the action from committing newly introduced gitlinks (mode 160000) after git add, which can happen when an embedded/nested .git directory is staged and would otherwise be recorded as a submodule-like reference.
Changes:
- Add parsing + assertion helpers to detect newly introduced mode-
160000entries ingit diff --cached --raw. - Enforce the check in the
add()flow so the action fails fast when unexpected gitlinks are staged. - Document the behavior and add unit tests for the parser/assert helpers.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/util.ts |
Adds gitlink detection/parsing and a failing assertion with remediation hints. |
src/main.ts |
Runs the cached raw diff check after git add and fails on unexpected gitlinks. |
test/util.test.ts |
Adds unit tests for the new gitlink parser and assertion helper. |
README.md |
Documents the new “refuse new gitlinks” behavior and remediation steps. |
Suppressed comments (2)
src/util.ts:190
- The remediation hint uses
git rm --cached ${p}without a--path separator. If a path begins with-, the pasted command can be interpreted as an option instead of a path. Adding--makes the suggested command safe for all path names.
const listed = paths.map(p => ` - ${p}`).join('\n');
const rmHints = paths.map(p => ` git rm --cached ${p}`).join('\n');
throw new Error(
`Refusing to commit unexpected gitlink(s) (embedded git repository staged as mode 160000):\n${listed}\n` +
'Git records a nested .git directory as a gitlink, not as its files. ' +
`Remove the nested .git directory, or unstage the path(s) with:\n${rmHints}`,
test/util.test.ts:235
- This assertion hard-codes the remediation hint without the
--path separator. If the hint is updated togit rm --cached -- <path>(recommended for safety), the test should assert the safer form to match.
);
expect(() => assertNoUnexpectedGitlinks(['evil_nested_repo'])).toThrow(
/git rm --cached evil_nested_repo/,
);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const match = line.match( | ||
| /^:(\d{6}) (\d{6}) [0-9a-f]+ [0-9a-f]+ [A-Z]\t(.+)$/, | ||
| ); | ||
| if (!match) continue; | ||
| const [, oldMode, newMode, filePath] = match; | ||
| if (newMode === '160000' && oldMode !== '160000') { | ||
| paths.push(filePath); | ||
| } |
| ).toStrictEqual(['was_a_file']); | ||
| }); | ||
|
|
||
| it('allows existing submodule SHA updates (160000→160000)', () => { |
Accept scored R/C statuses and destination paths, and use -- in rm remediation hints. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
git add, inspectgit diff --cached --rawand fail if any new mode-160000gitlink would be committed (embedded nested.git).160000→160000so existing submodule SHA bumps still work.Test plan
npm testpasses (parser fixtures: new gitlink, mode change into gitlink, submodule bump, mixed/empty).gitunder the working tree + defaultadd: '.'fails the action with remediation hints instead of committing a gitlink160000→160000) still commits successfullylib/index.jsmatches a clean rebuildMade with Cursor