Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 23 additions & 5 deletions crates/buzz-core/src/git_perms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ pub const MAX_WILDCARDS_PER_PATTERN: usize = 3;
/// A validated ref pattern for matching git refs.
///
/// Grammar: `segment ("/" segment)*` where segment is either a literal
/// `[a-zA-Z0-9._-]+` or `*` (matches exactly one path segment).
/// `[a-zA-Z0-9._+@-]+` or `*` (matches exactly one path segment). The literal
/// alphabet matches `is_safe_refname` in the relay's git manifest layer, so
/// every ref the relay accepts can also be named by a protection rule.
///
/// Patterns MUST start with `refs/`. No `**`, `?`, `[...]`, or partial globs.
#[derive(Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -147,10 +149,14 @@ impl RefPattern {
{
// Partial globs (e.g., "v*") are not allowed.
return Err(PatternError::InvalidSegment(part.to_string()));
} else if !part
.chars()
.all(|c| c.is_ascii_alphanumeric() || c == '.' || c == '_' || c == '-')
{
} else if !part.chars().all(|c| {
c.is_ascii_alphanumeric()
|| c == '.'
|| c == '_'
|| c == '-'
|| c == '+'
|| c == '@'
}) {
return Err(PatternError::InvalidSegment(part.to_string()));
} else {
segments.push(PatternSegment::Literal(part.to_string()));
Expand Down Expand Up @@ -650,6 +656,18 @@ mod tests {
assert!(!p.matches("refs/heads/release/v1/hotfix"));
}

#[test]
fn pattern_literal_accepts_git_legal_plus_and_at() {
// Refs containing `+`/`@` are pushable (#4194), so literal protection
// rules must be able to name them.
let p = RefPattern::parse("refs/heads/test/842+841-devnet").unwrap();
assert!(p.matches("refs/heads/test/842+841-devnet"));
assert!(!p.matches("refs/heads/test/842-841-devnet"));

let p = RefPattern::parse("refs/heads/dependabot/npm_and_yarn/@types/node-1.2.3").unwrap();
assert!(p.matches("refs/heads/dependabot/npm_and_yarn/@types/node-1.2.3"));
}

#[test]
fn pattern_rejects_partial_glob() {
assert!(matches!(
Expand Down
3 changes: 3 additions & 0 deletions crates/buzz-relay/src/api/git/hydrate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,9 @@ mod tests {
assert!(is_safe_refname("refs/heads/main"));
assert!(is_safe_refname("refs/tags/v1.0.0"));
assert!(is_safe_refname("refs/heads/feat/cas-publish"));
// Git-legal `+`/`@` refs hydrate as ordinary loose-ref paths (#4194).
assert!(is_safe_refname("refs/heads/test/842+841-devnet"));
assert!(is_safe_refname("refs/heads/user@host"));
assert!(!is_safe_refname("refs/heads/../escape"));
assert!(!is_safe_refname("HEAD"));
assert!(!is_safe_refname("refs/heads/"));
Expand Down
34 changes: 29 additions & 5 deletions crates/buzz-relay/src/api/git/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,24 @@ pub enum ManifestError {
MalformedPackKey(String),
}

/// Conservative refname validation, used symmetrically on both the write side
/// (in `Manifest::validate`, before `put_manifest`) and the read side (in
/// `api::git::hydrate`, before writing the ref to disk).
/// Conservative refname validation, used symmetrically on the write side
/// (in `Manifest::validate`, before `put_manifest`), the read side (in
/// `api::git::hydrate`, before writing the ref to disk), and the `info/refs`
/// fast-path eligibility gate (`api::git::transport`, before emitting refnames
/// into pkt-line bytes).
///
/// Refuses traversal (`..`), null/newline/control chars, non-`refs/` prefixes,
/// and leading/trailing/double slashes. Allowed alphabet:
/// `[a-zA-Z0-9_./-]`.
/// `[a-zA-Z0-9_./+@-]`.
///
/// `+` and `@` are git-legal (`git check-ref-format` accepts them anywhere in
/// a component) and inert here: refnames are stored inside the manifest JSON
/// body and written as loose-ref file paths — never used as object-store key
/// components — and both characters are valid filename bytes on Unix and
/// Windows. The two `@` forms git does forbid stay unreachable: a refname that
/// is *entirely* the single character `@` cannot occur behind the mandatory
/// `refs/` prefix, and the `@{` sequence requires `{`, which stays outside the
/// alphabet.
///
/// Sharing one predicate is load-bearing: any divergence creates the
/// "valid CAS, un-clone-able output" hazard.
Expand All @@ -147,7 +158,7 @@ pub fn is_safe_refname(s: &str) -> bool {
return false;
}
s.chars()
.all(|c| c.is_ascii_alphanumeric() || matches!(c, '/' | '_' | '.' | '-'))
.all(|c| c.is_ascii_alphanumeric() || matches!(c, '/' | '_' | '.' | '-' | '+' | '@'))
}

/// Hex-OID predicate. Accepts both SHA-1 (40 chars) and SHA-256 (64 chars) —
Expand Down Expand Up @@ -336,6 +347,19 @@ mod tests {
assert!(is_safe_refname("refs/heads/main"));
assert!(is_safe_refname("refs/tags/v1.0.0"));
assert!(is_safe_refname("refs/heads/feat/cas-publish"));
// Git-legal `+` and `@` (#4194): real-world refs that must round-trip.
assert!(is_safe_refname("refs/heads/test/842+841-devnet"));
assert!(is_safe_refname(
"refs/heads/dependabot/npm_and_yarn/@types/node-1.2.3"
));
assert!(is_safe_refname("refs/tags/v1.0.0+build.5"));
assert!(is_safe_refname("refs/heads/user@host"));
// A component that is only `@` is git-legal too (git forbids only the
// whole-refname `@`, unreachable behind the `refs/` prefix).
assert!(is_safe_refname("refs/heads/@"));
assert!(is_safe_refname("refs/@/x"));
// `@{` stays impossible: `{` is outside the alphabet.
assert!(!is_safe_refname("refs/heads/a@{0}"));
assert!(!is_safe_refname("refs/heads/../escape"));
assert!(!is_safe_refname("HEAD"));
assert!(!is_safe_refname(""));
Expand Down
47 changes: 38 additions & 9 deletions crates/buzz-relay/src/api/git/manifest_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ pub enum BuildError {
/// state announcements" semantics).
/// - OIDs validated as 40-hex (SHA-1) or 64-hex (SHA-256). Invalid OIDs are
/// skipped, not failed — same conservative behavior as the legacy code.
/// - Ref names validated (no `//`, no leading `/`, alphanumeric + `/_.-`).
/// - Ref names validated by the shared `is_safe_refname` predicate (the same
/// one the write path accepted the manifest with), so a pushable ref can
/// never be silently dropped from the ref-state event by alphabet drift.
/// - Output tag ordering is deterministic for testability: `d`, refs (sorted
/// by `BTreeMap` iteration), HEAD, p.
pub fn build_ref_state_event(
Expand Down Expand Up @@ -114,15 +116,16 @@ pub fn build_ref_state_event(
}

/// NIP-34 kind:30618 only emits refs under heads/ and tags/.
///
/// Character/structure rules delegate to [`super::manifest::is_safe_refname`]
/// — the predicate every ref in a committed manifest has already passed — so
/// this filter can only narrow by *namespace*, never by alphabet. A private
/// re-spelling of the alphabet here is exactly how a pushable ref becomes
/// invisible to every kind:30618 subscriber (#4194: `+`/`@` refs CASed fine
/// and then vanished from branch listings).
fn is_emittable_ref(name: &str) -> bool {
if !(name.starts_with("refs/heads/") || name.starts_with("refs/tags/")) {
return false;
}
if name.starts_with('/') || name.contains("//") {
return false;
}
name.chars()
.all(|c| c.is_ascii_alphanumeric() || "/_.-".contains(c))
(name.starts_with("refs/heads/") || name.starts_with("refs/tags/"))
&& super::manifest::is_safe_refname(name)
}

/// Accept SHA-1 (40 hex) and SHA-256 (64 hex) OIDs.
Expand Down Expand Up @@ -365,6 +368,32 @@ mod tests {
assert_eq!(tags_with_kind(&ev, "refs/heads//double").len(), 0);
}

/// A ref that passed manifest validation must reach the kind:30618 event:
/// every branch lister reads this event, so an alphabet re-spelling here
/// turns a pushable `+`/`@` ref into a branch that CASes, hydrates, and
/// clones — and is invisible in every client (#4194).
#[test]
fn emits_git_legal_plus_and_at_refs() {
let oid = "1111111111111111111111111111111111111111";
let refs = refs_with(&[
("refs/heads/test/842+841-devnet", oid),
("refs/heads/dependabot/npm_and_yarn/@types/node-1.2.3", oid),
("refs/tags/v1.0.0+build.5", oid),
]);
let inputs = RefStateInputs {
repo_id: "r",
head: "refs/heads/test/842+841-devnet",
refs: &refs,
actor_pubkey_hex: &owner_hex(),
};
let ev = build_ref_state_event(&inputs, &relay_keys()).unwrap();
assert!(first_tag(&ev, "refs/heads/test/842+841-devnet").is_some());
assert!(first_tag(&ev, "refs/heads/dependabot/npm_and_yarn/@types/node-1.2.3").is_some());
assert!(first_tag(&ev, "refs/tags/v1.0.0+build.5").is_some());
// HEAD pointing at a `+` branch emits too.
assert!(first_tag(&ev, "HEAD").is_some());
}

#[test]
fn rejects_invalid_actor_pubkey() {
let refs = refs_with(&[]);
Expand Down