Skip to content

Fix PowerShell quoting for runInTerminal environment values - #331753

Open
Zain Nadeem (zainnadeem786) wants to merge 3 commits into
microsoft:mainfrom
zainnadeem786:research/oss-assurance-review
Open

Fix PowerShell quoting for runInTerminal environment values#331753
Zain Nadeem (zainnadeem786) wants to merge 3 commits into
microsoft:mainfrom
zainnadeem786:research/oss-assurance-review

Conversation

@zainnadeem786

@zainnadeem786 Zain Nadeem (zainnadeem786) commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR fixes PowerShell environment value quoting when constructing commands for Debug Adapter Protocol runInTerminal requests.

Environment values were previously interpolated directly inside PowerShell single-quoted strings:

command += `\${env:${key}}='${value}'; `;

This works for ordinary values, but values containing embedded single quotes are not escaped correctly.

For example, an environment value such as:

hello'world

was represented as:

${env:QUOTE}='hello'world';

instead of the correctly escaped PowerShell representation:

${env:QUOTE}='hello''world';

Root cause

PowerShell single-quoted strings represent a literal single quote by doubling it ('').

The existing environment assignment wrapped values in single quotes without escaping embedded single quotes, so values containing ' were not represented correctly.

The existing PowerShell quote() helper used for command arguments cannot be reused directly for environment values because it also special-cases values ending in a backslash. That behavior is appropriate for argument construction, but PowerShell single-quoted strings treat backslashes literally, so applying it to environment values would change values such as:

C:\work\

into a value with an additional trailing backslash.

Change

Environment values now escape embedded single quotes directly while preserving literal backslashes:

- command += `\${env:${key}}='${value}'; `;
+ command += `\${env:${key}}='${value.replace(/'/g, '\'\'')}'; `;

This applies the escaping required for PowerShell single-quoted strings without applying argument-specific trailing-backslash handling to environment values.

Existing handling of null environment values remains unchanged.

Regression coverage

Added a focused prepareCommand regression test covering PowerShell environment values with:

  • ordinary text;
  • spaces;
  • an empty string;
  • a single embedded quote;
  • multiple embedded quotes;
  • a trailing backslash;
  • a combination of backslashes and embedded single quotes.

For example:

hello'world

is represented as:

${env:QUOTE}='hello''world';

A value ending in a backslash:

C:\work\

is preserved as:

${env:TRAILING}='C:\work\';

And a value containing both an embedded quote and backslashes is represented without altering the backslashes:

C:\it's\path\

becomes:

${env:BACKSLASH_QUOTE}='C:\it''s\path\';

Validation

The focused regression test passes:

powershell - quotes environment values
1 passing
errorlevel: 0

The existing prepareCommand test suite also passes:

Debug - prepareCommand
7 passing
errorlevel: 0

Client transpilation completes successfully:

npm run transpile-client
✓ completed successfully

git diff --check also completes with exit code 0.

The resulting PowerShell single-quoted representation was additionally verified directly in PowerShell.

Scope

The change is intentionally limited to PowerShell environment value construction in prepareCommand().

No behavior for other shells is changed, and existing handling of null environment values remains unchanged.

Close #331751

Copilot AI balanced review requested due to automatic review settings August 20, 2026 05:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes PowerShell quoting for runInTerminal environment values.

Changes:

  • Reuses PowerShell argument quoting for environment values.
  • Adds regression coverage for embedded single quotes.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/vs/workbench/contrib/debug/node/terminals.ts Updates PowerShell environment assignment quoting.
src/vs/workbench/contrib/debug/test/node/terminals.test.ts Tests common and quoted environment values.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/vs/workbench/contrib/debug/node/terminals.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/vs/workbench/contrib/debug/node/terminals.ts:106

  • The implementation contradicts the PR description and linked issue, which explicitly say this assignment now reuses quote(value). It instead creates a second PowerShell escaping path; this distinction is material because quote() also special-cases trailing backslashes while the new tests expect environment values not to. Please either share/factor the literal-escaping logic while preserving the intended trailing-backslash behavior, or update the PR rationale to explain why quote() cannot be reused.
						command += `\${env:${key}}='${value.replace(/'/g, '\'\'')}'; `;

@connor4312 Connor Peet (connor4312) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The ASCII-apostrophe fix and trailing-backslash handling look correct in both Windows PowerShell 5.1 and PowerShell 7. One remaining gap: PowerShell also treats U+2018/U+2019 smart single quotes as quote characters, but this only doubles ASCII '. Please escape those as well and add regression coverage.

Separately, ${env:X}='' removes X in PowerShell 5.1 and 7.0–7.4, while 7.5+ preserves an empty value. That is pre-existing and need not block this focused fix, but the snapshot test does not establish equivalent runtime behavior across versions.

@zainnadeem786

Copy link
Copy Markdown
Contributor Author

The ASCII-apostrophe fix and trailing-backslash handling look correct in both Windows PowerShell 5.1 and PowerShell 7. One remaining gap: PowerShell also treats U+2018/U+2019 smart single quotes as quote characters, but this only doubles ASCII '. Please escape those as well and add regression coverage.

Separately, ${env:X}='' removes X in PowerShell 5.1 and 7.0–7.4, while 7.5+ preserves an empty value. That is pre-existing and need not block this focused fix, but the snapshot test does not establish equivalent runtime behavior across versions.

Thanks for checking this across Windows PowerShell 5.1 and PowerShell 7. Good catch on the U+2018/U+2019 smart quotes. I'll update the escaping and regression coverage for those cases while keeping the empty-value behavior out of scope for this focused fix.

@zainnadeem786

Copy link
Copy Markdown
Contributor Author

Thanks for the review Connor Peet (@connor4312) . Confirmed the U+2018/U+2019 behavior in both Windows PowerShell 5.1 and PowerShell 7, and updated the escaping to double all three PowerShell single-quote characters while preserving trailing backslashes. I also added regression coverage for both smart-quote cases.

The focused prepareCommand test, full Debug - prepareCommand suite, client transpilation, and git diff --check all pass.

I’ve kept the version-dependent empty environment value behavior out of scope as suggested.

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.

PowerShell runInTerminal does not correctly quote environment values containing single quotes

3 participants