microsoft / microsoft/vscode

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

Open
#329,218 0 comments 0 reactions 1 assignee Claimed by @anthonykim1 View on GitHub
agent-host debt terminal-auto-approve
Dominant language
TypeScript
Stars
193k
Forks
42.4k
PR merge metrics
PR metrics pending

Description

Follow-up to https://github.com/microsoft/vscode/pull/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: https://github.com/microsoft/vscode/issues/328898 covers `sed -i` in-place parsing, and https://github.com/microsoft/vscode/issues/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 `.
- 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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.