fix: address security vulnerabilities across credential, path, and command handling by yiftach-armis · Pull Request #47 · ArmisSecurity/armis-cli

yiftach-armis · 2026-01-18T11:20:10Z

Description

This PR addresses multiple security vulnerabilities through comprehensive hardening across credential handling, path manipulation, command execution, and installation scripts.

Type of Change

Bug fix (non-breaking change which fixes an issue)
Security hardening

Changes Made

1. HTTPS Enforcement (API Client Security)

NewClient() now enforces HTTPS for all non-localhost URLs
Returns error instead of warning for non-HTTPS URLs with external hosts
Prevents credential transmission over unencrypted HTTP connections
Files: internal/api/client.go, internal/api/client_test.go
Impact: Protects against man-in-the-middle attacks on credentials

2. Enhanced Secret Masking

Extended credential patterns: SSH keys, OAuth secrets, database credentials, service tokens
Improved assignment operator detection (:=, =>)
Coverage for: client_secret, client_id, db_password, slack_token, github_token, signing_key, etc.
Files: internal/util/mask.go, internal/util/mask_test.go

3. Path Traversal Prevention (TOCTOU Fix)

SafeJoinPath() now validates base path exists and is a directory
SARIF output paths now sanitized
Files: internal/util/path.go, internal/util/path_test.go, internal/output/sarif.go
Impact: Prevents TOCTOU (Time-of-Check-Time-of-Use) vulnerabilities

4. Command Injection Prevention

GitHub Action replaced eval $SCAN_CMD with safe array-based execution
Added input validation for scan-type (repo/image only)
Commands built with bash arrays for safe expansion
Files: action.yml, .github/actions/armis-cli-action/action.yml

5. Least Privilege Principle

Added explicit permissions: contents: read to workflows
Files: .github/workflows/security-scan.yml, .github/workflows/armis-cli-marketplace-sample.yml

6. Installation Script Hardening

Cryptographically secure random temp directory names (GUID instead of predictable random)
Improved path traversal detection regex
Better error handling for temp directory creation
Files: scripts/install.sh, scripts/install.ps1

7. Defensive Coding

Nil response checks in HTTP client
Proper error handling for JSON marshaling in debug output
Files: internal/httpclient/client.go, internal/scan/image/image.go, internal/scan/repo/repo.go

Testing

Unit tests pass locally with my changes
All new security tests added and passing
Test coverage includes:
- HTTPS enforcement (localhost allowed, external HTTP rejected)
- Enhanced secret masking patterns
- Path traversal prevention scenarios
- Invalid input handling

Checklist

My code follows the project's code style
I have performed a self-review of my code
I have commented my code where necessary
My changes generate no new warnings
I have added tests that prove my fix is effective
New and existing unit tests pass locally with my changes

Related Issues

Addresses: Security audit findings

Notes

This PR includes a breaking change to NewClient() signature - it now returns (*Client, error) instead of *Client. All callers have been updated to handle the error return value.

…mmand handling This commit addresses multiple security vulnerabilities: - HTTPS Enforcement: NewClient() now enforces HTTPS for all non-localhost URLs, preventing credential transmission over unencrypted connections (CVE-class: credential exposure) - Enhanced Secret Masking: Extended credential masking patterns to detect SSH keys, OAuth secrets, database credentials, and service tokens across multiple formats - Path Traversal Prevention: SafeJoinPath() now requires base path to exist, preventing TOCTOU vulnerabilities - Command Injection Prevention: GitHub Action shell commands replaced with safe array-based execution and input validation (prevents eval-based injection) - Least Privilege: Added explicit permissions to workflows (contents: read) - Installation Script Hardening: Improved temp directory handling with cryptographically secure random names and better path traversal detection All changes include comprehensive test coverage.

github-actions · 2026-01-18T11:21:34Z

Test Coverage Report

total: (statements) 77.7%

Coverage by function

