fix(security): address config lockdown review findings

- Log leakage: remove valueToStore content from console.error at parse
  and success paths; replace with type/key metadata only. Also suppress
  the full config dump in getConfig() (key count only).
- Misleading editable flag: derive editable from !securityCritical so
  the config schema correctly reports security-critical keys as non-editable
  via the MCP tool.
- Auth bypass (opt-in hardening): add DESKTOP_COMMANDER_UI_TOKEN env-var
  enforcement for _internal_set_config_value. When the host sets this var,
  callers must supply a matching _uiToken in args (timing-safe comparison).
  The config-editor UI reads window.__DC_UI_TOKEN injected by the hosting
  app's preload and passes it automatically. When the env var is unset the
  behaviour is unchanged for backward compatibility.
- Docstrings: add JSDoc to pathExists, detectAvailableShells (config.ts),
  listProcesses, killProcess (process.ts), and isConfigFieldKey
  (config-field-definitions.ts) to restore docstring coverage above 80%.

Author: sorlen008

src/config-field-definitions.ts

@@ -54,6 +54,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);
@@ -1242,7 +1251,33 @@ server.setRequestHandler(CallToolRequestSchema, async (request: CallToolRequest)
             // 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.
-            case "_internal_set_config_value":
+            // 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) {
@@ -1253,6 +1288,7 @@ server.setRequestHandler(CallToolRequestSchema, async (request: CallToolRequest)
                     };
                 }
                 break;
+            }
 
             case "get_usage_stats":
                 try {

src/tools/config.ts

@@ -14,6 +14,10 @@ import {
 
 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);
@@ -23,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 => {
@@ -117,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",
@@ -135,7 +146,7 @@ export async function getConfig() {
             key,
             value,
             valueType: definition.valueType,
-            editable: true,
+            editable: !(definition.securityCritical ?? false),
             securityCritical: definition.securityCritical ?? false,
           };
         }),
@@ -215,7 +226,7 @@ export async function setConfigValue(args: unknown, callerOrigin: 'mcp' | 'ui' =
           (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}`);
         }
@@ -272,7 +283,7 @@ export async function setConfigValue(args: unknown, callerOrigin: 'mcp' | 'ui' =
       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

@@ -7,6 +7,10 @@ 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 {
@@ -40,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) {

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

@@ -435,9 +435,16 @@ export function createConfigEditorController(callTool: ToolCall, trackConfigUiEv
         }
 
         try {
+            // 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,
+                ...(uiToken !== undefined ? { _uiToken: uiToken } : {}),
             });
 
             if (isToolErrorResult(setResult)) {