diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c0e5cfd621..33f4437ce2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -196,13 +196,12 @@ jobs: timeout-minutes: 15 env: REASONIX_RELEASE_CACHE_GUARD: "1" - WINDOWS_SANDBOX_WAIT_MS: "20000" run: node scripts/windows-go-tests.mjs smoke - name: test (Windows credential ACL identity) if: env.RUN_STEPS == 'true' && runner.os == 'Windows' && github.event_name == 'pull_request' timeout-minutes: 3 - run: go test -timeout=2m -run '^TestCredentialAccessRepairsLegacyCredentialDeny|^TestRepairLegacyCredentialDenyMatchesFileAcrossPathAliases$' ./internal/config ./internal/winsandbox + run: go test -timeout=2m -run '^TestCredentialAccessRepairsLegacyCredentialDeny|^TestRepairLegacyCredentialDenyMatchesFileAcrossPathAliases$' ./internal/config ./internal/winaclresidue # The general Windows PR smoke list intentionally omits internal/tool. # Keep the session-temp portability contract covered without widening the @@ -257,9 +256,6 @@ jobs: # regression there silently tanks the cache hit rate the project is # built around. REASONIX_RELEASE_CACHE_GUARD: "1" - # Bound sandbox helper children in Windows tests so a failed OS-level - # launch cannot pin the Actions step after Go's package timeout fires. - WINDOWS_SANDBOX_WAIT_MS: "20000" run: node scripts/windows-go-tests.mjs full - name: test (Scoop desktop launch) @@ -329,7 +325,6 @@ jobs: timeout-minutes: 10 env: REASONIX_RELEASE_CACHE_GUARD: "1" - WINDOWS_SANDBOX_WAIT_MS: "20000" run: node scripts/windows-go-tests.mjs control windows-isolated: @@ -374,7 +369,6 @@ jobs: timeout-minutes: 15 env: REASONIX_RELEASE_CACHE_GUARD: "1" - WINDOWS_SANDBOX_WAIT_MS: "20000" run: node scripts/windows-go-tests.mjs ${{ matrix.group }} race: diff --git a/desktop/main.go b/desktop/main.go index 137c90f997..dc387716bf 100644 --- a/desktop/main.go +++ b/desktop/main.go @@ -13,8 +13,8 @@ import ( "os" "strings" - "reasonix/internal/sandbox" "reasonix/internal/skill/skillwatch" + "reasonix/internal/winaclresidue" // Blank imports wire compile-time built-ins into their registries, exactly as // cmd/reasonix does — boot.Build resolves providers/tools from these registries. @@ -49,17 +49,7 @@ func macSelfUpdateAllowed() bool { } } -func runWindowsSandboxHelperIfRequested(argv []string) (int, bool) { - if len(argv) > 1 && argv[1] == sandbox.WindowsHelperCommand { - return sandbox.RunWindowsSandboxHelper(argv[2:], os.Stdin, os.Stdout, os.Stderr), true - } - return 0, false -} - func main() { - if code, ok := runWindowsSandboxHelperIfRequested(os.Args); ok { - os.Exit(code) - } // Contract generation is a build-time operation. Dispatch it before crash // capture so packaging cannot create files in the operator's Reasonix home. if dir, ok := emitContractDir(os.Args[1:]); ok { @@ -71,7 +61,9 @@ func main() { if skillwatch.MaybeRunHelper() { return } - sandbox.RegisterHelperDispatch() + // Older Windows builds could leave sandbox ACL residue behind after a + // crash; sweep it in the background so startup never waits on icacls. + go winaclresidue.SweepStaleMarkers() // The detached macOS self-update child must run before any shell starts. if handled, exitCode := maybeRunMacUpdateHandoff(os.Args[1:]); handled { os.Exit(exitCode) diff --git a/desktop/windows_sandbox_helper_test.go b/desktop/windows_sandbox_helper_test.go deleted file mode 100644 index 26ae565ed1..0000000000 --- a/desktop/windows_sandbox_helper_test.go +++ /dev/null @@ -1,37 +0,0 @@ -package main - -import ( - "testing" - - "reasonix/internal/sandbox" -) - -// The Windows bash sandbox relaunches os.Executable() with -// sandbox.WindowsHelperCommand as argv[1]. If the desktop binary loses this -// route, every sandboxed command on Windows starts a second GUI instance and -// returns empty output (#6051, #6067, #6072). These tests pin the route. - -func TestWindowsSandboxHelperRouteRecognized(t *testing.T) { - // argv[2:] is empty, so the helper rejects it with a usage error — the - // point here is only that the route matched (ok == true) and the process - // would exit instead of booting the GUI. - code, ok := runWindowsSandboxHelperIfRequested([]string{"reasonix-desktop", sandbox.WindowsHelperCommand}) - if !ok { - t.Fatal("helper subcommand not routed; sandboxed commands would boot a second GUI instance") - } - if code == 0 { - t.Fatalf("helper with no payload should fail with a usage error, got exit code %d", code) - } -} - -func TestWindowsSandboxHelperRouteIgnoresNormalLaunch(t *testing.T) { - for _, argv := range [][]string{ - {"reasonix-desktop"}, - {"reasonix-desktop", "--some-flag"}, - {"reasonix-desktop", "not-the-helper", sandbox.WindowsHelperCommand}, - } { - if _, ok := runWindowsSandboxHelperIfRequested(argv); ok { - t.Fatalf("argv %v should not be treated as a helper launch", argv) - } - } -} diff --git a/docs/GUIDE.md b/docs/GUIDE.md index 27879ee47d..3f2bca713b 100644 --- a/docs/GUIDE.md +++ b/docs/GUIDE.md @@ -727,8 +727,7 @@ literal `/tmp` is no longer mapped by bubblewrap. **Windows note:** Windows has no OS-level shell sandbox. The restricted-token backend is retired from enforcement because denying the current user's own SID locked hosts out of their credential store and the token broke common -toolchains (see [Windows sandbox architecture](./WINDOWS_SANDBOX.md) for the -retired design). Permission presets still apply as Reasonix tool-layer +toolchains. Permission presets still apply as Reasonix tool-layer boundaries: Read only refuses file writes and asks before every shell command, and Workspace write keeps file tools inside `workspace_root` and `allow_write` and asks before writing elsewhere. Shell commands in every preset run as the diff --git a/docs/GUIDE.zh-CN.md b/docs/GUIDE.zh-CN.md index b88a9bf2f4..bfc3311ae4 100644 --- a/docs/GUIDE.zh-CN.md +++ b/docs/GUIDE.zh-CN.md @@ -579,7 +579,7 @@ MCP 等独立沙盒继续使用自己的隔离规范，不继承父会话临时 **Windows 说明：**Windows 没有 OS 级 Shell 沙箱。受限令牌后端已退出强制执行： 它对当前用户自身 SID 加拒绝项会把宿主锁在自己的凭据存储之外，受限令牌也会破坏常见 -工具链（退役设计见[Windows 沙箱架构](./WINDOWS_SANDBOX.zh-CN.md)）。权限模式仍作为 +工具链。权限模式仍作为 Reasonix 工具层边界生效：仅可查看拒绝文件写入并在每条 Shell 命令前询问；工作区内修改 把文件工具限定在 `workspace_root` 与 `allow_write` 内，越界写入前询问。所有模式下的 Shell 命令都以当前系统账户运行、不受约束，因此 `[sandbox] network` 与 Shell 层的 diff --git a/docs/TOOL_APPROVAL_MODES.md b/docs/TOOL_APPROVAL_MODES.md index b6c9174c5b..2c319c6c84 100644 --- a/docs/TOOL_APPROVAL_MODES.md +++ b/docs/TOOL_APPROVAL_MODES.md @@ -18,9 +18,7 @@ macOS enforces restricted presets with Seatbelt and Linux uses bubblewrap. If the required backend cannot start there, restricted presets fail closed; Reasonix never offers to silently rerun the command without isolation. Windows has no OS-level shell sandbox: restricted presets still confine Reasonix file -tools and request exact grants, but shell commands run as the current OS user -(see [Windows sandbox architecture](./WINDOWS_SANDBOX.md) for the retired -backend). +tools and request exact grants, but shell commands run as the current OS user. ## Approval scope diff --git a/docs/TOOL_APPROVAL_MODES.zh-CN.md b/docs/TOOL_APPROVAL_MODES.zh-CN.md index cb537f1a01..d229327481 100644 --- a/docs/TOOL_APPROVAL_MODES.zh-CN.md +++ b/docs/TOOL_APPROVAL_MODES.zh-CN.md @@ -11,7 +11,7 @@ Reasonix 的桌面端、CLI、Serve、ACP、机器人、自动任务和子代理 - **协作方式**（普通 / 计划 / 目标）决定 Reasonix 怎么推进任务。没有自动任务模式或可选质量底线；验证义务由真实工具动作、项目规则、任务风险和用户显式要求共同建立。 - **权限预设**决定强制执行的文件系统与进程边界，以及何时请求精确授权。 -macOS 使用 Seatbelt，Linux 使用 bubblewrap；这两个平台上受限模式所需的沙箱无法启动时，Reasonix 会失败关闭，不会提示用户关闭沙箱后重试。Windows 没有 OS 级 Shell 沙箱：受限模式仍约束 Reasonix 文件工具并请求精确授权，但 Shell 命令以当前系统账户运行（退役后端见 [Windows 沙箱架构](./WINDOWS_SANDBOX.zh-CN.md)）。 +macOS 使用 Seatbelt，Linux 使用 bubblewrap；这两个平台上受限模式所需的沙箱无法启动时，Reasonix 会失败关闭，不会提示用户关闭沙箱后重试。Windows 没有 OS 级 Shell 沙箱：受限模式仍约束 Reasonix 文件工具并请求精确授权，但 Shell 命令以当前系统账户运行。 ## 授权范围 diff --git a/docs/WINDOWS_SANDBOX.md b/docs/WINDOWS_SANDBOX.md deleted file mode 100644 index 9989dee640..0000000000 --- a/docs/WINDOWS_SANDBOX.md +++ /dev/null @@ -1,181 +0,0 @@ -# Windows sandbox architecture - -> **Status: retired from enforcement.** Reasonix no longer launches the -> restricted-token/AppContainer helper on Windows. `sandbox.Available()` is -> always false there, `[sandbox] bash` resolves to `off`, and permission presets -> apply only as Reasonix tool-layer boundaries. The backend locked hosts out of -> their own credential store through same-user deny ACEs and broke common -> toolchains (Git Bash/MSYS2, Node stdio pipes, mapped drives, cold-start -> budgets). The remaining sections describe the retired design and stay here -> until the code is removed; the credential ACL repair for older installs is -> the only part still exercised. - -Reasonix used the same restricted-token pattern as DeepSeek Harness commit -`c291e7961a` for shell and writer processes on Windows. The implementation is a -native Go port; it does not load Harness code at runtime. - -## Permission mapping - -| Permission preset | Windows execution | -| --- | --- | -| Read only | A `WRITE_RESTRICTED` primary token with no directory capability SID. Direct read-only tools may use AppContainer. | -| Workspace write | A `WRITE_RESTRICTED` token containing only the capability SIDs for the canonical workspace, approved extra directories, and the session-private temp directory. | -| Full access | The normal host execution path as the current OS user. The Windows sandbox, including its protected-root and network constraints, is not applied. Explicit host deny rules still apply before launch. | - -The backend is reported as `windows-write-restricted+appcontainer` with -`partial` enforcement. Capability snapshots also report the write, read, and -network isolation mechanisms independently so a desktop or remote client does -not mistake the Windows boundary for Seatbelt or bubblewrap parity. - -## Write-restricted shell lane - -For each canonical writable directory, Reasonix derives a deterministic -capability SID from a Reasonix-specific domain, the capability purpose, and the -case-folded path. A separate purpose derives the session-temp SID. The helper -adds one object-and-container-inheritable Modify-class ACE to the exact -directory. The mask excludes `WRITE_DAC` and `WRITE_OWNER`. - -The ACE is standing state, while authority is per process. A subsequent token -can use an ACE only when the host includes that exact SID in its restricting -SID set. Downgrading to read only, revoking an extra directory, switching a -session, or rotating the session temp omits the old SID and makes its ACE inert. -The helper revalidates the directory's Windows file identity after changing its -DACL and fails closed if the path was replaced. - -Before changing a workspace DACL, the helper atomically writes a `preparing` -record below the host-controlled Reasonix state root. After the exact ACE and -file identity are verified, the same record becomes `active`. It contains the -record version, canonical path, Windows object identity, SID, exact mask, -inheritance, purpose, and owner process. Session-temp records are deliberately -not persisted because the random generation directory and its ACE are deleted -together. - -The restricted token is created with `DISABLE_MAX_PRIVILEGE`, `LUA_TOKEN`, and -`WRITE_RESTRICTED`. Its restricting set includes the logon SID and Everyone so -PowerShell, DLL, CNG, pipe, and desktop initialization can succeed, plus the -authorized capability SIDs in workspace-write mode. The default DACL uses the -session-temp capability where available. The child starts suspended, is -assigned to a kill-on-close Job Object with UI restrictions, and is then -resumed. Closing or timing out the run terminates the process tree. - -`CREATE_NO_WINDOW` is intentionally omitted for this lane because restricted -PowerShell and Node initialization rely on normal console inheritance. The -Reasonix helper process remains hidden by the desktop process launcher. - -Like Harness, this lane supports child processes that inherit or ignore stdio -and PowerShell pipelines. Node/libuv captured stdio uses named pipes whose -fixed Windows default descriptor does not include a restricting SID, so -`stdio: "pipe"` fails with `EPERM`. Reasonix reports this runtime denial with a -single-use `denial_id`; after explicit approval, the exact command can be -retried with `danger-full-access`. It never weakens the file write boundary to -make named-pipe capture succeed. - -## Direct-tool and protected-read lane - -The existing AppContainer path remains for direct read-only tools and is the -only Windows lane that can remove network capabilities. Explicitly configured -`forbid_read` roots remain temporary deny ACEs because `WRITE_RESTRICTED` does -not isolate reads. Those mutations are snapshotted, crash-marked, restored, and -serialized for their mutation lifetime. Reasonix's global credential `.env` is -not automatically added to Windows `forbid_read`: doing so would deny the same -user identity used by the host settings process. Credential values are still -filtered from model-controlled child-process environments, but Windows local -tools can deliberately read files available to the current user. - -Standing capability ACE creation uses short named-mutex critical sections. -Parent and child paths share a lock domain, preventing concurrent DACL updates -from losing an ACE. Ordinary commands sharing a workspace run concurrently -after the exact ACE exists. - -Protected Reasonix state cannot be inside a granted workspace, and arbitrary -state descendants cannot become writable roots. The one explicit exception is -the desktop's direct `/global-workspace`: its ACE is placed on that exact -descendant and does not grant its parent. Session temp must be disjoint from -both workspace and protected roots. - -## Migration from the earlier Windows backend - -Credential reads first try normal file access. On access denied, Reasonix can -remove one explicit current-user `DENY RX` entry from its global `.env` only -when a matching sandbox crash-residue record has a provably exited owner and -no matching record has a live or unverifiable owner. The repair takes the -sandbox path locks and rechecks the ACL. Missing residue records, additional -current-user deny entries, or unsupported ACE types prevent automatic repair; -the denied access remains visible. An identical mask alone does not establish -that a rule belongs to Reasonix. A successful repair consumes the dead run's -residue record so it cannot authorize a later, unrelated ACL change. - -The previous shell path used a low-integrity token and recursively changed -workspace integrity labels. The new shell path does not relabel the workspace -and does not restore large descriptor trees after each command. Existing -AppContainer compatibility code may still use its historical ACL and integrity -label handling for direct tools. - -Old low-integrity labels are not required by the new shell lane. Capability -ACEs are harmless without a token carrying their derived SID and may remain on -a directory across upgrades. Removing a workspace or session-temp directory -removes the corresponding ACE with it. - -The helper payload is versioned and rejects missing versions, future versions, -unknown fields, and trailing data. A missing native API, path-identity mismatch, -protected-root conflict, or unsupported network request returns a structured -sandbox failure; Reasonix does not silently rerun the command without a sandbox. - -## Known Windows boundaries - -- Restricted tokens isolate write-class file access. They do not isolate - ordinary reads, network, registry, named objects, or process visibility. -- Everyone remains in the restricting set for Windows runtime compatibility. - A location that explicitly grants ambient writes to Everyone may remain - writable. -- NTFS hard links and filesystem objects with unusual ACL inheritance require - native regression coverage. Reasonix revalidates directory identity but does - not claim a general hard-link boundary. -- AppContainer can disable network for direct tools. Shell and writer launches - with `Network: false` fail closed because the restricted-token lane cannot - enforce that boundary. -- Windows therefore remains `partial`, including when every native API is - available. - -## Validation - -Long-running Windows services must use `pwsh(run_in_background=true)`. The -start call returns a `pwsh-*` job id without a foreground timeout; use -`job_output` for incremental logs and status and `job_kill` to terminate the -Job Object process tree. The ordinary `pwsh` path launches the sandbox -runner and requested PowerShell command once; there is no nested readiness -probe. Before-command helper failures use a private inherited handle to report -`dependency`, `authorization`, or `launch`; a non-zero exit after PowerShell -starts remains an `execution` failure. An `authorization` result under a -restricted preset includes a short-lived `denial_id` for an explicitly -approved retry with `danger-full-access`. Similar text printed by the command -cannot change the phase, and Reasonix never retries outside the sandbox by -itself. The report handle is cleared from the helper's environment and marked -non-inheritable before launching the restricted child; the child handle allowlist -contains only stdio. Its temporary backing file denies sharing and is deleted on -last close. Reports never come from stdout/stderr, even if a command replays an -old diagnostic and exits with the helper's failure code. Background runtime -permission denials offer the same single-use approval retry as foreground calls, -while retaining `execution` / `may_be_partial` metadata. - -启动前错误通过 helper 专用诊断句柄传递，不再解析 stdout/stderr 中的错误标记。 -受限子进程不继承该句柄；临时诊断文件禁止共享打开，在最后一个句柄关闭时删除。 -即使命令重放旧诊断并使用相同退出码，也不会被误判为“未启动”。后台运行期权限 -拒绝与前台共用单次授权重试机制，同时保留可能已部分执行的状态。 - -Run the native suite and the 100-launch cold/warm benchmark on each supported -Windows architecture: - -```powershell -./scripts/verify-windows-sandbox.ps1 -OutputDirectory .codex-build/windows-sandbox-native -``` - -CI includes `reasonix/internal/winsandbox` in the Windows smoke group. Non-Windows -hosts also cross-compile `internal/winsandbox` and `internal/sandbox` for -Windows amd64 and arm64. Cross-compilation validates bindings and build tags; -it is not a substitute for the native ACL, token, Job Object, PowerShell, and -Electron checks described in the evidence README. - -The ported design is derived from DeepSeek Harness' MIT-licensed -`packages/sandbox/sandbox-windows-acl`; attribution is recorded in -`internal/winsandbox/NOTICE.md`. diff --git a/docs/WINDOWS_SANDBOX.zh-CN.md b/docs/WINDOWS_SANDBOX.zh-CN.md deleted file mode 100644 index b1f2ba9d11..0000000000 --- a/docs/WINDOWS_SANDBOX.zh-CN.md +++ /dev/null @@ -1,126 +0,0 @@ -# Windows 沙箱架构 - -> **状态：已退出强制执行。**Reasonix 不再在 Windows 上启动受限令牌/AppContainer -> helper。`sandbox.Available()` 在 Windows 上恒为 false，`[sandbox] bash` 解析为 -> `off`，权限模式仅作为 Reasonix 工具层边界生效。该后端曾通过对当前用户自身加拒绝 -> ACE 把宿主锁在自己的凭据存储之外，并破坏常见工具链（Git Bash/MSYS2、Node stdio -> 管道、映射网络盘、冷启动预算）。以下章节描述的是退役设计，在代码移除前保留； -> 旧安装的凭据 ACL 修复是其中唯一仍在使用的部分。 - -Reasonix 曾在 Windows 的 Shell 和写入进程中采用 DeepSeek Harness -`c291e7961a` 的受限令牌思路。当前实现是原生 Go 移植，运行时不依赖 -Harness 代码。 - -## 权限模式映射 - -| 权限模式 | Windows 执行方式 | -| --- | --- | -| 仅可查看 | 使用不携带目录能力 SID 的 `WRITE_RESTRICTED` 主令牌；直接只读工具仍可使用 AppContainer。 | -| 工作区内修改 | `WRITE_RESTRICTED` 令牌只携带当前工作区、已授权额外目录和会话私有临时目录的能力 SID。 | -| 完全权限 | 以当前系统账户走宿主正常执行路径，不启用 Windows 沙箱及其中的受保护目录和网络约束；宿主仍在启动前执行显式禁止规则。 | - -后端对外报告为 `windows-write-restricted+appcontainer`，强制等级为 -`partial`。权限快照还分别说明写入、读取和网络隔离方式，桌面端和远程端 -不会把 Windows 的边界误认为与 Seatbelt 或 bubblewrap 完全相同。 - -## WRITE_RESTRICTED Shell 通道 - -Reasonix 按规范化目录、用途和 Reasonix 专用域确定性派生能力 SID。工作区 -和会话临时目录使用不同用途，因此 SID 不会复用。Helper 在精确目录上增加 -一条可向文件与子目录继承的 Modify 类 ACE，并明确排除 `WRITE_DAC` 和 -`WRITE_OWNER`。 - -ACE 会保留在目录上，真正的授权则属于每次启动的进程令牌。只有宿主把同一 -SID 放进限制 SID 集合时，该进程才能使用这条 ACE。降级为“仅可查看”、撤销 -额外目录授权、切换会话或轮换临时目录后，新令牌不再携带旧 SID，原 ACE -立即失去作用。Helper 修改 DACL 后会重新检查 Windows 文件对象身份；路径 -在准备期间被替换时失败关闭。 - -修改工作区 DACL 之前，Helper 会在宿主控制的 Reasonix 状态目录内原子写入 -`preparing` 记录；精确 ACE 和文件身份复核完成后，同一记录更新为 `active`。 -记录包含版本、规范路径、Windows 对象身份、SID、准确掩码、继承方式、用途和 -所有者进程。会话临时目录记录不持久化，因为随机 generation 目录删除时会连同 -它的 ACE 一起回收。 - -受限令牌使用 `DISABLE_MAX_PRIVILEGE`、`LUA_TOKEN` 和 -`WRITE_RESTRICTED`。限制集合包含登录 SID、Everyone，以及工作区模式下本次 -调用获准的能力 SID。保留登录 SID 和 Everyone 是为了让 PowerShell、DLL、 -CNG、管道及桌面初始化正常工作。默认 DACL 优先使用会话临时目录 SID。 -子进程先以挂起状态创建，加入启用了进程树清理和 UI 限制的 Job Object 后再 -恢复；任务停止或超时会终止整棵进程树。 - -此通道有意不使用 `CREATE_NO_WINDOW`，因为受限 PowerShell 和 Node 的初始化 -依赖正常控制台继承。Reasonix 的 Helper 本身仍由桌面进程启动器隐藏。 - -与 Harness 一致，此通道支持继承或忽略 stdio 的子进程，也支持 PowerShell -管道。Node/libuv 的捕获 stdio 使用命名管道，而 Windows 为该管道生成的固定默认 -安全描述符不包含限制 SID，因此 `stdio: "pipe"` 会返回 `EPERM`。Reasonix 会为 -这个运行期拒绝返回单次使用的 `denial_id`；用户明确批准后，可用 -`danger-full-access` 精确重试同一命令。Reasonix 不会为了命名管道捕获而放宽文件 -写入边界。 - -## 直接工具、禁读和并发 - -直接只读工具继续使用 AppContainer；Windows 上只有这条通道可以移除网络 -能力。`WRITE_RESTRICTED` 不隔离读取。用户显式配置的 `forbid_read` 仍使用临时 -拒绝 ACE，并保留安全描述符快照、崩溃残留标记、恢复和全生命周期串行化； -Reasonix 自己的全局凭据 `.env` 不会自动加入 Windows 的 `forbid_read`，避免 -同一用户身份的宿主设置进程被一并拒绝。凭据值仍会从模型控制的子进程环境中 -过滤，但 Windows 本地工具可以主动读取当前用户可读的文件。 - -能力 ACE 只在创建和复核时短暂持有命名互斥锁。父子路径共享锁域，避免并发 -DACL 修改丢失条目。精确 ACE 存在后，同一工作区的普通命令可以并发执行。 - -获准工作区不能包含 Reasonix 受保护状态目录，任意状态子目录也不能成为写入 -根。唯一例外是桌面的直属 `/global-workspace`；能力只授予该精确子目录， -不会授予父目录。会话临时目录必须同时与工作区、受保护目录分离。 - -## 从旧 Windows 后端迁移 - -凭据读取先尝试正常文件访问。遇到权限拒绝时，只有匹配的沙箱崩溃残留记录能够证明 -所属进程已退出，且没有对应的活跃或无法确认状态的记录，才会移除全局 `.env` 上唯一 -一条当前用户的显式 `DENY RX`。修复会获取沙箱路径锁并重新检查 ACL。缺少残留记录、 -存在其他当前用户拒绝条目或不支持的 ACE 类型时不会自动修复，权限错误仍会保留。 -仅凭相同权限掩码无法确定该规则属于 Reasonix。 -修复成功后会消费死进程的残留记录，避免它以后授权删除无关的 ACL 变更。 - -旧 Shell 通道使用低完整性令牌，并递归修改工作区完整性标签。新通道不再给 -工作区降完整性级别，也不再在每次命令结束后恢复大型目录树。为直接工具保留 -的 AppContainer 兼容代码仍可能使用原有 ACL 和完整性标签处理。 - -新 Shell 通道不依赖旧低完整性标签。能力 ACE 只有在令牌携带对应派生 SID -时才有效，因此可以跨版本保留；删除工作区或会话临时目录时，对应 ACE 会随 -目录一并删除。 - -Helper 载荷带严格版本号；缺少版本、未来版本、未知字段或尾随数据都会被 -拒绝。原生 API 缺失、路径身份变化、受保护目录冲突或无法实现的网络约束都会 -返回明确的沙箱失败，Reasonix 不会静默改为无沙箱重试。 - -## Windows 已知边界 - -- 受限令牌约束文件写入类访问，不隔离普通读取、网络、注册表、命名对象或 - 进程可见性。 -- 为兼容 Windows 运行时，Everyone 仍在限制集合中。若某个位置本身明确允许 - Everyone 写入，该位置可能仍可写。 -- 对 NTFS 硬链接和特殊 ACL 继承对象仍需原生回归验证。当前会复核目录对象 - 身份，但不宣称具备通用硬链接隔离能力。 -- AppContainer 可以禁止直接工具联网。Shell 或写入进程要求 `Network: false` - 时会失败关闭，因为受限令牌通道无法执行该网络边界。 -- 即使全部原生 API 可用，Windows 的强制等级仍为 `partial`。 - -## 验证方式 - -在每个支持的 Windows 架构上运行原生测试和 100 次冷启动／热启动基准： - -```powershell -./scripts/verify-windows-sandbox.ps1 -OutputDirectory .codex-build/windows-sandbox-native -``` - -CI 的 Windows smoke 组已经包含 `reasonix/internal/winsandbox`。非 Windows -主机还会为 Windows amd64 和 arm64 交叉编译 `internal/winsandbox` 与 -`internal/sandbox`。交叉编译只能验证绑定和构建标签，不能代替原生 ACL、 -令牌、Job Object、PowerShell 和 Electron 验证；执行状态记录在证据说明中。 - -本实现参考 DeepSeek Harness 的 MIT 许可模块 -`packages/sandbox/sandbox-windows-acl`，归属信息见 -`internal/winsandbox/NOTICE.md`。 diff --git a/docs/evidence/windows-sandbox/README.md b/docs/evidence/windows-sandbox/README.md deleted file mode 100644 index b28be67cbe..0000000000 --- a/docs/evidence/windows-sandbox/README.md +++ /dev/null @@ -1,26 +0,0 @@ -# Windows sandbox validation evidence - -This directory tracks the validation contract for the WriteRestricted Windows -sandbox port. Checked-in logs must identify the exact commit, Windows release, -architecture, Go version, and command. Do not label cross-compiled binaries as -native test results. - -## Required native matrix - -| Environment | Required checks | Current checked-in evidence | -| --- | --- | --- | -| Windows 11 amd64 | winsandbox/sandbox tests, 100 cold and warm launches, actual Electron shell flow | Not run in this macOS worktree | -| Windows 11 arm64 | winsandbox/sandbox tests, 100 cold and warm launches, actual Electron shell flow | Not run in this macOS worktree | -| Windows 10 or supported Server amd64 | winsandbox/sandbox tests, PowerShell and background process-tree termination | Not run in this macOS worktree | - -Run `scripts/verify-windows-sandbox.ps1` on each machine and attach the emitted -metadata, test, and benchmark logs here or to the release artifact. The native -tests cover workspace/outside writes, session-temp separation, read-only mode, -standing-ACE authorization, protected roots, forbid-read cleanup, AppContainer -network denial, path replacement checks, concurrent commands after short ACL -setup, PowerShell 5/7 and available Node/Python inline runtimes, -stdio/environment/cwd, timeout, and Job Object process-tree cleanup. - -The current non-Windows validation consists of Go unit tests for portable -projection/protocol logic plus Windows amd64 and arm64 cross-compilation. It -does not exercise ACL evaluation or Windows process creation. diff --git a/internal/boot/boot.go b/internal/boot/boot.go index 15872c6cf9..1dd1afed83 100644 --- a/internal/boot/boot.go +++ b/internal/boot/boot.go @@ -2719,16 +2719,10 @@ func applyMCPIsolation(spec *plugin.Spec, workspaceRoot string, opts PluginSpecO return } writerRoots := appendUniquePaths([]string{stateDir}, opts.WriterRoots...) - readerRoots := []string{workspaceRoot} - if home, err := os.UserHomeDir(); err == nil { - readerRoots = appendUniquePaths(readerRoots, home) - } spec.Sandbox = sandbox.Spec{ Mode: "enforce", WriteRoots: writerRoots, - ReadRoots: readerRoots, - AppContainerWriteRoots: append([]string(nil), writerRoots...), - ForbidReadRoots: append([]string(nil), opts.ForbidReadRoots...), - Network: opts.Network, MinimalWrites: true, + ForbidReadRoots: append([]string(nil), opts.ForbidReadRoots...), + Network: opts.Network, MinimalWrites: true, } } diff --git a/internal/cli/cli.go b/internal/cli/cli.go index d59fb72705..6af1acb53d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -40,10 +40,10 @@ import ( "reasonix/internal/plugin" "reasonix/internal/provider" "reasonix/internal/provider/openai" - "reasonix/internal/sandbox" "reasonix/internal/serve" "reasonix/internal/sessiontemp" "reasonix/internal/telemetry" + "reasonix/internal/winaclresidue" tea "charm.land/bubbletea/v2" "github.com/spf13/pflag" @@ -66,10 +66,9 @@ func Run(args []string, version string) int { // RunWithBuildInfo is the full CLI entry with optional build metadata for // `reasonix version --verbose` / `--json`. func RunWithBuildInfo(args []string, info BuildInfo) int { - sandbox.RegisterHelperDispatch() - if len(args) > 0 && args[0] == sandbox.WindowsHelperCommand { - return sandbox.RunWindowsSandboxHelper(args[1:], os.Stdin, os.Stdout, os.Stderr) - } + // Older Windows builds could leave sandbox ACL residue behind after a + // crash; sweep it in the background so no tool output waits on icacls. + go winaclresidue.SweepStaleMarkers() info = info.withDefaults() version := info.Version // Usage recording is asynchronous so provider/UI paths never wait on disk. diff --git a/internal/config/credential_acl_repair.go b/internal/config/credential_acl_repair.go index 388a578ef0..39568d8a50 100644 --- a/internal/config/credential_acl_repair.go +++ b/internal/config/credential_acl_repair.go @@ -6,11 +6,11 @@ import ( "path/filepath" "strings" - "reasonix/internal/sandbox" + "reasonix/internal/winaclresidue" ) -// readCredentialFile keeps successful reads independent of sandbox ACL locks. -// Repair is attempted only for a denied read of the global credential store. +// readCredentialFile repairs a denied read of the global credential store +// only when a retired-sandbox marker proves the deny came from Reasonix. func readCredentialFile(path string) ([]byte, error) { data, err := os.ReadFile(path) if err == nil || !os.IsPermission(err) || runtimeGOOS != "windows" { @@ -27,7 +27,7 @@ func readCredentialFile(path string) ([]byte, error) { if real, evalErr := filepath.EvalSymlinks(path); evalErr == nil { repairPath = real } - if repairErr := sandbox.RepairLegacyCredentialDeny(repairPath); repairErr != nil { + if repairErr := winaclresidue.RepairLegacyCredentialDeny(repairPath); repairErr != nil { slog.Warn("config: legacy credential ACL repair failed", "path", path, "err", repairErr) return nil, repairErr } diff --git a/internal/plugin/plugin.go b/internal/plugin/plugin.go index a8a5019636..0afa93896c 100644 --- a/internal/plugin/plugin.go +++ b/internal/plugin/plugin.go @@ -1163,8 +1163,6 @@ func canonicalMCPRuntimeTransport(raw string) string { func canonicalMCPRuntimeSandbox(in sandbox.Spec) sandbox.Spec { in.WriteRoots = nonEmptyStrings(in.WriteRoots) - in.ReadRoots = nonEmptyStrings(in.ReadRoots) - in.AppContainerWriteRoots = nonEmptyStrings(in.AppContainerWriteRoots) in.ForbidReadRoots = nonEmptyStrings(in.ForbidReadRoots) return in } diff --git a/internal/plugin/plugin_test.go b/internal/plugin/plugin_test.go index 3dcd96c7ae..ce4cf08987 100644 --- a/internal/plugin/plugin_test.go +++ b/internal/plugin/plugin_test.go @@ -143,8 +143,7 @@ func TestMCPRuntimeSpecMatchesExactHostIdentity(t *testing.T) { LauncherLocator: "pkg@1.0.0", LauncherResolvedVersion: "1.0.0", LauncherDigest: "digest-a", ProcessMode: MCPProcessConfined, Sandbox: sandbox.Spec{ - Mode: "enforce", WriteRoots: []string{"/write"}, ReadRoots: []string{"/read"}, - AppContainerWriteRoots: []string{"/state"}, ForbidReadRoots: []string{"/secret"}, + Mode: "enforce", WriteRoots: []string{"/write"}, ForbidReadRoots: []string{"/secret"}, Network: true, MinimalWrites: true, Shell: sandbox.Shell{Kind: sandbox.ShellBash, Path: "/bin/bash"}, }, StateDir: "/state", StripRawPrefix: "db_", LowPriority: true, diff --git a/internal/plugin/transport_stdio.go b/internal/plugin/transport_stdio.go index fcc76245b2..dcec31349c 100644 --- a/internal/plugin/transport_stdio.go +++ b/internal/plugin/transport_stdio.go @@ -162,7 +162,6 @@ func prepareMCPPrivateStateForOS(s Spec, processSandbox sandbox.Spec, env []stri env = setEnvValue(env, key, value) } processSandbox.WriteRoots = append(processSandbox.WriteRoots, root, privateRoot) - processSandbox.AppContainerWriteRoots = append(processSandbox.AppContainerWriteRoots, root, privateRoot) return processSandbox, env, nil } diff --git a/internal/sandbox/helper_other.go b/internal/sandbox/helper_other.go deleted file mode 100644 index 6a000cc1da..0000000000 --- a/internal/sandbox/helper_other.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build !windows - -package sandbox - -import ( - "fmt" - "os" -) - -// RunWindowsSandboxHelper is the hidden helper entry point on Windows. Other -// platforms keep a stub so the CLI can route the internal command uniformly. -func RunWindowsSandboxHelper(_ []string, _ *os.File, _ *os.File, stderr *os.File) int { - fmt.Fprintln(stderr, "windows sandbox helper is only available on Windows") - return 2 -} diff --git a/internal/sandbox/legacy_credential_deny_other.go b/internal/sandbox/legacy_credential_deny_other.go deleted file mode 100644 index 7bb78ea170..0000000000 --- a/internal/sandbox/legacy_credential_deny_other.go +++ /dev/null @@ -1,5 +0,0 @@ -//go:build !windows - -package sandbox - -func repairLegacyCredentialDeny(string) error { return nil } diff --git a/internal/sandbox/prepare.go b/internal/sandbox/prepare.go index bc36f5c470..9993b6c4a2 100644 --- a/internal/sandbox/prepare.go +++ b/internal/sandbox/prepare.go @@ -50,10 +50,7 @@ func PrepareArgs(spec Spec, args []string, sessionTemp string) Prepared { } // PrepareShellArgs is the interactive-shell counterpart of PrepareShell. -// Windows shells always use the restricted-token lane, including read-only -// shells (ReadOnly still forbids writes). Generic read helpers use AppContainer. func PrepareShellArgs(spec Spec, args []string, sessionTemp string) Prepared { - spec.DirectWrites = true return PrepareArgs(spec, args, sessionTemp) } diff --git a/internal/sandbox/runner_diagnostics.go b/internal/sandbox/runner_diagnostics.go deleted file mode 100644 index 2dd7152c28..0000000000 --- a/internal/sandbox/runner_diagnostics.go +++ /dev/null @@ -1,67 +0,0 @@ -package sandbox - -import ( - "encoding/json" - "errors" - "fmt" - "io" -) - -// RunnerFailure is created only from the host-owned helper report channel, -// never from a command's stdout/stderr or its numeric exit status. -type RunnerFailure struct { - Phase string - Detail string - Cause error -} - -func (e *RunnerFailure) Error() string { return e.Detail } -func (e *RunnerFailure) Unwrap() error { return e.Cause } - -func RunnerFailureFromError(err error) (string, string, bool) { - var failure *RunnerFailure - if errors.As(err, &failure) { - return failure.Phase, failure.Detail, true - } - return "", "", false -} - -const runnerReportLimit = 4096 - -type runnerReport struct { - Version int `json:"version"` - Phase string `json:"phase"` - Detail string `json:"detail"` -} - -func writeRunnerReport(w io.Writer, phase string, err error) error { - if w == nil { - return nil - } - detail := err.Error() - if len(detail) > 512 { - detail = detail[:512] - } - return json.NewEncoder(w).Encode(runnerReport{Version: 1, Phase: phase, Detail: detail}) -} - -func readRunnerReport(r io.Reader, cause error) error { - if cause == nil { - return nil - } - data, err := io.ReadAll(io.LimitReader(r, runnerReportLimit+1)) - if err != nil || len(data) == 0 { - return cause - } - var report runnerReport - if len(data) > runnerReportLimit || json.Unmarshal(data, &report) != nil || report.Version != 1 { - // Invalid/incomplete reports cannot prove the command never started. - return fmt.Errorf("invalid sandbox runner report: %w", cause) - } - switch report.Phase { - case WindowsSandboxFailureAuthorization, WindowsSandboxFailureDependency, WindowsSandboxFailureLaunch: - return &RunnerFailure{Phase: report.Phase, Detail: report.Detail, Cause: cause} - default: - return fmt.Errorf("unknown sandbox runner phase: %w", cause) - } -} diff --git a/internal/sandbox/runner_diagnostics_other.go b/internal/sandbox/runner_diagnostics_other.go deleted file mode 100644 index 82dc2009fb..0000000000 --- a/internal/sandbox/runner_diagnostics_other.go +++ /dev/null @@ -1,9 +0,0 @@ -//go:build !windows - -package sandbox - -import "os/exec" - -func PrepareRunnerDiagnostics(_ *exec.Cmd) (func(error) error, error) { - return func(err error) error { return err }, nil -} diff --git a/internal/sandbox/runner_diagnostics_test.go b/internal/sandbox/runner_diagnostics_test.go deleted file mode 100644 index 3b892f851d..0000000000 --- a/internal/sandbox/runner_diagnostics_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package sandbox - -import ( - "bytes" - "errors" - "strings" - "testing" -) - -func TestRunnerReportRequiresCompleteTypedRecord(t *testing.T) { - cause := errors.New("exit status 126") - for _, phase := range []string{WindowsSandboxFailureAuthorization, WindowsSandboxFailureDependency, WindowsSandboxFailureLaunch} { - var report bytes.Buffer - if err := writeRunnerReport(&report, phase, errors.New("failed "+phase)); err != nil { - t.Fatal(err) - } - result := readRunnerReport(&report, cause) - got, _, ok := RunnerFailureFromError(result) - if !ok || got != phase || !errors.Is(result, cause) { - t.Fatalf("phase=%s err=%v", got, result) - } - } - for _, data := range []string{"", `{"version":1`, `{"version":2,"phase":"launch"}`, `{"version":1,"phase":"execution"}`, strings.Repeat("x", runnerReportLimit+1)} { - err := readRunnerReport(strings.NewReader(data), cause) - if _, _, ok := RunnerFailureFromError(err); ok { - t.Fatalf("untrusted/incomplete record accepted: %q", data) - } - if !errors.Is(err, cause) { - t.Fatalf("original exit lost: %v", err) - } - } -} diff --git a/internal/sandbox/runner_diagnostics_windows.go b/internal/sandbox/runner_diagnostics_windows.go deleted file mode 100644 index 9fed20a98d..0000000000 --- a/internal/sandbox/runner_diagnostics_windows.go +++ /dev/null @@ -1,80 +0,0 @@ -//go:build windows - -package sandbox - -import ( - "crypto/rand" - "fmt" - "io" - "os" - "os/exec" - "path/filepath" - "strconv" - "strings" - "syscall" - "unsafe" - - "golang.org/x/sys/windows" -) - -const runnerReportEnvironment = "REASONIX_INTERNAL_RUNNER_REPORT_HANDLE" - -// PrepareRunnerDiagnostics gives only the helper an inherited report handle. -// The temporary file has sharing disabled and is deleted on last close, so the -// restricted child cannot reopen it even if the temp directory is writable. -// A file rather than a pipe also makes reads bounded after cancellation: a -// stuck helper cannot hold the caller waiting for EOF. No path enters argv/env. -func PrepareRunnerDiagnostics(cmd *exec.Cmd) (func(error) error, error) { - if len(cmd.Args) < 4 || cmd.Args[1] != WindowsHelperCommand || cmd.Args[3] != "--" { - return func(err error) error { return err }, nil - } - path, err := windows.UTF16PtrFromString(filepath.Join(os.TempDir(), "reasonix-runner-"+rand.Text())) - if err != nil { - return nil, err - } - sa := &windows.SecurityAttributes{Length: uint32(unsafe.Sizeof(windows.SecurityAttributes{})), InheritHandle: 1} - handle, err := windows.CreateFile(path, windows.GENERIC_READ|windows.GENERIC_WRITE, 0, sa, windows.CREATE_NEW, windows.FILE_ATTRIBUTE_TEMPORARY|windows.FILE_FLAG_DELETE_ON_CLOSE, 0) - if err != nil { - return nil, err - } - f := os.NewFile(uintptr(handle), "sandbox runner report") - if cmd.SysProcAttr == nil { - cmd.SysProcAttr = &syscall.SysProcAttr{} - } - cmd.SysProcAttr.AdditionalInheritedHandles = append(cmd.SysProcAttr.AdditionalInheritedHandles, syscall.Handle(handle)) - env := cmd.Env - if env == nil { - env = os.Environ() - } - cmd.Env = make([]string, 0, len(env)+1) - for _, entry := range env { - key, _, _ := strings.Cut(entry, "=") - if !strings.EqualFold(key, runnerReportEnvironment) { - cmd.Env = append(cmd.Env, entry) - } - } - cmd.Env = append(cmd.Env, runnerReportEnvironment+"="+strconv.FormatUint(uint64(handle), 10)) - return func(cause error) error { - defer f.Close() - return readRunnerReport(io.NewSectionReader(f, 0, runnerReportLimit+1), cause) - }, nil -} - -func openRunnerReport() (*os.File, error) { - value := os.Getenv(runnerReportEnvironment) - _ = os.Unsetenv(runnerReportEnvironment) - if value == "" { - return nil, nil - } // Standalone helper invocation. - n, err := strconv.ParseUint(value, 10, 64) - if err != nil || n == 0 { - return nil, fmt.Errorf("invalid runner report handle") - } - handle := windows.Handle(n) - // Clear inheritance before any child can start. winsandbox also passes an - // explicit allowlist containing only duplicates of the three stdio handles. - if err := windows.SetHandleInformation(handle, windows.HANDLE_FLAG_INHERIT, 0); err != nil { - return nil, err - } - return os.NewFile(uintptr(handle), "sandbox runner report"), nil -} diff --git a/internal/sandbox/runner_diagnostics_windows_test.go b/internal/sandbox/runner_diagnostics_windows_test.go deleted file mode 100644 index 94c7720277..0000000000 --- a/internal/sandbox/runner_diagnostics_windows_test.go +++ /dev/null @@ -1,90 +0,0 @@ -//go:build windows - -package sandbox - -import ( - "errors" - "fmt" - "os" - "os/exec" - "strings" - "testing" - - "golang.org/x/sys/windows" -) - -func TestRunnerDiagnosticsProcess(t *testing.T) { - mode := os.Getenv("REASONIX_TEST_RUNNER_REPORT") - if mode == "" { - return - } - report, err := openRunnerReport() - if err != nil || report == nil { - fmt.Fprintln(os.Stderr, "missing report", err) - os.Exit(90) - } - if os.Getenv(runnerReportEnvironment) != "" { - os.Exit(91) - } - if mode == "replay" { - fmt.Fprintln(os.Stderr, "__reasonix_windows_sandbox_failure__:old-payload:authorization") - } else { - if err := writeRunnerReport(report, mode, errors.New("test runner failure")); err != nil { - os.Exit(92) - } - } - _ = report.Close() - os.Exit(126) -} - -func TestWindowsRunnerReportIsPrivateAndNotCommandOutput(t *testing.T) { - for _, phase := range []string{"authorization", "dependency", "launch", "replay"} { - t.Run(phase, func(t *testing.T) { - cmd := exec.Command(os.Args[0], "-test.run=^TestRunnerDiagnosticsProcess$") - cmd.Env = append(os.Environ(), "REASONIX_TEST_RUNNER_REPORT="+phase) - // Test binaries do not dispatch the private CLI entry point. Keep - // the production report setup, then dispatch to this test helper. - args := cmd.Args - cmd.Args = []string{args[0], WindowsHelperCommand, "payload", "--", "pwsh"} - finish, err := PrepareRunnerDiagnostics(cmd) - if err != nil { - t.Fatal(err) - } - cmd.Args = args - handle := windows.Handle(cmd.SysProcAttr.AdditionalInheritedHandles[0]) - buf := make([]uint16, 32768) - n, err := windows.GetFinalPathNameByHandle(handle, &buf[0], uint32(len(buf)), 0) - if err != nil || n >= uint32(len(buf)) { - _ = finish(err) - t.Fatalf("report path: %v", err) - } - path := windows.UTF16ToString(buf[:n]) - if file, err := os.OpenFile(path, os.O_WRONLY, 0); err == nil { - file.Close() - _ = finish(nil) - t.Fatal("report could be reopened by name") - } - output, runErr := cmd.CombinedOutput() - err = finish(runErr) - got, _, trusted := RunnerFailureFromError(err) - if phase == "replay" { - if trusted || !strings.Contains(string(output), "__reasonix_windows_sandbox_failure__") { - t.Fatalf("command output became trusted: %v %s", err, output) - } - } else if !trusted || got != phase { - t.Fatalf("phase=%s trusted=%v err=%v output=%s", got, trusted, err, output) - } - if _, err := os.Stat(path); !os.IsNotExist(err) { - t.Fatalf("report file survived last close: %v", err) - } - }) - } -} - -func TestRuntimeErrorTextCannotClaimPreCommandFailure(t *testing.T) { - for _, text := range []string{"cleanup API unavailable", "wait not implemented"} { - if _, before := windowsSandboxFailurePhase(errors.New(text)); before { - t.Fatalf("post-start error misclassified: %s", text) - } - } -} diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go index 65b1488e8e..f81f4bda91 100644 --- a/internal/sandbox/sandbox.go +++ b/internal/sandbox/sandbox.go @@ -8,34 +8,7 @@ // OSSandboxSupported). File-writer built-ins are confined in tool/builtin. package sandbox -import ( - "runtime" - "sync/atomic" - "time" -) - -// WindowsHelperCommand is the private subprocess entry point for the native -// Windows sandbox. Host binaries must register and dispatch it before normal -// startup so an enforced launch can never fall through into an unconfined GUI -// or CLI process. -const WindowsHelperCommand = "__reasonix_windows_sandbox" - -var helperDispatchRegistered atomic.Bool - -func RegisterHelperDispatch() { helperDispatchRegistered.Store(true) } - -// RepairLegacyCredentialDeny removes only the exact current-user deny ACE that -// older Windows builds placed on Reasonix's credential file. Other platforms -// and unrelated ACLs are left untouched. -func RepairLegacyCredentialDeny(path string) error { - return repairLegacyCredentialDeny(path) -} - -const ( - WindowsSandboxFailureAuthorization = "authorization" - WindowsSandboxFailureDependency = "dependency" - WindowsSandboxFailureLaunch = "launch" -) +import "runtime" // Spec describes how to confine one command. The zero value (Mode == "") does // not enforce, so an unconfigured caller runs commands unchanged. @@ -50,18 +23,6 @@ type Spec struct { // plus any configured extras). Platforms may add command-scoped temp/cache // roots so builds and package managers keep working without broad writes. WriteRoots []string - // ReadRoots are explicit host paths a Windows AppContainer may read. The - // macOS/Linux profiles already mount the host read-only by default. - ReadRoots []string - // AppContainerWriteRoots are the small subset of WriteRoots that a - // read-only Windows AppContainer may write (for MCP this is only its - // private state/temp tree). macOS and Linux already enforce this through - // WriteRoots and ignore this platform-specific distinction. - AppContainerWriteRoots []string - // DirectWrites marks a raw-argv launch as a write-capable command. On - // Windows this selects the WRITE_RESTRICTED writer lane; it is deliberately - // false for ordinary read-only helpers such as rg. - DirectWrites bool // ForbidReadRoots are files or directories the command may not read from // when confined. The OS sandbox denies access to these paths (macOS Seatbelt // deny file-read* rules, Linux bubblewrap masks); on other platforms the @@ -90,10 +51,6 @@ type Spec struct { // even when a broader WriteRoot such as the user's home directory would // otherwise cover them. ProtectedWriteRoots []string - // WindowsLockWait bounds native ACL coordination. A short foreground - // default avoids hanging an approval; background launches may opt into a - // larger value. Other platforms ignore it. - WindowsLockWait time.Duration } // Enforce reports whether the spec asks for confinement. diff --git a/internal/sandbox/seatbelt_windows.go b/internal/sandbox/seatbelt_windows.go index 2941e8fef7..b99167b6b5 100644 --- a/internal/sandbox/seatbelt_windows.go +++ b/internal/sandbox/seatbelt_windows.go @@ -2,184 +2,18 @@ package sandbox -import ( - "bytes" - "encoding/base64" - "encoding/json" - "errors" - "fmt" - "io" - "os" - "strings" +// Windows has no OS-level shell sandbox (see OSSandboxSupported), so every +// launch runs unwrapped as the current OS user. - "reasonix/internal/winsandbox" -) - -type windowsSandboxPayload struct { - Version int `json:"version"` - Spec Spec `json:"spec"` - Writable bool `json:"writable"` +// Command returns the shell invocation unwrapped. +func Command(_ Spec, sh Shell, command string) ([]string, bool) { + return sh.argv(command), false } -const windowsSandboxPayloadVersion = 1 - -// Command returns the shell invocation unwrapped: the Windows backend is -// retired from enforcement (Available is always false), so the helper argv is -// never produced. The wrapping code remains only until the backend is removed. -func Command(spec Spec, sh Shell, command string) ([]string, bool) { - if ValidateShellPolicy(spec, sh) != nil { - return nil, false - } - // Shells must remain able to read ordinary user files, so they use the - // restricted-token lane in every constrained preset. Read-only receives no - // workspace write ACL; workspace access receives only its canonical roots. - return windowsSandboxCommand(spec, sh.argv(command), true) +// CommandArgs returns the raw argv unwrapped. +func CommandArgs(_ Spec, args []string) ([]string, bool) { + return args, false } -// CommandArgs is like Command but accepts the command as raw argv instead of a -// shell command string. -func CommandArgs(spec Spec, args []string) ([]string, bool) { - return windowsSandboxCommand(spec, args, spec.DirectWrites) -} - -func windowsSandboxCommand(spec Spec, args []string, writable bool) ([]string, bool) { - // Mirror the darwin/linux wrappers: when the backend (or this binary's - // helper dispatch) is unavailable, return the argv unwrapped so the bash - // tool takes its explicit fail-closed / escape-approval path instead of - // spawning a helper that cannot work. - if !spec.Enforce() || !Available() { - return args, false - } - exe, err := os.Executable() - if err != nil || exe == "" { - return args, false - } - payload, err := encodeWindowsSandboxPayload(windowsSandboxPayload{Spec: spec, Writable: writable}) - if err != nil { - return args, false - } - out := []string{exe, WindowsHelperCommand, payload, "--"} - out = append(out, args...) - return out, true -} - -// Available is always false on Windows: the restricted-token/AppContainer -// backend is retired from enforcement (see OSSandboxSupported). Reporting it -// as unavailable keeps every wrapper, MCP confinement, and capability surface -// on the unwrapped path without a second platform switch, and it guarantees -// the helper never mutates ACLs or creates restricted tokens on user machines. +// Available is always false on Windows. func Available() bool { return false } - -func repairLegacyCredentialDeny(path string) error { - return winsandbox.RepairLegacyCredentialDeny(path) -} - -func encodeWindowsSandboxPayload(payload windowsSandboxPayload) (string, error) { - payload.Version = windowsSandboxPayloadVersion - b, err := json.Marshal(payload) - if err != nil { - return "", err - } - return base64.RawURLEncoding.EncodeToString(b), nil -} - -func decodeWindowsSandboxPayload(s string) (windowsSandboxPayload, error) { - var payload windowsSandboxPayload - b, err := base64.RawURLEncoding.DecodeString(s) - if err != nil { - return payload, err - } - decoder := json.NewDecoder(bytes.NewReader(b)) - decoder.DisallowUnknownFields() - if err := decoder.Decode(&payload); err != nil { - return payload, err - } - var trailing any - if err := decoder.Decode(&trailing); err != io.EOF { - if err == nil { - return payload, fmt.Errorf("multiple JSON values") - } - return payload, err - } - if payload.Version != windowsSandboxPayloadVersion { - return payload, fmt.Errorf("unsupported payload version %d", payload.Version) - } - return payload, nil -} - -// RunWindowsSandboxHelper is the hidden process entry point used by Command and -// CommandArgs on native Windows. -func RunWindowsSandboxHelper(args []string, stdin *os.File, stdout *os.File, stderr *os.File) int { - report, reportErr := openRunnerReport() - if reportErr != nil { - fmt.Fprintln(stderr, "windows sandbox report channel:", reportErr) - return 126 - } - if report != nil { - defer report.Close() - } - reportFailure := func(phase string, err error) int { - if report != nil { - _ = writeRunnerReport(report, phase, err) - } - fmt.Fprintln(stderr, "windows sandbox:", err) - return 126 - } - if len(args) < 3 || args[1] != "--" { - return reportFailure(WindowsSandboxFailureLaunch, fmt.Errorf("usage: reasonix %s -- [args...]", WindowsHelperCommand)) - } - payload, err := decodeWindowsSandboxPayload(args[0]) - if err != nil { - return reportFailure(WindowsSandboxFailureDependency, fmt.Errorf("invalid windows sandbox payload: %w", err)) - } - child := args[2:] - if len(child) == 0 || strings.TrimSpace(child[0]) == "" { - return reportFailure(WindowsSandboxFailureLaunch, fmt.Errorf("windows sandbox command is required")) - } - result, err := winsandbox.Run(convertWindowsSandboxSpec(payload.Spec, payload.Writable), child, winsandbox.RunOptions{ - Stdin: stdin, - Stdout: stdout, - Stderr: stderr, - }) - if err != nil { - if phase, preCommand := windowsSandboxFailurePhase(err); preCommand { - return reportFailure(phase, err) - } else { - // The process was resumed before this failure. Keep it in the normal - // execution lane so mutation risk remains may_be_partial. - fmt.Fprintln(stderr, "windows sandbox:", err) - } - return 126 - } - return result.ExitCode -} - -func windowsSandboxFailurePhase(err error) (string, bool) { - if phase, ok := winsandbox.FailurePhaseOf(err); ok { - return string(phase), true - } - if errors.Is(err, winsandbox.ErrUnsupported) { - return WindowsSandboxFailureDependency, true - } - return "", false -} - -func convertWindowsSandboxSpec(spec Spec, writable bool) winsandbox.Spec { - roots := spec.WriteRoots - if spec.ReadOnly { - roots = spec.AppContainerWriteRoots - } - readableRoots := append([]string(nil), spec.ReadRoots...) - return winsandbox.Spec{ - ReadableRoots: readableRoots, - WritableRoots: append([]string(nil), roots...), - ForbidReadRoots: append([]string(nil), spec.ForbidReadRoots...), - ProtectedWriteRoots: append([]string(nil), spec.ProtectedWriteRoots...), - Network: spec.Network, - Writable: writable, - ReadOnly: spec.ReadOnly, - TempDir: spec.SessionTemp, - TempPrefix: "reasonix-sandbox-", - LockWait: spec.WindowsLockWait, - } -} diff --git a/internal/sandbox/seatbelt_windows_test.go b/internal/sandbox/seatbelt_windows_test.go deleted file mode 100644 index 1468e3a07c..0000000000 --- a/internal/sandbox/seatbelt_windows_test.go +++ /dev/null @@ -1,91 +0,0 @@ -//go:build windows - -package sandbox - -import ( - "encoding/base64" - "testing" -) - -// The Windows backend is retired from enforcement: even with the helper -// dispatch registered and the native APIs present, no launch path may wrap a -// command, and the effective spec must never demand confinement. -func TestWindowsNeverWrapsCommands(t *testing.T) { - RegisterHelperDispatch() - if Available() { - t.Fatal("Available() must be false on Windows") - } - if OSSandboxSupported() { - t.Fatal("OSSandboxSupported() must be false on Windows") - } - spec := Spec{Mode: "enforce", WriteRoots: []string{`C:\work`}, Network: true} - sh := Shell{Kind: ShellPowerShell, Path: "powershell"} - if argv, wrapped := Command(spec, sh, "Write-Output ok"); wrapped || len(argv) == 0 || argv[0] != sh.Path { - t.Fatalf("Command wrapped=%v argv=%v", wrapped, argv) - } - if argv, wrapped := CommandArgs(spec, []string{`C:\tools\rg.exe`, "needle"}); wrapped || argv[0] != `C:\tools\rg.exe` { - t.Fatalf("CommandArgs wrapped=%v argv=%v", wrapped, argv) - } - for _, launch := range []Prepared{ - PrepareShell(spec, sh, "exit 0", `C:\private-temp`), - PrepareShellArgs(spec, []string{sh.Path, "-NoLogo", "-NoProfile"}, `C:\private-temp`), - } { - if launch.Wrapped { - t.Fatalf("prepared launch wrapped: %+v", launch) - } - } -} - -func TestConvertWindowsSandboxSpec(t *testing.T) { - sessionTemp := `C:\private\session-temp` - protected := `C:\Users\agent\AppData\Roaming\Reasonix` - spec := Spec{ - Mode: "enforce", - WriteRoots: []string{`C:\work`}, - ForbidReadRoots: []string{`C:\work\secret`}, - ProtectedWriteRoots: []string{protected}, - SessionTemp: sessionTemp, - Network: true, - } - got := convertWindowsSandboxSpec(spec, true) - if !got.Writable || !got.Network || got.TempPrefix != "reasonix-sandbox-" { - t.Fatalf("converted flags = %+v", got) - } - if len(got.WritableRoots) != 1 || got.WritableRoots[0] != spec.WriteRoots[0] { - t.Fatalf("converted writable roots = %v", got.WritableRoots) - } - if len(got.ForbidReadRoots) != 1 || got.ForbidReadRoots[0] != spec.ForbidReadRoots[0] { - t.Fatalf("converted forbid roots = %v", got.ForbidReadRoots) - } - if got.TempDir != sessionTemp { - t.Fatalf("converted temp dir = %q, want %q", got.TempDir, sessionTemp) - } - if len(got.ProtectedWriteRoots) != 1 || got.ProtectedWriteRoots[0] != protected { - t.Fatalf("converted protected roots = %v", got.ProtectedWriteRoots) - } - if len(got.WritableRoots) != 1 { - t.Fatalf("session temp must remain a separate capability, got roots %v", got.WritableRoots) - } - - got.WritableRoots[0] = `C:\mutated` - got.ForbidReadRoots[0] = `C:\mutated` - if spec.WriteRoots[0] == got.WritableRoots[0] || spec.ForbidReadRoots[0] == got.ForbidReadRoots[0] { - t.Fatal("converted spec should not alias Reasonix slices") - } -} - -func TestWindowsSandboxPayloadRejectsUnknownOrStaleProtocol(t *testing.T) { - for name, raw := range map[string]string{ - "missing version": `{"spec":{},"writable":true}`, - "future version": `{"version":2,"spec":{},"writable":true}`, - "unknown field": `{"version":1,"spec":{},"writable":true,"surprise":1}`, - "trailing value": `{"version":1,"spec":{},"writable":true} {}`, - } { - t.Run(name, func(t *testing.T) { - encoded := base64.RawURLEncoding.EncodeToString([]byte(raw)) - if _, err := decodeWindowsSandboxPayload(encoded); err == nil { - t.Fatalf("decode accepted invalid payload %s", raw) - } - }) - } -} diff --git a/internal/shellrun/README.md b/internal/shellrun/README.md index fe8b22ae2d..b55d13533e 100644 --- a/internal/shellrun/README.md +++ b/internal/shellrun/README.md @@ -41,19 +41,9 @@ or sandbox enforcement are weakened by this change. 不兼容的 MSYS 子进程，所以业务命令中的此类失败仍按执行失败处理，不能声称整条命令 没有产生修改。本次改动不放宽令牌、文件权限或沙箱限制。 -Windows acceptance must run, not merely cross-compile: +The restricted-token runner has since been removed: Windows commands start +directly as the current OS user, so the MSYS/Cygwin token incompatibility no +longer applies and the boundary above is historical context only. -Windows 验收必须实际运行，交叉编译不能替代： - -``` -go test ./internal/shellrun -run TestWindowsNativeShellPreflightRetainsWriteBoundary -v -go test ./internal/winsandbox -run 'TestWindows(SandboxAllowsWorkspaceWriteAndDeniesOutside|WriteRestrictedReadOnlyAllowsReadsAndDeniesWrites|RestrictedRuntimeCompatibility)' -v -``` - -An MSYS compatibility repair additionally needs real Git Bash/grep child-process -tests in both read-only and workspace-write modes, with denied outside writes, -before changing the restricted-token implementation. This remains unqualified -until a Windows host is available. - -修改受限令牌实现前，还需要在真实 Windows 上验证 Git Bash/grep 子进程，覆盖只读与 -工作区可写两种模式，并证明工作区外写入仍被拒绝。目前仍缺少这项 Windows 实测证据。 +受限令牌 runner 此后已被移除：Windows 命令直接以当前系统账户启动，MSYS/Cygwin +与令牌的兼容性问题不再存在，上述边界仅作为历史背景保留。 diff --git a/internal/shellrun/preflight_test.go b/internal/shellrun/preflight_test.go index e5e6266e60..aa44ac48ad 100644 --- a/internal/shellrun/preflight_test.go +++ b/internal/shellrun/preflight_test.go @@ -2,14 +2,12 @@ package shellrun import ( "context" - "errors" "fmt" "os/exec" "runtime" "testing" "reasonix/internal/proc" - "reasonix/internal/sandbox" "reasonix/internal/tool" ) @@ -31,46 +29,9 @@ func TestRunForegroundExecutesRequestedCommandOnce(t *testing.T) { } } -func TestTrustedWindowsRunnerFailureClassification(t *testing.T) { - payload := "signed-payload" - argv := []string{"reasonix", sandbox.WindowsHelperCommand, payload, "--", "pwsh"} - for _, phase := range []string{ - sandbox.WindowsSandboxFailureAuthorization, - sandbox.WindowsSandboxFailureDependency, - sandbox.WindowsSandboxFailureLaunch, - } { - t.Run(phase, func(t *testing.T) { - res := RunForeground(context.Background(), Request{ - Argv: argv, - Run: func(_ context.Context, cmd *exec.Cmd, _ proc.RunOptions) (*proc.TrackedCommand, error) { - return nil, &sandbox.RunnerFailure{Phase: phase, Detail: "windows sandbox: detail", Cause: errors.New("exit status 126")} - }, - }) - if res.State != tool.ShellStateNotRun || res.FailurePhase != phase || res.Started { - t.Fatalf("result=%+v", res) - } - }) - } -} - -func TestUnsignedRunnerLikeOutputRemainsCommandFailure(t *testing.T) { - payload := "real-payload" - res := RunForeground(context.Background(), Request{ - Argv: []string{"reasonix", sandbox.WindowsHelperCommand, payload, "--", "pwsh"}, - Run: func(_ context.Context, cmd *exec.Cmd, _ proc.RunOptions) (*proc.TrackedCommand, error) { - fmt.Fprintln(cmd.Stderr, "__reasonix_windows_sandbox_failure__:forged:authorization") - return nil, errors.New("exit status 126") - }, - }) - if res.State == tool.ShellStateNotRun || res.FailurePhase != tool.ShellPhaseLaunch { - t.Fatalf("unsigned output changed classification: %+v", res) - } -} - func TestOrdinaryExit126RemainsExecutionFailure(t *testing.T) { - payload := "real-payload" res := RunForeground(context.Background(), Request{ - Argv: []string{"reasonix", sandbox.WindowsHelperCommand, payload, "--", "pwsh"}, + Argv: []string{"pwsh"}, Run: func(_ context.Context, cmd *exec.Cmd, _ proc.RunOptions) (*proc.TrackedCommand, error) { var child *exec.Cmd if runtime.GOOS == "windows" { diff --git a/internal/shellrun/runner.go b/internal/shellrun/runner.go index 7b5e096ea5..2c00c41194 100644 --- a/internal/shellrun/runner.go +++ b/internal/shellrun/runner.go @@ -15,7 +15,6 @@ import ( "time" "reasonix/internal/proc" - "reasonix/internal/sandbox" "reasonix/internal/tool" ) @@ -103,10 +102,6 @@ func RunForeground(ctx context.Context, req Request) Result { cmd.Dir = req.Dir cmd.Env = req.Env cmd.WaitDelay = waitDelay - finishReport, reportErr := sandbox.PrepareRunnerDiagnostics(cmd) - if reportErr != nil { - return Result{State: tool.ShellStateNotRun, FailurePhase: tool.ShellPhaseDependency, Err: reportErr} - } collector := newOutputCollector(combinedOutputMaxBytes, tool.OutputTailMaxBytes) var writers []io.Writer @@ -142,7 +137,6 @@ func RunForeground(ctx context.Context, req Request) Result { ShellPath: req.ShellPath, CommandPreview: req.CommandPreview, }) - err = finishReport(err) if progress != nil { progress.Flush() @@ -193,17 +187,6 @@ func classifyForegroundResult(runCtx context.Context, req Request, out Result, e out.OutputTail = "" return out } - if phase, detail, ok := sandbox.RunnerFailureFromError(err); ok { - out.Started = false - out.State = tool.ShellStateNotRun - out.FailurePhase = phase - out.ExitCode = exitCodeFromErr(err) - if detail == "" { - detail = "windows sandbox runner failed before the command started" - } - out.Err = fmt.Errorf("%s", detail) - return out - } if code := exitCodeFromErr(err); code != nil { out.ExitCode = code out.Started = true diff --git a/internal/shellrun/runner_replay_test.go b/internal/shellrun/runner_replay_test.go deleted file mode 100644 index 6825268599..0000000000 --- a/internal/shellrun/runner_replay_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package shellrun - -import ( - "context" - "crypto/sha256" - "fmt" - "os" - "os/exec" - "path/filepath" - "runtime" - "testing" - - "reasonix/internal/proc" - "reasonix/internal/sandbox" - "reasonix/internal/tool" -) - -func TestReplayedRunnerMarkerCannotEraseExecution(t *testing.T) { - payload := "same-payload-for-same-sandbox-settings" - marker := fmt.Sprintf("__reasonix_windows_sandbox_failure__:%x:authorization", sha256.Sum256([]byte(payload))) - mutated := filepath.Join(t.TempDir(), "mutated") - result := RunForeground(t.Context(), Request{ - Argv: []string{"reasonix", sandbox.WindowsHelperCommand, payload, "--", "pwsh"}, - Run: func(_ context.Context, cmd *exec.Cmd, _ proc.RunOptions) (*proc.TrackedCommand, error) { - var child *exec.Cmd - if runtime.GOOS == "windows" { - child = exec.Command("cmd", "/c", `echo changed>"`+mutated+`" & echo `+marker+` & exit /b 126`) - } else { - child = exec.Command("sh", "-c", `touch "$1"; printf '%s\n' "$2"; exit 126`, "test", mutated, marker) - } - child.Stdout, child.Stderr = cmd.Stdout, cmd.Stderr - err := child.Run() - cmd.Process, cmd.ProcessState = child.Process, child.ProcessState - return nil, err - }, - }) - if _, err := os.Stat(mutated); err != nil { - t.Fatal(err) - } - if !result.Started || result.FailurePhase != tool.ShellPhaseExecution || result.State != tool.ShellStateFailed { - t.Fatalf("mutation misreported: %+v", result) - } -} diff --git a/internal/tool/builtin/bash.go b/internal/tool/builtin/bash.go index fb9fd0e6e1..3131f33db8 100644 --- a/internal/tool/builtin/bash.go +++ b/internal/tool/builtin/bash.go @@ -243,12 +243,7 @@ func (b bash) ExecuteDetailed(ctx context.Context, args json.RawMessage) (tool.D out, runEx, err := b.runForegroundDetailed(ctx, p, sh, argv, wrapped, cmdEnv) mergeRunInto(ex, runEx) ex.DurationMs = time.Since(start).Milliseconds() - if runEx != nil && runEx.FailurePhase == tool.ShellPhaseAuthorization { - out = appendSessionDataHint(out, b.guard.CommandHint(b.workDir, p.Command)) - out = appendRunnerAuthorizationHint(out, p.Command, string(sandbox.PermissionPresetFrom(ctx))) - } else { - out = b.appendWriteHints(ctx, out, err, p, wrapped) - } + out = b.appendWriteHints(ctx, out, err, p, wrapped) return tool.DetailedResult{ Output: out, Execution: ex, @@ -520,17 +515,10 @@ func shouldTrackShellProcess(wrapped bool, sh sandbox.Shell, command string, pre if preserveBackgroundProcesses { return false } - if runtime.GOOS == "windows" && wrapped { - return false - } return !sh.Kind.IsPOSIX() || !hasExplicitBackgroundKeepalive(command) } func runShellProcess(ctx context.Context, cmd *exec.Cmd, sh sandbox.Shell, command string, track bool) (*proc.TrackedCommand, error) { - finishReport, err := sandbox.PrepareRunnerDiagnostics(cmd) - if err != nil { - return nil, &sandbox.RunnerFailure{Phase: sandbox.WindowsSandboxFailureDependency, Detail: err.Error(), Cause: err} - } source := "bash_tool" if sh.Kind == sandbox.ShellPowerShell { source = "pwsh_tool" @@ -543,7 +531,7 @@ func runShellProcess(ctx context.Context, cmd *exec.Cmd, sh sandbox.Shell, comma ShellPath: sh.Path, CommandPreview: commandPreview(command), }) - return tracked, finishReport(err) + return tracked, err } func reapShellProcess(cmd *exec.Cmd, tracked *proc.TrackedCommand) { diff --git a/internal/tool/builtin/bash_background.go b/internal/tool/builtin/bash_background.go index 0b65f9a2ec..b679721d3f 100644 --- a/internal/tool/builtin/bash_background.go +++ b/internal/tool/builtin/bash_background.go @@ -56,7 +56,6 @@ func (b bash) startBackground(ctx context.Context, p bashParams, sh sandbox.Shel } runErr = normalizeBashRunError(jobCtx, runErr, p.PreserveBackgroundProcesses) execution, classifiedErr := classifyBackgroundShellExecution(jobCtx, sh, argv, capture.String(), runErr, started) - classifiedErr = appendRunnerAuthorizationError(classifiedErr, execution, p.Command, permissionPreset) if classifiedErr != nil && execution.FailurePhase == tool.ShellPhaseExecution && wrapped { writeBackgroundSandboxHint(out, capture.String(), classifiedErr, p, jobSpec, permissionPreset) } @@ -120,18 +119,6 @@ func classifyBackgroundShellExecution(ctx context.Context, sh sandbox.Shell, arg ex.MutationRisk = tool.ShellMutationMayHaveCompleted return ex, nil } - if phase, detail, ok := sandbox.RunnerFailureFromError(runErr); ok { - ex.State = tool.ShellStateNotRun - ex.FailurePhase = phase - ex.MutationRisk = tool.ShellMutationNotStarted - if exitErr := (*exec.ExitError)(nil); errors.As(runErr, &exitErr) { - ex.ExitCode = tool.IntPtr(exitErr.ExitCode()) - } - if detail == "" { - detail = "windows sandbox runner failed before the command started" - } - return ex, fmt.Errorf("%s", detail) - } if exitErr := (*exec.ExitError)(nil); errors.As(runErr, &exitErr) { ex.State = tool.ShellStateFailed ex.FailurePhase = tool.ShellPhaseExecution diff --git a/internal/tool/builtin/bash_background_windows_test.go b/internal/tool/builtin/bash_background_windows_test.go index 4aa00f9c38..3c62eae2d1 100644 --- a/internal/tool/builtin/bash_background_windows_test.go +++ b/internal/tool/builtin/bash_background_windows_test.go @@ -21,10 +21,6 @@ import ( ) func TestWindowsWorkspaceWriteBackgroundHTTPJobLifecycle(t *testing.T) { - sandbox.RegisterHelperDispatch() - if !sandbox.Available() { - t.Skip("windows sandbox APIs unavailable") - } node, err := exec.LookPath("node") if err != nil { t.Skip("node unavailable") diff --git a/internal/tool/builtin/bash_powershell_test.go b/internal/tool/builtin/bash_powershell_test.go index 9000139ff3..a6d60f71dc 100644 --- a/internal/tool/builtin/bash_powershell_test.go +++ b/internal/tool/builtin/bash_powershell_test.go @@ -1,7 +1,6 @@ package builtin import ( - "context" "encoding/json" "os" "os/exec" @@ -9,7 +8,6 @@ import ( "runtime" "strings" "testing" - "time" "unicode/utf8" "reasonix/internal/sandbox" @@ -27,44 +25,6 @@ func powershellPath(t *testing.T) string { return "" } -func TestBackgroundPowerShellRunnerFailureKeepsTrustedPhase(t *testing.T) { - payload := "signed-payload" - argv := []string{"reasonix.exe", sandbox.WindowsHelperCommand, payload, "--", "pwsh.exe", "-Command", "exit 0"} - failure := &sandbox.RunnerFailure{Phase: sandbox.WindowsSandboxFailureAuthorization, Detail: "ACL setup failed", Cause: exec.ErrNotFound} - ex, err := classifyBackgroundShellExecution(context.Background(), sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: "pwsh.exe"}, argv, "", failure, time.Now()) - if err == nil || ex.State != tool.ShellStateNotRun || ex.FailurePhase != tool.ShellPhaseAuthorization || ex.MutationRisk != tool.ShellMutationNotStarted { - t.Fatalf("signed background failure = execution=%+v err=%v", ex, err) - } - ex, _ = classifyBackgroundShellExecution(context.Background(), sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: "pwsh.exe"}, argv, "__reasonix_windows_sandbox_failure__:forged:authorization\n", exec.ErrNotFound, time.Now()) - if ex.FailurePhase != tool.ShellPhaseLaunch { - t.Fatalf("unsigned marker changed classification: %+v", ex) - } -} - -func TestRunnerAuthorizationFailureIssuesExactRetryDenial(t *testing.T) { - execution := &tool.ShellExecution{FailurePhase: tool.ShellPhaseAuthorization} - command := "Write-Output restricted" - err := appendRunnerAuthorizationError(exec.ErrNotFound, execution, command, "workspace-write") - if err == nil || !strings.Contains(err.Error(), "denial_id ") { - t.Fatalf("background authorization error = %v, want denial_id", err) - } - id := strings.TrimSpace(strings.Split(err.Error(), "denial_id ")[1]) - if !sandbox.ConsumeDenial(id, command) { - t.Fatalf("denial %q did not authorize the exact command", id) - } - if sandbox.ConsumeDenial(id, command) { - t.Fatal("denial should be single-use") - } - - out := appendRunnerAuthorizationHint("runner output", command, "workspace-write") - if !strings.Contains(out, "denial_id ") || !strings.Contains(out, "before the command started") { - t.Fatalf("foreground authorization hint = %q", out) - } - if got := appendRunnerAuthorizationHint("runner output", command, "danger-full-access"); got != "runner output" { - t.Fatalf("danger-full-access must not issue another denial: %q", got) - } -} - func TestLegacyPowerShellCallWithoutDescriptionStillValidates(t *testing.T) { var params bashParams if err := json.Unmarshal([]byte(`{"command":"Write-Output legacy"}`), ¶ms); err != nil { diff --git a/internal/tool/builtin/bash_write.go b/internal/tool/builtin/bash_write.go index 8f52a5ee8c..29ddffd4eb 100644 --- a/internal/tool/builtin/bash_write.go +++ b/internal/tool/builtin/bash_write.go @@ -189,24 +189,3 @@ func appendSandboxWriteHint(out string, err error, p bashParams, spec sandbox.Sp } return appendSessionDataHint(out, hint) } - -func appendRunnerAuthorizationHint(out, command, preset string) string { - if strings.TrimSpace(preset) == string(permissionpreset.DangerFullAccess) { - return out - } - hint := "The Windows sandbox runner could not configure restricted authorization before the command started." - if denialID := sandbox.IssueDenial(command, preset); denialID != "" { - hint += " After explicit authorization, retry this exact command with sandbox_permissions danger-full-access and denial_id " + denialID + "." - } - return appendSessionDataHint(out, hint) -} - -func appendRunnerAuthorizationError(err error, execution *tool.ShellExecution, command, preset string) error { - if err == nil || execution == nil || execution.FailurePhase != tool.ShellPhaseAuthorization || strings.TrimSpace(preset) == string(permissionpreset.DangerFullAccess) { - return err - } - if denialID := sandbox.IssueDenial(command, preset); denialID != "" { - return fmt.Errorf("%w; after explicit authorization, retry this exact command with sandbox_permissions danger-full-access and denial_id %s", err, denialID) - } - return err -} diff --git a/internal/tool/builtin/confine.go b/internal/tool/builtin/confine.go index 86033eb830..3a6112e9b1 100644 --- a/internal/tool/builtin/confine.go +++ b/internal/tool/builtin/confine.go @@ -98,9 +98,6 @@ func RebindBashWriteRoots(tl tool.Tool, roots []string) (tool.Tool, bool) { // Sub-agent claims are strict capability boundaries. Do not add the normal // build-cache and temporary-directory allowances outside the claimed roots. spec.MinimalWrites = true - // Do not inherit a wider AppContainer write lane from the parent workspace - // confinement — the claim roots are the only allowed write surface. - spec.AppContainerWriteRoots = append([]string(nil), rs...) b.sb = spec // rootSet is preserved so later session grants still apply inside the claim. // sessionTemp is preserved: sub-agent write-root rebinding must not drop diff --git a/internal/tool/builtin/confine_test.go b/internal/tool/builtin/confine_test.go index 2c1b63dbc2..3afbcbfa31 100644 --- a/internal/tool/builtin/confine_test.go +++ b/internal/tool/builtin/confine_test.go @@ -115,9 +115,6 @@ func TestRebindBashWriteRootsUsesMinimalWriteSurface(t *testing.T) { if len(rebound.sb.WriteRoots) != 1 || rebound.sb.WriteRoots[0] != want[0] { t.Fatalf("write roots = %v, want %v", rebound.sb.WriteRoots, want) } - if len(rebound.sb.AppContainerWriteRoots) != 1 || rebound.sb.AppContainerWriteRoots[0] != want[0] { - t.Fatalf("app-container write roots = %v, want %v", rebound.sb.AppContainerWriteRoots, want) - } } func TestReboundBashCannotWriteOutsideClaim(t *testing.T) { diff --git a/internal/tool/builtin/shell_regression_test.go b/internal/tool/builtin/shell_regression_test.go index 446667fc45..bc80b0b610 100644 --- a/internal/tool/builtin/shell_regression_test.go +++ b/internal/tool/builtin/shell_regression_test.go @@ -4,7 +4,6 @@ import ( "context" "encoding/json" "io" - "os/exec" "runtime" "strings" "testing" @@ -93,17 +92,3 @@ func TestBackgroundDiagnosticCaptureKeepsFailureTail(t *testing.T) { t.Fatalf("bounded diagnostic lost failure tail: bytes=%d", len(w.String())) } } - -func TestBackgroundReplayedRunnerMarkerRemainsExecutionFailure(t *testing.T) { - var cmd *exec.Cmd - if runtime.GOOS == "windows" { - cmd = exec.Command("cmd", "/c", "exit 126") - } else { - cmd = exec.Command("sh", "-c", "exit 126") - } - err := cmd.Run() - ex, _ := classifyBackgroundShellExecution(t.Context(), sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: "pwsh"}, []string{"reasonix", sandbox.WindowsHelperCommand, "payload", "--", "pwsh"}, "__reasonix_windows_sandbox_failure__:payload:authorization", err, time.Now()) - if ex.FailurePhase != tool.ShellPhaseExecution || ex.MutationRisk != tool.ShellMutationMayBePartial { - t.Fatalf("replayed output changed classification: %+v", ex) - } -} diff --git a/internal/winaclresidue/acl_windows.go b/internal/winaclresidue/acl_windows.go new file mode 100644 index 0000000000..019b8c9276 --- /dev/null +++ b/internal/winaclresidue/acl_windows.go @@ -0,0 +1,156 @@ +//go:build windows + +package winaclresidue + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "time" + + "golang.org/x/sys/windows" + + "reasonix/internal/proc" +) + +const ( + allApplicationPackagesSID = "S-1-15-2-1" + allRestrictedApplicationPackagesSID = "S-1-15-2-2" + // stillActiveExitCode is STILL_ACTIVE from GetExitCodeProcess. + stillActiveExitCode = 259 + // icacls can stall on antivirus-scanned volumes; the bound keeps a stuck + // cleanup from pinning startup or a credential read. + icaclsTimeout = 30 * time.Second +) + +// markerDir is the directory the retired backend used for crash-residue +// markers; the name is part of the on-disk contract with older builds. +func markerDir() string { + return filepath.Join(os.TempDir(), "windows-sandbox-denylocks") +} + +// SweepStaleMarkers removes the ACEs recorded by crashed runs of the retired +// backend whose owning process is provably gone, then deletes their markers. +// It is best-effort: errors keep the marker for a later sweep and never block +// the caller. +func SweepStaleMarkers() { + dir := markerDir() + entries, err := os.ReadDir(dir) + if err != nil { + return + } + sids := residueSIDs() + for _, entry := range entries { + if entry.IsDir() { + continue + } + pid, ok := markerOwnerPID(entry.Name()) + if !ok || !processExited(pid) { + continue + } + sweepMarkerFile(filepath.Join(dir, entry.Name()), sids) + } +} + +// residueSIDs is the exact trustee set the retired backend applied; removing +// only these cannot disturb a legitimate ACL. +func residueSIDs() []string { + userSID, _ := currentProcessUserSIDString() + return dedupeSIDStrings([]string{allApplicationPackagesSID, allRestrictedApplicationPackagesSID, userSID}) +} + +// sweepMarkerFile removes the recorded ACEs and deletes the marker only when +// every removal succeeded, so a failed cleanup keeps its evidence for the next +// sweep instead of orphaning the ACE on disk. +func sweepMarkerFile(markerPath string, sids []string) { + clean := true + for _, e := range readResidueMarker(markerPath) { + if isWindowsSystemRoot(e.path) { + continue + } + if _, err := os.Stat(e.path); err != nil { + continue + } + flag := "/remove:g" + if e.kind == residueDeny { + flag = "/remove:d" + } + for _, sid := range sids { + if err := icacls(e.path, flag, "*"+sid, "/C"); err != nil { + clean = false + } + } + } + if clean { + _ = os.Remove(markerPath) + } +} + +// processExited reports whether a marker owner is provably gone. This package +// writes no markers, so one carrying our own PID was left by a dead +// predecessor after PID reuse. Access denied does not prove death. +func processExited(pidText string) bool { + if pidText == strconv.Itoa(os.Getpid()) { + return true + } + pid, err := strconv.ParseUint(pidText, 10, 32) + if err != nil || pid == 0 { + return false + } + handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) + if err != nil { + return errors.Is(err, windows.ERROR_INVALID_PARAMETER) + } + defer windows.CloseHandle(handle) + var code uint32 + if err := windows.GetExitCodeProcess(handle, &code); err != nil { + return false + } + return code != stillActiveExitCode +} + +func currentProcessUserSIDString() (string, error) { + user, err := windows.GetCurrentProcessToken().GetTokenUser() + if err != nil { + return "", err + } + if user == nil || user.User.Sid == nil { + return "", fmt.Errorf("current process token has no user SID") + } + return user.User.Sid.String(), nil +} + +// systemRootTool resolves a Windows system tool below %SystemRoot%\System32 +// so a PATH-shadowed binary can never run in its place. +func systemRootTool(name string) string { + root := os.Getenv("SystemRoot") + if root == "" { + root = os.Getenv("windir") + } + if root == "" { + root = `C:\Windows` + } + full := filepath.Join(root, "System32", name) + if _, err := os.Stat(full); err == nil { + return full + } + return name +} + +func icacls(path string, args ...string) error { + ctx, cancel := context.WithTimeout(context.Background(), icaclsTimeout) + defer cancel() + cmd := proc.CommandContext(ctx, systemRootTool("icacls.exe"), append([]string{path}, args...)...) + proc.HideWindow(cmd) + var out bytes.Buffer + cmd.Stdout, cmd.Stderr = &out, &out + if err := cmd.Run(); err != nil { + return fmt.Errorf("icacls %q %s: %w: %s", path, strings.Join(args, " "), err, strings.TrimSpace(out.String())) + } + return nil +} diff --git a/internal/winaclresidue/acl_windows_test.go b/internal/winaclresidue/acl_windows_test.go new file mode 100644 index 0000000000..e258151729 --- /dev/null +++ b/internal/winaclresidue/acl_windows_test.go @@ -0,0 +1,52 @@ +//go:build windows + +package winaclresidue + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +// A dead run's marker is swept (deny removed, marker deleted); a marker whose +// owner is still alive is left for that owner. +func TestSweepStaleMarkersRemovesDeadRunResidueOnly(t *testing.T) { + t.Setenv("TEMP", t.TempDir()) + userSID, err := currentProcessUserSIDString() + if err != nil { + t.Fatal(err) + } + dead := filepath.Join(t.TempDir(), "dead.txt") + live := filepath.Join(t.TempDir(), "live.txt") + for _, path := range []string{dead, live} { + if err := os.WriteFile(path, []byte("secret\n"), 0o600); err != nil { + t.Fatal(err) + } + installLegacyDeny(t, path, userSID) + if _, err := os.ReadFile(path); err == nil { + t.Fatalf("deny did not block reads of %s", path) + } + } + exited := exec.Command("cmd", "/c", "exit 0") + if err := exited.Run(); err != nil { + t.Fatal(err) + } + deadMarker := writeMarker(t, exited.ProcessState.Pid(), dead) + liveMarker := writeMarker(t, os.Getppid(), live) + + SweepStaleMarkers() + + if _, err := os.ReadFile(dead); err != nil { + t.Fatalf("dead run's deny survived the sweep: %v", err) + } + if _, err := os.Stat(deadMarker); !os.IsNotExist(err) { + t.Fatalf("dead marker survived the sweep: %v", err) + } + if _, err := os.ReadFile(live); err == nil { + t.Fatal("live owner's deny was removed") + } + if _, err := os.Stat(liveMarker); err != nil { + t.Fatalf("live marker was deleted: %v", err) + } +} diff --git a/internal/winsandbox/legacy_credential_deny_windows.go b/internal/winaclresidue/credential_windows.go similarity index 71% rename from internal/winsandbox/legacy_credential_deny_windows.go rename to internal/winaclresidue/credential_windows.go index acdd412676..07152fe937 100644 --- a/internal/winsandbox/legacy_credential_deny_windows.go +++ b/internal/winaclresidue/credential_windows.go @@ -1,6 +1,6 @@ //go:build windows -package winsandbox +package winaclresidue import ( "errors" @@ -8,9 +8,7 @@ import ( "os" "path/filepath" "runtime" - "strconv" "strings" - "time" "unsafe" "golang.org/x/sys/windows" @@ -18,10 +16,10 @@ import ( const legacyCredentialDenyMask = windows.ACCESS_MASK(windows.FILE_GENERIC_READ | windows.FILE_GENERIC_EXECUTE) -// RepairLegacyCredentialDeny removes the exact current-user DENY RX ACE used -// by older Reasonix forbid-read handling, provided a dead sandbox run recorded -// this path. An ACE's mask alone cannot establish its origin. Ambiguous ACLs -// and live or unverifiable marker owners are never modified. +// RepairLegacyCredentialDeny removes the exact current-user DENY RX ACE that +// the retired backend placed on the credential file, provided a dead run's +// marker recorded this path. A mask alone cannot prove origin, so ambiguous +// ACLs and markers with live or unverifiable owners are never modified. func RepairLegacyCredentialDeny(path string) error { if path == "" { return nil @@ -32,14 +30,6 @@ func RepairLegacyCredentialDeny(path string) error { } return err } - canonicalPath := canonicalWindowsPath(path) - - lock, err := lockWindowsRoots([]string{path}, nil, "legacy credential ACL repair", 2*time.Second) - if err != nil { - return fmt.Errorf("lock legacy credential ACL repair: %w", err) - } - defer lock.release() - userSID, err := currentProcessUserSIDString() if err != nil { return err @@ -54,7 +44,7 @@ func RepairLegacyCredentialDeny(path string) error { if other != 0 || legacy != 1 { return fmt.Errorf("refusing to alter non-legacy current-user deny ACL on %q", path) } - markers := staleCredentialDenyMarkers(path, canonicalPath) + markers := staleCredentialDenyMarkers(path, canonicalWindowsPath(path)) if len(markers) == 0 { return fmt.Errorf("refusing to alter credential deny ACL without a stale sandbox record on %q", path) } @@ -62,8 +52,9 @@ func RepairLegacyCredentialDeny(path string) error { removeErr := icacls(path, "/remove:d", "*"+userSID, "/C") remainingLegacy, remainingOther, verifyErr := currentUserDenyACECounts(path, userSID) if verifyErr == nil && remainingLegacy == 0 && remainingOther == 0 { + sids := residueSIDs() for _, marker := range markers { - sweepResidueMarkerFile(marker, sandboxResidueSIDs()) + sweepMarkerFile(marker, sids) } return nil } @@ -76,18 +67,20 @@ func RepairLegacyCredentialDeny(path string) error { return fmt.Errorf("legacy credential deny ACL remains on %q", path) } +// staleCredentialDenyMarkers returns the markers of exited runs that recorded +// a deny on path. Any marker with a live or unverifiable owner vetoes repair. func staleCredentialDenyMarkers(path, canonicalPath string) []string { - entries, err := os.ReadDir(windowsDenyMarkerDir()) + entries, err := os.ReadDir(markerDir()) if err != nil { return nil } var found []string for _, entry := range entries { - pid, ok := windowsDenyMarkerOwnerPID(entry.Name()) + pid, ok := markerOwnerPID(entry.Name()) if entry.IsDir() || !ok { continue } - marker := filepath.Join(windowsDenyMarkerDir(), entry.Name()) + marker := filepath.Join(markerDir(), entry.Name()) for _, residue := range readResidueMarker(marker) { if residue.kind != residueDeny { continue @@ -96,7 +89,7 @@ func staleCredentialDenyMarkers(path, canonicalPath string) []string { !strings.EqualFold(canonicalWindowsPath(residue.path), canonicalPath) { continue } - if !credentialMarkerOwnerExited(marker, pid) { + if !processExited(pid) { return nil } found = append(found, marker) @@ -107,8 +100,8 @@ func staleCredentialDenyMarkers(path, canonicalPath string) []string { } // canonicalWindowsPath expands 8.3 aliases without opening the protected file. -// Keep the original spelling on failure so the exact-path comparison remains -// available when a volume does not support long-name expansion. +// The original spelling is kept on failure so the exact-path comparison still +// works on volumes without long-name expansion. func canonicalWindowsPath(path string) string { input, err := windows.UTF16PtrFromString(filepath.Clean(path)) if err != nil { @@ -122,25 +115,6 @@ func canonicalWindowsPath(path string) string { return filepath.Clean(windows.UTF16ToString(buffer[:n])) } -func credentialMarkerOwnerExited(marker, pidText string) bool { - if pidText == strconv.Itoa(os.Getpid()) { - _, live := liveResidueMarkers.Load(marker) - return !live - } - pid, err := strconv.ParseUint(pidText, 10, 32) - if err != nil || pid == 0 { - return false - } - handle, err := windows.OpenProcess(windows.SYNCHRONIZE, false, uint32(pid)) - if err != nil { - // Access denied does not prove the owner is dead. - return errors.Is(err, windows.ERROR_INVALID_PARAMETER) - } - defer windows.CloseHandle(handle) - event, err := windows.WaitForSingleObject(handle, 0) - return err == nil && event == windows.WAIT_OBJECT_0 -} - func currentUserDenyACECounts(path, userSID string) (legacy, other int, err error) { sid, err := windows.StringToSid(userSID) if err != nil { diff --git a/internal/winsandbox/legacy_credential_deny_windows_test.go b/internal/winaclresidue/credential_windows_test.go similarity index 76% rename from internal/winsandbox/legacy_credential_deny_windows_test.go rename to internal/winaclresidue/credential_windows_test.go index fdabed6925..99c88ff22b 100644 --- a/internal/winsandbox/legacy_credential_deny_windows_test.go +++ b/internal/winaclresidue/credential_windows_test.go @@ -1,6 +1,6 @@ //go:build windows -package winsandbox +package winaclresidue import ( "fmt" @@ -14,6 +14,29 @@ import ( "golang.org/x/sys/windows" ) +func installLegacyDeny(t *testing.T, path, userSID string) { + t.Helper() + if err := icacls(path, "/deny", "*"+userSID+":(RX)"); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = icacls(path, "/remove:d", "*"+userSID, "/C") }) +} + +// writeMarker records a deny for path under a marker owned by pid. Our own PID +// stands for a crashed predecessor after PID reuse; the parent test runner's +// PID stands for a live owner. +func writeMarker(t *testing.T, pid int, path string) string { + t.Helper() + if err := os.MkdirAll(markerDir(), 0o700); err != nil { + t.Fatal(err) + } + marker := filepath.Join(markerDir(), strconv.Itoa(pid)+"-credential-test.txt") + if err := os.WriteFile(marker, []byte("deny\t"+path+"\n"), 0o600); err != nil { + t.Fatal(err) + } + return marker +} + func TestRepairLegacyCredentialDenyRemovesExactCurrentUserACE(t *testing.T) { t.Setenv("TEMP", t.TempDir()) path := filepath.Join(t.TempDir(), ".env") @@ -24,14 +47,12 @@ func TestRepairLegacyCredentialDenyRemovesExactCurrentUserACE(t *testing.T) { if err != nil { t.Fatal(err) } - if err := denyAppContainerSIDsWithInheritance(path, []string{userSID}, "RX", false); err != nil { - t.Fatal(err) - } + installLegacyDeny(t, path, userSID) legacy, other, err := currentUserDenyACECounts(path, userSID) if err != nil || legacy != 1 || other != 0 { t.Fatalf("deny counts before repair = legacy:%d other:%d err:%v", legacy, other, err) } - marker := writeStaleCredentialMarker(t, path) + marker := writeMarker(t, os.Getpid(), path) if err := RepairLegacyCredentialDeny(path); err != nil { t.Fatal(err) @@ -46,29 +67,12 @@ func TestRepairLegacyCredentialDenyRemovesExactCurrentUserACE(t *testing.T) { if _, err := os.Stat(marker); !os.IsNotExist(err) { t.Fatalf("stale marker survived repair: %v", err) } - if err := denyAppContainerSIDsWithInheritance(path, []string{userSID}, "RX", false); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { removeDeniedAppContainerSIDs(path, []string{userSID}) }) + installLegacyDeny(t, path, userSID) if err := RepairLegacyCredentialDeny(path); err == nil { t.Fatal("repair reused a consumed stale marker") } } -func writeStaleCredentialMarker(t *testing.T, path string) string { - t.Helper() - if err := os.MkdirAll(windowsDenyMarkerDir(), 0o700); err != nil { - t.Fatal(err) - } - // An unregistered self-PID marker represents a crashed predecessor after - // PID reuse, matching the existing sandbox residue lifecycle. - marker := filepath.Join(windowsDenyMarkerDir(), strconv.Itoa(os.Getpid())+"-credential-test.txt") - if err := os.WriteFile(marker, []byte("deny\t"+path+"\n"), 0o600); err != nil { - t.Fatal(err) - } - return marker -} - func TestRepairLegacyCredentialDenyMatchesFileAcrossPathAliases(t *testing.T) { tmp := t.TempDir() t.Setenv("TMP", tmp) @@ -94,11 +98,8 @@ func TestRepairLegacyCredentialDenyMatchesFileAcrossPathAliases(t *testing.T) { if err != nil { t.Fatal(err) } - if err := denyAppContainerSIDsWithInheritance(path, []string{userSID}, "RX", false); err != nil { - t.Fatal(err) - } - t.Cleanup(func() { removeDeniedAppContainerSIDs(path, []string{userSID}) }) - writeStaleCredentialMarker(t, alias) + installLegacyDeny(t, path, userSID) + writeMarker(t, os.Getpid(), alias) if err := RepairLegacyCredentialDeny(path); err != nil { t.Fatal(err) } @@ -119,25 +120,21 @@ func TestRepairLegacyCredentialDenyPreservesUnattributedAndLiveACL(t *testing.T) if err != nil { t.Fatal(err) } - if err := denyAppContainerSIDsWithInheritance(path, []string{userSID}, "RX", false); err != nil { - t.Fatal(err) - } + installLegacyDeny(t, path, userSID) switch source { case "live marker": - marker := writeStaleCredentialMarker(t, path) - liveResidueMarkers.Store(marker, struct{}{}) - t.Cleanup(func() { liveResidueMarkers.Delete(marker) }) + writeMarker(t, os.Getppid(), path) case "wrong path": - writeStaleCredentialMarker(t, path+"-other") + writeMarker(t, os.Getpid(), path+"-other") } - before, err := windowsPathDACLSDDL(path) + before, err := pathDACLSDDL(path) if err != nil { t.Fatal(err) } if err := RepairLegacyCredentialDeny(path); err == nil { t.Fatal("repair accepted an unattributed or live deny") } - after, err := windowsPathDACLSDDL(path) + after, err := pathDACLSDDL(path) if err != nil || after != before { t.Fatalf("DACL changed: before %s, after %s, err %v", before, after, err) } @@ -150,14 +147,14 @@ func TestRepairLegacyCredentialDenyLeavesOrdinaryACLAlone(t *testing.T) { if err := os.WriteFile(path, []byte("KEY=value\n"), 0o600); err != nil { t.Fatal(err) } - before, err := windowsPathDACLSDDL(path) + before, err := pathDACLSDDL(path) if err != nil { t.Fatal(err) } if err := RepairLegacyCredentialDeny(path); err != nil { t.Fatal(err) } - after, err := windowsPathDACLSDDL(path) + after, err := pathDACLSDDL(path) if err != nil { t.Fatal(err) } @@ -194,15 +191,14 @@ func TestRepairLegacyCredentialDenyRefusesMixedCurrentUserDenyACL(t *testing.T) if err != nil || legacy != 1 || other == 0 { t.Fatalf("deny counts before repair = legacy:%d other:%d err:%v", legacy, other, err) } - before, err := windowsPathDACLSDDL(path) + before, err := pathDACLSDDL(path) if err != nil { t.Fatal(err) } - if err := RepairLegacyCredentialDeny(path); err == nil { t.Fatal("RepairLegacyCredentialDeny accepted mixed current-user deny ACL") } - after, err := windowsPathDACLSDDL(path) + after, err := pathDACLSDDL(path) if err != nil { t.Fatal(err) } @@ -211,7 +207,7 @@ func TestRepairLegacyCredentialDenyRefusesMixedCurrentUserDenyACL(t *testing.T) } } -func windowsPathDACLSDDL(path string) (string, error) { +func pathDACLSDDL(path string) (string, error) { sd, err := windows.GetNamedSecurityInfo(path, windows.SE_FILE_OBJECT, windows.DACL_SECURITY_INFORMATION) if err != nil || sd == nil { return "", err diff --git a/internal/winaclresidue/doc.go b/internal/winaclresidue/doc.go new file mode 100644 index 0000000000..99c9aba35b --- /dev/null +++ b/internal/winaclresidue/doc.go @@ -0,0 +1,9 @@ +// Package winaclresidue cleans up ACL residue that the retired Windows sandbox +// backend (v1.38.8 to v1.38.10) left on user files. That backend applied +// temporary deny and grant ACEs and recorded each mutation in a per-process +// marker under %TEMP%; a crash left both behind. This package removes only +// those exact trustees from paths named in markers whose owner is provably +// gone, and repairs the one legacy DENY RX entry on the credential store when +// a dead-run marker proves its origin. It never creates markers or mutates +// ACLs for any other reason. Non-Windows builds are no-ops. +package winaclresidue diff --git a/internal/winaclresidue/marker.go b/internal/winaclresidue/marker.go new file mode 100644 index 0000000000..b29a6be76c --- /dev/null +++ b/internal/winaclresidue/marker.go @@ -0,0 +1,97 @@ +package winaclresidue + +import ( + "bufio" + "os" + "path/filepath" + "strconv" + "strings" +) + +type residueKind string + +const ( + residueDeny residueKind = "deny" + residueGrant residueKind = "grant" +) + +type residueEntry struct { + kind residueKind + path string +} + +// markerOwnerPID extracts the owning PID from a "[-nonce].txt" name. +func markerOwnerPID(name string) (string, bool) { + if !strings.HasSuffix(name, ".txt") { + return "", false + } + pid, _, _ := strings.Cut(strings.TrimSuffix(name, ".txt"), "-") + if pid == "" { + return "", false + } + if _, err := strconv.ParseUint(pid, 10, 32); err != nil { + return "", false + } + return pid, true +} + +// readResidueMarker parses "\t" lines. A tab separates the fields +// so paths with spaces survive; unrecognized lines are skipped rather than +// guessed at, so a corrupt marker cannot cause a wrong ACE removal. +func readResidueMarker(path string) []residueEntry { + f, err := os.Open(path) + if err != nil { + return nil + } + defer f.Close() + var out []residueEntry + scanner := bufio.NewScanner(f) + for scanner.Scan() { + kindStr, p, ok := strings.Cut(strings.TrimRight(scanner.Text(), "\r\n"), "\t") + if !ok || p == "" { + continue + } + switch kind := residueKind(kindStr); kind { + case residueDeny, residueGrant: + out = append(out, residueEntry{kind: kind, path: p}) + } + } + return out +} + +// isWindowsSystemRoot reports whether path lies under a shared system +// directory. Markers are untrusted input under %TEMP%; stripping the built-in +// package SIDs from System32 or Program Files would remove factory ACEs. +// Paths are compared with backslash separators so the rule is the same on +// every host that inspects a Windows marker. +func isWindowsSystemRoot(path string) bool { + clean := windowsPathKey(path) + for _, envVar := range []string{"SystemRoot", "windir", "ProgramFiles", "ProgramFiles(x86)", "ProgramW6432"} { + root := os.Getenv(envVar) + if root == "" { + continue + } + root = windowsPathKey(root) + if clean == root || strings.HasPrefix(clean, root+`\`) { + return true + } + } + return false +} + +func windowsPathKey(path string) string { + return strings.ToLower(strings.TrimRight(strings.ReplaceAll(filepath.Clean(path), "/", `\`), `\`)) +} + +func dedupeSIDStrings(sids []string) []string { + out := make([]string, 0, len(sids)) + seen := map[string]bool{} + for _, sid := range sids { + if sid == "" || seen[sid] { + continue + } + seen[sid] = true + out = append(out, sid) + } + return out +} diff --git a/internal/winaclresidue/marker_test.go b/internal/winaclresidue/marker_test.go new file mode 100644 index 0000000000..098fab1b06 --- /dev/null +++ b/internal/winaclresidue/marker_test.go @@ -0,0 +1,68 @@ +package winaclresidue + +import ( + "os" + "path/filepath" + "testing" +) + +func TestMarkerOwnerPIDParsesRetiredBackendNames(t *testing.T) { + for name, want := range map[string]string{ + "1234.txt": "1234", + "1234-deadbeef.txt": "1234", + "1234-1700000000-2.txt": "1234", + "1234": "", + "-abc.txt": "", + "notapid-1.txt": "", + "99999999999.txt": "", + } { + got, ok := markerOwnerPID(name) + if (want == "") == ok || got != want { + t.Fatalf("markerOwnerPID(%q) = %q,%v want %q", name, got, ok, want) + } + } +} + +func TestReadResidueMarkerSkipsCorruptLines(t *testing.T) { + marker := filepath.Join(t.TempDir(), "42.txt") + body := "deny\tC:\\Users\\me\\.env\r\n" + + "grant\tC:\\Tools\\with space\\bin\n" + + "deny C:\\no-tab\n" + + "unknown\tC:\\kind\n" + + "deny\t\n" + + "\n" + if err := os.WriteFile(marker, []byte(body), 0o600); err != nil { + t.Fatal(err) + } + got := readResidueMarker(marker) + if len(got) != 2 || got[0] != (residueEntry{residueDeny, `C:\Users\me\.env`}) || got[1] != (residueEntry{residueGrant, `C:\Tools\with space\bin`}) { + t.Fatalf("readResidueMarker = %+v", got) + } + if got := readResidueMarker(filepath.Join(t.TempDir(), "missing.txt")); got != nil { + t.Fatalf("missing marker parsed as %+v", got) + } +} + +func TestIsWindowsSystemRootUsesEnvironmentRoots(t *testing.T) { + t.Setenv("SystemRoot", `C:\Windows`) + t.Setenv("ProgramFiles", `C:\Program Files`) + for path, want := range map[string]bool{ + `C:\Windows\System32`: true, + `c:\windows`: true, + `C:\Program Files\Git\bin`: true, + `C:\WindowsOld\System32`: false, + `C:\Users\me\project`: false, + `C:\Program FilesX\tool`: false, + } { + if got := isWindowsSystemRoot(path); got != want { + t.Fatalf("isWindowsSystemRoot(%q) = %v, want %v", path, got, want) + } + } +} + +func TestDedupeSIDStringsDropsEmptyAndRepeats(t *testing.T) { + got := dedupeSIDStrings([]string{"S-1-15-2-1", "", "S-1-15-2-2", "S-1-15-2-1", "S-1-5-21-1"}) + if len(got) != 3 || got[0] != "S-1-15-2-1" || got[1] != "S-1-15-2-2" || got[2] != "S-1-5-21-1" { + t.Fatalf("dedupeSIDStrings = %v", got) + } +} diff --git a/internal/winaclresidue/stub_other.go b/internal/winaclresidue/stub_other.go new file mode 100644 index 0000000000..a76a96a3aa --- /dev/null +++ b/internal/winaclresidue/stub_other.go @@ -0,0 +1,9 @@ +//go:build !windows + +package winaclresidue + +// SweepStaleMarkers is a no-op outside Windows. +func SweepStaleMarkers() {} + +// RepairLegacyCredentialDeny is a no-op outside Windows. +func RepairLegacyCredentialDeny(string) error { return nil } diff --git a/internal/winsandbox/LICENSE b/internal/winsandbox/LICENSE deleted file mode 100644 index 67c485c43a..0000000000 --- a/internal/winsandbox/LICENSE +++ /dev/null @@ -1,22 +0,0 @@ -MIT License - -Copyright (c) 2026 windows-sandbox Contributors -Copyright (c) 2026 DeepSeek - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. diff --git a/internal/winsandbox/NOTICE.md b/internal/winsandbox/NOTICE.md deleted file mode 100644 index fe286f2b39..0000000000 --- a/internal/winsandbox/NOTICE.md +++ /dev/null @@ -1,24 +0,0 @@ -This package is adapted from `github.com/SivanCola/windows-sandbox` at commit -`6b29dd09f9cb5a85d7ac646dd8ade74d207bb47b`. - -The WRITE_RESTRICTED token, deterministic workspace/temp capability SID, exact -ACL grant, and restricted-token default-DACL design are adapted from DeepSeek -Harness at commit `c291e7961a`, package -`packages/sandbox/sandbox-windows-acl`: - -https://github.com/deepseek-ai/DeepSeek-Harness - -Both upstream projects are licensed under the MIT License. The license text is -kept in `LICENSE` in this directory. DeepSeek Harness carries: - -Copyright (c) 2026 DeepSeek - -Local modifications since vendoring: - -- `lockWindowsRoots`/`acquireNamedMutex` take an optional notice writer and - wait in slices, emitting a one-line message when a run queues behind another - sandboxed command's per-root lock instead of blocking silently. -- Capability hashes are domain-separated for Reasonix and support multiple - independently authorized writable roots. -- Existing Reasonix AppContainer read restrictions, forbid-read handling, - Job Object UI restrictions, and crash-residue recovery are retained. diff --git a/internal/winsandbox/README.md b/internal/winsandbox/README.md deleted file mode 100644 index 5ff2713c8c..0000000000 --- a/internal/winsandbox/README.md +++ /dev/null @@ -1,102 +0,0 @@ -# internal/winsandbox - -Reasonix's bundled native Windows process sandbox helpers. - -The package combines two Windows isolation lanes: - -- AppContainer for direct read-only tools, including optional network removal. -- `WRITE_RESTRICTED` primary tokens for shells and writer processes. -- Deterministic, per-directory capability SIDs for workspace writes. -- A distinct capability SID for the session-private temporary directory. -- Temporary deny ACEs for `ForbidReadRoots`. -- Kill-on-close Job Objects and UI restrictions for process-tree cleanup. -- Strict, versioned helper payloads and fail-closed native API handling. - -The package implements enforcement only. Product permission presets, approval -prompts, and grant policy live above this layer. - -## Write capability model - -For a workspace-write shell, Reasonix derives a capability SID from each -canonical writable root and adds a `Modify`-class inheritable ACE for that SID. -The ACE intentionally excludes `WRITE_DAC` and `WRITE_OWNER`. It remains on the -directory, so later commands hit an exact-ACE fast path instead of recursively -rewriting a large workspace on every launch. - -Each restricted token contains the logon SID, Everyone, and only the directory -capabilities authorized for that call. Windows performs the ordinary access -check and the restricting-SID check; a write succeeds only when both pass. -Read-only shells carry no directory capability, so standing workspace ACEs are -inert after a mode downgrade. - -The session temp directory uses a different SID namespace. Two sessions can -share workspace authority without receiving access to one another's private -temporary files. Deleting the session temp tree removes that temporary grant. - -`ProtectedWriteRoots` are checked before any grant is materialized. A writable -root that contains protected state, an arbitrary protected-state descendant, -or an overlapping session temp is rejected. The direct -`/global-workspace` child is the sole product-layout exception; its ACE -is applied only to that exact directory. - -Before a workspace ACE is applied, a versioned `preparing` record is written -under the protected state root. It is atomically replaced with `active` only -after the exact ACE and Windows file identity have been revalidated. - -```go -result, err := winsandbox.Run(winsandbox.Spec{ - WritableRoots: []string{workspace}, - ProtectedWriteRoots: []string{reasonixState}, - ForbidReadRoots: []string{secretDir}, - TempDir: sessionTemp, - Network: true, - Writable: true, -}, argv, winsandbox.RunOptions{ - Stdin: os.Stdin, Stdout: os.Stdout, Stderr: os.Stderr, -}) -``` - -Set `ReadOnly: true` for a shell that may read with the caller's normal access -but must not receive workspace or temp write capabilities. Set `Writable: -false` for the direct AppContainer lane. - -## Concurrency and crash safety - -Standing capability ACEs are materialized under short, per-root named mutexes. -Commands sharing a workspace can then run concurrently without grant/restore -races. `ForbidReadRoots` still use temporary current-user deny ACEs, so those -paths remain locked for the full command. A crash marker lets the next run -remove a deny left by a dead helper. - -The legacy AppContainer compatibility path retains its existing ACL residue -sweeper for direct tools. The `WRITE_RESTRICTED` lane never lowers the process -integrity level and never recursively relabels the workspace Low/Medium. - -## Network and read boundaries - -AppContainer omits network capabilities when `Network` is false. -`WRITE_RESTRICTED` controls write-class access only; it does not isolate reads, -network, registry access, or process visibility. A restricted-token launch with -`Network: false` therefore fails closed. Reasonix reports Windows enforcement as -`partial` and retains `ForbidReadRoots` for selected sensitive paths. - -Everyone remains in the restricting set because Windows shell, DLL, CNG, pipe, -and desktop initialization depends on it. A location that grants ambient write -access directly to Everyone can remain writable; this is a documented Windows -boundary of the approach. - -## Environment overrides - -| Variable | Effect | -| --- | --- | -| `WINDOWS_SANDBOX_WAIT_MS` | Maximum runtime before the Job Object is terminated. | -| `WINDOWS_SANDBOX_ICACLS_TIMEOUT_MS` | Timeout for legacy AppContainer ACL cleanup operations. | -| `WINDOWS_SANDBOX_LOCK_MS` | Maximum wait for a capability or temporary-deny root lock. | - -## Verification - -Windows-native tests cover workspace and private-temp writes, outside and -sibling-temp denial, read-only capability downgrades, protected-root overlap, -forbid-read cleanup, AppContainer network denial, stdio/environment/cwd, timeouts, -and Job Object process-tree termination. Cross-compilation on non-Windows hosts -verifies all Win32 bindings and helper wiring. diff --git a/internal/winsandbox/capability_record_windows.go b/internal/winsandbox/capability_record_windows.go deleted file mode 100644 index fe3906bbc9..0000000000 --- a/internal/winsandbox/capability_record_windows.go +++ /dev/null @@ -1,128 +0,0 @@ -//go:build windows - -package winsandbox - -import ( - "crypto/sha256" - "encoding/hex" - "encoding/json" - "fmt" - "os" - "path/filepath" - "strings" - "time" - - "golang.org/x/sys/windows" -) - -const capabilityRecordVersion = 1 - -type capabilityRecordStore struct { - dir string -} - -type capabilityRecord struct { - Version int `json:"version"` - Status string `json:"status"` - Purpose capabilityPurpose `json:"purpose"` - CanonicalPath string `json:"canonicalPath"` - SID string `json:"sid"` - AccessMask uint32 `json:"accessMask"` - Inheritance uint32 `json:"inheritance"` - VolumeSerial uint32 `json:"volumeSerial"` - FileIndexHigh uint32 `json:"fileIndexHigh"` - FileIndexLow uint32 `json:"fileIndexLow"` - OwnerPID int `json:"ownerPid"` - UpdatedAt time.Time `json:"updatedAt"` -} - -func newCapabilityRecordStore(protectedRoots, writableRoots []string) (*capabilityRecordStore, error) { - for _, root := range protectedRoots { - root = strings.TrimSpace(root) - if root == "" { - continue - } - abs, err := filepath.Abs(root) - if err != nil { - return nil, fmt.Errorf("resolve capability record root %q: %w", root, err) - } - if err := os.MkdirAll(abs, 0o700); err != nil { - return nil, fmt.Errorf("create capability record root %q: %w", abs, err) - } - canonical, err := canonicalWindowsDirectory(abs) - if err != nil { - return nil, fmt.Errorf("canonicalize capability record root %q: %w", abs, err) - } - dir := filepath.Join(canonical, "windows-sandbox-capabilities-v1") - if err := os.MkdirAll(dir, 0o700); err != nil { - return nil, fmt.Errorf("create capability record directory %q: %w", dir, err) - } - canonicalDir, err := canonicalWindowsDirectory(dir) - if err != nil { - return nil, fmt.Errorf("validate capability record directory %q: %w", dir, err) - } - if !windowsPathWithin(canonical, canonicalDir) { - return nil, fmt.Errorf("capability record directory %q escapes protected state root %q", canonicalDir, canonical) - } - for _, writable := range writableRoots { - if windowsPathsOverlap(canonicalDir, writable) { - return nil, fmt.Errorf("capability record directory %q overlaps writable root %q", canonicalDir, writable) - } - } - return &capabilityRecordStore{dir: canonicalDir}, nil - } - return nil, nil -} - -func (s *capabilityRecordStore) write(capability restrictedCapability, status string) error { - if s == nil || capability.purpose != capabilityWorkspace { - return nil - } - record := capabilityRecord{ - Version: capabilityRecordVersion, - Status: status, - Purpose: capability.purpose, - CanonicalPath: capability.root, - SID: capability.sidText, - AccessMask: uint32(capabilityWriteGrantMask), - Inheritance: uint32(windows.SUB_CONTAINERS_AND_OBJECTS_INHERIT), - VolumeSerial: capability.object.volumeSerial, - FileIndexHigh: capability.object.indexHigh, - FileIndexLow: capability.object.indexLow, - OwnerPID: os.Getpid(), - UpdatedAt: time.Now().UTC(), - } - nameHash := sha256.Sum256([]byte(capability.sidText)) - path := filepath.Join(s.dir, hex.EncodeToString(nameHash[:16])+".json") - tmp, err := os.CreateTemp(s.dir, ".capability-*.tmp") - if err != nil { - return fmt.Errorf("create capability record: %w", err) - } - tmpPath := tmp.Name() - cleanup := true - defer func() { - _ = tmp.Close() - if cleanup { - _ = os.Remove(tmpPath) - } - }() - if err := tmp.Chmod(0o600); err != nil { - return fmt.Errorf("protect capability record: %w", err) - } - encoder := json.NewEncoder(tmp) - encoder.SetIndent("", " ") - if err := encoder.Encode(record); err != nil { - return fmt.Errorf("encode capability record: %w", err) - } - if err := tmp.Sync(); err != nil { - return fmt.Errorf("flush capability record: %w", err) - } - if err := tmp.Close(); err != nil { - return fmt.Errorf("close capability record: %w", err) - } - if err := os.Rename(tmpPath, path); err != nil { - return fmt.Errorf("publish capability record: %w", err) - } - cleanup = false - return nil -} diff --git a/internal/winsandbox/capability_sid.go b/internal/winsandbox/capability_sid.go deleted file mode 100644 index 6981412958..0000000000 --- a/internal/winsandbox/capability_sid.go +++ /dev/null @@ -1,31 +0,0 @@ -package winsandbox - -import ( - "crypto/sha256" - "encoding/binary" - "fmt" -) - -type capabilityPurpose string - -const ( - capabilityWorkspace capabilityPurpose = "workspace-write" - capabilitySessionTemp capabilityPurpose = "session-temp-write" - capabilityHashDomain = "reasonix/windows-write-capability/v1" -) - -// deriveCapabilitySID returns an unforgeable-in-practice Windows capability -// identity for one canonical directory and purpose. The SID is authority-local -// (S-1-4) and gains power only where an ACL explicitly names it. -func deriveCapabilitySID(purpose capabilityPurpose, canonicalPath string) string { - first, second := capabilitySubauthorities(purpose, canonicalPath) - return fmt.Sprintf("S-1-4-%d-%d", first, second) -} - -func capabilitySubauthorities(purpose capabilityPurpose, canonicalPath string) (uint32, uint32) { - digest := sha256.Sum256([]byte(capabilityHashDomain + "\x00" + string(purpose) + "\x00" + canonicalPath)) - const modulus = uint32(1<<30 - 1) - first := binary.LittleEndian.Uint32(digest[0:4])%modulus + 1 - second := binary.LittleEndian.Uint32(digest[4:8])%modulus + 1 - return first, second -} diff --git a/internal/winsandbox/capability_sid_test.go b/internal/winsandbox/capability_sid_test.go deleted file mode 100644 index 5c80d2688b..0000000000 --- a/internal/winsandbox/capability_sid_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package winsandbox - -import ( - "regexp" - "testing" -) - -func TestCapabilitySIDIsDeterministicAndValid(t *testing.T) { - first := deriveCapabilitySID(capabilityWorkspace, `c:\work\reasonix`) - second := deriveCapabilitySID(capabilityWorkspace, `c:\work\reasonix`) - if first != second { - t.Fatalf("capability SID is not deterministic: %q != %q", first, second) - } - if !regexp.MustCompile(`^S-1-4-[1-9][0-9]*-[1-9][0-9]*$`).MatchString(first) { - t.Fatalf("capability SID has unexpected form: %q", first) - } -} - -func TestCapabilitySIDSeparatesPurposeAndPath(t *testing.T) { - workspace := deriveCapabilitySID(capabilityWorkspace, `c:\work\reasonix`) - temp := deriveCapabilitySID(capabilitySessionTemp, `c:\work\reasonix`) - otherWorkspace := deriveCapabilitySID(capabilityWorkspace, `c:\work\other`) - - if workspace == temp { - t.Fatalf("workspace and session-temp capabilities must be domain separated: %q", workspace) - } - if workspace == otherWorkspace { - t.Fatalf("different roots must receive different capabilities: %q", workspace) - } -} - -func TestCapabilitySIDSubauthoritiesStayInThirtyBitRange(t *testing.T) { - first, second := capabilitySubauthorities(capabilityWorkspace, `c:\work\reasonix`) - const max = uint32(1<<30 - 1) - if first < 1 || first > max || second < 1 || second > max { - t.Fatalf("subauthorities outside 1..2^30-1: %d, %d", first, second) - } -} diff --git a/internal/winsandbox/coordination_windows.go b/internal/winsandbox/coordination_windows.go deleted file mode 100644 index c84cb30b43..0000000000 --- a/internal/winsandbox/coordination_windows.go +++ /dev/null @@ -1,595 +0,0 @@ -//go:build windows - -package winsandbox - -import ( - "bufio" - "crypto/rand" - "crypto/sha1" - "encoding/hex" - "fmt" - "io" - "os" - "path/filepath" - "runtime" - "slices" - "sort" - "strconv" - "strings" - "sync" - "sync/atomic" - "time" - - "golang.org/x/sys/windows" -) - -// Reasonix launches every sandboxed command as its own helper process, so two -// commands preparing the same directory are separate OS processes. Standing -// capability ACEs use this lock only while the exact entry is materialized and -// its directory identity is revalidated. Temporary forbid-read ACEs and the -// legacy AppContainer lane still restore descriptor snapshots, so they keep the -// lock for their whole mutation window. A shared path has no per-process ACL -// view; serializing those mutations prevents one helper's cleanup from undoing -// another helper's boundary. -// -// windowsRootLock takes a per-root named mutex for the lifetime of a run. The -// mutex lives in the session-local namespace and the OS releases it -// automatically if the holder dies, so a crashed command never deadlocks the -// next one (WAIT_ABANDONED is treated as ownership). Multiple roots are locked -// in a stable sorted order so two processes cannot deadlock by acquiring them -// in opposite orders. -// -// Trade-off: a long-running command holds a root lock only when it has a -// temporary forbid-read or legacy AppContainer mutation on that root. Ordinary -// workspace capability setup is short and commands sharing the workspace run -// concurrently afterward. Lock waits remain bounded and identify their holder. - -// stillActiveExitCode is STILL_ACTIVE: GetExitCodeProcess reports it while a -// process is running. Used to tell a live marker-owner from a dead one. -const stillActiveExitCode = 259 - -// Windows mutexes have thread affinity: the thread that acquires a mutex is the -// only one that can release it (ReleaseMutex from another thread fails with -// ERROR_NOT_OWNER), and the owning thread re-acquiring is a no-op that would -// break serialization within a process. A Go goroutine can migrate across OS -// threads between the acquire and the deferred release, so the lock pins itself -// to one OS thread for its whole lifetime with runtime.LockOSThread and unpins -// on release. This keeps ReleaseMutex on the owning thread and prevents a -// concurrent goroutine that happens to land on the owner thread from re-entering -// the mutex. -type windowsRootLock struct { - handles []windows.Handle - names []string - pinned bool -} - -// lockWindowsRoots serializes access to every distinct root in roots. The -// returned lock must be released once the run's grants have been cleaned up. -// notice, when non-nil, receives a one-line message if acquisition has to -// queue behind another run for more than windowsRootLockNoticeAfter. -// holderLabel is this run's command preview, recorded so a queued command can -// name what is blocking it; lockWait is the caller's wait budget (see -// windowsRootLockTimeout). -func lockWindowsRoots(roots []string, notice io.Writer, holderLabel string, lockWait time.Duration) (*windowsRootLock, error) { - names := windowsRootLockNames(roots) - if len(names) == 0 { - return &windowsRootLock{}, nil - } - lock := &windowsRootLock{} - // Pin before acquiring the first mutex and stay pinned until release so every - // mutex is owned by, and released from, the same OS thread. - runtime.LockOSThread() - lock.pinned = true - timeout := windowsRootLockTimeout(lockWait) - for _, name := range names { - h, err := acquireNamedMutex(name, timeout, notice) - if err != nil { - lock.release() - return nil, err - } - lock.handles = append(lock.handles, h) - lock.names = append(lock.names, name) - writeWindowsLockHolder(name, holderLabel) - } - return lock, nil -} - -func (l *windowsRootLock) release() { - if l == nil { - return - } - for i, v := range slices.Backward(l.handles) { - h := v - if h == 0 { - continue - } - // Drop the holder record before releasing the mutex so the next owner - // never races its own record against ours. - removeWindowsLockHolder(l.names[i]) - _ = windows.ReleaseMutex(h) - _ = windows.CloseHandle(h) - } - l.handles = nil - l.names = nil - if l.pinned { - runtime.UnlockOSThread() - l.pinned = false - } -} - -// windowsRootLockNames maps roots to a deduplicated, sorted list of mutex -// names. Sorting guarantees a global acquisition order across processes so a -// multi-root run cannot deadlock against another acquiring the same roots in a -// different order. -func windowsRootLockNames(roots []string) []string { - seen := map[string]bool{} - var names []string - for _, root := range roots { - for _, key := range windowsRootLockDomains(root) { - if seen[key] { - continue - } - seen[key] = true - sum := sha1.Sum([]byte(key)) - names = append(names, `Local\windows-sandbox.`+hex.EncodeToString(sum[:16])) - } - } - sort.Strings(names) - return names -} - -// windowsRootLockDomains includes each directory and its non-volume ancestors. -// Consequently ACL mutations for a parent and child path share at least one -// mutex even when callers spell different roots. This is deliberately -// conservative: short standing-capability materialization may serialize with a -// sibling under a common ancestor, while whole-run locks remain limited to the -// comparatively rare forbid-read paths. -func windowsRootLockDomains(root string) []string { - root = strings.ToLower(filepath.Clean(strings.TrimSpace(root))) - if root == "" || root == "." { - return nil - } - volume := strings.ToLower(filepath.Clean(filepath.VolumeName(root) + string(filepath.Separator))) - var domains []string - for current := root; current != "" && current != "."; current = filepath.Dir(current) { - if current == volume || filepath.Dir(current) == current { - break - } - domains = append(domains, current) - } - return domains -} - -// windowsRootLockNoticeAfter is how long a lock acquisition may block silently -// before a one-line notice is emitted. Queueing behind another sandboxed -// command's whole-run lock is by design (see the package comment above), but -// before this notice existed a queued command was indistinguishable from a -// hung one — users read the silence as "bash is broken" (#6067). -const windowsRootLockNoticeAfter = 2 * time.Second - -func acquireNamedMutex(name string, timeout time.Duration, notice io.Writer) (windows.Handle, error) { - name16, err := windows.UTF16PtrFromString(name) - if err != nil { - return 0, err - } - // CreateMutex returns a valid handle even when the named mutex already - // exists (err == ERROR_ALREADY_EXISTS); only a zero handle is a real error. - h, err := windows.CreateMutex(nil, false, name16) - if h == 0 { - return 0, fmt.Errorf("create sandbox mutex %q: %w", name, err) - } - // Wait in slices so a long queue surfaces a notice instead of blocking - // silently until the deadline. - var waited time.Duration - noticed := false - for { - slice := windowsRootLockNoticeAfter - if timeout > 0 { - if remaining := timeout - waited; remaining < slice { - slice = remaining - } - } - event, werr := windows.WaitForSingleObject(h, uint32(slice/time.Millisecond)) - switch event { - case windows.WAIT_OBJECT_0, windows.WAIT_ABANDONED: - // WAIT_ABANDONED means the previous holder died without releasing. We now - // own the mutex; any filesystem residue that run left behind is cleared by - // sweepWindowsDenyResidue before we re-apply, and the integrity-label reset - // on cleanup returns the tree to Medium. - return h, nil - case uint32(windows.WAIT_TIMEOUT): - waited += slice - if timeout > 0 && waited >= timeout { - _ = windows.CloseHandle(h) - if holder := describeWindowsLockHolder(name); holder != "" { - return 0, fmt.Errorf("timed out waiting for sandbox lock %q after %s (%s; stop that command or set WINDOWS_SANDBOX_LOCK_MS higher)", name, timeout, holder) - } - return 0, fmt.Errorf("timed out waiting for sandbox lock %q after %s (stop the earlier sandboxed command or set WINDOWS_SANDBOX_LOCK_MS higher)", name, timeout) - } - if !noticed && notice != nil { - noticed = true - limit := "no limit" - if timeout > 0 { - limit = timeout.String() - } - if holder := describeWindowsLockHolder(name); holder != "" { - fmt.Fprintf(notice, "windows sandbox: waiting for another sandboxed command on this workspace to finish (%s; lock wait limit %s; stop that command or set WINDOWS_SANDBOX_LOCK_MS to adjust)\n", holder, limit) - } else { - fmt.Fprintf(notice, "windows sandbox: waiting for another sandboxed command on this workspace to finish (lock wait limit %s; stop the earlier background/long-running command or set WINDOWS_SANDBOX_LOCK_MS to adjust)\n", limit) - } - } - default: - _ = windows.CloseHandle(h) - if werr != nil { - return 0, fmt.Errorf("wait for sandbox lock %q: %w", name, werr) - } - return 0, fmt.Errorf("wait for sandbox lock %q returned %#x", name, event) - } - } -} - -// Lock-holder records live under the same user-shared %TEMP% as the residue -// markers, one file per root digest. All operations are best-effort: the record -// is diagnostic, so a write or read failure must never affect the run. -func windowsLockHolderDir() string { - return filepath.Join(os.TempDir(), "windows-sandbox-lockholders") -} - -func windowsLockHolderPath(mutexName string) string { - return filepath.Join(windowsLockHolderDir(), lockHolderFileName(mutexName)) -} - -func writeWindowsLockHolder(mutexName, label string) { - if err := os.MkdirAll(windowsLockHolderDir(), 0o700); err != nil { - return - } - rec := lockHolderRecord{pid: os.Getpid(), startUnixMS: time.Now().UnixMilli(), label: label} - _ = os.WriteFile(windowsLockHolderPath(mutexName), []byte(formatLockHolderRecord(rec)), 0o600) -} - -func removeWindowsLockHolder(mutexName string) { - _ = os.Remove(windowsLockHolderPath(mutexName)) -} - -// describeWindowsLockHolder names the run currently holding mutexName, or "" -// when no trustworthy record exists. A record whose PID is dead is a crash -// leftover (the OS already released the abandoned mutex); it is skipped, not -// deleted, because removal here could race the file the next live holder just -// wrote — the next acquire overwrites it anyway. -func describeWindowsLockHolder(mutexName string) string { - data, err := os.ReadFile(windowsLockHolderPath(mutexName)) - if err != nil { - return "" - } - rec, ok := parseLockHolderRecord(string(data)) - if !ok || !windowsProcessAlive(strconv.Itoa(rec.pid)) { - return "" - } - return describeLockHolder(rec, time.Now()) -} - -// windowsMutatedRoots is the set of paths a run edits ACLs on: its writable -// roots plus any forbid_read roots that exist. These are the paths that must be -// serialized against concurrent runs and tracked for crash-residue cleanup. -func windowsMutatedRoots(spec Spec) []string { - roots := append([]string(nil), windowsWritableRoots(spec)...) - for _, r := range normalizedWindowsRoots(spec.ForbidReadRoots) { - if pathExists(r) { - roots = append(roots, r) - } - } - return roots -} - -// windowsMutatedRootsForRun extends windowsMutatedRoots with the executable -// directories the run will actually mutate ACLs on: the non-system ones. A run -// snapshots/grants/restores argv[0]'s directory (and a Git install root), so two -// runs in different workspaces that share one tool directory would otherwise -// interleave their ACL snapshots and corrupt each other. System tool directories -// (System32, Program Files) are never mutated (grantAppContainerExecutable skips -// them), so they must stay out of the lock too — both sets draw from the same -// windowsMutableExecutableGrantRoots so the locked paths and the mutated paths -// cannot drift apart, and every command sharing the system shell is spared a -// needless serialization. -func windowsMutatedRootsForRun(spec Spec, exe string) []string { - roots := windowsMutatedRoots(spec) - return append(roots, windowsMutableExecutableGrantRoots(exe)...) -} - -// isWindowsSystemRoot reports whether path is inside a Windows system location -// (%SystemRoot%, the Program Files variants). Determined by path membership, not -// by attempting a write, so the result is stable regardless of the process's -// integrity level or admin rights. Backs windowsMutableExecutableGrantRoots, -// which keeps shared system directories out of both the per-root lock set and -// the executable grant/residue set. -func isWindowsSystemRoot(path string) bool { - clean := strings.ToLower(filepath.Clean(path)) - for _, envVar := range []string{"SystemRoot", "windir", "ProgramFiles", "ProgramFiles(x86)", "ProgramW6432"} { - root := os.Getenv(envVar) - if root == "" { - continue - } - root = strings.ToLower(filepath.Clean(root)) - if clean == root || strings.HasPrefix(clean, root+string(filepath.Separator)) { - return true - } - } - return false -} - -// forbid_read applies a deny ACE for the current user SID so a same-user -// low-integrity child cannot bypass the deny through normal file ACLs, and -// writable runs grant AppContainer/user read+execute on the tool executable's -// directory. Both are removed on normal cleanup, but a crash (or force-kill) -// skips cleanup and leaves residue: a deny ACE locks the user out of, e.g., -// ~/.ssh until they repair the ACL by hand; a stale grant silently widens read -// access to a tool directory. The residue tracker records each mutated path in a -// per-PID marker *before* the ACE is applied, so any crash point leaves a marker -// the next run can sweep; the next run removes the residue for markers whose -// owning process is gone. Only the stable, sandbox-applied trustees are removed, -// so legitimate ACLs are left untouched. -// -// Each marker line is " ", where kind is "deny" or "grant". Lines -// are appended and fsync'd one at a time, and a write failure aborts the run -// before the corresponding ACE is applied, so the marker can never lag behind -// the on-disk ACLs. - -type residueKind string - -const ( - residueDeny residueKind = "deny" - residueGrant residueKind = "grant" -) - -type residueEntry struct { - kind residueKind - path string -} - -func windowsDenyMarkerDir() string { - return filepath.Join(os.TempDir(), "windows-sandbox-denylocks") -} - -func windowsDenyMarkerPath() string { - return filepath.Join(windowsDenyMarkerDir(), strconv.Itoa(os.Getpid())+".txt") -} - -func windowsNewDenyMarkerPath() string { - pid := strconv.Itoa(os.Getpid()) - for range 8 { - if nonce, ok := windowsResidueNonce(); ok { - marker := filepath.Join(windowsDenyMarkerDir(), pid+"-"+nonce+".txt") - if !pathExists(marker) { - return marker - } - } - } - seq := residueMarkerSeq.Add(1) - return filepath.Join(windowsDenyMarkerDir(), fmt.Sprintf("%s-%d-%d.txt", pid, time.Now().UnixNano(), seq)) -} - -func windowsResidueNonce() (string, bool) { - var b [16]byte - if _, err := rand.Read(b[:]); err != nil { - return "", false - } - return hex.EncodeToString(b[:]), true -} - -func windowsDenyMarkerOwnerPID(name string) (string, bool) { - if !strings.HasSuffix(name, ".txt") { - return "", false - } - stem := strings.TrimSuffix(name, ".txt") - pid, _, _ := strings.Cut(stem, "-") - if pid == "" { - return "", false - } - if _, err := strconv.ParseUint(pid, 10, 32); err != nil { - return "", false - } - return pid, true -} - -var ( - residueMarkerSeq atomic.Uint64 - liveResidueMarkers sync.Map -) - -// windowsResidueRun owns one run-local crash-residue marker. Marker filenames -// include the process PID so a later process can tell whether the owner is gone, -// plus a nonce so multiple concurrent Run calls in the same process never share -// a marker. The live registry protects this process's own active markers from a -// concurrent sweep; an unregistered marker with our PID is therefore a crashed -// predecessor's residue after Windows reused the PID. -type windowsResidueRun struct { - marker string - owned atomic.Bool -} - -func newWindowsDenyResidueRun() *windowsResidueRun { - r := &windowsResidueRun{marker: windowsNewDenyMarkerPath()} - liveResidueMarkers.Store(r.marker, struct{}{}) - return r -} - -// recordBeforeApply appends one "\t" line to this run's marker and -// flushes it to disk. It is called immediately before the matching ACE is -// applied; a failure here must abort the run before the ACE is applied so the -// marker never lags the on-disk ACLs. Returns an error so the caller can fail -// closed. A tab separates the fields because Windows paths never contain one, so -// the path is recovered unambiguously regardless of spaces in it. -func (r *windowsResidueRun) recordBeforeApply(kind residueKind, path string) error { - if r == nil { - return fmt.Errorf("residue marker is required") - } - dir := windowsDenyMarkerDir() - if err := os.MkdirAll(dir, 0o700); err != nil { - return fmt.Errorf("create residue marker dir: %w", err) - } - f, err := os.OpenFile(r.marker, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0o600) - if err != nil { - return fmt.Errorf("open residue marker: %w", err) - } - defer f.Close() - r.owned.Store(true) - if _, err := f.WriteString(string(kind) + "\t" + path + "\n"); err != nil { - return fmt.Errorf("write residue marker: %w", err) - } - if err := f.Sync(); err != nil { - return fmt.Errorf("sync residue marker: %w", err) - } - return nil -} - -// clear drops this run's marker after its own cleanup has removed every -// recorded ACE. It only removes the run-local marker this object created; stale -// predecessor markers that happen to carry this process's PID stay available for -// the next sweep instead of being orphaned. -func (r *windowsResidueRun) clear() { - if r == nil { - return - } - liveResidueMarkers.Delete(r.marker) - if !r.owned.Load() { - return - } - _ = os.Remove(r.marker) - r.owned.Store(false) -} - -// sweepWindowsDenyResidue removes ACEs left by crashed runs. It only acts on -// markers whose owning process is provably gone: a PID that is no longer -// alive, or a file with this process's PID that is not in this process's live -// marker registry. Windows reuses PIDs, so an unregistered self-PID marker -// records a dead predecessor's residue, while registered self-PID markers belong -// to concurrent live Run calls. Only the stable sandbox-applied trustees are -// removed, so it never disturbs a live run or a legitimate ACL. Best-effort: any -// error is ignored so it can never block a run. -func sweepWindowsDenyResidue() { - dir := windowsDenyMarkerDir() - entries, err := os.ReadDir(dir) - if err != nil { - return - } - sandboxSIDs := sandboxResidueSIDs() - self := strconv.Itoa(os.Getpid()) - for _, entry := range entries { - if entry.IsDir() { - continue - } - pid, ok := windowsDenyMarkerOwnerPID(entry.Name()) - if !ok { - continue - } - markerPath := filepath.Join(dir, entry.Name()) - if pid == self { - // Our PID is live only for markers registered by this process. Any - // unregistered self-PID marker was left by a dead predecessor after PID - // reuse, so consume it like ordinary crash residue. - if _, live := liveResidueMarkers.Load(markerPath); live { - continue - } - } else if windowsProcessAlive(pid) { - continue - } - sweepResidueMarkerFile(markerPath, sandboxSIDs) - } -} - -// sandboxResidueSIDs is the trustee set the sweep removes. The sandbox only -// ever applies these trustees, for both grants and denies, so removing exactly -// them cannot disturb a legitimate ACL. -func sandboxResidueSIDs() []string { - userSID, _ := currentProcessUserSIDString() - return dedupeSIDStrings([]string{ - allApplicationPackagesSID, - allRestrictedApplicationPackagesSID, - userSID, - }) -} - -// sweepResidueMarkerFile removes the ACEs recorded in one dead run's marker, -// then the marker itself. -func sweepResidueMarkerFile(markerPath string, sandboxSIDs []string) { - for _, e := range readResidueMarker(markerPath) { - if !pathExists(e.path) || !sweepableResidue(e) { - continue - } - switch e.kind { - case residueDeny: - removeDeniedAppContainerSIDs(e.path, sandboxSIDs) - case residueGrant: - removeGrantedAppContainerSIDs(e.path, sandboxSIDs) - } - } - _ = os.Remove(markerPath) -} - -// sweepableResidue reports whether a marker entry names a path the sandbox -// could actually have mutated. The current code never records a system -// directory (windowsMutableExecutableGrantRoots filters them before any -// snapshot, grant, or marker line), but the marker file itself is untrusted -// input: an older sandbox binary may have written one before that filter -// existed, and the marker directory lives under the user's %TEMP%, writable to -// anything running as the same user. Removing the broad built-in package SIDs -// (ALL APPLICATION PACKAGES / ALL RESTRICTED APPLICATION PACKAGES) from -// System32 or Program Files would strip factory ACEs the OS itself relies on, -// so the sweep re-checks the invariant instead of trusting the marker. -func sweepableResidue(e residueEntry) bool { - return !isWindowsSystemRoot(e.path) -} - -// readResidueMarker parses "\t" lines. A tab splits the fields so a -// path containing spaces is preserved intact. An unrecognized line is skipped -// rather than guessed at, so a corrupt marker cannot cause a wrong ACE removal. -func readResidueMarker(path string) []residueEntry { - f, err := os.Open(path) - if err != nil { - return nil - } - defer f.Close() - var out []residueEntry - scanner := bufio.NewScanner(f) - for scanner.Scan() { - line := strings.TrimRight(scanner.Text(), "\r\n") - if line == "" { - continue - } - kindStr, p, ok := strings.Cut(line, "\t") - if !ok || p == "" { - continue - } - switch residueKind(kindStr) { - case residueDeny: - out = append(out, residueEntry{kind: residueDeny, path: p}) - case residueGrant: - out = append(out, residueEntry{kind: residueGrant, path: p}) - } - } - return out -} - -// windowsProcessAlive reports whether the given PID is still running. A parse -// failure or a process that cannot be opened is treated as not-alive so stale -// markers get cleaned; PID reuse can only delay cleanup, never corrupt a live -// run or lose residue: same-process live markers are protected by -// liveResidueMarkers, and a stale marker whose PID is reused by an unrelated -// process merely waits until that process exits — unless the reuser is this -// process, in which case the marker is unregistered and swept at run start. -func windowsProcessAlive(pidStr string) bool { - pid, err := strconv.ParseUint(pidStr, 10, 32) - if err != nil || pid == 0 { - return false - } - handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid)) - if err != nil { - return false - } - defer windows.CloseHandle(handle) - var code uint32 - if err := windows.GetExitCodeProcess(handle, &code); err != nil { - return false - } - return code == stillActiveExitCode -} diff --git a/internal/winsandbox/coordination_windows_test.go b/internal/winsandbox/coordination_windows_test.go deleted file mode 100644 index dd90f561cf..0000000000 --- a/internal/winsandbox/coordination_windows_test.go +++ /dev/null @@ -1,841 +0,0 @@ -//go:build windows - -package winsandbox - -import ( - "bytes" - "os" - "path/filepath" - "slices" - "sort" - "strconv" - "strings" - "sync" - "testing" - "time" -) - -// lockNoticeBuffer is an io.Writer safe to read while the contender goroutine -// writes the queue notice into it. -type lockNoticeBuffer struct { - mu sync.Mutex - b bytes.Buffer -} - -func (s *lockNoticeBuffer) Write(p []byte) (int, error) { - s.mu.Lock() - defer s.mu.Unlock() - return s.b.Write(p) -} - -func (s *lockNoticeBuffer) String() string { - s.mu.Lock() - defer s.mu.Unlock() - return s.b.String() -} - -func TestWindowsRootLockQueueEmitsNotice(t *testing.T) { - root := t.TempDir() - // Long enough for the contender to sit through one full silent slice - // (windowsRootLockNoticeAfter) and emit the queue notice before timing out. - t.Setenv("WINDOWS_SANDBOX_LOCK_MS", "5000") - held, err := lockWindowsRoots([]string{root}, nil, "", 0) - if err != nil { - t.Fatalf("hold lock: %v", err) - } - - var notice lockNoticeBuffer - done := make(chan struct{}) - go func() { - defer close(done) - lock, err := lockWindowsRoots([]string{root}, ¬ice, "", 0) - if err == nil { - lock.release() - } - }() - - deadline := time.Now().Add(4 * time.Second) - for time.Now().Before(deadline) { - if strings.Contains(notice.String(), "waiting for another sandboxed command") { - break - } - time.Sleep(50 * time.Millisecond) - } - if !strings.Contains(notice.String(), "waiting for another sandboxed command") { - t.Fatalf("queued lock never emitted a notice; got %q", notice.String()) - } - held.release() - select { - case <-done: - case <-time.After(3 * time.Second): - t.Fatal("contender never finished after release") - } -} - -// A held root lock records its holder (pid + command label); a queued -// contender's notice names that command so the user knows what to stop, and -// release removes the record again. -func TestWindowsRootLockHolderRecordedAndCleared(t *testing.T) { - root := t.TempDir() - t.Setenv("WINDOWS_SANDBOX_LOCK_MS", "5000") - held, err := lockWindowsRoots([]string{root}, nil, "npm run dev", 0) - if err != nil { - t.Fatalf("hold lock: %v", err) - } - name := windowsRootLockNames([]string{root})[0] - data, err := os.ReadFile(windowsLockHolderPath(name)) - if err != nil { - t.Fatalf("holder record not written: %v", err) - } - rec, ok := parseLockHolderRecord(string(data)) - if !ok || rec.pid != os.Getpid() || rec.label != "npm run dev" { - t.Fatalf("holder record = %+v ok=%v, want this process with label", rec, ok) - } - - var notice lockNoticeBuffer - done := make(chan struct{}) - go func() { - defer close(done) - lock, err := lockWindowsRoots([]string{root}, ¬ice, "", 0) - if err == nil { - lock.release() - } - }() - deadline := time.Now().Add(4 * time.Second) - for time.Now().Before(deadline) { - if strings.Contains(notice.String(), `held by "npm run dev"`) { - break - } - time.Sleep(50 * time.Millisecond) - } - if !strings.Contains(notice.String(), `held by "npm run dev"`) { - t.Fatalf("queued notice never named the holder; got %q", notice.String()) - } - held.release() - select { - case <-done: - case <-time.After(3 * time.Second): - t.Fatal("contender never finished after release") - } - if pathExists(windowsLockHolderPath(name)) { - t.Fatal("holder record not removed after release") - } -} - -func TestWindowsRootLockNamesAreSortedAndDeduped(t *testing.T) { - names := windowsRootLockNames([]string{ - `C:\work\b`, - `C:\work\a`, - `C:\WORK\A`, // same as a, different case - "", - ".", - }) - if len(names) < 2 { - t.Fatalf("lock names = %v, want roots and their ancestor domains", names) - } - if !sort.StringsAreSorted(names) { - t.Fatalf("lock names must be sorted for deadlock-free acquisition: %v", names) - } - for _, n := range names { - if !strings.HasPrefix(n, `Local\windows-sandbox.`) { - t.Fatalf("unexpected lock name %q", n) - } - } - // Case-insensitive dedup: A and a must collapse to one name. - same := windowsRootLockNames([]string{`C:\work\a`}) - if len(same) == 0 { - t.Fatal("single root produced no lock domains") - } - for _, name := range same { - if !contains(names, name) { - t.Fatalf("case-insensitive dedup broken: %v vs %v", names, same) - } - } - if len(windowsRootLockNames([]string{`C:\work\a`, `C:\WORK\A`})) != len(same) { - t.Fatalf("case-insensitive dedup broken: %v vs %v", names, same) - } -} - -func TestWindowsRootLockParentAndChildShareDomain(t *testing.T) { - parent := windowsRootLockNames([]string{`C:\work\repo`}) - child := windowsRootLockNames([]string{`C:\work\repo\secret`}) - shared := false - for _, candidate := range parent { - if contains(child, candidate) { - shared = true - break - } - } - if !shared { - t.Fatalf("parent and child locks do not conflict: parent=%v child=%v", parent, child) - } -} - -func TestWindowsRootLockSerializesSameRoot(t *testing.T) { - root := t.TempDir() - // The two locks target the same root, so the second acquire must block until - // the first releases. Use a short timeout so a regression fails fast. - t.Setenv("WINDOWS_SANDBOX_LOCK_MS", "2000") - - first, err := lockWindowsRoots([]string{root}, nil, "", 0) - if err != nil { - t.Fatalf("first lock: %v", err) - } - - acquired := make(chan *windowsRootLock, 1) - go func() { - second, err := lockWindowsRoots([]string{root}, nil, "", 0) - if err != nil { - acquired <- nil - return - } - acquired <- second - }() - - select { - case <-acquired: - t.Fatal("second lock acquired while first was held; roots not serialized") - case <-time.After(300 * time.Millisecond): - // Expected: still blocked. - } - - first.release() - select { - case second := <-acquired: - if second == nil { - t.Fatal("second lock failed to acquire after first released") - } - second.release() - case <-time.After(3 * time.Second): - t.Fatal("second lock never acquired after first released") - } -} - -func TestWindowsRootLockTimesOutWhenHeld(t *testing.T) { - root := t.TempDir() - t.Setenv("WINDOWS_SANDBOX_LOCK_MS", "300") - held, err := lockWindowsRoots([]string{root}, nil, "", 0) - if err != nil { - t.Fatalf("hold lock: %v", err) - } - defer held.release() - - // The contender must run on its own goroutine, not the holder's: a Windows - // mutex is recursive for its owning OS thread, so re-acquiring from the same - // thread would succeed instead of timing out. The holder pins its thread, so - // the contender goroutine lands on a different thread and genuinely blocks. - type result struct { - err error - elapsed time.Duration - } - done := make(chan result, 1) - go func() { - start := time.Now() - lock, err := lockWindowsRoots([]string{root}, nil, "", 0) - if lock != nil { - lock.release() - } - done <- result{err: err, elapsed: time.Since(start)} - }() - - select { - case r := <-done: - if r.err == nil { - t.Fatal("expected timeout acquiring a held lock") - } - if !strings.Contains(r.err.Error(), "timed out") { - t.Fatalf("error = %v, want timeout", r.err) - } - if r.elapsed > 2*time.Second { - t.Fatalf("lock timeout took too long: %s", r.elapsed) - } - case <-time.After(3 * time.Second): - t.Fatal("contender never returned") - } -} - -func TestWindowsRootLockMultiRootNoSelfDeadlock(t *testing.T) { - a := t.TempDir() - b := t.TempDir() - t.Setenv("WINDOWS_SANDBOX_LOCK_MS", "2000") - // Acquiring multiple roots in one call must not deadlock regardless of the - // order the caller passes them, because names are sorted internally. - lock, err := lockWindowsRoots([]string{b, a, b}, nil, "", 0) - if err != nil { - t.Fatalf("multi-root lock: %v", err) - } - lock.release() -} - -func TestWindowsResidueMarkerIncrementalRoundTrip(t *testing.T) { - // Redirect the marker dir into a temp dir so the test never touches the real - // %TEMP%. windowsDenyMarkerDir derives from os.TempDir, so overriding TMP is - // enough on Windows. - tmp := t.TempDir() - t.Setenv("TMP", tmp) - t.Setenv("TEMP", tmp) - - // Append entries one at a time, as the run does before applying each ACE. - run := newWindowsDenyResidueRun() - defer run.clear() - if err := run.recordBeforeApply(residueDeny, `C:\Users\me\.ssh`); err != nil { - t.Fatalf("record deny: %v", err) - } - if err := run.recordBeforeApply(residueGrant, `C:\Users\me\tools`); err != nil { - t.Fatalf("record grant: %v", err) - } - - marker := run.marker - if !pathExists(marker) { - t.Fatalf("marker not written at %s", marker) - } - got := readResidueMarker(marker) - want := []residueEntry{ - {kind: residueDeny, path: `C:\Users\me\.ssh`}, - {kind: residueGrant, path: `C:\Users\me\tools`}, - } - if len(got) != len(want) { - t.Fatalf("marker round-trip = %v, want %v", got, want) - } - for i := range want { - if got[i] != want[i] { - t.Fatalf("marker[%d] = %+v, want %+v", i, got[i], want[i]) - } - } - - run.clear() - if pathExists(marker) { - t.Fatal("marker not cleared") - } -} - -func TestWindowsResidueMarkerSkipsMalformedLines(t *testing.T) { - // A corrupt or unrecognized marker line must be skipped, never guessed at, so - // a damaged marker cannot drive a wrong ACE removal. - tmp := t.TempDir() - t.Setenv("TMP", tmp) - t.Setenv("TEMP", tmp) - if err := os.MkdirAll(windowsDenyMarkerDir(), 0o700); err != nil { - t.Fatal(err) - } - marker := windowsDenyMarkerPath() - content := "deny\tC:\\Users\\me\\.ssh\n" + // valid - "garbage-no-tab\n" + // no separator - "boguskind\tC:\\x\n" + // unknown kind - "grant\t\n" + // empty path - "grant\tC:\\Users\\me\\my tools\n" // valid, path with a space - if err := os.WriteFile(marker, []byte(content), 0o600); err != nil { - t.Fatal(err) - } - got := readResidueMarker(marker) - want := []residueEntry{ - {kind: residueDeny, path: `C:\Users\me\.ssh`}, - {kind: residueGrant, path: `C:\Users\me\my tools`}, - } - if len(got) != len(want) { - t.Fatalf("parsed = %v, want %v", got, want) - } - for i := range want { - if got[i] != want[i] { - t.Fatalf("entry %d = %+v, want %+v", i, got[i], want[i]) - } - } -} - -func TestWindowsResidueMarkerWriteFailurePropagates(t *testing.T) { - // If the marker cannot be created, recordResidueBeforeApply must return an - // error so the caller fails closed instead of applying an untracked ACE. Point - // TMP at a path that is a file, so MkdirAll of the marker dir underneath it - // fails. - tmp := t.TempDir() - fileAsTemp := filepath.Join(tmp, "not-a-dir") - if err := os.WriteFile(fileAsTemp, []byte("x"), 0o600); err != nil { - t.Fatal(err) - } - t.Setenv("TMP", fileAsTemp) - t.Setenv("TEMP", fileAsTemp) - run := newWindowsDenyResidueRun() - defer run.clear() - if err := run.recordBeforeApply(residueDeny, `C:\Users\me\.ssh`); err == nil { - t.Fatal("expected error when the marker directory cannot be created") - } -} - -func TestWindowsResidueMarkerPIDReuseLifecycle(t *testing.T) { - // The marker path is keyed by PID alone and Windows reuses PIDs, so a file - // at our own path is not necessarily ours. A run must not delete a - // predecessor's marker as if it were its own (that orphans the recorded - // residue forever), and the run-start sweep must consume it rather than - // skip it as "self". - tmp := t.TempDir() - t.Setenv("TMP", tmp) - t.Setenv("TEMP", tmp) - if err := os.MkdirAll(windowsDenyMarkerDir(), 0o700); err != nil { - t.Fatal(err) - } - - // A marker at our PID's path that this process never wrote models the dead - // predecessor. The recorded path deliberately does not exist so consuming - // it exercises only the marker lifecycle, not icacls. - stale := windowsDenyMarkerPath() - if err := os.WriteFile(stale, []byte("grant\t"+filepath.Join(tmp, "gone-tool-dir")+"\n"), 0o600); err != nil { - t.Fatal(err) - } - - // Cleanup must leave a marker this process did not write. - run := newWindowsDenyResidueRun() - run.clear() - if !pathExists(stale) { - t.Fatal("clear removed a marker this process never wrote, orphaning the predecessor's residue") - } - - // The sweep must consume it instead of skipping pid == self. - sweepWindowsDenyResidue() - if pathExists(stale) { - t.Fatal("sweep skipped a dead predecessor's marker at our own PID path") - } - - // Once this process records, the marker is owned: the sweep must leave it - // alone and cleanup must remove it. - run = newWindowsDenyResidueRun() - if err := run.recordBeforeApply(residueDeny, filepath.Join(tmp, "own-root")); err != nil { - t.Fatalf("record own entry: %v", err) - } - sweepWindowsDenyResidue() - if !pathExists(run.marker) { - t.Fatal("sweep removed this process's live marker") - } - run.clear() - if pathExists(run.marker) { - t.Fatal("clear did not remove this process's own marker") - } -} - -func TestWindowsResidueRecordDoesNotMixWithStalePredecessorMarker(t *testing.T) { - // If recording starts while a dead predecessor's same-PID marker is still - // present (a caller that records without sweeping first), this run must use a - // distinct marker. Mixing the two runs' lines would let this run's cleanup - // delete the predecessor's record while only this run's ACEs had been removed. - tmp := t.TempDir() - t.Setenv("TMP", tmp) - t.Setenv("TEMP", tmp) - if err := os.MkdirAll(windowsDenyMarkerDir(), 0o700); err != nil { - t.Fatal(err) - } - - stale := windowsDenyMarkerPath() - if err := os.WriteFile(stale, []byte("grant\t"+filepath.Join(tmp, "predecessor-tool-dir")+"\n"), 0o600); err != nil { - t.Fatal(err) - } - run := newWindowsDenyResidueRun() - defer run.clear() - ownRoot := filepath.Join(tmp, "own-root") - if err := run.recordBeforeApply(residueDeny, ownRoot); err != nil { - t.Fatalf("record own entry: %v", err) - } - got := readResidueMarker(run.marker) - want := []residueEntry{{kind: residueDeny, path: ownRoot}} - if len(got) != 1 || got[0] != want[0] { - t.Fatalf("marker after first record = %v, want only this run's entry %v", got, want) - } - if !pathExists(stale) { - t.Fatal("this run removed the predecessor's marker") - } - run.clear() - if pathExists(run.marker) { - t.Fatal("clear did not remove this process's own marker") - } - if !pathExists(stale) { - t.Fatal("clear removed the predecessor's marker") - } - sweepWindowsDenyResidue() - if pathExists(stale) { - t.Fatal("sweep did not consume the stale predecessor marker") - } -} - -func TestWindowsResidueConcurrentRunsOwnDistinctMarkers(t *testing.T) { - // Concurrent Run calls in one Go process share a PID but must not share one - // marker: the first run to clean up would otherwise delete the second run's - // residue record while the second run is still live. - tmp := t.TempDir() - t.Setenv("TMP", tmp) - t.Setenv("TEMP", tmp) - - a := newWindowsDenyResidueRun() - b := newWindowsDenyResidueRun() - defer a.clear() - defer b.clear() - if a.marker == b.marker { - t.Fatalf("concurrent runs share marker %s", a.marker) - } - if err := a.recordBeforeApply(residueDeny, filepath.Join(tmp, "a-root")); err != nil { - t.Fatalf("record run A: %v", err) - } - if err := b.recordBeforeApply(residueGrant, filepath.Join(tmp, "b-root")); err != nil { - t.Fatalf("record run B: %v", err) - } - - a.clear() - if pathExists(a.marker) { - t.Fatal("run A marker was not cleared") - } - if !pathExists(b.marker) { - t.Fatal("run A cleanup removed run B marker") - } - sweepWindowsDenyResidue() - if !pathExists(b.marker) { - t.Fatal("sweep removed a live same-process marker") - } - b.clear() - if pathExists(b.marker) { - t.Fatal("run B marker was not cleared") - } -} - -func TestSweepableResidueRefusesSystemPaths(t *testing.T) { - // The sweep must never act on a marker entry that names a system directory, - // even though the current recorder can no longer write one: a marker left by - // an older binary, or planted in the same-user-writable %TEMP%, would - // otherwise drive icacls /remove:g of the broad built-in package SIDs against - // System32 / Program Files and strip factory ACEs. User paths must stay - // sweepable or crash residue would never be cleaned. - userDir := t.TempDir() - for _, e := range []residueEntry{ - {kind: residueDeny, path: filepath.Join(userDir, ".ssh")}, - {kind: residueGrant, path: userDir}, - } { - if !sweepableResidue(e) { - t.Fatalf("user path %q must be sweepable", e.path) - } - } - - sysExe := systemRootTool("icacls.exe") - if !filepath.IsAbs(sysExe) { - t.Skip("no absolute system tool to derive a system directory from") - } - sysDir := filepath.Dir(sysExe) - for _, e := range []residueEntry{ - {kind: residueGrant, path: sysDir}, - {kind: residueDeny, path: sysDir}, - {kind: residueGrant, path: filepath.Join(sysDir, "sub")}, - } { - if sweepableResidue(e) { - t.Fatalf("system path %q must never be sweepable", e.path) - } - } - if root := os.Getenv("ProgramFiles"); root != "" { - if sweepableResidue(residueEntry{kind: residueGrant, path: root}) { - t.Fatalf("Program Files root %q must never be sweepable", root) - } - } -} - -func TestWindowsMutatedRootsForRunLocksNonSystemExeDirOnly(t *testing.T) { - workspace := t.TempDir() - // A non-system tool directory must join the lock set; a Windows system - // directory must not, or commands sharing the system shell would needlessly - // serialize. Membership is by path, not by a write probe, so this holds even - // when the test process is elevated (as CI's runner is). - toolDir := t.TempDir() - toolExe := filepath.Join(toolDir, "mytool.exe") - if err := os.WriteFile(toolExe, []byte("not really an exe"), 0o644); err != nil { - t.Fatal(err) - } - - got := windowsMutatedRootsForRun(Spec{WritableRoots: []string{workspace}, Writable: true}, toolExe) - if !containsWindowsPath(got, workspace) { - t.Fatalf("mutated roots = %v, want workspace", got) - } - if !containsWindowsPath(got, toolDir) { - t.Fatalf("mutated roots = %v, want tool dir %s", got, toolDir) - } - - // A system executable's directory (System32) must be excluded regardless of - // the process's integrity level. Resolve a real system tool to avoid - // depending on PATH layout. - sysExe := systemRootTool("icacls.exe") - if !filepath.IsAbs(sysExe) { - t.Skip("no absolute system tool to test the system-root exclusion") - } - sysRoots := windowsMutatedRootsForRun(Spec{WritableRoots: []string{workspace}, Writable: true}, sysExe) - if containsWindowsPath(sysRoots, filepath.Dir(sysExe)) { - t.Fatalf("system exe dir %s must not join the lock set: %v", filepath.Dir(sysExe), sysRoots) - } -} - -func TestWindowsMutableExecutableGrantRootsExcludesSystemDirs(t *testing.T) { - // The grant loop (grantAppContainerExecutable) and the per-root lock both draw - // their executable roots from windowsMutableExecutableGrantRoots, so it is the - // single guard that keeps system directories from being snapshotted, granted, - // or recorded as crash residue. A residue entry on System32 would let a later - // sweep run icacls /remove:g for the broad built-in package SIDs and strip the - // directory's factory ACEs, so a system tool dir must never appear here. - toolDir := t.TempDir() - toolExe := filepath.Join(toolDir, "mytool.exe") - if err := os.WriteFile(toolExe, []byte("not really an exe"), 0o644); err != nil { - t.Fatal(err) - } - if got := windowsMutableExecutableGrantRoots(toolExe); !containsWindowsPath(got, toolDir) { - t.Fatalf("grant roots = %v, want non-system tool dir %s", got, toolDir) - } - - sysExe := systemRootTool("icacls.exe") - if !filepath.IsAbs(sysExe) { - t.Skip("no absolute system tool to test the system-root exclusion") - } - sysDir := filepath.Dir(sysExe) - got := windowsMutableExecutableGrantRoots(sysExe) - if containsWindowsPath(got, sysDir) { - t.Fatalf("system exe dir %s must never be a mutable grant root: %v", sysDir, got) - } - // The same executable must still resolve to a non-empty grant candidate before - // the system filter, so the exclusion is what drops it — not a resolution miss. - if len(windowsExecutableGrantRoots(sysExe)) == 0 { - t.Fatalf("system exe %s should resolve to a grant candidate before filtering", sysExe) - } -} - -func TestIsWindowsSystemRoot(t *testing.T) { - // System locations are excluded by path, independent of writability. - sysExe := systemRootTool("icacls.exe") - if filepath.IsAbs(sysExe) && !isWindowsSystemRoot(filepath.Dir(sysExe)) { - t.Fatalf("%s should be classified as a system root", filepath.Dir(sysExe)) - } - if root := os.Getenv("ProgramFiles"); root != "" { - if !isWindowsSystemRoot(filepath.Join(root, "SomeApp")) { - t.Fatalf("a path under %s should be a system root", root) - } - } - // A user temp directory is never a system root. - if isWindowsSystemRoot(t.TempDir()) { - t.Fatal("a user temp dir must not be classified as a system root") - } -} - -func TestWindowsProcessAliveDetectsSelfAndDead(t *testing.T) { - if !windowsProcessAlive(strconv.Itoa(os.Getpid())) { - t.Fatal("current process should be reported alive") - } - // PID 0 and garbage never map to a live user process. - for _, dead := range []string{"0", "not-a-pid", ""} { - if windowsProcessAlive(dead) { - t.Fatalf("%q should not be reported alive", dead) - } - } -} - -func TestWindowsMutatedRootsIncludesForbidReadThatExists(t *testing.T) { - workspace := t.TempDir() - secret := filepath.Join(workspace, "secret") - if err := os.Mkdir(secret, 0o755); err != nil { - t.Fatal(err) - } - missing := filepath.Join(workspace, "does-not-exist") - got := windowsMutatedRoots(Spec{ - WritableRoots: []string{workspace}, - ForbidReadRoots: []string{secret, missing}, - Writable: true, - }) - if !containsWindowsPath(got, workspace) || !containsWindowsPath(got, secret) { - t.Fatalf("mutated roots = %v, want workspace and existing secret", got) - } - if containsWindowsPath(got, missing) { - t.Fatalf("mutated roots must skip missing forbid_read paths: %v", got) - } -} - -func TestWindowsICACLSTimeoutRecursiveVsFlat(t *testing.T) { - os.Unsetenv("WINDOWS_SANDBOX_ICACLS_TIMEOUT_MS") - if got := icaclsTimeoutForArgs([]string{"/setintegritylevel", "L", "/T", "/C"}); got != defaultICACLSRecursiveTimeout { - t.Fatalf("recursive timeout = %s, want %s", got, defaultICACLSRecursiveTimeout) - } - if got := icaclsTimeoutForArgs([]string{"/setintegritylevel", "M", "/C"}); got != defaultICACLSTimeout { - t.Fatalf("flat timeout = %s, want %s", got, defaultICACLSTimeout) - } - t.Setenv("WINDOWS_SANDBOX_ICACLS_TIMEOUT_MS", "1234") - if got := icaclsTimeoutForArgs([]string{"/T"}); got != 1234*time.Millisecond { - t.Fatalf("env override = %s, want 1.234s", got) - } -} - -func TestSystemRootToolResolvesUnderSystem32(t *testing.T) { - got := systemRootTool("icacls.exe") - // On any real Windows host this resolves to an absolute System32 path; the - // fallback (bare name) only happens if the file is genuinely missing. - if got == "icacls.exe" { - t.Skip("icacls.exe not found under System32 on this host") - } - if !filepath.IsAbs(got) || !strings.EqualFold(filepath.Base(got), "icacls.exe") { - t.Fatalf("resolved tool = %q, want absolute System32 icacls.exe", got) - } - if !strings.Contains(strings.ToLower(got), `system32`) { - t.Fatalf("resolved tool = %q, want a System32 path", got) - } -} - -// TestWindowsSandboxConcurrentWritesToSharedWorkspace exercises the concurrency -// fix end-to-end: several sandboxed writable commands run at once against the -// same non-empty workspace with nested directories. Before the root lock, their -// ACL/label mutations and snapshot restores would interleave and one command's -// cleanup could revoke another's write grant mid-run, surfacing as a spurious -// failure. With serialization every command must succeed and its file must be -// written. The empty-temp-dir CI coverage that shipped originally could not -// catch this. -func TestWindowsSandboxConcurrentWritesToSharedWorkspace(t *testing.T) { - if !Available() { - t.Skip("windows sandbox APIs unavailable") - } - sh := powershellArgvForTest(t, "") - if sh == nil { - t.Skip("PowerShell unavailable") - } - workspace := t.TempDir() - // Pre-populate with nested content so the writable relabel actually walks a - // subtree (the empty-dir case hid #1/#2/#3). - nested := filepath.Join(workspace, "pkg", "sub") - if err := os.MkdirAll(nested, 0o755); err != nil { - t.Fatal(err) - } - for _, f := range []string{"a.txt", filepath.Join("pkg", "b.txt"), filepath.Join("pkg", "sub", "c.txt")} { - if err := os.WriteFile(filepath.Join(workspace, f), []byte("old"), 0o644); err != nil { - t.Fatal(err) - } - } - - const n = 4 - var wg sync.WaitGroup - errs := make([]error, n) - for i := range n { - wg.Add(1) - go func(idx int) { - defer wg.Done() - target := filepath.Join(workspace, "out"+strconv.Itoa(idx)+".txt") - script := "$ErrorActionPreference='Stop'; Set-Content -LiteralPath " + psQuote(target) + " -Value ok" - result, err := Run( - Spec{WritableRoots: []string{workspace}, Network: true, Writable: true, TempPrefix: "windows-sandbox-test-"}, - append(sh, script), - RunOptions{Stdin: os.Stdin, Stdout: os.Stdout, Stderr: os.Stderr}, - ) - if err != nil { - errs[idx] = err - return - } - if result.ExitCode != 0 { - errs[idx] = errExitf(idx, result.ExitCode) - } - }(i) - } - wg.Wait() - - for i := range n { - if errs[i] != nil { - t.Fatalf("concurrent command %d failed: %v", i, errs[i]) - } - target := filepath.Join(workspace, "out"+strconv.Itoa(i)+".txt") - if got, err := os.ReadFile(target); err != nil || !strings.Contains(string(got), "ok") { - t.Fatalf("concurrent command %d output missing: %q err=%v", i, got, err) - } - } - // The deterministic capability ACE intentionally remains. Broad AppContainer - // grants and temporary current-user deny ACEs must not remain. - assertNoWindowsSandboxACEForTest(t, workspace) -} - -// TestWindowsSandboxConcurrentDistinctWorkspacesSharedToolDir is the P2 -// regression: two runs in *different* workspaces (so their workspace locks do -// not collide) that invoke a tool from one shared user-writable directory. That -// shared exe directory is snapshot/grant/restored by each run, so without -// folding it into the lock the two runs would interleave their ACL snapshots and -// one could fail or leave a stale grant. Both must succeed and the shared tool -// directory must carry no leftover sandbox ACE. -func TestWindowsSandboxConcurrentDistinctWorkspacesSharedToolDir(t *testing.T) { - if !Available() { - t.Skip("windows sandbox APIs unavailable") - } - // Stage a copy of cmd.exe in a shared user directory as the tool. cmd.exe is a - // single self-contained binary with no side-by-side runtime dependencies, so a - // copy runs standalone — unlike pwsh.exe, which needs its hostfxr/DLLs next to - // it. Each concurrent run resolves argv[0] into this shared directory, so the - // runs contend on it even though their workspaces differ. - cmdExe := systemRootTool("cmd.exe") - if !filepath.IsAbs(cmdExe) { - t.Skip("cmd.exe not found under System32") - } - toolDir := t.TempDir() - sharedTool := filepath.Join(toolDir, "sandbox-shared-tool.exe") - if err := copyFileForTest(t, cmdExe, sharedTool); err != nil { - t.Skipf("cannot stage shared tool: %v", err) - } - - const n = 4 - var wg sync.WaitGroup - errs := make([]error, n) - for i := range n { - wg.Add(1) - go func(idx int) { - defer wg.Done() - workspace := t.TempDir() // distinct per run: workspace locks do not collide - target := filepath.Join(workspace, "out.txt") - // cmd.exe /c echo ok> "". No spaces before '>' so the redirect - // target is exactly the path. - toolArgv := []string{sharedTool, "/c", "echo ok> " + target} - result, err := Run( - Spec{WritableRoots: []string{workspace}, Network: true, Writable: true, TempPrefix: "windows-sandbox-test-"}, - toolArgv, - RunOptions{Stdin: os.Stdin, Stdout: os.Stdout, Stderr: os.Stderr}, - ) - if err != nil { - errs[idx] = err - return - } - if result.ExitCode != 0 { - errs[idx] = errExitf(idx, result.ExitCode) - } - }(i) - } - wg.Wait() - - for i := range n { - if errs[i] != nil { - t.Fatalf("concurrent run %d sharing tool dir failed: %v", i, errs[i]) - } - } - // The shared tool directory must be left with no sandbox grant residue. - assertNoWindowsSandboxACEForTest(t, toolDir) -} - -func copyFileForTest(t *testing.T, src, dst string) error { - t.Helper() - data, err := os.ReadFile(src) - if err != nil { - return err - } - return os.WriteFile(dst, data, 0o755) -} - -func contains(haystack []string, needle string) bool { - return slices.Contains(haystack, needle) -} - -func containsWindowsPath(haystack []string, needle string) bool { - for _, h := range haystack { - if sameWindowsPath(h, needle) { - return true - } - } - return false -} - -func errExitf(idx, code int) error { - return &exitError{idx: idx, code: code} -} - -type exitError struct { - idx int - code int -} - -func (e *exitError) Error() string { - return "command " + strconv.Itoa(e.idx) + " exit code " + strconv.Itoa(e.code) -} diff --git a/internal/winsandbox/failure_phase_test.go b/internal/winsandbox/failure_phase_test.go deleted file mode 100644 index ec9df74a9f..0000000000 --- a/internal/winsandbox/failure_phase_test.go +++ /dev/null @@ -1,19 +0,0 @@ -package winsandbox - -import ( - "errors" - "testing" -) - -func TestFailurePhaseOfUsesTypedRunnerStage(t *testing.T) { - base := errors.New("same text for every stage") - for _, want := range []FailurePhase{FailureAuthorization, FailureDependency, FailureLaunch} { - got, ok := FailurePhaseOf(failureAt(want, base)) - if !ok || got != want { - t.Fatalf("phase = %q, %v; want %q", got, ok, want) - } - } - if _, ok := FailurePhaseOf(base); ok { - t.Fatal("an untyped post-start error must remain in the execution lane") - } -} diff --git a/internal/winsandbox/lockinfo.go b/internal/winsandbox/lockinfo.go deleted file mode 100644 index a6f0b81360..0000000000 --- a/internal/winsandbox/lockinfo.go +++ /dev/null @@ -1,126 +0,0 @@ -package winsandbox - -import ( - "fmt" - "os" - "strconv" - "strings" - "time" - "unicode" -) - -// The per-root lock serializes whole sandboxed runs (see the package comment in -// coordination_windows.go). An interactive command queued behind a long-running -// holder should fail fast with the holder named rather than hang its turn, so -// the default wait is short; a caller whose run nobody is blocked on (a -// background job) passes a longer budget via Spec.LockWait. -const defaultWindowsRootLockTimeout = time.Minute - -// windowsRootLockTimeout resolves one run's lock wait budget: the explicit -// WINDOWS_SANDBOX_LOCK_MS override wins, then the caller's Spec.LockWait, then -// the short interactive default. -func windowsRootLockTimeout(specWait time.Duration) time.Duration { - if raw := os.Getenv("WINDOWS_SANDBOX_LOCK_MS"); raw != "" { - if ms, err := strconv.ParseUint(raw, 10, 63); err == nil && ms > 0 { - return time.Duration(ms) * time.Millisecond - } - } - if specWait > 0 { - return specWait - } - return defaultWindowsRootLockTimeout -} - -// A lock-holder record names the run currently holding one root lock so a -// queued command can tell the user what to stop. It is diagnostic only: written -// best-effort after the mutex is acquired, removed before it is released, and -// validated against a live PID on read, so a stale file left by a crash (the OS -// releases the abandoned mutex itself) is ignored and overwritten by the next -// holder of the same root. One line: "\t\t