fix(security): server-side API key management and SSRF hardening by SweetSophia · Pull Request #32 · MiniMax-AI/OpenRoom

SweetSophia · 2026-04-03T20:18:48Z

Summary

Move API key storage from client-side localStorage to server-side ~/.openroom/config.json with proxy-based enforcement and redaction. Harden the LLM proxy against SSRF attacks.

Changes

Security

Server-side key storage: API keys persist in ~/.openroom/config.json only. Clients receive redacted metadata (hasApiKey: boolean) — secrets never reach the browser.
SSRF protection: Strict host allowlist (isAllowedTarget) blocks private/internal network addresses by default. Public provider hosts require HTTPS. Configurable ALLOW_LOCAL_LLM for local development.
Header scrubbing: Proxy strips sensitive headers (authorization, x-api-key, etc.) from client requests before forwarding.
Fetch timeout: 90s upstream timeout with AbortController, returns 504 on expiry.

Config Flow

Legacy migration: Automatically normalizes old single-provider config.json shapes to the new {llm, imageGen} structure.
Server-side caching: mtime-based config caching avoids blocking I/O on every request.
Scoped key injection: X-LLM-Config-Scope header selects the correct key server-side (LLM vs imageGen).

UI

SettingsModal: Placeholder •••••••• for existing keys, local-edit guards prevent prop clobbering, endpoint-change detection clears stale secrets.
Save feedback: Errors surfaced to user instead of silently swallowed.

Testing

25 vitest unit tests covering normalizeServerConfig, redactServerConfig, parseProxyTargetUrl, isAllowedTarget, selectServerApiKey, and inferProvider.
Updated regression suite (115 tests passing) for proxy-based config flow.

Checklist

pnpm run lint passes
pnpm build passes
Documentation updated (if applicable)
Self-reviewed the code changes
AI code review passed (auto-triggered on PR)

