fix(serve): reject negative cleanupPeriodDays values (#5906)

POST /workspace/settings accepted negative values for general.cleanupPeriodDays because numeric setting validation only checked Number.isFinite(). The housekeeping runtime already treats non-positive values as minimum retention, so a persisted negative value could appear in settings while being silently ignored at runtime.

Add an optional minimum constraint to setting definitions, set general.cleanupPeriodDays to minimum 0, and enforce that floor in the workspace settings route. The route tests cover rejecting -1/-5 while keeping 0 and 30 accepted.

packages/cli/src/config/settingsSchema.ts

@@ -86,6 +86,8 @@ export interface SettingDefinition {
   options?: readonly SettingEnumOption[];
   /** Schema for array items when type is 'array' */
   items?: SettingItemDefinition;
+  /** Minimum value for number-type settings. */
+  minimum?: number;
   /**
    * Primitive shapes a field accepted before it was expanded to its current
    * type. The exported JSON Schema wraps the field in `anyOf` so values from
@@ -493,6 +495,7 @@ const SETTINGS_SCHEMA = {
         // surprised when a mid-session edit doesn't take effect immediately.
         requiresRestart: true,
         default: 30,
+        minimum: 0,
         description:
           'Number of days to retain ~/.qwen/file-history/ session backups used by /rewind and background subagent transcripts under <projectDir>/subagents/. Data older than this is removed by a background housekeeping pass that runs at most once per day. Set to 0 for minimum retention (~1 hour) — protects sessions touched in the last hour, plus the currently active session.',
         showInDialog: true,

packages/cli/src/serve/routes/workspace-settings.test.ts

@@ -0,0 +1,83 @@
+/**
+ * @license
+ * Copyright 2025 Qwen Team
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import express from 'express';
+import request from 'supertest';
+import { registerWorkspaceSettingsRoutes } from './workspace-settings.js';
+
+function makeApp() {
+  const app = express();
+  app.use(express.json());
+
+  const persistSetting = vi.fn(async () => {});
+  const broadcastSettingsChanged = vi.fn();
+
+  registerWorkspaceSettingsRoutes(app, {
+    boundWorkspace: '/workspace',
+    mutate: () => (_req, _res, next) => next(),
+    safeBody: (req) =>
+      req.body && typeof req.body === 'object' ? req.body : {},
+    persistSetting,
+    broadcastSettingsChanged,
+    parseAndValidateClientId: () => undefined,
+  });
+
+  return { app, persistSetting, broadcastSettingsChanged };
+}
+
+describe('POST /workspace/settings', () => {
+  it('rejects negative general.cleanupPeriodDays values', async () => {
+    const { app, persistSetting, broadcastSettingsChanged } = makeApp();
+
+    for (const value of [-1, -5]) {
+      const res = await request(app).post('/workspace/settings').send({
+        scope: 'workspace',
+        key: 'general.cleanupPeriodDays',
+        value,
+      });
+
+      expect(res.status).toBe(400);
+      expect(res.body).toMatchObject({
+        code: 'invalid_value',
+        error: 'Value must be >= 0',
+      });
+    }
+
+    expect(persistSetting).not.toHaveBeenCalled();
+    expect(broadcastSettingsChanged).not.toHaveBeenCalled();
+  });
+
+  it.each([0, 30])('accepts general.cleanupPeriodDays=%s', async (value) => {
+    const { app, persistSetting, broadcastSettingsChanged } = makeApp();
+
+    const res = await request(app).post('/workspace/settings').send({
+      scope: 'workspace',
+      key: 'general.cleanupPeriodDays',
+      value,
+    });
+
+    expect(res.status).toBe(200);
+    expect(res.body).toMatchObject({
+      key: 'general.cleanupPeriodDays',
+      scope: 'workspace',
+      value,
+      requiresRestart: true,
+    });
+    expect(persistSetting).toHaveBeenCalledWith(
+      '/workspace',
+      expect.any(String),
+      'general.cleanupPeriodDays',
+      value,
+    );
+    expect(broadcastSettingsChanged).toHaveBeenCalledWith(
+      'general.cleanupPeriodDays',
+      value,
+      'workspace',
+      undefined,
+    );
+  });
+});

packages/cli/src/serve/routes/workspace-settings.ts

@@ -7,7 +7,6 @@
 import type { Application, Request, Response } from 'express';
 import { loadSettings, SettingScope } from '../../config/settings.js';
 import type {
-  SettingDefinition,
   SettingEnumOption,
   SettingsType,
   SettingsValue,
@@ -16,6 +15,7 @@ import {
   getDialogSettingKeys,
   getNestedProperty,
   getSettingDefinition,
+  validateSettingValue,
 } from '../../utils/settingsUtils.js';
 import { writeStderrLine } from '../../utils/stdioHelpers.js';
 
@@ -40,8 +40,6 @@ const WEB_SHELL_SETTINGS = new Set(['ui.compactMode', 'voiceModel']);
 
 const VALID_WRITE_SCOPES = new Set(['workspace']);
 
-const MAX_STRING_VALUE_LENGTH = 1024;
-
 interface SettingDescriptor {
   key: string;
   type: SettingsType;
@@ -139,35 +137,6 @@ function buildSettingsResponse(
   };
 }
 
-function validateSettingValue(
-  def: SettingDefinition,
-  value: unknown,
-): string | undefined {
-  switch (def.type) {
-    case 'boolean':
-      if (typeof value !== 'boolean') return 'Value must be a boolean';
-      break;
-    case 'number':
-      if (typeof value !== 'number' || !Number.isFinite(value))
-        return 'Value must be a finite number';
-      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 (!def.options?.some((opt) => opt.value === value)) {
-        const allowed = def.options?.map((o) => o.value).join(', ') ?? '';
-        return `Value must be one of: ${allowed}`;
-      }
-      break;
-    default:
-      return `Settings of type '${def.type}' cannot be modified via this API`;
-  }
-  return undefined;
-}
-
 const SCOPE_MAP: Record<string, SettingScope> = {
   workspace: SettingScope.Workspace,
 };

packages/cli/src/ui/components/SettingsDialog.test.tsx

@@ -171,6 +171,7 @@ vi.mock('../../utils/languageUtils.js', async () => {
   return {
     ...actual,
     updateOutputLanguageFile: vi.fn(),
+    writeOutputLanguageAndRegisterPath: vi.fn(),
   };
 });
 
@@ -524,6 +525,56 @@ describe('SettingsDialog', () => {
       unmount();
     });
 
+    it('should not save number settings below their configured minimum', async () => {
+      vi.mocked(saveModifiedSettings).mockClear();
+
+      const settings = createMockSettings();
+      const onSelect = vi.fn();
+      const component = (
+        <KeypressProvider kittyProtocolEnabled={false}>
+          <SettingsDialog settings={settings} onSelect={onSelect} />
+        </KeypressProvider>
+      );
+
+      const { stdin, unmount, lastFrame } = render(component);
+
+      await waitFor(() => {
+        expect(lastFrame()).toContain('Settings');
+      });
+
+      const cleanupPeriodIndex = getDialogSettingKeys().indexOf(
+        'general.cleanupPeriodDays',
+      );
+      expect(cleanupPeriodIndex).toBeGreaterThanOrEqual(0);
+
+      const press = async (key: string) => {
+        act(() => {
+          stdin.write(key);
+        });
+        await wait();
+      };
+
+      for (let i = 0; i < cleanupPeriodIndex; i++) {
+        await press(TerminalKeys.DOWN_ARROW as string);
+      }
+
+      await press(TerminalKeys.ENTER as string);
+      await press('-');
+      await press('1');
+      await press(TerminalKeys.ENTER as string);
+
+      await wait();
+
+      const cleanupPeriodCall = vi
+        .mocked(saveModifiedSettings)
+        .mock.calls.find((call) =>
+          (call[0] as Set<string>).has('general.cleanupPeriodDays'),
+        );
+      expect(cleanupPeriodCall).toBeUndefined();
+
+      unmount();
+    });
+
     describe('enum values', () => {
       it('toggles enum values with the enter key', async () => {
         vi.mocked(saveModifiedSettings).mockClear();

packages/cli/src/ui/components/SettingsDialog.tsx

@@ -26,6 +26,7 @@ import {
   setPendingSettingValueAny,
   getNestedValue,
   getEffectiveValue,
+  validateSettingValue,
 } from '../../utils/settingsUtils.js';
 import { writeOutputLanguageAndRegisterPath } from '../../utils/languageUtils.js';
 import {
@@ -325,6 +326,16 @@ export function SettingsDialog({
       }
     }
 
+    if (definition) {
+      const validationError = validateSettingValue(definition, parsed);
+      if (validationError) {
+        setEditingKey(null);
+        setEditBuffer('');
+        setEditCursorPos(0);
+        return;
+      }
+    }
+
     // Update pending
     setPendingSettings((prev) =>
       parsed === undefined

packages/cli/src/utils/settingsUtils.test.ts

@@ -36,6 +36,7 @@ import {
   getEffectiveDisplayValue,
   setNestedPropertySafe,
   setNestedPropertyForce,
+  validateSettingValue,
 } from './settingsUtils.js';
 import {
   getSettingsSchema,
@@ -79,6 +80,16 @@ describe('SettingsUtils', () => {
         description: 'A test field',
         showInDialog: true,
       },
+      numberWithMinimum: {
+        type: 'number',
+        label: 'Number With Minimum',
+        category: 'Basic',
+        requiresRestart: false,
+        default: 0,
+        minimum: 0,
+        description: 'A number field with a minimum.',
+        showInDialog: true,
+      },
       advanced: {
         type: 'object',
         label: 'Advanced',
@@ -211,6 +222,25 @@ describe('SettingsUtils', () => {
       });
     });
 
+    describe('validateSettingValue', () => {
+      it('accepts finite numbers at the configured minimum', () => {
+        const definition = getSettingDefinition('numberWithMinimum');
+        expect(definition).toBeDefined();
+
+        expect(validateSettingValue(definition!, 0)).toBeUndefined();
+        expect(validateSettingValue(definition!, 1)).toBeUndefined();
+      });
+
+      it('rejects numbers below the configured minimum', () => {
+        const definition = getSettingDefinition('numberWithMinimum');
+        expect(definition).toBeDefined();
+
+        expect(validateSettingValue(definition!, -1)).toBe(
+          'Value must be >= 0',
+        );
+      });
+    });
+
     describe('requiresRestart', () => {
       it('should return true for settings that require restart', () => {
         expect(requiresRestart('ui.requiresRestart')).toBe(true);

packages/cli/src/utils/settingsUtils.ts

@@ -309,6 +309,39 @@ const SETTINGS_DIALOG_ORDER: readonly string[] = [
   'privacy.usageStatisticsEnabled',
 ] as const;
 
+export const MAX_SETTING_STRING_VALUE_LENGTH = 1024;
+
+export function validateSettingValue(
+  def: SettingDefinition,
+  value: unknown,
+): string | undefined {
+  switch (def.type) {
+    case 'boolean':
+      if (typeof value !== 'boolean') return 'Value must be a boolean';
+      break;
+    case 'number':
+      if (typeof value !== 'number' || !Number.isFinite(value))
+        return 'Value must be a finite number';
+      if (def.minimum !== undefined && value < def.minimum)
+        return `Value must be >= ${def.minimum}`;
+      break;
+    case 'string':
+      if (typeof value !== 'string') return 'Value must be a string';
+      if (value.length > MAX_SETTING_STRING_VALUE_LENGTH)
+        return `Value exceeds ${MAX_SETTING_STRING_VALUE_LENGTH}-character limit`;
+      break;
+    case 'enum':
+      if (!def.options?.some((opt) => opt.value === value)) {
+        const allowed = def.options?.map((o) => o.value).join(', ') ?? '';
+        return `Value must be one of: ${allowed}`;
+      }
+      break;
+    default:
+      return `Settings of type '${def.type}' cannot be modified via this API`;
+  }
+  return undefined;
+}
+
 /**
  * Get all setting keys that should be shown in the dialog, sorted by display order
  */

packages/vscode-ide-companion/schemas/settings.schema.json

@@ -101,7 +101,8 @@
         "cleanupPeriodDays": {
           "description": "Number of days to retain ~/.qwen/file-history/ session backups used by /rewind and background subagent transcripts under <projectDir>/subagents/. Data older than this is removed by a background housekeeping pass that runs at most once per day. Set to 0 for minimum retention (~1 hour) — protects sessions touched in the last hour, plus the currently active session.",
           "type": "number",
-          "default": 30
+          "default": 30,
+          "minimum": 0
         },
         "gitCoAuthor": {
           "description": "Attribution added to git commits and pull requests created through Qwen Code.",

scripts/generate-settings-schema.ts

@@ -38,6 +38,7 @@ interface JsonSchemaProperty {
   items?: JsonSchemaProperty;
   enum?: (string | number)[];
   default?: unknown;
+  minimum?: number;
   additionalProperties?: boolean | JsonSchemaProperty;
   required?: string[];
   oneOf?: JsonSchemaProperty[];
@@ -184,6 +185,10 @@ function convertSettingToJsonSchema(
     }
   }
 
+  if (setting.type === 'number' && setting.minimum !== undefined) {
+    schema.minimum = setting.minimum;
+  }
+
   // If the field accepts a legacy primitive shape (e.g. a boolean that was
   // later expanded into an object), wrap with `anyOf` so existing values
   // in users' settings.json don't trip the IDE schema validator while