[PPSC-420] fix(security): mask secrets and fix user-facing bugs for 1.1.0#72
Merged
yiftach-armis merged 3 commits intomainfrom Feb 15, 2026
Merged
[PPSC-420] fix(security): mask secrets and fix user-facing bugs for 1.1.0#72yiftach-armis merged 3 commits intomainfrom
yiftach-armis merged 3 commits intomainfrom
Conversation
Critical fixes: - FAILED scan status now properly returns error instead of success - Reject --exit-code 0 which defeats --fail-on in CI pipelines - Increase API response limit from 1MB to 50MB with truncation detection High priority fixes: - Redirect Docker pull/save output to stderr to prevent stdout pollution - Add bounds check for CommitSHA slicing to prevent panic Medium priority fixes: - Validate --scan-timeout and --upload-timeout require values >= 1 - Warn when --sbom-output/--vex-output used without --sbom/--vex flags - JUnit formatter now respects --fail-on severities instead of hardcoding - Fix symlink detection using os.Lstat instead of os.Stat - Use runewidth.StringWidth for proper Unicode text wrapping - Use rune-based slicing for column highlighting with multi-byte chars - Validate path/tarball existence before making auth network calls - Warn when both --tarball and image name are provided
Adds defense-in-depth secret masking for fix content in SARIF output and proposed fixes to prevent leaking secrets in code patches. New utility functions mask secrets in multi-line strings and string maps. Adds corresponding unit tests for masking functionality.
🛡️ Armis Security Scan Results🟠 HIGH issues found
Total: 2 View all 2 findings🟠 HIGH (2)
|
Test Coverage Reporttotal: (statements) 78.7% Coverage by function |
There was a problem hiding this comment.
Pull request overview
This PR hardens the CLI for the 1.1.0 release by preventing secret leakage across outputs/debug paths and addressing multiple correctness/UX issues in scanning, output formatting, and API handling.
Changes:
- Add defense-in-depth secret masking for multi-line strings, fix proposals, and SARIF/JUnit-related output paths.
- Improve scan behavior and UX: better symlink handling, safer stdout/stderr separation, and additional CLI input/path validation.
- Increase allowed API response size and fix scan failure handling to return errors on FAILED states.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/util/mask.go | Adds helpers to mask secrets in multi-line strings and string maps. |
| internal/util/mask_test.go | Adds unit tests for the new masking helpers. |
| internal/scan/mask.go | Introduces scan.MaskFixSecrets to sanitize Fix payload code fields. |
| internal/scan/mask_test.go | Adds unit tests to verify fix masking behavior and immutability. |
| internal/scan/repo/repo.go | Attempts to improve symlink skipping and masks fix data in debug + findings. |
| internal/scan/image/image.go | Routes docker/podman output to stderr and masks fix data in debug + findings. |
| internal/output/sarif.go | Masks fix/patch content when emitting SARIF (defense-in-depth). |
| internal/output/output.go | Extends FormatOptions with FailOnSeverities. |
| internal/output/junit.go | Makes JUnit failure logic respect FailOnSeverities (with defaults). |
| internal/output/junit_test.go | Updates tests for new JUnit severity-based failure logic. |
| internal/output/human.go | Improves wrapping/highlighting for multi-byte characters; avoids short-SHA slicing panic. |
| internal/cmd/scan.go | Adds validation for exit code and timeout flags. |
| internal/cmd/scan_repo.go | Validates repo path early; warns on unused SBOM/VEX output flags; passes fail-on severities to formatters. |
| internal/cmd/scan_image.go | Validates tarball path early; warns on unused SBOM/VEX output flags and conflicting inputs; passes fail-on severities to formatters. |
| internal/api/client.go | Increases max API response size; returns error on FAILED ingest; adds “truncated response” detection. |
| internal/api/client_test.go | Updates tests to assert FAILED ingest returns an error even without LastError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
410
to
+418
| bodyBytes, err := io.ReadAll(io.LimitReader(resp.Body, MaxAPIResponseSize)) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to read response body: %w", err) | ||
| } | ||
|
|
||
| // Detect if response was truncated at the limit | ||
| if int64(len(bodyBytes)) >= MaxAPIResponseSize { | ||
| return nil, fmt.Errorf("response too large (exceeded %d MB limit); try reducing --page-limit", MaxAPIResponseSize/(1024*1024)) | ||
| } |
There was a problem hiding this comment.
FetchNormalizedResults reads the body with io.LimitReader(..., MaxAPIResponseSize) and then errors when len(bodyBytes) >= MaxAPIResponseSize. This can incorrectly fail when the response size is exactly at the limit (not truncated). Consider reading MaxAPIResponseSize+1 bytes and only erroring when the read exceeds the limit (e.g., len > MaxAPIResponseSize).
filepath.Walk already uses os.Lstat internally, so the info parameter already contains symlink information. The extra Lstat calls added unnecessary syscalls that could slow down scanning on large repos.
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
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.
Related Issue
Type of Change
Problem
Multiple security vulnerabilities and bugs identified for the 1.1.0 release:
Solution
Comprehensive fixes including: (1) defense-in-depth secret masking in SARIF output, proposed fixes, and debug output using new utility functions; (2) proper error handling for failed scans and increased MaxAPIResponseSize from 1MB to 50MB; (3) comprehensive input validation for exit codes, timeouts, and file paths; (4) symlink detection fixes using Lstat in tarGzDirectory and calculateDirSize; (5) JUnit format now respects --fail-on flag; (6) improved text wrapping with multi-byte character width calculation.
Testing
Checklist