* refactor: extract ChatPanel into focused modules Decompose the 1472-line ChatPanel god component into four focused files: - toolDefinitions.ts (133 lines): tool defs, system prompt builder, config guard - ChatSubComponents.tsx (178 lines): CharacterAvatar, StageIndicator, ActionsTaken - SettingsModal.tsx (270 lines): LLM + image generation configuration UI - useConversationEngine.ts (341 lines): conversation loop + tool dispatch hook ChatPanel/index.tsx reduced from 1472 → 632 lines (-57%), now a thin shell that wires state to UI. No behavior changes — pure structural refactor. Benefits: - Conversation engine is testable in isolation - Settings modal can be reasoned about independently - Each file has a single clear responsibility - Future PRs can target specific modules without touching the whole * fix: consolidate duplicate ModManager imports in useConversationEngine.ts Agent-Logs-Url: https://github.com/SweetSophia/OpenRoom/sessions/f4ebc43a-24c2-4853-b135-f234fd25d12b Co-authored-by: SweetSophia <44297511+SweetSophia@users.noreply.github.com> * fix: stabilize runConversation identity to prevent listener churn runConversation was recreated on every render, causing the onUserAction subscription effect to unsubscribe/resubscribe on each render. App actions emitted during the cleanup-rebind window could be silently dropped. Since runConversation only reads from refs (no reactive state), wrapping it in useCallback with an empty dependency array makes its identity stable across renders, eliminating the churn. * fix: address code review findings across ChatPanel modules useConversationEngine: - Wrap callbacks in ref to prevent stale closure in stabilized runConversation - Break outer loop after respond_to_user to avoid extra model round-trip - Add console.warn for unparseable tool call arguments - Handle loadMemories rejection with fallback to empty array ChatSubComponents: - Reactivate existing inactive layer instead of skipping it - Use ref for cleanup timeout to prevent stale timeout interference index.tsx: - Move setSessionPath call from render body to useEffect - Reduce reload effect deps to sessionPath only, add cancellation guard SettingsModal: - Sync local state from parent props via useEffect toolDefinitions: - Replace hardcoded emotion examples with generic reference to character keys * fix: address review feedback on types, timeout cleanup, and config guard Agent-Logs-Url: https://github.com/SweetSophia/OpenRoom/sessions/6cd6bd9b-6a80-474a-b067-8f2ff9f8f32a Co-authored-by: SweetSophia <44297511+SweetSophia@users.noreply.github.com> * fix: address Copilot review — tool loop, save snapshot, layer cleanup useConversationEngine: - Remove inner break on respond_to_user so sibling tool calls (e.g. finish_target) still execute before outer loop stops index.tsx: - Guard loadMemories .then/.catch with cancelled flag - Snapshot session/data at debounce schedule time instead of reading refs at fire-time, preventing cross-session data mixing - Remove now-unused sessionPathRef2 ChatSubComponents: - On layer reactivation, cancel pending cleanup timeout and explicitly deactivate all other layers to prevent dual-active state * fix: Prettier import style, stale memory guard, remove redundant ref useConversationEngine: - Break React import to multiline per Prettier config - Guard loadMemories handlers against session change during async gap index.tsx: - Remove unused sessionPathRef_forSet ref — setSessionPath in useEffect alone is sufficient for module-level sync * fix: flush debounced save on cleanup, remove stale dep in handleResetSession index.tsx: - Flush pending saveChatHistory in cleanup instead of discarding, preventing data loss on rapid session switches - Remove modCollection from handleResetSession dependency array (uses refs and functional state updates, no direct read needed) * fix(ChatPanel): useLayoutEffect for setSessionPath; fix debounced save write amplification Agent-Logs-Url: https://github.com/SweetSophia/OpenRoom/sessions/4ddaf906-91f2-4ac3-ac97-7ce3fd331545 Co-authored-by: SweetSophia <44297511+SweetSophia@users.noreply.github.com> * refactor: extract PendingSaveSnapshot type from inline ref type Agent-Logs-Url: https://github.com/SweetSophia/OpenRoom/sessions/4ddaf906-91f2-4ac3-ac97-7ce3fd331545 Co-authored-by: SweetSophia <44297511+SweetSophia@users.noreply.github.com> * fix: ModManager value import, sessionPath out of save deps, redact sensitive args from warn log Agent-Logs-Url: https://github.com/SweetSophia/OpenRoom/sessions/1954e0a2-8dec-4da9-85f4-92df5e2ddf00 Co-authored-by: SweetSophia <44297511+SweetSophia@users.noreply.github.com> * fix: only break loop when respond_to_user is sole tool call Previously the loop broke after respond_to_user even when other tools ran in the same batch. If the model planned to emit follow-up tool calls (e.g. finish_target) in a subsequent round-trip, those were silently skipped. Now the loop only terminates when respond_to_user was the only tool call — batched tool calls get their follow-up round-trip as intended. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>

…state 1. Error Boundaries: Wrap AppWindow and ChatPanel in ErrorBoundary components so crashes don't take down the entire Shell. 2. Path Traversal: Replace regex-based sanitization in session-data and session-reset endpoints with resolve()-based validation that ensures paths stay within SESSIONS_DIR. 3. SSRF/Header Injection: LLM proxy now uses an explicit allowlist for forwarded headers. x-custom- headers are validated to prevent auth injection. Target URLs are validated for protocol. 4. AbortController: Conversation loop accepts AbortSignal. handleSend and processActionQueue cancel previous conversations before starting new ones. 5. Stale State: handleSend reads chatHistory from chatHistoryRef instead of closure, fixing race condition.

API keys are no longer stored in or sent from the browser: - LLM proxy now reads keys from ~/.openroom/config.json and injects them server-side based on detected provider - Browser clients (llmClient, imageGenClient) no longer send Authorization or x-api-key headers - localStorage is no longer used as a config cache - Provider auto-detection from target URL with manual override via X-LLM-Provider header This eliminates XSS-based API key theft as an attack vector.

- Replace warn-only SSRF check with strict provider host allowlist - Only known API hosts allowed (openai, anthropic, deepseek, etc.) - Local LLM hosts only with ALLOW_LOCAL_LLM=true env var - Validate target host BEFORE injecting server-side API keys - Add isAllowedTarget() with test coverage

