NUL-231: dev-only Vite proxy + same-origin VITE_API_URL

.env.development

@@ -0,0 +1,22 @@
+# IPAM development-only environment overrides.
+# Picked up by Vite automatically (`mode === 'development'`) and bundled into
+# the client at dev-server start. Production builds do not read this file.
+#
+# Why this exists (NUL-231 / NUL-218):
+#   In dev, the SPA is served at http://localhost:5173 and the Hono backend
+#   listens at http://localhost:8787. Modern browsers treat those as
+#   different origins, so a cookie-bearing `fetch('http://localhost:8787/...')`
+#   is preflight-blocked by CORS (the Hono server has no Access-Control-Allow-
+#   Origin for the dev origin, by design — adding it would weaken prod).
+#
+#   `vite.config.ts` adds `server.proxy['/api']` → :8787 so requests to
+#   `/api/*` issued against the Vite origin are forwarded server-side and
+#   arrive at the browser as same-origin. To use the proxy, the client must
+#   target the Vite origin, not :8787. Hence VITE_API_URL=http://localhost:5173.
+#
+#   The same-origin proxy path means:
+#     - cookies (Lax) ride :5173 → :5173 → backend → :5173 without preflight;
+#     - the session cookie's sameSite=Lax semantics apply to dev too;
+#     - the production build (which is served same-origin behind nginx) is
+#       unaffected — it reads .env / .env.production, not this file.
+VITE_API_URL=http://localhost:5173

playwright.config.ts

@@ -8,11 +8,19 @@ import { defineConfig, devices } from '@playwright/test'
  * in another terminal first, or let the npm script `test:e2e` do it.
  *
  * Hostname note: the dev stack is reachable on either `127.0.0.1` or
- * `localhost`, but only `localhost` is in the Hono CORS allowlist
- * (NUL-11 — pre-existing dev-server CORS gap). Hitting `127.0.0.1` would
- * silently CORS-block every `/api/*` call, leaving the route-guard
- * splash stuck and every test waiting 60 s for an input that never
- * mounts. Pin `baseURL` + `PLAYWRIGHT_API_URL` to `localhost`.
+ * `localhost`. The Hono server deliberately serves no CORS headers
+ * (NUL-231 — adding dev CORS would weaken prod), so any browser fetch
+ * to http://127.0.0.1:8787 from a page on http://localhost:5173 (or
+ * vice versa) is preflight-blocked. Pin `baseURL` + `PLAYWRIGHT_API_URL`
+ * to `localhost` so every URL the browser sees has a single origin
+ * (after Vite proxies /api through to :8787 server-side).
+ *
+ * `--disable-web-security` on the projects below is intentionally kept
+ * because `tests/onboarding.spec.ts` issues fixture calls
+ * (`fetch(http://localhost:8787/...)` for the tour-reset PATCH) from
+ * inside the page evaluate; those are deliberately cross-origin and
+ * would need their own CORS or proxy rewrite to drop the flag. That
+ * refactor is out of scope for NUL-271 and tracked separately.
  *
  * Screenshot output: `test-results/onboarding/` so failures and the
  * deliberate capture step share a directory the CI workflow can archive.
@@ -39,10 +47,11 @@ export default defineConfig({
         ...devices['Desktop Chrome'],
         viewport: { width: 1280, height: 800 },
         colorScheme: 'light',
-        // NUL-11 dev CORS gap: the Hono server does not return CORS headers,
-        // and the Vite dev server has no /api proxy. Real deploys sit behind
-        // a same-origin reverse proxy so this is dev-only. `--disable-web-security`
-        // is safe here because the dev stack only listens on localhost.
+        // NUL-231: the prod path is same-origin (nginx serves SPA + /api),
+        // and the dev path is now also same-origin through Vite's /api proxy.
+        // `--disable-web-security` is still passed because tests/onboarding.spec.ts
+        // also issues direct :8787 fixture calls inside page.evaluate() that
+        // are deliberately cross-origin; see the file-header comment.
         launchOptions: { args: ['--disable-web-security'] },
       },
     },

