diff --git a/.github/workflows/bakery-build-native.yml b/.github/workflows/bakery-build-native.yml index 3f3a190e..a516d9b1 100644 --- a/.github/workflows/bakery-build-native.yml +++ b/.github/workflows/bakery-build-native.yml @@ -227,6 +227,7 @@ jobs: contents: read packages: write id-token: write + security-events: write needs: matrix # GitHub Actions fails (not skips) a matrix job when the matrix evaluates to []. # Guard here so an empty change-aware matrix (a push with nothing to build) @@ -431,6 +432,14 @@ jobs: --metadata-file "./${IMAGE_NAME}-${IMAGE_VERSION}-${NORMALIZED_PLATFORM}-metadata.json" \ --context "$CONTEXT" + # No `category:` on purpose: each file carries its own in + # automationDetails.id, and one category across all of them is rejected. + - name: Upload Trivy SARIF + if: ${{ inputs.push && matrix.img.latest && !cancelled() && hashFiles('results/trivy/**/*.sarif') != '' }} + uses: github/codeql-action/upload-sarif@5595ccaf912efad79be6eef63a5619ff05969be3 # v4.37.6 + with: + sarif_file: results/trivy + - name: Upload Metadata uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: diff --git a/posit-bakery/posit_bakery/plugins/builtin/trivy/__init__.py b/posit-bakery/posit_bakery/plugins/builtin/trivy/__init__.py index 5b40bafc..49d985c9 100644 --- a/posit-bakery/posit_bakery/plugins/builtin/trivy/__init__.py +++ b/posit-bakery/posit_bakery/plugins/builtin/trivy/__init__.py @@ -213,6 +213,7 @@ def scan( results = plugin.execute( c.base_path, c.targets, + scan_platform=platform, severity=severity, fail_on_severity=fail_on_severity, disabled_scanners=disabled_scanners, @@ -228,6 +229,7 @@ def execute( base_path: Path, targets: list[ImageTarget], *, + scan_platform: str | None = None, severity: str | None = None, fail_on_severity: str | None = None, disabled_scanners: str | None = None, @@ -238,6 +240,7 @@ def execute( suite = TrivySuite( base_path, targets, + scan_platform=scan_platform, severity=severity, disabled_scanners=disabled_scanners, timeout=timeout, diff --git a/posit-bakery/posit_bakery/plugins/builtin/trivy/command.py b/posit-bakery/posit_bakery/plugins/builtin/trivy/command.py index 5ebe579d..03ccad8c 100644 --- a/posit-bakery/posit_bakery/plugins/builtin/trivy/command.py +++ b/posit-bakery/posit_bakery/plugins/builtin/trivy/command.py @@ -1,3 +1,4 @@ +import re from pathlib import Path from typing import Annotated, Self @@ -5,6 +6,7 @@ from posit_bakery.image.image_target import ImageTarget, ImageTargetContext from posit_bakery.plugins.builtin.trivy.options import TrivyOptions +from posit_bakery.settings import SETTINGS from posit_bakery.util import find_bin TRIVY_ALL_SCANNERS = ["vuln", "secret", "license", "misconfig"] @@ -35,6 +37,11 @@ class TrivyCommand(BaseModel): # ToolOptions fields tool_options: Annotated[TrivyOptions | None, Field(default=None)] + # Platform actually being scanned, as resolved by the CLI. Not derivable from + # image_target: a target is not platform-scoped (image_os.platforms is a list), + # so the host architecture is only correct when the scan happens to be native. + scan_platform: Annotated[str, Field(default_factory=lambda: f"linux/{SETTINGS.architecture}")] + # CLI pass-through options severity: Annotated[str | None, Field(default=None)] disabled_scanners: Annotated[str | None, Field(default=None)] @@ -48,6 +55,7 @@ def from_image_target( results_dir: Path, *, tool_options: TrivyOptions | None = None, + scan_platform: str | None = None, severity: str | None = None, disabled_scanners: str | None = None, timeout: str | None = None, @@ -67,12 +75,36 @@ def from_image_target( image_target=image_target, results_file=results_file, tool_options=tool_options, + **({"scan_platform": scan_platform} if scan_platform else {}), severity=severity, disabled_scanners=disabled_scanners, timeout=timeout, trivy_config=trivy_config, ) + @computed_field + @property + def scan_category(self) -> str: + """Version-stable category key for GitHub Code Scanning. + + Omits the image version so the same category is reused across releases, + which is what lets code scanning diff a PR against its baseline instead + of reporting every finding as new. Uses tag display names (Variant, OS) + and the scanned platform's architecture to match published image tags. + + Deliberately *not* used as the results filename: two versions of the same + image share a category by design, so filenames stay uid-keyed and the + category travels in the SARIF's automationDetails.id instead. + """ + tv = self.image_target.tag_template_values + parts = [self.image_target.image_name] + if tv["Variant"]: + parts.append(tv["Variant"]) + if tv["OS"]: + parts.append(tv["OS"]) + parts.append(self.scan_platform.removeprefix("linux/")) + return re.sub(r"[ .+/]", "-", "-".join(parts)).lower() + @model_validator(mode="after") def check_trivy_bin(self) -> Self: if not self.trivy_bin: diff --git a/posit-bakery/posit_bakery/plugins/builtin/trivy/report.py b/posit-bakery/posit_bakery/plugins/builtin/trivy/report.py index 9eee44ed..2e5b67c2 100644 --- a/posit-bakery/posit_bakery/plugins/builtin/trivy/report.py +++ b/posit-bakery/posit_bakery/plugins/builtin/trivy/report.py @@ -42,15 +42,26 @@ def breaches(self, severities: list[str]) -> bool: return any(counts.get(sev.strip().upper(), 0) > 0 for sev in severities) @classmethod - def load(cls, filepath: Path) -> "TrivyReport": + def load(cls, filepath: Path, *, scan_category: str | None = None) -> "TrivyReport": """Load a TrivyReport from a Trivy SARIF output file. Re-writes the file with indentation for human readability, since Trivy outputs minified JSON by default. + + When ``scan_category`` is given, stamps it into each run's + ``automationDetails.id`` as ``"/"``. Everything before the last + slash is the code-scanning category, and the trailing slash leaves the run + ID empty. github/codeql-action/upload-sarif only fills automationDetails in + when it is absent, so stamping here lets one directory upload carry a + distinct category per file -- which a single ``category:`` input cannot do. """ raw = filepath.read_text() data = json.loads(raw) + if scan_category: + for run in data.get("runs", []) or []: + run["automationDetails"] = {"id": f"{scan_category}/"} + formatted = json.dumps(data, indent=2) + "\n" if formatted != raw: filepath.write_text(formatted) diff --git a/posit-bakery/posit_bakery/plugins/builtin/trivy/suite.py b/posit-bakery/posit_bakery/plugins/builtin/trivy/suite.py index bd5be865..23e82b70 100644 --- a/posit-bakery/posit_bakery/plugins/builtin/trivy/suite.py +++ b/posit-bakery/posit_bakery/plugins/builtin/trivy/suite.py @@ -20,6 +20,7 @@ def __init__( context: Path, image_targets: list[ImageTarget], *, + scan_platform: str | None = None, severity: str | None = None, disabled_scanners: str | None = None, timeout: str | None = None, @@ -32,6 +33,7 @@ def __init__( TrivyCommand.from_image_target( target, results_dir=self.results_dir, + scan_platform=scan_platform, severity=severity, disabled_scanners=disabled_scanners, timeout=timeout, @@ -79,7 +81,7 @@ def run(self) -> tuple[TrivyReportCollection, BakeryToolRuntimeError | BakeryToo if exit_code == 0: if trivy_command.results_file.exists(): try: - report = TrivyReport.load(trivy_command.results_file) + report = TrivyReport.load(trivy_command.results_file, scan_category=trivy_command.scan_category) report_collection.add_report(trivy_command.image_target, report) except Exception as e: log.error(f"Failed to parse trivy results for '{str(trivy_command.image_target)}': {e}") diff --git a/posit-bakery/test/plugins/builtin/trivy/test_command.py b/posit-bakery/test/plugins/builtin/trivy/test_command.py index 855b3ede..5d66da3c 100644 --- a/posit-bakery/test/plugins/builtin/trivy/test_command.py +++ b/posit-bakery/test/plugins/builtin/trivy/test_command.py @@ -1,9 +1,11 @@ +import re from unittest.mock import patch import pytest from pydantic import ValidationError from posit_bakery.plugins.builtin.trivy.command import TrivyCommand +from posit_bakery.settings import SETTINGS pytestmark = [ pytest.mark.unit, @@ -198,20 +200,72 @@ def test_results_file_is_uid_scoped(self, basic_standard_image_target): assert cmd.results_file.stem == basic_standard_image_target.uid assert cmd.results_file.suffix == ".sarif" - def test_results_files_are_unique_per_target(self, get_config_obj): - """Every target in a multi-version project must get its own SARIF file. + def test_scan_category_is_version_stable(self, basic_standard_image_target): + """scan_category must omit the image version so it stays stable across releases.""" + results_dir = basic_standard_image_target.context.base_path / "results" / "trivy" + cmd = TrivyCommand.from_image_target( + image_target=basic_standard_image_target, + results_dir=results_dir, + ) + assert basic_standard_image_target.image_version.name not in cmd.scan_category + + def test_scan_category_includes_image_variant_os(self, basic_standard_image_target): + """scan_category includes image name plus the Variant and OS tag display names.""" + results_dir = basic_standard_image_target.context.base_path / "results" / "trivy" + cmd = TrivyCommand.from_image_target( + image_target=basic_standard_image_target, + results_dir=results_dir, + ) + tv = basic_standard_image_target.tag_template_values + sanitized = lambda s: re.sub(r"[ .+/]", "-", s).lower() # noqa: E731 + assert sanitized(basic_standard_image_target.image_name) in cmd.scan_category + if tv["Variant"]: + assert sanitized(tv["Variant"]) in cmd.scan_category + if tv["OS"]: + assert sanitized(tv["OS"]) in cmd.scan_category + + def test_scan_category_uses_scanned_platform_not_host_arch(self, basic_standard_image_target): + """The category's arch must come from the scanned platform, not the host. + + A cross-arch scan (e.g. --image-platform linux/arm64 on an amd64 host) would + otherwise label arm64 results 'amd64' and collide both arches into one + code-scanning category, silently overwriting each other. + """ + results_dir = basic_standard_image_target.context.base_path / "results" / "trivy" + cmd = TrivyCommand.from_image_target( + image_target=basic_standard_image_target, + results_dir=results_dir, + scan_platform="linux/arm64", + ) + assert cmd.scan_category.endswith("-arm64") + assert "amd64" not in cmd.scan_category + + def test_scan_category_defaults_to_host_arch(self, basic_standard_image_target): + """With no explicit platform, the category falls back to the host architecture.""" + results_dir = basic_standard_image_target.context.base_path / "results" / "trivy" + cmd = TrivyCommand.from_image_target( + image_target=basic_standard_image_target, + results_dir=results_dir, + ) + assert cmd.scan_category.endswith(f"-{SETTINGS.architecture}") + + def test_category_is_shared_across_versions_but_files_are_not(self, get_config_obj): + """The 'changeset' fixture holds two versions of one image/variant/OS. - The uid is the only per-target identifier that includes the version, so a - results_file keyed on anything coarser (image/variant/OS/arch) silently - overwrites earlier versions' output within a single scan run. + Those two targets must share a single code-scanning category (that shared + key is what makes PR-vs-baseline diffing work) while still writing to + separate SARIF files. Deriving the filename from the category collapses + them onto one path, so the later scan silently overwrites the earlier. """ - config_obj = get_config_obj("basic") + config_obj = get_config_obj("changeset") results_dir = config_obj.base_path / "results" / "trivy" - files = [ - TrivyCommand.from_image_target(image_target=target, results_dir=results_dir).results_file + cmds = [ + TrivyCommand.from_image_target(image_target=target, results_dir=results_dir) for target in config_obj.targets ] - assert len(set(files)) == len(config_obj.targets) + assert len(cmds) > 1 + assert len(set(c.scan_category for c in cmds)) == 1 + assert len(set(c.results_file for c in cmds)) == len(cmds) def test_validate_no_trivy_bin(self, basic_standard_image_target): """Test that validation fails if trivy binary cannot be found.""" diff --git a/posit-bakery/test/plugins/builtin/trivy/test_report.py b/posit-bakery/test/plugins/builtin/trivy/test_report.py index cf1694e5..6c7ffade 100644 --- a/posit-bakery/test/plugins/builtin/trivy/test_report.py +++ b/posit-bakery/test/plugins/builtin/trivy/test_report.py @@ -53,6 +53,45 @@ def test_unknown_rule_id_counts_as_unknown(self, tmp_path): report = TrivyReport.load(result_file) assert report.unknown_count == 1 + def test_load_stamps_automation_details_id(self, tmp_path): + """scan_category is written to every run's automationDetails.id with a trailing slash. + + Code scanning reads everything before the last slash as the category; the + trailing slash leaves the run ID empty. This is how a single directory-wide + upload-sarif invocation gives each file its own category. + """ + result_file = tmp_path / "stamped.sarif" + result_file.write_text((TRIVY_TESTDATA_DIR / "scan_result.sarif").read_text()) + + TrivyReport.load(result_file, scan_category="connect-min-ubuntu-22-04-amd64") + + data = json.loads(result_file.read_text()) + assert data["runs"] + for run in data["runs"]: + assert run["automationDetails"]["id"] == "connect-min-ubuntu-22-04-amd64/" + + def test_load_without_category_leaves_automation_details_absent(self, tmp_path): + """No scan_category means no automationDetails, so upload-sarif can fill it in.""" + result_file = tmp_path / "unstamped.sarif" + result_file.write_text((TRIVY_TESTDATA_DIR / "scan_result.sarif").read_text()) + + TrivyReport.load(result_file) + + data = json.loads(result_file.read_text()) + assert all("automationDetails" not in run for run in data["runs"]) + + def test_load_stamping_preserves_counts(self, tmp_path): + """Stamping must not disturb the parsed severity counts.""" + result_file = tmp_path / "stamped.sarif" + result_file.write_text((TRIVY_TESTDATA_DIR / "scan_result.sarif").read_text()) + + report = TrivyReport.load(result_file, scan_category="some-category") + + assert report.critical_count == 1 + assert report.high_count == 2 + assert report.medium_count == 1 + assert report.total_count == 4 + @pytest.mark.parametrize( "severities,expected", [ diff --git a/posit-bakery/test/plugins/builtin/trivy/test_suite.py b/posit-bakery/test/plugins/builtin/trivy/test_suite.py index 3cd7a8c4..adeaa3dd 100644 --- a/posit-bakery/test/plugins/builtin/trivy/test_suite.py +++ b/posit-bakery/test/plugins/builtin/trivy/test_suite.py @@ -190,6 +190,11 @@ def test_run_integration(self, get_tmpconfig, monkeypatch): sarif_run = raw["runs"][0] assert sarif_run["tool"]["driver"]["name"].lower() == "trivy" + # The category must survive into real trivy output, not just the hand-written + # fixture: upload-sarif reads it from here to give each file in a directory + # upload its own code-scanning category. + assert sarif_run["automationDetails"]["id"] == f"{suite.trivy_commands[0].scan_category}/" + # Cross-check the parsed report against the real SARIF trivy wrote: every # counted severity bucket is non-negative, and they add up to exactly the # number of results trivy actually reported. This fails if TrivyReport.load()