Break CodeQL taint flow in /paths/compare so path-injection alerts resolve

[wojtekn]

Related issues

• Related to code scanning alerts #207 and #208
• Follow-up to Fix path-injection in /paths/compare by confining inputs to the sites root #4267, which added a sound runtime guard but left both CodeQL alerts open

How AI was used in this PR

Claude Code confirmed via the GitHub API that both alerts were still open on the latest trunk scan (which already contains #4267), traced why CodeQL's taint tracker did not treat the previous isPathWithin guard as a barrier, designed the restructured barrier pattern, wrote the helper and tests, and ran lint/typecheck/tests. I reviewed the approach and the diff.

Proposed Changes

PR #4267 confined the unauthenticated, loopback-only POST /api/paths/compare inputs to the sites root, which is a correct runtime mitigation — but the two high-severity js/path-injection alerts stayed open after it merged. CodeQL does not recognize the standalone isPathWithin(...) boolean check as a sanitizer: the untrusted values that actually reached fs.statSync (via path.resolve inside arePathsEqual) were the raw request values, so the taint flow req.body → path1/path2 → statSync was never broken in CodeQL's model.

This restructures the endpoint into the resolve → confine-to-root → use-the-returned-value pattern that CodeQL treats as a barrier:

• A new confineToRoot( root, candidate ) helper resolves the candidate against the root, checks it is the root or a descendant, and returns the normalized absolute path (or null when it escapes). isPathWithin now delegates to it.
• The /paths/compare handler passes only the confined, resolved paths into arePathsEqual — the raw, untrusted request values never reach any filesystem call.
• As additional hardening, relative candidates now resolve against the sites root rather than the process cwd.

No user-visible behavior change for legitimate use: the UI only ever compares real site paths under the sites root, which still compare correctly. The shared arePathsEqual helper is intentionally left untouched, since its other callers (CLI, desktop IPC, AI sessions) legitimately compare arbitrary absolute site paths that come from trusted local config.

Note: CodeQL can only re-confirm the alerts as resolved on the next scan after this lands on trunk. If the model still flags it, the fallback is a CodeQL sanitizer model or a justified dismissal — but the restructured flow is expected to clear it at the source.

Testing Instructions

• npm test -- packages/common/lib/tests/fs-utils.test.ts — new confineToRoot tests cover the root itself, descendants, in-root .. normalization, ../ escapes, the sibling-prefix edge case (studio-sites-other must not count as inside studio-sites), and relative-candidate resolution against the root. Existing isPathWithin tests still pass.
• npm run typecheck passes across all workspaces.
• Manual: POST /api/paths/compare with two real site paths under the sites root returns { equal: true } when they match; a body with a ../ traversal path returns { equal: false }.

Pre-merge Checklist

• Have you checked for TypeScript, React or other console errors?

[wpmobilebot]

📊 Performance Test Results

Comparing cbb78e0 vs trunk

app-size

Metric	trunk	cbb78e0	Diff	Change
App Size (Mac)	1375.36 MB	1381.73 MB	+6.37 MB	🔴 0.5%

site-editor

Metric	trunk	cbb78e0	Diff	Change
load	736 ms	1089 ms	+353 ms	🔴 48.0%

site-startup

Metric	trunk	cbb78e0	Diff	Change
siteCreation	6498 ms	6515 ms	+17 ms	⚪ 0.0%
siteStartup	2375 ms	2373 ms	2 ms	⚪ 0.0%

---

Results are median values from multiple test runs.

Legend: 🟢 Improvement (faster) | 🔴 Regression (slower) | ⚪ No change (<50ms diff)

[sejas]

@wojtekn, the code looks good, and everything worked well. I just found that we can remove the isPathWithin function and its tests. Otherwise, good to go.

[sejas]

Let's remove isPathWithin since it's not used anywhere in the code. It's only referenced in packages/common/lib/tests/fs-utils.test.ts

[wojtekn]

Good catch, removed in cbb78e0