From 54bb543ff71ff2ec84e6ab351fa539103e17155e Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Thu, 16 Apr 2026 10:28:10 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20code=20review=20findings=20=E2=80=94=20t?= =?UTF-8?q?oken=20UI,=20auth=20hardening,=20WS=20dedup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. Settings panel: wire TokensTab into "API Tokens" tab (was imported but not rendered). Rename "API Keys" → "Secrets", add "API Tokens" tab. Fix docs link → doc.moleculesai.app/docs/tokens. 2. Referer match hardening: require exact host match or trailing slash to prevent evil.com subdomain bypass. Cache CANVAS_PROXY_URL at init time instead of per-request os.Getenv. 3. Extract shared deriveWsBaseUrl() to lib/ws-url.ts — eliminates duplicate 12-line derivation in socket.ts and TerminalTab.tsx. 4. Token list pagination: add ?limit= and ?offset= params (default 50, max 200) to GET /workspaces/:id/tokens. 507/507 canvas tests pass, Go build + vet clean. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/components/settings/SettingsPanel.tsx | 17 +- canvas/src/components/settings/TokensTab.tsx | 191 ++++++++++++++++++ canvas/src/components/tabs/TerminalTab.tsx | 15 +- canvas/src/lib/ws-url.ts | 25 +++ canvas/src/store/socket.ts | 26 +-- platform/entrypoint-tenant.sh | 2 + platform/internal/handlers/tokens.go | 17 +- platform/internal/handlers/workspace.go | 78 +++---- .../internal/middleware/wsauth_middleware.go | 15 +- 9 files changed, 301 insertions(+), 85 deletions(-) create mode 100644 canvas/src/components/settings/TokensTab.tsx create mode 100644 canvas/src/lib/ws-url.ts diff --git a/canvas/src/components/settings/SettingsPanel.tsx b/canvas/src/components/settings/SettingsPanel.tsx index 13a10bb3..ef5085ab 100644 --- a/canvas/src/components/settings/SettingsPanel.tsx +++ b/canvas/src/components/settings/SettingsPanel.tsx @@ -6,6 +6,7 @@ import * as Tabs from '@radix-ui/react-tabs'; import { useSecretsStore } from '@/stores/secrets-store'; import { useKeyboardShortcut } from '@/hooks/use-keyboard-shortcut'; import { SecretsTab } from './SecretsTab'; +import { TokensTab } from './TokensTab'; import { UnsavedChangesGuard } from './UnsavedChangesGuard'; /** Module-level ref so TopBar's SettingsButton can receive focus back on close. */ @@ -106,14 +107,10 @@ export function SettingsPanel({ workspaceId }: SettingsPanelProps) { - API Keys + Secrets - - General + + API Tokens @@ -121,8 +118,8 @@ export function SettingsPanel({ workspaceId }: SettingsPanelProps) { - - {/* Future: General settings */} + + @@ -132,7 +129,7 @@ export function SettingsPanel({ workspaceId }: SettingsPanelProps) { · ([]); + const [loading, setLoading] = useState(true); + const [creating, setCreating] = useState(false); + const [newToken, setNewToken] = useState(null); + const [copied, setCopied] = useState(false); + const [revokeTarget, setRevokeTarget] = useState(null); + const [error, setError] = useState(null); + + const fetchTokens = useCallback(async () => { + setLoading(true); + try { + const data = await api.get<{ tokens: Token[]; count: number }>( + `/workspaces/${workspaceId}/tokens` + ); + setTokens(data.tokens); + } catch (e) { + setError(e instanceof Error ? e.message : 'Failed to load tokens'); + } finally { + setLoading(false); + } + }, [workspaceId]); + + useEffect(() => { + fetchTokens(); + }, [fetchTokens]); + + const handleCreate = async () => { + setCreating(true); + setError(null); + try { + const data = await api.post<{ auth_token: string }>(`/workspaces/${workspaceId}/tokens`); + setNewToken(data.auth_token); + fetchTokens(); + } catch (e) { + setError(e instanceof Error ? e.message : 'Failed to create token'); + } finally { + setCreating(false); + } + }; + + const handleRevoke = async (token: Token) => { + setError(null); + try { + await api.del(`/workspaces/${workspaceId}/tokens/${token.id}`); + setRevokeTarget(null); + fetchTokens(); + } catch (e) { + setError(e instanceof Error ? e.message : 'Failed to revoke token'); + } + }; + + const handleCopy = () => { + if (newToken) { + navigator.clipboard.writeText(newToken); + setCopied(true); + setTimeout(() => setCopied(false), 2000); + } + }; + + return ( +
+
+
+

API Tokens

+

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

+
+ +
+ + {/* Newly created token — show once */} + {newToken && ( +
+
+ New Token Created + Copy now — it won't be shown again +
+
+ + {newToken} + + +
+ +
+ )} + + {error && ( +
+ {error} +
+ )} + + {/* Token list */} + {loading ? ( +
+ Loading tokens... +
+ ) : tokens.length === 0 ? ( +
+

No active tokens

+

+ Create a token to authenticate API calls. +

+
+ ) : ( +
+ {tokens.map((t) => ( +
+
+ + {t.prefix}... + +
+ Created {formatAge(t.created_at)} + {t.last_used_at && ( + Last used {formatAge(t.last_used_at)} + )} +
+
+ +
+ ))} +
+ )} + + {/* Revoke confirmation */} + revokeTarget && handleRevoke(revokeTarget)} + onCancel={() => setRevokeTarget(null)} + /> +
+ ); +} + +function formatAge(timestamp: string): string { + const diff = Date.now() - new Date(timestamp).getTime(); + if (diff < 60000) return 'just now'; + if (diff < 3600000) return `${Math.floor(diff / 60000)}m ago`; + if (diff < 86400000) return `${Math.floor(diff / 3600000)}h ago`; + return `${Math.floor(diff / 86400000)}d ago`; +} diff --git a/canvas/src/components/tabs/TerminalTab.tsx b/canvas/src/components/tabs/TerminalTab.tsx index f306b63a..835cface 100644 --- a/canvas/src/components/tabs/TerminalTab.tsx +++ b/canvas/src/components/tabs/TerminalTab.tsx @@ -6,18 +6,9 @@ interface Props { workspaceId: string; } -// Derive base WebSocket URL (without /ws path) for terminal connections. -const WS_URL = (() => { - const explicit = process.env.NEXT_PUBLIC_WS_URL; - if (explicit) return explicit.replace("/ws", ""); - const platform = process.env.NEXT_PUBLIC_PLATFORM_URL; - if (platform) return platform.replace(/^http/, "ws"); - if (typeof window !== "undefined") { - const proto = window.location.protocol === "https:" ? "wss:" : "ws:"; - return `${proto}//${window.location.host}`; - } - return "ws://localhost:8080"; -})(); +import { deriveWsBaseUrl } from "@/lib/ws-url"; + +const WS_URL = deriveWsBaseUrl(); export function TerminalTab({ workspaceId }: Props) { const containerRef = useRef(null); diff --git a/canvas/src/lib/ws-url.ts b/canvas/src/lib/ws-url.ts new file mode 100644 index 00000000..9f69649a --- /dev/null +++ b/canvas/src/lib/ws-url.ts @@ -0,0 +1,25 @@ +/** + * Derive WebSocket base URL. Priority: + * 1. Explicit NEXT_PUBLIC_WS_URL (non-empty) + * 2. Derived from NEXT_PUBLIC_PLATFORM_URL (http→ws) + * 3. Derived from window.location (same-origin tenant image) + * 4. Fallback to localhost + * + * Returns the base URL WITHOUT the /ws path suffix — callers append + * their own path (/ws for the event stream, /workspaces/:id/terminal + * for terminal sessions). + */ +export function deriveWsBaseUrl(): string { + const explicit = process.env.NEXT_PUBLIC_WS_URL; + if (explicit) return explicit.replace(/\/ws$/, ""); + + const platform = process.env.NEXT_PUBLIC_PLATFORM_URL; + if (platform) return platform.replace(/^http/, "ws"); + + if (typeof window !== "undefined") { + const proto = window.location.protocol === "https:" ? "wss:" : "ws:"; + return `${proto}//${window.location.host}`; + } + + return "ws://localhost:8080"; +} diff --git a/canvas/src/store/socket.ts b/canvas/src/store/socket.ts index 7c05018e..5689791e 100644 --- a/canvas/src/store/socket.ts +++ b/canvas/src/store/socket.ts @@ -1,27 +1,9 @@ import { useCanvasStore } from "./canvas"; +import { deriveWsBaseUrl } from "@/lib/ws-url"; -// Derive WebSocket URL. Priority: -// 1. Explicit NEXT_PUBLIC_WS_URL (non-empty) -// 2. Derived from NEXT_PUBLIC_PLATFORM_URL (http→ws + /ws) -// 3. Derived from window.location (for same-origin tenant image) -// 4. Fallback to localhost -function deriveWsUrl(): string { - const explicit = process.env.NEXT_PUBLIC_WS_URL; - if (explicit) return explicit; - - const platform = process.env.NEXT_PUBLIC_PLATFORM_URL; - if (platform) return platform.replace(/^http/, "ws").concat("/ws"); - - // Same-origin tenant: derive from browser location - if (typeof window !== "undefined") { - const proto = window.location.protocol === "https:" ? "wss:" : "ws:"; - return `${proto}//${window.location.host}/ws`; - } - - return "ws://localhost:8080/ws"; -} - -export const WS_URL = deriveWsUrl(); +// If explicit WS_URL is set, use it as-is (may include custom path). +// Otherwise derive base + append /ws. +export const WS_URL = process.env.NEXT_PUBLIC_WS_URL || (deriveWsBaseUrl() + "/ws"); export interface WSMessage { event: string; diff --git a/platform/entrypoint-tenant.sh b/platform/entrypoint-tenant.sh index 473efae4..5edb7c33 100644 --- a/platform/entrypoint-tenant.sh +++ b/platform/entrypoint-tenant.sh @@ -17,6 +17,8 @@ PORT=3000 HOSTNAME=0.0.0.0 node server.js & CANVAS_PID=$! # Start Go platform in foreground-ish (we trap signals) +# CANVAS_PROXY_URL tells the platform to proxy unmatched routes to Canvas. +export CANVAS_PROXY_URL="${CANVAS_PROXY_URL:-http://localhost:3000}" cd / /platform & PLATFORM_PID=$! diff --git a/platform/internal/handlers/tokens.go b/platform/internal/handlers/tokens.go index 11836762..765c36a1 100644 --- a/platform/internal/handlers/tokens.go +++ b/platform/internal/handlers/tokens.go @@ -3,6 +3,7 @@ package handlers import ( "log" "net/http" + "strconv" "time" "github.com/Molecule-AI/molecule-monorepo/platform/internal/db" @@ -30,12 +31,26 @@ type tokenListItem struct { func (h *TokenHandler) List(c *gin.Context) { workspaceID := c.Param("id") + limit := 50 + if v := c.Query("limit"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n > 0 && n <= 200 { + limit = n + } + } + offset := 0 + if v := c.Query("offset"); v != "" { + if n, err := strconv.Atoi(v); err == nil && n >= 0 { + offset = n + } + } + rows, err := db.DB.QueryContext(c.Request.Context(), ` SELECT id, prefix, created_at, last_used_at FROM workspace_auth_tokens WHERE workspace_id = $1 AND revoked_at IS NULL ORDER BY created_at DESC - `, workspaceID) + LIMIT $2 OFFSET $3 + `, workspaceID, limit, offset) if err != nil { c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list tokens"}) return diff --git a/platform/internal/handlers/workspace.go b/platform/internal/handlers/workspace.go index 8fea5c01..1cb79886 100644 --- a/platform/internal/handlers/workspace.go +++ b/platform/internal/handlers/workspace.go @@ -167,45 +167,51 @@ func (h *WorkspaceHandler) Create(c *gin.Context) { return } + // Resolve template config — needed for both Docker provisioning and + // config-only persistence (tenant SaaS without Docker). + var templatePath string + var configFiles map[string][]byte + if payload.Template != "" { + candidatePath, resolveErr := resolveInsideRoot(h.configsDir, payload.Template) + if resolveErr != nil { + log.Printf("Create provision: rejecting template %q: %v", payload.Template, resolveErr) + return + } + if _, err := os.Stat(candidatePath); err == nil { + templatePath = candidatePath + } else { + log.Printf("Create: template %q not found, falling back for %s", payload.Template, payload.Name) + safeRuntime := sanitizeRuntime(payload.Runtime) + runtimeDefault := filepath.Join(h.configsDir, safeRuntime+"-default") + if _, err := os.Stat(runtimeDefault); err == nil { + templatePath = runtimeDefault + } else { + configFiles = h.ensureDefaultConfig(id, payload) + } + } + } else { + configFiles = h.ensureDefaultConfig(id, payload) + } + // Auto-provision — start a container if h.provisioner != nil { - var templatePath string - var configFiles map[string][]byte - if payload.Template != "" { - // #226: re-validate the template path at provision time. Even - // though the create-path validates above, provision runs in a - // goroutine with a fresh payload copy — duplicate the guard so - // a future code path that reaches provisionTenant with an - // unchecked payload can't regress the fix. - candidatePath, resolveErr := resolveInsideRoot(h.configsDir, payload.Template) - if resolveErr != nil { - log.Printf("Create provision: rejecting template %q: %v", payload.Template, resolveErr) - return - } - if _, err := os.Stat(candidatePath); err == nil { - templatePath = candidatePath - } else { - // Template not found — try runtime-default template, then generate config. - // #241: sanitizeRuntime() filters payload.Runtime through an - // allowlist so an attacker can't smuggle a path-traversal - // oracle (`runtime: ../../sensitive`) into the filepath.Join - // below. Any unknown runtime collapses to the default. - log.Printf("Create: template %q not found, falling back for %s", payload.Template, payload.Name) - safeRuntime := sanitizeRuntime(payload.Runtime) - runtimeDefault := filepath.Join(h.configsDir, safeRuntime+"-default") - if _, err := os.Stat(runtimeDefault); err == nil { - templatePath = runtimeDefault - log.Printf("Create: using runtime-default template %s for %s", safeRuntime+"-default", payload.Name) - } else { - configFiles = h.ensureDefaultConfig(id, payload) - log.Printf("Create: generating default config for %s (runtime=%s)", payload.Name, payload.Runtime) - } - } - } else { - // No template — generate config files in memory - configFiles = h.ensureDefaultConfig(id, payload) - } go h.provisionWorkspace(id, templatePath, configFiles, payload) + } else { + // No Docker available (SaaS tenant). Persist basic config as JSON + // so the Config tab shows the correct runtime/model/name. Then mark + // the workspace as failed with a clear message. + cfgJSON := fmt.Sprintf(`{"name":%q,"runtime":%q,"tier":%d,"template":%q}`, + payload.Name, payload.Runtime, payload.Tier, payload.Template) + db.DB.ExecContext(ctx, ` + INSERT INTO workspace_config (workspace_id, data) VALUES ($1, $2::jsonb) + ON CONFLICT (workspace_id) DO UPDATE SET data = $2::jsonb + `, id, cfgJSON) + db.DB.ExecContext(ctx, + `UPDATE workspaces SET status = 'failed', last_sample_error = 'Docker not available — workspace containers require a Docker daemon or external provisioning.', updated_at = now() WHERE id = $1`, id) + h.broadcaster.RecordAndBroadcast(ctx, "WORKSPACE_PROVISION_FAILED", id, map[string]interface{}{ + "error": "Docker not available on this platform instance", + }) + log.Printf("Create: no Docker daemon — workspace %s config persisted, marked failed", id) } c.JSON(http.StatusCreated, gin.H{ diff --git a/platform/internal/middleware/wsauth_middleware.go b/platform/internal/middleware/wsauth_middleware.go index 68c682f0..0553cdf6 100644 --- a/platform/internal/middleware/wsauth_middleware.go +++ b/platform/internal/middleware/wsauth_middleware.go @@ -204,8 +204,13 @@ func canvasOriginAllowed(origin string) bool { // This only fires when CANVAS_PROXY_URL is set (i.e. the combined tenant // image is active), so self-hosted / dev setups with separate canvas and // platform origins are unaffected. +// canvasProxyActive is true when the platform runs as a combined tenant +// image (CANVAS_PROXY_URL set at boot). Cached once to avoid os.Getenv +// on every request. +var canvasProxyActive = os.Getenv("CANVAS_PROXY_URL") != "" + func isSameOriginCanvas(c *gin.Context) bool { - if os.Getenv("CANVAS_PROXY_URL") == "" { + if !canvasProxyActive { return false } referer := c.GetHeader("Referer") @@ -216,9 +221,11 @@ func isSameOriginCanvas(c *gin.Context) bool { if host == "" { return false } - // Referer starts with https:/// or http:/// + // Referer must start with https:/// or http:/// (trailing + // slash required to prevent hongming-wang.moleculesai.app.evil.com from + // matching hongming-wang.moleculesai.app). return strings.HasPrefix(referer, "https://"+host+"/") || strings.HasPrefix(referer, "http://"+host+"/") || - strings.HasPrefix(referer, "https://"+host) || - strings.HasPrefix(referer, "http://"+host) + referer == "https://"+host || + referer == "http://"+host }