COM server security improvements

[JohnMcPMS]

📖 Description

Cherry pick #6568 to main.

Microsoft Reviewers: Open in CodeFlow

[Copilot]

Copilot review overview

🟡 Changes recommended

Named-object pre-creation vulnerabilities and several build and test reliability defects remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 4 · 3 · 1

Open (8)

• Test-hook flag reference breaks disabled-test-hook builds · New
• Win32 security-check failures incorrectly fail open · New
• Mutex security ignored when attacker pre-creates the object · New
• Event security ignored when attacker pre-creates the object · New
• Missing dependency on WinGetServer MIDL project · New
• Server process leaks when readiness wait fails · New
• RPC test always uses development CLSID · New
• Document and configure new E2E run parameters · New

What changed in this PR

Cherry-picks COM server security hardening and accompanying E2E coverage.

Changes:

• Migrates manual activation to authenticated ncalrpc.
• Adds per-user synchronization security and RPC management denial.
• Adds an RPC security test helper and explicit E2E tests.

File	Description
src/​WinGetServer/​WinMain.cpp	Hardens RPC server registration and lifetime locking.
src/​WinGetServer/​WinGetServerManualActivation_Client.h	Adds a test-only launch suppression flag.
src/​WinGetServer/​WinGetServerManualActivation_Client.cpp	Configures authenticated RPC client bindings.
src/​WinGetServer/​Utils.h	Declares security and synchronization helpers.
src/​WinGetServer/​Utils.cpp	Implements secured per-user objects and SID utilities.
src/​WinGetRpcTestHelper/​WinGetRpcTestHelper.vcxproj	Defines the native security test helper.
src/​WinGetRpcTestHelper/​WinGetRpcTestHelper.cpp	Implements RPC and object-security probes.
src/​WinGetRpcTestHelper/​packages.config	Adds the WIL dependency.
src/​AppInstallerCLIE2ETests/​RpcSecurityTests.cs	Adds explicit RPC security E2E tests.
src/​AppInstallerCLIE2ETests/​Helpers/​TestSetup.cs	Adds executable path parameters.
src/​AppInstallerCLIE2ETests/​Constants.cs	Defines the new parameter names.
src/​AppInstallerCLI.sln	Registers the helper project.
.github/​actions/​spelling/​expect.txt	Adds expected technical terms.

---

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

[Copilot]

Copilot review overview

🟡 Changes recommended

The renamed public readiness event breaks the stated compatibility with previously shipped clients.

Review effort: Balanced
Findings: 1

Open (1)

• Preserve legacy public event name for client compatibility · New

Resolved since last review (8)

• Event security ignored when attacker pre-creates the object
• Mutex security ignored when attacker pre-creates the object
• Win32 security-check failures incorrectly fail open
• Test-hook flag reference breaks disabled-test-hook builds
• RPC test always uses development CLSID
• Server process leaks when readiness wait fails
• Missing dependency on WinGetServer MIDL project
• Document and configure new E2E run parameters

[ranm-msft]

I walked the security path here rather than the diff shape. The invariant I was checking is that manual activation should only succeed between the same user at high integrity and the genuine server, and that a medium-integrity process running as the same user should not be able to squat an endpoint-adjacent named object or convince the client to accept a fake server.

I did not find a bypass at the head. The endpoint and interface descriptors both carry the restriction, the client requires mutual authentication against a server security descriptor rather than trusting the binding, the synchronization objects that actually gate activation moved into a high-integrity current-user private namespace, and the descriptor is validated on open instead of being assumed. The pre-creation concerns raised on the release branch version look materially answered by that pairing, and denying the management interface is a sensible narrowing.

I am not going to treat my read as sufficient to approve this one. It is security boundary code, and I would want the owner's approval recorded against this exact head rather than an earlier commit.

The one item I would not want to lose is the existing thread about the public readiness event name no longer matching what shipped clients wait on. That reads to me as a compatibility and timeout risk rather than an authentication one, but it deserves a deliberate answer before merge rather than being carried along.