diff --git a/.github/workflows/security-scan.yml b/.github/workflows/security-scan.yml index 500e22b4ab..e04d7bf8f3 100644 --- a/.github/workflows/security-scan.yml +++ b/.github/workflows/security-scan.yml @@ -164,7 +164,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 @@ -182,7 +182,7 @@ jobs: with: scan-args: | --format=json - --output=old-results.json + --output-file=old-results.json --no-resolve --allow-no-lockfiles -r @@ -215,7 +215,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 @@ -233,7 +233,7 @@ jobs: with: scan-args: | --format=json - --output=new-results.json + --output-file=new-results.json --no-resolve --allow-no-lockfiles -r @@ -286,7 +286,7 @@ jobs: uses: google/osv-scanner-action/osv-reporter-action@8e5cf47b818121e8b405931c82126c2630b0b20d # v2.3.8 with: scan-args: | - --output=results.sarif + --output-files=results.sarif --old=old-results.json --new=new-results.json --gh-annotations=true @@ -323,6 +323,10 @@ jobs: uses: github/codeql-action/upload-sarif@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 with: sarif_file: results.sarif + # The exact head checkout lives in `source`, not the workspace root; + # without this binding upload-sarif logs "does not appear to be a git + # repository" twice and falls back to server-derived commit identity. + checkout_path: ${{ github.workspace }}/source # results.sarif is produced after checkout of the pull request head. # Uploading it against refs/pull/*/merge can race GitHub's synthetic # merge ref and fail with "commit_oid is not a merge commit". diff --git a/tests/test_required_workflow_queue_contract.py b/tests/test_required_workflow_queue_contract.py index 19fe6b0f7f..87277d45f5 100644 --- a/tests/test_required_workflow_queue_contract.py +++ b/tests/test_required_workflow_queue_contract.py @@ -1674,8 +1674,8 @@ def test_security_scan_preserves_base_output_across_cross_fork_checkout() -> Non assert workflow.count("--allow-no-lockfiles") == 4 assert workflow.count("path: source") == 2 - assert workflow.count("--output=old-results.json") == 2 - assert workflow.count("--output=new-results.json") == 2 + assert workflow.count("--output-file=old-results.json") == 2 + assert workflow.count("--output-file=new-results.json") == 2 assert workflow.count("source/") == 4 assert "clean: false" not in workflow assert "test -s old-results.json" in workflow @@ -1732,12 +1732,47 @@ 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_osv_scan_uses_current_output_flags_and_binds_sarif_checkout_path() -> None: + """Drop deprecated OSV output flags and bind upload-sarif to the real checkout. + + Live evidence (ContextualWisdomLab/.github#2132): the pinned + `ghcr.io/google/osv-scanner-action:v2.5.1` image warns + `--output has been deprecated in favor of --output-file` (scanner) and + `... in favor of --output-files` (reporter), and `upload-sarif` logged + twice that the workspace root "does not appear to be a git repository" + because the exact head is checked out into `source`. A bare + `--output-files=` defaults to the sarif format in v2.5.1, so the + reporter's output is unchanged. The checkout-path assertion is the + negative fixture: an absent or wrong `checkout_path` fails here instead + of silently relying on server-derived commit identity. + """ + workflow = workflow_text("security-scan.yml") + + # Check each named scanner/reporter step on its own, so a flag removed from + # one step cannot hide behind the same string appearing elsewhere. + for step_name, output_flag in ( + ("Scan base with OSV", "--output-file=old-results.json"), + ("Retry base OSV without transitive resolution", "--output-file=old-results.json"), + ("Scan head with OSV", "--output-file=new-results.json"), + ("Retry head OSV without transitive resolution", "--output-file=new-results.json"), + ("Report PR-introduced OSV findings", "--output-files=results.sarif"), + ): + step = workflow_step(workflow, step_name) + assert output_flag in step, step_name + assert "\n --output=" not in step, step_name + + head_checkout = workflow_step(workflow, "Checkout head") + checkout_dir = re.search(r"(?m)^\s+path: (\S+)$", head_checkout).group(1) + upload_step = workflow_step(workflow, "Upload OSV SARIF to code scanning") + assert f"checkout_path: ${{{{ github.workspace }}}}/{checkout_dir}" in upload_step + + def test_osv_sarif_upload_is_marked_comprehensive_after_clean_comparison( tmp_path: Path, ) -> None: