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
37 changes: 33 additions & 4 deletions crates/buzz-core/src/git_perms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down Expand Up @@ -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";
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"));
// `+` / `@` — 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/"));
Expand Down
29 changes: 26 additions & 3 deletions crates/buzz-relay/src/api/git/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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) —
Expand Down Expand Up @@ -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]
Expand Down
43 changes: 42 additions & 1 deletion crates/buzz-relay/src/api/git/manifest_event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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(&[
Expand Down