feat: app hardening + per-user local settings#1
Merged
Conversation
The chat resolver loaded provider API keys from a global app_settings row (category='ai_provider', user_id IS NULL), which let any guest chat through whichever key the admin had stored. That's a real cost leak. Each *load*ApiKey now consults env vars only. Per-user keys (read by getUserScopedKey) still work; guests with no key now get a clean "API key is not set" 400 from the harness's hasProviderCredentialFor gate instead of silently riding the global key. The encrypted global row in app_settings is now dead data and can be scrubbed at any time.
Three High-severity findings from the security audit: 1. /api/credits/purchase had no payment verification - db.addCredits ran directly on any signed-in user's POST, letting them mint gems for free. Returns 501 until a real payment provider is wired up. 2. /api/upload had no auth and no rate limit. Anonymous requests could trigger paid vision/PDF processing on 20MB uploads. Now requires a session (validateSessionOrGuest) and runs through uploadLimiter. 3. /api/audio had no auth, no rate limit, and no size cap. Anonymous requests could submit arbitrary audio to paid providers and OOM the worker via base64-encoded buffers. Now: auth + new audioLimiter (10/min) + 10MB size cap before arrayBuffer(). Includes incidental lint cleanup (sort-keys, catch-unknown) in the two endpoint files so the pre-commit hook passes; no behavior change.
Three Medium-severity findings from the security audit: S4 — Open redirect on login. /api/auth/login?returnTo=https://attacker.example would redirect authenticated users (302) and propagate the absolute URL into the magnova-auth handoff. New safeReturnTo() coerces the value to a same-origin relative path, falling back to '/'. getAuthUrl is reworked to always re-anchor on the request origin. S5 — Spoofable rate-limit keys. getClientKey trusted x-forwarded-for unconditionally — any client could send arbitrary IPs to evade rate limits. New priority: cf-connecting-ip → x-real-ip → x-forwarded-for (only if TRUST_PROXY=true) → 'unknown'. .env.example documents the var. S6 — Permissive CORS on chat. /api/chat returned Access-Control-Allow-Origin: * with credentialed cookies. Cross-origin sites could trigger chat actions on behalf of signed-in users. The OPTIONS handler and CORS headers on the stream response are removed; chat is now same-origin only (which matches the actual consumer — the app shell).
S7 — workspace file content validation was only enforced in the chat tool path (fileSystem.ts). The REST routes (POST /api/workspace-files and PATCH /api/workspace-files/[id]) accepted any content for any kind, so a client could store markdown in a .mermaid (renders as raw text) or a Mermaid diagram declaration in a .md (breaks the prose renderer). Extracted the validator to $lib/server/workspace-content-validation.ts and applied it to both REST handlers; the chat tool path now imports the same function. S8 — file-store.ts pasted the user-supplied filename into a tmp path under os.tmpdir(). UUID prefix prevents collisions but doesn't stop weird chars (path separators, control chars, shell metacharacters) from leaking into the filesystem. New sanitizeFilenameSegment strips to [a-zA-Z0-9._-] and caps at 80 chars; UUID prefix stays. Includes incidental sort-keys cleanup in file-store; no behavior change beyond what's described above.
Foundation for the local-settings revamp. Two thin helpers:
src/lib/server/auth/provider-keys.ts
- ProviderKeys type (openrouter, openai, anthropic + anthropicAuthToken,
gemini, braveSearch, tavily)
- extractProviderKeys(request) reads x-provider-* headers
- missingProviderKeyError(field) throws a 400 with a Settings-routable
message
src/lib/client/util/provider-keys.ts
- providerKeyHeaders(aiSettings) builds the x-provider-* header bag
from the localStorage-backed AI settings store
- Empty/missing values are dropped so the server's missing-key guard
fires deterministically
AISettings interface gains braveSearchApiKey + tavilyApiKey fields so
the search tools can travel through the same per-request channel.
No call sites use these yet; subsequent passes (B1+) wire them in.
…back model.ts is now a pure resolver. Every function takes the credentials it needs as a parameter: resolveChatModelFor(modelId, providerHint, keys) buildOpenRouterChat(modelId, apiKey) exported for subagent tools buildOpenAiChat(modelId, apiKey) buildAnthropicChat(modelId, apiKey, authToken) hasProviderKey(provider, keys) Removed (dead after this pass — replaced by ProviderKeys threading): - load*ApiKey / loadProviderApiKeys - runtime*ApiKey module-level globals - setRuntime*ApiKey setters - getUserScopedKey / getOpenRouterKeyFor / getOpenAiKeyFor / getAnthropicKeyFor - hasProviderCredentialFor / missingProviderCredentialMessage - getAnthropicAuthHeaders / getAnthropicAuthToken / getAnthropicApiKey / getOpenRouterApiKey / getOpenAiApiKey - openrouterFastChat (subagent helper) — subagents now call buildOpenRouterChat directly with their threaded keys - resolveChatModel (deprecated env-only resolver) ToolContext now carries `keys: ProviderKeys`; createDiagramTools takes keys as its sixth argument so subagent tools can see them. The harness reads keys via extractProviderKeys(request) at the chat boundary and threads them through createDiagramTools, buildChatContext (for the summarizer model), and resolveChatModelFor. Cascade errors are intentional in this commit: - tools/diagram*.ts and tools/markdown*.ts still import openrouterFastChat - api/audio/+server.ts and api/upload/+server.ts still call loadOpenRouterApiKey - api/admin/+server.ts still imports setRuntime*ApiKey - lib/server/model-lab.ts still imports the load* family Pass B2 fixes the subagent tools; pass B3 fixes the rest. The chat endpoint itself works end-to-end at this commit because the harness boundary is closed.
…ow via ToolContext The diagram*/markdown* tools each imported openrouterFastChat + generateText from earlier subagent days, but those imports were dead — the tools no longer make their own LLM calls (file-centric refactor in 1598085 moved that work elsewhere). Removed the dead imports. Tools' /* eslint-disable @typescript-eslint/no-unused-vars */ directives stay; other entries in those files are still legitimately unused. webSearch.ts is the one tool that does need provider keys: it calls Tavily and Brave directly. resolveSearchKeyFor (DB-backed) is replaced by resolveSearchKey(keys: ProviderKeys), and webSearch reads keys from ToolContext now. Same shape as model.ts — keys come in by parameter, no DB lookup. Includes incidental sort-keys cleanup so pre-commit lint passes; no behavior change beyond the import/key-source rewires above. This commit completes Pass B2. Pass B3 fixes the remaining cascade in model-lab.ts, /api/audio, /api/upload, /api/admin.
Closes the cascade left by Pass B1. The non-chat endpoints that used to
call load*ApiKey() now extract keys from the request via
extractProviderKeys() and pass them through:
- /api/audio: transcribeWithGemini and transcribeWithOpenRouter take
the key as their first arg. POST handler reads keys.gemini and
keys.openrouter; 400 if both are missing.
- /api/upload: describeImageWithVision takes the key as its first arg.
POST handler reads keys.openrouter and passes it through; missing
key still degrades to a placeholder description rather than failing
the upload (text/PDF uploads don't need a key).
- /api/admin: setOpenRouterApiKey and setProviderApiKey cases return
410 Gone. Server-side global key storage was the original guest-leak
vector and is intentionally gone — admins use Settings > Models &
Keys like everyone else.
- /api/model-lab: GET and POST extract keys and pass them through to
searchProviderModels / smokeTestProviderModel.
- lib/server/model-lab.ts: searchProviderModels and
smokeTestProviderModel both take a `keys: ProviderKeys` parameter.
Direct env reads are gone. anthropicListHeaders is local to this
file (mirrors buildAnthropicChat's OAuth-vs-api-key precedence
without depending on the deleted getAnthropicAuthHeaders).
Test files updated to match the new contract:
- tests/unit/model-lab.test.ts: passes keysWith({ ... }) instead of
vi.stubEnv. Route tests pass x-provider-* headers via Request.
- tests/unit/server-harness.test.ts: live-trace harness reads keys
from env directly via liveTraceProviderKeys() (test-only env-key
escape hatch). Removed the load*ApiKey imports. resolveChatModel
call sites use resolveChatModelFor with explicit keys.
- createIconSearchTool calls now pass an empty ProviderKeys (icon
search doesn't actually use any).
Pre-existing GraphiniAgentId / diagramStore errors in
server-harness.test.ts are out of scope.
This commit completes Pass B3. The branch compiles end-to-end again.
The KV store no longer mirrors writes to /api/kv. Each user keeps their own settings on their own browser; nothing crosses the wire. /api/kv is deleted in a later pass. Removed: - init() server fetch - pendingWrites / flushTimer / scheduleFlush - flush() POST /api/kv - delete() DELETE /api/kv - beforeunload sendBeacon Public API preserved exactly so callers (Chat.simple, SettingsModal, exportState, persist.ts) keep working without changes: - init / get / set / delete / getCategory / getAll - flush() becomes a no-op resolved promise - isAuthenticated stays as a reactive flag (always true after init) - reset() still wipes memCache + localStorage on logout The local-only writes still skip redundant payloads via shallowEqual — without that guard, streaming chat parts would rewrite the same large payload on every tick.
Settings (UI, AI keys, personalization, editor) and panel layout are now keyed by the active user id, so two users on the same browser don't share each other's data. Mechanism: - settings.svelte.ts grows `setActiveUserId(userId)` and `getActiveUserNamespace()`. - Every PersistentSetting<T> registers itself; setActiveUserId rebinds each one's storage key (`mermaid_<userNs>_<key>`) and reloads value from the new slot. UI sees the swap reactively via $state. - A second registration channel — `subscribeNamespaceChange(fn)` — lets non-PersistentSetting consumers (panels) reload too. - panels.svelte.ts namespaces its own kv keys (`graphini_panels_v4_<userNs>` / `graphini_panel_order_v1_<userNs>`), subscribes to namespace changes, and reloads via `reloadFromActiveNamespace` which also cancels any in-flight debounced save so user A's pending state never lands in user B's slot. auth.svelte.ts wires the active-user signal: - module-level: `setActiveUserId(cached.user?.id ?? null)` on import so the first paint after a hard reload reads from the right slot - fetchMe / loginLocal / register / logout: rebind on every state.user mutation panels.svelte.ts also drops its server-sync writes: - `savePrefsToServer` (PUT /api/user/preferences) is gone — panels are per-browser now, no cross-device sync. - `syncPreferencesFromServer` becomes a no-op compatibility shim while callers still exist; safe to delete in a follow-up. Sets in settings.svelte.ts use plain Set rather than SvelteSet (documented inline) — they're mutated synchronously in setActiveUserId and never observed reactively. Pre-existing sort-keys cleanup in auth.svelte.ts so the pre-commit hook passes; no behavior change.
Chat.simple.svelte now injects provider-key headers built from the
localStorage-backed aiSettings store on every fetch that hits a paid
endpoint:
- /api/chat (3 sites: streamed turn, prompt-improver, conversation
title generation)
- /api/audio (TTS)
- /api/upload (vision / PDF processing)
Without these, every request to the per-request-keys server (Pass B1+)
returns "API key is not set" — chat works again with whatever key the
user pasted into Settings > Models & Keys.
Headers are spread after Content-Type so JSON requests keep their
explicit content type. Multipart requests (audio, upload) only carry
the x-provider-* headers; the FormData boundary is set automatically.
Pre-commit hook runs prettier + eslint on staged files and produced no
warnings — the surface change is purely additive headers.
SettingsModal.svelte's /api/model-lab caller and the full
no-server-roundtrips revamp are deferred to Pass B7, which will handle
that file as a unit.
The Settings panel's per-user state moves entirely to localStorage:
- AI provider keys (OpenAI / Anthropic / OpenRouter): saveProviderCredential
drops the adminPost('setProviderApiKey', ...) call. updateProviderCredential
already wrote to aiSettings; that's the only thing that runs now. The
admin endpoint is gone (returns 410 from B3) and was the original
guest-leak vector — admins set their own keys in Settings like everyone
else.
- Search-provider keys (Brave / Tavily): saveUserApiKey / clearUserApiKey
drop the /api/user/api-keys PUT/DELETE calls and write directly to
aiSettings.{braveSearchApiKey, tavilyApiKey}. braveSearchPresent and
tavilyPresent are now $derived from aiSettings instead of fetching the
server's "presence flags" GET.
- loadUserApiKeys() is gone; onMount no longer fetches /api/user/api-keys.
- /api/model-lab fetch sends provider-key headers (B6 plumbing).
Pre-commit lint debt fixed in the same pass since the file is in a
known-good state:
- Object.fromEntries(filter(...)) instead of `delete obj[dyn]` for
safe key removal in deleteMemory
- Replaced `Record<string, any>` with typed EnabledModelRow and
ProviderModelRow interfaces
- Replaced loose `any` casts with narrowed property access
- Added each-block keys for tools, categories, and memory-save modes
- {@const} hoisted to the immediate child of {#each}
No behavior change beyond the server roundtrips that were intentionally
removed — every UI affordance still saves and reads the same way from the
user's perspective, just locally.
These three endpoints existed solely to mirror per-user state to the
database. The local-settings revamp moved that state entirely into
localStorage (with per-user namespacing) — no client code calls them
anymore (verified end-to-end across passes B4–B7).
- /api/kv (POST/GET/DELETE/batch) → kvStore is localStorage-only as of B4
- /api/user/api-keys (PUT/GET/DELETE) → SettingsModal writes to
aiSettings.{openrouterApiKey, openaiApiKey, anthropicApiKey,
anthropicAuthToken, braveSearchApiKey, tavilyApiKey} as of B7
- /api/user/preferences (PUT/GET) → panel layout is localStorage-only
as of B5; syncPreferencesFromServer is a no-op shim
The encryption gates in $lib/server/crypto are now unused for these
categories but stay in place — they don't hurt anything and removing
them risks regressing other categories that still use the same machinery
(prompt_enhancer.model, voice.model, chat_compaction.model).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related workstreams in one stack of commits:
chore/security-hardeningancestors): closes High/Medium audit findings — fake purchases gate, auth+rate-limit on upload/audio, safe returnTo redirect, IP-trust hardening, wildcard CORS removal, shared workspace-content validator, uploaded filename sanitization.x-provider-*headers; settings are namespaced by user id.Why
Guests were silently using the admin's globally-stored OpenRouter / OpenAI / Anthropic keys (encrypted in
app_settings) — real cost leak. The plan was: kill env/DB fallbacks for keys, move per-user state to localStorage, send keys per-request via headers. See~/.claude/plans/graphini-app-hardening.mdfor the full plan, decisions, and execution passes.What changed
Server (no longer reads any AI provider key from DB or env)
src/lib/server/auth/provider-keys.ts(new):extractProviderKeys(request),missingProviderKeyError(field),ProviderKeystype.src/lib/server/chat/model.ts: pure resolver.resolveChatModelFor(modelId, hint, keys). Removed allload*ApiKey,runtime*ApiKey,setRuntime*,getUserScopedKey,hasProviderCredentialFor,getAnthropicAuthHeaders,resolveChatModel,openrouterFastChat.src/lib/server/chat/harness/index.ts: extracts keys at the chat boundary, threads them intocreateDiagramTools,buildChatContext,resolveChatModelFor.src/lib/server/chat/tools/context.ts:ToolContext.keys: ProviderKeys.webSearch.tsuses keys-from-context for Brave/Tavily; the diagram*/markdown* tools dropped their deadopenrouterFastChatimports.src/lib/server/chat/search.ts:resolveSearchKey(keys)instead of DB lookup.src/lib/server/model-lab.ts:searchProviderModels({ keys, ... })andsmokeTestProviderModel({ keys, ... }).src/routes/api/chat,/api/audio,/api/upload,/api/model-lab: read keys viaextractProviderKeys./api/admin:setOpenRouterApiKey/setProviderApiKeycases return 410.Client (per-user localStorage)
src/lib/client/util/provider-keys.ts(new):providerKeyHeaders(aiSettings).src/lib/client/stores/settings.svelte.ts:setActiveUserId(id),subscribeNamespaceChange(fn),getActiveUserNamespace().PersistentSetting<T>storage key ismermaid_<userNs>_<key>.aiSettingsinterface gainedbraveSearchApiKeyandtavilyApiKey.src/lib/client/stores/kvStore.svelte.ts: localStorage-only. Stripped server fetch /pendingWrites/flushPOST /sendBeacon/deleteDELETE.src/lib/client/stores/auth.svelte.ts: rebinds the namespace on everystate.usermutation (init/login/register/logout).src/lib/client/stores/panels.svelte.ts: panel layout namespaced by user id; subscribes tosubscribeNamespaceChangeto reload on user swap; droppedsavePrefsToServer(PUT /api/user/preferences).src/lib/client/features/chat/components/Chat.simple.svelte: every fetch to chat / audio / upload sendsproviderKeyHeaders(aiSettings.value).src/lib/client/features/settings/SettingsModal.svelte: dropped/api/user/api-keysandadminPost('setProviderApiKey', ...)calls. Search-provider keys live inaiSettings.{braveSearchApiKey, tavilyApiKey}./api/model-labfetch sends provider headers.Server endpoints deleted
/api/user/api-keys(PUT/GET/DELETE)/api/user/preferences(PUT/GET)/api/kv(POST/GET/DELETE/batch)Security audit findings (3 commits at the bottom of the stack)
/api/credits/purchase(was minting gems with no payment verification).validateSessionOrGuest+uploadLimiter+ 20MB cap on/api/upload.validateSessionOrGuest+ newaudioLimiter(10/min) + 10MB cap on/api/audio.safeReturnTohelper; login redirect no longer accepts external URLs.getClientKey: cf-connecting-ip → x-real-ip → x-forwarded-for (only withTRUST_PROXY=true) → unknown.Access-Control-Allow-*from/api/chat(same-origin only).validateContentForKindto$lib/server/workspace-content-validation.ts; REST/api/workspace-filesand chatfileSystemtool now share validation.file-store.ts.Database changes (already executed)
Ran on production:
Manual operator follow-ups (not in this PR)
The DB credentials and signing secrets in the local
.envwere sitting next to the leaked global keys. Rotate them in the Neon console + your prod env:neondb_ownerpassword in Neon (https://console.neon.tech/app/projects/late-sky-61210792 → Branches → main → Roles → reset).APP_SETTINGS_ENCRYPTION_KEY(openssl rand -base64 48) and update local + prod.COOKIE_SECRET(openssl rand -base64 48) and update local + prod. Note: this invalidates every existing session cookie (intentional — old secret leaked).TRUST_PROXY=truein prod env (Vercel sets x-real-ip but x-forwarded-for is still useful as a fallback).Test plan
?returnTo=https://attacker.exampleon login → redirects to/.fetch('/api/chat')from a different origin in the browser → CORS error (no wildcard).POST /api/credits/purchase→ 501.POST /api/upload→ 401.POST /api/audio→ 401.Out of scope