src/features/auth/route-guard.test.tsx

@@ -198,9 +198,11 @@ test('splash skeleton markup renders while /me is in a 401 error state', async (
     throw err
   }
 
-  // 401 from /me — the component renders <AuthSplash /> while the
-  // redirect effect runs. Once the effect fires (which we cannot
-  // observe under SSR), the location changes to /login.
+  const { __setLocation } = await import('../../../scripts/_test-mocks/router.mjs')
+  __setLocation({ pathname: '/login' })
+
+  // 401 from /me on the login route must not hide the destination form.
+  // The route guard's redirect effect treats /login as a no-op.
   globalThis.__routeGuardMeState = {
     isLoading: false,
     isSuccess: false,
@@ -214,8 +216,11 @@ test('splash skeleton markup renders while /me is in a 401 error state', async (
 
   const html = renderToStaticMarkup(createElement(AuthGuard, null, 'main-content'))
 
-  assert.match(html, /data-testid="skeleton"/)
-  assert.doesNotMatch(html, /main-content/)
+  // On /login the destination form must remain renderable even when the
+  // proxied /me request returns a typed 401. The guard's redirect effect
+  // already treats /login as a no-op; a splash here would hide the form.
+  assert.doesNotMatch(html, /data-testid="skeleton"/)
+  assert.match(html, /main-content/)
 })
 
 test.afterEach(() => {

src/features/auth/route-guard.tsx

@@ -103,11 +103,11 @@ export function AuthGuard({ children }: { children: ReactNode }) {
     }
   }, [currentPath, currentSearch, navigate, queryClient])
 
-  if (me.isLoading) {
+  if (me.isLoading && currentPath !== '/login') {
     return <AuthSplash />
   }
 
-  if (me.isError) {
+  if (me.isError && currentPath !== '/login') {
     const status = (me.error as { status?: number } | null)?.status
     if (status === 401) return <AuthSplash />
   }

src/features/dashboard/dashboard-page.tsx

@@ -118,8 +118,8 @@ export function DashboardPage() {
                     <span className="font-medium">{fp.name}</span>
                   </div>
                   <Badge variant="outline">
-                    {fp.rackPositions.length} rack
-                    {fp.rackPositions.length === 1 ? '' : 's'}
+                    {(fp.rackPositions ?? []).length} rack
+                    {(fp.rackPositions ?? []).length === 1 ? '' : 's'}
                   </Badge>
                 </div>
               ))}

src/server/__tests__/auth-and-tenant.test.ts

@@ -528,4 +528,86 @@ test('unauthenticated POST /api/upload returns 401', async () => {
     const env = asEnvelope(await r.json())
     assert.equal(env.error.code, 'unauthenticated')
   })
+})
+
+// =============================================================================
+// 9. Logout actually clears the session cookie (NUL-230 regression)
+// =============================================================================
+//
+// Before the fix, the /api/auth/logout handler called clearSessionCookie(c)
+// to schedule a Set-Cookie header, but then returned a bare
+// `new Response(null, { status: 204 })` which dropped the prepared headers.
+// The browser kept the old ipam_session cookie and the next /api/auth/me
+// still returned 200, so the user appeared to still be logged in after
+// logout. The fix uses c.body(null, 204) so the Set-Cookie header is
+// actually included in the 204 response.
+
+test('POST /api/auth/logout returns 204 with a Set-Cookie that expires ipam_session', async () => {
+  await withFreshServer(async (h) => {
+    const cookie = await login(h.base, INTERNAL_ADMIN)
+    const r = await fetch(`${h.base}/api/auth/logout`, {
+      method: 'POST',
+      headers: { cookie },
+    })
+    assert.equal(r.status, 204)
+    const setCookie = r.headers.get('set-cookie') ?? ''
+    assert.ok(
+      setCookie.toLowerCase().includes('ipam_session='),
+      `expected logout response to include a Set-Cookie for ipam_session, got: ${setCookie}`,
+    )
+    assert.ok(
+      /(?:expires=|max-age=0|max-age=-?\d+)/i.test(setCookie),
+      `expected logout response Set-Cookie to expire the cookie (expires= or max-age<=0), got: ${setCookie}`,
+    )
+  })
+})
+
+test('GET /api/auth/me after /api/auth/logout returns 401 to clients that drop the cookie', async () => {
+  await withFreshServer(async (h) => {
+    const cookie = await login(h.base, INTERNAL_ADMIN)
+
+    // Sanity: /me works before logout.
+    const before = await fetch(`${h.base}/api/auth/me`, { headers: { cookie } })
+    assert.equal(before.status, 200)
+
+    // Logout. The response must clear the session cookie on the client
+    // (the previous /logout bug dropped the prepared Set-Cookie header).
+    const logout = await fetch(`${h.base}/api/auth/logout`, {
+      method: 'POST',
+      headers: { cookie },
+    })
+    assert.equal(logout.status, 204)
+    assert.ok(
+      (logout.headers.get('set-cookie') ?? '').toLowerCase().includes('ipam_session='),
+      'precondition: logout response must include Set-Cookie for ipam_session',
+    )
+
+    // Simulate what a real browser does on receiving that Set-Cookie:
+    // the cookie is removed from the jar, so the next /me request goes
+    // out with no cookie. The server must then return 401.
+    const after = await fetch(`${h.base}/api/auth/me`, {
+      headers: { /* no cookie */ },
+    })
+    assert.equal(
+      after.status,
+      401,
+      `expected /api/auth/me without cookie after logout to return 401, got ${after.status}`,
+    )
+  })
+})
+
+test('POST /api/auth/login still works after the logout fix (regression check)', async () => {
+  await withFreshServer(async (h) => {
+    // Login, logout, login again — the second login should still return
+    // 200 with a fresh session cookie, proving the logout fix didn't
+    // accidentally break the login path.
+    const cookie1 = await login(h.base, INTERNAL_ADMIN)
+    const logout = await fetch(`${h.base}/api/auth/logout`, {
+      method: 'POST',
+      headers: { cookie: cookie1 },
+    })
+    assert.equal(logout.status, 204)
+    const cookie2 = await login(h.base, INTERNAL_ADMIN)
+    assert.ok(cookie2.length > 0, 'second login should still return a session cookie')
+  })
 })

