fix(security): store API key only in httpOnly cookies (#77)

[teddylee777]

Context Block / 컨텍스트 블록

Key	Value
Type	fix(security)
Scope	frontend / auth / connection
Base	main
Branch	fix/issue-77-apikey-storage
Diff	+38 / -20
Files	5 changed

Summary / 요약

• WHAT: Funnel every LangSmith API-key write through a server action so the value lives only in an httpOnly cookie. Removes a document.cookie = write in ApiKeyLoginForm and a localStorage.setItem in ConnectionList. Deletes the now-dead getApiKey() helper.
• WHY: Both writers stored the API key in JS-readable storage, giving any XSS payload an immediate exfiltration channel for a paid LangSmith key (issue 🟠[P1] fix(frontend): API key written to JS-readable cookie via document.cookie + localStorage #77, P1).
• HOW: Added setApiKeyCookieAction in frontend/src/app/actions.ts that mirrors the existing secureOptions pattern from updateConnectionAction. The proxy at frontend/src/app/api/[..._path]/route.ts:122-127 already reads the cookie server-side, so no consumer changes are needed.
• IMPACT: API-key login mode users (cookie now set via server action) and multi-connection users (no functional change, the switchConnection server action already wrote the same cookie). No backend changes.
• RISK: Low — the safe path already existed in code; this PR removes the duplicate JS-readable writes.

Motivation / Context / 동기 / 배경

Problem Statement / 문제 정의

ApiKeyLoginForm.tsx:71 set the API key via document.cookie = "lg_apiKey=..." without an httpOnly flag, and ConnectionList.tsx:113 mirrored the key into localStorage. Either copy is readable from any JavaScript that runs in the page (XSS, malicious dependency), so an attacker could exfiltrate the LangSmith API key and run up usage charges or download trace data.

Trigger / 트리거

Detected by omb:issue scan (issue #77, checklist ISS-UI-R9). Same vulnerability class as #58 (NEXT_PUBLIC_LANGCHAIN_API_KEY browser exposure).

Prior State / 이전 상태

A safe path already existed: updateConnectionAction in frontend/src/app/actions.ts writes connection cookies with httpOnly: isProduction and secure: isProduction. The connection-switch flow correctly used it via switchConnection(), but ConnectionList then wrote the key to localStorage again, and the API-key login form bypassed the server action entirely.

Changes / 변경 사항

File	Action	Description	Design Rationale
frontend/src/app/actions.ts	Modify	Add setApiKeyCookieAction(apiKey) server action that sets only the lg_apiKey cookie via cookies().set with httpOnly/secure in production; deletes the cookie when given an empty string	Login form only collects an API key (no apiUrl/assistantId), so reusing updateConnectionAction (which requires apiUrl) would couple the form to default-connection knowledge
frontend/src/app/(auth)/login/ApiKeyLoginForm.tsx	Modify	Replace document.cookie = "lg_apiKey=..." with await setApiKeyCookieAction(apiKey.trim())	Single-purpose call site, async flow already in place
frontend/src/shared/components/settings/ConnectionList.tsx	Modify	Remove the if (connection.apiKey) { localStorage.setItem("lg:chat:apiKey", ...) } block; switchConnection() already calls updateConnectionAction(...) which sets the same cookie via cookies().set	The localStorage write was redundant + insecure; no replacement needed
frontend/src/lib/api-key.tsx	Delete	Remove dead getApiKey() helper	Sole consumer (Thread.tsx) is client-side, where the LangGraph SDK client ignores apiKey (proxy handles it server-side); kept around it would invite future regressions
frontend/src/providers/Thread.tsx	Modify	Drop getApiKey import; simplify connection.apiKey || getApiKey() || undefined to connection.apiKey || undefined	See above — the fallback was never actually consulted on the request path

Dependency & Impact Analysis / 의존성 및 영향 분석

• Upstream Dependencies: Existing requireAuth() (frontend/src/lib/auth/require-auth.ts) — setApiKeyCookieAction calls it, matching updateConnectionAction. In api-key and custom-jwt modes requireAuth() returns null (no NextAuth session needed), so the login flow is unaffected.
• Downstream Consumers:
  • frontend/src/middleware.ts:107 reads req.cookies.get("lg_apiKey") — server-side, works the same with the httpOnly cookie.
  • frontend/src/app/api/[..._path]/route.ts:122-127 reads the same cookie and injects x-api-key into the upstream LangGraph request — unchanged.
  • frontend/src/providers/client.ts:32-36 already explicitly skips apiKey when running in the browser (isClient ? undefined : apiKey), so removing the localStorage fallback in Thread.tsx does not change request behaviour.
• Blast Radius: Login + connection management flows in the API-key auth mode. NextAuth, OAuth, and standalone modes are not affected by the writers being removed.

Test Plan / 테스트 계획

#	Type	Command	Expected Result	Status
1	Static (grep)	grep -rn 'document\.cookie\s*=.*lg_apiKey' frontend/src	0 hits	pass
2	Static (grep)	grep -rn 'localStorage\.setItem.*lg:chat:apiKey' frontend/src	0 hits	pass
3	Static (grep)	grep -rn 'getApiKey\b' frontend/src/providers	0 hits	pass
4	Static (grep)	grep -rn 'from \"@/lib/api-key\"' frontend/src	0 hits	pass
5	File deletion	test ! -f frontend/src/lib/api-key.tsx	exists nowhere	pass
6	Type check	cd frontend && pnpm tsc --noEmit	0 errors	pass
7	Lint	cd frontend && pnpm lint	clean	pass
8	Manual	API-key login → check document.cookie in DevTools	no lg_apiKey value	pending
9	Manual	API-key login → localStorage.getItem('lg:chat:apiKey')	null	pending
10	Manual	Switch connections in Settings → request still authenticates upstream	requests succeed	pending

Note: frontend/ has no test framework today, so unit tests for setApiKeyCookieAction are deferred to a follow-up PR (see Reviewer Notes). All static checks pass.

Reviewer Notes / 리뷰어 참고 사항

• The httpOnly / secure flags follow the existing dev-vs-prod gate (process.env.NODE_ENV === \"production\") used by updateConnectionAction. In local dev the cookie is still readable from JS so DevTools-based debugging keeps working; in production it is httpOnly. This trade-off is preserved deliberately for consistency.
• frontend/src/shared/components/settings/SettingsDialog.tsx:59 still calls localStorage.removeItem(\"lg:chat:apiKey\") in the reset flow. That call is idempotent and helps clean up data for users who already had the key in localStorage from a previous version, so it is intentionally kept.

Follow-up Issues (recommended)

• P2: frontend/src/lib/connections/storage.ts still serialises connection.apiKey into the lg:connections localStorage entry (addConnection / loadConnections). Same vulnerability class, separate writer. Out of scope for this PR.
• P2: Introduce vitest in frontend/ so we can write unit tests for security-sensitive server actions like setApiKeyCookieAction.

Related Issues / 관련 이슈

Closes #77

Checklist / 체크리스트

• Branch name follows naming convention (fix/issue-77-apikey-storage)
• Conventional commit message
• Type check passes (pnpm tsc --noEmit)
• Linter passes (pnpm lint)
• No secrets committed
• No unrelated changes bundled
• Impact analysis completed (proxy + middleware + client SDK paths verified)
• Rollback strategy: revert the commit; the previous behaviour relies only on JS-readable storage, no migration needed