fix: address security vulnerabilities in CLI and workflows#41
Merged
yiftach-armis merged 8 commits intomainfrom Jan 15, 2026
Merged
fix: address security vulnerabilities in CLI and workflows#41yiftach-armis merged 8 commits intomainfrom
yiftach-armis merged 8 commits intomainfrom
Conversation
- Fix path traversal bypass in install.ps1 by normalizing paths before validation - Add missing path traversal patterns in install.sh with realpath normalization - Fix command injection in GitHub Actions workflow by using env: block - Improve secret masking in mask.go to prevent prefix exposure (JWT, API keys) - Add URL parse error handling in API client with warning output - Check os.Stdout.Sync() error and log to stderr before exit All fixes maintain backward compatibility and follow security best practices.
Test Coverage Reporttotal: (statements) 77.6% Coverage by function |
- Remove redundant validation block after realpath normalization (realpath already resolves all '..' segments, so the case statement was dead code) - Add comment explaining GNU-specific -m flag for realpath portability - Use length ranges instead of exact lengths in maskValue to prevent token type fingerprinting (e.g., GitHub PATs ~40 chars, AWS keys 40 chars)
There was a problem hiding this comment.
Pull request overview
This PR addresses 6 security vulnerabilities across the CLI and GitHub Actions workflows. The changes strengthen input validation, prevent information leakage through secrets, and improve error handling.
Changes:
- Enhanced secret masking to prevent prefix leakage that could identify secret types
- Added command injection prevention in GitHub Actions workflow via environment variables
- Strengthened path traversal defenses in installation scripts with normalization and extended pattern matching
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/util/mask.go | Complete secret masking without revealing identifying prefixes or partial content |
| .github/workflows/reusable-security-scan.yml | Prevents command injection by using env block instead of inline expression interpolation |
| scripts/install.ps1 | Path normalization before validation to prevent path traversal bypasses |
| scripts/install.sh | Additional pattern matching and realpath validation for path traversal defense |
| internal/api/client.go | Explicit URL parsing error logging instead of silent failures |
| internal/output/output.go | Error handling for stdout sync failures with stderr logging |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add package-level variables for os.Exit, stdout sync, and stderr in output.go to enable mocking in tests - Add tests for ExitIfNeeded covering exit behavior, code normalization, and stdout sync error handling - Add tests verifying secret prefixes don't leak in masked output - Fix path validation pattern in install.sh (remove redundant case)
- Align stdoutSyncer variable with other variables in the block - Explicitly ignore fmt.Fprintf return values to satisfy errcheck
The normalized_dir variable was calculated but never used. Now the normalized path is also validated for invalid characters and path traversal segments as an additional defense-in-depth measure.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Change pattern from `/..*` to `/..` to only match the exact `/..` path traversal, not valid directory names like `/..foo`.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Change `length <= 8` to `length < 10` so that 9-character values show exact asterisks instead of the misleading "10-20" range.
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.
Description
Fix 6 security vulnerabilities across the CLI and GitHub Actions workflows, addressing issues raised in security audit.
Changes
Type of Change
Testing
Checklist