From d2ee26cccd70dc117260ce166a0f45ed7136efcf Mon Sep 17 00:00:00 2001 From: xarmian Date: Fri, 2 Oct 2026 21:50:05 +0000 Subject: [PATCH] fix(auth): platform administration refuses an API token (BUG-3361) BREAKING for anyone driving /api/v1/admin/* or admin account creation with a personal access token: those now answer 403 session_required. Use a `pad auth login` CLI session, or on self-host set PAD_ADMIN_API_TOKENS=allow. An admin's PAT carried full platform authority. codex reproduced the chain during BUG-3336's review: register an account with a chosen password, promote it to admin, sign in as it, and reset any account's password. A leaked admin token therefore escalated to interactive sessions for any account. - refuseAPITokenForAdmin guards the whole /api/v1/admin route group, so an admin route added later is refused by construction. TestBUG3361_EveryAdminRouteRefusesAnAdminPAT walks the router to hold it. - handleRegister refuses admin-create signup with an API token. - PAD_ADMIN_API_TOKENS=allow is the self-host escape hatch. It is read at startup, logs a WARN, and is never honoured in cloud mode. Documented in docs/deployment.md, Security. - A browser session and a CLI session (padsess_) are unaffected. So is the Pad Cloud sidecar, which authenticates with the cloud secret, never a PAT. /audit-log already refused a PAT (BUG-3334). Claude-Session: https://claude.ai/code/session_01H63zaWhkb5pAafx2WGjrVC --- docs/deployment.md | 1 + internal/server/bug3361_admin_pat_test.go | 110 ++++++++++++++++++++++ internal/server/handlers_auth.go | 6 ++ internal/server/server.go | 48 +++++++++- 4 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 internal/server/bug3361_admin_pat_test.go diff --git a/docs/deployment.md b/docs/deployment.md index 5848af68f..b3822d7f3 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -1096,6 +1096,7 @@ degraded, not a peer broken. |----------|---------|-------------| | `PAD_SECURE_COOKIES` | `false` | Set `Secure` flag on session cookies (requires TLS) | | `PAD_CORS_ORIGINS` | — | Comma-separated allowed CORS origins | +| `PAD_ADMIN_API_TOKENS` | off | `allow` lets an admin's API token (PAT) use the platform administration API (`/api/v1/admin/*`, and creating accounts). By default these require an interactive session, a browser or `pad auth login`, because a leaked admin token could otherwise create and promote accounts. For operators who script admin tasks; logs a warning at startup; ignored on Pad Cloud. | ### MCP for agents (self-hosted) diff --git a/internal/server/bug3361_admin_pat_test.go b/internal/server/bug3361_admin_pat_test.go new file mode 100644 index 000000000..8f6672a9e --- /dev/null +++ b/internal/server/bug3361_admin_pat_test.go @@ -0,0 +1,110 @@ +package server + +import ( + "net/http" + "regexp" + "strings" + "testing" + + "github.com/go-chi/chi/v5" + + "github.com/PerpetualSoftware/pad/internal/models" +) + +// BUG-3361: platform administration requires an interactive session. An +// admin's API token is refused on every /api/v1/admin route and on +// admin-create signup, unless a self-hosted operator opted in with +// PAD_ADMIN_API_TOKENS=allow (never honoured on Pad Cloud). + +type adminPATFixture struct { + srv *Server + pat string + session string +} + +func newAdminPATFixture(t *testing.T) adminPATFixture { + t.Helper() + srv := testServer(t) + admin, err := srv.store.CreateUser(models.UserCreate{Email: "admin-3361@example.com", Name: "Admin", Password: "correct-horse-battery-staple", Role: "admin"}) + if err != nil { + t.Fatal(err) + } + ws, err := srv.store.CreateWorkspace(models.WorkspaceCreate{Name: "Admin WS", OwnerID: admin.ID}) + if err != nil { + t.Fatal(err) + } + pat, err := srv.store.CreateAPIToken(admin.ID, models.APITokenCreate{Name: "admin-pat", WorkspaceID: ws.ID}, 0, 0) + if err != nil { + t.Fatal(err) + } + return adminPATFixture{srv: srv, pat: pat.Token, session: loginUser(t, srv, "admin-3361@example.com", "correct-horse-battery-staple")} +} + +// Every route the router registers under /api/v1/admin refuses an admin's +// PAT, so an admin route added later is refused by construction and by test. +func TestBUG3361_EveryAdminRouteRefusesAnAdminPAT(t *testing.T) { + t.Setenv("PAD_DISABLE_RATE_LIMITS", "1") + f := newAdminPATFixture(t) + f.srv.ensureRouter() + param := regexp.MustCompile(`\{[^}]*\}`) + checked := 0 + err := chi.Walk(f.srv.router, func(method, route string, _ http.Handler, _ ...func(http.Handler) http.Handler) error { + if !strings.HasPrefix(route, "/api/v1/admin/") { + return nil + } + path := param.ReplaceAllString(route, "x") + checked++ + rr := doRequestWithBearer(f.srv, method, path, f.pat, nil) + if rr.Code != http.StatusForbidden || !strings.Contains(rr.Body.String(), "session_required") { + t.Errorf("admin PAT %s %s: %d %s, want 403 session_required", method, route, rr.Code, rr.Body.String()) + } + return nil + }) + if err != nil { + t.Fatal(err) + } + if checked < 20 { + t.Fatalf("only %d admin routes were walked; the walk is not seeing the router", checked) + } + // The admin's own sessions still administer. + if rr := doRequestWithBearer(f.srv, "GET", "/api/v1/admin/settings", f.session, nil); rr.Code != http.StatusOK { + t.Fatalf("admin CLI session on /admin/settings: %d %s", rr.Code, rr.Body.String()) + } + if rr := doRequestWithCookie(f.srv, "GET", "/api/v1/admin/settings", nil, f.session); rr.Code != http.StatusOK { + t.Fatalf("admin browser session on /admin/settings: %d %s", rr.Code, rr.Body.String()) + } +} + +// Admin-create signup is administration too: with a PAT it was the first +// step of minting a second admin. +func TestBUG3361_AdminCreateSignupRefusesAnAdminPAT(t *testing.T) { + f := newAdminPATFixture(t) + body := map[string]any{"email": "new-3361@example.com", "name": "New", "password": "a-long-enough-passphrase-3361"} + rr := doRequestWithBearer(f.srv, "POST", "/api/v1/auth/register", f.pat, body) + if rr.Code != http.StatusForbidden || errorCode(t, rr) != "session_required" { + t.Fatalf("admin PAT creating an account: %d %s, want 403 session_required", rr.Code, rr.Body.String()) + } + if u, _ := f.srv.store.GetUserByEmail("new-3361@example.com"); u != nil { + t.Fatal("the refused signup created the account") + } + if rr := doRequestWithBearer(f.srv, "POST", "/api/v1/auth/register", f.session, body); rr.Code != http.StatusCreated { + t.Fatalf("admin CLI session creating an account: %d %s, want 201", rr.Code, rr.Body.String()) + } +} + +// The self-host escape hatch: PAD_ADMIN_API_TOKENS=allow lets an admin PAT +// administer, and is ignored on Pad Cloud. +func TestBUG3361_EscapeHatchIsSelfHostOnly(t *testing.T) { + t.Setenv("PAD_ADMIN_API_TOKENS", "allow") + f := newAdminPATFixture(t) + if !f.srv.adminAPITokensAllowed { + t.Fatal("PAD_ADMIN_API_TOKENS=allow was not read at startup") + } + if rr := doRequestWithBearer(f.srv, "GET", "/api/v1/admin/settings", f.pat, nil); rr.Code != http.StatusOK { + t.Fatalf("admin PAT with the escape hatch on: %d %s, want 200", rr.Code, rr.Body.String()) + } + f.srv.cloudMode = true + if rr := doRequestWithBearer(f.srv, "GET", "/api/v1/admin/settings", f.pat, nil); rr.Code != http.StatusForbidden { + t.Fatalf("admin PAT with the escape hatch on, in cloud mode: %d, want 403", rr.Code) + } +} diff --git a/internal/server/handlers_auth.go b/internal/server/handlers_auth.go index f4992b786..8652e3500 100644 --- a/internal/server/handlers_auth.go +++ b/internal/server/handlers_auth.go @@ -650,6 +650,12 @@ func (s *Server) handleRegister(w http.ResponseWriter, r *http.Request) { reqUser := currentUser(r) isAdmin := reqUser != nil && reqUser.Role == "admin" switch { + case isAdmin && isAPITokenAuth(r) && !s.adminAcceptsAPITokens(): + // Creating an account is platform administration (BUG-3361): + // with a PAT, it was the first step of minting a second admin. + writeError(w, http.StatusForbidden, "session_required", + "Creating an account requires an interactive session, not an API token") + return case isAdmin: // Admin-created account — stays verified. adminCreated = true diff --git a/internal/server/server.go b/internal/server/server.go index 01c1128ba..9e948d8b5 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -72,6 +72,10 @@ type Server struct { // load-bearing, not incidental (codex round 3). afterItemPreRead func(itemID string) + // adminAPITokensAllowed is PAD_ADMIN_API_TOKENS=allow, read at startup + // (BUG-3361). See adminAcceptsAPITokens. + adminAPITokensAllowed bool + // visibleCollectionIDsFault is a TEST-ONLY seam, nil in production // (BUG-3334). When set, visibleCollectionIDs calls it first and returns // its error. Every handler that resolves visibility through the helper @@ -654,12 +658,45 @@ func New(s *store.Store) *Server { if disabled, _ := strconv.ParseBool(os.Getenv("PAD_DISABLE_RATE_LIMITS")); disabled { rl = nil } - return &Server{ - store: s, - rateLimiters: rl, - storageInfoCache: newStorageInfoCache(storageInfoTTL), - clientWrites: newClientWriteMarks(), + // PAD_ADMIN_API_TOKENS=allow is the self-host escape hatch for operators + // who script platform administration with a PAT (BUG-3361). Off by + // default, never honoured in cloud mode, and loud when on. + adminTokens := os.Getenv("PAD_ADMIN_API_TOKENS") == "allow" + if adminTokens { + slog.Warn("PAD_ADMIN_API_TOKENS=allow: platform administration accepts API tokens. " + + "A leaked admin token can then administer the instance; prefer a CLI session (`pad auth login`). " + + "Ignored in cloud mode.") } + return &Server{ + store: s, + rateLimiters: rl, + storageInfoCache: newStorageInfoCache(storageInfoTTL), + clientWrites: newClientWriteMarks(), + adminAPITokensAllowed: adminTokens, + } +} + +// adminAcceptsAPITokens reports whether platform administration accepts an +// API token: only when the operator opted in, and never on Pad Cloud. +func (s *Server) adminAcceptsAPITokens() bool { + return s.adminAPITokensAllowed && !s.cloudMode +} + +// refuseAPITokenForAdmin guards every /api/v1/admin route (BUG-3361). An +// admin's PAT carries their identity, not their platform authority: one used +// to be enough to register an account, promote it to admin, sign in as it, +// and reset any password. A browser or `pad auth login` CLI session is +// required, the line BUG-2890 drew for token minting. The cloud sidecar +// authenticates with the cloud secret, never a PAT, so it is unaffected. +func (s *Server) refuseAPITokenForAdmin(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if isAPITokenAuth(r) && !s.adminAcceptsAPITokens() { + writeError(w, http.StatusForbidden, "session_required", + "Platform administration requires an interactive session, not an API token") + return + } + next.ServeHTTP(w, r) + }) } // Init2FASecret loads the 2FA challenge signing key from platform_settings. @@ -1664,6 +1701,7 @@ func (s *Server) setupRouter() { // Admin endpoints (admin-only, handlers check role internally) r.Route("/admin", func(r chi.Router) { + r.Use(s.refuseAPITokenForAdmin) r.Get("/settings", s.handleGetPlatformSettings) r.Patch("/settings", s.handleUpdatePlatformSettings) r.Get("/decision-provider", s.handleGetDecisionSettings)