feat: add --include-files flag for targeted file scanning #46
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🛡️ Armis Security Scan Results✅ No issues
Total: 0 View full resultsNo security issues found. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Test Coverage Reporttotal: (statements) 74.7% Coverage by function |
…lows - Implement --include-files CLI flag to scan specific files instead of entire directory - Add FileList struct with path validation and security checks (path traversal prevention) - Add tarGzFiles method to create tarballs with only specified files - Add include-files input to reusable-security-scan.yml workflow - Create pr-security-scan.yml workflow for scanning only changed files in PRs - Include comprehensive unit tests for file path handling and validation - Enables efficient PR security scanning by targeting only modified files Fixes #N/A
The test TestParseFileListAbsolutePathOutsideRepo was using Unix-style paths (/etc/passwd) which are not recognized as absolute paths on Windows. Updated to use real temp directories which work on all platforms.
yiftach-armis
force-pushed
the
feature/scan-changed-files
branch
from
0f3911c to
d25446e
Compare
- Change reusable workflow to use local action (`./) instead of @main so the PR can test its own changes - Refactor action.yml to use bash arrays instead of string concatenation with eval, preventing potential command injection
Add build-from-source option to action.yml that builds the CLI from source instead of downloading the released version. This allows PRs that add new CLI flags (like --include-files) to be tested before the feature is released. The reusable workflow defaults to build-from-source=true since it's used for self-testing in this repository.
Resolved conflict in action.yml by keeping security improvements from main (env vars for command injection prevention) while adding include-files support from feature branch.
…ents - Add additional SARIF properties (type, codeSnippet, CVEs, CWEs, package, version, fixVersion) - Enhance PR comment to show detailed findings grouped by severity with expandable sections - Add resource leak prevention with pipe close in Scan function - Simplify file handling in tarGzFiles function with deferred close
Using defer inside a loop doesn't close files until the function returns, which can exhaust file descriptors when processing many files. This fix closes each file immediately after use with proper error handling.
- Replace curl|bash pattern with direct binary download and checksum verification in action.yml. This addresses the insecure pipe-to-shell anti-pattern flagged by security scanners. - Exclude test files (*_test.go) from PR security scans to avoid false positives from intentional path traversal test data.
- Add MaxFiles limit (1000) to prevent resource exhaustion from large file lists - Add isPathContained helper for defense-in-depth path validation in tarGzFiles and calculateFilesSize - Always apply MaskSecretInLine to CodeSnippet in SARIF output - Document TOCTOU handling as acceptable (files disappearing are handled gracefully) - Improve pipe close comment explaining why error is ignored - Add tests for MaxFiles limit validation
- Add security documentation for path traversal protection in scan_repo.go - Make tarFunc goroutine context-aware to prevent resource leaks - Enhance TOCTOU security annotation in repo.go - Add top-level permissions to pr-security-scan.yml and reusable-security-scan.yml Addresses: CWE-22 (path traversal), CWE-772 (resource leak), CWE-362 (TOCTOU), and CKV2_GHA_1 (workflow permissions)
When context is cancelled before tarFunc executes, the goroutine was returning early without closing the pipe writer. This caused StartIngest to hang forever waiting for data from the pipe (never receiving EOF). The fix closes the pipe writer before returning early, which properly signals EOF to StartIngest and allows the test to complete. Fixes TestScan/respects_context_cancellation timeout on all platforms.
The error from pw.Close() is intentionally not checked because: 1. PipeWriter.Close() rarely returns meaningful errors 2. The purpose is to signal EOF to the reader, not to handle close errors 3. Any actual issues will surface through the main error flow
- CWE-22: Replace strings.HasPrefix with filepath.EvalSymlinks for secure path containment checking in files.go, preventing symlink-based escapes - CWE-770: Cap SARIF results slice capacity to prevent resource exhaustion from large finding lists
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
Change build-from-source default from true to false to avoid self-referential security scanning where untested code scans itself. This ensures scans use a known-good released version of the CLI.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Implement
--include-filesCLI flag to enable targeted file scanning for specific files instead of entire repositories. This allows PR workflows to scan only changed files, improving scan efficiency.Type of Change
Changes
--include-filesflag accepting comma-separated file pathsFileListstruct with path validation and security checkstarGzFiles()method to create tarballs with only specified filespr-security-scan.ymlworkflow for PR delta scans usingtj-actions/changed-filesinclude-filesinput toreusable-security-scan.ymlworkflowTesting
Checklist