ci: add automated dependency review workflow#2980
ci: add automated dependency review workflow#2980jstarks wants to merge 3 commits intomicrosoft:mainfrom
Conversation
Replace the CODEOWNERS gate on Cargo.lock with a smarter GitHub Actions workflow that only requests dependency reviewer approval when needed. Internal-only Cargo.lock changes (adding/removing workspace crates) no longer require dependency team review. The workflow detects: 1. New or version-bumped external (3rd-party) crates 2. Dependency containment policy violations (e.g., support/ crates depending on crates outside support/) When issues are found, review is requested from @microsoft/openvmm-dependency-reviewers. When resolved, the request is removed. The workflow uses pull_request_target for review-request permissions but only checks out the base branch and fetches PR files via the GitHub API, so no untrusted code is executed. Concurrent runs on the same PR are coalesced via a concurrency group. Includes: - .github/scripts/dep-review.js: core logic with local --check mode - .github/scripts/dep-review.test.js: 27 tests using node:test - .github/workflows/dep-review.yml: GitHub Actions workflow - .github/dep-policy.json: containment rules - .github/CODEOWNERS: remove Cargo.lock gate, protect dep-review files
There was a problem hiding this comment.
Pull request overview
This PR replaces the broad CODEOWNERS gate on Cargo.lock with a pull_request_target GitHub Actions workflow that requests dependency-team review only when external (3rd-party) dependencies change or when internal dependency-containment rules are violated.
Changes:
- Add
dep-reviewworkflow to detect external dependency additions/version bumps and containment violations, requesting/removing dependency-team review accordingly. - Add a Node-based dependency review script with local
--checkmode plus unit tests. - Introduce a JSON containment policy and update CODEOWNERS to remove the
Cargo.lockgate while protecting the new dep-review infrastructure.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/dep-review.yml | New workflow that runs dep-review logic on pull_request_target and manages review requests. |
| .github/scripts/dep-review.js | Core logic: parses Cargo.lock, diffs external deps, checks containment policy, calls GitHub APIs to request/remove review. |
| .github/scripts/dep-review.test.js | Node unit tests for parsing/diffing and containment logic. |
| .github/dep-policy.json | Defines containment rules (currently support/ prefix rule). |
| .github/CODEOWNERS | Removes Cargo.lock ownership gate and adds ownership for dep-review infrastructure. |
You can also share your feedback on Copilot code review. Take the survey.
- CODEOWNERS: add openvmm-ci-reviewers alongside dependency reviewers so dep-review file changes still require CI team approval - parseExternalDeps: use Set keyed by name+version+source to correctly track multiple versions of the same crate (e.g., windows-sys) - diffDeps: simplify to only report added entries (version bumps are naturally new tuples) - run(): remove stale review request when Cargo.lock is no longer modified - tests: update for new Set-based API, add multi-version crate test
There was a problem hiding this comment.
Pull request overview
Adds a targeted dependency review mechanism to replace the coarse CODEOWNERS gate on Cargo.lock, so dependency team review is only requested when external crates change or when internal dependency-containment rules are violated.
Changes:
- Introduces a
pull_request_targetworkflow that conditionally requests/removes dependency team review based onCargo.lockanalysis. - Adds a Node.js script (plus unit tests) to detect external dependency changes and containment-policy violations.
- Updates CODEOWNERS to remove the
Cargo.lockgate and to protect the new dependency-review infrastructure.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/dep-review.yml | New workflow that runs dependency review logic and manages team review requests. |
| .github/scripts/dep-review.js | Implements lockfile parsing, diffing, containment checks, and reviewer request/removal. |
| .github/scripts/dep-review.test.js | Adds unit/integration tests for the dependency review logic using node:test. |
| .github/dep-policy.json | Defines containment rules used by the workflow/script. |
| .github/CODEOWNERS | Removes Cargo.lock ownership gate; adds ownership for dep-review infra files. |
- Fetch PR files via refs/pull/N/head from the base repo instead of accessing the fork directly, which fails for private forks. - Warn and assume Cargo.lock changed when the file list pagination cap (3000 files) is exceeded, instead of silently missing it. - Clarify --help text that --manifest and --policy must be specified together for containment checks.
|
I am really not a fan of encoding significant logic directly into the js like this. Is there no way we can move this to be an xtask or similar? Like we already have Cargo.toml parsing readily available in Rust. |
| @@ -0,0 +1,8 @@ | |||
| { | |||
| "containment": [ | |||
There was a problem hiding this comment.
I wonder if we could have some sort of tree-like model of dependencies. Like crates in support can only depend on external deps or other support crates, but then crates in say vm/ can only depend on external deps or support/vm crates, and then openvmm crates can only depend on those (so nothing in openhcl/), etc.
There was a problem hiding this comment.
Yes, I do think this makes sense. But I figured it would be easier to do that as an incremental change.
That will make the job a lot heavier. We'll have to install Rust, build xtask. |
|
It also makes the job a lot more portable though, and easier for others to review, runnable locally as part of a pre-commit check, etc |
Replace the CODEOWNERS gate on Cargo.lock with a smarter GitHub Actions workflow that only requests dependency reviewer approval when needed.
Internal-only Cargo.lock changes (adding/removing workspace crates) no longer require dependency team review. The workflow detects:
When issues are found, review is requested from
@microsoft/openvmm-dependency-reviewers. When resolved, the request is removed.
The workflow uses pull_request_target for review-request permissions but only checks out the base branch and fetches PR files via the GitHub API, so no untrusted code is executed. Concurrent runs on the same PR are coalesced via a concurrency group.
Includes: