Skip to content

injection fix using shell=false#45740

Open
ayushhgarg-work wants to merge 9 commits intoAzure:mainfrom
ayushhgarg-work:ayushhgarg/macos3
Open

injection fix using shell=false#45740
ayushhgarg-work wants to merge 9 commits intoAzure:mainfrom
ayushhgarg-work:ayushhgarg/macos3

Conversation

@ayushhgarg-work
Copy link
Member

@ayushhgarg-work ayushhgarg-work commented Mar 17, 2026

Description

What

Fix command injection vulnerability (CWE-94) in commandline_utility.py by removing
unsafe shell=True + string-join pattern.

Related: MSRC Case 106104 (Moderate - Remote Code Execution)

Why

The run_cli_command function previously joined command arguments into a single string
via " ".join(cmd_arguments) and executed it with subprocess.check_output(..., shell=True).
This allowed shell metacharacters in user-controlled input (e.g., scoring_script path
in a deployment YAML) to break out of the command and execute arbitrary code.

Changes

  • Set shell=False and pass cmd_arguments as a list directly to
    subprocess.check_output on all platforms — shell metacharacters are never interpreted
  • Removed the " ".join(cmd_arguments) pattern and the outdated comment
    referencing the old shell=True approach
  • Added unit tests for run_cli_command covering:
    • Correct subprocess argument forwarding (shell, stderr, env)
    • Shell metacharacter injection prevention (;, & payloads matching MSRC attack vectors)
    • CalledProcessError propagation

Testing

  • Verified on Windows that malicious arguments containing &, ;, | are passed as
    literal strings and do not result in command injection
  • All 7 unit tests pass on Windows
image

Copy link
Contributor

Copilot AI left a comment

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 updates the local endpoints CLI invocation helper to mitigate command injection by avoiding shell=True + string-joined commands on non-Windows platforms, and introduces Windows-specific handling for commands that are provided as .cmd/.bat shims.

Changes:

  • Switch non-Windows execution to shell=False with argv list.
  • Add Windows branch that builds a command line string via subprocess.list2cmdline(...) and executes with shell=True.
  • Minor refactor/formatting around command printing and subprocess args construction.

You can also share your feedback on Copilot code review. Take the survey.

@ayushhgarg-work ayushhgarg-work enabled auto-merge (squash) March 18, 2026 11:26
@ayushhgarg-work
Copy link
Member Author

@microsoft-github-policy-service rerun

@ayushhgarg-work
Copy link
Member Author

@microsoft-github-policy-service agree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants