From b1d9f66988c09aa1a7b8eca1711d5896b19c014f Mon Sep 17 00:00:00 2001 From: meganrogge Date: Mon, 13 Apr 2026 16:30:27 -0400 Subject: [PATCH 1/5] fixes #309607 --- .../browser/tools/runInTerminalTool.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index 24a20be24e1ba..f31d5025629a5 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -2132,6 +2132,15 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { return; } + // If the user manually stopped the agent, do not interject with + // new steering requests. Dispose the background notification so + // no further events fire. + if (sessionRef.object.lastRequest?.response?.isCanceled) { + this._logService.debug(`RunInTerminalTool: Suppressing input-needed notification for terminal ${termId} because session was cancelled`); + disposeNotification(); + return; + } + const execution = RunInTerminalTool._activeExecutions.get(termId); if (!execution) { return; @@ -2185,6 +2194,14 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { return; } + // If the user manually stopped the agent, do not interject with + // new steering requests. + if (sessionRef.object.lastRequest?.response?.isCanceled) { + this._logService.debug(`RunInTerminalTool: Suppressing completion notification for terminal ${termId} because session was cancelled`); + disposeNotification(); + return; + } + // Dispose after first notification to avoid chatty repeated messages // if the user runs additional commands via send_to_terminal. disposeNotification(); From 19eeaf63e1523635b48eb6df72daf25b83037c37 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Mon, 13 Apr 2026 16:31:51 -0400 Subject: [PATCH 2/5] dedupe --- .../browser/tools/runInTerminalTool.ts | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index f31d5025629a5..34ed73d5c7ab2 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -2111,6 +2111,18 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { // stops asking questions and lets the user finish interacting with the terminal. let userIsReplyingDirectly = false; + const disposeNotification = () => this._backgroundNotifications.deleteAndDispose(termId); + + // If the user manually stopped the agent, suppress background + // steering requests and tear down the notification listeners. + const isSessionCancelled = () => { + if (sessionRef.object.lastRequest?.response?.isCanceled) { + disposeNotification(); + return true; + } + return false; + }; + if (outputMonitor) { let lastInputNeededOutput = ''; let lastInputNeededNotificationTime = 0; @@ -2132,12 +2144,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { return; } - // If the user manually stopped the agent, do not interject with - // new steering requests. Dispose the background notification so - // no further events fire. - if (sessionRef.object.lastRequest?.response?.isCanceled) { - this._logService.debug(`RunInTerminalTool: Suppressing input-needed notification for terminal ${termId} because session was cancelled`); - disposeNotification(); + if (isSessionCancelled()) { return; } @@ -2185,8 +2192,6 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { store.add(sessionRef); - const disposeNotification = () => this._backgroundNotifications.deleteAndDispose(termId); - store.add(commandDetection.onCommandFinished(command => { const execution = RunInTerminalTool._activeExecutions.get(termId); if (!execution) { @@ -2194,11 +2199,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { return; } - // If the user manually stopped the agent, do not interject with - // new steering requests. - if (sessionRef.object.lastRequest?.response?.isCanceled) { - this._logService.debug(`RunInTerminalTool: Suppressing completion notification for terminal ${termId} because session was cancelled`); - disposeNotification(); + if (isSessionCancelled()) { return; } From b97c65a7145bf0d827f0511f052e7472d9cd88e2 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Mon, 13 Apr 2026 16:37:59 -0400 Subject: [PATCH 3/5] fix ReDoS in PowerShell prompt regex, add regression test --- .../browser/tools/monitoring/outputMonitor.ts | 5 +++-- .../chatAgentTools/test/browser/outputMonitor.test.ts | 9 +++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts index f0f6bdda85a46..a2bf46a04e18c 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts @@ -468,8 +468,9 @@ export function matchTerminalPromptOption(options: readonly string[], suggestedO export function detectsInputRequiredPattern(cursorLine: string): boolean { return [ // PowerShell-style multi-option line (supports [?] Help and optional default suffix) ending - // in whitespace - /\s*(?:\[[^\]]\]\s+[^\[\s][^\[]*\s*)+(?:\(default is\s+"[^"]+"\):)?\s+$/, + // in whitespace. The label part uses [^\[\s]+(?:\s+[^\[\s]+)* to support multi-word + // labels (e.g. "Yes to All") while avoiding overlap with \s* that caused ReDoS. + /\s*(?:\[[^\]]\]\s+[^\[\s]+(?:\s+[^\[\s]+)*\s*)+(?:\(default is\s+"[^"]+"\):)?\s+$/, // Bracketed/parenthesized yes/no pairs at end of line: (y/n), [Y/n], (yes/no), [no/yes] /(?:\(|\[)\s*(?:y(?:es)?\s*\/\s*n(?:o)?|n(?:o)?\s*\/\s*y(?:es)?)\s*(?:\]|\))\s+$/i, // Same as above but allows a preceding '?' or ':' and optional wrappers e.g. diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/outputMonitor.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/outputMonitor.test.ts index 9c4bfd155bff1..0430a4d4d1111 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/outputMonitor.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/outputMonitor.test.ts @@ -271,6 +271,15 @@ suite('OutputMonitor', () => { false ); }); + test('PowerShell regex does not cause catastrophic backtracking (ReDoS)', () => { + // Pathological input: many spaces not followed by a bracket. + // With the old overlapping regex this would hang; it must return promptly. + const start = performance.now(); + const pathological = '[Y] Yes' + ' '.repeat(200) + 'x'; + detectsInputRequiredPattern(pathological); + const elapsed = performance.now() - start; + assert.ok(elapsed < 500, `Regex took ${elapsed}ms on pathological input, expected < 500ms`); + }); test('Line ends with colon', () => { assert.strictEqual(detectsInputRequiredPattern('Enter your name: '), true); assert.strictEqual(detectsInputRequiredPattern('Password: '), true); From c46214a1f048558a2844fe38f2749210268d975f Mon Sep 17 00:00:00 2001 From: meganrogge Date: Mon, 13 Apr 2026 16:50:06 -0400 Subject: [PATCH 4/5] better handling --- .../browser/tools/runInTerminalTool.ts | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts index 34ed73d5c7ab2..fb3748b6963b1 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/runInTerminalTool.ts @@ -28,7 +28,7 @@ import { ITerminalLogService, ITerminalProfile } from '../../../../../../platfor import { IRemoteAgentService } from '../../../../../services/remote/common/remoteAgentService.js'; import { TerminalToolConfirmationStorageKeys } from '../../../../chat/browser/widget/chatContentParts/toolInvocationParts/chatTerminalToolConfirmationSubPart.js'; import { IChatService, ChatRequestQueueKind, ElicitationState, type IChatExternalToolInvocationUpdate, type IChatTerminalToolInvocationData } from '../../../../chat/common/chatService/chatService.js'; -import { constObservable, type IObservable } from '../../../../../../base/common/observable.js'; +import { autorun, constObservable, type IObservable } from '../../../../../../base/common/observable.js'; import { ChatModel, type IChatRequestModeInfo } from '../../../../chat/common/model/chatModel.js'; import type { UserSelectedTools } from '../../../../chat/common/participants/chatAgents.js'; import { CountTokensCallback, ILanguageModelToolsService, IPreparedToolInvocation, IToolConfirmationMessages, IStreamedToolInvocation, IToolData, IToolImpl, IToolInvocation, IToolInvocationPreparationContext, IToolInvocationStreamContext, IToolResult, ToolDataSource, ToolInvocationPresentation, ToolProgress } from '../../../../chat/common/tools/languageModelToolsService.js'; @@ -2115,7 +2115,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { // If the user manually stopped the agent, suppress background // steering requests and tear down the notification listeners. - const isSessionCancelled = () => { + const handleSessionCancelled = (): boolean => { if (sessionRef.object.lastRequest?.response?.isCanceled) { disposeNotification(); return true; @@ -2123,6 +2123,21 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { return false; }; + // Proactively detect session cancellation so that all background + // listeners are torn down immediately, rather than waiting for the + // next terminal event to fire and discover the cancelled state. + store.add(autorun(reader => { + const request = sessionRef.object.lastRequestObs.read(reader); + if (!request?.response) { + return; + } + reader.store.add(request.response.onDidChange(ev => { + if (ev.reason === 'completedRequest' && request.response!.isCanceled) { + disposeNotification(); + } + })); + })); + if (outputMonitor) { let lastInputNeededOutput = ''; let lastInputNeededNotificationTime = 0; @@ -2144,7 +2159,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { return; } - if (isSessionCancelled()) { + if (handleSessionCancelled()) { return; } @@ -2199,7 +2214,7 @@ export class RunInTerminalTool extends Disposable implements IToolImpl { return; } - if (isSessionCancelled()) { + if (handleSessionCancelled()) { return; } From 8865c875b7c8fd26d88c7bcc59496730d6997fc6 Mon Sep 17 00:00:00 2001 From: meganrogge Date: Tue, 14 Apr 2026 11:56:05 -0400 Subject: [PATCH 5/5] fix test --- .../test/electron-browser/runInTerminalTool.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts index 92e8111240c67..06f4aa1c4e8f6 100644 --- a/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts +++ b/src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/electron-browser/runInTerminalTool.test.ts @@ -7,6 +7,7 @@ import { ok, strictEqual } from 'assert'; import { Separator } from '../../../../../../base/common/actions.js'; import { CancellationToken } from '../../../../../../base/common/cancellation.js'; import { Emitter, Event } from '../../../../../../base/common/event.js'; +import { constObservable } from '../../../../../../base/common/observable.js'; import { Schemas } from '../../../../../../base/common/network.js'; import { isLinux, isWindows, OperatingSystem } from '../../../../../../base/common/platform.js'; import { count } from '../../../../../../base/common/strings.js'; @@ -150,6 +151,7 @@ suite('RunInTerminalTool', () => { acquireExistingSession: () => ({ object: { lastRequest: undefined, + lastRequestObs: constObservable(undefined), onDidChange: Event.None, }, dispose: () => { },