Repository navigation
feat(scripts): allow git repository build approvals - #1010
Conversation
📝 WalkthroughWalkthroughGit-backed packages now expose approval keys based on repository identity rather than resolved commits. Build policies classify and evaluate these keys, and lifecycle, ignored-build, graph-hash, and security-documentation paths use the repository-aware approval flow. ChangesGit repository build approvals
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LockedPackage
participant BuildPolicy
participant InstallLifecycle
LockedPackage->>BuildPolicy: derive repository URL approval key
InstallLifecycle->>BuildPolicy: evaluate package with source and repository keys
BuildPolicy->>InstallLifecycle: return allow, deny, or jail decision
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c6489be. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/aube/src/commands/install/materialize.rs (1)
151-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the duplicated
allowclosure into a shared helper.This closure is identical in logic to the one in
link.rs:269-276. Both extractsource_approval_key()+git_repository_approval_key()and calldecide_package_with_git_repositorywith the same parameter order. Divergence between the two would cause inconsistent build-approval decisions across the prewarm and link phases — a security-relevant risk.♻️ Proposed shared helper
/// Returns `true` if `pkg` is explicitly allowed to build under `policy`, /// considering both source and git-repository approval keys. fn is_package_build_allowed( pkg: &aube_lockfile::LockedPackage, policy: &aube_scripts::BuildPolicy, ) -> bool { let source_key = pkg.source_approval_key(); let git_repository_key = pkg.git_repository_approval_key(); matches!( policy.decide_package_with_git_repository( pkg.registry_name(), &pkg.version, source_key.as_deref(), git_repository_key.as_deref(), ), aube_scripts::AllowDecision::Allow ) }Then both call sites reduce to:
let allow = |pkg: &aube_lockfile::LockedPackage| { - let source_key = pkg.source_approval_key(); - let git_repository_key = pkg.git_repository_approval_key(); - matches!( - build_policy_for_hash.decide_package_with_git_repository( - pkg.registry_name(), - &pkg.version, - source_key.as_deref(), - git_repository_key.as_deref(), - ), - aube_scripts::AllowDecision::Allow - ) + is_package_build_allowed(pkg, &build_policy_for_hash) };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/aube/src/commands/install/materialize.rs` around lines 151 - 158, Extract the duplicated package-approval closure into a shared `is_package_build_allowed` helper, colocated in an appropriate common module. Have it accept `LockedPackage` and `BuildPolicy`, extract both approval keys, and call `decide_package_with_git_repository` with the existing parameter order; update the prewarm logic here and the corresponding closure in `link.rs` to use it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/aube/src/commands/ignored_builds.rs`:
- Around line 161-168: In `ignored-builds` processing, `seen` is updated before
the source-specific decision, allowing an approved repository to suppress an
unapproved one with the same package coordinates. Evaluate
`policy.decide_package_with_git_repository` first, then perform the `(name,
version)` deduplication and update `seen` only after the policy check identifies
the entry for reporting.
---
Nitpick comments:
In `@crates/aube/src/commands/install/materialize.rs`:
- Around line 151-158: Extract the duplicated package-approval closure into a
shared `is_package_build_allowed` helper, colocated in an appropriate common
module. Have it accept `LockedPackage` and `BuildPolicy`, extract both approval
keys, and call `decide_package_with_git_repository` with the existing parameter
order; update the prewarm logic here and the corresponding closure in `link.rs`
to use it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ed385ca-4297-4de3-946a-e86877c3ac16
📒 Files selected for processing (7)
crates/aube-lockfile/src/lib.rscrates/aube-scripts/src/policy.rscrates/aube/src/commands/ignored_builds.rscrates/aube/src/commands/install/lifecycle.rscrates/aube/src/commands/install/link.rscrates/aube/src/commands/install/materialize.rsdocs/security.md
Greptile SummaryThis PR introduces repository-scoped
Confidence Score: 5/5Safe to merge — the implementation is complete, internally consistent, and well-tested across all affected code paths. All six affected callsites correctly thread both keys into the new decision API. The deny-wins invariant is maintained at every check level: bare-name, wildcard, source-key, git-repository, and versioned-name denies are all evaluated before any allow is returned. The deduplication ordering fix in ignored_builds.rs is a genuine correctness improvement with no observable regression. Comprehensive unit tests cover the new code paths including native-git transport, cross-repo isolation, and the interaction between repository allows and package-name denies. No files require special attention. Important Files Changed
Reviews (2): Last reviewed commit: "fix(scripts): normalize git repository a..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Summary
allowBuildsrules keyed by a package and normalized Git repository URLTests
cargo fmt --checkcargo test -p aube -p aube-scripts -p aube-lockfilecargo clippy -p aube -p aube-scripts -p aube-lockfile --all-targets -- -D warningsThis pull request was generated by Codex.
Note
Medium Risk
Changes dependency lifecycle allow/deny logic for Git sources; scope is limited to explicit repo keys with denies still overriding allows.
Overview
Adds repository-scoped
allowBuildsrules for Git dependencies, e.g.native-addon@git+https://github.com/acme/native-addon.git, so an audited repo can move to new commits without re-approving each resolved SHA. Keys omit the commit (LockedPackage::git_repository_approval_key); exact per-source keys and name-only registry approvals stay unchanged.BuildPolicyparses these into separate allow/deny sets, evaluates them indecide_package_with_git_repository(repo deny/allow with existing deny-wins precedence), and documents the new key shape. Install paths sharepackage_build_is_allowedfor lifecycle runs, script jailing, ignored-build reporting, and graph-hash “build allowed” checks. Security docs describe the YAML pattern.Reviewed by Cursor Bugbot for commit c40a0ae. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
git+prefix correctly.allowBuildsguidance with examples for approving package and repository URL keys (not commits).