feat(webhooks): add objgit-webhook-set SSH command

cmd/objgitd/ssh.go

@@ -119,14 +119,19 @@ func newSSHServer(d *daemon, addr string) (*ssh.Server, error) {
 
 // handleSSH services one git-over-SSH exec request: parse the command, authorize,
 // resolve the repository, and hand the session to the matching go-git transport
-// command. The session is the protocol stream (reader and writer). The one
-// non-git command, sh, goes to handleShell.
+// command. The session is the protocol stream (reader and writer). The non-git
+// commands go to their own handlers: sh to handleShell, and the webhook
+// settings commands to handleSSHWebhookSettings and handleSSHWebhookSet.
 func (d *daemon) handleSSH(s ssh.Session) {
 	cmd := s.Command()
 	if len(cmd) > 0 && cmd[0] == "sh" {
 		d.handleShell(s, cmd[1:])
 		return
 	}
+	if len(cmd) > 0 && cmd[0] == "objgit-webhook-set" {
+		d.handleSSHWebhookSet(s, cmd[1:])
+		return
+	}
 	if len(cmd) < 2 {
 		fmt.Fprintln(s.Stderr(), "objgitd: this is a git SSH endpoint; interactive shells are not supported")
 		if d.allowHooks {

cmd/objgitd/webhook_settings.go

@@ -1,11 +1,15 @@
 package main
 
 import (
+	"errors"
+	"flag"
 	"fmt"
 	"log/slog"
 	"net/http"
+	"strings"
 
 	ssh "github.com/gliderlabs/ssh"
+	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"
@@ -79,6 +83,104 @@ func (d *daemon) handleSSHWebhookSettings(s ssh.Session, repo string) {
 	_, _ = s.Write(append(body, '\n'))
 }
 
+const webhookSetUsage = "usage: objgit-webhook-set org/repo [-url URL] [-rotate-secret]"
+
+// handleSSHWebhookSet changes a repository's webhook settings. Each flag sets
+// one non-secret setting; an omitted flag keeps the current value. The secret
+// is kept, or generated when there is none or -rotate-secret is set. The
+// command prints the stored settings, secret included, as ProtoJSON.
+func (d *daemon) handleSSHWebhookSet(s ssh.Session, args []string) {
+	if len(args) == 0 || strings.HasPrefix(args[0], "-") {
+		fmt.Fprintln(s.Stderr(), "objgitd: "+webhookSetUsage)
+		_ = s.Exit(1)
+		return
+	}
+	fset := flag.NewFlagSet("objgit-webhook-set", flag.ContinueOnError)
+	fset.SetOutput(s.Stderr())
+	fset.Usage = func() {
+		fmt.Fprintln(fset.Output(), webhookSetUsage)
+		fset.PrintDefaults()
+	}
+	url := fset.String("url", "", "webhook destination URL; HTTPS, or HTTP to loopback only")
+	rotate := fset.Bool("rotate-secret", false, "replace the signing secret with a new random one")
+	if err := fset.Parse(args[1:]); err != nil {
+		_ = s.Exit(1)
+		return
+	}
+	if fset.NArg() != 0 {
+		fset.Usage()
+		_ = s.Exit(1)
+		return
+	}
+
+	ref, err := repofs.Parse(args[0])
+	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
+	}
+
+	// Unreadable settings block a partial update, because the kept values are
+	// unknown. Setting both -url and -rotate-secret replaces them.
+	current, err := webhook.ReadSettings(d.sysFS, ref.Path())
+	if err != nil {
+		if *url == "" || !*rotate {
+			slog.Error("read webhook settings", "repo", ref.Path(), "err", err)
+			fmt.Fprintln(s.Stderr(), "objgitd: cannot read webhook settings; set -url and -rotate-secret to replace them")
+			_ = s.Exit(1)
+			return
+		}
+		current = nil
+	}
+	settings := &settingsv1.Settings{Url: current.GetUrl(), Secret: current.GetSecret()}
+	if *url != "" {
+		settings.Url = *url
+	}
+	if settings.GetUrl() == "" {
+		fmt.Fprintln(s.Stderr(), "objgitd: no webhook URL is set; use -url")
+		_ = s.Exit(1)
+		return
+	}
+	rotated := *rotate || settings.GetSecret() == ""
+	if rotated {
+		settings.Secret = webhook.NewSecret()
+	}
+	if err := webhook.WriteSettings(d.sysFS, ref.Path(), settings); err != nil {
+		slog.Error("write webhook settings", "repo", ref.Path(), "err", err)
+		fmt.Fprintf(s.Stderr(), "objgitd: cannot write webhook settings: %v\n", err)
+		_ = s.Exit(1)
+		return
+	}
+	slog.Info("webhook settings updated",
+		"repo", ref.Path(),
+		"remote", s.RemoteAddr().String(),
+		"secret_rotated", rotated,
+	)
+
+	body, found, err := d.webhookSettingsJSON(ref)
+	if err != nil || !found {
+		if err == nil {
+			err = errors.New("settings missing after write")
+		}
+		slog.Error("read webhook settings", "repo", ref.Path(), "err", err)
+		fmt.Fprintln(s.Stderr(), "objgitd: webhook settings were written but cannot be read back")
+		_ = 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 {

cmd/objgitd/webhook_settings_test.go

@@ -9,12 +9,17 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/go-git/go-billy/v6/memfs"
+	"github.com/go-git/go-billy/v6/util"
+	settingsv1 "github.com/tigrisdata/objgit/gen/tigrisdata/objgit/webhooks/v1"
 	"github.com/tigrisdata/objgit/internal/auth"
 	"github.com/tigrisdata/objgit/internal/webhook"
 	gossh "golang.org/x/crypto/ssh"
+	"google.golang.org/protobuf/encoding/protojson"
+	"google.golang.org/protobuf/proto"
 )
 
 type authorizeFunc func(context.Context, auth.Request) auth.Decision
@@ -52,7 +57,7 @@ func TestSSHWebhookSettingsAuthorization(t *testing.T) {
 		authz      auth.Authorizer
 		wantSecret bool
 	}{
-		{"default denies", auth.AllowAnonymous{AllowWrite: true}, false},
+		{"default allows", auth.AllowAnonymous{AllowWrite: true}, true},
 		{"authorizer allows", authz, true},
 	}
 	for _, tt := range tests {
@@ -113,8 +118,8 @@ func TestHTTPWebhookSettingsAuthorization(t *testing.T) {
 		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},
+		{"anonymous default", auth.AllowAnonymous{AllowWrite: true}, "", "", "/_objgit/webhooks/acme/widgets/settings", http.StatusOK, true},
+		{"basic default", auth.AllowAnonymous{AllowWrite: true}, "operator", "admin-password", "/_objgit/webhooks/acme/widgets/settings", http.StatusOK, true},
 		{"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},
@@ -140,3 +145,89 @@ func TestHTTPWebhookSettingsAuthorization(t *testing.T) {
 		})
 	}
 }
+
+func TestSSHWebhookSet(t *testing.T) {
+	checkSSHBinaries(t)
+	deny := authorizeFunc(func(context.Context, auth.Request) auth.Decision { return auth.Deny })
+	tests := []struct {
+		name       string
+		existing   string // "", "valid", or "corrupt"
+		authz      auth.Authorizer
+		args       []string
+		wantErr    string // stderr substring; empty means success
+		wantURL    string
+		keepSecret bool
+	}{
+		{name: "create", args: []string{"acme/widgets", "-url", "https://example.com/new"}, wantURL: "https://example.com/new"},
+		{name: "change URL keeps secret", existing: "valid", args: []string{"acme/widgets", "-url", "https://example.com/new"}, wantURL: "https://example.com/new", keepSecret: true},
+		{name: "rotate keeps URL", existing: "valid", args: []string{"acme/widgets", "-rotate-secret"}, wantURL: "https://example.com/events"},
+		{name: "replace unreadable", existing: "corrupt", args: []string{"acme/widgets", "-url", "https://example.com/new", "-rotate-secret"}, wantURL: "https://example.com/new"},
+		{name: "partial update of unreadable", existing: "corrupt", args: []string{"acme/widgets", "-url", "https://example.com/new"}, wantErr: "cannot read webhook settings"},
+		{name: "no URL", args: []string{"acme/widgets"}, wantErr: "no webhook URL"},
+		{name: "plain HTTP", existing: "valid", args: []string{"acme/widgets", "-url", "http://example.com/new"}, wantErr: "must use HTTPS"},
+		{name: "unknown flag", existing: "valid", args: []string{"acme/widgets", "-secret", "x"}, wantErr: "flag provided but not defined"},
+		{name: "extra argument", existing: "valid", args: []string{"acme/widgets", "-url", "https://example.com/new", "extra"}, wantErr: "usage:"},
+		{name: "no repository", args: []string{"-url", "https://example.com/new"}, wantErr: "usage:"},
+		{name: "invalid repository", args: []string{"../widgets", "-url", "https://example.com/new"}, wantErr: "invalid repository"},
+		{name: "denied", existing: "valid", authz: deny, args: []string{"acme/widgets", "-url", "https://example.com/new"}, wantErr: "admin access denied"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			fs := memfs.New()
+			switch tt.existing {
+			case "valid":
+				writeWebhookSettings(t, fs, "acme/widgets", "https://example.com/events")
+			case "corrupt":
+				settingsPath, _ := webhook.SettingsPath("acme/widgets")
+				if err := util.WriteFile(fs, settingsPath, []byte("{not json"), 0o600); err != nil {
+					t.Fatal(err)
+				}
+			}
+			before, _ := util.ReadFile(fs, ".objgit/webhooks/acme/widgets/settings.json")
+			addr, _ := startSSHServer(t, false, false, func(d *daemon) {
+				d.sysFS = fs
+				if tt.authz != nil {
+					d.authz = tt.authz
+				}
+			})
+
+			stdout, stderr, code := runSSHCommand(t, addr, append([]string{"objgit-webhook-set"}, tt.args...)...)
+			if tt.wantErr != "" {
+				if code == 0 {
+					t.Fatalf("command succeeded, want error %q\nstdout: %s", tt.wantErr, stdout)
+				}
+				if !strings.Contains(stderr, tt.wantErr) {
+					t.Errorf("stderr = %q, want substring %q", stderr, tt.wantErr)
+				}
+				if after, _ := util.ReadFile(fs, ".objgit/webhooks/acme/widgets/settings.json"); !bytes.Equal(before, after) {
+					t.Errorf("failed command changed stored settings to %q", after)
+				}
+				return
+			}
+			if code != 0 {
+				t.Fatalf("exit %d\nstderr: %s", code, stderr)
+			}
+
+			var printed settingsv1.Settings
+			if err := protojson.Unmarshal([]byte(stdout), &printed); err != nil {
+				t.Fatalf("stdout is not settings JSON: %v\n%s", err, stdout)
+			}
+			stored, err := webhook.ReadSettings(fs, "acme/widgets")
+			if err != nil || stored == nil {
+				t.Fatalf("ReadSettings = %v, %v", stored, err)
+			}
+			if !proto.Equal(&printed, stored) {
+				t.Errorf("printed %v, stored %v", &printed, stored)
+			}
+			if stored.GetUrl() != tt.wantURL {
+				t.Errorf("url = %q, want %q", stored.GetUrl(), tt.wantURL)
+			}
+			if gotKept := stored.GetSecret() == "test-secret"; gotKept != tt.keepSecret {
+				t.Errorf("secret = %q, keepSecret = %v", stored.GetSecret(), tt.keepSecret)
+			}
+			if !tt.keepSecret && len(stored.GetSecret()) != 64 {
+				t.Errorf("new secret %q is not 32 hex-encoded bytes", stored.GetSecret())
+			}
+		})
+	}
+}

docs/architecture/auth.md

@@ -61,9 +61,11 @@ 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. 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.
+only when `AllowWrite` is set. For now, it also allows `Admin` for everyone.
+The webhook-settings commands request `Admin`: the queries over HTTP and SSH,
+and `objgit-webhook-set` over SSH. Thus, any client can read the raw signing
+secret of every repository, and change its settings. This is a
+temporary measure until an ACL-backed authorizer replaces it.
 
 `main.go` wires it as `AllowAnonymous{AllowWrite: *allowPush}`. The
 `-allow-push` flag is therefore configuration for this default, and not a

docs/usage/webhooks.md

@@ -23,8 +23,8 @@ 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:
+The Git HTTP and SSH listeners expose admin-only queries for the raw settings.
+The output includes the signing secret:
 
 ```sh
 curl --user 'operator:<admin-password>' \
@@ -38,11 +38,40 @@ 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.
+operation on that repository. For now, the default `AllowAnonymous` authorizer
+allows `Admin` for all clients. Thus, any client that can reach the daemon can
+read the signing secret. To restrict access, deploy an authorizer that grants
+`Admin` only to the credentials in your admin ACL.
+
+## Change settings
+
+To change the settings of a repository, use the `objgit-webhook-set` SSH
+command:
+
+```sh
+ssh git@git.example.com objgit-webhook-set acme/widgets -url https://events.example.com/objgit
+```
+
+The command has these flags:
+
+| Flag             | Function                                                          |
+| ---------------- | ----------------------------------------------------------------- |
+| `-url`           | Sets the destination URL. The same URL rules as above apply.      |
+| `-rotate-secret` | Replaces the signing secret with a new random secret of 32 bytes. |
+
+Put the repository first, then the flags. If you do not give a flag, the
+command keeps the current value of that setting. If the repository has no
+settings, you must give `-url`. If no secret exists, the command generates
+one.
+
+The command writes the settings object and then prints it as ProtoJSON. The
+output includes the signing secret. Give this secret to the webhook receiver.
+The next push uses the new settings.
+
+If the current settings object is not valid, the command cannot keep its
+values. To replace such an object, give both `-url` and `-rotate-secret`.
+
+The command asks the `Authorizer` for the `Admin` operation, as the queries do.
 
 The settings schema is
 [settings.proto](../../proto/tigrisdata/objgit/webhooks/v1/settings.proto),

internal/auth/auth.go

@@ -87,10 +87,13 @@ type Authorizer interface {
 	Authorize(ctx context.Context, req Request) Decision
 }
 
-// AllowAnonymous is the permissive default: read for everyone, write only when
-// AllowWrite is set. "Dangerously allow everything the server is configured to
-// allow" — never more open than the -allow-push gate. It ignores the credential
-// entirely and never returns Unauthenticated.
+// AllowAnonymous is the permissive default: read and admin for everyone, write
+// only when AllowWrite is set. It ignores the credential entirely and never
+// returns Unauthenticated.
+//
+// TODO: Admin is granted to everyone as a temporary measure. This exposes
+// every repository's webhook signing secret to any client. Replace it with an
+// ACL-backed authorizer.
 type AllowAnonymous struct{ AllowWrite bool }
 
 func (a AllowAnonymous) Authorize(_ context.Context, req Request) Decision {
@@ -102,6 +105,8 @@ func (a AllowAnonymous) Authorize(_ context.Context, req Request) Decision {
 			return Allow
 		}
 		return Deny
+	case Admin:
+		return Allow
 	default:
 		return Deny
 	}

internal/auth/auth_test.go

@@ -16,7 +16,8 @@ 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},
+		{name: "no-write/admin", allowWrite: false, op: Admin, want: Allow},
+		{name: "allow-write/admin", allowWrite: true, op: Admin, want: Allow},
 	}
 
 	for _, tt := range tests {