CLI-1079 Harden change-set path handling

[nquinquenel]

---

Summary by Gitar

• Changeset handling:
  • Filter out files resolving outside the repository using toRelativePosixPath in sqaa-changeset.ts

_{This will update automatically on new commits.}

[netlify]

✅ Deploy Preview for sonarqube-cli canceled.

Name	Link
🔨 Latest commit	a3122eb
🔍 Latest deploy log	https://app.netlify.com/projects/sonarqube-cli/deploys/6a9fa7ace1a987000948c54f

[hashicorp-vault-sonar-prod]

CLI-1079

[gitar-bot]

Code Review ✅ Approved 5 resolved / 5 findings

Hardens change-set path handling by canonicalizing paths once and filtering out files that resolve outside the repository using toRelativePosixPath in sqaa-changeset.ts. Resolves issues with out-of-repo change-set files, repoRoot re-canonicalization overhead, symlink path normalization on macOS, and POSIX normalization in reports.

✅ 5 resolved

✅ Quality: Out-of-repo change-set files vanish with no diagnostic

📄 src/commands/analyze/sqaa-changeset.ts:80
A tracked symlink whose target resolves outside the repository (a common monorepo/shared-config pattern) is now removed from absolute and lands in neither files nor ignored, so nothing downstream can report it. Before this PR the same input produced a clear InvalidOptionError ("File must be inside '<repoRoot>'") from sqaa-run.ts:323; now a user who stages only such a file gets "Vortex analysis: no files in the change set to analyze." (sqaa.ts:219), which contradicts what git reported and gives no way to diagnose the omission. Note that simply pushing these paths into ignored would reintroduce the crash at sqaa-run.ts:343, which maps ignored through the throwing toRelativePosixPath, so surfacing them needs either a separate result field or a null-safe mapping there.

✅ Quality: New test has no positive control, passes if all files drop

📄 tests/unit/commands/analyze/sqaa-changeset.test.ts:128-137
The test only asserts that files and ignored are empty, so it stays green for any regression that drops every path — e.g. swapping the filter arguments to toRelativePosixPathOrNull(repoRoot, file) makes relative() yield '..' for in-repo files, returning null for all of them, and the assertions still hold. Include a real in-repo file in the mocked diff output and assert it survives alongside the excluded symlink so the test pins both directions of the filter.

✅ Performance: repoRoot re-canonicalized with a realpath syscall per file

📄 src/commands/analyze/sqaa-changeset.ts:80
toRelativePosixPath canonicalizes both arguments, so calling it inside the filter issues a realpathSync.native on repoRoot once per change-set entry — pure waste, since resolveRepoRoot already returns a canonicalized root (worktree.ts:76). On large change sets (the code has a confirmLargeRunIfNeeded guard, so thousands of files are expected) this doubles the syscalls this filter needs. Canonicalize the file once and compare with the existing isAncestorOrSelf helper from fs-utils.

✅ Bug: New symlink test asserts uncanonicalized path, fails on macOS

📄 tests/unit/commands/analyze/sqaa-changeset.test.ts:122-136 📄 src/commands/analyze/sqaa-changeset.ts:79-88
resolveChangeSet builds ignored paths as join(repoRoot, f) where repoRoot comes from resolveGitRepoRoot, which runs canonicalizePath on git's output (src/core/host/git/worktree.ts:76). On macOS tmpdir() is /var/folders/..., a symlink to /private/var/folders/..., so the recorded path is /private/var/.../repo/external.ts while the test's symlinkPath is /var/.../repo/external.ts — the strengthened toEqual([{ path: symlinkPath, ... }]) assertion fails for every developer running bun test:unit on macOS (CI only runs unit tests on Linux/Windows, so it passes there and the breakage is silent until someone runs it locally). Canonicalize the temp dir before deriving the expected paths.

✅ Quality: outside-repository paths bypass POSIX normalization in reports

📄 src/commands/analyze/sqaa-display-json.ts:103 📄 src/commands/analyze/sqaa-run.ts:344
For outside-repository entries toRelativePosixPath always returns null (that is why they were ignored), so the ?? f.path fallback emits the raw OS-native absolute path — backslash-separated on Windows — into SqaaJsonReport.ignored[].path and into the text/progress ignoredPaths, while every other path in the same report is a repo-relative POSIX path. Run the fallback through normalizePath so the report keeps a single path convention across platforms.

Implementation Status ✅ 2 of 2 objectives covered

✅ CLI-1079 - 2 of 2 objectives covered

This PR validates change-set inputs against the repository boundary before file inspection and adds regression coverage for paths resolving outside the repository.

✅ 2 covered here

• ✅ Add regression coverage for paths resolving outside the repository
• ✅ Validate change-set inputs against the repository boundary before file inspection

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Counting what did not apply, without listing it.

Comment with these commands to change the behavior for this request:

Auto-apply	Compact
gitar auto-apply:on	gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Gitar}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues
0 New dependency risks

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud