Stop leaving Appx sideloading policy permanently enabled - #5
Merged
Merged
Conversation
Installing any MSIX through the module wrote AllowAllTrustedApps=1 to both HKLM\SOFTWARE\Policies\Microsoft\Windows\Appx and AppModelUnlock, and never put them back. Every machine that ever installed an MSIX was left with sideloading permanently enabled, including machines where the policy had previously been explicitly disabled. It also did this unconditionally, before finding out whether it was needed. A correctly signed package usually provisions with no policy change at all. Provisioning is now attempted as the machine is configured. Only if that is refused is the policy relaxed, and it is restored in a finally block that runs on the success path, the per-user fallback path, and the throw. Restoration is exact rather than "set it back to 0": the prior state of each value is recorded first, and the restore puts back the previous value if there was one, removes just the value if the key existed without it, or removes the key entirely if we created it. A pre-existing AllowAllTrustedApps=0 therefore comes back as 0, not as a deleted value. Push-SideloadPolicy deliberately never throws. If it did, the state describing what had already been changed would be lost with it, and the caller's finally block would have nothing to restore from, leaving the policy relaxed. Failures are reported through the returned object instead. Pop-SideloadPolicy indexes its list directly rather than wrapping it in @(). On PowerShell 7.6 / .NET 10, @() over a List[object] throws "Argument types do not match", which aborted the restore and left the policy relaxed. That was caught by the round-trip test below and is the exact failure this change exists to prevent. Verified against a scratch HKCU key across all three prior states: key absent, key present without the value, and key present with AllowAllTrustedApps=0. All three read 1 while relaxed and are byte-identical to their original state afterwards, with an unrelated value in the same key left untouched.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 problem
Installing any MSIX through the module wrote
AllowAllTrustedApps=1to bothHKLM\SOFTWARE\Policies\Microsoft\Windows\AppxandAppModelUnlock, and never put them back.Every machine that has installed an MSIX through TecharyGet is left with sideloading permanently enabled, including machines where that policy had been explicitly set to disabled, which the old code silently overwrote. It also did this unconditionally, before establishing whether it was needed — a correctly signed package usually provisions with no policy change at all.
What changed
finally, covering the success path, the per-user fallback and the throw.Restoration is exact rather than "set it back to 0". The prior state of each value is recorded first, then put back:
AllowAllTrustedApps=00, not deletedAn unrelated value in the same key is left untouched.
Two design points
Push-SideloadPolicynever throws. If it did, the state describing what it had already changed would be lost with it, the caller'sfinallywould have nothing to restore from, and the policy would stay relaxed. Failures are returned through the result object instead.Pop-SideloadPolicyindexes its list rather than wrapping it in@(). On PowerShell 7.6 / .NET 10,@()over aList[object]throwsArgument types do not match, which aborted the restore mid-way and left the policy relaxed. Caught by the round-trip test below.List[string]is unaffected; this is specific toList[object].Verification
Round trip against a scratch
HKCUkey covering all three prior states:The registry paths are parameters so the round trip is testable without modifying live machine policy.
Coverage limitation: this fixes the leak going forward. Machines already left in the relaxed state by previous runs are not cleaned up by this change.