fix: code review findings — token UI, auth hardening, WS dedup

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) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang 2026-04-16 10:28:10 -07:00
parent 5d4ee18c72
commit 54bb543ff7
9 changed files with 301 additions and 85 deletions

View File

@ -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) {
<Tabs.Root defaultValue="api-keys">
<Tabs.List className="settings-panel__tabs" aria-label="Settings sections">
<Tabs.Trigger value="api-keys" className="settings-panel__tab">
API Keys
Secrets
</Tabs.Trigger>
<Tabs.Trigger
value="general"
className="settings-panel__tab"
disabled
>
General
<Tabs.Trigger value="tokens" className="settings-panel__tab">
API Tokens
</Tabs.Trigger>
</Tabs.List>
@ -121,8 +118,8 @@ export function SettingsPanel({ workspaceId }: SettingsPanelProps) {
<SecretsTab workspaceId={workspaceId} />
</Tabs.Content>
<Tabs.Content value="general" className="settings-panel__content">
{/* Future: General settings */}
<Tabs.Content value="tokens" className="settings-panel__content">
<TokensTab workspaceId={workspaceId} />
</Tabs.Content>
</Tabs.Root>
@ -132,7 +129,7 @@ export function SettingsPanel({ workspaceId }: SettingsPanelProps) {
</span>
<span className="settings-panel__separator">·</span>
<a
href="https://docs.example.com/secrets"
href="https://doc.moleculesai.app/docs/tokens"
target="_blank"
rel="noopener noreferrer"
className="settings-panel__docs-link"

View File

@ -0,0 +1,191 @@
'use client';
import { useState, useEffect, useCallback } from 'react';
import { api } from '@/lib/api';
import { Spinner } from '@/components/Spinner';
import { ConfirmDialog } from '@/components/ConfirmDialog';
interface Token {
id: string;
prefix: string;
created_at: string;
last_used_at: string | null;
}
interface TokensTabProps {
workspaceId: string;
}
export function TokensTab({ workspaceId }: TokensTabProps) {
const [tokens, setTokens] = useState<Token[]>([]);
const [loading, setLoading] = useState(true);
const [creating, setCreating] = useState(false);
const [newToken, setNewToken] = useState<string | null>(null);
const [copied, setCopied] = useState(false);
const [revokeTarget, setRevokeTarget] = useState<Token | null>(null);
const [error, setError] = useState<string | null>(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 (
<div className="p-4 space-y-4">
<div className="flex items-center justify-between">
<div>
<h3 className="text-sm font-semibold text-zinc-200">API Tokens</h3>
<p className="text-[10px] text-zinc-500 mt-0.5">
Bearer tokens for authenticating API calls to this workspace.
</p>
</div>
<button
onClick={handleCreate}
disabled={creating}
className="px-3 py-1.5 bg-blue-600/20 hover:bg-blue-600/30 border border-blue-500/30 rounded-lg text-[11px] text-blue-300 font-medium transition-colors disabled:opacity-50 disabled:cursor-not-allowed flex items-center gap-1.5"
>
{creating ? <><Spinner size="sm" /> Creating...</> : '+ New Token'}
</button>
</div>
{/* Newly created token — show once */}
{newToken && (
<div className="bg-emerald-950/30 border border-emerald-800/40 rounded-lg p-3 space-y-2">
<div className="flex items-center gap-2">
<span className="text-[10px] text-emerald-400 font-semibold uppercase tracking-wider">New Token Created</span>
<span className="text-[9px] text-emerald-500/70">Copy now it won't be shown again</span>
</div>
<div className="flex items-center gap-2">
<code className="flex-1 text-[11px] text-emerald-200 bg-emerald-950/50 px-2 py-1.5 rounded font-mono break-all select-all">
{newToken}
</code>
<button
onClick={handleCopy}
className="shrink-0 px-2 py-1.5 bg-emerald-800/40 hover:bg-emerald-700/50 border border-emerald-700/40 rounded text-[10px] text-emerald-300 transition-colors"
>
{copied ? 'Copied' : 'Copy'}
</button>
</div>
<button
onClick={() => setNewToken(null)}
className="text-[9px] text-emerald-500/60 hover:text-emerald-400 transition-colors"
>
Dismiss
</button>
</div>
)}
{error && (
<div className="px-3 py-2 bg-red-950/40 border border-red-800/50 rounded-lg text-[10px] text-red-400">
{error}
</div>
)}
{/* Token list */}
{loading ? (
<div className="flex items-center justify-center gap-2 py-6 text-zinc-500 text-xs">
<Spinner /> Loading tokens...
</div>
) : tokens.length === 0 ? (
<div className="text-center py-6">
<p className="text-xs text-zinc-500">No active tokens</p>
<p className="text-[10px] text-zinc-600 mt-1">
Create a token to authenticate API calls.
</p>
</div>
) : (
<div className="space-y-1.5">
{tokens.map((t) => (
<div
key={t.id}
className="flex items-center justify-between bg-zinc-800/40 border border-zinc-700/30 rounded-lg px-3 py-2"
>
<div className="flex items-center gap-3 min-w-0">
<code className="text-[11px] font-mono text-zinc-300 bg-zinc-900/60 px-1.5 py-0.5 rounded">
{t.prefix}...
</code>
<div className="text-[9px] text-zinc-500 space-x-3">
<span>Created {formatAge(t.created_at)}</span>
{t.last_used_at && (
<span>Last used {formatAge(t.last_used_at)}</span>
)}
</div>
</div>
<button
onClick={() => setRevokeTarget(t)}
className="text-[10px] text-red-400/70 hover:text-red-400 transition-colors px-2 py-1"
>
Revoke
</button>
</div>
))}
</div>
)}
{/* Revoke confirmation */}
<ConfirmDialog
open={!!revokeTarget}
title="Revoke Token"
message={`Revoke token ${revokeTarget?.prefix}...? Any agent or script using this token will immediately lose access.`}
confirmLabel="Revoke"
confirmVariant="danger"
onConfirm={() => revokeTarget && handleRevoke(revokeTarget)}
onCancel={() => setRevokeTarget(null)}
/>
</div>
);
}
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`;
}

View File

@ -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<HTMLDivElement>(null);

25
canvas/src/lib/ws-url.ts Normal file
View File

@ -0,0 +1,25 @@
/**
* Derive WebSocket base URL. Priority:
* 1. Explicit NEXT_PUBLIC_WS_URL (non-empty)
* 2. Derived from NEXT_PUBLIC_PLATFORM_URL (httpws)
* 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";
}

View File

@ -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;

View File

@ -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=$!

View File

@ -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

View File

@ -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{

View File

@ -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://<host>/ or http://<host>/
// Referer must start with https://<host>/ or http://<host>/ (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
}