From d84a877cde3dc4979f87b46b89cb57989d0f72aa Mon Sep 17 00:00:00 2001 From: Eugene Blikh Date: Sun, 9 Aug 2026 00:19:36 +0300 Subject: [PATCH] internalauth: take both ends of the internal protocol from ecore MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard on /internal/repos and the header cmd/dolt-git-hook minted for it were two hand-written halves of one protocol in two packages that shared no type, no constant and no test. Both are now sr-ht-ecore/internalauth: Guard on the receiving end, AuthorizationAs on the calling one, over one Auth struct. The guard also pins the caller, which the old copy did not: core-go only asks that a token name some client and node, and on an endpoint that provisions a database for an arbitrary user that means any holder of the network key will do. The pinned pair lives in core so the mint and the pin cannot drift apart. The hook test now runs internalauth.Identify — the real receiving end — over the header the hook produced, so the two ends are checked against each other rather than against a third copy of the decode. --- cmd/dolt-git-hook/main.go | 32 +++++++------- cmd/dolt-git-hook/main_test.go | 69 ++++++++++++++++-------------- core/internal.go | 24 +++++++++++ web/handlers_internal.go | 76 ++++++---------------------------- web/handlers_internal_test.go | 27 ++++++++++-- web/router.go | 16 ++++++- 6 files changed, 129 insertions(+), 115 deletions(-) create mode 100644 core/internal.go diff --git a/cmd/dolt-git-hook/main.go b/cmd/dolt-git-hook/main.go index cd8f55bd42c55404032d02fbbbc6bfbbaaae1f9a..38008328a57ba89cc8273548235b281aadac0966 100644 --- a/cmd/dolt-git-hook/main.go +++ b/cmd/dolt-git-hook/main.go @@ -33,6 +33,10 @@ import ( "sourcecraft.dev/bigbes/sr-ht-core/config" "sourcecraft.dev/bigbes/sr-ht-core/crypto" + + "sourcecraft.dev/bigbes/sr-ht-ecore/internalauth" + + "sourcecraft.dev/bigbes/sr-ht-dolt/core" ) // defaultDelegate is the stock git.sr.ht hook this wrapper wraps. The apk still @@ -96,15 +100,6 @@ type pushContext struct { } `json:"repo"` } -// internalAuth mirrors core-go's client.InternalAuth wire shape. crypto.Encrypt -// seals it with the shared [sr.ht]network-key; dolt.sr.ht's internalAuthGuard -// decrypts and trusts it. -type internalAuth struct { - Name string `json:"name,omitempty"` - ClientID string `json:"client_id"` - NodeID string `json:"node_id"` -} - // provisionDolt asks dolt.sr.ht to create the companion database for the pushed // repo. Every failure mode is swallowed (logged to stderr at most): this is a // convenience, not part of the push contract. @@ -166,11 +161,18 @@ func createCompanion(out io.Writer, origin string, pc pushContext) { } body, _ := json.Marshal(reqBody) - authBlob, _ := json.Marshal(internalAuth{ - Name: pc.Repo.OwnerName, - ClientID: "git.sr.ht", - NodeID: "dolt-git-hook", - }) + // The other end of this is web's internalauth.Guard, and the two are now one + // package rather than a struct literal here and a decode there. The call is + // made on the pushing repo's owner's behalf; the ids are the pinned pair the + // guard accepts. A missing network key surfaces as an error instead of a nil + // dereference inside fernet, which on this side matters — it must cost a + // companion database, never the push. + authorization, err := internalauth.AuthorizationAs( + pc.Repo.OwnerName, core.InternalClientID, core.InternalNodeID) + if err != nil { + fmt.Fprintf(out, "dolt-git-hook: cannot sign the provisioning call: %v\n", err) + return + } req, err := http.NewRequest("POST", strings.TrimRight(origin, "/")+"/internal/repos", bytes.NewReader(body)) @@ -178,7 +180,7 @@ func createCompanion(out io.Writer, origin string, pc pushContext) { return } req.Header.Set("Content-Type", "application/json") - req.Header.Set("Authorization", "Internal "+string(crypto.Encrypt(authBlob))) + req.Header.Set("Authorization", authorization) client := &http.Client{Timeout: provisionTimeout} resp, err := client.Do(req) diff --git a/cmd/dolt-git-hook/main_test.go b/cmd/dolt-git-hook/main_test.go index 29c31389d99607fb2ebc5c8aa28648159960b90b..5bbfd502db63d009a6c5fdaeb5a4ca1e49586ecd 100644 --- a/cmd/dolt-git-hook/main_test.go +++ b/cmd/dolt-git-hook/main_test.go @@ -6,21 +6,35 @@ import ( "io" "net/http" "net/http/httptest" + "os" "strings" "testing" - "time" + "testing/fstest" - "sourcecraft.dev/bigbes/sr-ht-core/crypto" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sourcecraft.dev/bigbes/sr-ht-core/config" "sourcecraft.dev/bigbes/sr-ht-ecore/ecoretest" + "sourcecraft.dev/bigbes/sr-ht-ecore/internalauth" + + "sourcecraft.dev/bigbes/sr-ht-dolt/core" ) -// initTestCrypto installs sr-ht-ecore's fixed test keyset into the shared -// crypto globals, so createCompanion's crypto.Encrypt and the test server's -// DecryptWithExpiration share one. -func initTestCrypto(t *testing.T) { - t.Helper() +// TestMain installs the two pieces of core-go process state the provisioning +// call reads and neither creates: the fixed test keyset, so the token this hook +// seals is one internalauth can open, and a loaded config, whose only job here +// is to fill the internal network list — without it that list is empty, every +// address is external, and the httptest server's loopback caller is refused +// before its token is ever looked at. +func TestMain(m *testing.M) { + config.FS = fstest.MapFS{ + "config.ini": &fstest.MapFile{Data: []byte("[sr.ht]\nsite-name=srht.example\n")}, + } + config.LoadConfig() ecoretest.InitCrypto() + os.Exit(m.Run()) } func samplePush() pushContext { @@ -31,37 +45,30 @@ func samplePush() pushContext { return pc } -// decodeInternalAuth verifies an incoming "Internal " header the same way -// dolt.sr.ht's internalAuthGuard does and returns the decoded claims. -func decodeInternalAuth(t *testing.T, r *http.Request) internalAuth { +// checkInternalAuth runs the real receiving end over the header the hook minted +// — the same internalauth.Identify, against the same pinned ids, that web's +// guard on /internal/repos runs — and returns the caller it identified. +// +// It is the guard itself rather than a decode written here on purpose: this is +// the one test in the repo where both ends of the protocol are present, so it +// is where a drift between them has to fail. +func checkInternalAuth(t *testing.T, r *http.Request) internalauth.Auth { t.Helper() - parts := strings.SplitN(r.Header.Get("Authorization"), " ", 2) - if len(parts) != 2 || !strings.EqualFold(parts[0], "internal") { - t.Fatalf("missing/invalid Internal auth header: %q", r.Header.Get("Authorization")) - } - payload := crypto.DecryptWithExpiration([]byte(parts[1]), 30*time.Second) - if payload == nil { - t.Fatalf("auth token did not decrypt (wrong key or expired)") - } - var ia internalAuth - if err := json.Unmarshal(payload, &ia); err != nil { - t.Fatalf("auth payload not JSON: %v", err) - } - return ia + auth, err := internalauth.Identify(r, core.InternalClientID, core.InternalNodeID) + require.NoError(t, err, "the header the hook minted must satisfy the guard") + return auth } func TestCreateCompanionSignsAndAnnouncesOnCreate(t *testing.T) { - initTestCrypto(t) - var gotBody map[string]string srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/internal/repos" || r.Method != http.MethodPost { t.Errorf("unexpected request %s %s", r.Method, r.URL.Path) } - ia := decodeInternalAuth(t, r) - if ia.ClientID != "git.sr.ht" || ia.NodeID != "dolt-git-hook" { - t.Errorf("unexpected auth claims: %+v", ia) - } + auth := checkInternalAuth(t, r) + // The call is made on the pushing owner's behalf, so the receiving + // service can see whose push provoked it. + assert.Equal(t, "alice", auth.Name) raw, _ := io.ReadAll(r.Body) _ = json.Unmarshal(raw, &gotBody) w.WriteHeader(http.StatusCreated) @@ -85,9 +92,8 @@ func TestCreateCompanionSignsAndAnnouncesOnCreate(t *testing.T) { } func TestCreateCompanionQuietWhenExists(t *testing.T) { - initTestCrypto(t) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - decodeInternalAuth(t, r) // still must be authenticated + checkInternalAuth(t, r) // still must be authenticated w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte(`{"url":"https://dolt.example/~alice/widgets","created":false}`)) })) @@ -101,7 +107,6 @@ func TestCreateCompanionQuietWhenExists(t *testing.T) { } func TestCreateCompanionWarnsOnError(t *testing.T) { - initTestCrypto(t) srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusInternalServerError) _, _ = w.Write([]byte("boom")) diff --git a/core/internal.go b/core/internal.go new file mode 100644 index 0000000000000000000000000000000000000000..d8550fb6541e48d78a5ed062a6fd689c016d1e38 --- /dev/null +++ b/core/internal.go @@ -0,0 +1,24 @@ +package core + +// The identity dolt.sr.ht's companion-provisioning call travels under: the +// "Authorization: Internal " payload cmd/dolt-git-hook mints and the +// caller web's guard on /internal/repos pins. +// +// They are two constants in the pure domain package rather than two string +// literals in two commands because the guard now *pins* them (it refuses a +// token naming anyone else), and a pin whose two ends are spelled separately is +// a pin that can be half-changed. Renaming the hook's node id in one file would +// not fail to compile and would not fail a test — companion databases would +// simply stop being provisioned on the next push, which is the failure mode +// sr-ht-ecore/internalauth exists to make impossible. Here it is a compile-time +// fact shared by both ends. +const ( + // InternalClientID names the calling service. It is git.sr.ht's, not + // dolt.sr.ht's: the hook runs as git.sr.ht's post-update script and calls + // in on git.sr.ht's behalf. + InternalClientID = "git.sr.ht" + + // InternalNodeID names the program within that service — the hook binary, + // which is the only thing on the instance that may drive /internal/repos. + InternalNodeID = "dolt-git-hook" +) diff --git a/web/handlers_internal.go b/web/handlers_internal.go index 4be196fab7163fc9b86c452e1ed0a9deef3865f1..cba12881f365f9ce6660ba4976b5f6bb85355f40 100644 --- a/web/handlers_internal.go +++ b/web/handlers_internal.go @@ -4,13 +4,12 @@ import ( "encoding/json" "errors" "fmt" - "net" "net/http" "strings" - "time" "sourcecraft.dev/bigbes/sr-ht-core/config" - "sourcecraft.dev/bigbes/sr-ht-core/crypto" + + "sourcecraft.dev/bigbes/sr-ht-ecore/instconf" "sourcecraft.dev/bigbes/sr-ht-dolt/core" "sourcecraft.dev/bigbes/sr-ht-dolt/db" @@ -21,8 +20,8 @@ import ( // caller is git.sr.ht's post-update hook, which POSTs here whenever a git repo // is pushed so a matching Dolt DB exists at ~owner/name before the user's first // `dolt push`. It is NOT a browser route: it carries no CSRF token and no -// unified-login cookie, and is guarded by internalAuthGuard instead of the -// cookie auth the rest of web/ uses. +// unified-login cookie, and is guarded by sr-ht-ecore's internalauth (see the +// mount in router.go) instead of the cookie auth the rest of web/ uses. // internalCreateRequest is the JSON body POSTed to /internal/repos. Owner is a // SourceHut username (with or without the leading "~"); Name is the database @@ -43,49 +42,6 @@ type internalCreateResponse struct { Created bool `json:"created"` } -// internalAuthGuard authenticates a service-to-service request the same way -// core-go's auth.internalAuth does — the source IP must fall inside -// [sr.ht]internal-ipnet AND the Authorization header must be a valid, -// unexpired "Internal " minted with the shared [sr.ht]network-key. -// The network-key check is the real guard (only internal services hold it); the -// IP check is defense-in-depth against the endpoint being reachable through the -// public Traefik route. We reimplement rather than reuse auth.Middleware because -// that middleware 401s any request without a cookie/bearer and pulls in the -// full user-resolution path we do not need here — the owner comes from the body. -func internalAuthGuard(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - host, _, err := net.SplitHostPort(r.RemoteAddr) - if err != nil { - host = r.RemoteAddr - } - ip := net.ParseIP(host) - if ip == nil || !config.IsInternalIP(ip) { - http.Error(w, "internal auth: source IP not permitted", http.StatusUnauthorized) - return - } - - parts := strings.SplitN(r.Header.Get("Authorization"), " ", 2) - if len(parts) != 2 || !strings.EqualFold(parts[0], "internal") { - http.Error(w, "internal auth: Internal authorization required", http.StatusUnauthorized) - return - } - payload := crypto.DecryptWithExpiration([]byte(parts[1]), 30*time.Second) - if payload == nil { - http.Error(w, "internal auth: invalid or expired token", http.StatusForbidden) - return - } - var ia struct { - ClientID string `json:"client_id"` - NodeID string `json:"node_id"` - } - if err := json.Unmarshal(payload, &ia); err != nil || ia.ClientID == "" || ia.NodeID == "" { - http.Error(w, "internal auth: malformed token", http.StatusForbidden) - return - } - next.ServeHTTP(w, r) - }) -} - // handleInternalCreate provisions a Dolt database for owner/name. It mirrors // handleCreate's insert-row-then-init-store ordering (so metadata and disk // never diverge) but is idempotent: a repeated call for an existing companion @@ -186,7 +142,14 @@ func (a *app) handleInternalCreate(w http.ResponseWriter, r *http.Request) { authorName = caller.Username } if authorEmail == "" { - authorEmail = caller.Username + "@" + hostOf(a.chrome.SelfOrigin()) + // The authority and not the bare host: this is the domain half of an + // address that identifies *this* deployment, and two instances behind + // one hostname on two ports are two of them. An origin that names no + // host at all yields "", and then there is no domain to synthesize — + // leave the address empty rather than emit "alice@". + if authority := instconf.OriginAuthority(a.chrome.SelfOrigin()); authority != "" { + authorEmail = caller.Username + "@" + authority + } } if err := a.cfg.Stores.InitStore(ctx, diskPath, authorName, authorEmail); err != nil { // InitStore self-cleans its directory; undo the metadata row too so a @@ -206,21 +169,6 @@ func (a *app) repoURL(owner, name string) string { return fmt.Sprintf("%s/~%s/%s", a.chrome.SelfOrigin(), owner, name) } -// hostOf returns the host authority of a URL, or the input unchanged if it does -// not parse as one (used only to synthesize a cosmetic commit-author email). -func hostOf(origin string) string { - if i := strings.Index(origin, "://"); i >= 0 { - origin = origin[i+3:] - } - if i := strings.IndexByte(origin, '/'); i >= 0 { - origin = origin[:i] - } - if origin == "" { - return "localhost" - } - return origin -} - // writeJSON writes v as a JSON response with the given status. func writeJSON(w http.ResponseWriter, status int, v any) { w.Header().Set("Content-Type", "application/json") diff --git a/web/handlers_internal_test.go b/web/handlers_internal_test.go index 7a980ec7a9e6af06089af39f3e11045f84a0ab21..6c92bf7a04653b0c0891084ff2b577079dc898fa 100644 --- a/web/handlers_internal_test.go +++ b/web/handlers_internal_test.go @@ -15,9 +15,11 @@ import ( ) // postInternalCreate calls handleInternalCreate directly with a JSON body, -// bypassing the router (and thus internalAuthGuard, which is exercised -// separately). The auth guard needs the process-global crypto/config state a -// unit test does not set up; the business logic under test here does not. +// bypassing the router and thus the internalauth guard. The guard's own +// behaviour is sr-ht-ecore's to test, and its agreement with the side that mints +// the header is cmd/dolt-git-hook's; what is left for here is the endpoint's +// business logic, which needs neither the network key nor a loaded +// internal-network list. func (h *harness) postInternalCreate(body string) *httptest.ResponseRecorder { req := httptest.NewRequest("POST", "/internal/repos", strings.NewReader(body)) req.Header.Set("Content-Type", "application/json") @@ -26,6 +28,25 @@ func (h *harness) postInternalCreate(body string) *httptest.ResponseRecorder { return rec } +// TestInternalCreateRouteIsGuarded is the one assertion the suite above cannot +// make by calling the handler: that /internal/repos is behind the guard at all. +// Nothing here fills the internal network list, so httptest's default TEST-NET-1 +// source address is external and the call is refused before its (absent) token +// matters — which is exactly the assertion. An unmounted guard would answer this +// body with a 422 for the unknown owner instead. +func TestInternalCreateRouteIsGuarded(t *testing.T) { + h := newHarness(t) + + req := httptest.NewRequest("POST", "/internal/repos", + strings.NewReader(`{"owner":"alice","name":"widgets"}`)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + h.router.ServeHTTP(rec, req) + + assert.Equal(t, http.StatusUnauthorized, rec.Code, rec.Body.String()) + assert.Empty(t, h.store.createdCalls, "an unauthenticated call must not reach the handler") +} + func TestInternalCreateProvisions(t *testing.T) { h := newHarness(t) h.users.byName["alice"] = &core.Caller{UserID: 7, Username: "alice"} diff --git a/web/router.go b/web/router.go index 5d4b65a7a0fde444173e4c9867aff6b1fd73e9f7..6c908a83434ade9e9ed9bf20b8347f173530240c 100644 --- a/web/router.go +++ b/web/router.go @@ -11,6 +11,7 @@ import ( "sourcecraft.dev/bigbes/sr-ht-ecore/assets" "sourcecraft.dev/bigbes/sr-ht-ecore/chrome" "sourcecraft.dev/bigbes/sr-ht-ecore/csrf" + "sourcecraft.dev/bigbes/sr-ht-ecore/internalauth" "sourcecraft.dev/bigbes/sr-ht-ecore/middleware" "sourcecraft.dev/bigbes/sr-ht-ecore/pages" @@ -156,7 +157,20 @@ func (a *app) mount(r chi.Router) { // Guarded by internal-network + network-key auth, and deliberately mounted // outside the same-origin group below: it is not a browser route, it carries // no Origin or Referer, and the CSRF guard would refuse every call. - r.With(internalAuthGuard).Post("/internal/repos", a.handleInternalCreate) + // + // The caller is pinned rather than left open. core-go's own check only asks + // that a token name *some* client and node, which on this endpoint would + // mean any program holding the instance's network key may provision a + // database for any user — and exactly one program on the instance has any + // business doing that, cmd/dolt-git-hook, whose ids these are. Widening this + // to a second caller should be a line changed here, not something that + // starts working by accident. + // + // The refusal is internalauth's own plain-text Deny (the nil handler): this + // endpoint answers a hook and not a browser, so the error page the rest of + // web/ renders would only be something for a Go client to discard. + r.With(internalauth.Guard(core.InternalClientID, core.InternalNodeID, nil)). + Post("/internal/repos", a.handleInternalCreate) // Everything a browser reaches. The same-origin guard is the group's, not // each mutating handler's: a predicate spelled per handler is protection