Skip to content

feat(scripts): allow git repository build approvals #1010

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jdx merged 2 commits into from
Jul 13, 2026
Merged

feat(scripts): allow git repository build approvals #1010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6489be
feat(scripts): allow git repository build approvals
jdx Jul 9, 2026
c40a0ae
fix(scripts): normalize git repository approvals
jdx Jul 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions crates/aube-lockfile/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,22 @@ impl LockedPackage {
.map(|source| format!("{}@{}", self.registry_name(), source.specifier()))
}

/// Repository-wide approval key for a Git-backed package source.
///
/// Unlike [`Self::source_approval_key`], this deliberately omits the
/// resolved commit. It is only used for an explicit `allowBuilds`
/// `git+<repository>` rule, never for package-name approval.
pub fn git_repository_approval_key(&self) -> Option<String> {
let LocalSource::Git(git) = self.local_source.as_ref()? else {
return None;
};
Some(format!(
"{}@git+{}",
self.registry_name(),
git.url.strip_prefix("git+").unwrap_or(&git.url)
))
}

/// Declared peer ranges with pnpm's meta-only peers folded in as `*`.
///
/// pnpm records a `peerDependencies: { x: '*' }` entry for every
Expand Down Expand Up @@ -588,6 +604,40 @@ mod locked_package_tests {
Some("pkg@https://example.com/pkg.tgz".to_string())
);
}

#[test]
fn git_repository_approval_key_omits_resolved_commit() {
let mut pkg = pkg();
pkg.local_source = Some(LocalSource::Git(GitSource {
url: "https://github.com/acme/pkg.git".to_string(),
committish: Some("main".to_string()),
resolved: "0123456789012345678901234567890123456789".to_string(),
integrity: None,
subpath: None,
}));

assert_eq!(
pkg.git_repository_approval_key(),
Some("pkg@git+https://github.com/acme/pkg.git".to_string())
);
}

#[test]
fn git_repository_approval_key_normalizes_an_existing_git_prefix() {
let mut pkg = pkg();
pkg.local_source = Some(LocalSource::Git(GitSource {
url: "git+ssh://git@github.com/acme/pkg.git".to_string(),
committish: None,
resolved: "0123456789012345678901234567890123456789".to_string(),
integrity: None,
subpath: None,
}));

assert_eq!(
pkg.git_repository_approval_key(),
Some("pkg@git+ssh://git@github.com/acme/pkg.git".to_string())
);
}
}

