You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The package broker runs elevated operations with the requesting user's administrator token, without a UAC prompt. It now restricts what those operations can select, independently of the package policy:
A request with an explicit user scope, set as a request option or through custom parameters, now runs with the standard token even when elevation is requested, and policy rules match it as a standard operation.
The broker no longer runs requests for a full administrator token without UAC (User Account Control disabled, or the built-in Administrator account): standard operations now fail instead of running with an elevated token.
Operations now run in the session of the requesting client, rather than in any session of the same user.
Kill-before-operation process names without an extension now get .exe appended, so chrome targets chrome.exe. Names with another extension, wildcards, paths, or quotes are rejected. Processes are only terminated in the requester's session, and only the requester's own processes for elevated operations.
Elevated operations now reject a custom install location that non-administrators can write, including through inherited permissions, or that goes through a reparse point. An existing custom install folder must also be in a folder that non-administrators cannot write or pass write access through, so an existing folder directly under a drive root such as C:\App is rejected.
An existing custom install folder stays locked against rename and deletion during an elevated operation, so installers that rename or delete their own install folder (such as rename-and-swap upgrades) can fail there; installs into new folders, for example under Program Files, are not affected.
The broker no longer uninstalls the Devolutions Agent, including through the WinGet ARP identifier of its installed MSI product.
The policy validator's audit mode warning now states that elevated requests run without a UAC prompt while audit mode is enabled.
Audit mode cannot override these restrictions. Sample policy rules show how to protect other critical software from elevated uninstall and replacement.
Elevated broker operations run with the user's administrator token
without a UAC prompt, so the broker now limits what they can select
regardless of the policy:
- Requests with an explicit user scope, typed or passed through
custom parameters, run with the standard token even when elevation
is requested; policy rules match that elevation.
- Kill-before-operation process names without an extension get .exe
appended. Names with another extension, wildcards, paths, or quotes
are rejected. Processes are only terminated in the requester's
session, and only the requester's own processes for elevated
operations.
- Elevated operations reject a custom install location that
non-administrators can write, including through inherited
permissions, or that goes through a reparse point.
- The broker no longer uninstalls the Devolutions Agent.
- The audit mode policy warning now states that elevated requests run
without a UAC prompt while audit mode is enabled.
Audit mode cannot override these restrictions. A sample policy rule
shows how to protect other critical software from elevated uninstall.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Single elevation decision (G1).evaluator::effective_elevation(requested, scope) is the only place that decides the execution token: Machine scope means elevated, User scope means standard, otherwise the requested elevation. A user scope passed only through WinGet/Scoop custom parameters also lowers the elevation; a custom machine scope never raises it. Policy matching, the server pre/post-command gate, ExecutionContext::requires_elevation() in the executor and the command builders all use it, so evaluation and execution always agree. An info log records when an elevated request is lowered.
Pre-policy gates. Unacceptable kill-before-operation names and Agent uninstalls are rejected in evaluate_request with ValidationFailed, next to the existing pre/post-command and install-location gates, so audit mode cannot override them. evaluate() also denies them for direct callers.
Kill-before-operation (G3).normalize_request appends .exe to names without an extension (UniGetUI sends names as typed), so evaluation, responses and the executor see one value. taskkill gets /FI "SESSION eq N", plus /FI "USERNAME eq DOMAIN\user" from the requester's SID for elevated runs, each as separate argv elements. If the account lookup fails, the plan fails instead of running an unfiltered kill. The executor re-validates names as defense in depth.
Install location (G2).policy_security::verify_elevated_install_location reuses the existing owner/DACL checker with a new TrustedWriters::InstallLocation mode: inheritable and inherit-only ACEs count, and CREATOR OWNER is accepted on inherit-only ACEs. The nearest existing folder is opened through the user-impersonating UserEnv (which now also accepts drive roots), must be on a local disk volume, must not be a reparse point and must resolve to itself. Ancestors go through the existing executable ancestor walk. Handles are held until the plan finishes. The folder itself is opened with delete sharing so installers can still replace it; non-administrators can't, because delete rights are part of the rejected mask. The UserEnv is now built once per plan and shared with main-command preparation.
Protected packages (G4).PROTECTED_PACKAGES lists WinGet Devolutions.Agent and Chocolatey devo-agent, compared with Deny-rule identifier semantics (case-insensitive, decorated identifiers). Uninstall, and UninstallPrevious (typed or custom parameter), are blocked. WinGet ARP\... identifiers are not covered and are documented as a limitation; the sample policy shows a Deny rule for other critical software.
Validator (G5). Audit mode allows every request that passes the built-in gates, including elevated ones the rules deny, so the audit-mode warning is now always "audit mode does not enforce decisions; elevated requests run without a UAC prompt while it is enabled". VALIDATOR_VERSION is now /12 because findings are bound into receipts.
Not covered by automated tests, since these need a broker running as SYSTEM with a logged-on user: path resolution of the install location through the elevated session's device map, LookupAccountSid and taskkill USERNAME/SESSION filtering against a real session user, and impersonated opens of drive roots. The ACL logic is covered with SDDL and real-folder tests (inheritable Users ACE rejected, admin-only and Program Files accepted, symlink rejected).
Main-command preparation now reuses the environment captured before the pre-operation hook. A standard hook that persists a new VCPKG_ROOT with setx, or updates the user PATH, is therefore ignored by executable resolution. The operation can select the old manager or fail to find the newly configured one. Previously, prepare_main_command loaded the user environment after the hook. Reload it here before resolving the main command; the earlier snapshot can still serve install-location validation.
The main command now uses the environment snapshot captured before the pre-operation command. If a standard pip request's pre-command updates the persisted user PATH to select another Python installation, prepare_pip_command still resolves python.exe against the old PATH. The later environment reload in create_process cannot correct the already-resolved absolute path, so pip can install into the wrong interpreter or fail to find the new one. Reload the environment here, as the removed prepare_main_command helper did; keep the earlier snapshot for install-location validation.
This guard prevents a Standard policy decision from running with an elevated token, but no executor test reaches this rejection. The new server test uses CapturingExecutor, and the user/session mismatch tests exit before this check. Add deterministic coverage showing that an elevated token rejects a Standard plan before any kill or pre-command runs, while valid token/plan combinations pass this guard. Use a test seam for token elevation if necessary.
[!NOTE]
LLM-assisted content (no human feedback).
Refresh environment after pre-hook before main-command preparation
Main-command preparation now reuses the environment captured before the pre-operation command. A standard pre-hook that updates the user's persisted VCPKG_ROOT or PATH can succeed, but preparation still resolves the executable using the old values and can fail or select the old executable. Previously, this environment was loaded after the hook. Refresh it here; the earlier snapshot can remain for install-location verification.
Addressed the two "previously missed" items from the latest Copilot review in ba02bae:
The elevated-token check is now check_execution_token_elevation, covered by a deterministic test. A standard plan, including an elevated user-scope request, is rejected for an elevated token. An elevated plan passes, but not with pre/post commands. The check runs before any kill or pre-command.
Main-command preparation reloads the user environment after the pre-operation command again, so updated persisted PATH or VCPKG_ROOT values are used. The earlier snapshot still serves install-location verification.
The reason will be displayed to describe this comment to others. Learn more.
Could this account for the manager's effective default scope too? With options.scope unset, policy matching treats PowerShell installs/updates and .NET tools as user-scope, while their command builders emit -Scope CurrentUser or --global. Here the scope stays None, so an elevated request retains the administrator token and can write through the user's profile without the elevated install-location guard. Please derive elevation from the same effective scope used for policy matching and command construction.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 125c56c: execution elevation now reuses the policy scope resolver for implicit user scope, including manager defaults, while retaining typed-scope precedence and preventing implicit machine scope from raising privileges.
Reviewed the implicit user-scope finding and fixed it in 125c56c.
Execution elevation now reuses the scope resolver used by policy matching when typed scope is absent. .NET tools and PowerShell install/update defaults therefore run with the standard token. The same applies to the other user-default managers. Typed scope retains precedence; implicit machine scope cannot raise privileges; unknown scopes, including PowerShell/pip uninstall defaults, retain the requested elevation.
Validation:
The new policy regression failed before the fix and passed afterward; it covers .NET install/update/uninstall and PowerShell 5/7 install/update.
Full broker suite: 590 passed, 2 intentionally ignored helper tests. Seven filesystem-permission failures in the sandbox disappeared when rerun outside it.
Broker and workspace Clippy with --tests -- -D warnings passed; workspace formatting and git diff --check passed.
Full workspace tests could not run: the build reported missing rstest and required .rlib artifacts in unrelated Gateway targets. The earlier sandboxed attempt also hit a Cargo-cache write denial in libsql-sqlite3-parser. No workspace-wide test pass is claimed.
Self-review used the local skeptical-reviewer, code-compressor, rust-style-reviewer, test-reviewer, and test-oracle-reviewer skills. An independent review of the final diff returned: “No actionable findings in the final nine-file diff.” This verdict applies to 125c56cf7de329ace5c4e38311cd00fdd8fb67ec. No dependencies or applicable intent files changed.
Change size: production +4/−9 lines (including comments); tests +80/−24; other files 0. No new UNTESTED: properties were introduced; live SYSTEM-token/package-manager execution was not exercised. The optional registry/environment efficiency suggestions remain outside this correctness fix.
Resolved the merge conflict in eebe70e by merging master at 9806db7.
The sole conflict was adjacent additions in policy_security.rs; both the elevated install-location protection and master's Store app-alias pinning helper were retained verbatim. The implicit user-scope fix from 125c56c remains intact. No additional behavior changes were needed.
Validation: 598 broker tests passed, 2 helper tests ignored; broker formatting, scoped whitespace checks, and strict Clippy passed. Independent review found no merge-conflict issues. GitHub now reports MERGEABLE; its overall merge state remains BLOCKED by other requirements.
Tie regression oracles to reported defects and record measured faults for tests without an independent source. Verify rejection of receipts from the previous validator version without fixing the current version in tests.
Remove four overlapping tests whose sampled faults remain detected by the remaining suite. Drop duplicate validator-version assertions and the unsupported unresolved-product-code expectation.
Fixed and pushed in 448cf41 and 9066763. The earlier implicit-scope fix (125c56c) and master conflict resolution (eebe70e) remain in the branch.
The reported defect is that an elevated request with omitted scope can keep administrator privileges although the manager executes in user scope, bypassing the elevated install-location check. The reference is the actual defect report, not the fact that this PR added tests. There is no Jira ticket. Broad PR citations have been replaced with specific defect reports or explicitly weaker mutant evidence.
Removed four redundant tests: the direct elevation truth table, typed-user npm acceptance, administrator-only ACL acceptance, and the synthetic Program Files ACL fixture. Replays confirm that remaining tests catch every sampled fault these tests caught. Removed three exact-current-validator assertions and the unsupported requirement that an unresolved product-code lookup return false. The receipt test now checks rejection of the previous validator version.
Validation: clean broker suite 594 passed, 2 ignored; formatting and strict Clippy completed successfully (one filesystem warning while finalizing the incremental cache); local oracle marker check passed against the merged base. Two existing private RDM references received syntax-only validation because the token cannot read them. The live agent-policy end-to-end test was not rerun locally. New CI is running; GitHub reports MERGEABLE.
This follow-up changes test code/comments only: +39/-54 lines, production runtime code 0, other files 0. Review used the repository test-pruner, oracle/test reviewer, and Rust/review skills; a separate reviewer caught two overstated oracle claims, now corrected.
Oracle decisions and deletion evidence
Classification
Evidence and disposition
Independent regression
Explicit review reports: implicit user scope r4379344015; actual-token mismatch r4192110367; requester-session mismatch r4192110409; hidden install path r4192245600; root rename r4192245640; protected ARP uninstall r4192380021; parent replacement r4283609524; inherited write rights r4283749914. Existing non-SYSTEM profile and elevated pre/post defects are sourced to PRs #1892 and #1903 respectively. Retained.
No independent source
Do not treat implementation descriptions, version-number choices, or reasons for adding tests as specifications. Retained tests record measured faults with mutant markers; parsing/bounds checks remain protected.
Removed elevation truth table
User-scope positive control and machine-scope faults remain caught by behavior tests.
Removed typed-user npm duplicate
Raw-requested-elevation fault remains caught by implicit-user npm coverage.
Removed administrator ACL fixture
Administrator-owner over-rejection remains caught by inherited-permission coverage.
Removed synthetic Program Files fixture
Creator Owner over-rejection remains caught by the real Program Files path test.
Removed exact version assertions
Changing /12 to /13 is unconstrained; reverting to /11 still fails at stale receipt acceptance.
Removed unresolved ARP assertion
The cited defect permits failing closed; it does not specify false. The protected-product bypass mutant still fails after removal.
Mutation results and limits
The scoped pass took approximately 58 minutes, including repeated clean baselines, source review, challenges and final verification. Mutants ran in a separate clone. Twenty-five domain faults and two cargo-mutants account-name faults were exercised; 25 were caught and two survived. Full broker library tests were used, with identical settings for before/after deletion replays. The following table records failing tests from each fault's latest run; unaffected faults were not rerun after pruning.
Current validator /12 → /13: unconstrained by an independent requirement; exact current-version assertions were intentionally removed.
Removing only the early session-zero rejection: observationally equivalent in this environment because the later token lookup also rejects it. A service-token fallback fault was caught uniquely by the retained session-zero test; this does not prove behavior in every service configuration.
Generic mutation coverage was limited to the account-name helper to stay within the one-hour budget. No exhaustive mutation score or full system behavior is claimed. No production bug was found by this pruning pass.
Note
Human-tuned, LLM-assisted content.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The package broker runs elevated operations with the requesting user's administrator token, without a UAC prompt. It now restricts what those operations can select, independently of the package policy:
.exeappended, sochrometargetschrome.exe. Names with another extension, wildcards, paths, or quotes are rejected. Processes are only terminated in the requester's session, and only the requester's own processes for elevated operations.C:\Appis rejected.Audit mode cannot override these restrictions. Sample policy rules show how to protect other critical software from elevated uninstall and replacement.
Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com