diff --git a/src/config-field-definitions.ts b/src/config-field-definitions.ts index 77efa9bf..a991b717 100644 --- a/src/config-field-definitions.ts +++ b/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); } diff --git a/src/server.ts b/src/server.ts index abc8d2be..e51fb99f 100644 --- a/src/server.ts +++ b/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; + 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 { diff --git a/src/tools/config.ts b/src/tools/config.ts index 6d681da5..9c111a16 100644 --- a/src/tools/config.ts +++ b/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 { try { await access(pathValue, fsConstants.X_OK); @@ -22,6 +27,13 @@ async function pathExists(pathValue: string): Promise { } } +/** + * 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): Promise { const detected = new Set(); 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)[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", diff --git a/src/tools/process.ts b/src/tools/process.ts index 676f66f9..4e77735d 100644 --- a/src/tools/process.ts +++ b/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 { const command = os.platform() === 'win32' ? 'tasklist' : 'ps aux'; try { @@ -39,6 +44,13 @@ export async function listProcesses(): Promise { } } +/** + * 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 { const parsed = KillProcessArgsSchema.safeParse(args); if (!parsed.success) { @@ -48,6 +60,18 @@ export async function killProcess(args: unknown): Promise { }; } + // 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 { diff --git a/src/tools/schemas.ts b/src/tools/schemas.ts index 774fb99e..a2cdb39e 100644 --- a/src/tools/schemas.ts +++ b/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 diff --git a/src/ui/config-editor/src/app.ts b/src/ui/config-editor/src/app.ts index 8aacd518..b8f6d711 100644 --- a/src/ui/config-editor/src/app.ts +++ b/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)) {