fix(agent): authenticate and select the WTS session by SID

[Vladyslav Nikonov (vnikonov-devolutions)]

The package broker matched the pipe client identity against the request's effective_user by comparing DOMAIN\name strings, and an unqualified name ("alice") ignored the domain entirely. On a host with both MACHINE\alice and DOMAIN\alice, the authenticated caller and the session the operation was launched into could differ, allowing the broker to run the operation in the wrong user's session.

The broker now captures the caller's SID at connect and uses it as the single source of truth for identity: the request's effective_user is resolved to a SID and compared against the caller SID, and the target WTS session is selected by matching the session token's user SID instead of user/domain name strings. Two accounts sharing a display name can no longer be confused.

Issue: DGW-418

[Vladyslav Nikonov (vnikonov-devolutions)]

Implementation notes:

• PipeClient (broker/auth.rs) now keeps the caller SID obtained from the pipe client process token at connect time; domain/name strings are retained only for error messages and logging.
• validate_effective_user resolves the request's effective_user with LookupAccountNameW and compares SIDs. An unresolvable account name is now a hard authentication failure.
• ExecutionContext gained a user_sid: Sid field threaded from the authenticated client through BrokerConnection::execute; effective_user remains for display/logging only.
• find_user_session now enumerates active WTS sessions and matches the session token user SID via WTSQueryUserToken + GetTokenInformation(TokenUser) instead of comparing WTSUserName/WTSDomainName strings. It returns the session token alongside the session ID, removing the second WTSQueryUserToken call. Sessions whose token cannot be queried are skipped with a debug log.
• The redundant PackageBrokerServer impl on BrokerState was folded into inherent methods since execute now takes the extra SID parameter.
• Tests: SID resolution and validation are covered with well-known accounts (NT AUTHORITY\SYSTEM, Everyone) so they run on any Windows host without fixtures.

Addresses #1847 (comment)

[github-actions]

Let maintainers know that an action is required on their side

• Add the label release-required Please cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module) when you request a maintainer to cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module)

• Add the label release-blocker Follow-up is required before cutting a new release if a follow-up is required before cutting a new release

• Add the label publish-required Please publish libraries (`Devolutions.Gateway.Utils`, OpenAPI clients, etc) when you request a maintainer to publish libraries (Devolutions.Gateway.Utils, OpenAPI clients, etc.)

• Add the label publish-blocker Follow-up is required before publishing libraries if a follow-up is required before publishing libraries

[Copilot]

Pull request overview

Updates the Windows package broker to consistently authenticate and select user sessions by SID.

Changes:

• Validates effective_user against the pipe client SID.
• Selects active WTS sessions by token SID.
• Propagates the authenticated SID into execution contexts.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
devolutions-agent/src/broker/server/mod.rs	Passes authenticated SIDs into executions.
devolutions-agent/src/broker/executor/windows/token.rs	Matches sessions by token SID.
devolutions-agent/src/broker/executor/windows/mod.rs	Uses the matched session token.
devolutions-agent/src/broker/executor/mod.rs	Adds SID to execution context and logging.
devolutions-agent/src/broker/auth.rs	Implements SID-based validation and tests.
devolutions-agent/Cargo.toml	Adds widestring dependency.
Cargo.lock	Locks the new direct dependency.

Comments suppressed due to low confidence (2)

devolutions-agent/src/broker/auth.rs:226

• The unqualified SYSTEM display name is localized as well, so this assertion is host-language dependent. Obtain the localized name from the well-known SID first; that preserves the intended unqualified-name coverage on every Windows locale.

    fn resolve_account_sid_resolves_unqualified_name() {
        let sid = resolve_account_sid("SYSTEM").expect("SYSTEM account should resolve");

devolutions-agent/src/broker/auth.rs:243

• Everyone is an English localized display name, so on localized Windows this lookup fails rather than testing the intended different-SID comparison. Build the world SID and use its host-localized account name instead.

    fn validate_effective_user_rejects_different_account() {
        // "Everyone" resolves to a different SID than the SYSTEM caller.
        assert!(system_client().validate_effective_user("Everyone").is_err());

---

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

[Vladyslav Nikonov (vnikonov-devolutions)]

Fixed in 733cbf0: the tests now resolve the well-known SIDs (LocalSystem, World) back to the host-localized qualified names via LookupAccountSidW before exercising the name-to-SID path, so they no longer depend on English display names.