From bd033d46688974507a239e1622e41ada774cdd1b Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 1 Aug 2026 11:07:51 +0900 Subject: [PATCH 1/2] fix(security): eliminate clean-scan warning output --- .github/workflows/security-scan.yml | 39 +++++++++++++-- .../test_required_workflow_queue_contract.py | 47 +++++++++++++++++-- 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index c3b8fa5db..69077eb0d 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -66,6 +66,17 @@ jobs: - name: Explain OSV scan mode and timeout budget run: | echo "::notice::OSV hard gate scans direct manifest and lockfile evidence with --no-resolve so external transitive registry resolver stalls cannot hold the required-check queue indefinitely. The job is capped at 25 minutes; if this budget is exceeded, rerun after the upstream registry/service recovers or inspect the uploaded debug artifacts." + - name: Checkout synthetic merge attribution commit + # Preserve github.sha in the local object database before the base/head + # comparison checks out two other revisions. upload-sarif consults this + # trusted merge object while calculating analysis metadata even though + # the final OSV upload is explicitly attributed to the PR head. + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + repository: ${{ github.event.pull_request.base.repo.full_name }} + ref: ${{ github.sha }} + fetch-depth: 1 + persist-credentials: false - name: Checkout base uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: @@ -81,7 +92,7 @@ jobs: with: scan-args: | --format=json - --output=old-results.json + --output-file=old-results.json --maven-registry=https://maven-central.storage-download.googleapis.com/maven2 --no-resolve --allow-no-lockfiles @@ -99,7 +110,7 @@ jobs: with: scan-args: | --format=json - --output=old-results.json + --output-file=old-results.json --no-resolve --allow-no-lockfiles -r @@ -120,7 +131,7 @@ jobs: with: scan-args: | --format=json - --output=new-results.json + --output-file=new-results.json --maven-registry=https://maven-central.storage-download.googleapis.com/maven2 --no-resolve --allow-no-lockfiles @@ -138,7 +149,7 @@ jobs: with: scan-args: | --format=json - --output=new-results.json + --output-file=new-results.json --no-resolve --allow-no-lockfiles -r @@ -191,7 +202,7 @@ jobs: uses: google/osv-scanner-action/osv-reporter-action@8dc09193bb540e09b23da07ad7e30bd33bf87018 # v2.3.8 with: scan-args: | - --output=results.sarif + --output-files=results.sarif --old=old-results.json --new=new-results.json --gh-annotations=true @@ -219,6 +230,20 @@ jobs: "comprehensive so fixed PR-introduced alerts close after a clean " "base/head comparison." ) + - name: Verify synthetic merge commit for SARIF attribution + env: + EXPECTED_MERGE_SHA: ${{ github.sha }} + run: | + set -euo pipefail + if ! [[ "$EXPECTED_MERGE_SHA" =~ ^[0-9a-fA-F]{40}$ ]]; then + echo "::error::Synthetic merge attribution SHA is malformed." + exit 1 + fi + if ! git cat-file -e "${EXPECTED_MERGE_SHA}^{commit}" 2>/dev/null; then + echo "::error::Synthetic merge attribution commit is absent from the local object database." + exit 1 + fi + echo "Verified synthetic merge attribution commit before OSV SARIF upload." - name: Upload OSV SARIF to code scanning id: upload_osv_sarif if: always() && hashFiles('results.sarif') != '' @@ -321,6 +346,10 @@ jobs: scan-type: fs scan-ref: . scanners: vuln,secret,misconfig + # Trivy otherwise emits a pip license-discovery warning even though + # license scanning is not selected. The required SARIF file and the + # explicit parser below retain every Medium+ finding and fail closed. + hide-progress: true severity: CRITICAL,HIGH,MEDIUM ignore-unfixed: true format: sarif diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 1c7b6f3ff..95e6e09f7 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -842,8 +842,8 @@ def test_security_scan_allows_repositories_without_supported_lockfiles() -> None workflow = workflow_text("security-scan.yml") assert workflow.count("--allow-no-lockfiles") == 4 - assert "--output=old-results.json" in workflow - assert "--output=new-results.json" in workflow + assert "--output-file=old-results.json" in workflow + assert "--output-file=new-results.json" in workflow assert "test -s old-results.json" in workflow assert "test -s new-results.json" in workflow @@ -914,12 +914,51 @@ def test_osv_scan_logs_and_retries_without_transitive_resolution_on_resolver_fai "Retry head OSV without transitive resolution\n if: steps.osv_head.outcome == 'failure'\n continue-on-error: true" in workflow ) - assert "--output=old-results.json" in workflow - assert "--output=new-results.json" in workflow + assert "--output-file=old-results.json" in workflow + assert "--output-file=new-results.json" in workflow assert "Print OSV findings being compared" in workflow assert "OSV {label} scan produced {len(findings)} finding(s)" in workflow +def test_security_scan_avoids_warning_class_scanner_output() -> None: + workflow = workflow_text("security-scan.yml") + trivy_step = workflow_step(workflow, "Trivy filesystem scan") + merge_checkout_step = workflow_step( + workflow, "Checkout synthetic merge attribution commit" + ) + merge_verify_step = workflow_step( + workflow, "Verify synthetic merge commit for SARIF attribution" + ) + + assert workflow.count("--output-file=old-results.json") == 2 + assert workflow.count("--output-file=new-results.json") == 2 + assert "--output-files=results.sarif" in workflow + assert "--output=old-results.json" not in workflow + assert "--output=new-results.json" not in workflow + assert "--output=results.sarif" not in workflow + + assert "scanners: vuln,secret,misconfig" in trivy_step + assert "hide-progress: true" in trivy_step + assert "limit-severities-for-sarif: true" in trivy_step + + assert ( + "repository: ${{ github.event.pull_request.base.repo.full_name }}" + in merge_checkout_step + ) + assert "ref: ${{ github.sha }}" in merge_checkout_step + assert "fetch-depth: 1" in merge_checkout_step + assert "persist-credentials: false" in merge_checkout_step + assert "EXPECTED_MERGE_SHA: ${{ github.sha }}" in merge_verify_step + assert '[[ "$EXPECTED_MERGE_SHA" =~ ^[0-9a-fA-F]{40}$ ]]' in merge_verify_step + assert 'git cat-file -e "${EXPECTED_MERGE_SHA}^{commit}"' in merge_verify_step + assert workflow.index( + "Checkout synthetic merge attribution commit" + ) < workflow.index("Checkout base") + assert workflow.index( + "Verify synthetic merge commit for SARIF attribution" + ) < workflow.index("Upload OSV SARIF to code scanning") + + def test_osv_sarif_upload_is_marked_comprehensive_after_clean_comparison( tmp_path: Path, ) -> None: From 579a2b20344ea49414952390b6e1333e4e4c3081 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Sat, 1 Aug 2026 11:20:51 +0900 Subject: [PATCH 2/2] fix(security): gate SARIF upload on attribution --- .github/workflows/security-scan.yml | 9 +++++++-- tests/test_required_workflow_queue_contract.py | 11 +++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 69077eb0d..68d06270a 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -203,9 +203,9 @@ jobs: with: scan-args: | --output-files=results.sarif + --output-files=gh-annotations:#stderr --old=old-results.json --new=new-results.json - --gh-annotations=true --fail-on-vuln=true - name: Mark clean OSV SARIF as comprehensive if: always() && hashFiles('results.sarif') != '' @@ -231,6 +231,8 @@ jobs: "base/head comparison." ) - name: Verify synthetic merge commit for SARIF attribution + id: verify_merge_commit + if: always() && hashFiles('results.sarif') != '' env: EXPECTED_MERGE_SHA: ${{ github.sha }} run: | @@ -246,7 +248,10 @@ jobs: echo "Verified synthetic merge attribution commit before OSV SARIF upload." - name: Upload OSV SARIF to code scanning id: upload_osv_sarif - if: always() && hashFiles('results.sarif') != '' + if: >- + always() && + hashFiles('results.sarif') != '' && + steps.verify_merge_commit.outcome == 'success' # The reporter above is the vulnerability gate. Preserve an upload # quota failure in this step's log without reclassifying it as a CVE. continue-on-error: true diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 95e6e09f7..faf99f845 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -929,13 +929,16 @@ def test_security_scan_avoids_warning_class_scanner_output() -> None: merge_verify_step = workflow_step( workflow, "Verify synthetic merge commit for SARIF attribution" ) + upload_step = workflow_step(workflow, "Upload OSV SARIF to code scanning") assert workflow.count("--output-file=old-results.json") == 2 assert workflow.count("--output-file=new-results.json") == 2 assert "--output-files=results.sarif" in workflow + assert "--output-files=gh-annotations:#stderr" in workflow assert "--output=old-results.json" not in workflow assert "--output=new-results.json" not in workflow assert "--output=results.sarif" not in workflow + assert "--gh-annotations=true" not in workflow assert "scanners: vuln,secret,misconfig" in trivy_step assert "hide-progress: true" in trivy_step @@ -948,9 +951,17 @@ def test_security_scan_avoids_warning_class_scanner_output() -> None: assert "ref: ${{ github.sha }}" in merge_checkout_step assert "fetch-depth: 1" in merge_checkout_step assert "persist-credentials: false" in merge_checkout_step + assert "id: verify_merge_commit" in merge_verify_step + assert "if: always() && hashFiles('results.sarif') != ''" in merge_verify_step assert "EXPECTED_MERGE_SHA: ${{ github.sha }}" in merge_verify_step assert '[[ "$EXPECTED_MERGE_SHA" =~ ^[0-9a-fA-F]{40}$ ]]' in merge_verify_step assert 'git cat-file -e "${EXPECTED_MERGE_SHA}^{commit}"' in merge_verify_step + assert "always() &&" in upload_step + assert "hashFiles('results.sarif') != '' &&" in upload_step + assert ( + "steps.verify_merge_commit.outcome == 'success'" + in upload_step + ) assert workflow.index( "Checkout synthetic merge attribution commit" ) < workflow.index("Checkout base")