diff --git a/CLAUDE.md b/CLAUDE.md index 5385bd8..f871565 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -194,7 +194,7 @@ PR 12 (owner's script Parts 1–3: AI review layer, Control Point attach bundle) Audit 1 (security, from the full audit of 2026-10-25): - **OAuth state.** `/oauth/start` no longer writes `oauth-state:*` to KV (anyone could spend the 1,000 daily free-plan writes by opening it). The state rides in a signed, HttpOnly `controlkit_oauth` cookie (HMAC-SHA-256, key derived from `SESSION_ENCRYPTION_KEY`, `Path=/oauth`, 10 minutes), so a redirect also completes only in the browser that started sign-in (no login CSRF). A one-use `oauth-used:{state}` key is written only after Onshape accepts the code, so made-up callbacks write nothing. -- **Session.** Only `{id, name}` from `sessioninfo` is kept. `/api/session` re-saves a session at most once a day (`savedAt`). `/api/logout` checks Origin. +- **Session.** Only `{id, name}` from `sessioninfo` is kept. `/api/session` re-saves a session at most once a day (`savedAt`). `/api/logout` checks Origin. The `controlkit_session` cookie carries `Max-Age` 30 days (2026-10-29, owner: "why do i have to connect onshape everytime"): it had none, so the browser dropped it on every close while the KV session lived on. Each `/api/session` answer sets it again, restarting the 30 days without a KV write, so a team that opens the panel at least monthly stays connected. - **AI route.** `trustedAiRequest` rebuilds every line from the caller's own stored scan (name, description, material, part number, configuration, places used, quantity, source, review reason). Only the browser's catalog candidates (each id must equal its vendor plus normalized SKU, names cut to 160 plain characters) and its enum-checked conclusion are kept; unknown fields are dropped. Body cap 150 KB. Cached answers write nothing. The cache key includes both configured models. A blank key counts as no key, for enabling and for choosing the provider. - **AI allowance.** A Durable Object, `AiAllowance` (`apps/worker/src/ai-allowance.ts`, binding `AI_ALLOWANCE`, SQLite-backed so it runs on the free plan), holds the per-person 20/day, the global 300/day, and the 200-second per-person lock. It reserves atomically, so concurrent reviews cannot exceed either limit. It decides the UTC day itself, drops older days' counts on a new day, and keeps running-review locks across midnight. AI lines join the scan by `bomLineKey` (sent beside the request, never to the model), not by name. The review is off without the binding. - **Shared part types.** An override must name a part that is in the writer's scan. diff --git a/apps/worker/src/index.test.ts b/apps/worker/src/index.test.ts index 63d5c2f..d8fa97b 100644 --- a/apps/worker/src/index.test.ts +++ b/apps/worker/src/index.test.ts @@ -180,8 +180,13 @@ describe('Onshape OAuth and session against mock Onshape', () => { it('keeps only the Onshape id and name, refreshes the session at most daily, and refuses cross-site logout', async () => { const start = await call('/oauth/start'); const state = new URL(start.headers.get('location')!).searchParams.get('state')!; - const cookie = (await call(`/oauth/redirect?code=mock-code&state=${state}`)).headers.get('set-cookie')!.split(';')[0]; - expect(Object.keys((await (await call('/api/session', cookie)).json()).user).sort()).toEqual(['id', 'name']); + const issued = (await call(`/oauth/redirect?code=mock-code&state=${state}`)).headers.get('set-cookie')!; + const cookie = issued.split(';')[0]; + // The session cookie outlives the browser: 30 days, like the stored session, restarted on each panel load. + expect(issued).toMatch(/^controlkit_session=[^;]+; HttpOnly; Secure; SameSite=None; Path=\/; Max-Age=2592000/); + const panel = await call('/api/session', cookie); + expect(panel.headers.get('set-cookie')).toBe(`${cookie}; HttpOnly; Secure; SameSite=None; Path=/; Max-Age=2592000`); + expect(Object.keys((await panel.json()).user).sort()).toEqual(['id', 'name']); const key = `session:${cookie.split('=')[1]}`; const saved = env.APP_STATE.values.get(key); await call('/api/session', cookie); diff --git a/apps/worker/src/index.ts b/apps/worker/src/index.ts index 819ff59..a0af5b6 100644 --- a/apps/worker/src/index.ts +++ b/apps/worker/src/index.ts @@ -22,6 +22,13 @@ type OAuthState = { context?: Record; redirectOnshapeUri?: strin const cookieName = 'controlkit_session'; // SameSite=None is required because Onshape embeds Control Kit in a cross-site iframe. const cookieFlags = 'HttpOnly; Secure; SameSite=None; Path=/'; +/** + * A session lasts 30 days from its last use, in KV and in the browser alike. Without Max-Age the cookie was a browser + * session cookie, dropped whenever the browser closed, so people had to connect Onshape again though the session was + * still stored. + */ +const sessionTtl = 30 * 86400; +const sessionCookie = (id: string) => `${cookieName}=${id}; ${cookieFlags}; Max-Age=${sessionTtl}`; const encode = new TextEncoder(); const random = () => crypto.randomUUID(); async function lineKeyDigest(lineKey: string) { @@ -114,7 +121,7 @@ async function getSession(request: Request, env: Env) { if (!id || !value) return null; try { return { id, data: await unseal(env, value) }; } catch { return null; } } -async function save(env: Env, id: string, data: Session) { data.savedAt = Date.now(); await env.APP_STATE.put(`session:${id}`, await seal(env, data), { expirationTtl: 2592000 }); } +async function save(env: Env, id: string, data: Session) { data.savedAt = Date.now(); await env.APP_STATE.put(`session:${id}`, await seal(env, data), { expirationTtl: sessionTtl }); } /** * OAuth state lives in a signed, HttpOnly cookie on the browser that started sign-in, not in KV: starting sign-in * writes nothing (so nobody can spend the daily KV writes by opening /oauth/start), and a redirect only completes in @@ -225,7 +232,7 @@ async function route(request: Request, env: Env): Promise { const destination = stateData.redirectOnshapeUri ? new URL(stateData.redirectOnshapeUri) : new URL('/', env.WEB_ORIGIN); for (const [k, v] of Object.entries(stateData.context ?? {})) destination.searchParams.set(k, v); const headers = new Headers({ location: destination.toString() }); - headers.append('set-cookie', `${cookieName}=${id}; ${cookieFlags}`); + headers.append('set-cookie', sessionCookie(id)); headers.append('set-cookie', clearCookie(stateCookie)); return new Response(null, { status: 302, headers }); } @@ -235,7 +242,11 @@ async function route(request: Request, env: Env): Promise { if (!current) return reply({ connected: false }, 401); // Keep an active session alive, but write it at most once a day rather than on every panel load. if (!current.data.savedAt || Date.now() - current.data.savedAt > 86_400_000) await save(env, current.id, current.data); - return reply({ connected: true, user: { id: current.data.user.id, name: current.data.user.name } }); + // The cookie's 30 days restart on every panel load (a header, not a KV write), so a team that opens Control Kit at + // least once a month never connects again; it also gives a cookie set before Max-Age existed its lifetime. + const answer = reply({ connected: true, user: { id: current.data.user.id, name: current.data.user.name } }); + answer.headers.append('set-cookie', sessionCookie(current.id)); + return answer; } if (url.pathname === '/api/logout' && request.method === 'POST') { if (request.headers.get('origin') !== env.WEB_ORIGIN) return reply({ error: 'Invalid request origin.' }, 403);