diff --git a/cmd/pad/task3866_read_only_key_test.go b/cmd/pad/task3866_read_only_key_test.go new file mode 100644 index 000000000..3288a5e9f --- /dev/null +++ b/cmd/pad/task3866_read_only_key_test.go @@ -0,0 +1,136 @@ +package main + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +// TASK-3866 done-when: a key made read-only through the same request the +// web settings form sends can read and cannot write, both over REST and +// over remote MCP. The MCP leg runs through wireMCP, the production +// wiring, so the catalog, transport and dispatcher are the shipped ones. +func TestTASK3866_ReadOnlyKeyRefusesWritesOverRESTAndMCP(t *testing.T) { + f := newCapFixture(t, cfgHTTP, false, nil, "true") + if err := f.store.SeedCollectionsFromTemplate(f.ws.ID, "startup"); err != nil { + t.Fatal(err) + } + session, err := f.store.CreateSession(f.member.ID, "web", "192.0.2.10", "", time.Hour) + if err != nil { + t.Fatal(err) + } + + do := func(method, path, bearer string, body any, header map[string]string) *httptest.ResponseRecorder { + t.Helper() + var buf bytes.Buffer + if body != nil { + if err := json.NewEncoder(&buf).Encode(body); err != nil { + t.Fatal(err) + } + } + req := httptest.NewRequest(method, path, &buf) + req.Host = f.host + req.RemoteAddr = "192.0.2.10:4242" + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+bearer) + for k, v := range header { + req.Header.Set(k, v) + } + rr := newMCPRecorder() + f.srv.ServeHTTP(rr, req) + return rr.ResponseRecorder + } + mint := func(name, scopes string) string { + t.Helper() + rr := do("POST", "/api/v1/auth/tokens", session, map[string]any{"name": name, "scopes": scopes}, nil) + if rr.Code != http.StatusCreated { + t.Fatalf("mint %s: %d %s", name, rr.Code, rr.Body.String()) + } + var tok struct { + Token string `json:"token"` + Scopes string `json:"scopes"` + } + if err := json.Unmarshal(rr.Body.Bytes(), &tok); err != nil || tok.Token == "" { + t.Fatalf("mint %s: %s", name, rr.Body.String()) + } + if tok.Scopes != scopes { + t.Fatalf("mint %s: stored %q, want %q", name, tok.Scopes, scopes) + } + return tok.Token + } + readKey := mint("bot read", `["read"]`) + writeKey := mint("bot write", `["*"]`) + + itemsPath := "/api/v1/workspaces/" + f.ws.Slug + "/collections/tasks/items" + countTitled := func(title string) int { + t.Helper() + rr := do("GET", itemsPath, writeKey, nil, nil) + if rr.Code != http.StatusOK { + t.Fatalf("list: %d %s", rr.Code, rr.Body.String()) + } + return strings.Count(rr.Body.String(), `"title":`+mustJSON(t, title)) + } + + t.Run("REST", func(t *testing.T) { + if rr := do("GET", itemsPath, readKey, nil, nil); rr.Code != http.StatusOK { + t.Errorf("read key, GET items: %d %s", rr.Code, rr.Body.String()) + } + rr := do("POST", itemsPath, readKey, map[string]any{"title": "rest by read key"}, nil) + if rr.Code != http.StatusForbidden { + t.Errorf("read key, POST item: %d %s, want 403", rr.Code, rr.Body.String()) + } + if n := countTitled("rest by read key"); n != 0 { + t.Errorf("the refused REST write created %d items", n) + } + if rr := do("POST", itemsPath, writeKey, map[string]any{"title": "rest by write key"}, nil); rr.Code != http.StatusCreated { + t.Errorf("write key, POST item: %d %s, want 201", rr.Code, rr.Body.String()) + } + }) + + t.Run("MCP", func(t *testing.T) { + const proto = "2025-11-25" + mcpCall := func(key string, id int, method string, params map[string]any) string { + t.Helper() + body := map[string]any{"jsonrpc": "2.0", "id": id, "method": method, "params": params} + rr := do("POST", "/mcp", key, body, map[string]string{ + "Accept": "application/json, text/event-stream", + "MCP-Protocol-Version": proto, + }) + if rr.Code != http.StatusOK { + t.Fatalf("%s: %d %s", method, rr.Code, rr.Body.String()) + } + return rr.Body.String() + } + tool := func(key, action string, args map[string]any) string { + t.Helper() + args["action"] = action + args["workspace"] = f.ws.Slug + mcpCall(key, 1, "initialize", map[string]any{ + "protocolVersion": proto, "capabilities": map[string]any{}, + "clientInfo": map[string]any{"name": "task3866", "version": "1"}, + }) + return mcpCall(key, 2, "tools/call", map[string]any{"name": "pad_item", "arguments": args}) + } + + if out := tool(readKey, "list", map[string]any{"collection": "tasks"}); strings.Contains(out, `"isError":true`) { + t.Errorf("read key, pad_item list: %s", out) + } + out := tool(readKey, "create", map[string]any{"collection": "tasks", "title": "mcp by read key"}) + if !strings.Contains(out, `"isError":true`) || !strings.Contains(out, "permission_denied") { + t.Errorf("read key, pad_item create: want an isError permission_denied result, got %s", out) + } + if n := countTitled("mcp by read key"); n != 0 { + t.Errorf("the refused MCP write created %d items", n) + } + if out := tool(writeKey, "create", map[string]any{"collection": "tasks", "title": "mcp by write key"}); strings.Contains(out, `"isError":true`) { + t.Errorf("write key, pad_item create: %s", out) + } + if n := countTitled("mcp by write key"); n != 1 { + t.Errorf("write key over MCP created %d items, want 1", n) + } + }) +} diff --git a/docs/api.md b/docs/api.md index c998372ed..a3f5ab42d 100644 --- a/docs/api.md +++ b/docs/api.md @@ -121,6 +121,7 @@ One browser tab's content writes to an item are ordered by the server, also **pe - `POST /api/v1/auth/cli/sessions`, `GET /api/v1/auth/cli/sessions/{code}`, `POST /api/v1/auth/cli/sessions/{code}/approve`, `POST /api/v1/auth/cli/sessions/{code}/deny` — browser sign-in for `pad auth login`: the CLI creates a session and polls it, and a signed-in user approves it at `/auth/cli/{code}`, which mints a 30-day session the poll hands to the CLI. Since TASK-2253 the session records `requester_ip` (the server's view of the creating client) and `requester_user_agent` (what the creator sent, capped at 200 characters, so requester-controlled text), and a PENDING poll returns them with `created_at` (additive, omitempty) for the approval page, which renders the agent as text labelled "reported by the requester". `deny` checks no account, because the code is the authority (it is already a bearer secret: whoever holds it can poll and collect an approval's token); it moves pending → denied, answers 200 again on a denied session, and 409 `already_approved` / 410 `expired` otherwise. A denied session POLLS as **410 `cli_auth_denied`**, not as a 200 status: a CLI from before Deny has no case for an unknown status and would poll it for 20 minutes, whereas a non-2xx counts as a poll error there and it stops after five in a row, printing the message; a current CLI stops on the first. Approving a denied session answers 409 `cli_auth_denied`, and an approval that loses to a concurrent deny ends the session it minted - `POST /api/v1/auth/local-reset` — localhost-only account recovery (self-host, non-cloud). Loopback-gated, no auth — the bootstrap trust model. Returns a single-use reset link, or a temp password with `{"temp_password": true}`. Backs `pad auth reset-password`. - `GET/POST/DELETE /api/v1/auth/tokens` — user-scoped API tokens. **Minting and rotating require an INTERACTIVE SESSION** (BUG-2890): a call authenticated by a PAT is refused `403 session_required` on `POST /auth/tokens`, `POST /auth/tokens/{id}/rotate` and `POST /workspaces/{ws}/tokens`, because a token that can mint tokens outlives its own revocation. A session cookie and a `padsess_` CLI bearer both count as interactive; LIST and REVOKE stay PAT-reachable, deliberately — revocation is the compromised-credential response + - **`scopes` on create** (TASK-3866), on both `POST /auth/tokens` and `POST /workspaces/{ws}/tokens`: a STRING holding a JSON array of `*`, `read`, `write`, `pad:read`, `pad:write`, `pad:admin` (exactly what the request-time check `tokenScopeAllows` recognizes). It is stored compact (`[ "read" ]` → `["read"]`). Only an omitted field keeps the default `["*"]`. Refused `400 validation_error`, with the message naming the bad value: a `scopes` field that is JSON `null` or not a string (an explicit null used to read as omitted and mint a full-access key), the empty string and the empty array (`""` and `[]` are the legacy unrestricted forms, and no new key is minted in them; omit `scopes` instead), a string that is not a JSON array of strings (including `"null"`), and any value outside the vocabulary. These used to be stored verbatim, minting a key that every request then refused. There are two levels: `read`/`pad:read` allow GET, HEAD and OPTIONS (plus the playbook `run` POST), and every other recognized value allows everything the user's workspace role allows. `pad:admin` grants nothing beyond `write` for an API key. The web settings form offers "Read and write" (`["*"]`) and "Read only" (`["read"]`), and the token list labels each key from `web/src/lib/settings/tokenScopes.ts`, which is held to the server by `internal/server/testdata/token_scope_levels.json`. - `GET/PATCH /api/v1/admin/settings` — platform settings (admin-only) - `GET/PUT /api/v1/admin/mcp` — MCP over HTTP (PLAN-2310, admin-only). GET returns `{enabled, source: setting|environment|cloud, locked, readiness}`, where readiness is the resolved origin, MCP URL and issuer, `https`, `auth_methods` (`["oauth","pat"]` on https, `["pat"]` on http, `[]` with no usable origin), `state` (`on` / `off` / `blocked` with the reason) and `resume` counts (OAuth connections and PATs that work again when it is turned on: turning it off revokes nothing). PUT `{enabled}` stores `platform_settings.mcp_enabled` (read per request, no restart); `409 set_by_environment` when `PAD_MCP_ENABLED` forces it, `403 managed_by_operator` on Pad Cloud. MCP is available when cloud, or the setting is on with a usable origin (`PAD_URL` / toml `url` / `--url`, else `PUBLIC_URL`; no `http://host:port` fallback); the MCP URL is `PAD_MCP_PUBLIC_URL` else `/mcp`, the issuer `PAD_AUTH_SERVER_URL` else the origin, all startup-time. Operator docs: `docs/deployment.md` "MCP for agents (self-hosted)" - `GET/PUT /api/v1/admin/decision-provider` — decision provider setting (admin-only, TASK-3121): enable toggle, model pin, and a WRITE-ONLY API key stored encrypted (the response carries `api_key_set`, never the key or a mask of it). Resolved config file < this setting < environment, per field; a PUT to a field the environment sets is refused `409 set_by_environment`. A change rebuilds the provider in place, with no restart. On Pad Cloud the environment is the only source and PUT is `403 managed_by_operator`. `pad server info` resolves only the config file and environment on the CLI host, and says so diff --git a/internal/server/handlers_tokens.go b/internal/server/handlers_tokens.go index 5833afaae..d6099e201 100644 --- a/internal/server/handlers_tokens.go +++ b/internal/server/handlers_tokens.go @@ -2,9 +2,12 @@ package server import ( "database/sql" + "encoding/json" "fmt" "net/http" + "slices" "strconv" + "strings" "time" "github.com/go-chi/chi/v5" @@ -85,16 +88,20 @@ func (s *Server) handleCreateToken(w http.ResponseWriter, r *http.Request) { return } - var input models.APITokenCreate - if err := decodeJSON(r, &input); err != nil { + var req apiTokenCreateRequest + if err := decodeJSON(r, &req); err != nil { writeError(w, http.StatusBadRequest, "bad_request", err.Error()) return } + input := req.APITokenCreate if input.Name == "" { writeError(w, http.StatusBadRequest, "bad_request", "name is required") return } + if !normalizeTokenScopesInput(w, &input, req.RawScopes) { + return + } input.WorkspaceID = workspaceID defaultDays, maxDays := s.getTokenExpirySettings() @@ -206,16 +213,20 @@ func (s *Server) handleCreateUserToken(w http.ResponseWriter, r *http.Request) { return } - var input models.APITokenCreate - if err := decodeJSON(r, &input); err != nil { + var req apiTokenCreateRequest + if err := decodeJSON(r, &req); err != nil { writeError(w, http.StatusBadRequest, "bad_request", err.Error()) return } + input := req.APITokenCreate if input.Name == "" { writeError(w, http.StatusBadRequest, "bad_request", "name is required") return } + if !normalizeTokenScopesInput(w, &input, req.RawScopes) { + return + } // A token may be pinned only to a live workspace the caller belongs to // (BUG-3346). Anything else is the workspace 404, identical for a @@ -346,3 +357,76 @@ func setTokenExpiryWarning(w http.ResponseWriter, token *models.APIToken) { w.Header().Set("X-Token-Expires-At", token.ExpiresAt.Format(time.RFC3339)) } } + +// apiTokenScopeVocabulary is every scope value tokenScopeAllows recognizes +// (TASK-3866). A key is minted only with these: anything else is stored +// as written and then refused on every request, so a create that names +// one would hand back a key that can do nothing. +var apiTokenScopeVocabulary = []string{"*", "read", "write", "pad:read", "pad:write", "pad:admin"} + +// apiTokenCreateRequest is a create body with scopes kept raw, so an +// explicit "scopes": null can be told apart from an omitted field (Codex +// r1 on TASK-3866). Decoded into models.APITokenCreate alone, null and +// absent both arrive as "" and both would mint a full-access key; the +// shallower RawScopes field takes the "scopes" key from the embedded one. +type apiTokenCreateRequest struct { + models.APITokenCreate + RawScopes json.RawMessage `json:"scopes,omitempty"` +} + +// normalizeTokenScopesInput checks a create request's scopes and rewrites +// them to their compact JSON form, writing a 400 validation_error and +// returning false when they cannot mint a working key. raw is the request's +// "scopes" value as sent: only an absent field keeps the store's default, +// ["*"]. Refused: a JSON null field, a value that is not a string, the +// empty string and the empty array (the legacy unrestricted forms, which +// no current caller sends and no new key should carry), a string that is +// not a JSON array of strings (including "null"), and any value outside +// the vocabulary. +func normalizeTokenScopesInput(w http.ResponseWriter, input *models.APITokenCreate, raw json.RawMessage) bool { + allowed := strings.Join(apiTokenScopeVocabulary, ", ") + input.Scopes = "" + if len(raw) == 0 { + return true + } + // Into a pointer, so null is nil however it is spaced. + var field *string + if err := json.Unmarshal(raw, &field); err != nil || field == nil { + writeError(w, http.StatusBadRequest, "validation_error", + fmt.Sprintf("scopes must be a string holding a JSON array of: %s (got %s); omit scopes for full access", allowed, raw)) + return false + } + // "" is the stored legacy unrestricted form, refused for the reason [] + // is (Codex r2): only an omitted field gets the default. + if *field == "" { + writeError(w, http.StatusBadRequest, "validation_error", + fmt.Sprintf("scopes must name at least one of: %s; omit scopes for full access", allowed)) + return false + } + input.Scopes = *field + var scopes []string + if err := json.Unmarshal([]byte(input.Scopes), &scopes); err != nil || scopes == nil { + writeError(w, http.StatusBadRequest, "validation_error", + fmt.Sprintf("scopes must be a JSON array of: %s (got %q)", allowed, input.Scopes)) + return false + } + if len(scopes) == 0 { + writeError(w, http.StatusBadRequest, "validation_error", + fmt.Sprintf("scopes must name at least one of: %s; omit scopes for full access", allowed)) + return false + } + for _, scope := range scopes { + if !slices.Contains(apiTokenScopeVocabulary, scope) { + writeError(w, http.StatusBadRequest, "validation_error", + fmt.Sprintf("unknown scope %q; allowed values: %s", scope, allowed)) + return false + } + } + encoded, err := json.Marshal(scopes) + if err != nil { + writeInternalError(w, err) + return false + } + input.Scopes = string(encoded) + return true +} diff --git a/internal/server/task3866_token_scopes_create_test.go b/internal/server/task3866_token_scopes_create_test.go new file mode 100644 index 000000000..f82177c49 --- /dev/null +++ b/internal/server/task3866_token_scopes_create_test.go @@ -0,0 +1,127 @@ +package server + +import ( + "encoding/json" + "net/http" + "strings" + "testing" +) + +// TASK-3866: both doors that mint an API key accept exactly the scope +// values tokenScopeAllows recognizes, store them compact, and refuse +// anything that would mint a key the server then refuses everywhere, or +// one in the legacy unrestricted [] form. +func TestTASK3866_CreateTokenScopes(t *testing.T) { + t.Parallel() + env := setupTokenAuthEnv(t) + doors := map[string]string{ + "user": "/api/v1/auth/tokens", + "workspace": "/api/v1/workspaces/" + env.ws.Slug + "/tokens", + } + + accepted := []struct { + name string + scopes any // omitted when nil + stored string + }{ + {"omitted keeps the default", nil, `["*"]`}, + {"read", `["read"]`, `["read"]`}, + {"wildcard", `["*"]`, `["*"]`}, + {"padded and mixed are stored compact", `[ "read" , "pad:write" ]`, `["read","pad:write"]`}, + {"every vocabulary value", `["*","read","write","pad:read","pad:write","pad:admin"]`, `["*","read","write","pad:read","pad:write","pad:admin"]`}, + } + // Not strings: the field itself is null, an array or a number. A JSON + // null used to decode as "" and mint a full-access key (Codex r1). + refusedRaw := []struct { + name string + scopes json.RawMessage + named string + }{ + {"a JSON null field", json.RawMessage(`null`), "got null"}, + {"an array, not a string", json.RawMessage(`["read"]`), `got ["read"]`}, + {"a number", json.RawMessage(`1`), "got 1"}, + {"a spaced JSON null field", json.RawMessage(" null "), "got null"}, + {"an empty string", json.RawMessage(`""`), "omit scopes"}, + } + refused := []struct { + name, scopes, named string + }{ + {"null", `null`, `"null"`}, + {"unparseable", `read`, `"read"`}, + {"an object", `{}`, `"{}"`}, + {"a bare string", `"read"`, `"\"read\""`}, + {"a non-string element", `["read",1]`, `"[\"read\",1]"`}, + {"empty array", `[]`, "omit scopes"}, + {"padded empty array", `[ ]`, "omit scopes"}, + {"unknown value", `["read-only"]`, `"read-only"`}, + {"case matters", `["Read"]`, `"Read"`}, + {"unknown beside a known one", `["read","docs"]`, `"docs"`}, + {"null element", `["read",null]`, `""`}, + } + + for door, path := range doors { + for i, tc := range accepted { + body := map[string]any{"name": door + "-ok-" + string(rune('a'+i))} + if tc.scopes != nil { + body["scopes"] = tc.scopes + } + rr := doRequestWithBearer(env.srv, "POST", path, env.sessTok, body) + if rr.Code != http.StatusCreated { + t.Errorf("%s door, %s: %d %s, want 201", door, tc.name, rr.Code, rr.Body.String()) + continue + } + var got struct { + Scopes string `json:"scopes"` + } + if err := json.Unmarshal(rr.Body.Bytes(), &got); err != nil { + t.Fatal(err) + } + if got.Scopes != tc.stored { + t.Errorf("%s door, %s: stored %q, want %q", door, tc.name, got.Scopes, tc.stored) + } + } + for _, tc := range refused { + rr := doRequestWithBearer(env.srv, "POST", path, env.sessTok, + map[string]any{"name": door + "-bad", "scopes": tc.scopes}) + if rr.Code != http.StatusBadRequest || errorCode(t, rr) != "validation_error" { + t.Errorf("%s door, %s: %d %s, want 400 validation_error", door, tc.name, rr.Code, rr.Body.String()) + continue + } + if !strings.Contains(rr.Body.String(), jsonEscapedForBody(tc.named)) { + t.Errorf("%s door, %s: message does not name %s: %s", door, tc.name, tc.named, rr.Body.String()) + } + } + } + + for door, path := range doors { + for _, tc := range refusedRaw { + rr := doRequestWithBearer(env.srv, "POST", path, env.sessTok, + map[string]any{"name": door + "-bad", "scopes": tc.scopes}) + if rr.Code != http.StatusBadRequest || errorCode(t, rr) != "validation_error" { + t.Errorf("%s door, %s: %d %s, want 400 validation_error", door, tc.name, rr.Code, rr.Body.String()) + continue + } + if !strings.Contains(rr.Body.String(), jsonEscapedForBody(tc.named)) { + t.Errorf("%s door, %s: message does not name %s: %s", door, tc.name, tc.named, rr.Body.String()) + } + } + } + + // Nothing refused was minted. + tokens, err := env.srv.store.ListUserAPITokens(env.user.ID) + if err != nil { + t.Fatal(err) + } + for _, tok := range tokens { + if strings.HasSuffix(tok.Name, "-bad") { + t.Errorf("a refused create minted %q with scopes %q", tok.Name, tok.Scopes) + } + } +} + +// jsonEscapedForBody is s as it appears inside a JSON string value, the +// form the error message takes in the response body. +func jsonEscapedForBody(s string) string { + b, _ := json.Marshal(s) + return string(b[1 : len(b)-1]) +} diff --git a/internal/server/testdata/token_scope_levels.json b/internal/server/testdata/token_scope_levels.json new file mode 100644 index 000000000..a1f4bdc24 --- /dev/null +++ b/internal/server/testdata/token_scope_levels.json @@ -0,0 +1,37 @@ +{ + "_comment": "TASK-3866: what each stored api_tokens.scopes value lets a key do. Read by TestTokenScopeLevelsFixture (Go, against tokenScopeAllows) and by web/src/lib/settings/tokenScopes.test.ts (against describeTokenScopes), so the web's label cannot drift from the server's check. level: write = reads and writes, read = reads only, none = every request refused. legacy marks the unrestricted pre-enforcement forms. unknown lists the values the server ignores.", + "rows": [ + { "scopes": "[\"*\"]", "level": "write" }, + { "scopes": "[ \"*\" ]", "level": "write" }, + { "scopes": "\u00a0[\"*\"]", "level": "write" }, + { "scopes": "\u0085[\"*\"]", "level": "write" }, + { "scopes": "\ufeff[\"*\"]", "level": "none" }, + { "scopes": "[\"write\"]", "level": "write" }, + { "scopes": "[\"pad:write\"]", "level": "write" }, + { "scopes": "[\"pad:admin\"]", "level": "write" }, + { "scopes": "[\"read\",\"write\"]", "level": "write" }, + { "scopes": "[\"write\",\"read\"]", "level": "write" }, + { "scopes": "[\"*\",\"pad:read\"]", "level": "write" }, + { "scopes": "[\"read\"]", "level": "read" }, + { "scopes": "[\"pad:read\"]", "level": "read" }, + { "scopes": "[\"read\",\"pad:read\"]", "level": "read" }, + { "scopes": "", "level": "write", "legacy": true }, + { "scopes": "[]", "level": "write", "legacy": true }, + { "scopes": "[ ]", "level": "write", "legacy": true }, + { "scopes": "[\n]", "level": "write", "legacy": true }, + { "scopes": "null", "level": "none" }, + { "scopes": " ", "level": "none" }, + { "scopes": "not-json", "level": "none" }, + { "scopes": "{}", "level": "none" }, + { "scopes": "\"read\"", "level": "none" }, + { "scopes": "[\"read\",1]", "level": "none" }, + { "scopes": "\u00a0[\"read\"]", "level": "none" }, + { "scopes": "[\"docs\"]", "level": "none", "unknown": ["docs"] }, + { "scopes": "[\"read-only\"]", "level": "none", "unknown": ["read-only"] }, + { "scopes": "[\"Read\"]", "level": "none", "unknown": ["Read"] }, + { "scopes": "[null]", "level": "none", "unknown": [""] }, + { "scopes": "[\"docs\",\"read\"]", "level": "read", "unknown": ["docs"] }, + { "scopes": "[\"read\",null]", "level": "read", "unknown": [""] }, + { "scopes": "[\"docs\",\"*\"]", "level": "write", "unknown": ["docs"] } + ] +} diff --git a/internal/server/token_scope_levels_test.go b/internal/server/token_scope_levels_test.go new file mode 100644 index 000000000..2e2e40038 --- /dev/null +++ b/internal/server/token_scope_levels_test.go @@ -0,0 +1,48 @@ +package server + +import ( + "encoding/json" + "net/http" + "os" + "testing" +) + +// TestTokenScopeLevelsFixture holds tokenScopeAllows to the level each row +// of testdata/token_scope_levels.json claims (TASK-3866). The web's token +// list labels keys from the same file (web/src/lib/settings/tokenScopes.test.ts), +// so a change to the server's check that the label does not follow turns +// one of the two red. +func TestTokenScopeLevelsFixture(t *testing.T) { + raw, err := os.ReadFile("testdata/token_scope_levels.json") + if err != nil { + t.Fatal(err) + } + var fixture struct { + Rows []struct { + Scopes string `json:"scopes"` + Level string `json:"level"` + } `json:"rows"` + } + if err := json.Unmarshal(raw, &fixture); err != nil { + t.Fatal(err) + } + if len(fixture.Rows) == 0 { + t.Fatal("fixture has no rows") + } + for _, row := range fixture.Rows { + read := tokenScopeAllows(row.Scopes, http.MethodGet, "/api/v1/workspaces") + write := tokenScopeAllows(row.Scopes, http.MethodPatch, "/api/v1/workspaces") + got := "none" + switch { + case read && write: + got = "write" + case read: + got = "read" + case write: + got = "write-without-read" + } + if got != row.Level { + t.Errorf("scopes %q: server grants %s, fixture says %s", row.Scopes, got, row.Level) + } + } +} diff --git a/web/e2e/task-3866-token-access.spec.ts b/web/e2e/task-3866-token-access.spec.ts new file mode 100644 index 000000000..57bb96e07 --- /dev/null +++ b/web/e2e/task-3866-token-access.spec.ts @@ -0,0 +1,59 @@ +/** + * TASK-3866: a key made Read only in console settings is listed as Read + * only, and the key the form reveals can read the workspace and is refused + * a write. The Go test TestTASK3866_ReadOnlyKeyRefusesWritesOverRESTAndMCP + * covers the same key over remote MCP; this spec proves the form sends what + * that test mints. + */ +import { request as playwrightRequest } from '@playwright/test'; +import { test, expect } from './fixtures'; + +test('a Read only key made in settings is listed as Read only and cannot write', async ({ page, fixture }, testInfo) => { + test.skip(testInfo.project.name !== 'desktop-chromium', 'one project is enough for a form-to-server contract'); + + const name = `task-3866 read-only ${Date.now()}`; + const admin = await playwrightRequest.newContext({ + baseURL: fixture.baseURL, + extraHTTPHeaders: { Authorization: `Bearer ${fixture.adminSessionToken}` } + }); + try { + // Minting needs an interactive session (BUG-2890): the suite's + // default PAT is refused session_required. A padsess_ bearer counts + // as interactive and, unlike a cookie, is not bound to a User-Agent. + await page.context().setExtraHTTPHeaders({ Authorization: `Bearer ${fixture.adminSessionToken}` }); + await page.goto('/console/settings#api-tokens'); + const section = page.locator('#api-tokens'); + await section.getByRole('textbox', { name: 'Token name' }).fill(name); + // Read and write is the default; Read only is a deliberate choice. + await expect(section.getByRole('radio', { name: /Read and write/ })).toBeChecked(); + await section.getByRole('radio', { name: /Read only/ }).check(); + await section.getByRole('button', { name: 'Create', exact: true }).click(); + + const secret = (await section.locator('.token-value').textContent())?.trim() ?? ''; + expect(secret).toMatch(/^pad_/); + const row = section.locator('.token-row', { hasText: name }); + await expect(row.getByTestId('token-access')).toHaveText('Read only'); + // The form resets to the default for the next key. + await expect(section.getByRole('radio', { name: /Read and write/ })).toBeChecked(); + + const key = await playwrightRequest.newContext({ + baseURL: fixture.baseURL, + extraHTTPHeaders: { Authorization: `Bearer ${secret}` } + }); + try { + const items = `/api/v1/workspaces/${fixture.workspaceSlug}/collections/tasks/items`; + const read = await key.get(items); + expect(read.status(), await read.text()).toBe(200); + const write = await key.post(items, { data: { title: 'written by a read-only key' } }); + expect(write.status(), await write.text()).toBe(403); + } finally { + await key.dispose(); + } + } finally { + const list = await admin.get('/api/v1/auth/tokens'); + for (const t of (await list.json()) as { id: string; name: string }[]) { + if (t.name === name) await admin.delete(`/api/v1/auth/tokens/${t.id}`); + } + await admin.dispose(); + } +}); diff --git a/web/src/lib/api/client.ts b/web/src/lib/api/client.ts index a98129c90..3f9be2233 100644 --- a/web/src/lib/api/client.ts +++ b/web/src/lib/api/client.ts @@ -3158,10 +3158,12 @@ export const api = { }, tokens: { list: () => request('/auth/tokens'), - create: (name: string) => + /** scopes is the JSON-array string the server stores, e.g. '["read"]' + * (TASK-3866); omitted means full access. */ + create: (name: string, scopes?: string) => request('/auth/tokens', { method: 'POST', - body: JSON.stringify({ name }) + body: JSON.stringify(scopes ? { name, scopes } : { name }) }), delete: (tokenId: string) => request(`/auth/tokens/${tokenId}`, { method: 'DELETE' }), diff --git a/web/src/lib/settings/tokenScopes.test.ts b/web/src/lib/settings/tokenScopes.test.ts new file mode 100644 index 000000000..dc6ef3b1c --- /dev/null +++ b/web/src/lib/settings/tokenScopes.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest'; +import fixture from '../../../../internal/server/testdata/token_scope_levels.json'; +import { describeTokenScopes } from './tokenScopes'; + +interface Row { + scopes: string; + level: string; + legacy?: boolean; + unknown?: string[]; +} + +// The same rows TestTokenScopeLevelsFixture holds tokenScopeAllows to. +describe('describeTokenScopes agrees with the server (TASK-3866)', () => { + const rows = (fixture as { rows: Row[] }).rows; + + it('has rows', () => { + expect(rows.length).toBeGreaterThan(0); + }); + + for (const row of rows) { + it(`${JSON.stringify(row.scopes)} → ${row.level}`, () => { + const got = describeTokenScopes(row.scopes); + expect(got.level).toBe(row.level); + expect(got.legacy).toBe(row.legacy ?? false); + expect(got.unknown).toEqual(row.unknown ?? []); + }); + } + + it('a missing or non-string value is No access, never full access', () => { + for (const raw of [undefined, null, 0, ['*']]) { + expect(describeTokenScopes(raw).level).toBe('none'); + } + }); + + it('never labels a refused key as having access', () => { + for (const raw of ['null', 'not-json', '["docs"]']) { + expect(describeTokenScopes(raw).label).toBe('No access'); + } + }); +}); diff --git a/web/src/lib/settings/tokenScopes.ts b/web/src/lib/settings/tokenScopes.ts new file mode 100644 index 000000000..956b05eaa --- /dev/null +++ b/web/src/lib/settings/tokenScopes.ts @@ -0,0 +1,83 @@ +// What an API key's stored scopes let it do, in words (TASK-3866). +// +// This mirrors tokenScopeAllows (internal/server/middleware_auth.go) +// rather than reading the scope names at face value. The server sees two +// levels, read and write ("pad:admin" grants exactly what "write" does), +// and some stored values deny everything. The label must never say a key +// can do something the server would refuse. The shared fixture +// internal/server/testdata/token_scope_levels.json holds both sides to +// the same answers. + +export type TokenAccessLevel = 'write' | 'read' | 'none'; + +export interface TokenAccess { + level: TokenAccessLevel; + label: string; + /** "" or []: a key from before scopes were enforced, unrestricted. */ + legacy: boolean; + /** Values the server ignores, in stored order. */ + unknown: string[]; +} + +const WRITE_SCOPES = new Set(['*', 'write', 'pad:write', 'pad:admin']); +const READ_SCOPES = new Set(['read', 'pad:read']); + +// Go's unicode.IsSpace, which strings.TrimSpace uses for the server's +// ["*"] fast path. JS trim() differs (it trims U+FEFF and not U+0085), and +// the fast path is the only place the server trims anything, so a padded +// ["*"] is full access while a padded ["read"] fails to parse. +const GO_SPACE_CLASS = '[\\t\\n\\v\\f\\r \\u0085\\u00a0\\u1680\\u2000-\\u200a\\u2028\\u2029\\u202f\\u205f\\u3000]'; +const GO_SPACE = new RegExp(`^${GO_SPACE_CLASS}+|${GO_SPACE_CLASS}+$`, 'g'); + +const LABELS: Record = { + write: 'Read and write', + read: 'Read only', + none: 'No access', +}; + +function result(level: TokenAccessLevel, legacy = false, unknown: string[] = []): TokenAccess { + return { level, label: LABELS[level], legacy, unknown }; +} + +// parseScopes returns what Go's json.Unmarshal into []string yields: +// undefined where it fails (deny), null for a JSON null (deny), else the +// strings, with a null element read as "" as Go reads it. +function parseScopes(raw: string): string[] | null | undefined { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return undefined; + } + if (parsed === null) return null; + if (!Array.isArray(parsed)) return undefined; + const out: string[] = []; + for (const v of parsed) { + if (v === null) out.push(''); + else if (typeof v === 'string') out.push(v); + else return undefined; + } + return out; +} + +export function describeTokenScopes(raw: unknown): TokenAccess { + // The API always sends scopes as a string. Anything else is a value we + // cannot read, and a key we cannot read is never shown with access. + if (typeof raw !== 'string') return result('none'); + if (raw === '') return result('write', true); + if (raw.replace(GO_SPACE, '') === '["*"]') return result('write'); + + const scopes = parseScopes(raw); + if (!scopes) return result('none'); + if (scopes.length === 0) return result('write', true); + + let level: TokenAccessLevel = 'none'; + const unknown: string[] = []; + for (const s of scopes) { + if (WRITE_SCOPES.has(s)) level = 'write'; + else if (READ_SCOPES.has(s)) { + if (level === 'none') level = 'read'; + } else unknown.push(s); + } + return result(level, false, unknown); +} diff --git a/web/src/routes/console/settings/+page.svelte b/web/src/routes/console/settings/+page.svelte index 03d82a252..5b348b091 100644 --- a/web/src/routes/console/settings/+page.svelte +++ b/web/src/routes/console/settings/+page.svelte @@ -13,6 +13,7 @@ import Modal from '$lib/components/common/Modal.svelte'; import CharacterShortcutsToggle from '$lib/components/common/CharacterShortcutsToggle.svelte'; import type { User, APIToken, APITokenWithSecret, TOTPSetupResponse } from '$lib/types'; + import { describeTokenScopes } from '$lib/settings/tokenScopes'; // Profile let profile = $state(null); @@ -79,6 +80,12 @@ // Tokens let tokens = $state([]); let newTokenName = $state(''); + // TASK-3866: the access a new key gets. Read and write sends ["*"], the + // store's own default, so a key made here is identical to one made with + // no scopes. There is no Admin choice: the server grants pad:admin + // exactly what write gets. + let newTokenAccess = $state<'write' | 'read'>('write'); + const TOKEN_ACCESS_SCOPES = { write: '["*"]', read: '["read"]' } as const; let createdToken = $state(null); let tokenCreating = $state(false); // TASK-2193: deleting a token used to fire on the first click and swallow @@ -413,12 +420,13 @@ tokenCreating = true; try { - const token = await api.auth.tokens.create(newTokenName.trim()); + const token = await api.auth.tokens.create(newTokenName.trim(), TOKEN_ACCESS_SCOPES[newTokenAccess]); createdToken = token; rotatedFrom = null; tokenCopied = false; tokens = [...tokens, token]; newTokenName = ''; + newTokenAccess = 'write'; } catch (err) { if (isPlanLimitError(err)) { showPlanLimitToast(err); @@ -978,6 +986,23 @@ {tokenCreating ? 'Creating...' : 'Create'} +
+ Access + + +
{#if tokenError}

{tokenError}

@@ -986,9 +1011,13 @@ {#if tokens.length > 0}
{#each tokens as token (token.id)} + {@const access = describeTokenScopes(token.scopes)}
{token.name} + + {access.label}{#if access.legacy} (older key, unrestricted){/if}{#if access.level === 'none'} (its access can't be read, so every request is refused; delete it and make a new one){:else if access.unknown.length > 0} (ignores: {access.unknown.map((u) => u || '(empty)').join(', ')}){/if} + {token.prefix}... · Created {formatDate(token.created_at)} {#if token.expires_at} @@ -1184,6 +1213,7 @@

It keeps its name, scopes and workspace and gets a new key. The current key stops working immediately, so update every agent, script or MCP connection that uses it.

+

Access stays {describeTokenScopes(tokenToRotate.scopes).label}.

{#if tokenRotateError} {/if} @@ -1485,6 +1515,62 @@ color: var(--text-muted); } + .token-access { + display: flex; + flex-direction: column; + gap: var(--space-1); + margin: var(--space-2) 0 0; + padding: 0; + border: 0; + } + + .token-access legend { + font-size: 0.8rem; + font-weight: 500; + color: var(--text-secondary); + margin-bottom: var(--space-1); + } + + .token-access-option { + display: flex; + align-items: flex-start; + gap: var(--space-2); + font-size: 0.85rem; + cursor: pointer; + } + + .token-access-option input { + margin-top: 3px; + } + + .token-access-label { + display: block; + color: var(--text-primary); + } + + .token-access-hint { + display: block; + font-size: 0.75rem; + color: var(--text-muted); + } + + .token-access-badge { + font-size: 0.75rem; + color: var(--text-secondary); + } + + .token-access-badge.none { + color: var(--accent-red); + } + + .token-access-note { + color: var(--text-muted); + } + + .token-access-badge.none .token-access-note { + color: inherit; + } + .empty-text { color: var(--text-muted); font-size: 0.85rem;