fix(web): validate git ref and file path inputs#965
Conversation
…commands Reject ref values starting with '-' to prevent flag injection into git commands, and apply path traversal validation to file source lookups. Returns 400 INVALID_GIT_REF or 404 FILE_NOT_FOUND instead of passing unsanitized input to the underlying git CLI. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WalkthroughAdds runtime validation for git refs and file paths in multiple git-related API routes. Introduces Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
… invalidGitRef Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/web/src/features/git/listCommitsApi.test.ts (1)
29-32: Add test cases for git ref validation.The
invalidGitRefmock is correctly structured, but there are no test cases exercising the ref validation logic. The implementation validates that refs cannot start with-usingisGitRefValid, yet the test suite has no coverage for this. Add tests for the scenarios mentioned in the PR objectives:
list_commitswithref=--allreturns 400INVALID_GIT_REFlist_commitswithref=-rreturns 400INVALID_GIT_REF🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/features/git/listCommitsApi.test.ts` around lines 29 - 32, Add two unit tests to listCommitsApi.test.ts that exercise the ref validation: call the list_commits endpoint (the code path using isGitRefValid) with query param ref=--all and with ref=-r, and assert each response has HTTP 400 and the body matches the invalidGitRef mock (errorCode 'INVALID_GIT_REF' and the expected message). Place the tests alongside the existing list_commits tests so they exercise the same request helper and response assertions used elsewhere in this file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/web/src/features/git/listCommitsApi.test.ts`:
- Around line 29-32: Add two unit tests to listCommitsApi.test.ts that exercise
the ref validation: call the list_commits endpoint (the code path using
isGitRefValid) with query param ref=--all and with ref=-r, and assert each
response has HTTP 400 and the body matches the invalidGitRef mock (errorCode
'INVALID_GIT_REF' and the expected message). Place the tests alongside the
existing list_commits tests so they exercise the same request helper and
response assertions used elsewhere in this file.
Summary
refvalues starting with-inlist_commits,list_tree, andread_file— returns400 INVALID_GIT_REFinstead of passing the value unsanitized to the git CLIisPathValid()path traversal check toread_file— returns404 FILE_NOT_FOUNDinstead of an unhandled exceptionINVALID_GIT_REFerror code andinvalidGitRefservice error helper (400 Bad Request)Test plan
list_commitswithref=--allreturns 400INVALID_GIT_REFlist_commitswithref=-rreturns 400INVALID_GIT_REFlist_treewithref=--allreturns 400INVALID_GIT_REFread_filewithpath=../../../etc/passwdreturns 404FILE_NOT_FOUNDlist_commitswithref=maincontinues to work normally🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests
Chores