Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions dev/MRTCore/mrt/Core/src/MRM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,19 @@

#include <memory>
#include <string>
#include <cstdlib>

// Bug 63048673: Restore MrmGetFilePathFromName always-succeed contract (return S_OK with a
// best-effort path when no PRI file exists) instead of failing with ERROR_FILE_NOT_FOUND.
#define WINAPPSDK_CHANGEID_63048673 63048673

// Bug 63098344/63098302: MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY is a process environment variable
// (inherited across CreateProcess). Honor it only when it was stamped by the current process, so a
// child does not resolve the parent's resources.pri. The change is gated so apps can revert to the
// prior (unguarded) behavior via RuntimeCompatibilityOptions.
// See https://github.com/microsoft/WindowsAppSDK/issues/5987.
#define WINAPPSDK_CHANGEID_63098302 63098302

using namespace Microsoft::Resources;

typedef struct _MrmObjects
Expand Down Expand Up @@ -956,6 +964,26 @@ STDAPI_(void) MrmFreeResource(_In_opt_ void* resource)
return;
}

// The base directory is communicated via the MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY process
// environment variable, which is inherited by child processes spawned via CreateProcess. Its owner
// also stamps its process id in MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID. Honor the base
// directory only when that stamp matches the current process; otherwise it was inherited from a
// parent and must be ignored (a child would otherwise resolve resources from the parent's directory).
// See https://github.com/microsoft/WindowsAppSDK/issues/5987.
static bool IsBaseDirectoryStampedByCurrentProcess()
{
wchar_t stampedPidText[16]{};
const DWORD length{ ::GetEnvironmentVariableW(
L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID", stampedPidText, ARRAYSIZE(stampedPidText)) };
if ((length == 0) || (length >= ARRAYSIZE(stampedPidText)))
{
// Not stamped (absent) or unexpectedly long: do not honor the base directory.
return false;
}

return static_cast<DWORD>(wcstoul(stampedPidText, nullptr, 10)) == ::GetCurrentProcessId();
}

// When filename is provided, append filename to current module path. If the file doesn't exist, it will
// append the filename to parent path. If none exists, file in current module
// path will be returned.
Expand All @@ -980,6 +1008,10 @@ STDAPI MrmGetFilePathFromName(_In_opt_ PCWSTR filename, _Outptr_ PWSTR* filePath
// ERROR_FILE_NOT_FOUND when no PRI file exists.
const bool fallbackChangeEnabled = WinAppSdk::Containment::IsChangeEnabled<WINAPPSDK_CHANGEID_63048673>();

// When disabled via RuntimeCompatibilityOptions, retain the pre-fix behavior of honoring an
// inherited MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY regardless of which process stamped it.
const bool baseDirPidGuardEnabled = WinAppSdk::Containment::IsChangeEnabled<WINAPPSDK_CHANGEID_63098302>();

wil::unique_cotaskmem_string exeDir;
RETURN_IF_FAILED(wil::GetModuleFileNameW(nullptr, exeDir));
PCWSTR exeName = wil::find_last_path_segment(exeDir.get());
Expand Down Expand Up @@ -1015,9 +1047,20 @@ STDAPI MrmGetFilePathFromName(_In_opt_ PCWSTR filename, _Outptr_ PWSTR* filePath
searchStart = SearchPass::exeDirForResourcesPri;
if (SUCCEEDED(wil::TryGetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", baseDir)) && baseDir)
{
searchStart = SearchPass::BaseDirForResourcesPri;
RETURN_IF_FAILED(StringCchLengthW(baseDir.get(), STRSAFE_MAX_CCH, &baseDirCount));
RETURN_IF_FAILED(SizeTAdd(baseDirCount, 1, &baseDirCount));
// The base directory is a process environment variable (inherited across CreateProcess);
// when the guard is enabled and the value was not stamped by this process, ignore it so a
// child does not load the parent's resources.pri. When the guard is disabled, honor it
// regardless (pre-fix behavior).
if (baseDirPidGuardEnabled && !IsBaseDirectoryStampedByCurrentProcess())
{
baseDir.reset();
}
else
{
searchStart = SearchPass::BaseDirForResourcesPri;
RETURN_IF_FAILED(StringCchLengthW(baseDir.get(), STRSAFE_MAX_CCH, &baseDirCount));
RETURN_IF_FAILED(SizeTAdd(baseDirCount, 1, &baseDirCount));
}
}
}

Expand Down
289 changes: 289 additions & 0 deletions dev/MRTCore/mrt/Core/unittests/MrmTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,17 @@
#include "..\src\MRM.h"
#include <FrameworkUdk/Containment.h>

#include <string>
#include <cstdlib>

// Change ID that gates the MrmGetFilePathFromName always-succeed fallback (see MRM.cpp). Defined
// here (mirroring the inline definition in MRM.cpp) so the disabled-behavior test can name it.
#define WINAPPSDK_CHANGEID_63048673 63048673

// Change ID that gates the MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY per-process PID guard (see
// MRM.cpp). Defined here, mirroring MRM.cpp, so the disabled-behavior test can name it.
#define WINAPPSDK_CHANGEID_63098302 63098302

using namespace WEX::Common;
using namespace WEX::TestExecution;
using namespace WEX::Logging;
Expand Down Expand Up @@ -768,4 +775,286 @@ class ContainmentDisabledTest
VERIFY_IS_NULL(path);
}
};

// Regression tests for https://github.com/microsoft/WindowsAppSDK/issues/5987 (enabled/default state).
//
// MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY is a process environment variable set by the Windows App
// SDK auto-initializer (for PublishSingleFile SxS redirection). Because it is a process environment
// variable it is inherited by child processes spawned via CreateProcess. To keep a child from loading
// the parent's resources.pri, the auto-initializer also stamps MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID
// with the owning process id, and MrmGetFilePathFromName honors the base directory only when that stamp
// matches the current process (WINAPPSDK_CHANGEID_63098302, enabled by default).
class EnvironmentVariableIsolationTests
{
public:
TEST_CLASS(EnvironmentVariableIsolationTests);

// In-process guard: the base directory is honored only when its PID stamp matches this process.
TEST_METHOD(GetFilePath_HonorsBaseDirectoryOnlyWhenStampedByCurrentProcess)
{
const std::wstring baseDir{ CreateTempDirWithResourcesPri() };
// AppContext.BaseDirectory (the real setter's value) ends with a directory separator, and
// MrmGetFilePathFromName's buffer sizing relies on that, so mimic it here.
const std::wstring baseDirWithSeparator{ baseDir + L"\\" };
const std::wstring moduleDir{ GetCurrentProcessModuleDirectory() };

VERIFY_WIN32_BOOL_SUCCEEDED(SetEnvironmentVariableW(
L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", baseDirWithSeparator.c_str()));

wchar_t* path{};

// Stamped by the current process: the base directory is honored.
SetBaseDirectoryPid(GetCurrentProcessId());
VERIFY_ARE_EQUAL(S_OK, MrmGetFilePathFromName(nullptr, &path));
VERIFY_IS_TRUE(IsUnderDirectory(path, baseDirWithSeparator),
String().Format(L"Expected a path under the base directory, got '%s'.", path));
MrmFreeResource(path);
path = nullptr;

// Stamped by a foreign process (as when inherited across CreateProcess): the base directory is
// ignored and the module (exe) directory is used instead.
SetBaseDirectoryPid(GetCurrentProcessId() ^ 0x1);
VERIFY_ARE_EQUAL(S_OK, MrmGetFilePathFromName(nullptr, &path));
VERIFY_IS_FALSE(IsUnderDirectory(path, baseDirWithSeparator),
String().Format(L"Expected the inherited base directory to be ignored, got '%s'.", path));
VERIFY_IS_TRUE(IsUnderDirectory(path, moduleDir),
String().Format(L"Expected a path under the module directory, got '%s'.", path));
MrmFreeResource(path);
path = nullptr;

// No stamp at all: the base directory is ignored.
VERIFY_WIN32_BOOL_SUCCEEDED(SetEnvironmentVariableW(
L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID", nullptr));
VERIFY_ARE_EQUAL(S_OK, MrmGetFilePathFromName(nullptr, &path));
VERIFY_IS_FALSE(IsUnderDirectory(path, baseDirWithSeparator),
String().Format(L"Expected an unstamped base directory to be ignored, got '%s'.", path));
MrmFreeResource(path);
path = nullptr;

SetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", nullptr);
SetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID", nullptr);
RemoveTempDirectory(baseDir);
}

// End-to-end insulation: a real child process must ignore the inherited base directory. This
// reproduces the reported scenario where one Windows App SDK app launches another via CreateProcess.
TEST_METHOD(ChildProcessIsInsulatedFromInheritedBaseDirectory)
{
const std::wstring baseDir{ CreateTempDirWithResourcesPri() };
const std::wstring baseDirWithSeparator{ baseDir + L"\\" };

// Configure the environment exactly as the setter does for this (parent) process, then spawn a
// child that inherits it. The child's process id differs from this stamp, so it must not honor
// the base directory.
VERIFY_WIN32_BOOL_SUCCEEDED(SetEnvironmentVariableW(
L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", baseDirWithSeparator.c_str()));
SetBaseDirectoryPid(GetCurrentProcessId());
VERIFY_WIN32_BOOL_SUCCEEDED(SetEnvironmentVariableW(L"MRM_TEST_CHILD_WORKER", L"1"));

const std::wstring commandLine{ BuildChildWorkerCommandLine() };
Log::Comment(String().Format(L"Spawning child: %s", commandLine.c_str()));

std::wstring mutableCommandLine{ commandLine };
STARTUPINFOW startupInfo{};
startupInfo.cb = sizeof(startupInfo);
PROCESS_INFORMATION processInfo{};
const BOOL created{ CreateProcessW(nullptr, mutableCommandLine.data(), nullptr, nullptr,
/*bInheritHandles*/ FALSE, /*dwCreationFlags*/ 0, /*lpEnvironment*/ nullptr,
/*lpCurrentDirectory*/ nullptr, &startupInfo, &processInfo) };

// Clear the environment before asserting so it never leaks into sibling tests.
SetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", nullptr);
SetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID", nullptr);
SetEnvironmentVariableW(L"MRM_TEST_CHILD_WORKER", nullptr);

VERIFY_WIN32_BOOL_SUCCEEDED(created);
VERIFY_ARE_EQUAL(WAIT_OBJECT_0, WaitForSingleObject(processInfo.hProcess, 60 * 1000));

DWORD exitCode{ MAXDWORD };
VERIFY_WIN32_BOOL_SUCCEEDED(GetExitCodeProcess(processInfo.hProcess, &exitCode));
CloseHandle(processInfo.hThread);
CloseHandle(processInfo.hProcess);
RemoveTempDirectory(baseDir);

// The child runs only ChildProcessInsulationWorker; a zero exit code means it passed (i.e. it
// ignored the inherited base directory and resolved resources under its own module directory).
VERIFY_ARE_EQUAL(0u, exitCode,
String().Format(L"Child insulation worker exit code: %u (0 == insulated).", exitCode));
}

// Runs only inside the child spawned by ChildProcessIsInsulatedFromInheritedBaseDirectory (gated on
// the MRM_TEST_CHILD_WORKER sentinel). It is skipped during a normal test pass.
TEST_METHOD(ChildProcessInsulationWorker)
{
wchar_t sentinel[8]{};
if ((GetEnvironmentVariableW(L"MRM_TEST_CHILD_WORKER", sentinel, ARRAYSIZE(sentinel)) == 0) ||
(wcscmp(sentinel, L"1") != 0))
{
Log::Comment(L"Skipping: this worker runs only when spawned by the parent insulation test.");
Log::Result(TestResults::Skipped);
return;
}

// This process inherited MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY (and its foreign PID stamp)
// from the parent. Confirm MRTCore does not resolve resources under that inherited directory.
wchar_t inheritedBaseDir[MAX_PATH]{};
GetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY",
inheritedBaseDir, ARRAYSIZE(inheritedBaseDir));
const std::wstring moduleDir{ GetCurrentProcessModuleDirectory() };

wchar_t* path{};
VERIFY_ARE_EQUAL(S_OK, MrmGetFilePathFromName(nullptr, &path));
Log::Comment(String().Format(
L"Child resolved '%s' (inherited base directory '%s', module directory '%s').",
path, inheritedBaseDir, moduleDir.c_str()));

VERIFY_IS_TRUE(IsUnderDirectory(path, moduleDir),
String().Format(L"Child was not insulated: '%s' is not under its module directory.", path));
if (inheritedBaseDir[0] != L'\0')
{
VERIFY_IS_FALSE(IsUnderDirectory(path, inheritedBaseDir),
String().Format(L"Child was not insulated: '%s' is under the inherited base directory.", path));
}
MrmFreeResource(path);
}

private:
static void SetBaseDirectoryPid(DWORD pid)
{
wchar_t pidText[16]{};
VERIFY_ARE_EQUAL(0, _ultow_s(pid, pidText, ARRAYSIZE(pidText), 10));
VERIFY_WIN32_BOOL_SUCCEEDED(SetEnvironmentVariableW(
L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID", pidText));
}

static bool IsUnderDirectory(PCWSTR path, const std::wstring& directoryWithSeparator)
{
return (directoryWithSeparator.size() > 0) &&
(_wcsnicmp(path, directoryWithSeparator.c_str(), directoryWithSeparator.size()) == 0);
}

static std::wstring GetCurrentProcessModuleDirectory()
{
wchar_t modulePath[MAX_PATH]{};
VERIFY_ARE_NOT_EQUAL(0u, GetModuleFileNameW(nullptr, modulePath, ARRAYSIZE(modulePath)));
std::wstring directory{ modulePath };
directory.resize(directory.find_last_of(L'\\') + 1); // keep the trailing separator
return directory;
}

static std::wstring CreateTempDirWithResourcesPri()
{
wchar_t tempPath[MAX_PATH]{};
const DWORD tempLength{ GetTempPathW(ARRAYSIZE(tempPath), tempPath) };
VERIFY_IS_TRUE((tempLength > 0) && (tempLength < ARRAYSIZE(tempPath)));

const std::wstring directory{ std::wstring(tempPath) + L"MrmBaseDirTest_" +
std::to_wstring(GetCurrentProcessId()) + L"_" + std::to_wstring(GetTickCount64()) };
VERIFY_IS_TRUE(CreateDirectoryW(directory.c_str(), nullptr) ||
(GetLastError() == ERROR_ALREADY_EXISTS));

const std::wstring resourcesPri{ directory + L"\\resources.pri" };
const HANDLE file{ CreateFileW(resourcesPri.c_str(), GENERIC_WRITE, 0, nullptr,
CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr) };
VERIFY_ARE_NOT_EQUAL(INVALID_HANDLE_VALUE, file);
CloseHandle(file);
return directory;
}

static void RemoveTempDirectory(const std::wstring& directory)
{
DeleteFileW((directory + L"\\resources.pri").c_str());
RemoveDirectoryW(directory.c_str());
}

static std::wstring BuildChildWorkerCommandLine()
{
// Prefer the TAEF console runner (te.exe) sitting next to whatever process is hosting this test
// (te.exe or TE.ProcessHost.exe); fall back to the current host if it isn't found.
wchar_t hostPath[MAX_PATH]{};
VERIFY_ARE_NOT_EQUAL(0u, GetModuleFileNameW(nullptr, hostPath, ARRAYSIZE(hostPath)));
const std::wstring hostDirectory{ GetCurrentProcessModuleDirectory() };
std::wstring runnerPath{ hostDirectory + L"te.exe" };
if (GetFileAttributesW(runnerPath.c_str()) == INVALID_FILE_ATTRIBUTES)
{
runnerPath = hostPath;
}

// Resolve this test binary's own path so the child loads the same DLL.
HMODULE selfModule{};
VERIFY_WIN32_BOOL_SUCCEEDED(GetModuleHandleExW(
GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_UNCHANGED_REFCOUNT,
reinterpret_cast<LPCWSTR>(&EnvironmentVariableIsolationTests::BuildChildWorkerCommandLine),
&selfModule));
wchar_t selfPath[MAX_PATH]{};
VERIFY_ARE_NOT_EQUAL(0u, GetModuleFileNameW(selfModule, selfPath, ARRAYSIZE(selfPath)));

// Run only the worker, in-process, so the child's module identity is the runner.
return L"\"" + runnerPath + L"\" \"" + std::wstring(selfPath) +
L"\" /inproc /name:*ChildProcessInsulationWorker";
}
};

// Verifies the containment behavior of the base-directory PID guard (WINAPPSDK_CHANGEID_63098302).
// When an app disables the change, MrmGetFilePathFromName must revert to the pre-fix behavior of
// honoring MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY even when it was stamped by (inherited from) a
// different process.
//
// IsolationLevel=Method: WinAppSdk::Containment::IsChangeEnabled<>() caches its result in a
// process-lifetime function-local static, so the disabled-state test must run in its own process,
// separate from EnvironmentVariableIsolationTests (which exercises the enabled default state).
class BaseDirectoryContainmentDisabledTest
{
public:
BEGIN_TEST_CLASS(BaseDirectoryContainmentDisabledTest)
TEST_CLASS_PROPERTY(L"IsolationLevel", L"Method")
END_TEST_CLASS()

TEST_METHOD(InheritedBaseDirectoryHonoredWhenDisabled)
{
// Disable the base-directory PID guard at the FrameworkUdk containment worker.
const UINT32 disabledChanges[] = { 63098302 };
WinAppSdk::Containment::WinAppSDKRuntimeConfiguration config{};
config.disabledChanges = disabledChanges;
config.disabledChangesCount = ARRAYSIZE(disabledChanges);
VERIFY_SUCCEEDED(WinAppSdk::Containment::SetConfiguration(&config));
VERIFY_IS_FALSE((WinAppSdk::Containment::IsChangeEnabled<WINAPPSDK_CHANGEID_63098302>()));

// Create a base directory (with resources.pri) distinct from this module's directory.
wchar_t tempPath[MAX_PATH]{};
const DWORD tempLength{ GetTempPathW(ARRAYSIZE(tempPath), tempPath) };
VERIFY_IS_TRUE((tempLength > 0) && (tempLength < ARRAYSIZE(tempPath)));
const std::wstring baseDir{ std::wstring(tempPath) + L"MrmBaseDirDisabled_" +
std::to_wstring(GetCurrentProcessId()) + L"_" + std::to_wstring(GetTickCount64()) };
VERIFY_IS_TRUE(CreateDirectoryW(baseDir.c_str(), nullptr) || (GetLastError() == ERROR_ALREADY_EXISTS));
const std::wstring resourcesPri{ baseDir + L"\\resources.pri" };
const HANDLE file{ CreateFileW(resourcesPri.c_str(), GENERIC_WRITE, 0, nullptr,
CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr) };
VERIFY_ARE_NOT_EQUAL(INVALID_HANDLE_VALUE, file);
CloseHandle(file);

const std::wstring baseDirWithSeparator{ baseDir + L"\\" };
VERIFY_WIN32_BOOL_SUCCEEDED(SetEnvironmentVariableW(
L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", baseDirWithSeparator.c_str()));

// Stamp a FOREIGN process id (as when inherited across CreateProcess).
wchar_t pidText[16]{};
VERIFY_ARE_EQUAL(0, _ultow_s(GetCurrentProcessId() ^ 0x1, pidText, ARRAYSIZE(pidText), 10));
VERIFY_WIN32_BOOL_SUCCEEDED(SetEnvironmentVariableW(
L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID", pidText));

// With the guard disabled, the foreign-stamped base directory is honored (pre-fix behavior):
// the resolved path is under the base directory, not the module directory.
wchar_t* path{};
VERIFY_ARE_EQUAL(S_OK, MrmGetFilePathFromName(nullptr, &path));
VERIFY_IS_TRUE(_wcsnicmp(path, baseDirWithSeparator.c_str(), baseDirWithSeparator.size()) == 0,
String().Format(L"Expected the inherited base directory to be honored (disabled), got '%s'.", path));
MrmFreeResource(path);

SetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY", nullptr);
SetEnvironmentVariableW(L"MICROSOFT_WINDOWSAPPRUNTIME_BASE_DIRECTORY_PID", nullptr);
DeleteFileW(resourcesPri.c_str());
RemoveDirectoryW(baseDir.c_str());
}
};
} // namespace UnitTest
Loading