From b2f187079cc4562eccb79b7afdc098862d974059 Mon Sep 17 00:00:00 2001 From: openhands Date: Sat, 21 Mar 2026 12:24:40 -0700 Subject: [PATCH] Refactor: extract shared utilities, fix handler leak, improve accessibility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract generateSecureToken() and hashToken() to hub/src/lib/crypto.ts (removes 3 duplicate definitions, fixes layering violation where hashToken lived in ws/channel.ts but was imported by API files) - Move TIER_LIMITS to hub/src/config.ts (single source of truth) - Fix profile PATCH to return full profile object (was returning { ok: true }) - Use -1 sentinel for unlimited tier (Infinity is not JSON-serializable) - Parallelize independent DB queries with Promise.all (sessions, plugin) - Extract hubFetch/authHeaders to web/src/lib/api.ts (replaces ~30 repetitive lines across 5 hook files) - Wrap hook mutation functions in useCallback (prevents unnecessary re-renders) - Fix handler leak in Layout.tsx (subscribe called every render without cleanup, handlers accumulated unboundedly) - Fix Sidebar action buttons: span → button for keyboard accessibility - Deduplicate LoadingScreen and goToChat in App.tsx Co-Authored-By: Claude Opus 4.6 (1M context) --- hub/src/api/api-keys.ts | 11 +----- hub/src/api/plugin.ts | 24 ++++-------- hub/src/api/profile.ts | 28 ++++++++++---- hub/src/api/sessions.ts | 35 +++++------------ hub/src/auth/api-key-middleware.ts | 2 +- hub/src/config.ts | 7 ++++ hub/src/lib/crypto.ts | 13 +++++++ hub/src/ws/channel.ts | 9 +---- web/src/App.tsx | 62 ++++++++++-------------------- web/src/components/Layout.tsx | 20 +++++----- web/src/components/Sidebar.tsx | 14 ++++--- web/src/hooks/useApiKey.ts | 28 +++++--------- web/src/hooks/useChat.ts | 6 +-- web/src/hooks/useSessions.ts | 45 ++++++++-------------- web/src/lib/api.ts | 15 ++++++++ 15 files changed, 143 insertions(+), 176 deletions(-) create mode 100644 hub/src/lib/crypto.ts create mode 100644 web/src/lib/api.ts diff --git a/hub/src/api/api-keys.ts b/hub/src/api/api-keys.ts index 0ba54c9..6dc226b 100644 --- a/hub/src/api/api-keys.ts +++ b/hub/src/api/api-keys.ts @@ -1,16 +1,9 @@ import { Hono } from 'hono' import { createApiKey, listApiKeys, revokeApiKey } from '../db/dal' -import { hashToken } from '../ws/channel' +import { generateSecureToken, hashToken } from '../lib/crypto' const apiKeys = new Hono() -function generateApiKey(): string { - const bytes = crypto.getRandomValues(new Uint8Array(32)) - const b64 = btoa(String.fromCharCode(...bytes)) - .replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') - return `remokey_${b64}` -} - // List API keys (never returns raw key) apiKeys.get('/', async (c) => { const sb = c.get('supabase') @@ -21,7 +14,7 @@ apiKeys.get('/', async (c) => { // Generate new API key (revokes existing active key) apiKeys.post('/', async (c) => { const userId = c.get('userId') - const rawKey = generateApiKey() + const rawKey = generateSecureToken('remokey_') const keyHash = await hashToken(rawKey) const key = await createApiKey(userId, keyHash) return c.json({ ...key, key: rawKey }, 201) diff --git a/hub/src/api/plugin.ts b/hub/src/api/plugin.ts index 8248b41..5a486d7 100644 --- a/hub/src/api/plugin.ts +++ b/hub/src/api/plugin.ts @@ -1,20 +1,13 @@ import { Hono } from 'hono' import { z } from 'zod' import { findOrCreateSession } from '../db/dal' -import { hashToken } from '../ws/channel' +import { generateSecureToken, hashToken } from '../lib/crypto' import { getChannel } from '../ws/registry' import { supabaseAdmin } from '../db/supabase' -import { TIER_LIMITS } from './profile' +import { TIER_LIMITS } from '../config' const plugin = new Hono() -function generateToken(): string { - const bytes = crypto.getRandomValues(new Uint8Array(32)) - const b64 = btoa(String.fromCharCode(...bytes)) - .replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') - return `remo_${b64}` -} - const PluginSessionBody = z.object({ project_dir: z.string().min(1).max(500), }) @@ -43,20 +36,19 @@ plugin.post('/sessions', async (c) => { if (!existingSession) { // New session — check tier limit - const { data: prof } = await supabaseAdmin.from('profiles').select('tier').eq('id', userId).single() + const [{ data: prof }, { count }] = await Promise.all([ + supabaseAdmin.from('profiles').select('tier').eq('id', userId).single(), + supabaseAdmin.from('sessions').select('id', { count: 'exact', head: true }).eq('user_id', userId), + ]) const tier = prof?.tier || 'free' const limit = TIER_LIMITS[tier] || 1 - const { count } = await supabaseAdmin - .from('sessions') - .select('id', { count: 'exact', head: true }) - .eq('user_id', userId) - if ((count || 0) >= limit) { + if (limit !== -1 && (count || 0) >= limit) { return c.json({ error: 'session limit reached', tier, limit, current: count }, 403) } } - const rawToken = generateToken() + const rawToken = generateSecureToken('remo_') const tokenHash = await hashToken(rawToken) const result = await findOrCreateSession(userId, parsed.data.project_dir, tokenHash) diff --git a/hub/src/api/profile.ts b/hub/src/api/profile.ts index eceb024..e4b4de8 100644 --- a/hub/src/api/profile.ts +++ b/hub/src/api/profile.ts @@ -1,15 +1,10 @@ import { Hono } from 'hono' import { z } from 'zod' import { supabaseAdmin } from '../db/supabase' +import { TIER_LIMITS } from '../config' const profile = new Hono() -const TIER_LIMITS: Record = { - free: 1, - pro: 10, - max: Infinity, -} - // Get current user's profile profile.get('/', async (c) => { const userId = c.get('userId') @@ -50,7 +45,24 @@ profile.patch('/', async (c) => { .eq('id', userId) if (error) return c.json({ error: 'update failed' }, 500) - return c.json({ ok: true }) + + // Return the updated profile (same shape as GET) + const { data } = await supabaseAdmin + .from('profiles') + .select('id, email, display_name, role, tier, stripe_customer_id, created_at') + .eq('id', userId) + .single() + + const { count } = await supabaseAdmin + .from('sessions') + .select('id', { count: 'exact', head: true }) + .eq('user_id', userId) + + return c.json({ + ...data, + session_count: count || 0, + session_limit: TIER_LIMITS[data?.tier || 'free'] || 1, + }) }) -export { profile, TIER_LIMITS } +export { profile } diff --git a/hub/src/api/sessions.ts b/hub/src/api/sessions.ts index fe3720d..499c76c 100644 --- a/hub/src/api/sessions.ts +++ b/hub/src/api/sessions.ts @@ -1,10 +1,10 @@ import { Hono } from 'hono' import { z } from 'zod' import { createSession, listSessions, getSession, deleteSession, updateSessionToken } from '../db/dal' -import { hashToken } from '../ws/channel' +import { generateSecureToken, hashToken } from '../lib/crypto' import { getChannel } from '../ws/registry' import { supabaseAdmin } from '../db/supabase' -import { TIER_LIMITS } from './profile' +import { TIER_LIMITS } from '../config' const CreateSessionBody = z.object({ name: z.string().min(1).max(100).trim(), @@ -13,13 +13,6 @@ const CreateSessionBody = z.object({ const sessions = new Hono() -function generateToken(): string { - const bytes = crypto.getRandomValues(new Uint8Array(32)) - const b64 = btoa(String.fromCharCode(...bytes)) - .replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') - return `remo_${b64}` -} - // List all sessions for the authenticated user sessions.get('/', async (c) => { const sb = c.get('supabase') @@ -48,26 +41,18 @@ sessions.post('/', async (c) => { } // Check session limit based on tier - const { data: prof } = await supabaseAdmin.from('profiles').select('tier').eq('id', userId).single() + const [{ data: prof }, { count }] = await Promise.all([ + supabaseAdmin.from('profiles').select('tier').eq('id', userId).single(), + supabaseAdmin.from('sessions').select('id', { count: 'exact', head: true }).eq('user_id', userId), + ]) const tier = prof?.tier || 'free' const limit = TIER_LIMITS[tier] || 1 - const { count } = await supabaseAdmin - .from('sessions') - .select('id', { count: 'exact', head: true }) - .eq('user_id', userId) - - if ((count || 0) >= limit) { - return c.json({ - error: 'session limit reached', - tier, - limit, - current: count, - upgrade_url: '/settings?tab=billing', - }, 403) + if (limit !== -1 && (count || 0) >= limit) { + return c.json({ error: 'session limit reached', tier, limit, current: count }, 403) } - const rawToken = generateToken() + const rawToken = generateSecureToken('remo_') const tokenHash = await hashToken(rawToken) const session = await createSession(sb, userId, parsed.data.name, parsed.data.project_dir || null, tokenHash) @@ -98,7 +83,7 @@ sessions.post('/:id/rotate-token', async (c) => { return c.json({ error: 'not found' }, 404) } - const rawToken = generateToken() + const rawToken = generateSecureToken('remo_') const tokenHash = await hashToken(rawToken) await updateSessionToken(sb, sessionId, tokenHash) diff --git a/hub/src/auth/api-key-middleware.ts b/hub/src/auth/api-key-middleware.ts index 8a23da4..e7ae83f 100644 --- a/hub/src/auth/api-key-middleware.ts +++ b/hub/src/auth/api-key-middleware.ts @@ -1,6 +1,6 @@ import type { Context, Next } from 'hono' import { verifyApiKey } from '../db/dal' -import { hashToken } from '../ws/channel' +import { hashToken } from '../lib/crypto' export async function apiKeyMiddleware(c: Context, next: Next) { const authHeader = c.req.header('Authorization') diff --git a/hub/src/config.ts b/hub/src/config.ts index 8c7ff8e..f0a06e1 100644 --- a/hub/src/config.ts +++ b/hub/src/config.ts @@ -1,3 +1,10 @@ +// -1 = unlimited (JSON cannot represent Infinity) +export const TIER_LIMITS: Record = { + free: 1, + pro: 10, + max: -1, +} + export const config = { port: Number(process.env.PORT || 3040), supabaseUrl: process.env.SUPABASE_URL!, diff --git a/hub/src/lib/crypto.ts b/hub/src/lib/crypto.ts new file mode 100644 index 0000000..2371b5b --- /dev/null +++ b/hub/src/lib/crypto.ts @@ -0,0 +1,13 @@ +export function generateSecureToken(prefix: string): string { + const bytes = crypto.getRandomValues(new Uint8Array(32)) + const b64 = btoa(String.fromCharCode(...bytes)) + .replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') + return `${prefix}${b64}` +} + +export async function hashToken(token: string): Promise { + const encoder = new TextEncoder() + const data = encoder.encode(token) + const hash = await crypto.subtle.digest('SHA-256', data) + return Array.from(new Uint8Array(hash)).map(b => b.toString(16).padStart(2, '0')).join('') +} diff --git a/hub/src/ws/channel.ts b/hub/src/ws/channel.ts index 2c764b0..b08bc9d 100644 --- a/hub/src/ws/channel.ts +++ b/hub/src/ws/channel.ts @@ -3,8 +3,8 @@ import { timingSafeEqual } from 'crypto' import { ChannelInbound } from './protocol' import { verifyChannelToken, setSessionStatus, insertMessage } from '../db/dal' import { registerChannel, unregisterChannel, broadcastToSubscribers, broadcastToUser } from './registry' -import { listSessions } from '../db/dal' import { supabaseAdmin } from '../db/supabase' +import { hashToken } from '../lib/crypto' const AUTH_TIMEOUT_MS = 5_000 const HEARTBEAT_INTERVAL_MS = 30_000 @@ -33,13 +33,6 @@ export function createChannelWsData(): ChannelWsData { } } -export async function hashToken(token: string): Promise { - const encoder = new TextEncoder() - const data = encoder.encode(token) - const hash = await crypto.subtle.digest('SHA-256', data) - return Array.from(new Uint8Array(hash)).map(b => b.toString(16).padStart(2, '0')).join('') -} - export function handleChannelOpen(ws: ServerWebSocket) { const data = ws.data // Require auth within 5 seconds diff --git a/web/src/App.tsx b/web/src/App.tsx index bc58742..92ed468 100644 --- a/web/src/App.tsx +++ b/web/src/App.tsx @@ -7,6 +7,7 @@ import { Layout } from './components/Layout' import { SettingsPage } from './components/SettingsPage' import { AdminDashboard } from './components/AdminDashboard' import { PricingModal } from './components/PricingModal' +import { HUB_URL } from './lib/api' type Route = 'chat' | 'settings' | 'admin' @@ -17,6 +18,14 @@ function getRoute(): Route { return 'chat' } +function LoadingScreen() { + return ( +
+
Loading...
+
+ ) +} + export default function App() { const { session, user, loading, signOut } = useAuth() const { profile, loading: profileLoading, updateProfile, isAdmin } = useProfile(session) @@ -25,8 +34,7 @@ export default function App() { const [showPricing, setShowPricing] = useState(false) useEffect(() => { - const hubUrl = import.meta.env.VITE_HUB_URL || '' - fetch(`${hubUrl}/api/setup/status`) + fetch(`${HUB_URL}/api/setup/status`) .then(r => r.json()) .then(data => setNeedsSetup(data.needs_setup)) .catch(() => setNeedsSetup(false)) @@ -43,34 +51,18 @@ export default function App() { window.location.hash = hash }, []) - const goToChat = useCallback(() => { - window.location.hash = '#/' - }, []) - - if (loading || needsSetup === null) { - return ( -
-
Loading...
-
- ) - } + if (loading || needsSetup === null) return if (needsSetup) { return setNeedsSetup(false)} /> } - if (!session || !user) { - return - } + if (!session || !user) return - // Wait for profile to load before rendering gated routes - if (profileLoading || !profile) { - return ( -
-
Loading...
-
- ) - } + if (profileLoading || !profile) return + + // Non-admin on admin route falls through to chat + const effectiveRoute = (route === 'admin' && !isAdmin) ? 'chat' : route return ( <> @@ -82,36 +74,24 @@ export default function App() { /> )} - {route === 'settings' && ( + {effectiveRoute === 'settings' && ( navigate('#/')} onShowPricing={() => setShowPricing(true)} /> )} - {route === 'admin' && isAdmin && ( + {effectiveRoute === 'admin' && ( - )} - - {/* Redirect non-admin from admin route */} - {route === 'admin' && !isAdmin && ( - setShowPricing(true)} + onBack={() => navigate('#/')} /> )} - {route === 'chat' && ( + {effectiveRoute === 'chat' && ( { - if (msg.type === 'session_status') { - sessionsHook.updateSessionStatus(msg.session_id, msg.status) - } - if (msg.type === 'session_list' && msg.sessions) { - sessionsHook.setSessions(msg.sessions) - } - }) + useEffect(() => { + return subscribe((msg) => { + if (msg.type === 'session_status') { + sessionsHook.updateSessionStatus(msg.session_id, msg.status) + } + if (msg.type === 'session_list' && msg.sessions) { + sessionsHook.setSessions(msg.sessions) + } + }) + }, [subscribe, sessionsHook.updateSessionStatus, sessionsHook.setSessions]) // Wrap createSession to detect 403 session limit const handleCreateSession = useCallback(async (name: string, projectDir?: string) => { @@ -113,7 +115,7 @@ export function Layout({ session, user, signOut, profile, onNavigate, onShowPric onShowConnect={setConnectData} onShowApiKey={() => setShowApiKey(true)} onNavigate={onNavigate} - isAdmin={profile.role === 'admin'} + isAdmin={profile?.role === 'admin'} connected={connected} user={user} signOut={signOut} diff --git a/web/src/components/Sidebar.tsx b/web/src/components/Sidebar.tsx index b9df4a2..7ed0c72 100644 --- a/web/src/components/Sidebar.tsx +++ b/web/src/components/Sidebar.tsx @@ -92,26 +92,28 @@ export function Sidebar({ {s.name} {/* Action buttons — visible on hover */} - { e.stopPropagation(); handleReconnect(s) }} - className="px-1.5 py-0.5 text-[10px] text-slate-400 hover:text-indigo-300 bg-slate-700/80 hover:bg-slate-600/80 rounded transition-colors cursor-pointer" + className="px-1.5 py-0.5 text-[10px] text-slate-400 hover:text-indigo-300 bg-slate-700/80 hover:bg-slate-600/80 rounded transition-colors" title="Get new connection token" > - - + {s.project_dir && ( diff --git a/web/src/hooks/useApiKey.ts b/web/src/hooks/useApiKey.ts index 6ae0d5c..01ec96e 100644 --- a/web/src/hooks/useApiKey.ts +++ b/web/src/hooks/useApiKey.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback } from 'react' import type { Session } from '@supabase/supabase-js' +import { hubFetch } from '../lib/api' export interface ApiKey { id: string @@ -15,40 +16,29 @@ export function useApiKey(session: Session | null) { const fetchKeys = useCallback(async () => { if (!session?.access_token) return - const hubUrl = import.meta.env.VITE_HUB_URL || '' - const res = await fetch(`${hubUrl}/api/api-keys`, { - headers: { Authorization: `Bearer ${session.access_token}` }, - }) + const res = await hubFetch('/api/api-keys', session.access_token) if (res.ok) setKeys(await res.json()) setLoading(false) }, [session?.access_token]) useEffect(() => { fetchKeys() }, [fetchKeys]) - const generateKey = async () => { + const generateKey = useCallback(async () => { if (!session?.access_token) return null - const hubUrl = import.meta.env.VITE_HUB_URL || '' - const res = await fetch(`${hubUrl}/api/api-keys`, { - method: 'POST', - headers: { Authorization: `Bearer ${session.access_token}` }, - }) + const res = await hubFetch('/api/api-keys', session.access_token, { method: 'POST' }) if (res.ok) { const data = await res.json() await fetchKeys() - return data // { id, name, created_at, key: "remokey_..." } + return data } return null - } + }, [session?.access_token, fetchKeys]) - const revokeKey = async (id: string) => { + const revokeKey = useCallback(async (id: string) => { if (!session?.access_token) return - const hubUrl = import.meta.env.VITE_HUB_URL || '' - await fetch(`${hubUrl}/api/api-keys/${id}`, { - method: 'DELETE', - headers: { Authorization: `Bearer ${session.access_token}` }, - }) + await hubFetch(`/api/api-keys/${id}`, session.access_token, { method: 'DELETE' }) await fetchKeys() - } + }, [session?.access_token, fetchKeys]) const activeKey = keys.find(k => !k.revoked_at) || null diff --git a/web/src/hooks/useChat.ts b/web/src/hooks/useChat.ts index eb63f70..38da879 100644 --- a/web/src/hooks/useChat.ts +++ b/web/src/hooks/useChat.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback } from 'react' import type { Session } from '@supabase/supabase-js' +import { hubFetch } from '../lib/api' export interface ChatMessage { id: string @@ -26,10 +27,7 @@ export function useChat( } setLoading(true) - const hubUrl = import.meta.env.VITE_HUB_URL || '' - fetch(`${hubUrl}/api/messages/${activeSessionId}?limit=50`, { - headers: { Authorization: `Bearer ${session.access_token}` }, - }) + hubFetch(`/api/messages/${activeSessionId}?limit=50`, session.access_token) .then(r => r.ok ? r.json() : []) .then(data => { setMessages(data) diff --git a/web/src/hooks/useSessions.ts b/web/src/hooks/useSessions.ts index 61f308a..d0b4dcb 100644 --- a/web/src/hooks/useSessions.ts +++ b/web/src/hooks/useSessions.ts @@ -1,5 +1,6 @@ import { useState, useEffect, useCallback } from 'react' import type { Session as SupaSession } from '@supabase/supabase-js' +import { hubFetch } from '../lib/api' export interface CodeSession { id: string @@ -16,10 +17,7 @@ export function useSessions(session: SupaSession | null) { const fetchSessions = useCallback(async () => { if (!session?.access_token) return - const hubUrl = import.meta.env.VITE_HUB_URL || '' - const res = await fetch(`${hubUrl}/api/sessions`, { - headers: { Authorization: `Bearer ${session.access_token}` }, - }) + const res = await hubFetch('/api/sessions', session.access_token) if (res.ok) { setSessions(await res.json()) } @@ -28,52 +26,39 @@ export function useSessions(session: SupaSession | null) { useEffect(() => { fetchSessions() }, [fetchSessions]) - const createSession = async (name: string, projectDir?: string): Promise => { + const createSession = useCallback(async (name: string, projectDir?: string): Promise => { if (!session?.access_token) return null - const hubUrl = import.meta.env.VITE_HUB_URL || '' - const res = await fetch(`${hubUrl}/api/sessions`, { + const res = await hubFetch('/api/sessions', session.access_token, { method: 'POST', - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${session.access_token}`, - }, body: JSON.stringify({ name, project_dir: projectDir }), }) if (res.ok) { const data = await res.json() await fetchSessions() - return data // includes { ...session, token: "remo_..." } + return data } if (res.status === 403) { return { error: 'session_limit_reached', status: 403 } } return null - } + }, [session?.access_token, fetchSessions]) - const deleteSession = async (id: string) => { + const deleteSession = useCallback(async (id: string) => { if (!session?.access_token) return - const hubUrl = import.meta.env.VITE_HUB_URL || '' - await fetch(`${hubUrl}/api/sessions/${id}`, { - method: 'DELETE', - headers: { Authorization: `Bearer ${session.access_token}` }, - }) + await hubFetch(`/api/sessions/${id}`, session.access_token, { method: 'DELETE' }) await fetchSessions() - } + }, [session?.access_token, fetchSessions]) - const rotateToken = async (id: string) => { + const rotateToken = useCallback(async (id: string) => { if (!session?.access_token) return null - const hubUrl = import.meta.env.VITE_HUB_URL || '' - const res = await fetch(`${hubUrl}/api/sessions/${id}/rotate-token`, { - method: 'POST', - headers: { Authorization: `Bearer ${session.access_token}` }, - }) - if (res.ok) return res.json() // { token: "remo_..." } + const res = await hubFetch(`/api/sessions/${id}/rotate-token`, session.access_token, { method: 'POST' }) + if (res.ok) return res.json() return null - } + }, [session?.access_token]) - const updateSessionStatus = (sessionId: string, status: string) => { + const updateSessionStatus = useCallback((sessionId: string, status: string) => { setSessions(prev => prev.map(s => s.id === sessionId ? { ...s, status } : s)) - } + }, []) return { sessions, setSessions, loading, createSession, deleteSession, rotateToken, updateSessionStatus, refetch: fetchSessions } } diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts new file mode 100644 index 0000000..e3d138d --- /dev/null +++ b/web/src/lib/api.ts @@ -0,0 +1,15 @@ +export const HUB_URL = import.meta.env.VITE_HUB_URL || '' + +export function authHeaders(token: string): Record { + return { + 'Content-Type': 'application/json', + Authorization: `Bearer ${token}`, + } +} + +export function hubFetch(path: string, token: string, init?: RequestInit): Promise { + return fetch(`${HUB_URL}${path}`, { + ...init, + headers: { ...authHeaders(token), ...init?.headers }, + }) +}