diff --git a/CLAUDE.md b/CLAUDE.md index 765d0bd1..54454b5a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -54,7 +54,7 @@ Issue **import + traceability** (#565) is **complete**: `POST /api/v2/integratio **Phase 3.5C is complete** — `CaptureGlitchModal` form (description/markdown, source, scope, gate obligations, severity, expiry) reachable from the PROOF9 page and the persistent sidebar "Capture Glitch" button. REQ detail view (`/proof/[req_id]`) ships markdown description rendering, `ProofScope` metadata display, obligations table with `Latest Run` column, sortable/filterable evidence history, and empty-state CTA. Backend: `ScopeOut` model on `RequirementResponse`. Issues #568, #569. -**v2 API auth enforcement (#336) is complete** — all 22 v2 REST routers require auth (`require_auth`: JWT Bearer or `X-API-Key`) via router-level dependencies in `server.py`; env-gated `CODEFRAME_AUTH_REQUIRED` (default **ON**; set `false` for local dev — read at request time). Streams never carry the JWT in the URL (#745): an authenticated `POST /auth/stream-ticket` (write scope; `has_scope`, so admin implies write) mints a 60s **single-use** ticket (`codeframe/auth/stream_tickets.py`, in-process store — multi-worker caveat documented in the module), redeemed as `?ticket=` **only** on the two SSE routes (allowlist `_QUERY_TICKET_PATHS` in `codeframe/auth/dependencies.py`) and by `authenticate_websocket` for the two WS routes; `?token=` is no longer accepted anywhere. Frontend fetches a fresh ticket per (re)connect via `fetchStreamTicket()`/`withStreamTicket()`; `useEventSource`/`useTerminalSocket` take an async `buildUrl` re-resolved on every retry. `/auth/register` admits only the bootstrap first user (403 after; seeded `!DISABLED!` admin excluded; in-process lock closes the TOCTOU window). Web UI: `/login` page (sign-in + create-first-account), proactive client-side auth guard in `AppLayout` (token-present → allow; no token → `checkAuthAccess` probe → redirect or fail-open; #651), axios Bearer interceptor for reactive 401→`/login` redirect, SSE/WS hooks probe the `require_auth`-gated `/api/v2/settings/keys` (which respects `CODEFRAME_AUTH_REQUIRED`) on stream failure to catch token expiry (#651), SSE hooks append a fresh stream ticket, sidebar logout; `/auth/*` proxied in `next.config.js`. Backend tests run auth-off via root `tests/conftest.py` `setdefault`; `tests/ui/test_v2_auth_enforcement.py` opts back in. +**v2 API auth enforcement (#336) is complete** — all 22 v2 REST routers require auth (`require_auth`: JWT Bearer or `X-API-Key`) via router-level dependencies in `server.py`; env-gated `CODEFRAME_AUTH_REQUIRED` (default **ON**; set `false` for local dev — read at request time). Streams never carry the JWT in the URL (#745): an authenticated `POST /auth/stream-ticket` (write scope; `has_scope`, so admin implies write) mints a 60s **single-use** ticket (`codeframe/auth/stream_tickets.py`, in-process store — multi-worker caveat documented in the module), redeemed as `?ticket=` **only** on the two SSE routes (allowlist `_QUERY_TICKET_PATHS` in `codeframe/auth/dependencies.py`) and by `authenticate_websocket` for the two WS routes; `?token=` is no longer accepted anywhere. Frontend fetches a fresh ticket per (re)connect via `fetchStreamTicket()`/`withStreamTicket()`; `useEventSource`/`useTerminalSocket` take an async `buildUrl` re-resolved on every retry. `/auth/register` admits only the bootstrap first user (403 after; seeded `!DISABLED!` admin excluded; in-process lock closes the TOCTOU window). Web UI: `/login` page (sign-in + create-first-account), proactive client-side auth guard in `AppLayout` (token-present → allow; no token → `checkAuthAccess` probe → allow only on explicit 2xx, else fail closed to `/login`; #651, #783), axios Bearer interceptor for reactive 401→`/login` redirect, SSE/WS hooks probe the `require_auth`-gated `/api/v2/settings/keys` (which respects `CODEFRAME_AUTH_REQUIRED`) on stream failure to catch token expiry (#651), SSE hooks append a fresh stream ticket, sidebar logout; `/auth/*` proxied in `next.config.js`. Backend tests run auth-off via root `tests/conftest.py` `setdefault`; `tests/ui/test_v2_auth_enforcement.py` opts back in. **PROOF9 merge gate is enforced (#731)** — `POST /api/v2/pr/{n}/merge` blocks while open (non-waived) requirements exist (409 with a blocking-requirement summary; ledger failure → explicit 500, never a silent pass-through). Explicit bypass: `override: true` + `override_reason` in `MergePRRequest`; the override is persisted to the `pr_merge_overrides` ledger table (actor from `require_auth`, reason, bypassed requirements, timestamp) and surfaced as `merge_override` on `GET /api/v2/pr/history` items. `cf pr merge` enforces the same gate on the cwd workspace (`--override --reason "..."` to bypass, same audit record; no workspace in cwd → no gate). Scope is workspace-global open requirements — the same condition as the frontend `canMerge` in `PRStatusPanel`. Known follow-ups (not blockers): per-branch scope filtering, `vacuous_pass` distinction (#556), and no frontend override UI yet (API/CLI-only). diff --git a/web-ui/security-headers.js b/web-ui/security-headers.js index 752721f2..c2e6b293 100644 --- a/web-ui/security-headers.js +++ b/web-ui/security-headers.js @@ -30,13 +30,22 @@ function buildCsp(env = process.env) { apiUrl: env.NEXT_PUBLIC_API_URL, wsUrl: env.NEXT_PUBLIC_WS_URL, }); + // unsafe-eval is only needed by the Next.js dev runtime (React Refresh / + // eval source maps); production bundles never eval, so it ships dev-only. + const scriptSrc = env.NODE_ENV === 'development' + ? "script-src 'self' 'unsafe-inline' 'unsafe-eval'" + : "script-src 'self' 'unsafe-inline'"; return [ "default-src 'self'", - // ponytail: 'unsafe-inline'/'unsafe-eval' are required by the Next.js App - // Router without a per-request nonce middleware (a much larger change). - // Exfil containment comes from connect-src/img-src/object-src below — not - // script-src — so the token can't be POSTed/GET'd to an attacker origin. - "script-src 'self' 'unsafe-inline' 'unsafe-eval'", + // ponytail: 'unsafe-inline' is required by the Next.js App Router without + // a per-request nonce middleware (a much larger change). Residual risk + // (#783): with the JWT in localStorage and inline scripts allowed, an + // injected inline script can read the token. connect-src/img-src/ + // object-src below close the fetch/XHR/img/plugin exfil channels, but NOT + // top-level navigation (`window.location = attacker + token`) — CSP has + // no deployable navigate-to directive, so that channel stays open until + // the real fix: nonce middleware and/or an httpOnly-cookie token. + scriptSrc, "style-src 'self' 'unsafe-inline'", `img-src 'self' data: blob: ${AVATAR_HOST}`, "font-src 'self' data:", diff --git a/web-ui/src/__tests__/components/layout/AppLayout.test.tsx b/web-ui/src/__tests__/components/layout/AppLayout.test.tsx index 6bd9a621..6d98aed0 100644 --- a/web-ui/src/__tests__/components/layout/AppLayout.test.tsx +++ b/web-ui/src/__tests__/components/layout/AppLayout.test.tsx @@ -97,7 +97,7 @@ describe('AppLayout', () => { expect(mockReplace).not.toHaveBeenCalled(); }); - it('fails open (renders the shell) when the backend probe errors', async () => { + it('fails closed (redirects to /login, no shell) when the backend probe errors (#783)', async () => { mockIsAuthenticated.mockReturnValue(false); mockCheckAuthAccess.mockResolvedValue('error'); render( @@ -105,7 +105,9 @@ describe('AppLayout', () => {
content
); - expect(await screen.findByTestId('app-sidebar')).toBeInTheDocument(); - expect(mockReplace).not.toHaveBeenCalled(); + await waitFor(() => expect(mockReplace).toHaveBeenCalledWith('/login')); + expect(screen.queryByTestId('app-sidebar')).not.toBeInTheDocument(); + expect(screen.queryByTestId('child')).not.toBeInTheDocument(); + expect(screen.getByRole('status')).toBeInTheDocument(); }); }); diff --git a/web-ui/src/__tests__/components/layout/AppSidebar.navigation.test.tsx b/web-ui/src/__tests__/components/layout/AppSidebar.navigation.test.tsx index 2fe98e42..954805e7 100644 --- a/web-ui/src/__tests__/components/layout/AppSidebar.navigation.test.tsx +++ b/web-ui/src/__tests__/components/layout/AppSidebar.navigation.test.tsx @@ -82,7 +82,7 @@ describe('AppSidebar', () => { expect(screen.getByText('PRD')).toBeInTheDocument(); }); - it('renders all 8 navigation items', () => { + it('renders all 10 navigation items', () => { mockGetWorkspacePath.mockReturnValue('/home/user/projects/test'); render(); @@ -94,21 +94,16 @@ describe('AppSidebar', () => { expect(screen.getByText('Blockers')).toBeInTheDocument(); expect(screen.getByText('Review')).toBeInTheDocument(); expect(screen.getByText('Proof')).toBeInTheDocument(); + expect(screen.getByText('Costs')).toBeInTheDocument(); + expect(screen.getByText('Settings')).toBeInTheDocument(); }); - it('renders enabled items as links', () => { + it('renders navigation items as links (#783: no disabled state exists)', () => { mockGetWorkspacePath.mockReturnValue('/home/user/projects/test'); render(); expect(screen.getByRole('link', { name: /workspace/i })).toHaveAttribute('href', '/'); expect(screen.getByRole('link', { name: /prd/i })).toHaveAttribute('href', '/prd'); - }); - - it('renders all navigation items as links when enabled', () => { - mockGetWorkspacePath.mockReturnValue('/home/user/projects/test'); - render(); - - // All nav items are enabled including Review expect(screen.getByRole('link', { name: /^tasks$/i })).toBeInTheDocument(); expect(screen.getByRole('link', { name: /^execution$/i })).toBeInTheDocument(); expect(screen.getByRole('link', { name: /^blockers$/i })).toBeInTheDocument(); diff --git a/web-ui/src/__tests__/lib/verifyAuthAfterStreamFailure.test.ts b/web-ui/src/__tests__/lib/verifyAuthAfterStreamFailure.test.ts index 7d7dfdb7..be4dde7f 100644 --- a/web-ui/src/__tests__/lib/verifyAuthAfterStreamFailure.test.ts +++ b/web-ui/src/__tests__/lib/verifyAuthAfterStreamFailure.test.ts @@ -70,7 +70,7 @@ describe('checkAuthAccess', () => { await expect(checkAuthAccess()).resolves.toBe('allowed'); expect(mockBareGet).toHaveBeenCalledWith( expect.stringContaining(PROBE_PATH), - expect.objectContaining({ withCredentials: true }) + expect.objectContaining({ withCredentials: true, timeout: 5000 }) ); }); @@ -80,7 +80,7 @@ describe('checkAuthAccess', () => { await expect(checkAuthAccess()).resolves.toBe('denied'); }); - it('returns "error" on a network/other failure (fail open)', async () => { + it('returns "error" on a network/other failure (caller fails closed, #783)', async () => { mockIsAxiosError = () => true; mockBareGet.mockRejectedValue({ response: { status: 503 } }); await expect(checkAuthAccess()).resolves.toBe('error'); diff --git a/web-ui/src/__tests__/security-headers.test.js b/web-ui/src/__tests__/security-headers.test.js index 930634b5..8deb806f 100644 --- a/web-ui/src/__tests__/security-headers.test.js +++ b/web-ui/src/__tests__/security-headers.test.js @@ -38,6 +38,17 @@ describe('security headers (#657)', () => { expect(cs).toContain('ws://localhost:8000'); }); + test('production CSP does not allow eval (#783)', () => { + // unsafe-eval is only needed by the Next.js dev runtime (React Refresh); + // shipping it in production widens the XSS surface for no benefit. + expect(buildCsp({})).not.toContain("'unsafe-eval'"); + expect(buildCsp({ NODE_ENV: 'production' })).not.toContain("'unsafe-eval'"); + }); + + test('dev CSP allows eval for the Next.js dev runtime', () => { + expect(buildCsp({ NODE_ENV: 'development' })).toContain("'unsafe-eval'"); + }); + test('securityHeaders ships the CSP plus the hardening header set', () => { const keys = securityHeaders({}).map((h) => h.key); expect(keys).toEqual( diff --git a/web-ui/src/components/layout/AppLayout.tsx b/web-ui/src/components/layout/AppLayout.tsx index 9eec529e..2a56501d 100644 --- a/web-ui/src/components/layout/AppLayout.tsx +++ b/web-ui/src/components/layout/AppLayout.tsx @@ -41,7 +41,9 @@ function FullPageLoader() { * permitted before deciding: * - allowed (valid token, or auth disabled) → render the shell; * - denied (auth required, no token) → redirect to /login; - * - error (backend unreachable) → fail open, render the shell. + * - error (backend unreachable) → fail closed, redirect to + * /login (#783) — the shell is useless without a backend anyway, and the + * login page surfaces the connectivity error on submit. * * Throughout, an unauthenticated visitor only ever sees a neutral loader — the * sidebar/shell never renders for them, so there's no "shell → flicker → login" @@ -73,11 +75,13 @@ export function AppLayout({ children }: AppLayoutProps) { let cancelled = false; checkAuthAccess().then((result) => { if (cancelled) return; - if (result === 'denied') { - router.replace('/login'); - } else { - // 'allowed' (auth disabled) or 'error' (fail open) → render the app. + if (result === 'allowed') { + // Auth disabled on the backend → token-less access is fine. setAccess('allowed'); + } else { + // 'denied' (auth required) or 'error' (backend unreachable) → fail + // closed to the login page (#783). + router.replace('/login'); } }); return () => { diff --git a/web-ui/src/components/layout/AppSidebar.tsx b/web-ui/src/components/layout/AppSidebar.tsx index 54cdb8c5..b8d348ea 100644 --- a/web-ui/src/components/layout/AppSidebar.tsx +++ b/web-ui/src/components/layout/AppSidebar.tsx @@ -31,20 +31,19 @@ interface NavItem { href: string; label: string; icon: IconSvgElement; - enabled: boolean; } const NAV_ITEMS: NavItem[] = [ - { href: '/', label: 'Workspace', icon: Home01Icon, enabled: true }, - { href: '/prd', label: 'PRD', icon: FileEditIcon, enabled: true }, - { href: '/tasks', label: 'Tasks', icon: Task01Icon, enabled: true }, - { href: '/execution', label: 'Execution', icon: PlayIcon, enabled: true }, - { href: '/sessions', label: 'Sessions', icon: CommandLineIcon, enabled: true }, - { href: '/blockers', label: 'Blockers', icon: Alert02Icon, enabled: true }, - { href: '/review', label: 'Review', icon: GitBranchIcon, enabled: true }, - { href: '/proof', label: 'Proof', icon: CheckmarkCircle01Icon, enabled: true }, - { href: '/costs', label: 'Costs', icon: ChartLineData01Icon, enabled: true }, - { href: '/settings', label: 'Settings', icon: Settings01Icon, enabled: true }, + { href: '/', label: 'Workspace', icon: Home01Icon }, + { href: '/prd', label: 'PRD', icon: FileEditIcon }, + { href: '/tasks', label: 'Tasks', icon: Task01Icon }, + { href: '/execution', label: 'Execution', icon: PlayIcon }, + { href: '/sessions', label: 'Sessions', icon: CommandLineIcon }, + { href: '/blockers', label: 'Blockers', icon: Alert02Icon }, + { href: '/review', label: 'Review', icon: GitBranchIcon }, + { href: '/proof', label: 'Proof', icon: CheckmarkCircle01Icon }, + { href: '/costs', label: 'Costs', icon: ChartLineData01Icon }, + { href: '/settings', label: 'Settings', icon: Settings01Icon }, ]; export function AppSidebar() { @@ -106,24 +105,11 @@ export function AppSidebar() { {/* Navigation */}