Fix path traversal part1

[zhangxin511]

Fix flagged parse traversal by MSRC team

[Copilot]

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

[github-actions]

Hi! Thank you for opening this PR. Want me to review it?

Based on the diff (129 lines, 8 files), I've queued these reviewers:

• Correctness — logic errors, race conditions, lifecycle issues
• Security — vulnerabilities, secret exposure, injection
• API Compatibility — breaking changes, release tags, type design
• Performance — algorithmic regressions, memory leaks
• Testing — coverage gaps, hollow tests

How this works

• Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

• Tick Start review below to dispatch the review fleet.

• After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

• This comment updates as new commits land; your reviewer selections are preserved.

• Start review

[github-actions]

Bundle size comparison

Base commit: 9b7be4bc0eb9a8902617bba0e65b99dce5159385
Head commit: ac7dc0b9a6a57865adf0e87ab265d3d6784b30f7

Pending — Build - client packages is running. Results will appear here when the build completes.

[github-actions]

🔭 PR Review Fleet Report

Note

This report is generated by an experimental AI review fleet and is provided as a beta feature. Findings are a starting point for discussion, not a gate. Use your own judgement.

Verdict: ⚠️ Approve with Suggestions

0 Exterminate, 1 Squash, 1 Investigate

Findings

Sev	#	Area	File	What	Fix
🦟 Squash	H1	Testing	server/gitrest/packages/gitrest-base/src/routes/summaries.ts:45	getFullSummaryDirectory() was changed to throw a 400 NetworkError when documentId (sourced from the Storage-Routing-Id header) is empty or fails isInvalidRepoPathComponent, closing a path-traversal hole. No test exercises this new validation or its call sites (lines 85, 132, 205). summaries.spec.ts only ever passes valid uuid() values as documentId, so a regression that removes/weakens the check (or a future change to isInvalidRepoPathComponent) would not be caught by any test in this package.	Add a test in summaries.spec.ts (or a new unit test around getFullSummaryDirectory if exported) that hits one of the summary routes with a Storage-Routing-Id documentId of ".." (and/or "" and "a/b"), and asserts the response is a 400 with a message matching /Invalid document id/, and that no filesystem access outside the repo directory occurs.
🐜 Investigate	M1	Testing	server/gitrest/packages/gitrest-base/src/utils/helpers.ts:220	isInvalidRepoPathComponent explicitly checks path.win32.isAbsolute(value) to catch Windows-style absolute paths (e.g. "C:\foo" or "\foo"), but helpers.spec.ts only tests the POSIX absolute case ("/etc"). The equivalent Windows-style cases are covered for containsPathTraversal in services-shared/src/test/http.spec.ts ("\path") but not for this near-identical gitrest helper, so a regression that drops the win32.isAbsolute check here would go undetected.	Add a test in helpers.spec.ts: assert.throws(() => getRepoPath("tenant", "\\etc"), /Invalid repo name \(documentId\)/) and one for a drive-letter path like getRepoPath("tenant", "C:\\etc"), both expected to throw.

---

View workflow run