github.com/ArmisSecurity/armis-cli/cmd/armis-cli/main.go:16:			main					0.0%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:35:			WithHTTPClient				100.0%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:48:			NewClient				100.0%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:98:			IsDebug					100.0%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:103:			StartIngest				78.4%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:170:			GetIngestStatus				84.2%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:203:			WaitForIngest				0.0%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:245:			FetchNormalizedResults			78.6%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:292:			FetchAllNormalizedResults		91.7%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:317:			GetScanResult				66.7%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:345:			WaitForScan				0.0%
github.com/ArmisSecurity/armis-cli/internal/api/client.go:366:			formatBytes				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/context.go:14:			NewSignalContext			100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/context.go:21:			handleScanError				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:41:			SetVersion				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:49:			Execute					100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:53:			init					100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:65:			getEnvOrDefault				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:72:			getEnvOrDefaultInt			100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:82:			getAPIBaseURL				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:89:			getToken				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:96:			getTenantID				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:103:			getPageLimit				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:110:			validatePageLimit			100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:120:			validateFailOn				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/root.go:134:			getFailOn				100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/scan.go:21:			init					100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/scan_image.go:100:		init					100.0%
github.com/ArmisSecurity/armis-cli/internal/cmd/scan_repo.go:78:		init					100.0%
github.com/ArmisSecurity/armis-cli/internal/httpclient/client.go:30:		NewClient				100.0%
github.com/ArmisSecurity/armis-cli/internal/httpclient/client.go:56:		Do					85.3%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:31:			write					66.7%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:62:			Write					90.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:93:			Format					100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:98:			FormatWithOptions			96.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:136:		getSeverityIcon				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:153:		getSeverityColor			100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:182:		init					50.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:189:		disableColors				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:201:		sortFindingsBySeverity			100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:230:		loadSnippetFromFile			75.5%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:312:		formatCodeSnippet			0.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:349:		highlightColumns			0.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:385:		detectLanguage				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:683:		scanDuration				26.3%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:715:		renderSummaryDashboard			61.2%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:804:		renderFindings				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:819:		renderFinding				62.5%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:876:		renderGroupedFindings			100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:897:		groupFindings				96.6%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:952:		severityRank				75.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:966:		isGitRepo				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:973:		getGitBlame				0.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:1010:		parseGitBlame				85.7%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:1046:		maskEmail				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/human.go:1069:		getTopLevelDomain			75.0%
github.com/ArmisSecurity/armis-cli/internal/output/json.go:14:			Format					100.0%
github.com/ArmisSecurity/armis-cli/internal/output/json.go:21:			FormatWithOptions			66.7%
github.com/ArmisSecurity/armis-cli/internal/output/json.go:29:			formatWithDebug				0.0%
github.com/ArmisSecurity/armis-cli/internal/output/junit.go:43:			Format					83.3%
github.com/ArmisSecurity/armis-cli/internal/output/junit.go:67:			convertToJUnitCases			91.7%
github.com/ArmisSecurity/armis-cli/internal/output/junit.go:99:			countFailures				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/junit.go:112:		FormatWithOptions			100.0%
github.com/ArmisSecurity/armis-cli/internal/output/output.go:32:		GetFormatter				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/output.go:48:		ShouldFail				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/output.go:64:		ExitIfNeeded				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/sarif.go:89:			Format					100.0%
github.com/ArmisSecurity/armis-cli/internal/output/sarif.go:116:		buildRules				100.0%
github.com/ArmisSecurity/armis-cli/internal/output/sarif.go:141:		convertToSarifResults			85.7%
github.com/ArmisSecurity/armis-cli/internal/output/sarif.go:189:		severityToSarifLevel			100.0%
github.com/ArmisSecurity/armis-cli/internal/output/sarif.go:208:		severityToSecurityScore			100.0%
github.com/ArmisSecurity/armis-cli/internal/output/sarif.go:226:		FormatWithOptions			100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:25:		IsCI					100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:47:		NewReader				100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:62:		NewWriter				50.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:96:		NewSpinner				100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:104:		NewSpinnerWithTimeout			100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:120:		NewSpinnerWithContext			100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:128:		SetWriter				100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:137:		Start					89.6%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:225:		Stop					100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:260:		UpdateMessage				100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:267:		Update					100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:274:		GetElapsed				100.0%
github.com/ArmisSecurity/armis-cli/internal/progress/progress.go:281:		formatDuration				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/finding_type.go:9:		DeriveFindingType			100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:41:		NewScanner				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:55:		WithPollInterval			100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:61:		ScanImage				0.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:93:		ScanTarball				93.1%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:141:		exportImage				0.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:174:		isDockerAvailable			42.9%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:188:		getDockerCommand			75.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:197:		validateDockerCommand			100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:204:		buildScanResult				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:231:		convertNormalizedFindings		89.1%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:323:		shouldFilterByExploitability		100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:342:		cleanDescription			100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:361:		isEmptyFinding				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:374:		mapSeverity				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/image.go:389:		formatElapsed				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/image/validate.go:11:		validateImageName			100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/ignore.go:18:		LoadIgnorePatterns			75.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/ignore.go:52:		loadIgnoreFile				89.5%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/ignore.go:86:		Match					100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/ignore.go:98:		shouldSkipDir				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:39:		NewScanner				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:53:		WithPollInterval			100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:59:		Scan					84.6%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:146:		tarGzDirectory				71.8%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:223:		calculateDirSize			81.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:262:		shouldSkip				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:293:		isTestFile				88.9%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:336:		buildScanResult				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:363:		convertNormalizedFindings		89.1%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:455:		shouldFilterByExploitability		100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:474:		cleanDescription			100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:493:		isEmptyFinding				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:506:		mapSeverity				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/repo/repo.go:521:		formatElapsed				100.0%
github.com/ArmisSecurity/armis-cli/internal/scan/testhelpers/findings.go:9:	CreateNormalizedFinding			0.0%
github.com/ArmisSecurity/armis-cli/internal/scan/testhelpers/findings.go:14:	CreateNormalizedFindingWithLabels	0.0%
github.com/ArmisSecurity/armis-cli/internal/scan/testhelpers/findings.go:19:	CreateNormalizedFindingFull		0.0%
github.com/ArmisSecurity/armis-cli/internal/util/mask.go:54:			MaskSecretInLine			81.2%
github.com/ArmisSecurity/armis-cli/internal/util/mask.go:93:			maskValue				83.3%
github.com/ArmisSecurity/armis-cli/internal/util/mask.go:119:			MaskSecretInLines			100.0%
github.com/ArmisSecurity/armis-cli/internal/util/path.go:13:			SanitizePath				90.9%
github.com/ArmisSecurity/armis-cli/internal/util/path.go:51:			SafeJoinPath				87.5%
github.com/ArmisSecurity/armis-cli/test/sample-repo/src/main.go:6:		main					0.0%
total:										(statements)				77.7%

yiftach-armis merged commit 8c295d1 into main Jan 18, 2026
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: address security vulnerabilities across credential, path, and command handling#47

fix: address security vulnerabilities across credential, path, and command handling#47
yiftach-armis merged 1 commit intomainfrom
fix/security-vulns

yiftach-armis commented Jan 18, 2026

Uh oh!

github-actions bot commented Jan 18, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

yiftach-armis commented Jan 18, 2026

Description

Type of Change

Changes Made

1. HTTPS Enforcement (API Client Security)

2. Enhanced Secret Masking

3. Path Traversal Prevention (TOCTOU Fix)

4. Command Injection Prevention

5. Least Privilege Principle

6. Installation Script Hardening

7. Defensive Coding

Testing

Checklist

Related Issues

Notes

Uh oh!

github-actions bot commented Jan 18, 2026

Test Coverage Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant