Fix MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY leaking to child processes (#5987)#6619
Conversation
…sses (#5987) The Windows App SDK auto-initializer sets the process environment variable MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY (a requirement for PublishSingleFile SxS manifest redirection). Because it is a process environment variable, it is inherited by child processes spawned via CreateProcess: a child's MRTCore then resolved the parent's resources.pri, and reg-free WinRT redirection could be steered by a parent-controlled directory. Stamp the owning process id in MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID and honor the base directory only when the stamp matches the current process: - MRM.cpp and catalog.cpp ignore the base directory unless it was stamped by the current process, so inherited (foreign) values fall back to the module directory. A parent cannot forge a stamp equal to the not-yet-known child pid, so this also prevents steering where an elevated child reads data files from. - Move the base-directory setter out of the URFW auto-initializer into a new, clearly-named WindowsAppRuntimeBaseDirectoryAutoInitializer; the URFW file is now runtime-load only. This addresses the filename confusion raised on the issue. Add MrtCore EnvironmentVariableIsolationTests: an in-process PID-guard test and a real cross-process test proving a spawned child ignores the inherited base directory. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dac4165f-ec13-4406-9206-60cd55b24c2b
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 2 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. |
|
but.. but... environment variables are for communicating things to other processes. do we literally not have a better way to make two DLLs loaded into the same address space communicate a single piece of information? now we have to work around inheritance, whereas if we didn't use an inheritable primitive we wouldn't have to work around inheritance... |
unfortunately we do not. this all stems from the need to dynamically assign the location of host dlls for activatableClass entries in fusion manifests. that location is only known at runtime in some scenarios (such as PublilshSingleFile), not statically at build time (manifest generation/modification time). the only mechanism I could find to support that dynamism was env var substitution. that said, the PID check here should be sufficient to protect against accidental/malicious misuse of the env var. |
…hild-process isolation (#5987) (#6632) * [2.0-stable] Gated port: MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY child-process isolation (#5987) Ports the base-directory PID-guard fix (main #6619) into release/2.0-stable, gated behind a runtime-compatibility containment change (WINAPPSDK_CHANGEID_63098302) so apps can revert. MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY is set by the Windows App SDK auto-initializer (a requirement for PublishSingleFile SxS manifest redirection). Because it is a process environment variable it is inherited by child processes spawned via CreateProcess, so a child's MRTCore resolved the parent's resources.pri and reg-free WinRT redirection could be steered by a parent-controlled directory. The setter now also stamps MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID with the owning process id. Gated behind WINAPPSDK_CHANGEID_63098302 (ContainmentV2 form): - Enabled (default): MRM.cpp and catalog.cpp honor the base directory only when the stamp matches the current process; inherited (foreign) values fall back to the module directory. - Disabled (via RuntimeCompatibilityOptions): the base directory is honored regardless of the stamp (byte-for-byte prior behavior). Changes: - UndockedRegFreeWinRT-AutoInitializer.cs: stamp the owning process id (ungated; consumers ignore it when the change is disabled). - MRM.cpp: add WINAPPSDK_CHANGEID_63098302 and gate the base-directory PID guard. - catalog.cpp: gate the %MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY% strip on the change (WindowsAppRuntime_DLL already links the FrameworkUdk containment worker). - MrmTests.cpp: EnvironmentVariableIsolationTests (enabled: in-process guard + real cross-process insulation) and BaseDirectoryContainmentDisabledTest (IsolationLevel=Method; disables 63098302 and verifies the inherited base directory is honored). Verified locally (x64 Debug): MrmUnitTest 25/26 (1 skipped child-only worker), both enabled and disabled cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dac4165f-ec13-4406-9206-60cd55b24c2b * catalog.cpp: only consult containment when the base-dir token is present (#5987) RCA of a PR-validation regression in CompatibilityTests (CanSetRuntimeCompatibilityOptions and VerifyDisabledChanges failing intermittently with E_ILLEGAL_STATE_CHANGE, "Configuration already set or locked"): WinAppSdk::Containment::IsChangeEnabled<>() locks the process's runtime-compatibility configuration on first call. catalog.cpp's WinRTLoadComponentFromString runs during reg-free WinRT activation - before an app (or test) can call RuntimeCompatibilityOptions.Apply(). Calling IsChangeEnabled unconditionally there locked the configuration early, so a subsequent Apply() failed. The manifest only references %MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY% for PublishSingleFile apps, so only consult the containment change when that token is actually present. Non-single-file activation (the common case, including CompatibilityTests) no longer calls IsChangeEnabled and no longer locks the configuration. MRTCore MrmUnitTest is unaffected (catalog.cpp is not in that binary). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dac4165f-ec13-4406-9206-60cd55b24c2b --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…hild-process isolation (#5987) (#6631) * [1.8-stable] Gated port: MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY child-process isolation (#5987) Ports the base-directory PID-guard fix (main #6619) into release/1.8-stable, gated behind a runtime-compatibility containment change (WINAPPSDK_CHANGEID_63098344) so apps can revert. MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY is set by the Windows App SDK auto-initializer (a requirement for PublishSingleFile SxS manifest redirection). Because it is a process environment variable it is inherited by child processes spawned via CreateProcess, so a child's MRTCore resolved the parent's resources.pri and reg-free WinRT redirection could be steered by a parent-controlled directory. The setter now also stamps MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID with the owning process id. Gated behind WINAPPSDK_CHANGEID_63098344 (2-arg ContainmentV1 form, target WinAppSDK_1_8_11): - Enabled (default): MRM.cpp and catalog.cpp honor the base directory only when the stamp matches the current process; inherited (foreign) values fall back to the module directory. - Disabled (via RuntimeCompatibilityOptions): the base directory is honored regardless of the stamp (byte-for-byte prior behavior). Changes: - UndockedRegFreeWinRT-AutoInitializer.cs: stamp the owning process id (ungated; consumers ignore it when the change is disabled). - MRM.cpp: add WINAPPSDK_CHANGEID_63098344 and gate the base-directory PID guard. - catalog.cpp: gate the %MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY% strip on the change (WindowsAppRuntime_DLL already links the FrameworkUdk containment worker). - MrmTests.cpp: EnvironmentVariableIsolationTests (enabled: in-process guard + real cross-process insulation) and BaseDirectoryContainmentDisabledTest (IsolationLevel=Method; disables 63098344 and verifies the inherited base directory is honored). Verified locally (x64 Debug): MrmUnitTest 25/26 (1 skipped child-only worker), both enabled and disabled cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dac4165f-ec13-4406-9206-60cd55b24c2b * catalog.cpp: only consult containment when the base-dir token is present (#5987) RCA of a PR-validation regression in CompatibilityTests (CanSetRuntimeCompatibilityOptions and VerifyDisabledChanges failing intermittently with E_ILLEGAL_STATE_CHANGE, "Configuration already set or locked"): WinAppSdk::Containment::IsChangeEnabled<>() locks the process's runtime-compatibility configuration on first call. catalog.cpp's WinRTLoadComponentFromString runs during reg-free WinRT activation - before an app (or test) can call RuntimeCompatibilityOptions.Apply(). Calling IsChangeEnabled unconditionally there locked the configuration early, so a subsequent Apply() failed. The manifest only references %MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY% for PublishSingleFile apps, so only consult the containment change when that token is actually present. Non-single-file activation (the common case, including CompatibilityTests) no longer calls IsChangeEnabled and no longer locks the configuration. MRTCore MrmUnitTest is unaffected (catalog.cpp is not in that binary). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dac4165f-ec13-4406-9206-60cd55b24c2b --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes #5987.
Problem
MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORYis set by the Windows App SDK auto-initializer (a requirement for PublishSingleFile SxS manifest redirection). Because it is a process environment variable, it is inherited by child processes spawned viaCreateProcess: a child Windows App SDK app's MRTCore then resolved the parent'sresources.pri, and reg-free WinRT redirection could be steered by a parent-controlled directory. As noted on the issue, this also lets a user-controlled environment variable influence where an elevated process reads a data file from.Fix — PID stamp
The setter now also stamps
MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PIDwith the owning process id. MRTCore (MRM.cpp) and reg-free WinRT (catalog.cpp) honor the base directory only when the stamp matches the current process; inherited (foreign) or absent stamps are ignored and resolution falls back to the module (exe) directory. A parent cannot forge a stamp equal to the not-yet-known child pid, so this also closes the elevation-steering concern.Refactor — clearer naming
Per feedback on the issue thread, the base-directory logic is moved out of
UndockedRegFreeWinRT-AutoInitializer.cs(which is now runtime-load only) into a new, clearly-namedWindowsAppRuntimeBaseDirectoryAutoInitializer. Behavior is unchanged — it runs under the same self-contained trigger. Only the C# path sets the base directory (PublishSingleFile is .NET-only); the native URFW auto-initializer never set it, so there is no C++ change.Tests
New MrtCore
EnvironmentVariableIsolationTests:GetFilePath_HonorsBaseDirectoryOnlyWhenStampedByCurrentProcess— in-process: base directory is honored when the stamp matches, ignored when the stamp is foreign or absent.ChildProcessIsInsulatedFromInheritedBaseDirectory— spawns a real child process that inherits the environment and verifies it resolves resources under its own directory, not the inherited one.Validated locally (VS x64 Debug):
MRM.vcxprojandMrmUnitTest.vcxprojbuild clean (0 warnings / 0 errors); the new tests pass; existingBasicTestsuite is 21/21 (no regression).Servicing
The same issue is present in
release/1.8-stableandrelease/2.0-stable(identical unconditional setter + consumers without a PID guard). Ports to 1.8 and 2.0 will follow after this merges.