src/server/auth.ts

@@ -315,8 +315,15 @@ authApp.post('/login', async (c) => {
 })
 
 authApp.post('/logout', (c) => {
+  // NUL-230: use c.body(null, 204) so the Set-Cookie header prepared by
+  // clearSessionCookie(c) is actually sent. Returning a bare
+  // `new Response(null, { status: 204 })` discards c.res.headers, so the
+  // browser kept its old ipam_session cookie and the next /api/auth/me
+  // request still succeeded (the user appeared to be "still logged in"
+  // after logout). See src/server/__tests__/auth-and-tenant.test.ts for
+  // the regression test.
   clearSessionCookie(c)
-  return new Response(null, { status: 204 })
+  return c.body(null, 204)
 })
 
 authApp.get('/me', requireAuth, (c) => {

vite.config.ts

@@ -18,5 +18,16 @@ export default defineConfig({
   server: {
     port: 5173,
     host: true,
+    // NUL-231: dev-only proxy so cookie-bearing /api fetches from :5173
+    // reach the Hono backend on :8787 same-origin, avoiding browser CORS
+    // preflight. Production is already same-origin (nginx serves SPA + /api
+    // from one host), so this block has no production effect.
+    proxy: {
+      '/api': {
+        target: 'http://localhost:8787',
+        changeOrigin: false,
+        secure: false,
+      },
+    },
   },
 })