/// Metadata about a single declared peer dependency. Matches the shape of
Expand Down
143 changes: 142 additions & 1 deletion crates/aube-scripts/src/policy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
//! - `"esbuild"` — bare name, matches every version of the package
//! - `"esbuild@0.19.0"` — exact version match
//! - `"esbuild@0.19.0 || 0.20.0"` — exact version union
//! - `"esbuild@git+https://github.com/acme/esbuild.git"` — every commit
//! from one Git repository
//!
//! Semver ranges are intentionally *not* supported, matching pnpm's
//! `expandPackageVersionSpecs` behavior: if you pin a version in the
Expand Down Expand Up @@ -56,6 +58,12 @@ pub struct BuildPolicy {
/// entries so a typo like `pkg@latest` still warns.
allowed_sources: HashSet<String>,
denied_sources: HashSet<String>,
/// Git repository approval keys such as
/// `pkg@git+https://github.com/acme/pkg.git`. These intentionally omit
/// the resolved commit so an audited branch may advance without a new
/// approval, while never approving a different repository by name alone.
allowed_git_repositories: HashSet<String>,
denied_git_repositories: HashSet<String>,
/// Bare-name patterns containing `*` wildcards. Checked with a
/// linear scan after the exact-match sets; wildcard rules are rare
/// enough that the linear pass is cheaper than building an
Expand Down Expand Up @@ -101,6 +109,8 @@ impl BuildPolicy {
let mut denied = HashSet::new();
let mut allowed_sources = HashSet::new();
let mut denied_sources = HashSet::new();
let mut allowed_git_repositories = HashSet::new();
let mut denied_git_repositories = HashSet::new();
let mut allowed_wildcards = Vec::new();
let mut denied_wildcards = Vec::new();
let mut warnings = Vec::new();
Expand Down Expand Up @@ -139,7 +149,12 @@ impl BuildPolicy {
} else {
&mut denied_sources
};
sort_entries(expanded, exact, source, wild);
let git_repositories = if bool_value {
&mut allowed_git_repositories
} else {
&mut denied_git_repositories
};
sort_entries(expanded, exact, source, git_repositories, wild);
}
Err(e) => warnings.push(e),
}
Expand All @@ -156,6 +171,7 @@ impl BuildPolicy {
expanded,
&mut allowed,
&mut allowed_sources,
&mut allowed_git_repositories,
&mut allowed_wildcards,
),
Err(e) => warnings.push(e),
Expand All @@ -167,6 +183,7 @@ impl BuildPolicy {
expanded,
&mut denied,
&mut denied_sources,
&mut denied_git_repositories,
&mut denied_wildcards,
),
Err(e) => warnings.push(e),
Expand All @@ -180,6 +197,8 @@ impl BuildPolicy {
denied,
allowed_sources,
denied_sources,
allowed_git_repositories,
denied_git_repositories,
allowed_wildcards,
denied_wildcards,
},
Expand All @@ -191,6 +210,7 @@ impl BuildPolicy {
pub fn denylist(denied_patterns: &[String]) -> (Self, Vec<BuildPolicyError>) {
let mut denied = HashSet::new();
let mut denied_sources = HashSet::new();
let mut denied_git_repositories = HashSet::new();
let mut denied_wildcards = Vec::new();
let mut warnings = Vec::new();
for pattern in denied_patterns {
Expand All @@ -199,6 +219,7 @@ impl BuildPolicy {
expanded,
&mut denied,
&mut denied_sources,
&mut denied_git_repositories,
&mut denied_wildcards,
),
Err(e) => warnings.push(e),
Expand All @@ -211,6 +232,8 @@ impl BuildPolicy {
denied,
allowed_sources: HashSet::new(),
denied_sources,
allowed_git_repositories: HashSet::new(),
denied_git_repositories,
allowed_wildcards: Vec::new(),
denied_wildcards,
},
Expand All @@ -236,6 +259,18 @@ impl BuildPolicy {
name: &str,
version: &str,
source_key: Option<&str>,
) -> AllowDecision {
self.decide_package_with_git_repository(name, version, source_key, None)
}

/// Decide whether a package may run lifecycle scripts, accepting an
/// optional repository identity for Git-backed packages.
pub fn decide_package_with_git_repository(
&self,
name: &str,
version: &str,
source_key: Option<&str>,
git_repository_key: Option<&str>,
) -> AllowDecision {
// Reusable thread-local buffer for the `name@version` probe key.
// Avoids a `format!` allocation on every call — ~2k throwaway
Expand All @@ -254,6 +289,11 @@ impl BuildPolicy {
{
return AllowDecision::Deny;
}
if let Some(git_repository_key) = git_repository_key
&& self.denied_git_repositories.contains(git_repository_key)
{
return AllowDecision::Deny;
}
// Build the `name@version` probe key once and answer both the
// deny and the allow lookups from a single buffer borrow.
let (denied_versioned, allowed_versioned) = KEY_BUF.with(|buf| {
Expand All @@ -270,6 +310,11 @@ impl BuildPolicy {
if self.allow_all {
return AllowDecision::Allow;
}
if let Some(git_repository_key) = git_repository_key
&& self.allowed_git_repositories.contains(git_repository_key)
{
return AllowDecision::Allow;
}
if let Some(source_key) = source_key {
return if self.allowed_sources.contains(source_key) {
AllowDecision::Allow
Expand All @@ -293,6 +338,7 @@ impl BuildPolicy {
self.allow_all
|| !self.allowed.is_empty()
|| !self.allowed_sources.is_empty()
|| !self.allowed_git_repositories.is_empty()
|| !self.allowed_wildcards.is_empty()
}

Expand All @@ -306,6 +352,10 @@ impl BuildPolicy {
.extend(other.allowed_sources.iter().cloned());
self.denied_sources
.extend(other.denied_sources.iter().cloned());
self.allowed_git_repositories
.extend(other.allowed_git_repositories.iter().cloned());
self.denied_git_repositories
.extend(other.denied_git_repositories.iter().cloned());
merge_unique(&mut self.allowed_wildcards, &other.allowed_wildcards);
merge_unique(&mut self.denied_wildcards, &other.denied_wildcards);
}
Expand Down Expand Up @@ -342,13 +392,16 @@ fn sort_entries(
entries: Vec<String>,
exact: &mut HashSet<String>,
sources: &mut HashSet<String>,
git_repositories: &mut HashSet<String>,
wildcards: &mut Vec<String>,
) {
for entry in entries {
if entry.contains('*') {
if !wildcards.iter().any(|p| p == &entry) {
wildcards.push(entry);
}
} else if is_git_repository_key(&entry) {
git_repositories.insert(entry);
} else if is_source_key(&entry) {
sources.insert(entry);
} else {
Expand Down Expand Up @@ -454,6 +507,20 @@ fn is_source_key(key: &str) -> bool {
is_source_version(version)
}

fn is_git_repository_key(key: &str) -> bool {
let (_, source) = split_name_and_versions(key);
!source.contains('#')
&& [
"git+https://",
"git+http://",
"git+ssh://",
"git+file://",
"git+git://",
]
.iter()
.any(|prefix| source.starts_with(prefix))
}
Comment thread
jdx marked this conversation as resolved.
Resolved

fn is_source_version(version: &str) -> bool {
[
"file+",
Expand Down Expand Up @@ -583,6 +650,80 @@ mod tests {
);
}

#[test]
fn git_repository_rule_allows_every_resolved_commit() {
let p = policy(&[("gitdep@git+https://github.com/acme/gitdep.git", true)]);

for source_key in [
"gitdep@https://github.com/acme/gitdep.git#0123456789012345678901234567890123456789",
"gitdep@https://github.com/acme/gitdep.git#abcdefabcdefabcdefabcdefabcdefabcdefabcd",
] {
assert_eq!(
p.decide_package_with_git_repository(
"gitdep",
"1.0.0",
Some(source_key),
Some("gitdep@git+https://github.com/acme/gitdep.git"),
),
AllowDecision::Allow
);
}

assert_eq!(
p.decide_package_with_git_repository(
"gitdep",
"1.0.0",
Some("gitdep@https://github.com/acme/other.git#0123456789012345678901234567890123456789"),
Some("gitdep@git+https://github.com/acme/other.git"),
),
AllowDecision::Unspecified
);
}

#[test]
fn git_repository_rule_accepts_native_git_transport() {
let p = policy(&[("gitdep@git+git://github.com/acme/gitdep.git", true)]);

assert_eq!(
p.decide_package_with_git_repository(
"gitdep",
"1.0.0",
Some("gitdep@git://github.com/acme/gitdep.git#0123456789012345678901234567890123456789"),
Some("gitdep@git+git://github.com/acme/gitdep.git"),
),
AllowDecision::Allow
);
}

#[test]
fn git_repository_deny_and_package_deny_override_repository_allow() {
let p = policy(&[
("gitdep@git+https://github.com/acme/gitdep.git", true),
("other@git+https://github.com/acme/other.git", false),
("blocked", false),
("blocked@git+https://github.com/acme/blocked.git", true),
]);

assert_eq!(
p.decide_package_with_git_repository(
"other",
"1.0.0",
Some("other@https://github.com/acme/other.git#0123456789012345678901234567890123456789"),
Some("other@git+https://github.com/acme/other.git"),
),
AllowDecision::Deny
);
assert_eq!(
p.decide_package_with_git_repository(
"blocked",
"1.0.0",
Some("blocked@https://github.com/acme/blocked.git#0123456789012345678901234567890123456789"),
Some("blocked@git+https://github.com/acme/blocked.git"),
),
AllowDecision::Deny
);
}

#[test]
fn source_backed_package_name_deny_still_wins() {
let p = policy(&[("esbuild", false), ("esbuild@file+abc123", true)]);
Expand Down
12 changes: 4 additions & 8 deletions crates/aube/src/commands/ignored_builds.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,17 +151,13 @@ pub(super) fn collect_ignored(project_dir: &std::path::Path) -> miette::Result<V
let mut out: Vec<IgnoredEntry> = Vec::new();

for pkg in graph.packages.values() {
if !seen.insert((pkg.name.clone(), pkg.version.clone())) {
continue;
}
// Match on registry_name, not pkg.name. Allowlist pins the
// real pkg name. npm: alias would sneak past otherwise. Same
// fix as every other policy.decide callsite.
let source_key = pkg.source_approval_key();
if matches!(
policy.decide_package(pkg.registry_name(), &pkg.version, source_key.as_deref()),
aube_scripts::AllowDecision::Allow
) {
if super::install::package_build_is_allowed(&policy, pkg) {
continue;
}
if !seen.insert((pkg.name.clone(), pkg.version.clone())) {
continue;
}
let Some(suspicions) = lifecycle_scripts_with_suspicions(
Expand Down
Loading
Loading