Skip to content

Repo: Audit powershell, use powershell_builder more - #4093

Merged
Steven Malis (smalis-msft) merged 2 commits into
microsoft:mainfrom
smalis-msft:powershell-cleanup
Jul 30, 2026
Merged

Repo: Audit powershell, use powershell_builder more#4093
Steven Malis (smalis-msft) merged 2 commits into
microsoft:mainfrom
smalis-msft:powershell-cleanup

Conversation

@smalis-msft

Copy link
Copy Markdown
Contributor

An internal audit flagged that our powershell handling was vulnerable to command injection in certain cases. Fix this in powershell_builder, and then update other powershell use sites to use powershell_builder instead of duplicating the logic around. This is not a security concern as all of this powershell is for local utilities/testing/CI, none of it is shipped.

@smalis-msft
Steven Malis (smalis-msft) requested a review from a team as a code owner July 30, 2026 17:22
Copilot AI review requested due to automatic review settings July 30, 2026 17:22

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

This PR addresses a command-injection risk in how PowerShell command lines were being constructed by hardening powershell_builder’s quoting behavior and migrating several call sites to use the centralized builder instead of ad-hoc quoting/escaping.

Changes:

  • Harden PowerShell argument quoting and hashtable serialization in powershell_builder (single-quoted literals; hashtable keys now go through AsVal) and add unit tests.
  • Migrate Hyper-V-related utilities to build PowerShell invocations via powershell_builder and tighten GUID typing in PowerShell param blocks / scripts.
  • Improve Flowey Windows PowerShell invocations and reuse powershell_builder::quote_str for PowerShell script generation.

Reviewed changes

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

Show a summary per file
File Description
support/powershell_builder/src/lib.rs Switch to single-quote literal quoting, improve trust-boundary docs, adjust hashtable key handling, add unit tests.
petri/src/vm/hyperv/utilities.psm1 Cast VM ID to [guid] to avoid unsafe string interpolation behavior.
petri/src/vm/hyperv/powershell.rs Stop manually pre-quoting hashtable keys; use typed keys (GUIDs/integers) with powershell_builder quoting rules.
openhcl/diag_client/src/lib.rs Replace ad-hoc PowerShell script formatting with powershell_builder composition for COM port queries.
openhcl/diag_client/Cargo.toml Add powershell_builder as a Windows-only dependency.
hyperv/tools/hypestv/src/windows/vm.rs Tighten script param types from [string] to [guid] for VM IDs.
hyperv/tools/hypestv/src/windows/hyperv.rs Rework powershell_script to use powershell_builder for safe argument quoting and add better process-launch error context.
hyperv/tools/hypestv/Cargo.toml Add powershell_builder dependency.
flowey/flowey_lib_common/src/install_git.rs Make PowerShell invocation explicitly -NoProfile -NonInteractive -Command ... for consistency/safety.
flowey/flowey_lib_common/src/install_dotnet_cli.rs Add -NoProfile -NonInteractive to PowerShell used for dotnet installer script.
flowey/flowey_lib_common/src/gen_cargo_nextest_run_cmd.rs Centralize shell quoting per-shell; use powershell_builder::quote_str for PowerShell quoting.
flowey/flowey_lib_common/Cargo.toml Add powershell_builder dependency.
Cargo.lock Lockfile updates due to new workspace dependency edges.

Comment thread support/powershell_builder/src/lib.rs
Comment thread openhcl/diag_client/src/lib.rs Outdated
Comment thread openhcl/diag_client/src/lib.rs Outdated

@tjones60 Trevor Jones (tjones60) 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.

Looks good aside from a few nits.

Copilot AI review requested due to automatic review settings July 30, 2026 17:37
@smalis-msft
Steven Malis (smalis-msft) enabled auto-merge (squash) July 30, 2026 17:37

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 12 out of 13 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

support/powershell_builder/src/lib.rs:195

  • quote_str panics on non-UTF8 OsStr via todo!(). This can crash callers (e.g. flowey script generation) when a path/env value contains non-UTF8 / non-UTF-8-representable data. Prefer a non-panicking quoting strategy (lossy or explicit encoding) so quoting is always safe.
    if let Some(s) = s.to_str() {
        quoted.push(s.replace(r#"'"#, r#"''"#));
    } else {
        todo!("quote_str: non-UTF8 string {:?}", s);
    }

petri/src/vm/hyperv/powershell.rs:773

  • vsid is &guid::Guid here, and powershell_builder::AsVal is not implemented for Guid, so using it as a HashTable key will fail to compile. Convert it to a String (it will be quoted by the builder).
                    vsid,

petri/src/vm/hyperv/powershell.rs:750

  • vsid is a guid::Guid key, but powershell_builder::AsVal is not implemented for Guid. This will not compile with the new HashTable<K: AsVal> bounds; use a string key instead.
                vsid,

Comment thread petri/src/vm/hyperv/powershell.rs
@github-actions

Copy link
Copy Markdown

@smalis-msft
Steven Malis (smalis-msft) merged commit 50f2923 into microsoft:main Jul 30, 2026
129 of 133 checks passed
@smalis-msft
Steven Malis (smalis-msft) deleted the powershell-cleanup branch July 30, 2026 21:21
@smalis-msft Steven Malis (smalis-msft) added the backport_1.8.2607 Change should be backported to the release/1.8.2607 branch label Jul 30, 2026
@smalis-msft Steven Malis (smalis-msft) removed the backport_1.8.2607 Change should be backported to the release/1.8.2607 branch label Jul 31, 2026
Steven Malis (smalis-msft) added a commit that referenced this pull request Aug 4, 2026
Backport of #4093 to `release/1.8.2607`.

The cherry-pick of 50f2923 applied cleanly onto `release/1.8.2607`
with no conflicts and no manual edits.

Original PR: #4093

---
*This backport PR was created by an AI agent (GitHub Copilot) on behalf
of @smalis-msft.*
@benhillis

Copy link
Copy Markdown
Member

Backported to release/1.8.2607 in #4129

@benhillis Ben Hillis (benhillis) added the backported_1.8.2607 PR that has been backported to release/1.8.2607 label Aug 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backported_1.8.2607 PR that has been backported to release/1.8.2607

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants