From 90d7800f3ea650c6c49b04d2d451fe9029e64d18 Mon Sep 17 00:00:00 2001 From: iroiro147 Date: Sun, 2 Aug 2026 10:44:30 +0530 Subject: [PATCH 1/2] fix(git): widen is_safe_refname alphabet to include `+` and `@` (#4194) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `git check-ref-format` accepts `+` and `@` in ref components (no newline, no NUL, no control chars, no `.` at the start of a component, no trailing `/`), but `is_safe_refname` rejected anything outside `[a-zA-Z0-9_./-]`. Real-world upstream branches like `OriginTrail/dkg`'s `refs/heads/test/842+841-devnet` were un-mirrorable into Buzz git. Both characters have no meaning to the object-store key scheme or to path traversal (`.`-based traversal protection is unchanged, and refnames are still pinned to the `refs/` prefix so they cannot escape into other key spaces). The predicate is shared symmetrically by write-side `validate` and read-side hydration, so widening once fixes both sides of the seam. Keep the widening conservative — the other git-legal chars called out in the issue (`=`, `,`, `!`, `]`) are not observed failing at any upstream mirror and remain excluded to preserve a small attack surface. They can be added in a follow-up if a real failure surfaces. Regression coverage extends the existing `safe_refnames` tests in both `manifest.rs` and `hydrate.rs`: - positive: `refs/heads/test/842+841-devnet` and `refs/tags/release@v1` - negative: `=`, `,`, `!`, `]` remain rejected `cargo check -p buzz-relay` and `cargo test -p buzz-relay --lib safe_refnames` are green (2/2 tests pass; the 10 unrelated pre-existing failures in api::media + telemetry suites are untouched and out of scope). Refs #4194 Signed-off-by: Sarthak Singh --- crates/buzz-relay/src/api/git/hydrate.rs | 3 +++ crates/buzz-relay/src/api/git/manifest.rs | 26 +++++++++++++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) 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..98df70cced 100644 --- a/crates/buzz-relay/src/api/git/manifest.rs +++ b/crates/buzz-relay/src/api/git/manifest.rs @@ -135,7 +135,14 @@ 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_./+-@]`. +/// +/// `+` 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 +154,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 +339,28 @@ 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")); - 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] From 4a88e60179bf62079f816d7cda43956f390d69f8 Mon Sep 17 00:00:00 2001 From: iroiro147 Date: Sun, 2 Aug 2026 21:12:18 +0530 Subject: [PATCH 2/2] fix(git): widen is_emittable_ref and RefPattern alphabets to match is_safe_refname (#4194) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two sibling predicates re-spelled the pre-widening alphabet from is_safe_refname and silently dropped refs containing + or @: - is_emittable_ref in buzz-relay/src/api/git/manifest_event.rs — refs were continued out of the kind:30618 ref-state event, so refs/heads/test/842+841-devnet pushed clean, CASed, hydrated, cloned, and then never appeared in any branch lister (desktop projects/hooks.ts, web repos/use-repo-refs.ts). - RefPattern::parse in buzz-core/src/git_perms.rs:152 — literal protection rules could not name +/@ refs; those refs were pushable-but-unnameable from the enforcement side. Caught during review of #4257 (thanks @alanshurafa). Widening is_safe_refname alone was insufficient: a ref that pushes, CASes, hydrates, and clones — but never appears in any lister — is strictly worse than today's 400-class rejection because nothing reports it. Two small items folded in: - Restore assert!(!is_safe_refname("")) in manifest.rs — load-bearing (ManifestError::EmptyHead doc relies on it). - Doc notation [a-zA-Z0-9_./+@-], not [a-zA-Z0-9_./+-@] — unambiguous (avoids char-class range interpretation +-@ which would include : ; < = > ?). Coverage: - manifest_event.rs::emits_refs_with_plus_and_at_in_component — event build includes refs with +/@ in its tag lists. - manifest_event.rs::is_emittable_ref_widened_alphabet — direct predicate test. - git_perms.rs::pattern_literal_accepts_plus_and_at — literal pattern can now name such refs; wildcard semantics unchanged. Test counts: 2/2 safe_refnames + 11/11 manifest_event + 35/35 git_perms, all green. Refs #4194 Signed-off-by: Sarthak Singh --- crates/buzz-core/src/git_perms.rs | 37 ++++++++++++++-- crates/buzz-relay/src/api/git/manifest.rs | 7 ++- .../buzz-relay/src/api/git/manifest_event.rs | 43 ++++++++++++++++++- 3 files changed, 81 insertions(+), 6 deletions(-) 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/manifest.rs b/crates/buzz-relay/src/api/git/manifest.rs index 98df70cced..874a9f7d18 100644 --- a/crates/buzz-relay/src/api/git/manifest.rs +++ b/crates/buzz-relay/src/api/git/manifest.rs @@ -135,7 +135,8 @@ 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 @@ -351,6 +352,10 @@ mod tests { 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")); 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(&[