diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index f8c4b6278..08fff5f33 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -55,7 +55,6 @@ jobs: run: | python - <<'PY' import os - import json from pathlib import PurePosixPath import subprocess @@ -84,6 +83,8 @@ jobs: } def is_dependency_input(path: PurePosixPath) -> bool: + # Workflow code changes are covered by CI tests; they do not + # change a dependency graph and should not force a comparison. name = path.name return ( name in exact_names @@ -96,186 +97,36 @@ jobs: or path.as_posix() in { ".github/dependabot.yaml", ".github/dependabot.yml", - ".github/workflows/dependency-review.yml", "vendor/modules.txt", } ) changed = sorted(path.as_posix() for path in paths if is_dependency_input(path)) - # A lockfile-only PR needs a snapshot of its changed ecosystem, - # not a repo-wide PipReport resolution of unrelated requirements. - # Keep the full scan for mixed/unsupported inputs and main pushes. - snapshot_detector = "" - if changed and all(path in {"pyproject.toml", "uv.lock"} for path in changed): - if "uv.lock" in changed: - snapshot_detector = "UvLock" - elif changed and all(PurePosixPath(path).name in {"package.json", "package-lock.json"} for path in changed): - locks = {path for path in changed if PurePosixPath(path).name == "package-lock.json"} - if locks: - workspace_patterns = [] - root_lock_packages = {} - root_valid = True - if "package-lock.json" in locks: - def head_json(path): - content = subprocess.run( - ["git", "show", f"{head}:{path}"], - check=True, capture_output=True, - ).stdout - return json.loads(content) - - try: - root = head_json("package.json") - workspaces = root.get("workspaces", []) - workspace_patterns = workspaces.get("packages", []) if isinstance(workspaces, dict) else workspaces - root_lock_packages = head_json("package-lock.json").get("packages", {}) - root_valid = isinstance(workspace_patterns, list) and isinstance(root_lock_packages, dict) - except (subprocess.CalledProcessError, json.JSONDecodeError, AttributeError): - root_valid = False - - def covered(manifest): - path = PurePosixPath(manifest) - if path.with_name("package-lock.json").as_posix() in locks: - return True - directory = path.parent.as_posix() - return ( - "package-lock.json" in locks - and directory in root_lock_packages - and any(PurePosixPath(directory).match(pattern) for pattern in workspace_patterns) - ) - - if root_valid and all(covered(path) for path in changed if PurePosixPath(path).name == "package.json"): - snapshot_detector = "NpmLockfile3" - elif changed == [".github/workflows/dependency-review.yml"]: - # Exercise both focused detectors on the workflow change itself. - snapshot_detector = "UvLock,NpmLockfile3" with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as handle: handle.write(f"changed={'true' if changed else 'false'}\n") handle.write(f"count={len(changed)}\n") - handle.write(f"snapshot_detector={snapshot_detector}\n") with open(os.environ["GITHUB_STEP_SUMMARY"], "a", encoding="utf-8") as handle: if changed: handle.write("Dependency inputs changed:\n\n") handle.writelines(f"- `{path}`\n" for path in changed) else: handle.write( - "No dependency manifests, lockfiles, or dependency-review " - "configuration changed between the exact base and head trees.\n" + "No dependency inputs changed between the exact base " + "and head trees.\n" ) PY - - name: Stage focused dependency inputs - if: >- - ${{ github.event_name == 'pull_request' && - github.event.pull_request.head.repo.full_name == github.repository && - steps.dependency-changes.outputs.snapshot_detector != '' }} - shell: bash - env: - BASE_SHA: ${{ github.event.pull_request.base.sha }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - SNAPSHOT_DETECTOR: ${{ steps.dependency-changes.outputs.snapshot_detector }} - SNAPSHOT_BASE_SOURCE: ${{ runner.temp }}/dependency-review-base - SNAPSHOT_HEAD_SOURCE: ${{ runner.temp }}/dependency-review-head - run: | - python - <<'PY' - import os - import json - from pathlib import Path - import subprocess - - detector = os.environ["SNAPSHOT_DETECTOR"] - if detector == "UvLock": - locks = [Path("uv.lock")] - elif detector == "NpmLockfile3": - delta = subprocess.run( - ["git", "diff", "--name-only", "-z", os.environ["BASE_SHA"], os.environ["HEAD_SHA"]], - check=True, capture_output=True, - ).stdout - locks = [Path(raw.decode("utf-8")) for raw in delta.split(b"\0") if raw - and Path(raw.decode("utf-8")).name == "package-lock.json"] - else: - # The workflow-only PR exercises both lockfile ecosystems. - locks = {Path("uv.lock")} - for sha in (os.environ["BASE_SHA"], os.environ["HEAD_SHA"]): - tree = subprocess.run( - ["git", "ls-tree", "-r", "--name-only", "-z", sha], - check=True, capture_output=True, - ).stdout - locks.update(Path(raw.decode("utf-8")) for raw in tree.split(b"\0") if raw - and Path(raw.decode("utf-8")).name == "package-lock.json") - - for sha, directory in ( - (os.environ["BASE_SHA"], os.environ["SNAPSHOT_BASE_SOURCE"]), - (os.environ["HEAD_SHA"], os.environ["SNAPSHOT_HEAD_SOURCE"]), - ): - target = Path(directory) - target.mkdir(parents=True, exist_ok=True) - for lock in locks: - manifest = lock.with_name("pyproject.toml" if lock.name == "uv.lock" else "package.json") - sources = {lock, manifest} - if lock == Path("package-lock.json"): - root_result = subprocess.run( - ["git", "show", f"{sha}:package.json"], - capture_output=True, - ) - lock_result = subprocess.run( - ["git", "show", f"{sha}:package-lock.json"], - capture_output=True, - ) - if root_result.returncode == 0 and lock_result.returncode == 0: - root = json.loads(root_result.stdout) - workspaces = root.get("workspaces", []) - patterns = workspaces.get("packages", []) if isinstance(workspaces, dict) else workspaces - lock_data = json.loads(lock_result.stdout) - for name in lock_data.get("packages", {}): - if name and any(Path(name).match(pattern) for pattern in patterns): - sources.add(Path(name) / "package.json") - for source in sources: - result = subprocess.run( - ["git", "show", f"{sha}:{source.as_posix()}"], capture_output=True, - ) - if result.returncode == 0: - destination = target / source - destination.parent.mkdir(parents=True, exist_ok=True) - destination.write_bytes(result.stdout) - PY - - - name: Submit focused base dependency snapshot - if: >- - ${{ github.event_name == 'pull_request' && - github.event.pull_request.head.repo.full_name == github.repository && - steps.dependency-changes.outputs.snapshot_detector != '' }} - uses: advanced-security/component-detection-dependency-submission-action@b282c67b1008fde9b60da4bdfcd8849613e293b2 # v0.1.6 - with: - snapshot-sha: ${{ github.event.pull_request.base.sha }} - snapshot-ref: ${{ format('refs/heads/{0}', github.event.pull_request.base.ref) }} - filePath: ${{ runner.temp }}/dependency-review-base - correlator: focused-lockfile-review - detectorArgs: UvLock=EnableIfDefaultOff,NpmLockfile3=EnableIfDefaultOff - - - name: Submit focused dependency snapshot - if: >- - ${{ github.event_name == 'pull_request' && - github.event.pull_request.head.repo.full_name == github.repository && - steps.dependency-changes.outputs.snapshot_detector != '' }} - uses: advanced-security/component-detection-dependency-submission-action@b282c67b1008fde9b60da4bdfcd8849613e293b2 # v0.1.6 - with: - snapshot-sha: ${{ github.event.pull_request.head.sha }} - snapshot-ref: ${{ format('refs/heads/{0}', github.event.pull_request.head.ref) }} - filePath: ${{ runner.temp }}/dependency-review-head - correlator: focused-lockfile-review - detectorArgs: UvLock=EnableIfDefaultOff,NpmLockfile3=EnableIfDefaultOff - - name: Submit dependency snapshots if: >- ${{ github.event_name == 'push' || (github.event.pull_request.head.repo.full_name == github.repository && - steps.dependency-changes.outputs.changed == 'true' && - steps.dependency-changes.outputs.snapshot_detector == '') }} - # Keep scanning the full tree, but bound each pip report. On a report - # timeout component-detection falls back to declared source dependencies. + steps.dependency-changes.outputs.changed == 'true') }} + # Scan the same full tree on main and on dependency PRs so GitHub compares + # matching snapshot sets. Bound each pip report; on a timeout, component- + # detection falls back to declared source dependencies. env: PipReportFileLevelTimeoutSeconds: "120" - uses: advanced-security/component-detection-dependency-submission-action@b282c67b1008fde9b60da4bdfcd8849613e293b2 # v0.1.6 + uses: advanced-security/component-detection-dependency-submission-action@a02aeda7a175ad4dd38a13c4d3ff2df9da8f2cf0 # v0.2.0 with: snapshot-sha: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} snapshot-ref: ${{ github.event_name == 'pull_request' && format('refs/heads/{0}', github.event.pull_request.head.ref) || github.ref }} @@ -352,8 +203,9 @@ jobs: pkg:npm/@img/sharp-win32-x64 comment-summary-in-pr: always - # Preserve the graph comparison even when a composite license is rejected. - # An empty/missing action output stays empty; it is not a clean result. + # Fetch the same dependency diff from GitHub's API instead of expanding + # dependency-changes in an Actions expression. Large grouped updates can + # exceed the runner's template-object limit before the record step starts. - name: Record dependency review evidence if: >- ${{ always() && github.event_name == 'pull_request' && @@ -362,41 +214,8 @@ jobs: BASE_SHA: ${{ github.event.pull_request.base.sha }} HEAD_SHA: ${{ github.event.pull_request.head.sha }} REVIEW_OUTCOME: ${{ steps.review.outcome }} - DEPENDENCY_CHANGES: ${{ steps.review.outputs.dependency-changes }} - INVALID_LICENSE_CHANGES: ${{ steps.review.outputs.invalid-license-changes }} - VULNERABLE_CHANGES: ${{ steps.review.outputs.vulnerable-changes }} - DENIED_CHANGES: ${{ steps.review.outputs.denied-changes }} - run: | - python3 - <<'PY' - import json - import os - from pathlib import Path - - evidence = { - "schema_version": 1, - "base_sha": os.environ["BASE_SHA"], - "head_sha": os.environ["HEAD_SHA"], - "run_id": os.environ["GITHUB_RUN_ID"], - "run_attempt": os.environ["GITHUB_RUN_ATTEMPT"], - "review_outcome": os.environ["REVIEW_OUTCOME"], - # Retain the exact raw JSON strings, including duplicate entries, - # full SPDX expressions, and empty outputs from incomplete reviews. - "raw_outputs": { - name: os.environ[variable] - for name, variable in { - "dependency-changes": "DEPENDENCY_CHANGES", - "invalid-license-changes": "INVALID_LICENSE_CHANGES", - "vulnerable-changes": "VULNERABLE_CHANGES", - "denied-changes": "DENIED_CHANGES", - }.items() - }, - } - destination = Path("dependency-review-evidence") - destination.mkdir(exist_ok=True) - (destination / "review.json").write_text( - json.dumps(evidence, indent=2) + "\n", encoding="utf-8" - ) - PY + GITHUB_TOKEN: ${{ github.token }} + run: python3 scripts/ci/record_dependency_review.py - name: Upload dependency review evidence if: >- @@ -405,7 +224,7 @@ jobs: uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: dependency-review-${{ github.event.pull_request.head.sha }}-${{ github.run_attempt }} - path: dependency-review-evidence/review.json + path: dependency-review-evidence/ if-no-files-found: error retention-days: 30 diff --git a/package-lock.json b/package-lock.json index 2784aabd0..5fcfc6905 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10612,9 +10612,9 @@ "license": "MIT" }, "node_modules/fast-uri": { - "version": "3.1.7", - "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.7.tgz", - "integrity": "sha512-dOvZVzjdZdz7phd9v6jCbwxrBW3fK6n8Rc0CtdmM4bumzMnxywBYhuph6J819RRw/ku+rLbelwfMunktuzVVHg==", + "version": "3.1.9", + "resolved": "https://registry.npmjs.org/fast-uri/-/fast-uri-3.1.9.tgz", + "integrity": "sha512-hS/0Wt3Ya043oKXs0sIxrMpKQITXhMLIrVAKKXi4gozGC9v7YRWVnXIZzjNPTO02M9BdxLCWqK1yRXgNziVyJQ==", "dev": true, "funding": [ { diff --git a/scripts/ci/record_dependency_review.py b/scripts/ci/record_dependency_review.py new file mode 100644 index 000000000..a63eafc90 --- /dev/null +++ b/scripts/ci/record_dependency_review.py @@ -0,0 +1,71 @@ +"""Save the complete GitHub dependency diff without Actions output expansion.""" + +from __future__ import annotations + +import hashlib +import json +import os +import re +from pathlib import Path +from urllib.request import Request, urlopen + + +def record_dependency_review( + environment: dict[str, str], + destination: Path, + opener=urlopen, +) -> dict: + base = environment["BASE_SHA"] + head = environment["HEAD_SHA"] + if not all(re.fullmatch(r"[0-9a-f]{40}", sha) for sha in (base, head)): + raise ValueError("Dependency review requires exact commit SHAs") + + repository = environment["GITHUB_REPOSITORY"] + if not re.fullmatch(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+", repository): + raise ValueError("Invalid GitHub repository") + + api_root = environment.get("GITHUB_API_URL", "https://api.github.com").rstrip("/") + url = f"{api_root}/repos/{repository}/dependency-graph/compare/{base}...{head}" + request = Request( + url, + headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {environment['GITHUB_TOKEN']}", + "X-GitHub-Api-Version": "2022-11-28", + }, + ) + with opener(request, timeout=90) as response: + raw_diff = response.read() + snapshot_warning = response.headers.get( + "x-github-dependency-graph-snapshot-warnings" + ) + if snapshot_warning: + raise ValueError( + "Dependency graph reported missing snapshots; refusing to save " + "an incomplete comparison as review evidence" + ) + changes = json.loads(raw_diff) + if not isinstance(changes, list) or not all(isinstance(item, dict) for item in changes): + raise ValueError("Dependency diff API did not return a complete change list") + + destination.mkdir(parents=True, exist_ok=True) + (destination / "dependency-diff.json").write_bytes(raw_diff) + evidence = { + "schema_version": 2, + "base_sha": base, + "head_sha": head, + "run_id": environment["GITHUB_RUN_ID"], + "run_attempt": environment["GITHUB_RUN_ATTEMPT"], + "review_outcome": environment["REVIEW_OUTCOME"], + "dependency_diff_source": url, + "dependency_diff_count": len(changes), + "dependency_diff_sha256": hashlib.sha256(raw_diff).hexdigest(), + "dependency_diff_file": "dependency-diff.json", + } + (destination / "review.json").write_text(json.dumps(evidence, indent=2) + "\n") + return evidence + + +if __name__ == "__main__": + result = record_dependency_review(os.environ, Path("dependency-review-evidence")) + print(f"Saved {result['dependency_diff_count']} dependency changes") diff --git a/tests/unit/test_dependency_review_workflow.py b/tests/unit/test_dependency_review_workflow.py index 978a9ae50..08447c6ad 100644 --- a/tests/unit/test_dependency_review_workflow.py +++ b/tests/unit/test_dependency_review_workflow.py @@ -1,9 +1,11 @@ from __future__ import annotations +import hashlib +import io import json import os +import runpy import subprocess -import sys from pathlib import Path import pytest @@ -60,49 +62,17 @@ def test_dependency_review_submits_pr_head_snapshot_and_retries_warnings() -> No "vendor/modules.txt", "Dockerfile", ".github/dependabot.yaml", - ".github/workflows/dependency-review.yml", ): assert protected_input in detection_script assert '"git", "diff", "--name-only", "-z", base, head' in detection_script - focused = next( - step for step in steps if step["name"] == "Submit focused dependency snapshot" - ) - assert "steps.dependency-changes.outputs.snapshot_detector != ''" in focused["if"] - assert ( - "github.event.pull_request.head.repo.full_name == github.repository" - in focused["if"] - ) - assert focused["with"]["filePath"] == "${{ runner.temp }}/dependency-review-head" - assert focused["with"]["correlator"] == "focused-lockfile-review" - assert "UvLock=EnableIfDefaultOff" in focused["with"]["detectorArgs"] - assert "NpmLockfile3=EnableIfDefaultOff" in focused["with"]["detectorArgs"] - assert ( - focused["with"]["snapshot-sha"] == "${{ github.event.pull_request.head.sha }}" - ) - - focused_base = next( - step - for step in steps - if step["name"] == "Submit focused base dependency snapshot" - ) - assert ( - focused_base["with"]["filePath"] == "${{ runner.temp }}/dependency-review-base" - ) - assert focused_base["with"]["correlator"] == focused["with"]["correlator"] - assert ( - focused_base["with"]["snapshot-sha"] - == "${{ github.event.pull_request.base.sha }}" - ) - submission = next( step for step in steps if step["name"] == "Submit dependency snapshots" ) assert "github.event_name == 'push'" in submission["if"] assert "steps.dependency-changes.outputs.changed == 'true'" in submission["if"] - assert ( - "steps.dependency-changes.outputs.snapshot_detector == ''" in submission["if"] - ) + assert "snapshot_detector" not in submission["if"] + assert not any("focused" in step["name"].lower() for step in steps) assert submission["with"]["snapshot-sha"] == ( "${{ github.event_name == 'pull_request' && " "github.event.pull_request.head.sha || github.sha }}" @@ -134,29 +104,22 @@ def test_dependency_review_submits_pr_head_snapshot_and_retries_warnings() -> No @pytest.mark.parametrize( - ("changed_paths", "expected_changed", "expected_detector"), + ("changed_paths", "expected_changed"), [ - (["pyproject.toml", "uv.lock"], "true", "UvLock"), - ( - ["apps/web/package.json", "apps/web/package-lock.json"], - "true", - "NpmLockfile3", - ), - (["requirements.txt"], "true", ""), - (["uv.lock", "requirements.txt"], "true", ""), - ([".github/workflows/dependency-review.yml"], "true", "UvLock,NpmLockfile3"), - (["src/app.py"], "false", ""), + (["pyproject.toml", "uv.lock"], "true"), + (["apps/web/package.json", "apps/web/package-lock.json"], "true"), + (["requirements.txt"], "true"), + (["uv.lock", "requirements.txt"], "true"), + ([".github/workflows/dependency-review.yml"], "false"), + (["src/app.py"], "false"), ], ) -def test_snapshot_detector_keeps_unrelated_pip_resolves_out_of_lockfile_prs( - tmp_path: Path, - changed_paths: list[str], - expected_changed: str, - expected_detector: str, +def test_dependency_detection_selects_full_scan_for_manifest_changes( + tmp_path: Path, changed_paths: list[str], expected_changed: str ) -> None: repo = tmp_path / "repo" repo.mkdir() - all_paths = { + for name in { "pyproject.toml", "uv.lock", "apps/web/package.json", @@ -164,8 +127,7 @@ def test_snapshot_detector_keeps_unrelated_pip_resolves_out_of_lockfile_prs( "requirements.txt", "src/app.py", ".github/workflows/dependency-review.yml", - } - for name in all_paths: + }: path = repo / name path.parent.mkdir(parents=True, exist_ok=True) path.write_text("before\n") @@ -201,122 +163,6 @@ def git(*args: str) -> str: ) head = git("rev-parse", "HEAD") - detection = next( - step - for step in _load_workflow()["jobs"]["dependency-review"]["steps"] - if step["name"] == "Detect dependency manifest changes" - ) - output = tmp_path / "github-output" - summary = tmp_path / "github-summary" - env = os.environ | { - "BASE_SHA": base, - "HEAD_SHA": head, - "GITHUB_OUTPUT": str(output), - "GITHUB_STEP_SUMMARY": str(summary), - } - subprocess.run(["bash", "-c", detection["run"]], cwd=repo, env=env, check=True) - values = dict(line.split("=", 1) for line in output.read_text().splitlines()) - assert values["changed"] == expected_changed - assert values["snapshot_detector"] == expected_detector - if expected_detector: - stage = next( - step - for step in _load_workflow()["jobs"]["dependency-review"]["steps"] - if step["name"] == "Stage focused dependency inputs" - ) - base_source = tmp_path / "focused-base" - head_source = tmp_path / "focused-head" - subprocess.run( - ["bash", "-c", stage["run"]], - cwd=repo, - check=True, - env=env - | { - "SNAPSHOT_BASE_SOURCE": str(base_source), - "SNAPSHOT_HEAD_SOURCE": str(head_source), - "SNAPSHOT_DETECTOR": expected_detector, - }, - ) - expected = ( - {"uv.lock", "pyproject.toml"} - if expected_detector == "UvLock" - else {"apps/web/package-lock.json", "apps/web/package.json"} - if expected_detector == "NpmLockfile3" - else { - "uv.lock", - "pyproject.toml", - "apps/web/package-lock.json", - "apps/web/package.json", - } - ) - for source in (base_source, head_source): - staged = { - path.relative_to(source).as_posix() - for path in source.rglob("*") - if path.is_file() - } - assert staged == expected - for name in changed_paths: - if name in expected: - assert (base_source / name).read_text() == "before\n" - assert (head_source / name).read_text() == "after\n" - - -@pytest.mark.parametrize( - ("changed_paths", "expected_detector"), - [ - (["package-lock.json"], "NpmLockfile3"), - (["package-lock.json", "apps/web/package.json"], "NpmLockfile3"), - (["package-lock.json", "sdk/typescript/package.json"], ""), - (["apps/web/package.json"], ""), - ], -) -def test_root_npm_lock_stages_workspace_roots_and_rejects_uncovered_manifests( - tmp_path: Path, changed_paths: list[str], expected_detector: str -) -> None: - repo = tmp_path / "repo" - for name, content in { - "package.json": '{"workspaces": ["apps/*"]}\n', - "package-lock.json": '{"packages": {"": {}, "apps/web": {}}}\n', - "apps/web/package.json": '{"name": "eventrelay-web"}\n', - "sdk/typescript/package.json": '{"name": "eventrelay-sdk"}\n', - }.items(): - path = repo / name - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(content) - - def git(*args: str) -> str: - return subprocess.run( - ["git", *args], cwd=repo, check=True, capture_output=True, text=True - ).stdout.strip() - - git("init", "-q") - git("add", ".") - git( - "-c", - "user.name=Workflow test", - "-c", - "user.email=workflow@example.com", - "commit", - "-qm", - "base", - ) - base = git("rev-parse", "HEAD") - for name in changed_paths: - path = repo / name - path.write_text(path.read_text().rstrip()[:-1] + ', "test": "after"}\n') - git("add", ".") - git( - "-c", - "user.name=Workflow test", - "-c", - "user.email=workflow@example.com", - "commit", - "-qm", - "change", - ) - head = git("rev-parse", "HEAD") - steps = _load_workflow()["jobs"]["dependency-review"]["steps"] detection = next( step for step in steps if step["name"] == "Detect dependency manifest changes" @@ -330,32 +176,16 @@ def git(*args: str) -> str: } subprocess.run(["bash", "-c", detection["run"]], cwd=repo, env=env, check=True) values = dict(line.split("=", 1) for line in output.read_text().splitlines()) - assert values["snapshot_detector"] == expected_detector - if expected_detector: - stage = next( - step for step in steps if step["name"] == "Stage focused dependency inputs" - ) - base_source = tmp_path / "focused-base" - head_source = tmp_path / "focused-head" - subprocess.run( - ["bash", "-c", stage["run"]], - cwd=repo, - check=True, - env=env - | { - "SNAPSHOT_BASE_SOURCE": str(base_source), - "SNAPSHOT_HEAD_SOURCE": str(head_source), - "SNAPSHOT_DETECTOR": expected_detector, - }, - ) - expected = {"package-lock.json", "package.json", "apps/web/package.json"} - for source in (base_source, head_source): - staged = { - path.relative_to(source).as_posix() - for path in source.rglob("*") - if path.is_file() - } - assert staged == expected + assert values["changed"] == expected_changed + assert values["count"] == str( + len(changed_paths) if expected_changed == "true" else 0 + ) + assert "snapshot_detector" not in values + submission = next( + step for step in steps if step["name"] == "Submit dependency snapshots" + ) + assert "steps.dependency-changes.outputs.changed == 'true'" in submission["if"] + assert "steps.dependency-changes.outputs.snapshot_detector" not in submission["if"] def test_numpy_remains_subject_to_the_unchanged_license_policy() -> None: @@ -391,11 +221,9 @@ def test_numpy_remains_subject_to_the_unchanged_license_policy() -> None: ) -@pytest.mark.parametrize( - "review_outcome,raw_output", [("failure", None), ("success", "")] -) -def test_dependency_review_evidence_preserves_composite_license_and_unknown_outputs( - tmp_path: Path, review_outcome: str, raw_output: str | None +@pytest.mark.parametrize("review_outcome", ["failure", "success"]) +def test_dependency_review_evidence_preserves_large_complete_api_diff( + tmp_path: Path, review_outcome: str ) -> None: steps = _load_workflow()["jobs"]["dependency-review"]["steps"] review = next(step for step in steps if step["name"] == "Dependency Review") @@ -410,47 +238,112 @@ def test_dependency_review_evidence_preserves_composite_license_and_unknown_outp assert "always()" in step["if"] assert "github.event_name == 'pull_request'" in step["if"] assert "steps.dependency-changes.outputs.changed == 'true'" in step["if"] - assert record["env"]["INVALID_LICENSE_CHANGES"] == ( - "${{ steps.review.outputs.invalid-license-changes }}" - ) + assert record["run"] == "python3 scripts/ci/record_dependency_review.py" + assert not any("steps.review.outputs" in value for value in record["env"].values()) assert "github.event.pull_request.head.sha" in upload["with"]["name"] - assert upload["with"]["path"] == "dependency-review-evidence/review.json" + assert upload["with"]["path"] == "dependency-review-evidence/" assert upload["with"]["if-no-files-found"] == "error" expression = "BSD-3-Clause AND 0BSD AND MIT AND Zlib AND CC0-1.0" finding = {"name": "numpy", "version": "2.5.4", "license": expression} - # Keep duplicated occurrences, the full AND expression, and hostile text as data. - invalid = json.dumps( - {"forbidden": [finding, finding], "unresolved": [], "unlicensed": []} - ) - dependencies = json.dumps([finding, {"name": "$(touch injected)"}]) + # This response exceeds the Actions template-object limit hit by grouped PRs. + changes = [finding, finding, {"name": "$(touch injected)"}] + [ + {"name": f"package-{index}", "license": "MIT", "version": "1.0.0"} + for index in range(1500) + ] + raw = json.dumps(changes).encode() + assert len(raw) > 64_000 environment = { - # A failing subprocess may print its environment in pytest's traceback. - # Pass only this test's inputs, never credentials from the caller. - "PATH": os.pathsep.join((str(Path(sys.executable).parent), os.defpath)), "BASE_SHA": "a" * 40, "HEAD_SHA": "b" * 40, + "GITHUB_REPOSITORY": "groupthinking/EventRelay", + "GITHUB_API_URL": "https://api.github.com", + "GITHUB_TOKEN": "dummy-test-token", "GITHUB_RUN_ID": "123", "GITHUB_RUN_ATTEMPT": "2", "REVIEW_OUTCOME": review_outcome, - "DEPENDENCY_CHANGES": dependencies if raw_output is None else raw_output, - "INVALID_LICENSE_CHANGES": invalid if raw_output is None else raw_output, - "VULNERABLE_CHANGES": "[]" if raw_output is None else raw_output, - "DENIED_CHANGES": "", } - subprocess.run( - ["bash", "-e", "-c", record["run"]], cwd=tmp_path, env=environment, check=True + module = runpy.run_path( + str(WORKFLOW_PATH.parents[2] / "scripts/ci/record_dependency_review.py") ) - saved = json.loads((tmp_path / upload["with"]["path"]).read_text()) + seen_requests = [] + + class ApiResponse(io.BytesIO): + headers: dict[str, str] = {} + + def fetch(request, timeout): + seen_requests.append((request, timeout)) + return ApiResponse(raw) + + destination = tmp_path / "dependency-review-evidence" + module["record_dependency_review"](environment, destination, opener=fetch) + saved = json.loads((destination / "review.json").read_text()) assert saved["base_sha"] == "a" * 40 assert saved["head_sha"] == "b" * 40 assert saved["run_id"] == "123" assert saved["run_attempt"] == "2" assert saved["review_outcome"] == review_outcome - assert saved["raw_outputs"] == { - "dependency-changes": environment["DEPENDENCY_CHANGES"], - "invalid-license-changes": environment["INVALID_LICENSE_CHANGES"], - "vulnerable-changes": environment["VULNERABLE_CHANGES"], - "denied-changes": "", - } + assert saved["dependency_diff_count"] == len(changes) + assert saved["dependency_diff_sha256"] == hashlib.sha256(raw).hexdigest() + assert (destination / saved["dependency_diff_file"]).read_bytes() == raw + assert saved["dependency_diff_source"] == seen_requests[0][0].full_url + assert seen_requests[0][1] == 90 + assert seen_requests[0][0].get_header("Authorization") == "Bearer dummy-test-token" assert not (tmp_path / "injected").exists() + + +@pytest.mark.parametrize("raw", [b"[]", b'[{"name":"partial"}]']) +def test_dependency_review_evidence_rejects_snapshot_warnings_before_writing( + tmp_path: Path, raw: bytes +) -> None: + module = runpy.run_path( + str(WORKFLOW_PATH.parents[2] / "scripts/ci/record_dependency_review.py") + ) + environment = { + "BASE_SHA": "a" * 40, + "HEAD_SHA": "b" * 40, + "GITHUB_REPOSITORY": "groupthinking/EventRelay", + "GITHUB_TOKEN": "dummy-test-token", + "GITHUB_RUN_ID": "123", + "GITHUB_RUN_ATTEMPT": "1", + "REVIEW_OUTCOME": "success", + } + + class ApiResponse(io.BytesIO): + headers = { + "x-github-dependency-graph-snapshot-warnings": "bWlzc2luZyBzbmFwc2hvdHM=" + } + + destination = tmp_path / "dependency-review-evidence" + with pytest.raises(ValueError, match="missing snapshots"): + module["record_dependency_review"]( + environment, destination, opener=lambda request, timeout: ApiResponse(raw) + ) + assert not destination.exists() + + +def test_dependency_review_evidence_saves_warning_free_empty_diff( + tmp_path: Path, +) -> None: + module = runpy.run_path( + str(WORKFLOW_PATH.parents[2] / "scripts/ci/record_dependency_review.py") + ) + environment = { + "BASE_SHA": "a" * 40, + "HEAD_SHA": "b" * 40, + "GITHUB_REPOSITORY": "groupthinking/EventRelay", + "GITHUB_TOKEN": "dummy-test-token", + "GITHUB_RUN_ID": "123", + "GITHUB_RUN_ATTEMPT": "1", + "REVIEW_OUTCOME": "success", + } + + class ApiResponse(io.BytesIO): + headers: dict[str, str] = {} + + destination = tmp_path / "dependency-review-evidence" + saved = module["record_dependency_review"]( + environment, destination, opener=lambda request, timeout: ApiResponse(b"[]") + ) + assert saved["dependency_diff_count"] == 0 + assert (destination / "dependency-diff.json").read_bytes() == b"[]"