Fix/path guards root level#2559
Open
SyedaAnshrahGillani wants to merge 2 commits into
Open
Conversation
|
@github-actions[bot] is attempting to deploy a commit to the NexusCore Team on Vercel. A member of the Team first needs to authorize it. |
Collaborator
|
I would hold this until full CI runs. Right now I only see labeler checks plus a Vercel authorization failure, with no CI Gate. This PR touches multiple path traversal/deletion guard paths, so it needs the normal required checks before review/merge. Please also update the title to conventional format and add coverage for the affected guard surfaces, especially Windows drive-root behavior, not just the single / case in server-validation.test.ts. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR fixes a bug where repository path comparison and sandbox traversal guards duplicate slashes when the target path ends with a separator (like the root directory
/on Unix or drive rootC:\on Windows).Motivation / context
When a repository or target directory is located at a drive root (e.g.,
C:\on Windows) or the root filesystem (e.g.,/on Unix), suffixingpath.sep(e.g.root + path.sep) duplicates the separator (forming//orC:\\\). This causes prefix matching and traversal guards (which checkstartsWith) to fail. This change handles the trailing separator dynamically.Areas touched
gitnexus/(CLI / core / MCP server)gitnexus-web/(Vite / React UI).github/(workflows, actions)eval/or other toolingAGENTS.md,CLAUDE.md,.cursor/,llms.txt, etc.)Scope & constraints
In scope
checkCwdMatchingit-staleness.tsto support root-level directories.assertSafePathinvalidation.tsto support root-level directories.resolveContainedDestinupload-ingest.tsto correctly handle root-level staging directory sandbox checks.lbug-adapter.tsfor root-level parents.assertSafePathinlocal-backend.ts.UPLOAD_ROOTdeletion check and file search path traversal guard inapi.ts.isWithinRepoinnuxt-auto-imports.tsfor root-level repositories.server-validation.test.tsverifyingassertSafePathwith root-level/.Explicitly out of scope / not done here
Implementation notes
startsWithcheck by checkingendsWith(path.sep) ? root : root + path.sepinstead of blindly appendingpath.sep.Testing & verification
cd gitnexus && npm test(Verified through specific unit tests suite)cd gitnexus && npm run test:integration(if core/indexing/MCP paths changed)cd gitnexus && npx tsc --noEmitcd gitnexus-web && npm test(if web changed)cd gitnexus-web && npx tsc -b --noEmit(if web changed)gitnexus-web/e2e/)Ran:
npx vitest run test/unit/staleness.test.ts test/unit/sibling-clone-drift.test.ts test/unit/server-validation.test.tsnpx tsc --noEmitRisk & rollout
Low risk. Changes only refine boundary comparison for root-level directory path checks.
Checklist
AGENTS.md/ overlays changed: headers, scope block, and changelog updated per project conventions