diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index af840dbe4..8aed0ac94 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -32,16 +32,15 @@ jobs: permissions: contents: write pull-requests: write - timeout-minutes: 10 # Prevent long-running jobs + timeout-minutes: 30 # Allow full-tree scan, scoped lock submissions, and base/head snapshot retry steps: - name: Checkout code uses: actions/checkout@v7 with: ref: ${{ github.event_name == 'pull_request' && github.event.pull_request.head.sha || github.sha }} - # PR runs compare the exact base and head trees before asking the - # dependency graph to compare snapshots. This prevents an unrelated - # source/test-only PR from inheriting stale graph metadata as a fake - # dependency addition. + # PR runs inspect changes introduced after the shared ancestor. + # A dependency update merged into main after this PR branched must + # not turn a source-only PR into a dependency-review run. fetch-depth: 0 persist-credentials: false @@ -55,14 +54,19 @@ jobs: run: | python - <<'PY' import os - import json from pathlib import PurePosixPath import subprocess base = os.environ["BASE_SHA"] head = os.environ["HEAD_SHA"] + merge_base = subprocess.run( + ["git", "merge-base", base, head], + check=True, + capture_output=True, + text=True, + ).stdout.strip() output = subprocess.run( - ["git", "diff", "--name-only", "-z", base, head], + ["git", "diff", "--name-only", "-z", merge_base, head], check=True, capture_output=True, ).stdout @@ -84,8 +88,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. + # Workflow and Dependabot configuration changes are covered by + # CI tests; they do not change the installed dependency graph. name = path.name return ( name in exact_names @@ -95,181 +99,51 @@ jobs: and name.endswith((".in", ".txt")) ) or name.endswith((".csproj", ".fsproj", ".gradle", ".gradle.kts")) - or path.as_posix() in { - ".github/dependabot.yaml", - ".github/dependabot.yml", - "vendor/modules.txt", - } + or path.as_posix() == "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" 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 inputs changed between the exact base " - "and head trees.\n" + "No dependency inputs introduced by this PR since " + "its merge base.\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: - raise ValueError(f"Unexpected focused snapshot detector: {detector}") - - 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. A 120-second + # bound produced base/head drift in the #2501 exact-lock canary. env: - PipReportFileLevelTimeoutSeconds: "120" - uses: advanced-security/component-detection-dependency-submission-action@b282c67b1008fde9b60da4bdfcd8849613e293b2 # v0.1.6 + PipReportFileLevelTimeoutSeconds: "240" + 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 }} + - name: Submit complete lock snapshot + if: >- + ${{ github.event_name == 'push' || + (github.event.pull_request.head.repo.full_name == github.repository && + steps.dependency-changes.outputs.changed == 'true') }} + env: + 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 }} + GITHUB_TOKEN: ${{ github.token }} + run: python3 -m scripts.ci.submit_npm_lock_snapshot + - name: Dependency Review id: review if: >- @@ -280,11 +154,11 @@ jobs: base-ref: ${{ github.event.pull_request.base.sha }} head-ref: ${{ github.event.pull_request.head.sha }} fail-on-severity: moderate - # The dependency graph API can briefly report "no snapshots found for the - # head SHA" when a PR's dependency snapshot is submitted asynchronously and - # hasn't finished indexing yet. Retry instead of failing/warning on that race. + # Base and head snapshot submissions can overlap. In the exact-lock + # canary, the base snapshots became available after the former 300-second + # retry expired; wait for both sides before judging dependency changes. retry-on-snapshot-warnings: true - retry-on-snapshot-warnings-timeout: 300 + retry-on-snapshot-warnings-timeout: 600 # GHSA-w5hq-g745-h8pq: transitive uuid@8.3.2 pulled by next-auth@4.24 (which pins uuid ^8.3.2). # The moderate advisory affects uuid v3/v5/v6 when a caller-provided buffer is passed; next-auth # only calls v4(), so the vulnerable code path is unreachable here. The first patched uuid is @@ -354,7 +228,7 @@ jobs: HEAD_SHA: ${{ github.event.pull_request.head.sha }} REVIEW_OUTCOME: ${{ steps.review.outcome }} GITHUB_TOKEN: ${{ github.token }} - run: python3 scripts/ci/record_dependency_review.py + run: python3 -m scripts.ci.record_dependency_review - name: Upload dependency review evidence if: >- diff --git a/scripts/ci/record_dependency_review.py b/scripts/ci/record_dependency_review.py index 96b6b080a..d24131593 100644 --- a/scripts/ci/record_dependency_review.py +++ b/scripts/ci/record_dependency_review.py @@ -7,13 +7,22 @@ import os import re import subprocess +import tomllib from pathlib import Path from urllib.request import Request, urlopen +from scripts.ci.submit_uv_lock_snapshot import uv_manifest + def _changed_files(base: str, head: str) -> set[str]: + merge_base = subprocess.run( + ["git", "merge-base", base, head], + check=True, + capture_output=True, + text=True, + ).stdout.strip() output = subprocess.run( - ["git", "diff", "--name-only", "--no-renames", "-z", base, head], + ["git", "diff", "--name-only", "--no-renames", "-z", merge_base, head], check=True, capture_output=True, ).stdout @@ -29,24 +38,43 @@ def _npm_lock_components(sha: str, manifest: str) -> set[tuple[str, str, str]]: packages = lock.get("packages") if not isinstance(packages, dict): raise ValueError(f"Invalid npm lockfile in {sha}:{manifest}") - components = set() + components: dict[tuple[str, str], str] = {} for location, package in packages.items(): if not isinstance(location, str) or not isinstance(package, dict): raise ValueError(f"Invalid npm lockfile entry in {sha}:{manifest}") + if "node_modules/" not in location: + continue name = package.get("name") if not name and "node_modules/" in location: name = location.rsplit("node_modules/", 1)[1] version = package.get("version") if isinstance(name, str) and isinstance(version, str): scope = "development" if package.get("dev") is True else "runtime" - components.add((name, version, scope)) - return components + identity = (name, version) + if identity not in components or scope == "runtime": + components[identity] = scope + return {(name, version, scope) for (name, version), scope in components.items()} + + +def _uv_lock_components(sha: str, manifest: str) -> set[tuple[str, str, str]]: + result = subprocess.run(["git", "show", f"{sha}:{manifest}"], capture_output=True) + if result.returncode: + return set() + lock = tomllib.loads(result.stdout.decode()) + resolved = uv_manifest(lock, manifest)["resolved"] + return { + (key.rsplit("@", 1)[0], key.rsplit("@", 1)[1], package["scope"]) + for key, package in resolved.items() + } def _validate_changes( changes: list[dict], base: str, head: str, changed_files: set[str] ) -> None: npm_locks: dict[tuple[str, str], set[tuple[str, str, str]]] = {} + observed_npm: dict[str, dict[str, set[tuple[str, str, str]]]] = {} + uv_locks: dict[tuple[str, str], set[tuple[str, str, str]]] = {} + observed_uv: dict[str, dict[str, set[tuple[str, str, str]]]] = {} for change in changes: manifest = change.get("manifest") if not isinstance(manifest, str) or manifest not in changed_files: @@ -54,6 +82,18 @@ def _validate_changes( "Dependency graph includes a change in an unchanged manifest: " f"{manifest!r}" ) + if manifest == "uv.lock": + name, version, scope, kind = ( + change.get("name"), change.get("version"), + change.get("scope"), change.get("change_type"), + ) + if not all(isinstance(value, str) for value in (name, version, scope)): + raise ValueError("uv dependency change lacks name, version, or scope") + if kind not in {"added", "removed"}: + raise ValueError(f"Unexpected uv dependency change type: {kind!r}") + observed_uv.setdefault(manifest, {"added": set(), "removed": set()})[kind].add( + (name, version, scope) + ) if change.get("ecosystem") != "npm" or not manifest.endswith( "package-lock.json" ): @@ -73,13 +113,42 @@ def _validate_changes( on_head = component in npm_locks[(head, manifest)] kind = change.get("change_type") if kind == "added" and on_head and not on_base: + observed_npm.setdefault(manifest, {"added": set(), "removed": set()})[ + "added" + ].add(component) continue if kind == "removed" and on_base and not on_head: + observed_npm.setdefault(manifest, {"added": set(), "removed": set()})[ + "removed" + ].add(component) continue raise ValueError( "npm dependency change does not match lockfile version/scope: " f"{manifest} {name}@{version} {scope} {kind}" ) + for manifest in changed_files: + if not manifest.endswith("package-lock.json"): + continue + for sha in (base, head): + if (sha, manifest) not in npm_locks: + npm_locks[(sha, manifest)] = _npm_lock_components(sha, manifest) + expected = { + "added": npm_locks[(head, manifest)] - npm_locks[(base, manifest)], + "removed": npm_locks[(base, manifest)] - npm_locks[(head, manifest)], + } + if observed_npm.get(manifest, {"added": set(), "removed": set()}) != expected: + raise ValueError(f"Incomplete npm dependency diff for {manifest}") + for manifest in changed_files: + if manifest != "uv.lock": + continue + for sha in (base, head): + uv_locks[(sha, manifest)] = _uv_lock_components(sha, manifest) + expected = { + "added": uv_locks[(head, manifest)] - uv_locks[(base, manifest)], + "removed": uv_locks[(base, manifest)] - uv_locks[(head, manifest)], + } + if observed_uv.get(manifest, {"added": set(), "removed": set()}) != expected: + raise ValueError("uv dependency diff does not match lockfile versions and scopes") def record_dependency_review( @@ -122,13 +191,12 @@ def record_dependency_review( isinstance(item, dict) for item in changes ): raise ValueError("Dependency diff API did not return a complete change list") - if changes: - _validate_changes( - changes, - base, - head, - changed_files if changed_files is not None else _changed_files(base, head), - ) + _validate_changes( + changes, + base, + head, + changed_files if changed_files is not None else _changed_files(base, head), + ) destination.mkdir(parents=True, exist_ok=True) (destination / "dependency-diff.json").write_bytes(raw_diff) diff --git a/scripts/ci/submit_npm_lock_snapshot.py b/scripts/ci/submit_npm_lock_snapshot.py new file mode 100644 index 000000000..b6a01362d --- /dev/null +++ b/scripts/ci/submit_npm_lock_snapshot.py @@ -0,0 +1,180 @@ +"""Submit resolved npm and uv lockfiles in one dependency graph snapshot. + +Component Detection can omit a nested second version of one package. Use the +lockfile's installed paths as unique keys so both versions reach the review API. +The review action requests five snapshots per page; one combined lock submission +keeps the full-tree and lock submissions within that page on both sides. +""" + +from __future__ import annotations + +import datetime +import json +import os +import re +import subprocess +import tomllib +from pathlib import Path +from urllib.parse import quote +from urllib.request import Request, urlopen + +from scripts.ci.submit_uv_lock_snapshot import uv_manifest + +CORRELATOR = "0-eventrelay-complete-locks" + + +def _package_name(location: str, package: dict) -> str | None: + name = package.get("name") + if isinstance(name, str) and name: + return name + if "node_modules/" in location: + return location.rsplit("node_modules/", 1)[1] + return None + + +def _resolve_child(location: str, name: str, locations: set[str]) -> str | None: + cursor = location + while True: + candidate = ( + f"{cursor}/node_modules/{name}" if cursor else f"node_modules/{name}" + ) + if candidate in locations: + return candidate + if "/node_modules/" in cursor: + cursor = cursor.rsplit("/node_modules/", 1)[0] + elif cursor.startswith("node_modules/"): + cursor = "" + else: + cursor = "" + if not cursor: + candidate = f"node_modules/{name}" + return candidate if candidate in locations else None + + +def npm_manifest(lock: dict, location: str) -> dict: + packages = lock.get("packages") + if lock.get("lockfileVersion") not in {2, 3} or not isinstance(packages, dict): + raise ValueError(f"Unsupported npm lockfile: {location}") + direct_names: set[str] = set() + for package_location, package in packages.items(): + if not isinstance(package_location, str) or not isinstance(package, dict): + raise ValueError( + f"Invalid npm lockfile entry: {location}:{package_location}" + ) + if "node_modules/" in package_location: + continue + for field in ("dependencies", "devDependencies", "optionalDependencies"): + direct_names.update(package.get(field, {})) + + entries: dict[str, tuple[str, str, dict]] = {} + for package_location, package in packages.items(): + if "node_modules/" not in package_location: + continue + name = _package_name(package_location, package) + version = package.get("version") + if name and isinstance(version, str) and not package.get("link"): + entries[package_location] = (name, version, package) + + keys = {path: f"{name}@{version}" for path, (name, version, _) in entries.items()} + locations = set(entries) + resolved = {} + for path, (name, version, package) in entries.items(): + children = set() + for field in ("dependencies", "optionalDependencies"): + for child_name in package.get(field, {}): + child = _resolve_child(path, child_name, locations) + if child is not None: + children.add(keys[child]) + key = keys[path] + scope = "development" if package.get("dev") is True else "runtime" + if key not in resolved: + resolved[key] = { + "package_url": f"pkg:npm/{quote(name, safe='/')}@{quote(version, safe='')}", + "relationship": "direct" if name in direct_names else "indirect", + "scope": scope, + "dependencies": set(), + } + elif scope == "runtime": + # The same version may occur at both dev and runtime paths. Its + # effective graph scope must be stable and include runtime. + resolved[key]["scope"] = "runtime" + resolved[key]["dependencies"].update(children) + for package in resolved.values(): + package["dependencies"] = sorted(package["dependencies"]) + return { + "name": location, + "file": {"source_location": location}, + "resolved": resolved, + } + + +def tracked_npm_locks() -> list[Path]: + output = subprocess.run( + ["git", "ls-files", "-z", "*package-lock.json"], + check=True, + capture_output=True, + ).stdout + return [Path(os.fsdecode(path)) for path in output.split(b"\0") if path] + + +def submit_snapshot(environment: dict[str, str], opener=urlopen) -> dict: + sha, ref = environment["SNAPSHOT_SHA"], environment["SNAPSHOT_REF"] + if not re.fullmatch(r"[0-9a-f]{40}", sha) or not ref.startswith("refs/heads/"): + raise ValueError("Snapshot requires an exact SHA and branch ref") + repository = environment["GITHUB_REPOSITORY"] + if not re.fullmatch(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+", repository): + raise ValueError("Invalid GitHub repository") + manifests = {} + for path in tracked_npm_locks(): + manifests[path.as_posix()] = npm_manifest( + json.loads(path.read_text()), path.as_posix() + ) + uv_path = Path("uv.lock") + if uv_path.is_file(): + with uv_path.open("rb") as stream: + manifests[uv_path.as_posix()] = uv_manifest( + tomllib.load(stream), uv_path.as_posix() + ) + if not manifests: + return {"snapshot_id": None, "manifest_count": 0, "package_count": 0} + snapshot = { + "version": 0, + "sha": sha, + "ref": ref, + "job": { + "correlator": CORRELATOR, + "id": environment["GITHUB_RUN_ID"], + "html_url": f"https://github.com/{repository}/actions/runs/{environment['GITHUB_RUN_ID']}", + }, + "detector": { + "name": "EventRelay complete npm and scoped uv locks", + "version": "2", + "url": f"https://github.com/{repository}/blob/{sha}/scripts/ci/submit_npm_lock_snapshot.py", + }, + "scanned": datetime.datetime.now(datetime.UTC).isoformat(timespec="seconds"), + "manifests": manifests, + } + request = Request( + f"https://api.github.com/repos/{repository}/dependency-graph/snapshots", + data=json.dumps(snapshot, separators=(",", ":")).encode(), + headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {environment['GITHUB_TOKEN']}", + "Content-Type": "application/json", + "X-GitHub-Api-Version": "2022-11-28", + }, + method="POST", + ) + with opener(request, timeout=90) as response: + result = json.load(response) + if not isinstance(result, dict) or not result.get("id"): + raise ValueError("GitHub did not return a dependency snapshot ID") + return { + "snapshot_id": result["id"], + "manifest_count": len(manifests), + "package_count": sum(len(item["resolved"]) for item in manifests.values()), + } + + +if __name__ == "__main__": + print(json.dumps(submit_snapshot(os.environ), sort_keys=True)) diff --git a/scripts/ci/submit_uv_lock_snapshot.py b/scripts/ci/submit_uv_lock_snapshot.py new file mode 100644 index 000000000..d47a2f664 --- /dev/null +++ b/scripts/ci/submit_uv_lock_snapshot.py @@ -0,0 +1,145 @@ +"""Submit a uv.lock snapshot with scopes derived from the editable project root. + +Component Detection currently treats an editable uv root as an ordinary package, +so its uv detector marks every package as runtime. This submission gives GitHub +the lockfile's actual runtime and development reachability for that manifest. +""" + +from __future__ import annotations + +import datetime +import json +import os +import re +import tomllib +from collections import defaultdict, deque +from pathlib import Path +from urllib.parse import quote +from urllib.request import Request, urlopen + +DEVELOPMENT_EXTRAS = frozenset({"dev", "docs"}) +CORRELATOR = "0-eventrelay-uv-lock-scoped" + + +def _reachable(roots: list[dict], packages_by_name: dict[str, list[dict]]) -> set[str]: + visited: set[tuple[str, tuple[str, ...]]] = set() + reachable: set[str] = set() + pending = deque(roots) + while pending: + dependency = pending.popleft() + name = dependency["name"] + extras = tuple(sorted(dependency.get("extra", []))) + if (name, extras) in visited: + continue + visited.add((name, extras)) + reachable.add(name) + for package in packages_by_name.get(name, []): + pending.extend(package.get("dependencies", [])) + for extra in extras: + pending.extend(package.get("optional-dependencies", {}).get(extra, [])) + return reachable + + +def uv_manifest(lock: dict, location: str = "uv.lock") -> dict: + if lock.get("version") != 1 or not isinstance(lock.get("package"), list): + raise ValueError("Unsupported uv.lock format") + packages = lock["package"] + roots = [ + package for package in packages + if "editable" in package.get("source", {}) or "virtual" in package.get("source", {}) + ] + if len(roots) != 1: + raise ValueError(f"Expected one uv project root; found {len(roots)}") + root = roots[0] + packages_by_name: dict[str, list[dict]] = defaultdict(list) + for package in packages: + packages_by_name[package["name"]].append(package) + + runtime_roots = list(root.get("dependencies", [])) + development_roots: list[dict] = [] + for extra, dependencies in root.get("optional-dependencies", {}).items(): + target = development_roots if extra in DEVELOPMENT_EXTRAS else runtime_roots + target.extend(dependencies) + for group, dependencies in root.get("dev-dependencies", {}).items(): + target = development_roots if group in DEVELOPMENT_EXTRAS else runtime_roots + target.extend(dependencies) + + direct_runtime = {dependency["name"] for dependency in runtime_roots} + direct_development = {dependency["name"] for dependency in development_roots} + runtime = _reachable(runtime_roots, packages_by_name) + development = _reachable(development_roots, packages_by_name) + resolved = {} + for package in packages: + if "registry" not in package.get("source", {}): + continue + name, version = package["name"], package["version"] + key = f"{name}@{version}" + if key in resolved: + raise ValueError(f"Duplicate uv package: {key}") + dependency_keys = set() + for dependency in package.get("dependencies", []): + candidates = packages_by_name.get(dependency["name"], []) + if "version" in dependency: + candidates = [item for item in candidates if item["version"] == dependency["version"]] + if not candidates: + raise ValueError(f"Unresolved uv dependency: {name} -> {dependency['name']}") + dependency_keys.update( + f"{item['name']}@{item['version']}" + for item in candidates if "registry" in item.get("source", {}) + ) + entry = { + "package_url": f"pkg:pypi/{quote(name, safe='')}@{quote(version, safe='')}", + "relationship": "direct" if name in direct_runtime | direct_development else "indirect", + "scope": "development" if name in development and name not in runtime else "runtime", + "dependencies": sorted(dependency_keys), + } + resolved[key] = entry + return {"name": location, "file": {"source_location": location}, "resolved": resolved} + + +def submit_snapshot(lock_path: Path, environment: dict[str, str], opener=urlopen) -> dict: + sha, ref = environment["SNAPSHOT_SHA"], environment["SNAPSHOT_REF"] + if not re.fullmatch(r"[0-9a-f]{40}", sha) or not ref.startswith("refs/heads/"): + raise ValueError("Snapshot requires an exact SHA and branch ref") + repository = environment["GITHUB_REPOSITORY"] + if not re.fullmatch(r"[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+", repository): + raise ValueError("Invalid GitHub repository") + with lock_path.open("rb") as stream: + manifest = uv_manifest(tomllib.load(stream), lock_path.as_posix()) + snapshot = { + "version": 0, + "sha": sha, + "ref": ref, + "job": { + "correlator": CORRELATOR, + "id": environment["GITHUB_RUN_ID"], + "html_url": f"https://github.com/{repository}/actions/runs/{environment['GITHUB_RUN_ID']}", + }, + "detector": { + "name": "EventRelay uv lock scope", + "version": "1", + "url": f"https://github.com/{repository}/blob/{sha}/scripts/ci/submit_uv_lock_snapshot.py", + }, + "scanned": datetime.datetime.now(datetime.UTC).isoformat(timespec="seconds"), + "manifests": {lock_path.as_posix(): manifest}, + } + request = Request( + f"https://api.github.com/repos/{repository}/dependency-graph/snapshots", + data=json.dumps(snapshot, separators=(",", ":")).encode(), + headers={ + "Accept": "application/vnd.github+json", + "Authorization": f"Bearer {environment['GITHUB_TOKEN']}", + "Content-Type": "application/json", + "X-GitHub-Api-Version": "2022-11-28", + }, + method="POST", + ) + with opener(request, timeout=90) as response: + result = json.load(response) + if not isinstance(result, dict) or not result.get("id"): + raise ValueError("GitHub did not return a dependency snapshot ID") + return {"snapshot_id": result["id"], "package_count": len(manifest["resolved"])} + + +if __name__ == "__main__": + print(json.dumps(submit_snapshot(Path("uv.lock"), os.environ), sort_keys=True)) diff --git a/tests/unit/test_dependency_review_workflow.py b/tests/unit/test_dependency_review_workflow.py index 70376021f..14af291a7 100644 --- a/tests/unit/test_dependency_review_workflow.py +++ b/tests/unit/test_dependency_review_workflow.py @@ -61,48 +61,21 @@ def test_dependency_review_submits_pr_head_snapshot_and_retries_warnings() -> No "environment.yml", "vendor/modules.txt", "Dockerfile", - ".github/dependabot.yaml", ): 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 }}" - ) + assert '"git", "merge-base", base, head' in detection_script + assert '"git", "diff", "--name-only", "-z", merge_base, head' in detection_script 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["uses"] == ( + "advanced-security/component-detection-dependency-submission-action" + "@a02aeda7a175ad4dd38a13c4d3ff2df9da8f2cf0" ) assert submission["with"]["snapshot-sha"] == ( "${{ github.event_name == 'pull_request' && " @@ -112,19 +85,35 @@ 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 submission["env"] == {"PipReportFileLevelTimeoutSeconds": "240"} + assert 240 <= int(submission["env"]["PipReportFileLevelTimeoutSeconds"]) + assert int(submission["env"]["PipReportFileLevelTimeoutSeconds"]) < ( + job["timeout-minutes"] * 60 ) assert "detectorsFilter" not in submission["with"] + complete_npm = next( + step for step in steps if step["name"] == "Submit complete lock snapshot" + ) + assert steps.index(complete_npm) == steps.index(submission) + 1 + assert "github.event_name == 'push'" in complete_npm["if"] + assert ( + "github.event.pull_request.head.repo.full_name == github.repository" + in complete_npm["if"] + ) + assert "steps.dependency-changes.outputs.changed == 'true'" in complete_npm["if"] + assert complete_npm["run"] == "python3 -m scripts.ci.submit_npm_lock_snapshot" + assert complete_npm["env"]["SNAPSHOT_SHA"] == submission["with"]["snapshot-sha"] + assert complete_npm["env"]["SNAPSHOT_REF"] == submission["with"]["snapshot-ref"] + assert complete_npm["env"]["GITHUB_TOKEN"] == "${{ github.token }}" + review = next(step for step in steps if step["name"] == "Dependency Review") assert "github.event_name == 'pull_request'" in review["if"] assert "steps.dependency-changes.outputs.changed == 'true'" in review["if"] assert review["with"]["retry-on-snapshot-warnings"] is True - assert review["with"]["retry-on-snapshot-warnings-timeout"] == 300 + retry_seconds = review["with"]["retry-on-snapshot-warnings-timeout"] + assert retry_seconds >= 600 + assert job["timeout-minutes"] * 60 >= retry_seconds + 600 assert review["with"]["base-ref"] == "${{ github.event.pull_request.base.sha }}" assert review["with"]["head-ref"] == "${{ github.event.pull_request.head.sha }}" @@ -135,29 +124,25 @@ 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", "expected_count"), [ - (["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"], "false", ""), - (["src/app.py"], "false", ""), + (["pyproject.toml", "uv.lock"], "true", 2), + (["apps/web/package.json", "apps/web/package-lock.json"], "true", 2), + (["requirements.txt"], "true", 1), + (["uv.lock", "requirements.txt"], "true", 2), + ([".github/workflows/dependency-review.yml"], "false", 0), + ([".github/dependabot.yml"], "false", 0), + ([".github/dependabot.yaml"], "false", 0), + ([".github/dependabot.yml", "apps/web/package.json"], "true", 1), + (["src/app.py"], "false", 0), ], ) -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, expected_count: int ) -> None: repo = tmp_path / "repo" repo.mkdir() - all_paths = { + for name in { "pyproject.toml", "uv.lock", "apps/web/package.json", @@ -165,8 +150,9 @@ 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: + ".github/dependabot.yml", + ".github/dependabot.yaml", + }: path = repo / name path.parent.mkdir(parents=True, exist_ok=True) path.write_text("before\n") @@ -202,122 +188,75 @@ def git(*args: str) -> str: ) head = git("rev-parse", "HEAD") + steps = _load_workflow()["jobs"]["dependency-review"]["steps"] detection = next( - step - for step in _load_workflow()["jobs"]["dependency-review"]["steps"] - if step["name"] == "Detect dependency manifest changes" + step for step in 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), + "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["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" + assert values["count"] == str(expected_count) + 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"] @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"], ""), - ], + ("pr_path", "expected_changed"), + [("src/app.py", "false"), ("uv.lock", "true")], ) -def test_root_npm_lock_stages_workspace_roots_and_rejects_uncovered_manifests( - tmp_path: Path, changed_paths: list[str], expected_detector: str +def test_dependency_detection_ignores_base_only_update_on_stale_pr( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + pr_path: str, + expected_changed: 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(): + repo.mkdir() + for name in ("package-lock.json", "uv.lock", "src/app.py"): path = repo / name path.parent.mkdir(parents=True, exist_ok=True) - path.write_text(content) + 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("init", "-q", "-b", "main") git("add", ".") - git( + commit = ( "-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(*commit, "initial") + git("switch", "-q", "-c", "feature") + (repo / pr_path).write_text("pr change\n") git("add", ".") - git( - "-c", - "user.name=Workflow test", - "-c", - "user.email=workflow@example.com", - "commit", - "-qm", - "change", - ) + git(*commit, "pr") head = git("rev-parse", "HEAD") + git("switch", "-q", "main") + (repo / "package-lock.json").write_text("base update\n") + git("add", ".") + git(*commit, "base dependency update") + base = git("rev-parse", "HEAD") + assert "package-lock.json" in git("diff", "--name-only", base, head) + steps = _load_workflow()["jobs"]["dependency-review"]["steps"] detection = next( step for step in steps if step["name"] == "Detect dependency manifest changes" @@ -331,32 +270,19 @@ 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"] == ("1" if expected_changed == "true" else "0") + monkeypatch.chdir(repo) + recorder = runpy.run_path( + str(WORKFLOW_PATH.parents[2] / "scripts/ci/record_dependency_review.py") + ) + changed_files = recorder["_changed_files"](base, head) + assert changed_files == {pr_path} + if expected_changed == "true": + with pytest.raises(ValueError, match="unchanged manifest"): + recorder["_validate_changes"]( + [{"manifest": "package-lock.json"}], base, head, changed_files + ) def test_numpy_remains_subject_to_the_unchanged_license_policy() -> None: @@ -409,7 +335,7 @@ def test_dependency_review_evidence_preserves_large_complete_api_diff( 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["run"] == "python3 scripts/ci/record_dependency_review.py" + assert record["run"] == "python3 -m scripts.ci.record_dependency_review" 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/" @@ -527,7 +453,10 @@ class ApiResponse(io.BytesIO): destination = tmp_path / "dependency-review-evidence" saved = module["record_dependency_review"]( - environment, destination, opener=lambda request, timeout: ApiResponse(b"[]") + environment, + destination, + opener=lambda request, timeout: ApiResponse(b"[]"), + changed_files=set(), ) assert saved["dependency_diff_count"] == 0 assert (destination / "dependency-diff.json").read_bytes() == b"[]" @@ -544,13 +473,17 @@ def test_dependency_review_rejects_warning_free_graph_drift( ["git", "config", "user.email", "test@example.com"], cwd=repo, check=True ) - def commit_lock(version: str) -> str: + def commit_lock(version: str, nested_version: str) -> str: lock = { "lockfileVersion": 3, "packages": { "": {"name": "eventrelay", "version": "1.0.0"}, "node_modules/fast-uri": {"version": version, "dev": True}, "node_modules/hono": {"version": "4.13.7", "dev": True}, + "node_modules/filelist/node_modules/brace-expansion": { + "version": nested_version, + "dev": True, + }, }, } (repo / "package-lock.json").write_text(json.dumps(lock) + "\n") @@ -560,8 +493,8 @@ def commit_lock(version: str) -> str: ["git", "rev-parse", "HEAD"], cwd=repo, text=True ).strip() - base = commit_lock("3.1.7") - head = commit_lock("3.1.9") + base = commit_lock("3.1.7", "2.1.4") + head = commit_lock("3.1.9", "2.1.7") monkeypatch.chdir(repo) module = runpy.run_path( str(WORKFLOW_PATH.parents[2] / "scripts/ci/record_dependency_review.py") @@ -584,6 +517,12 @@ def commit_lock(version: str) -> str: "scope": "development", } removed = {**added, "change_type": "removed", "version": "3.1.7"} + nested_added = { + **added, + "name": "brace-expansion", + "version": "2.1.7", + } + nested_removed = {**nested_added, "change_type": "removed", "version": "2.1.4"} class ApiResponse(io.BytesIO): headers: dict[str, str] = {} @@ -599,12 +538,18 @@ def record(rows: list[dict], name: str) -> Path: return destination assert json.loads( - (record([added, removed], "valid") / "dependency-diff.json").read_text() + ( + record([added, removed, nested_added, nested_removed], "valid") + / "dependency-diff.json" + ).read_text() ) == [ added, removed, + nested_added, + nested_removed, ] invalid_cases = [ + ([added, removed], "Incomplete npm dependency diff"), ([added, {**removed, "manifest": "uv.lock"}], "unchanged manifest"), ([added, {**removed, "scope": "runtime"}], "lockfile version/scope"), ( diff --git a/tests/unit/test_submit_npm_lock_snapshot.py b/tests/unit/test_submit_npm_lock_snapshot.py new file mode 100644 index 000000000..c1b487e38 --- /dev/null +++ b/tests/unit/test_submit_npm_lock_snapshot.py @@ -0,0 +1,192 @@ +from __future__ import annotations + +import io +import json +import subprocess +from pathlib import Path + +import pytest + +from scripts.ci.submit_npm_lock_snapshot import ( + CORRELATOR, + npm_manifest, + submit_snapshot, + tracked_npm_locks, +) + + +def test_nested_npm_versions_survive_as_distinct_scoped_dependencies() -> None: + lock = { + "lockfileVersion": 3, + "packages": { + "": { + "dependencies": {"app": "1.0.0"}, + "devDependencies": {"filelist": "1.0.0"}, + }, + "node_modules/app": { + "version": "1.0.0", + "dependencies": {"brace-expansion": "1.1.21"}, + }, + "node_modules/brace-expansion": {"version": "1.1.21"}, + "node_modules/filelist": { + "version": "1.0.0", + "dev": True, + "dependencies": {"brace-expansion": "2.1.7"}, + }, + "node_modules/filelist/node_modules/brace-expansion": { + "version": "2.1.7", + "dev": True, + "license": "MIT", + }, + }, + } + manifest = npm_manifest(lock, "docs/fetch/package-lock.json") + rows = list(manifest["resolved"].values()) + braces = [row for row in rows if "brace-expansion" in row["package_url"]] + assert len(braces) == 2 + assert {row["package_url"] for row in braces} == { + "pkg:npm/brace-expansion@1.1.21", + "pkg:npm/brace-expansion@2.1.7", + } + assert {row["scope"] for row in braces} == {"runtime", "development"} + filelist = next( + row for row in rows if row["package_url"] == "pkg:npm/filelist@1.0.0" + ) + nested_key = next( + key + for key, row in manifest["resolved"].items() + if row["package_url"] == "pkg:npm/brace-expansion@2.1.7" + ) + assert nested_key in filelist["dependencies"] + assert manifest["file"]["source_location"] == "docs/fetch/package-lock.json" + + +def test_real_npm_locks_include_all_resolved_versions() -> None: + locks = tracked_npm_locks() + assert {path.as_posix() for path in locks} == { + "package-lock.json", + "sdk/typescript/package-lock.json", + "docs/knowledge_prototypes/mcp-servers/fetch-mcp/package-lock.json", + } + for path in locks: + lock = json.loads(Path(path).read_text()) + manifest = npm_manifest(lock, path.as_posix()) + expected = len( + { + ( + package.get("name") or location.rsplit("node_modules/", 1)[1], + package.get("version"), + ) + for location, package in lock["packages"].items() + if "node_modules/" in location + and package.get("version") + and not package.get("link") + } + ) + assert len(manifest["resolved"]) == expected + + +def test_duplicate_version_uses_runtime_scope_regardless_of_path_order() -> None: + paths = [ + ("node_modules/dev/node_modules/lru-cache", {"version": "11.5.3", "dev": True}), + ("node_modules/runtime/node_modules/lru-cache", {"version": "11.5.3"}), + ] + for ordered in (paths, list(reversed(paths))): + lock = {"lockfileVersion": 3, "packages": dict(ordered)} + rows = npm_manifest(lock, "package-lock.json")["resolved"] + assert list(rows) == ["lru-cache@11.5.3"] + assert rows["lru-cache@11.5.3"]["scope"] == "runtime" + + +def test_npm_manifest_rejects_unknown_lock_format() -> None: + with pytest.raises(ValueError, match="Unsupported npm lockfile"): + npm_manifest({"lockfileVersion": 1, "packages": {}}, "package-lock.json") + + +def test_snapshot_submits_all_tracked_locks_at_exact_commit( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + subprocess.run(["git", "init", "-q"], check=True) + lock = { + "lockfileVersion": 3, + "packages": {"node_modules/brace-expansion": {"version": "2.1.7", "dev": True}}, + } + for path in (Path("package-lock.json"), Path("sdk/typescript/package-lock.json")): + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(lock)) + subprocess.run(["git", "add", "."], check=True) + seen = [] + + def opener(request, timeout): + seen.append((request, timeout)) + return io.BytesIO(b'{"id": 12345}') + + result = submit_snapshot( + { + "SNAPSHOT_SHA": "a" * 40, + "SNAPSHOT_REF": "refs/heads/codex/test", + "GITHUB_REPOSITORY": "groupthinking/EventRelay", + "GITHUB_RUN_ID": "987", + "GITHUB_TOKEN": "test-token", + }, + opener=opener, + ) + assert result == {"snapshot_id": 12345, "manifest_count": 2, "package_count": 2} + request, timeout = seen[0] + snapshot = json.loads(request.data) + assert timeout == 90 + assert snapshot["sha"] == "a" * 40 + assert snapshot["ref"] == "refs/heads/codex/test" + assert snapshot["job"]["correlator"] == CORRELATOR + assert set(snapshot["manifests"]) == { + "package-lock.json", + "sdk/typescript/package-lock.json", + } + assert request.get_header("Authorization") == "Bearer test-token" + + +def test_snapshot_combines_npm_and_scoped_uv_in_one_submission( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.chdir(tmp_path) + subprocess.run(["git", "init", "-q"], check=True) + Path("package-lock.json").write_text( + json.dumps( + { + "lockfileVersion": 3, + "packages": {"node_modules/brace-expansion": {"version": "2.1.7"}}, + } + ) + ) + Path("uv.lock").write_text( + 'version = 1\n[[package]]\nname = "example"\nversion = "1.0"\n' + 'source = { editable = "." }\n[package.optional-dependencies]\n' + 'dev = [{ name = "black" }]\n[[package]]\nname = "black"\n' + 'version = "24.0"\nsource = { registry = "https://pypi.org/simple" }\n' + ) + subprocess.run(["git", "add", "."], check=True) + seen = [] + + def opener(request, timeout): + seen.append((request, timeout)) + return io.BytesIO(b'{"id": 12345}') + + result = submit_snapshot( + { + "SNAPSHOT_SHA": "a" * 40, + "SNAPSHOT_REF": "refs/heads/codex/test", + "GITHUB_REPOSITORY": "groupthinking/EventRelay", + "GITHUB_RUN_ID": "987", + "GITHUB_TOKEN": "test-token", + }, + opener=opener, + ) + assert len(seen) == 1 + assert result == {"snapshot_id": 12345, "manifest_count": 2, "package_count": 2} + snapshot = json.loads(seen[0][0].data) + assert set(snapshot["manifests"]) == {"package-lock.json", "uv.lock"} + assert ( + snapshot["manifests"]["uv.lock"]["resolved"]["black@24.0"]["scope"] + == "development" + ) diff --git a/tests/unit/test_submit_uv_lock_snapshot.py b/tests/unit/test_submit_uv_lock_snapshot.py new file mode 100644 index 000000000..9783f5fd3 --- /dev/null +++ b/tests/unit/test_submit_uv_lock_snapshot.py @@ -0,0 +1,177 @@ +from __future__ import annotations + +import io +import json +import runpy +import subprocess +import tomllib +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[2] +MODULE = runpy.run_path(str(ROOT / "scripts/ci/submit_uv_lock_snapshot.py")) + + +def _lock(version: str) -> dict: + return { + "version": 1, + "package": [ + { + "name": "example", "version": "1.0", "source": {"editable": "."}, + "dependencies": [{"name": "shared"}], + "optional-dependencies": { + "dev": [{"name": "black"}], + "docs": [{"name": "notebook"}], + "cloud": [{"name": "cloud-api"}], + }, + }, + {"name": "shared", "version": "1.0", "source": {"registry": "https://pypi.org/simple"}}, + { + "name": "black", "version": version, + "source": {"registry": "https://pypi.org/simple"}, + "dependencies": [{"name": "shared"}, {"name": "dev-helper"}], + }, + {"name": "notebook", "version": "2.0", "source": {"registry": "https://pypi.org/simple"}}, + {"name": "cloud-api", "version": "3.0", "source": {"registry": "https://pypi.org/simple"}}, + {"name": "dev-helper", "version": "4.0", "source": {"registry": "https://pypi.org/simple"}}, + ], + } + + +def test_uv_manifest_classifies_editable_root_and_transitive_packages() -> None: + resolved = MODULE["uv_manifest"](_lock("24.0"))["resolved"] + assert len(resolved) == 5 + assert resolved["black@24.0"]["scope"] == "development" + assert resolved["black@24.0"]["relationship"] == "direct" + assert resolved["black@24.0"]["dependencies"] == ["dev-helper@4.0", "shared@1.0"] + assert resolved["notebook@2.0"]["scope"] == "development" + assert resolved["dev-helper@4.0"]["scope"] == "development" + assert resolved["shared@1.0"]["scope"] == "runtime" + assert resolved["cloud-api@3.0"]["scope"] == "runtime" + + +def test_requested_runtime_extra_overrides_dev_reachability() -> None: + lock = _lock("24.0") + lock["package"][0]["dependencies"].append( + {"name": "uvicorn", "extra": ["standard"]} + ) + lock["package"][0]["optional-dependencies"]["dev"].append( + {"name": "watchfiles"} + ) + lock["package"].extend( + [ + { + "name": "uvicorn", "version": "1.0", + "source": {"registry": "https://pypi.org/simple"}, + "optional-dependencies": {"standard": [{"name": "watchfiles"}]}, + }, + { + "name": "watchfiles", "version": "2.0", + "source": {"registry": "https://pypi.org/simple"}, + }, + ] + ) + resolved = MODULE["uv_manifest"](lock)["resolved"] + assert resolved["watchfiles@2.0"]["scope"] == "runtime" + + +def test_current_uv_lock_keeps_dev_only_changes_out_of_runtime() -> None: + with (ROOT / "uv.lock").open("rb") as stream: + resolved = MODULE["uv_manifest"](tomllib.load(stream))["resolved"] + for name in ("black", "notebook", "pyjwt", "brotli", "virtualenv"): + assert next(value for key, value in resolved.items() if key.startswith(f"{name}@"))[ + "scope" + ] == "development" + assert next(value for key, value in resolved.items() if key.startswith("aiohttp@"))[ + "scope" + ] == "runtime" + + +def test_snapshot_submission_uses_exact_commit_and_scoped_manifest(tmp_path: Path) -> None: + # A compact TOML fixture keeps this test independent of any GitHub service. + lock_path = tmp_path / "uv.lock" + lock_path.write_text( + 'version = 1\n[[package]]\nname = "example"\nversion = "1.0"\n' + 'source = { editable = "." }\n[package.optional-dependencies]\n' + 'dev = [{ name = "black" }]\n[[package]]\nname = "black"\n' + 'version = "24.0"\nsource = { registry = "https://pypi.org/simple" }\n' + ) + captured = {} + + class Response(io.BytesIO): + def __enter__(self): + return self + + def __exit__(self, *args): + self.close() + + def opener(request, timeout): + captured["url"] = request.full_url + captured["body"] = json.loads(request.data) + captured["timeout"] = timeout + return Response(b'{"id": 1234}') + + sha = "a" * 40 + result = MODULE["submit_snapshot"]( + lock_path, + { + "SNAPSHOT_SHA": sha, + "SNAPSHOT_REF": "refs/heads/codex/example", + "GITHUB_REPOSITORY": "groupthinking/EventRelay", + "GITHUB_RUN_ID": "123", + "GITHUB_TOKEN": "dummy-test-token", + }, + opener=opener, + ) + assert result == {"snapshot_id": 1234, "package_count": 1} + assert captured["body"]["sha"] == sha + assert captured["body"]["ref"] == "refs/heads/codex/example" + assert captured["body"]["manifests"][lock_path.as_posix()]["resolved"]["black@24.0"][ + "scope" + ] == "development" + assert captured["url"].endswith("/dependency-graph/snapshots") + + +def test_recorder_rejects_runtime_label_for_dev_only_uv_package( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + repo = tmp_path / "repo" + repo.mkdir() + for command in ( + ["git", "init", "-q"], + ["git", "config", "user.name", "Test"], + ["git", "config", "user.email", "test@example.com"], + ): + subprocess.run(command, cwd=repo, check=True) + lock_path = repo / "uv.lock" + + def commit(version: str) -> str: + lock_path.write_text( + 'version = 1\n[[package]]\nname = "example"\nversion = "1.0"\n' + 'source = { editable = "." }\n[package.optional-dependencies]\n' + 'dev = [{ name = "black" }]\n[[package]]\nname = "black"\n' + f'version = "{version}"\n' + 'source = { registry = "https://pypi.org/simple" }\n' + ) + subprocess.run(["git", "add", "uv.lock"], cwd=repo, check=True) + subprocess.run(["git", "commit", "-qm", version], cwd=repo, check=True) + return subprocess.check_output( + ["git", "rev-parse", "HEAD"], cwd=repo, text=True + ).strip() + + base, head = commit("24.0"), commit("25.0") + recorder = runpy.run_path(str(ROOT / "scripts/ci/record_dependency_review.py")) + monkeypatch.chdir(repo) + changes = [ + {"manifest": "uv.lock", "ecosystem": "pip", "name": "black", "version": version, + "scope": scope, "change_type": kind} + for kind, version, scope in ( + ("removed", "24.0", "runtime"), ("added", "25.0", "runtime") + ) + ] + with pytest.raises(ValueError, match="uv dependency diff"): + recorder["_validate_changes"](changes, base, head, {"uv.lock"}) + for change in changes: + change["scope"] = "development" + recorder["_validate_changes"](changes, base, head, {"uv.lock"})