fix(serve): address wenshao review — scope restriction, restart message, and hardening

- Restrict POST /workspace/settings to workspace scope only (remove user scope)
- Fix requiresRestart message being cleared by useEffect (track restartPending state)
- Remove explicit reload() — let SSE event-driven reload handle it (fixes double reload)
- Add busyKey guard to prevent double-submit during save
- Add string max length validation (1024 chars)
- Sanitize error messages — don't leak filesystem paths in HTTP responses
- Remove corruptedPath from GET response — only return recovered boolean
- Extract shared getAllowedKeys() to deduplicate filter logic
- Replace scopeToEnum with explicit SCOPE_MAP
- Separate persist and broadcast try/catch blocks
- Add settings_changed case to asKnownDaemonEvent and reducer
- Add workspace_settings to EXPECTED_REGISTERED_FEATURES test array

Author: doudouOUC

packages/cli/src/serve/routes/workspaceSettings.ts

@@ -33,7 +33,9 @@ const TUI_ONLY_SETTINGS = new Set([
   'ui.accessibility.enableLoadingPhrases',
 ]);
 
-const VALID_WRITE_SCOPES = new Set(['user', 'workspace']);
+const VALID_WRITE_SCOPES = new Set(['workspace']);
+
+const MAX_STRING_VALUE_LENGTH = 1024;
 
 
 interface SettingDescriptor {
@@ -56,18 +58,22 @@ interface SettingsResponse {
   v: 1;
   warnings?: Array<{
     type: 'corrupted';
-    scope: string;
-    path: string;
     recovered: boolean;
   }>;
   settings: SettingDescriptor[];
 }
 
+function getAllowedKeys(): Set<string> {
+  return new Set(
+    getDialogSettingKeys().filter((k) => !TUI_ONLY_SETTINGS.has(k)),
+  );
+}
+
 function buildSettingsResponse(
   boundWorkspace: string,
 ): SettingsResponse {
   const loaded = loadSettings(boundWorkspace);
-  const keys = getDialogSettingKeys().filter((k) => !TUI_ONLY_SETTINGS.has(k));
+  const keys = Array.from(getAllowedKeys());
 
   const settings: SettingDescriptor[] = [];
   for (const key of keys) {
@@ -110,8 +116,6 @@ function buildSettingsResponse(
   if (loaded.corruptedPath) {
     warnings.push({
       type: 'corrupted',
-      scope: 'unknown',
-      path: loaded.corruptedPath,
       recovered: loaded.wasRecovered,
     });
   }
@@ -137,6 +141,8 @@ function validateSettingValue(
       break;
     case 'string':
       if (typeof value !== 'string') return 'Value must be a string';
+      if (value.length > MAX_STRING_VALUE_LENGTH)
+        return `Value exceeds ${MAX_STRING_VALUE_LENGTH}-character limit`;
       break;
     case 'enum':
       if (
@@ -154,9 +160,10 @@ function validateSettingValue(
   return undefined;
 }
 
-function scopeToEnum(scope: string): SettingScope {
-  return scope === 'user' ? SettingScope.User : SettingScope.Workspace;
-}
+const SCOPE_MAP: Record<string, SettingScope> = {
+  user: SettingScope.User,
+  workspace: SettingScope.Workspace,
+};
 
 export interface WorkspaceSettingsRouteDeps {
   boundWorkspace: string;
@@ -193,9 +200,7 @@ export function registerWorkspaceSettingsRoutes(
     parseAndValidateClientId,
   } = deps;
 
-  const allowedKeys = new Set(
-    getDialogSettingKeys().filter((k) => !TUI_ONLY_SETTINGS.has(k)),
-  );
+  const allowedKeys = getAllowedKeys();
 
   app.get('/workspace/settings', (_req: Request, res: Response) => {
     try {
@@ -208,7 +213,7 @@ export function registerWorkspaceSettingsRoutes(
         }`,
       );
       res.status(500).json({
-        error: err instanceof Error ? err.message : String(err),
+        error: 'Failed to load settings',
         code: 'internal_error',
       });
     }
@@ -277,27 +282,36 @@ export function registerWorkspaceSettingsRoutes(
       if (clientId === null) return;
 
       try {
-        await persistSetting(boundWorkspace, scopeToEnum(scope), key, value);
-
-        broadcastSettingsChanged(key, value, scope, clientId);
-
-        res.status(200).json({
-          key,
-          scope,
-          value,
-          requiresRestart: def.requiresRestart,
-        });
+        await persistSetting(boundWorkspace, SCOPE_MAP[scope]!, key, value);
       } catch (err) {
         writeStderrLine(
-          `qwen serve: POST /workspace/settings error: ${
+          `qwen serve: POST /workspace/settings persist error: ${
             err instanceof Error ? err.message : String(err)
           }`,
         );
         res.status(500).json({
-          error: err instanceof Error ? err.message : String(err),
+          error: 'Failed to persist setting',
           code: 'persist_error',
         });
+        return;
+      }
+
+      try {
+        broadcastSettingsChanged(key, value, scope, clientId);
+      } catch (err) {
+        writeStderrLine(
+          `qwen serve: POST /workspace/settings broadcast error: ${
+            err instanceof Error ? err.message : String(err)
+          }`,
+        );
       }
+
+      res.status(200).json({
+        key,
+        scope,
+        value,
+        requiresRestart: def.requiresRestart,
+      });
     },
   );
 }

packages/cli/src/serve/server.test.ts

@@ -207,6 +207,7 @@ const EXPECTED_REGISTERED_FEATURES = [
   'permission_mediation',
   'prompt_absolute_deadline',
   'writer_idle_timeout',
+  'workspace_settings',
   'non_blocking_prompt',
 ] as const;
 

packages/sdk-typescript/src/daemon/events.ts

@@ -1301,6 +1301,10 @@ export function asKnownDaemonEvent(
       return isToolToggledData(event.data)
         ? (event as DaemonToolToggledEvent)
         : undefined;
+    case 'settings_changed':
+      return event.data != null && typeof event.data === 'object'
+        ? (event as DaemonEventEnvelope<'settings_changed', Record<string, unknown>>)
+        : undefined;
     case 'workspace_initialized':
       return isWorkspaceInitializedData(event.data)
         ? (event as DaemonWorkspaceInitializedEvent)
@@ -1638,14 +1642,13 @@ export function reduceDaemonSessionEvent(
         lastApprovalModeChange: mergeOriginator(event.data, event),
       };
     case 'tool_toggled':
-      // Workspace-scoped — same `tool_toggled` envelope is fan-out to
-      // every session, so adapters can render "this tool was disabled
-      // by another client" without polling.
       return {
         ...base,
         toolToggleCount: base.toolToggleCount + 1,
         lastToolToggle: mergeOriginator(event.data, event),
       };
+    case 'settings_changed':
+      return base;
     case 'workspace_initialized':
       // Workspace-scoped fan-out. Non-terminal — just records that a
       // QWEN.md scaffold was performed.

packages/sdk-typescript/src/daemon/types.ts

@@ -1020,8 +1020,6 @@ export interface DaemonWorkspaceSettingsStatus {
   v: 1;
   warnings?: Array<{
     type: 'corrupted';
-    scope: string;
-    path: string;
     recovered: boolean;
   }>;
   settings: DaemonSettingDescriptor[];

packages/web-shell/client/components/dialogs/SettingsDialog.tsx

@@ -171,8 +171,8 @@ export function SettingsDialog({
           )
           .join('; '),
       );
-    else if (settings.length > 0) setMessage(null);
-  }, [error, settings, status, t]);
+    else if (settings.length > 0 && !restartPending) setMessage(null);
+  }, [error, settings, status, t, restartPending]);
 
   useEffect(() => {
     if (selectedIdx >= rows.length && rows.length > 0) {
@@ -193,14 +193,16 @@ export function SettingsDialog({
     }
   }, [editMode]);
 
+  const [restartPending, setRestartPending] = useState(false);
+
   const handleSetValue = useCallback(
     (key: string, value: unknown) => {
       setMessage(null);
       setBusyKey(key);
       setValue(scope, key, value)
         .then((result) => {
-          reload();
           if (result?.requiresRestart) {
+            setRestartPending(true);
             setMessage(t('settings.requiresRestart'));
           }
         })
@@ -209,7 +211,7 @@ export function SettingsDialog({
         })
         .finally(() => setBusyKey(null));
     },
-    [scope, setValue, reload, t],
+    [scope, setValue, t],
   );
 
   const handleAction = useCallback(
@@ -297,7 +299,7 @@ export function SettingsDialog({
         reload();
         return;
       }
-      if (e.key === 'Enter' || e.key === ' ') {
+      if ((e.key === 'Enter' || e.key === ' ') && !busyKey) {
         e.preventDefault();
         const row = rows[selectedIdx];
         if (row?.type === 'setting' && row.setting) {
@@ -306,6 +308,7 @@ export function SettingsDialog({
       }
     },
     [
+      busyKey,
       editMode,
       handleAction,
       handleEditSubmit,