fix(security-review): Handle null shell output (#231)

Normalize shell command stdout and stderr before the prompt-shell path and shared tool-result mappers use string operations. This prevents /security-review from crashing when a shell tool returns null output fields and adds regression coverage for both direct mapper calls and prompt generation.

Fixes #165

Co-authored-by: Claude <noreply@anthropic.com>

src/tools/shellToolResultMappers.test.ts

@@ -0,0 +1,71 @@
+import { expect, test } from 'bun:test'
+import { BashTool } from './BashTool/BashTool.js'
+import { PowerShellTool } from './PowerShellTool/PowerShellTool.js'
+
+test('BashTool result mapper tolerates null stderr', () => {
+  const result = BashTool.mapToolResultToToolResultBlockParam(
+    {
+      stdout: 'ok',
+      stderr: null as unknown as string,
+      interrupted: false,
+    },
+    'tool-1',
+  )
+
+  expect(result).toMatchObject({
+    type: 'tool_result',
+    tool_use_id: 'tool-1',
+    content: 'ok',
+  })
+})
+
+test('BashTool result mapper tolerates null stdout', () => {
+  const result = BashTool.mapToolResultToToolResultBlockParam(
+    {
+      stdout: null as unknown as string,
+      stderr: 'problem',
+      interrupted: false,
+    },
+    'tool-2',
+  )
+
+  expect(result).toMatchObject({
+    type: 'tool_result',
+    tool_use_id: 'tool-2',
+    content: 'problem',
+  })
+})
+
+test('PowerShellTool result mapper tolerates null stderr', () => {
+  const result = PowerShellTool.mapToolResultToToolResultBlockParam(
+    {
+      stdout: 'ok',
+      stderr: null as unknown as string,
+      interrupted: false,
+    },
+    'tool-3',
+  )
+
+  expect(result).toMatchObject({
+    type: 'tool_result',
+    tool_use_id: 'tool-3',
+    content: 'ok',
+  })
+})
+
+test('PowerShellTool result mapper tolerates null stdout', () => {
+  const result = PowerShellTool.mapToolResultToToolResultBlockParam(
+    {
+      stdout: null as unknown as string,
+      stderr: 'problem',
+      interrupted: false,
+    },
+    'tool-4',
+  )
+
+  expect(result).toMatchObject({
+    type: 'tool_result',
+    tool_use_id: 'tool-4',
+    content: 'problem',
+  })
+})

src/utils/promptShellExecution.test.ts

@@ -0,0 +1,77 @@
+import { afterEach, expect, test } from 'bun:test'
+import { getEmptyToolPermissionContext } from '../Tool.js'
+import { BashTool } from '../tools/BashTool/BashTool.js'
+import { executeShellCommandsInPrompt } from './promptShellExecution.js'
+
+const originalCall = BashTool.call
+const originalMapToolResultToToolResultBlockParam =
+  BashTool.mapToolResultToToolResultBlockParam
+
+afterEach(() => {
+  BashTool.call = originalCall
+  BashTool.mapToolResultToToolResultBlockParam =
+    originalMapToolResultToToolResultBlockParam
+})
+
+test('executeShellCommandsInPrompt normalizes null shell output', async () => {
+  let normalizedResult:
+    | { stdout: string; stderr: string; interrupted: boolean }
+    | undefined
+
+  BashTool.call = (async () => ({
+    data: {
+      stdout: null,
+      stderr: null,
+      interrupted: false,
+    },
+  })) as unknown as typeof BashTool.call
+
+  BashTool.mapToolResultToToolResultBlockParam = (result, toolUseID) => {
+    normalizedResult = result as {
+      stdout: string
+      stderr: string
+      interrupted: boolean
+    }
+    return originalMapToolResultToToolResultBlockParam(result, toolUseID)
+  }
+
+  await executeShellCommandsInPrompt(
+    '```!\ngit status\n```',
+    {
+      abortController: new AbortController(),
+      options: {
+        commands: [],
+        debug: false,
+        mainLoopModel: 'sonnet',
+        tools: new Map(),
+        verbose: false,
+        thinkingConfig: { type: 'disabled' },
+        mcpClients: [],
+        mcpResources: {},
+        isNonInteractiveSession: false,
+        agentDefinitions: {
+          systemDefinitions: [],
+          projectDefinitions: [],
+          userDefinitions: [],
+        },
+      },
+      readFileState: new Map(),
+      getAppState() {
+        return {
+          toolPermissionContext: {
+            ...getEmptyToolPermissionContext(),
+            alwaysAllowRules: { command: ['Bash(*)'] },
+          },
+        }
+      },
+      setAppState() {},
+    } as never,
+    'security-review',
+  )
+
+  expect(normalizedResult).toEqual({
+    stdout: '',
+    stderr: '',
+    interrupted: false,
+  })
+})

src/utils/promptShellExecution.ts

@@ -16,7 +16,11 @@ import { processToolResultBlock } from './toolResultStorage.js'
 // _simulatedSedEdit) that PowerShellTool's does not.
 // NOTE: call() is invoked directly here, bypassing validateInput — any
 // load-bearing check must live in call() itself (see PR #23311).
-type ShellOut = { stdout: string; stderr: string; interrupted: boolean }
+type ShellOut = {
+  stdout: string | null | undefined
+  stderr: string | null | undefined
+  interrupted: boolean
+}
 type PromptShellTool = Tool & {
   call(
     input: { command: string },
@@ -113,17 +117,25 @@ export async function executeShellCommandsInPrompt(
           }
 
           const { data } = await shellTool.call({ command }, context)
+          const normalizedData = {
+            ...data,
+            stdout: typeof data.stdout === 'string' ? data.stdout : '',
+            stderr: typeof data.stderr === 'string' ? data.stderr : '',
+          }
           // Reuse the same persistence flow as regular Bash tool calls
           const toolResultBlock = await processToolResultBlock(
             shellTool,
-            data,
+            normalizedData,
             randomUUID(),
           )
           // Extract the string content from the block
           const output =
             typeof toolResultBlock.content === 'string'
               ? toolResultBlock.content
-              : formatBashOutput(data.stdout, data.stderr)
+              : formatBashOutput(
+                  normalizedData.stdout,
+                  normalizedData.stderr,
+                )
           // Function replacer — String.replace interprets $$, $&, $`, $' in
           // the replacement string even with a string search pattern. Shell
           // output (especially PowerShell: $env:PATH, $$, $PSVersionTable)
@@ -143,21 +155,23 @@ export async function executeShellCommandsInPrompt(
 }
 
 function formatBashOutput(
-  stdout: string,
+  stdout: string | null | undefined,
-  stderr: string,
+  stderr: string | null | undefined,
   inline = false,
 ): string {
+  const normalizedStdout = typeof stdout === 'string' ? stdout : ''
+  const normalizedStderr = typeof stderr === 'string' ? stderr : ''
   const parts: string[] = []
 
-  if (stdout.trim()) {
+  if (normalizedStdout.trim()) {
-    parts.push(stdout.trim())
+    parts.push(normalizedStdout.trim())
   }
 
-  if (stderr.trim()) {
+  if (normalizedStderr.trim()) {
     if (inline) {
-      parts.push(`[stderr: ${stderr.trim()}]`)
+      parts.push(`[stderr: ${normalizedStderr.trim()}]`)
     } else {
-      parts.push(`[stderr]\n${stderr.trim()}`)
+      parts.push(`[stderr]\n${normalizedStderr.trim()}`)
     }
   }