From 6d7fd4a91df8ed4ccec1ff09223dd23be09240ee Mon Sep 17 00:00:00 2001 From: Advait Patel Date: Sun, 20 Sep 2026 13:20:51 -0500 Subject: [PATCH 1/4] fix: defects found by end-to-end testing of 2026.9.20 Six defects, each found by running the released artifact rather than reading it. The first shipped because nothing in CI or the test suite exercised entrypoint.sh with command-line arguments. - The container image ignored every command-line argument. entrypoint.sh built its args only from the Action's INPUT_* variables and never forwarded "$@", so `docker run ghcr.io/owasp/docksec:latest --version` and --help both failed with "Dockerfile path is required". Both forms now work and can be combined. - Fix commands ignored the EPSS priority just computed above them. The output printed a Fix Now / Fix Soon breakdown and then sorted commands by severity and package name, burying the exploited CVEs among hundreds of lower-priority entries. Ordering now leads with the priority tier, each row is labelled, and the CVE driving a package's tier is kept in the displayed IDs. - SARIF set security-severity to an empty string whenever a finding had no CVSS score - 14 of 25 rules on an nginx scan. GitHub Code Scanning ranks by that property, so those findings did not surface at the severity DockSec assigned. It now falls back to the severity band. - SARIF carried no EPSS data at all. Results now include epss, epssPercentile and priority. - Compose scans never pulled images, so on a clean machine both services of the insecure example reported as unscanned. Missing images are now pulled on demand, never under --offline, and switchable off with DOCKSEC_PULL_MISSING_IMAGES=false. - Coverage notes now state the Trivy and Hadolint versions used, so the image's pinned versions differing from a local install is visible rather than looking like a bug. test_stage0_correctness's missing-image test relied on the runner not having nginx and postgres cached, so it passed on CI and failed on a developer machine. It now names images that cannot resolve. --- .gitignore | 5 +- CHANGELOG.md | 38 ++++++ README.md | 10 ++ docksec/completeness.py | 52 +++++++++ docksec/docker_scanner.py | 54 ++++++++- docksec/output.py | 20 +++- docksec/remediation.py | 27 ++++- docksec/report_generator.py | 43 ++++++- entrypoint.sh | 13 ++- tests/test_e2e_regressions.py | 193 +++++++++++++++++++++++++++++++ tests/test_stage0_correctness.py | 37 ++++-- 11 files changed, 470 insertions(+), 22 deletions(-) create mode 100644 tests/test_e2e_regressions.py diff --git a/.gitignore b/.gitignore index bf7cd96..83b5e6e 100644 --- a/.gitignore +++ b/.gitignore @@ -201,10 +201,13 @@ CURSOR_CONFIGURATION.md docs/promptfoo-benchmark-review.md docs/docksec-strategy-2026.md docs/cve-lite-cli-comparison.md +docs/session-handoff.md ROADMAP.md # Claude Code project instructions (local only, not for public repository) CLAUDE.md # macOS -.DS_Store \ No newline at end of file +.DS_Store +docs/e2e-findings-2026-09-20.md + diff --git a/CHANGELOG.md b/CHANGELOG.md index 7431f51..ce817b8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,44 @@ All notable changes to DockSec are documented in this file. ### Fixed +- **The container image ignored every command-line argument.** `entrypoint.sh` + built its argument list only from the Action's `INPUT_*` variables and never + forwarded `"$@"`, so `docker run ghcr.io/owasp/docksec:latest --version` and + `--help` - the first two commands anyone runs against an unfamiliar image - + both failed with "Dockerfile path is required". The Action path was + unaffected. Command-line arguments are now appended after the `INPUT_*` + handling, so both forms work and can be combined. + +- **Fix commands ignored the EPSS priority DockSec had just computed.** The + output printed a `Fix Now` / `Fix Soon` breakdown and then ordered the + commands by severity and package name, so on a Postgres image the three + genuinely-exploited OpenSSL CVEs sat alphabetically among 160+ lower-priority + entries with nothing marking them. Commands are now ordered by priority tier + first, each row is labelled with its tier, and where several CVEs share a + package the one driving the tier is kept in the displayed IDs. + +- **SARIF reported no severity for findings without a CVSS score.** The + `security-severity` property was set to `str(CVSS or "")`, which is an empty + string for every Hadolint and `trivy config` rule and for many CVEs - 14 of 25 + rules on an nginx scan. GitHub Code Scanning ranks by that property, so those + findings did not surface at the severity DockSec assigned. It now falls back + to the severity band when no CVSS score is available. + +- **SARIF carried no EPSS data.** Code Scanning users, the largest integration + surface, saw none of the exploitation signal driving DockSec's own ordering. + Results now carry `epss`, `epssPercentile` and `priority` properties. + +- **Compose scans never pulled images.** A service whose image was not already + local failed with "not found locally", so on a clean machine or a fresh CI + runner the insecure compose example reported both services unscanned and + scored from static rules alone. Missing images are now pulled on demand. This + never happens under `--offline`, and can be disabled with + `DOCKSEC_PULL_MISSING_IMAGES=false`. + +- Coverage notes now state the Trivy and Hadolint versions used. The published + image pins versions a local install does not, so the same Dockerfile could + yield 9 findings locally and 11 in CI with nothing explaining the difference. + - DockSec's own Dockerfile was missing `--no-install-recommends`, which the new self-scan caught. The root-user finding is waived in a committed `.docksec-ignore.yml` with the reason recorded: the image backs the GitHub diff --git a/README.md b/README.md index 4c4c5d1..49e35d0 100644 --- a/README.md +++ b/README.md @@ -615,6 +615,16 @@ Image scan results are cached (default: 24 hours, override with such as a reused `:latest` always gets a fresh scan. Use `--no-cache` (or `DOCKSEC_USE_CACHE=false`) to bypass the cache for a run. +### Pulling images that are not local + +Scanning an image that is not present locally pulls it first. A compose stack +routinely names images the machine has never pulled, and without this every one +of those services is reported as unscanned. + +Set `DOCKSEC_PULL_MISSING_IMAGES=false` to turn this off and fail instead, which +is worth doing on a metered connection or a shared runner. `--offline` never +pulls, regardless of this setting. + --- ## AI-assistant skills (`install-skill`) diff --git a/docksec/completeness.py b/docksec/completeness.py index 5f31d8d..4f27f34 100644 --- a/docksec/completeness.py +++ b/docksec/completeness.py @@ -17,11 +17,48 @@ scope from silence. """ +import re +import subprocess from typing import Dict, List, Optional DETECTION = "detection" REMEDIATION = "remediation" +# Cached for the process: these shell out, and the versions cannot change +# mid-run. +_TOOL_VERSIONS: Optional[Dict[str, str]] = None + + +def _tool_version(tool: str) -> Optional[str]: + """Return a scanner's version string, or None if it cannot be determined.""" + try: + proc = subprocess.run( + [tool, "--version"], + capture_output=True, + text=True, + timeout=10, + shell=False, + ) + except (OSError, subprocess.SubprocessError): + return None + if proc.returncode != 0: + return None + match = re.search(r"(\d+\.\d+(?:\.\d+)?)", proc.stdout or "") + return match.group(1) if match else None + + +def tool_versions() -> Dict[str, str]: + """The scanner versions this run used, for the coverage block.""" + global _TOOL_VERSIONS + if _TOOL_VERSIONS is None: + found = {} + for tool in ("trivy", "hadolint"): + version = _tool_version(tool) + if version: + found[tool] = version + _TOOL_VERSIONS = found + return _TOOL_VERSIONS + class ScanCompleteness: """Accumulates gaps encountered during a scan.""" @@ -100,6 +137,21 @@ def coverage_notes(results: Dict, ai_ran: bool = False) -> List[str]: ) notes.append("Secrets committed elsewhere in the build context are not scanned.") + + # Results depend on the scanner versions, and the published image pins + # versions a local install does not. Stating them turns "CI found one more + # finding than my laptop" from a suspected bug into a visible difference. + # An explicit (even empty) mapping in results wins; detection is only the + # fallback for callers that do not record versions themselves. + versions = results.get("tool_versions") + if versions is None: + versions = tool_versions() + reported = [f"{name} {version}" for name, version in sorted(versions.items()) if version] + if reported: + notes.append( + f"Findings depend on scanner versions: {', '.join(reported)}. " + f"A different version may report a different set." + ) return notes diff --git a/docksec/docker_scanner.py b/docksec/docker_scanner.py index 5257f07..217a1cc 100644 --- a/docksec/docker_scanner.py +++ b/docksec/docker_scanner.py @@ -317,6 +317,14 @@ def __init__(self, dockerfile_path: Optional[str], image_name: Optional[str], re self.cache = ScanResultsCache(self.RESULTS_DIR) self.use_cache = os.getenv("DOCKSEC_USE_CACHE", "true").lower() == "true" + # A compose stack routinely names images the user has never pulled, and + # refusing to fetch them reports every service as unscanned. Pulling is + # a network operation that can move gigabytes, so it stays switchable + # and is never attempted under --offline. + self.pull_missing_images = ( + os.getenv("DOCKSEC_PULL_MISSING_IMAGES", "true").lower() == "true" + ) + # Verify required tools missing_tools = self._check_tools() if missing_tools: @@ -353,8 +361,26 @@ def __init__(self, dockerfile_path: Optional[str], image_name: Optional[str], re f" 3. If you must use sudo, run DockSec with sudo (not recommended for security reasons)\n" f"Original error: {e.stderr.strip() if e.stderr else str(e)}" ) - # If it's not a permission error, assume the image doesn't exist - raise ValueError(f"Docker image '{self.image_name}' not found locally") + # Not a permission problem, so the image is simply not present + # locally. A compose stack routinely names images the user has + # never pulled, and refusing there reports every service as + # unscanned on an otherwise healthy machine. Pull it once, then + # let a second failure stand. + if self.offline or not self.pull_missing_images: + reason = ( + "--offline forbids pulling it" + if self.offline + else "DOCKSEC_PULL_MISSING_IMAGES=false forbids pulling it" + ) + raise ValueError( + f"Docker image '{self.image_name}' not found locally and " + f"{reason}. Pull it before scanning." + ) + if not self._pull_image(): + raise ValueError( + f"Docker image '{self.image_name}' not found locally and could " + f"not be pulled. Check the name and your registry access." + ) except FileNotFoundError: raise ValueError( "Docker command not found. Please ensure Docker is installed and accessible in your PATH." @@ -465,6 +491,30 @@ def run_image_only_scan(self, severity: str = "CRITICAL,HIGH") -> Dict: return results + def _pull_image(self) -> bool: + """Fetch an image that is not present locally. + + Returns True when the image is available afterwards. Network failures, + bad names and private registries all surface the same way - as a False + the caller turns into a clear error - because the distinction does not + change what the user has to do. + """ + logger.info(f"Image {self.image_name} not present locally; pulling it") + try: + subprocess.run( + ['docker', 'pull', self.image_name], + capture_output=True, + check=True, + text=True, + timeout=600, + shell=False, + ) + return True + except (subprocess.CalledProcessError, subprocess.TimeoutExpired, FileNotFoundError) as e: + stderr = getattr(e, 'stderr', '') or '' + logger.error(f"Failed to pull {self.image_name}: {stderr.strip() or e}") + return False + def _check_tools(self) -> List[str]: """Check if all required tools are installed and return list of missing tools.""" missing_tools = [] diff --git a/docksec/output.py b/docksec/output.py index 5db2674..bd949b9 100644 --- a/docksec/output.py +++ b/docksec/output.py @@ -260,19 +260,27 @@ def fix_plan(plan) -> None: console = get_console() if plan.package_upgrades: + from docksec.epss import PRIORITY_LABELS + console.print() _line(Text("Fix commands", style="bold cyan")) for item in plan.package_upgrades: ids = ", ".join(item["ids"]) more = "" if item["finding_count"] <= len(item["ids"]) else f" +{item['finding_count'] - len(item['ids'])}" _line(Text.assemble((" > ", "dim"), (item["command"], "green"))) - _line( - Text( - f" {item['severity']} - {item['installed']} -> {item['fixed']}" - f" ({ids}{more})", - style="dim", - ) + # The commands are ordered by priority, which is invisible in a long + # list unless each row says which tier put it there. + priority = item.get("priority") + label = PRIORITY_LABELS.get(priority) if priority else None + detail = Text(" ", style="dim") + if label: + detail.append(f"{label} ", style=_PRIORITY_STYLES.get(priority, "default")) + detail.append( + f"{item['severity']} - {item['installed']} -> {item['fixed']}" + f" ({ids}{more})", + style="dim", ) + _line(detail) if plan.dockerfile_edits: console.print() diff --git a/docksec/remediation.py b/docksec/remediation.py index db2ad3b..05103ec 100644 --- a/docksec/remediation.py +++ b/docksec/remediation.py @@ -16,6 +16,7 @@ from typing import Dict, List, Optional from docksec.enums import Severity +from docksec.epss import priority_rank # Dockerfile rules with a deterministic edit. Maps a rule ID to a short # instruction and, where the change is mechanical enough to apply @@ -212,13 +213,25 @@ def build_plan(findings: List[Dict], dockerfile_path: Optional[str] = None) -> F "fixed": fixed, "manager": _package_manager_hint(finding), "severity": finding.get("Severity"), + "priority": finding.get("Priority"), "finding_count": 0, "ids": [], }) entry["finding_count"] += 1 vuln_id = finding.get("VulnerabilityID") - if vuln_id and len(entry["ids"]) < 3: - entry["ids"].append(vuln_id) + finding_priority = finding.get("Priority") + more_urgent = priority_rank(finding_priority) > priority_rank(entry["priority"]) + if vuln_id and vuln_id not in entry["ids"]: + if len(entry["ids"]) < 3: + entry["ids"].append(vuln_id) + elif more_urgent: + # The displayed IDs are capped, so make sure the one driving + # this entry's priority is not the one left out. + entry["ids"][-1] = vuln_id + # One upgrade covers every finding against the package, so the entry + # carries the most urgent tier any of them reached. + if more_urgent: + entry["priority"] = finding_priority if Severity.rank(finding.get("Severity")) > Severity.rank(entry["severity"]): entry["severity"] = finding.get("Severity") # Upgrading once must satisfy every finding against this package. @@ -233,8 +246,16 @@ def build_plan(findings: List[Dict], dockerfile_path: Optional[str] = None) -> F entry["package"], entry["fixed"], entry["manager"] ) plan.package_upgrades.append(entry) + # EPSS priority leads the ordering: the whole point of computing a tier is + # that a HIGH with real exploitation beats a CRITICAL nobody is exploiting. + # Severity breaks ties within a tier, and the package name keeps the output + # stable between runs. plan.package_upgrades.sort( - key=lambda e: (-Severity.rank(e["severity"]), e["package"]) + key=lambda e: ( + -priority_rank(e.get("priority")), + -Severity.rank(e["severity"]), + e["package"], + ) ) # Dockerfile and compose edits, deduplicated by rule. diff --git a/docksec/report_generator.py b/docksec/report_generator.py index b249950..77ecea5 100644 --- a/docksec/report_generator.py +++ b/docksec/report_generator.py @@ -281,6 +281,27 @@ def _sarif_level(severity) -> str: } return mapping.get(str(severity or "UNKNOWN").upper(), "note") + # GitHub reads `security-severity` to decide how to surface a finding. When + # a finding has no CVSS score - true of every Hadolint and `trivy config` + # rule, and of many CVEs - an empty string means GitHub falls back to its + # own default instead of the severity DockSec assigned. These are the + # midpoints of the CVSS v3 qualitative bands, used only as a fallback. + _SEVERITY_SCORES = { + "CRITICAL": "9.0", + "HIGH": "7.0", + "MEDIUM": "5.0", + "LOW": "3.0", + } + + @staticmethod + def _sarif_security_severity(vuln: Dict) -> str: + """The numeric score GitHub ranks by, falling back to severity.""" + cvss = str(vuln.get("CVSS") or "").strip() + if cvss: + return cvss + severity = str(vuln.get("Severity") or "").upper() + return ReportGenerator._SEVERITY_SCORES.get(severity, "") + @staticmethod def _sarif_rule(rule_id: str, vuln: Dict) -> Dict: """Build the SARIF rule (reportingDescriptor) for one finding type.""" @@ -290,7 +311,7 @@ def _sarif_rule(rule_id: str, vuln: Dict) -> Dict: "shortDescription": {"text": vuln.get("Title") or rule_id}, "fullDescription": {"text": vuln.get("Description") or vuln.get("Title") or rule_id}, "defaultConfiguration": {"level": ReportGenerator._sarif_level(vuln.get("Severity"))}, - "properties": {"security-severity": str(vuln.get("CVSS") or "")}, + "properties": {"security-severity": ReportGenerator._sarif_security_severity(vuln)}, } primary_url = vuln.get("PrimaryURL") if primary_url: @@ -315,13 +336,31 @@ def _sarif_result(rule_id: str, vuln: Dict, artifact_uri: str) -> Dict: if region: location["physicalLocation"]["region"] = region - return { + result = { "ruleId": rule_id, "level": ReportGenerator._sarif_level(vuln.get("Severity")), "message": {"text": message}, "locations": [location], } + # Carry the EPSS signal into SARIF. Without it, Code Scanning users - + # the largest integration surface - lose the exploitation data that + # drives DockSec's own ordering. + properties = {} + epss = vuln.get("EPSS") + if epss is not None: + properties["epss"] = epss + percentile = vuln.get("EPSSPercentile") + if percentile is not None: + properties["epssPercentile"] = percentile + priority = vuln.get("Priority") + if priority: + properties["priority"] = priority + if properties: + result["properties"] = properties + + return result + @staticmethod def _sarif_region(target, line=None) -> Optional[Dict]: """Build a SARIF region for a finding, when its position is known. diff --git a/entrypoint.sh b/entrypoint.sh index a0b190b..73afa99 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -68,5 +68,16 @@ if [ "${INPUT_IMAGE_ONLY}" = "true" ]; then ARGS+=("--image-only") fi -printf 'Running: docksec'; printf ' %q' "${ARGS[@]}"; printf '\n' +# Forward anything given on the command line. Without this the image only +# honours the INPUT_* variables, so `docker run ... --version` and every other +# direct CLI invocation fails with "Dockerfile path is required". +ARGS+=("$@") + +# printf runs its format once even for an empty array, so guard the log line: +# otherwise a bare `docksec` is reported as `docksec ''`. +if [ ${#ARGS[@]} -eq 0 ]; then + printf 'Running: docksec\n' +else + printf 'Running: docksec'; printf ' %q' "${ARGS[@]}"; printf '\n' +fi docksec "${ARGS[@]}" diff --git a/tests/test_e2e_regressions.py b/tests/test_e2e_regressions.py new file mode 100644 index 0000000..8d2d729 --- /dev/null +++ b/tests/test_e2e_regressions.py @@ -0,0 +1,193 @@ +"""Regressions for defects found by end-to-end testing of 2026.9.20. + +Each test here corresponds to something that shipped working-by-inspection and +was only caught by running the released artifact. The entrypoint cases matter +most: nothing in the suite referenced `entrypoint.sh` at all, and the CI smoke +test reaches DockSec either through `--entrypoint sh` or the INPUT_* variables, +so a shell script that dropped every command-line argument passed green. +""" + +import json +import os +import shutil +import subprocess +import unittest + +from docksec.completeness import coverage_notes +from docksec.remediation import build_plan +from docksec.report_generator import ReportGenerator + +ENTRYPOINT = os.path.join(os.path.dirname(os.path.dirname(__file__)), "entrypoint.sh") + + +def _vuln(pkg, fixed, severity="HIGH", vuln_id="CVE-0000-0001", priority=None, **extra): + finding = { + "VulnerabilityID": vuln_id, "PkgName": pkg, "InstalledVersion": "1.0", + "FixedVersion": fixed, "Severity": severity, "Target": "debian", + } + if priority: + finding["Priority"] = priority + finding.update(extra) + return finding + + +@unittest.skipUnless(shutil.which("bash"), "bash is required") +class TestEntrypointForwardsArguments(unittest.TestCase): + """The image must behave like the CLI it wraps. + + `docker run --version` and `--help` are the first two commands + anyone runs against an unfamiliar image; both failed with "Dockerfile path + is required" because only INPUT_* was honoured. + """ + + def _run(self, args, env=None): + environ = dict(os.environ) + environ.pop("INPUT_DOCKERFILE", None) + # Stand in for the real binary so the test asserts on argument passing + # rather than on a scan. + environ["PATH"] = self.bindir + os.pathsep + environ["PATH"] + environ.update(env or {}) + return subprocess.run( + ["bash", ENTRYPOINT, *args], + capture_output=True, text=True, timeout=60, env=environ, + ) + + def setUp(self): + import tempfile + self.tmp = tempfile.mkdtemp() + self.bindir = self.tmp + stub = os.path.join(self.bindir, "docksec") + with open(stub, "w") as fh: + fh.write('#!/bin/sh\nprintf "ARGS:"\nfor a in "$@"; do printf " %s" "$a"; done\nprintf "\\n"\n') + os.chmod(stub, 0o755) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def test_version_flag_reaches_the_cli(self): + result = self._run(["--version"]) + self.assertIn("ARGS: --version", result.stdout) + + def test_help_flag_reaches_the_cli(self): + result = self._run(["--help"]) + self.assertIn("ARGS: --help", result.stdout) + + def test_positional_and_flags_are_forwarded_in_order(self): + result = self._run(["Dockerfile", "--scan-only"]) + self.assertIn("ARGS: Dockerfile --scan-only", result.stdout) + + def test_arguments_with_spaces_survive(self): + result = self._run(["my Dockerfile"]) + self.assertIn("ARGS: my Dockerfile", result.stdout) + + def test_action_inputs_still_work(self): + """The GitHub Action path must not regress.""" + result = self._run([], env={"INPUT_DOCKERFILE": "Dockerfile", "INPUT_SCAN_ONLY": "true"}) + self.assertIn("ARGS: Dockerfile --scan-only", result.stdout) + + def test_action_inputs_and_cli_arguments_combine(self): + result = self._run(["--no-color"], env={"INPUT_DOCKERFILE": "Dockerfile"}) + self.assertIn("ARGS: Dockerfile --no-color", result.stdout) + + def test_no_arguments_logs_a_bare_command(self): + """`printf '%q'` emits its format once even for an empty array, which + reported a bare run as `docksec ''`.""" + result = self._run([]) + self.assertIn("Running: docksec", result.stdout) + self.assertNotIn("docksec ''", result.stdout) + + +class TestFixCommandsFollowPriority(unittest.TestCase): + """The tool printed `Fix Now 3` and then ordered the commands by severity + and package name, so the exploited finding sat unmarked among hundreds.""" + + def test_fix_now_sorts_above_a_higher_severity_without_epss(self): + plan = build_plan([ + _vuln("acritical", "2.0", severity="CRITICAL", vuln_id="CVE-1", priority="fix_soon"), + _vuln("zhigh", "2.0", severity="HIGH", vuln_id="CVE-2", priority="fix_now"), + ]) + self.assertEqual([e["package"] for e in plan.package_upgrades], ["zhigh", "acritical"]) + + def test_severity_still_breaks_ties_inside_a_tier(self): + plan = build_plan([ + _vuln("alow", "2.0", severity="MEDIUM", vuln_id="CVE-1", priority="fix_soon"), + _vuln("zhigh", "2.0", severity="CRITICAL", vuln_id="CVE-2", priority="fix_soon"), + ]) + self.assertEqual([e["package"] for e in plan.package_upgrades], ["zhigh", "alow"]) + + def test_entry_carries_the_most_urgent_tier_of_its_findings(self): + plan = build_plan([ + _vuln("openssl", "2.0", vuln_id="CVE-1", priority="fix_soon"), + _vuln("openssl", "2.0", vuln_id="CVE-2", priority="fix_now"), + ]) + self.assertEqual(plan.package_upgrades[0]["priority"], "fix_now") + + def test_the_cve_driving_the_priority_is_not_dropped_from_display(self): + """Displayed IDs cap at three; the urgent one must not be the casualty.""" + findings = [ + _vuln("openssl", "2.0", vuln_id=f"CVE-000{i}", priority="fix_soon") + for i in range(3) + ] + findings.append(_vuln("openssl", "2.0", vuln_id="CVE-URGENT", priority="fix_now")) + plan = build_plan(findings) + self.assertIn("CVE-URGENT", plan.package_upgrades[0]["ids"]) + + def test_unscored_findings_sort_last_but_are_kept(self): + plan = build_plan([ + _vuln("unscored", "2.0", severity="CRITICAL", vuln_id="CVE-1"), + _vuln("scored", "2.0", severity="LOW", vuln_id="CVE-2", priority="fix_now"), + ]) + self.assertEqual([e["package"] for e in plan.package_upgrades], ["scored", "unscored"]) + + +class TestSarifSeverityAndEpss(unittest.TestCase): + """GitHub ranks by `security-severity`; an empty string means it ignores + the severity DockSec assigned. 14 of 25 rules emitted one.""" + + def test_missing_cvss_falls_back_to_severity(self): + rule = ReportGenerator._sarif_rule("DL3008", {"Severity": "HIGH"}) + self.assertEqual(rule["properties"]["security-severity"], "7.0") + + def test_real_cvss_is_preferred_over_the_fallback(self): + rule = ReportGenerator._sarif_rule("CVE-1", {"Severity": "HIGH", "CVSS": "9.8"}) + self.assertEqual(rule["properties"]["security-severity"], "9.8") + + def test_every_severity_maps_to_a_number(self): + for severity in ("CRITICAL", "HIGH", "MEDIUM", "LOW"): + rule = ReportGenerator._sarif_rule("R", {"Severity": severity}) + self.assertNotEqual(rule["properties"]["security-severity"], "") + + def test_unknown_severity_stays_empty_rather_than_inventing_a_score(self): + rule = ReportGenerator._sarif_rule("R", {"Severity": "UNKNOWN"}) + self.assertEqual(rule["properties"]["security-severity"], "") + + def test_epss_reaches_sarif_results(self): + result = ReportGenerator._sarif_result( + "CVE-1", + {"Severity": "HIGH", "EPSS": 0.48, "EPSSPercentile": 0.988, "Priority": "fix_now"}, + "Dockerfile", + ) + self.assertEqual(result["properties"]["epss"], 0.48) + self.assertEqual(result["properties"]["epssPercentile"], 0.988) + self.assertEqual(result["properties"]["priority"], "fix_now") + + def test_findings_without_epss_carry_no_properties_bag(self): + result = ReportGenerator._sarif_result("CVE-1", {"Severity": "HIGH"}, "Dockerfile") + self.assertNotIn("properties", result) + + +class TestCoverageReportsScannerVersions(unittest.TestCase): + """A local run and a CI run differ when their scanners differ; the output + should say so rather than leave it looking like a bug.""" + + def test_versions_are_reported_when_known(self): + notes = coverage_notes({"tool_versions": {"trivy": "0.74.0", "hadolint": "2.15.1"}}) + self.assertTrue(any("trivy 0.74.0" in n and "hadolint 2.15.1" in n for n in notes)) + + def test_no_version_note_when_nothing_is_detected(self): + notes = coverage_notes({"tool_versions": {}}) + self.assertFalse(any("scanner versions" in n for n in notes)) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_stage0_correctness.py b/tests/test_stage0_correctness.py index 433a535..42f6c50 100644 --- a/tests/test_stage0_correctness.py +++ b/tests/test_stage0_correctness.py @@ -40,17 +40,40 @@ def test_no_failures_reports_empty(self): def test_orchestrator_records_failures_for_missing_images(self): """The orchestrator must populate failed_services; the CLI's exit code - depends on it.""" + depends on it. + + Pulling is disabled here so the test stays hermetic and offline: the + scanner now fetches missing images by default, which would otherwise + turn this into a multi-gigabyte network test. + + The compose file names images that cannot resolve, rather than relying + on the runner happening not to have nginx and postgres cached - which + is what made this test pass on CI and fail on a developer machine. + """ + import os + import tempfile + from unittest import mock + from docksec.compose_scanner import ComposeOrchestrator - orchestrator = ComposeOrchestrator( - "examples/compose/docker-compose-insecure.yml", - scan_only=True, - skip_ai_scoring=True, + compose = ( + "services:\n" + " web:\n" + " image: docksec.invalid/definitely-not-a-real-image:notatag\n" + " db:\n" + " image: docksec.invalid/also-not-real:notatag\n" ) - results = orchestrator.run_full_scan("CRITICAL,HIGH") + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, "docker-compose.yml") + with open(path, "w") as fh: + fh.write(compose) + + with mock.patch.dict(os.environ, {"DOCKSEC_PULL_MISSING_IMAGES": "false"}): + orchestrator = ComposeOrchestrator( + path, scan_only=True, skip_ai_scoring=True, + ) + results = orchestrator.run_full_scan("CRITICAL,HIGH") - # Images are not pulled in the test environment, so both services fail. self.assertTrue( results.get("failed_services"), "failed services must be recorded so the CLI can exit non-zero", From 27c6375d84102212883742c4330f7175a7a53181 Mon Sep 17 00:00:00 2001 From: Advait Patel Date: Sun, 20 Sep 2026 13:27:02 -0500 Subject: [PATCH 2/4] fix: remove unused json import in the regression tests --- tests/test_e2e_regressions.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_e2e_regressions.py b/tests/test_e2e_regressions.py index 8d2d729..81a911e 100644 --- a/tests/test_e2e_regressions.py +++ b/tests/test_e2e_regressions.py @@ -7,7 +7,6 @@ so a shell script that dropped every command-line argument passed green. """ -import json import os import shutil import subprocess From 60f8da27592faba6094eceb8f17a5338dd847513 Mon Sep 17 00:00:00 2001 From: Advait Patel Date: Sun, 20 Sep 2026 13:30:46 -0500 Subject: [PATCH 3/4] ci: gate the hardened example on config rules, assert CLI args reach the image Both of these were exposed by compose scans now pulling images that are not present locally. The hardened example's services previously failed to scan, so the assertion over them was running against an empty result and had never tested what it claimed. Gate the hardened example on configuration findings rather than on every CRITICAL/HIGH finding. "Hardened" is a statement about configuration, and zero compose rules fire on that example - the 35 findings are real CVEs in the pinned nginx:1.25.3-alpine and postgres:15.5-alpine base images. Those are true positives, and failing on them would break this job every time upstream publishes an advisory against a deliberately held pin. The CVE count is printed so it stays visible. The score check goes with it: the score is driven by those same CVEs. The guard still discriminates - the hardened example reports 0 config findings and the insecure one reports 6, so a rule that starts misfiring on hardened input still fails CI. Separately, assert that command-line arguments reach the CLI through the image entrypoint. Every other step in the smoke workflow reaches DockSec via `--entrypoint sh` or INPUT_*, which is why an entrypoint that dropped "$@" shipped green. Verified locally against both entrypoints: the assertion passes with the fix and fails without it. --- .github/workflows/docker-image-smoke.yml | 36 ++++++++++++++++++++++++ .github/workflows/self-scan.yml | 34 ++++++++++++++-------- 2 files changed, 59 insertions(+), 11 deletions(-) diff --git a/.github/workflows/docker-image-smoke.yml b/.github/workflows/docker-image-smoke.yml index 62c209e..2dd093d 100644 --- a/.github/workflows/docker-image-smoke.yml +++ b/.github/workflows/docker-image-smoke.yml @@ -94,6 +94,42 @@ jobs: CMD ["node","server.js"] EOF + # The image must also behave like the CLI it wraps. `--version` and + # `--help` are the first two commands anyone runs against an unfamiliar + # image, and both failed in 2026.9.20: entrypoint.sh built its arguments + # only from INPUT_* and never forwarded "$@". Every other step in this + # file reaches DockSec through `--entrypoint sh` or INPUT_*, so nothing + # here caught it. + - name: Assert command-line arguments reach the CLI + run: | + set -euo pipefail + + # The entrypoint echoes the command it is about to run, so match + # anywhere in the output rather than anchoring at the start. + version_output="$(docker run --rm docksec:smoke --version)" + echo "${version_output}" + case "${version_output}" in + *DockSec*) ;; + *) echo "::error::--version did not reach the CLI"; exit 1 ;; + esac + + help_output="$(docker run --rm docksec:smoke --help)" + case "${help_output}" in + *"usage: docksec"*) ;; + *) echo "::error::--help did not reach the CLI"; exit 1 ;; + esac + + # A positional argument plus a flag: the shape a direct CLI user types. + docker run --rm \ + -v "${PWD}/smoke-workspace:/work" -w /work \ + docksec:smoke Dockerfile --scan-only > cli-form.txt 2>&1 || true + if ! grep -q "security findings" cli-form.txt; then + echo "::error::a positional Dockerfile argument did not reach the CLI" + cat cli-form.txt + exit 1 + fi + echo "command-line arguments reach the CLI" + # The core assertion: a scan runs to completion through the Action's own # entrypoint and produces findings. Uses --scan-only so no API key is # needed; AI analysis is not what this test covers. diff --git a/.github/workflows/self-scan.yml b/.github/workflows/self-scan.yml index 7d6426e..5719d9b 100644 --- a/.github/workflows/self-scan.yml +++ b/.github/workflows/self-scan.yml @@ -62,8 +62,13 @@ jobs: --sarif --output-dir ./self-scan-results # The bundled examples are part of the product: the insecure one must - # produce findings and the hardened one must not. A rule that starts - # firing on the hardened example is a false positive users will hit. + # produce findings and the hardened one must not trip a config rule. + # + # "Hardened" is a statement about configuration, so that is what this + # gates on. A CVE in a pinned upstream base image is a true positive, not + # a rule misfiring, and gating on those would fail this job every time + # upstream publishes an advisory against a pin the example deliberately + # holds. Image CVEs are printed so the count stays visible. - name: Verify the bundled examples still behave if: always() run: | @@ -74,15 +79,22 @@ jobs: python3 - <<'PY' import json, sys data = json.load(open("secure.json")) - counts = data["severity_counts"] - serious = counts.get("CRITICAL", 0) + counts.get("HIGH", 0) - if serious: - sys.exit(f"::error::the hardened example produced {serious} CRITICAL/HIGH " - f"finding(s); a false positive users will hit") - score = data["scan_info"]["analysis_score"] - if score is not None and score < 70: - sys.exit(f"::error::the hardened example scored {score}; it should score well") - print(f"hardened example: {serious} serious findings, score {score}") + findings = data.get("vulnerabilities", []) + serious = [ + f for f in findings + if str(f.get("Severity", "")).upper() in ("CRITICAL", "HIGH") + ] + config_findings = [ + f for f in serious + if not str(f.get("VulnerabilityID", "")).startswith("CVE-") + ] + if config_findings: + ids = ", ".join(sorted({str(f.get("VulnerabilityID")) for f in config_findings})) + sys.exit(f"::error::the hardened example tripped config rule(s): {ids}; " + f"a false positive users will hit") + cve_count = len(serious) - len(config_findings) + print(f"hardened example: 0 config findings, " + f"{cve_count} CRITICAL/HIGH CVE(s) in its pinned base images") PY docksec --compose examples/compose/docker-compose-insecure.yml --no-config \ From 2075ff2cea88286245e00da4e3888f4c5f02fca4 Mon Sep 17 00:00:00 2001 From: Advait Patel Date: Sun, 20 Sep 2026 13:34:11 -0500 Subject: [PATCH 4/4] test: restrict the entrypoint stub to owner-only permissions CodeQL flagged py/overly-permissive-file: the stub was created 0o755, world-readable and world-executable. Only the test process ever runs it, so 0o700 is sufficient. --- tests/test_e2e_regressions.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_e2e_regressions.py b/tests/test_e2e_regressions.py index 81a911e..79bd06e 100644 --- a/tests/test_e2e_regressions.py +++ b/tests/test_e2e_regressions.py @@ -58,7 +58,8 @@ def setUp(self): stub = os.path.join(self.bindir, "docksec") with open(stub, "w") as fh: fh.write('#!/bin/sh\nprintf "ARGS:"\nfor a in "$@"; do printf " %s" "$a"; done\nprintf "\\n"\n') - os.chmod(stub, 0o755) + # Owner-only: nothing but this test process ever executes the stub. + os.chmod(stub, 0o700) def tearDown(self): shutil.rmtree(self.tmp, ignore_errors=True)