From e2fa35cf4941218c01dc26d802d6cad29a339bd7 Mon Sep 17 00:00:00 2001 From: Famous Date: Sun, 1 Mar 2026 16:28:50 +0530 Subject: [PATCH 01/27] fix(accessibility): add screen reader support to RewindViewer --- .../cli/src/ui/components/RewindViewer.tsx | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index 048511dd77f..0ce330d0580 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -6,7 +6,7 @@ import type React from 'react'; import { useMemo, useState } from 'react'; -import { Box, Text } from 'ink'; +import { Box, Text, useIsScreenReaderEnabled } from 'ink'; import { useUIState } from '../contexts/UIStateContext.js'; import { type ConversationRecord, @@ -50,6 +50,7 @@ export const RewindViewer: React.FC = ({ }) => { const [isRewinding, setIsRewinding] = useState(false); const { terminalWidth, terminalHeight } = useUIState(); + const isScreenReaderEnabled = useIsScreenReaderEnabled(); const { selectedMessageId, getStats, @@ -182,6 +183,22 @@ export const RewindViewer: React.FC = ({ ); } + if (isScreenReaderEnabled) { + return ( + + Rewind - Select a conversation point: + {items.map((item, idx) => ( + + {idx + 1}.{' '} + {item.value.id === 'current-position' + ? 'Stay at current position' + : getCleanedRewindText(item.value)} + + ))} + Press Esc to exit, Enter to select, arrow keys to navigate. + + ); + } return ( Date: Sun, 1 Mar 2026 16:45:36 +0530 Subject: [PATCH 02/27] fix(accessibility): use BaseSelectionList for interactive screen reader view --- .../cli/src/ui/components/RewindViewer.tsx | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index 0ce330d0580..dc929e6560c 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -187,14 +187,34 @@ export const RewindViewer: React.FC = ({ return ( Rewind - Select a conversation point: - {items.map((item, idx) => ( - - {idx + 1}.{' '} - {item.value.id === 'current-position' - ? 'Stay at current position' - : getCleanedRewindText(item.value)} - - ))} + { + if (item?.id) { + if (item.id === 'current-position') { + onExit(); + } else { + selectMessage(item.id); + } + } + }} + renderItem={(itemWrapper) => { + const item = itemWrapper.value; + const text = + item.id === 'current-position' + ? 'Stay at current position' + : getCleanedRewindText(item); + return ( + + {itemWrapper.index + 1}. {text} + + ); + }} + /> Press Esc to exit, Enter to select, arrow keys to navigate. ); From 05f403d10dfc07993d4c1dc9b02a543fb3c63d2d Mon Sep 17 00:00:00 2001 From: Famous Date: Tue, 3 Mar 2026 01:58:34 +0530 Subject: [PATCH 03/27] fix(accessibility): address screen reader review feedback for RewindViewer --- .../src/ui/components/RewindConfirmation.tsx | 53 +++++++++++++++++++ .../src/ui/components/RewindViewer.test.tsx | 21 +++++++- .../cli/src/ui/components/RewindViewer.tsx | 26 ++++++--- 3 files changed, 92 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/ui/components/RewindConfirmation.tsx b/packages/cli/src/ui/components/RewindConfirmation.tsx index 5ff7e5e10ca..7c2726a7580 100644 --- a/packages/cli/src/ui/components/RewindConfirmation.tsx +++ b/packages/cli/src/ui/components/RewindConfirmation.tsx @@ -50,6 +50,8 @@ interface RewindConfirmationProps { onConfirm: (outcome: RewindOutcome) => void; terminalWidth: number; timestamp?: string; + // FIX #3: accept isScreenReaderEnabled to render accessible confirmation view + isScreenReaderEnabled?: boolean; } export const RewindConfirmation: React.FC = ({ @@ -57,6 +59,7 @@ export const RewindConfirmation: React.FC = ({ onConfirm, terminalWidth, timestamp, + isScreenReaderEnabled = false, }) => { useKeypress( (key) => { @@ -84,6 +87,56 @@ export const RewindConfirmation: React.FC = ({ ); }, [stats]); + // FIX #3: accessible plain-text confirmation view for screen reader users + if (isScreenReaderEnabled) { + return ( + + Confirm Rewind + + {stats && ( + + + {stats.fileCount === 1 + ? `File: ${stats.details?.at(0)?.fileName}` + : `${stats.fileCount} files affected`} + + Lines added: {stats.addedLines} + Lines removed: {stats.removedLines} + {timestamp && ({formatTimeAgo(timestamp)})} + + Note: Rewinding does not affect files edited manually or by the + shell tool. + + + )} + + {!stats && ( + + No code changes to revert. + {timestamp && ( + + {' '} + ({formatTimeAgo(timestamp)}) + + )} + + )} + + Select an action: + {/* FIX #6: use theme.text.secondary for navigation hint */} + + Use arrow keys to navigate, Enter to confirm, Esc to cancel. + + + + + ); + } + return ( { vi.restoreAllMocks(); }); + describe('Screen Reader Accessibility', () => { + it('renders the rewind viewer with conversation items', async () => { + const conversation = createConversation([ + { type: 'user', content: 'Hello', id: '1', timestamp: '1' }, + ]); + const { lastFrame, waitUntilReady, unmount } = renderWithProviders( + , + ); + await waitUntilReady(); + expect(lastFrame()).toContain('Rewind'); + expect(lastFrame()).toContain('Hello'); + unmount(); + }); + }); + describe('Rendering', () => { it.each([ { name: 'nothing interesting for empty conversation', messages: [] }, diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index dc929e6560c..906e4f5c7f0 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -130,6 +130,7 @@ export const RewindViewer: React.FC = ({ terminalHeight - DIALOG_PADDING - HEADER_HEIGHT - CONTROLS_HEIGHT - 2, ); + // FIX #5: maxItemsToShow used in both visual and accessible views const maxItemsToShow = Math.max(1, Math.floor(listHeight / 4)); if (selectedMessageId) { @@ -159,10 +160,13 @@ export const RewindViewer: React.FC = ({ (m) => m.id === selectedMessageId, ); return ( + // FIX #3: RewindConfirmation now receives isScreenReaderEnabled + // so it can render an accessible view for the confirmation step { if (outcome === RewindOutcome.Cancel) { clearSelection(); @@ -184,14 +188,18 @@ export const RewindViewer: React.FC = ({ } if (isScreenReaderEnabled) { + // FIX #5: slice items to maxItemsToShow to prevent overflow in small windows + const visibleItems = items.slice(0, maxItemsToShow); + return ( Rewind - Select a conversation point: { if (item?.id) { @@ -209,16 +217,20 @@ export const RewindViewer: React.FC = ({ ? 'Stay at current position' : getCleanedRewindText(item); return ( - - {itemWrapper.index + 1}. {text} - + // FIX #2: aria-state tied to index via showNumbers={true}, + // so screen readers will announce selection changes correctly + {text} ); }} /> - Press Esc to exit, Enter to select, arrow keys to navigate. + {/* FIX #6: use theme.text.secondary for navigation instructions */} + + Press Esc to exit, Enter to select, arrow keys to navigate. + ); } + return ( Date: Thu, 5 Mar 2026 02:05:41 +0530 Subject: [PATCH 04/27] fix(accessibility): address screen reader review feedback for RewindViewer --- .../src/ui/components/RewindConfirmation.tsx | 4 +- .../src/ui/components/RewindViewer.test.tsx | 33 ++ .../cli/src/ui/components/RewindViewer.tsx | 8 +- .../__snapshots__/RewindViewer.test.tsx.snap | 319 ++++++++++++++++++ 4 files changed, 356 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/ui/components/RewindConfirmation.tsx b/packages/cli/src/ui/components/RewindConfirmation.tsx index 7c2726a7580..6edaab78894 100644 --- a/packages/cli/src/ui/components/RewindConfirmation.tsx +++ b/packages/cli/src/ui/components/RewindConfirmation.tsx @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { Box, Text } from 'ink'; +import { Box, Text, useIsScreenReaderEnabled } from 'ink'; import type React from 'react'; import { useMemo } from 'react'; import { theme } from '../semantic-colors.js'; @@ -59,8 +59,8 @@ export const RewindConfirmation: React.FC = ({ onConfirm, terminalWidth, timestamp, - isScreenReaderEnabled = false, }) => { + const isScreenReaderEnabled = useIsScreenReaderEnabled(); useKeypress( (key) => { if (keyMatchers[Command.ESCAPE](key)) { diff --git a/packages/cli/src/ui/components/RewindViewer.test.tsx b/packages/cli/src/ui/components/RewindViewer.test.tsx index fd855838094..35b86b667cc 100644 --- a/packages/cli/src/ui/components/RewindViewer.test.tsx +++ b/packages/cli/src/ui/components/RewindViewer.test.tsx @@ -14,6 +14,11 @@ import type { MessageRecord, } from '@google/gemini-cli-core'; +vi.mock('ink', async () => { + const actual = await vi.importActual('ink'); + return { ...actual, useIsScreenReaderEnabled: vi.fn(() => false) }; +}); + vi.mock('./CliSpinner.js', () => ({ CliSpinner: () => 'MockSpinner', })); @@ -419,3 +424,31 @@ describe('RewindViewer', () => { unmount2(); }); }); +it('renders accessible screen reader view when screen reader is enabled', async () => { + const { useIsScreenReaderEnabled } = await import('ink'); + vi.mocked(useIsScreenReaderEnabled).mockReturnValue(true); + + const messages: MessageRecord[] = [ + { type: 'user', content: 'Hello world', id: '1', timestamp: '1' }, + { type: 'user', content: 'Second message', id: '2', timestamp: '2' }, + ]; + const conversation = createConversation(messages); + const onExit = vi.fn(); + const onRewind = vi.fn(); + + const { lastFrame, waitUntilReady, unmount } = renderWithProviders( + , + ); + await waitUntilReady(); + + const frame = lastFrame(); + expect(frame).toContain('Rewind - Select a conversation point:'); + expect(frame).toContain('Stay at current position'); + + vi.mocked(useIsScreenReaderEnabled).mockReturnValue(false); + unmount(); +}); diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index 906e4f5c7f0..0ac7dcc9e80 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -166,7 +166,6 @@ export const RewindViewer: React.FC = ({ stats={confirmationStats} terminalWidth={terminalWidth} timestamp={selectedMessage?.timestamp} - isScreenReaderEnabled={isScreenReaderEnabled} onConfirm={(outcome) => { if (outcome === RewindOutcome.Cancel) { clearSelection(); @@ -188,15 +187,12 @@ export const RewindViewer: React.FC = ({ } if (isScreenReaderEnabled) { - // FIX #5: slice items to maxItemsToShow to prevent overflow in small windows - const visibleItems = items.slice(0, maxItemsToShow); - return ( Rewind - Select a conversation point: Rendering > renders 'nothing interesting for empty conve " `; +exports[`RewindViewer > Screen Reader Accessibility > Content Filtering > 'removes reference markers' 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ some command @file │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Content Filtering > 'strips expanded MCP resource content' 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ read @server3:mcp://demo-resource hello │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Content Filtering > 'uses displayContent if present and do…' 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ clean display content │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Interaction Selection > 'cancels on Escape' > confirmation-dialog 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ Confirm Rewind │ +│ │ +│ No code changes to revert. (some time ago) │ +│ │ +│ Select an action: │ +│ │ +│ ● 1. Rewind conversation │ +│ 2. Do nothing (esc) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Interaction Selection > 'confirms on Enter' > confirmation-dialog 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ Confirm Rewind │ +│ │ +│ No code changes to revert. (some time ago) │ +│ │ +│ Select an action: │ +│ │ +│ ● 1. Rewind conversation │ +│ 2. Do nothing (esc) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Navigation > handles 'down' navigation > after-down 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Q1 │ +│ No files have been changed │ +│ │ +│ Q2 │ +│ No files have been changed │ +│ │ +│ Q3 │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Navigation > handles 'up' navigation > after-up 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Q1 │ +│ No files have been changed │ +│ │ +│ Q2 │ +│ No files have been changed │ +│ │ +│ ● Q3 │ +│ No files have been changed │ +│ │ +│ Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Navigation > handles cyclic navigation > cyclic-down 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Q1 │ +│ No files have been changed │ +│ │ +│ Q2 │ +│ No files have been changed │ +│ │ +│ Q3 │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Navigation > handles cyclic navigation > cyclic-up 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Q1 │ +│ No files have been changed │ +│ │ +│ Q2 │ +│ No files have been changed │ +│ │ +│ ● Q3 │ +│ No files have been changed │ +│ │ +│ Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Rendering > renders 'a single interaction' 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Hello │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Rendering > renders 'full text for selected item' 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ 1 │ +│ 2... │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > Rendering > renders 'nothing interesting for empty convers…' 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > updates content when conversation changes (background update) > after-update 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Message 1 │ +│ No files have been changed │ +│ │ +│ Message 2 │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > updates content when conversation changes (background update) > initial 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Message 1 │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > updates selection and expansion on navigation > after-down 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Line A │ +│ Line B... │ +│ No files have been changed │ +│ │ +│ Line 1 │ +│ Line 2... │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + +exports[`RewindViewer > Screen Reader Accessibility > updates selection and expansion on navigation > initial-state 1`] = ` +"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ +│ │ +│ > Rewind │ +│ │ +│ Line A │ +│ Line B... │ +│ No files have been changed │ +│ │ +│ Line 1 │ +│ Line 2... │ +│ No files have been changed │ +│ │ +│ ● Stay at current position │ +│ Cancel rewind and stay here │ +│ │ +│ │ +│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ +│ │ +╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ +" +`; + exports[`RewindViewer > updates content when conversation changes (background update) > after-update 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ From 311f2496b34a205bc95fea6db163d7d54d53413f Mon Sep 17 00:00:00 2001 From: Famous Date: Fri, 6 Mar 2026 00:06:30 +0530 Subject: [PATCH 05/27] fix(accessibility): remove dev comments, redundant prop, and fix screen reader test snapshots --- .../src/ui/components/RewindConfirmation.tsx | 5 - .../src/ui/components/RewindViewer.test.tsx | 10 + .../cli/src/ui/components/RewindViewer.tsx | 12 +- .../__snapshots__/RewindViewer.test.tsx.snap | 319 ------------------ 4 files changed, 11 insertions(+), 335 deletions(-) diff --git a/packages/cli/src/ui/components/RewindConfirmation.tsx b/packages/cli/src/ui/components/RewindConfirmation.tsx index 6edaab78894..bbfbf9dbee5 100644 --- a/packages/cli/src/ui/components/RewindConfirmation.tsx +++ b/packages/cli/src/ui/components/RewindConfirmation.tsx @@ -50,8 +50,6 @@ interface RewindConfirmationProps { onConfirm: (outcome: RewindOutcome) => void; terminalWidth: number; timestamp?: string; - // FIX #3: accept isScreenReaderEnabled to render accessible confirmation view - isScreenReaderEnabled?: boolean; } export const RewindConfirmation: React.FC = ({ @@ -86,8 +84,6 @@ export const RewindConfirmation: React.FC = ({ option.value !== RewindOutcome.RevertOnly, ); }, [stats]); - - // FIX #3: accessible plain-text confirmation view for screen reader users if (isScreenReaderEnabled) { return ( @@ -123,7 +119,6 @@ export const RewindConfirmation: React.FC = ({ )} Select an action: - {/* FIX #6: use theme.text.secondary for navigation hint */} Use arrow keys to navigate, Enter to confirm, Esc to cancel. diff --git a/packages/cli/src/ui/components/RewindViewer.test.tsx b/packages/cli/src/ui/components/RewindViewer.test.tsx index 35b86b667cc..7d99bd8eb26 100644 --- a/packages/cli/src/ui/components/RewindViewer.test.tsx +++ b/packages/cli/src/ui/components/RewindViewer.test.tsx @@ -77,6 +77,16 @@ describe('RewindViewer', () => { }); describe('Screen Reader Accessibility', () => { + beforeEach(async () => { + const { useIsScreenReaderEnabled } = await import('ink'); + vi.mocked(useIsScreenReaderEnabled).mockReturnValue(true); + }); + + afterEach(async () => { + const { useIsScreenReaderEnabled } = await import('ink'); + vi.mocked(useIsScreenReaderEnabled).mockReturnValue(false); + }); + it('renders the rewind viewer with conversation items', async () => { const conversation = createConversation([ { type: 'user', content: 'Hello', id: '1', timestamp: '1' }, diff --git a/packages/cli/src/ui/components/RewindViewer.tsx b/packages/cli/src/ui/components/RewindViewer.tsx index 0ac7dcc9e80..26f7282f619 100644 --- a/packages/cli/src/ui/components/RewindViewer.tsx +++ b/packages/cli/src/ui/components/RewindViewer.tsx @@ -129,8 +129,6 @@ export const RewindViewer: React.FC = ({ 5, terminalHeight - DIALOG_PADDING - HEADER_HEIGHT - CONTROLS_HEIGHT - 2, ); - - // FIX #5: maxItemsToShow used in both visual and accessible views const maxItemsToShow = Math.max(1, Math.floor(listHeight / 4)); if (selectedMessageId) { @@ -160,8 +158,6 @@ export const RewindViewer: React.FC = ({ (m) => m.id === selectedMessageId, ); return ( - // FIX #3: RewindConfirmation now receives isScreenReaderEnabled - // so it can render an accessible view for the confirmation step = ({ items={items} initialIndex={items.length - 1} isFocused={true} - // FIX #4: use showNumbers={true} instead of manually prepending numbers showNumbers={true} wrapAround={false} onSelect={(item: MessageRecord) => { @@ -212,14 +207,9 @@ export const RewindViewer: React.FC = ({ item.id === 'current-position' ? 'Stay at current position' : getCleanedRewindText(item); - return ( - // FIX #2: aria-state tied to index via showNumbers={true}, - // so screen readers will announce selection changes correctly - {text} - ); + return {text}; }} /> - {/* FIX #6: use theme.text.secondary for navigation instructions */} Press Esc to exit, Enter to select, arrow keys to navigate. diff --git a/packages/cli/src/ui/components/__snapshots__/RewindViewer.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/RewindViewer.test.tsx.snap index 87b1c1e74ed..06f7d49105f 100644 --- a/packages/cli/src/ui/components/__snapshots__/RewindViewer.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/RewindViewer.test.tsx.snap @@ -234,325 +234,6 @@ exports[`RewindViewer > Rendering > renders 'nothing interesting for empty conve " `; -exports[`RewindViewer > Screen Reader Accessibility > Content Filtering > 'removes reference markers' 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ some command @file │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Content Filtering > 'strips expanded MCP resource content' 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ read @server3:mcp://demo-resource hello │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Content Filtering > 'uses displayContent if present and do…' 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ clean display content │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Interaction Selection > 'cancels on Escape' > confirmation-dialog 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ Confirm Rewind │ -│ │ -│ No code changes to revert. (some time ago) │ -│ │ -│ Select an action: │ -│ │ -│ ● 1. Rewind conversation │ -│ 2. Do nothing (esc) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Interaction Selection > 'confirms on Enter' > confirmation-dialog 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ Confirm Rewind │ -│ │ -│ No code changes to revert. (some time ago) │ -│ │ -│ Select an action: │ -│ │ -│ ● 1. Rewind conversation │ -│ 2. Do nothing (esc) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Navigation > handles 'down' navigation > after-down 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ Q1 │ -│ No files have been changed │ -│ │ -│ Q2 │ -│ No files have been changed │ -│ │ -│ Q3 │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Navigation > handles 'up' navigation > after-up 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ Q1 │ -│ No files have been changed │ -│ │ -│ Q2 │ -│ No files have been changed │ -│ │ -│ ● Q3 │ -│ No files have been changed │ -│ │ -│ Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Navigation > handles cyclic navigation > cyclic-down 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ Q1 │ -│ No files have been changed │ -│ │ -│ Q2 │ -│ No files have been changed │ -│ │ -│ Q3 │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Navigation > handles cyclic navigation > cyclic-up 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ Q1 │ -│ No files have been changed │ -│ │ -│ Q2 │ -│ No files have been changed │ -│ │ -│ ● Q3 │ -│ No files have been changed │ -│ │ -│ Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Rendering > renders 'a single interaction' 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ Hello │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Rendering > renders 'full text for selected item' 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ 1 │ -│ 2... │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > Rendering > renders 'nothing interesting for empty convers…' 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > updates content when conversation changes (background update) > after-update 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ Message 1 │ -│ No files have been changed │ -│ │ -│ Message 2 │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > updates content when conversation changes (background update) > initial 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ Message 1 │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > updates selection and expansion on navigation > after-down 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ Line A │ -│ Line B... │ -│ No files have been changed │ -│ │ -│ Line 1 │ -│ Line 2... │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - -exports[`RewindViewer > Screen Reader Accessibility > updates selection and expansion on navigation > initial-state 1`] = ` -"╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ -│ │ -│ > Rewind │ -│ │ -│ Line A │ -│ Line B... │ -│ No files have been changed │ -│ │ -│ Line 1 │ -│ Line 2... │ -│ No files have been changed │ -│ │ -│ ● Stay at current position │ -│ Cancel rewind and stay here │ -│ │ -│ │ -│ (Use Enter to select a message, Esc to close, Right/Left to expand/collapse) │ -│ │ -╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ -" -`; - exports[`RewindViewer > updates content when conversation changes (background update) > after-update 1`] = ` "╭──────────────────────────────────────────────────────────────────────────────────────────────────╮ │ │ From 06146ba07b68aa24bd0c7424a7c756aae731c993 Mon Sep 17 00:00:00 2001 From: Famous Date: Mon, 30 Mar 2026 03:19:43 +0530 Subject: [PATCH 06/27] fix(shell): block command injection in run_shell_command #14926 --- packages/core/src/tools/shell.test.ts | 62 +++++++++++++++++++++++++- packages/core/src/tools/shell.ts | 12 +++++ packages/core/src/utils/shell-utils.ts | 47 +++++++++++++++++++ 3 files changed, 120 insertions(+), 1 deletion(-) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index d3e47de17f5..b0bc3328494 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -164,7 +164,7 @@ describe('ShellTool', () => { 'C:\\Windows\\System32\\WindowsPowerShell\\v1.0\\powershell.exe'; // Capture the output callback to simulate streaming events from the service - mockShellExecutionService.mockImplementation((_cmd, _cwd, callback) => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => { mockShellOutputCallback = callback; return { pid: 12345, @@ -838,4 +838,64 @@ describe('ShellTool', () => { expect(schema.description).toMatchSnapshot(); }); }); + + describe('command injection detection', () => { + it('should block $() command substitution', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo $(whoami)' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should block backtick command substitution', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo `whoami`' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow normal commands without substitution', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: 'hello', + rawOutput: Buffer.from('hello'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo hello' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow single quoted strings with special chars', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(not substituted)', + rawOutput: Buffer.from('$(not substituted)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: "echo '$(not substituted)'", + }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + }); }); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 4ea83b0af44..e3c9bf3e6c2 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -39,6 +39,7 @@ import { stripShellWrapper, parseCommandDetails, hasRedirection, + detectCommandSubstitution, } from '../utils/shell-utils.js'; import { SHELL_TOOL_NAME } from './tool-names.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; @@ -154,6 +155,17 @@ export class ShellToolInvocation extends BaseToolInvocation< ): Promise { const strippedCommand = stripShellWrapper(this.params.command); + if (detectCommandSubstitution(strippedCommand)) { + return { + llmContent: + 'Command injection detected: command substitution syntax ' + + '($(), backticks, <>()) found in command arguments. ' + + 'This is a security risk and the command was blocked.', + returnDisplay: + 'Blocked: command substitution detected in shell command.', + }; + } + if (signal.aborted) { return { llmContent: 'Command was cancelled by user before it could start.', diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 00b35334001..63f2793f1a8 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -726,6 +726,53 @@ export function stripShellWrapper(command: string): string { * @param command The shell command string to check * @returns true if command substitution would be executed by bash */ +export function detectCommandSubstitution(command: string): boolean { + let inSingleQuote = false; + let inDoubleQuote = false; + let i = 0; + + while (i < command.length) { + const char = command[i]; + + if (char === "'" && !inDoubleQuote) { + inSingleQuote = !inSingleQuote; + i++; + continue; + } + + if (char === '"' && !inSingleQuote) { + inDoubleQuote = !inDoubleQuote; + i++; + continue; + } + + if (inSingleQuote) { + i++; + continue; + } + + if (char === '\\' && inDoubleQuote) { + i += 2; + continue; + } + + if (char === '$' && command[i + 1] === '(') { + return true; + } + + if (char === '<' && command[i + 1] === '(') { + return true; + } + + if (char === '`') { + return true; + } + + i++; + } + + return false; +} /** * Determines whether a given shell command is allowed to execute based on * the tool's configuration including allowlists and blocklists. From 9b29ca0a37d432e9eb9553592c173805c8d4dc2b Mon Sep 17 00:00:00 2001 From: Famous Date: Mon, 30 Mar 2026 03:52:01 +0530 Subject: [PATCH 07/27] fix(shell): address bot review suggestions --- packages/core/src/tools/shell.test.ts | 71 +++++++++++++++++++++++--- packages/core/src/utils/shell-utils.ts | 4 +- 2 files changed, 66 insertions(+), 9 deletions(-) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index b0bc3328494..d5fa8bc6af8 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -165,7 +165,7 @@ describe('ShellTool', () => { // Capture the output callback to simulate streaming events from the service mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => { - mockShellOutputCallback = callback; + mockShellOutputCallback = _callback; return { pid: 12345, result: new Promise((resolve) => { @@ -760,7 +760,6 @@ describe('ShellTool', () => { const result = await promise; expect(result.llmContent).not.toContain('Process Group PGID:'); }); - it('should have minimal output for successful command', async () => { const invocation = shellTool.build({ command: 'echo hello' }); const promise = invocation.execute(mockAbortSignal); @@ -855,6 +854,50 @@ describe('ShellTool', () => { }); it('should allow normal commands without substitution', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: 'hello', + rawOutput: Buffer.from('hello'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo hello' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow single quoted strings with special chars', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(not substituted)', + rawOutput: Buffer.from('$(not substituted)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: "echo '$(not substituted)'", + }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow escaped backtick outside double quotes', async () => { mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ pid: 12345, result: Promise.resolve({ @@ -870,17 +913,31 @@ describe('ShellTool', () => { }), })); const tool = new ShellTool(mockConfig, createMockMessageBus()); - const invocation = tool.build({ command: 'echo hello' }); + const invocation = tool.build({ command: 'echo \\`hello\\`' }); const result = await invocation.execute(new AbortController().signal); expect(result.returnDisplay).not.toContain('Blocked'); }); - it('should allow single quoted strings with special chars', async () => { + it('should block $() inside double quotes', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "$(whoami)"' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should block >() process substitution', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo >(whoami)' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow $() inside single quotes', async () => { mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ pid: 12345, result: Promise.resolve({ - output: '$(not substituted)', - rawOutput: Buffer.from('$(not substituted)'), + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), exitCode: 0, signal: null, error: null, @@ -892,7 +949,7 @@ describe('ShellTool', () => { })); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ - command: "echo '$(not substituted)'", + command: "echo '$(whoami)'", }); const result = await invocation.execute(new AbortController().signal); expect(result.returnDisplay).not.toContain('Blocked'); diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 63f2793f1a8..8df057fbaec 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -751,7 +751,7 @@ export function detectCommandSubstitution(command: string): boolean { continue; } - if (char === '\\' && inDoubleQuote) { + if (char === '\\' && !inSingleQuote) { i += 2; continue; } @@ -760,7 +760,7 @@ export function detectCommandSubstitution(command: string): boolean { return true; } - if (char === '<' && command[i + 1] === '(') { + if ((char === '<' || char === '>') && command[i + 1] === '(') { return true; } From 1018980073ac0399d25c5475f29e5de43198facb Mon Sep 17 00:00:00 2001 From: Famous Date: Tue, 31 Mar 2026 02:10:15 +0530 Subject: [PATCH 08/27] fix(shell): refine escape and process substitution handling --- packages/core/src/utils/shell-utils.ts | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 8df057fbaec..ffc9f55e58d 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -751,16 +751,32 @@ export function detectCommandSubstitution(command: string): boolean { continue; } - if (char === '\\' && !inSingleQuote) { - i += 2; - continue; + if (char === '\\') { + if (inSingleQuote) { + i++; + continue; + } + if (inDoubleQuote) { + const next = command[i + 1]; + if (['$', '`', '"', '\\', '\n'].includes(next)) { + i += 2; + continue; + } + } else if (!inDoubleQuote) { + i += 2; + continue; + } } if (char === '$' && command[i + 1] === '(') { return true; } - if ((char === '<' || char === '>') && command[i + 1] === '(') { + if ( + !inDoubleQuote && + (char === '<' || char === '>') && + command[i + 1] === '(' + ) { return true; } From 907c3209193584de56fa904657f148f0b2777710 Mon Sep 17 00:00:00 2001 From: Famous Date: Wed, 1 Apr 2026 02:43:42 +0530 Subject: [PATCH 09/27] fix(shell): add PowerShell injection detection and improve error message --- packages/core/src/tools/shell.test.ts | 63 +++++-- packages/core/src/tools/shell.ts | 2 +- packages/core/src/utils/shell-utils.ts | 220 +++++++------------------ 3 files changed, 114 insertions(+), 171 deletions(-) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index d5fa8bc6af8..3e95216164a 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -899,19 +899,19 @@ describe('ShellTool', () => { it('should allow escaped backtick outside double quotes', async () => { mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: 'hello', + rawOutput: Buffer.from('hello'), + exitCode: 0, + signal: null, + error: null, + aborted: false, pid: 12345, - result: Promise.resolve({ - output: 'hello', - rawOutput: Buffer.from('hello'), - exitCode: 0, - signal: null, - error: null, - aborted: false, - pid: 12345, - executionMethod: 'child_process', - backgrounded: false, - }), - })); + executionMethod: 'child_process', + backgrounded: false, + }), + })); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo \\`hello\\`' }); const result = await invocation.execute(new AbortController().signal); @@ -933,6 +933,45 @@ describe('ShellTool', () => { }); it('should allow $() inside single quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: "echo '$(whoami)'", + }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + it('should block PowerShell @() array subexpression', async () => { + mockPlatform.mockReturnValue('win32'); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo @(whoami)' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should block PowerShell $() subexpression', async () => { + mockPlatform.mockReturnValue('win32'); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo $(whoami)' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow PowerShell single quoted strings', async () => { + mockPlatform.mockReturnValue('win32'); mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ pid: 12345, result: Promise.resolve({ diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index e3c9bf3e6c2..c2fdfb0a160 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -159,7 +159,7 @@ export class ShellToolInvocation extends BaseToolInvocation< return { llmContent: 'Command injection detected: command substitution syntax ' + - '($(), backticks, <>()) found in command arguments. ' + + '($(), backticks, <() or >()) found in command arguments. ' + 'This is a security risk and the command was blocked.', returnDisplay: 'Blocked: command substitution detected in shell command.', diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index ffc9f55e58d..8b396525bdc 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -8,12 +8,7 @@ import os from 'node:os'; import fs from 'node:fs'; import path from 'node:path'; import { quote } from 'shell-quote'; -import { - spawn, - spawnSync, - type SpawnOptionsWithoutStdio, -} from 'node:child_process'; -import * as readline from 'node:readline'; +import { spawnSync } from 'node:child_process'; import { Language, Parser, Query, type Node, type Tree } from 'web-tree-sitter'; import { loadWasmBinary } from './fileUtils.js'; import { debugLogger } from './debugLogger.js'; @@ -727,6 +722,17 @@ export function stripShellWrapper(command: string): string { * @returns true if command substitution would be executed by bash */ export function detectCommandSubstitution(command: string): boolean { + const shell = getShellConfiguration().shell; + + // PowerShell uses different syntax and escaping rules than bash + if (shell === 'powershell') { + return detectPowerShellSubstitution(command); + } + + return detectBashSubstitution(command); +} + +function detectBashSubstitution(command: string): boolean { let inSingleQuote = false; let inDoubleQuote = false; let i = 0; @@ -734,44 +740,48 @@ export function detectCommandSubstitution(command: string): boolean { while (i < command.length) { const char = command[i]; + // Toggle single quote — nothing is special inside single quotes if (char === "'" && !inDoubleQuote) { inSingleQuote = !inSingleQuote; i++; continue; } + // Toggle double quote if (char === '"' && !inSingleQuote) { inDoubleQuote = !inDoubleQuote; i++; continue; } + // Inside single quotes — everything is literal if (inSingleQuote) { i++; continue; } + // Handle backslash escaping if (char === '\\') { - if (inSingleQuote) { - i++; - continue; - } if (inDoubleQuote) { + // In double quotes, only these chars can be escaped const next = command[i + 1]; if (['$', '`', '"', '\\', '\n'].includes(next)) { i += 2; continue; } - } else if (!inDoubleQuote) { + } else { + // Outside quotes, backslash escapes the next character i += 2; continue; } } + // Detect $() command substitution if (char === '$' && command[i + 1] === '(') { return true; } + // Detect <() and >() process substitution (not valid inside double quotes) if ( !inDoubleQuote && (char === '<' || char === '>') && @@ -780,6 +790,7 @@ export function detectCommandSubstitution(command: string): boolean { return true; } + // Detect backtick substitution if (char === '`') { return true; } @@ -789,164 +800,57 @@ export function detectCommandSubstitution(command: string): boolean { return false; } -/** - * Determines whether a given shell command is allowed to execute based on - * the tool's configuration including allowlists and blocklists. - * - * This function operates in "default allow" mode. It is a wrapper around - * `checkCommandPermissions`. - * - * @param command The shell command string to validate. - * @param config The application configuration. - * @returns An object with 'allowed' boolean and optional 'reason' string if not allowed. - */ -export const spawnAsync = ( - command: string, - args: string[], - options?: SpawnOptionsWithoutStdio, -): Promise<{ stdout: string; stderr: string }> => - new Promise((resolve, reject) => { - const child = spawn(command, args, options); - let stdout = ''; - let stderr = ''; - - child.stdout.on('data', (data) => { - stdout += data.toString(); - }); - child.stderr.on('data', (data) => { - stderr += data.toString(); - }); - - child.on('close', (code) => { - if (code === 0) { - resolve({ stdout, stderr }); - } else { - reject(new Error(`Command failed with exit code ${code}:\n${stderr}`)); - } - }); - - child.on('error', (err) => { - reject(err); - }); - }); - -/** - * Executes a command and yields lines of output as they appear. - * Use for large outputs where buffering is not feasible. - * - * @param command The executable to run - * @param args Arguments for the executable - * @param options Spawn options (cwd, env, etc.) - */ -export async function* execStreaming( - command: string, - args: string[], - options?: SpawnOptionsWithoutStdio & { - signal?: AbortSignal; - allowedExitCodes?: number[]; - }, -): AsyncGenerator { - const child = spawn(command, args, { - ...options, - // ensure we don't open a window on windows if possible/relevant - windowsHide: true, - }); - - const rl = readline.createInterface({ - input: child.stdout, - terminal: false, - }); +function detectPowerShellSubstitution(command: string): boolean { + let inSingleQuote = false; + let inDoubleQuote = false; + let i = 0; - const errorChunks: Buffer[] = []; - let stderrTotalBytes = 0; - const MAX_STDERR_BYTES = 20 * 1024; // 20KB limit + while (i < command.length) { + const char = command[i]; - child.stderr.on('data', (chunk) => { - if (stderrTotalBytes < MAX_STDERR_BYTES) { - errorChunks.push(chunk); - stderrTotalBytes += chunk.length; + // In PowerShell, single quotes are literal — no substitution possible + if (char === "'" && !inDoubleQuote) { + inSingleQuote = !inSingleQuote; + i++; + continue; } - }); - - let error: Error | null = null; - child.on('error', (err) => { - error = err; - }); - const onAbort = () => { - // If manually aborted by signal, we kill immediately. - if (!child.killed) child.kill(); - }; + if (char === '"' && !inSingleQuote) { + inDoubleQuote = !inDoubleQuote; + i++; + continue; + } - if (options?.signal?.aborted) { - onAbort(); - } else { - options?.signal?.addEventListener('abort', onAbort); - } + if (inSingleQuote) { + i++; + continue; + } - let finished = false; - try { - for await (const line of rl) { - if (options?.signal?.aborted) break; - yield line; - } - finished = true; - } finally { - rl.close(); - options?.signal?.removeEventListener('abort', onAbort); - - // Ensure process is killed when the generator is closed (consumer breaks loop) - let killedByGenerator = false; - if (!finished && child.exitCode === null && !child.killed) { - try { - child.kill(); - } catch (_e) { - // ignore error if process is already dead - } - killedByGenerator = true; + // In PowerShell, backtick ` is the escape character (not backslash) + if (char === '`' && !inSingleQuote) { + // Skip next character — it's escaped + i += 2; + continue; } - // Ensure we wait for the process to exit to check codes - await new Promise((resolve, reject) => { - // If an error occurred before we got here (e.g. spawn failure), reject immediately. - if (error) { - reject(error); - return; - } + // Detect $() subexpression + if (char === '$' && command[i + 1] === '(') { + return true; + } - function checkExit(code: number | null) { - // If we aborted or killed it manually, we treat it as success (stop waiting) - if (options?.signal?.aborted || killedByGenerator) { - resolve(); - return; - } + // Detect @() array subexpression — can execute commands in PowerShell + if (char === '@' && command[i + 1] === '(') { + return true; + } - const allowed = options?.allowedExitCodes ?? [0]; - if (code !== null && allowed.includes(code)) { - resolve(); - } else { - // If we have an accumulated error or explicit error event - if (error) reject(error); - else { - const stderr = Buffer.concat(errorChunks).toString('utf8'); - const truncatedMsg = - stderrTotalBytes >= MAX_STDERR_BYTES ? '...[truncated]' : ''; - reject( - new Error( - `Process exited with code ${code}: ${stderr}${truncatedMsg}`, - ), - ); - } - } - } + // Detect bare (...) subexpression in PowerShell + if (char === '(' && !inDoubleQuote) { + return true; + } - if (child.exitCode !== null) { - checkExit(child.exitCode); - } else { - child.on('close', (code) => checkExit(code)); - child.on('error', (err) => reject(err)); - } - }); + i++; } + + return false; } From 0a415e588e1d8ea4730dc7f022b540bf44b122d5 Mon Sep 17 00:00:00 2001 From: Famous Date: Wed, 1 Apr 2026 03:38:59 +0530 Subject: [PATCH 10/27] fix(shell): add PowerShell injection detection and improve error message --- packages/core/src/utils/shell-utils.ts | 207 ++++++++++++++++++++----- 1 file changed, 168 insertions(+), 39 deletions(-) diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 8b396525bdc..3222ffe474f 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -8,7 +8,12 @@ import os from 'node:os'; import fs from 'node:fs'; import path from 'node:path'; import { quote } from 'shell-quote'; -import { spawnSync } from 'node:child_process'; +import { + spawn, + spawnSync, + type SpawnOptionsWithoutStdio, +} from 'node:child_process'; +import * as readline from 'node:readline'; import { Language, Parser, Query, type Node, type Tree } from 'web-tree-sitter'; import { loadWasmBinary } from './fileUtils.js'; import { debugLogger } from './debugLogger.js'; @@ -721,14 +726,173 @@ export function stripShellWrapper(command: string): string { * @param command The shell command string to check * @returns true if command substitution would be executed by bash */ +/** + * Determines whether a given shell command is allowed to execute based on + * the tool's configuration including allowlists and blocklists. + * + * This function operates in "default allow" mode. It is a wrapper around + * `checkCommandPermissions`. + * + * @param command The shell command string to validate. + * @param config The application configuration. + * @returns An object with 'allowed' boolean and optional 'reason' string if not allowed. + */ +export const spawnAsync = ( + command: string, + args: string[], + options?: SpawnOptionsWithoutStdio, +): Promise<{ stdout: string; stderr: string }> => + new Promise((resolve, reject) => { + const child = spawn(command, args, options); + let stdout = ''; + let stderr = ''; + + child.stdout.on('data', (data) => { + stdout += data.toString(); + }); + + child.stderr.on('data', (data) => { + stderr += data.toString(); + }); + + child.on('close', (code) => { + if (code === 0) { + resolve({ stdout, stderr }); + } else { + reject(new Error(`Command failed with exit code ${code}:\n${stderr}`)); + } + }); + + child.on('error', (err) => { + reject(err); + }); + }); + +/** + * Executes a command and yields lines of output as they appear. + * Use for large outputs where buffering is not feasible. + * + * @param command The executable to run + * @param args Arguments for the executable + * @param options Spawn options (cwd, env, etc.) + */ +export async function* execStreaming( + command: string, + args: string[], + options?: SpawnOptionsWithoutStdio & { + signal?: AbortSignal; + allowedExitCodes?: number[]; + }, +): AsyncGenerator { + const child = spawn(command, args, { + ...options, + // ensure we don't open a window on windows if possible/relevant + windowsHide: true, + }); + + const rl = readline.createInterface({ + input: child.stdout, + terminal: false, + }); + + const errorChunks: Buffer[] = []; + let stderrTotalBytes = 0; + const MAX_STDERR_BYTES = 20 * 1024; // 20KB limit + + child.stderr.on('data', (chunk) => { + if (stderrTotalBytes < MAX_STDERR_BYTES) { + errorChunks.push(chunk); + stderrTotalBytes += chunk.length; + } + }); + + let error: Error | null = null; + child.on('error', (err) => { + error = err; + }); + + const onAbort = () => { + // If manually aborted by signal, we kill immediately. + if (!child.killed) child.kill(); + }; + + if (options?.signal?.aborted) { + onAbort(); + } else { + options?.signal?.addEventListener('abort', onAbort); + } + + let finished = false; + try { + for await (const line of rl) { + if (options?.signal?.aborted) break; + yield line; + } + finished = true; + } finally { + rl.close(); + options?.signal?.removeEventListener('abort', onAbort); + + // Ensure process is killed when the generator is closed (consumer breaks loop) + let killedByGenerator = false; + if (!finished && child.exitCode === null && !child.killed) { + try { + child.kill(); + } catch (_e) { + // ignore error if process is already dead + } + killedByGenerator = true; + } + + // Ensure we wait for the process to exit to check codes + await new Promise((resolve, reject) => { + // If an error occurred before we got here (e.g. spawn failure), reject immediately. + if (error) { + reject(error); + return; + } + + function checkExit(code: number | null) { + // If we aborted or killed it manually, we treat it as success (stop waiting) + if (options?.signal?.aborted || killedByGenerator) { + resolve(); + return; + } + + const allowed = options?.allowedExitCodes ?? [0]; + if (code !== null && allowed.includes(code)) { + resolve(); + } else { + // If we have an accumulated error or explicit error event + if (error) reject(error); + else { + const stderr = Buffer.concat(errorChunks).toString('utf8'); + const truncatedMsg = + stderrTotalBytes >= MAX_STDERR_BYTES ? '...[truncated]' : ''; + reject( + new Error( + `Process exited with code ${code}: ${stderr}${truncatedMsg}`, + ), + ); + } + } + } + + if (child.exitCode !== null) { + checkExit(child.exitCode); + } else { + child.on('close', (code) => checkExit(code)); + child.on('error', (err) => reject(err)); + } + }); + } +} + export function detectCommandSubstitution(command: string): boolean { const shell = getShellConfiguration().shell; - - // PowerShell uses different syntax and escaping rules than bash if (shell === 'powershell') { return detectPowerShellSubstitution(command); } - return detectBashSubstitution(command); } @@ -736,52 +900,37 @@ function detectBashSubstitution(command: string): boolean { let inSingleQuote = false; let inDoubleQuote = false; let i = 0; - while (i < command.length) { const char = command[i]; - - // Toggle single quote — nothing is special inside single quotes if (char === "'" && !inDoubleQuote) { inSingleQuote = !inSingleQuote; i++; continue; } - - // Toggle double quote if (char === '"' && !inSingleQuote) { inDoubleQuote = !inDoubleQuote; i++; continue; } - - // Inside single quotes — everything is literal if (inSingleQuote) { i++; continue; } - - // Handle backslash escaping if (char === '\\') { if (inDoubleQuote) { - // In double quotes, only these chars can be escaped const next = command[i + 1]; if (['$', '`', '"', '\\', '\n'].includes(next)) { i += 2; continue; } } else { - // Outside quotes, backslash escapes the next character i += 2; continue; } } - - // Detect $() command substitution if (char === '$' && command[i + 1] === '(') { return true; } - - // Detect <() and >() process substitution (not valid inside double quotes) if ( !inDoubleQuote && (char === '<' || char === '>') && @@ -789,15 +938,11 @@ function detectBashSubstitution(command: string): boolean { ) { return true; } - - // Detect backtick substitution if (char === '`') { return true; } - i++; } - return false; } @@ -805,52 +950,36 @@ function detectPowerShellSubstitution(command: string): boolean { let inSingleQuote = false; let inDoubleQuote = false; let i = 0; - while (i < command.length) { const char = command[i]; - - // In PowerShell, single quotes are literal — no substitution possible if (char === "'" && !inDoubleQuote) { inSingleQuote = !inSingleQuote; i++; continue; } - if (char === '"' && !inSingleQuote) { inDoubleQuote = !inDoubleQuote; i++; continue; } - if (inSingleQuote) { i++; continue; } - - // In PowerShell, backtick ` is the escape character (not backslash) if (char === '`' && !inSingleQuote) { - // Skip next character — it's escaped i += 2; continue; } - - // Detect $() subexpression if (char === '$' && command[i + 1] === '(') { return true; } - - // Detect @() array subexpression — can execute commands in PowerShell if (char === '@' && command[i + 1] === '(') { return true; } - - // Detect bare (...) subexpression in PowerShell if (char === '(' && !inDoubleQuote) { return true; } - i++; } - return false; } From 0ba66ddfaaa8102ba17fce7f443d6c35d376eadf Mon Sep 17 00:00:00 2001 From: Famous Date: Wed, 1 Apr 2026 12:18:54 +0530 Subject: [PATCH 11/27] fix(shell): refine PowerShell detection to avoid false positives --- packages/core/src/utils/shell-utils.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 3222ffe474f..b1fe813bdc8 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -976,9 +976,6 @@ function detectPowerShellSubstitution(command: string): boolean { if (char === '@' && command[i + 1] === '(') { return true; } - if (char === '(' && !inDoubleQuote) { - return true; - } i++; } return false; From a8f7efc0dd7db0552008f4a6537e714dc7e9419a Mon Sep 17 00:00:00 2001 From: Famous Date: Wed, 1 Apr 2026 18:55:18 +0530 Subject: [PATCH 12/27] fix(shell): implemented the suggestions --- docs/CONTRIBUTING.md | 2 +- packages/core/src/utils/shell-utils.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 44fcc634393..5c10c35f62e 120000 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -1 +1 @@ -../CONTRIBUTING.md \ No newline at end of file +../CONTRIBUTING.md diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index b1fe813bdc8..88d52749d82 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -916,7 +916,7 @@ function detectBashSubstitution(command: string): boolean { i++; continue; } - if (char === '\\') { + if (char === '\\' && i + 1 < command.length) { if (inDoubleQuote) { const next = command[i + 1]; if (['$', '`', '"', '\\', '\n'].includes(next)) { @@ -966,7 +966,7 @@ function detectPowerShellSubstitution(command: string): boolean { i++; continue; } - if (char === '`' && !inSingleQuote) { + if (char === '`' && !inSingleQuote && i + 1 < command.length) { i += 2; continue; } From e21fab127f1d9448b4b2cabc13bacccab862d73c Mon Sep 17 00:00:00 2001 From: Famous Date: Sun, 5 Apr 2026 16:37:28 +0530 Subject: [PATCH 13/27] test: add missing customMatchers import and command injection detection tests --- packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts b/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts index 9cbfd9457b5..c1a4b55d4b0 100644 --- a/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts +++ b/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts @@ -5,6 +5,7 @@ */ import { describe, it, expect } from 'vitest'; +import '../../../test-utils/customMatchers.js'; import { handleVimAction } from './vim-buffer-actions.js'; import type { TextBufferState, VisualLayout } from './text-buffer.js'; From df1b4688c4bd38a2c8562069977c3a4cc19e28fd Mon Sep 17 00:00:00 2001 From: Famous Date: Sun, 5 Apr 2026 16:44:45 +0530 Subject: [PATCH 14/27] merge: resolve conflict keeping detectCommandSubstitution import --- packages/core/src/tools/shell.test.ts | 93 +++++++++++++++++++++++---- 1 file changed, 81 insertions(+), 12 deletions(-) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 3e95216164a..7719b8fe584 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -973,19 +973,19 @@ describe('ShellTool', () => { it('should allow PowerShell single quoted strings', async () => { mockPlatform.mockReturnValue('win32'); mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, pid: 12345, - result: Promise.resolve({ - output: '$(whoami)', - rawOutput: Buffer.from('$(whoami)'), - exitCode: 0, - signal: null, - error: null, - aborted: false, - pid: 12345, - executionMethod: 'child_process', - backgrounded: false, - }), - })); + executionMethod: 'child_process', + backgrounded: false, + }), + })); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: "echo '$(whoami)'", @@ -993,5 +993,74 @@ describe('ShellTool', () => { const result = await invocation.execute(new AbortController().signal); expect(result.returnDisplay).not.toContain('Blocked'); }); + it('should allow escaped substitution outside quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo \\$(whoami)' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow process substitution inside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '<(whoami)', + rawOutput: Buffer.from('<(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "<(whoami)"' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should block process substitution without quotes', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo <(whoami)' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow escaped substitution inside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "\\$(whoami)"' }); + const result = await invocation.execute(new AbortController().signal); + expect(result.returnDisplay).not.toContain('Blocked'); + }); }); }); From d486fc1b6957bcd9abe93f7ce99d1677a0aa47ad Mon Sep 17 00:00:00 2001 From: Famous Date: Sun, 5 Apr 2026 17:25:19 +0530 Subject: [PATCH 15/27] fix: resolve eslint unsafe-return and unsafe-assignment errors in TableRenderer and integrity --- packages/cli/src/ui/utils/TableRenderer.tsx | 8 ++++++++ packages/core/src/config/extensions/integrity.ts | 9 ++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/ui/utils/TableRenderer.tsx b/packages/cli/src/ui/utils/TableRenderer.tsx index b6a30792ca2..c06ddd26d8b 100644 --- a/packages/cli/src/ui/utils/TableRenderer.tsx +++ b/packages/cli/src/ui/utils/TableRenderer.tsx @@ -69,7 +69,9 @@ export const TableRenderer: React.FC = ({ }) => { const styledHeaders = useMemo( () => + // eslint-disable-next-line @typescript-eslint/no-unsafe-return headers.map((header) => + // eslint-disable-next-line @typescript-eslint/no-unsafe-return parseMarkdownToStyledLine( stripUnsafeCharacters(header), theme.text.link, @@ -81,7 +83,9 @@ export const TableRenderer: React.FC = ({ const styledRows = useMemo( () => rows.map((row) => + // eslint-disable-next-line @typescript-eslint/no-unsafe-return row.map((cell) => + // eslint-disable-next-line @typescript-eslint/no-unsafe-return parseMarkdownToStyledLine( stripUnsafeCharacters(cell), theme.text.primary, @@ -100,11 +104,13 @@ export const TableRenderer: React.FC = ({ // --- Define Constraints per Column --- const constraints = Array.from({ length: numColumns }).map( (_, colIndex) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const headerStyledLine = styledHeaders[colIndex] || StyledLine.empty(0); let { contentWidth: maxContentWidth, maxWordWidth } = calculateWidths(headerStyledLine); styledRows.forEach((row) => { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const cellStyledLine = row[colIndex] || StyledLine.empty(0); const { contentWidth: cellWidth, maxWordWidth: cellWordWidth } = calculateWidths(cellStyledLine); @@ -180,6 +186,7 @@ export const TableRenderer: React.FC = ({ const rowResult: ProcessedLine[][] = []; // Ensure we iterate up to numColumns, filling with empty cells if needed for (let colIndex = 0; colIndex < numColumns; colIndex++) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const cellStyledLine = row[colIndex] || StyledLine.empty(0); const allocatedWidth = finalContentWidths[colIndex]; const contentWidth = Math.max(1, allocatedWidth); @@ -196,6 +203,7 @@ export const TableRenderer: React.FC = ({ ); const lines = wrappedStyledLines.map((line) => ({ + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment text: styledLineToString(line), width: styledCharsWidth(line), })); diff --git a/packages/core/src/config/extensions/integrity.ts b/packages/core/src/config/extensions/integrity.ts index 95418df4771..cd272174b17 100644 --- a/packages/core/src/config/extensions/integrity.ts +++ b/packages/core/src/config/extensions/integrity.ts @@ -147,7 +147,8 @@ class ExtensionIntegrityStore { const { store, signature: actualSignature } = rawStore; // Re-generate the expected signature for the store content. - const storeContent = stableStringify(store) ?? ''; + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const storeContent: string = stableStringify(store) ?? ''; const expectedSignature = await this.generateSignature(storeContent); // Verify the store hasn't been tampered with. @@ -165,7 +166,8 @@ class ExtensionIntegrityStore { */ async save(store: ExtensionIntegrityMap): Promise { // Generate a signature for the entire map to prevent manual tampering. - const storeContent = stableStringify(store) ?? ''; + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const storeContent: string = stableStringify(store) ?? ''; const storeSignature = await this.generateSignature(storeContent); const finalData: IntegrityStore = { @@ -190,7 +192,8 @@ class ExtensionIntegrityStore { * Generates a deterministic SHA-256 hash of the metadata. */ generateHash(metadata: ExtensionInstallMetadata): string { - const content = stableStringify(metadata) ?? ''; + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const content: string = stableStringify(metadata) ?? ''; return createHash('sha256').update(content).digest('hex'); } From bfc3b3d8222139854494d1718e428296774b1ac6 Mon Sep 17 00:00:00 2001 From: Famous Date: Sun, 5 Apr 2026 17:34:10 +0530 Subject: [PATCH 16/27] fix: resolve snapshot conflicts after upstream merge --- .../__snapshots__/AskUserDialog.test.tsx.snap | 75 ++++++++++++ .../ExitPlanModeDialog.test.tsx.snap | 108 ++++++++++++++++++ 2 files changed, 183 insertions(+) diff --git a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap index cdc060d9d72..9ea304ce1ab 100644 --- a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap @@ -11,6 +11,17 @@ Enter to submit · Esc to cancel " `; +exports[`AskUserDialog > Choice question placeholder > uses default placeholder when not provided 2`] = ` +"Select your preferred language: + + 1. TypeScript + 2. JavaScript +● 3. Enter a custom value + +Enter to submit · Esc to cancel +" +`; + exports[`AskUserDialog > Choice question placeholder > uses placeholder for "Other" option when provided 1`] = ` "Select your preferred language: @@ -22,6 +33,17 @@ Enter to submit · Esc to cancel " `; +exports[`AskUserDialog > Choice question placeholder > uses placeholder for "Other" option when provided 2`] = ` +"Select your preferred language: + + 1. TypeScript + 2. JavaScript +● 3. Type another language... + +Enter to submit · Esc to cancel +" +`; + exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: false) > shows scroll arrows correctly when useAlternateBuffer is false 1`] = ` "Choose an option @@ -38,6 +60,20 @@ Enter to select · ↑/↓ to navigate · Esc to cancel " `; +exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: false) > shows scroll arrows correctly when useAlternateBuffer is false 2`] = ` +"Choose an option + +▲ +● 1. Option 1 + Description 1 + 2. Option 2 + Description 2 +▼ + +Enter to select · ↑/↓ to navigate · Esc to cancel +" +`; + exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: true) > shows scroll arrows correctly when useAlternateBuffer is true 1`] = ` "Choose an option @@ -54,6 +90,45 @@ Enter to select · ↑/↓ to navigate · Esc to cancel " `; +exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: true) > shows scroll arrows correctly when useAlternateBuffer is true 2`] = ` +"Choose an option + +● 1. Option 1 + Description 1 + 2. Option 2 + Description 2 + 3. Option 3 + Description 3 + 4. Option 4 + Description 4 + 5. Option 5 + Description 5 + 6. Option 6 + Description 6 + 7. Option 7 + Description 7 + 8. Option 8 + Description 8 + 9. Option 9 + Description 9 + 10. Option 10 + Description 10 + 11. Option 11 + Description 11 + 12. Option 12 + Description 12 + 13. Option 13 + Description 13 + 14. Option 14 + Description 14 + 15. Option 15 + Description 15 + 16. Enter a custom value + +Enter to select · ↑/↓ to navigate · Esc to cancel +" +`; + exports[`AskUserDialog > Text type questions > renders text input for type: "text" 1`] = ` "What should we name this component? diff --git a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap index 073c106ceb3..9e210e34381 100644 --- a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap @@ -27,6 +27,33 @@ Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; +exports[`ExitPlanModeDialog > useAlternateBuffer: false > bubbles up Ctrl+C when feedback is empty while editing 2`] = ` +"Overview + +Add user authentication to the CLI application. + +Implementation Steps + + 1. Create src/auth/AuthService.ts with login/logout methods + 2. Add session storage in src/storage/SessionStore.ts + 3. Update src/commands/index.ts to check auth status + 4. Add tests in src/auth/__tests__/ + +Files to Modify + + - src/index.ts - Add auth middleware + - src/config.ts - Add auth configuration options + + 1. Yes, automatically accept edits + Approves plan and allows tools to run automatically + 2. Yes, manually accept edits + Approves plan but requires confirmation for each tool +● 3. Type your feedback... + +Enter to submit · Ctrl+X to edit plan · Esc to cancel +" +`; + exports[`ExitPlanModeDialog > useAlternateBuffer: false > calls onFeedback when feedback is typed and submitted 1`] = ` "Overview @@ -54,6 +81,33 @@ Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; +exports[`ExitPlanModeDialog > useAlternateBuffer: false > calls onFeedback when feedback is typed and submitted 2`] = ` +"Overview + +Add user authentication to the CLI application. + +Implementation Steps + + 1. Create src/auth/AuthService.ts with login/logout methods + 2. Add session storage in src/storage/SessionStore.ts + 3. Update src/commands/index.ts to check auth status + 4. Add tests in src/auth/__tests__/ + +Files to Modify + + - src/index.ts - Add auth middleware + - src/config.ts - Add auth configuration options + + 1. Yes, automatically accept edits + Approves plan and allows tools to run automatically + 2. Yes, manually accept edits + Approves plan but requires confirmation for each tool +● 3. Add tests + +Enter to submit · Ctrl+X to edit plan · Esc to cancel +" +`; + exports[`ExitPlanModeDialog > useAlternateBuffer: false > displays error state when file read fails 1`] = ` " Error reading plan: File not found " @@ -140,6 +194,33 @@ Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; +exports[`ExitPlanModeDialog > useAlternateBuffer: true > bubbles up Ctrl+C when feedback is empty while editing 2`] = ` +"Overview + +Add user authentication to the CLI application. + +Implementation Steps + + 1. Create src/auth/AuthService.ts with login/logout methods + 2. Add session storage in src/storage/SessionStore.ts + 3. Update src/commands/index.ts to check auth status + 4. Add tests in src/auth/__tests__/ + +Files to Modify + + - src/index.ts - Add auth middleware + - src/config.ts - Add auth configuration options + + 1. Yes, automatically accept edits + Approves plan and allows tools to run automatically + 2. Yes, manually accept edits + Approves plan but requires confirmation for each tool +● 3. Type your feedback... + +Enter to submit · Ctrl+X to edit plan · Esc to cancel +" +`; + exports[`ExitPlanModeDialog > useAlternateBuffer: true > calls onFeedback when feedback is typed and submitted 1`] = ` "Overview @@ -167,6 +248,33 @@ Enter to select · ↑/↓ to navigate · Ctrl+X to edit plan · Esc to cancel " `; +exports[`ExitPlanModeDialog > useAlternateBuffer: true > calls onFeedback when feedback is typed and submitted 2`] = ` +"Overview + +Add user authentication to the CLI application. + +Implementation Steps + + 1. Create src/auth/AuthService.ts with login/logout methods + 2. Add session storage in src/storage/SessionStore.ts + 3. Update src/commands/index.ts to check auth status + 4. Add tests in src/auth/__tests__/ + +Files to Modify + + - src/index.ts - Add auth middleware + - src/config.ts - Add auth configuration options + + 1. Yes, automatically accept edits + Approves plan and allows tools to run automatically + 2. Yes, manually accept edits + Approves plan but requires confirmation for each tool +● 3. Add tests + +Enter to submit · Ctrl+X to edit plan · Esc to cancel +" +`; + exports[`ExitPlanModeDialog > useAlternateBuffer: true > displays error state when file read fails 1`] = ` " Error reading plan: File not found " From d14576e36c5b60c25966c6e68b2776a97886bc4d Mon Sep 17 00:00:00 2001 From: Famous Date: Tue, 14 Apr 2026 11:06:48 +0530 Subject: [PATCH 17/27] revert: remove unintended files from PR, keep only shell injection fix --- docs/CONTRIBUTING.md | 2 +- .../__snapshots__/AskUserDialog.test.tsx.snap | 75 ------------ .../ExitPlanModeDialog.test.tsx.snap | 108 ------------------ .../shared/vim-buffer-actions.test.ts | 1 - packages/cli/src/ui/utils/TableRenderer.tsx | 8 -- .../core/src/config/extensions/integrity.ts | 9 +- 6 files changed, 4 insertions(+), 199 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 5c10c35f62e..44fcc634393 120000 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -1 +1 @@ -../CONTRIBUTING.md +../CONTRIBUTING.md \ No newline at end of file diff --git a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap index 9ea304ce1ab..cdc060d9d72 100644 --- a/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/AskUserDialog.test.tsx.snap @@ -11,17 +11,6 @@ Enter to submit · Esc to cancel " `; -exports[`AskUserDialog > Choice question placeholder > uses default placeholder when not provided 2`] = ` -"Select your preferred language: - - 1. TypeScript - 2. JavaScript -● 3. Enter a custom value - -Enter to submit · Esc to cancel -" -`; - exports[`AskUserDialog > Choice question placeholder > uses placeholder for "Other" option when provided 1`] = ` "Select your preferred language: @@ -33,17 +22,6 @@ Enter to submit · Esc to cancel " `; -exports[`AskUserDialog > Choice question placeholder > uses placeholder for "Other" option when provided 2`] = ` -"Select your preferred language: - - 1. TypeScript - 2. JavaScript -● 3. Type another language... - -Enter to submit · Esc to cancel -" -`; - exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: false) > shows scroll arrows correctly when useAlternateBuffer is false 1`] = ` "Choose an option @@ -60,20 +38,6 @@ Enter to select · ↑/↓ to navigate · Esc to cancel " `; -exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: false) > shows scroll arrows correctly when useAlternateBuffer is false 2`] = ` -"Choose an option - -▲ -● 1. Option 1 - Description 1 - 2. Option 2 - Description 2 -▼ - -Enter to select · ↑/↓ to navigate · Esc to cancel -" -`; - exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: true) > shows scroll arrows correctly when useAlternateBuffer is true 1`] = ` "Choose an option @@ -90,45 +54,6 @@ Enter to select · ↑/↓ to navigate · Esc to cancel " `; -exports[`AskUserDialog > Scroll Arrows (useAlternateBuffer: true) > shows scroll arrows correctly when useAlternateBuffer is true 2`] = ` -"Choose an option - -● 1. Option 1 - Description 1 - 2. Option 2 - Description 2 - 3. Option 3 - Description 3 - 4. Option 4 - Description 4 - 5. Option 5 - Description 5 - 6. Option 6 - Description 6 - 7. Option 7 - Description 7 - 8. Option 8 - Description 8 - 9. Option 9 - Description 9 - 10. Option 10 - Description 10 - 11. Option 11 - Description 11 - 12. Option 12 - Description 12 - 13. Option 13 - Description 13 - 14. Option 14 - Description 14 - 15. Option 15 - Description 15 - 16. Enter a custom value - -Enter to select · ↑/↓ to navigate · Esc to cancel -" -`; - exports[`AskUserDialog > Text type questions > renders text input for type: "text" 1`] = ` "What should we name this component? diff --git a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap index 5b0bfd1d6fc..71acb9388cf 100644 --- a/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap +++ b/packages/cli/src/ui/components/__snapshots__/ExitPlanModeDialog.test.tsx.snap @@ -27,33 +27,6 @@ Enter to select · ↑/↓ to navigate · Ctrl+G to edit plan · Esc to cancel " `; -exports[`ExitPlanModeDialog > useAlternateBuffer: false > bubbles up Ctrl+C when feedback is empty while editing 2`] = ` -"Overview - -Add user authentication to the CLI application. - -Implementation Steps - - 1. Create src/auth/AuthService.ts with login/logout methods - 2. Add session storage in src/storage/SessionStore.ts - 3. Update src/commands/index.ts to check auth status - 4. Add tests in src/auth/__tests__/ - -Files to Modify - - - src/index.ts - Add auth middleware - - src/config.ts - Add auth configuration options - - 1. Yes, automatically accept edits - Approves plan and allows tools to run automatically - 2. Yes, manually accept edits - Approves plan but requires confirmation for each tool -● 3. Type your feedback... - -Enter to submit · Ctrl+X to edit plan · Esc to cancel -" -`; - exports[`ExitPlanModeDialog > useAlternateBuffer: false > calls onFeedback when feedback is typed and submitted 1`] = ` "Overview @@ -81,33 +54,6 @@ Enter to select · ↑/↓ to navigate · Ctrl+G to edit plan · Esc to cancel " `; -exports[`ExitPlanModeDialog > useAlternateBuffer: false > calls onFeedback when feedback is typed and submitted 2`] = ` -"Overview - -Add user authentication to the CLI application. - -Implementation Steps - - 1. Create src/auth/AuthService.ts with login/logout methods - 2. Add session storage in src/storage/SessionStore.ts - 3. Update src/commands/index.ts to check auth status - 4. Add tests in src/auth/__tests__/ - -Files to Modify - - - src/index.ts - Add auth middleware - - src/config.ts - Add auth configuration options - - 1. Yes, automatically accept edits - Approves plan and allows tools to run automatically - 2. Yes, manually accept edits - Approves plan but requires confirmation for each tool -● 3. Add tests - -Enter to submit · Ctrl+X to edit plan · Esc to cancel -" -`; - exports[`ExitPlanModeDialog > useAlternateBuffer: false > displays error state when file read fails 1`] = ` " Error reading plan: File not found " @@ -194,33 +140,6 @@ Enter to select · ↑/↓ to navigate · Ctrl+G to edit plan · Esc to cancel " `; -exports[`ExitPlanModeDialog > useAlternateBuffer: true > bubbles up Ctrl+C when feedback is empty while editing 2`] = ` -"Overview - -Add user authentication to the CLI application. - -Implementation Steps - - 1. Create src/auth/AuthService.ts with login/logout methods - 2. Add session storage in src/storage/SessionStore.ts - 3. Update src/commands/index.ts to check auth status - 4. Add tests in src/auth/__tests__/ - -Files to Modify - - - src/index.ts - Add auth middleware - - src/config.ts - Add auth configuration options - - 1. Yes, automatically accept edits - Approves plan and allows tools to run automatically - 2. Yes, manually accept edits - Approves plan but requires confirmation for each tool -● 3. Type your feedback... - -Enter to submit · Ctrl+X to edit plan · Esc to cancel -" -`; - exports[`ExitPlanModeDialog > useAlternateBuffer: true > calls onFeedback when feedback is typed and submitted 1`] = ` "Overview @@ -248,33 +167,6 @@ Enter to select · ↑/↓ to navigate · Ctrl+G to edit plan · Esc to cancel " `; -exports[`ExitPlanModeDialog > useAlternateBuffer: true > calls onFeedback when feedback is typed and submitted 2`] = ` -"Overview - -Add user authentication to the CLI application. - -Implementation Steps - - 1. Create src/auth/AuthService.ts with login/logout methods - 2. Add session storage in src/storage/SessionStore.ts - 3. Update src/commands/index.ts to check auth status - 4. Add tests in src/auth/__tests__/ - -Files to Modify - - - src/index.ts - Add auth middleware - - src/config.ts - Add auth configuration options - - 1. Yes, automatically accept edits - Approves plan and allows tools to run automatically - 2. Yes, manually accept edits - Approves plan but requires confirmation for each tool -● 3. Add tests - -Enter to submit · Ctrl+X to edit plan · Esc to cancel -" -`; - exports[`ExitPlanModeDialog > useAlternateBuffer: true > displays error state when file read fails 1`] = ` " Error reading plan: File not found " diff --git a/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts b/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts index 07327f44948..9f163f0c540 100644 --- a/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts +++ b/packages/cli/src/ui/components/shared/vim-buffer-actions.test.ts @@ -5,7 +5,6 @@ */ import { describe, it, expect } from 'vitest'; -import '../../../test-utils/customMatchers.js'; import { handleVimAction } from './vim-buffer-actions.js'; import type { TextBufferState, VisualLayout } from './text-buffer.js'; diff --git a/packages/cli/src/ui/utils/TableRenderer.tsx b/packages/cli/src/ui/utils/TableRenderer.tsx index c06ddd26d8b..b6a30792ca2 100644 --- a/packages/cli/src/ui/utils/TableRenderer.tsx +++ b/packages/cli/src/ui/utils/TableRenderer.tsx @@ -69,9 +69,7 @@ export const TableRenderer: React.FC = ({ }) => { const styledHeaders = useMemo( () => - // eslint-disable-next-line @typescript-eslint/no-unsafe-return headers.map((header) => - // eslint-disable-next-line @typescript-eslint/no-unsafe-return parseMarkdownToStyledLine( stripUnsafeCharacters(header), theme.text.link, @@ -83,9 +81,7 @@ export const TableRenderer: React.FC = ({ const styledRows = useMemo( () => rows.map((row) => - // eslint-disable-next-line @typescript-eslint/no-unsafe-return row.map((cell) => - // eslint-disable-next-line @typescript-eslint/no-unsafe-return parseMarkdownToStyledLine( stripUnsafeCharacters(cell), theme.text.primary, @@ -104,13 +100,11 @@ export const TableRenderer: React.FC = ({ // --- Define Constraints per Column --- const constraints = Array.from({ length: numColumns }).map( (_, colIndex) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const headerStyledLine = styledHeaders[colIndex] || StyledLine.empty(0); let { contentWidth: maxContentWidth, maxWordWidth } = calculateWidths(headerStyledLine); styledRows.forEach((row) => { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const cellStyledLine = row[colIndex] || StyledLine.empty(0); const { contentWidth: cellWidth, maxWordWidth: cellWordWidth } = calculateWidths(cellStyledLine); @@ -186,7 +180,6 @@ export const TableRenderer: React.FC = ({ const rowResult: ProcessedLine[][] = []; // Ensure we iterate up to numColumns, filling with empty cells if needed for (let colIndex = 0; colIndex < numColumns; colIndex++) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment const cellStyledLine = row[colIndex] || StyledLine.empty(0); const allocatedWidth = finalContentWidths[colIndex]; const contentWidth = Math.max(1, allocatedWidth); @@ -203,7 +196,6 @@ export const TableRenderer: React.FC = ({ ); const lines = wrappedStyledLines.map((line) => ({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment text: styledLineToString(line), width: styledCharsWidth(line), })); diff --git a/packages/core/src/config/extensions/integrity.ts b/packages/core/src/config/extensions/integrity.ts index cd272174b17..95418df4771 100644 --- a/packages/core/src/config/extensions/integrity.ts +++ b/packages/core/src/config/extensions/integrity.ts @@ -147,8 +147,7 @@ class ExtensionIntegrityStore { const { store, signature: actualSignature } = rawStore; // Re-generate the expected signature for the store content. - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const storeContent: string = stableStringify(store) ?? ''; + const storeContent = stableStringify(store) ?? ''; const expectedSignature = await this.generateSignature(storeContent); // Verify the store hasn't been tampered with. @@ -166,8 +165,7 @@ class ExtensionIntegrityStore { */ async save(store: ExtensionIntegrityMap): Promise { // Generate a signature for the entire map to prevent manual tampering. - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const storeContent: string = stableStringify(store) ?? ''; + const storeContent = stableStringify(store) ?? ''; const storeSignature = await this.generateSignature(storeContent); const finalData: IntegrityStore = { @@ -192,8 +190,7 @@ class ExtensionIntegrityStore { * Generates a deterministic SHA-256 hash of the metadata. */ generateHash(metadata: ExtensionInstallMetadata): string { - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const content: string = stableStringify(metadata) ?? ''; + const content = stableStringify(metadata) ?? ''; return createHash('sha256').update(content).digest('hex'); } From 591b74c3b137e0174195115762fded4079072169 Mon Sep 17 00:00:00 2001 From: Famous Date: Thu, 16 Apr 2026 22:11:41 +0530 Subject: [PATCH 18/27] fix(shell): update execute() calls to use ExecuteOptions object --- packages/core/src/tools/shell-utils.ts | 5 +++ packages/core/src/tools/shell.test.ts | 60 +++++++++++++++++++------- 2 files changed, 50 insertions(+), 15 deletions(-) create mode 100644 packages/core/src/tools/shell-utils.ts diff --git a/packages/core/src/tools/shell-utils.ts b/packages/core/src/tools/shell-utils.ts new file mode 100644 index 00000000000..f85b3e5e97c --- /dev/null +++ b/packages/core/src/tools/shell-utils.ts @@ -0,0 +1,5 @@ +/** + * @license + * Copyright 2026 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 6abd1ec1665..af9b981c1c2 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -1200,14 +1200,18 @@ describe('ShellTool', () => { it('should block $() command substitution', async () => { const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo $(whoami)' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toContain('Blocked'); }); it('should block backtick command substitution', async () => { const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo `whoami`' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toContain('Blocked'); }); @@ -1228,7 +1232,9 @@ describe('ShellTool', () => { })); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo hello' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).not.toContain('Blocked'); }); @@ -1251,7 +1257,9 @@ describe('ShellTool', () => { const invocation = tool.build({ command: "echo '$(not substituted)'", }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).not.toContain('Blocked'); }); @@ -1272,21 +1280,27 @@ describe('ShellTool', () => { })); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo \\`hello\\`' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).not.toContain('Blocked'); }); it('should block $() inside double quotes', async () => { const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo "$(whoami)"' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toContain('Blocked'); }); it('should block >() process substitution', async () => { const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo >(whoami)' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toContain('Blocked'); }); @@ -1309,14 +1323,18 @@ describe('ShellTool', () => { const invocation = tool.build({ command: "echo '$(whoami)'", }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).not.toContain('Blocked'); }); it('should block PowerShell @() array subexpression', async () => { mockPlatform.mockReturnValue('win32'); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo @(whoami)' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toContain('Blocked'); }); @@ -1324,7 +1342,9 @@ describe('ShellTool', () => { mockPlatform.mockReturnValue('win32'); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo $(whoami)' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toContain('Blocked'); }); @@ -1348,7 +1368,9 @@ describe('ShellTool', () => { const invocation = tool.build({ command: "echo '$(whoami)'", }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).not.toContain('Blocked'); }); it('should allow escaped substitution outside quotes', async () => { @@ -1368,7 +1390,9 @@ describe('ShellTool', () => { })); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo \\$(whoami)' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).not.toContain('Blocked'); }); @@ -1389,14 +1413,18 @@ describe('ShellTool', () => { })); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo "<(whoami)"' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).not.toContain('Blocked'); }); it('should block process substitution without quotes', async () => { const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo <(whoami)' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).toContain('Blocked'); }); @@ -1417,7 +1445,9 @@ describe('ShellTool', () => { })); const tool = new ShellTool(mockConfig, createMockMessageBus()); const invocation = tool.build({ command: 'echo "\\$(whoami)"' }); - const result = await invocation.execute(new AbortController().signal); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); expect(result.returnDisplay).not.toContain('Blocked'); }); }); From 41d209b60259796f79a8ce9a9204e4cccbc618e0 Mon Sep 17 00:00:00 2001 From: Famous Date: Fri, 17 Apr 2026 11:59:33 +0530 Subject: [PATCH 19/27] fix(shell): use path-based PowerShell detection instead of exact string match --- packages/core/src/tools/shell-utils.ts | 5 -- packages/core/src/tools/shell.test.ts | 78 ++++++++++++++++++++++++++ packages/core/src/utils/shell-utils.ts | 6 +- 3 files changed, 83 insertions(+), 6 deletions(-) delete mode 100644 packages/core/src/tools/shell-utils.ts diff --git a/packages/core/src/tools/shell-utils.ts b/packages/core/src/tools/shell-utils.ts deleted file mode 100644 index f85b3e5e97c..00000000000 --- a/packages/core/src/tools/shell-utils.ts +++ /dev/null @@ -1,5 +0,0 @@ -/** - * @license - * Copyright 2026 Google LLC - * SPDX-License-Identifier: Apache-2.0 - */ diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index af9b981c1c2..47d6f13af9d 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -1428,6 +1428,84 @@ describe('ShellTool', () => { expect(result.returnDisplay).toContain('Blocked'); }); + it('should allow escaped $() outside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo \\$(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow output process substitution inside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '<(whoami)', + rawOutput: Buffer.from('<(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "<(whoami)"' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should block <() process substitution without quotes', async () => { + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo <(whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); + + it('should allow escaped $() inside double quotes', async () => { + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: '$(whoami)', + rawOutput: Buffer.from('$(whoami)'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo "\\$(whoami)"' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + it('should allow escaped substitution inside double quotes', async () => { mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ pid: 12345, diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 1b3ef59b76d..d69c776e7c1 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -1023,7 +1023,11 @@ export async function* execStreaming( export function detectCommandSubstitution(command: string): boolean { const shell = getShellConfiguration().shell; - if (shell === 'powershell') { + const isPowerShell = + typeof shell === 'string' && + (shell.toLowerCase().includes('powershell') || + shell.toLowerCase().includes('pwsh')); + if (isPowerShell) { return detectPowerShellSubstitution(command); } return detectBashSubstitution(command); From 4366b1d56fe85cd3e27b7d719a79606dc2b1ac5c Mon Sep 17 00:00:00 2001 From: Famous Date: Fri, 17 Apr 2026 12:06:39 +0530 Subject: [PATCH 20/27] fix(shell): block PowerShell bare () grouping operator and use path-based PowerShell detection --- packages/core/src/tools/shell.test.ts | 9 +++++++++ packages/core/src/utils/shell-utils.ts | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 47d6f13af9d..741d880cac4 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -1482,6 +1482,15 @@ describe('ShellTool', () => { }); expect(result.returnDisplay).toContain('Blocked'); }); + it('should block PowerShell bare () grouping operator', async () => { + mockPlatform.mockReturnValue('win32'); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ command: 'echo (whoami)' }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).toContain('Blocked'); + }); it('should allow escaped $() inside double quotes', async () => { mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index d69c776e7c1..10b1f94a207 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -1113,6 +1113,12 @@ function detectPowerShellSubstitution(command: string): boolean { if (char === '@' && command[i + 1] === '(') { return true; } + if (char === '(' && !inDoubleQuote) { + const prev = i > 0 ? command[i - 1] : ''; + if (prev !== '$' && prev !== '@') { + return true; + } + } i++; } return false; From ab84bca9b92b030b003e419235f677bbc42bd631 Mon Sep 17 00:00:00 2001 From: Famous Date: Mon, 20 Apr 2026 23:37:30 +0530 Subject: [PATCH 21/27] fix(shell): fix PowerShell detection to allow method calls and function definitions --- packages/core/src/utils/shell-utils.ts | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index 10b1f94a207..e82edb0ec05 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -1110,14 +1110,28 @@ function detectPowerShellSubstitution(command: string): boolean { if (char === '$' && command[i + 1] === '(') { return true; } - if (char === '@' && command[i + 1] === '(') { + if (char === '@' && command[i + 1] === '(' && !inDoubleQuote) { return true; } if (char === '(' && !inDoubleQuote) { const prev = i > 0 ? command[i - 1] : ''; - if (prev !== '$' && prev !== '@') { - return true; + if (prev === '$' || prev === '@' || prev === '.' || prev === ':') { + i++; + continue; + } + if (/\w/.test(prev)) { + i++; + continue; } + const beforeParen = command.slice(0, i).trimEnd(); + const contextMatch = beforeParen.match( + /\b(if|elseif|else|foreach|for|while|do|switch|catch|trap|until|function|filter)(\s+\w+)?\s*$/i, + ); + if (contextMatch) { + i++; + continue; + } + return true; } i++; } From a2a048a77a2550142d737af33c91c1c07291554b Mon Sep 17 00:00:00 2001 From: Famous Date: Mon, 20 Apr 2026 23:40:45 +0530 Subject: [PATCH 22/27] style: fix trailing newline formatting --- docs/CONTRIBUTING.md | 2 +- packages/core/src/tools/shell.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 44fcc634393..5c10c35f62e 120000 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -1 +1 @@ -../CONTRIBUTING.md \ No newline at end of file +../CONTRIBUTING.md diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index 3db63d3f306..ef49d885579 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -1027,4 +1027,3 @@ export class ShellTool extends BaseDeclarativeTool< return resolveToolDeclaration(definition, modelId); } } - From 4fe4ec99fc777ae1166464f3cb88006da4f0d79c Mon Sep 17 00:00:00 2001 From: Famous Date: Tue, 21 Apr 2026 22:38:06 +0530 Subject: [PATCH 23/27] fix(shell): revert CONTRIBUTING.md, refine PowerShell detection with keyword regex and nested paren support --- docs/CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 5c10c35f62e..44fcc634393 120000 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -1 +1 @@ -../CONTRIBUTING.md +../CONTRIBUTING.md \ No newline at end of file From 4e63e6311beab940b45c8378402a2c92de233a25 Mon Sep 17 00:00:00 2001 From: Famous Date: Tue, 21 Apr 2026 22:48:38 +0530 Subject: [PATCH 24/27] Implemented mentor's suggestion --- packages/core/src/tools/shell.test.ts | 52 ++++++++++++++++++++++++++ packages/core/src/tools/shell.ts | 1 + packages/core/src/utils/shell-utils.ts | 27 +++++++------ 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 741d880cac4..a92a4c7c44d 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -1537,5 +1537,57 @@ describe('ShellTool', () => { }); expect(result.returnDisplay).not.toContain('Blocked'); }); + + it('should allow PowerShell keyword with flag e.g. switch -regex ($x)', async () => { + mockPlatform.mockReturnValue('win32'); + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: 'result', + rawOutput: Buffer.from('result'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: 'switch -regex ($x) { "a" { 1 } }', + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); + + it('should allow PowerShell nested parentheses e.g. if ((condition))', async () => { + mockPlatform.mockReturnValue('win32'); + mockShellExecutionService.mockImplementation((_cmd, _cwd, _callback) => ({ + pid: 12345, + result: Promise.resolve({ + output: 'result', + rawOutput: Buffer.from('result'), + exitCode: 0, + signal: null, + error: null, + aborted: false, + pid: 12345, + executionMethod: 'child_process', + backgrounded: false, + }), + })); + const tool = new ShellTool(mockConfig, createMockMessageBus()); + const invocation = tool.build({ + command: 'if ((condition)) { Write-Host ok }', + }); + const result = await invocation.execute({ + abortSignal: new AbortController().signal, + }); + expect(result.returnDisplay).not.toContain('Blocked'); + }); }); }); diff --git a/packages/core/src/tools/shell.ts b/packages/core/src/tools/shell.ts index ef49d885579..4f3e9342fc7 100644 --- a/packages/core/src/tools/shell.ts +++ b/packages/core/src/tools/shell.ts @@ -448,6 +448,7 @@ export class ShellToolInvocation extends BaseToolInvocation< llmContent: 'Command injection detected: command substitution syntax ' + '($(), backticks, <() or >()) found in command arguments. ' + + 'On PowerShell, @() array subexpressions and $() subexpressions are also blocked. ' + 'This is a security risk and the command was blocked.', returnDisplay: 'Blocked: command substitution detected in shell command.', diff --git a/packages/core/src/utils/shell-utils.ts b/packages/core/src/utils/shell-utils.ts index e82edb0ec05..6f02579df91 100644 --- a/packages/core/src/utils/shell-utils.ts +++ b/packages/core/src/utils/shell-utils.ts @@ -1083,12 +1083,16 @@ function detectBashSubstitution(command: string): boolean { return false; } +const POWERSHELL_KEYWORD_RE = + /\b(if|elseif|else|foreach|for|while|do|switch|try|catch|finally|until|trap|function|filter)(\s+[-\w]+)*\s*$/i; + function detectPowerShellSubstitution(command: string): boolean { let inSingleQuote = false; let inDoubleQuote = false; let i = 0; while (i < command.length) { const char = command[i]; + if (char === "'" && !inDoubleQuote) { inSingleQuote = !inSingleQuote; i++; @@ -1099,40 +1103,35 @@ function detectPowerShellSubstitution(command: string): boolean { i++; continue; } + if (inSingleQuote) { i++; continue; } - if (char === '`' && !inSingleQuote && i + 1 < command.length) { + if (char === '`' && i + 1 < command.length) { i += 2; continue; } if (char === '$' && command[i + 1] === '(') { return true; } - if (char === '@' && command[i + 1] === '(' && !inDoubleQuote) { + if (!inDoubleQuote && char === '@' && command[i + 1] === '(') { return true; } - if (char === '(' && !inDoubleQuote) { - const prev = i > 0 ? command[i - 1] : ''; - if (prev === '$' || prev === '@' || prev === '.' || prev === ':') { - i++; - continue; - } - if (/\w/.test(prev)) { + if (!inDoubleQuote && char === '(') { + const before = command.slice(0, i).trimEnd(); + const prevChar = before[before.length - 1]; + if (prevChar === '(') { i++; continue; } - const beforeParen = command.slice(0, i).trimEnd(); - const contextMatch = beforeParen.match( - /\b(if|elseif|else|foreach|for|while|do|switch|catch|trap|until|function|filter)(\s+\w+)?\s*$/i, - ); - if (contextMatch) { + if (POWERSHELL_KEYWORD_RE.test(before)) { i++; continue; } return true; } + i++; } return false; From 8612fc55f171bcd1f358b6afd9ca5b625f969b25 Mon Sep 17 00:00:00 2001 From: Famous Date: Wed, 22 Apr 2026 22:08:16 +0530 Subject: [PATCH 25/27] style: run npm run format --- docs/CONTRIBUTING.md | 2 +- packages/core/src/tools/shell.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 44fcc634393..5c10c35f62e 120000 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -1 +1 @@ -../CONTRIBUTING.md \ No newline at end of file +../CONTRIBUTING.md diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 13940e3770e..dd49a9c8008 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -210,7 +210,7 @@ describe('ShellTool', () => { mockShellOutputCallback = callback; const match = cmd.match(/pgrep -g 0 >([^ ]+)/); if (match) { - extractedTmpFile = match[1].replace(/['"]/g, ''); + extractedTmpFile = match[1].replace(/['"]/g, ''); } return { pid: 12345, From 3595594d373db29ee463c7041a49bc4b8eed5a0d Mon Sep 17 00:00:00 2001 From: Famous Date: Wed, 22 Apr 2026 23:03:53 +0530 Subject: [PATCH 26/27] Revert "style: run npm run format" This reverts commit 8612fc55f171bcd1f358b6afd9ca5b625f969b25. --- docs/CONTRIBUTING.md | 2 +- packages/core/src/tools/shell.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 5c10c35f62e..44fcc634393 120000 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -1 +1 @@ -../CONTRIBUTING.md +../CONTRIBUTING.md \ No newline at end of file diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index dd49a9c8008..13940e3770e 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -210,7 +210,7 @@ describe('ShellTool', () => { mockShellOutputCallback = callback; const match = cmd.match(/pgrep -g 0 >([^ ]+)/); if (match) { - extractedTmpFile = match[1].replace(/['"]/g, ''); + extractedTmpFile = match[1].replace(/['"]/g, ''); } return { pid: 12345, From d076b2125890aa52349c7263f6365292100b163b Mon Sep 17 00:00:00 2001 From: Famous Date: Wed, 22 Apr 2026 23:06:38 +0530 Subject: [PATCH 27/27] style: run npm run format --- packages/core/src/tools/shell.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/tools/shell.test.ts b/packages/core/src/tools/shell.test.ts index 13940e3770e..dd49a9c8008 100644 --- a/packages/core/src/tools/shell.test.ts +++ b/packages/core/src/tools/shell.test.ts @@ -210,7 +210,7 @@ describe('ShellTool', () => { mockShellOutputCallback = callback; const match = cmd.match(/pgrep -g 0 >([^ ]+)/); if (match) { - extractedTmpFile = match[1].replace(/['"]/g, ''); + extractedTmpFile = match[1].replace(/['"]/g, ''); } return { pid: 12345,