diff --git a/canvas/src/components/settings/TokensTab.tsx b/canvas/src/components/settings/TokensTab.tsx index 092e4df56..5210b8a17 100644 --- a/canvas/src/components/settings/TokensTab.tsx +++ b/canvas/src/components/settings/TokensTab.tsx @@ -16,7 +16,40 @@ interface TokensTabProps { workspaceId: string; } +// The settings panel passes the literal sentinel "global" when no canvas +// node is selected. Workspace tokens are inherently per-workspace — there +// is no /workspaces/global/tokens endpoint (querying the uuid column with +// "global" 500s on Postgres). The org-wide equivalent lives in the +// separate "Org API Keys" tab. Mirrors the sentinel-awareness that +// api/secrets.ts already has (workspaceId === 'global' → /settings/secrets). +const GLOBAL_WORKSPACE_ID = 'global'; + export function TokensTab({ workspaceId }: TokensTabProps) { + if (workspaceId === GLOBAL_WORKSPACE_ID) { + return ( +
+
+

API Tokens

+

+ Bearer tokens for authenticating API calls to this workspace. +

+
+
+

Select a workspace node first

+

+ Workspace tokens are scoped to a single workspace. Select a node + on the canvas to manage its tokens, or use the{' '} + Org API Keys tab + for org-wide API keys. +

