test: add StartMCPServer input validation tests (TODO johannesjo#35)

Extract validateStartMCPServerArgs from the handler into a testable
exported function, then add Layer 4 tests covering all five rejection
paths (non-absolute projectRoot/worktreePath, ".." traversal, non-string
agentArgs elements, shell-special dockerContainerName). Each rejection
test also spies on fs.writeFileSync/copyFileSync to confirm no I/O
occurs before the error is thrown. Two positive tests verify optional
fields (worktreePath, dockerContainerName) can be omitted.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Author: brooksc

electron/ipc/register-mcp.test.ts

@@ -19,6 +19,7 @@ import {
   buildCoordinatorMCPConfig,
   getDockerMcpServerDestPath,
   selectMcpJsonDir,
+  validateStartMCPServerArgs,
 } from './register.js';
 import { getMCPRemoteServerUrl } from '../mcp/config.js';
 import { startRemoteServer } from '../remote/server.js';
@@ -193,6 +194,95 @@ describe('Layer 3 — MCP startup pipeline (no Electron, real FS)', () => {
   });
 });
 
+// ─────────────────────────────────────────────────────────────────────────────
+// Layer 4: StartMCPServer input validation
+// ─────────────────────────────────────────────────────────────────────────────
+
+const VALID_ARGS = {
+  coordinatorTaskId: 'coord-1',
+  projectId: 'proj-1',
+  projectRoot: '/absolute/project',
+  worktreePath: '/absolute/worktree',
+  agentArgs: ['--flag', 'value'],
+  dockerContainerName: 'my-container',
+};
+
+describe('Layer 4 — StartMCPServer input validation', () => {
+  it('accepts valid args without throwing', () => {
+    expect(() => validateStartMCPServerArgs(VALID_ARGS)).not.toThrow();
+  });
+
+  it('rejects non-absolute projectRoot', () => {
+    const writeFileSpy = vi.spyOn(fs, 'writeFileSync');
+    const copyFileSpy = vi.spyOn(fs, 'copyFileSync');
+
+    expect(() =>
+      validateStartMCPServerArgs({ ...VALID_ARGS, projectRoot: 'relative/path' }),
+    ).toThrow('projectRoot must be absolute');
+
+    expect(writeFileSpy).not.toHaveBeenCalled();
+    expect(copyFileSpy).not.toHaveBeenCalled();
+  });
+
+  it('rejects projectRoot containing ".."', () => {
+    const writeFileSpy = vi.spyOn(fs, 'writeFileSync');
+    const copyFileSpy = vi.spyOn(fs, 'copyFileSync');
+
+    expect(() => validateStartMCPServerArgs({ ...VALID_ARGS, projectRoot: '/tmp/../etc' })).toThrow(
+      'projectRoot must not contain ".."',
+    );
+
+    expect(writeFileSpy).not.toHaveBeenCalled();
+    expect(copyFileSpy).not.toHaveBeenCalled();
+  });
+
+  it('rejects non-absolute worktreePath', () => {
+    const writeFileSpy = vi.spyOn(fs, 'writeFileSync');
+    const copyFileSpy = vi.spyOn(fs, 'copyFileSync');
+
+    expect(() =>
+      validateStartMCPServerArgs({ ...VALID_ARGS, worktreePath: 'relative/worktree' }),
+    ).toThrow('worktreePath must be absolute');
+
+    expect(writeFileSpy).not.toHaveBeenCalled();
+    expect(copyFileSpy).not.toHaveBeenCalled();
+  });
+
+  it('rejects agentArgs containing a non-string element', () => {
+    const writeFileSpy = vi.spyOn(fs, 'writeFileSync');
+    const copyFileSpy = vi.spyOn(fs, 'copyFileSync');
+
+    expect(() => validateStartMCPServerArgs({ ...VALID_ARGS, agentArgs: [1, 'foo'] })).toThrow(
+      'agentArgs must be a string array',
+    );
+
+    expect(writeFileSpy).not.toHaveBeenCalled();
+    expect(copyFileSpy).not.toHaveBeenCalled();
+  });
+
+  it('rejects dockerContainerName with shell-special characters', () => {
+    const writeFileSpy = vi.spyOn(fs, 'writeFileSync');
+    const copyFileSpy = vi.spyOn(fs, 'copyFileSync');
+
+    expect(() =>
+      validateStartMCPServerArgs({ ...VALID_ARGS, dockerContainerName: '; rm -rf /' }),
+    ).toThrow('dockerContainerName contains invalid characters');
+
+    expect(writeFileSpy).not.toHaveBeenCalled();
+    expect(copyFileSpy).not.toHaveBeenCalled();
+  });
+
+  it('accepts worktreePath undefined (optional field)', () => {
+    const { worktreePath: _, ...argsWithoutWorktree } = VALID_ARGS;
+    expect(() => validateStartMCPServerArgs(argsWithoutWorktree)).not.toThrow();
+  });
+
+  it('accepts dockerContainerName undefined (optional field)', () => {
+    const { dockerContainerName: _, ...argsWithoutDocker } = VALID_ARGS;
+    expect(() => validateStartMCPServerArgs(argsWithoutDocker)).not.toThrow();
+  });
+});
+
 // ─────────────────────────────────────────────────────────────────────────────
 // Layer 5: Failure-mode tests
 // ─────────────────────────────────────────────────────────────────────────────

