socle

validators and error classes: fail-closed checks under every handler and git invocation

commit 2fcf91c586d2ff50542a9d87513f7d11ffdfe70e · parent 5152650dd9 · browse this tree
Ximianth, 2026-10-26 19:40:27 UTC

Three validators (repo names, refs, tree paths) and two error sentinels
(errInvalid, errNotFound) that the serve layer will map to 400/404;
gitError carries internal failure detail for everything else.

Refs get a deliberately narrow charset: it admits branches, tags, HEAD,
and hex shas while excluding all rev-parse operator syntax, so nothing
that passes validation can mean anything to git but a plain ref.
Tree paths stay charset-open (committed filenames are arbitrary) and
only reject traversal, empty segments, and control bytes: they are only
ever used after the colon of a <sha>:<path> argument, where git parses
no options or rev syntax.

diff --git a/errors.go b/errors.go
new file mode 100644
index 0000000..0b45884
--- /dev/null
+++ b/errors.go
@@ -0,0 +1,53 @@
+package main
+
+import (
+	"errors"
+	"fmt"
+	"strings"
+)
+
+// Error classes for the plumbing layer.
+// The serve layer maps these to HTTP statuses with errors.Is:
+// errInvalid means the request was malformed (400),
+// errNotFound means the asked-for thing doesn't exist (404),
+// and anything else is internal (500).
+var (
+	// errInvalid marks input rejected by a validator: a malformed repo name, ref, or tree path.
+	errInvalid = errors.New("invalid")
+	// errNotFound marks a lookup that came back empty: an unknown repo, an unresolvable ref, or a path absent from a tree.
+	errNotFound = errors.New("not found")
+)
+
+// gitError reports a git invocation that failed.
+// Read operations translate the failures that mean "doesn't exist" into errNotFound;
+// a gitError that escapes to a caller is an internal problem (missing binary, repo corruption, parse surprise).
+type gitError struct {
+	args     []string // the git argv, excluding the leading "git" itself
+	exitCode int      // the process's exit code; -1 if it never ran
+	stderr   string   // trimmed stderr capture, for diagnostics
+	cause    error    // the underlying exec error, if any
+}
+
+func (e *gitError) Error() string {
+	msg := fmt.Sprintf("git %s: exit %d", strings.Join(e.args, " "), e.exitCode)
+	if e.stderr != "" {
+		msg += ": " + e.stderr
+	}
+	if e.cause != nil {
+		msg += ": " + e.cause.Error()
+	}
+	return msg
+}
+
+func (e *gitError) Unwrap() error { return e.cause }
+
+// notFoundOnExit converts a git failure into errNotFound when the process ran and exited nonzero:
+// the operations that use this are ones where any clean nonzero exit means "no such ref/path".
+// Failures to run at all (or errors that aren't gitError) pass through unchanged, staying internal.
+func notFoundOnExit(err error, what string) error {
+	var ge *gitError
+	if errors.As(err, &ge) && ge.exitCode > 0 {
+		return fmt.Errorf("%w: %s", errNotFound, what)
+	}
+	return err
+}
diff --git a/validate.go b/validate.go
new file mode 100644
index 0000000..1fc805c
--- /dev/null
+++ b/validate.go
@@ -0,0 +1,113 @@
+package main
+
+import (
+	"fmt"
+	"strings"
+)
+
+// Validators for the names that arrive from the outside world:
+// repo names (from config), refs (from URLs), and tree paths (from URLs).
+// Every route handler and every git invocation funnels through these, and they fail closed:
+// anything not positively allowed is rejected with errInvalid.
+//
+// Injection through argv is already impossible (git is exec'd with argument vectors, never a shell),
+// so what these guard against is git misparsing an argument:
+// options (leading dash), rev-parse operators (.. @{} ^ ~ :), and path traversal.
+
+// validateRepoName checks a configured repo name (which is also its URL prefix).
+// Names are slash-separated segments of [A-Za-z0-9._-], with no segment
+// empty, starting with a dot or a dash, or ending in ".git"
+// (a leading dot covers rejecting ".", "..", and ".git" themselves;
+// a ".git" suffix is rejected because URLs treat a trailing ".git" as an alias for the plain name,
+// and a configured name ending that way would make the alias ambiguous).
+func validateRepoName(name string) error {
+	if name == "" {
+		return fmt.Errorf("%w: repo name is empty", errInvalid)
+	}
+	for _, seg := range strings.Split(name, "/") {
+		if seg == "" {
+			return fmt.Errorf("%w: repo name %q: empty path segment", errInvalid, name)
+		}
+		if seg[0] == '.' || seg[0] == '-' {
+			return fmt.Errorf("%w: repo name %q: segment %q starts with %q", errInvalid, name, seg, seg[0:1])
+		}
+		if strings.HasSuffix(seg, ".git") {
+			return fmt.Errorf("%w: repo name %q: segment %q ends in .git (reserved as a URL alias for the plain name)", errInvalid, name, seg)
+		}
+		for _, c := range seg {
+			if !nameChar(c) {
+				return fmt.Errorf("%w: repo name %q: character %q not allowed (name segments are [A-Za-z0-9._-])", errInvalid, name, c)
+			}
+		}
+	}
+	return nil
+}
+
+func nameChar(c rune) bool {
+	return c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' ||
+		c == '.' || c == '_' || c == '-'
+}
+
+// validateRef checks a ref name before it goes anywhere near a git invocation.
+// The allowed shape is deliberately narrower than git's own ref rules:
+// slash-separated segments of [A-Za-z0-9._-], no segment empty, starting with a dot, or ending in ".lock",
+// no ".." anywhere, no leading dash, and at most 255 bytes.
+// This admits branch names, tag names, "HEAD", and full hex shas,
+// while excluding every rev-parse operator (ranges, reflog @{} syntax, ^ and ~ suffixes, :paths).
+func validateRef(ref string) error {
+	if ref == "" {
+		return fmt.Errorf("%w: ref is empty", errInvalid)
+	}
+	if len(ref) > 255 {
+		return fmt.Errorf("%w: ref is unreasonably long (%d bytes)", errInvalid, len(ref))
+	}
+	if ref[0] == '-' {
+		return fmt.Errorf("%w: ref %q starts with a dash", errInvalid, ref)
+	}
+	if strings.Contains(ref, "..") {
+		return fmt.Errorf("%w: ref %q contains %q", errInvalid, ref, "..")
+	}
+	for _, seg := range strings.Split(ref, "/") {
+		if seg == "" {
+			return fmt.Errorf("%w: ref %q: empty path segment", errInvalid, ref)
+		}
+		if seg[0] == '.' {
+			return fmt.Errorf("%w: ref %q: segment %q starts with a dot", errInvalid, ref, seg)
+		}
+		if strings.HasSuffix(seg, ".lock") {
+			return fmt.Errorf("%w: ref %q: segment %q ends in .lock", errInvalid, ref, seg)
+		}
+		for _, c := range seg {
+			if !nameChar(c) {
+				return fmt.Errorf("%w: ref %q: character %q not allowed", errInvalid, ref, c)
+			}
+		}
+	}
+	return nil
+}
+
+// validateTreePath checks a path within a repo's tree.  The empty string is valid and means the root.
+// Unlike names and refs, the character set stays open (committed filenames are arbitrary:
+// spaces, unicode, whatever), because a tree path is only ever used after a ":" in a <sha>:<path> argument,
+// where git applies no option parsing or rev syntax.
+// What's rejected: traversal dots, empty segments, leading or trailing slashes, and control bytes
+// (never legitimate in a URL-supplied filename, and they'd make logs lie).
+func validateTreePath(path string) error {
+	if path == "" {
+		return nil
+	}
+	for _, c := range path {
+		if c < 0x20 || c == 0x7f {
+			return fmt.Errorf("%w: path contains control byte 0x%02x", errInvalid, c)
+		}
+	}
+	for _, seg := range strings.Split(path, "/") {
+		if seg == "" {
+			return fmt.Errorf("%w: path %q: empty segment", errInvalid, path)
+		}
+		if seg == "." || seg == ".." {
+			return fmt.Errorf("%w: path %q: %q segment", errInvalid, path, seg)
+		}
+	}
+	return nil
+}
diff --git a/validate_test.go b/validate_test.go
new file mode 100644
index 0000000..79d8f34
--- /dev/null
+++ b/validate_test.go
@@ -0,0 +1,105 @@
+package main
+
+import (
+	"errors"
+	"testing"
+)
+
+func TestValidateRepoName(t *testing.T) {
+	t.Parallel()
+	runs(t, validateRepoName, []vcase{
+		{"plain", "parnass", true},
+		{"nested", "work/infra/deploy", true},
+		{"dots and dashes inside", "a-b.c_d", true},
+		{"digits", "repo2", true},
+		{"empty", "", false},
+		{"leading slash", "/foo", false},
+		{"trailing slash", "foo/", false},
+		{"double slash", "foo//bar", false},
+		{"dot segment", "foo/./bar", false},
+		{"dotdot segment", "foo/../bar", false},
+		{"leading dot", ".hidden", false},
+		{"leading dash", "-flag", false},
+		{"leading dash in segment", "ok/-flag", false},
+		{"git suffix", "foo.git", false},
+		{"git suffix in segment", "foo.git/bar", false},
+		{"bare .git segment", "foo/.git", false},
+		{"space", "fo o", false},
+		{"colon", "fo:o", false},
+		{"unicode", "caf\u00e9", false},
+	})
+}
+
+func TestValidateRef(t *testing.T) {
+	t.Parallel()
+	runs(t, validateRef, []vcase{
+		{"branch", "main", true},
+		{"HEAD", "HEAD", true},
+		{"tag", "v1.0", true},
+		{"slashed branch", "feature/shiny", true},
+		{"full sha", "a94a8fe5ccb19ba61c4c0873d391e987982fbbd3", true},
+		{"empty", "", false},
+		{"leading dash", "-option", false},
+		{"range", "main..dev", false},
+		{"triple dot range", "main...dev", false},
+		{"reflog syntax", "HEAD@{1}", false},
+		{"parent suffix", "HEAD^", false},
+		{"tilde suffix", "HEAD~2", false},
+		{"colon path", "main:README.md", false},
+		{"space", "ma in", false},
+		{"dot-led segment", "foo/.bar", false},
+		{"lock suffix", "main.lock", false},
+		{"empty segment", "foo//bar", false},
+		{"leading slash", "/main", false},
+		{"trailing slash", "main/", false},
+		{"overlong", string(make([]byte, 256)), false},
+	})
+}
+
+func TestValidateTreePath(t *testing.T) {
+	t.Parallel()
+	runs(t, validateTreePath, []vcase{
+		{"root", "", true},
+		{"file", "README.md", true},
+		{"nested", "docs/notes.md", true},
+		{"dotfile", ".gitignore", true},
+		{"space in name", "docs/with space.txt", true},
+		{"unicode", "caf\u00e9/menu.md", true},
+		{"leading dash", "-dashed", true}, // safe: paths ride after the colon of <sha>:<path>
+		{"dot segment", "./README.md", false},
+		{"dotdot segment", "docs/../secret", false},
+		{"bare dotdot", "..", false},
+		{"leading slash", "/etc/passwd", false},
+		{"trailing slash", "docs/", false},
+		{"double slash", "docs//notes.md", false},
+		{"newline", "do\ncs", false},
+		{"nul", "do\x00cs", false},
+	})
+}
+
+type vcase struct {
+	name  string
+	input string
+	ok    bool
+}
+
+// runs drives one validator through a case table,
+// checking both acceptance and that every rejection is an errInvalid.
+func runs(t *testing.T, validate func(string) error, cases []vcase) {
+	t.Helper()
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			err := validate(c.input)
+			if c.ok && err != nil {
+				t.Errorf("%q: expected acceptance, got %v", c.input, err)
+			}
+			if !c.ok {
+				if err == nil {
+					t.Errorf("%q: expected rejection, got acceptance", c.input)
+				} else if !errors.Is(err, errInvalid) {
+					t.Errorf("%q: rejection should wrap errInvalid, got %v", c.input, err)
+				}
+			}
+		})
+	}
+}