diff --git a/crates/buzz-core/src/git_perms.rs b/crates/buzz-core/src/git_perms.rs index 391781163b..6e8db6be91 100644 --- a/crates/buzz-core/src/git_perms.rs +++ b/crates/buzz-core/src/git_perms.rs @@ -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)] @@ -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())); @@ -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!( diff --git a/crates/buzz-relay/src/api/git/hydrate.rs b/crates/buzz-relay/src/api/git/hydrate.rs index 3ce809d18f..dd1851af0f 100644 --- a/crates/buzz-relay/src/api/git/hydrate.rs +++ b/crates/buzz-relay/src/api/git/hydrate.rs @@ -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/")); diff --git a/crates/buzz-relay/src/api/git/manifest.rs b/crates/buzz-relay/src/api/git/manifest.rs index baf109c1ad..9c728cf8b4 100644 --- a/crates/buzz-relay/src/api/git/manifest.rs +++ b/crates/buzz-relay/src/api/git/manifest.rs @@ -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. @@ -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) — @@ -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("")); diff --git a/crates/buzz-relay/src/api/git/manifest_event.rs b/crates/buzz-relay/src/api/git/manifest_event.rs index 698f27e9d1..6707784a32 100644 --- a/crates/buzz-relay/src/api/git/manifest_event.rs +++ b/crates/buzz-relay/src/api/git/manifest_event.rs @@ -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( @@ -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. @@ -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(&[]);