diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index c1322b36b..f8c4b6278 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -55,6 +55,7 @@ jobs: run: | python - <<'PY' import os + import json from pathlib import PurePosixPath import subprocess @@ -101,9 +102,56 @@ jobs: ) 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") @@ -115,11 +163,118 @@ jobs: ) 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.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. + env: + PipReportFileLevelTimeoutSeconds: "120" uses: advanced-security/component-detection-dependency-submission-action@b282c67b1008fde9b60da4bdfcd8849613e293b2 # v0.1.6 with: snapshot-sha: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} diff --git a/tests/unit/test_dependency_review_workflow.py b/tests/unit/test_dependency_review_workflow.py index d096188f3..978a9ae50 100644 --- a/tests/unit/test_dependency_review_workflow.py +++ b/tests/unit/test_dependency_review_workflow.py @@ -65,15 +65,44 @@ def test_dependency_review_submits_pr_head_snapshot_and_retries_warnings() -> No assert protected_input in detection_script assert '"git", "diff", "--name-only", "-z", base, head' in detection_script - submission = next( + 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.get("uses", "").startswith( - "advanced-security/component-detection-dependency-submission-action@" - ) + 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 submission["with"]["snapshot-sha"] == ( "${{ github.event_name == 'pull_request' && " "github.event.pull_request.head.sha || github.sha }}" @@ -82,6 +111,13 @@ def test_dependency_review_submits_pr_head_snapshot_and_retries_warnings() -> No "${{ github.event_name == 'pull_request' && " "format('refs/heads/{0}', github.event.pull_request.head.ref) || github.ref }}" ) + assert submission["env"] == {"PipReportFileLevelTimeoutSeconds": "120"} + assert ( + 0 + < int(submission["env"]["PipReportFileLevelTimeoutSeconds"]) + < (job["timeout-minutes"] * 60) + ) + assert "detectorsFilter" not in submission["with"] review = next(step for step in steps if step["name"] == "Dependency Review") assert "github.event_name == 'pull_request'" in review["if"] @@ -97,6 +133,231 @@ def test_dependency_review_submits_pr_head_snapshot_and_retries_warnings() -> No assert "steps.dependency-changes.outputs.changed == 'false'" in no_delta["if"] +@pytest.mark.parametrize( + ("changed_paths", "expected_changed", "expected_detector"), + [ + (["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", ""), + ], +) +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, +) -> None: + repo = tmp_path / "repo" + repo.mkdir() + all_paths = { + "pyproject.toml", + "uv.lock", + "apps/web/package.json", + "apps/web/package-lock.json", + "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") + + 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: + (repo / name).write_text("after\n") + git("add", ".") + git( + "-c", + "user.name=Workflow test", + "-c", + "user.email=workflow@example.com", + "commit", + "-qm", + "change", + ) + 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" + ) + output = tmp_path / "github-output" + env = os.environ | { + "BASE_SHA": base, + "HEAD_SHA": head, + "GITHUB_OUTPUT": str(output), + "GITHUB_STEP_SUMMARY": str(tmp_path / "github-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["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 + + def test_numpy_remains_subject_to_the_unchanged_license_policy() -> None: steps = _load_workflow()["jobs"]["dependency-review"]["steps"] review = next(step for step in steps if step["name"] == "Dependency Review") @@ -115,6 +376,8 @@ def test_numpy_remains_subject_to_the_unchanged_license_policy() -> None: "MPL-2.0", "CC0-1.0", "0BSD", + "MIT-CMU", + "Unlicense", } exclusions = settings["allow-dependencies-licenses"].split(",") assert not any(