From cec7be05b50c3681b11882f25d5b53b6146f3c71 Mon Sep 17 00:00:00 2001 From: Xe Iaso Date: Wed, 23 Sep 2026 15:46:25 -0400 Subject: [PATCH 1/3] feat: add signed push webhooks Define versioned Buf push events with every new commit and its file changes. Deliver signed ProtoJSON after accepted ref updates and expose net file metadata to shell hooks. Assisted-by: GPT-6 via Codex Signed-off-by: Xe Iaso --- .objgit/hooks/receive-pack | 14 +- buf.gen.yaml | 12 + buf.yaml | 9 + cmd/objgitd/git_protocol.go | 2 + cmd/objgitd/hooks.go | 100 +++- cmd/objgitd/hooks_test.go | 126 ++++- cmd/objgitd/main.go | 13 +- cmd/objgitd/receivepack.go | 187 ++++--- cmd/objgitd/refbatch_test.go | 110 +++- cmd/objgitd/webhooks.go | 66 +++ cmd/objgitd/webhooks_test.go | 244 +++++++++ docs/architecture/README.md | 3 +- docs/architecture/hooks.md | 17 +- docs/architecture/metrics.md | 6 + docs/plans/webhook-push-events.md | 51 ++ docs/usage/hooks.md | 55 +- docs/usage/webhooks.md | 70 +++ .../objgit/events/push/v1/push.pb.go | 500 ++++++++++++++++++ go.mod | 2 +- internal/metrics/metrics.go | 21 + internal/pushevents/push.go | 338 ++++++++++++ internal/pushevents/push_test.go | 449 ++++++++++++++++ internal/storage/tigris/refcache.go | 37 +- internal/storage/tigris/refcommit_test.go | 38 ++ internal/storage/tigris/reffold_test.go | 27 + internal/storage/tigris/refs.go | 27 + internal/webhook/webhook.go | 288 ++++++++++ internal/webhook/webhook_test.go | 274 ++++++++++ proto/README.md | 28 + .../objgit/events/push/v1/push.example.json | 64 +++ .../objgit/events/push/v1/push.proto | 92 ++++ 31 files changed, 3134 insertions(+), 136 deletions(-) create mode 100644 buf.gen.yaml create mode 100644 buf.yaml create mode 100644 cmd/objgitd/webhooks.go create mode 100644 cmd/objgitd/webhooks_test.go create mode 100644 docs/plans/webhook-push-events.md create mode 100644 docs/usage/webhooks.md create mode 100644 gen/tigrisdata/objgit/events/push/v1/push.pb.go create mode 100644 internal/pushevents/push.go create mode 100644 internal/pushevents/push_test.go create mode 100644 internal/webhook/webhook.go create mode 100644 internal/webhook/webhook_test.go create mode 100644 proto/README.md create mode 100644 proto/tigrisdata/objgit/events/push/v1/push.example.json create mode 100644 proto/tigrisdata/objgit/events/push/v1/push.proto diff --git a/.objgit/hooks/receive-pack b/.objgit/hooks/receive-pack index 3843503..04c173b 100755 --- a/.objgit/hooks/receive-pack +++ b/.objgit/hooks/receive-pack @@ -6,7 +6,7 @@ # -allow-hooks. It does NOT run on a normal `git` server — it only has meaning # when this repository is served by objgitd. # -# Execution environment (see CLAUDE.md "Push hooks"): +# Execution environment (see docs/usage/hooks.md): # * It runs in a kefka virtual shell, NOT a real OS shell. Only kefka's # built-in commands are available (coreutils: cat, ls, echo, printf, head, # tail, cut, sort, uniq, wc, sha256sum, grep, ...). There is no PATH to @@ -15,9 +15,9 @@ # starts here. # * /tmp is the only writable location (also $HOME and $TMPDIR). Writing # anywhere else fails; a redirect into /src aborts the script. -# * The hook runs asynchronously after the push response, so it cannot reject -# a push. Its stdout/stderr and exit status are written to the server log -# only — the person pushing never sees them. +# * The hook runs after refs are updated. It cannot reject a push. Its output +# streams to the pushing client when sideband is available, and its exit +# status is logged. The push waits for the hook to finish. # * One run happens per created or updated branch. Branch deletions are # skipped. # @@ -28,9 +28,15 @@ # OBJGIT_BRANCH short branch name, e.g. main # OBJGIT_OLD_SHA previous tip (all zeros when the branch was created) # OBJGIT_NEW_SHA new tip +# OBJGIT_ADDED_FILES_JSON, OBJGIT_CHANGED_FILES_JSON, +# OBJGIT_DELETED_FILES_JSON are JSON arrays of net file changes. +# OBJGIT_CHANGES_FILE points to the full JSON object in /tmp. # git's usual " " line is also fed on stdin. echo "push to ${OBJGIT_REPO} ${OBJGIT_REF}: ${OBJGIT_OLD_SHA} -> ${OBJGIT_NEW_SHA}" +echo "file changes:" +cat "${OBJGIT_CHANGES_FILE}" +echo # /src is the checkout of the new commit. List what landed at the top level. echo "top-level contents:" diff --git a/buf.gen.yaml b/buf.gen.yaml new file mode 100644 index 0000000..459939b --- /dev/null +++ b/buf.gen.yaml @@ -0,0 +1,12 @@ +version: v2 +managed: + enabled: true + override: + - file_option: go_package_prefix + value: github.com/tigrisdata/objgit/gen +plugins: + - remote: buf.build/protocolbuffers/go:v1.36.8 + out: gen + opt: paths=source_relative +inputs: + - directory: proto diff --git a/buf.yaml b/buf.yaml new file mode 100644 index 0000000..95c2f2f --- /dev/null +++ b/buf.yaml @@ -0,0 +1,9 @@ +version: v2 +modules: + - path: proto +lint: + use: + - STANDARD +breaking: + use: + - WIRE_JSON diff --git a/cmd/objgitd/git_protocol.go b/cmd/objgitd/git_protocol.go index 68d560c..38e7627 100644 --- a/cmd/objgitd/git_protocol.go +++ b/cmd/objgitd/git_protocol.go @@ -15,6 +15,7 @@ import ( "github.com/tigrisdata/objgit/internal/auth" "github.com/tigrisdata/objgit/internal/metrics" "github.com/tigrisdata/objgit/internal/repofs" + "github.com/tigrisdata/objgit/internal/webhook" ) // handshakeTimeout bounds how long a client has to send its git-proto-request. @@ -53,6 +54,7 @@ type daemon struct { // allowHooks gates running .objgit/hooks/receive-pack after a push. allowHooks bool hookTimeout time.Duration + webhooks *webhook.Client // pushes bounds how many pushes unpack a packfile at once, which is the // only thing that bounds the daemon's resident set under concurrent pushes. diff --git a/cmd/objgitd/hooks.go b/cmd/objgitd/hooks.go index 8c818f4..40b2e82 100644 --- a/cmd/objgitd/hooks.go +++ b/cmd/objgitd/hooks.go @@ -3,6 +3,7 @@ package main import ( "bytes" "context" + "encoding/json" "errors" "io" "log/slog" @@ -18,6 +19,7 @@ import ( "github.com/tigrisdata/objgit/internal/kefkash" "github.com/tigrisdata/objgit/internal/metrics" "github.com/tigrisdata/objgit/internal/mountfs" + "github.com/tigrisdata/objgit/internal/pushevents" "github.com/tigrisdata/objgit/internal/treefs" "mvdan.cc/sh/v3/expand" "mvdan.cc/sh/v3/interp" @@ -77,48 +79,38 @@ func diffRefs(before, after map[plumbing.ReferenceName]plumbing.Hash) []refUpdat return updates } -// receivePack runs the receive-pack service and, when hooks are enabled, fires -// the repository's receive-pack hook for each updated branch once the push -// succeeds — synchronously, streaming hook output to the client over the -// sideband progress channel (rendered as "remote: " lines) before the response -// stream is closed. st is both what the service writes through and the storer -// used for ref snapshots and hook checkouts — all three transports now share the -// same Scanner-bounded PackfileWriter path (see writePack), so no transport needs -// a capability-hiding wrapper. +// receivePack runs the receive-pack service and dispatches post-receive work +// for the commands whose ref updates succeeded. Hook output streams to the +// client before the response closes. Webhook delivery also finishes before the +// response closes, but cannot reject an already accepted push. // // This is also the one place every push funnels through — smart HTTP and SSH // both land here, and git:// never serves receive-pack at all — so it is where // the push concurrency cap is applied, via the d.pushes.admit seam. func (d *daemon) receivePack(ctx context.Context, st storage.Storer, repoPath string, r io.ReadCloser, w io.WriteCloser, req *transport.ReceivePackRequest) error { - if !d.allowHooks { + if !d.allowHooks && (d.webhooks == nil || !d.webhooks.Enabled(repoPath)) { err := receivePackStreaming(ctx, st, r, w, req, d.pushes.admit, nil) d.healHEADAfterPush(err, st, repoPath) return err } - before, err := snapshotRefs(st) - if err != nil { - slog.Warn("hook: ref snapshot before push failed", "path", repoPath, "err", err) - } - - // onUpdated runs after refs are updated and report-status is sent, but - // before the response stream closes, so hook output reaches the client live. - // progress is the sideband band-2 writer, or nil when the client did not - // negotiate sideband (hooks then fall back to logging only). - onUpdated := func(progress io.Writer) { - after, err := snapshotRefs(st) - if err != nil { - slog.Error("hook: ref snapshot after push failed", "path", repoPath, "err", err) - return + // The callback runs after refs change and report-status is attempted. The + // updates are taken from this request's successful commands, so another + // concurrent push cannot be attributed to this one. + onUpdated := func(progress io.Writer, updates []refUpdate, acceptedAt time.Time) { + if d.allowHooks { + var branches []refUpdate + for _, u := range updates { + if u.Name.IsBranch() { + branches = append(branches, u) + } + } + d.runHooks(repoPath, "receive-pack", st, branches, progress) } - updates := diffRefs(before, after) - if len(updates) == 0 { - return - } - d.runHooks(repoPath, "receive-pack", st, updates, progress) + d.emitPushWebhooks(ctx, st, repoPath, updates, acceptedAt) } - err = receivePackStreaming(ctx, st, r, w, req, d.pushes.admit, onUpdated) + err := receivePackStreaming(ctx, st, r, w, req, d.pushes.admit, onUpdated) d.healHEADAfterPush(err, st, repoPath) return err } @@ -185,6 +177,52 @@ func (d *daemon) runHook(repoPath, service string, st storage.Storer, u refUpdat ctx, cancel := context.WithTimeout(context.Background(), d.hookTimeout) defer cancel() + changes, err := pushevents.Diff(ctx, st, u.Old, u.New) + if err != nil { + log.Error("hook: diff changed files", "err", err) + return + } + // Keep empty lists as JSON arrays. Paths can contain whitespace or newlines, + // so JSON is the only unambiguous representation in environment variables. + added := append([]string{}, changes.GetAdded()...) + changed := append([]string{}, changes.GetChanged()...) + deleted := append([]string{}, changes.GetDeleted()...) + addedJSON, err := json.Marshal(added) + if err != nil { + log.Error("hook: encode added files", "err", err) + return + } + changedJSON, err := json.Marshal(changed) + if err != nil { + log.Error("hook: encode changed files", "err", err) + return + } + deletedJSON, err := json.Marshal(deleted) + if err != nil { + log.Error("hook: encode deleted files", "err", err) + return + } + metadata, err := json.Marshal(struct { + Added []string `json:"added"` + Changed []string `json:"changed"` + Deleted []string `json:"deleted"` + }{added, changed, deleted}) + if err != nil { + log.Error("hook: encode changed files", "err", err) + return + } + const changesFile = "/tmp/objgit-changes.json" + f, err := fsys.Create(changesFile) + if err != nil { + log.Error("hook: create changes file", "err", err) + return + } + _, writeErr := io.Copy(f, bytes.NewReader(metadata)) + closeErr := f.Close() + if err := errors.Join(writeErr, closeErr); err != nil { + log.Error("hook: write changes file", "err", err) + return + } // When the client negotiated sideband, stream stdout+stderr straight to it // ("remote: " lines); otherwise buffer for the log. git does not distinguish @@ -216,6 +254,10 @@ func (d *daemon) runHook(repoPath, service string, st storage.Storer, u refUpdat "OBJGIT_BRANCH="+u.Name.Short(), "OBJGIT_OLD_SHA="+u.Old.String(), "OBJGIT_NEW_SHA="+u.New.String(), + "OBJGIT_ADDED_FILES_JSON="+string(addedJSON), + "OBJGIT_CHANGED_FILES_JSON="+string(changedJSON), + "OBJGIT_DELETED_FILES_JSON="+string(deletedJSON), + "OBJGIT_CHANGES_FILE="+changesFile, ) // Mirror git's post-receive stdin: " \n". stdin := strings.NewReader(u.Old.String() + " " + u.New.String() + " " + u.Name.String() + "\n") diff --git a/cmd/objgitd/hooks_test.go b/cmd/objgitd/hooks_test.go index 3ec3f5d..e91f3d9 100644 --- a/cmd/objgitd/hooks_test.go +++ b/cmd/objgitd/hooks_test.go @@ -3,11 +3,13 @@ package main import ( "bytes" "context" + "encoding/json" "log/slog" "net" "os" "os/exec" "path/filepath" + "slices" "strings" "sync" "testing" @@ -73,7 +75,7 @@ func TestDiffRefs(t *testing.T) { } // syncBuffer is a goroutine-safe buffer for capturing slog output while the -// server and an async hook write concurrently. +// server handles a push on another goroutine. type syncBuffer struct { mu sync.Mutex buf bytes.Buffer @@ -171,6 +173,128 @@ func TestReceivePackHook(t *testing.T) { } } +func TestReceivePackHookFileChanges(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + d := &daemon{ + sysFS: memfs.New(), + resolver: repofs.BucketResolver{Base: newMemBase()}, + authz: auth.AllowAnonymous{AllowWrite: true}, + allowHooks: true, + hookTimeout: 30 * time.Second, + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + go func() { _ = d.ServeGitProtocol(ctx, ln) }() + remote := "git://" + ln.Addr().String() + "/acme/changes.git" + + work := t.TempDir() + runGit(t, work, "init", "-b", "main") + runGit(t, work, "config", "user.email", "test@example.com") + runGit(t, work, "config", "user.name", "Test") + writeFile(t, filepath.Join(work, ".objgit", "hooks", "receive-pack"), strings.Join([]string{ + `printf 'ADDED=%s\n' "$OBJGIT_ADDED_FILES_JSON"`, + `printf 'CHANGED=%s\n' "$OBJGIT_CHANGED_FILES_JSON"`, + `printf 'DELETED=%s\n' "$OBJGIT_DELETED_FILES_JSON"`, + `printf 'DOCUMENT=%s\n' "$(cat "$OBJGIT_CHANGES_FILE")"`, + }, "\n")+"\n") + + // The second push contains two commits. A path added and then deleted + // within that push must not appear in the net file lists. + for _, tt := range []struct { + name string + prepare func(t *testing.T) + wantAdded []string + wantChanged []string + wantDeleted []string + }{ + { + name: "new branch compares against empty tree", + prepare: func(t *testing.T) { + writeFile(t, filepath.Join(work, "changed.txt"), "before\n") + writeFile(t, filepath.Join(work, "deleted.txt"), "before\n") + }, + wantAdded: []string{".objgit/hooks/receive-pack", "changed.txt", "deleted.txt"}, + }, + { + name: "updated branch reports escaped paths and net changes", + prepare: func(t *testing.T) { + writeFile(t, filepath.Join(work, "changed.txt"), "after\n") + writeFile(t, filepath.Join(work, "a b\nc.txt"), "new\n") + writeFile(t, filepath.Join(work, "transient.txt"), "temporary\n") + runGit(t, work, "add", ".") + runGit(t, work, "commit", "-m", "add transient") + if err := os.Remove(filepath.Join(work, "transient.txt")); err != nil { + t.Fatalf("remove transient: %v", err) + } + if err := os.Remove(filepath.Join(work, "deleted.txt")); err != nil { + t.Fatalf("remove deleted: %v", err) + } + }, + wantAdded: []string{"a b\nc.txt"}, + wantChanged: []string{"changed.txt"}, + wantDeleted: []string{"deleted.txt"}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + tt.prepare(t) + runGit(t, work, "add", "-A") + runGit(t, work, "commit", "-m", tt.name) + pushOut := runGit(t, work, "push", remote, "main") + + want := map[string][]string{ + "ADDED": tt.wantAdded, + "CHANGED": tt.wantChanged, + "DELETED": tt.wantDeleted, + } + for key, paths := range want { + if paths == nil { + paths = []string{} + } + encoded, err := json.Marshal(paths) + if err != nil { + t.Fatalf("marshal expected %s: %v", key, err) + } + if !strings.Contains(pushOut, key+"="+string(encoded)) { + t.Errorf("push output lacks %s=%s; output:\n%s", key, encoded, pushOut) + } + } + + var document struct { + Added []string `json:"added"` + Changed []string `json:"changed"` + Deleted []string `json:"deleted"` + } + found := false + for _, line := range strings.Split(pushOut, "\n") { + if _, raw, ok := strings.Cut(line, "DOCUMENT="); ok { + found = true + if err := json.Unmarshal([]byte(strings.TrimSpace(raw)), &document); err != nil { + t.Fatalf("parse changes file %q: %v", raw, err) + } + } + } + if !found { + t.Fatalf("hook did not print changes file; output:\n%s", pushOut) + } + for key, got := range map[string][]string{"added": document.Added, "changed": document.Changed, "deleted": document.Deleted} { + if got == nil { + t.Errorf("changes file %s is missing or null; want a JSON array", key) + } + if !slices.Equal(got, want[strings.ToUpper(key)]) { + t.Errorf("changes file %s = %q, want %q", key, got, want[strings.ToUpper(key)]) + } + } + }) + } +} + // TestReceivePackHookAbsent confirms a push with no hook file is a no-op (push // still succeeds, nothing logged as a hook run). func TestReceivePackHookAbsent(t *testing.T) { diff --git a/cmd/objgitd/main.go b/cmd/objgitd/main.go index b970979..a78f0e1 100644 --- a/cmd/objgitd/main.go +++ b/cmd/objgitd/main.go @@ -26,6 +26,7 @@ import ( "github.com/tigrisdata/objgit/internal/repofs" "github.com/tigrisdata/objgit/internal/s3fs" "github.com/tigrisdata/objgit/internal/storage/tigris" + "github.com/tigrisdata/objgit/internal/webhook" tstorage "github.com/tigrisdata/storage-go" "golang.org/x/sync/errgroup" @@ -40,8 +41,9 @@ var ( allowPush = flag.Bool("allow-push", false, "allow unauthenticated git-receive-pack (push) requests") slogLevel = flag.String("slog-level", "INFO", "log level (DEBUG, INFO, WARN, ERROR)") - allowHooks = flag.Bool("allow-hooks", false, "run .objgit/hooks/receive-pack in a sandbox after a successful push") - hookTimeout = flag.Duration("hook-timeout", 60*time.Second, "wall-clock limit for a single hook run") + allowHooks = flag.Bool("allow-hooks", false, "run .objgit/hooks/receive-pack in a sandbox after a successful push") + hookTimeout = flag.Duration("hook-timeout", 60*time.Second, "wall-clock limit for a single hook run") + webhookConfig = flag.String("webhook-config", "", "path to JSON per-repository webhook endpoints and signing secrets; empty disables webhooks") packCacheDir = flag.String("pack-cache-dir", "", "parent directory for the local pack cache; empty uses the OS temp directory") packCacheBytes = flag.Int64("pack-cache-bytes", 2<<30, "disk budget for the local pack cache, least-recently-used eviction; 0 disables caching") @@ -82,6 +84,11 @@ func main() { slog.Error("at least one of -http-bind or -ssh-bind must be set") os.Exit(1) } + webhooks, err := webhook.Load(*webhookConfig) + if err != nil { + slog.Error("can't load webhook configuration", "path", *webhookConfig, "err", err) + os.Exit(1) + } ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer cancel() @@ -145,6 +152,7 @@ func main() { authz: auth.AllowAnonymous{AllowWrite: *allowPush}, allowHooks: *allowHooks, hookTimeout: *hookTimeout, + webhooks: webhooks, pushes: newPushLimiter(*maxConcurrentPushes, *pushQueueTimeout), } @@ -156,6 +164,7 @@ func main() { "bucket", *bucket, "allow_push", *allowPush, "allow_hooks", *allowHooks, + "webhook_configured", *webhookConfig != "", "pack_cache_bytes", *packCacheBytes, "max_concurrent_pushes", *maxConcurrentPushes, "push_queue_timeout", *pushQueueTimeout, diff --git a/cmd/objgitd/receivepack.go b/cmd/objgitd/receivepack.go index dcb8cf8..3982c4e 100644 --- a/cmd/objgitd/receivepack.go +++ b/cmd/objgitd/receivepack.go @@ -3,8 +3,11 @@ package main import ( "bufio" "context" + "errors" "fmt" "io" + "log/slog" + "time" "github.com/go-git/go-git/v6/config" "github.com/go-git/go-git/v6/plumbing" @@ -28,7 +31,8 @@ import ( // capabilities are known and before the packfile is read, and the slot it // returns is held until this function returns, which covers unpacking, the // ref update, and any hooks. A nil admit means unlimited. -// - onUpdated runs after refs are updated and report-status is sent, but +// - onUpdated receives only the ref commands this request committed. It runs +// after refs are updated and report-status is attempted, but // *before* the closing sideband flush-pkt. go-git keeps its sideband Muxer // internal and flushes it before returning, so there is no other way to // stream "remote:" progress to the client. The seam hands back a band-2 @@ -52,7 +56,7 @@ func receivePackStreaming( w io.WriteCloser, opts *transport.ReceivePackRequest, admit admitFunc, - onUpdated func(progress io.Writer), + onUpdated func(progress io.Writer, updates []refUpdate, acceptedAt time.Time), ) error { if w == nil { return fmt.Errorf("nil writer") @@ -186,22 +190,29 @@ func receivePackStreaming( var firstErr error cmdStatus := make(map[plumbing.ReferenceName]error) - updateReferences(st, updreq, cmdStatus, &firstErr) + acceptedAt := updateReferences(st, updreq, cmdStatus, &firstErr) - if err := sendReportStatus(writeCloser, firstErr, cmdStatus); err != nil { - return err - } + // Record the commands that landed in this request, rather than diffing + // repository-wide snapshots that can include a concurrent push. + updates := successfulRefUpdates(updreq, cmdStatus) + statusErr := sendReportStatus(writeCloser, firstErr, cmdStatus) - // Stream hook output over the sideband progress channel before the closing - // flush-pkt; once the client sees that flush it stops reading the sideband. - if onUpdated != nil { + // A failed status write does not roll back committed refs. Run the callback + // anyway so a client disconnect does not suppress a webhook or push hook. + if onUpdated != nil && len(updates) > 0 { var progress io.Writer - if useSideband { + if statusErr == nil && useSideband { progress = sidebandProgress{mux: mux} } - onUpdated(progress) + onUpdated(progress, updates, acceptedAt) + } + + if statusErr != nil { + return statusErr } + // Stream hook output over the sideband progress channel before the closing + // flush-pkt; once the client sees that flush it stops reading the sideband. if useSideband { if err := pktline.WriteFlush(w); err != nil { return fmt.Errorf("flushing sideband: %w", err) @@ -213,6 +224,22 @@ func receivePackStreaming( return closeWriter(w) } +// successfulRefUpdates preserves command order and excludes rejected, failed, +// and no-op commands. The old IDs were validated against the ref values at +// commit, so they describe the actual transition instead of a stale +// advertisement. +func successfulRefUpdates(req *packp.UpdateRequests, status map[plumbing.ReferenceName]error) []refUpdate { + var updates []refUpdate + for _, cmd := range req.Commands { + err, ok := status[cmd.Name] + if !ok || err != nil || cmd.Old == cmd.New { + continue + } + updates = append(updates, refUpdate{Name: cmd.Name, Old: cmd.Old, New: cmd.New}) + } + return updates +} + // writePack stores the incoming packfile as a single packfile object via the // storer's PackfileWriter, delimiting the pack with a Scanner rather than waiting // for the reader to reach io.EOF. go-git's default PackfileWriter path @@ -304,13 +331,38 @@ func setStatus(cmdStatus map[plumbing.ReferenceName]error, firstErr *error, ref } } -func referenceExists(s storer.ReferenceStorer, n plumbing.ReferenceName) (bool, error) { - _, err := s.Reference(n) - if err == plumbing.ErrReferenceNotFound { - return false, nil +// validateRefCommand checks the advertised old hash against the current ref. +// Packed refs repeat this check inside their atomic CAS update. +func validateRefCommand(st storage.Storer, cmd *packp.Command) error { + current, err := st.Reference(cmd.Name) + if err != nil && !errors.Is(err, plumbing.ErrReferenceNotFound) { + return err + } + exists := err == nil + switch cmd.Action() { + case packp.Create: + if exists { + return transport.ErrUpdateReference + } + case packp.Update, packp.Delete: + if !exists || current.Type() != plumbing.HashReference || current.Hash() != cmd.Old { + return transport.ErrUpdateReference + } + default: + return transport.ErrUpdateReference } + return nil +} - return err == nil, err +// refCommandVisible checks the effective ref after a post-commit cleanup +// error. Legacy loose refs can shadow an accepted packed write; only visible +// updates should be reported as successful or dispatched. +func refCommandVisible(st storage.Storer, cmd *packp.Command) bool { + current, err := st.Reference(cmd.Name) + if cmd.New.IsZero() { + return errors.Is(err, plumbing.ErrReferenceNotFound) + } + return err == nil && current.Type() == plumbing.HashReference && current.Hash() == cmd.New } // refUpdater is the optional bulk ref-update surface. A storer that has one @@ -326,12 +378,19 @@ type refUpdater interface { UpdateReferences(sets []*plumbing.Reference, removes []plumbing.ReferenceName) error } -func updateReferences(st storage.Storer, req *packp.UpdateRequests, cmdStatus map[plumbing.ReferenceName]error, firstErr *error) { +type checkedRefUpdater interface { + SupportsCheckedRefUpdates() bool + UpdateReferencesChecked(sets []*plumbing.Reference, removes []plumbing.ReferenceName, expected map[plumbing.ReferenceName]plumbing.Hash) (time.Time, error) +} + +func updateReferences(st storage.Storer, req *packp.UpdateRequests, cmdStatus map[plumbing.ReferenceName]error, firstErr *error) time.Time { + if checked, ok := st.(checkedRefUpdater); ok && !checked.SupportsCheckedRefUpdates() { + return updateReferencesOneByOne(st, req, cmdStatus, firstErr) + } if bu, ok := st.(refUpdater); ok { - updateReferencesBatched(bu, st, req, cmdStatus, firstErr) - return + return updateReferencesBatched(bu, st, req, cmdStatus, firstErr) } - updateReferencesOneByOne(st, req, cmdStatus, firstErr) + return updateReferencesOneByOne(st, req, cmdStatus, firstErr) } // updateReferencesBatched validates every command first, then applies the @@ -347,92 +406,96 @@ func updateReferences(st storage.Storer, req *packp.UpdateRequests, cmdStatus ma // mid-loop leaves earlier commands applied. All-or-nothing is the better // behavior — it is what git push --atomic means — but report-status now // carries one shared error where it used to carry a mix. -func updateReferencesBatched(bu refUpdater, st storage.Storer, req *packp.UpdateRequests, cmdStatus map[plumbing.ReferenceName]error, firstErr *error) { +func updateReferencesBatched(bu refUpdater, st storage.Storer, req *packp.UpdateRequests, cmdStatus map[plumbing.ReferenceName]error, firstErr *error) time.Time { var ( - sets []*plumbing.Reference - removes []plumbing.ReferenceName - staged []plumbing.ReferenceName + sets []*plumbing.Reference + removes []plumbing.ReferenceName + staged []*packp.Command + expected = make(map[plumbing.ReferenceName]plumbing.Hash) ) for _, cmd := range req.Commands { - exists, err := referenceExists(st, cmd.Name) - if err != nil { + if err := validateRefCommand(st, cmd); err != nil { setStatus(cmdStatus, firstErr, cmd.Name, err) continue } switch cmd.Action() { case packp.Create: - if exists { - setStatus(cmdStatus, firstErr, cmd.Name, transport.ErrUpdateReference) - continue - } sets = append(sets, plumbing.NewHashReference(cmd.Name, cmd.New)) case packp.Delete: - if !exists { - setStatus(cmdStatus, firstErr, cmd.Name, transport.ErrUpdateReference) - continue - } removes = append(removes, cmd.Name) case packp.Update: - if !exists { - setStatus(cmdStatus, firstErr, cmd.Name, transport.ErrUpdateReference) - continue - } sets = append(sets, plumbing.NewHashReference(cmd.Name, cmd.New)) default: continue } - staged = append(staged, cmd.Name) + staged = append(staged, cmd) + expected[cmd.Name] = cmd.Old } if len(staged) == 0 { - return + return time.Time{} + } + + var err error + var acceptedAt time.Time + if checked, ok := st.(checkedRefUpdater); ok && checked.SupportsCheckedRefUpdates() { + acceptedAt, err = checked.UpdateReferencesChecked(sets, removes, expected) + } else { + err = bu.UpdateReferences(sets, removes) + acceptedAt = time.Now() + } + var postCommit interface{ RefUpdateCommitted() bool } + if err != nil && errors.As(err, &postCommit) && postCommit.RefUpdateCommitted() { + slog.Warn("ref cleanup failed after packed update", "err", err) + for _, cmd := range staged { + if refCommandVisible(st, cmd) { + setStatus(cmdStatus, firstErr, cmd.Name, nil) + } else { + setStatus(cmdStatus, firstErr, cmd.Name, err) + } + } + return acceptedAt } - - err := bu.UpdateReferences(sets, removes) - for _, n := range staged { - setStatus(cmdStatus, firstErr, n, err) + for _, cmd := range staged { + setStatus(cmdStatus, firstErr, cmd.Name, err) } + return acceptedAt } // updateReferencesOneByOne is the pre-batch path, kept for any storer without // a refUpdater — memory.Storage in the tests, most notably. -func updateReferencesOneByOne(st storage.Storer, req *packp.UpdateRequests, cmdStatus map[plumbing.ReferenceName]error, firstErr *error) { +func updateReferencesOneByOne(st storage.Storer, req *packp.UpdateRequests, cmdStatus map[plumbing.ReferenceName]error, firstErr *error) time.Time { + var acceptedAt time.Time for _, cmd := range req.Commands { - exists, err := referenceExists(st, cmd.Name) - if err != nil { + if err := validateRefCommand(st, cmd); err != nil { setStatus(cmdStatus, firstErr, cmd.Name, err) continue } switch cmd.Action() { case packp.Create: - if exists { - setStatus(cmdStatus, firstErr, cmd.Name, transport.ErrUpdateReference) - continue - } - ref := plumbing.NewHashReference(cmd.Name, cmd.New) err := st.SetReference(ref) setStatus(cmdStatus, firstErr, cmd.Name, err) - case packp.Delete: - if !exists { - setStatus(cmdStatus, firstErr, cmd.Name, transport.ErrUpdateReference) - continue + if err == nil { + acceptedAt = time.Now() } - + case packp.Delete: err := st.RemoveReference(cmd.Name) setStatus(cmdStatus, firstErr, cmd.Name, err) - case packp.Update: - if !exists { - setStatus(cmdStatus, firstErr, cmd.Name, transport.ErrUpdateReference) - continue + if err == nil { + acceptedAt = time.Now() } - + case packp.Update: ref := plumbing.NewHashReference(cmd.Name, cmd.New) err := st.SetReference(ref) setStatus(cmdStatus, firstErr, cmd.Name, err) + if err == nil { + acceptedAt = time.Now() + } } } + return acceptedAt } diff --git a/cmd/objgitd/refbatch_test.go b/cmd/objgitd/refbatch_test.go index 7614272..dd5b930 100644 --- a/cmd/objgitd/refbatch_test.go +++ b/cmd/objgitd/refbatch_test.go @@ -4,6 +4,7 @@ import ( "errors" "net/http/httptest" "os/exec" + "slices" "strconv" "sync" "testing" @@ -25,11 +26,13 @@ type batchingStorer struct { // mu guards the recorded fields. The end-to-end test in this file reads // them from the test goroutine while a server goroutine may still be // unwinding, which -race notices without it. - mu sync.Mutex - calls int - sets []*plumbing.Reference - removes []plumbing.ReferenceName - err error + mu sync.Mutex + calls int + sets []*plumbing.Reference + removes []plumbing.ReferenceName + err error + afterCommitErr bool + shadow map[plumbing.ReferenceName]plumbing.Hash } // record reports what the storer has seen: batch calls, total sets, total @@ -48,7 +51,7 @@ func (b *batchingStorer) UpdateReferences(sets []*plumbing.Reference, removes [] failWith := b.err b.mu.Unlock() - if failWith != nil { + if failWith != nil && !b.afterCommitErr { return failWith } for _, r := range sets { @@ -61,9 +64,18 @@ func (b *batchingStorer) UpdateReferences(sets []*plumbing.Reference, removes [] return err } } - return nil + for name, hash := range b.shadow { + if err := b.Storer.SetReference(plumbing.NewHashReference(name, hash)); err != nil { + return err + } + } + return failWith } +type committedTestError struct{ error } + +func (committedTestError) RefUpdateCommitted() bool { return true } + // batchBase is memBase with a batch ref-update surface on every repository, so // an end-to-end push takes the refUpdater path. memBase itself hands out a bare // memory.Storage, which has no such surface and would take the fallback. @@ -109,6 +121,48 @@ func hashOf(t *testing.T, hex string) plumbing.Hash { return h } +func TestSuccessfulRefUpdates(t *testing.T) { + old := plumbing.NewHash("1111111111111111111111111111111111111111") + newHash := plumbing.NewHash("2222222222222222222222222222222222222222") + commands := []*packp.Command{ + {Name: "refs/heads/main", Old: old, New: newHash}, + {Name: "refs/heads/rejected", Old: old, New: newHash}, + {Name: "refs/heads/unknown", Old: old, New: newHash}, + {Name: "refs/heads/unchanged", Old: old, New: old}, + {Name: "refs/tags/v1", Old: plumbing.ZeroHash, New: newHash}, + } + for _, tt := range []struct { + name string + status map[plumbing.ReferenceName]error + want []refUpdate + }{ + { + name: "successful commands only, in request order", + status: map[plumbing.ReferenceName]error{ + "refs/heads/main": nil, + "refs/heads/rejected": errors.New("rejected"), + "refs/heads/unchanged": nil, + "refs/tags/v1": nil, + }, + want: []refUpdate{ + {Name: "refs/heads/main", Old: old, New: newHash}, + {Name: "refs/tags/v1", Old: plumbing.ZeroHash, New: newHash}, + }, + }, + { + name: "no accepted commands", + status: map[plumbing.ReferenceName]error{}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + updates := successfulRefUpdates(&packp.UpdateRequests{Commands: commands}, tt.status) + if !slices.Equal(updates, tt.want) { + t.Errorf("updates = %v, want %v", updates, tt.want) + } + }) + } +} + func TestUpdateReferencesBatchesWhenTheStorerCan(t *testing.T) { t.Parallel() @@ -194,6 +248,14 @@ func TestUpdateReferencesBatchesWhenTheStorerCan(t *testing.T) { wantCalls: 0, wantStatus: map[string]error{"refs/heads/main": transport.ErrUpdateReference}, }, + { + name: "stale advertised old hash is rejected", + seed: map[string]string{"refs/heads/main": hexA}, + commands: []*packp.Command{ + {Name: "refs/heads/main", Old: hashOf(t, hexB), New: hashOf(t, hexA)}, + }, + wantStatus: map[string]error{"refs/heads/main": transport.ErrUpdateReference}, + }, } for _, tt := range tests { @@ -242,6 +304,40 @@ func TestUpdateReferencesBatchesWhenTheStorerCan(t *testing.T) { } } +func TestPostCommitCleanupDispatchesVisibleRefs(t *testing.T) { + old := plumbing.NewHash("1111111111111111111111111111111111111111") + newHash := plumbing.NewHash("2222222222222222222222222222222222222222") + mem := memory.NewStorage() + if err := mem.SetReference(plumbing.NewHashReference("refs/heads/main", old)); err != nil { + t.Fatal(err) + } + cleanupErr := committedTestError{errors.New("loose-ref cleanup failed")} + st := &batchingStorer{ + Storer: mem, err: cleanupErr, afterCommitErr: true, + shadow: map[plumbing.ReferenceName]plumbing.Hash{"refs/heads/main": old}, + } + req := &packp.UpdateRequests{Commands: []*packp.Command{ + {Name: "refs/heads/topic", Old: plumbing.ZeroHash, New: newHash}, + {Name: "refs/heads/main", Old: old, New: newHash}, + }} + status := map[plumbing.ReferenceName]error{} + var firstErr error + updateReferences(st, req, status, &firstErr) + if status["refs/heads/topic"] != nil { + t.Errorf("visible new ref was rejected: %v", status["refs/heads/topic"]) + } + if !errors.Is(status["refs/heads/main"], cleanupErr) { + t.Errorf("shadowed update status = %v, want cleanup error", status["refs/heads/main"]) + } + if !errors.Is(firstErr, cleanupErr) { + t.Errorf("first error = %v, want cleanup error", firstErr) + } + updates := successfulRefUpdates(req, status) + if !slices.Equal(updates, []refUpdate{{Name: "refs/heads/topic", Old: plumbing.ZeroHash, New: newHash}}) { + t.Errorf("dispatched updates = %v", updates) + } +} + // TestUpdateReferencesFallsBackWithoutTheInterface pins that a storer with no // batch surface still works, one ref at a time. func TestUpdateReferencesFallsBackWithoutTheInterface(t *testing.T) { diff --git a/cmd/objgitd/webhooks.go b/cmd/objgitd/webhooks.go new file mode 100644 index 0000000..b46c2e8 --- /dev/null +++ b/cmd/objgitd/webhooks.go @@ -0,0 +1,66 @@ +package main + +import ( + "context" + "crypto/rand" + "encoding/hex" + "log/slog" + "time" + + "github.com/go-git/go-git/v6/storage" + "github.com/tigrisdata/objgit/internal/metrics" + "github.com/tigrisdata/objgit/internal/pushevents" +) + +// emitPushWebhooks builds one event for each ref changed by this receive-pack. +// Delivery runs after the ref update and cannot reject or undo it. A detached, +// bounded context lets an event finish even if the Git client disconnects +// after the server has committed the refs. +func (d *daemon) emitPushWebhooks(ctx context.Context, st storage.Storer, repo string, updates []refUpdate, acceptedAt time.Time) { + if d.webhooks == nil || !d.webhooks.Enabled(repo) || len(updates) == 0 { + return + } + + pushID, err := newEventID() + if err != nil { + slog.Error("webhook: create push ID", "repo", repo, "err", err) + return + } + + for _, u := range updates { + eventID, err := newEventID() + if err != nil { + slog.Error("webhook: create event ID", "repo", repo, "ref", u.Name.String(), "err", err) + continue + } + + eventCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) + start := time.Now() + event, err := pushevents.Build(eventCtx, st, repo, pushevents.Update{ + Name: u.Name, + Old: u.Old, + New: u.New, + }, pushID, eventID, acceptedAt) + if err == nil { + err = d.webhooks.Deliver(eventCtx, repo, event) + } + cancel() + + status := "ok" + if err != nil { + status = "error" + slog.Error("webhook: delivery failed", "repo", repo, "ref", u.Name.String(), "event_id", eventID, "err", err) + } else { + slog.Info("webhook: delivered", "repo", repo, "ref", u.Name.String(), "event_id", eventID) + } + metrics.ObserveWebhook(status, time.Since(start)) + } +} + +func newEventID() (string, error) { + var id [16]byte + if _, err := rand.Read(id[:]); err != nil { + return "", err + } + return hex.EncodeToString(id[:]), nil +} diff --git a/cmd/objgitd/webhooks_test.go b/cmd/objgitd/webhooks_test.go new file mode 100644 index 0000000..a249752 --- /dev/null +++ b/cmd/objgitd/webhooks_test.go @@ -0,0 +1,244 @@ +package main + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "slices" + "strings" + "testing" + "time" + + "github.com/go-git/go-billy/v6/memfs" + pushv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/events/push/v1" + "github.com/tigrisdata/objgit/internal/auth" + "github.com/tigrisdata/objgit/internal/repofs" + "github.com/tigrisdata/objgit/internal/webhook" + "google.golang.org/protobuf/encoding/protojson" +) + +func TestSmartHTTPPushWebhook(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + type received struct { + event *pushv1.PushEvent + delivery string + eventType string + } + events := make(chan received, 3) + nextEvent := func() received { + t.Helper() + select { + case event := <-events: + return event + case <-time.After(3 * time.Second): + t.Fatal("timed out waiting for push webhook") + return received{} + } + } + destination := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("read webhook: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + var event pushv1.PushEvent + if err := protojson.Unmarshal(body, &event); err != nil { + t.Errorf("decode webhook: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + events <- received{&event, r.Header.Get("X-Objgit-Delivery"), r.Header.Get("X-Objgit-Event")} + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(destination.Close) + + config, err := json.Marshal(map[string]any{"repositories": map[string]any{ + "acme/webhook": map[string]string{"url": destination.URL, "secret": "test-secret"}, + }}) + if err != nil { + t.Fatal(err) + } + configPath := filepath.Join(t.TempDir(), "webhooks.json") + if err := os.WriteFile(configPath, config, 0600); err != nil { + t.Fatal(err) + } + client, err := webhook.Load(configPath) + if err != nil { + t.Fatal(err) + } + d := &daemon{ + sysFS: memfs.New(), + resolver: repofs.BucketResolver{Base: newMemBase()}, + authz: auth.AllowAnonymous{AllowWrite: true}, + webhooks: client, + } + server := httptest.NewServer(d.httpHandler()) + t.Cleanup(server.Close) + remote := server.URL + "/acme/webhook.git" + + work := t.TempDir() + runGit(t, work, "init", "-b", "main") + runGit(t, work, "config", "user.email", "test@example.com") + runGit(t, work, "config", "user.name", "Test") + writeFile(t, filepath.Join(work, "readme.txt"), "first\n") + runGit(t, work, "add", ".") + runGit(t, work, "commit", "-m", "first") + first := strings.TrimSpace(runGit(t, work, "rev-parse", "HEAD")) + writeFile(t, filepath.Join(work, "readme.txt"), "second\n") + writeFile(t, filepath.Join(work, "transient.txt"), "temporary\n") + runGit(t, work, "add", ".") + runGit(t, work, "commit", "-m", "second") + second := strings.TrimSpace(runGit(t, work, "rev-parse", "HEAD")) + + if out, err := tryGit(work, "push", remote, "main"); err != nil { + t.Fatalf("first push: %v\n%s", err, out) + } + firstEvent := nextEvent() + if firstEvent.event.GetRef() != "refs/heads/main" || !firstEvent.event.GetCreated() { + t.Errorf("first event ref/created = %s/%v", firstEvent.event.GetRef(), firstEvent.event.GetCreated()) + } + if got := commitIDs(firstEvent.event); !slices.Equal(got, []string{first, second}) { + t.Errorf("first push commits = %v, want [%s %s]", got, first, second) + } + if got := firstEvent.event.GetFiles().GetAdded(); !slices.Equal(got, []string{"readme.txt", "transient.txt"}) { + t.Errorf("first push net added files = %v", got) + } + + if err := os.Remove(filepath.Join(work, "transient.txt")); err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(work, "readme.txt"), "third\n") + runGit(t, work, "add", "-A") + runGit(t, work, "commit", "-m", "third") + third := strings.TrimSpace(runGit(t, work, "rev-parse", "HEAD")) + writeFile(t, filepath.Join(work, "new.txt"), "new\n") + runGit(t, work, "add", ".") + runGit(t, work, "commit", "-m", "fourth") + fourth := strings.TrimSpace(runGit(t, work, "rev-parse", "HEAD")) + runGit(t, work, "tag", "v1") + + if out, err := tryGit(work, "push", remote, "main", "refs/tags/v1"); err != nil { + t.Fatalf("second push: %v\n%s", err, out) + } + got := map[string]received{} + for range 2 { + rec := nextEvent() + got[rec.event.GetRef()] = rec + if rec.delivery != rec.event.GetEventId() || rec.eventType != "push" { + t.Errorf("headers do not match event: delivery=%q event=%q type=%q", rec.delivery, rec.event.GetEventId(), rec.eventType) + } + } + branch := got["refs/heads/main"].event + tag := got["refs/tags/v1"].event + if branch == nil || tag == nil { + t.Fatalf("second push refs = %v", got) + } + if branch.GetPushId() == "" || branch.GetPushId() != tag.GetPushId() || branch.GetEventId() == tag.GetEventId() { + t.Errorf("second push IDs: branch=%s/%s tag=%s/%s", branch.GetPushId(), branch.GetEventId(), tag.GetPushId(), tag.GetEventId()) + } + if !branch.GetPushedAt().AsTime().Equal(tag.GetPushedAt().AsTime()) { + t.Errorf("events from one ref update batch have different acceptance times: branch=%v tag=%v", branch.GetPushedAt(), tag.GetPushedAt()) + } + for _, tt := range []struct { + name string + event *pushv1.PushEvent + want []string + }{ + {"branch update", branch, []string{third, fourth}}, + {"tag creation", tag, []string{first, second, third, fourth}}, + } { + t.Run(tt.name, func(t *testing.T) { + if ids := commitIDs(tt.event); !slices.Equal(ids, tt.want) { + t.Errorf("commits = %v, want %v", ids, tt.want) + } + }) + } + if !slices.Equal(branch.GetFiles().GetAdded(), []string{"new.txt"}) || + !slices.Equal(branch.GetFiles().GetChanged(), []string{"readme.txt"}) || + !slices.Equal(branch.GetFiles().GetDeleted(), []string{"transient.txt"}) { + t.Errorf("branch net files = %+v", branch.GetFiles()) + } +} + +func commitIDs(event *pushv1.PushEvent) []string { + ids := make([]string, 0, len(event.GetCommits())) + for _, commit := range event.GetCommits() { + ids = append(ids, commit.GetId()) + } + return ids +} + +func TestPushWebhookAcceptanceTimePrecedesHook(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + type delivery struct { + pushedAt time.Time + received time.Time + } + deliveries := make(chan delivery, 1) + destination := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + var event pushv1.PushEvent + if err := protojson.Unmarshal(body, &event); err != nil { + w.WriteHeader(http.StatusBadRequest) + return + } + deliveries <- delivery{pushedAt: event.GetPushedAt().AsTime(), received: time.Now()} + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(destination.Close) + + config, err := json.Marshal(map[string]any{"repositories": map[string]any{ + "acme/timed": map[string]string{"url": destination.URL, "secret": "test-secret"}, + }}) + if err != nil { + t.Fatal(err) + } + configPath := filepath.Join(t.TempDir(), "webhooks.json") + if err := os.WriteFile(configPath, config, 0600); err != nil { + t.Fatal(err) + } + client, err := webhook.Load(configPath) + if err != nil { + t.Fatal(err) + } + server := httptest.NewServer((&daemon{ + sysFS: memfs.New(), resolver: repofs.BucketResolver{Base: newMemBase()}, + authz: auth.AllowAnonymous{AllowWrite: true}, webhooks: client, + allowHooks: true, hookTimeout: 5 * time.Second, + }).httpHandler()) + t.Cleanup(server.Close) + + work := t.TempDir() + runGit(t, work, "init", "-b", "main") + runGit(t, work, "config", "user.email", "test@example.com") + runGit(t, work, "config", "user.name", "Test") + writeFile(t, filepath.Join(work, ".objgit", "hooks", "receive-pack"), "sleep 1\n") + writeFile(t, filepath.Join(work, "readme.txt"), "hello\n") + runGit(t, work, "add", ".") + runGit(t, work, "commit", "-m", "initial") + if out, err := tryGit(work, "push", server.URL+"/acme/timed.git", "main"); err != nil { + t.Fatalf("push: %v\n%s", err, out) + } + select { + case got := <-deliveries: + if elapsed := got.received.Sub(got.pushedAt); elapsed < 500*time.Millisecond { + t.Errorf("pushedAt lags hook execution: delivery arrived only %v after acceptance", elapsed) + } + case <-time.After(3 * time.Second): + t.Fatal("timed out waiting for webhook") + } +} diff --git a/docs/architecture/README.md b/docs/architecture/README.md index 97c34a3..504eb9e 100644 --- a/docs/architecture/README.md +++ b/docs/architecture/README.md @@ -34,7 +34,7 @@ Three behaviors live on `*daemon`, so every transport acts the same way: | ------------------------------------ | ---------------------------------------------------------------- | | [transports.md](transports.md) | Smart HTTP, git://, and SSH. Two protocol points that are easy to get wrong. | | [auth.md](auth.md) | The one authorization interface that every transport calls. | -| [hooks.md](hooks.md) | Push hooks, live output streaming, and the kefka sandbox. | +| [hooks.md](hooks.md) | Push hooks, webhook dispatch, live output streaming, and the kefka sandbox. | | [metrics.md](metrics.md) | Prometheus vectors and the three instrumentation seams. | | [tigris-storer.md](tigris-storer.md) | Repository storage: a `storage.Storer` on one Tigris bucket. | | [s3fs.md](s3fs.md) | Daemon-level state: a `billy.Filesystem` on Tigris. | @@ -44,4 +44,5 @@ Related documents outside this directory: - [../reference/tigris-backend.md](../reference/tigris-backend.md) — the `.cue` binary layout, the failure modes, and the build order. - [../usage/hooks.md](../usage/hooks.md) — how to write a hook script. +- [../usage/webhooks.md](../usage/webhooks.md) — how to configure and verify push webhooks. - `../plans/` — plans for work that is not yet complete. diff --git a/docs/architecture/hooks.md b/docs/architecture/hooks.md index 3d8472c..b03f2cc 100644 --- a/docs/architecture/hooks.md +++ b/docs/architecture/hooks.md @@ -21,8 +21,8 @@ Live streaming needs a seam that go-git does not expose. the closing flush-pkt before it returns. `cmd/objgitd/receivepack.go` holds a small fork, `receivePackStreaming`. The -fork adds one callback, `onUpdated(progress io.Writer)`, which runs after -report-status and before that final flush. +fork adds one callback, `onUpdated(progress io.Writer, updates []refUpdate, acceptedAt time.Time)`, +which runs after report-status is attempted and before that final flush. `progress` writes to the sideband `ProgressMessage` channel, which is band 2, when the client negotiated sideband. Otherwise `progress` is `nil`. Hook @@ -34,10 +34,15 @@ exit status goes to `slog` in both cases. All three transports call `d.receivePack` in `hooks.go`, which drives the fork. -`transport.ReceivePack` does not report which refs it changed. `onUpdated` -therefore snapshots the branch refs before and after the push, then diffs -them with `snapshotRefs` and `diffRefs`. Each hook then runs synchronously and -streams through `progress`. +`transport.ReceivePack` does not report which refs it changed. The fork passes +the successful ref commands from the current request to `onUpdated`, with a +timestamp captured immediately after the ref update. Packed-ref writes check +each advertised old hash at the atomic commit point, including after a CAS +retry. If legacy loose-ref cleanup fails after the packed write, receive-pack +checks which updates are visible before dispatch. Shell hooks +run for updated branches, and push webhooks run for every updated ref with a +configured destination. Both run synchronously after the ref update and cannot +reject it. See [../usage/webhooks.md](../usage/webhooks.md) for delivery details. HTTP needs one extra piece. It wraps its `ResponseWriter` in `flushWriter` (`http.go`), a flush-on-write writer, so `net/http` buffering does not hold diff --git a/docs/architecture/metrics.md b/docs/architecture/metrics.md index cbea291..9d63954 100644 --- a/docs/architecture/metrics.md +++ b/docs/architecture/metrics.md @@ -24,6 +24,7 @@ The package exposes thin helpers, so no call site carries label plumbing: | `TrackInFlight` | Returns a deferred decrement. | | `ObserveAuth` | Maps the `auth` enums to labels. | | `ObserveHook` | One hook run. | +| `ObserveWebhook` | One push webhook event. | | `ReposCreated` | A new repository. | | `TrackPushWait` | Returns a deferred decrement. | | `TrackPushSlot` | Returns a deferred decrement. | @@ -78,3 +79,8 @@ keyed by protocol and service. HTTP folds an authorization denial into the `error` git status. The exact denial stays visible in `objgit_auth_requests_total`. git:// and SSH record `denied` directly. + +Webhook delivery reports `objgit_webhook_deliveries_total` by final status +(`ok` or `error`) and `objgit_webhook_delivery_duration_seconds` for event +construction and delivery, including retries. Neither metric uses a repository +label. diff --git a/docs/plans/webhook-push-events.md b/docs/plans/webhook-push-events.md new file mode 100644 index 0000000..99f2808 --- /dev/null +++ b/docs/plans/webhook-push-events.md @@ -0,0 +1,51 @@ +# Plan: push webhook events + +## Contract + +The event schema lives at +`proto/tigrisdata/objgit/events/push/v1/push.proto`. The Buf module root is +`proto/`, so later event packages can sit beside `events/push/v1`. A raw +ProtoJSON example lives next to the schema. Message and field comments provide +the starting descriptions for future gnostic OpenAPI annotations and generated +documentation. The schema does not import gnostic until that generation path is +chosen. + +One accepted ref update produces one `PushEvent`. Multiple ref updates in one +receive-pack request share a `push_id`; each event has its own `event_id`, which +stays fixed on retries. `commits` contains every commit newly reachable from +the new tip relative to the old tip, ordered with parents before children. +Each commit's file lists compare it with its first parent. The event's `files` +summarizes the net change between old and new trees. Deletes have no commits. + +The JSON example includes two commits and all three file-change categories. +Delivery uses ProtoJSON and emits default scalar/list values so receivers see +explicit `false` and empty arrays as in the example. + +## Implementation + +1. Make receive-pack return the ref commands that actually committed, rather + than attributing a before/after repository snapshot to a request. Check + advertised old hashes at the packed-refs commit point. After a loose-ref + cleanup error, dispatch only updates visible in the effective ref view. +2. Walk each updated ref's commit graph. Exclude commits reachable from the old + tip, sort the remainder with parents before children, and diff each commit + against its first parent. Diff the old and new trees once more for `files`. + Share that net diff with the per-ref shell hook metadata. +3. Resolve operator-owned webhook destinations and secrets. Serialize the + event with ProtoJSON, sign the exact request bytes, and send with bounded + timeout and retries. Keep a delivery ID stable across attempts. +4. Large events are currently sent whole. Event construction has a 30-second + deadline per ref. A push too large to build within that limit logs a delivery + failure; it does not silently truncate the commit or path lists. A future + continuation or retrieval API can provide a better way to handle these + pushes. +5. Protocol and package tests cover multi-commit pushes, ref changes, commit + graph traversal, file metadata, signing, and delivery outcomes. + +Delivery is currently best effort. It has bounded retries during the Git +request, but no durable queue or replay after daemon shutdown. Operators can +observe failures in logs and `objgit_webhook_deliveries_total`. + +Run `buf lint` and `buf build` when editing the schema. Once an earlier schema +exists, run `buf breaking --against '.git#branch=main'`. `WIRE_JSON` is the +compatibility target because webhook deliveries use ProtoJSON. diff --git a/docs/usage/hooks.md b/docs/usage/hooks.md index ccc25f5..08a66b9 100644 --- a/docs/usage/hooks.md +++ b/docs/usage/hooks.md @@ -26,17 +26,16 @@ push, so in practice you want both. - **On push only.** The hook is named after the git service that triggered it, and the only service that runs a hook is `receive-pack` (push). Fetches, clones, and archives never run hooks. -- **After the push completes.** Refs are already updated and the client has - already received its response when the hook starts. A hook therefore **cannot - reject a push** and its output is never shown to the person pushing — it goes - to the server log only. This is a post-receive hook, not a pre-receive gate. +- **After refs are updated.** The hook runs after Git's status report is sent, + but before the push response closes. Its stdout and stderr stream to the + pushing client as `remote:` lines when sideband is available. A hook + **cannot reject a push**. This is a post-receive hook, not a pre-receive gate. - **Once per changed branch.** If a push updates or creates several branches, the hook runs once for each, with environment variables describing that branch (see below). Branch **deletions** are skipped (there is nothing to check out). -- **Asynchronously.** The push returns immediately; the hook runs in the - background. On server shutdown, in-flight hooks are given a short grace period - to finish. +- **Synchronously.** The client waits for the hook to finish or reach + `-hook-timeout` before the push response closes. The script is read from the **commit that was just pushed**, so a hook travels with the branch — different branches can carry different hooks, and updating a @@ -80,14 +79,34 @@ redirections like `echo x > out` — must target `/tmp`. Each run gets variables describing the branch that triggered it: -| Variable | Example | Notes | -| ---------------- | ----------------- | --------------------------------------------------- | -| `OBJGIT_REPO` | `/myproject.git` | Repository path | -| `OBJGIT_SERVICE` | `receive-pack` | Always `receive-pack` | -| `OBJGIT_REF` | `refs/heads/main` | Full ref name | -| `OBJGIT_BRANCH` | `main` | Short branch name | -| `OBJGIT_OLD_SHA` | `0000…0000` | Previous tip; all zeros when the branch was created | -| `OBJGIT_NEW_SHA` | `f43417…` | New tip | +| Variable | Example | Notes | +| --------------------------- | -------------------------- | --------------------------------------------------------- | +| `OBJGIT_REPO` | `/myproject.git` | Repository path | +| `OBJGIT_SERVICE` | `receive-pack` | Always `receive-pack` | +| `OBJGIT_REF` | `refs/heads/main` | Full ref name | +| `OBJGIT_BRANCH` | `main` | Short branch name | +| `OBJGIT_OLD_SHA` | `0000…0000` | Previous tip; all zeros when the branch was created | +| `OBJGIT_NEW_SHA` | `f43417…` | New tip | +| `OBJGIT_ADDED_FILES_JSON` | `["new.txt"]` | Added paths, as a JSON array | +| `OBJGIT_CHANGED_FILES_JSON` | `["README.md"]` | Changed paths, as a JSON array | +| `OBJGIT_DELETED_FILES_JSON` | `["old.txt"]` | Deleted paths, as a JSON array | +| `OBJGIT_CHANGES_FILE` | `/tmp/objgit-changes.json` | File containing all three arrays in a single JSON object | + +The file lists describe the **net change of the branch tip** across the push. +If one commit adds a path and a later commit deletes it, that path does not +appear in these lists. A new branch compares its tip against an empty tree. +Renames appear as one added path and one deleted path. Mode-only changes +appear in `changed`. Whitespace and newlines in paths are JSON escaped; +parse the JSON rather than splitting on spaces or lines. A path with invalid +UTF-8 bytes is represented as `/objgit/raw-path/base64url/` followed by the +unpadded base64url encoding of its raw bytes. + +`OBJGIT_CHANGES_FILE` lives in the hook's writable `/tmp` filesystem and +contains the same complete lists. For example: + +```json +{"added":["new.txt"],"changed":["README.md"],"deleted":["old.txt"]} +``` For compatibility with scripts written for stock git, the same information is also fed on **stdin** as a single ` ` line. @@ -140,8 +159,8 @@ All hook activity is logged through the server's structured (`slog`) logger: - `hook: no hook file in pushed tree` (debug level) — the push had no `.objgit/hooks/receive-pack`, so nothing ran. -Because output is log-only, a hook cannot communicate back to the client that -pushed. +When sideband is unavailable, hook output is captured in the server log. +Hook failure is logged, but it cannot undo the accepted push. ## Limitations @@ -149,4 +168,4 @@ pushed. scratch space. - No way to reject a push from a hook (it runs after the fact). - No system tooling, network, or arbitrary executables — only kefka built-ins. -- Output is not relayed to the pusher. +- Hook output reaches the pusher only when the client negotiated sideband. diff --git a/docs/usage/webhooks.md b/docs/usage/webhooks.md new file mode 100644 index 0000000..4674a2a --- /dev/null +++ b/docs/usage/webhooks.md @@ -0,0 +1,70 @@ +# Push webhooks + +Set `-webhook-config` to a JSON file owned by the daemon operator. The flag +also accepts `WEBHOOK_CONFIG` through the usual environment flag mapping. +An empty value disables delivery. The daemon reads the file at startup, and +rejects invalid entries before it starts serving Git requests. + +```json +{ + "repositories": { + "acme/widgets": { + "url": "https://events.example.com/objgit", + "secret": "replace-with-a-long-random-secret" + } + } +} +``` + +Each repository has one destination. The repository key is its canonical +`org/name` path. Configure HTTPS endpoints; HTTP is accepted only for loopback +addresses. Store the config with restricted permissions because it contains +signing secrets. Restart the daemon after changing the file. + +## Event body + +An accepted ref update sends one `push` event. If one Git push updates several +refs, each ref gets an event with a shared `pushId` and a distinct `eventId`. +Each event contains **all commits newly reachable from that ref** relative to +its previous tip, ordered with parents before children. `commits[].files` +lists additions, changes, and deletions against each commit's first parent. +The top-level `files` lists the net change between the old and new trees. +For example, a file added and deleted across two commits appears in those +commits but not in the top-level list. A deleted ref has an empty commit list. + +The schema is [push.proto](../../proto/tigrisdata/objgit/events/push/v1/push.proto), +and [push.example.json](../../proto/tigrisdata/objgit/events/push/v1/push.example.json) +shows the raw JSON sent to receivers. The body is ProtoJSON with default scalar +and list values included. A rename appears as one deletion and one addition. +File contents and patches are not included. +Valid UTF-8 paths are literal. A path containing invalid UTF-8 bytes is +represented as `/objgit/raw-path/base64url/` followed by the unpadded +base64url encoding of its original bytes. Git paths cannot start with `/`, +so receivers can distinguish this representation from a literal path. +Non-UTF-8 ref names use the same representation; Git ref names also cannot +start with `/`. +Commit messages and author or committer names and email addresses use U+FFFD +for invalid UTF-8 bytes so the JSON body remains valid. + +## Headers and verification + +Each request is an HTTP `POST` with `Content-Type: application/json` and these +headers: + +| Header | Value | +| ------ | ----- | +| `X-Objgit-Event` | `push` | +| `X-Objgit-Delivery` | The event's `eventId`, unchanged on retries. | +| `X-Objgit-Signature-256` | `sha256=` followed by the lowercase hex HMAC-SHA256 of the **exact request body bytes**, using the configured secret. | + +Verify the HMAC with a constant-time comparison before processing the body. +Use `X-Objgit-Delivery` as an idempotency key: the server can retry the same +event after a network failure. A successful HTTP 2xx response completes the +delivery. The server retries network failures, HTTP 429, and HTTP 5xx up to +three attempts within five seconds. Other status codes end the delivery. + +Delivery runs after the ref update, while the Git request is still open. +Delivery failures are logged and counted in +`objgit_webhook_deliveries_total{status="error"}`; they do not reject the +accepted push. The daemon has no durable delivery queue. If it stops after +accepting a ref but before completing delivery, the event may be lost. diff --git a/gen/tigrisdata/objgit/events/push/v1/push.pb.go b/gen/tigrisdata/objgit/events/push/v1/push.pb.go new file mode 100644 index 0000000..0d9b09b --- /dev/null +++ b/gen/tigrisdata/objgit/events/push/v1/push.pb.go @@ -0,0 +1,500 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.36.8 +// protoc (unknown) +// source: tigrisdata/objgit/events/push/v1/push.proto + +package pushv1 + +import ( + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" + timestamppb "google.golang.org/protobuf/types/known/timestamppb" + reflect "reflect" + sync "sync" + unsafe "unsafe" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +// PushEvent describes one accepted Git ref update. A push that updates several +// refs produces one event per ref, with the same push_id in each event. +type PushEvent struct { + state protoimpl.MessageState `protogen:"open.v1"` + // A stable identifier for this event, reused across delivery attempts. + EventId string `protobuf:"bytes,1,opt,name=event_id,json=eventId,proto3" json:"event_id,omitempty"` + // Identifies the receive-pack request that produced this ref update. + PushId string `protobuf:"bytes,2,opt,name=push_id,json=pushId,proto3" json:"push_id,omitempty"` + // Canonical repository path, such as "acme/widgets". + Repository string `protobuf:"bytes,3,opt,name=repository,proto3" json:"repository,omitempty"` + // Full Git ref name, such as "refs/heads/main". Non-UTF-8 ref names use + // the reversible base64url representation described for FileChanges paths. + Ref string `protobuf:"bytes,4,opt,name=ref,proto3" json:"ref,omitempty"` + // Ref value before the update, or the all-zero hash for a new ref. + Before string `protobuf:"bytes,5,opt,name=before,proto3" json:"before,omitempty"` + // Ref value after the update, or the all-zero hash for a deleted ref. + After string `protobuf:"bytes,6,opt,name=after,proto3" json:"after,omitempty"` + // True when this update created the ref. + Created bool `protobuf:"varint,7,opt,name=created,proto3" json:"created,omitempty"` + // True when this update deleted the ref. + Deleted bool `protobuf:"varint,8,opt,name=deleted,proto3" json:"deleted,omitempty"` + // True for a non-fast-forward update of an existing ref; false for ref + // creation and deletion. + Forced bool `protobuf:"varint,9,opt,name=forced,proto3" json:"forced,omitempty"` + // Time the server accepted the ref update, not a commit's author time. + PushedAt *timestamppb.Timestamp `protobuf:"bytes,10,opt,name=pushed_at,json=pushedAt,proto3" json:"pushed_at,omitempty"` + // Every commit reachable from after but not reachable from before, with + // parents before children. Empty for deletion or a non-commit ref target. + Commits []*Commit `protobuf:"bytes,11,rep,name=commits,proto3" json:"commits,omitempty"` + // Net file changes from the before tree to the after tree. A file added + // and then deleted within this push does not appear here; its two changes + // still appear in the corresponding commits. + Files *FileChanges `protobuf:"bytes,12,opt,name=files,proto3" json:"files,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *PushEvent) Reset() { + *x = PushEvent{} + mi := &file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *PushEvent) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*PushEvent) ProtoMessage() {} + +func (x *PushEvent) ProtoReflect() protoreflect.Message { + mi := &file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes[0] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use PushEvent.ProtoReflect.Descriptor instead. +func (*PushEvent) Descriptor() ([]byte, []int) { + return file_tigrisdata_objgit_events_push_v1_push_proto_rawDescGZIP(), []int{0} +} + +func (x *PushEvent) GetEventId() string { + if x != nil { + return x.EventId + } + return "" +} + +func (x *PushEvent) GetPushId() string { + if x != nil { + return x.PushId + } + return "" +} + +func (x *PushEvent) GetRepository() string { + if x != nil { + return x.Repository + } + return "" +} + +func (x *PushEvent) GetRef() string { + if x != nil { + return x.Ref + } + return "" +} + +func (x *PushEvent) GetBefore() string { + if x != nil { + return x.Before + } + return "" +} + +func (x *PushEvent) GetAfter() string { + if x != nil { + return x.After + } + return "" +} + +func (x *PushEvent) GetCreated() bool { + if x != nil { + return x.Created + } + return false +} + +func (x *PushEvent) GetDeleted() bool { + if x != nil { + return x.Deleted + } + return false +} + +func (x *PushEvent) GetForced() bool { + if x != nil { + return x.Forced + } + return false +} + +func (x *PushEvent) GetPushedAt() *timestamppb.Timestamp { + if x != nil { + return x.PushedAt + } + return nil +} + +func (x *PushEvent) GetCommits() []*Commit { + if x != nil { + return x.Commits + } + return nil +} + +func (x *PushEvent) GetFiles() *FileChanges { + if x != nil { + return x.Files + } + return nil +} + +// Commit describes a commit included in a ref update. +type Commit struct { + state protoimpl.MessageState `protogen:"open.v1"` + // Git object ID of the commit. + Id string `protobuf:"bytes,1,opt,name=id,proto3" json:"id,omitempty"` + // Git object ID of the commit's tree. + TreeId string `protobuf:"bytes,2,opt,name=tree_id,json=treeId,proto3" json:"tree_id,omitempty"` + // Full commit message. Invalid UTF-8 bytes are replaced with U+FFFD. + Message string `protobuf:"bytes,3,opt,name=message,proto3" json:"message,omitempty"` + Author *Signature `protobuf:"bytes,4,opt,name=author,proto3" json:"author,omitempty"` + Committer *Signature `protobuf:"bytes,5,opt,name=committer,proto3" json:"committer,omitempty"` + // Parent commit IDs, in Git's recorded order. + ParentIds []string `protobuf:"bytes,6,rep,name=parent_ids,json=parentIds,proto3" json:"parent_ids,omitempty"` + // File changes against the first parent's tree, or the empty tree for a + // root commit. For merges, this is a first-parent diff. + Files *FileChanges `protobuf:"bytes,7,opt,name=files,proto3" json:"files,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *Commit) Reset() { + *x = Commit{} + mi := &file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes[1] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *Commit) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*Commit) ProtoMessage() {} + +func (x *Commit) ProtoReflect() protoreflect.Message { + mi := &file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes[1] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use Commit.ProtoReflect.Descriptor instead. +func (*Commit) Descriptor() ([]byte, []int) { + return file_tigrisdata_objgit_events_push_v1_push_proto_rawDescGZIP(), []int{1} +} + +func (x *Commit) GetId() string { + if x != nil { + return x.Id + } + return "" +} + +func (x *Commit) GetTreeId() string { + if x != nil { + return x.TreeId + } + return "" +} + +func (x *Commit) GetMessage() string { + if x != nil { + return x.Message + } + return "" +} + +func (x *Commit) GetAuthor() *Signature { + if x != nil { + return x.Author + } + return nil +} + +func (x *Commit) GetCommitter() *Signature { + if x != nil { + return x.Committer + } + return nil +} + +func (x *Commit) GetParentIds() []string { + if x != nil { + return x.ParentIds + } + return nil +} + +func (x *Commit) GetFiles() *FileChanges { + if x != nil { + return x.Files + } + return nil +} + +// Signature records a Git commit's author or committer identity and time. +// Invalid UTF-8 bytes in name and email are replaced with U+FFFD. +type Signature struct { + state protoimpl.MessageState `protogen:"open.v1"` + Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` + Email string `protobuf:"bytes,2,opt,name=email,proto3" json:"email,omitempty"` + When *timestamppb.Timestamp `protobuf:"bytes,3,opt,name=when,proto3" json:"when,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *Signature) Reset() { + *x = Signature{} + mi := &file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes[2] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *Signature) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*Signature) ProtoMessage() {} + +func (x *Signature) ProtoReflect() protoreflect.Message { + mi := &file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes[2] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use Signature.ProtoReflect.Descriptor instead. +func (*Signature) Descriptor() ([]byte, []int) { + return file_tigrisdata_objgit_events_push_v1_push_proto_rawDescGZIP(), []int{2} +} + +func (x *Signature) GetName() string { + if x != nil { + return x.Name + } + return "" +} + +func (x *Signature) GetEmail() string { + if x != nil { + return x.Email + } + return "" +} + +func (x *Signature) GetWhen() *timestamppb.Timestamp { + if x != nil { + return x.When + } + return nil +} + +// FileChanges lists paths only; it does not include file contents or patches. +// A rename appears as one deleted path and one added path. Mode-only changes +// appear in changed. Valid UTF-8 paths are literal. A path containing invalid +// UTF-8 bytes is encoded as "/objgit/raw-path/base64url/" followed by the +// unpadded base64url encoding of its original bytes. Git paths cannot start +// with "/", so this representation is reversible without collisions. +type FileChanges struct { + state protoimpl.MessageState `protogen:"open.v1"` + Added []string `protobuf:"bytes,1,rep,name=added,proto3" json:"added,omitempty"` + Changed []string `protobuf:"bytes,2,rep,name=changed,proto3" json:"changed,omitempty"` + Deleted []string `protobuf:"bytes,3,rep,name=deleted,proto3" json:"deleted,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *FileChanges) Reset() { + *x = FileChanges{} + mi := &file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes[3] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *FileChanges) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*FileChanges) ProtoMessage() {} + +func (x *FileChanges) ProtoReflect() protoreflect.Message { + mi := &file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes[3] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use FileChanges.ProtoReflect.Descriptor instead. +func (*FileChanges) Descriptor() ([]byte, []int) { + return file_tigrisdata_objgit_events_push_v1_push_proto_rawDescGZIP(), []int{3} +} + +func (x *FileChanges) GetAdded() []string { + if x != nil { + return x.Added + } + return nil +} + +func (x *FileChanges) GetChanged() []string { + if x != nil { + return x.Changed + } + return nil +} + +func (x *FileChanges) GetDeleted() []string { + if x != nil { + return x.Deleted + } + return nil +} + +var File_tigrisdata_objgit_events_push_v1_push_proto protoreflect.FileDescriptor + +const file_tigrisdata_objgit_events_push_v1_push_proto_rawDesc = "" + + "\n" + + "+tigrisdata/objgit/events/push/v1/push.proto\x12 tigrisdata.objgit.events.push.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"\xad\x03\n" + + "\tPushEvent\x12\x19\n" + + "\bevent_id\x18\x01 \x01(\tR\aeventId\x12\x17\n" + + "\apush_id\x18\x02 \x01(\tR\x06pushId\x12\x1e\n" + + "\n" + + "repository\x18\x03 \x01(\tR\n" + + "repository\x12\x10\n" + + "\x03ref\x18\x04 \x01(\tR\x03ref\x12\x16\n" + + "\x06before\x18\x05 \x01(\tR\x06before\x12\x14\n" + + "\x05after\x18\x06 \x01(\tR\x05after\x12\x18\n" + + "\acreated\x18\a \x01(\bR\acreated\x12\x18\n" + + "\adeleted\x18\b \x01(\bR\adeleted\x12\x16\n" + + "\x06forced\x18\t \x01(\bR\x06forced\x127\n" + + "\tpushed_at\x18\n" + + " \x01(\v2\x1a.google.protobuf.TimestampR\bpushedAt\x12B\n" + + "\acommits\x18\v \x03(\v2(.tigrisdata.objgit.events.push.v1.CommitR\acommits\x12C\n" + + "\x05files\x18\f \x01(\v2-.tigrisdata.objgit.events.push.v1.FileChangesR\x05files\"\xbf\x02\n" + + "\x06Commit\x12\x0e\n" + + "\x02id\x18\x01 \x01(\tR\x02id\x12\x17\n" + + "\atree_id\x18\x02 \x01(\tR\x06treeId\x12\x18\n" + + "\amessage\x18\x03 \x01(\tR\amessage\x12C\n" + + "\x06author\x18\x04 \x01(\v2+.tigrisdata.objgit.events.push.v1.SignatureR\x06author\x12I\n" + + "\tcommitter\x18\x05 \x01(\v2+.tigrisdata.objgit.events.push.v1.SignatureR\tcommitter\x12\x1d\n" + + "\n" + + "parent_ids\x18\x06 \x03(\tR\tparentIds\x12C\n" + + "\x05files\x18\a \x01(\v2-.tigrisdata.objgit.events.push.v1.FileChangesR\x05files\"e\n" + + "\tSignature\x12\x12\n" + + "\x04name\x18\x01 \x01(\tR\x04name\x12\x14\n" + + "\x05email\x18\x02 \x01(\tR\x05email\x12.\n" + + "\x04when\x18\x03 \x01(\v2\x1a.google.protobuf.TimestampR\x04when\"W\n" + + "\vFileChanges\x12\x14\n" + + "\x05added\x18\x01 \x03(\tR\x05added\x12\x18\n" + + "\achanged\x18\x02 \x03(\tR\achanged\x12\x18\n" + + "\adeleted\x18\x03 \x03(\tR\adeletedB\xa0\x02\n" + + "$com.tigrisdata.objgit.events.push.v1B\tPushProtoP\x01ZHgithub.com/tigrisdata/objgit/gen/tigrisdata/objgit/events/push/v1;pushv1\xa2\x02\x04TOEP\xaa\x02 Tigrisdata.Objgit.Events.Push.V1\xca\x02 Tigrisdata\\Objgit\\Events\\Push\\V1\xe2\x02,Tigrisdata\\Objgit\\Events\\Push\\V1\\GPBMetadata\xea\x02$Tigrisdata::Objgit::Events::Push::V1b\x06proto3" + +var ( + file_tigrisdata_objgit_events_push_v1_push_proto_rawDescOnce sync.Once + file_tigrisdata_objgit_events_push_v1_push_proto_rawDescData []byte +) + +func file_tigrisdata_objgit_events_push_v1_push_proto_rawDescGZIP() []byte { + file_tigrisdata_objgit_events_push_v1_push_proto_rawDescOnce.Do(func() { + file_tigrisdata_objgit_events_push_v1_push_proto_rawDescData = protoimpl.X.CompressGZIP(unsafe.Slice(unsafe.StringData(file_tigrisdata_objgit_events_push_v1_push_proto_rawDesc), len(file_tigrisdata_objgit_events_push_v1_push_proto_rawDesc))) + }) + return file_tigrisdata_objgit_events_push_v1_push_proto_rawDescData +} + +var file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes = make([]protoimpl.MessageInfo, 4) +var file_tigrisdata_objgit_events_push_v1_push_proto_goTypes = []any{ + (*PushEvent)(nil), // 0: tigrisdata.objgit.events.push.v1.PushEvent + (*Commit)(nil), // 1: tigrisdata.objgit.events.push.v1.Commit + (*Signature)(nil), // 2: tigrisdata.objgit.events.push.v1.Signature + (*FileChanges)(nil), // 3: tigrisdata.objgit.events.push.v1.FileChanges + (*timestamppb.Timestamp)(nil), // 4: google.protobuf.Timestamp +} +var file_tigrisdata_objgit_events_push_v1_push_proto_depIdxs = []int32{ + 4, // 0: tigrisdata.objgit.events.push.v1.PushEvent.pushed_at:type_name -> google.protobuf.Timestamp + 1, // 1: tigrisdata.objgit.events.push.v1.PushEvent.commits:type_name -> tigrisdata.objgit.events.push.v1.Commit + 3, // 2: tigrisdata.objgit.events.push.v1.PushEvent.files:type_name -> tigrisdata.objgit.events.push.v1.FileChanges + 2, // 3: tigrisdata.objgit.events.push.v1.Commit.author:type_name -> tigrisdata.objgit.events.push.v1.Signature + 2, // 4: tigrisdata.objgit.events.push.v1.Commit.committer:type_name -> tigrisdata.objgit.events.push.v1.Signature + 3, // 5: tigrisdata.objgit.events.push.v1.Commit.files:type_name -> tigrisdata.objgit.events.push.v1.FileChanges + 4, // 6: tigrisdata.objgit.events.push.v1.Signature.when:type_name -> google.protobuf.Timestamp + 7, // [7:7] is the sub-list for method output_type + 7, // [7:7] is the sub-list for method input_type + 7, // [7:7] is the sub-list for extension type_name + 7, // [7:7] is the sub-list for extension extendee + 0, // [0:7] is the sub-list for field type_name +} + +func init() { file_tigrisdata_objgit_events_push_v1_push_proto_init() } +func file_tigrisdata_objgit_events_push_v1_push_proto_init() { + if File_tigrisdata_objgit_events_push_v1_push_proto != nil { + return + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: unsafe.Slice(unsafe.StringData(file_tigrisdata_objgit_events_push_v1_push_proto_rawDesc), len(file_tigrisdata_objgit_events_push_v1_push_proto_rawDesc)), + NumEnums: 0, + NumMessages: 4, + NumExtensions: 0, + NumServices: 0, + }, + GoTypes: file_tigrisdata_objgit_events_push_v1_push_proto_goTypes, + DependencyIndexes: file_tigrisdata_objgit_events_push_v1_push_proto_depIdxs, + MessageInfos: file_tigrisdata_objgit_events_push_v1_push_proto_msgTypes, + }.Build() + File_tigrisdata_objgit_events_push_v1_push_proto = out.File + file_tigrisdata_objgit_events_push_v1_push_proto_goTypes = nil + file_tigrisdata_objgit_events_push_v1_push_proto_depIdxs = nil +} diff --git a/go.mod b/go.mod index 14c81f9..e5b068e 100644 --- a/go.mod +++ b/go.mod @@ -68,7 +68,7 @@ require ( golang.org/x/term v0.43.0 // indirect golang.org/x/text v0.37.0 // indirect golang.org/x/tools v0.45.0 // indirect - google.golang.org/protobuf v1.36.8 // indirect + google.golang.org/protobuf v1.36.8 honnef.co/go/tools v0.7.0 // indirect ) diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 1ac4e50..2f1acbd 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -87,6 +87,21 @@ var ( Buckets: prometheus.DefBuckets, }) + webhookDeliveries = promauto.NewCounterVec(prometheus.CounterOpts{ + Namespace: namespace, + Subsystem: "webhook", + Name: "deliveries_total", + Help: "Push webhook events by final outcome (ok or error).", + }, []string{"status"}) + + webhookDuration = promauto.NewHistogram(prometheus.HistogramOpts{ + Namespace: namespace, + Subsystem: "webhook", + Name: "delivery_duration_seconds", + Help: "Time to build and deliver a push webhook event, including retries.", + Buckets: prometheus.DefBuckets, + }) + reposCreated = promauto.NewCounter(prometheus.CounterOpts{ Namespace: namespace, Name: "repos_created_total", @@ -239,6 +254,12 @@ func ObserveHook(status string, dur time.Duration) { hookDuration.Observe(dur.Seconds()) } +// ObserveWebhook records the final outcome and elapsed time of one event. +func ObserveWebhook(status string, dur time.Duration) { + webhookDeliveries.WithLabelValues(status).Inc() + webhookDuration.Observe(dur.Seconds()) +} + // ReposCreated counts a repository auto-created on first push. func ReposCreated() { reposCreated.Inc() diff --git a/internal/pushevents/push.go b/internal/pushevents/push.go new file mode 100644 index 0000000..a00fba3 --- /dev/null +++ b/internal/pushevents/push.go @@ -0,0 +1,338 @@ +// Package pushevents builds push webhook payloads from accepted Git ref updates. +package pushevents + +import ( + "context" + "encoding/base64" + "fmt" + "sort" + "strings" + "time" + "unicode/utf8" + + "github.com/go-git/go-git/v6/plumbing" + "github.com/go-git/go-git/v6/plumbing/filemode" + "github.com/go-git/go-git/v6/plumbing/object" + "github.com/go-git/go-git/v6/storage" + pushv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/events/push/v1" + "google.golang.org/protobuf/types/known/timestamppb" +) + +// Update records one accepted ref change. A zero hash means the ref did not +// exist on that side of the update. +type Update struct { + Name plumbing.ReferenceName + Old plumbing.Hash + New plumbing.Hash +} + +// Build returns the event for one accepted ref update. Commit order is stable +// and always places parents before their children. For a merge, the commit's +// files are compared with its first parent. The event's files are the net +// difference between the two ref targets. Invalid UTF-8 bytes in commit +// messages and author/committer names and emails are replaced with U+FFFD so +// ProtoJSON can serialize the event. File paths use a reversible encoding; +// see Diff. +func Build(ctx context.Context, st storage.Storer, repo string, update Update, pushID, eventID string, at time.Time) (*pushv1.PushEvent, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + event := &pushv1.PushEvent{ + EventId: eventID, + PushId: pushID, + Repository: repo, + Ref: encodePath(update.Name.String()), + Before: update.Old.String(), + After: update.New.String(), + Created: update.Old.IsZero() && !update.New.IsZero(), + Deleted: update.New.IsZero() && !update.Old.IsZero(), + PushedAt: timestamppb.New(at), + Files: &pushv1.FileChanges{}, + } + + // Annotated tags and other non-commit targets have no commit tree to + // compare. They retain the ref metadata but have no file metadata. + oldCommit, oldIsCommit, err := commitAt(st, update.Old) + if err != nil { + return nil, fmt.Errorf("load old ref target %s: %w", update.Old, err) + } + newCommit, newIsCommit, err := commitAt(st, update.New) + if err != nil { + return nil, fmt.Errorf("load new ref target %s: %w", update.New, err) + } + if !oldIsCommit || !newIsCommit { + return event, nil + } + + event.Files, err = Diff(ctx, st, update.Old, update.New) + if err != nil { + return nil, fmt.Errorf("diff ref %s: %w", update.Name, err) + } + if newCommit == nil { // A deletion introduces no commits. + return event, nil + } + + oldReachable, err := reachable(ctx, st, oldCommit) + if err != nil { + return nil, fmt.Errorf("walk old ref %s: %w", update.Name, err) + } + commits, oldIsAncestor, err := collectNew(ctx, st, newCommit, update.Old, oldReachable) + if err != nil { + return nil, fmt.Errorf("walk new ref %s: %w", update.Name, err) + } + event.Forced = oldCommit != nil && !oldIsAncestor + for _, commit := range commits { + var firstParent plumbing.Hash + if len(commit.ParentHashes) != 0 { + firstParent = commit.ParentHashes[0] + } + files, err := Diff(ctx, st, firstParent, commit.Hash) + if err != nil { + return nil, fmt.Errorf("diff commit %s: %w", commit.Hash, err) + } + parents := make([]string, len(commit.ParentHashes)) + for i, parent := range commit.ParentHashes { + parents[i] = parent.String() + } + event.Commits = append(event.Commits, &pushv1.Commit{ + Id: commit.Hash.String(), + TreeId: commit.TreeHash.String(), + Message: strings.ToValidUTF8(commit.Message, "\ufffd"), + Author: signature(commit.Author), + Committer: signature(commit.Committer), + ParentIds: parents, + Files: files, + }) + } + return event, nil +} + +func signature(sig object.Signature) *pushv1.Signature { + return &pushv1.Signature{ + Name: strings.ToValidUTF8(sig.Name, "\ufffd"), + Email: strings.ToValidUTF8(sig.Email, "\ufffd"), + When: timestamppb.New(sig.When), + } +} + +// Diff compares the trees at two commits. A zero hash represents an empty +// tree, so creation lists additions and deletion lists removals. Paths are +// sorted in each category. Renames are an addition and a deletion. Valid UTF-8 +// paths are unchanged. Since Protobuf strings require valid UTF-8, a path with +// invalid bytes is encoded as "/objgit/raw-path/base64url/" followed by its +// unpadded base64url bytes. Git paths cannot start with "/", so this encoding +// is reversible without colliding with a valid path. +func Diff(ctx context.Context, st storage.Storer, oldHash, newHash plumbing.Hash) (*pushv1.FileChanges, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + oldTree, err := treeAt(st, oldHash) + if err != nil { + return nil, fmt.Errorf("load old tree: %w", err) + } + newTree, err := treeAt(st, newHash) + if err != nil { + return nil, fmt.Errorf("load new tree: %w", err) + } + files := &pushv1.FileChanges{} + if err := diffTrees(ctx, st, oldTree, newTree, "", files); err != nil { + return nil, fmt.Errorf("diff trees: %w", err) + } + sort.Strings(files.Added) + sort.Strings(files.Changed) + sort.Strings(files.Deleted) + return files, nil +} + +func diffTrees(ctx context.Context, st storage.Storer, oldTree, newTree *object.Tree, prefix string, files *pushv1.FileChanges) error { + if err := ctx.Err(); err != nil { + return err + } + if oldTree != nil && newTree != nil && oldTree.Hash == newTree.Hash { + return nil + } + oldEntries := make(map[string]object.TreeEntry) + newEntries := make(map[string]object.TreeEntry) + if oldTree != nil { + for _, entry := range oldTree.Entries { + oldEntries[entry.Name] = entry + } + } + if newTree != nil { + for _, entry := range newTree.Entries { + newEntries[entry.Name] = entry + } + } + for name, oldEntry := range oldEntries { + newEntry, hasNew := newEntries[name] + if hasNew && oldEntry.Hash == newEntry.Hash && oldEntry.Mode == newEntry.Mode { + continue + } + path := prefix + name + if oldEntry.Mode == filemode.Dir || (hasNew && newEntry.Mode == filemode.Dir) { + if oldEntry.Mode == filemode.Dir { + var newSubtree *object.Tree + if hasNew && newEntry.Mode == filemode.Dir { + var err error + newSubtree, err = object.GetTree(st, newEntry.Hash) + if err != nil { + return fmt.Errorf("load new subtree %q: %w", path, err) + } + } + oldSubtree, err := object.GetTree(st, oldEntry.Hash) + if err != nil { + return fmt.Errorf("load old subtree %q: %w", path, err) + } + if err := diffTrees(ctx, st, oldSubtree, newSubtree, path+"/", files); err != nil { + return err + } + } else { + files.Deleted = append(files.Deleted, encodePath(path)) + } + if hasNew && newEntry.Mode == filemode.Dir && oldEntry.Mode != filemode.Dir { + newSubtree, err := object.GetTree(st, newEntry.Hash) + if err != nil { + return fmt.Errorf("load new subtree %q: %w", path, err) + } + if err := diffTrees(ctx, st, nil, newSubtree, path+"/", files); err != nil { + return err + } + } else if hasNew && newEntry.Mode != filemode.Dir { + files.Added = append(files.Added, encodePath(path)) + } + continue + } + if !hasNew { + files.Deleted = append(files.Deleted, encodePath(path)) + } else { + files.Changed = append(files.Changed, encodePath(path)) + } + } + for name, newEntry := range newEntries { + if _, exists := oldEntries[name]; exists { + continue + } + path := prefix + name + if newEntry.Mode == filemode.Dir { + newSubtree, err := object.GetTree(st, newEntry.Hash) + if err != nil { + return fmt.Errorf("load new subtree %q: %w", path, err) + } + if err := diffTrees(ctx, st, nil, newSubtree, path+"/", files); err != nil { + return err + } + } else { + files.Added = append(files.Added, encodePath(path)) + } + } + return nil +} + +func encodePath(path string) string { + if utf8.ValidString(path) { + return path + } + return "/objgit/raw-path/base64url/" + base64.RawURLEncoding.EncodeToString([]byte(path)) +} + +func treeAt(st storage.Storer, hash plumbing.Hash) (*object.Tree, error) { + if hash.IsZero() { + return nil, nil + } + commit, err := object.GetCommit(st, hash) + if err != nil { + return nil, err + } + return commit.Tree() +} + +// commitAt distinguishes a non-commit target from a missing object. Zero +// hashes stand for an absent ref and are treated as commit-compatible. +func commitAt(st storage.Storer, hash plumbing.Hash) (*object.Commit, bool, error) { + if hash.IsZero() { + return nil, true, nil + } + obj, err := st.EncodedObject(plumbing.AnyObject, hash) + if err != nil { + return nil, false, err + } + if obj.Type() != plumbing.CommitObject { + return nil, false, nil + } + commit, err := object.DecodeCommit(st, obj) + return commit, true, err +} + +func reachable(ctx context.Context, st storage.Storer, tip *object.Commit) (map[plumbing.Hash]struct{}, error) { + seen := make(map[plumbing.Hash]struct{}) + if tip == nil { + return seen, nil + } + stack := []plumbing.Hash{tip.Hash} + for len(stack) != 0 { + if err := ctx.Err(); err != nil { + return nil, err + } + hash := stack[len(stack)-1] + stack = stack[:len(stack)-1] + if _, ok := seen[hash]; ok { + continue + } + seen[hash] = struct{}{} + commit, err := object.GetCommit(st, hash) + if err != nil { + return nil, err + } + stack = append(stack, commit.ParentHashes...) + } + return seen, nil +} + +// collectNew traverses in postorder, which guarantees parent-before-child +// output even with merges. The parent's recorded order is the tie breaker. +func collectNew(ctx context.Context, st storage.Storer, tip *object.Commit, oldHash plumbing.Hash, excluded map[plumbing.Hash]struct{}) ([]*object.Commit, bool, error) { + if tip.Hash == oldHash { + return nil, true, nil + } + type frame struct { + commit *object.Commit + next int + } + var ordered []*object.Commit + stack := []frame{{commit: tip}} + seen := make(map[plumbing.Hash]struct{}) + oldIsAncestor := false + for len(stack) != 0 { + if err := ctx.Err(); err != nil { + return nil, false, err + } + current := &stack[len(stack)-1] + if current.commit.Hash == oldHash { + oldIsAncestor = true + } + if current.next < len(current.commit.ParentHashes) { + parentHash := current.commit.ParentHashes[current.next] + current.next++ + if parentHash == oldHash { + oldIsAncestor = true + } + if _, ok := excluded[parentHash]; ok { + continue + } + if _, ok := seen[parentHash]; ok { + continue + } + parent, err := object.GetCommit(st, parentHash) + if err != nil { + return nil, false, err + } + seen[parentHash] = struct{}{} + stack = append(stack, frame{commit: parent}) + continue + } + ordered = append(ordered, current.commit) + stack = stack[:len(stack)-1] + } + return ordered, oldIsAncestor, nil +} diff --git a/internal/pushevents/push_test.go b/internal/pushevents/push_test.go new file mode 100644 index 0000000..9f4b107 --- /dev/null +++ b/internal/pushevents/push_test.go @@ -0,0 +1,449 @@ +package pushevents + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "io" + "reflect" + "sort" + "strings" + "testing" + "time" + + "github.com/go-git/go-git/v6/plumbing" + "github.com/go-git/go-git/v6/plumbing/filemode" + "github.com/go-git/go-git/v6/plumbing/object" + "github.com/go-git/go-git/v6/storage" + "github.com/go-git/go-git/v6/storage/memory" + pushv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/events/push/v1" + "google.golang.org/protobuf/encoding/protojson" +) + +var testTime = time.Date(2026, time.September, 23, 12, 30, 0, 0, time.UTC) + +func putBlob(t *testing.T, st storage.Storer, body string) plumbing.Hash { + t.Helper() + obj := st.NewEncodedObject() + obj.SetType(plumbing.BlobObject) + w, err := obj.Writer() + if err != nil { + t.Fatal(err) + } + if _, err := io.WriteString(w, body); err != nil { + t.Fatal(err) + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + hash, err := st.SetEncodedObject(obj) + if err != nil { + t.Fatal(err) + } + return hash +} + +func putTree(t *testing.T, st storage.Storer, files map[string]string) plumbing.Hash { + t.Helper() + entries := make([]object.TreeEntry, 0, len(files)) + for name, body := range files { + entries = append(entries, object.TreeEntry{Name: name, Mode: filemode.Regular, Hash: putBlob(t, st, body)}) + } + sort.Sort(object.TreeEntrySorter(entries)) + obj := st.NewEncodedObject() + if err := (&object.Tree{Entries: entries}).Encode(obj); err != nil { + t.Fatal(err) + } + hash, err := st.SetEncodedObject(obj) + if err != nil { + t.Fatal(err) + } + return hash +} + +func putCommit(t *testing.T, st storage.Storer, message string, files map[string]string, parents ...plumbing.Hash) plumbing.Hash { + t.Helper() + return putCommitWithTree(t, st, message, putTree(t, st, files), parents...) +} + +func putCommitWithTree(t *testing.T, st storage.Storer, message string, treeHash plumbing.Hash, parents ...plumbing.Hash) plumbing.Hash { + t.Helper() + commit := &object.Commit{ + TreeHash: treeHash, + ParentHashes: parents, + Message: message, + Author: object.Signature{Name: "Author", Email: "author@example.com", When: testTime}, + Committer: object.Signature{Name: "Committer", Email: "committer@example.com", When: testTime.Add(time.Minute)}, + } + obj := st.NewEncodedObject() + if err := commit.Encode(obj); err != nil { + t.Fatal(err) + } + hash, err := st.SetEncodedObject(obj) + if err != nil { + t.Fatal(err) + } + return hash +} + +// Go-git's Tree.Encode validates names more narrowly than Git. Write a tree +// object directly to exercise names that are valid in Git but rejected there. +func putRawTree(t *testing.T, st storage.Storer, name, body string) plumbing.Hash { + t.Helper() + blob := putBlob(t, st, body) + obj := st.NewEncodedObject() + obj.SetType(plumbing.TreeObject) + w, err := obj.Writer() + if err != nil { + t.Fatal(err) + } + if _, err := fmt.Fprintf(w, "%o %s", filemode.Regular, name); err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte{0}); err != nil { + t.Fatal(err) + } + if _, err := blob.WriteTo(w); err != nil { + t.Fatal(err) + } + if err := w.Close(); err != nil { + t.Fatal(err) + } + hash, err := st.SetEncodedObject(obj) + if err != nil { + t.Fatal(err) + } + return hash +} + +func TestBuild(t *testing.T) { + tests := []struct { + name string + setup func(*testing.T, storage.Storer) (Update, []plumbing.Hash) + wantForced bool + wantCreated bool + wantDeleted bool + wantFiles *pushv1.FileChanges + wantPerFiles []*pushv1.FileChanges + }{ + { + name: "new branch contains all ancestors in parent-first order", + setup: func(t *testing.T, st storage.Storer) (Update, []plumbing.Hash) { + first := putCommit(t, st, "add", map[string]string{"a.txt": "a"}) + second := putCommit(t, st, "edit", map[string]string{"a.txt": "b", "b.txt": "b"}, first) + third := putCommit(t, st, "remove", map[string]string{"a.txt": "b"}, second) + return Update{Name: "refs/heads/main", New: third}, []plumbing.Hash{first, second, third} + }, + wantCreated: true, + wantFiles: &pushv1.FileChanges{Added: []string{"a.txt"}}, + wantPerFiles: []*pushv1.FileChanges{ + {Added: []string{"a.txt"}}, + {Added: []string{"b.txt"}, Changed: []string{"a.txt"}}, + {Deleted: []string{"b.txt"}}, + }, + }, + { + name: "fast-forward includes only newly reachable commits", + setup: func(t *testing.T, st storage.Storer) (Update, []plumbing.Hash) { + first := putCommit(t, st, "first", map[string]string{"a.txt": "a"}) + second := putCommit(t, st, "second", map[string]string{"a.txt": "b"}, first) + return Update{Name: "refs/heads/main", Old: first, New: second}, []plumbing.Hash{second} + }, + wantFiles: &pushv1.FileChanges{Changed: []string{"a.txt"}}, + wantPerFiles: []*pushv1.FileChanges{{Changed: []string{"a.txt"}}}, + }, + { + name: "merge places both parents before merge and uses first-parent files", + setup: func(t *testing.T, st storage.Storer) (Update, []plumbing.Hash) { + root := putCommit(t, st, "root", map[string]string{"root.txt": "r"}) + left := putCommit(t, st, "left", map[string]string{"root.txt": "r", "left.txt": "l"}, root) + right := putCommit(t, st, "right", map[string]string{"root.txt": "r", "right.txt": "r"}, root) + merge := putCommit(t, st, "merge", map[string]string{"root.txt": "r", "left.txt": "l", "right.txt": "r"}, left, right) + return Update{Name: "refs/heads/main", Old: root, New: merge}, []plumbing.Hash{left, right, merge} + }, + wantFiles: &pushv1.FileChanges{Added: []string{"left.txt", "right.txt"}}, + wantPerFiles: []*pushv1.FileChanges{ + {Added: []string{"left.txt"}}, + {Added: []string{"right.txt"}}, + {Added: []string{"right.txt"}}, + }, + }, + { + name: "force push excludes old ancestors and reports net changes", + setup: func(t *testing.T, st storage.Storer) (Update, []plumbing.Hash) { + root := putCommit(t, st, "root", map[string]string{"root.txt": "r"}) + old := putCommit(t, st, "old", map[string]string{"root.txt": "r", "old.txt": "o"}, root) + newTip := putCommit(t, st, "new", map[string]string{"root.txt": "r", "new.txt": "n"}, root) + return Update{Name: "refs/heads/main", Old: old, New: newTip}, []plumbing.Hash{newTip} + }, + wantForced: true, + wantFiles: &pushv1.FileChanges{Added: []string{"new.txt"}, Deleted: []string{"old.txt"}}, + wantPerFiles: []*pushv1.FileChanges{{Added: []string{"new.txt"}}}, + }, + { + name: "branch deletion has net removals and no commits", + setup: func(t *testing.T, st storage.Storer) (Update, []plumbing.Hash) { + old := putCommit(t, st, "old", map[string]string{"a.txt": "a"}) + return Update{Name: "refs/heads/main", Old: old}, nil + }, + wantDeleted: true, + wantFiles: &pushv1.FileChanges{Deleted: []string{"a.txt"}}, + }, + { + name: "unchanged ref has no commits or files", + setup: func(t *testing.T, st storage.Storer) (Update, []plumbing.Hash) { + tip := putCommit(t, st, "tip", map[string]string{"a.txt": "a"}) + return Update{Name: "refs/heads/main", Old: tip, New: tip}, nil + }, + wantFiles: &pushv1.FileChanges{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + st := memory.NewStorage() + update, wantCommits := tt.setup(t, st) + got, err := Build(context.Background(), st, "acme/widgets", update, "push-1", "event-1", testTime) + if err != nil { + t.Fatalf("Build: %v", err) + } + if got.GetRepository() != "acme/widgets" || got.GetPushId() != "push-1" || got.GetEventId() != "event-1" || got.GetRef() != update.Name.String() { + t.Errorf("event identity = %v", got) + } + if got.GetBefore() != update.Old.String() || got.GetAfter() != update.New.String() { + t.Errorf("event hashes = (%s, %s), want (%s, %s)", got.GetBefore(), got.GetAfter(), update.Old, update.New) + } + if got.GetCreated() != tt.wantCreated || got.GetDeleted() != tt.wantDeleted || got.GetForced() != tt.wantForced { + t.Errorf("flags = (%v, %v, %v), want (%v, %v, %v)", got.GetCreated(), got.GetDeleted(), got.GetForced(), tt.wantCreated, tt.wantDeleted, tt.wantForced) + } + if !got.GetPushedAt().AsTime().Equal(testTime) { + t.Errorf("pushed at = %v, want %v", got.GetPushedAt().AsTime(), testTime) + } + if !sameFiles(got.GetFiles(), tt.wantFiles) { + t.Errorf("net files = %v, want %v", got.GetFiles(), tt.wantFiles) + } + if len(got.GetCommits()) != len(wantCommits) { + t.Fatalf("commit count = %d, want %d", len(got.GetCommits()), len(wantCommits)) + } + for i, wantHash := range wantCommits { + commit := got.GetCommits()[i] + if commit.GetId() != wantHash.String() { + t.Errorf("commit %d = %s, want %s", i, commit.GetId(), wantHash) + } + if !sameFiles(commit.GetFiles(), tt.wantPerFiles[i]) { + t.Errorf("commit %d files = %v, want %v", i, commit.GetFiles(), tt.wantPerFiles[i]) + } + if commit.GetAuthor().GetName() != "Author" || !commit.GetAuthor().GetWhen().AsTime().Equal(testTime) { + t.Errorf("commit %d author = %v", i, commit.GetAuthor()) + } + } + }) + } +} + +func sameFiles(got, want *pushv1.FileChanges) bool { + return reflect.DeepEqual(got.GetAdded(), want.GetAdded()) && + reflect.DeepEqual(got.GetChanged(), want.GetChanged()) && + reflect.DeepEqual(got.GetDeleted(), want.GetDeleted()) +} + +func TestDiffCancellation(t *testing.T) { + st := memory.NewStorage() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + _, err := Diff(ctx, st, plumbing.ZeroHash, plumbing.ZeroHash) + if !errors.Is(err, context.Canceled) { + t.Errorf("Diff error = %v, want context.Canceled", err) + } +} + +func TestBuildNonCommitTarget(t *testing.T) { + st := memory.NewStorage() + blob := putBlob(t, st, "not a commit") + got, err := Build(context.Background(), st, "acme/widgets", Update{Name: "refs/tags/data", New: blob}, "push-1", "event-1", testTime) + if err != nil { + t.Fatalf("Build: %v", err) + } + if !got.GetCreated() || got.GetAfter() != blob.String() || len(got.GetCommits()) != 0 || !sameFiles(got.GetFiles(), &pushv1.FileChanges{}) { + t.Errorf("non-commit event = %v", got) + } +} + +func TestDiffModeChange(t *testing.T) { + st := memory.NewStorage() + blob := putBlob(t, st, "same bytes") + putModeCommit := func(mode filemode.FileMode) plumbing.Hash { + t.Helper() + treeObj := st.NewEncodedObject() + if err := (&object.Tree{Entries: []object.TreeEntry{{Name: "script.sh", Mode: mode, Hash: blob}}}).Encode(treeObj); err != nil { + t.Fatal(err) + } + treeHash, err := st.SetEncodedObject(treeObj) + if err != nil { + t.Fatal(err) + } + commitObj := st.NewEncodedObject() + if err := (&object.Commit{ + TreeHash: treeHash, + Message: "mode", + Author: object.Signature{Name: "Author", Email: "a@example.com", When: testTime}, + Committer: object.Signature{Name: "Author", Email: "a@example.com", When: testTime}, + }).Encode(commitObj); err != nil { + t.Fatal(err) + } + hash, err := st.SetEncodedObject(commitObj) + if err != nil { + t.Fatal(err) + } + return hash + } + oldHash := putModeCommit(filemode.Regular) + newHash := putModeCommit(filemode.Executable) + got, err := Diff(context.Background(), st, oldHash, newHash) + if err != nil { + t.Fatalf("Diff: %v", err) + } + if !sameFiles(got, &pushv1.FileChanges{Changed: []string{"script.sh"}}) { + t.Errorf("mode-only files = %v, want changed script.sh", got) + } +} + +func TestDiffDirectoryToFile(t *testing.T) { + st := memory.NewStorage() + innerTree := putTree(t, st, map[string]string{"old.txt": "old"}) + putSingleEntryTree := func(entry object.TreeEntry) plumbing.Hash { + t.Helper() + obj := st.NewEncodedObject() + if err := (&object.Tree{Entries: []object.TreeEntry{entry}}).Encode(obj); err != nil { + t.Fatal(err) + } + hash, err := st.SetEncodedObject(obj) + if err != nil { + t.Fatal(err) + } + return hash + } + oldTree := putSingleEntryTree(object.TreeEntry{Name: "docs", Mode: filemode.Dir, Hash: innerTree}) + newTree := putSingleEntryTree(object.TreeEntry{Name: "docs", Mode: filemode.Regular, Hash: putBlob(t, st, "replacement")}) + oldCommit := putCommitWithTree(t, st, "directory", oldTree) + newCommit := putCommitWithTree(t, st, "file", newTree, oldCommit) + got, err := Diff(context.Background(), st, oldCommit, newCommit) + if err != nil { + t.Fatalf("Diff: %v", err) + } + want := &pushv1.FileChanges{Added: []string{"docs"}, Deleted: []string{"docs/old.txt"}} + if !sameFiles(got, want) { + t.Errorf("files = %v, want %v", got, want) + } +} + +func TestBuildGitFilenames(t *testing.T) { + for _, tt := range []struct { + name string + gitPath string + wantPath string + encoded bool + }{ + {name: "newline remains literal in Protobuf string", gitPath: "a b\nc.txt", wantPath: "a b\nc.txt"}, + {name: "invalid UTF-8 uses reversible base64url path", gitPath: "bad-\xff.txt", encoded: true}, + } { + t.Run(tt.name, func(t *testing.T) { + st := memory.NewStorage() + tip := putCommitWithTree(t, st, "add special path", putRawTree(t, st, tt.gitPath, "body")) + event, err := Build(context.Background(), st, "acme/widgets", Update{Name: "refs/heads/main", New: tip}, "push-1", "event-1", testTime) + if err != nil { + t.Fatalf("Build: %v", err) + } + if _, err := protojson.Marshal(event); err != nil { + t.Fatalf("ProtoJSON marshal: %v", err) + } + if len(event.GetFiles().GetAdded()) != 1 { + t.Fatalf("added paths = %v, want one", event.GetFiles().GetAdded()) + } + path := event.GetFiles().GetAdded()[0] + if tt.encoded { + const prefix = "/objgit/raw-path/base64url/" + if !strings.HasPrefix(path, prefix) { + t.Fatalf("encoded path = %q, want prefix %q", path, prefix) + } + raw, err := base64.RawURLEncoding.DecodeString(strings.TrimPrefix(path, prefix)) + if err != nil { + t.Fatalf("decode path: %v", err) + } + if string(raw) != tt.gitPath { + t.Errorf("decoded path = %q, want %q", raw, tt.gitPath) + } + } else if path != tt.wantPath { + t.Errorf("path = %q, want %q", path, tt.wantPath) + } + if got := event.GetCommits()[0].GetFiles().GetAdded()[0]; got != path { + t.Errorf("commit path = %q, want %q", got, path) + } + }) + } +} + +func TestBuildInvalidUTF8CommitText(t *testing.T) { + st := memory.NewStorage() + treeHash := putTree(t, st, map[string]string{"readme": "hello"}) + obj := st.NewEncodedObject() + commit := &object.Commit{ + TreeHash: treeHash, + Message: "bad-\xff-message", + Author: object.Signature{Name: "bad-\xff-author", Email: "bad-\xff@example.com", When: testTime}, + Committer: object.Signature{Name: "bad-\xff-committer", Email: "bad-\xff@example.com", When: testTime}, + } + if err := commit.Encode(obj); err != nil { + t.Fatalf("encode commit: %v", err) + } + tip, err := st.SetEncodedObject(obj) + if err != nil { + t.Fatalf("store commit: %v", err) + } + event, err := Build(context.Background(), st, "acme/widgets", Update{Name: "refs/heads/main", New: tip}, "push-1", "event-1", testTime) + if err != nil { + t.Fatalf("Build: %v", err) + } + if _, err := protojson.Marshal(event); err != nil { + t.Fatalf("ProtoJSON marshal: %v", err) + } + got := event.GetCommits()[0] + if got.GetMessage() != "bad-\ufffd-message" || got.GetAuthor().GetName() != "bad-\ufffd-author" || got.GetAuthor().GetEmail() != "bad-\ufffd@example.com" || got.GetCommitter().GetName() != "bad-\ufffd-committer" { + t.Errorf("normalized commit text = %v", got) + } +} + +func TestBuildRefNameEncoding(t *testing.T) { + for _, tt := range []struct { + name string + ref plumbing.ReferenceName + }{ + {name: "UTF-8 ref stays literal", ref: "refs/heads/main"}, + {name: "invalid UTF-8 ref is reversible", ref: "refs/heads/bad-\xff"}, + } { + t.Run(tt.name, func(t *testing.T) { + st := memory.NewStorage() + tip := putCommit(t, st, "message", map[string]string{"readme": "hello"}) + event, err := Build(context.Background(), st, "acme/widgets", Update{Name: tt.ref, New: tip}, "push-1", "event-1", testTime) + if err != nil { + t.Fatal(err) + } + if _, err := protojson.Marshal(event); err != nil { + t.Fatalf("ProtoJSON marshal: %v", err) + } + got := event.GetRef() + if got == tt.ref.String() { + return + } + const prefix = "/objgit/raw-path/base64url/" + if !strings.HasPrefix(got, prefix) { + t.Fatalf("encoded ref %q lacks prefix", got) + } + raw, err := base64.RawURLEncoding.DecodeString(strings.TrimPrefix(got, prefix)) + if err != nil || string(raw) != tt.ref.String() { + t.Errorf("decoded ref = %q, err=%v, want %q", raw, err, tt.ref) + } + }) + } +} diff --git a/internal/storage/tigris/refcache.go b/internal/storage/tigris/refcache.go index a618ee8..d94e759 100644 --- a/internal/storage/tigris/refcache.go +++ b/internal/storage/tigris/refcache.go @@ -34,6 +34,15 @@ type refCache struct { loose map[plumbing.ReferenceName]*plumbing.Reference } +// refCommitCleanupError means the packed-refs write landed, but removing +// legacy loose refs failed afterward. Callers must inspect the effective refs: +// an old loose ref can still shadow the packed value until cleanup succeeds. +type refCommitCleanupError struct{ err error } + +func (e *refCommitCleanupError) Error() string { return e.err.Error() } +func (e *refCommitCleanupError) Unwrap() error { return e.err } +func (e *refCommitCleanupError) RefUpdateCommitted() bool { return true } + func newRefCache() *refCache { return &refCache{} } // ensureRefsBuilt fills the cache once. Two calls: one GetObject for @@ -129,11 +138,13 @@ func (s *Storer) invalidateRefs() { // something is wrong that another attempt will not fix. const maxRefCASRetries = 8 -// refExpectation is one CheckAndSetReference precondition: the caller believes -// that name currently holds old. A nil old means "the ref must not exist". +// refExpectation records the tip a caller advertised. A strict nil old +// requires the ref to be absent; the older CheckAndSetReference path retains +// its lenient nil/missing behavior when strict is false. type refExpectation struct { - name plumbing.ReferenceName - old *plumbing.Reference + name plumbing.ReferenceName + old *plumbing.Reference + strict bool } // commitRefs applies a whole batch of ref mutations in one conditional @@ -145,6 +156,12 @@ type refExpectation struct { // upload has not finished — and one flush per batch instead of one per ref is // most of why this path is faster than the loose one. func (s *Storer) commitRefs(sets []*plumbing.Reference, removes []plumbing.ReferenceName, expect []refExpectation) error { + return s.commitRefsNotifying(sets, removes, expect, nil) +} + +// commitRefsNotifying calls onCommit immediately after the conditional packed +// write succeeds, before legacy loose-ref cleanup can delay the caller. +func (s *Storer) commitRefsNotifying(sets []*plumbing.Reference, removes []plumbing.ReferenceName, expect []refExpectation, onCommit func(time.Time)) error { if len(sets) == 0 && len(removes) == 0 { return nil } @@ -212,6 +229,9 @@ func (s *Storer) commitRefs(sets []*plumbing.Reference, removes []plumbing.Refer default: return fmt.Errorf("tigris: commit refs: %w", err) } + if onCommit != nil { + onCommit(time.Now()) + } // The commit landed. Adopt it in place rather than re-reading, and hand // the folded names to the loose-key cleanup. @@ -222,7 +242,10 @@ func (s *Storer) commitRefs(sets []*plumbing.Reference, removes []plumbing.Refer s.refs.built = true s.refs.mu.Unlock() - return s.dropFoldedLooseRefs(folded, removes) + if err := s.dropFoldedLooseRefs(folded, removes); err != nil { + return &refCommitCleanupError{err: err} + } + return nil } return fmt.Errorf("%w after %d attempts", ErrRefContention, maxRefCASRetries) } @@ -235,6 +258,10 @@ func checkRefExpectations(view map[plumbing.ReferenceName]*plumbing.Reference, e for _, e := range expect { cur, ok := view[e.name] switch { + case e.strict && e.old == nil && ok: + return storage.ErrReferenceHasChanged + case e.strict && e.old != nil && (!ok || cur.Hash() != e.old.Hash()): + return storage.ErrReferenceHasChanged case e.old == nil: // A nil old is lenient, matching the in-memory storer: a missing // current reference falls through to creation. diff --git a/internal/storage/tigris/refcommit_test.go b/internal/storage/tigris/refcommit_test.go index ca153a2..5160e0f 100644 --- a/internal/storage/tigris/refcommit_test.go +++ b/internal/storage/tigris/refcommit_test.go @@ -226,6 +226,44 @@ func TestCommitRefsRevalidatesExpectations(t *testing.T) { } } +func TestUpdateReferencesCheckedRejectsRacingTip(t *testing.T) { + for _, tt := range []struct { + name string + seed bool + old string + }{ + {name: "update races another update", seed: true, old: headAB}, + {name: "create races another create", old: plumbing.ZeroHash.String()}, + } { + t.Run(tt.name, func(t *testing.T) { + f := newFakeS3(t) + s := packedTestStorer(t, f) + if tt.seed { + seedPackedRefs(t, f, s, hashRef("refs/heads/main", headAB)) + } + f.putHook = func(p *putSnapshot) { + if p.n == 1 { + f.putLocked(packedRefsKey, string(encodePackedRefs(refSet(hashRef("refs/heads/main", headCD)))), nil) + } + } + acceptedAt, err := s.UpdateReferencesChecked( + []*plumbing.Reference{hashRef("refs/heads/main", headAB)}, nil, + map[plumbing.ReferenceName]plumbing.Hash{"refs/heads/main": plumbing.NewHash(tt.old)}, + ) + if !errors.Is(err, storage.ErrReferenceHasChanged) { + t.Fatalf("checked update error = %v, want ErrReferenceHasChanged", err) + } + if !acceptedAt.IsZero() { + t.Errorf("rejected update has acceptance time %v", acceptedAt) + } + ref, err := s.Reference("refs/heads/main") + if err != nil || ref.Hash().String() != headCD { + t.Errorf("racing value was overwritten: ref=%v err=%v", ref, err) + } + }) + } +} + // TestCommitRefsFlushesOnce pins the invariant refs.go protects — a ref never // names an object whose upload has not finished — and pins that a batch pays // for one flush rather than one per ref. diff --git a/internal/storage/tigris/reffold_test.go b/internal/storage/tigris/reffold_test.go index e84ec41..3c872c1 100644 --- a/internal/storage/tigris/reffold_test.go +++ b/internal/storage/tigris/reffold_test.go @@ -118,6 +118,10 @@ func TestFoldStoppedBeforeCleanupKeepsRefsReadable(t *testing.T) { if err == nil { t.Fatal("a failed loose-key delete must surface, not be swallowed") } + var committed interface{ RefUpdateCommitted() bool } + if !errors.As(err, &committed) || !committed.RefUpdateCommitted() { + t.Errorf("cleanup error did not identify the completed packed write: %v", err) + } // packed-refs holds both refs, and the loose key is still there. if _, ok := f.objs[packedRefsKey]; !ok { @@ -150,6 +154,29 @@ func TestFoldStoppedBeforeCleanupKeepsRefsReadable(t *testing.T) { } } +func TestCheckedUpdateRecordsCommitBeforeCleanupFailure(t *testing.T) { + f := newFakeS3(t) + s := packedTestStorer(t, f) + f.put(refPrefix+"refs/heads/main", headAB+"\n", nil) + f.batchDelErr = errors.New("delete failed") + + acceptedAt, err := s.UpdateReferencesChecked( + []*plumbing.Reference{hashRef("refs/heads/topic", headCD)}, nil, + map[plumbing.ReferenceName]plumbing.Hash{"refs/heads/topic": plumbing.ZeroHash}, + ) + if acceptedAt.IsZero() { + t.Error("packed write has no acceptance timestamp") + } + var committed interface{ RefUpdateCommitted() bool } + if !errors.As(err, &committed) || !committed.RefUpdateCommitted() { + t.Fatalf("cleanup error did not identify committed write: %v", err) + } + ref, err := s.Reference("refs/heads/topic") + if err != nil || ref.Hash().String() != headCD { + t.Errorf("new ref is not visible after cleanup failure: ref=%v err=%v", ref, err) + } +} + // TestRemoveDeletesTheLooseKeyToo pins that a removal cannot resurrect from // the loose layer. func TestRemoveDeletesTheLooseKeyToo(t *testing.T) { diff --git a/internal/storage/tigris/refs.go b/internal/storage/tigris/refs.go index 687186c..a83356b 100644 --- a/internal/storage/tigris/refs.go +++ b/internal/storage/tigris/refs.go @@ -88,6 +88,33 @@ func (s *Storer) UpdateReferences(sets []*plumbing.Reference, removes []plumbing return nil } +// SupportsCheckedRefUpdates reports whether this storer can compare every +// advertised old hash at the atomic packed-refs commit point. Loose refs have +// no batch compare-and-swap, so receive-pack uses its per-command path there. +func (s *Storer) SupportsCheckedRefUpdates() bool { return s.packedRefs } + +// UpdateReferencesChecked applies a batch only if every ref still has the +// value advertised by the pushing client. A zero expected hash requires the +// ref to be absent. The check is repeated after each packed-refs CAS retry. +// It returns the packed write's acceptance time, even when subsequent +// loose-ref cleanup fails. +func (s *Storer) UpdateReferencesChecked(sets []*plumbing.Reference, removes []plumbing.ReferenceName, expected map[plumbing.ReferenceName]plumbing.Hash) (time.Time, error) { + if !s.packedRefs { + return time.Time{}, errors.New("checked ref updates require packed refs") + } + expectations := make([]refExpectation, 0, len(expected)) + for name, old := range expected { + var ref *plumbing.Reference + if !old.IsZero() { + ref = plumbing.NewHashReference(name, old) + } + expectations = append(expectations, refExpectation{name: name, old: ref, strict: true}) + } + var acceptedAt time.Time + err := s.commitRefsNotifying(sets, removes, expectations, func(at time.Time) { acceptedAt = at }) + return acceptedAt, err +} + // setLooseReference writes one refs/ object. It flushes every upload // queued through this Storer first, so a ref can never point at an object that // failed — or hasn't yet finished — its asynchronous upload (see upload.go). diff --git a/internal/webhook/webhook.go b/internal/webhook/webhook.go new file mode 100644 index 0000000..7c6428f --- /dev/null +++ b/internal/webhook/webhook.go @@ -0,0 +1,288 @@ +// Package webhook delivers signed push events to operator-configured HTTP endpoints. +package webhook + +import ( + "bytes" + "context" + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/netip" + "net/url" + "os" + "strings" + "time" + + pushv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/events/push/v1" + "github.com/tigrisdata/objgit/internal/repofs" + "google.golang.org/protobuf/encoding/protojson" +) + +const ( + deliveryTimeout = 5 * time.Second + attemptTimeout = 2 * time.Second + maxAttempts = 3 + maxConfigBytes = 1 << 20 +) + +// Client is an immutable collection of webhook destinations. Its zero value +// has no destinations and is safe to use. +type Client struct { + repositories map[string]destination +} + +type destination struct { + url string + secret []byte + client *http.Client +} + +type config struct { + Repositories map[string]repositoryConfig `json:"repositories"` +} + +type repositoryConfig struct { + URL string `json:"url"` + Secret string `json:"secret"` +} + +// Load reads a JSON configuration file with a repositories object mapping +// canonical "org/name" paths to URL and secret pairs. An empty path disables +// webhook delivery. +func Load(path string) (*Client, error) { + c := &Client{repositories: make(map[string]destination)} + if path == "" { + return c, nil + } + + f, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("open webhook configuration: %w", err) + } + defer f.Close() + + data, err := io.ReadAll(io.LimitReader(f, maxConfigBytes+1)) + if err != nil { + return nil, fmt.Errorf("read webhook configuration: %w", err) + } + if len(data) > maxConfigBytes { + return nil, errors.New("webhook configuration exceeds 1 MiB") + } + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + var cfg config + if err := dec.Decode(&cfg); err != nil { + return nil, fmt.Errorf("decode webhook configuration: %w", err) + } + var trailing any + if err := dec.Decode(&trailing); err != io.EOF { + return nil, errors.New("webhook configuration contains trailing data") + } + + for repo, entry := range cfg.Repositories { + ref, err := repofs.Parse(repo) + if err != nil || ref.Path() != repo { + return nil, fmt.Errorf("webhook configuration has invalid repository %q", repo) + } + if entry.Secret == "" { + return nil, fmt.Errorf("webhook configuration for %q has no secret", repo) + } + u, loopback, err := validateURL(entry.URL) + if err != nil { + return nil, fmt.Errorf("webhook configuration for %q: %w", repo, err) + } + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = nil + transport.DialContext = safeDialContext(u.Hostname(), loopback) + transport.MaxIdleConnsPerHost = 2 + c.repositories[repo] = destination{ + url: u.String(), + secret: []byte(entry.Secret), + client: &http.Client{ + Timeout: attemptTimeout, + Transport: transport, + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, + }, + } + } + return c, nil +} + +// Enabled reports whether repo has a configured webhook destination. +func (c *Client) Enabled(repo string) bool { + if c == nil { + return false + } + _, ok := c.repositories[repo] + return ok +} + +// Deliver sends a signed ProtoJSON push event. A delivery ID stays fixed over +// all attempts. The whole call is bounded by five seconds or the caller's +// earlier deadline. The caller decides how to handle a failed delivery. +func (c *Client) Deliver(ctx context.Context, repo string, event *pushv1.PushEvent) error { + if !c.Enabled(repo) { + return nil + } + if event == nil { + return errors.New("webhook event is nil") + } + if event.GetEventId() == "" { + return errors.New("webhook event ID is empty") + } + if event.GetRepository() != repo { + return fmt.Errorf("webhook event repository %q does not match destination %q", event.GetRepository(), repo) + } + + body, err := (protojson.MarshalOptions{EmitDefaultValues: true}).Marshal(event) + if err != nil { + return fmt.Errorf("marshal webhook event: %w", err) + } + dst := c.repositories[repo] + mac := hmac.New(sha256.New, dst.secret) + _, _ = mac.Write(body) + signature := "sha256=" + hex.EncodeToString(mac.Sum(nil)) + + ctx, cancel := context.WithTimeout(ctx, deliveryTimeout) + defer cancel() + var lastErr error + for attempt := range maxAttempts { + if err := ctx.Err(); err != nil { + return fmt.Errorf("deliver webhook: %w", err) + } + retry, err := dst.deliverOnce(ctx, body, event.GetEventId(), signature) + if err == nil { + return nil + } + lastErr = err + if !retry || attempt == maxAttempts-1 { + break + } + timer := time.NewTimer(time.Duration(100<= 200 && resp.StatusCode < 300 { + return false, nil + } + return resp.StatusCode == http.StatusTooManyRequests || resp.StatusCode >= 500, + fmt.Errorf("destination returned HTTP %d", resp.StatusCode) +} + +func validateURL(raw string) (*url.URL, bool, error) { + u, err := url.Parse(raw) + if err != nil { + return nil, false, errors.New("invalid webhook URL") + } + if u.Hostname() == "" || u.User != nil || u.Fragment != "" || u.Opaque != "" { + return nil, false, errors.New("webhook URL must have a host and no user info or fragment") + } + host := strings.TrimSuffix(strings.ToLower(u.Hostname()), ".") + ip, parseErr := netip.ParseAddr(host) + loopback := host == "localhost" || parseErr == nil && ip.Unmap().IsLoopback() + if u.Scheme != "https" && !(u.Scheme == "http" && loopback) { + return nil, false, errors.New("webhook URL must use HTTPS; HTTP is allowed only for loopback") + } + if parseErr == nil && !loopback && !publicIP(ip) { + return nil, false, errors.New("webhook URL has a nonpublic IP address") + } + if port := u.Port(); port != "" { + if _, err := net.LookupPort("tcp", port); err != nil { + return nil, false, errors.New("webhook URL has an invalid port") + } + } + return u, loopback, nil +} + +func safeDialContext(expectedHost string, loopback bool) func(context.Context, string, string) (net.Conn, error) { + return func(ctx context.Context, network, address string) (net.Conn, error) { + host, port, err := net.SplitHostPort(address) + if err != nil || !strings.EqualFold(strings.TrimSuffix(host, "."), strings.TrimSuffix(expectedHost, ".")) { + return nil, errors.New("webhook destination changed") + } + ips, err := net.DefaultResolver.LookupNetIP(ctx, "ip", host) + if err != nil { + return nil, fmt.Errorf("resolve webhook destination: %w", err) + } + dialer := &net.Dialer{Timeout: attemptTimeout} + var dialErr error + for _, ip := range ips { + if loopback && !ip.Unmap().IsLoopback() || !loopback && !publicIP(ip) { + continue + } + conn, err := dialer.DialContext(ctx, network, net.JoinHostPort(ip.String(), port)) + if err == nil { + return conn, nil + } + dialErr = err + } + if dialErr != nil { + return nil, dialErr + } + return nil, errors.New("webhook destination resolved only to disallowed addresses") + } +} + +var nonpublicRanges = []netip.Prefix{ + netip.MustParsePrefix("100.64.0.0/10"), + netip.MustParsePrefix("192.0.0.0/24"), + netip.MustParsePrefix("192.0.2.0/24"), + netip.MustParsePrefix("198.18.0.0/15"), + netip.MustParsePrefix("198.51.100.0/24"), + netip.MustParsePrefix("203.0.113.0/24"), + netip.MustParsePrefix("224.0.0.0/4"), + netip.MustParsePrefix("240.0.0.0/4"), + netip.MustParsePrefix("2001:db8::/32"), +} + +func publicIP(ip netip.Addr) bool { + ip = ip.Unmap() + if !ip.IsGlobalUnicast() || ip.IsPrivate() || ip.IsLoopback() || ip.IsLinkLocalUnicast() { + return false + } + for _, prefix := range nonpublicRanges { + if prefix.Contains(ip) { + return false + } + } + return true +} diff --git a/internal/webhook/webhook_test.go b/internal/webhook/webhook_test.go new file mode 100644 index 0000000..d7f0cb1 --- /dev/null +++ b/internal/webhook/webhook_test.go @@ -0,0 +1,274 @@ +package webhook + +import ( + "context" + "crypto/hmac" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/netip" + "os" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "testing" + + pushv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/events/push/v1" + "google.golang.org/protobuf/encoding/protojson" + "google.golang.org/protobuf/proto" +) + +func writeConfig(t *testing.T, content string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "webhooks.json") + if err := os.WriteFile(path, []byte(content), 0600); err != nil { + t.Fatal(err) + } + return path +} + +func configFor(t *testing.T, endpoint string) string { + t.Helper() + data, err := json.Marshal(config{Repositories: map[string]repositoryConfig{ + "acme/widgets": {URL: endpoint, Secret: "test-secret"}, + }}) + if err != nil { + t.Fatal(err) + } + return writeConfig(t, string(data)) +} + +func TestLoad(t *testing.T) { + tests := []struct { + name string + config string + wantEnabled bool + wantError string + }{ + {name: "valid loopback", config: `{"repositories":{"acme/widgets":{"url":"http://127.0.0.1:1234/hook","secret":"key"}}}`, wantEnabled: true}, + {name: "valid public HTTPS", config: `{"repositories":{"acme/widgets":{"url":"https://example.com/hook","secret":"key"}}}`, wantEnabled: true}, + {name: "bad repository", config: `{"repositories":{"acme/widgets.git":{"url":"https://example.com/hook","secret":"key"}}}`, wantError: "invalid repository"}, + {name: "missing secret", config: `{"repositories":{"acme/widgets":{"url":"https://example.com/hook"}}}`, wantError: "no secret"}, + {name: "plain HTTP", config: `{"repositories":{"acme/widgets":{"url":"http://example.com/hook","secret":"key"}}}`, wantError: "must use HTTPS"}, + {name: "metadata IP", config: `{"repositories":{"acme/widgets":{"url":"https://169.254.169.254/latest","secret":"key"}}}`, wantError: "nonpublic IP"}, + {name: "private IP", config: `{"repositories":{"acme/widgets":{"url":"https://10.0.0.1/hook","secret":"key"}}}`, wantError: "nonpublic IP"}, + {name: "URL user info", config: `{"repositories":{"acme/widgets":{"url":"https://user:pass@example.com/hook","secret":"key"}}}`, wantError: "no user info"}, + {name: "unknown config field", config: `{"repositories":{},"extra":true}`, wantError: "unknown field"}, + {name: "trailing data", config: `{"repositories":{}} {}`, wantError: "trailing data"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c, err := Load(writeConfig(t, tt.config)) + if tt.wantError != "" { + if err == nil || !strings.Contains(err.Error(), tt.wantError) { + t.Fatalf("error = %v, want substring %q", err, tt.wantError) + } + return + } + if err != nil { + t.Fatal(err) + } + if got := c.Enabled("acme/widgets"); got != tt.wantEnabled { + t.Errorf("Enabled = %v, want %v", got, tt.wantEnabled) + } + }) + } +} + +func TestLoadEmptyPath(t *testing.T) { + c, err := Load("") + if err != nil { + t.Fatal(err) + } + if c.Enabled("acme/widgets") { + t.Error("empty path enabled a webhook") + } + if err := c.Deliver(context.Background(), "acme/widgets", nil); err != nil { + t.Errorf("disabled Deliver = %v, want nil", err) + } +} + +func TestDeliver(t *testing.T) { + event := &pushv1.PushEvent{ + EventId: "delivery-1", + Repository: "acme/widgets", + Ref: "refs/heads/main", + Commits: []*pushv1.Commit{{ + Id: "abc123", + Files: &pushv1.FileChanges{Added: []string{"one.txt"}}, + }}, + } + var mu sync.Mutex + var gotBodies [][]byte + var gotHeaders []http.Header + attempts := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, err := io.ReadAll(r.Body) + if err != nil { + t.Errorf("read request: %v", err) + } + mu.Lock() + defer mu.Unlock() + attempts++ + gotBodies = append(gotBodies, body) + gotHeaders = append(gotHeaders, r.Header.Clone()) + if attempts < 3 { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + w.WriteHeader(http.StatusNoContent) + })) + defer server.Close() + + c, err := Load(configFor(t, server.URL+"/hook")) + if err != nil { + t.Fatal(err) + } + if err := c.Deliver(context.Background(), "acme/widgets", event); err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + if attempts != 3 { + t.Fatalf("attempts = %d, want 3", attempts) + } + for i, body := range gotBodies { + if got := gotHeaders[i].Get("Content-Type"); got != "application/json" { + t.Errorf("Content-Type = %q", got) + } + if got := gotHeaders[i].Get("X-Objgit-Event"); got != "push" { + t.Errorf("X-Objgit-Event = %q", got) + } + if got := gotHeaders[i].Get("X-Objgit-Delivery"); got != event.EventId { + t.Errorf("X-Objgit-Delivery = %q", got) + } + mac := hmac.New(sha256.New, []byte("test-secret")) + _, _ = mac.Write(body) + wantSignature := "sha256=" + hex.EncodeToString(mac.Sum(nil)) + if got := gotHeaders[i].Get("X-Objgit-Signature-256"); got != wantSignature { + t.Errorf("signature = %q, want %q", got, wantSignature) + } + var decoded pushv1.PushEvent + if err := protojson.Unmarshal(body, &decoded); err != nil { + t.Fatal(err) + } + if !proto.Equal(&decoded, event) { + t.Errorf("decoded event = %v, want %v", &decoded, event) + } + if !strings.Contains(string(body), `"created":false`) || !strings.Contains(string(body), `"deleted":false`) { + t.Errorf("default scalar fields missing from body: %s", body) + } + if i > 0 && string(body) != string(gotBodies[0]) { + t.Error("retry body changed") + } + } +} + +func TestDeliverNoRedirect(t *testing.T) { + var followed atomic.Int32 + target := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) { followed.Add(1) })) + defer target.Close() + source := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, target.URL, http.StatusTemporaryRedirect) + })) + defer source.Close() + c, err := Load(configFor(t, source.URL)) + if err != nil { + t.Fatal(err) + } + err = c.Deliver(context.Background(), "acme/widgets", &pushv1.PushEvent{EventId: "id", Repository: "acme/widgets"}) + if err == nil || !strings.Contains(err.Error(), "HTTP 307") { + t.Errorf("Deliver error = %v, want HTTP 307", err) + } + if got := followed.Load(); got != 0 { + t.Errorf("redirect followed %d times", got) + } +} + +func TestDeliverPermanentFailure(t *testing.T) { + var attempts atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts.Add(1) + w.WriteHeader(http.StatusBadRequest) + })) + defer server.Close() + c, err := Load(configFor(t, server.URL)) + if err != nil { + t.Fatal(err) + } + err = c.Deliver(context.Background(), "acme/widgets", &pushv1.PushEvent{EventId: "id", Repository: "acme/widgets"}) + if err == nil || !strings.Contains(err.Error(), "HTTP 400") { + t.Errorf("Deliver error = %v, want HTTP 400", err) + } + if got := attempts.Load(); got != 1 { + t.Errorf("attempts = %d, want 1", got) + } +} + +func TestDeliverBadEvent(t *testing.T) { + c, err := Load(configFor(t, "http://127.0.0.1:1234")) + if err != nil { + t.Fatal(err) + } + tests := []struct { + name string + event *pushv1.PushEvent + wantError string + }{ + {name: "nil", wantError: "nil"}, + {name: "no ID", event: &pushv1.PushEvent{Repository: "acme/widgets"}, wantError: "ID is empty"}, + {name: "wrong repository", event: &pushv1.PushEvent{EventId: "id", Repository: "other/repo"}, wantError: "does not match"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := c.Deliver(context.Background(), "acme/widgets", tt.event) + if err == nil || !strings.Contains(err.Error(), tt.wantError) { + t.Errorf("Deliver error = %v, want substring %q", err, tt.wantError) + } + }) + } +} + +func TestPublicIP(t *testing.T) { + tests := []struct { + name string + ip string + want bool + }{ + {name: "public IPv4", ip: "8.8.8.8", want: true}, + {name: "public IPv6", ip: "2606:4700:4700::1111", want: true}, + {name: "private IPv4", ip: "172.16.0.1"}, + {name: "link local", ip: "169.254.169.254"}, + {name: "carrier NAT", ip: "100.64.0.1"}, + {name: "benchmark", ip: "198.19.0.1"}, + {name: "mapped loopback", ip: "::ffff:127.0.0.1"}, + {name: "documentation IPv6", ip: "2001:db8::1"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ip := netip.MustParseAddr(tt.ip) + if got := publicIP(ip); got != tt.want { + t.Errorf("publicIP(%s) = %v, want %v", tt.ip, got, tt.want) + } + }) + } +} + +func TestDeliverDoesNotExposeURLQuery(t *testing.T) { + c, err := Load(configFor(t, "http://127.0.0.1:1/hook?token=hidden-token")) + if err != nil { + t.Fatal(err) + } + err = c.Deliver(context.Background(), "acme/widgets", &pushv1.PushEvent{EventId: "id", Repository: "acme/widgets"}) + if err == nil { + t.Fatal("Deliver succeeded against a closed port") + } + if strings.Contains(fmt.Sprint(err), "hidden-token") { + t.Errorf("error exposed URL query: %v", err) + } +} diff --git a/proto/README.md b/proto/README.md new file mode 100644 index 0000000..0c059ef --- /dev/null +++ b/proto/README.md @@ -0,0 +1,28 @@ +# Event schemas + +`proto/` is the Buf module root. Push webhook messages are defined in +[`tigrisdata/objgit/events/push/v1/push.proto`](tigrisdata/objgit/events/push/v1/push.proto). +The adjacent [`push.example.json`](tigrisdata/objgit/events/push/v1/push.example.json) +is a complete raw ProtoJSON payload with two commits and file changes. Keep it +in sync when changing the schema; future generated documentation can include +it as an example. + +Run these commands from the repository root: + +```text +buf format --diff --exit-code +buf lint +buf build +buf generate +``` + +`buf generate` writes Go types under `gen/`, following the Xe/x Buf layout. +The schema does not yet carry gnostic OpenAPI annotations. Its versioned +package, comments, and example are the starting point for adding those +annotations and generated docs later. + +Validate the raw example as ProtoJSON with: + +```text +buf convert proto --type=tigrisdata.objgit.events.push.v1.PushEvent --from=proto/tigrisdata/objgit/events/push/v1/push.example.json --to=-#format=binpb > /dev/null +``` diff --git a/proto/tigrisdata/objgit/events/push/v1/push.example.json b/proto/tigrisdata/objgit/events/push/v1/push.example.json new file mode 100644 index 0000000..cec2140 --- /dev/null +++ b/proto/tigrisdata/objgit/events/push/v1/push.example.json @@ -0,0 +1,64 @@ +{ + "eventId": "11111111-1111-4111-8111-111111111111", + "pushId": "22222222-2222-4222-8222-222222222222", + "repository": "acme/widgets", + "ref": "refs/heads/main", + "before": "1111111111111111111111111111111111111111", + "after": "3333333333333333333333333333333333333333", + "created": false, + "deleted": false, + "forced": false, + "pushedAt": "2026-09-23T18:00:00Z", + "commits": [ + { + "id": "2222222222222222222222222222222222222222", + "treeId": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", + "message": "Add API draft", + "author": { + "name": "A. Developer", + "email": "dev@example.com", + "when": "2026-09-23T17:30:00Z" + }, + "committer": { + "name": "A. Developer", + "email": "dev@example.com", + "when": "2026-09-23T17:30:00Z" + }, + "parentIds": ["1111111111111111111111111111111111111111"], + "files": { + "added": ["api/openapi.yaml"], + "changed": ["README.md"], + "deleted": [] + } + }, + { + "id": "3333333333333333333333333333333333333333", + "treeId": "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb", + "message": "Add push event schema", + "author": { + "name": "A. Developer", + "email": "dev@example.com", + "when": "2026-09-23T17:45:00Z" + }, + "committer": { + "name": "A. Developer", + "email": "dev@example.com", + "when": "2026-09-23T17:45:00Z" + }, + "parentIds": ["2222222222222222222222222222222222222222"], + "files": { + "added": ["proto/tigrisdata/objgit/events/push/v1/push.proto"], + "changed": ["README.md"], + "deleted": ["old/schema.json"] + } + } + ], + "files": { + "added": [ + "api/openapi.yaml", + "proto/tigrisdata/objgit/events/push/v1/push.proto" + ], + "changed": ["README.md"], + "deleted": ["old/schema.json"] + } +} diff --git a/proto/tigrisdata/objgit/events/push/v1/push.proto b/proto/tigrisdata/objgit/events/push/v1/push.proto new file mode 100644 index 0000000..d0225a6 --- /dev/null +++ b/proto/tigrisdata/objgit/events/push/v1/push.proto @@ -0,0 +1,92 @@ +syntax = "proto3"; + +package tigrisdata.objgit.events.push.v1; + +import "google/protobuf/timestamp.proto"; + +// PushEvent describes one accepted Git ref update. A push that updates several +// refs produces one event per ref, with the same push_id in each event. +message PushEvent { + // A stable identifier for this event, reused across delivery attempts. + string event_id = 1; + + // Identifies the receive-pack request that produced this ref update. + string push_id = 2; + + // Canonical repository path, such as "acme/widgets". + string repository = 3; + + // Full Git ref name, such as "refs/heads/main". Non-UTF-8 ref names use + // the reversible base64url representation described for FileChanges paths. + string ref = 4; + + // Ref value before the update, or the all-zero hash for a new ref. + string before = 5; + + // Ref value after the update, or the all-zero hash for a deleted ref. + string after = 6; + + // True when this update created the ref. + bool created = 7; + + // True when this update deleted the ref. + bool deleted = 8; + + // True for a non-fast-forward update of an existing ref; false for ref + // creation and deletion. + bool forced = 9; + + // Time the server accepted the ref update, not a commit's author time. + google.protobuf.Timestamp pushed_at = 10; + + // Every commit reachable from after but not reachable from before, with + // parents before children. Empty for deletion or a non-commit ref target. + repeated Commit commits = 11; + + // Net file changes from the before tree to the after tree. A file added + // and then deleted within this push does not appear here; its two changes + // still appear in the corresponding commits. + FileChanges files = 12; +} + +// Commit describes a commit included in a ref update. +message Commit { + // Git object ID of the commit. + string id = 1; + + // Git object ID of the commit's tree. + string tree_id = 2; + + // Full commit message. Invalid UTF-8 bytes are replaced with U+FFFD. + string message = 3; + + Signature author = 4; + Signature committer = 5; + + // Parent commit IDs, in Git's recorded order. + repeated string parent_ids = 6; + + // File changes against the first parent's tree, or the empty tree for a + // root commit. For merges, this is a first-parent diff. + FileChanges files = 7; +} + +// Signature records a Git commit's author or committer identity and time. +// Invalid UTF-8 bytes in name and email are replaced with U+FFFD. +message Signature { + string name = 1; + string email = 2; + google.protobuf.Timestamp when = 3; +} + +// FileChanges lists paths only; it does not include file contents or patches. +// A rename appears as one deleted path and one added path. Mode-only changes +// appear in changed. Valid UTF-8 paths are literal. A path containing invalid +// UTF-8 bytes is encoded as "/objgit/raw-path/base64url/" followed by the +// unpadded base64url encoding of its original bytes. Git paths cannot start +// with "/", so this representation is reversible without collisions. +message FileChanges { + repeated string added = 1; + repeated string changed = 2; + repeated string deleted = 3; +} From 36ae1a24aec7d0163f220510663b1711c1883468 Mon Sep 17 00:00:00 2001 From: Xe Iaso Date: Wed, 23 Sep 2026 16:05:15 -0400 Subject: [PATCH 2/3] feat: store webhook settings per repository Load each repository's ProtoJSON settings from bucket state and expose raw settings queries through the Admin Authorizer operation on HTTP and SSH. Assisted-by: GPT-6 via Codex Signed-off-by: Xe Iaso --- cmd/objgitd/git_protocol.go | 4 +- cmd/objgitd/hooks.go | 9 +- cmd/objgitd/http.go | 1 + cmd/objgitd/main.go | 17 +-- cmd/objgitd/ssh.go | 9 ++ cmd/objgitd/webhook_settings.go | 89 +++++++++++ cmd/objgitd/webhook_settings_test.go | 142 ++++++++++++++++++ cmd/objgitd/webhooks.go | 7 +- cmd/objgitd/webhooks_test.go | 68 +++++---- docs/architecture/auth.md | 26 ++-- docs/architecture/s3fs.md | 8 +- docs/plans/webhook-push-events.md | 4 + docs/usage/webhooks.md | 63 +++++--- .../objgit/webhooks/v1/settings.pb.go | 138 +++++++++++++++++ internal/auth/auth.go | 18 ++- internal/auth/auth_test.go | 1 + internal/metrics/metrics.go | 5 +- internal/webhook/webhook.go | 124 ++++++++------- internal/webhook/webhook_test.go | 73 +++++---- proto/README.md | 4 + .../objgit/webhooks/v1/settings.example.json | 4 + .../objgit/webhooks/v1/settings.proto | 14 ++ 22 files changed, 646 insertions(+), 182 deletions(-) create mode 100644 cmd/objgitd/webhook_settings.go create mode 100644 cmd/objgitd/webhook_settings_test.go create mode 100644 gen/tigrisdata/objgit/webhooks/v1/settings.pb.go create mode 100644 proto/tigrisdata/objgit/webhooks/v1/settings.example.json create mode 100644 proto/tigrisdata/objgit/webhooks/v1/settings.proto diff --git a/cmd/objgitd/git_protocol.go b/cmd/objgitd/git_protocol.go index 38e7627..4f335e2 100644 --- a/cmd/objgitd/git_protocol.go +++ b/cmd/objgitd/git_protocol.go @@ -15,7 +15,6 @@ import ( "github.com/tigrisdata/objgit/internal/auth" "github.com/tigrisdata/objgit/internal/metrics" "github.com/tigrisdata/objgit/internal/repofs" - "github.com/tigrisdata/objgit/internal/webhook" ) // handshakeTimeout bounds how long a client has to send its git-proto-request. @@ -46,7 +45,7 @@ func (d *daemon) authorize(ctx context.Context, req auth.Request) auth.Decision // repo. type daemon struct { // sysFS holds daemon-level state that is not scoped to a repository (the SSH - // host key); repository storage is resolved per request via resolver. + // host key and webhook settings); repository storage is resolved per request. sysFS billy.Filesystem resolver repofs.Resolver authz auth.Authorizer @@ -54,7 +53,6 @@ type daemon struct { // allowHooks gates running .objgit/hooks/receive-pack after a push. allowHooks bool hookTimeout time.Duration - webhooks *webhook.Client // pushes bounds how many pushes unpack a packfile at once, which is the // only thing that bounds the daemon's resident set under concurrent pushes. diff --git a/cmd/objgitd/hooks.go b/cmd/objgitd/hooks.go index 40b2e82..cef5a76 100644 --- a/cmd/objgitd/hooks.go +++ b/cmd/objgitd/hooks.go @@ -21,6 +21,7 @@ import ( "github.com/tigrisdata/objgit/internal/mountfs" "github.com/tigrisdata/objgit/internal/pushevents" "github.com/tigrisdata/objgit/internal/treefs" + "github.com/tigrisdata/objgit/internal/webhook" "mvdan.cc/sh/v3/expand" "mvdan.cc/sh/v3/interp" "mvdan.cc/sh/v3/syntax" @@ -88,7 +89,11 @@ func diffRefs(before, after map[plumbing.ReferenceName]plumbing.Hash) []refUpdat // both land here, and git:// never serves receive-pack at all — so it is where // the push concurrency cap is applied, via the d.pushes.admit seam. func (d *daemon) receivePack(ctx context.Context, st storage.Storer, repoPath string, r io.ReadCloser, w io.WriteCloser, req *transport.ReceivePackRequest) error { - if !d.allowHooks && (d.webhooks == nil || !d.webhooks.Enabled(repoPath)) { + webhooks, settingsErr := webhook.Load(d.sysFS, repoPath) + if settingsErr != nil { + slog.Error("webhook: load repository settings", "repo", repoPath, "err", settingsErr) + } + if !d.allowHooks && (webhooks == nil || !webhooks.Enabled(repoPath)) { err := receivePackStreaming(ctx, st, r, w, req, d.pushes.admit, nil) d.healHEADAfterPush(err, st, repoPath) return err @@ -107,7 +112,7 @@ func (d *daemon) receivePack(ctx context.Context, st storage.Storer, repoPath st } d.runHooks(repoPath, "receive-pack", st, branches, progress) } - d.emitPushWebhooks(ctx, st, repoPath, updates, acceptedAt) + d.emitPushWebhooks(ctx, st, repoPath, webhooks, updates, acceptedAt) } err := receivePackStreaming(ctx, st, r, w, req, d.pushes.admit, onUpdated) diff --git a/cmd/objgitd/http.go b/cmd/objgitd/http.go index e5bf428..ef496b2 100644 --- a/cmd/objgitd/http.go +++ b/cmd/objgitd/http.go @@ -25,6 +25,7 @@ import ( // matches a pattern and falls through to ServeMux's 404. func (d *daemon) httpHandler() http.Handler { mux := http.NewServeMux() + mux.HandleFunc("GET /_objgit/webhooks/{orgID}/{repoName}/settings", d.handleHTTPWebhookSettings) mux.HandleFunc("GET /{orgID}/{repoName}/info/refs", d.handleInfoRefs) mux.HandleFunc("POST /{orgID}/{repoName}/git-upload-pack", func(w http.ResponseWriter, r *http.Request) { d.handleRPC(w, r, transport.UploadPackService) diff --git a/cmd/objgitd/main.go b/cmd/objgitd/main.go index a78f0e1..9ece4e0 100644 --- a/cmd/objgitd/main.go +++ b/cmd/objgitd/main.go @@ -26,7 +26,6 @@ import ( "github.com/tigrisdata/objgit/internal/repofs" "github.com/tigrisdata/objgit/internal/s3fs" "github.com/tigrisdata/objgit/internal/storage/tigris" - "github.com/tigrisdata/objgit/internal/webhook" tstorage "github.com/tigrisdata/storage-go" "golang.org/x/sync/errgroup" @@ -41,9 +40,8 @@ var ( allowPush = flag.Bool("allow-push", false, "allow unauthenticated git-receive-pack (push) requests") slogLevel = flag.String("slog-level", "INFO", "log level (DEBUG, INFO, WARN, ERROR)") - allowHooks = flag.Bool("allow-hooks", false, "run .objgit/hooks/receive-pack in a sandbox after a successful push") - hookTimeout = flag.Duration("hook-timeout", 60*time.Second, "wall-clock limit for a single hook run") - webhookConfig = flag.String("webhook-config", "", "path to JSON per-repository webhook endpoints and signing secrets; empty disables webhooks") + allowHooks = flag.Bool("allow-hooks", false, "run .objgit/hooks/receive-pack in a sandbox after a successful push") + hookTimeout = flag.Duration("hook-timeout", 60*time.Second, "wall-clock limit for a single hook run") packCacheDir = flag.String("pack-cache-dir", "", "parent directory for the local pack cache; empty uses the OS temp directory") packCacheBytes = flag.Int64("pack-cache-bytes", 2<<30, "disk budget for the local pack cache, least-recently-used eviction; 0 disables caching") @@ -84,11 +82,6 @@ func main() { slog.Error("at least one of -http-bind or -ssh-bind must be set") os.Exit(1) } - webhooks, err := webhook.Load(*webhookConfig) - if err != nil { - slog.Error("can't load webhook configuration", "path", *webhookConfig, "err", err) - os.Exit(1) - } ctx, cancel := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) defer cancel() @@ -108,8 +101,8 @@ func main() { } // Harden the client's HTTP path so stale keep-alive connections to Tigris // fail fast and retry on a fresh connection instead of hanging the request - // forever (see internal/s3fs/resilient.go). Only sysFS (the SSH host key) - // uses this client; internal/storage/tigris dials its own. + // forever (see internal/s3fs/resilient.go). Only sysFS (the SSH host key + // and webhook settings) uses this client; internal/storage/tigris dials its own. client := s3fs.Harden(rawClient) fsys, err := s3fs.NewS3FS(client, *bucket) @@ -152,7 +145,6 @@ func main() { authz: auth.AllowAnonymous{AllowWrite: *allowPush}, allowHooks: *allowHooks, hookTimeout: *hookTimeout, - webhooks: webhooks, pushes: newPushLimiter(*maxConcurrentPushes, *pushQueueTimeout), } @@ -164,7 +156,6 @@ func main() { "bucket", *bucket, "allow_push", *allowPush, "allow_hooks", *allowHooks, - "webhook_configured", *webhookConfig != "", "pack_cache_bytes", *packCacheBytes, "max_concurrent_pushes", *maxConcurrentPushes, "push_queue_timeout", *pushQueueTimeout, diff --git a/cmd/objgitd/ssh.go b/cmd/objgitd/ssh.go index eeb61de..02ca1f5 100644 --- a/cmd/objgitd/ssh.go +++ b/cmd/objgitd/ssh.go @@ -127,6 +127,15 @@ func (d *daemon) handleSSH(s ssh.Session) { _ = s.Exit(1) return } + if cmd[0] == "objgit-webhook-settings" { + if len(cmd) != 2 { + fmt.Fprintln(s.Stderr(), "objgitd: usage: objgit-webhook-settings org/repo") + _ = s.Exit(1) + return + } + d.handleSSHWebhookSettings(s, cmd[1]) + return + } service, ok := gitServiceFor(cmd[0]) if !ok { diff --git a/cmd/objgitd/webhook_settings.go b/cmd/objgitd/webhook_settings.go new file mode 100644 index 0000000..400eae0 --- /dev/null +++ b/cmd/objgitd/webhook_settings.go @@ -0,0 +1,89 @@ +package main + +import ( + "fmt" + "log/slog" + "net/http" + + ssh "github.com/gliderlabs/ssh" + "github.com/tigrisdata/objgit/internal/auth" + "github.com/tigrisdata/objgit/internal/repofs" + "github.com/tigrisdata/objgit/internal/webhook" + "google.golang.org/protobuf/encoding/protojson" +) + +func (d *daemon) handleHTTPWebhookSettings(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-store") + ref, ok := repoRef(w, r) + if !ok { + return + } + cred, _ := credFromRequest(r) + decision := d.authorize(r.Context(), auth.Request{ + Repo: ref.Path(), Operation: auth.Admin, Cred: cred, Transport: "http", + }) + switch decision { + case auth.Allow: + case auth.Unauthenticated: + w.Header().Set("WWW-Authenticate", `Basic realm="objgit admin"`) + http.Error(w, "admin authentication required", http.StatusUnauthorized) + return + default: + http.Error(w, "admin access denied", http.StatusForbidden) + return + } + body, found, err := d.webhookSettingsJSON(ref) + if err != nil { + slog.Error("read webhook settings", "repo", ref.Path(), "err", err) + http.Error(w, "cannot read webhook settings", http.StatusInternalServerError) + return + } + if !found { + http.NotFound(w, r) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write(body) +} + +func (d *daemon) handleSSHWebhookSettings(s ssh.Session, repo string) { + ref, err := repofs.Parse(repo) + if err != nil { + fmt.Fprintln(s.Stderr(), "objgitd: invalid repository") + _ = s.Exit(1) + return + } + var cred auth.Credential = auth.Anonymous{} + if key := s.PublicKey(); key != nil { + cred = auth.PublicKey{Key: key} + } + if d.authorize(s.Context(), auth.Request{ + Repo: ref.Path(), Operation: auth.Admin, Cred: cred, Transport: "ssh", + }) != auth.Allow { + fmt.Fprintln(s.Stderr(), "objgitd: admin access denied") + _ = s.Exit(1) + return + } + body, found, err := d.webhookSettingsJSON(ref) + if err != nil { + slog.Error("read webhook settings", "repo", ref.Path(), "err", err) + fmt.Fprintln(s.Stderr(), "objgitd: cannot read webhook settings") + _ = s.Exit(1) + return + } + if !found { + fmt.Fprintln(s.Stderr(), "objgitd: webhook settings not found") + _ = s.Exit(1) + return + } + _, _ = s.Write(append(body, '\n')) +} + +func (d *daemon) webhookSettingsJSON(ref repofs.RepoRef) ([]byte, bool, error) { + settings, err := webhook.ReadSettings(d.sysFS, ref.Path()) + if err != nil || settings == nil { + return nil, false, err + } + body, err := (protojson.MarshalOptions{EmitDefaultValues: true}).Marshal(settings) + return body, true, err +} diff --git a/cmd/objgitd/webhook_settings_test.go b/cmd/objgitd/webhook_settings_test.go new file mode 100644 index 0000000..a9e6d58 --- /dev/null +++ b/cmd/objgitd/webhook_settings_test.go @@ -0,0 +1,142 @@ +package main + +import ( + "bytes" + "context" + "net" + "net/http" + "net/http/httptest" + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/go-git/go-billy/v6/memfs" + "github.com/tigrisdata/objgit/internal/auth" + "github.com/tigrisdata/objgit/internal/webhook" + gossh "golang.org/x/crypto/ssh" +) + +type authorizeFunc func(context.Context, auth.Request) auth.Decision + +func (f authorizeFunc) Authorize(ctx context.Context, req auth.Request) auth.Decision { + return f(ctx, req) +} + +func TestSSHWebhookSettingsAuthorization(t *testing.T) { + checkSSHBinaries(t) + fs := memfs.New() + writeWebhookSettings(t, fs, "acme/widgets", "https://example.com/events") + keyPath := filepath.Join(t.TempDir(), "id_ed25519") + if out, err := exec.Command("ssh-keygen", "-t", "ed25519", "-N", "", "-f", keyPath).CombinedOutput(); err != nil { + t.Fatalf("ssh-keygen: %v: %s", err, out) + } + pub, err := os.ReadFile(keyPath + ".pub") + if err != nil { + t.Fatal(err) + } + adminKey, _, _, _, err := gossh.ParseAuthorizedKey(pub) + if err != nil { + t.Fatal(err) + } + authz := authorizeFunc(func(_ context.Context, req auth.Request) auth.Decision { + if req.Operation == auth.Admin && req.Transport == "ssh" { + if cred, ok := req.Cred.(auth.PublicKey); ok && cred.Key != nil && bytes.Equal(cred.Key.Marshal(), adminKey.Marshal()) { + return auth.Allow + } + } + return auth.Deny + }) + tests := []struct { + name string + authz auth.Authorizer + wantSecret bool + }{ + {"default denies", auth.AllowAnonymous{AllowWrite: true}, false}, + {"authorizer allows", authz, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := &daemon{sysFS: fs, authz: tt.authz} + srv, err := newSSHServer(d, "") + if err != nil { + t.Fatal(err) + } + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + go srv.Serve(ln) //nolint:errcheck // closed by test cleanup + t.Cleanup(func() { srv.Close(); ln.Close() }) + host, port, err := net.SplitHostPort(ln.Addr().String()) + if err != nil { + t.Fatal(err) + } + cmd := exec.Command("ssh", "-p", port, "-i", keyPath, "-o", "IdentitiesOnly=yes", "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", "git@"+host, "objgit-webhook-settings", "acme/widgets") + out, err := cmd.CombinedOutput() + if tt.wantSecret && err != nil { + t.Fatalf("ssh query: %v: %s", err, out) + } + if !tt.wantSecret && err == nil { + t.Fatalf("ssh query unexpectedly succeeded: %s", out) + } + if got := bytes.Contains(out, []byte("test-secret")); got != tt.wantSecret { + t.Errorf("secret present = %v, want %v: %s", got, tt.wantSecret, out) + } + }) + } +} + +func TestHTTPWebhookSettingsAuthorization(t *testing.T) { + fs := memfs.New() + writeWebhookSettings(t, fs, "acme/widgets", "https://example.com/events") + settingsPath, err := webhook.SettingsPath("acme/widgets") + if err != nil { + t.Fatal(err) + } + if settingsPath != ".objgit/webhooks/acme/widgets/settings.json" { + t.Fatalf("settings path = %q", settingsPath) + } + allowed := authorizeFunc(func(_ context.Context, req auth.Request) auth.Decision { + if req.Operation == auth.Admin && req.Transport == "http" { + if cred, ok := req.Cred.(auth.BasicAuth); ok && cred.Username == "operator" && cred.Password == "admin-password" { + return auth.Allow + } + } + return auth.Unauthenticated + }) + tests := []struct { + name string + authz auth.Authorizer + user, password string + path string + wantStatus int + wantSecret bool + }{ + {"anonymous default", auth.AllowAnonymous{AllowWrite: true}, "", "", "/_objgit/webhooks/acme/widgets/settings", http.StatusForbidden, false}, + {"basic default", auth.AllowAnonymous{AllowWrite: true}, "operator", "admin-password", "/_objgit/webhooks/acme/widgets/settings", http.StatusForbidden, false}, + {"wrong credential", allowed, "operator", "wrong", "/_objgit/webhooks/acme/widgets/settings", http.StatusUnauthorized, false}, + {"admin credential", allowed, "operator", "admin-password", "/_objgit/webhooks/acme/widgets/settings", http.StatusOK, true}, + {"missing settings", allowed, "operator", "admin-password", "/_objgit/webhooks/acme/missing/settings", http.StatusNotFound, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := &daemon{sysFS: fs, authz: tt.authz} + req := httptest.NewRequest(http.MethodGet, tt.path, nil) + if tt.user != "" { + req.SetBasicAuth(tt.user, tt.password) + } + w := httptest.NewRecorder() + d.httpHandler().ServeHTTP(w, req) + if w.Code != tt.wantStatus { + t.Fatalf("status = %d, want %d", w.Code, tt.wantStatus) + } + if w.Header().Get("Cache-Control") != "no-store" { + t.Error("response must not be cached") + } + if got := bytes.Contains(w.Body.Bytes(), []byte("test-secret")); got != tt.wantSecret { + t.Errorf("secret present = %v, want %v", got, tt.wantSecret) + } + }) + } +} diff --git a/cmd/objgitd/webhooks.go b/cmd/objgitd/webhooks.go index b46c2e8..c2f7643 100644 --- a/cmd/objgitd/webhooks.go +++ b/cmd/objgitd/webhooks.go @@ -10,14 +10,15 @@ import ( "github.com/go-git/go-git/v6/storage" "github.com/tigrisdata/objgit/internal/metrics" "github.com/tigrisdata/objgit/internal/pushevents" + "github.com/tigrisdata/objgit/internal/webhook" ) // emitPushWebhooks builds one event for each ref changed by this receive-pack. // Delivery runs after the ref update and cannot reject or undo it. A detached, // bounded context lets an event finish even if the Git client disconnects // after the server has committed the refs. -func (d *daemon) emitPushWebhooks(ctx context.Context, st storage.Storer, repo string, updates []refUpdate, acceptedAt time.Time) { - if d.webhooks == nil || !d.webhooks.Enabled(repo) || len(updates) == 0 { +func (d *daemon) emitPushWebhooks(ctx context.Context, st storage.Storer, repo string, webhooks *webhook.Client, updates []refUpdate, acceptedAt time.Time) { + if webhooks == nil || !webhooks.Enabled(repo) || len(updates) == 0 { return } @@ -42,7 +43,7 @@ func (d *daemon) emitPushWebhooks(ctx context.Context, st storage.Storer, repo s New: u.New, }, pushID, eventID, acceptedAt) if err == nil { - err = d.webhooks.Deliver(eventCtx, repo, event) + err = webhooks.Deliver(eventCtx, repo, event) } cancel() diff --git a/cmd/objgitd/webhooks_test.go b/cmd/objgitd/webhooks_test.go index a249752..909e538 100644 --- a/cmd/objgitd/webhooks_test.go +++ b/cmd/objgitd/webhooks_test.go @@ -1,26 +1,53 @@ package main import ( - "encoding/json" "io" "net/http" "net/http/httptest" "os" "os/exec" + "path" "path/filepath" "slices" "strings" "testing" "time" + "github.com/go-git/go-billy/v6" "github.com/go-git/go-billy/v6/memfs" pushv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/events/push/v1" + settingsv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/webhooks/v1" "github.com/tigrisdata/objgit/internal/auth" "github.com/tigrisdata/objgit/internal/repofs" "github.com/tigrisdata/objgit/internal/webhook" "google.golang.org/protobuf/encoding/protojson" ) +func writeWebhookSettings(t *testing.T, fs billy.Filesystem, repo, endpoint string) { + t.Helper() + settingsPath, err := webhook.SettingsPath(repo) + if err != nil { + t.Fatal(err) + } + if err := fs.MkdirAll(path.Dir(settingsPath), 0700); err != nil { + t.Fatal(err) + } + body, err := protojson.Marshal(&settingsv1.Settings{Url: endpoint, Secret: "test-secret"}) + if err != nil { + t.Fatal(err) + } + f, err := fs.Create(settingsPath) + if err != nil { + t.Fatal(err) + } + if _, err := f.Write(body); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } +} + func TestSmartHTTPPushWebhook(t *testing.T) { if _, err := exec.LookPath("git"); err != nil { t.Skip("git not installed") @@ -60,25 +87,12 @@ func TestSmartHTTPPushWebhook(t *testing.T) { })) t.Cleanup(destination.Close) - config, err := json.Marshal(map[string]any{"repositories": map[string]any{ - "acme/webhook": map[string]string{"url": destination.URL, "secret": "test-secret"}, - }}) - if err != nil { - t.Fatal(err) - } - configPath := filepath.Join(t.TempDir(), "webhooks.json") - if err := os.WriteFile(configPath, config, 0600); err != nil { - t.Fatal(err) - } - client, err := webhook.Load(configPath) - if err != nil { - t.Fatal(err) - } + sysFS := memfs.New() + writeWebhookSettings(t, sysFS, "acme/webhook", destination.URL) d := &daemon{ - sysFS: memfs.New(), + sysFS: sysFS, resolver: repofs.BucketResolver{Base: newMemBase()}, authz: auth.AllowAnonymous{AllowWrite: true}, - webhooks: client, } server := httptest.NewServer(d.httpHandler()) t.Cleanup(server.Close) @@ -201,23 +215,11 @@ func TestPushWebhookAcceptanceTimePrecedesHook(t *testing.T) { })) t.Cleanup(destination.Close) - config, err := json.Marshal(map[string]any{"repositories": map[string]any{ - "acme/timed": map[string]string{"url": destination.URL, "secret": "test-secret"}, - }}) - if err != nil { - t.Fatal(err) - } - configPath := filepath.Join(t.TempDir(), "webhooks.json") - if err := os.WriteFile(configPath, config, 0600); err != nil { - t.Fatal(err) - } - client, err := webhook.Load(configPath) - if err != nil { - t.Fatal(err) - } + sysFS := memfs.New() + writeWebhookSettings(t, sysFS, "acme/timed", destination.URL) server := httptest.NewServer((&daemon{ - sysFS: memfs.New(), resolver: repofs.BucketResolver{Base: newMemBase()}, - authz: auth.AllowAnonymous{AllowWrite: true}, webhooks: client, + sysFS: sysFS, resolver: repofs.BucketResolver{Base: newMemBase()}, + authz: auth.AllowAnonymous{AllowWrite: true}, allowHooks: true, hookTimeout: 5 * time.Second, }).httpHandler()) t.Cleanup(server.Close) diff --git a/docs/architecture/auth.md b/docs/architecture/auth.md index dcce0e1..14d478e 100644 --- a/docs/architecture/auth.md +++ b/docs/architecture/auth.md @@ -18,23 +18,23 @@ Authorizer.Authorize(ctx, auth.Request) auth.Decision `auth.Request` carries four fields: -| Field | Meaning | -| ------------ | -------------------------------- | -| `Repo` | The repository path. | -| `Operation` | `Read` or `Write`. | -| `Credential` | What the client presented. | -| `Transport` | Which transport asked. | +| Field | Meaning | +| ------------ | ---------------------------- | +| `Repo` | The repository path. | +| `Operation` | `Read`, `Write`, or `Admin`. | +| `Credential` | What the client presented. | +| `Transport` | Which transport asked. | ## Credential `Credential` is a sum type. An unexported method seals it, so no other package can add a case. -| Case | Source | -| -------------------------------- | ----------------------------------------------- | -| `Anonymous{}` | git://, or HTTP and SSH with nothing presented. | -| `PublicKey{Key}` | SSH. | -| `BasicAuth{Username, Password}` | HTTP. | +| Case | Source | +| ------------------------------- | ----------------------------------------------- | +| `Anonymous{}` | git://, or HTTP and SSH with nothing presented. | +| `PublicKey{Key}` | SSH. | +| `BasicAuth{Username, Password}` | HTTP. | CAUTION: `BasicAuth` is unvalidated. The `Authorizer` owns the user store, so the `Authorizer` must check the password itself. @@ -61,7 +61,9 @@ and stderr plus a non-zero exit status for SSH. ## The one implementation today `auth.AllowAnonymous{AllowWrite}` allows read for everyone. It allows write -only when `AllowWrite` is set. +only when `AllowWrite` is set. It always denies `Admin`. The webhook-settings +queries over HTTP and SSH request `Admin`; an ACL-backed authorizer must +explicitly grant it before they reveal the raw signing secret. `main.go` wires it as `AllowAnonymous{AllowWrite: *allowPush}`. The `-allow-push` flag is therefore configuration for this default, and not a diff --git a/docs/architecture/s3fs.md b/docs/architecture/s3fs.md index 73d119f..85ea208 100644 --- a/docs/architecture/s3fs.md +++ b/docs/architecture/s3fs.md @@ -2,11 +2,11 @@ This package is vendored from the s3fs of Austin Poor. It is adapted to **billy v6** and to the Tigris `storage-go` client. It treats an S3 bucket as -a filesystem, so go-git's `filesystem.NewStorage` can store loose objects and -packs against it. +a filesystem. -In `cmd/objgitd` this package now backs only `sysFS`, which is daemon-level -state and holds the SSH host key. Repository git storage is +In `cmd/objgitd` this package backs `sysFS`, which is daemon-level state and +holds the SSH host key and per-repository webhook settings at +`.objgit/webhooks///settings.json`. Repository git storage is [`internal/storage/tigris`](tigris-storer.md), and not this package. ## The temp filesystem (`tempfs.go`) diff --git a/docs/plans/webhook-push-events.md b/docs/plans/webhook-push-events.md index 99f2808..a75357d 100644 --- a/docs/plans/webhook-push-events.md +++ b/docs/plans/webhook-push-events.md @@ -34,6 +34,10 @@ explicit `false` and empty arrays as in the example. 3. Resolve operator-owned webhook destinations and secrets. Serialize the event with ProtoJSON, sign the exact request bytes, and send with bounded timeout and retries. Keep a delivery ID stable across attempts. + Each repository now reads its settings from + `.objgit/webhooks///settings.json` in daemon bucket state. + Raw settings queries on HTTP and SSH use the shared `Authorizer` seam with + the `Admin` operation. The permissive default denies this operation. 4. Large events are currently sent whole. Event construction has a 30-second deadline per ref. A push too large to build within that limit logs a delivery failure; it does not silently truncate the commit or path lists. A future diff --git a/docs/usage/webhooks.md b/docs/usage/webhooks.md index 4674a2a..b92829f 100644 --- a/docs/usage/webhooks.md +++ b/docs/usage/webhooks.md @@ -1,25 +1,52 @@ # Push webhooks -Set `-webhook-config` to a JSON file owned by the daemon operator. The flag -also accepts `WEBHOOK_CONFIG` through the usual environment flag mapping. -An empty value disables delivery. The daemon reads the file at startup, and -rejects invalid entries before it starts serving Git requests. +Store one `settings.json` object for each repository in the daemon's Tigris +bucket. For `acme/widgets`, the key is +`.objgit/webhooks/acme/widgets/settings.json`. This is daemon state, alongside +`.objgit/ssh_host_ed25519_key`. It is not a file in the Git repository's tree. +The object contains one ProtoJSON settings message: ```json { - "repositories": { - "acme/widgets": { - "url": "https://events.example.com/objgit", - "secret": "replace-with-a-long-random-secret" - } - } + "url": "https://events.example.com/objgit", + "secret": "replace-with-a-long-random-secret" } ``` -Each repository has one destination. The repository key is its canonical -`org/name` path. Configure HTTPS endpoints; HTTP is accepted only for loopback -addresses. Store the config with restricted permissions because it contains -signing secrets. Restart the daemon after changing the file. +Each repository has one destination. Configure HTTPS endpoints; HTTP is +accepted only for loopback addresses. Generate a distinct secret for each +repository with `openssl rand -hex 32`. Restrict bucket access to these +objects because they contain signing secrets. The daemon reads settings for +each push, so the next push uses an updated object without a restart. A +missing object disables webhooks for that repository; invalid settings are +logged and the push continues without webhook delivery. + +## Query settings + +The Git HTTP and SSH listeners expose admin-only queries for the raw settings, +including the signing secret: + +```sh +curl --user 'operator:' \ + https://git.example.com/_objgit/webhooks/acme/widgets/settings +ssh git@git.example.com objgit-webhook-settings acme/widgets +``` + +The HTTP response uses ProtoJSON and has `Cache-Control: no-store`. The SSH +command prints the same JSON. The HTTP endpoint returns 404 when settings are +absent; the SSH command exits with an error. Use HTTPS for the HTTP query +because it carries a credential and a raw secret. + +Both queries ask the configured `internal/auth.Authorizer` for the `Admin` +operation on that repository. The default `AllowAnonymous` authorizer denies +`Admin`, even when `-allow-push` is set. Deploy an authorizer that checks your +admin ACL and grants `Admin` only to eligible HTTP or SSH credentials before +using these queries. Ordinary repository read or write permission does not +grant access to the secret. + +The settings schema is +[settings.proto](../../proto/tigrisdata/objgit/webhooks/v1/settings.proto), +with a [raw JSON example](../../proto/tigrisdata/objgit/webhooks/v1/settings.example.json). ## Event body @@ -51,10 +78,10 @@ for invalid UTF-8 bytes so the JSON body remains valid. Each request is an HTTP `POST` with `Content-Type: application/json` and these headers: -| Header | Value | -| ------ | ----- | -| `X-Objgit-Event` | `push` | -| `X-Objgit-Delivery` | The event's `eventId`, unchanged on retries. | +| Header | Value | +| ------------------------ | --------------------------------------------------------------------------------------------------------------------- | +| `X-Objgit-Event` | `push` | +| `X-Objgit-Delivery` | The event's `eventId`, unchanged on retries. | | `X-Objgit-Signature-256` | `sha256=` followed by the lowercase hex HMAC-SHA256 of the **exact request body bytes**, using the configured secret. | Verify the HMAC with a constant-time comparison before processing the body. diff --git a/gen/tigrisdata/objgit/webhooks/v1/settings.pb.go b/gen/tigrisdata/objgit/webhooks/v1/settings.pb.go new file mode 100644 index 0000000..f125f97 --- /dev/null +++ b/gen/tigrisdata/objgit/webhooks/v1/settings.pb.go @@ -0,0 +1,138 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// versions: +// protoc-gen-go v1.36.8 +// protoc (unknown) +// source: tigrisdata/objgit/webhooks/v1/settings.proto + +package webhooksv1 + +import ( + reflect "reflect" + sync "sync" + unsafe "unsafe" + + protoreflect "google.golang.org/protobuf/reflect/protoreflect" + protoimpl "google.golang.org/protobuf/runtime/protoimpl" +) + +const ( + // Verify that this generated code is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(20 - protoimpl.MinVersion) + // Verify that runtime/protoimpl is sufficiently up-to-date. + _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) +) + +// Settings configures delivery for one repository. Its repository identity +// comes from the bucket key .objgit/webhooks///settings.json. +type Settings struct { + state protoimpl.MessageState `protogen:"open.v1"` + // HTTPS destination URL. HTTP is accepted only for loopback destinations. + Url string `protobuf:"bytes,1,opt,name=url,proto3" json:"url,omitempty"` + // Shared secret used as the HMAC-SHA256 key for every delivery to this URL. + // Generate at least 32 random bytes, for example with openssl rand -hex 32. + Secret string `protobuf:"bytes,2,opt,name=secret,proto3" json:"secret,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *Settings) Reset() { + *x = Settings{} + mi := &file_tigrisdata_objgit_webhooks_v1_settings_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *Settings) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*Settings) ProtoMessage() {} + +func (x *Settings) ProtoReflect() protoreflect.Message { + mi := &file_tigrisdata_objgit_webhooks_v1_settings_proto_msgTypes[0] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use Settings.ProtoReflect.Descriptor instead. +func (*Settings) Descriptor() ([]byte, []int) { + return file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDescGZIP(), []int{0} +} + +func (x *Settings) GetUrl() string { + if x != nil { + return x.Url + } + return "" +} + +func (x *Settings) GetSecret() string { + if x != nil { + return x.Secret + } + return "" +} + +var File_tigrisdata_objgit_webhooks_v1_settings_proto protoreflect.FileDescriptor + +const file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDesc = "" + + "\n" + + ",tigrisdata/objgit/webhooks/v1/settings.proto\x12\x1dtigrisdata.objgit.webhooks.v1\"4\n" + + "\bSettings\x12\x10\n" + + "\x03url\x18\x01 \x01(\tR\x03url\x12\x16\n" + + "\x06secret\x18\x02 \x01(\tR\x06secretB\x94\x02\n" + + "!com.tigrisdata.objgit.webhooks.v1B\rSettingsProtoP\x01ZIgithub.com/tigrisdata/objgit/gen/tigrisdata/objgit/webhooks/v1;webhooksv1\xa2\x02\x03TOW\xaa\x02\x1dTigrisdata.Objgit.Webhooks.V1\xca\x02\x1dTigrisdata\\Objgit\\Webhooks\\V1\xe2\x02)Tigrisdata\\Objgit\\Webhooks\\V1\\GPBMetadata\xea\x02 Tigrisdata::Objgit::Webhooks::V1b\x06proto3" + +var ( + file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDescOnce sync.Once + file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDescData []byte +) + +func file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDescGZIP() []byte { + file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDescOnce.Do(func() { + file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDescData = protoimpl.X.CompressGZIP(unsafe.Slice(unsafe.StringData(file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDesc), len(file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDesc))) + }) + return file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDescData +} + +var file_tigrisdata_objgit_webhooks_v1_settings_proto_msgTypes = make([]protoimpl.MessageInfo, 1) +var file_tigrisdata_objgit_webhooks_v1_settings_proto_goTypes = []any{ + (*Settings)(nil), // 0: tigrisdata.objgit.webhooks.v1.Settings +} +var file_tigrisdata_objgit_webhooks_v1_settings_proto_depIdxs = []int32{ + 0, // [0:0] is the sub-list for method output_type + 0, // [0:0] is the sub-list for method input_type + 0, // [0:0] is the sub-list for extension type_name + 0, // [0:0] is the sub-list for extension extendee + 0, // [0:0] is the sub-list for field type_name +} + +func init() { file_tigrisdata_objgit_webhooks_v1_settings_proto_init() } +func file_tigrisdata_objgit_webhooks_v1_settings_proto_init() { + if File_tigrisdata_objgit_webhooks_v1_settings_proto != nil { + return + } + type x struct{} + out := protoimpl.TypeBuilder{ + File: protoimpl.DescBuilder{ + GoPackagePath: reflect.TypeOf(x{}).PkgPath(), + RawDescriptor: unsafe.Slice(unsafe.StringData(file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDesc), len(file_tigrisdata_objgit_webhooks_v1_settings_proto_rawDesc)), + NumEnums: 0, + NumMessages: 1, + NumExtensions: 0, + NumServices: 0, + }, + GoTypes: file_tigrisdata_objgit_webhooks_v1_settings_proto_goTypes, + DependencyIndexes: file_tigrisdata_objgit_webhooks_v1_settings_proto_depIdxs, + MessageInfos: file_tigrisdata_objgit_webhooks_v1_settings_proto_msgTypes, + }.Build() + File_tigrisdata_objgit_webhooks_v1_settings_proto = out.File + file_tigrisdata_objgit_webhooks_v1_settings_proto_goTypes = nil + file_tigrisdata_objgit_webhooks_v1_settings_proto_depIdxs = nil +} diff --git a/internal/auth/auth.go b/internal/auth/auth.go index dc23abf..a3afe88 100644 --- a/internal/auth/auth.go +++ b/internal/auth/auth.go @@ -6,13 +6,14 @@ import ( gossh "golang.org/x/crypto/ssh" ) -// Operation is the access a request needs. Transports map the git service: -// upload-pack/upload-archive → Read, receive-pack → Write. +// Operation is the access a request needs. Git upload services use Read, +// receive-pack uses Write, and the webhook settings query uses Admin. type Operation int const ( Read Operation = iota Write + Admin ) // String renders the operation for logs and metrics. @@ -22,6 +23,8 @@ func (o Operation) String() string { return "read" case Write: return "write" + case Admin: + return "admin" default: return "unknown" } @@ -91,8 +94,15 @@ type Authorizer interface { type AllowAnonymous struct{ AllowWrite bool } func (a AllowAnonymous) Authorize(_ context.Context, req Request) Decision { - if req.Operation == Write && !a.AllowWrite { + switch req.Operation { + case Read: + return Allow + case Write: + if a.AllowWrite { + return Allow + } + return Deny + default: return Deny } - return Allow } diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index cc893db..50d3852 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -16,6 +16,7 @@ func TestAllowAnonymousAuthorize(t *testing.T) { {name: "no-write/write", allowWrite: false, op: Write, want: Deny}, {name: "allow-write/read", allowWrite: true, op: Read, want: Allow}, {name: "allow-write/write", allowWrite: true, op: Write, want: Allow}, + {name: "allow-write/admin", allowWrite: true, op: Admin, want: Deny}, } for _, tt := range tests { diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 2f1acbd..0158fd3 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -230,10 +230,7 @@ func ObserveAuth(transport string, op auth.Operation, d auth.Decision, start tim } func operationLabel(op auth.Operation) string { - if op == auth.Write { - return "write" - } - return "read" + return op.String() } func decisionLabel(d auth.Decision) string { diff --git a/internal/webhook/webhook.go b/internal/webhook/webhook.go index 7c6428f..7164b39 100644 --- a/internal/webhook/webhook.go +++ b/internal/webhook/webhook.go @@ -1,4 +1,5 @@ -// Package webhook delivers signed push events to operator-configured HTTP endpoints. +// Package webhook delivers signed push events using per-repository settings +// stored alongside daemon state in the bucket. package webhook import ( @@ -7,7 +8,6 @@ import ( "crypto/hmac" "crypto/sha256" "encoding/hex" - "encoding/json" "errors" "fmt" "io" @@ -16,10 +16,13 @@ import ( "net/netip" "net/url" "os" + "path" "strings" "time" + "github.com/go-git/go-billy/v6" pushv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/events/push/v1" + settingsv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/webhooks/v1" "github.com/tigrisdata/objgit/internal/repofs" "google.golang.org/protobuf/encoding/protojson" ) @@ -43,77 +46,82 @@ type destination struct { client *http.Client } -type config struct { - Repositories map[string]repositoryConfig `json:"repositories"` -} - -type repositoryConfig struct { - URL string `json:"url"` - Secret string `json:"secret"` +// SettingsPath returns the daemon-state key for a canonical repository path. +// The repository components must not escape the webhooks directory. +func SettingsPath(repo string) (string, error) { + ref, err := repofs.Parse(repo) + if err != nil || ref.Path() != repo || ref.OrgID == "." || ref.OrgID == ".." || ref.Name == "." || ref.Name == ".." { + return "", fmt.Errorf("invalid repository %q", repo) + } + return path.Join(".objgit", "webhooks", ref.OrgID, ref.Name, "settings.json"), nil } -// Load reads a JSON configuration file with a repositories object mapping -// canonical "org/name" paths to URL and secret pairs. An empty path disables -// webhook delivery. -func Load(path string) (*Client, error) { +// Load reads one repository's ProtoJSON settings from the daemon's bucket +// filesystem. A missing file disables delivery for that repository. Callers +// load once per push so the destination and secret stay fixed for its events. +func Load(fs billy.Filesystem, repo string) (*Client, error) { c := &Client{repositories: make(map[string]destination)} - if path == "" { - return c, nil + settings, err := ReadSettings(fs, repo) + if err != nil || settings == nil { + return c, err + } + settingsPath, _ := SettingsPath(repo) + if settings.GetSecret() == "" { + return nil, fmt.Errorf("webhook settings %s have no secret", settingsPath) + } + u, loopback, err := validateURL(settings.GetUrl()) + if err != nil { + return nil, fmt.Errorf("webhook settings %s: %w", settingsPath, err) } + transport := http.DefaultTransport.(*http.Transport).Clone() + transport.Proxy = nil + transport.DialContext = safeDialContext(u.Hostname(), loopback) + transport.MaxIdleConnsPerHost = 2 + c.repositories[repo] = destination{ + url: u.String(), + secret: []byte(settings.GetSecret()), + client: &http.Client{ + Timeout: attemptTimeout, + Transport: transport, + CheckRedirect: func(*http.Request, []*http.Request) error { + return http.ErrUseLastResponse + }, + }, + } + return c, nil +} - f, err := os.Open(path) +// ReadSettings returns the raw settings for an admin query. A missing file +// returns nil. Callers must authorize before calling it or serializing it. +func ReadSettings(fs billy.Filesystem, repo string) (*settingsv1.Settings, error) { + settingsPath, err := SettingsPath(repo) + if err != nil { + return nil, err + } + if fs == nil { + return nil, nil + } + f, err := fs.Open(settingsPath) if err != nil { - return nil, fmt.Errorf("open webhook configuration: %w", err) + if errors.Is(err, os.ErrNotExist) { + return nil, nil + } + return nil, fmt.Errorf("open webhook settings %s: %w", settingsPath, err) } defer f.Close() data, err := io.ReadAll(io.LimitReader(f, maxConfigBytes+1)) if err != nil { - return nil, fmt.Errorf("read webhook configuration: %w", err) + return nil, fmt.Errorf("read webhook settings %s: %w", settingsPath, err) } if len(data) > maxConfigBytes { - return nil, errors.New("webhook configuration exceeds 1 MiB") - } - dec := json.NewDecoder(bytes.NewReader(data)) - dec.DisallowUnknownFields() - var cfg config - if err := dec.Decode(&cfg); err != nil { - return nil, fmt.Errorf("decode webhook configuration: %w", err) + return nil, fmt.Errorf("webhook settings %s exceed 1 MiB", settingsPath) } - var trailing any - if err := dec.Decode(&trailing); err != io.EOF { - return nil, errors.New("webhook configuration contains trailing data") - } - - for repo, entry := range cfg.Repositories { - ref, err := repofs.Parse(repo) - if err != nil || ref.Path() != repo { - return nil, fmt.Errorf("webhook configuration has invalid repository %q", repo) - } - if entry.Secret == "" { - return nil, fmt.Errorf("webhook configuration for %q has no secret", repo) - } - u, loopback, err := validateURL(entry.URL) - if err != nil { - return nil, fmt.Errorf("webhook configuration for %q: %w", repo, err) - } - transport := http.DefaultTransport.(*http.Transport).Clone() - transport.Proxy = nil - transport.DialContext = safeDialContext(u.Hostname(), loopback) - transport.MaxIdleConnsPerHost = 2 - c.repositories[repo] = destination{ - url: u.String(), - secret: []byte(entry.Secret), - client: &http.Client{ - Timeout: attemptTimeout, - Transport: transport, - CheckRedirect: func(*http.Request, []*http.Request) error { - return http.ErrUseLastResponse - }, - }, - } + var settings settingsv1.Settings + if err := protojson.Unmarshal(data, &settings); err != nil { + return nil, fmt.Errorf("decode webhook settings %s: %w", settingsPath, err) } - return c, nil + return &settings, nil } // Enabled reports whether repo has a configured webhook destination. diff --git a/internal/webhook/webhook_test.go b/internal/webhook/webhook_test.go index d7f0cb1..f8eb666 100644 --- a/internal/webhook/webhook_test.go +++ b/internal/webhook/webhook_test.go @@ -11,32 +11,44 @@ import ( "net/http" "net/http/httptest" "net/netip" - "os" - "path/filepath" "strings" "sync" "sync/atomic" "testing" + "github.com/go-git/go-billy/v6" + "github.com/go-git/go-billy/v6/memfs" pushv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/events/push/v1" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" ) -func writeConfig(t *testing.T, content string) string { +func writeConfig(t *testing.T, content string) billy.Filesystem { t.Helper() - path := filepath.Join(t.TempDir(), "webhooks.json") - if err := os.WriteFile(path, []byte(content), 0600); err != nil { + fs := memfs.New() + settingsPath, err := SettingsPath("acme/widgets") + if err != nil { + t.Fatal(err) + } + if err := fs.MkdirAll(".objgit/webhooks/acme/widgets", 0700); err != nil { + t.Fatal(err) + } + f, err := fs.Create(settingsPath) + if err != nil { t.Fatal(err) } - return path + if _, err := io.WriteString(f, content); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + return fs } -func configFor(t *testing.T, endpoint string) string { +func configFor(t *testing.T, endpoint string) billy.Filesystem { t.Helper() - data, err := json.Marshal(config{Repositories: map[string]repositoryConfig{ - "acme/widgets": {URL: endpoint, Secret: "test-secret"}, - }}) + data, err := json.Marshal(map[string]string{"url": endpoint, "secret": "test-secret"}) if err != nil { t.Fatal(err) } @@ -46,24 +58,29 @@ func configFor(t *testing.T, endpoint string) string { func TestLoad(t *testing.T) { tests := []struct { name string + repo string config string wantEnabled bool wantError string }{ - {name: "valid loopback", config: `{"repositories":{"acme/widgets":{"url":"http://127.0.0.1:1234/hook","secret":"key"}}}`, wantEnabled: true}, - {name: "valid public HTTPS", config: `{"repositories":{"acme/widgets":{"url":"https://example.com/hook","secret":"key"}}}`, wantEnabled: true}, - {name: "bad repository", config: `{"repositories":{"acme/widgets.git":{"url":"https://example.com/hook","secret":"key"}}}`, wantError: "invalid repository"}, - {name: "missing secret", config: `{"repositories":{"acme/widgets":{"url":"https://example.com/hook"}}}`, wantError: "no secret"}, - {name: "plain HTTP", config: `{"repositories":{"acme/widgets":{"url":"http://example.com/hook","secret":"key"}}}`, wantError: "must use HTTPS"}, - {name: "metadata IP", config: `{"repositories":{"acme/widgets":{"url":"https://169.254.169.254/latest","secret":"key"}}}`, wantError: "nonpublic IP"}, - {name: "private IP", config: `{"repositories":{"acme/widgets":{"url":"https://10.0.0.1/hook","secret":"key"}}}`, wantError: "nonpublic IP"}, - {name: "URL user info", config: `{"repositories":{"acme/widgets":{"url":"https://user:pass@example.com/hook","secret":"key"}}}`, wantError: "no user info"}, - {name: "unknown config field", config: `{"repositories":{},"extra":true}`, wantError: "unknown field"}, - {name: "trailing data", config: `{"repositories":{}} {}`, wantError: "trailing data"}, + {name: "valid loopback", config: `{"url":"http://127.0.0.1:1234/hook","secret":"key"}`, wantEnabled: true}, + {name: "valid public HTTPS", config: `{"url":"https://example.com/hook","secret":"key"}`, wantEnabled: true}, + {name: "bad repository", repo: "../widgets", config: `{"url":"https://example.com/hook","secret":"key"}`, wantError: "invalid repository"}, + {name: "missing secret", config: `{"url":"https://example.com/hook"}`, wantError: "no secret"}, + {name: "plain HTTP", config: `{"url":"http://example.com/hook","secret":"key"}`, wantError: "must use HTTPS"}, + {name: "metadata IP", config: `{"url":"https://169.254.169.254/latest","secret":"key"}`, wantError: "nonpublic IP"}, + {name: "private IP", config: `{"url":"https://10.0.0.1/hook","secret":"key"}`, wantError: "nonpublic IP"}, + {name: "URL user info", config: `{"url":"https://user:pass@example.com/hook","secret":"key"}`, wantError: "no user info"}, + {name: "unknown config field", config: `{"extra":true}`, wantError: "unknown field"}, + {name: "trailing data", config: `{"url":"https://example.com","secret":"key"} {}`, wantError: "unexpected"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - c, err := Load(writeConfig(t, tt.config)) + repo := tt.repo + if repo == "" { + repo = "acme/widgets" + } + c, err := Load(writeConfig(t, tt.config), repo) if tt.wantError != "" { if err == nil || !strings.Contains(err.Error(), tt.wantError) { t.Fatalf("error = %v, want substring %q", err, tt.wantError) @@ -80,8 +97,8 @@ func TestLoad(t *testing.T) { } } -func TestLoadEmptyPath(t *testing.T) { - c, err := Load("") +func TestLoadMissingSettings(t *testing.T) { + c, err := Load(memfs.New(), "acme/widgets") if err != nil { t.Fatal(err) } @@ -125,7 +142,7 @@ func TestDeliver(t *testing.T) { })) defer server.Close() - c, err := Load(configFor(t, server.URL+"/hook")) + c, err := Load(configFor(t, server.URL+"/hook"), "acme/widgets") if err != nil { t.Fatal(err) } @@ -177,7 +194,7 @@ func TestDeliverNoRedirect(t *testing.T) { http.Redirect(w, r, target.URL, http.StatusTemporaryRedirect) })) defer source.Close() - c, err := Load(configFor(t, source.URL)) + c, err := Load(configFor(t, source.URL), "acme/widgets") if err != nil { t.Fatal(err) } @@ -197,7 +214,7 @@ func TestDeliverPermanentFailure(t *testing.T) { w.WriteHeader(http.StatusBadRequest) })) defer server.Close() - c, err := Load(configFor(t, server.URL)) + c, err := Load(configFor(t, server.URL), "acme/widgets") if err != nil { t.Fatal(err) } @@ -211,7 +228,7 @@ func TestDeliverPermanentFailure(t *testing.T) { } func TestDeliverBadEvent(t *testing.T) { - c, err := Load(configFor(t, "http://127.0.0.1:1234")) + c, err := Load(configFor(t, "http://127.0.0.1:1234"), "acme/widgets") if err != nil { t.Fatal(err) } @@ -260,7 +277,7 @@ func TestPublicIP(t *testing.T) { } func TestDeliverDoesNotExposeURLQuery(t *testing.T) { - c, err := Load(configFor(t, "http://127.0.0.1:1/hook?token=hidden-token")) + c, err := Load(configFor(t, "http://127.0.0.1:1/hook?token=hidden-token"), "acme/widgets") if err != nil { t.Fatal(err) } diff --git a/proto/README.md b/proto/README.md index 0c059ef..5ea14fe 100644 --- a/proto/README.md +++ b/proto/README.md @@ -17,6 +17,10 @@ buf generate ``` `buf generate` writes Go types under `gen/`, following the Xe/x Buf layout. +Per-repository webhook settings are defined in +[`tigrisdata/objgit/webhooks/v1/settings.proto`](tigrisdata/objgit/webhooks/v1/settings.proto), +with an adjacent raw +[`settings.example.json`](tigrisdata/objgit/webhooks/v1/settings.example.json). The schema does not yet carry gnostic OpenAPI annotations. Its versioned package, comments, and example are the starting point for adding those annotations and generated docs later. diff --git a/proto/tigrisdata/objgit/webhooks/v1/settings.example.json b/proto/tigrisdata/objgit/webhooks/v1/settings.example.json new file mode 100644 index 0000000..503c7fd --- /dev/null +++ b/proto/tigrisdata/objgit/webhooks/v1/settings.example.json @@ -0,0 +1,4 @@ +{ + "url": "https://events.example.com/objgit", + "secret": "replace-with-a-unique-random-secret" +} diff --git a/proto/tigrisdata/objgit/webhooks/v1/settings.proto b/proto/tigrisdata/objgit/webhooks/v1/settings.proto new file mode 100644 index 0000000..7609f48 --- /dev/null +++ b/proto/tigrisdata/objgit/webhooks/v1/settings.proto @@ -0,0 +1,14 @@ +syntax = "proto3"; + +package tigrisdata.objgit.webhooks.v1; + +// Settings configures delivery for one repository. Its repository identity +// comes from the bucket key .objgit/webhooks///settings.json. +message Settings { + // HTTPS destination URL. HTTP is accepted only for loopback destinations. + string url = 1; + + // Shared secret used as the HMAC-SHA256 key for every delivery to this URL. + // Generate at least 32 random bytes, for example with openssl rand -hex 32. + string secret = 2; +} From 00e67414dfc383328daf09da49b32f11b7e5943e Mon Sep 17 00:00:00 2001 From: Xe Iaso Date: Wed, 23 Sep 2026 16:18:28 -0400 Subject: [PATCH 3/3] refactor(objgitd): remove unused ref snapshot helpers Receive-pack now reports the ref updates it applied directly, so the before/after snapshot diff is dead code. Staticcheck failed CI on the unused snapshotRefs (U1000); diffRefs survived only through its test. Assisted-by: Claude Opus 5.5 via Claude Code Signed-off-by: Xe Iaso --- cmd/objgitd/hooks.go | 45 +------------------------------- cmd/objgitd/hooks_test.go | 54 --------------------------------------- 2 files changed, 1 insertion(+), 98 deletions(-) diff --git a/cmd/objgitd/hooks.go b/cmd/objgitd/hooks.go index cef5a76..3245630 100644 --- a/cmd/objgitd/hooks.go +++ b/cmd/objgitd/hooks.go @@ -29,7 +29,7 @@ import ( "tangled.org/xeiaso.net/kefka/command/registry/coreutils" ) -// refUpdate records a single branch ref change observed across a receive-pack. +// refUpdate records a single branch ref change applied by a receive-pack. // A zero Old means the branch was created; a zero New means it was deleted. type refUpdate struct { Name plumbing.ReferenceName @@ -37,49 +37,6 @@ type refUpdate struct { New plumbing.Hash } -// snapshotRefs returns the current hash of every branch ref in st. go-git's -// transport.ReceivePack does not report which refs it changed, so we diff a -// snapshot taken before the push against one taken after. -func snapshotRefs(st storage.Storer) (map[plumbing.ReferenceName]plumbing.Hash, error) { - it, err := st.IterReferences() - if err != nil { - return nil, err - } - defer it.Close() - - out := map[plumbing.ReferenceName]plumbing.Hash{} - err = it.ForEach(func(r *plumbing.Reference) error { - if r.Type() == plumbing.HashReference && r.Name().IsBranch() { - out[r.Name()] = r.Hash() - } - return nil - }) - if err != nil { - return nil, err - } - return out, nil -} - -// diffRefs computes the branch ref changes between two snapshots. -func diffRefs(before, after map[plumbing.ReferenceName]plumbing.Hash) []refUpdate { - var updates []refUpdate - for name, newHash := range after { - oldHash, ok := before[name] - switch { - case !ok: - updates = append(updates, refUpdate{Name: name, Old: plumbing.ZeroHash, New: newHash}) - case oldHash != newHash: - updates = append(updates, refUpdate{Name: name, Old: oldHash, New: newHash}) - } - } - for name, oldHash := range before { - if _, ok := after[name]; !ok { - updates = append(updates, refUpdate{Name: name, Old: oldHash, New: plumbing.ZeroHash}) - } - } - return updates -} - // receivePack runs the receive-pack service and dispatches post-receive work // for the commands whose ref updates succeeded. Hook output streams to the // client before the response closes. Webhook delivery also finishes before the diff --git a/cmd/objgitd/hooks_test.go b/cmd/objgitd/hooks_test.go index e91f3d9..83f479f 100644 --- a/cmd/objgitd/hooks_test.go +++ b/cmd/objgitd/hooks_test.go @@ -16,64 +16,10 @@ import ( "time" "github.com/go-git/go-billy/v6/memfs" - "github.com/go-git/go-git/v6/plumbing" "github.com/tigrisdata/objgit/internal/auth" "github.com/tigrisdata/objgit/internal/repofs" ) -func TestDiffRefs(t *testing.T) { - main := plumbing.NewBranchReferenceName("main") - dev := plumbing.NewBranchReferenceName("dev") - h1 := plumbing.NewHash("1111111111111111111111111111111111111111") - h2 := plumbing.NewHash("2222222222222222222222222222222222222222") - - tests := []struct { - name string - before map[plumbing.ReferenceName]plumbing.Hash - after map[plumbing.ReferenceName]plumbing.Hash - want []refUpdate - }{ - { - name: "created", - before: map[plumbing.ReferenceName]plumbing.Hash{}, - after: map[plumbing.ReferenceName]plumbing.Hash{main: h1}, - want: []refUpdate{{Name: main, Old: plumbing.ZeroHash, New: h1}}, - }, - { - name: "updated", - before: map[plumbing.ReferenceName]plumbing.Hash{main: h1}, - after: map[plumbing.ReferenceName]plumbing.Hash{main: h2}, - want: []refUpdate{{Name: main, Old: h1, New: h2}}, - }, - { - name: "deleted", - before: map[plumbing.ReferenceName]plumbing.Hash{main: h1, dev: h2}, - after: map[plumbing.ReferenceName]plumbing.Hash{main: h1}, - want: []refUpdate{{Name: dev, Old: h2, New: plumbing.ZeroHash}}, - }, - { - name: "unchanged", - before: map[plumbing.ReferenceName]plumbing.Hash{main: h1}, - after: map[plumbing.ReferenceName]plumbing.Hash{main: h1}, - want: nil, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got := diffRefs(tt.before, tt.after) - if len(got) != len(tt.want) { - t.Fatalf("diffRefs = %v, want %v", got, tt.want) - } - for i, u := range got { - if u != tt.want[i] { - t.Errorf("update[%d] = %+v, want %+v", i, u, tt.want[i]) - } - } - }) - } -} - // syncBuffer is a goroutine-safe buffer for capturing slog output while the // server handles a push on another goroutine. type syncBuffer struct {