feat(coverage): validate that file paths are within repository boundary#2550
feat(coverage): validate that file paths are within repository boundary#2550
Conversation
Add workspace containment validation to coverage uploads. Previously, a path like `../../../foo/bar.sh` that existed on disk but was outside the repository would pass validation. Now such files are detected and tracked separately as "outside workspace" files. Changes: - Add workspace_root field to Plan and pass through from Planner - Add outside_workspace_files tracking to Report and Processor - Add is_within_workspace() check in Processor to categorize files - Update Validator to track files_outside_workspace in ValidationResult - Update CLI to display outside-workspace files when present - Files outside workspace count as "not present" for validation threshold 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
No issue mentions found. Please mention an issue in the pull request description. Use GitHub automation to close the issue when a PR is merged |
There was a problem hiding this comment.
Pull request overview
This PR adds workspace boundary validation to coverage uploads to prevent files outside the repository from being counted towards coverage validation thresholds.
- Files existing outside the repository workspace are now tracked separately as "outside workspace"
- The validation threshold calculation excludes these files (treating them similarly to missing files)
- CLI provides clear feedback about files outside the workspace when present
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| qlty-coverage/src/validate.rs | Added workspace containment check in validator with is_within_workspace() method and new field files_outside_workspace in ValidationResult |
| qlty-coverage/src/publish/processor.rs | Added workspace boundary checking during file processing with is_within_workspace() method to populate outside_workspace_files |
| qlty-coverage/src/publish/report.rs | Added outside_workspace_files field to track files outside workspace |
| qlty-coverage/src/publish/planner.rs | Updated to accept and pass workspace_root parameter through to Plan |
| qlty-coverage/src/publish/plan.rs | Added workspace_root field to store workspace boundary |
| qlty-cli/src/commands/coverage/publish.rs | Updated to obtain workspace root, display outside-workspace files, and include them in validation error messages |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
qlty-coverage/src/validate.rs
Outdated
| fn is_within_workspace(&self, file_path: &Path) -> bool { | ||
| let Some(ref workspace_root) = self.workspace_root else { | ||
| return true; | ||
| }; | ||
|
|
||
| match (file_path.canonicalize(), workspace_root.canonicalize()) { | ||
| (Ok(canonical_file), Ok(canonical_root)) => canonical_file.starts_with(&canonical_root), | ||
| _ => false, | ||
| } | ||
| } |
There was a problem hiding this comment.
The is_within_workspace method is duplicated identically in processor.rs (lines 95-104). Consider extracting this logic into a shared utility function to maintain DRY (Don't Repeat Yourself) principle and ensure consistent behavior across the codebase.
| fn is_within_workspace(&self, file_path: &Path) -> bool { | ||
| let Some(ref workspace_root) = self.plan.workspace_root else { | ||
| return true; | ||
| }; | ||
|
|
||
| match (file_path.canonicalize(), workspace_root.canonicalize()) { | ||
| (Ok(canonical_file), Ok(canonical_root)) => canonical_file.starts_with(&canonical_root), | ||
| _ => false, | ||
| } | ||
| } |
There was a problem hiding this comment.
The is_within_workspace method is duplicated identically in validate.rs (lines 78-87). Consider extracting this logic into a shared utility function to maintain DRY (Don't Repeat Yourself) principle and ensure consistent behavior across the codebase.
1 new issue
|
…update snapshot - Fix pluralization: "1 file" vs "X files" for both missing and outside repository counts - Update publish_validate.stderr snapshot to include the new detail line 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
Diff Coverage for ubuntu-latest: The code coverage on the diff in this pull request is 73.9%. Total Coverage for ubuntu-latest: This PR will decrease coverage by 0.05%. File Coverage Changes
🛟 Help
|
|
Diff Coverage for macos-15: The code coverage on the diff in this pull request is 73.9%. Total Coverage for macos-15: This PR will decrease coverage by 0.05%. File Coverage Changes
🛟 Help
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add is_path_within_workspace function to utils.rs - Remove wrapper methods and call utility directly from validate.rs and processor.rs - Clean up unused Path imports 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
691a925 to
020ddd0
Compare
Summary
Changes
qlty-coverage/src/publish/plan.rs- Addedworkspace_root: Option<PathBuf>fieldqlty-coverage/src/publish/planner.rs- Updatednew()to acceptworkspace_root, pass it to Planqlty-coverage/src/publish/report.rs- Addedoutside_workspace_files: HashSet<String>fieldqlty-coverage/src/publish/processor.rs- Addedis_within_workspace()check, populatesoutside_workspace_filesqlty-coverage/src/validate.rs- Addedworkspace_rootto Validator,files_outside_workspaceto ValidationResult, containment checkqlty-cli/src/commands/coverage/publish.rs- Gets workspace root, passes to Planner/Validator, displays outside-workspace filesTest plan
validate.rscargo checkpassesqlty fmtandqlty check --level=low --fixpass with no issues🤖 Generated with Claude Code