Skip to content

Fix sudo -S sensitive input detection#317594

Merged
meganrogge merged 2 commits into
microsoft:mainfrom
meganrogge:fix-terminal-sensitive-sudo-s
May 20, 2026
Merged

Fix sudo -S sensitive input detection#317594
meganrogge merged 2 commits into
microsoft:mainfrom
meganrogge:fix-terminal-sensitive-sudo-s

Conversation

@meganrogge
Copy link
Copy Markdown
Collaborator

@meganrogge meganrogge commented May 20, 2026

Fixes #317490

Summary

When run_in_terminal sees a sudo password prompt, it currently always classifies it as sensitive interactive input and triggers the sensitive-input flow. That is correct for plain sudo, but it is wrong for sudo -S, where the password is expected on stdin and the [sudo] password for ...: line is just status output.

This change keeps the existing sensitive-input behavior for interactive prompts, but treats the canonical sudo prompt as non-sensitive when the command includes sudo -S, so the terminal stays on the normal input-needed path instead of being auto-cancelled in auto-approve mode.

Changes

  • route sensitive prompt checks through OutputMonitor._isSensitivePrompt(...)
  • add a narrow exception for sudo -S with the canonical [sudo] password for ...: prompt
  • add focused tests covering both sudo -S and plain sudo

Copilot AI review requested due to automatic review settings May 20, 2026 18:45
@meganrogge meganrogge self-assigned this May 20, 2026
@meganrogge meganrogge added this to the 1.122.0 milestone May 20, 2026
@meganrogge meganrogge closed this May 20, 2026
@meganrogge meganrogge reopened this May 20, 2026
@meganrogge meganrogge enabled auto-merge (squash) May 20, 2026 18:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts terminal agent tooling’s OutputMonitor so the canonical sudo password prompt ([sudo] password for …:) is not treated as a sensitive prompt when the executed command includes sudo -S, preventing the sensitive-input flow from triggering in that scenario. It also adds unit tests to cover sudo -S vs plain sudo.

Changes:

  • Centralize sensitive-prompt classification behind OutputMonitor._isSensitivePrompt(...).
  • Add a sudo -S exception for the canonical [sudo] password for …: line.
  • Add focused unit tests asserting sudo -S routes to onDidDetectInputNeeded while plain sudo still routes to onDidDetectSensitiveInputNeeded.
Show a summary per file
File Description
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts Routes sensitive-prompt checks through a helper and introduces the sudo -S exception.
src/vs/workbench/contrib/terminalContrib/chatAgentTools/test/browser/outputMonitor.test.ts Adds tests for sudo -S vs plain sudo prompt classification.

Copilot's findings

Comments suppressed due to low confidence (1)

src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/tools/monitoring/outputMonitor.ts:609

  • isNonInteractiveSudoSensitivePrompt reads like it returns true when a prompt is sensitive, but the only call site uses it to return false (ie treat the prompt as non-sensitive). Renaming it (or inverting the predicate) would make the intent clearer and reduce the chance of future misuse when additional exceptions are added.
function isNonInteractiveSudoSensitivePrompt(command: string, prompt: string): boolean {
	return /(?:^|\s)sudo\s+-S(?:\s|$)/.test(command) && /^\[sudo\]\s+password for .+:\s*$/i.test(prompt);
}
  • Files reviewed: 2/2 changed files
  • Comments generated: 2

@meganrogge meganrogge merged commit 2e3819a into microsoft:main May 20, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: add setting to disable sensitive input detection in terminal agent tools

3 participants