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

* 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.

Copilot AI left a comment

PR MiniMax-AI#33 and MiniMax-AI#34 review feedback:
Security:
- Config file now written with mode 0600, directory with mode 0700
- Block hop-by-hop headers (RFC 9110) in proxy passthrough
- Validate header values are strings, not arrays, before use
- Fix misleading comment about symlink resolution in sanitizeRelativePath
- Redact sensitive data from file tool call summaries in chat history
UX / Correctness:
- Thread AbortSignal through chat() to fetch() for real request cancellation
- SettingsModal trims all inputs on save to prevent whitespace issues
- onSave returns proper error feedback when config reload fails
- hasUsableLLMConfig normalizes config in-place (trimmed baseUrl/model)
- Whitespace-only image gen API key no longer treated as enabled
Accessibility:
- ErrorBoundary: add role=alert, aria-live=assertive, type=button
Race conditions:
- handleMediaReady ignores stale loads when user switches emotions quickly
Tests: 118 passing

- Chess: add WebGL capability detection before Canvas mount; show
 diagnostic fallback panel when WebGL context creation fails instead
 of crashing silently into ErrorBoundary
- Engine tests: add useConversationEngine test suite (4 cases) covering
 AbortSignal threading, file-tool summary redaction, whitespace-only
 image-gen key handling, and mid-loop abort behavior
- Bugfix: include 'file_path' in tool summary extraction keys so
 file_write summaries show 'file_write(file_path: ...)' instead of
 bare tool name