electron/ipc/register.ts

@@ -164,6 +164,22 @@ function validatePath(p: unknown, label: string): void {
   if (p.includes('..')) throw new Error(`${label} must not contain ".."`);
 }
 
+/** Validates renderer-supplied args for StartMCPServer before any file I/O. Exported for testing. */
+export function validateStartMCPServerArgs(args: Record<string, unknown>): void {
+  assertString(args.coordinatorTaskId, 'coordinatorTaskId');
+  assertString(args.projectId, 'projectId');
+  validatePath(args.projectRoot, 'projectRoot');
+  if (args.worktreePath !== undefined) validatePath(args.worktreePath, 'worktreePath');
+  if (args.agentCommand !== undefined) assertString(args.agentCommand, 'agentCommand');
+  if (args.agentArgs !== undefined) assertStringArray(args.agentArgs, 'agentArgs');
+  if (args.dockerContainerName !== undefined) {
+    assertString(args.dockerContainerName, 'dockerContainerName');
+    if (!/^[a-zA-Z0-9_.-]+$/.test(args.dockerContainerName as string)) {
+      throw new Error('dockerContainerName contains invalid characters');
+    }
+  }
+}
+
 /** Reject relative paths that attempt directory traversal or are absolute. */
 function validateRelativePath(p: unknown, label: string): void {
   if (typeof p !== 'string') throw new Error(`${label} must be a string`);
@@ -1267,18 +1283,7 @@ export function registerAllHandlers(win: BrowserWindow): void {
         dockerContainerName?: string;
       },
     ) => {
-      assertString(args.coordinatorTaskId, 'coordinatorTaskId');
-      assertString(args.projectId, 'projectId');
-      validatePath(args.projectRoot, 'projectRoot');
-      if (args.worktreePath !== undefined) validatePath(args.worktreePath, 'worktreePath');
-      if (args.agentCommand !== undefined) assertString(args.agentCommand, 'agentCommand');
-      if (args.agentArgs !== undefined) assertStringArray(args.agentArgs, 'agentArgs');
-      if (args.dockerContainerName !== undefined) {
-        assertString(args.dockerContainerName, 'dockerContainerName');
-        if (!/^[a-zA-Z0-9_.-]+$/.test(args.dockerContainerName)) {
-          throw new Error('dockerContainerName contains invalid characters');
-        }
-      }
+      validateStartMCPServerArgs(args as unknown as Record<string, unknown>);
 
       await enableCoordinatorMode();
       if (!coordinator) return;