fix(ingestion): close the PR #152 review findings on REST auth
- main.go's wiring was untested (the test built its own copy of the router, so dropping auth in main or re-registering POST /query as public stayed green). Routing is now one function, buildMux, which main calls and the tests call: it constructs the bearer middleware itself, registers public handlers only for patterns on a fixed allowlist (a public REST pattern would beat the authenticated catch-all by specificity), refuses an empty token, and returns an http.Handler so nothing can add a route to the result. - hyperguild CLI: PassRate sent no token and would 401. Every request now goes through brainClient.do, which adds it; the test covers Query, Write and PassRate and is named for that. - ResolveIngestPath returns one refusal for every bad path; the raw EvalSymlinks error let a caller probe which server files exist and leaked the brain directory. - PROJECT.md (and derived adapters) and the pass-rate runbook state that the REST API needs BRAIN_MCP_TOKEN and is tailnet/in-cluster. Refs #149 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -503,9 +503,10 @@ func main() {
|
||||
logger.Info("embed sync started", "interval_s", embedSyncInterval)
|
||||
}
|
||||
|
||||
mux := http.NewServeMux()
|
||||
// Public routes only; buildMux mounts the authenticated REST API (#149).
|
||||
public := map[string]http.Handler{}
|
||||
if webhookHandler != nil {
|
||||
mux.Handle("POST /webhooks/brain-sync", webhookHandler)
|
||||
public["POST /webhooks/brain-sync"] = webhookHandler
|
||||
}
|
||||
// Resource-metadata URL is only emitted on 401 when Dex OAuth is
|
||||
// configured. Static-Bearer-only deployments leave this empty so
|
||||
@@ -513,21 +514,14 @@ func main() {
|
||||
var resourceMetadataURL string
|
||||
if dexURL := os.Getenv("DEX_ISSUER_URL"); dexURL != "" {
|
||||
resourceURL := os.Getenv("MCP_RESOURCE_URL")
|
||||
mux.HandleFunc("GET /.well-known/oauth-protected-resource",
|
||||
public["GET /.well-known/oauth-protected-resource"] = http.HandlerFunc(
|
||||
chassisauth.ProtectedResourceHandler(resourceURL, dexURL))
|
||||
if resourceURL != "" {
|
||||
resourceMetadataURL = strings.TrimRight(resourceURL, "/") + "/.well-known/oauth-protected-resource"
|
||||
}
|
||||
}
|
||||
|
||||
mux.Handle("/mcp", chassisauth.BearerMiddleware(mcpToken, jwtValidator, "brain", resourceMetadataURL, mcpSrv))
|
||||
// The REST API, authenticated as a whole (hyperguild#149). Same credential
|
||||
// as /mcp: callers (brain-ui, brain-ingest-ntfy, the hyperguild CLI) send
|
||||
// BRAIN_MCP_TOKEN. Registered after the public routes on purpose only for
|
||||
// reading order; ServeMux precedence is by pattern specificity.
|
||||
mountREST(mux, h, func(next http.Handler) http.Handler {
|
||||
return chassisauth.BearerMiddleware(mcpToken, jwtValidator, "brain", resourceMetadataURL, next)
|
||||
})
|
||||
public["/mcp"] = chassisauth.BearerMiddleware(mcpToken, jwtValidator, "brain", resourceMetadataURL, mcpSrv)
|
||||
|
||||
// POST /capture (#53/#54): the uniform capture REST door. Needs a ticket
|
||||
// tracker to file action items, so it only mounts when Gitea is
|
||||
@@ -549,7 +543,7 @@ func main() {
|
||||
sovereign := splitList(os.Getenv("BRAIN_CAPTURE_SOVEREIGN_PRINCIPALS"))
|
||||
resolver := capturehttp.NewOriginResolver(sovereign)
|
||||
captureH := capturehttp.New(captureSvc, jwtValidator, mcpToken, "local-cli", resolver)
|
||||
mux.Handle("POST /capture", captureH)
|
||||
public["POST /capture"] = captureH
|
||||
// Same use-case behind the MCP `capture` tool (#55 relay) so MCP-native
|
||||
// harnesses (claude.ai, Crush, Pi, LLM Council) reach capture through
|
||||
// the existing /mcp OAuth connector. mcpSrv is already wrapped above;
|
||||
@@ -579,9 +573,9 @@ func main() {
|
||||
logger.Error("OAUTH_CLIENT_ID/SECRET set but MCP_RESOURCE_URL is empty; cannot derive issuer")
|
||||
os.Exit(1)
|
||||
}
|
||||
mux.HandleFunc("GET /.well-known/oauth-authorization-server",
|
||||
public["GET /.well-known/oauth-authorization-server"] = http.HandlerFunc(
|
||||
oauth.MetadataHandler(issuer))
|
||||
mux.HandleFunc("POST /oauth/token", oauth.TokenHandler(oauth.TokenConfig{
|
||||
public["POST /oauth/token"] = http.HandlerFunc(oauth.TokenHandler(oauth.TokenConfig{
|
||||
ClientID: oauthID,
|
||||
ClientSecret: oauthSecret,
|
||||
AccessToken: mcpToken,
|
||||
@@ -600,7 +594,17 @@ func main() {
|
||||
// real request latency. /metrics itself is excluded from its own
|
||||
// observation by registering it on the outer mux (post-wrap).
|
||||
reg := metrics.New()
|
||||
mux.HandleFunc("GET /metrics", reg.Handler())
|
||||
public["GET /metrics"] = http.HandlerFunc(reg.Handler())
|
||||
|
||||
// The whole router, built in one place so the test of buildMux is a test
|
||||
// of production wiring (PR #152 review). The REST API's credential is the
|
||||
// same as /mcp's.
|
||||
mux, err := buildMux(restAuth{token: mcpToken, validator: jwtValidator, realm: "brain",
|
||||
resourceMetadataURL: resourceMetadataURL}, h, public)
|
||||
if err != nil {
|
||||
logger.Error("build router", "err", err)
|
||||
os.Exit(1)
|
||||
}
|
||||
logger.Info("metrics endpoint registered", "path", "/metrics")
|
||||
|
||||
addr := ":" + port
|
||||
|
||||
@@ -1,18 +1,65 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"net/http"
|
||||
|
||||
chassisauth "git.d-ma.be/mathias/mcp-chassis/auth"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
|
||||
)
|
||||
|
||||
// publicRoutes is the complete set of patterns allowed on the outer mux,
|
||||
// outside the REST API's auth (hyperguild#149). Each authenticates itself or is
|
||||
// meant to be open: /mcp (bearer/JWT), /capture (bearer/JWT), the webhook
|
||||
// (HMAC), /oauth/token (client secret), OAuth metadata and /metrics (open).
|
||||
// A REST pattern added here would beat the authenticated catch-all by
|
||||
// specificity and ship open, which is why buildMux refuses anything else.
|
||||
var publicRoutes = map[string]bool{
|
||||
"/mcp": true,
|
||||
"POST /capture": true,
|
||||
"POST /webhooks/brain-sync": true,
|
||||
"GET /.well-known/oauth-protected-resource": true,
|
||||
"GET /.well-known/oauth-authorization-server": true,
|
||||
"POST /oauth/token": true,
|
||||
"GET /metrics": true,
|
||||
}
|
||||
|
||||
// restAuth is the credential the REST API checks: the same as /mcp's.
|
||||
type restAuth struct {
|
||||
token string
|
||||
validator *chassisauth.JWTValidator
|
||||
realm string
|
||||
resourceMetadataURL string
|
||||
}
|
||||
|
||||
// buildMux is the server's whole router. The REST API is mounted at "/"
|
||||
// behind bearer auth that buildMux constructs itself, so a caller cannot pass
|
||||
// a no-op; public handlers are registered as given, and only for patterns on
|
||||
// the publicRoutes allowlist. It returns an http.Handler, not the
|
||||
// *ServeMux, so nothing can register a route on the result afterwards and
|
||||
// bypass the allowlist.
|
||||
func buildMux(auth restAuth, h *api.Handler, public map[string]http.Handler) (http.Handler, error) {
|
||||
if auth.token == "" {
|
||||
return nil, errors.New("buildMux: REST auth needs a token")
|
||||
}
|
||||
mux := http.NewServeMux()
|
||||
for pattern, handler := range public {
|
||||
if !publicRoutes[pattern] {
|
||||
return nil, fmt.Errorf("buildMux: %q is not an allowed public route", pattern)
|
||||
}
|
||||
mux.Handle(pattern, handler)
|
||||
}
|
||||
mountREST(mux, h, func(next http.Handler) http.Handler {
|
||||
return chassisauth.BearerMiddleware(auth.token, auth.validator, auth.realm, auth.resourceMetadataURL, next)
|
||||
})
|
||||
return mux, nil
|
||||
}
|
||||
|
||||
// mountREST registers the REST API on its own mux and mounts it at "/" behind
|
||||
// auth (hyperguild#149). Authentication is a property of the mux, not of each
|
||||
// route: anything not registered on the outer mux as explicitly public (/mcp,
|
||||
// /capture, the HMAC webhook, OAuth metadata, /metrics) lands here and is
|
||||
// authenticated, including routes added later. Before this, only /mcp was
|
||||
// wrapped, and the REST routes beside it were open; infra#517 found them on
|
||||
// the internet.
|
||||
// auth: anything not registered as public lands here and is authenticated,
|
||||
// including routes added later.
|
||||
func mountREST(mux *http.ServeMux, h *api.Handler, auth func(http.Handler) http.Handler) {
|
||||
rest := http.NewServeMux()
|
||||
rest.HandleFunc("POST /query", h.Query)
|
||||
|
||||
@@ -9,8 +9,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
|
||||
chassisauth "git.d-ma.be/mathias/mcp-chassis/auth"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/api"
|
||||
"github.com/mathiasbq/hyperguild/ingestion/internal/pipeline"
|
||||
@@ -18,22 +17,27 @@ import (
|
||||
|
||||
const routesTok = "routes-test-token"
|
||||
|
||||
// hyperguild#149: only /mcp was authenticated; every REST route on the same
|
||||
// mux was open, and infra#517 found them on the internet. The REST API is now
|
||||
// mounted at "/" behind the bearer middleware, so any route not registered
|
||||
// as explicitly public is authenticated, including routes added later.
|
||||
func testRouter(t *testing.T) http.Handler {
|
||||
func okHandler(body string) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte(body)) })
|
||||
}
|
||||
|
||||
// testMux builds the router exactly as main does: buildMux with the full
|
||||
// public set main registers (stub handlers) and real bearer config.
|
||||
// hyperguild#149: the REST API is authenticated as a whole; the PR #152
|
||||
// review showed a test of a COPY of the router let main.go regress unseen.
|
||||
func testMux(t *testing.T) http.Handler {
|
||||
t.Helper()
|
||||
h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
|
||||
mux := http.NewServeMux()
|
||||
mux.HandleFunc("GET /metrics", func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte("ok")) })
|
||||
mountREST(mux, h, func(next http.Handler) http.Handler {
|
||||
return chassisauth.BearerMiddleware(routesTok, nil, "brain", "", next)
|
||||
})
|
||||
public := map[string]http.Handler{}
|
||||
for p := range publicRoutes {
|
||||
public[p] = okHandler("public " + p)
|
||||
}
|
||||
mux, err := buildMux(restAuth{token: routesTok, realm: "brain"}, h, public)
|
||||
require.NoError(t, err)
|
||||
return mux
|
||||
}
|
||||
|
||||
func do(t *testing.T, h http.Handler, method, path, authz string) int {
|
||||
func do(t *testing.T, h http.Handler, method, path, authz string) (int, string) {
|
||||
t.Helper()
|
||||
req := httptest.NewRequest(method, path, strings.NewReader(`{"query":"x"}`))
|
||||
if authz != "" {
|
||||
@@ -41,34 +45,62 @@ func do(t *testing.T, h http.Handler, method, path, authz string) int {
|
||||
}
|
||||
rec := httptest.NewRecorder()
|
||||
h.ServeHTTP(rec, req)
|
||||
return rec.Code
|
||||
return rec.Code, rec.Body.String()
|
||||
}
|
||||
|
||||
func TestRESTRoutesRefuseRequestsWithoutAToken(t *testing.T) {
|
||||
r := testRouter(t)
|
||||
func TestBuildMuxRefusesRESTRoutesWithoutAValidToken(t *testing.T) {
|
||||
r := testMux(t)
|
||||
for _, rt := range []struct{ method, path string }{
|
||||
{"POST", "/query"}, {"POST", "/write"}, {"POST", "/index"}, {"POST", "/ingest"},
|
||||
{"POST", "/ingest-path"}, {"POST", "/ingest-raw"}, {"POST", "/backfill-refs"},
|
||||
{"GET", "/pending"}, {"POST", "/promote"}, {"POST", "/backfill-embeddings"}, {"GET", "/pass-rate"},
|
||||
} {
|
||||
assert.Equal(t, http.StatusUnauthorized, do(t, r, rt.method, rt.path, ""), "%s %s without a token", rt.method, rt.path)
|
||||
assert.Equal(t, http.StatusUnauthorized, do(t, r, rt.method, rt.path, "Bearer wrong"), "%s %s with a wrong token", rt.method, rt.path)
|
||||
code, _ := do(t, r, rt.method, rt.path, "")
|
||||
assert.Equal(t, http.StatusUnauthorized, code, "%s %s without a token", rt.method, rt.path)
|
||||
code, _ = do(t, r, rt.method, rt.path, "Bearer wrong")
|
||||
assert.Equal(t, http.StatusUnauthorized, code, "%s %s with a wrong token", rt.method, rt.path)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRESTRoutesServeAValidToken(t *testing.T) {
|
||||
r := testRouter(t)
|
||||
assert.NotEqual(t, http.StatusUnauthorized, do(t, r, "POST", "/query", "Bearer "+routesTok))
|
||||
assert.Equal(t, http.StatusOK, do(t, r, "GET", "/pending", "Bearer "+routesTok))
|
||||
func TestBuildMuxServesRESTRoutesToAValidToken(t *testing.T) {
|
||||
r := testMux(t)
|
||||
code, _ := do(t, r, "GET", "/pending", "Bearer "+routesTok)
|
||||
assert.Equal(t, http.StatusOK, code)
|
||||
}
|
||||
|
||||
// Default-deny: a path nobody registered is still behind auth, so a route
|
||||
// added to the REST mux later cannot ship open by omission.
|
||||
func TestUnregisteredPathIsAuthenticatedByDefault(t *testing.T) {
|
||||
assert.Equal(t, http.StatusUnauthorized, do(t, testRouter(t), "POST", "/some-future-route", ""))
|
||||
func TestBuildMuxAuthenticatesAnUnregisteredPath(t *testing.T) {
|
||||
code, _ := do(t, testMux(t), "POST", "/some-future-route", "")
|
||||
assert.Equal(t, http.StatusUnauthorized, code)
|
||||
}
|
||||
|
||||
// Explicitly public routes registered on the outer mux keep working.
|
||||
func TestMetricsStaysOpenForPrometheus(t *testing.T) {
|
||||
assert.Equal(t, http.StatusOK, do(t, testRouter(t), "GET", "/metrics", ""))
|
||||
// buildMux registers public handlers as given, without adding auth: each
|
||||
// public route owns its own (bearer on /mcp, HMAC on the webhook, client
|
||||
// secret on /oauth/token) or is meant to be open (/metrics, OAuth metadata).
|
||||
func TestBuildMuxDoesNotWrapPublicRoutes(t *testing.T) {
|
||||
r := testMux(t)
|
||||
for p := range publicRoutes {
|
||||
method, path, ok := strings.Cut(p, " ")
|
||||
if !ok {
|
||||
method, path = "GET", p
|
||||
}
|
||||
code, body := do(t, r, method, path, "")
|
||||
assert.Equal(t, http.StatusOK, code, p)
|
||||
assert.Equal(t, "public "+p, body, p)
|
||||
}
|
||||
}
|
||||
|
||||
// The allowlist is the control: a REST pattern registered as public would win
|
||||
// over the authenticated catch-all (more specific pattern), so it is refused.
|
||||
func TestBuildMuxRefusesAPublicPatternOffTheAllowlist(t *testing.T) {
|
||||
h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
|
||||
for _, p := range []string{"POST /query", "GET /pending", "/", "POST /metrics"} {
|
||||
_, err := buildMux(restAuth{token: routesTok, realm: "brain"}, h, map[string]http.Handler{p: okHandler("x")})
|
||||
assert.Error(t, err, "public pattern %q must be refused", p)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildMuxRefusesAnEmptyToken(t *testing.T) {
|
||||
h := api.NewHandler(t.TempDir(), slog.New(slog.NewTextHandler(os.Stderr, nil)), pipeline.Config{})
|
||||
_, err := buildMux(restAuth{token: "", realm: "brain"}, h, nil)
|
||||
assert.Error(t, err)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user