Skip to content

Improve argument handling in ProcessHelper and use read-only store access in CertHelper#5151

Merged
LoopedBard3 merged 1 commit intodotnet:mainfrom
LoopedBard3:GeneralUpdate
Mar 17, 2026
Merged

Improve argument handling in ProcessHelper and use read-only store access in CertHelper#5151
LoopedBard3 merged 1 commit intodotnet:mainfrom
LoopedBard3:GeneralUpdate

Conversation

@LoopedBard3
Copy link
Copy Markdown
Member

Improve argument handling in ProcessHelper and use read-only store access in CertHelper

  • Add -- separator to sudo invocations in RawProcessHelper and ManagedProcessHelper to properly delimit the command from sudo options
  • Change certificate store open from ReadWrite to ReadOnly when only reading certificates in CertHelper

…cess in CertHelper

- Add -- separator to sudo invocations in RawProcessHelper and ManagedProcessHelper to properly delimit the command from sudo options
- Change certificate store open from ReadWrite to ReadOnly when only reading certificates in CertHelper

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
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 two internal tools: it hardens how elevated (sudo) processes are invoked in ScenarioMeasurement, and reduces certificate store permissions in CertHelper.

Changes:

  • Prefix sudo-invoked commands with -- to stop sudo option parsing before the executable/args.
  • Open the CurrentUser My certificate store as read-only when exporting matching certificates.

Reviewed changes

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

File Description
src/tools/ScenarioMeasurement/Util/ProcessHelper.cs Adjusts sudo argument construction to include -- before passing executable/args.
src/tools/CertHelper/Program.cs Switches cert store open flags from read-write to read-only for export-only usage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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

Comment thread src/tools/ScenarioMeasurement/Util/ProcessHelper.cs
Comment thread src/tools/ScenarioMeasurement/Util/ProcessHelper.cs
Copy link
Copy Markdown
Member

@DrewScoggins DrewScoggins left a comment

Choose a reason for hiding this comment

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

LGTM

@LoopedBard3 LoopedBard3 enabled auto-merge (squash) March 17, 2026 16:53
@LoopedBard3 LoopedBard3 merged commit cdace9d into dotnet:main Mar 17, 2026
78 checks passed
@LoopedBard3 LoopedBard3 deleted the GeneralUpdate branch March 17, 2026 16:53
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.

4 participants