Frontend hardening (8 findings)

frontend/src/App.tsx

@@ -1,12 +1,13 @@
 // Orchestration Engine - App Router
 //
-// Depends on: hooks/useAuth.tsx, components/AuthGuard.tsx, components/Layout.tsx,
-//             components/ErrorBoundary.tsx
+// Depends on: hooks/useAuth.tsx, components/AuthGuard.tsx, components/RequireRole.tsx,
+//             components/Layout.tsx, components/ErrorBoundary.tsx
 // Used by:    main.tsx
 
 import { BrowserRouter, Routes, Route } from 'react-router-dom'
 import { AuthProvider } from './hooks/useAuth'
 import AuthGuard from './components/AuthGuard'
+import RequireRole from './components/RequireRole'
 import ErrorBoundary from './components/ErrorBoundary'
 import Layout from './components/Layout'
 import Login from './pages/Login'
@@ -41,9 +42,12 @@ export default function App() {
                 <Route path="/project/:id/task/:taskId" element={<TaskDetail />} />
                 <Route path="/usage" element={<Usage />} />
                 <Route path="/services" element={<Services />} />
-                <Route path="/admin" element={<Admin />} />
-                <Route path="/analytics" element={<Analytics />} />
                 <Route path="/rag" element={<RAG />} />
+                {/* Admin-only routes */}
+                <Route element={<RequireRole role="admin" />}>
+                  <Route path="/admin" element={<Admin />} />
+                  <Route path="/analytics" element={<Analytics />} />
+                </Route>
               </Route>
             </Route>
 

frontend/src/api/client.ts

@@ -10,7 +10,7 @@ import { getAccessToken, apiRefresh } from './auth'
 
 const BASE = '/api'
 
-async function authFetch(path: string, init?: RequestInit): Promise<Response> {
+export async function authFetch(path: string, init?: RequestInit): Promise<Response> {
   const token = getAccessToken()
   const headers = new Headers(init?.headers)
   if (token) {

frontend/src/api/projects.ts

@@ -1,6 +1,6 @@
 // Orchestration Engine - Projects API
 
-import { apiFetch, apiPost, apiPatch, apiDelete } from './client'
+import { apiFetch, apiPost, apiPatch, apiDelete, authFetch } from './client'
 import type { Project, Plan, Task, Checkpoint, CoverageReport, PlanningRigor } from '../types'
 
 export const listProjects = (status?: string) =>
@@ -83,11 +83,7 @@ export const cloneProject = (projectId: string) =>
   apiPost<Project>(`/projects/${projectId}/clone`)
 
 export const exportProject = async (projectId: string) => {
-  const { getAccessToken } = await import('./auth')
-  const token = getAccessToken()
-  const resp = await fetch(`/api/projects/${projectId}/export`, {
-    headers: token ? { Authorization: `Bearer ${token}` } : {},
-  })
+  const resp = await authFetch(`/projects/${projectId}/export`)
   if (!resp.ok) throw new Error('Export failed')
   const blob = await resp.blob()
   const url = URL.createObjectURL(blob)

frontend/src/components/Layout.tsx

@@ -13,7 +13,7 @@ export default function Layout() {
 
   return (
     <div className="layout">
-      <nav className="sidebar">
+      <nav className="sidebar" aria-label="Main navigation">
         <h1>Orchestration</h1>
         <NavLink to="/" className={({ isActive }) => isActive ? 'active' : ''} end>
           Dashboard

frontend/src/components/Modal.test.tsx

@@ -37,4 +37,20 @@ describe('Modal', () => {
     fireEvent.click(screen.getByText('Content'))
     expect(onClose).not.toHaveBeenCalled()
   })
+
+  it('has correct ARIA attributes', () => {
+    render(<Modal open={true} onClose={vi.fn()} title="Accessible Modal">Body</Modal>)
+    const dialog = screen.getByRole('dialog')
+    expect(dialog).toHaveAttribute('aria-modal', 'true')
+    expect(dialog).toHaveAttribute('aria-labelledby')
+
+    // Title is connected via aria-labelledby
+    const titleId = dialog.getAttribute('aria-labelledby')!
+    expect(document.getElementById(titleId)?.textContent).toBe('Accessible Modal')
+  })
+
+  it('close button has aria-label', () => {
+    render(<Modal open={true} onClose={vi.fn()} title="Test">Content</Modal>)
+    expect(screen.getByLabelText('Close')).toBeInTheDocument()
+  })
 })

frontend/src/components/Modal.tsx

@@ -1,11 +1,12 @@
 // Orchestration Engine - Modal Component
 //
-// Portal-based modal with backdrop click and Escape key to close.
+// Portal-based modal with backdrop click, Escape key, focus trap,
+// and ARIA attributes for accessibility.
 //
 // Depends on: (none)
 // Used by:    pages/TaskDetail.tsx
 
-import { useEffect, useRef } from 'react'
+import { useEffect, useRef, useId } from 'react'
 import { createPortal } from 'react-dom'
 
 interface ModalProps {
@@ -17,7 +18,28 @@ interface ModalProps {
 
 export default function Modal({ open, onClose, title, children }: ModalProps) {
   const overlayRef = useRef<HTMLDivElement>(null)
+  const contentRef = useRef<HTMLDivElement>(null)
+  const previousFocusRef = useRef<Element | null>(null)
+  const titleId = useId()
 
+  // Save/restore focus and lock body scroll
+  useEffect(() => {
+    if (!open) return
+    previousFocusRef.current = document.activeElement
+    document.body.style.overflow = 'hidden'
+
+    // Focus the modal content for keyboard users
+    requestAnimationFrame(() => {
+      contentRef.current?.focus()
+    })
+
+    return () => {
+      document.body.style.overflow = '';
+      (previousFocusRef.current as HTMLElement)?.focus?.()
+    }
+  }, [open])
+
+  // Escape key
   useEffect(() => {
     if (!open) return
     const handleEsc = (e: KeyboardEvent) => {
@@ -27,6 +49,29 @@ export default function Modal({ open, onClose, title, children }: ModalProps) {
     return () => window.removeEventListener('keydown', handleEsc)
   }, [open, onClose])
 
+  // Focus trap: Tab cycles within modal
+  useEffect(() => {
+    if (!open) return
+    const handleTab = (e: KeyboardEvent) => {
+      if (e.key !== 'Tab' || !contentRef.current) return
+      const focusable = contentRef.current.querySelectorAll<HTMLElement>(
+        'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
+      )
+      if (focusable.length === 0) return
+      const first = focusable[0]
+      const last = focusable[focusable.length - 1]
+      if (e.shiftKey && document.activeElement === first) {
+        e.preventDefault()
+        last.focus()
+      } else if (!e.shiftKey && document.activeElement === last) {
+        e.preventDefault()
+        first.focus()
+      }
+    }
+    window.addEventListener('keydown', handleTab)
+    return () => window.removeEventListener('keydown', handleTab)
+  }, [open])
+
   if (!open) return null
 
   return createPortal(
@@ -35,12 +80,20 @@ export default function Modal({ open, onClose, title, children }: ModalProps) {
       ref={overlayRef}
       onClick={e => { if (e.target === overlayRef.current) onClose() }}
     >
-      <div className="modal-content">
+      <div
+        className="modal-content"
+        ref={contentRef}
+        role="dialog"
+        aria-modal="true"
+        aria-labelledby={titleId}
+        tabIndex={-1}
+      >
         <div className="flex-between mb-2">
-          <h3 style={{ margin: 0 }}>{title}</h3>
+          <h3 id={titleId} style={{ margin: 0 }}>{title}</h3>
           <button
             className="btn btn-sm"
             onClick={onClose}
+            aria-label="Close"
             style={{ background: 'transparent', color: 'var(--text-dim)' }}
           >
             &times;

frontend/src/components/RequireRole.test.tsx

@@ -0,0 +1,74 @@
+import { describe, it, expect, vi } from 'vitest'
+import { render, screen } from '@testing-library/react'
+import { MemoryRouter, Route, Routes } from 'react-router-dom'
+import RequireRole from './RequireRole'
+
+vi.mock('../hooks/useAuth', () => ({
+  useAuth: vi.fn(),
+}))
+
+import { useAuth } from '../hooks/useAuth'
+
+const mockUseAuth = vi.mocked(useAuth)
+
+function renderWithRouter(role: string, userRole?: string, userPresent = true) {
+  mockUseAuth.mockReturnValue({
+    user: userPresent ? { id: '1', email: 'test@example.com', display_name: 'Test', role: userRole! } : null,
+    loading: false,
+    login: vi.fn(),
+    logout: vi.fn(),
+    loginWithOIDC: vi.fn(),
+    setUserFromOIDC: vi.fn(),
+  })
+
+  return render(
+    <MemoryRouter initialEntries={['/admin']}>
+      <Routes>
+        <Route element={<RequireRole role={role} />}>
+          <Route path="/admin" element={<div>Admin Content</div>} />
+        </Route>
+      </Routes>
+    </MemoryRouter>
+  )
+}
+
+describe('RequireRole', () => {
+  it('renders outlet for correct role', () => {
+    renderWithRouter('admin', 'admin')
+    expect(screen.getByText('Admin Content')).toBeInTheDocument()
+  })
+
+  it('denies access for wrong role', () => {
+    renderWithRouter('admin', 'user')
+    expect(screen.queryByText('Admin Content')).not.toBeInTheDocument()
+    expect(screen.getByText(/Access denied/)).toBeInTheDocument()
+  })
+
+  it('denies access when no user', () => {
+    renderWithRouter('admin', undefined, false)
+    expect(screen.queryByText('Admin Content')).not.toBeInTheDocument()
+    expect(screen.getByText(/Access denied/)).toBeInTheDocument()
+  })
+
+  it('renders custom fallback', () => {
+    mockUseAuth.mockReturnValue({
+      user: { id: '1', email: 'a@b.com', display_name: 'X', role: 'user' },
+      loading: false,
+      login: vi.fn(),
+      logout: vi.fn(),
+      loginWithOIDC: vi.fn(),
+      setUserFromOIDC: vi.fn(),
+    })
+
+    render(
+      <MemoryRouter initialEntries={['/admin']}>
+        <Routes>
+          <Route element={<RequireRole role="admin" fallback={<div>Custom Denied</div>} />}>
+            <Route path="/admin" element={<div>Admin Content</div>} />
+          </Route>
+        </Routes>
+      </MemoryRouter>
+    )
+    expect(screen.getByText('Custom Denied')).toBeInTheDocument()
+  })
+})

frontend/src/components/RequireRole.tsx

@@ -0,0 +1,29 @@
+// Orchestration Engine - Role Guard Component
+//
+// Route-level guard that restricts access by user role.
+// Renders <Outlet /> for authorized users, denial message otherwise.
+//
+// Depends on: hooks/useAuth.tsx
+// Used by:    App.tsx
+
+import { Outlet } from 'react-router-dom'
+import { useAuth } from '../hooks/useAuth'
+
+interface RequireRoleProps {
+  role: string
+  fallback?: React.ReactNode
+}
+
+export default function RequireRole({ role, fallback }: RequireRoleProps) {
+  const { user } = useAuth()
+
+  if (!user || user.role !== role) {
+    return fallback ? <>{fallback}</> : (
+      <p className="text-dim" style={{ padding: '2rem' }}>
+        Access denied. {role} role required.
+      </p>
+    )
+  }
+
+  return <Outlet />
+}

frontend/src/hooks/useFetch.test.ts

@@ -66,6 +66,27 @@ describe('useFetch', () => {
     expect(result.current.error).toBeNull()
   })
 
+  it('does not setState after unmount (cancellation guard)', async () => {
+    let resolve: (v: string) => void
+    const fetchFn = vi.fn().mockReturnValue(
+      new Promise<string>(r => { resolve = r })
+    )
+
+    const { result, unmount } = renderHook(() => useFetch(fetchFn))
+
+    // Still loading
+    expect(result.current.loading).toBe(true)
+
+    // Unmount before the fetch resolves
+    unmount()
+
+    // Resolve the fetch — should NOT throw or update state
+    resolve!('late-data')
+
+    // No error means the cancellation guard prevented setState after unmount
+    // (React would warn about setState on unmounted component without the guard)
+  })
+
   it('re-fetches when deps change', async () => {
     const fetchFn = vi.fn().mockResolvedValue('data')
     let dep = 'a'

frontend/src/hooks/useFetch.ts

@@ -25,20 +25,24 @@ export function useFetch<T>(
   const fetchRef = useRef(fetchFn)
   fetchRef.current = fetchFn
 
+  const cancelledRef = useRef(false)
+
   const doFetch = useCallback(async () => {
+    cancelledRef.current = false
     setLoading(true)
     setError(null)
     try {
       const result = await fetchRef.current()
-      setData(result)
+      if (!cancelledRef.current) setData(result)
     } catch (e) {
-      setError(String(e))
+      if (!cancelledRef.current) setError(String(e))
     }
-    setLoading(false)
+    if (!cancelledRef.current) setLoading(false)
   }, [])
 
   useEffect(() => {
     doFetch()
+    return () => { cancelledRef.current = true }
     // eslint-disable-next-line react-hooks/exhaustive-deps
   }, deps)