[PPSC-526] feat(scan): add --changed flag for scanning only git-changed files#93
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a --changed flag to armis-cli scan repo to scan only git-changed files (uncommitted, staged-only, or changes since a ref), enabling faster local feedback loops during development.
Changes:
- Introduces git change detection utilities (
GitChangedFiles, ref/staged/uncommitted modes) built on the nativegitbinary. - Adds comprehensive unit tests for git change detection/path filtering helpers.
- Wires the new
--changedflag intoscan repo, including error handling and mutual exclusivity with--include-files.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
internal/scan/repo/gitchanges.go |
Implements git-based changed-file discovery and path scoping for repo scans. |
internal/scan/repo/gitchanges_test.go |
Adds unit tests covering changed-file detection modes and helper functions. |
internal/cmd/scan_repo.go |
Adds the --changed CLI flag and integrates it into scanner configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Test Coverage Reporttotal: (statements) 80.6% Coverage by function |
…PSC-526] Add a new --changed flag to `scan repo` that enables scanning only files that have been modified in the git repository, providing faster feedback during local development. Three modes are supported: - `--changed` (no value): scan uncommitted changes (staged + unstaged + untracked) - `--changed staged`: scan only staged files - `--changed <ref>`: scan files changed since a git ref (e.g., main, HEAD~1) Features: - Works from any subdirectory within a git repository - Excludes deleted files (can't be tarred) - Mutually exclusive with --include-files - Graceful error handling for edge cases (not a git repo, no changes, invalid ref) Security: - Validates ref doesn't start with dash (prevents git argument injection) - Uses exec.Command (not shell) for subprocess calls - Leverages existing ParseFileList for path traversal validation Includes comprehensive test coverage for all modes and error conditions.
- Fix HEAD detection in changedUncommitted to check for specific git
error messages ("unknown revision", "bad revision") instead of just "HEAD"
- Return error when both primary and fallback diffs fail instead of
silently continuing with empty output
- Add unit tests for --changed flag: non-git repo error and no-changes
early return
- Fix stale comment in gitchanges_test.go about cleanup function
- Quiet runGitCmd output in tests by capturing stdout/stderr and only
logging on failure
87dea8a to
e9bc6ae
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Resolve symlinks in gitRepoRoot for macOS temp dir compatibility - Preserve spaces in filenames when parsing git output - Add clear error message for --changed= with empty value - Add git availability checks in tests - Fix test cleanup order for flag state restoration
The filterToScanPath function was silently ignoring errors from filepath.Rel, which could occur when paths are on different drives (Windows) or cannot be made relative. This was flagged as CWE-253: Incorrect Check of Function Return Value. Now the function properly returns an error that is propagated to the caller, providing a clear error message instead of silently returning an empty result.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add filepath.Clean in filterToScanPath to prevent path traversal (CWE-22) - Add control character and whitespace validation for git refs (CWE-628) - Document "staged" and "uncommitted" as reserved words in --changed help - Fix test isolation: reset flag.Changed in cleanup to prevent state leak - Rename wantNl to wantNil for clarity in gitchanges_test.go
Related Issue
Type of Change
Problem
Users want to scan only changed files locally for faster feedback during development. Previously, this required manually combining
git diffwith--include-files, making the workflow cumbersome.Solution
Added a
--changedflag toarmis-cli scan repothat automatically detects git-changed files and scans only those. Three modes are supported:--changed- scans uncommitted changes (staged + unstaged + untracked)--changed=staged- scans only staged files--changed=<ref>- scans files changed since a git reference (e.g.,--changed=main)Uses native
gitbinary for reliability and performance. Works from any subdirectory, scoping to files within that subtree only.Testing
Automated Tests
gitchanges_test.goManual Testing
--include-filesReviewer Notes
Key implementation details:
gitbinary viaexec.Commandfor better reliability than go-gitFileListandParseFileListinfrastructure for security validation (path traversal protection, MaxFiles limit)/tmp→/private/tmp) withfilepath.EvalSymlinksBehavior decisions:
.armisignorefiles are NOT applied to changed files (consistent with--include-files)--changedwhen--include-tests=false(explicit file selection overrides default filtering)Checklist