Sign in

Search issue titles and bodies, change requests and comments. Private repositories you cannot read are not included.

vole
Clone

Read only, no credentials:

https://rodent.software/vole.git

Write over SSH with a registered key:

git clone ssh://git@rodent.software:2222/vole.git

A free and open-source forge that lives in your Git repo.

Refuse a protected-ref pattern that can never match

93c7cae8ebb7d6d853dca476851e2bd0cbe10079 · Keenan Nemetz · 1d ago · parent 2c988ecd

Refuse a protected-ref pattern that can never match

acl protect stored any string: refs/head/main read like a protected branch in
acl show while the hook compared it against refs/heads/main and matched
nothing, so approvals and required checks were off on the branch everything
lands on.

Validate the pattern in forge/access, where the rule is stored, and call it
from both acl protect add|set and the repository settings form. Only the two
shapes IsProtected understands are accepted, exact ref and one trailing
wildcard, the ref part must be a name git could store, and a singular
category is named as the typo it is.

forge/access/acl.go +120−1

@@ -4,9 +4,11 @@ package access
4 4
5 5 import (
6 6 "encoding/json"
7 "errors"
7 8 "fmt"
8 9 "sort"
9 10 "strings"
11 "unicode"
10 12
11 13 "github.com/rodent-software/vole/core/role"
12 14 )
@@ -355,8 +357,125 @@ func (a *ACL) CanTriage(user string) bool { return a.RoleFor(user).AtLeast(Maint
355 357 // the request's author id.
356 358 func (a *ACL) CanApprove(user string) bool { return a.RoleFor(user).AtLeast(Maintain) }
357 359
360 // ValidateProtectedPattern checks a pattern where an operator writes it, because
361 // a pattern IsProtected can never match is worse than none at all: the repository
362 // reads as protected while the branch is open. Only the two shapes IsProtected
363 // understands are accepted, an exact ref and a ref prefix ending in one "*", and
364 // the ref part must be a name Git could actually store. A prefix for a namespace
365 // with nothing in it yet is allowed: "refs/heads/release/*" is a legitimate rule
366 // for the first release branch.
367 //
368 // The rule lives here, not in the CLI, so the browser form and any other writer
369 // cannot store what the CLI refuses.
370 func ValidateProtectedPattern(pattern string) error {
371 if pattern == "" {
372 return errors.New(`a protected pattern cannot be empty: name a ref, as in "refs/heads/main"`)
373 }
374 if pattern != strings.TrimSpace(pattern) || strings.ContainsFunc(pattern, unicode.IsControl) {
375 return fmt.Errorf("protected pattern %q cannot match any ref: it carries whitespace or control characters", pattern)
376 }
377 if !strings.HasPrefix(pattern, "refs/") {
378 if fixed, ok := likelyRef(pattern); ok {
379 return fmt.Errorf("protected pattern %q cannot match any ref: did you mean %q?", pattern, fixed)
380 }
381 return fmt.Errorf(`protected pattern %q cannot match any ref: it must start with "refs/", like "refs/heads/main"`, pattern)
382 }
383 if fixed, ok := likelyRef(pattern); ok {
384 return fmt.Errorf("protected pattern %q cannot match any ref: did you mean %q?", pattern, fixed)
385 }
386 prefix := strings.HasSuffix(pattern, "*")
387 if n := strings.Count(pattern, "*"); n > 1 || (n == 1 && !prefix) {
388 return fmt.Errorf(`protected pattern %q cannot match any ref: the only wildcard understood is one trailing "*", as in "refs/heads/release/*"`, pattern)
389 }
390 base := strings.TrimSuffix(pattern, "*")
391 // "refs/*" is the one base with no name left after the wildcard; it matches
392 // every ref by design and there is nothing further to check.
393 if prefix && base == "refs/" {
394 return nil
395 }
396 // A prefix names the namespace refs live in, so its trailing slash is the
397 // boundary the wildcard stands in for, and the name before it may end in a
398 // dot or ".lock" the way a real ref may not.
399 name := base
400 if prefix {
401 name = strings.TrimSuffix(base, "/")
402 }
403 if reason := refNameProblem(name, !prefix); reason != "" {
404 if !prefix && strings.HasSuffix(base, "/") {
405 return fmt.Errorf("protected pattern %q cannot match any ref: %s; use %q to protect everything beneath it", pattern, reason, base+"*")
406 }
407 return fmt.Errorf("protected pattern %q cannot match any ref: %s", pattern, reason)
408 }
409 // A ref is at least a category and a name (refs/heads/main), so a base that
410 // stops after one slash names a category the repository does not store.
411 if !prefix && strings.Count(base, "/") < 2 {
412 return fmt.Errorf("protected pattern %q names a ref category, not a ref: use %q to protect everything in it", pattern, base+"/*")
413 }
414 return nil
415 }
416
417 // ValidateProtectedPatterns is ValidateProtectedPattern for a whole list, so a
418 // surface that accepts several patterns rejects the first bad one before it
419 // writes any of them.
420 func ValidateProtectedPatterns(patterns []string) error {
421 for _, p := range patterns {
422 if err := ValidateProtectedPattern(p); err != nil {
423 return err
424 }
425 }
426 return nil
427 }
428
429 // likelyRef turns the singular category names Git does not use into the ref the
430 // operator probably meant. A one-letter slip is the whole difference between a
431 // protected branch and a rule that never fires.
432 func likelyRef(pattern string) (string, bool) {
433 for _, fix := range []struct{ wrong, right string }{
434 {"refs/head/", "refs/heads/"},
435 {"refs/tag/", "refs/tags/"},
436 {"heads/", "refs/heads/"},
437 {"tags/", "refs/tags/"},
438 } {
439 if rest, ok := strings.CutPrefix(pattern, fix.wrong); ok {
440 return fix.right + rest, true
441 }
442 }
443 return "", false
444 }
445
446 // refNameProblem reports why a string cannot be a ref name, using git's own
447 // ref-format rules, or "" when it can. exact says the whole string is the ref;
448 // without it the string is a prefix a wildcard stands after, so its last element
449 // is incomplete and the rules about how a name ends do not apply.
450 func refNameProblem(name string, exact bool) string {
451 switch {
452 case strings.Contains(name, "//"):
453 return "a ref never contains an empty path component"
454 case strings.Contains(name, ".."):
455 return `a ref never contains ".."`
456 case strings.Contains(name, "@{"):
457 return `a ref never contains "@{"`
458 case strings.ContainsAny(name, " ~^:?*[\\"):
459 return `a ref never contains a space or one of ~ ^ : ? * [ \`
460 case name == "refs":
461 return `a ref has a category and a name, not "refs" alone`
462 case strings.HasSuffix(name, "/"):
463 return "a ref never ends in a slash"
464 }
465 if exact {
466 switch {
467 case strings.HasSuffix(name, "."):
468 return "a ref never ends in a dot"
469 case strings.HasSuffix(name, ".lock"):
470 return `a ref never ends in ".lock"`
471 }
472 }
473 return ""
474 }
475
358 476 // IsProtected reports whether ref matches a protected pattern. Patterns are an
359 // exact ref or a trailing "*" prefix such as "refs/heads/release/*".
477 // exact ref or a trailing "*" prefix such as "refs/heads/release/*". Every
478 // pattern stored through ValidateProtectedPattern is one of the two.
360 479 func (a *ACL) IsProtected(ref string) bool {
361 480 for _, p := range a.Protected {
362 481 if strings.HasSuffix(p, "*") {