…ffect clobbering - index.tsx onSave: fall back to submitted values on reload failure, only close modal when reload succeeds - SettingsModal: always include customHeaders (send empty string to clear), add hasLocalEditsRef to prevent useEffect from clobbering in-progress edits - vite.config.ts: inferProvider wraps URL parse in try/catch for malformed input - viteConfig.test.ts: consistent env restore pattern, add malformed URL test for inferProvider - e2e/app.spec.ts: stub /api/llm-config for deterministic unconfigured state in settings test

- import useRef in SettingsModal and mark API key edits as local edits - avoid storing update objects/raw keys in ChatPanel state on reload failure - return structured save results from config persistence and llmClient - return 500 for server-side /api/llm-config write failures - fix E2E config stub to return valid empty JSON - update tests for structured save results

- mark provider/model UI interactions as local edits in SettingsModal - clear preserved API keys when provider/base URL changes without a new key - require HTTPS for public provider proxy targets - add upstream proxy timeout handling - expand vite config helper coverage for null/URL/env edge cases

Covers empty string, missing key, and whitespace-only apiKey — all should produce hasApiKey=false.

SweetSophia · 2026-04-03T20:20:50Z

@CodeRabbit review full

Copilot

Pull request overview

This PR moves LLM/image-gen API key handling fully server-side (persisted in ~/.openroom/config.json and injected by the dev-server proxy) and hardens the proxy against SSRF by enforcing a strict hostname allowlist and scrubbing client-supplied sensitive headers.

Changes:

Server-side config persistence with redaction (hasApiKey flags) and legacy config-shape normalization/merging.
SSRF + header hardening in /api/llm-proxy, plus upstream timeout handling.
ChatPanel refactor (extracted conversation engine, tool definitions, settings modal) and updated unit/e2e tests.

Reviewed changes

Copilot reviewed 39 out of 39 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
e2e/app.spec.ts	Updates E2E behavior when config is missing to open Settings instead of sending.
apps/webuiapps/vite.config.ts	Implements server-side config read/write/redaction, SSRF allowlist, header scrubbing, key injection, and path sanitization.
apps/webuiapps/src/pages/Twitter/twitter_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Twitter/twitter_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/MusicApp/meta/meta_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/MusicApp/meta/meta_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Gomoku/meta/meta_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Gomoku/meta/meta_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/FreeCell/freecell_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/FreeCell/freecell_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/EvidenceVault/evidencevault_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/EvidenceVault/evidencevault_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Email/email_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Email/email_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Diary/diary_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Diary/diary_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/CyberNews/meta/meta_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/CyberNews/meta/meta_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Chess/chess_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Chess/chess_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Album/album_en/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/pages/Album/album_cn/guide.md	Doc formatting/layout updates.
apps/webuiapps/src/lib/llmModels.ts	Adds `hasApiKey` + update payload type to support server-side secret preservation.
apps/webuiapps/src/lib/llmClient.ts	Removes localStorage caching and routes all requests through proxy with scope header (server-injected auth).
apps/webuiapps/src/lib/imageGenTools.ts	Uses `hasUsableImageGenConfig()` for redacted config support.
apps/webuiapps/src/lib/imageGenClient.ts	Removes localStorage caching, adds `hasApiKey` + update type, and adds `hasUsableImageGenConfig()`.
apps/webuiapps/src/lib/configPersistence.ts	Saves/loads redacted server config and returns structured save results with error details.
apps/webuiapps/src/lib/tests/viteConfig.test.ts	Adds unit tests for config normalization/redaction and proxy SSRF/key selection helpers.
apps/webuiapps/src/lib/tests/llmClient.test.ts	Updates tests for server-injected auth flow and error propagation on save.
apps/webuiapps/src/lib/tests/imageGenClient.test.ts	Updates tests for redacted config and proxy scope headers.
apps/webuiapps/src/lib/tests/configPersistence.test.ts	Updates tests for save result/error handling and redacted config loading.
apps/webuiapps/src/components/Shell/index.tsx	Wraps App windows + ChatPanel in an ErrorBoundary to prevent whole-shell crashes.
apps/webuiapps/src/components/ErrorBoundary.tsx	Adds reusable ErrorBoundary component with retry UI.
apps/webuiapps/src/components/ErrorBoundary.module.scss	Styles for ErrorBoundary fallback UI.
apps/webuiapps/src/components/ChatPanel/useConversationEngine.ts	Extracts the conversation/tool loop into a hook with cancellation plumbing.
apps/webuiapps/src/components/ChatPanel/toolDefinitions.ts	Extracts tool definitions + system prompt builder; adds config usability guard.
apps/webuiapps/src/components/ChatPanel/SettingsModal.tsx	New settings UI supporting redacted keys, endpoint-change invalidation, and surfaced save errors.
apps/webuiapps/src/components/ChatPanel/index.tsx	Refactors ChatPanel to use extracted modules and server-side config flow.
apps/webuiapps/src/components/ChatPanel/ChatSubComponents.tsx	Extracts StageIndicator/ActionsTaken/CharacterAvatar/renderMessageContent helpers.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-03T20:24:25Z

