From 10cacd32e0aea1e403efc12ee4e425e9b8ee5f75 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Tue, 13 Jan 2026 22:31:33 +0200 Subject: [PATCH 1/8] fix: address security vulnerabilities in CLI and workflows - 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. --- .github/workflows/reusable-security-scan.yml | 7 ++++-- internal/api/client.go | 8 +++++-- internal/output/output.go | 5 +++- internal/util/mask.go | 24 +++++++++++--------- scripts/install.ps1 | 13 +++++++---- scripts/install.sh | 16 +++++++++++-- 6 files changed, 50 insertions(+), 23 deletions(-) diff --git a/.github/workflows/reusable-security-scan.yml b/.github/workflows/reusable-security-scan.yml index dd02331..f9cb187 100644 --- a/.github/workflows/reusable-security-scan.yml +++ b/.github/workflows/reusable-security-scan.yml @@ -157,8 +157,11 @@ jobs: - name: Check for Failures if: always() + env: + SCAN_OUTCOME: ${{ steps.armis_scan.outcome }} + FAIL_ON_THRESHOLD: ${{ inputs.fail-on }} run: | - if [ "${{ steps.armis_scan.outcome }}" = "failure" ]; then - echo "::error::Security vulnerabilities detected by Armis (threshold: ${{ inputs.fail-on }})" + if [ "$SCAN_OUTCOME" = "failure" ]; then + echo "::error::Security vulnerabilities detected by Armis (threshold: $FAIL_ON_THRESHOLD)" exit 1 fi diff --git a/internal/api/client.go b/internal/api/client.go index 5cb57df..ceca5ba 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -42,8 +42,12 @@ func WithHTTPClient(client *httpclient.Client) ClientOption { // NewClient creates a new API client with the given configuration. func NewClient(baseURL, token string, debug bool, uploadTimeout time.Duration, opts ...ClientOption) *Client { - // Warn about non-HTTPS URLs (except localhost) as credentials may be exposed - if parsedURL, err := url.Parse(baseURL); err == nil && parsedURL.Scheme != "https" { + // Validate and warn about URL issues + parsedURL, err := url.Parse(baseURL) + if err != nil { + fmt.Fprintf(os.Stderr, "Warning: invalid base URL %q: %v - requests may fail\n", baseURL, err) + } else if parsedURL.Scheme != "https" { + // Warn about non-HTTPS URLs (except localhost) as credentials may be exposed host := parsedURL.Hostname() if host != "localhost" && host != "127.0.0.1" { fmt.Fprintf(os.Stderr, "Warning: using non-HTTPS URL %q - credentials may be transmitted insecurely\n", baseURL) diff --git a/internal/output/output.go b/internal/output/output.go index 8b8ef89..24f9e14 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -61,7 +61,10 @@ func ExitIfNeeded(result *model.ScanResult, failOnSeverities []string, exitCode exitCode = 1 } // Flush stdout to ensure all output is written before exit - _ = os.Stdout.Sync() + if err := os.Stdout.Sync(); err != nil { + // Log flush failure to stderr (stdout may be broken) + fmt.Fprintf(os.Stderr, "Warning: failed to flush stdout before exit: %v\n", err) + } os.Exit(exitCode) } } diff --git a/internal/util/mask.go b/internal/util/mask.go index f0b4204..399c5b5 100644 --- a/internal/util/mask.go +++ b/internal/util/mask.go @@ -2,6 +2,7 @@ package util import ( + "fmt" "regexp" "strings" ) @@ -74,20 +75,21 @@ func MaskSecretInLine(line string) string { return result } -// maskValue masks a secret value, preserving some structure hints. -// Shows first 2 chars and last 2 chars if long enough, otherwise all asterisks. +// maskValue masks a secret value completely for security. +// Only reveals the length of the original value, not any actual characters. +// This prevents leaking prefixes that identify secret types (e.g., "eyJ" for JWT, +// "ghp_" for GitHub tokens, "AKIA" for AWS keys, "sk_live_" for Stripe). func maskValue(value string) string { - if len(value) <= 4 { - return strings.Repeat("*", len(value)) + length := len(value) + if length == 0 { + return "" } - - // For longer values, show partial hints - if len(value) <= 8 { - return value[:1] + strings.Repeat("*", len(value)-2) + value[len(value)-1:] + if length <= 8 { + // For short values, show asterisks matching length + return strings.Repeat("*", length) } - - // For very long values, show first 2 and last 2 - return value[:2] + strings.Repeat("*", len(value)-4) + value[len(value)-2:] + // For longer values, show fixed asterisks with length indicator + return fmt.Sprintf("********[%d]", length) } // MaskSecretInLines masks secrets in multiple lines. diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 20ec660..ca76bb6 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -96,13 +96,16 @@ function Main { Write-Host "" # Validate InstallDir to prevent path traversal attacks - # Reject any input containing ".." for clarity and defense-in-depth - if ($InstallDir -match '\.\.') { - Write-Error "Invalid install directory: path traversal sequences (..) are not allowed" + # First normalize the path to resolve any relative segments (like ..\..), + # then validate the normalized result for defense-in-depth + $script:InstallDir = [System.IO.Path]::GetFullPath($InstallDir) + + # After normalization, verify no ".." segments remain + # GetFullPath should resolve all "..", but we double-check for defense-in-depth + if ($InstallDir -match '\\\.\.\\|\\\.\.($)') { + Write-Error "Invalid install directory: path traversal detected after normalization" exit 1 } - # Normalize the path for consistent handling - $script:InstallDir = [System.IO.Path]::GetFullPath($InstallDir) # Validate Version format (except for the special 'latest' value) if ($Version -ne "latest" -and $Version -notmatch '^v?\d+\.\d+\.\d+(-[0-9A-Za-z\.-]+)?$') { diff --git a/scripts/install.sh b/scripts/install.sh index 0c0218d..d4053bd 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -44,14 +44,26 @@ validate_install_dir() { exit 1 fi - # Disallow parent directory traversal segments like "../" or "/../" + # Disallow parent directory traversal segments like "../" or "/../" or "/.." # This allows valid paths with ".." in filenames (e.g., /opt/app..v1/bin) case "$dir" in - */../*|../*|*/..) + */../*|../*|*/..|/..|/..*) echo "Error: Install directory cannot contain parent directory segment '..': $dir" >&2 exit 1 ;; esac + + # If realpath is available, normalize and re-validate the resolved path + # This catches cases like /foo/bar/../../../etc that resolve outside bounds + if command -v realpath > /dev/null 2>&1; then + normalized_dir=$(realpath -m "$dir" 2>/dev/null) || normalized_dir="$dir" + case "$normalized_dir" in + */../*|../*|*/..|/..|/..*) + echo "Error: Path traversal detected after normalization: $dir resolves to $normalized_dir" >&2 + exit 1 + ;; + esac + fi } USER_BIN="$HOME/.local/bin" From 2e9431334311ed922ef8a2e6d68b7d0598b99be2 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Tue, 13 Jan 2026 22:34:04 +0200 Subject: [PATCH 2/8] style: format Go files with gofmt --- internal/scan/image/image.go | 32 ++++++++++++++++---------------- internal/scan/repo/repo.go | 32 ++++++++++++++++---------------- 2 files changed, 32 insertions(+), 32 deletions(-) diff --git a/internal/scan/image/image.go b/internal/scan/image/image.go index d0fcab5..2e7660a 100644 --- a/internal/scan/image/image.go +++ b/internal/scan/image/image.go @@ -11,11 +11,11 @@ import ( "strings" "time" - "github.com/ArmisSecurity/armis-cli/internal/api" - "github.com/ArmisSecurity/armis-cli/internal/model" - "github.com/ArmisSecurity/armis-cli/internal/progress" - "github.com/ArmisSecurity/armis-cli/internal/scan" - "github.com/ArmisSecurity/armis-cli/internal/util" + "github.com/ArmisSecurity/armis-cli/internal/api" + "github.com/ArmisSecurity/armis-cli/internal/model" + "github.com/ArmisSecurity/armis-cli/internal/progress" + "github.com/ArmisSecurity/armis-cli/internal/scan" + "github.com/ArmisSecurity/armis-cli/internal/util" ) const ( @@ -294,19 +294,19 @@ func convertNormalizedFindings(normalizedFindings []model.NormalizedFinding, deb finding.CodeSnippet = *loc.Snippet } - if loc.SnippetStartLine != nil { - finding.SnippetStartLine = *loc.SnippetStartLine - } + if loc.SnippetStartLine != nil { + finding.SnippetStartLine = *loc.SnippetStartLine + } - finding.Type = scan.DeriveFindingType( - len(nf.NormalizedRemediation.VulnerabilityTypeMetadata.CVEs) > 0, - loc.HasSecret, - finding.FindingCategory, - ) + finding.Type = scan.DeriveFindingType( + len(nf.NormalizedRemediation.VulnerabilityTypeMetadata.CVEs) > 0, + loc.HasSecret, + finding.FindingCategory, + ) - if loc.HasSecret && finding.CodeSnippet != "" { - finding.CodeSnippet = util.MaskSecretInLine(finding.CodeSnippet) - } + if loc.HasSecret && finding.CodeSnippet != "" { + finding.CodeSnippet = util.MaskSecretInLine(finding.CodeSnippet) + } finding.Title = finding.Description diff --git a/internal/scan/repo/repo.go b/internal/scan/repo/repo.go index 4364808..929fb06 100644 --- a/internal/scan/repo/repo.go +++ b/internal/scan/repo/repo.go @@ -13,11 +13,11 @@ import ( "strings" "time" - "github.com/ArmisSecurity/armis-cli/internal/api" - "github.com/ArmisSecurity/armis-cli/internal/model" - "github.com/ArmisSecurity/armis-cli/internal/progress" - "github.com/ArmisSecurity/armis-cli/internal/scan" - "github.com/ArmisSecurity/armis-cli/internal/util" + "github.com/ArmisSecurity/armis-cli/internal/api" + "github.com/ArmisSecurity/armis-cli/internal/model" + "github.com/ArmisSecurity/armis-cli/internal/progress" + "github.com/ArmisSecurity/armis-cli/internal/scan" + "github.com/ArmisSecurity/armis-cli/internal/util" ) // MaxRepoSize is the maximum allowed size for repositories. @@ -432,19 +432,19 @@ func convertNormalizedFindings(normalizedFindings []model.NormalizedFinding, deb finding.CodeSnippet = *loc.Snippet } - if loc.SnippetStartLine != nil { - finding.SnippetStartLine = *loc.SnippetStartLine - } + if loc.SnippetStartLine != nil { + finding.SnippetStartLine = *loc.SnippetStartLine + } - finding.Type = scan.DeriveFindingType( - len(nf.NormalizedRemediation.VulnerabilityTypeMetadata.CVEs) > 0, - loc.HasSecret, - finding.FindingCategory, - ) + finding.Type = scan.DeriveFindingType( + len(nf.NormalizedRemediation.VulnerabilityTypeMetadata.CVEs) > 0, + loc.HasSecret, + finding.FindingCategory, + ) - if loc.HasSecret && finding.CodeSnippet != "" { - finding.CodeSnippet = util.MaskSecretInLine(finding.CodeSnippet) - } + if loc.HasSecret && finding.CodeSnippet != "" { + finding.CodeSnippet = util.MaskSecretInLine(finding.CodeSnippet) + } finding.Title = finding.Description From 57d348fc45be8e214affdd40148353ab8f6f9dd3 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Tue, 13 Jan 2026 22:37:31 +0200 Subject: [PATCH 3/8] Address code review comments - 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) --- internal/util/mask.go | 19 ++++++++++++++++--- scripts/install.sh | 14 +++++--------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/internal/util/mask.go b/internal/util/mask.go index 399c5b5..6640df1 100644 --- a/internal/util/mask.go +++ b/internal/util/mask.go @@ -76,9 +76,10 @@ func MaskSecretInLine(line string) string { } // maskValue masks a secret value completely for security. -// Only reveals the length of the original value, not any actual characters. +// Only reveals a length range of the original value, not any actual characters. // This prevents leaking prefixes that identify secret types (e.g., "eyJ" for JWT, // "ghp_" for GitHub tokens, "AKIA" for AWS keys, "sk_live_" for Stripe). +// Length ranges are used instead of exact lengths to prevent token type identification. func maskValue(value string) string { length := len(value) if length == 0 { @@ -88,8 +89,20 @@ func maskValue(value string) string { // For short values, show asterisks matching length return strings.Repeat("*", length) } - // For longer values, show fixed asterisks with length indicator - return fmt.Sprintf("********[%d]", length) + // For longer values, show length range to prevent token type identification + // (e.g., GitHub PATs ~40 chars, AWS keys 40 chars could be fingerprinted) + var rangeStr string + switch { + case length <= 20: + rangeStr = "10-20" + case length <= 40: + rangeStr = "20-40" + case length <= 80: + rangeStr = "40-80" + default: + rangeStr = "80+" + } + return fmt.Sprintf("********[%s]", rangeStr) } // MaskSecretInLines masks secrets in multiple lines. diff --git a/scripts/install.sh b/scripts/install.sh index d4053bd..6315146 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -53,16 +53,12 @@ validate_install_dir() { ;; esac - # If realpath is available, normalize and re-validate the resolved path - # This catches cases like /foo/bar/../../../etc that resolve outside bounds + # If realpath is available, normalize the path to resolve any remaining traversal + # Note: -m flag is GNU-specific (works even if path doesn't exist yet). + # On BSD/macOS, this may fail and fall back to the original path, which is + # already validated above. This is a defense-in-depth measure. if command -v realpath > /dev/null 2>&1; then - normalized_dir=$(realpath -m "$dir" 2>/dev/null) || normalized_dir="$dir" - case "$normalized_dir" in - */../*|../*|*/..|/..|/..*) - echo "Error: Path traversal detected after normalization: $dir resolves to $normalized_dir" >&2 - exit 1 - ;; - esac + normalized_dir=$(realpath -m "$dir" 2>/dev/null) || true fi } From 6375aafe944d83aea5d5823840110e36a5921a00 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 14 Jan 2026 11:46:50 +0200 Subject: [PATCH 4/8] refactor: improve testability and add comprehensive test coverage - 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) --- internal/output/output.go | 13 +++- internal/output/output_test.go | 136 +++++++++++++++++++++++++++++++++ internal/util/mask_test.go | 50 ++++++++++++ scripts/install.sh | 2 +- 4 files changed, 197 insertions(+), 4 deletions(-) diff --git a/internal/output/output.go b/internal/output/output.go index 24f9e14..d6f6b4d 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -8,6 +8,13 @@ import ( "github.com/ArmisSecurity/armis-cli/internal/model" ) +// Package-level variables for testability +var ( + stdoutSyncer = func() error { return os.Stdout.Sync() } + stderrWriter io.Writer = os.Stderr + osExit = os.Exit +) + // FormatOptions contains options for formatting scan results. type FormatOptions struct { GroupBy string @@ -61,10 +68,10 @@ func ExitIfNeeded(result *model.ScanResult, failOnSeverities []string, exitCode exitCode = 1 } // Flush stdout to ensure all output is written before exit - if err := os.Stdout.Sync(); err != nil { + if err := stdoutSyncer(); err != nil { // Log flush failure to stderr (stdout may be broken) - fmt.Fprintf(os.Stderr, "Warning: failed to flush stdout before exit: %v\n", err) + fmt.Fprintf(stderrWriter, "Warning: failed to flush stdout before exit: %v\n", err) } - os.Exit(exitCode) + osExit(exitCode) } } diff --git a/internal/output/output_test.go b/internal/output/output_test.go index ab649b2..42d0a84 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -1,6 +1,8 @@ package output import ( + "bytes" + "errors" "testing" "github.com/ArmisSecurity/armis-cli/internal/model" @@ -155,3 +157,137 @@ func TestShouldFail_CaseSensitive(t *testing.T) { t.Error("ShouldFail should match 'HIGH' with 'HIGH'") } } + +func TestExitIfNeeded_ExitsOnMatchingSeverity(t *testing.T) { + // Save original and restore after test + originalOsExit := osExit + defer func() { osExit = originalOsExit }() + + var exitCode int + exitCalled := false + osExit = func(code int) { + exitCode = code + exitCalled = true + } + + result := &model.ScanResult{ + Findings: []model.Finding{ + {Severity: model.SeverityCritical}, + }, + } + + ExitIfNeeded(result, []string{"CRITICAL"}, 2) + + if !exitCalled { + t.Error("ExitIfNeeded should call osExit when severity matches") + } + if exitCode != 2 { + t.Errorf("ExitIfNeeded called osExit with code %d, want 2", exitCode) + } +} + +func TestExitIfNeeded_NoExitWhenNoMatch(t *testing.T) { + // Save original and restore after test + originalOsExit := osExit + defer func() { osExit = originalOsExit }() + + exitCalled := false + osExit = func(code int) { + exitCalled = true + } + + result := &model.ScanResult{ + Findings: []model.Finding{ + {Severity: model.SeverityLow}, + }, + } + + ExitIfNeeded(result, []string{"CRITICAL", "HIGH"}, 1) + + if exitCalled { + t.Error("ExitIfNeeded should not call osExit when severity does not match") + } +} + +func TestExitIfNeeded_NormalizesExitCode(t *testing.T) { + // Save original and restore after test + originalOsExit := osExit + defer func() { osExit = originalOsExit }() + + tests := []struct { + name string + inputCode int + expectedCode int + }{ + {"negative code normalizes to 1", -1, 1}, + {"code above 255 normalizes to 1", 256, 1}, + {"code 0 stays 0", 0, 0}, + {"code 255 stays 255", 255, 255}, + {"code 100 stays 100", 100, 100}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var exitCode int + osExit = func(code int) { + exitCode = code + } + + result := &model.ScanResult{ + Findings: []model.Finding{ + {Severity: model.SeverityCritical}, + }, + } + + ExitIfNeeded(result, []string{"CRITICAL"}, tt.inputCode) + + if exitCode != tt.expectedCode { + t.Errorf("ExitIfNeeded with code %d called osExit with %d, want %d", + tt.inputCode, exitCode, tt.expectedCode) + } + }) + } +} + +func TestExitIfNeeded_StdoutSyncError(t *testing.T) { + // Save originals and restore after test + originalStdoutSyncer := stdoutSyncer + originalStderrWriter := stderrWriter + originalOsExit := osExit + defer func() { + stdoutSyncer = originalStdoutSyncer + stderrWriter = originalStderrWriter + osExit = originalOsExit + }() + + // Mock stdoutSyncer to return an error + stdoutSyncer = func() error { + return errors.New("sync failed") + } + + // Capture stderr output + var stderrBuf bytes.Buffer + stderrWriter = &stderrBuf + + // Mock osExit to not actually exit + osExit = func(code int) {} + + result := &model.ScanResult{ + Findings: []model.Finding{ + {Severity: model.SeverityCritical}, + }, + } + + ExitIfNeeded(result, []string{"CRITICAL"}, 1) + + stderrOutput := stderrBuf.String() + if stderrOutput == "" { + t.Error("ExitIfNeeded should write warning to stderr when stdout sync fails") + } + if !bytes.Contains(stderrBuf.Bytes(), []byte("Warning")) { + t.Errorf("stderr output should contain 'Warning', got: %s", stderrOutput) + } + if !bytes.Contains(stderrBuf.Bytes(), []byte("sync failed")) { + t.Errorf("stderr output should contain error message, got: %s", stderrOutput) + } +} diff --git a/internal/util/mask_test.go b/internal/util/mask_test.go index 89ea029..a43c212 100644 --- a/internal/util/mask_test.go +++ b/internal/util/mask_test.go @@ -150,3 +150,53 @@ func TestMaskSecretInLines_EmptySlice(t *testing.T) { t.Errorf("MaskSecretInLines([]) = %v, want empty slice", result) } } + +// TestMaskSecretInLine_NoPrefixLeakage verifies that identifying secret prefixes +// are NOT leaked in masked output. This prevents attackers from identifying +// secret types from masked output. +func TestMaskSecretInLine_NoPrefixLeakage(t *testing.T) { + tests := []struct { + name string + input string + forbiddenPrefixes []string // prefixes that must NOT appear in output + }{ + { + name: "JWT token prefix eyJ must not leak", + input: `token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"`, + forbiddenPrefixes: []string{"eyJ"}, + }, + { + name: "GitHub token prefix ghp_ must not leak", + input: `auth_token = "ghp_xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"`, + forbiddenPrefixes: []string{"ghp_"}, + }, + { + name: "AWS key prefix AKIA must not leak", + input: `AWS_ACCESS_KEY_ID=AKIAIOSFODNN7EXAMPLE`, + forbiddenPrefixes: []string{"AKIA"}, + }, + { + name: "Stripe key prefix sk_live_ must not leak", + input: `api_key = "sk_live_51H7example123456789"`, + forbiddenPrefixes: []string{"sk_live_"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := util.MaskSecretInLine(tt.input) + + // Verify forbidden prefixes are NOT in output + for _, prefix := range tt.forbiddenPrefixes { + if strings.Contains(result, prefix) { + t.Errorf("MaskSecretInLine(%q) = %q, must NOT contain identifying prefix %q", tt.input, result, prefix) + } + } + + // Verify output contains masking pattern (asterisks with length range) + if !strings.Contains(result, "********") { + t.Errorf("MaskSecretInLine(%q) = %q, expected masking pattern with asterisks", tt.input, result) + } + }) + } +} diff --git a/scripts/install.sh b/scripts/install.sh index 6315146..ec5cc63 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -47,7 +47,7 @@ validate_install_dir() { # Disallow parent directory traversal segments like "../" or "/../" or "/.." # This allows valid paths with ".." in filenames (e.g., /opt/app..v1/bin) case "$dir" in - */../*|../*|*/..|/..|/..*) + */../*|../*|*/..|/..*) echo "Error: Install directory cannot contain parent directory segment '..': $dir" >&2 exit 1 ;; From 4a772e49161c16d1e84c0f098908ccbe5abaf603 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 14 Jan 2026 13:27:28 +0200 Subject: [PATCH 5/8] style: fix gofmt alignment and errcheck lint violation - Align stdoutSyncer variable with other variables in the block - Explicitly ignore fmt.Fprintf return values to satisfy errcheck --- internal/output/output.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/output/output.go b/internal/output/output.go index d6f6b4d..d604b8c 100644 --- a/internal/output/output.go +++ b/internal/output/output.go @@ -10,7 +10,7 @@ import ( // Package-level variables for testability var ( - stdoutSyncer = func() error { return os.Stdout.Sync() } + stdoutSyncer = func() error { return os.Stdout.Sync() } stderrWriter io.Writer = os.Stderr osExit = os.Exit ) @@ -70,7 +70,7 @@ func ExitIfNeeded(result *model.ScanResult, failOnSeverities []string, exitCode // Flush stdout to ensure all output is written before exit if err := stdoutSyncer(); err != nil { // Log flush failure to stderr (stdout may be broken) - fmt.Fprintf(stderrWriter, "Warning: failed to flush stdout before exit: %v\n", err) + _, _ = fmt.Fprintf(stderrWriter, "Warning: failed to flush stdout before exit: %v\n", err) } osExit(exitCode) } From 39ad880da3b8208557d6f653ec6f7eebd21b97a7 Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 14 Jan 2026 15:39:14 +0200 Subject: [PATCH 6/8] fix: validate normalized path in install script 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. --- scripts/install.sh | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/scripts/install.sh b/scripts/install.sh index ec5cc63..00625ef 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -59,6 +59,24 @@ validate_install_dir() { # already validated above. This is a defense-in-depth measure. if command -v realpath > /dev/null 2>&1; then normalized_dir=$(realpath -m "$dir" 2>/dev/null) || true + + # Re-validate the normalized path as an additional defense-in-depth check. + if [ -n "$normalized_dir" ]; then + # Only allow safe characters in the normalized path + if ! printf '%s' "$normalized_dir" | grep -qE '^[a-zA-Z0-9/_.-]+$'; then + echo "Error: Normalized install directory contains invalid characters: $normalized_dir" >&2 + echo "Only alphanumeric characters, underscores, hyphens, dots, and forward slashes are allowed." >&2 + exit 1 + fi + + # Disallow parent directory traversal segments in the normalized path + case "$normalized_dir" in + */../*|../*|*/..|/..*) + echo "Error: Normalized install directory cannot contain parent directory segment '..': $normalized_dir" >&2 + exit 1 + ;; + esac + fi fi } From 8da7d8bbd973feafb6eaf55b6a6df9c048a2332b Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 14 Jan 2026 16:04:04 +0200 Subject: [PATCH 7/8] fix: avoid false positives for valid directory names starting with .. Change pattern from `/..*` to `/..` to only match the exact `/..` path traversal, not valid directory names like `/..foo`. --- scripts/install.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/install.sh b/scripts/install.sh index 00625ef..049a868 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -47,7 +47,7 @@ validate_install_dir() { # Disallow parent directory traversal segments like "../" or "/../" or "/.." # This allows valid paths with ".." in filenames (e.g., /opt/app..v1/bin) case "$dir" in - */../*|../*|*/..|/..*) + */../*|../*|*/..|/..) echo "Error: Install directory cannot contain parent directory segment '..': $dir" >&2 exit 1 ;; @@ -71,7 +71,7 @@ validate_install_dir() { # Disallow parent directory traversal segments in the normalized path case "$normalized_dir" in - */../*|../*|*/..|/..*) + */../*|../*|*/..|/..) echo "Error: Normalized install directory cannot contain parent directory segment '..': $normalized_dir" >&2 exit 1 ;; From 5a698beab2bb161707e750a999e6900a7ad3160e Mon Sep 17 00:00:00 2001 From: Yiftach Cohen Date: Wed, 14 Jan 2026 16:48:35 +0200 Subject: [PATCH 8/8] fix: correct boundary condition in maskValue length check Change `length <= 8` to `length < 10` so that 9-character values show exact asterisks instead of the misleading "10-20" range. --- internal/util/mask.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/util/mask.go b/internal/util/mask.go index 6640df1..1beae31 100644 --- a/internal/util/mask.go +++ b/internal/util/mask.go @@ -85,8 +85,8 @@ func maskValue(value string) string { if length == 0 { return "" } - if length <= 8 { - // For short values, show asterisks matching length + if length < 10 { + // For short values (up to 9 chars), show asterisks matching length return strings.Repeat("*", length) } // For longer values, show length range to prevent token type identification