Add CodeQL suppressions for EventLog, NativeCommandProcessor, TypeCatalogGen, Process methods #26467
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds CodeQL suppression comments to document expected security-related behaviors in PowerShell's codebase. The changes clarify that certain security warnings (command-line injection, SSRF, weak cryptography) are intentional design decisions based on PowerShell's trust model where users have full control and responsibility for the code they execute.
Key changes:
- Corrects spelling of "Poweshell" to "PowerShell" in 7 existing CodeQL suppression comments
- Adds 5 new CodeQL suppression comments for command-line injection warnings in process-related code
- Adds 1 new CodeQL suppression comment for SHA-1 weak cryptography usage (backwards compatibility)
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| TypeCatalogGen.cs | Adds weak-crypto suppression for SHA-1 hash algorithm used for backwards compatibility |
| FileSystemProvider.cs | Corrects "Poweshell" to "PowerShell" in command-line-injection-shell-execution suppression |
| UpdatableHelpSystem.cs | Corrects "Poweshell" to "PowerShell" in two SSRF suppressions |
| RunspaceConnectionInfo.cs | Corrects "Poweshell" to "PowerShell" in SSH process command-line-injection suppression |
| NativeCommandProcessor.cs | Adds new command-line-injection suppression and corrects "Poweshell" to "PowerShell" in two existing suppressions |
| WebRequestPSCmdlet.Common.cs | Corrects "Poweshell" to "PowerShell" in SSRF suppression |
| Process.cs | Adds two new command-line-injection suppressions for working directory paths and corrects "Poweshell" to "PowerShell" in existing suppression |
| Eventlog.cs | Adds new command-line-injection suppression for ComputerName parameter in event viewer launch |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // codeql[cs/microsoft/command-line-injection-shell-execution] - This is expected Poweshell behavior where user inputted paths are supported for the context of this method. The user assumes trust for the file path specified, so any file executed in the runspace would be in the user's local system/process or a system they have access to in which case restricted remoting security guidelines should be used. | ||
| ProcessStartInfo startInfo = new(filePath); | ||
| // codeql[cs/microsoft/command-line-injection-shell-execution] - This is expected PowerShell behavior where user inputted paths are supported for the context of this method. The user assumes trust for the file path specified, so any file executed in the runspace would be in the user's local system/process or a system they have access to in which case restricted remoting security guidelines should be used. | ||
| System.Diagnostics.ProcessStartInfo startInfo = new System.Diagnostics.ProcessStartInfo(filePath); |
There was a problem hiding this comment.
The change from ProcessStartInfo to System.Diagnostics.ProcessStartInfo is inconsistent with the existing code style in this file and adds unnecessary verbosity. The ProcessStartInfo type is already imported via using System.Diagnostics; at the top of the file, so the fully qualified name is redundant. This change should be reverted to maintain consistency with the rest of the codebase.
| System.Diagnostics.ProcessStartInfo startInfo = new System.Diagnostics.ProcessStartInfo(filePath); | |
| ProcessStartInfo startInfo = new ProcessStartInfo(filePath); |
Add CodeQL suppression comments to clarify that certain potential security issues (such as command-line injection and weak cryptography) are expected behaviors in PowerShell, given its design and user trust model. These comments provide context for static analysis tools and future maintainers. No functional code changes are included. Also fixed typo and updated prior CodeQL suppressions.
These changes do not affect runtime behavior but improve code documentation and static analysis compliance.
PR Summary
PR Context
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header