+function redactConfigSection(section?: ServerConfigSection): ServerConfigSection | undefined {
+  if (!section) return undefined;
+  return {
+    ...section,
+    apiKey: '',
+    hasApiKey: !!getStoredApiKey(section),
+  };


redactConfigSection() spreads the full section back to the client, which means non-apiKey secrets (e.g. customHeaders values) can still be returned by GET /api/llm-config and end up in the browser. If the intent is that secrets never reach the client, consider redacting/omitting customHeaders (or introducing an allowlist of safe fields) and injecting any needed server-side headers in the proxy instead.

Copilot · 2026-04-03T20:24:25Z

+  const resolved = resolve(baseDir, safe);
+  // Normalize both paths for comparison (resolve handles .. and symlinks)
+  const normalizedBase = resolve(baseDir);
+  if (!resolved.startsWith(normalizedBase + sep) && resolved !== normalizedBase) {
+    return null;


sanitizeRelativePath() comment says resolve() handles symlinks, but path.resolve does not dereference symlinks. If symlink-escape protection is desired for session reads/writes, use realpath-based checks (e.g. fs.realpathSync on base + resolved) or update the comment to avoid implying symlink safety that isn’t actually provided.

Copilot · 2026-04-03T20:24:25Z

+      while (iterations < maxIterations) {
+        if (signal?.aborted) break;
+        iterations++;
+        const response = await chat(currentMessages, tools, cfg);
+
+        if (response.toolCalls.length === 0) {
+          // No tool calls — fallback plain text
+          if (response.content) {
+            callbacksRef.current.addMessage({


runConversation() accepts an AbortSignal, but the signal is not passed into the underlying network request (chat() / fetch), and there’s no abort check immediately after awaiting chat(). If the signal is aborted mid-request, the response/tool calls can still be applied to state. Consider threading the AbortSignal into chat() so fetch can be cancelled, and add a signal.aborted guard right after the await before executing tool calls / state updates.

SweetSophia · 2026-04-03T21:14:48Z

Closing — wrong target. Will merge via fork instead.

SweetSophia and others added 11 commits April 1, 2026 19:44

fix: harden key redaction flow

beb2d71

fix: polish secure config flow

2b52426

fix: mark local edits in SettingsModal provider/model handlers

9241ec1

test: add redactServerConfig edge cases for empty/missing apiKey

1ce893d

Covers empty string, missing key, and whitespace-only apiKey — all should produce hasApiKey=false.

Copilot AI review requested due to automatic review settings April 3, 2026 20:18

Copilot started reviewing on behalf of SweetSophia April 3, 2026 20:19 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

SweetSophia closed this Apr 3, 2026

SweetSophia deleted the fix/server-side-api-keys branch April 3, 2026 21:20

SweetSophia restored the fix/server-side-api-keys branch April 3, 2026 21:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(security): server-side API key management and SSRF hardening#32

fix(security): server-side API key management and SSRF hardening#32
SweetSophia wants to merge 11 commits into
MiniMax-AI:mainfrom
SweetSophia:fix/server-side-api-keys

SweetSophia commented Apr 3, 2026

Uh oh!

SweetSophia commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

Copilot AI Apr 3, 2026

Uh oh!

SweetSophia commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

SweetSophia commented Apr 3, 2026

Summary

Changes

Security

Config Flow

UI

Testing

Checklist

Uh oh!

SweetSophia commented Apr 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

SweetSophia commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants