diff --git a/crates/buzz-core/src/git_perms.rs b/crates/buzz-core/src/git_perms.rs index 391781163b..0b72133f90 100644 --- a/crates/buzz-core/src/git_perms.rs +++ b/crates/buzz-core/src/git_perms.rs @@ -147,10 +147,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())); @@ -703,6 +707,31 @@ mod tests { assert!(!p.matches("refs/heads")); } + #[test] + fn pattern_literal_accepts_plus_and_at() { + // `+` and `@` are now legal inside `is_safe_refname` (#4194). A literal + // protection rule must be able to name refs containing them, or those + // refs become pushable-but-unnameable — the protection layer can't + // enforce anything on 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/other")); + + let p = RefPattern::parse("refs/tags/release@v1").unwrap(); + assert!(p.matches("refs/tags/release@v1")); + + // Single-segment wildcard still matches across the widened alphabet + // (one segment only — `refs/heads/test/842+841-devnet` has two + // segments under `heads/` and so does NOT match `refs/heads/*`). + let p = RefPattern::parse("refs/heads/*").unwrap(); + assert!(p.matches("refs/heads/test")); + assert!(!p.matches("refs/heads/test/842+841-devnet")); + + // Recursive wildcard matches refs with `+`/`@` in any component. + let p = RefPattern::parse("refs/heads/**").unwrap(); + assert!(p.matches("refs/heads/test/842+841-devnet")); + } + #[test] fn classify_create() { let zero = "0000000000000000000000000000000000000000"; diff --git a/crates/buzz-relay/src/api/git/hydrate.rs b/crates/buzz-relay/src/api/git/hydrate.rs index 3ce809d18f..b32a534b93 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")); + // `+` / `@` — legal git, safe for CAS keys (see manifest.rs note). + assert!(is_safe_refname("refs/heads/test/842+841-devnet")); + assert!(is_safe_refname("refs/tags/release@v1")); 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..874a9f7d18 100644 --- a/crates/buzz-relay/src/api/git/manifest.rs +++ b/crates/buzz-relay/src/api/git/manifest.rs @@ -135,7 +135,15 @@ pub enum ManifestError { /// /// Refuses traversal (`..`), null/newline/control chars, non-`refs/` prefixes, /// and leading/trailing/double slashes. Allowed alphabet: -/// `[a-zA-Z0-9_./-]`. +/// `[a-zA-Z0-9_./+@-]` — note `+` immediately before `@` to avoid reading as +/// a character-class range (`+-@` would include `: ; < = > ?`). +/// +/// `+` and `@` are legal git ref characters (`git check-ref-format`) with no +/// meaning to the object-store key scheme or path traversal — they were +/// excluded historically for paranoia, not safety. Real-world branches like +/// `refs/heads/test/842+841-devnet` (seen in `OriginTrail/dkg`) were rejected +/// outright. Widening the predicate is symmetric: `validate` gates write, +/// hydration gates read, and both share this function. /// /// Sharing one predicate is load-bearing: any divergence creates the /// "valid CAS, un-clone-able output" hazard. @@ -147,7 +155,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) — @@ -332,17 +340,32 @@ mod tests { } #[test] - fn safe_refnames_predicate() { + fn safe_refnames() { 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 alphabet — `+` and `@` have no meaning to CAS keys or + // path traversal. (`OriginTrail/dkg`'s `test/842+841-devnet` was + // the motivating case; `@` joins it for symmetry and because `@{` + // reflog syntax never reaches manifest paths.) + assert!(is_safe_refname("refs/heads/test/842+841-devnet")); + assert!(is_safe_refname("refs/tags/release@v1")); assert!(!is_safe_refname("refs/heads/../escape")); assert!(!is_safe_refname("HEAD")); + // The empty-string reject is load-bearing: `ManifestError::EmptyHead`'s + // doc relies on the read side never accepting `""` as a head, so a + // missing `head` field is distinguishable from `""` (which is invalid). assert!(!is_safe_refname("")); assert!(!is_safe_refname("refs/heads/")); assert!(!is_safe_refname("/refs/heads/main")); assert!(!is_safe_refname("refs/heads/main\nrefs/heads/evil")); assert!(!is_safe_refname("refs/heads/main\0")); + // Other git-legal-but-unneeded chars stay out: `=`, `,`, `!`, `]` + // were never observed failing upstream and reduce the attack surface. + assert!(!is_safe_refname("refs/heads/feat=v2")); + assert!(!is_safe_refname("refs/heads/feat,name")); + assert!(!is_safe_refname("refs/heads/feat!hot")); + assert!(!is_safe_refname("refs/heads/feat]branch")); } #[test] diff --git a/crates/buzz-relay/src/api/git/manifest_event.rs b/crates/buzz-relay/src/api/git/manifest_event.rs index 698f27e9d1..431e693555 100644 --- a/crates/buzz-relay/src/api/git/manifest_event.rs +++ b/crates/buzz-relay/src/api/git/manifest_event.rs @@ -122,7 +122,7 @@ fn is_emittable_ref(name: &str) -> bool { return false; } name.chars() - .all(|c| c.is_ascii_alphanumeric() || "/_.-".contains(c)) + .all(|c| c.is_ascii_alphanumeric() || "/_.-+@".contains(c)) } /// Accept SHA-1 (40 hex) and SHA-256 (64 hex) OIDs. @@ -333,6 +333,47 @@ mod tests { assert!(first_tag(&ev, "refs/heads/ok").is_some()); } + #[test] + fn emits_refs_with_plus_and_at_in_component() { + // `+` and `@` are git-legal ref characters now accepted by + // `is_safe_refname` (#4194). They must NOT be silently dropped from + // kind:30618 ref-state events, or those refs would push successfully + // but never appear in branch listers (desktop `projects/hooks.ts`, + // web `repos/use-repo-refs.ts`). + let oid = "0123456789012345678901234567890123456789"; + let refs = refs_with(&[ + ("refs/heads/test/842+841-devnet", oid), + ("refs/tags/release@v1", oid), + ]); + let inputs = RefStateInputs { + repo_id: "r", + head: "refs/heads/main", + 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(), + "ref with `+` must be emitted in kind:30618 (#4194)" + ); + assert!( + first_tag(&ev, "refs/tags/release@v1").is_some(), + "ref with `@` must be emitted in kind:30618 (#4194)" + ); + } + + #[test] + fn is_emittable_ref_widened_alphabet() { + // Direct portable test of the predicate — kept next to + // `rejects_malformed_ref_names` so both rejection and allowance + // live in one place. + assert!(is_emittable_ref("refs/heads/test/842+841-devnet")); + assert!(is_emittable_ref("refs/tags/release@v1")); + assert!(!is_emittable_ref("refs/heads/space ref")); + assert!(!is_emittable_ref("refs/heads//double")); + assert!(!is_emittable_ref("refs/heads/\n")); + } + #[test] fn rejects_malformed_ref_names() { let refs = refs_with(&[