Handle container creation and policy more cleanly

build_debug.bat

@@ -0,0 +1,3 @@
+@echo off
+echo Building Debug configuration...
+msbuild wxc.sln /p:Configuration=Debug /p:Platform=x64 /t:Rebuild /nologo /verbosity:minimal

cli/src/cli.ts

@@ -148,8 +148,7 @@ program
       // Spawn the process
       // NOTE: For now, we will force winpty.
       const pty = spawnSandbox(config.script, policy, {
-        debug: options.debug ?? false,
-        useConpty: false
+        debug: options.debug ?? false
       }, config.workingDirectory, config.appContainer?.name);
 
       // Handle output

sdk/src/sandbox.ts

@@ -67,12 +67,6 @@ export interface SandboxSpawnOptions {
    */
   debug?: boolean;
 
-  /** 
-   * Use the conpty DLL instead of the default winpty backend on Windows 11.
-   * This may provide better performance and compatibility.
-   */
-  useConpty?: boolean;
-
   /**
    * PTY options to pass to node-pty
    */
@@ -158,7 +152,6 @@ export function spawnSandbox(
     rows: 80,
     cwd: workingDirectory || process.cwd(),
     env: process.env,
-    useConpty: options.useConpty,
     ...options.ptyOptions,
   };
 

test_configs/pwsh_setlocation.json

@@ -0,0 +1,17 @@
+{
+  "script": "pwsh.exe -nop -nol -c \"Set-PSReadLineOption -HistorySaveStyle SaveNothing; Set-Location c:\\temp\"; Get-ChildItem",
+  "appContainer": {
+    "name": "CLI-Pwsh"
+  },
+  "filesystem": {
+    "readwritePaths": [
+      "C:\\Program Files\\PowerShell\\7",
+      "C:\\temp",
+      "C:\\Users",
+      "C:\\Users\\st\\AppData\\Roaming\\Microsoft\\Windows\\PowerShell\\PSReadLine"
+    ],
+    "readonlyPaths": [
+      "C:\\"
+    ]
+  }
+}

test_scripts/run_pwsh_test.bat

@@ -0,0 +1,2 @@
+@echo off
+..\outputs\wxc\x64\Debug\wxc-exec.exe --debug ..\test_configs\pwsh_setlocation.json

wxc_common/AppContainerScriptRunner.cpp

@@ -146,8 +146,8 @@ ScriptResponse AppContainerScriptRunner::RunInternal(const CodexRequest& request
     siEx.StartupInfo.dwFlags |= STARTF_USESTDHANDLES;
     siEx.StartupInfo.lpDesktop = const_cast<LPWSTR>(L"winsta0\\default");
 
-    // Initialize attribute list (security caps + optional LPAC policy)
-    DWORD attrCount = request.policy.leastPrivilegeMode ? 2 : 1;
+    // Initialize attribute list (security caps + handle list + optional LPAC policy)
+    DWORD attrCount = request.policy.leastPrivilegeMode ? 3 : 2;
     SIZE_T attributeListSize = 0;
     ::InitializeProcThreadAttributeList(nullptr, attrCount, 0, &attributeListSize);
 
@@ -185,6 +185,16 @@ ScriptResponse AppContainerScriptRunner::RunInternal(const CodexRequest& request
         }
     }
 
+    // Explicitly list only the pipe handles the child container needs to inherit.
+    // This lets us pass bInheritHandles=TRUE to CreateProcessW while still tightly
+    // controlling which handles the child can access.
+    HANDLE inheritHandles[] = {hStdInRead.get(), hStdOutWrite.get(), hStdErrWrite.get()};
+    if (!::UpdateProcThreadAttribute(siEx.lpAttributeList, 0, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, inheritHandles,
+                                     sizeof(inheritHandles), nullptr, nullptr))
+    {
+        return CreateErrorResponse(L"Failed to update HANDLE_LIST attribute.");
+    }
+
     // Create the process
     std::vector<wchar_t> cmdLineBuffer(request.scriptCode.begin(), request.scriptCode.end());
     cmdLineBuffer.push_back(L'\0');
@@ -239,19 +249,34 @@ ScriptResponse AppContainerScriptRunner::RunInternal(const CodexRequest& request
     params3.hWrite = hParentStdErr;
     hThread3.reset(::CreateThread(nullptr, 0, WXC::PipeThread, &params3, 0, nullptr));
 
-    // Wait for child process to exit
-    ::WaitForSingleObject(hProcess.get(), GetTimeoutMilliseconds(request.scriptTimeout));
+    // Wait for the child process to exit, or for an output relay thread to finish.
+    // Threads 2 and 3 exit when the child closes its stdout/stderr (which happens on exit),
+    // so any of these handles signaling indicates the child session is over.
+    HANDLE completionHandles[] = {hProcess.get(), hThread2.get(), hThread3.get()};
+    DWORD waitResult = ::WaitForMultipleObjects(3, completionHandles, FALSE,
+                                                GetTimeoutMilliseconds(request.scriptTimeout));
 
-    DWORD exitCode = 0;
-    ::GetExitCodeProcess(hProcess.get(), &exitCode);
+    if (waitResult == WAIT_TIMEOUT)
+    {
+        // Timeout elapsed before the child exited: forcibly terminate it.
+        ::TerminateProcess(hProcess.get(), static_cast<UINT>(-1));
+        // Block until the OS confirms the process is gone so GetExitCodeProcess is valid.
+        ::WaitForSingleObject(hProcess.get(), INFINITE);
+    }
+
+    // Shut down Thread 1 (stdin relay). CancelSynchronousIo interrupts its blocking
+    // ReadFile call, causing PipeThread to break out of its loop. Closing hStdInWrite
+    // ensures that any WriteFile already in flight also fails promptly.
+    ::CancelSynchronousIo(hThread1.get());
+    hStdInWrite.reset();
 
-    // Wait for threads to finish (with 1 second timeout)
-    HANDLE threads[] = {hThread1.get(), hThread2.get(), hThread3.get()};
-    WaitForMultipleObjects(3, threads, TRUE, 1000);
+    // Wait for all relay threads to finish draining and exit cleanly.
+    HANDLE allThreads[] = {hThread1.get(), hThread2.get(), hThread3.get()};
+    ::WaitForMultipleObjects(3, allThreads, TRUE, 2000);
 
-    // TODO: If the process is still running after timeout, terminate it
+    DWORD exitCode = 0;
+    ::GetExitCodeProcess(hProcess.get(), &exitCode);
 
-    // TODO: Decide if we need one shot still and script response, or just error code and logging
     ScriptResponse result;
     result.ExitCode = static_cast<int>(exitCode);
 

wxc_common/FileSystemBfsManager.cpp

@@ -16,7 +16,8 @@ bool FileSystemBfsManager::Configure(ContainerPolicy policy, std::wstring& error
     // Configure BFS for allowed paths
     for (const auto& path : policy.readwritePaths)
     {
-        if (!AddBfsPath(path, errorMsg))
+        bool inherit = TestForRootPath(path);
+        if (!AddBfsPath(path, errorMsg, inherit))
         {
             RemoveConfiguration();
             return false;
@@ -30,7 +31,8 @@ bool FileSystemBfsManager::Configure(ContainerPolicy policy, std::wstring& error
     // Configure BFS for allowed read-only paths
     for (const auto& path : policy.readonlyPaths)
     {
-        if (!AddReadOnlyBfsPath(path, errorMsg))
+        bool inherit = TestForRootPath(path);
+        if (!AddReadOnlyBfsPath(path, errorMsg, inherit))
         {
             RemoveConfiguration();
             return false;
@@ -81,26 +83,41 @@ bool FileSystemBfsManager::ExecuteBfsCfgOperation(std::span<std::wstring_view> a
     return true;
 }
 
-bool FileSystemBfsManager::AddBfsPath(std::wstring_view path, std::wstring& errorMsg)
+bool FileSystemBfsManager::AddBfsPath(std::wstring_view path, std::wstring& errorMsg, bool inherit)
 {
     std::vector<std::wstring_view> args = {
-        L"--addpolicy", L"--policybroker", L"--filename", path, L"--appid", _appContainerName, L"--containerinherit",
+        L"--addpolicy", L"--policybroker", 
+        L"--filename", path, L"--appid", _appContainerName
     };
+    if (inherit)
+    {
+        args.push_back(L"--containerinherit");
+    }
     return ExecuteBfsCfgOperation(
         args, L"Failed to add BFS path " + std::wstring{path} + L" for AppContainer " + _appContainerName, errorMsg);
 }
 
-bool FileSystemBfsManager::AddReadOnlyBfsPath(std::wstring_view path, std::wstring& errorMsg)
+bool FileSystemBfsManager::AddReadOnlyBfsPath(std::wstring_view path, std::wstring& errorMsg, bool inherit)
 {
     std::vector<std::wstring_view> args = {
-        L"--addpolicy", L"--policybrokerreadonly", L"--filename",         path,
-        L"--appid",     _appContainerName,         L"--containerinherit",
+        L"--addpolicy", L"--policybrokerreadonly", 
+        L"--filename", path, L"--appid",     _appContainerName
     };
+    if (inherit)
+    {
+        args.push_back(L"--containerinherit");
+    }
     return ExecuteBfsCfgOperation(
         args, L"Failed to add read-only BFS path " + std::wstring{path} + L" for AppContainer " + _appContainerName,
         errorMsg);
 }
 
+bool FileSystemBfsManager::TestForRootPath(std::wstring_view path)
+{
+    // Test to see if the path is "C:\", if so DO NOT inherit
+    return (path == L"C:\\") ? false: true;
+}
+
 bool FileSystemBfsManager::RemoveConfiguration(std::wstring& errorMsg)
 {
     std::vector<std::wstring_view> args = {L"--clearpolicy", L"--appid", _appContainerName};

wxc_common/ProcessUtilities.cpp

@@ -19,11 +19,13 @@ DWORD WINAPI PipeThread(LPVOID param)
 
     while (true)
     {
+        // If the process has closed the pipe or an error occurs, exit the loop
         if (!ReadFile(hRead, buffer, BUFFER_SIZE, &bytesRead, nullptr) || bytesRead == 0)
         {
             break;
         }
 
+        // Write to the destination pipe. If an error occurs, exit the loop.
         if (!WriteFile(hWrite, buffer, bytesRead, &bytesWritten, nullptr) || bytesWritten != bytesRead)
         {
             break;

wxc_common/include/FileSystemBfsManager.h

@@ -29,9 +29,9 @@ class FileSystemBfsManager
     WXC::Logger& _logger;
     bool _configured = false;
 
-    bool AddBfsPath(std::wstring_view path, std::wstring& errorMsg);
+    bool AddBfsPath(std::wstring_view path, std::wstring& errorMsg, bool inherit = true);
 
-    bool AddReadOnlyBfsPath(std::wstring_view path, std::wstring& errorMsg);
+    bool AddReadOnlyBfsPath(std::wstring_view path, std::wstring& errorMsg, bool inherit = true);
 
     bool RemoveConfiguration(std::wstring& errorMsg);
 
@@ -41,4 +41,7 @@ class FileSystemBfsManager
 
     // Helper: Run bfscfg.exe with arguments
     std::wstring RunBfsCfg(std::span<std::wstring_view> args, std::wstring& errorMsg);
+
+    // Helper: Test a path to see if it is the root of a drive
+    bool TestForRootPath(std::wstring_view path);
 };