[JanProvaznik]

Context

Follow-up to an analysis of whether the MSBuild server mutexes are redundant with its named pipe. They are not, but the analysis surfaced a separate scoping bug in how the server's names are derived.

Problem

Named pipes and named mutexes live in machine-wide namespaces, while MSBuild creates its pipes current-user-only (PipeOptions.CurrentUserOnly). MSBuild server derives every such name from ServerNodeHandshake.ComputeHash(), which is built only from handshake options, salt and file version — ServerNodeHandshake passes includeSessionId: false — so all users on a machine computed identical names.

Two users sharing a machine (Remote Desktop, shared CI agent) therefore collide:

1. User B sees user A's Global\msbuild-server-running-{hash} mutex.
2. B concludes a server is already running and skips launching its own.
3. B cannot connect to A's pipe, because it is restricted to A.
4. B cannot create a pipe under that name either, because A holds it.

User B is locked out of MSBuild server entirely by user A. This matters more now that -mt implicitly opts into the server.

Fix

Mix the current user into ServerNodeHandshake.ComputeHash(), via a new GetKeyWithUserName(). That single change covers the server pipe name, the running/busy/launch mutexes, and the RAR node pipe names, since all of them derive from that hash.

Not a protocol change

ComputeHash() is used only to generate names. The handshake actually exchanged over the wire comes from RetrieveHandshakeComponents(), which is untouched, so this does not affect handshake compatibility with other MSBuild versions. A test pins that invariant.

Cross-user access remains enforced by the OS as before (pipe DACL on Windows, GetPeerID check on Unix); this change prevents the denial-of-service, it is not the security boundary.

Coordinator: no behavior change

The coordinator already scopes itself per user through its default pipe name, and its mutex names are derived from that pipe name, so it is already correct. It only moves to the new cached accessor. GetMutexName stays a pure function of PipeName — an explicit MSBUILDCOORDINATORPIPENAME override is honored as given, and that intent is now documented.

EnvironmentUtilities.CurrentUserName

A single cached accessor for both call sites. Caching is worth it by a wide margin — measured locally:

raw= 75078.50 ns/op   cached=0.62 ns/op
ReferenceEquals across calls: False

Environment.UserName queries the OS and allocates a fresh string on every call (~75us on Windows via GetUserNameExW), with no runtime-side caching. Notably this was measured on a machine that is not domain-joined, so it is not an AD round-trip.

Validation

• Full build.cmd: 0 warnings, 0 errors.
• New ServerNodeHandshake_Tests: 4/4 pass.
• MSBuild.Coordinator.UnitTests: 79/79 pass.
• AppHostSupport_Tests + UnixNodeReuseFixes_Tests (handshake key regression): pass.
• RAR node tests: 8/8 pass.
• MSBuildServer_Tests: 17 failed / 4 passed — identical to the unmodified baseline on this machine (verified by stashing, rebuilding and re-running). These failures are pre-existing and environmental; the server cannot start in this sandbox.

Note for reviewers

Environment.UserName returns the bare account name with no domain, so two accounts with the same short name from different domains would still collide. Using the user SID would match the pipe DACL exactly; Environment.UserName was chosen for cross-platform simplicity. Happy to switch if preferred.

[Copilot]

Pull request overview

This PR fixes a cross-user contention bug in MSBuild Server by ensuring that machine-wide resource names (named pipes and mutexes) derived from ServerNodeHandshake.ComputeHash() are scoped to the current user, preventing one user’s server instance from locking out other users on the same machine. It also introduces a cached EnvironmentUtilities.CurrentUserName accessor and adds unit tests to pin key invariants around name derivation.

Changes:

• Add EnvironmentUtilities.CurrentUserName as a cached username accessor and use it for coordinator default pipe naming.
• Update ServerNodeHandshake.ComputeHash() to incorporate the current user into the hashed key via GetKeyWithUserName().
• Add unit tests covering coordinator mutex naming behavior and handshake hashing/name-derivation invariants.

Reviewed changes

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

Show a summary per file

File	Description
src/MSBuild.Coordinator.UnitTests/CoordinatorSettings_Tests.cs	Adds tests validating coordinator mutex name derivation and scoping.
src/Framework/EnvironmentUtilities.cs	Introduces cached CurrentUserName accessor for performance and reuse.
src/Framework/Coordinator/CoordinatorSettings.cs	Switches default pipe name to use cached current-user accessor.
src/Framework/BackEnd/ServerNodeHandshake.cs	Scopes server-related derived resource names per user by hashing a user-including key.
src/Build.UnitTests/BackEnd/ServerNodeHandshake_Tests.cs	Adds tests asserting user scoping and stability/legality of derived names.

[AR-May]

LGTM