From 0bc409b5b42474e161ee87a31e391d489f2455ef Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:20:16 -0400 Subject: [PATCH 1/4] fix: stop disabling TruffleHog verification for verified-only scans Turning trufflehog_show_unverified off passed --no-verification, which disabled verification outright instead of returning only verified findings. Severity is derived from each finding's Verified flag, so every result came back Verified: false -> low severity -> non-blocking. On the setting's default path no secret could ever block a run: the exact inverse of intent. Verification now always runs, and the setting selects result types only: --results=verified when off, --results=verified,unverified,unknown when on (the latter matching TruffleHog's own default). Verified findings stay critical/blocking, unverified stay low/non-blocking. --include-detectors=all is now passed unconditionally so detector selection no longer changes as a side effect of the setting. TruffleHog already defaults to all detectors, so this is a no-op in practice. Verified against the pinned trufflesecurity/trufflehog:3.96.0 binary: an unverifiable private key is returned under the "on" filter and dropped under the "off" filter. Two upgrade consequences, documented in the changelog: runs with the setting off will start reporting blocking critical findings previously downgraded to low, and verification is a live check, so TruffleHog now makes outbound requests to third-party credential-validation endpoints. Also corrects TruffleHog parameter docs (exclude paths accept files and globs, not just directories; show-unverified widens result types rather than toggling verification) and a JSON config example that named a nonexistent show_unverified key. Fixes CE-351 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 43 ++++++++ action.yml | 2 +- docs/github-action.md | 17 ++- docs/parameters.md | 21 +++- socket_basics/connectors.yaml | 2 +- .../core/connector/trufflehog/__init__.py | 11 +- tests/test_changed_files_scope.py | 3 +- tests/test_trufflehog_verification.py | 100 ++++++++++++++++++ 8 files changed, 187 insertions(+), 12 deletions(-) create mode 100644 tests/test_trufflehog_verification.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 470706c..2cc4161 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,49 @@ 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` when off, `--results=verified,unverified,unknown` when on. + Verified findings are critical and blocking; unverified 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 under-report.** When verification + cannot reach a validation endpoint, TruffleHog classifies the result as + `unknown` rather than verified or unverified. With + `trufflehog_show_unverified` off, `--results=verified` drops those, so a + scanner with no egress reports *zero* findings rather than failing loudly. + **If your runners cannot reach the public internet, set + `trufflehog_show_unverified: true`** so `unknown` results are still + reported (as low severity). (#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..2ac0df5 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 and unknown secrets in TruffleHog results; verification always runs, and by default only verified secrets are reported" required: false default: "false" use_custom_sast_rules: diff --git a/docs/github-action.md b/docs/github-action.md index db7165e..176aba8 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -222,12 +222,19 @@ 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 and unknown secrets too (default: verified only) trufflehog_show_unverified: 'true' ``` +> **Secret verification runs on every scan and requires network egress.** By default only +> verified secrets are reported, and those are critical and blocking. TruffleHog confirms +> each candidate against third-party validation endpoints; when a runner cannot reach them +> the result is `unknown` and is dropped in the default verified-only mode. On air-gapped or +> proxied runners, set `trufflehog_show_unverified: 'true'` so `unknown` results are still +> reported as low-severity findings instead of silently disappearing. + **Container Scanning:** ```yaml - uses: SocketDev/socket-basics@v3.1.0 @@ -849,8 +856,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 and unknown secrets alongside verified ones - `socket_tier_1_enabled` — Socket Tier 1 reachability **Container Scanning (configuration surface):** diff --git a/docs/parameters.md b/docs/parameters.md index f40ec6b..951da93 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 and unknown secrets in TruffleHog results. TruffleHog always performs +verification; this flag only widens which result types are reported. By default only +verified secrets are returned (`--results=verified`); with this flag, verified, unverified, +and unknown results are all returned (`--results=verified,unverified,unknown`). + +Verified findings are reported as critical and block; unverified findings are reported as +low and do not block. + +> **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`, which is *not* returned in the +> default verified-only mode — an air-gapped scan will report zero findings rather than +> failing. Set `--show-unverified` on egress-restricted runners so `unknown` results are +> still reported. **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..ba0e93e 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 and unknown secrets in TruffleHog results; verification always runs, and by default only verified secrets 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..b4eeefa 100644 --- a/socket_basics/core/connector/trufflehog/__init__.py +++ b/socket_basics/core/connector/trufflehog/__init__.py @@ -266,11 +266,20 @@ 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. + show_unverified = self.config.get('trufflehog_show_unverified', False) + results_filter = ( + 'verified,unverified,unknown' if show_unverified else 'verified' + ) + cmd = [ 'trufflehog', 'filesystem', '--json', - '--no-verification' if not self.config.get('trufflehog_show_unverified', False) else '--include-detectors=all' + '--include-detectors=all', + f'--results={results_filter}', ] # TruffleHog accepts --exclude-paths only once and expects a file diff --git a/tests/test_changed_files_scope.py b/tests/test_changed_files_scope.py index ec66254..54a1717 100644 --- a/tests/test_changed_files_scope.py +++ b/tests/test_changed_files_scope.py @@ -887,7 +887,8 @@ def record_run(cmd, *args, **kwargs): "trufflehog", "filesystem", "--json", - "--no-verification", + "--include-detectors=all", + "--results=verified", str(pr_repo), ] ] diff --git a/tests/test_trufflehog_verification.py b/tests/test_trufflehog_verification.py new file mode 100644 index 0000000..b1a8269 --- /dev/null +++ b/tests/test_trufflehog_verification.py @@ -0,0 +1,100 @@ +"""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. +""" + +from types import SimpleNamespace + +from socket_basics.core.connector.trufflehog import TruffleHogScanner + + +def _scanner(tmp_path, show_unverified): + values = { + "secret_scanning_enabled": True, + "scan_all": True, + "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 + scanner.is_enabled = lambda: True + 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_verified_results_only(tmp_path, monkeypatch): + cmd = _captured_cmd(tmp_path, monkeypatch, show_unverified=False) + + assert "--results=verified" 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_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): + return { + "DetectorName": "AWS", + "Verified": verified, + "Raw": "AKIAIOSFODNN7EXAMPLE", + "SourceMetadata": { + "Data": {"Filesystem": {"file": "config/secrets.py", "line": 12}} + }, + } + + +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"]["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"]["riskLevel"] == "low" From 12048e0e4afaf0180221ed603c0219a05844be15 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Thu, 3 Sep 2026 14:55:48 -0400 Subject: [PATCH 2/4] fix: read trufflehog_show_unverified through coerce_bool Only the environment loader coerces bool params (config.py:924). A Socket dashboard config is passed through verbatim (config.py:1309) and outranks env, so a dashboard-supplied string "false" reached the connector as a truthy value. Under the old --no-verification code that misread was a harmless no-op: it selected --include-detectors=all, which is TruffleHog's default anyway. Now it would select --results=verified,unverified,unknown and report unverified secrets to someone who explicitly asked for verified-only, so the same latent misread became a real behavioral bug. Covers "false"/"False"/"0"/"no", the true forms, and the unset cases (None and the empty string an unset action input forwards). Verified the new test fails against the truthiness read and passes with coerce_bool. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 6 ++++ .../core/connector/trufflehog/__init__.py | 14 ++++++++- tests/test_trufflehog_verification.py | 29 +++++++++++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2cc4161..26a91af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `trufflehog_show_unverified: true`** so `unknown` results are still reported (as low severity). (#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) + ### 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 diff --git a/socket_basics/core/connector/trufflehog/__init__.py b/socket_basics/core/connector/trufflehog/__init__.py index b4eeefa..971e3c3 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 @@ -269,7 +273,15 @@ def scan(self) -> Dict[str, Any]: # 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. - show_unverified = self.config.get('trufflehog_show_unverified', False) + # + # 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 asked for verified-only. + show_unverified = coerce_bool( + self.config.get('trufflehog_show_unverified'), False + ) results_filter = ( 'verified,unverified,unknown' if show_unverified else 'verified' ) diff --git a/tests/test_trufflehog_verification.py b/tests/test_trufflehog_verification.py index b1a8269..99059b4 100644 --- a/tests/test_trufflehog_verification.py +++ b/tests/test_trufflehog_verification.py @@ -61,6 +61,35 @@ def test_show_unverified_on_requests_every_result_type(tmp_path, monkeypatch): assert "--no-verification" not in cmd +def test_string_false_from_a_dashboard_config_stays_verified_only( + 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" 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_verified_only(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" in cmd, repr(raw) + + 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) From 64cf02832c5233ec31c354913ae186caf21d3a5d Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Thu, 3 Sep 2026 15:05:40 -0400 Subject: [PATCH 3/4] fix: fail the run when TruffleHog cannot scan Any non-zero exit was logged and converted 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. Non-zero exits and a missing trufflehog binary now raise SystemExit with the exit code and TruffleHog's own stderr. SystemExit is deliberate: the connector manager catches Exception, and this must not be downgraded to a skipped connector. The exclude-file cleanup in the finally block still runs on the failure path. This closes the last open acceptance criterion from CE-347 ("a malformed/unopenable exclude value no longer causes trufflehog to silently return zero findings for the whole run"). Its other two criteria shipped in 2.2.1 via #94, which referenced CE-347 without closing it. Matches the fail-closed idiom already used for an unresolvable changed_files scope, which also raises SystemExit with an actionable message. test_scan_cleans_exclude_file_when_trufflehog_fails asserted the old swallowing behavior; it now expects the failure to surface while still asserting the temp filter file is cleaned up. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 8 ++++ .../core/connector/trufflehog/__init__.py | 25 +++++++++-- tests/test_trufflehog_excludes.py | 7 ++- tests/test_trufflehog_verification.py | 43 +++++++++++++++++++ 4 files changed, 79 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 26a91af..0053a8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). `trufflehog_show_unverified: true`** so `unknown` results are still reported (as low severity). (#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. 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, diff --git a/socket_basics/core/connector/trufflehog/__init__.py b/socket_basics/core/connector/trufflehog/__init__.py index 971e3c3..572d2d0 100644 --- a/socket_basics/core/connector/trufflehog/__init__.py +++ b/socket_basics/core/connector/trufflehog/__init__.py @@ -336,8 +336,22 @@ 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 or a broken + # install silently zeroes out every secret finding for the run. + # A scanner that cannot scan 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} and scanned nothing" + 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 = [] @@ -395,7 +409,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: 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 index 99059b4..e749f21 100644 --- a/tests/test_trufflehog_verification.py +++ b/tests/test_trufflehog_verification.py @@ -6,8 +6,11 @@ secret as unverified/low and nothing ever blocks. """ +from pathlib import Path from types import SimpleNamespace +import pytest + from socket_basics.core.connector.trufflehog import TruffleHogScanner @@ -127,3 +130,43 @@ def test_unverified_findings_are_low_and_nonblocking(tmp_path): assert alert["action"] == "ignore" assert alert["props"]["verified"] is False assert alert["props"]["riskLevel"] == "low" + + +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() From 6091e52025dd0807dfe7571e94be8555de5414f7 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Wed, 9 Sep 2026 13:19:13 -0400 Subject: [PATCH 4/4] fix: incorporate TruffleHog review feedback --- CHANGELOG.md | 30 +++--- action.yml | 2 +- docs/github-action.md | 15 +-- docs/parameters.md | 18 ++-- socket_basics/connectors.yaml | 2 +- .../core/connector/trufflehog/__init__.py | 45 +++++++-- tests/test_changed_files_scope.py | 3 +- tests/test_trufflehog_verification.py | 95 ++++++++++++++++--- 8 files changed, 154 insertions(+), 56 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0053a8f..5ee8458 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,9 +15,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). 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` when off, `--results=verified,unverified,unknown` when on. - Verified findings are critical and blocking; unverified findings remain low and - non-blocking. + `--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:** @@ -31,28 +31,30 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). 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 under-report.** When verification - cannot reach a validation endpoint, TruffleHog classifies the result as - `unknown` rather than verified or unverified. With - `trufflehog_show_unverified` off, `--results=verified` drops those, so a - scanner with no egress reports *zero* findings rather than failing loudly. - **If your runners cannot reach the public internet, set - `trufflehog_show_unverified: true`** so `unknown` results are still - reported (as low severity). (#110) + 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. 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) + 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 diff --git a/action.yml b/action.yml index 2ac0df5..e5906d6 100644 --- a/action.yml +++ b/action.yml @@ -444,7 +444,7 @@ inputs: required: false default: "" trufflehog_show_unverified: - description: "Include unverified and unknown secrets in TruffleHog results; verification always runs, and by default only verified secrets are reported" + 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 176aba8..7e50d22 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -224,16 +224,17 @@ Include these in your workflow's `jobs..permissions` section. secret_scanning_enabled: 'true' # Optional: exclude paths (directory names, file names, or globs) trufflehog_exclude_dir: 'node_modules,vendor,dist,**/appsettings.*.json' - # Optional: report unverified and unknown secrets too (default: verified only) + # 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 secrets are reported, and those are critical and blocking. TruffleHog confirms -> each candidate against third-party validation endpoints; when a runner cannot reach them -> the result is `unknown` and is dropped in the default verified-only mode. On air-gapped or -> proxied runners, set `trufflehog_show_unverified: 'true'` so `unknown` results are still -> reported as low-severity findings instead of silently disappearing. +> 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 @@ -857,7 +858,7 @@ See [`action.yml`](../action.yml) for the complete list of inputs. **Security Scanning:** - `secret_scanning_enabled` — Enable secret scanning - `trufflehog_exclude_dir` — Comma-separated paths to exclude (directory names, file names, or globs) -- `trufflehog_show_unverified` — Include unverified and unknown secrets alongside verified ones +- `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 951da93..c5a3e7a 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -394,20 +394,20 @@ socket-basics --secrets --trufflehog-notify slack ``` ### `--show-unverified` -Include unverified and unknown secrets in TruffleHog results. TruffleHog always performs -verification; this flag only widens which result types are reported. By default only -verified secrets are returned (`--results=verified`); with this flag, verified, unverified, +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 findings are reported as -low and do not block. +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`, which is *not* returned in the -> default verified-only mode — an air-gapped scan will report zero findings rather than -> failing. Set `--show-unverified` on egress-restricted runners so `unknown` results are -> still reported. +> 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 diff --git a/socket_basics/connectors.yaml b/socket_basics/connectors.yaml index ba0e93e..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: "Include unverified and unknown secrets in TruffleHog results; verification always runs, and by default only verified secrets are reported" + 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 572d2d0..715668e 100644 --- a/socket_basics/core/connector/trufflehog/__init__.py +++ b/socket_basics/core/connector/trufflehog/__init__.py @@ -37,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: @@ -262,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 @@ -278,12 +280,14 @@ def scan(self) -> Dict[str, Any]: # 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 asked for verified-only. + # 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' + 'verified,unverified,unknown' + if show_unverified + else 'verified,unknown' ) cmd = [ @@ -291,6 +295,7 @@ def scan(self) -> Dict[str, Any]: 'filesystem', '--json', '--include-detectors=all', + '--fail-on-scan-errors', f'--results={results_filter}', ] @@ -337,16 +342,17 @@ def scan(self) -> Dict[str, Any]: if result.returncode != 0: # Fail closed. Returning {} here reports "no secrets found" and - # exits green, so a malformed exclude pattern or a broken - # install silently zeroes out every secret finding for the run. - # A scanner that cannot scan must not look like a clean scan. + # 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} and scanned nothing" + 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 " @@ -514,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) @@ -532,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** @@ -574,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 54a1717..f2a5603 100644 --- a/tests/test_changed_files_scope.py +++ b/tests/test_changed_files_scope.py @@ -888,7 +888,8 @@ def record_run(cmd, *args, **kwargs): "filesystem", "--json", "--include-detectors=all", - "--results=verified", + "--fail-on-scan-errors", + "--results=verified,unknown", str(pr_repo), ] ] diff --git a/tests/test_trufflehog_verification.py b/tests/test_trufflehog_verification.py index e749f21..6753b0f 100644 --- a/tests/test_trufflehog_verification.py +++ b/tests/test_trufflehog_verification.py @@ -3,7 +3,8 @@ ``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. +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 @@ -14,10 +15,16 @@ from socket_basics.core.connector.trufflehog import TruffleHogScanner -def _scanner(tmp_path, show_unverified): +def _scanner( + tmp_path, + show_unverified, + *, + scan_all=True, + secret_scanning_enabled=True, +): values = { - "secret_scanning_enabled": True, - "scan_all": True, + "secret_scanning_enabled": secret_scanning_enabled, + "scan_all": scan_all, "trufflehog_exclude_dir": "", "trufflehog_show_unverified": show_unverified, } @@ -30,7 +37,6 @@ def _scanner(tmp_path, show_unverified): config.get_scan_targets = lambda: [str(tmp_path)] scanner = TruffleHogScanner.__new__(TruffleHogScanner) scanner.config = config - scanner.is_enabled = lambda: True return scanner @@ -50,10 +56,10 @@ def record_run(cmd, *args, **kwargs): return invocations[0] -def test_show_unverified_off_requests_verified_results_only(tmp_path, monkeypatch): +def test_show_unverified_off_requests_high_confidence_results(tmp_path, monkeypatch): cmd = _captured_cmd(tmp_path, monkeypatch, show_unverified=False) - assert "--results=verified" in cmd + assert "--results=verified,unknown" in cmd assert "--no-verification" not in cmd @@ -64,7 +70,13 @@ def test_show_unverified_on_requests_every_result_type(tmp_path, monkeypatch): assert "--no-verification" not in cmd -def test_string_false_from_a_dashboard_config_stays_verified_only( +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. @@ -75,7 +87,7 @@ def test_string_false_from_a_dashboard_config_stays_verified_only( """ for raw in ("false", "False", "0", "no"): cmd = _captured_cmd(tmp_path, monkeypatch, show_unverified=raw) - assert "--results=verified" in cmd, raw + assert "--results=verified,unknown" in cmd, raw def test_string_true_from_a_dashboard_config_widens_result_types( @@ -86,11 +98,51 @@ def test_string_true_from_a_dashboard_config_widens_result_types( assert "--results=verified,unverified,unknown" in cmd, raw -def test_unset_setting_defaults_to_verified_only(tmp_path, monkeypatch): +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" in cmd, repr(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): @@ -103,8 +155,8 @@ def test_detector_selection_is_independent_of_the_setting(tmp_path, monkeypatch) assert detectors == [arg for arg in on if arg.startswith("--include-detectors")] -def _finding(verified): - return { +def _finding(verified, verification_error=None): + finding = { "DetectorName": "AWS", "Verified": verified, "Raw": "AKIAIOSFODNN7EXAMPLE", @@ -112,6 +164,9 @@ def _finding(verified): "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): @@ -120,6 +175,7 @@ def test_verified_findings_are_critical_and_blocking(tmp_path): assert alert["severity"] == "critical" assert alert["action"] == "error" assert alert["props"]["verified"] is True + assert alert["props"]["verificationStatus"] == "verified" assert alert["props"]["riskLevel"] == "critical" @@ -129,9 +185,22 @@ def test_unverified_findings_are_low_and_nonblocking(tmp_path): 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.