Skip to content

[Routine - QP] fix: close prefix-match bypass in PathUtils.validatePath#44

Draft
winterdrive wants to merge 1 commit into
masterfrom
routine/qp-fix-path-traversal-prefix-260702
Draft

[Routine - QP] fix: close prefix-match bypass in PathUtils.validatePath#44
winterdrive wants to merge 1 commit into
masterfrom
routine/qp-fix-path-traversal-prefix-260702

Conversation

@winterdrive

Copy link
Copy Markdown
Owner

1. Pre-flight Check

Open PRs / active branches checked before starting (gh pr list --state open, git branch -r):

  • [Routine - QP] fix: streamline quick create workspace routing #43 fix/quick-create-workspace-ux (open, draft) — touches src/commands.ts, src/promptProvider.ts, src/test/unit/promptProviderMultiroot.test.ts. No overlap with this PR.
  • Several routine/qp-fix-* branches exist on the remote ([Routine - QP] refactor: replace deprecated String.substr() with substring() #33[Routine - QP] fix: guard VersionHistoryService against corrupted/malformed history JSON #41) but all are CLOSED, not merged — they are stale duplicate attempts at guarding ClipboardManager.ts / PromptManager.ts / VersionManager.ts / VersionHistoryService.ts / aiEngine.ts / openAIClient.ts against corrupted JSON. I confirmed VersionHistoryService.loadHistory() on current master already has the isValidHistory shape-guard and SyntaxError handling those closed PRs proposed (verified via the passing versionHistoryService.test.ts suite), so that area is already covered and I avoided touching it again to prevent further duplicate churn.
  • This PR touches only src/core/PathUtils.ts and its test file src/test/unit/pathUtils.test.ts, neither of which appears in any open PR's diff or in the closed routine branches' file lists. No overlap.

2. Changes

  • PathUtils.validatePath() used normalizedPath.startsWith(normalizedRoot) to check whether a resolved path lies inside the workspace root. This is a plain string-prefix check, so a sibling directory that merely shares a name prefix (e.g. workspace root /foo/bar vs. a path resolving to /foo/bar-evil/file.json) would incorrectly pass validation.
  • Fixed to require an exact match on the root itself, or a match bounded by path.sep (normalizedPath === normalizedRoot || normalizedPath.startsWith(normalizedRoot + path.sep)).
  • Added two test cases to pathUtils.test.ts: one confirming the sibling-directory prefix bypass is now rejected, and one confirming the workspace root itself is still accepted.
  • Does not modify any MCP tool schema/name/parameters in mcp-server/src/tools/, does not add/remove/update dependencies, and does not touch package.json/package-lock.json/CHANGELOG.md.

3. Safety Verification

Commands run locally, all passed:

  • npx tsc -p ./ — no errors
  • npm run test (Jest, --runInBand) — 7 suites / 109 tests passed, including the 2 new tests

No lint or format:check script exists in package.json, so those steps were skipped.

4. CI / Release Gate Note

This is a daily routine Draft PR. Package version bump and CHANGELOG consolidation are intentionally deferred to the weekend release/integration PR. If CI's version-bump gate blocks this PR for lacking a version bump or the release-ready label, that is expected release-readiness behavior, not a code validation failure.

startsWith() treated any path whose string prefix matched the workspace
root as inside the workspace, so a sibling directory like "/foo/bar-evil"
would pass validation against workspace root "/foo/bar". Require an exact
match or a path.sep-bounded prefix instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant