fix: address Copilot review findings across security, a11y, and UX

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

Author: SweetSophia

apps/webuiapps/src/components/ChatPanel/ChatSubComponents.tsx

@@ -142,7 +142,13 @@ export const CharacterAvatar: React.FC<{
     });
   }, [media?.url, activeUrl]);
 
+  const desiredUrlRef = useRef<string | undefined>(media?.url);
+  desiredUrlRef.current = media?.url;
+
   const handleMediaReady = useCallback((readyUrl: string) => {
+    // Ignore stale loads — if the user has since switched to a different emotion,
+    // don't activate the old layer
+    if (desiredUrlRef.current && readyUrl !== desiredUrlRef.current) return;
     setLayers((prev) => {
       const staleUrls = prev.filter((l) => l.url !== readyUrl).map((l) => l.url);
       if (cleanupRef.current) clearTimeout(cleanupRef.current);

apps/webuiapps/src/components/ChatPanel/SettingsModal.tsx

@@ -344,20 +344,33 @@ const SettingsModal: React.FC<SettingsModalProps> = ({
               setSaving(true);
               const llmCfg: LLMConfigUpdate = {
                 provider,
-                baseUrl,
-                model,
-                customHeaders,
+                baseUrl: baseUrl.trim(),
+                model: model.trim(),
+                customHeaders: customHeaders.trim(),
               };
               if (apiKeyDirty) {
-                llmCfg.apiKey = apiKey;
+                llmCfg.apiKey = apiKey.trim();
               } else if (llmEndpointChanged) {
                 llmCfg.apiKey = '';
               }
               const nextImageGenConfig = buildImageGenConfig();
               const igCfg: ImageGenConfigUpdate | null = imageGenConfig
-                ? nextImageGenConfig
+                ? {
+                    ...nextImageGenConfig,
+                    baseUrl: nextImageGenConfig.baseUrl.trim(),
+                    model: nextImageGenConfig.model.trim(),
+                    customHeaders: nextImageGenConfig.customHeaders?.trim(),
+                    ...(nextImageGenConfig.apiKey
+                      ? { apiKey: nextImageGenConfig.apiKey.trim() }
+                      : {}),
+                  }
                 : igApiKey.trim()
-                  ? { ...nextImageGenConfig, apiKey: igApiKey }
+                  ? {
+                      ...nextImageGenConfig,
+                      apiKey: igApiKey.trim(),
+                      baseUrl: nextImageGenConfig.baseUrl.trim(),
+                      model: nextImageGenConfig.model.trim(),
+                    }
                   : null;
               try {
                 const result = await onSave(llmCfg, igCfg);

apps/webuiapps/src/components/ChatPanel/index.tsx

@@ -689,6 +689,17 @@ const ChatPanel: React.FC<{
               setShowSettings(false);
             }
 
+            if (nextConfig === null || !imageGenReloadSucceeded) {
+              const reloadFailures = [
+                ...(nextConfig === null ? ['chat settings'] : []),
+                ...(!imageGenReloadSucceeded ? ['image generation settings'] : []),
+              ];
+              return {
+                ok: false,
+                error: `Settings were saved, but reloading ${reloadFailures.join(' and ')} failed. Please reopen settings to verify.`,
+              };
+            }
+
             return { ok: true };
           }}
           onClose={() => setShowSettings(false)}

apps/webuiapps/src/components/ChatPanel/toolDefinitions.ts

@@ -18,6 +18,11 @@ import type { LLMConfig } from '@/lib/llmModels';
 export function hasUsableLLMConfig(config: LLMConfig | null | undefined): config is LLMConfig {
   const baseUrl = typeof config?.baseUrl === 'string' ? config.baseUrl.trim() : '';
   const model = typeof config?.model === 'string' ? config.model.trim() : '';
+  // Normalize the config in-place so downstream callers get trimmed values
+  if (config) {
+    config.baseUrl = baseUrl;
+    config.model = model;
+  }
   return !!baseUrl && !!model;
 }
 

apps/webuiapps/src/components/ChatPanel/useConversationEngine.ts

@@ -15,7 +15,7 @@ import {
 } from 'react';
 import { chat, type ChatMessage, type ToolCall } from '@/lib/llmClient';
 import type { LLMConfig } from '@/lib/llmModels';
-import { hasUsableImageGenConfig, type ImageGenConfig } from '@/lib/imageGenClient';
+import { type ImageGenConfig } from '@/lib/imageGenClient';
 import type { CharacterConfig } from '@/lib/characterManager';
 import { clearEmotionVideoCache } from '@/lib/characterManager';
 import type { MemoryEntry } from '@/lib/memoryManager';
@@ -110,7 +110,7 @@ export function useConversationEngine(deps: ConversationEngineDeps) {
     async (history: ChatMessage[], cfg: LLMConfig, signal?: AbortSignal) => {
       await seedMetaFiles();
       await loadActionsFromMeta();
-      const hasImageGen = hasUsableImageGenConfig(imageGenConfigRef.current);
+      const hasImageGen = (imageGenConfigRef.current?.apiKey?.trim().length ?? 0) > 0;
       const mm = modManagerRef.current;
       const char = characterRef.current;
 
@@ -138,7 +138,7 @@ export function useConversationEngine(deps: ConversationEngineDeps) {
       while (iterations < maxIterations) {
         if (signal?.aborted) break;
         iterations++;
-        const response = await chat(currentMessages, tools, cfg);
+        const response = await chat(currentMessages, tools, cfg, signal);
 
         if (response.toolCalls.length === 0) {
           // No tool calls — fallback plain text
@@ -290,9 +290,17 @@ async function executeToolCall(
 
   // ---- File tools ----
   if (isFileTool(tc.function.name)) {
-    ctx.pendingToolCallsRef.current.push(
-      `${tc.function.name}(${JSON.stringify(params).slice(0, 60)})`,
-    );
+    // Only log tool name + path to avoid persisting sensitive file contents
+    const pathKeys = ['path', 'filePath', 'sourcePath', 'targetPath', 'destinationPath'] as const;
+    let summary = tc.function.name;
+    for (const key of pathKeys) {
+      const value = (params as Record<string, unknown>)[key];
+      if (typeof value === 'string' && value.length > 0) {
+        summary = `${tc.function.name}(${key}: ${value})`;
+        break;
+      }
+    }
+    ctx.pendingToolCallsRef.current.push(summary);
     try {
       const result = await executeFileTool(tc.function.name, params as Record<string, string>);
       return toolResult(result);

apps/webuiapps/src/components/ErrorBoundary.tsx

@@ -40,13 +40,18 @@ export class ErrorBoundary extends Component<Props, State> {
   render(): ReactNode {
     if (this.state.hasError) {
       return (
-        <div className={styles.fallback} data-testid="error-boundary-fallback">
+        <div
+          className={styles.fallback}
+          data-testid="error-boundary-fallback"
+          role="alert"
+          aria-live="assertive"
+        >
           <div className={styles.icon}>⚠️</div>
           <div className={styles.message}>
             {this.props.name ? `${this.props.name} crashed` : 'Something went wrong'}
           </div>
           <div className={styles.detail}>{this.state.error?.message}</div>
-          <button className={styles.retryBtn} onClick={this.handleRetry}>
+          <button type="button" className={styles.retryBtn} onClick={this.handleRetry}>
             Retry
           </button>
         </div>

apps/webuiapps/src/lib/llmClient.ts

@@ -178,6 +178,7 @@ export async function chat(
   messages: ChatMessage[],
   tools: ToolDef[],
   config: LLMConfig,
+  signal?: AbortSignal,
 ): Promise<LLMResponse> {
   logger.info(
     'LLM',
@@ -189,15 +190,16 @@ export async function chat(
     messages.length,
   );
   if (config.provider === 'anthropic' || config.provider === 'minimax') {
-    return chatAnthropic(messages, tools, config);
+    return chatAnthropic(messages, tools, config, signal);
   }
-  return chatOpenAI(messages, tools, config);
+  return chatOpenAI(messages, tools, config, signal);
 }
 
 async function chatOpenAI(
   messages: ChatMessage[],
   tools: ToolDef[],
   config: LLMConfig,
+  signal?: AbortSignal,
 ): Promise<LLMResponse> {
   const body: Record<string, unknown> = {
     model: config.model,
@@ -227,6 +229,7 @@ async function chatOpenAI(
     method: 'POST',
     headers,
     body: JSON.stringify(body),
+    signal,
   });
 
   logger.info('LLM', 'Response status:', res.status);
@@ -263,6 +266,7 @@ async function chatAnthropic(
   messages: ChatMessage[],
   tools: ToolDef[],
   config: LLMConfig,
+  signal?: AbortSignal,
 ): Promise<LLMResponse> {
   const systemMsg = messages.find((m) => m.role === 'system')?.content || '';
   const nonSystemMessages = messages.filter((m) => m.role !== 'system');
@@ -338,6 +342,7 @@ async function chatAnthropic(
     method: 'POST',
     headers,
     body: JSON.stringify(body),
+    signal,
   });
 
   logger.info('LLM', 'Anthropic Response status:', res.status);

apps/webuiapps/vite.config.ts

@@ -149,8 +149,12 @@ function llmConfigPlugin(): Plugin {
             }
 
             try {
-              fs.mkdirSync(resolve(os.homedir(), '.openroom'), { recursive: true });
-              fs.writeFileSync(LLM_CONFIG_FILE, JSON.stringify(mergedConfig), 'utf-8');
+              const configDir = resolve(os.homedir(), '.openroom');
+              fs.mkdirSync(configDir, { recursive: true, mode: 0o700 });
+              fs.writeFileSync(LLM_CONFIG_FILE, JSON.stringify(mergedConfig), {
+                encoding: 'utf-8',
+                mode: 0o600,
+              });
               const stat = fs.statSync(LLM_CONFIG_FILE);
               cachedServerConfig = { mtimeMs: stat.mtimeMs, value: mergedConfig };
               res.writeHead(200);
@@ -182,7 +186,7 @@ function sanitizeRelativePath(relPath: string, baseDir: string): string | null {
   const safe = cleaned.replace(/[^a-zA-Z0-9_\-./]/g, '_');
   // Resolve to absolute and verify it stays within baseDir
   const resolved = resolve(baseDir, safe);
-  // Normalize both paths for comparison (resolve handles .. and symlinks)
+  // Normalize both paths for comparison (resolve handles .. sequences)
   const normalizedBase = resolve(baseDir);
   if (!resolved.startsWith(normalizedBase + sep) && resolved !== normalizedBase) {
     return null;
@@ -506,6 +510,16 @@ function isBlockedPassthroughHeader(headerName: string): boolean {
       'proxy-authorization',
       'x-api-key',
       'x-goog-api-key',
+      // Hop-by-hop headers per RFC 9110 — forwarding these can cause
+      // request smuggling or broken upstream responses
+      'content-length',
+      'transfer-encoding',
+      'content-encoding',
+      'accept-encoding',
+      'te',
+      'trailer',
+      'upgrade',
+      'keep-alive',
     ].includes(headerName)
   );
 }
@@ -540,7 +554,14 @@ function llmProxyPlugin(): Plugin {
     name: 'llm-proxy',
     configureServer(server) {
       server.middlewares.use('/api/llm-proxy', async (req, res) => {
-        const targetUrl = req.headers['x-llm-target-url'] as string;
+        // Normalize header values — Node http headers can be string[] for duplicates
+        const getHeader = (name: string): string | undefined => {
+          const val = req.headers[name];
+          if (Array.isArray(val)) return val[0];
+          return val;
+        };
+
+        const targetUrl = getHeader('x-llm-target-url');
         if (!targetUrl) {
           res.writeHead(400, { 'Content-Type': 'application/json' });
           res.end(JSON.stringify({ error: 'Missing X-LLM-Target-URL header' }));
@@ -595,8 +616,8 @@ function llmProxyPlugin(): Plugin {
             injectServerApiKey(
               headers,
               parsedTargetUrl,
-              req.headers['x-llm-provider'] as string,
-              req.headers['x-llm-config-scope'] as string,
+              getHeader('x-llm-provider'),
+              getHeader('x-llm-config-scope'),
             );
 
             const controller = new AbortController();