+
+
+ ); + } + return ; +} + +function WorkspaceTokensTab({ workspaceId }: TokensTabProps) { const [tokens, setTokens] = useState([]); const [loading, setLoading] = useState(true); const [creating, setCreating] = useState(false); diff --git a/canvas/src/components/settings/__tests__/TokensTab.test.tsx b/canvas/src/components/settings/__tests__/TokensTab.test.tsx index cb923de55..f9409583b 100644 --- a/canvas/src/components/settings/__tests__/TokensTab.test.tsx +++ b/canvas/src/components/settings/__tests__/TokensTab.test.tsx @@ -302,3 +302,35 @@ describe("TokensTab — error", () => { expect(document.querySelector('[role="status"]')).toBeNull(); }); }); + +// ─── "global" sentinel (no node selected) ──────────────────────────────────── +// +// Regression: SettingsPanel passes the literal "global" when no canvas +// node is selected. workspace tokens are per-workspace and there is no +// /workspaces/global/tokens endpoint — calling it 500'd +// ("invalid input syntax for type uuid: global"). The tab must NOT call +// the API in that state and must point the user at the Org API Keys tab. +describe("TokensTab — global sentinel (no node selected)", () => { + beforeEach(() => { + mockApiGet.mockReset(); + mockApiPost.mockReset(); + mockApiGet.mockRejectedValue(new Error("should not be called")); + }); + + it("does not call the API and shows a pointer to Org API Keys", async () => { + render(); + await flush(); + expect(mockApiGet).not.toHaveBeenCalled(); + expect(mockApiPost).not.toHaveBeenCalled(); + expect(document.body.textContent).toContain("Select a workspace node"); + expect(document.body.textContent).toContain("Org API Keys"); + // No error banner, no scary 500 surfacing. + expect(document.querySelector(".text-bad")).toBeNull(); + }); + + it("has no create button in the global state", async () => { + render(); + await flush(); + expect(document.body.textContent).not.toContain("New Token"); + }); +}); diff --git a/workspace-server/internal/handlers/tokens.go b/workspace-server/internal/handlers/tokens.go index c41f8c518..4ce7be815 100644 --- a/workspace-server/internal/handlers/tokens.go +++ b/workspace-server/internal/handlers/tokens.go @@ -10,8 +10,20 @@ import ( "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" "github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth" "github.com/gin-gonic/gin" + "github.com/google/uuid" ) +// validWorkspaceID returns true when id is a syntactically valid UUID. +// workspace_id is a `uuid` column; passing a non-UUID (e.g. the canvas +// "global" sentinel sent when no node is selected) makes Postgres raise +// `invalid input syntax for type uuid`, which previously leaked as an +// opaque 500. Reject up front with a clean 400 instead. Mirrors the +// uuid.Parse guard already used in handlers/activity.go. +func validWorkspaceID(id string) bool { + _, err := uuid.Parse(id) + return err == nil +} + // TokenHandler exposes user-facing token management for workspaces. // Routes: GET/POST/DELETE /workspaces/:id/tokens (behind WorkspaceAuth). type TokenHandler struct{} @@ -31,6 +43,10 @@ type tokenListItem struct { // never the plaintext or hash). func (h *TokenHandler) List(c *gin.Context) { workspaceID := c.Param("id") + if !validWorkspaceID(workspaceID) { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace id"}) + return + } limit := 50 if v := c.Query("limit"); v != "" { @@ -53,6 +69,7 @@ func (h *TokenHandler) List(c *gin.Context) { LIMIT $2 OFFSET $3 `, workspaceID, limit, offset) if err != nil { + log.Printf("tokens: list query failed for workspace %s: %v", workspaceID, err) c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list tokens"}) return } @@ -85,6 +102,10 @@ const maxTokensPerWorkspace = 50 // exactly once in the response — it cannot be recovered afterwards. func (h *TokenHandler) Create(c *gin.Context) { workspaceID := c.Param("id") + if !validWorkspaceID(workspaceID) { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace id"}) + return + } // Rate limit: max active tokens per workspace var count int @@ -117,6 +138,10 @@ func (h *TokenHandler) Create(c *gin.Context) { func (h *TokenHandler) Revoke(c *gin.Context) { workspaceID := c.Param("id") tokenID := c.Param("tokenId") + if !validWorkspaceID(workspaceID) { + c.JSON(http.StatusBadRequest, gin.H{"error": "invalid workspace id"}) + return + } result, err := db.DB.ExecContext(c.Request.Context(), ` UPDATE workspace_auth_tokens diff --git a/workspace-server/internal/handlers/tokens_sqlmock_test.go b/workspace-server/internal/handlers/tokens_sqlmock_test.go index b0166293e..b3a2d9a52 100644 --- a/workspace-server/internal/handlers/tokens_sqlmock_test.go +++ b/workspace-server/internal/handlers/tokens_sqlmock_test.go @@ -41,6 +41,15 @@ import ( func init() { gin.SetMode(gin.TestMode) } +// Workspace IDs are validated as UUIDs up front (tokens.go validWorkspaceID), +// so handler tests must pass syntactically valid UUIDs. Fixed values keep +// sqlmock WithArgs assertions deterministic. +const ( + wsUUID1 = "11111111-1111-1111-1111-111111111111" + wsUUID2 = "22222222-2222-2222-2222-222222222222" + wsUUID3 = "33333333-3333-3333-3333-333333333333" +) + // withMockDB swaps `db.DB` for a sqlmock and returns the mock plus a // restore func. Tests use this in place of setupTokenTestDB which // skips on a missing real DB. @@ -81,13 +90,13 @@ func TestTokenHandler_List_HappyPath(t *testing.T) { created := time.Date(2026, 4, 1, 12, 0, 0, 0, time.UTC) last := created.Add(time.Hour) mock.ExpectQuery(`SELECT id, prefix, created_at, last_used_at\s+FROM workspace_auth_tokens`). - WithArgs("ws-1", 50, 0). + WithArgs(wsUUID1, 50, 0). WillReturnRows(sqlmock.NewRows([]string{"id", "prefix", "created_at", "last_used_at"}). AddRow("tok-1", "abc12345", created, last). AddRow("tok-2", "def67890", created, nil)) w := makeReq(t, NewTokenHandler().List, "GET", - "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}}) + "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}}) if w.Code != http.StatusOK { t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) @@ -121,7 +130,7 @@ func TestTokenHandler_List_EmptyResult(t *testing.T) { WillReturnRows(sqlmock.NewRows([]string{"id", "prefix", "created_at", "last_used_at"})) w := makeReq(t, NewTokenHandler().List, "GET", - "/workspaces/ws-2/tokens", gin.Params{{Key: "id", Value: "ws-2"}}) + "/workspaces/ws-2/tokens", gin.Params{{Key: "id", Value: wsUUID2}}) if w.Code != http.StatusOK { t.Fatalf("expected 200 on empty list, got %d", w.Code) @@ -146,7 +155,7 @@ func TestTokenHandler_List_QueryError(t *testing.T) { WillReturnError(errors.New("connection refused")) w := makeReq(t, NewTokenHandler().List, "GET", - "/workspaces/ws-3/tokens", gin.Params{{Key: "id", Value: "ws-3"}}) + "/workspaces/ws-3/tokens", gin.Params{{Key: "id", Value: wsUUID3}}) if w.Code != http.StatusInternalServerError { t.Errorf("query error must surface as 500, got %d", w.Code) @@ -158,13 +167,13 @@ func TestTokenHandler_List_RespectsLimit(t *testing.T) { defer cleanup() mock.ExpectQuery(`SELECT id, prefix, created_at, last_used_at`). - WithArgs("ws-1", 10, 5). + WithArgs(wsUUID1, 10, 5). WillReturnRows(sqlmock.NewRows([]string{"id", "prefix", "created_at", "last_used_at"})) w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) c.Request = httptest.NewRequest("GET", "/workspaces/ws-1/tokens?limit=10&offset=5", nil) - c.Params = gin.Params{{Key: "id", Value: "ws-1"}} + c.Params = gin.Params{{Key: "id", Value: wsUUID1}} NewTokenHandler().List(c) if w.Code != http.StatusOK { @@ -186,7 +195,7 @@ func TestTokenHandler_List_ScanError(t *testing.T) { AddRow("tok-1", "abc", "not-a-timestamp", nil)) w := makeReq(t, NewTokenHandler().List, "GET", - "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}}) + "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}}) if w.Code != http.StatusInternalServerError { t.Errorf("scan error must surface as 500, got %d: %s", w.Code, w.Body.String()) @@ -201,11 +210,11 @@ func TestTokenHandler_Create_RateLimited(t *testing.T) { // Count query returns 50 (== max) → 429. mock.ExpectQuery(`SELECT COUNT\(\*\) FROM workspace_auth_tokens`). - WithArgs("ws-1"). + WithArgs(wsUUID1). WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(50)) w := makeReq(t, NewTokenHandler().Create, "POST", - "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}}) + "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}}) if w.Code != http.StatusTooManyRequests { t.Errorf("max active tokens should 429, got %d", w.Code) @@ -225,7 +234,7 @@ func TestTokenHandler_Create_IssueFails(t *testing.T) { WillReturnError(errors.New("disk full")) w := makeReq(t, NewTokenHandler().Create, "POST", - "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}}) + "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}}) if w.Code != http.StatusInternalServerError { t.Errorf("IssueToken DB error must 500, got %d", w.Code) @@ -242,7 +251,7 @@ func TestTokenHandler_Create_HappyPath(t *testing.T) { WillReturnResult(sqlmock.NewResult(1, 1)) w := makeReq(t, NewTokenHandler().Create, "POST", - "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: "ws-1"}}) + "/workspaces/ws-1/tokens", gin.Params{{Key: "id", Value: wsUUID1}}) if w.Code != http.StatusCreated { t.Fatalf("expected 201, got %d: %s", w.Code, w.Body.String()) @@ -257,7 +266,7 @@ func TestTokenHandler_Create_HappyPath(t *testing.T) { if body.AuthToken == "" { t.Errorf("auth_token must be present and non-empty in response") } - if body.WorkspaceID != "ws-1" { + if body.WorkspaceID != wsUUID1 { t.Errorf("workspace_id mismatch: %q", body.WorkspaceID) } } @@ -269,12 +278,12 @@ func TestTokenHandler_Revoke_HappyPath(t *testing.T) { defer cleanup() mock.ExpectExec(`UPDATE workspace_auth_tokens\s+SET revoked_at = now\(\)`). - WithArgs("tok-1", "ws-1"). + WithArgs("tok-1", wsUUID1). WillReturnResult(sqlmock.NewResult(0, 1)) w := makeReq(t, NewTokenHandler().Revoke, "DELETE", "/workspaces/ws-1/tokens/tok-1", gin.Params{ - {Key: "id", Value: "ws-1"}, + {Key: "id", Value: wsUUID1}, {Key: "tokenId", Value: "tok-1"}, }) @@ -289,12 +298,12 @@ func TestTokenHandler_Revoke_NotFound(t *testing.T) { // 0 rows affected → token not found OR already revoked. mock.ExpectExec(`UPDATE workspace_auth_tokens`). - WithArgs("tok-ghost", "ws-1"). + WithArgs("tok-ghost", wsUUID1). WillReturnResult(sqlmock.NewResult(0, 0)) w := makeReq(t, NewTokenHandler().Revoke, "DELETE", "/workspaces/ws-1/tokens/tok-ghost", gin.Params{ - {Key: "id", Value: "ws-1"}, + {Key: "id", Value: wsUUID1}, {Key: "tokenId", Value: "tok-ghost"}, }) @@ -312,7 +321,7 @@ func TestTokenHandler_Revoke_DBError(t *testing.T) { w := makeReq(t, NewTokenHandler().Revoke, "DELETE", "/workspaces/ws-1/tokens/tok-1", gin.Params{ - {Key: "id", Value: "ws-1"}, + {Key: "id", Value: wsUUID1}, {Key: "tokenId", Value: "tok-1"}, }) @@ -321,6 +330,59 @@ func TestTokenHandler_Revoke_DBError(t *testing.T) { } } +// ---- UUID validation (regression: "global" sentinel 500) ------------ + +// The canvas Settings → Workspace Tokens tab sent the literal sentinel +// "global" as the workspace id when no node was selected. workspace_id +// is a `uuid` column, so the query raised +// `invalid input syntax for type uuid: "global"` which leaked as an +// opaque 500. List/Create/Revoke now reject any non-UUID id with a +// clean 400 before touching the DB. No DB expectation is set on the +// mock — a DB hit would fail ExpectationsWereMet, proving short-circuit. +func TestTokenHandler_RejectsNonUUIDWorkspaceID(t *testing.T) { + h := NewTokenHandler() + cases := []struct { + name string + run func(c *gin.Context) + method string + params gin.Params + }{ + {"List", h.List, "GET", gin.Params{{Key: "id", Value: "global"}}}, + {"Create", h.Create, "POST", gin.Params{{Key: "id", Value: "global"}}}, + {"Revoke", h.Revoke, "DELETE", gin.Params{ + {Key: "id", Value: "global"}, + {Key: "tokenId", Value: "tok-1"}, + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + mock, cleanup := withMockDB(t) + defer cleanup() + + w := makeReq(t, tc.run, tc.method, + "/workspaces/global/tokens", tc.params) + + if w.Code != http.StatusBadRequest { + t.Fatalf("%s with non-UUID id must 400, got %d: %s", + tc.name, w.Code, w.Body.String()) + } + var body struct { + Error string `json:"error"` + } + _ = json.Unmarshal(w.Body.Bytes(), &body) + if body.Error != "invalid workspace id" { + t.Errorf("%s: want error=%q, got %q", + tc.name, "invalid workspace id", body.Error) + } + // No query/exec was expected → if the handler hit the DB + // this fails, proving the guard short-circuits before SQL. + if err := mock.ExpectationsWereMet(); err != nil { + t.Errorf("%s leaked a DB call past the uuid guard: %v", tc.name, err) + } + }) + } +} + // Compile-time noise removal: the imports list pulls in the sql / // driver packages and the silenced ctx so a future scenario that // needs them doesn't have to re-add the import. Documented here so diff --git a/workspace-server/internal/handlers/tokens_test.go b/workspace-server/internal/handlers/tokens_test.go index 1b62e1064..bf6d66030 100644 --- a/workspace-server/internal/handlers/tokens_test.go +++ b/workspace-server/internal/handlers/tokens_test.go @@ -11,6 +11,7 @@ import ( "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" "github.com/Molecule-AI/molecule-monorepo/platform/internal/wsauth" "github.com/gin-gonic/gin" + "github.com/google/uuid" ) func init() { gin.SetMode(gin.TestMode) } @@ -167,11 +168,14 @@ func TestTokenHandler_RevokeWrongWorkspace(t *testing.T) { h := NewTokenHandler() - // Try to revoke with a different workspace ID — should 404 + // Try to revoke with a different (valid-UUID) workspace ID that does + // not own the token — should 404. A valid UUID is required so this + // exercises the ownership branch, not the up-front uuid-shape 400. + otherWS := uuid.NewString() w := httptest.NewRecorder() c, _ := gin.CreateTestContext(w) - c.Params = gin.Params{{Key: "id", Value: "wrong-workspace-id"}, {Key: "tokenId", Value: tokenID}} - c.Request = httptest.NewRequest("DELETE", "/workspaces/wrong/tokens/"+tokenID, nil) + c.Params = gin.Params{{Key: "id", Value: otherWS}, {Key: "tokenId", Value: tokenID}} + c.Request = httptest.NewRequest("DELETE", "/workspaces/"+otherWS+"/tokens/"+tokenID, nil) h.Revoke(c) if w.Code != http.StatusNotFound {