fix(vscode): prevent SMB credentials from reaching telemetry

[Riley Evans (rllyy97)]

Commit Type

• feature - New functionality
• fix - Bug fix
• refactor - Code restructuring without behavior change
• perf - Performance improvement
• docs - Documentation update
• test - Test-related changes
• chore - Maintenance/tooling

Risk Level

• Low - Minor changes, limited scope
• Medium - Moderate changes, some user impact
• High - Major changes, significant user/system impact

What & Why

Fixes a CWE-532 vulnerability where SMB passwords interpolated into exec(net use ...) could propagate through child-process errors, the webview, and LoggerService into Application Insights. The interpolation also enabled shell injection.

The fix invokes WNetAddConnection2W using a fixed, encoded PowerShell payload, the absolute System32 PowerShell path, and shell: false. Credentials are supplied only through stdin, child output is ignored, and callers receive stable, credential-free errors.

Impact of Change

• Users: SMB authentication no longer risks exposing passwords in telemetry or permitting shell injection. Connection failures remain actionable without including credentials.
• Developers: SMB connection handling now uses a fixed PowerShell/native API path and stable sanitized errors. No new dependency is introduced.
• System: Credentials are isolated to child-process stdin, while stdout and stderr are ignored. This materially changes a security-sensitive system path and therefore carries high risk.

Test Plan

• Unit tests added/updated
• E2E tests added/updated
• Manual testing completed
• Tested in:

Validation evidence:

• Biome clean.
• Focused Vitest passes 6/6.
• Production file ESLint passes.
• Embedded PowerShell smoke test reaches the expected native failure path.
• Extension bundling succeeds.
• Repository TypeScript check does not pass overall: it still reports 32 pre-existing, unrelated errors in validateNodeJsIsLatest.ts, registerCommands.ts, workspaceWebviewCommandHandler.ts, switchToDotnetProject.ts, azureClients.ts, requestUtils.ts, and existing src/test/ui files.

Contributors

Riley Evans (@rllyy97)

Screenshots/Videos

Not applicable because there are no visual changes.

[github-actions]

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

✅ PR Title

• Current: fix(vscode): prevent SMB credentials from reaching telemetry
• Issue: None — uses a valid fix(scope): prefix and clearly describes the security fix.
• Recommendation: No change needed.

---

✅ Commit Type

• Exactly one type selected: fix - Bug fix.
• Matches the title prefix and the nature of the change (CWE-532 credential-leak fix).

---

✅ Risk Level

• High is selected in the body and the risk:high label is present, and this matches the advised estimate. This change touches credential handling in the shipped VS Code extension (SMB password flow, child-process isolation, telemetry serialization), which is security-sensitive → HIGH. Label, body, and estimate all agree.

---

✅ What & Why

• Current: Clear explanation of the CWE-532 vulnerability, shell-injection risk, and the WNetAddConnection2W + stdin-only fix.
• Issue: None.
• Recommendation: No change needed.

---

✅ Impact of Change

• Users, Developers, and System are all addressed with meaningful detail.
• Recommendation:
  • Users: No longer risks exposing SMB passwords in telemetry or shell injection — already stated.
  • Developers: Fixed PowerShell/native API path with stable sanitized errors — already stated.
  • System: Credentials isolated to child-process stdin; stdout/stderr ignored — already stated.

---

✅ Test Plan

• Unit tests added/updated confirmed in the diff (fileSystemConnection.test.ts with 8 cases covering stdin isolation, stable errors, and argument invariance; plus Logger.spec.ts cases ensuring raw errors/circular data aren't leaked). Manual testing also noted. Satisfies CHECK TESTS.

---

✅ Contributors

• @rllyy97 credited. Good.

---

✅ Screenshots/Videos

• Correctly marked not applicable — changes are in extension host logic and the Logger service, not visual UI components. No screenshot required.

---

Summary Table

Section	Status	Recommendation
Title	✅	No change needed
Commit Type	✅	No change needed
Risk Level	✅	Correctly declared High (matches estimate + label)
What & Why	✅	No change needed
Impact of Change	✅	No change needed
Test Plan	✅	Unit tests present
Contributors	✅	No change needed
Screenshots/Videos	✅	Not applicable

---

✅ All checks passed — this PR meets the template requirements and the declared High risk correctly reflects the security-sensitive credential-handling change. Cleared to merge.

---

Powered by: Copilot CLI (claude-opus-4.8) | Last updated: Tue, 18 Aug 2026 21:36:38 GMT

[github-actions]

📊 Coverage Check

The following changed files need attention:

⚠️ apps/vs-code-react/src/app/services/Logger.ts - 75% covered (needs improvement)

Please add tests for the uncovered files before merging.

[Copilot]

Pull request overview

Secures VS Code SMB connections by preventing credential exposure and shell injection.

Changes:

• Replaces exec(net use) with a fixed PowerShell/native API flow.
• Passes credentials through stdin and returns sanitized errors.
• Adds focused security and failure-path unit tests.

Reviewed changes

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

File	Description
fileSystemConnection.ts	Implements secured SMB connection handling.
fileSystemConnection.test.ts	Tests credential isolation and sanitized failures.

---

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