fix(security): lock down security-critical config keys from runtime modification

src/config-field-definitions.ts

@@ -4,6 +4,21 @@ export type ConfigFieldDefinition = {
   label: string;
   description: string;
   valueType: ConfigFieldValueType;
+  /**
+   * Whether this field is editable in the config-editor UI.
+   * Defaults to true when omitted — all fields can be modified via the UI.
+   * Note: fields with securityCritical:true additionally require
+   * callerOrigin:'ui' in the server-side setConfigValue() check; this flag
+   * only controls whether the edit control is shown in the UI.
+   */
+  editable?: boolean;
+  /**
+   * When true, this key cannot be changed by MCP tool calls — only via the
+   * config-editor UI (which uses the internal `_internal_set_config_value`
+   * handler with a trusted server-side callerOrigin).  This prevents
+   * prompt-injection attacks from disabling safety controls at runtime.
+   */
+  securityCritical?: boolean;
 };
 
 // Single source of truth for user-editable configuration fields.
@@ -12,16 +27,19 @@ export const CONFIG_FIELD_DEFINITIONS = {
     label: 'Blocked Commands',
     description: 'This is your personal safety blocklist. If a command appears here, Desktop Commander will refuse to run it even if a prompt asks for it. Add risky commands you never want executed by mistake.',
     valueType: 'array',
+    securityCritical: true,
   },
   allowedDirectories: {
     label: 'Allowed Folders',
     description: 'These are the folders Desktop Commander is allowed to read and edit. Think of this as a permission list. Keeping it small is safer. If this list is empty, Desktop Commander can access your entire filesystem.',
     valueType: 'array',
+    securityCritical: true,
   },
   defaultShell: {
     label: 'Default Shell',
     description: 'This is the shell used for new command sessions (for example /bin/bash or /bin/zsh). Only change this if you know your environment requires a specific shell.',
     valueType: 'string',
+    securityCritical: true,
   },
   telemetryEnabled: {
     label: 'Anonymous Telemetry',
@@ -44,6 +62,10 @@ export type ConfigFieldKey = keyof typeof CONFIG_FIELD_DEFINITIONS;
 
 export const CONFIG_FIELD_KEYS = Object.keys(CONFIG_FIELD_DEFINITIONS) as ConfigFieldKey[];
 
+/**
+ * Type guard that checks whether `value` is a valid config field key.
+ * Used to validate tool-argument keys before accessing CONFIG_FIELD_DEFINITIONS.
+ */
 export function isConfigFieldKey(value: string): value is ConfigFieldKey {
   return Object.prototype.hasOwnProperty.call(CONFIG_FIELD_DEFINITIONS, value);
 }

src/server.ts

@@ -13,8 +13,17 @@ import {
     type InitializeRequest,
 } from "@modelcontextprotocol/sdk/types.js";
 import { zodToJsonSchema } from "zod-to-json-schema";
+import { timingSafeEqual } from 'crypto';
 import { getSystemInfo, getOSSpecificGuidance, getPathGuidance, getDevelopmentToolGuidance } from './utils/system-info.js';
 
+// Optional per-session token for the config-editor UI.
+// When the env var DESKTOP_COMMANDER_UI_TOKEN is set by the hosting app, callers of
+// _internal_set_config_value must supply the matching value in args._uiToken.  The
+// hosting app is responsible for injecting the same token into the UI webview (e.g.
+// via window.__DC_UI_TOKEN in an Electron preload script).
+// If the env var is NOT set, token validation is skipped for backward compatibility.
+const UI_SESSION_TOKEN: string | null = process.env.DESKTOP_COMMANDER_UI_TOKEN ?? null;
+
 // Get system information once at startup
 const SYSTEM_INFO = getSystemInfo();
 const OS_GUIDANCE = getOSSpecificGuidance(SYSTEM_INFO);
@@ -253,21 +262,21 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
                 name: "set_config_value",
                 description: `
                         Set a specific configuration value by key.
-
-                        WARNING: Should be used in a separate chat from file operations and 
-                        command execution to prevent security issues.
-
-                        Config keys include:
-                        - blockedCommands (array)
-                        - defaultShell (string)
-                        - allowedDirectories (array of paths)
+
+                        Security-critical keys (blockedCommands, allowedDirectories, defaultShell)
+                        can ONLY be changed through the config editor UI, not via this tool.
+                        This prevents prompt-injection attacks from disabling safety controls.
+
+                        Config keys settable by the AI agent:
                         - fileReadLineLimit (number, max lines for read_file)
                         - fileWriteLineLimit (number, max lines per write_file call)
                         - telemetryEnabled (boolean)
-
-                        IMPORTANT: Setting allowedDirectories to an empty array ([]) allows full access 
-                        to the entire file system, regardless of the operating system.
-
+
+                        Config keys settable only via config editor UI:
+                        - blockedCommands (array)
+                        - defaultShell (string)
+                        - allowedDirectories (array of paths)
+
                         ${CMD_PREFIX_DESCRIPTION}`,
                 inputSchema: zodToJsonSchema(SetConfigValueArgsSchema),
                 annotations: {
@@ -1029,7 +1038,8 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
                 description: `
                         Terminate a running process by PID.
 
-                        Use with caution as this will forcefully terminate the specified process.
+                        Only processes started by Desktop Commander (via start_process) can be
+                        terminated. Arbitrary system PIDs are rejected for safety.
 
                         ${CMD_PREFIX_DESCRIPTION}`,
                 inputSchema: zodToJsonSchema(KillProcessArgsSchema),
@@ -1190,9 +1200,9 @@ server.setRequestHandler(CallToolRequestSchema, async (request: CallToolRequest)
             }
         }
 
-        if (name === 'set_config_value' && args && typeof args === 'object' && 'key' in args) {
+        if ((name === 'set_config_value' || name === '_internal_set_config_value') && args && typeof args === 'object' && 'key' in args) {
             telemetryData.set_config_value_key_name = (args as any).key;
-            telemetryData.call_origin = (args as any).origin === 'ui' ? 'ui' : 'llm';
+            telemetryData.call_origin = name === '_internal_set_config_value' ? 'ui' : 'mcp';
         }
         if (name === 'get_prompts' && args && typeof args === 'object') {
             const promptArgs = args as any;
@@ -1229,7 +1239,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request: CallToolRequest)
                 break;
             case "set_config_value":
                 try {
-                    result = await setConfigValue(args);
+                    result = await setConfigValue(args, 'mcp');
                 } catch (error) {
                     capture('server_request_error', { message: `Error in set_config_value handler: ${error}` });
                     result = {
@@ -1238,6 +1248,47 @@ server.setRequestHandler(CallToolRequestSchema, async (request: CallToolRequest)
                     };
                 }
                 break;
+            // Internal-only handler for the config editor UI.  Not listed in
+            // the tools catalog, so AI agents cannot discover or call it.
+            // The 'ui' callerOrigin allows security-critical keys to be changed.
+            // A per-session token (UI_SESSION_TOKEN) provides a second layer of
+            // defence: callers must supply the token in args._uiToken.  The
+            // hosting app is responsible for injecting the token into the UI.
+            case "_internal_set_config_value": {
+                // Validate the per-session UI token when enforcement is active.
+                // Enforcement is opt-in: set DESKTOP_COMMANDER_UI_TOKEN in the
+                // server environment to enable it.  The hosting app must also
+                // inject the same token into the UI webview (window.__DC_UI_TOKEN).
+                if (UI_SESSION_TOKEN !== null) {
+                    const internalArgs = (args ?? {}) as Record<string, unknown>;
+                    const providedToken = typeof internalArgs._uiToken === 'string'
+                        ? internalArgs._uiToken : '';
+                    const expectedBuf = Buffer.from(UI_SESSION_TOKEN);
+                    const providedBuf = Buffer.allocUnsafe(expectedBuf.length);
+                    providedBuf.fill(0);
+                    Buffer.from(providedToken).copy(providedBuf, 0, 0, expectedBuf.length);
+                    const tokenValid = providedToken.length === UI_SESSION_TOKEN.length
+                        && timingSafeEqual(expectedBuf, providedBuf);
+                    if (!tokenValid) {
+                        capture('server_request_error', { message: 'Rejected _internal_set_config_value: invalid or missing UI session token' });
+                        result = {
+                            content: [{ type: "text", text: `Error: Unauthorized` }],
+                            isError: true,
+                        };
+                        break;
+                    }
+                }
+                try {
+                    result = await setConfigValue(args, 'ui');
+                } catch (error) {
+                    capture('server_request_error', { message: `Error in _internal_set_config_value handler: ${error}` });
+                    result = {
+                        content: [{ type: "text", text: `Error: Failed to set configuration value` }],
+                        isError: true,
+                    };
+                }
+                break;
+            }
 
             case "get_usage_stats":
                 try {

src/tools/config.ts

@@ -9,10 +9,15 @@ import {
   CONFIG_FIELD_DEFINITIONS,
   CONFIG_FIELD_KEYS,
   isConfigFieldKey,
+  type ConfigFieldDefinition,
 } from '../config-field-definitions.js';
 
 const ALLOWED_CONFIG_KEYS = new Set(CONFIG_FIELD_KEYS);
 
+/**
+ * Returns true if the given path is accessible and executable.
+ * Used to probe shell binary availability before reporting them in config.
+ */
 async function pathExists(pathValue: string): Promise<boolean> {
   try {
     await access(pathValue, fsConstants.X_OK);
@@ -22,6 +27,13 @@ async function pathExists(pathValue: string): Promise<boolean> {
   }
 }
 
+/**
+ * Probes for shells available on the current system by testing known binary
+ * paths from the system-info default shell and a fixed candidate list.
+ *
+ * @param systemInfo System information object returned by getSystemInfo().
+ * @returns Sorted, deduplicated array of absolute shell paths that exist on disk.
+ */
 async function detectAvailableShells(systemInfo: ReturnType<typeof getSystemInfo>): Promise<string[]> {
   const detected = new Set<string>();
   const add = (shell: string): void => {
@@ -116,7 +128,7 @@ export async function getConfig() {
     };
     const availableShells = await detectAvailableShells(systemInfo);
 
-    console.error(`getConfig result: ${JSON.stringify(configWithSystemInfo, null, 2)}`);
+    console.error(`getConfig: returning config with ${CONFIG_FIELD_KEYS.length} keys`);
     return {
       content: [{
         type: "text",
@@ -128,13 +140,14 @@ export async function getConfig() {
           availableShells,
         },
         entries: CONFIG_FIELD_KEYS.map((key) => {
-          const definition = CONFIG_FIELD_DEFINITIONS[key];
+          const definition: ConfigFieldDefinition = CONFIG_FIELD_DEFINITIONS[key];
           const value = (configWithSystemInfo as Record<string, unknown>)[key];
           return {
             key,
             value,
             valueType: definition.valueType,
-            editable: true,
+            editable: definition.editable ?? true,
+            securityCritical: definition.securityCritical ?? false,
           };
         }),
       },
@@ -153,10 +166,16 @@ export async function getConfig() {
 }
 
 /**
- * Set a specific config value
+ * Set a specific config value.
+ *
+ * @param args  Tool arguments (key + value).  Parsed via SetConfigValueArgsSchema.
+ * @param callerOrigin  Trusted, server-set origin.  Only `'ui'` (set by the
+ *   internal config-editor handler) may modify security-critical keys.  MCP
+ *   tool calls always pass `'mcp'` (the default), so the AI agent can never
+ *   reach security-critical keys regardless of what it sends in the arguments.
  */
-export async function setConfigValue(args: unknown) {
-  console.error(`setConfigValue called with args: ${JSON.stringify(args)}`);
+export async function setConfigValue(args: unknown, callerOrigin: 'mcp' | 'ui' = 'mcp') {
+  console.error(`setConfigValue called with callerOrigin: ${callerOrigin}`);
   try {
     const parsed = SetConfigValueArgsSchema.safeParse(args);
     if (!parsed.success) {
@@ -180,6 +199,23 @@ export async function setConfigValue(args: unknown) {
       };
     }
 
+    // Security-critical keys (blockedCommands, allowedDirectories, defaultShell)
+    // can only be changed through the config-editor UI, not by LLM tool calls.
+    // This prevents prompt-injection attacks from disabling safety controls.
+    // The callerOrigin is set server-side — it is never taken from tool arguments.
+    const fieldDef: ConfigFieldDefinition = CONFIG_FIELD_DEFINITIONS[parsed.data.key];
+    if (fieldDef.securityCritical && callerOrigin !== 'ui') {
+      return {
+        content: [{
+          type: "text",
+          text: `Security-critical key "${parsed.data.key}" cannot be modified by the AI agent. ` +
+            `This restriction prevents prompt-injection attacks from disabling safety controls. ` +
+            `To change this setting, use the Desktop Commander config editor UI (get_config tool).`
+        }],
+        isError: true
+      };
+    }
+
     try {
       const fieldDefinition = CONFIG_FIELD_DEFINITIONS[parsed.data.key];
       // Parse string values that should be arrays or objects
@@ -190,7 +226,7 @@ export async function setConfigValue(args: unknown) {
           (valueToStore.startsWith('[') || valueToStore.startsWith('{'))) {
         try {
           valueToStore = JSON.parse(valueToStore);
-          console.error(`Parsed string value to object/array: ${JSON.stringify(valueToStore)}`);
+          console.error(`Parsed string value to object/array for key ${parsed.data.key} (type: ${typeof valueToStore})`);
         } catch (parseError) {
           console.error(`Failed to parse string as JSON, using as-is: ${parseError}`);
         }
@@ -247,7 +283,7 @@ export async function setConfigValue(args: unknown) {
       await configManager.setValue(parsed.data.key, valueToStore);
       // Get the updated configuration to show the user
       const updatedConfig = await configManager.getConfig();
-      console.error(`setConfigValue: Successfully set ${parsed.data.key} to ${JSON.stringify(valueToStore)}`);
+      console.error(`setConfigValue: Successfully set ${parsed.data.key} (type: ${typeof valueToStore}, isArray: ${Array.isArray(valueToStore)})`);
       return {
         content: [{
           type: "text",

src/tools/process.ts

@@ -3,9 +3,14 @@ import { promisify } from 'util';
 import os from 'os';
 import { ProcessInfo, ServerResult } from '../types.js';
 import { KillProcessArgsSchema } from './schemas.js';
+import { terminalManager } from '../terminal-manager.js';
 
 const execAsync = promisify(exec);
 
+/**
+ * Lists all running system processes with their PID, command, CPU, and memory usage.
+ * Uses `ps aux` on Unix and `tasklist` on Windows.
+ */
 export async function listProcesses(): Promise<ServerResult> {
   const command = os.platform() === 'win32' ? 'tasklist' : 'ps aux';
   try {
@@ -39,6 +44,13 @@ export async function listProcesses(): Promise<ServerResult> {
   }
 }
 
+/**
+ * Terminates a process started via Desktop Commander's start_process tool.
+ * Only processes tracked by the internal terminal manager can be killed,
+ * preventing the AI agent from terminating arbitrary system processes.
+ *
+ * @param args Tool arguments containing the target PID. Parsed via KillProcessArgsSchema.
+ */
 export async function killProcess(args: unknown): Promise<ServerResult> {
   const parsed = KillProcessArgsSchema.safeParse(args);
   if (!parsed.success) {
@@ -48,6 +60,18 @@ export async function killProcess(args: unknown): Promise<ServerResult> {
     };
   }
 
+  // Scope kill_process to sessions managed by Desktop Commander.
+  // This prevents the AI agent from terminating arbitrary system processes.
+  const session = terminalManager.getSession(parsed.data.pid);
+  if (!session) {
+    return {
+      content: [{ type: "text", text: `Error: PID ${parsed.data.pid} is not a process managed by Desktop Commander. ` +
+        `kill_process and force_terminate can only terminate processes started via start_process. ` +
+        `For other system processes, use OS-level tools directly.` }],
+      isError: true,
+    };
+  }
+
   try {
     process.kill(parsed.data.pid);
     return {

src/tools/schemas.ts

@@ -12,7 +12,6 @@ export const SetConfigValueArgsSchema = z.object({
     z.array(z.string()),
     z.null(),
   ]),
-  origin: z.enum(['ui', 'llm']).optional(),
 });
 
 // Empty schemas

src/ui/config-editor/src/app.ts

@@ -435,10 +435,16 @@ export function createConfigEditorController(callTool: ToolCall, trackConfigUiEv
         }
 
         try {
-            const setResult = await callTool('set_config_value', {
+            // Include the UI session token when the hosting app has injected it
+            // (window.__DC_UI_TOKEN).  The server validates this token when
+            // DESKTOP_COMMANDER_UI_TOKEN is set in its environment.
+            const uiToken = (typeof window !== 'undefined' && (window as any).__DC_UI_TOKEN)
+                ? String((window as any).__DC_UI_TOKEN)
+                : undefined;
+            const setResult = await callTool('_internal_set_config_value', {
                 key: selected.key,
                 value: parsed.value,
-                origin: 'ui',
+                ...(uiToken !== undefined ? { _uiToken: uiToken } : {}),
             });
 
             if (isToolErrorResult(setResult)) {