[Richard Markiewicz (thenextman)]

Support elevated NOW execution through Windows UAC for Run, Process, Batch, Windows PowerShell, and PowerShell 7 using now-proto-pdu 0.5.0.

Negotiate elevation support, reject requests that cannot be elevated without falling back to standard execution, report denied consent, and signal unavailable stdio redirection. Track attached elevated process exit codes and retain script files while their processes run.

[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

[Richard Markiewicz (thenextman)]

Draft for review; Windows runtime validation remains outstanding. The Windows-target library and tests compile with cargo check --locked -p devolutions-session --tests --target x86_64-pc-windows-gnu; this does not execute the tests or exercise UAC consent.

Changed lines (additions / deletions):

Category	Lines
Production code	+368 / -60
Tests	+33 / -5
Other files (manifest and lockfile)	+22 / -37

Runtime review should cover consent granted/denied, channel loss or renegotiation during consent, elevated cancellation/abort failures, script lifetime, exit codes, and STARTED_NO_IO_REDIRECTION across supported execution styles. Elevated processes are not assigned to the channel job and may outlive their channel.

The touched Run path retains an existing UNTESTED property: the directory buffer must remain alive through ShellExecuteW because consuming it would leave a dangling pointer; successful launches cannot establish pointer validity.

Note

LLM-assisted content (no human feedback).

[Copilot]

🟡 Changes recommended

Stale worker notifications after capability renegotiation can remove newly reused sessions and report the wrong execution’s outcome.

4 open issues

• Filter stale worker events after capability renegotiation · New
• Wait for elevated process after notification send failure · New
• Propagate elevated cancellation posting failures · New
• Map elevation failures to portable status codes · New

What changed in this PR

Adds Windows UAC elevation support to NOW execution in the session host, using now-proto-pdu 0.5.0.

Changes:

• Negotiates elevation for Run, Process, Batch, Windows PowerShell, and PowerShell 7 requests.
• Adds asynchronous elevated launches, consent-denial reporting, and unavailable-stdio signaling.
• Tracks attached elevated process exit codes and retains temporary scripts during execution.

File	Description
devolutions-session/​src/​dvc/​task.rs	Negotiates elevation and routes elevated execution requests.
devolutions-session/​src/​dvc/​process.rs	Implements elevated launches, notifications, and process lifetime handling.
devolutions-session/​src/​dvc/​now_message_dissector.rs	Adapts decode-error handling to the updated dependency API.
devolutions-session/​Cargo.toml	Updates now-proto-pdu to 0.5.0.
crates/​win-api-wrappers/​src/​process.rs	Adds process-ID lookup and adjusts shell execution support.
Cargo.lock	Locks the protocol upgrade and consolidates IronRDP dependencies.

_{Review effort: Balanced}

[Richard Markiewicz (thenextman)]

Addressed all four Copilot findings in 59bda9a and requested another review.

Each negotiation now has its own notification channel, elevated process cleanup retains scripts until exit, cancellation errors are reported without ending process monitoring, and known elevation failures use portable NOW statuses while ordinary launch failures retain platform errors.

Current PR line counts: production Rust +415/-91; tests +217/-6; manifests and lockfile +22/-37.

Windows-target Clippy with warnings denied, formatting, and oracle-marker checks pass locally. The new Windows regression tests are compiled but have not run locally; CI is running. Interactive UAC consent, credential prompts, and real cross-integrity cancellation still need Windows runtime verification; the lifecycle tests use ordinary Windows child processes to exercise cleanup and cancellation-failure handling without prompting for elevation.

Note

LLM-assisted content (no human feedback).

[Copilot]

🟡 Changes recommended

An aborted UAC launch can later report failure against a reused session ID, removing the replacement session and attributing the old launch’s error to it.

2 open issues

• Stale launch failures can remove a replacement session · New
• Cloning Handle duplicates ownership without duplicating the OS handle · New

1 previously missed issue

In code that hasn't changed since last review

Detached batch requests incorrectly use /C instead of /K

devolutions-session/​src/​dvc/​task.rs:1110

For a non-elevated batch request with NO_EXIT | IO_REDIRECTION | DETACHED, this selects /C even though detached execution never enables redirection, so the console closes despite NO_EXIT. NOW-spec §NOW_EXEC_BATCH_MSG suppresses NO_EXIT only when the session actually redirects stdio. Exclude detached requests from this redirection argument so they select /K.

4 resolved since last review

• Filter stale worker events after capability renegotiation
• Map elevation failures to portable status codes
• Propagate elevated cancellation posting failures
• Wait for elevated process after notification send failure

_{Review effort: Balanced}

[Richard Markiewicz (thenextman)]

The next review confirmed the original four fixes and found two additional issues plus a detached Batch flag interaction; all three are addressed in 4ab831c.

Attached executions now invalidate queued and late notifications when dropped, protecting IDs reused after abort within the same negotiation. Both lifecycle tests now duplicate OS process handles with try_clone(), and detached Batch requests honor NO_EXIT even when IO_REDIRECTION is also set. Regression coverage was added for stale notifications and the Batch combination.

Windows-target Clippy and oracle-marker checks pass locally; Windows runtime tests and another Copilot review are pending.

Note

LLM-assisted content (no human feedback).

[Copilot]

🔵 Needs a closer look

Elevated Run and detached Process launches unnecessarily retain worker threads and process handles until exit, causing resource accumulation; cleanup waits should be limited to launches with temporary files.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Elevated launches leak waiting threads when no temp files need cleanup

devolutions-session/​src/​dvc/​process.rs:184

Every elevated Run and detached Process launch retains its worker thread and process handle until the application exits, even when there are no temporary scripts to protect. Repeatedly opening long-lived applications therefore accumulates waiting OS threads after launch success has already been reported. Restrict this cleanup wait to contexts with temporary files; attached executions already wait for exit in their monitoring loop.

2 resolved since last review

• Stale launch failures can remove a replacement session
• Cloning Handle duplicates ownership without duplicating the OS handle

_{Review effort: Balanced}

[Richard Markiewicz (thenextman)]

Copilot confirmed the two latest fixes on 4ab831c and reported one additional resource-lifetime issue: elevated fire-and-forget launches without scripts retained a waiting thread until process exit.

Cleanup now waits only when temporary scripts need protection; attached executions still monitor exit normally. Added a regression test that requires script-free cleanup to finish while its child remains running. Windows-target Clippy and oracle-marker checks pass locally; Windows runtime execution remains with CI.

Note

LLM-assisted content (no human feedback).

[Copilot]

🔵 Needs a closer look

Human assessment is needed because aborting a detached elevation during UAC consent can leave its session ID reserved and allow a later result, conflicting with the protocol’s immediate-abort semantics.

No open issues 🎉

_{Review effort: Balanced}

[Richard Markiewicz (thenextman)]

The review on 67f578b reported “No open issues” but flagged pending detached elevation aborts for human assessment. NOW_EXEC_ABORT_MSG says the session ends immediately and its ID can be reused; Elevated Execution says abort cannot dismiss pending consent. Fixed in a620fba by releasing the pending ID and invalidating that launch’s notifications, while leaving any eventual process cleanup intact.

Added a regression test for abort, ID reuse, and suppression of the old launch result. Windows-target Clippy and oracle-marker checks pass locally; CI and the next review are pending.

Note

LLM-assisted content (no human feedback).

[Copilot]

🔵 Needs a closer look

Human assessment is needed because cancellation requests during UAC consent can block the shared dispatcher, stalling all sessions and preventing shutdown until consent resolves.

No open issues 🎉

_{Review effort: Balanced}

[Richard Markiewicz (thenextman)]

The review on a620fba reported “No open issues” but correctly identified that repeated control requests during pending UAC consent can fill the worker queue and block the shared dispatcher.

Control enqueueing is now nonblocking for cancel, abort, and shutdown. An enqueue failure produces a cancel error response without closing the channel; abort still invalidates the session immediately. A regression test fills the control queue and requires all three operations to finish without waiting for consent.

Windows-target Clippy and oracle-marker checks pass locally; CI and the next review are pending.

Note

LLM-assisted content (no human feedback).

[Copilot]

🟡 Changes recommended

A full control queue can prevent abort delivery while the session is still removed, leaving a non-elevated child running without an addressable session ID.

1 open issue

• Full control queue can orphan a running child after session removal · New

_{Review effort: Balanced}

[Copilot]

🔵 Needs a closer look

The UAC launch paths need interactive Windows validation of consent and credential prompts, denial, and elevated process control because the added tests simulate elevation rather than exercise ShellExecuteExW.

No open issues 🎉

1 resolved since last review

• Full control queue can orphan a running child after session removal

_{Review effort: Balanced}

[Richard Markiewicz (thenextman)]

Copilot’s final verdict for 31316ab:

🔵 Needs a closer look

The UAC launch paths need interactive Windows validation of consent and credential prompts, denial, and elevated process control because the added tests simulate elevation rather than exercise ShellExecuteExW.

No open issues 🎉

All review threads are resolved. No further code findings were accepted from this round. Interactive Windows UAC verification remains outstanding, and this PR stays draft. CI has no reported failures so far, but Windows tests and several build jobs are still running.

Final PR line counts: production Rust +539/-103; tests +406/-7; manifest and lockfile +22/-37.

Note

LLM-assisted content (no human feedback).

[Richard Markiewicz (thenextman)]

Fixed the rubber-duck review finding in 9d8fd37 by adding SEE_MASK_FLAG_NO_UI to ShellExecuteExW. Microsoft documents that this suppresses ordinary error dialogs but exempts security prompts, so UAC consent and credential prompts remain enabled: https://learn.microsoft.com/en-us/windows/win32/api/shellapi/ns-shellapi-shellexecuteinfow#members

UNTESTED: A missing executable must return a launch error without modal interaction, while a valid runas launch must still show UAC when required. This is marked at the flags and needs an interactive Windows desktop to verify; no flag-only unit test would establish the behavior.

This update changes production code/comments only (+6/-2), with no test or other-file changes. Windows-target Clippy and formatting pass locally.

Note

LLM-assisted content (no human feedback).

[Copilot]

🔵 Needs a closer look

The new UAC launch path requires interactive Windows validation of consent/denial, launch-error dialog suppression, and elevated exit tracking because the added tests only simulate elevation.

No open issues 🎉

_{Review effort: Balanced}

[Richard Markiewicz (thenextman)]

Fixed two pre-existing PowerShell defects found during live NOW testing:

• Temporary scripts now use -File in Windows PowerShell and pwsh so explicit exits such as exit 17 retain their actual code instead of being normalized to 1 by -Command <script path>.
• pwsh streams now pass through unchanged, as the NOW UTF-8 contract requires, rather than being transcoded as OEM data.

The follow-up changes contain 3 added / 13 removed production-code lines, 138 added / 2 removed test lines, and 2 added / 7 removed documentation lines; no dependency or tooling changes. Five protocol-loop regressions cover explicit exit codes, terminating script errors, and pwsh UTF-8 stdin/stdout/stderr.

The old installed Agent still reproduces both interpreter exit-code failures. The live Unicode and elevated cancellation investigation is ongoing; no new Agent binary has been installed, so these changes are not yet claimed as live-verified. Native Windows CI: https://github.com/Devolutions/devolutions-gateway/actions/runs/42895279911

Note

LLM-assisted content (no human feedback).

[Copilot]

🔵 Needs a closer look

Interactive Windows validation is still needed to verify real UAC prompts, elevated-process control, and launch-error dialog suppression without suppressing security prompts, since the added tests do not exercise real elevation.

No open issues 🎉

_{Review effort: Balanced}

[Richard Markiewicz (thenextman)]

Fixed-build live verification completed on Windows 11 ARM64 after manual installation of the artifact from CI run 42895279911, commit 6d51ef7. Installed Agent and Session executable hashes match that artifact.

Standard and elevated Windows PowerShell/pwsh preserved explicit exit 17 and returned exit 1 for terminating script errors. UTF-8 lambda/CJK text passed in the supported pwsh Unicode mode; representable OEM text passed in Process transcoding mode. The earlier general Unicode-failure claim came from mismatching UTF-8 child configuration with Process OEM transcoding.

With the corrected test client continuing to drain events, elevated cancellation promptly reported WinAPI 2147942405 (access denied), kept execution tracked, and later reported actual exit 17. This is the documented higher-integrity cancellation failure, not a pending Agent operation. Standard cancellation was acknowledged, but the retained process check occurred after its deadline, so early termination is not proven. The pre-existing now-client acknowledgement-based Cancelled classification remains unchanged; no protocol change was made.

The actual STARTED_NO_IO_REDIRECTION bit remains unverified because published now-client 0.2.0 does not expose it. The IronRDP cancellation-response fix currently requires an unpublished additive now-client API; its dependency delivery is separate from this Agent PR.

Note

LLM-assisted content (no human feedback).