Skip to content

Replace sed script-argument deny regexes with a shared script analyzer #329218

Description

@anthonykim1

Follow-up to #329216.

That PR extends the default sed deny patterns to cover standalone script commands (e, r, R, w, W) passed as a positional script argument, which the previous flag-based patterns never inspected. The fix is regex-only so it could stay small and low-risk.

Related: #328898 covers sed -i in-place parsing, and #329028 covers moving shared auto-approval helpers into a lower layer. This issue is the script-argument half that neither one owns.

Debt

The new patterns hardcode a specific address grammar, so each one only matches the shapes we enumerated. Anything outside that grammar is not inspected, and there are now three dense regexes duplicated across commandAutoApprover.ts and terminalChatAgentToolsConfiguration.ts that have to stay in sync. Every future gap means editing six patterns.

Known shapes the current patterns do not cover:

  • No whitespace between the command and its argument, such as 1w/path.
  • Whitespace between the address and the command, such as 1 e <cmd>.
  • Address negation with !.
  • GNU step addresses using first~step.
  • Regex address modifiers I and M.
  • Newline as a command separator inside the script.
  • A script positioned after a BSD-style empty backup suffix, such as -i ''.

Proposal

Replace the regexes with a shared script analyzer under vs/platform/terminal/common/autoApprove:

  • Reuse the tokenizer and argument-classification logic already in SedFileWriteParser rather than adding a second tokenizer.
  • Identify which argument is the script (-e/--expression value, -f/--file, or the first positional when neither is present).
  • Walk the script for command letters in command position, accounting for addresses, !, {} blocks, ; and newline separators, and comments.
  • Return a structured result covering command execution, file writes, and file reads, so writes can be checked against the existing workspace-scoped write-destination logic instead of a blanket deny.
  • Fail closed and require confirmation when the script cannot be parsed confidently.

Both commandAutoApprover.ts and terminalChatAgentToolsConfiguration.ts carry TODO comments pointing here.

Add tests for the shapes listed above, plus negatives so ordinary usage such as sed 's/foo/bar/g' file.txt and sed -n '1,10p' file.txt keeps auto-approving.

Metadata

Metadata

Assignees

Labels

agent-hostIssues for the agent-host implementation (not the protocol)debtCode quality issuesterminal-auto-approveRelated to the terminal's auto approve logic

Type

No type

Projects

Status
No status

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions