diff --git a/cmd/specsrht/graphql.go b/cmd/specsrht/graphql.go new file mode 100644 index 0000000000000000000000000000000000000000..548dd97ca40ff60a19dfae37b7b67b3c89999d47 --- /dev/null +++ b/cmd/specsrht/graphql.go @@ -0,0 +1,55 @@ +package main + +import ( + "strconv" + + "github.com/vaughan0/go-ini" +) + +// queryRoute is where the GraphQL schema answers. It is core-go's own path, +// because that is where every SourceHut client — hut, api.sr.ht, meta's +// personal-token page — already looks. The file beside it is served by +// sr-ht-ecore's apimeta, at apimeta.Path, because a service that mounts its own +// /query is the one thing core-go does not serve that file for. +const queryRoute = "/query" + +// apiScopes is what this service publishes at apimeta.Path: nothing. +// +// A scope is the part after the service name in a meta.sr.ht personal-token +// grant, and spec.sr.ht defines none — no AccessScope enum, no @access directive +// on any field, and no code path that reads one. Its grant vocabulary is +// tokens.sr.ht's (authn.ActionRead, authn.ActionPropose), which meta neither +// mints nor advertises, so the honest list is empty and not a placeholder. +// +// It is a variable so that what the daemon serves and what its test asserts are +// one value rather than two spellings of an intention. apimeta marshals it as [] +// and never as null; see the test for why that distinction is instance-wide. +var apiScopes []string + +// defaultMaxComplexity is the bound core-go's server.WithSchema would have +// applied. It is repeated here because this daemon does not call WithSchema — +// /query is mounted on the anonymous router with spec's own credential plane in +// front of it — and the value has a second reader that has nothing to do with +// HTTP: the webhook delivery worker runs a subscriber's stored query through +// corewebhooks.Exec, which compares its complexity against Server.MaxComplexity +// and refuses everything above it. Leaving the field at its zero value would +// therefore not mean "no limit"; it would mean every webhook delivery fails. +const defaultMaxComplexity = 250 + +// maxComplexity is [spec.sr.ht::api] max-complexity, or defaultMaxComplexity +// when the instance does not set it. +// +// An unparseable value is a configuration error and is reported as one, rather +// than being read as "the operator meant the default": a limit somebody wrote +// down and got wrong must not be silently replaced by a different limit. +func maxComplexity(conf ini.File) (int, error) { + raw, ok := conf.Get(serviceName+"::api", "max-complexity") + if !ok || raw == "" { + return defaultMaxComplexity, nil + } + limit, err := strconv.Atoi(raw) + if err != nil { + return 0, err + } + return limit, nil +} diff --git a/cmd/specsrht/graphql_test.go b/cmd/specsrht/graphql_test.go new file mode 100644 index 0000000000000000000000000000000000000000..be5678c9638a57acabed6b75df6684e96ff281cd --- /dev/null +++ b/cmd/specsrht/graphql_test.go @@ -0,0 +1,172 @@ +package main + +import ( + "database/sql" + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/vaughan0/go-ini" + + "sourcecraft.dev/bigbes/sr-ht-core/config" + "sourcecraft.dev/bigbes/sr-ht-core/crypto" + "sourcecraft.dev/bigbes/sr-ht-core/database" + "sourcecraft.dev/bigbes/sr-ht-ecore/apimeta" +) + +// TestMain initialises the crypto globals apimeta.Handler reads the webhook +// public key out of. core-go's server.New does it in the daemon; here the +// config from main_test.go stands in. +func TestMain(m *testing.M) { + crypto.InitCrypto(completeConfig()) + os.Exit(m.Run()) +} + +// The route the schema answers on is core-go's own, so that a client which found +// this service through meta.sr.ht — hut, api.sr.ht, a script written against +// git.sr.ht's API — finds /query where it already looks. A service that mounts +// its own endpoint gets no help from core-go here, which is exactly why the +// constant is asserted rather than assumed. +func TestQueryRouteIsCoreGosPath(t *testing.T) { + assert.Equal(t, "/query", queryRoute) + assert.Equal(t, queryRoute+"/api-meta.json", apimeta.Path) +} + +// api-meta.json must be served, and its scope list must be an empty array and +// never a JSON null. +// +// meta.sr.ht fetches this file from every service it discovers when it renders +// /oauth2/personal-token, and iterates the "scopes" field to build the grant +// checkboxes. A null there is a nil iteration in meta — a 500 on that page for +// the WHOLE instance, every service's grants and not just this one's. It is a +// failure nobody would find by testing the service that caused it, which is why +// the assertion lives here even though the marshalling is sr-ht-ecore's. +// +// Empty is also the honest answer for spec.sr.ht rather than a placeholder: this +// service defines no meta OAuth scope and no @access directive to check one +// against. Its grant vocabulary is tokens.sr.ht's — authn.ActionRead and +// authn.ActionPropose — which meta neither mints nor advertises. +func TestAPIMetaAdvertisesNoScopeAndNeverNull(t *testing.T) { + rec := httptest.NewRecorder() + // apiScopes and not a literal: this asserts what mountWeb actually serves. + apimeta.Handler(apiScopes...).ServeHTTP(rec, httptest.NewRequest(http.MethodGet, apimeta.Path, nil)) + + require.Equal(t, http.StatusOK, rec.Code) + assert.Contains(t, rec.Body.String(), `"scopes":[]`, + "a JSON null here is a 500 on meta's personal-token page for the whole instance") + + var got apimeta.Meta + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &got)) + assert.NotNil(t, got.Scopes) + assert.Empty(t, got.Scopes) + assert.NotEmpty(t, got.WebhookPubkey, "a webhook consumer verifies payloads with this") +} + +// The wiring itself, over a real chi router: the route the endpoint answers on, +// the two core-go context values the webhook resolvers reach for, and the +// api-meta.json beside it. +// +// The database context is the one worth a test rather than a comment. /query +// moved off the authenticated router, and WithDefaultMiddleware installs +// database.Middleware there and nowhere else — so without this Group every +// webhook mutation would panic on the first transaction, and no read would, +// which is exactly the shape of a bug that reaches production. +func TestMountGraphQL(t *testing.T) { + conf := completeConfig() + pool := &sql.DB{} // never queried: the handler only proves the context carries it + + var ( + reached bool + gotConf ini.File + gotPool *sql.DB + endpoint = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reached = true + gotConf = config.ForContext(r.Context()) + // DBForContext and not ForContext: the latter dials a connection, + // and the pool here is a zero value that would panic on one. Both + // read the same context value, which is what is under test. + gotPool = database.DBForContext(r.Context()) + w.WriteHeader(http.StatusTeapot) + }) + ) + + router := chi.NewRouter() + mountGraphQL(router, conf, pool, endpoint) + // The web UI claims "/", and "query" is a legal space name. chi resolves by + // trie specificity rather than by registration order — measured, not assumed + // — so /query wins over the catch-all; this is here to prove that rather + // than to rely on the order the daemon happens to register them in. + router.Mount("/", http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusGone) + })) + + srv := httptest.NewServer(router) + defer srv.Close() + + t.Run("the endpoint answers at /query with the core-go context", func(t *testing.T) { + resp, err := srv.Client().Post(srv.URL+queryRoute, "application/json", strings.NewReader(`{}`)) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusTeapot, resp.StatusCode, "the web UI's catch-all swallowed /query") + assert.True(t, reached) + assert.Equal(t, conf, gotConf, "config.ForContext panics without config.Middleware") + assert.Same(t, pool, gotPool, "the webhook resolvers open transactions through this") + }) + + t.Run("api-meta.json is served beside it", func(t *testing.T) { + resp, err := srv.Client().Get(srv.URL + apimeta.Path) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusOK, resp.StatusCode) + var got apimeta.Meta + require.NoError(t, json.NewDecoder(resp.Body).Decode(&got)) + assert.NotNil(t, got.Scopes) + assert.Empty(t, got.Scopes) + }) +} + +// The complexity bound core-go's WithSchema would have set. The daemon does not +// call WithSchema any more, and the value's second reader is not the HTTP +// surface at all: the webhook delivery worker runs a subscriber's stored query +// through corewebhooks.Exec, which refuses anything above Server.MaxComplexity. +// Zero there would fail every delivery rather than impose no limit, so what this +// function returns when the instance says nothing is load-bearing. +func TestMaxComplexity(t *testing.T) { + t.Run("defaults to core-go's bound", func(t *testing.T) { + limit, err := maxComplexity(completeConfig()) + require.NoError(t, err) + assert.Equal(t, 250, limit) + assert.Equal(t, defaultMaxComplexity, limit) + }) + + t.Run("an empty value is no value", func(t *testing.T) { + conf := completeConfig() + conf[serviceName+"::api"] = ini.Section{"max-complexity": ""} + limit, err := maxComplexity(conf) + require.NoError(t, err) + assert.Equal(t, defaultMaxComplexity, limit) + }) + + t.Run("the instance's value wins", func(t *testing.T) { + conf := completeConfig() + conf[serviceName+"::api"] = ini.Section{"max-complexity": "400"} + limit, err := maxComplexity(conf) + require.NoError(t, err) + assert.Equal(t, 400, limit) + }) + + t.Run("a value that does not parse is an error, not the default", func(t *testing.T) { + conf := completeConfig() + conf[serviceName+"::api"] = ini.Section{"max-complexity": "lots"} + _, err := maxComplexity(conf) + require.Error(t, err, "a limit somebody wrote down and got wrong must not be silently replaced") + }) +} diff --git a/cmd/specsrht/main.go b/cmd/specsrht/main.go index 8d678a15609bec4a671e88bcb8c3405c01d7d5e7..9b3f2b5eb1265a8eeb23b4e75856cd37f9457f66 100644 --- a/cmd/specsrht/main.go +++ b/cmd/specsrht/main.go @@ -81,16 +81,16 @@ "strings" "syscall" "time" - "github.com/99designs/gqlgen/graphql" "github.com/go-chi/chi/v5" chimw "github.com/go-chi/chi/v5/middleware" _ "github.com/lib/pq" // registers the "postgres" database/sql driver "github.com/vaughan0/go-ini" "go.bigb.es/auxilia/scribe" + "sourcecraft.dev/bigbes/sr-ht-ecore/apimeta" "sourcecraft.dev/bigbes/sr-ht-ecore/logging" - "sourcecraft.dev/bigbes/sr-ht-core/auth" "sourcecraft.dev/bigbes/sr-ht-core/config" + "sourcecraft.dev/bigbes/sr-ht-core/database" coreserver "sourcecraft.dev/bigbes/sr-ht-core/server" "sourcecraft.dev/bigbes/sr-ht-core/webhooks" @@ -347,30 +347,45 @@ } // server.New parses -b/-d/-m/-p and runs crypto.InitCrypto(conf), whose // two required keys validateConfig already checked, so it cannot fatal - // here for a reason we have not already reported. - // /query is served on the authenticated router so core-go installs the - // auth/database/server context the webhook engine needs — WithDefaultMiddleware - // brings that whole stack (and, deliberately, core-go auth: agents therefore - // read via MCP/REST, not /query). ownerOnly restricts /query to the instance - // owner and maps them to AUTH_INTERNAL so the webhook engine's - // NewAuthConfig/FilterWebhooks (which refuse cookie auth) accept them. - // WithQueues starts the webhook delivery worker with a context carrying that - // same stack; the queue executes a subscription's stored query against the - // shared schema at delivery time. - // The scope list must be an empty slice and not nil. core-go serves it - // verbatim at /query/api-meta.json, where a nil slice marshals to - // `"scopes": null` — and meta.sr.ht's OAuth page iterates that field for - // every service it discovers, so one null there is a 500 on - // /oauth2/personal-token for the whole instance, not a degraded entry. - // Empty is also the honest answer: this service is owner-only (see - // ownerOnly above) and defines no AccessScope enum to grant against. - webhookQueue := webhooks.NewQueue(surf.schema, conf) + // here for a reason we have not already reported. It must run before + // mountRoutes: apimeta.Handler reads the webhook public key InitCrypto + // establishes, once, when the handler is built. + // + // WithDefaultMiddleware is here for the database pool, the redis client and + // the email queue that WithQueues hands the webhook delivery worker — not + // for the authenticated router it decorates, which now carries no routes at + // all. There is deliberately no WithSchema: it would mount /query on that + // authenticated router, behind core-go's auth.Middleware, which speaks + // meta.sr.ht's OAuth vocabulary and not the tokens.sr.ht one every other + // surface of this service accepts. /query is mounted on the anonymous + // router by mountRoutes instead, with graph's own credential middleware in + // front of it, and api-meta.json is served there too because core-go serves + // that file only for the schemas it hosts itself. + // + // MaxComplexity is the one thing WithSchema set that still has to be set, + // and its reader has nothing to do with serving /query: the webhook delivery + // worker runs a subscriber's stored query through corewebhooks.Exec, which + // reads the bound off this field — through the context WithQueues gives it, + // which is the one context in this daemon that still carries core-go's + // server — and refuses everything above it. Zero does not mean "no limit" + // there; it means every delivery fails, logged and not raised. Measured, by + // removing this line: "operation has complexity 2, which exceeds the maximum + // of 0" and no delivery. + // + // Server.Schema is deliberately not set. WithSchema assigns it, but nothing + // in core-go reads it back — the delivery worker executes against the schema + // it was handed in NewQueue, and the resolvers that used to reach for it + // through the server context now hold their own. + limit, err := maxComplexity(conf) + if err != nil { + return fmt.Errorf("[%s::api] max-complexity: %w", serviceName, err) + } + webhookQueue := webhooks.NewQueue(surf.gql.Schema(), conf) srv := coreserver.New(serviceName, defaultBind, conf, os.Args). - WithDefaultMiddleware(). - WithMiddleware(ownerOnly(cfg.Instance.OwnerName)). - WithSchema(surf.schema, []string{}). - WithQueues(webhookQueue.Queue) - mountRoutes(srv.AnonRouter(), conf, surf) + WithDefaultMiddleware() + srv.MaxComplexity = limit + srv.WithQueues(webhookQueue.Queue) + mountRoutes(srv.AnonRouter(), conf, pool, surf) // Now that the webhook queue is started (WithQueues gave its worker the // server+database+config context), install the sink so proposal lifecycle @@ -501,7 +516,7 @@ return nil } // mountRoutes installs what the daemon serves over HTTP. -func mountRoutes(router chi.Router, conf ini.File, surfaces *surfaces) { +func mountRoutes(router chi.Router, conf ini.File, pool *sql.DB, surfaces *surfaces) { // server.New already froze the anonymous router for direct middleware // registration, so middleware and routes go in together inside a Group — // which chi permits on a fresh inline mux sharing the same routing tree. @@ -514,7 +529,7 @@ fmt.Fprint(w, "ok") }) }) - mountWeb(router, conf, surfaces) + mountWeb(router, conf, pool, surfaces) } // surfaces are the three Phase 2 read surfaces, assembled once at startup. @@ -527,13 +542,12 @@ web *web.Server mcp http.Handler api http.Handler - // schema is the GraphQL executable schema. Unlike the other surfaces it is - // not a mounted handler: /query is served by core-go's server (WithSchema) - // on the authenticated router, because the webhook engine needs core-go's - // auth/database/server context there. The same schema is also handed to the - // webhook queue, which executes a subscription's stored query against it at - // delivery time — one schema, both callers. - schema graphql.ExecutableSchema + // gql is the /query endpoint. Like /mcp and /api it carries its own + // credential middleware and is mounted on the anonymous router; unlike them + // it also owns the executable schema, which is handed to the webhook queue + // so a subscription's stored query is executed at delivery time against + // exactly the schema its author wrote it for. + gql *graph.Server } // newSurfaces opens the index and builds the three read surfaces over it. @@ -586,7 +600,11 @@ // /query and the web UI apply. spec_propose stays fail-closed in service/ too; // the gate just makes the read tools match. mcp = svc.Resolver().Middleware()(mcpsrv.Gate(mcp)) - schema, err := graph.NewSchema(graph.Options{ + // /query, with the same credential plane /mcp and /api use — the resolver is + // the one svc holds, so a token that reads through one surface reads through + // all three. graph.Server installs that middleware itself, which is why it is + // mounted on the anonymous router below and not through core-go's WithSchema. + gql, err := graph.New(graph.Options{ Reader: svc, Searcher: index, Proposals: graph.NewProposals(svc), @@ -594,7 +612,7 @@ Resolver: svc.Resolver(), }) if err != nil { index.Close() - return nil, fmt.Errorf("assemble the GraphQL schema: %w", err) + return nil, fmt.Errorf("assemble the GraphQL surface: %w", err) } rest, err := api.New(api.Options{Writer: svc, Resolver: svc.Resolver()}) @@ -603,7 +621,7 @@ index.Close() return nil, fmt.Errorf("assemble the REST write surface: %w", err) } - return &surfaces{index: index, web: site, mcp: mcp, api: rest.Handler(), schema: schema}, nil + return &surfaces{index: index, web: site, mcp: mcp, api: rest.Handler(), gql: gql}, nil } func (s *surfaces) Close() error { @@ -614,53 +632,65 @@ return s.index.Close() } // mountWeb attaches the anonymous-router HTTP surfaces: the web UI, the MCP -// endpoint, and the REST write plane. /query is NOT here — it is served by -// core-go's server on the authenticated router (see run), because the webhook -// engine needs core-go's auth/database/server context, which only -// WithDefaultMiddleware installs. +// endpoint, the REST write plane and /query. // -// Order is load-bearing: /mcp and /api are registered before the web UI, which -// mounts at "/" and would otherwise swallow them as document paths — "mcp" and -// "api" are both legal space names as far as the router is concerned. -// -// These three keep spec's own authentication (agent bearer tokens, -// anonymous-capable reads, login redirects), which is why they stay on the anon -// router rather than behind core-go's 401-by-default auth. -// ownerOnly is the /query access gate and the webhook engine's auth adapter, in -// one middleware. It runs after core-go's auth.Middleware (which has already -// 401'd anyone without a valid credential and resolved a username), so: +// All four keep spec's own authentication (tokens.sr.ht working tokens, +// anonymous-capable reads, login redirects for the browser), which is why they +// are here rather than behind core-go's 401-by-default auth. /query used to be +// the exception, served by core-go's server.WithSchema on the authenticated +// router; it authenticated with meta's OAuth vocabulary there, which is not the +// vocabulary the other three speak, and a caller holding a working token that +// works everywhere else on this service was refused by the one surface meant to +// be the instance-native read plane. // -// - A non-owner authenticated user is refused with 403. core-go's auth admits -// any valid meta user — it JIT-creates a row on a table miss — but spec.sr.ht -// is single-owner: only the configured owner-name may reach /query at all. -// This restores what spec's own authn did (everyone but the owner is nobody) -// now that /query is behind core-go auth. -// - The owner's context is remapped to AUTH_INTERNAL. The webhook engine's -// NewAuthConfig and FilterWebhooks refuse AUTH_COOKIE outright, and INTERNAL -// bypasses the (unused) @access scope checks — so this remap is what lets the -// single owner, authenticated by a web cookie, manage webhooks. -func ownerOnly(owner string) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ac := auth.ForContext(r.Context()) - if ac.Username != owner { - http.Error(w, "spec.sr.ht: only the instance owner may use /query", http.StatusForbidden) - return - } - internal := *ac - internal.AuthMethod = auth.AUTH_INTERNAL - next.ServeHTTP(w, r.WithContext(auth.Context(r.Context(), &internal))) - }) - } -} - -func mountWeb(router chi.Router, _ ini.File, s *surfaces) { +// "mcp", "api" and "query" are all legal space names as far as the router is +// concerned, so each of them is a path the web UI's "/" mount could plausibly +// serve as a document. It does not: chi resolves by trie specificity and not by +// registration order, which was measured rather than assumed — a preceding +// comment here asserted the opposite, and moving mountGraphQL after the "/" +// mount changes no route. The registration order below is for reading, not for +// routing. +func mountWeb(router chi.Router, conf ini.File, pool *sql.DB, s *surfaces) { if s == nil { return } router.Handle("/mcp", s.mcp) router.Mount("/api", s.api) + mountGraphQL(router, conf, pool, s.gql) router.Mount("/", s.web.Handler()) +} + +// mountGraphQL installs /query and the api-meta.json beside it. +// +// The endpoint carries its own credential middleware, so it goes here on the +// anonymous router rather than through core-go's server.WithSchema. It does need +// two things from the router that /mcp and /api do not: +// +// - core-go's config and database middleware. The webhook management resolvers +// open transactions through core-go's database context, and +// WithDefaultMiddleware installs that on the authenticated router only — +// which this is not. +// - api-meta.json, at core-go's own path. meta.sr.ht fetches that file from +// every service it discovers to build /oauth2/personal-token, core-go serves +// it only for the schemas it hosts itself, and this service now hosts its +// own. A 404 there is a broken personal-token page for the whole instance. +// +// The middleware goes on a Group and not on router, because chi refuses a Use +// once any route exists on a mux, and this router already has some. The Group is +// a fresh inline mux over the same routing tree, which is where middleware and +// routes can still be attached together. +// +// api-meta.json is outside that Group deliberately: it is a static document that +// reads neither the config nor the database, and giving it a database +// transaction's worth of setup for every poll from meta would be work done for +// nobody. +func mountGraphQL(router chi.Router, conf ini.File, pool *sql.DB, gql http.Handler) { + router.Group(func(r chi.Router) { + r.Use(config.Middleware(conf, serviceName)) + r.Use(database.Middleware(pool)) + r.Handle(queryRoute, gql) + }) + router.Get(apimeta.Path, apimeta.Handler(apiScopes...)) } // pushNotifier is what the daemon does when a push lands. diff --git a/go.mod b/go.mod index 6927b489000693679a5b47c1e2a6880d5a9c2e30..4f50a6b00752c9d6176b6244e6ddea2c10ae3840 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ github.com/yuin/goldmark v1.8.2 go.bigb.es/auxilia v0.7.0 gopkg.in/yaml.v3 v3.0.1 sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260718185800-dd418a200152 - sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808210553-b6bf6d28db15 + sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260815100501-2e37ec734334 ) require ( diff --git a/go.sum b/go.sum index 8627a42adc5bda0f2eb5e6f0396c9453e8ba33ea..9a406a279b272e2a2ac622869a4c44d1e074ec39 100644 --- a/go.sum +++ b/go.sum @@ -421,5 +421,5 @@ modernc.org/sqlite v1.38.2 h1:Aclu7+tgjgcQVShZqim41Bbw9Cho0y/7WzYptXqkEek= modernc.org/sqlite v1.38.2/go.mod h1:cPTJYSlgg3Sfg046yBShXENNtPrWrDX8bsbAQBzgQ5E= sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260718185800-dd418a200152 h1:9kQC+tDO2CO8avlKadb9Z0if4a6vJuEK80+4zcb6/fU= sourcecraft.dev/bigbes/sr-ht-core v0.0.0-20260718185800-dd418a200152/go.mod h1:Mu1Vx39ws/OTKWGoVERXvkdRSPLBdhuFTYv0ftVV31c= -sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808210553-b6bf6d28db15 h1:zVtZHaKHtdD5Q2Stw8OwuMENl6xvzp32IoHikrB0QWM= -sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260808210553-b6bf6d28db15/go.mod h1:JjrjFxb+PSG0AT9waiK7teFbpZmBPisX3MuqI7Z+ZO8= +sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260815100501-2e37ec734334 h1:xpY9JZpz+T924BI2VRiZcY8mFnZ7LtyrIAI8hjLsBzc= +sourcecraft.dev/bigbes/sr-ht-ecore v0.0.0-20260815100501-2e37ec734334/go.mod h1:JjrjFxb+PSG0AT9waiK7teFbpZmBPisX3MuqI7Z+ZO8= diff --git a/graph/credential_test.go b/graph/credential_test.go new file mode 100644 index 0000000000000000000000000000000000000000..5cc0698441768dd3bd3431ea3fec9bc28d2f6c7a --- /dev/null +++ b/graph/credential_test.go @@ -0,0 +1,219 @@ +package graph + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "sourcecraft.dev/bigbes/sr-ht-core/auth" + "sourcecraft.dev/bigbes/sr-ht-ecore/bearer" + + "sourcecraft.dev/bigbes/sr-ht-spec/authn" +) + +// The credential plane of /query, end to end through the real handler chain. +// +// grant_test.go exercises the gate as a unit, over a principal somebody handed +// it. These go in at the door instead — an HTTP request carrying a credential — +// because what changed in the conversion is which credentials reach the gate at +// all, and a test written against a Principal cannot see that. + +// probeQuery is a cheap read: it needs no fixture beyond the fake reader and it +// is refused before parsing when the caller has no authority, so the status is +// the whole answer. +const probeQuery = `{ spaces { ref } }` + +// request builds the POST the harness would send, and hands back the recorder +// as well, for the assertions that are about a header rather than a body. +func request(t *testing.T, q string, credential func(*http.Request)) (*http.Request, *httptest.ResponseRecorder) { + t.Helper() + body, err := json.Marshal(map[string]any{"query": q}) + require.NoError(t, err) + req := httptest.NewRequest(http.MethodPost, "/query", bytes.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + if credential != nil { + credential(req) + } + return req, httptest.NewRecorder() +} + +// metaPAT mints a bearer token sealed with the instance's key but stamped with +// an OAuth client id rather than tokens.sr.ht's. That is exactly what a +// meta.sr.ht personal access token is on this instance — same format, same key, +// different issuer — and the client id is the only thing that tells the two +// apart (see sr-ht-ecore/bearer, step 2). +func metaPAT() string { + bt := &auth.BearerToken{ + Version: auth.TokenVersion, + Expires: auth.ToTimestamp(time.Now().Add(time.Hour)), + Grants: "", + ClientID: "00000000-0000-0000-0000-00000000beef", + Username: "bigbes", + } + return bt.Encode() +} + +// foreignToken mints a working token belonging to somebody who is not the +// instance owner. +func foreignToken(grantString string) string { + bt := &auth.BearerToken{ + Version: auth.TokenVersion, + Expires: auth.ToTimestamp(time.Now().Add(time.Hour)), + Grants: grantString, + ClientID: bearer.TokensClientID, + Username: "somebody-else", + } + return bt.Encode() +} + +// A tokens.sr.ht working token carrying spec:read reads. This is the credential +// the whole conversion is for: the same one /mcp and the REST write plane take, +// so a token that works against one surface of this service works against all of +// them. +func TestWorkingTokenWithTheReadGrantReads(t *testing.T) { + h := newHarness(t, false) + + r := post(t, h, probeQuery, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+agentToken(authn.ActionRead)) + }) + + require.Equal(t, http.StatusOK, r.status, "body %s", r.body) + assert.Empty(t, r.errText()) + assert.Contains(t, string(r.Data), "~bigbes/rfcs") +} + +// The universal grant covers spec:read like any other action, so a token minted +// with "*" reads too. It is asserted separately because "*" is not a member of +// the set and a grant check written as a set lookup would refuse it. +func TestUniversalGrantReads(t *testing.T) { + h := newHarness(t, false) + + r := post(t, h, probeQuery, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+agentToken("*")) + }) + + require.Equal(t, http.StatusOK, r.status, "body %s", r.body) + assert.Empty(t, r.errText()) +} + +// A working token that verifies but was not minted for reading is 403 and not +// 401: the credential is good and the holder is known, so retrying with it is +// pointless and what they need is a wider grant. The refusal names the grant, +// because a client that is not told which one it lacks cannot ask for it. +func TestWorkingTokenWithoutTheReadGrantIsRefused(t *testing.T) { + h := newHarness(t, false) + + r := post(t, h, probeQuery, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+agentToken(authn.ActionPropose)) + }) + + require.Equal(t, http.StatusForbidden, r.status, "body %s", r.body) + assert.Contains(t, r.body, authn.ActionRead) + assert.NotContains(t, r.body, "~bigbes/rfcs", "the refusal leaked content") +} + +// The unified-login cookie is not a credential on this endpoint, and the owner's +// own cookie is refused along with everybody else's. That is the half of the +// conversion a status code alone would not prove, so it is asserted twice: on +// the bare handler, and mounted under the very middleware that would resolve the +// cookie into the owner principal. The second case is the one that matters — +// resolveCaller overwrites the principal rather than inheriting it, so no +// arrangement of middleware above the mount point can promote a browser session +// into read authority here. +func TestCookieIsNotACredentialHere(t *testing.T) { + t.Run("bare handler", func(t *testing.T) { + h := newHarness(t, false) + r := post(t, h, probeQuery, func(req *http.Request) { login(req, "bigbes") }) + + assert.Equal(t, http.StatusUnauthorized, r.status, "body %s", r.body) + assert.NotContains(t, r.body, "~bigbes/rfcs", "the refusal leaked content") + }) + + t.Run("under a router that does resolve the cookie", func(t *testing.T) { + resolver := testResolver(t) + srv, err := New(Options{Reader: newFakeReader(), Searcher: &fakeSearcher{}, Resolver: resolver}) + require.NoError(t, err) + + // The cookie plane, installed above the endpoint. It resolves the + // owner's cookie to authn.KindOwner, which CanRead admits. + h := harness{handler: resolver.Middleware()(srv)} + r := post(t, h, probeQuery, func(req *http.Request) { login(req, "bigbes") }) + + assert.Equal(t, http.StatusUnauthorized, r.status, "body %s", r.body) + assert.NotContains(t, r.body, "~bigbes/rfcs", "the refusal leaked content") + }) +} + +// A meta.sr.ht personal access token is refused, and this is a deliberate +// difference from dolt.sr.ht rather than an oversight. +// +// spec.sr.ht authenticates through one issuer (authn's package comment) and +// publishes no OAuth scope for meta to grant against — cmd/specsrht serves +// api-meta.json with an empty scope list — so there is nothing a PAT could be +// scoped *for* here. bearer classifies it ErrNotOurs and authn calls that +// permanent, hence 401 with the challenge rather than a 503. +// +// It is the credential this endpoint used to take, when core-go's +// auth.Middleware stood in front of it, and the test exists to pin the change +// rather than to celebrate it: reopening that plane means giving spec.sr.ht a +// meta scope first. +func TestMetaPersonalAccessTokenIsRefused(t *testing.T) { + h := newHarness(t, false) + + r := post(t, h, probeQuery, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+metaPAT()) + }) + + assert.Equal(t, http.StatusUnauthorized, r.status, "body %s", r.body) + assert.NotContains(t, r.body, "~bigbes/rfcs", "the refusal leaked content") +} + +// A working token belonging to another meta.sr.ht account is 403, not 401 and +// not admitted as a second identity: the token verifies and the holder is who +// they say they are, there is simply nothing on this single-owner instance to +// grant them. This is the ownerOnly rule the conversion had to keep, moved from +// a comparison against auth.AuthContext.Username to authn's own. +func TestWorkingTokenOfAnotherOwnerIsRefused(t *testing.T) { + h := newHarness(t, false) + + r := post(t, h, probeQuery, func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+foreignToken(authn.ActionRead)) + }) + + assert.Equal(t, http.StatusForbidden, r.status, "body %s", r.body) + assert.NotContains(t, r.body, "~bigbes/rfcs", "the refusal leaked content") +} + +// Every 401 carries the challenge, naming the scheme and this service's config +// section as the realm. RFC 9110 requires it, and the caller here is always a +// machine holding a bearer token: without it nothing tells the client which +// credential was refused. +func TestEveryUnauthorizedCarriesTheBearerChallenge(t *testing.T) { + h := newHarness(t, false) + + cases := map[string]func(*http.Request){ + "no credential": nil, + "owner's cookie": func(req *http.Request) { login(req, "bigbes") }, + "a forged token": func(req *http.Request) { + req.Header.Set("Authorization", "Bearer not-a-real-token") + }, + "a meta PAT": func(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+metaPAT()) + }, + } + for name, credential := range cases { + t.Run(name, func(t *testing.T) { + req, rec := request(t, probeQuery, credential) + h.handler.ServeHTTP(rec, req) + + require.Equal(t, http.StatusUnauthorized, rec.Code, "body %s", rec.Body.String()) + assert.Equal(t, authn.Challenge(), rec.Header().Get("WWW-Authenticate")) + }) + } +} diff --git a/graph/graph_test.go b/graph/graph_test.go index b6aed3a1a3d5da2b9d2693d2866c2a76ca2a7c49..d6733c3154268d9ed1c81c96531638437ab09cd2 100644 --- a/graph/graph_test.go +++ b/graph/graph_test.go @@ -340,7 +340,7 @@ srv, err := New(opts) if err != nil { t.Fatalf("New: %v", err) } - return harness{handler: srv.Handler(), searcher: searcher, proposals: proposals} + return harness{handler: srv, searcher: searcher, proposals: proposals} } // response is one GraphQL response, decoded far enough to assert on. @@ -364,10 +364,18 @@ } return strings.Join(msgs, "; ") } -// query POSTs a GraphQL query as the instance owner. +// query POSTs a GraphQL query with the credential this endpoint accepts: a +// tokens.sr.ht working token belonging to the instance owner and carrying +// spec:read. It used to seal the owner's unified-login cookie, which /query no +// longer looks at — see TestCookieIsNotACredentialHere. func query(t *testing.T, h harness, q string) response { t.Helper() - return post(t, h, q, func(req *http.Request) { login(req, "bigbes") }) + return post(t, h, q, readToken) +} + +// readToken presents a working token carrying authn.ActionRead. +func readToken(req *http.Request) { + req.Header.Set("Authorization", "Bearer "+agentToken(authn.ActionRead)) } func post(t *testing.T, h harness, q string, auth func(*http.Request)) response { @@ -1071,7 +1079,7 @@ if err != nil { t.Fatalf("New: %v", err) } router := chi.NewRouter() - router.Handle("/query", srv.Handler()) + router.Handle("/query", srv) h := harness{handler: router} var got struct{ Space struct{ Ref string } } diff --git a/graph/resolver.go b/graph/resolver.go index 4e517ca74b0c7c958c47fc1e619f3c8cecbed796..08d0a752f1c1d79ea77e50777496229eccb31bdb 100644 --- a/graph/resolver.go +++ b/graph/resolver.go @@ -4,6 +4,8 @@ import ( "context" "time" + "github.com/99designs/gqlgen/graphql" + "sourcecraft.dev/bigbes/sr-ht-spec/core" "sourcecraft.dev/bigbes/sr-ht-spec/doc" "sourcecraft.dev/bigbes/sr-ht-spec/search" @@ -97,10 +99,18 @@ _ Reader = (*service.Service)(nil) _ Searcher = (*search.Index)(nil) ) -// Resolver is the root resolver. It holds only the seams above; every field of -// the schema is answered by calling through them. +// Resolver is the root resolver. It holds the seams above — every read field of +// the schema is answered by calling through them — and the schema it is itself +// part of. type Resolver struct { reader Reader searcher Searcher proposals Proposals + + // schema is this service's executable schema, set by newSchema once the + // resolver it is built from exists. The webhook management resolvers need + // it: createUserWebhook validates a subscriber's stored query against it. + // It used to be read off core-go's server context, which is installed on + // the authenticated router and which /query no longer runs behind. + schema graphql.ExecutableSchema } diff --git a/graph/schema.resolvers.go b/graph/schema.resolvers.go index f391d51e147f4f4d2cccccbfd4c155062682fc47..e03bbbf69e481903edea6c9370e5662f471683d8 100644 --- a/graph/schema.resolvers.go +++ b/graph/schema.resolvers.go @@ -11,16 +11,13 @@ "errors" "fmt" "net/url" "strings" - "time" sq "github.com/Masterminds/squirrel" - "github.com/google/uuid" "github.com/lib/pq" "sourcecraft.dev/bigbes/sr-ht-core/auth" "sourcecraft.dev/bigbes/sr-ht-core/database" coreerrors "sourcecraft.dev/bigbes/sr-ht-core/errors" model1 "sourcecraft.dev/bigbes/sr-ht-core/model" - "sourcecraft.dev/bigbes/sr-ht-core/server" corewebhooks "sourcecraft.dev/bigbes/sr-ht-core/webhooks" "sourcecraft.dev/bigbes/sr-ht-spec/core" "sourcecraft.dev/bigbes/sr-ht-spec/doc" @@ -40,8 +37,10 @@ if err := webhookAuthorized(ctx); err != nil { return nil, err } - schema := server.ForContext(ctx).Schema - if err := corewebhooks.Validate(schema, config.Query); err != nil { + // The schema the root resolver holds, and not core-go's server context: + // /query is served on the anonymous router, where that context does not + // exist. It is the same schema either way — the daemon builds exactly one. + if err := corewebhooks.Validate(r.schema, config.Query); err != nil { return nil, err } @@ -459,61 +458,37 @@ return &model.WebhookDeliveryCursor{Results: deliveries, Cursor: cursor}, nil } // Sample is the resolver for the sample field. +// +// It is the one field of this schema that /query cannot answer, and it says so +// rather than failing at a lower level with something a caller cannot read. +// +// The reason is core-go's, not this service's. corewebhooks.WebhookContext.Exec +// — which is what renders a sample, by running the subscriber's own stored query +// against a synthetic payload — reads the complexity limit off core-go's server +// context and panics when there is none. That context is installed by +// server.WithDefaultMiddleware on the *authenticated* router, and this endpoint +// is on the anonymous one because it authenticates with tokens.sr.ht working +// tokens rather than with meta's OAuth vocabulary. There is no exported way to +// put that value on a context; the alternatives are to fork Exec into this +// service or to let the panic surface as "internal system error", and a named +// refusal beats both. +// +// Delivery is unaffected: the webhook queue's own context comes from +// server.WithQueues, which does carry it, so a real delivery renders exactly as +// before. func (r *userWebhookSubscriptionResolver) Sample(ctx context.Context, obj *model.UserWebhookSubscription, event model.WebhookEvent) (string, error) { - payloadUUID := uuid.New() - webhook := corewebhooks.WebhookContext{ - User: auth.ForContext(ctx), - PayloadUUID: payloadUUID, - Name: "user", - Event: event.String(), - Subscription: &corewebhooks.WebhookSubscription{ - ID: obj.ID, - URL: obj.URL, - Query: obj.Query, - AuthMethod: obj.AuthMethod, - TokenHash: obj.TokenHash, - Grants: obj.Grants, - ClientID: obj.ClientID, - Expires: obj.Expires, - NodeID: obj.NodeID, - }, - } - switch event { case model.WebhookEventProposalOpened, model.WebhookEventProposalMerged, model.WebhookEventProposalRejected: - // A synthetic proposal so the sample renders without a DB round-trip. The - // payload carries a fully-populated *model.Proposal, so ProposalEvent's - // generated field resolvers read it straight off the struct. - now := time.Now().UTC() - webhook.Payload = &model.ProposalEvent{ - UUID: payloadUUID.String(), - Event: event, - Date: now, - Proposal: &model.Proposal{ - ID: -1, - Space: "~owner/example", - Title: "Example proposal", - Rationale: "A sample proposal for webhook testing.", - BaseRev: "0000000000000000000000000000000000000000", - Branch: "proposals/0", - State: model.ProposalStateOpen, - Agent: "example-agent/sample", - AgentSession: "00000000-0000-0000-0000-000000000000", - Created: now, - }, - } default: return "", fmt.Errorf("unsupported event %s", event.String()) } - - subctx := corewebhooks.Context(ctx, webhook.Payload) - bytes, err := webhook.Exec(subctx, server.ForContext(ctx).Schema) - if err != nil { - return "", err - } - return string(bytes), nil + return "", fmt.Errorf( + "sample is not available on this endpoint: rendering one needs core-go's server " + + "context, which exists only on the authenticated router, and spec.sr.ht serves " + + "/query on the anonymous router so that a tokens.sr.ht working token is the " + + "credential; subscribe and trigger the event to see a real delivery") } // Subscription is the resolver for the subscription field. @@ -763,11 +738,24 @@ } return ref, nil } -// webhookAuthorized permits only the instance owner to manage webhooks. The -// /query owner-only middleware maps the owner (and only the owner) to -// AUTH_INTERNAL; core-go's auth would otherwise admit any authenticated meta -// user (it JIT-creates a row on a table miss), and spec.sr.ht is single-owner. -// A non-INTERNAL method here is a non-owner and is refused. +// webhookAuthorized permits only the instance owner, and the agents acting for +// it, to manage webhooks. +// +// The AuthContext it reads is coreauth's: the graph server derives one from the +// principal /query's credential gate already admitted, and coreauth maps the +// owner and its agents — and nobody else — to AUTH_INTERNAL. So a non-INTERNAL +// method here is a caller with no authority over this instance's webhooks and is +// refused. +// +// What it does NOT check is a grant, and that is a gap rather than a decision: +// the endpoint's gate admits authn.ActionRead, so a working token carrying only +// spec:read reaches these mutations and may manage subscriptions with them. The +// identity requirement is unchanged — the token has to belong to [sr.ht] +// owner-name or authn refuses it at the door — but the grant vocabulary has no +// entry that means "manage this service's webhooks", and inventing one here +// would be a string tokens.sr.ht has never minted and no existing token carries. +// Closing it is a vocabulary change, declared in authn beside ActionRead and +// ActionPropose, and it is deliberately not done in passing. func webhookAuthorized(ctx context.Context) error { if auth.ForContext(ctx).AuthMethod != auth.AUTH_INTERNAL { return coreerrors.ErrAccessDenied diff --git a/graph/server.go b/graph/server.go index ec2f2e62d04c19bbf655ab0c7796f64e122f0bb7..ed6f989d88e743915241c63e6c06dfc3eb5e6b12 100644 --- a/graph/server.go +++ b/graph/server.go @@ -2,14 +2,16 @@ // Package graph is spec.sr.ht's GraphQL read schema, served at /query. // // # Read only, deliberately // -// There are no mutations here. The design defers them until the proposal state -// machine has settled, and the reason is technical rather than scope -// discipline: the write plane's concurrency story is `If-Match: `, an -// HTTP idiom with well-defined 409 semantics that agents get right by default, -// and a type that has been federated into api.sr.ht is a consumed contract — -// expensive to churn. Read types (space, document, project, search) are stable -// from the start; the review types are not, and that is where the line is -// drawn. +// There are no proposal mutations here. The design defers them until the +// proposal state machine has settled, and the reason is technical rather than +// scope discipline: the write plane's concurrency story is `If-Match: +// `, an HTTP idiom with well-defined 409 semantics that agents get +// right by default, and a type that has been federated into api.sr.ht is a +// consumed contract — expensive to churn. Read types (space, document, project, +// search) are stable from the start; the review types are not, and that is +// where the line is drawn. The webhook management mutations are the exception +// and are not a proposal write: core-go's webhook engine is GraphQL-native and +// has no other surface. // // Serving GraphQL at all is justified without federation: Phase 5's webhooks // are GraphQL-native so gqlgen arrives regardless, and this is the read surface @@ -18,15 +20,32 @@ // Federating into api.sr.ht is then one `api-origin=` line on the gateway that // nothing here depends on — `hut` builds its endpoint from the per-service // origin and talks to this /query directly either way. // -// # Who may read +// # Who may read, and with what +// +// The read plane is fail-closed and this is the same one-line ACL web/ and +// mcpsrv/ apply: the instance owner's agents may read, and nobody else may. A +// viewer with no read authority gets 401 before the query is parsed — including +// for introspection, which is why a gateway federating this schema has to +// present a token like any other client. +// +// The credential is the bearer plane /mcp and the REST write plane already +// define, and nothing else: // -// The read plane is fail-closed and this is the same one-line ACL web/ applies: -// the instance owner and its agents may read, and nobody else may. A viewer -// with no read authority gets 401 before the query is parsed — including for -// introspection, which is why a gateway federating this schema has to present a -// token like any other client. +// - A tokens.sr.ht working token, verified through sr-ht-ecore's bearer +// package by authn.Resolver, owned by [sr.ht] owner-name, and carrying +// authn.ActionRead. A token that verifies but does not carry that grant is +// 403; one that does not verify is 401 with the bearer challenge. +// - No cookie. An API client is not a browser. The unified-login cookie is +// web/'s plane, and this endpoint is deliberately outside it — so the +// principal is overwritten with the anonymous one when no bearer credential +// is presented, rather than inherited from whatever middleware happens to +// sit above the mount point. +// - No meta.sr.ht personal access token. spec.sr.ht authenticates through one +// issuer (see authn's package comment) and publishes no OAuth scope for meta +// to grant against, so there is nothing a PAT could be scoped for here. A +// PAT is bearer.ErrNotOurs and is refused at the door with 401. // -// # What the cmd layer must wire +// # What the cmd layer wires // // gql, err := graph.New(graph.Options{ // Reader: svc, // *service.Service @@ -36,23 +55,36 @@ // }) // if err != nil { // return err // } -// router.Handle("/query", gql.Handler()) +// router.Handle("/query", gql) +// +// Server installs its own credential middleware, so it goes on the *anonymous* +// router: core-go's server.WithSchema would mount it on the authenticated one, +// whose auth.Middleware answers meta's OAuth vocabulary and 401s anything else +// — the vocabulary this service deliberately does not speak. A service that +// mounts its own /query owes the instance api-meta.json as well, because +// core-go serves that file only for the schemas it hosts itself; sr-ht-ecore's +// apimeta package is what serves it, and cmd/specsrht wires it beside the +// route. // -// [Server.Handler] installs authn's principal middleware itself, so it can be -// mounted on a router that has none. A caller whose router already resolves a -// principal uses [Server.Endpoint] instead. +// The router it is mounted on must carry core-go's config and database +// middleware. The read path does not need either, but the webhook management +// resolvers open transactions through core-go's database context, and +// WithDefaultMiddleware installs that on the authenticated router only. package graph import ( "fmt" + "log/slog" "net/http" "github.com/99designs/gqlgen/graphql" "github.com/99designs/gqlgen/graphql/handler" "github.com/99designs/gqlgen/graphql/handler/extension" "github.com/99designs/gqlgen/graphql/handler/transport" + "go.bigb.es/auxilia/scribe" "sourcecraft.dev/bigbes/sr-ht-spec/authn" + "sourcecraft.dev/bigbes/sr-ht-spec/coreauth" "sourcecraft.dev/bigbes/sr-ht-spec/graph/api" ) @@ -71,23 +103,26 @@ // and while it is nil the `proposals` field of the schema fails with an // error saying so rather than answering "none". Proposals Proposals - // Resolver turns the unified-login cookie or an agent bearer token into a - // principal. Handler installs its middleware; Endpoint does not. + // Resolver verifies the bearer credential a caller presents. Its cookie + // plane is not used here: see the package comment. Resolver *authn.Resolver } -// Server is the /query endpoint: the executable schema plus the read gate. It -// is built once at startup and is safe for concurrent use. +// Server is the /query endpoint: the executable schema behind the credential +// gate. It is built once at startup and is safe for concurrent use. type Server struct { - exec http.Handler - resolver *authn.Resolver + http http.Handler + schema graphql.ExecutableSchema } -// NewSchema builds the executable schema over the seams in opts. The daemon -// hands it to core-go's server.WithSchema (to serve /query) and to -// webhooks.NewQueue (which executes a subscription's stored query against it at -// delivery time), so both share exactly one schema. -func NewSchema(opts Options) (graphql.ExecutableSchema, error) { +// newSchema builds the executable schema over the seams in opts. +// +// It is unexported now that nothing outside this package wants a schema without +// an endpoint. The daemon needs both — the endpoint to serve /query, and the +// schema to hand to webhooks.NewQueue, which executes a subscription's stored +// query at delivery time — and takes them from one Server, so that the two +// cannot become two schemas. +func newSchema(opts Options) (graphql.ExecutableSchema, error) { if opts.Reader == nil { return nil, fmt.Errorf("graph: Reader is required") } @@ -99,15 +134,23 @@ reader: opts.Reader, searcher: opts.Searcher, proposals: opts.Proposals, } - return api.NewExecutableSchema(api.Config{Resolvers: root}), nil + schema := api.NewExecutableSchema(api.Config{Resolvers: root}) + // The root resolver holds the schema it is part of. The knot is deliberate: + // the webhook resolvers validate and execute a subscriber's stored query + // against this service's schema, and used to reach it through core-go's + // server context — which exists on the authenticated router and nowhere + // else. Holding it here is what lets /query move to the anonymous router + // without those resolvers reaching for a context value that is not there. + root.schema = schema + return schema, nil } -// New assembles the executable schema over the seams in opts. +// New assembles the /query endpoint over the seams in opts. func New(opts Options) (*Server, error) { if opts.Resolver == nil { return nil, fmt.Errorf("graph: authn Resolver is required") } - schema, err := NewSchema(opts) + schema, err := newSchema(opts) if err != nil { return nil, err } @@ -123,37 +166,107 @@ exec := handler.New(schema) exec.AddTransport(transport.POST{}) exec.Use(extension.Introspection{}) - return &Server{exec: exec, resolver: opts.Resolver}, nil + return &Server{ + schema: schema, + http: resolveCaller(opts.Resolver, gate(coreContext(exec))), + }, nil } -// Handler is the /query handler with authn's principal middleware installed, so -// it can be mounted on a router that has none: +// Schema is the executable schema this endpoint serves. The daemon hands it to +// webhooks.NewQueue so that the query a subscriber stored is executed against +// exactly the schema they wrote it for. +func (s *Server) Schema() graphql.ExecutableSchema { return s.schema } + +// ServeHTTP serves /query behind the chain New built. +func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { s.http.ServeHTTP(w, r) } + +// resolveCaller turns the presented bearer credential into this request's +// principal, or refuses the request. It is the whole credential plane of this +// endpoint, and it is authn.Resolver's bearer arm and not its Middleware: +// Middleware also reads the unified-login cookie, and a cookie is not a +// credential here. +// +// A request with no Authorization header is given the anonymous principal +// explicitly rather than being passed through untouched. That overwrite is the +// "no cookie" rule made structural: mounted under a router that already +// resolved a cookie identity, this endpoint still sees anonymous and still +// answers 401. // -// router.Handle("/query", gql.Handler()) +// The statuses are authn.StatusFor's, which is the one table this service maps +// a credential failure with: 401 for a credential that does not verify — forged, +// expired, revoked, or issued by somebody else — 403 for one that verifies and +// belongs to a human this single-owner instance has nothing to grant, and 503 +// for a credential that could not be *checked*. The last is not "your token is +// bad": answering 401 to a restart of tokens.sr.ht tells every agent to re-mint +// credentials that were never broken. // -// Installing that middleware twice is harmless — it is idempotent — so a router -// that already applies it may use this too. -func (s *Server) Handler() http.Handler { - return s.resolver.Middleware()(s.Endpoint()) +// The messages are written here from what the caller already knows, never from +// the error's own text: authn's errors name usernames and token ids. +func resolveCaller(rs *authn.Resolver, next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + presented := authn.BearerFromRequest(r) + if presented == "" { + next.ServeHTTP(w, r.WithContext( + authn.WithPrincipal(r.Context(), authn.Anonymous()))) + return + } + + p, err := rs.ResolveAgent(r.Context(), presented, + r.Header.Get(authn.HeaderAgent), r.Header.Get(authn.HeaderAgentSession)) + if err != nil { + status := authn.StatusFor(err) + if status >= http.StatusInternalServerError { + // Fail closed and loudly. The alternative — degrading to + // anonymous — would turn an unreachable tokens.sr.ht into every + // agent silently losing its read access. + slog.ErrorContext(r.Context(), "a bearer credential could not be checked", + "component", "graph", "path", r.URL.Path, "status", status, scribe.Err(err)) + } + if status == http.StatusUnauthorized { + // RFC 9110 requires the challenge on a 401, and every caller + // here is a machine holding a bearer token: naming the scheme + // and the realm is what tells it which credential was refused. + w.Header().Set("WWW-Authenticate", authn.Challenge()) + } + http.Error(w, refusalMessage(status), status) + return + } + next.ServeHTTP(w, r.WithContext(authn.WithPrincipal(r.Context(), p))) + }) } -// Endpoint is the /query handler without any middleware of its own. The router -// it is mounted on must already resolve a principal into the request context -// (authn.Resolver.Middleware), or every caller looks anonymous and is refused. -func (s *Server) Endpoint() http.Handler { - return gate(s.exec) +// refusalMessage is what a refused caller is told. It is keyed on the status and +// not on the error, so that nothing about whose token it was, or whether a row +// exists, leaks to a caller holding a credential this service did not accept. +func refusalMessage(status int) string { + switch status { + case http.StatusUnauthorized: + return "the bearer token presented was refused" + case http.StatusForbidden: + return "this token does not authorize requests to " + authn.ConfigSection + default: + return "the credential could not be verified, try again" + } } // gate refuses a caller with no read authority before the query is parsed. // // The ACL is authn.Principal.CanRead — the owner and its agents may read and // nobody else may — the same predicate web/ and mcpsrv/ apply, so the three read -// surfaces cannot drift into three policies, which is how a corpus leaks. +// surfaces cannot drift into three policies, which is how a corpus leaks. On +// this endpoint the owner half of it is unreachable in practice: the owner is +// recognised by a cookie, and resolveCaller above accepts none. It is asked +// anyway because it is the shared predicate and not this endpoint's own. // // A caller that clears it and authenticated with a tokens.sr.ht working token // must also hold spec:read — the grant half of the same question, asked here -// because here is where the action ("read") is known. It is a no-op for the -// owner's cookie and for the local agent token, neither of which carries grants. +// because here is where the action ("read") is known. +// +// It is one check at the boundary rather than one per field, because every read +// field of this schema is a read and the surface has one action. The webhook +// mutations must NOT rely on it: they would be admitted by a read grant, which +// is not what a read grant says, so they carry their own owner gate in the +// resolver. // // The refusal is a 401 — or a 403 for the missing grant — with a line of text // and never a redirect to meta's login: every caller here is a machine, and @@ -177,3 +290,18 @@ } next.ServeHTTP(w, r) }) } + +// coreContext derives core-go's AuthContext from the principal the gate has +// already admitted, because core-go's webhook engine reads one out of the +// context and this endpoint no longer runs behind the middleware that puts it +// there. +// +// The user id is the credential's own: authn resolved the token's owner to a +// local row and refused the token outright if that row had no id, so there is +// nothing to substitute and no default to invent here. +func coreContext(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + p := authn.PrincipalFromContext(r.Context()) + next.ServeHTTP(w, r.WithContext(coreauth.Context(r.Context(), p, p.UserID))) + }) +}