diff --git a/CHANGELOG.md b/CHANGELOG.md index 470706c..5ee8458 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,65 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed +- **Behavioral:** TruffleHog secret scanning no longer passes `--no-verification` + when `trufflehog_show_unverified` is off. That flag disabled verification + entirely, so every finding came back with `Verified: false` and was reported as + low severity — meaning no secret ever blocked a run on the setting's default + path, the exact inverse of the intended behavior. Verification now always runs, + and the setting controls only which result types are returned: + `--results=verified,unknown` when off, + `--results=verified,unverified,unknown` when on. Verified findings are critical + and blocking; unverified and unknown findings remain low and non-blocking. + + **Three consequences on upgrade — read before bumping:** + + 1. **Runs will start failing that previously passed.** With + `trufflehog_show_unverified` off, verified secrets are now reported as + critical and block. Previously they were downgraded to low and ignored. This + is the intended behavior, but it lands as newly-red pipelines on the first + run after upgrading. It is not a new detection — those secrets were always + there, they were just never surfaced as blocking. + 2. **TruffleHog now makes outbound network requests.** Verification is a live + check against third-party credential-validation endpoints (AWS, GitHub, + Slack, etc.) for every candidate secret. Runs that previously scanned fully + offline no longer do. + 3. **Air-gapped and proxied environments will surface unknown results.** When + verification cannot reach a validation endpoint, TruffleHog classifies the + result as `unknown` rather than verified or unverified. Unknown results are + returned by default and reported as low severity/non-blocking so a scanner + with no egress does not silently appear clean. (#110) + +- **Behavioral:** a TruffleHog run that exits non-zero, or a missing + `trufflehog` binary, now **fails the run** instead of being reported as a + clean scan. Previously any non-zero exit was logged and turned into an empty + result, so a malformed exclude pattern or a broken install silently zeroed out + every secret finding while the run exited green — a scanner that could not + scan looked identical to a repository with no secrets. Socket Basics now also + passes TruffleHog's `--fail-on-scan-errors` flag so source/enumeration errors + produce the non-zero exit that the wrapper enforces. The error names the exit + code and TruffleHog's own stderr. This closes the last open item from CE-347, + whose other halves shipped in 2.2.1. (#110) +- `trufflehog_show_unverified` is now read through `coerce_bool` rather than + tested for truthiness. Only the environment loader coerces boolean params; + a Socket dashboard config is passed through verbatim and at higher priority, + so a dashboard-supplied string `"false"` was truthy and would have reported + unverified secrets to someone who explicitly turned them off. (#110) +- `secret_scanning_enabled` and the TruffleHog `scan_all` fallback check now use + the same boolean coercion, so dashboard/JSON strings such as `"false"` cannot + unexpectedly enable secret scanning or widen a staged-file scan. (#110) + +### Changed +- `--include-detectors=all` is now passed unconditionally rather than only when + `trufflehog_show_unverified` is on, so detector selection no longer changes as a + side effect of that setting. TruffleHog already defaults to all detectors, so + this is a no-op in practice. (#110) +- Clarified TruffleHog parameter documentation: `trufflehog_exclude_dir` accepts + directory names, file names, and glob patterns (not just directories), matching + is case-sensitive, and excluded paths are removed from the scan entirely rather + than filtered from results. `trufflehog_show_unverified` is documented as + widening result types, not as toggling verification. (#110) + ## [3.1.0] - 2026-09-02 ### Added diff --git a/action.yml b/action.yml index d200ae2..e5906d6 100644 --- a/action.yml +++ b/action.yml @@ -444,7 +444,7 @@ inputs: required: false default: "" trufflehog_show_unverified: - description: "Show unverified secrets in TruffleHog results" + description: "Include unverified secrets in TruffleHog results; verification always runs, and by default verified and unknown results are reported" required: false default: "false" use_custom_sast_rules: diff --git a/docs/github-action.md b/docs/github-action.md index db7165e..7e50d22 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -222,12 +222,20 @@ Include these in your workflow's `jobs..permissions` section. with: github_token: ${{ secrets.GITHUB_TOKEN }} secret_scanning_enabled: 'true' - # Optional: exclude directories - trufflehog_exclude_dir: 'node_modules,vendor,dist' - # Optional: show unverified secrets + # Optional: exclude paths (directory names, file names, or globs) + trufflehog_exclude_dir: 'node_modules,vendor,dist,**/appsettings.*.json' + # Optional: report unverified secrets too (default: verified and unknown) trufflehog_show_unverified: 'true' ``` +> **Secret verification runs on every scan and requires network egress.** By default only +> verified and unknown results are reported. Verified secrets are critical and blocking; +> unknown results are low severity and nonblocking. TruffleHog confirms each candidate +> against third-party validation endpoints, and a runner that cannot reach one reports the +> candidate as `unknown` instead of silently dropping it. Set +> `trufflehog_show_unverified: 'true'` to include candidates that were checked but not +> confirmed as valid as well. + **Container Scanning:** ```yaml - uses: SocketDev/socket-basics@v3.1.0 @@ -849,8 +857,8 @@ See [`action.yml`](../action.yml) for the complete list of inputs. **Security Scanning:** - `secret_scanning_enabled` — Enable secret scanning -- `trufflehog_exclude_dir` — Directories to exclude -- `trufflehog_show_unverified` — Show unverified secrets +- `trufflehog_exclude_dir` — Comma-separated paths to exclude (directory names, file names, or globs) +- `trufflehog_show_unverified` — Include unverified secrets alongside the verified and unknown results reported by default - `socket_tier_1_enabled` — Socket Tier 1 reachability **Container Scanning (configuration surface):** diff --git a/docs/parameters.md b/docs/parameters.md index f40ec6b..c5a3e7a 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -376,7 +376,9 @@ socket-basics --disable-secrets ### `--exclude-dir EXCLUDE_DIR` Comma-separated literal directory/file names or glob patterns to exclude from secret scanning beneath the workspace root. Matching is case-sensitive. For -example, `**/appsettings.*.json` matches files at any directory depth. +example, `**/appsettings.*.json` matches files at any directory depth. Excluded +paths are removed from the scan entirely — they are not scanned for verified or +unverified secrets. **Example:** ```bash @@ -392,7 +394,20 @@ socket-basics --secrets --trufflehog-notify slack ``` ### `--show-unverified` -Show unverified secrets in TruffleHog results (by default only verified secrets are shown). +Include unverified secrets in TruffleHog results. TruffleHog always performs verification; +this flag only widens which result types are reported. By default verified and unknown +results are returned (`--results=verified,unknown`); with this flag, verified, unverified, +and unknown results are all returned (`--results=verified,unverified,unknown`). + +Verified findings are reported as critical and block. Unverified and unknown findings are +reported as low and do not block; unknown means verification could not complete because of +a network or API error. + +> **Verification makes live network requests.** TruffleHog validates candidate secrets +> against third-party endpoints (AWS, GitHub, Slack, and so on). If a runner cannot reach +> those endpoints, the result is classified as `unknown` and returned as a low-severity, +> nonblocking finding. This keeps verification failures visible on air-gapped or proxied +> runners without treating an inconclusive candidate as a confirmed live credential. **Example:** ```bash @@ -660,7 +675,7 @@ You can provide configuration via a JSON file using `--config`: "secrets_enabled": true, "trufflehog_exclude_dir": "node_modules,vendor,dist,.git", - "show_unverified": false, + "trufflehog_show_unverified": false, "socket_tier_1_enabled": true, "socket_org": "your-org-slug", diff --git a/socket_basics/connectors.yaml b/socket_basics/connectors.yaml index b18f461..392ae84 100644 --- a/socket_basics/connectors.yaml +++ b/socket_basics/connectors.yaml @@ -418,7 +418,7 @@ connectors: default: "" - name: trufflehog_show_unverified option: --show-unverified - description: "Show unverified secrets in TruffleHog results" + description: "Include unverified secrets in TruffleHog results; verification always runs, and by default verified and unknown results are reported" env_variable: INPUT_TRUFFLEHOG_SHOW_UNVERIFIED type: bool default: false diff --git a/socket_basics/core/connector/trufflehog/__init__.py b/socket_basics/core/connector/trufflehog/__init__.py index eeec661..715668e 100644 --- a/socket_basics/core/connector/trufflehog/__init__.py +++ b/socket_basics/core/connector/trufflehog/__init__.py @@ -16,6 +16,10 @@ from ..base import BaseConnector +# coerce_bool lives in the config layer because the environment loader, a +# Socket dashboard config, and a JSON config each deliver booleans differently. +from ...config import coerce_bool + # Import individual notifier modules from . import github_pr, slack, ms_teams, ms_sentinel, sumologic, console, jira, webhook, json_notifier @@ -33,7 +37,9 @@ def __init__(self, config): def is_enabled(self) -> bool: """Check if secret scanning should be enabled""" - return self.config.get('secret_scanning_enabled', False) + return coerce_bool( + self.config.get('secret_scanning_enabled'), False + ) @staticmethod def _path_regex(value: str) -> str: @@ -258,7 +264,7 @@ def scan(self) -> Dict[str, Any]: if ( not changed_files and not scope_requested - and not self.config.get('scan_all', False) + and not coerce_bool(self.config.get('scan_all'), False) ): try: from socket_basics.core.config import _detect_git_changed_files @@ -266,11 +272,31 @@ def scan(self) -> Dict[str, Any]: except Exception: changed_files = [] + # Verification always runs so that findings carry a trustworthy + # Verified flag; the setting only controls which result types are + # returned. Detector selection is deliberately independent of it. + # + # coerce_bool, not truthiness: only the environment loader coerces + # bool params, while a Socket dashboard config is passed through + # verbatim at higher priority. A dashboard-supplied string "false" + # is truthy, and reading it as "on" would report unverified secrets + # to someone who explicitly left unverified results off. + show_unverified = coerce_bool( + self.config.get('trufflehog_show_unverified'), False + ) + results_filter = ( + 'verified,unverified,unknown' + if show_unverified + else 'verified,unknown' + ) + cmd = [ 'trufflehog', 'filesystem', '--json', - '--no-verification' if not self.config.get('trufflehog_show_unverified', False) else '--include-detectors=all' + '--include-detectors=all', + '--fail-on-scan-errors', + f'--results={results_filter}', ] # TruffleHog accepts --exclude-paths only once and expects a file @@ -315,8 +341,23 @@ def scan(self) -> Dict[str, Any]: result = subprocess.run(cmd, capture_output=True, text=True) if result.returncode != 0: - logger.error(f"Trufflehog failed: {result.stderr}") - return {} + # Fail closed. Returning {} here reports "no secrets found" and + # exits green, so a malformed exclude pattern, source error, or + # broken install can silently hide all or part of the scan. + # An incomplete scanner run must not look like a clean scan. + # SystemExit is deliberate: the connector manager catches + # Exception, and this must not be downgraded to a skipped + # connector. + stderr = (result.stderr or '').strip() + detail = f": {stderr}" if stderr else '' + raise SystemExit( + f"TruffleHog exited {result.returncode} before the scan " + "completed successfully" + f"{detail}\nSecret scanning results are incomplete, so the " + "run is failing rather than reporting a clean scan. Check " + "the exclude patterns in 'trufflehog_exclude_dir' and that " + "the trufflehog binary is working." + ) # Parse JSON output line by line findings = [] @@ -374,7 +415,12 @@ def scan(self) -> Dict[str, Any]: } except FileNotFoundError: - logger.error("Trufflehog not found. Please install Trufflehog") + # Also fail closed: secret scanning was asked for and did not run. + raise SystemExit( + "TruffleHog is enabled but the 'trufflehog' binary was not " + "found, so no secret scanning ran. Install TruffleHog or use " + "the Socket Basics container image, which bundles it." + ) except Exception as e: logger.error(f"Error running Trufflehog: {e}") finally: @@ -474,6 +520,24 @@ def _create_alert(self, finding: Dict[str, Any]) -> Dict[str, Any]: """Create a generic alert from a Trufflehog finding""" detector_name = finding.get('DetectorName', 'unknown') verified = finding.get('Verified', False) + verification_error = finding.get('VerificationError') + if verified: + verification_status = 'verified' + risk_assessment = ( + "**CRITICAL**: This secret has been verified and is likely active!" + ) + elif verification_error: + verification_status = 'unknown' + risk_assessment = ( + "**LOW**: Verification could not complete, so this secret's " + "validity is unknown." + ) + else: + verification_status = 'unverified' + risk_assessment = ( + "**LOW**: This appears to be a potential secret but was not " + "confirmed as valid." + ) file_path = finding.get('SourceMetadata', {}).get('Data', {}).get('Filesystem', {}).get('file', 'unknown') line = finding.get('SourceMetadata', {}).get('Data', {}).get('Filesystem', {}).get('line', 0) @@ -492,11 +556,11 @@ def _create_alert(self, finding: Dict[str, Any]) -> Dict[str, Any]: - **File**: `{file_path}` - **Line**: {line} - **Detector**: {detector_name} -- **Verified**: {"✅ Yes" if verified else "❌ No"} +- **Verification status**: {verification_status} - **Redacted Value**: `{redacted_secret}` ### Risk Assessment -{"**CRITICAL**: This secret has been verified and is likely active!" if verified else "**LOW**: This appears to be a potential secret but has not been verified."} +{risk_assessment} ### Immediate Actions Required 1. **Rotate the credential immediately** @@ -534,6 +598,7 @@ def _create_alert(self, finding: Dict[str, Any]) -> Dict[str, Any]: "props": { "ruleId": detector_name, "verified": verified, + "verificationStatus": verification_status, "filePath": file_path, "lineNumber": line, "secretType": detector_name.lower(), diff --git a/tests/test_changed_files_scope.py b/tests/test_changed_files_scope.py index ec66254..f2a5603 100644 --- a/tests/test_changed_files_scope.py +++ b/tests/test_changed_files_scope.py @@ -887,7 +887,9 @@ def record_run(cmd, *args, **kwargs): "trufflehog", "filesystem", "--json", - "--no-verification", + "--include-detectors=all", + "--fail-on-scan-errors", + "--results=verified,unknown", str(pr_repo), ] ] diff --git a/tests/test_trufflehog_excludes.py b/tests/test_trufflehog_excludes.py index df7199f..e0c54f2 100644 --- a/tests/test_trufflehog_excludes.py +++ b/tests/test_trufflehog_excludes.py @@ -4,6 +4,8 @@ import re from types import SimpleNamespace +import pytest + from socket_basics.core.connector.trufflehog import TruffleHogScanner @@ -488,6 +490,9 @@ def fake_run(command, **kwargs): fake_run, ) - scanner.scan() + # A failed run surfaces rather than reporting a clean scan (CE-347); the + # temporary filter file must still be cleaned up on that path. + with pytest.raises(SystemExit): + scanner.scan() assert not captured["exclude_path"].exists() diff --git a/tests/test_trufflehog_verification.py b/tests/test_trufflehog_verification.py new file mode 100644 index 0000000..6753b0f --- /dev/null +++ b/tests/test_trufflehog_verification.py @@ -0,0 +1,241 @@ +"""Regression coverage for TruffleHog verification behavior. + +``trufflehog_show_unverified`` selects which *result types* TruffleHog returns. +It must never disable verification itself: severity is derived from each +finding's ``Verified`` flag, so a run with verification turned off reports every +secret as unverified/low and nothing ever blocks. Unknown results remain visible +by default because they indicate that verification could not complete. +""" + +from pathlib import Path +from types import SimpleNamespace + +import pytest + +from socket_basics.core.connector.trufflehog import TruffleHogScanner + + +def _scanner( + tmp_path, + show_unverified, + *, + scan_all=True, + secret_scanning_enabled=True, +): + values = { + "secret_scanning_enabled": secret_scanning_enabled, + "scan_all": scan_all, + "trufflehog_exclude_dir": "", + "trufflehog_show_unverified": show_unverified, + } + config = SimpleNamespace(workspace=tmp_path, _config=values) + config.get = lambda key, default=None: values.get(key, default) + config.get_action_for_severity = lambda severity: { + "critical": "error", + "low": "ignore", + }[severity] + config.get_scan_targets = lambda: [str(tmp_path)] + scanner = TruffleHogScanner.__new__(TruffleHogScanner) + scanner.config = config + return scanner + + +def _captured_cmd(tmp_path, monkeypatch, show_unverified): + invocations = [] + + def record_run(cmd, *args, **kwargs): + invocations.append(cmd) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setattr( + "socket_basics.core.connector.trufflehog.subprocess.run", record_run + ) + _scanner(tmp_path, show_unverified).scan() + + assert len(invocations) == 1 + return invocations[0] + + +def test_show_unverified_off_requests_high_confidence_results(tmp_path, monkeypatch): + cmd = _captured_cmd(tmp_path, monkeypatch, show_unverified=False) + + assert "--results=verified,unknown" in cmd + assert "--no-verification" not in cmd + + +def test_show_unverified_on_requests_every_result_type(tmp_path, monkeypatch): + cmd = _captured_cmd(tmp_path, monkeypatch, show_unverified=True) + + assert "--results=verified,unverified,unknown" in cmd + assert "--no-verification" not in cmd + + +def test_trufflehog_scan_errors_are_fatal(tmp_path, monkeypatch): + cmd = _captured_cmd(tmp_path, monkeypatch, show_unverified=False) + + assert "--fail-on-scan-errors" in cmd + + +def test_string_false_from_a_dashboard_config_excludes_unverified( + tmp_path, monkeypatch +): + """A dashboard config is passed through verbatim, so "false" arrives as a string. + + Only the environment loader coerces bool params, and dashboard config + outranks it. Reading the raw value for truthiness would report unverified + secrets to someone who explicitly turned them off. + """ + for raw in ("false", "False", "0", "no"): + cmd = _captured_cmd(tmp_path, monkeypatch, show_unverified=raw) + assert "--results=verified,unknown" in cmd, raw + + +def test_string_true_from_a_dashboard_config_widens_result_types( + tmp_path, monkeypatch +): + for raw in ("true", "True", "1", "yes"): + cmd = _captured_cmd(tmp_path, monkeypatch, show_unverified=raw) + assert "--results=verified,unverified,unknown" in cmd, raw + + +def test_unset_setting_defaults_to_high_confidence_results(tmp_path, monkeypatch): + """An unset action input arrives as an empty string, not as None.""" + for raw in (None, ""): + cmd = _captured_cmd(tmp_path, monkeypatch, show_unverified=raw) + assert "--results=verified,unknown" in cmd, repr(raw) + + +def test_string_false_disables_secret_scanning(tmp_path, monkeypatch): + def unexpected_run(*args, **kwargs): + raise AssertionError("TruffleHog should not run when scanning is disabled") + + monkeypatch.setattr( + "socket_basics.core.connector.trufflehog.subprocess.run", unexpected_run + ) + + for raw in ("false", "False", "0", "no"): + assert _scanner( + tmp_path, + show_unverified=False, + secret_scanning_enabled=raw, + ).scan() == {} + + +def test_string_false_scan_all_uses_staged_file_fallback(tmp_path, monkeypatch): + changed_file = tmp_path / "changed.py" + changed_file.write_text("print('changed')\n", encoding="utf-8") + invocations = [] + + monkeypatch.setattr( + "socket_basics.core.config._detect_git_changed_files", + lambda *args, **kwargs: ["changed.py"], + ) + monkeypatch.setattr( + "socket_basics.core.connector.trufflehog.subprocess.run", + lambda cmd, *args, **kwargs: ( + invocations.append(cmd) + or SimpleNamespace(returncode=0, stdout="", stderr="") + ), + ) + + _scanner(tmp_path, show_unverified=False, scan_all="false").scan() + + assert len(invocations) == 1 + assert str(changed_file) in invocations[0] + assert str(tmp_path) not in invocations[0] + + +def test_detector_selection_is_independent_of_the_setting(tmp_path, monkeypatch): + """Toggling the setting must not change which detectors run.""" + off = _captured_cmd(tmp_path, monkeypatch, show_unverified=False) + on = _captured_cmd(tmp_path, monkeypatch, show_unverified=True) + + detectors = [arg for arg in off if arg.startswith("--include-detectors")] + assert detectors == ["--include-detectors=all"] + assert detectors == [arg for arg in on if arg.startswith("--include-detectors")] + + +def _finding(verified, verification_error=None): + finding = { + "DetectorName": "AWS", + "Verified": verified, + "Raw": "AKIAIOSFODNN7EXAMPLE", + "SourceMetadata": { + "Data": {"Filesystem": {"file": "config/secrets.py", "line": 12}} + }, + } + if verification_error: + finding["VerificationError"] = verification_error + return finding + + +def test_verified_findings_are_critical_and_blocking(tmp_path): + alert = _scanner(tmp_path, show_unverified=False)._create_alert(_finding(True)) + + assert alert["severity"] == "critical" + assert alert["action"] == "error" + assert alert["props"]["verified"] is True + assert alert["props"]["verificationStatus"] == "verified" + assert alert["props"]["riskLevel"] == "critical" + + +def test_unverified_findings_are_low_and_nonblocking(tmp_path): + alert = _scanner(tmp_path, show_unverified=True)._create_alert(_finding(False)) + + assert alert["severity"] == "low" + assert alert["action"] == "ignore" + assert alert["props"]["verified"] is False + assert alert["props"]["verificationStatus"] == "unverified" + assert alert["props"]["riskLevel"] == "low" + + +def test_unknown_findings_are_low_and_identified_as_unknown(tmp_path): + alert = _scanner(tmp_path, show_unverified=False)._create_alert( + _finding(False, verification_error="network is unreachable") + ) + + assert alert["severity"] == "low" + assert alert["action"] == "ignore" + assert alert["props"]["verified"] is False + assert alert["props"]["verificationStatus"] == "unknown" + assert "validity is unknown" in alert["props"]["detailedReport"]["content"] + + +def test_a_failed_trufflehog_run_fails_the_scan(tmp_path, monkeypatch): + """A non-zero exit must not be reported as a clean scan. + + Returning {} here exits green having scanned nothing, so a malformed + exclude pattern or a broken install silently zeroes out every secret + finding for the run (CE-347). + """ + def failing_run(cmd, *args, **kwargs): + return SimpleNamespace( + returncode=1, + stdout="", + stderr="trufflehog: error: flag 'exclude-paths' cannot be repeated", + ) + + monkeypatch.setattr( + "socket_basics.core.connector.trufflehog.subprocess.run", failing_run + ) + + with pytest.raises(SystemExit) as excinfo: + _scanner(tmp_path, show_unverified=False).scan() + + message = str(excinfo.value) + assert "exited 1" in message + assert "cannot be repeated" in message + + +def test_a_missing_trufflehog_binary_fails_the_scan(tmp_path, monkeypatch): + def missing_binary(cmd, *args, **kwargs): + raise FileNotFoundError(2, "No such file or directory", "trufflehog") + + monkeypatch.setattr( + "socket_basics.core.connector.trufflehog.subprocess.run", missing_binary + ) + + with pytest.raises(SystemExit) as excinfo: + _scanner(tmp_path, show_unverified=False).scan() + + assert "not" in str(excinfo.value).lower()