From 700c71cf668ec78ef7ad848157fe06a60849a501 Mon Sep 17 00:00:00 2001 From: liugddx Date: Mon, 17 Aug 2026 17:30:28 +0800 Subject: [PATCH 1/9] feat(windows-sandbox): implement production-identity readiness probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows AppContainer backend advertised availability from file existence alone (`existsSync(clientPath)`), which never proves the host can actually stand up the sandbox identity. Replace that with a real readiness probe (RFC §6.4) and align the RFC to the shipped behavior. - launcher: add `--readiness-probe`, which creates the real AppContainer identity/token and a kill-on-close Job and launches a throwaway confined child, failing closed if the host cannot create or enforce the boundary. - runtime: back the Windows backend's `isAvailable` with a memoized spawnSync of `--readiness-probe`, so `auto`/`require` fail closed on hosts where the OS cannot create the boundary rather than trusting the packaged binary's presence. - CI: add readiness-probe-smoke.ps1 to the W0 lane (exit 0, repeatable, argument-rejecting). - docs: align RFC §6.4/§6.5 (EN + zh-CN) — mark the readiness probe as implemented and enforced, and keep private desktop, full per-profile policy at readiness, and launcher signature/version as later gates. Advances #2142 Phase 4 (align the RFC's guarantees with the shipped slice). Co-Authored-By: Claude Fable 5 Generated-by: Claude Fable 5 --- .github/workflows/windows-sandbox-w0.yml | 3 + docs/architecture/windows-sandbox-rfc-v1.md | 60 +++++- .../windows-sandbox-rfc-v1.zh-CN.md | 32 ++- .../windows-sandbox/launcher/src/main.rs | 6 + .../launcher/src/windows_launcher.rs | 196 +++++++++++++++++- .../windows-sandbox/readiness-probe-smoke.ps1 | 43 ++++ .../src/sandbox/default-sandbox-manager.ts | 44 +++- 7 files changed, 369 insertions(+), 15 deletions(-) create mode 100644 experiments/windows-sandbox/readiness-probe-smoke.ps1 diff --git a/.github/workflows/windows-sandbox-w0.yml b/.github/workflows/windows-sandbox-w0.yml index 85f2048913..605da92129 100644 --- a/.github/workflows/windows-sandbox-w0.yml +++ b/.github/workflows/windows-sandbox-w0.yml @@ -53,6 +53,9 @@ jobs: - name: Verify AppContainer identity candidate shell: pwsh run: ./experiments/windows-sandbox/appcontainer-smoke.ps1 + - name: Verify production-identity readiness probe + shell: pwsh + run: ./experiments/windows-sandbox/readiness-probe-smoke.ps1 - name: Verify filesystem-worker stdio relay shell: pwsh run: ./experiments/windows-sandbox/stdio-relay-smoke.ps1 diff --git a/docs/architecture/windows-sandbox-rfc-v1.md b/docs/architecture/windows-sandbox-rfc-v1.md index 77dc6a228f..d861424a01 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.md +++ b/docs/architecture/windows-sandbox-rfc-v1.md @@ -1,6 +1,6 @@ # Windows sandbox backend RFC v1 -- Status: implementation baseline selected; product integration under release validation +- Status: implementation baseline selected; first preview slice ([#2961](https://github.com/maka-agent/maka-agent/pull/2961)) merged 2026-08-17; product integration continuing under release validation (preview scope in §6.5) - Tracking: Windows Phase 4 in [issue #2142](https://github.com/maka-agent/maka-agent/issues/2142) - Updated: 2026-08-14 - Owners: `@maka/runtime` sandbox boundary and Runtime Host execution composition @@ -148,7 +148,9 @@ Lexical prefix checks are never authorization evidence. - The Job kills all descendants when its owner closes and does not permit breakaway. - Only declared stdio/protocol handles are inherited through `PROC_THREAD_ATTRIBUTE_HANDLE_LIST`. - Non-interactive workers use a private desktop and cannot read the clipboard, broadcast window - messages, install global hooks, or interact with the user's desktop. + messages, install global hooks, or interact with the user's desktop. _(Later gate: not yet + enforced in the shipped preview slice; the worker rides on AppContainer confinement rather than a + separate desktop — see §6.5.)_ - The token removes privileges and uses restricting SIDs; low integrity is defense in depth, not the filesystem policy by itself. - The child receives an allowlisted environment. Credentials, tokens, proxy variables, shell startup @@ -162,13 +164,65 @@ Lexical prefix checks are never authorization evidence. - Readiness launches a real probe under the production identity, token, Job, desktop, handles, filesystem policy, and offline network policy. OS version checks alone are insufficient. -- Launcher signature, version, and digest are verified against packaged metadata. + _(Implemented: the preview's `--readiness-probe` stands up the real AppContainer identity and + token and a kill-on-close Job and launches a throwaway confined child, failing closed if the host + cannot create or enforce the boundary rather than trusting file presence alone. The private + desktop and the full per-profile filesystem/offline-network policy are not yet exercised at + readiness — see §6.5.)_ +- Launcher signature, version, and digest are verified against packaged metadata. _(Later gate: the + per-launch request digest is recomputed and enforced in-broker today; verifying the launcher + binary's signature and version against packaged metadata is deferred with Phase 3 signing — see + §6.5.)_ - Missing setup, identity drift, ACL-state corruption, ineffective network policy, unsupported filesystem, helper mismatch, or a failed probe returns a stable typed unavailable reason. - `auto` and `require` never fall back to host execution for a restricted managed profile. - Diagnostics expose the backend, setup version, and failure stage without paths, SIDs, credentials, environment values, or firewall details. +### 6.5 Preview implementation status (2026-08-17) + +The first product slice — the packaged Windows 11 x64 AppContainer backend in +[#2961](https://github.com/maka-agent/maka-agent/pull/2961), merged 2026-08-17 — enforces a subset +of the guarantees above. This subsection aligns the documented guarantees with what the preview +actually ships so the RFC does not overclaim. The remaining guarantees are designed but explicitly +deferred as later gates, tracked by Phase 4 in [#2142](https://github.com/maka-agent/maka-agent/issues/2142). + +Enforced in the preview slice: + +- default-deny filesystem with distinct read/write roots compiled from the exact profile (§6.1); +- recursive reparse-point rejection and multi-hard-link rejection before ACL mutation (§5, §6.1); +- a fresh request-derived AppContainer SID, per-launch ACL grants in a versioned recovery ledger, + and stale-ledger reconciliation at startup (§6.1, §7.1); +- an AppContainer token with no network capabilities (§6.2); +- atomic kill-on-close Job membership through `PROC_THREAD_ATTRIBUTE_JOB_LIST` (§6.3); +- inheritance limited to declared stdio/protocol handles through `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` + (§6.3); +- a closed, sorted, allowlisted environment (§6.3); +- a production-identity readiness probe (§6.4): `--readiness-probe` stands up the real AppContainer + identity and token and a kill-on-close Job and launches a throwaway confined child, so + availability fails closed on hosts where the OS cannot create the boundary rather than on the + packaged binary's presence alone; +- fail-closed capability outcomes with no unsandboxed fallback for `auto`/`require` (§6.4). + +Designed but deferred as later gates (not enforced in the preview slice): + +- Private desktop / window-station isolation (§6.3). The worker does not yet run on a separate + desktop; clipboard, window-message, and global-hook isolation currently ride on AppContainer + confinement rather than a dedicated desktop. +- Full-policy readiness coverage (§6.4). The readiness probe already stands up the production + AppContainer identity, token, and kill-on-close Job and launches a confined child, but it does not + yet compile and exercise the exact per-profile filesystem roots, the offline-network policy, or a + private desktop at readiness; those are enforced per launch rather than re-proven at readiness. +- Launcher signature/version verification at readiness (§6.4). The per-launch request digest is + recomputed and enforced in-broker on every launch; verifying the launcher binary's Authenticode + signature and version against packaged metadata is deferred together with Phase 3 signing. + +Deferral narrows readiness richness and desktop-layer defense-in-depth, not the enforcement +boundary: an unavailable, drifted, or failed backend still fails closed, and a restricted managed +profile never falls back to host execution. The lifecycle evidence for cancellation, parent-death, +concurrency, process-drain, and residual ACL/state release tracked by W1 (§9) and Phase 4 (#2142) +remains release evidence, not an assumption. + ## 7. Selected architecture ```mermaid diff --git a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md index 2c4a382c5a..fe1ceffb2c 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md +++ b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md @@ -1,6 +1,6 @@ # Windows 沙箱后端 RFC v1 -- 状态：实现基线已选定；产品接入正在做发布验证 +- 状态：实现基线已选定；首个预览切片（[#2961](https://github.com/maka-agent/maka-agent/pull/2961)）已于 2026-08-17 合并；产品接入继续做发布验证（预览范围见 §6.5） - 跟踪：[Issue #2142](https://github.com/maka-agent/maka-agent/issues/2142) Windows Phase 4 - 更新日期：2026-08-14 - Owner：`@maka/runtime` sandbox boundary 与 Runtime Host execution composition @@ -125,7 +125,7 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 - child 通过 `PROC_THREAD_ATTRIBUTE_JOB_LIST` 在创建时进入 Job，不存在可运行的 pre-assignment window； - Job owner close 时杀死所有 descendant，禁止 breakaway； - 仅通过 `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` 继承声明的 stdio/protocol handle； -- 非交互 worker 使用 private desktop，不能读 clipboard、广播 window message、装 global hook 或操作用户桌面； +- 非交互 worker 使用 private desktop，不能读 clipboard、广播 window message、装 global hook 或操作用户桌面； _(后续门禁：当前预览切片尚未强制；worker 依赖 AppContainer 约束而非独立 desktop —— 见 §6.5。)_ - token 移除 privilege 并使用 restricting SID；low integrity 只是 defense in depth，不是文件策略； - child 只接收 allowlist 环境，不隐式继承 credential、token、proxy、shell startup hook、用户 PATH 或 loader injection variable； @@ -134,13 +134,37 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 ### 6.4 能力与失败 -- readiness 必须在生产 identity/token/Job/desktop/handle/filesystem/offline network 下启动真实 probe； -- launcher signature/version/digest 必须与 package metadata 一致； +- readiness 必须在生产 identity/token/Job/desktop/handle/filesystem/offline network 下启动真实 probe； _(已实现:预览版的 `--readiness-probe` 会真正建立 AppContainer identity/token 与 kill-on-close Job 并启动一个抛弃式受限子进程,宿主无法创建或强制边界时 fail closed,而非仅凭二进制存在即注册;private desktop 与完整的按 profile filesystem/offline network 策略尚未在 readiness 阶段演练 —— 见 §6.5。)_ +- launcher signature/version/digest 必须与 package metadata 一致； _(后续门禁：每次启动的 request digest 目前已在 broker 内重算并强制；对照打包 metadata 校验 launcher 二进制的 signature 与 version 随 Phase 3 签名一并暂缓 —— 见 §6.5。)_ - setup 缺失、identity drift、ACL state 损坏、网络策略无效、文件系统不支持、helper 不匹配、probe 失败都返回 stable typed unavailable reason； - restricted managed profile 在 `auto`/`require` 下绝不 fallback host execution； - diagnostics 只暴露 backend、setup version 与 failure stage，不暴露 path、SID、credential、env 或 firewall detail。 +### 6.5 预览实现状态（2026-08-17） + +首个预览切片 [#2961](https://github.com/maka-agent/maka-agent/pull/2961) 已于 2026-08-17 合并，强制上述保证的一个子集。本节把文档与已交付切片对齐，使 RFC 不 overclaim：§6.3/§6.4 中尚未强制的保证在此显式标为后续门禁。 + +**预览切片已强制：** + +- 默认拒绝文件系统，读/写 grant 分离（§6.1）； +- ACL 修改前拒绝 reparse point 与多硬链接对象（§5/§6.1）； +- 每次启动使用 request-derived 独立 AppContainer SID + 版本化 ledger + startup reconcile（§6.1/§7.1）； +- 不授予网络 capability 的 AppContainer token（§6.2）； +- 创建时原子附加、close 时杀整棵树的 kill-on-close Job（§6.3）； +- 仅通过 `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` 继承声明的 handle（§6.3）； +- 封闭、排序后的 allowlist 环境（§6.3）； +- 生产 identity readiness probe（§6.4）:`--readiness-probe` 真正建立 AppContainer identity/token 与 kill-on-close Job 并启动抛弃式受限子进程,使可用性在宿主无法创建边界时 fail closed,而非仅凭打包二进制存在; +- fail-closed capability check，绝不 unsandboxed fallback（§6.4）。 + +**已设计但作为后续门禁暂缓（预览切片尚未强制）：** + +- private desktop / window station（§6.3）； +- readiness 阶段的完整策略覆盖（§6.4):readiness probe 已建立生产 AppContainer identity/token 与 kill-on-close Job 并启动受限子进程,但尚未在 readiness 阶段编译并演练按 profile 的精确 filesystem 根、offline network 策略与 private desktop —— 这些目前按每次启动强制,而非在 readiness 阶段复证; +- 随 Phase 3 签名一并落地的 launcher signature/version 校验（§6.4）。 + +暂缓收窄的是 readiness 丰富度与 desktop 层的 defense-in-depth，而非强制边界本身：backend 不可用、identity drift 或启动失败仍然 fail closed，受限 managed profile 也绝不回退到宿主执行。 + ## 7. 选定架构 ```mermaid diff --git a/experiments/windows-sandbox/launcher/src/main.rs b/experiments/windows-sandbox/launcher/src/main.rs index 777bac3fd5..45340f88a7 100644 --- a/experiments/windows-sandbox/launcher/src/main.rs +++ b/experiments/windows-sandbox/launcher/src/main.rs @@ -59,6 +59,12 @@ fn run() -> Result { } return windows_launcher::self_probe(); } + if first == "--readiness-probe" { + if args.next().is_some() { + return Err("--readiness-probe does not accept arguments".to_owned()); + } + return windows_launcher::readiness_probe(); + } if first == "--stdio-probe" { let sleep_seconds = match args.next() { None => 0, diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher.rs b/experiments/windows-sandbox/launcher/src/windows_launcher.rs index 55156444b8..2d34a62bb1 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher.rs @@ -34,13 +34,13 @@ use windows_sys::Win32::System::JobObjects::{ QueryInformationJobObject, SetInformationJobObject, TerminateJobObject, }; use windows_sys::Win32::System::Threading::{ - CREATE_SUSPENDED, CREATE_UNICODE_ENVIRONMENT, CreateProcessAsUserW, CreateProcessW, - CreateProcessWithTokenW, DeleteProcThreadAttributeList, EXTENDED_STARTUPINFO_PRESENT, - GetCurrentProcess, GetExitCodeProcess, GetProcessId, InitializeProcThreadAttributeList, - OpenProcessToken, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, PROC_THREAD_ATTRIBUTE_JOB_LIST, - PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES, PROCESS_INFORMATION, ResumeThread, - STARTF_USESTDHANDLES, STARTUPINFOEXW, STARTUPINFOW, TerminateProcess, - UpdateProcThreadAttribute, WaitForSingleObject, + CREATE_NO_WINDOW, CREATE_SUSPENDED, CREATE_UNICODE_ENVIRONMENT, CreateProcessAsUserW, + CreateProcessW, CreateProcessWithTokenW, DeleteProcThreadAttributeList, + EXTENDED_STARTUPINFO_PRESENT, GetCurrentProcess, GetExitCodeProcess, GetProcessId, + InitializeProcThreadAttributeList, OpenProcessToken, PROC_THREAD_ATTRIBUTE_HANDLE_LIST, + PROC_THREAD_ATTRIBUTE_JOB_LIST, PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES, + PROCESS_INFORMATION, ResumeThread, STARTF_USESTDHANDLES, STARTUPINFOEXW, STARTUPINFOW, + TerminateProcess, UpdateProcThreadAttribute, WaitForSingleObject, }; use crate::acl_ledger::{LaunchFailure, with_acl_grants}; @@ -137,6 +137,188 @@ pub fn launch_appcontainer(request: &LaunchRequest) -> Result { } } +/// Bound on the throwaway readiness child. It only runs `cmd.exe /c exit 0`, +/// so anything beyond a couple of seconds means the machine cannot stand up the +/// boundary and the probe must fail closed rather than hang availability. +const READINESS_PROBE_TIMEOUT_MS: u64 = 10_000; + +/// Production-identity readiness probe (RFC §6.4). +/// +/// Availability is not just "the packaged binary exists on disk". This builds +/// the real production identity — a kill-on-close Job plus a per-launch +/// AppContainer profile and SID — and launches a throwaway child +/// (`cmd.exe /c exit 0`) under that AppContainer token inside the Job, proving +/// the OS can actually create and account the sandbox boundary on this machine +/// before any workload is admitted. Anything short of a child that is both +/// AppContainer-confined and Job-accounted, and that exits cleanly, fails +/// closed so a restricted managed profile is never selected on a host that +/// cannot enforce it. +pub fn readiness_probe() -> Result { + let system_root = std::env::var_os("SystemRoot") + .or_else(|| std::env::var_os("windir")) + .ok_or_else(|| "readiness probe: SystemRoot is not set".to_owned())?; + let mut cmd_path = std::path::PathBuf::from(system_root); + cmd_path.push("System32"); + cmd_path.push("cmd.exe"); + let cmd_path = cmd_path.to_string_lossy().into_owned(); + + unsafe { + let job = create_kill_on_close_job()?; + // A per-process identity so concurrent probes never collide on a + // profile name; the profile is deleted when it drops. + let request_id = format!("readiness-probe.{}", std::process::id()); + let profile = match AppContainerProfile::create(&request_id) { + Ok(profile) => profile, + Err(error) => { + CloseHandle(job); + return Err(error); + } + }; + let result = probe_appcontainer_child(&cmd_path, job, profile.sid); + // Closing the last Job handle is the kernel backstop: it terminates the + // throwaway child if the wait above did not already reap it. + CloseHandle(job); + drop(profile); + result + } +} + +/// Launch the throwaway readiness child under the AppContainer token and Job. +/// Mirrors `create_appcontainer_child` but carries no stdio/handle inheritance +/// (nothing is relayed) and inherits the parent environment, since the child +/// only needs to reach `exit 0`. +unsafe fn probe_appcontainer_child( + cmd_path: &str, + job: HANDLE, + app_container_sid: *mut c_void, +) -> Result { + let executable = wide(cmd_path); + // Build the command line directly: cmd.exe's `/c` switch must stay + // unquoted, so the per-token quoting used for arbitrary workloads would + // make cmd treat "/c" as a program to run and exit non-zero. + let mut command = wide(&format!("\"{cmd_path}\" /c exit 0")); + // Run from System32 rather than inheriting the launcher's cwd: the probe + // grants no filesystem roots, so an inherited working directory would be + // default-denied and cmd.exe would fail to initialize. System32 is readable + // by ALL APPLICATION PACKAGES, so the AppContainer child can start there. + let cwd = std::path::Path::new(cmd_path) + .parent() + .map(|parent| wide(&parent.to_string_lossy())); + let cwd_ptr = cwd.as_ref().map_or(null(), |value| value.as_ptr()); + + let mut attribute_size = 0usize; + unsafe { InitializeProcThreadAttributeList(null_mut(), 2, 0, &mut attribute_size) }; + if attribute_size == 0 { + return Err(last_error( + "InitializeProcThreadAttributeList(readiness size)", + )); + } + let words = attribute_size.div_ceil(size_of::()); + let mut attribute_storage = vec![0usize; words]; + let attribute_list = attribute_storage.as_mut_ptr() as *mut c_void; + if unsafe { InitializeProcThreadAttributeList(attribute_list, 2, 0, &mut attribute_size) } == 0 + { + return Err(last_error("InitializeProcThreadAttributeList(readiness)")); + } + + let mut job_value = job; + let mut capabilities = SECURITY_CAPABILITIES { + AppContainerSid: app_container_sid, + Capabilities: null_mut(), + CapabilityCount: 0, + Reserved: 0, + }; + let attributes = (|| -> Result<(), String> { + if unsafe { + UpdateProcThreadAttribute( + attribute_list, + 0, + PROC_THREAD_ATTRIBUTE_JOB_LIST as usize, + &mut job_value as *mut HANDLE as *const c_void, + size_of::(), + null_mut(), + null(), + ) + } == 0 + { + return Err(last_error("UpdateProcThreadAttribute(readiness JOB_LIST)")); + } + if unsafe { + UpdateProcThreadAttribute( + attribute_list, + 0, + PROC_THREAD_ATTRIBUTE_SECURITY_CAPABILITIES as usize, + &mut capabilities as *mut SECURITY_CAPABILITIES as *const c_void, + size_of::(), + null_mut(), + null(), + ) + } == 0 + { + return Err(last_error( + "UpdateProcThreadAttribute(readiness SECURITY_CAPABILITIES)", + )); + } + Ok(()) + })(); + if let Err(error) = attributes { + unsafe { DeleteProcThreadAttributeList(attribute_list) }; + return Err(error); + } + + let mut startup: STARTUPINFOEXW = unsafe { zeroed() }; + startup.StartupInfo.cb = size_of::() as u32; + startup.lpAttributeList = attribute_list; + let mut process: PROCESS_INFORMATION = unsafe { zeroed() }; + let creation_flags = CREATE_SUSPENDED | EXTENDED_STARTUPINFO_PRESENT | CREATE_NO_WINDOW; + let created = unsafe { + CreateProcessW( + executable.as_ptr(), + command.as_mut_ptr(), + null(), + null(), + // No handle list is declared, so no handles are inherited. + 0, + creation_flags, + // Inherit the parent environment: a readiness check needs no + // sanitized block, only enough to reach `exit 0`. + null(), + cwd_ptr, + &startup.StartupInfo, + &mut process, + ) + }; + unsafe { DeleteProcThreadAttributeList(attribute_list) }; + if created == 0 { + return Err(last_error("CreateProcessW(readiness appcontainer)")); + } + + let result = (|| -> Result { + if unsafe { ResumeThread(process.hThread) } == u32::MAX { + return Err(last_error("ResumeThread(readiness)")); + } + let child_app_container = unsafe { child_token_is_appcontainer(process.hProcess) }?; + let child_in_job = unsafe { child_process_is_in_job(process.hProcess) }?; + if !(child_app_container && child_in_job) { + return Err( + "readiness probe child did not establish the AppContainer token and Job boundary" + .to_owned(), + ); + } + let exit = unsafe { wait_for_child(process.hProcess, Some(READINESS_PROBE_TIMEOUT_MS)) }?; + if exit != 0 { + return Err(format!("readiness probe child exited with status {exit}")); + } + Ok(0) + })(); + + unsafe { + CloseHandle(process.hThread); + CloseHandle(process.hProcess); + } + result +} + pub fn appcontainer_sid_string(request_id: &str) -> Result { unsafe { let name = appcontainer_profile_name(request_id); diff --git a/experiments/windows-sandbox/readiness-probe-smoke.ps1 b/experiments/windows-sandbox/readiness-probe-smoke.ps1 new file mode 100644 index 0000000000..63d9d96019 --- /dev/null +++ b/experiments/windows-sandbox/readiness-probe-smoke.ps1 @@ -0,0 +1,43 @@ +param( + [string]$LauncherPath +) + +$ErrorActionPreference = 'Stop' +$PSNativeCommandUseErrorActionPreference = $false + +$launcher = if ($LauncherPath) { + $LauncherPath +} else { + Join-Path $PSScriptRoot 'launcher\target\debug\maka-windows-sandbox.exe' +} +if (-not (Test-Path -LiteralPath $launcher)) { + throw "Missing launcher binary: $launcher" +} + +# The production-identity readiness probe (RFC §6.4) stands up the real +# AppContainer identity, token, and kill-on-close Job and launches a throwaway +# confined child. A clean exit 0 proves the host can actually create and enforce +# the boundary — this is what the runtime's `isAvailable` fails closed on, so it +# must never regress into file-presence-only availability. +$firstOutput = & $launcher --readiness-probe 2>&1 +if ($LASTEXITCODE -ne 0) { + throw "Readiness probe did not succeed: exit=$LASTEXITCODE output=$($firstOutput -join "`n")" +} + +# Availability is memoized per host, but the probe itself must be repeatable: +# a second run proves the confined child and Job teardown leave no residue that +# blocks the next probe. +$secondOutput = & $launcher --readiness-probe 2>&1 +if ($LASTEXITCODE -ne 0) { + throw "Readiness probe was not repeatable: exit=$LASTEXITCODE output=$($secondOutput -join "`n")" +} + +# The probe takes no arguments; passing one must fail closed rather than silently +# ignore unexpected input. +$rejected = & $launcher --readiness-probe unexpected 2>&1 +if ($LASTEXITCODE -eq 0) { + throw "Readiness probe accepted an unexpected argument: $($rejected -join "`n")" +} + +$global:LASTEXITCODE = 0 +Write-Host 'Production-identity readiness probe verified: exit 0, repeatable, argument-rejecting' diff --git a/packages/runtime/src/sandbox/default-sandbox-manager.ts b/packages/runtime/src/sandbox/default-sandbox-manager.ts index a3db49b17a..bf3d1c2e7a 100644 --- a/packages/runtime/src/sandbox/default-sandbox-manager.ts +++ b/packages/runtime/src/sandbox/default-sandbox-manager.ts @@ -1,3 +1,4 @@ +import { spawnSync } from 'node:child_process'; import { existsSync } from 'node:fs'; import { join } from 'node:path'; @@ -24,6 +25,47 @@ function builtinWindowsClientPath( return existsSync(clientPath) ? clientPath : undefined; } +/** + * Upper bound for the launcher's own readiness probe (10s) plus process spawn + * overhead. If the probe cannot answer within this window the host cannot stand + * up the boundary quickly enough, so availability fails closed. + */ +const WINDOWS_READINESS_PROBE_TIMEOUT_MS = 15_000; + +/** + * Production-identity readiness probe for the Windows backend (RFC §6.4). + * + * `existsSync` alone only proves the packaged launcher is present, not that the + * OS can actually create the AppContainer identity, token, and Job on this + * machine (AppContainer can be disabled by policy, the edition may not support + * it, etc.). This runs the launcher's `--readiness-probe`, which stands up the + * real production identity and launches a throwaway confined child, and caches + * the result for the process lifetime — availability is stable per host, and + * the sync `SandboxBackend.isAvailable` contract must stay cheap after the + * first call. Anything other than a clean exit 0 fails closed. + */ +function createWindowsReadinessProbe(clientPath: string): () => boolean { + let cached: boolean | undefined; + return () => { + if (cached !== undefined) return cached; + if (!existsSync(clientPath)) { + cached = false; + return cached; + } + try { + const result = spawnSync(clientPath, ['--readiness-probe'], { + timeout: WINDOWS_READINESS_PROBE_TIMEOUT_MS, + windowsHide: true, + stdio: 'ignore', + }); + cached = result.error === undefined && result.status === 0; + } catch { + cached = false; + } + return cached; + }; +} + function builtinWindowsBackend( platform: SandboxPlatform, resourcesPath: string | undefined, @@ -33,7 +75,7 @@ function builtinWindowsBackend( return new WindowsBrokerSandboxBackend({ clientPath, writeManifest: createWindowsBrokerManifestWriter(), - isAvailable: () => existsSync(clientPath), + isAvailable: createWindowsReadinessProbe(clientPath), }); } From efa032fd2b97811c5d6848c5a5392386637f4c85 Mon Sep 17 00:00:00 2001 From: liugddx Date: Mon, 17 Aug 2026 19:09:16 +0800 Subject: [PATCH 2/9] fix(windows-sandbox): harden readiness probe against PID reuse and identity spoofing Address adversarial review on the production-identity readiness probe: - Make the throwaway AppContainer profile name unique per invocation with a monotonic nonce (SystemTime nanos + PID) so a recycled PID can never collide with a still-registered profile and deadlock the probe (finding A). - Prove enforcement, not mere presence: verify the confined child runs under the SPECIFIC requested Job (IsProcessInJob against our handle) and carries the EXACT requested AppContainer SID (TokenAppContainerSid + EqualSid), failing closed otherwise. "some job / some AppContainer" is not evidence the boundary we asked for is real (finding B). - Cache Windows readiness at module scope keyed by client path so backends sharing a launcher share one probe result; distinct paths stay independent (finding D). - Sync RFC (EN + zh) Updated: date to 2026-08-17 (finding C). Co-Authored-By: Claude Fable 5 Generated-by: Claude Fable 5 --- docs/architecture/windows-sandbox-rfc-v1.md | 2 +- .../windows-sandbox-rfc-v1.zh-CN.md | 2 +- .../launcher/src/windows_launcher.rs | 99 ++++++++++++++++--- .../src/sandbox/default-sandbox-manager.ts | 34 ++++--- 4 files changed, 111 insertions(+), 26 deletions(-) diff --git a/docs/architecture/windows-sandbox-rfc-v1.md b/docs/architecture/windows-sandbox-rfc-v1.md index d861424a01..59ce221d65 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.md +++ b/docs/architecture/windows-sandbox-rfc-v1.md @@ -2,7 +2,7 @@ - Status: implementation baseline selected; first preview slice ([#2961](https://github.com/maka-agent/maka-agent/pull/2961)) merged 2026-08-17; product integration continuing under release validation (preview scope in §6.5) - Tracking: Windows Phase 4 in [issue #2142](https://github.com/maka-agent/maka-agent/issues/2142) -- Updated: 2026-08-14 +- Updated: 2026-08-17 - Owners: `@maka/runtime` sandbox boundary and Runtime Host execution composition - Chinese version: [windows-sandbox-rfc-v1.zh-CN.md](./windows-sandbox-rfc-v1.zh-CN.md) diff --git a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md index fe1ceffb2c..9d6b468052 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md +++ b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md @@ -2,7 +2,7 @@ - 状态：实现基线已选定；首个预览切片（[#2961](https://github.com/maka-agent/maka-agent/pull/2961)）已于 2026-08-17 合并；产品接入继续做发布验证（预览范围见 §6.5） - 跟踪：[Issue #2142](https://github.com/maka-agent/maka-agent/issues/2142) Windows Phase 4 -- 更新日期：2026-08-14 +- 更新日期：2026-08-17 - Owner：`@maka/runtime` sandbox boundary 与 Runtime Host execution composition - 英文版：[windows-sandbox-rfc-v1.md](./windows-sandbox-rfc-v1.md) diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher.rs b/experiments/windows-sandbox/launcher/src/windows_launcher.rs index 2d34a62bb1..8808a995b7 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher.rs @@ -4,7 +4,7 @@ use std::mem::{size_of, zeroed}; use std::os::windows::ffi::OsStrExt; use std::ptr::{null, null_mut}; use std::thread; -use std::time::{Duration, Instant}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; use sha2::{Digest, Sha256}; use windows_sys::Win32::Foundation::{ @@ -16,10 +16,10 @@ use windows_sys::Win32::Security::Isolation::{ CreateAppContainerProfile, DeleteAppContainerProfile, DeriveAppContainerSidFromAppContainerName, }; use windows_sys::Win32::Security::{ - CreateRestrictedToken, DISABLE_MAX_PRIVILEGE, DuplicateTokenEx, FreeSid, GetTokenInformation, - IsTokenRestricted, LUA_TOKEN, SECURITY_ATTRIBUTES, SECURITY_CAPABILITIES, - SecurityImpersonation, TOKEN_ALL_ACCESS, TOKEN_DUPLICATE, TOKEN_QUERY, TOKEN_USER, - TokenIsAppContainer, TokenPrimary, TokenUser, + CreateRestrictedToken, DISABLE_MAX_PRIVILEGE, DuplicateTokenEx, EqualSid, FreeSid, + GetTokenInformation, IsTokenRestricted, LUA_TOKEN, SECURITY_ATTRIBUTES, SECURITY_CAPABILITIES, + SecurityImpersonation, TOKEN_ALL_ACCESS, TOKEN_APPCONTAINER_INFORMATION, TOKEN_DUPLICATE, + TOKEN_QUERY, TOKEN_USER, TokenAppContainerSid, TokenIsAppContainer, TokenPrimary, TokenUser, }; use windows_sys::Win32::Storage::FileSystem::{ CreateFileW, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_EXISTING, @@ -164,9 +164,17 @@ pub fn readiness_probe() -> Result { unsafe { let job = create_kill_on_close_job()?; - // A per-process identity so concurrent probes never collide on a - // profile name; the profile is deleted when it drops. - let request_id = format!("readiness-probe.{}", std::process::id()); + // A per-invocation identity so probes never collide on a profile name: + // the profile is deleted on drop, but if a probe is killed before that + // runs the profile leaks. A PID-only name would then make every later + // probe on a reused PID fail closed on `CreateAppContainerProfile` + // (already exists), so mix in the wall-clock nanos as a nonce; the PID + // stays only to disambiguate concurrent probes. + let nonce = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|elapsed| elapsed.as_nanos()) + .unwrap_or(0); + let request_id = format!("readiness-probe.{}.{nonce}", std::process::id()); let profile = match AppContainerProfile::create(&request_id) { Ok(profile) => profile, Err(error) => { @@ -297,11 +305,16 @@ unsafe fn probe_appcontainer_child( if unsafe { ResumeThread(process.hThread) } == u32::MAX { return Err(last_error("ResumeThread(readiness)")); } - let child_app_container = unsafe { child_token_is_appcontainer(process.hProcess) }?; - let child_in_job = unsafe { child_process_is_in_job(process.hProcess) }?; - if !(child_app_container && child_in_job) { + // Verify the *requested* boundary, not just any confinement: match the + // child's AppContainer SID against the profile we created (a non-null + // match also proves it is an AppContainer token), and confirm membership + // in this probe's Job specifically rather than any ambient Job. + let child_sid_matches = + unsafe { child_token_appcontainer_sid_matches(process.hProcess, app_container_sid) }?; + let child_in_job = unsafe { child_process_is_in_specific_job(process.hProcess, job) }?; + if !(child_sid_matches && child_in_job) { return Err( - "readiness probe child did not establish the AppContainer token and Job boundary" + "readiness probe child did not run under the requested AppContainer SID and Job" .to_owned(), ); } @@ -1073,6 +1086,68 @@ unsafe fn child_process_is_in_job(process: HANDLE) -> Result { Ok(in_job != 0) } +/// Confirm the child belongs to `job` specifically. `IsProcessInJob` with a +/// non-null Job handle answers "is the process in *this* Job", unlike the +/// null-handle form which accepts membership in any ambient Job. +unsafe fn child_process_is_in_specific_job(process: HANDLE, job: HANDLE) -> Result { + let mut in_job = 0; + if unsafe { IsProcessInJob(process, job, &mut in_job) } == 0 { + return Err(last_error("IsProcessInJob(child specific)")); + } + Ok(in_job != 0) +} + +/// Confirm the child's token carries the exact AppContainer SID we created, +/// proving the requested identity was applied rather than some other +/// AppContainer. A non-null matching SID also implies the token is an +/// AppContainer token, so this subsumes the `TokenIsAppContainer` check. +unsafe fn child_token_appcontainer_sid_matches( + process: HANDLE, + expected_sid: *mut c_void, +) -> Result { + let mut token = null_mut(); + if unsafe { OpenProcessToken(process, TOKEN_QUERY, &mut token) } == 0 { + return Err(last_error("OpenProcessToken(appcontainer sid)")); + } + let result = unsafe { token_appcontainer_sid_matches(token, expected_sid) }; + unsafe { CloseHandle(token) }; + result +} + +unsafe fn token_appcontainer_sid_matches( + token: HANDLE, + expected_sid: *mut c_void, +) -> Result { + // First call sizes the buffer; TokenAppContainerSid returns a variable-size + // TOKEN_APPCONTAINER_INFORMATION whose SID data trails the struct. + let mut needed = 0u32; + unsafe { GetTokenInformation(token, TokenAppContainerSid, null_mut(), 0, &mut needed) }; + if needed == 0 { + return Err(last_error("GetTokenInformation(TokenAppContainerSid size)")); + } + let mut buffer = vec![0u8; needed as usize]; + if unsafe { + GetTokenInformation( + token, + TokenAppContainerSid, + buffer.as_mut_ptr() as *mut c_void, + needed, + &mut needed, + ) + } == 0 + { + return Err(last_error("GetTokenInformation(TokenAppContainerSid)")); + } + let info = buffer.as_ptr() as *const TOKEN_APPCONTAINER_INFORMATION; + let sid = unsafe { (*info).TokenAppContainer }; + if sid.is_null() { + // A non-AppContainer token reports a null SID: the boundary was not + // applied, so fail the match rather than erroring. + return Ok(false); + } + Ok(unsafe { EqualSid(sid, expected_sid) } != 0) +} + fn quote_command(executable: &str, arguments: &[String]) -> Vec { let command = iter::once(executable) .chain(arguments.iter().map(String::as_str)) diff --git a/packages/runtime/src/sandbox/default-sandbox-manager.ts b/packages/runtime/src/sandbox/default-sandbox-manager.ts index bf3d1c2e7a..f658a0cf10 100644 --- a/packages/runtime/src/sandbox/default-sandbox-manager.ts +++ b/packages/runtime/src/sandbox/default-sandbox-manager.ts @@ -32,6 +32,14 @@ function builtinWindowsClientPath( */ const WINDOWS_READINESS_PROBE_TIMEOUT_MS = 15_000; +/** + * Availability is a property of the host and the launcher binary, not of any one + * backend instance, so the probe result is cached at module scope keyed by the + * resolved client path. Two backends built for the same launcher share the one + * probe; distinct paths (tests, side-by-side installs) stay independent. + */ +const windowsReadinessCache = new Map(); + /** * Production-identity readiness probe for the Windows backend (RFC §6.4). * @@ -44,26 +52,28 @@ const WINDOWS_READINESS_PROBE_TIMEOUT_MS = 15_000; * the sync `SandboxBackend.isAvailable` contract must stay cheap after the * first call. Anything other than a clean exit 0 fails closed. */ -function createWindowsReadinessProbe(clientPath: string): () => boolean { - let cached: boolean | undefined; - return () => { - if (cached !== undefined) return cached; - if (!existsSync(clientPath)) { - cached = false; - return cached; - } +function probeWindowsReadiness(clientPath: string): boolean { + const cached = windowsReadinessCache.get(clientPath); + if (cached !== undefined) return cached; + let available = false; + if (existsSync(clientPath)) { try { const result = spawnSync(clientPath, ['--readiness-probe'], { timeout: WINDOWS_READINESS_PROBE_TIMEOUT_MS, windowsHide: true, stdio: 'ignore', }); - cached = result.error === undefined && result.status === 0; + available = result.error === undefined && result.status === 0; } catch { - cached = false; + available = false; } - return cached; - }; + } + windowsReadinessCache.set(clientPath, available); + return available; +} + +function createWindowsReadinessProbe(clientPath: string): () => boolean { + return () => probeWindowsReadiness(clientPath); } function builtinWindowsBackend( From 4506446f2a0933fd6d3ab382b6dee8387dc92130 Mon Sep 17 00:00:00 2001 From: liugddx Date: Tue, 18 Aug 2026 14:57:19 +0800 Subject: [PATCH 3/9] fix(windows-sandbox): consolidate availability authority and settle readiness probe deterministically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address maintainer review on the readiness-probe PR: - Availability: make `isBuiltinFilesystemWorkerSandboxAvailable(win32)` consume the memoized readiness probe instead of treating launcher-file presence as a second authority. Runtime Host composition calls this at startup, warming the readiness cache so the backend's later synchronous `isAvailable()` on the transform hot path hits the cache rather than spawning on the event loop. Export `probeWindowsReadiness` with an injectable spawn seam and cover it with unit tests (clean/non-zero/spawn-error/external-timeout/memoized/missing). - Settlement: route the readiness child's post-resume outcomes (verification failure, timeout, clean exit) through explicit TerminateProcess plus the existing `terminate_and_drain_job` primitive, so a child found outside the Job is settled rather than relying on Job close as the only backstop. - Cleanup: give the readiness AppContainer profile one fixed, self-reconciling name that best-effort-deletes any leftover before create, so a profile leaked by an externally-killed probe is reclaimed by the next probe. Replaces the PID+nonce scheme. - Docs: mark the structured unavailable-reason / diagnostics surface as a deferred later gate in both RFC language variants (§6.4/§6.5). Co-Authored-By: Claude Fable 5 Generated-by: Claude Fable 5 --- docs/architecture/windows-sandbox-rfc-v1.md | 11 +- .../windows-sandbox-rfc-v1.zh-CN.md | 8 +- .../launcher/src/windows_launcher.rs | 109 +++++++++++++++--- .../launcher/src/windows_launcher_tests.rs | 17 ++- .../__tests__/default-sandbox-manager.test.ts | 77 +++++++++++++ .../src/sandbox/default-sandbox-manager.ts | 43 +++++-- 6 files changed, 235 insertions(+), 30 deletions(-) diff --git a/docs/architecture/windows-sandbox-rfc-v1.md b/docs/architecture/windows-sandbox-rfc-v1.md index 59ce221d65..d13154584e 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.md +++ b/docs/architecture/windows-sandbox-rfc-v1.md @@ -174,10 +174,14 @@ Lexical prefix checks are never authorization evidence. binary's signature and version against packaged metadata is deferred with Phase 3 signing — see §6.5.)_ - Missing setup, identity drift, ACL-state corruption, ineffective network policy, unsupported - filesystem, helper mismatch, or a failed probe returns a stable typed unavailable reason. + filesystem, helper mismatch, or a failed probe returns a stable typed unavailable reason. _(Later + gate: the readiness probe today collapses every failure to a single fail-closed boolean surfaced as + `backend_not_available`; the structured typed reasons are deferred — see §6.5.)_ - `auto` and `require` never fall back to host execution for a restricted managed profile. - Diagnostics expose the backend, setup version, and failure stage without paths, SIDs, credentials, - environment values, or firewall details. + environment values, or firewall details. _(Later gate: the probe runs with `stdio: 'ignore'` and + retains only the exit result, so setup version and failure stage are not yet propagated — deferred + with the structured unavailable reasons, see §6.5.)_ ### 6.5 Preview implementation status (2026-08-17) @@ -216,6 +220,9 @@ Designed but deferred as later gates (not enforced in the preview slice): - Launcher signature/version verification at readiness (§6.4). The per-launch request digest is recomputed and enforced in-broker on every launch; verifying the launcher binary's Authenticode signature and version against packaged metadata is deferred together with Phase 3 signing. +- Structured unavailable reasons and diagnostics (§6.4). The readiness probe fails closed as a single + boolean surfaced as `backend_not_available`; the stable typed unavailable reasons and the + setup-version/failure-stage diagnostics are designed but not yet implemented or propagated. Deferral narrows readiness richness and desktop-layer defense-in-depth, not the enforcement boundary: an unavailable, drifted, or failed backend still fails closed, and a restricted managed diff --git a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md index 9d6b468052..c2a4220ef6 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md +++ b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md @@ -137,9 +137,10 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 - readiness 必须在生产 identity/token/Job/desktop/handle/filesystem/offline network 下启动真实 probe； _(已实现:预览版的 `--readiness-probe` 会真正建立 AppContainer identity/token 与 kill-on-close Job 并启动一个抛弃式受限子进程,宿主无法创建或强制边界时 fail closed,而非仅凭二进制存在即注册;private desktop 与完整的按 profile filesystem/offline network 策略尚未在 readiness 阶段演练 —— 见 §6.5。)_ - launcher signature/version/digest 必须与 package metadata 一致； _(后续门禁：每次启动的 request digest 目前已在 broker 内重算并强制；对照打包 metadata 校验 launcher 二进制的 signature 与 version 随 Phase 3 签名一并暂缓 —— 见 §6.5。)_ - setup 缺失、identity drift、ACL state 损坏、网络策略无效、文件系统不支持、helper 不匹配、probe 失败都返回 - stable typed unavailable reason； + stable typed unavailable reason； _(后续门禁:readiness probe 目前把每种失败收敛为单一 fail-closed 布尔,统一以 + `backend_not_available` 呈现;结构化 typed reason 尚未实现,暂缓 —— 见 §6.5。)_ - restricted managed profile 在 `auto`/`require` 下绝不 fallback host execution； -- diagnostics 只暴露 backend、setup version 与 failure stage，不暴露 path、SID、credential、env 或 firewall detail。 +- diagnostics 只暴露 backend、setup version 与 failure stage，不暴露 path、SID、credential、env 或 firewall detail。 _(后续门禁:probe 以 `stdio: 'ignore'` 运行且只保留退出结果,setup version 与 failure stage 尚未传播,与结构化 unavailable reason 一并暂缓 —— 见 §6.5。)_ ### 6.5 预览实现状态（2026-08-17） @@ -162,6 +163,9 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 - private desktop / window station（§6.3）； - readiness 阶段的完整策略覆盖（§6.4):readiness probe 已建立生产 AppContainer identity/token 与 kill-on-close Job 并启动受限子进程,但尚未在 readiness 阶段编译并演练按 profile 的精确 filesystem 根、offline network 策略与 private desktop —— 这些目前按每次启动强制,而非在 readiness 阶段复证; - 随 Phase 3 签名一并落地的 launcher signature/version 校验（§6.4）。 +- 结构化 unavailable reason 与 diagnostics（§6.4）:readiness probe 以单一 fail-closed 布尔(呈现为 + `backend_not_available`)收敛所有失败;stable typed unavailable reason 与 setup-version/failure-stage + 诊断已设计但尚未实现或传播。 暂缓收窄的是 readiness 丰富度与 desktop 层的 defense-in-depth，而非强制边界本身：backend 不可用、identity drift 或启动失败仍然 fail closed，受限 managed profile 也绝不回退到宿主执行。 diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher.rs b/experiments/windows-sandbox/launcher/src/windows_launcher.rs index 8808a995b7..75153259a2 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher.rs @@ -4,7 +4,7 @@ use std::mem::{size_of, zeroed}; use std::os::windows::ffi::OsStrExt; use std::ptr::{null, null_mut}; use std::thread; -use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; +use std::time::{Duration, Instant}; use sha2::{Digest, Sha256}; use windows_sys::Win32::Foundation::{ @@ -164,18 +164,17 @@ pub fn readiness_probe() -> Result { unsafe { let job = create_kill_on_close_job()?; - // A per-invocation identity so probes never collide on a profile name: - // the profile is deleted on drop, but if a probe is killed before that - // runs the profile leaks. A PID-only name would then make every later - // probe on a reused PID fail closed on `CreateAppContainerProfile` - // (already exists), so mix in the wall-clock nanos as a nonce; the PID - // stays only to disambiguate concurrent probes. - let nonce = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map(|elapsed| elapsed.as_nanos()) - .unwrap_or(0); - let request_id = format!("readiness-probe.{}.{nonce}", std::process::id()); - let profile = match AppContainerProfile::create(&request_id) { + // Deterministic, retryable profile lifecycle. The readiness child is a + // throwaway `cmd.exe /c exit 0` granted no filesystem roots, so — unlike + // a production launch, which must never reuse a profile lest it inherit + // stale filesystem ACEs — it can use a fixed, self-reconciling name. + // `create_readiness` reclaims any registration leaked by a previously + // killed probe (e.g. the Node-side timeout firing before `Drop` runs) + // before creating a fresh one, so at most one readiness profile ever + // exists and it is always the current probe's. This bounds cleanup + // without a persistent ledger and without relying on process + // destruction to run `Drop`. + let profile = match AppContainerProfile::create_readiness() { Ok(profile) => profile, Err(error) => { CloseHandle(job); @@ -183,8 +182,10 @@ pub fn readiness_probe() -> Result { } }; let result = probe_appcontainer_child(&cmd_path, job, profile.sid); - // Closing the last Job handle is the kernel backstop: it terminates the - // throwaway child if the wait above did not already reap it. + // Closing the last Job handle is the kernel backstop after the probe has + // already explicitly terminated and drained its child (see + // `probe_appcontainer_child`); it terminates any straggler the drain + // did not reap. CloseHandle(job); drop(profile); result @@ -301,7 +302,7 @@ unsafe fn probe_appcontainer_child( return Err(last_error("CreateProcessW(readiness appcontainer)")); } - let result = (|| -> Result { + let verify = (|| -> Result { if unsafe { ResumeThread(process.hThread) } == u32::MAX { return Err(last_error("ResumeThread(readiness)")); } @@ -325,11 +326,40 @@ unsafe fn probe_appcontainer_child( Ok(0) })(); + // Route *every* post-resume outcome — clean exit, verification failure, or + // timeout — through explicit termination and a Job drain before releasing + // the identity. Closing the Job alone is only a kernel backstop, and it + // misses the exact failure the verification guards against: a child found + // *outside* this Job. Terminating the throwaway child directly reaps that + // case, then terminate-and-drain the Job so no descendant outlives the probe. + let settled = unsafe { settle_probe_child(job, process.hProcess) }; + unsafe { CloseHandle(process.hThread); CloseHandle(process.hProcess); } - result + + // A verification/exit failure is the primary diagnosis; a settlement failure + // is itself a fail-closed signal (something still holds the identity), so it + // is surfaced when the probe otherwise succeeded. + match (verify, settled) { + (Err(error), _) => Err(error), + (Ok(_), Err(cleanup)) => Err(cleanup), + (Ok(exit), Ok(())) => Ok(exit), + } +} + +/// Deterministically reap the throwaway readiness child. The child is a +/// disposable `cmd.exe /c exit 0`, so terminate it outright before draining: +/// `TerminateProcess` reaches a child that escaped this Job (the exact case the +/// Job-close backstop cannot), and `terminate_and_drain_job` — the same +/// settlement primitive the production launch path uses — then kills any +/// Job-accounted descendant and confirms the Job is empty. An already-exited +/// child makes `TerminateProcess` fail with `ERROR_ACCESS_DENIED`, which is +/// benign here and intentionally ignored. +unsafe fn settle_probe_child(job: HANDLE, child: HANDLE) -> Result<(), String> { + unsafe { TerminateProcess(child, 124) }; + unsafe { terminate_and_drain_job(job, child) } } pub fn appcontainer_sid_string(request_id: &str) -> Result { @@ -425,8 +455,53 @@ impl AppContainerProfile { } Ok(Self { sid, name }) } + + /// Create the throwaway readiness profile under a fixed, self-reconciling + /// name. The readiness child is granted no filesystem roots and does no + /// filesystem work, so — unlike `create`, which fails closed on a leftover + /// to avoid inheriting stale ACEs — it is safe to reclaim a leaked + /// registration under this stable name first. That makes the profile + /// lifecycle deterministic and retryable: a profile leaked by an + /// externally-killed probe is reclaimed by the next probe rather than + /// accumulating under an unrecoverable unique name. + unsafe fn create_readiness() -> Result { + let name = appcontainer_profile_name(READINESS_PROFILE_REQUEST_ID); + // Best-effort reclaim of a profile leaked by a previously killed probe. + // The name is fixed, so this is the only registration that can exist + // under it; a missing profile makes this a no-op. + unsafe { DeleteAppContainerProfile(name.as_ptr()) }; + let display_name = wide("Maka Windows Sandbox Readiness"); + let description = + wide("Throwaway AppContainer profile for the Maka sandbox readiness probe"); + let mut sid = null_mut(); + let result = unsafe { + CreateAppContainerProfile( + name.as_ptr(), + display_name.as_ptr(), + description.as_ptr(), + null(), + 0, + &mut sid, + ) + }; + if result < 0 { + return Err(format!( + "CreateAppContainerProfile(readiness) failed closed: HRESULT 0x{:08x}", + result as u32 + )); + } + Ok(Self { sid, name }) + } } +/// Fixed request identity for the readiness profile. Unlike production launches +/// this carries no PID or nonce: a leaked profile is deterministically +/// reclaimed by the next probe (see `AppContainerProfile::create_readiness`), +/// which also removes any PID-reuse collision by construction. Readiness is +/// invoked once per host through the memoized TypeScript availability gate, so +/// this fixed name is not exercised concurrently in practice. +pub(crate) const READINESS_PROFILE_REQUEST_ID: &str = "readiness-probe"; + impl Drop for AppContainerProfile { fn drop(&mut self) { unsafe { diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs index 519925ad8f..a96b9dc5b1 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs @@ -2,7 +2,9 @@ mod tests { use std::collections::BTreeMap; - use crate::windows_launcher::{appcontainer_profile_name, environment_block}; + use crate::windows_launcher::{ + READINESS_PROFILE_REQUEST_ID, appcontainer_profile_name, environment_block, + }; fn wide(text: &str) -> Vec { text.encode_utf16().collect() @@ -96,4 +98,17 @@ mod tests { assert!(rendered.starts_with("maka.sandbox.")); assert_eq!(rendered.len(), "maka.sandbox.".len() + 32); } + + #[test] + fn readiness_profile_name_is_fixed_and_self_reconciling() { + // The readiness profile uses one stable name so a profile leaked by an + // externally-killed probe is reclaimed by the next probe rather than + // accumulating under a unique per-invocation name. + let first = appcontainer_profile_name(READINESS_PROFILE_REQUEST_ID); + let second = appcontainer_profile_name(READINESS_PROFILE_REQUEST_ID); + assert_eq!(first, second); + // It must stay distinct from any production request identity so the two + // lifecycles never share a registration. + assert_ne!(first, appcontainer_profile_name("request-one")); + } } diff --git a/packages/runtime/src/__tests__/default-sandbox-manager.test.ts b/packages/runtime/src/__tests__/default-sandbox-manager.test.ts index 5956b204a2..96f3a72bdb 100644 --- a/packages/runtime/src/__tests__/default-sandbox-manager.test.ts +++ b/packages/runtime/src/__tests__/default-sandbox-manager.test.ts @@ -10,8 +10,21 @@ import { createBuiltinSandboxManager, createDefaultSandboxManager, isBuiltinFilesystemWorkerSandboxAvailable, + probeWindowsReadiness, + type WindowsReadinessSpawn, } from '../sandbox/default-sandbox-manager.js'; +async function withFakeLauncher(run: (clientPath: string) => Promise | T): Promise { + const dir = await mkdtemp(join(tmpdir(), 'maka-windows-readiness-')); + const clientPath = join(dir, 'maka-windows-sandbox.exe'); + await writeFile(clientPath, 'test'); + try { + return await run(clientPath); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + describe('createDefaultSandboxManager', () => { it('registers platform backends without requiring the host platform at import time', () => { const manager = createDefaultSandboxManager(); @@ -96,4 +109,68 @@ describe('isBuiltinFilesystemWorkerSandboxAvailable', () => { false, ); }); + + it('reports Windows unavailable when the launcher is absent, so no worker is published', () => { + // execution-composition gates filesystem-worker wiring on this helper; with + // no packaged launcher there is nothing to probe and it must fail closed. + assert.equal( + isBuiltinFilesystemWorkerSandboxAvailable('win32', undefined, 'x64', undefined), + false, + ); + }); +}); + +describe('probeWindowsReadiness', () => { + it('is available only on a clean exit 0 from the launcher probe', async () => { + await withFakeLauncher((clientPath) => { + const spawn: WindowsReadinessSpawn = () => ({ status: 0 }); + assert.equal(probeWindowsReadiness(clientPath, spawn), true); + }); + }); + + it('fails closed when the probe exits non-zero', async () => { + await withFakeLauncher((clientPath) => { + const spawn: WindowsReadinessSpawn = () => ({ status: 1 }); + assert.equal(probeWindowsReadiness(clientPath, spawn), false); + }); + }); + + it('fails closed on a spawn error', async () => { + await withFakeLauncher((clientPath) => { + const spawn: WindowsReadinessSpawn = () => ({ status: null, error: new Error('ENOENT') }); + assert.equal(probeWindowsReadiness(clientPath, spawn), false); + }); + }); + + it('fails closed on an external timeout that leaves no exit status', async () => { + await withFakeLauncher((clientPath) => { + // spawnSync surfaces a `timeout` kill as status === null. + const spawn: WindowsReadinessSpawn = () => ({ status: null }); + assert.equal(probeWindowsReadiness(clientPath, spawn), false); + }); + }); + + it('reuses the memoized result so repeated checks never re-probe', async () => { + await withFakeLauncher((clientPath) => { + let calls = 0; + const spawn: WindowsReadinessSpawn = () => { + calls += 1; + return { status: 0 }; + }; + assert.equal(probeWindowsReadiness(clientPath, spawn), true); + assert.equal(probeWindowsReadiness(clientPath, spawn), true); + assert.equal(calls, 1); + }); + }); + + it('fails closed when the launcher file does not exist without spawning', () => { + let calls = 0; + const spawn: WindowsReadinessSpawn = () => { + calls += 1; + return { status: 0 }; + }; + const missing = join(tmpdir(), 'maka-windows-readiness-missing', 'maka-windows-sandbox.exe'); + assert.equal(probeWindowsReadiness(missing, spawn), false); + assert.equal(calls, 0); + }); }); diff --git a/packages/runtime/src/sandbox/default-sandbox-manager.ts b/packages/runtime/src/sandbox/default-sandbox-manager.ts index f658a0cf10..29b77b96d8 100644 --- a/packages/runtime/src/sandbox/default-sandbox-manager.ts +++ b/packages/runtime/src/sandbox/default-sandbox-manager.ts @@ -40,6 +40,24 @@ const WINDOWS_READINESS_PROBE_TIMEOUT_MS = 15_000; */ const windowsReadinessCache = new Map(); +/** Outcome of one launcher `--readiness-probe` invocation. */ +export interface WindowsReadinessSpawnResult { + readonly status: number | null; + readonly error?: Error; +} + +/** Runs one readiness probe. Injectable so tests can drive it deterministically. */ +export type WindowsReadinessSpawn = (clientPath: string) => WindowsReadinessSpawnResult; + +function spawnReadinessProbe(clientPath: string): WindowsReadinessSpawnResult { + const result = spawnSync(clientPath, ['--readiness-probe'], { + timeout: WINDOWS_READINESS_PROBE_TIMEOUT_MS, + windowsHide: true, + stdio: 'ignore', + }); + return { status: result.status, error: result.error }; +} + /** * Production-identity readiness probe for the Windows backend (RFC §6.4). * @@ -50,19 +68,19 @@ const windowsReadinessCache = new Map(); * real production identity and launches a throwaway confined child, and caches * the result for the process lifetime — availability is stable per host, and * the sync `SandboxBackend.isAvailable` contract must stay cheap after the - * first call. Anything other than a clean exit 0 fails closed. + * first call. Anything other than a clean exit 0 fails closed (a non-zero + * status, a spawn error, or an external timeout that leaves `status === null`). */ -function probeWindowsReadiness(clientPath: string): boolean { +export function probeWindowsReadiness( + clientPath: string, + spawn: WindowsReadinessSpawn = spawnReadinessProbe, +): boolean { const cached = windowsReadinessCache.get(clientPath); if (cached !== undefined) return cached; let available = false; if (existsSync(clientPath)) { try { - const result = spawnSync(clientPath, ['--readiness-probe'], { - timeout: WINDOWS_READINESS_PROBE_TIMEOUT_MS, - windowsHide: true, - stdio: 'ignore', - }); + const result = spawn(clientPath); available = result.error === undefined && result.status === 0; } catch { available = false; @@ -112,7 +130,16 @@ export function isBuiltinFilesystemWorkerSandboxAvailable( resourcesPath: string | undefined = (process as ElectronProcess).resourcesPath, ): boolean { if (platform === 'darwin') return true; - if (platform === 'win32') return builtinWindowsClientPath(platform, resourcesPath) !== undefined; + if (platform === 'win32') { + // Single source of truth for Windows availability: file presence only + // discovers the launcher path; the memoized readiness probe decides + // availability. Runtime Host composition calls this at startup, which warms + // `windowsReadinessCache`, so the backend's later synchronous `isAvailable` + // on the operation hot path hits the cache instead of spawning. File + // presence is no longer a second availability authority. + const clientPath = builtinWindowsClientPath(platform, resourcesPath); + return clientPath !== undefined && probeWindowsReadiness(clientPath); + } return ( platform === 'linux' && linuxCapability !== undefined && From c8b6527d22603c07aa9be4487ed8cc230bcb5132 Mon Sep 17 00:00:00 2001 From: liugddx Date: Tue, 18 Aug 2026 16:38:01 +0800 Subject: [PATCH 4/9] fix(windows-sandbox): serialize and bound the readiness probe lifecycle Address four readiness-probe lifecycle blockers on the availability path: 1. Reserve the `readiness-probe` request_id in protocol and reject it in LaunchRequest::validate; derive the probe profile under a disjoint `maka.readiness.` namespace so no production launch can resolve to the profile the probe deletes and recreates. 2. Serialize the whole delete->create->probe->settle->drop window across processes with the DACL-hardened named mutex the ACL ledger uses (LedgerLock), scoped by SID under Global\Maka.WindowsSandbox.ReadinessProfile.v1, failing closed on timeout. 3. Preserve the AppContainer registration on drop when the Job could not be proven empty (keep_on_drop + preserve()), mirroring the production Unsettled contract, instead of deleting a possibly-live identity. 4. Cache negative Windows readiness for a bounded TTL (60s) rather than the process lifetime, and make the synchronous isAvailable hot path strictly cache-only (readCachedWindowsReadiness) so it never spawns on the event loop; positive results stay cached permanently. Co-Authored-By: Claude Fable 5 Generated-by: Claude Fable 5 --- docs/architecture/windows-sandbox-rfc-v1.md | 23 +++- .../windows-sandbox-rfc-v1.zh-CN.md | 8 +- .../launcher/src/acl_ledger.rs | 23 +++- .../windows-sandbox/launcher/src/protocol.rs | 15 +++ .../launcher/src/windows_launcher.rs | 118 +++++++++++++++--- .../launcher/src/windows_launcher_tests.rs | 79 +++++++++++- .../__tests__/default-sandbox-manager.test.ts | 96 ++++++++++++++ .../src/sandbox/default-sandbox-manager.ts | 74 ++++++++--- 8 files changed, 392 insertions(+), 44 deletions(-) diff --git a/docs/architecture/windows-sandbox-rfc-v1.md b/docs/architecture/windows-sandbox-rfc-v1.md index d13154584e..343bd9afbf 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.md +++ b/docs/architecture/windows-sandbox-rfc-v1.md @@ -2,7 +2,7 @@ - Status: implementation baseline selected; first preview slice ([#2961](https://github.com/maka-agent/maka-agent/pull/2961)) merged 2026-08-17; product integration continuing under release validation (preview scope in §6.5) - Tracking: Windows Phase 4 in [issue #2142](https://github.com/maka-agent/maka-agent/issues/2142) -- Updated: 2026-08-17 +- Updated: 2026-08-18 - Owners: `@maka/runtime` sandbox boundary and Runtime Host execution composition - Chinese version: [windows-sandbox-rfc-v1.zh-CN.md](./windows-sandbox-rfc-v1.zh-CN.md) @@ -169,6 +169,17 @@ Lexical prefix checks are never authorization evidence. cannot create or enforce the boundary rather than trusting file presence alone. The private desktop and the full per-profile filesystem/offline-network policy are not yet exercised at readiness — see §6.5.)_ +- The readiness probe's throwaway profile lifecycle is isolated and fail-closed. _(Implemented: the + probe profile lives under a dedicated `maka.readiness.` namespace that is structurally disjoint + from the production `maka.sandbox.` namespace, and its reserved `requestId` is rejected by launch + validation, so no production launch can ever resolve to the profile the probe deletes and + recreates. The whole delete→create→probe→settle→drop cycle is serialized across processes by a + DACL-hardened per-user named mutex — the same primitive the ACL ledger uses — so concurrent probes + cannot delete each other's live registration. When the probe cannot prove its Job empty it + preserves the registration for a later lease-holder to reclaim rather than deleting an identity a + descendant may still hold. On the consumer side a negative availability result is cached only for a + bounded TTL, so a single transient failure re-probes on the next composition rather than disabling + the sandbox until the Runtime Host restarts; a positive result is cached for the process lifetime.)_ - Launcher signature, version, and digest are verified against packaged metadata. _(Later gate: the per-launch request digest is recomputed and enforced in-broker today; verifying the launcher binary's signature and version against packaged metadata is deferred with Phase 3 signing — see @@ -206,6 +217,11 @@ Enforced in the preview slice: identity and token and a kill-on-close Job and launches a throwaway confined child, so availability fails closed on hosts where the OS cannot create the boundary rather than on the packaged binary's presence alone; +- a dedicated, cross-process-serialized readiness profile lifecycle (§6.4): the probe profile lives + in a namespace disjoint from production, its reserved `requestId` is rejected by validation, a + DACL-hardened per-user named mutex serializes its delete→create→probe→drop cycle, an unsettled + probe preserves rather than deletes its registration, and negative availability is cached with a + bounded TTL so one transient failure does not disable the sandbox until restart; - fail-closed capability outcomes with no unsandboxed fallback for `auto`/`require` (§6.4). Designed but deferred as later gates (not enforced in the preview slice): @@ -223,6 +239,11 @@ Designed but deferred as later gates (not enforced in the preview slice): - Structured unavailable reasons and diagnostics (§6.4). The readiness probe fails closed as a single boolean surfaced as `backend_not_available`; the stable typed unavailable reasons and the setup-version/failure-stage diagnostics are designed but not yet implemented or propagated. +- Concurrent real-machine readiness race coverage (§6.4). The readiness profile lifecycle is + serialized by a named mutex and covered by unit tests over the mutex-name, namespace, and + validation primitives; a multi-process race test that spawns real concurrent probes on a live + Windows host is deferred as disproportionate for a throwaway diagnostic probe and inherently flaky + in CI. The serialization primitive itself, not an end-to-end race harness, is the enforced contract. Deferral narrows readiness richness and desktop-layer defense-in-depth, not the enforcement boundary: an unavailable, drifted, or failed backend still fails closed, and a restricted managed diff --git a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md index c2a4220ef6..1ec9831142 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md +++ b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md @@ -2,7 +2,7 @@ - 状态：实现基线已选定；首个预览切片（[#2961](https://github.com/maka-agent/maka-agent/pull/2961)）已于 2026-08-17 合并；产品接入继续做发布验证（预览范围见 §6.5） - 跟踪：[Issue #2142](https://github.com/maka-agent/maka-agent/issues/2142) Windows Phase 4 -- 更新日期：2026-08-17 +- 更新日期：2026-08-18 - Owner：`@maka/runtime` sandbox boundary 与 Runtime Host execution composition - 英文版：[windows-sandbox-rfc-v1.md](./windows-sandbox-rfc-v1.md) @@ -135,6 +135,7 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 ### 6.4 能力与失败 - readiness 必须在生产 identity/token/Job/desktop/handle/filesystem/offline network 下启动真实 probe； _(已实现:预览版的 `--readiness-probe` 会真正建立 AppContainer identity/token 与 kill-on-close Job 并启动一个抛弃式受限子进程,宿主无法创建或强制边界时 fail closed,而非仅凭二进制存在即注册;private desktop 与完整的按 profile filesystem/offline network 策略尚未在 readiness 阶段演练 —— 见 §6.5。)_ +- readiness probe 的抛弃式 profile 生命周期必须隔离且 fail closed； _(已实现:probe profile 位于专属 `maka.readiness.` 命名空间,与生产 `maka.sandbox.` 命名空间结构性不相交,其保留的 `requestId` 被 launch validation 拒绝,任何生产启动都无法解析到 probe 删除并重建的那个 profile;整个 delete→create→probe→settle→drop 生命周期由一个 DACL 加固的按用户命名互斥量跨进程串行——与 ACL ledger 复用同一原语——使并发 probe 不会互删对方的 active 注册;当 probe 无法证明其 Job 清空时,保留该注册交由后续持锁者 reclaim,而非删除一个 descendant 可能仍持有的 identity;消费侧对负可用性结果只按有界 TTL 缓存,一次瞬时失败会在下次 composition 重探,而非把沙箱禁用到 Runtime Host 重启,正结果则按进程生命周期缓存。)_ - launcher signature/version/digest 必须与 package metadata 一致； _(后续门禁：每次启动的 request digest 目前已在 broker 内重算并强制；对照打包 metadata 校验 launcher 二进制的 signature 与 version 随 Phase 3 签名一并暂缓 —— 见 §6.5。)_ - setup 缺失、identity drift、ACL state 损坏、网络策略无效、文件系统不支持、helper 不匹配、probe 失败都返回 stable typed unavailable reason； _(后续门禁:readiness probe 目前把每种失败收敛为单一 fail-closed 布尔,统一以 @@ -156,6 +157,7 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 - 仅通过 `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` 继承声明的 handle（§6.3）； - 封闭、排序后的 allowlist 环境（§6.3）； - 生产 identity readiness probe（§6.4）:`--readiness-probe` 真正建立 AppContainer identity/token 与 kill-on-close Job 并启动抛弃式受限子进程,使可用性在宿主无法创建边界时 fail closed,而非仅凭打包二进制存在; +- 专属且跨进程串行的 readiness profile 生命周期（§6.4）:probe profile 位于与生产不相交的命名空间,其保留 `requestId` 被 validation 拒绝,一个 DACL 加固的按用户命名互斥量串行其 delete→create→probe→drop 生命周期,未证清空的 probe 保留而非删除其注册,负可用性按有界 TTL 缓存,使一次瞬时失败不会把沙箱禁用到重启; - fail-closed capability check，绝不 unsandboxed fallback（§6.4）。 **已设计但作为后续门禁暂缓（预览切片尚未强制）：** @@ -166,6 +168,10 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 - 结构化 unavailable reason 与 diagnostics（§6.4）:readiness probe 以单一 fail-closed 布尔(呈现为 `backend_not_available`)收敛所有失败;stable typed unavailable reason 与 setup-version/failure-stage 诊断已设计但尚未实现或传播。 +- readiness 的跨进程并发真机竞态覆盖（§6.4）:readiness profile 生命周期已由命名互斥量串行,并有针对 + 互斥量名、命名空间与 validation 原语的单元测试;在真实 Windows 宿主上 spawn 多个并发 probe 的多进程 + 竞态测试暂缓——对一个抛弃式诊断探针不成比例且在 CI 中天然 flaky。被强制的契约是串行化原语本身,而非 + 端到端竞态 harness。 暂缓收窄的是 readiness 丰富度与 desktop 层的 defense-in-depth，而非强制边界本身：backend 不可用、identity drift 或启动失败仍然 fail closed，受限 managed profile 也绝不回退到宿主执行。 diff --git a/experiments/windows-sandbox/launcher/src/acl_ledger.rs b/experiments/windows-sandbox/launcher/src/acl_ledger.rs index 463c611374..a78c8d0a38 100644 --- a/experiments/windows-sandbox/launcher/src/acl_ledger.rs +++ b/experiments/windows-sandbox/launcher/src/acl_ledger.rs @@ -44,6 +44,23 @@ pub(crate) fn acl_mutex_name(user_sid: &str) -> String { format!(r"Global\Maka.WindowsSandbox.AclLedger.v2.{user_sid}") } +/// Serializes the readiness probe's AppContainer profile lifecycle. The probe +/// profile (`maka.readiness.`) is a per-user, machine-wide registration +/// just like the ledger's objects, so two concurrent probes in different +/// sessions would otherwise delete→create→drop each other's live profile. The +/// lease is scoped by SID and carries the same SYSTEM+owner-only DACL as the +/// ledger mutex (see [`acl_mutex_name`]), so it is `Global\` for the identical +/// cross-session reason and fails closed against a squatted name. +pub(crate) fn readiness_mutex_name(user_sid: &str) -> String { + format!(r"Global\Maka.WindowsSandbox.ReadinessProfile.v1.{user_sid}") +} + +/// Timeout for acquiring the readiness profile lease. The readiness probe is a +/// short throwaway `cmd.exe /c exit 0`, so a same-user contender releases the +/// lease well within this bound; exceeding it means a stuck holder and the +/// probe fails closed rather than racing the profile lifecycle unlocked. +pub(crate) const READINESS_MUTEX_TIMEOUT_MS: u32 = 30_000; + /// Distinguishes launch failures by whether the Job was proven empty. /// Cleanup semantics differ: a settled failure may release grants and /// ledger normally, while an unsettled one must preserve its recovery @@ -195,15 +212,17 @@ pub fn with_acl_grants( } } -struct LedgerLock { +pub(crate) struct LedgerLock { handle: HANDLE, } impl LedgerLock { - fn acquire(name: &str, user_sid: &str, timeout_ms: u32) -> Result { + pub(crate) fn acquire(name: &str, user_sid: &str, timeout_ms: u32) -> Result { Self::try_acquire(name, user_sid, timeout_ms)?.ok_or_else(|| { if name.contains(".AclLease.") { "acquire ACL ledger lease timed out".to_owned() + } else if name.contains(".ReadinessProfile.") { + "acquire readiness profile lease timed out".to_owned() } else { "acquire ACL ledger mutex timed out".to_owned() } diff --git a/experiments/windows-sandbox/launcher/src/protocol.rs b/experiments/windows-sandbox/launcher/src/protocol.rs index 419812ca26..a1b4b7d21e 100644 --- a/experiments/windows-sandbox/launcher/src/protocol.rs +++ b/experiments/windows-sandbox/launcher/src/protocol.rs @@ -31,6 +31,16 @@ pub const MIN_LAUNCH_TIMEOUT_MS: u64 = 1_000; pub const MAX_LAUNCH_TIMEOUT_MS: u64 = 600_000; pub const DEFAULT_LAUNCH_TIMEOUT_MS: u64 = 30_000; +/// `request_id` reserved for the internal Windows readiness probe. Its own +/// AppContainer profile/SID is derived from this exact identity, and +/// `create_readiness` best-effort *deletes* that profile before recreating it. +/// A production launch is therefore forbidden from carrying it: otherwise an +/// ordinary launch would resolve to the same profile the probe reclaims, +/// letting the two identities collide. `validate` rejects it so the readiness +/// namespace is one production requests can never enter — enforced, not merely +/// documented. +pub const RESERVED_READINESS_REQUEST_ID: &str = "readiness-probe"; + #[derive(Debug, Clone, Deserialize, serde::Serialize)] #[serde(deny_unknown_fields)] #[serde(rename_all = "camelCase")] @@ -91,6 +101,11 @@ impl LaunchRequest { { return Err("requestId must use 1-128 safe ASCII characters".to_owned()); } + if self.request_id == RESERVED_READINESS_REQUEST_ID { + return Err(format!( + "requestId '{RESERVED_READINESS_REQUEST_ID}' is reserved for the internal readiness probe" + )); + } validate_path(&self.executable, "executable")?; validate_path(&self.cwd, "cwd")?; validate_roots(&self.read_roots, "readRoots")?; diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher.rs b/experiments/windows-sandbox/launcher/src/windows_launcher.rs index 75153259a2..c0df97a6f9 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher.rs @@ -43,8 +43,12 @@ use windows_sys::Win32::System::Threading::{ TerminateProcess, UpdateProcThreadAttribute, WaitForSingleObject, }; -use crate::acl_ledger::{LaunchFailure, with_acl_grants}; -use crate::protocol::{DEFAULT_LAUNCH_TIMEOUT_MS, LaunchRequest, NetworkMode}; +use crate::acl_ledger::{ + LaunchFailure, LedgerLock, READINESS_MUTEX_TIMEOUT_MS, readiness_mutex_name, with_acl_grants, +}; +use crate::protocol::{ + DEFAULT_LAUNCH_TIMEOUT_MS, LaunchRequest, NetworkMode, RESERVED_READINESS_REQUEST_ID, +}; pub fn self_probe() -> Result { unsafe { @@ -162,6 +166,22 @@ pub fn readiness_probe() -> Result { cmd_path.push("cmd.exe"); let cmd_path = cmd_path.to_string_lossy().into_owned(); + // Serialize the whole readiness profile lifecycle across processes. The + // profile name is fixed and the registration is per-user machine-wide, so + // two concurrent probes — side-by-side installs, or a direct + // `--readiness-probe` racing the memoized availability gate — would + // otherwise delete→create→drop each other's live profile. The lease is the + // same DACL-hardened named mutex the ACL ledger uses, scoped to a distinct + // readiness name, held (RAII) across the entire + // delete→create→probe→settle→drop window and failing closed on timeout so + // the lifecycle never runs unlocked. + let user_sid = current_user_sid_string()?; + let _lease = LedgerLock::acquire( + &readiness_mutex_name(&user_sid), + &user_sid, + READINESS_MUTEX_TIMEOUT_MS, + )?; + unsafe { let job = create_kill_on_close_job()?; // Deterministic, retryable profile lifecycle. The readiness child is a @@ -173,20 +193,30 @@ pub fn readiness_probe() -> Result { // before creating a fresh one, so at most one readiness profile ever // exists and it is always the current probe's. This bounds cleanup // without a persistent ledger and without relying on process - // destruction to run `Drop`. - let profile = match AppContainerProfile::create_readiness() { + // destruction to run `Drop`. The lease above makes this reclaim safe: + // no other process can hold the profile while we delete and recreate it. + let mut profile = match AppContainerProfile::create_readiness() { Ok(profile) => profile, Err(error) => { CloseHandle(job); return Err(error); } }; - let result = probe_appcontainer_child(&cmd_path, job, profile.sid); + let mut unsettled = false; + let result = probe_appcontainer_child(&cmd_path, job, profile.sid, &mut unsettled); // Closing the last Job handle is the kernel backstop after the probe has // already explicitly terminated and drained its child (see // `probe_appcontainer_child`); it terminates any straggler the drain // did not reap. CloseHandle(job); + if unsettled { + // The Job was not proven empty, so a descendant may still hold this + // AppContainer identity. Preserve the registration rather than + // deleting a profile that could still be in use; the next + // lease-holding `create_readiness` best-effort reclaims it — safe + // because the lease serializes that reclaim against any live holder. + profile.preserve(); + } drop(profile); result } @@ -200,6 +230,7 @@ unsafe fn probe_appcontainer_child( cmd_path: &str, job: HANDLE, app_container_sid: *mut c_void, + unsettled: &mut bool, ) -> Result { let executable = wide(cmd_path); // Build the command line directly: cmd.exe's `/c` switch must stay @@ -339,6 +370,14 @@ unsafe fn probe_appcontainer_child( CloseHandle(process.hProcess); } + // Report whether the identity is proven drained. A settlement failure means + // the Job was not confirmed empty, so a descendant may still hold this + // AppContainer profile; the caller must preserve the registration instead of + // deleting one that could still be in use. This is independent of `verify`: + // a verification failure whose Job *did* drain cleanly leaves nothing behind + // and the profile is safe to delete. + *unsettled = settled.is_err(); + // A verification/exit failure is the primary diagnosis; a settlement failure // is itself a fail-closed signal (something still holds the identity), so it // is surfaced when the probe otherwise succeeded. @@ -426,9 +465,22 @@ unsafe fn sid_string(sid: *mut c_void) -> Result { struct AppContainerProfile { sid: *mut c_void, name: Vec, + /// When set, `Drop` skips `DeleteAppContainerProfile`. The readiness probe + /// sets it when its Job could not be proven empty: a descendant may still + /// hold this identity, so the registration is preserved for a later + /// lease-holding `create_readiness` to reclaim rather than deleted while + /// possibly in use. Production launches never set it (each has a unique + /// identity that no live launch reuses). + keep_on_drop: bool, } impl AppContainerProfile { + /// Preserve this profile's registration on drop instead of deleting it. + /// Called only when the readiness probe could not prove its Job empty. + fn preserve(&mut self) { + self.keep_on_drop = true; + } + unsafe fn create(request_id: &str) -> Result { let name = appcontainer_profile_name(request_id); let display_name = wide("Maka Windows Sandbox"); @@ -453,7 +505,11 @@ impl AppContainerProfile { result as u32 )); } - Ok(Self { sid, name }) + Ok(Self { + sid, + name, + keep_on_drop: false, + }) } /// Create the throwaway readiness profile under a fixed, self-reconciling @@ -465,10 +521,11 @@ impl AppContainerProfile { /// externally-killed probe is reclaimed by the next probe rather than /// accumulating under an unrecoverable unique name. unsafe fn create_readiness() -> Result { - let name = appcontainer_profile_name(READINESS_PROFILE_REQUEST_ID); + let name = appcontainer_readiness_profile_name(); // Best-effort reclaim of a profile leaked by a previously killed probe. // The name is fixed, so this is the only registration that can exist - // under it; a missing profile makes this a no-op. + // under it; a missing profile makes this a no-op. The caller holds the + // readiness lease, so no concurrent probe can be mid-lifecycle here. unsafe { DeleteAppContainerProfile(name.as_ptr()) }; let display_name = wide("Maka Windows Sandbox Readiness"); let description = @@ -490,27 +547,38 @@ impl AppContainerProfile { result as u32 )); } - Ok(Self { sid, name }) + Ok(Self { + sid, + name, + keep_on_drop: false, + }) } } /// Fixed request identity for the readiness profile. Unlike production launches /// this carries no PID or nonce: a leaked profile is deterministically /// reclaimed by the next probe (see `AppContainerProfile::create_readiness`), -/// which also removes any PID-reuse collision by construction. Readiness is -/// invoked once per host through the memoized TypeScript availability gate, so -/// this fixed name is not exercised concurrently in practice. -pub(crate) const READINESS_PROFILE_REQUEST_ID: &str = "readiness-probe"; +/// which also removes any PID-reuse collision by construction, and the readiness +/// lease serializes that reclaim across processes. This is the single source +/// shared with `protocol::RESERVED_READINESS_REQUEST_ID`, which +/// `LaunchRequest::validate` rejects so no production launch can ever carry the +/// readiness identity. +pub(crate) const READINESS_PROFILE_REQUEST_ID: &str = RESERVED_READINESS_REQUEST_ID; impl Drop for AppContainerProfile { fn drop(&mut self) { unsafe { FreeSid(self.sid); - // The request-derived name is never reused by another live launch. - // Best-effort deletion keeps the user profile store bounded; a - // crash can leave this registration behind, but a future request - // has a different SID and cannot inherit its ACL authority. - DeleteAppContainerProfile(self.name.as_ptr()); + if !self.keep_on_drop { + // The request-derived name is never reused by another live + // launch. Best-effort deletion keeps the user profile store + // bounded; a crash can leave this registration behind, but a + // future request has a different SID and cannot inherit its ACL + // authority. Skipped when `keep_on_drop` is set: the readiness + // probe could not prove its Job empty, so the identity may still + // be held and must be preserved for a later reclaim. + DeleteAppContainerProfile(self.name.as_ptr()); + } } } } @@ -520,6 +588,20 @@ pub(crate) fn appcontainer_profile_name(request_id: &str) -> Vec { wide(&format!("maka.sandbox.{}", &digest[..32])) } +/// The readiness probe's AppContainer profile name. It lives under a distinct +/// `maka.readiness.` prefix, structurally disjoint from the production +/// `maka.sandbox.` namespace [`appcontainer_profile_name`] derives, so even if +/// `validate` were somehow bypassed no production launch could resolve to the +/// profile the probe deletes and recreates — the two namespaces can never +/// collide by construction. +pub(crate) fn appcontainer_readiness_profile_name() -> Vec { + let digest = format!( + "{:x}", + Sha256::digest(READINESS_PROFILE_REQUEST_ID.as_bytes()) + ); + wide(&format!("maka.readiness.{}", &digest[..32])) +} + fn validate_appcontainer_policy(request: &LaunchRequest) -> Result<(), String> { if !matches!(request.network, NetworkMode::Restricted) { return Err("AppContainer backend only implements restricted networking".to_owned()); diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs index a96b9dc5b1..59377031d5 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs @@ -2,10 +2,29 @@ mod tests { use std::collections::BTreeMap; + use crate::acl_ledger::readiness_mutex_name; + use crate::protocol::{LaunchRequest, NetworkMode, RESERVED_READINESS_REQUEST_ID}; use crate::windows_launcher::{ - READINESS_PROFILE_REQUEST_ID, appcontainer_profile_name, environment_block, + appcontainer_profile_name, appcontainer_readiness_profile_name, environment_block, }; + fn valid_launch_request(request_id: &str) -> LaunchRequest { + LaunchRequest { + version: 1, + request_id: request_id.to_owned(), + executable: "C:\\Windows\\System32\\cmd.exe".to_owned(), + arguments: Vec::new(), + cwd: "C:\\Windows\\System32".to_owned(), + read_roots: Vec::new(), + write_roots: Vec::new(), + exact_read_roots: Vec::new(), + exact_write_roots: Vec::new(), + network: NetworkMode::Restricted, + environment: BTreeMap::new(), + timeout_ms: None, + } + } + fn wide(text: &str) -> Vec { text.encode_utf16().collect() } @@ -104,11 +123,59 @@ mod tests { // The readiness profile uses one stable name so a profile leaked by an // externally-killed probe is reclaimed by the next probe rather than // accumulating under a unique per-invocation name. - let first = appcontainer_profile_name(READINESS_PROFILE_REQUEST_ID); - let second = appcontainer_profile_name(READINESS_PROFILE_REQUEST_ID); + let first = appcontainer_readiness_profile_name(); + let second = appcontainer_readiness_profile_name(); assert_eq!(first, second); - // It must stay distinct from any production request identity so the two - // lifecycles never share a registration. - assert_ne!(first, appcontainer_profile_name("request-one")); + let rendered = String::from_utf16(&first[..first.len() - 1]).expect("readiness name"); + assert!(rendered.starts_with("maka.readiness.")); + assert_eq!(rendered.len(), "maka.readiness.".len() + 32); + } + + #[test] + fn readiness_namespace_is_disjoint_from_production() { + // Structural separation, not just a different hash input: the readiness + // profile lives under `maka.readiness.` while every production profile + // lives under `maka.sandbox.`, so the two lifecycles can never resolve to + // the same registration even if `validate` were bypassed. Feeding the + // reserved id through the production deriver must still land in the + // production namespace, disjoint from the readiness one. + let readiness = appcontainer_readiness_profile_name(); + let production_reserved = appcontainer_profile_name(RESERVED_READINESS_REQUEST_ID); + assert_ne!(readiness, production_reserved); + let readiness_str = String::from_utf16(&readiness[..readiness.len() - 1]).expect("name"); + let production_str = + String::from_utf16(&production_reserved[..production_reserved.len() - 1]) + .expect("name"); + assert!(readiness_str.starts_with("maka.readiness.")); + assert!(production_str.starts_with("maka.sandbox.")); + } + + #[test] + fn validate_rejects_reserved_readiness_request_id() { + // A production launch may never carry the readiness identity: otherwise + // it would resolve to a profile the readiness probe deletes and recreates. + let reserved = valid_launch_request(RESERVED_READINESS_REQUEST_ID); + let error = reserved + .validate() + .expect_err("reserved id must be rejected"); + assert!( + error.contains(RESERVED_READINESS_REQUEST_ID) && error.contains("reserved"), + "unexpected error: {error}" + ); + // The same request with an ordinary id is otherwise valid, proving the id + // is the sole reason for rejection. + assert!(valid_launch_request("request-one").validate().is_ok()); + } + + #[test] + fn readiness_lease_name_is_scoped_and_distinct() { + // The readiness lease is a machine-wide (`Global\`) named mutex scoped by + // the owning user's SID, and carries a distinct object name from the ACL + // ledger mutex so the two never contend. + let sid = "S-1-5-21-1111111111-2222222222-3333333333-1001"; + let name = readiness_mutex_name(sid); + assert!(name.starts_with(r"Global\Maka.WindowsSandbox.ReadinessProfile.")); + assert!(name.ends_with(sid)); + assert!(!name.contains("AclLedger")); } } diff --git a/packages/runtime/src/__tests__/default-sandbox-manager.test.ts b/packages/runtime/src/__tests__/default-sandbox-manager.test.ts index 96f3a72bdb..4c76109222 100644 --- a/packages/runtime/src/__tests__/default-sandbox-manager.test.ts +++ b/packages/runtime/src/__tests__/default-sandbox-manager.test.ts @@ -11,6 +11,7 @@ import { createDefaultSandboxManager, isBuiltinFilesystemWorkerSandboxAvailable, probeWindowsReadiness, + readCachedWindowsReadiness, type WindowsReadinessSpawn, } from '../sandbox/default-sandbox-manager.js'; @@ -173,4 +174,99 @@ describe('probeWindowsReadiness', () => { assert.equal(probeWindowsReadiness(missing, spawn), false); assert.equal(calls, 0); }); + + it('re-probes a transient negative after its TTL but caches a positive permanently', async () => { + await withFakeLauncher((clientPath) => { + let now = 1_000; + const clock = () => now; + let calls = 0; + let status: number = 1; + const spawn: WindowsReadinessSpawn = () => { + calls += 1; + return { status }; + }; + + // A transient failure is cached false, but only for the negative TTL. + assert.equal(probeWindowsReadiness(clientPath, spawn, clock), false); + assert.equal(calls, 1); + // Within the TTL the cached negative is reused, not re-probed. + now += 59_000; + assert.equal(probeWindowsReadiness(clientPath, spawn, clock), false); + assert.equal(calls, 1); + + // Past the TTL it re-probes; the host has recovered, so it flips to true. + now += 2_000; + status = 0; + assert.equal(probeWindowsReadiness(clientPath, spawn, clock), true); + assert.equal(calls, 2); + + // A positive result is cached permanently — no re-probe however far the + // clock advances. + now += 10 * 60_000; + assert.equal(probeWindowsReadiness(clientPath, spawn, clock), true); + assert.equal(calls, 2); + }); + }); + + it('never re-poisons after a positive: only negatives expire', async () => { + await withFakeLauncher((clientPath) => { + let now = 5_000; + const clock = () => now; + let calls = 0; + const spawn: WindowsReadinessSpawn = () => { + calls += 1; + return { status: 0 }; + }; + assert.equal(probeWindowsReadiness(clientPath, spawn, clock), true); + // Even a subsequent spawn that *would* fail is never consulted, because the + // positive entry never expires. + const failing: WindowsReadinessSpawn = () => { + calls += 1; + return { status: 1 }; + }; + now += 24 * 60 * 60_000; + assert.equal(probeWindowsReadiness(clientPath, failing, clock), true); + assert.equal(calls, 1); + }); + }); +}); + +describe('readCachedWindowsReadiness', () => { + it('is strictly cache-only: never spawns, fails closed until the cache is warmed', async () => { + await withFakeLauncher((clientPath) => { + let now = 2_000; + const clock = () => now; + + // Cold cache: the hot path must fail closed without any spawn (there is no + // spawn to inject — the read is structurally incapable of one). + assert.equal(readCachedWindowsReadiness(clientPath, clock), false); + + // The warmer populates a positive entry; the hot path then reflects it. + let calls = 0; + const spawn: WindowsReadinessSpawn = () => { + calls += 1; + return { status: 0 }; + }; + assert.equal(probeWindowsReadiness(clientPath, spawn, clock), true); + assert.equal(readCachedWindowsReadiness(clientPath, clock), true); + assert.equal(calls, 1); + }); + }); + + it('fails closed on an expired negative entry without re-probing on the hot path', async () => { + await withFakeLauncher((clientPath) => { + let now = 3_000; + const clock = () => now; + const spawn: WindowsReadinessSpawn = () => ({ status: 1 }); + + // Warm a negative entry via the spawning warmer. + assert.equal(probeWindowsReadiness(clientPath, spawn, clock), false); + // Before expiry the hot path returns the cached negative. + assert.equal(readCachedWindowsReadiness(clientPath, clock), false); + // After expiry the hot path still fails closed — it must NOT spawn to + // refresh; only the warmer may. The stale entry simply reads as false. + now += 61_000; + assert.equal(readCachedWindowsReadiness(clientPath, clock), false); + }); + }); }); diff --git a/packages/runtime/src/sandbox/default-sandbox-manager.ts b/packages/runtime/src/sandbox/default-sandbox-manager.ts index 29b77b96d8..80f2e31add 100644 --- a/packages/runtime/src/sandbox/default-sandbox-manager.ts +++ b/packages/runtime/src/sandbox/default-sandbox-manager.ts @@ -32,13 +32,32 @@ function builtinWindowsClientPath( */ const WINDOWS_READINESS_PROBE_TIMEOUT_MS = 15_000; +/** + * A negative readiness result may be transient (a spawn timeout under load, an + * antivirus scan briefly holding the launcher, a momentary AppContainer service + * hiccup), so it is cached only for this window before the next warmer re-probes. + * A single blip must not disable the Windows sandbox until the Runtime Host + * restarts. Positive results are cached permanently: host capability does not + * regress within a process lifetime. + */ +const WINDOWS_READINESS_NEGATIVE_TTL_MS = 60_000; + +interface WindowsReadinessCacheEntry { + readonly available: boolean; + /** Epoch ms after which this entry is stale; `Infinity` for positive results. */ + readonly expiresAt: number; +} + /** * Availability is a property of the host and the launcher binary, not of any one * backend instance, so the probe result is cached at module scope keyed by the * resolved client path. Two backends built for the same launcher share the one - * probe; distinct paths (tests, side-by-side installs) stay independent. + * probe; distinct paths (tests, side-by-side installs) stay independent. Entries + * carry an expiry so a transient negative result re-probes (see + * `WINDOWS_READINESS_NEGATIVE_TTL_MS`) instead of poisoning availability for the + * process lifetime. */ -const windowsReadinessCache = new Map(); +const windowsReadinessCache = new Map(); /** Outcome of one launcher `--readiness-probe` invocation. */ export interface WindowsReadinessSpawnResult { @@ -59,24 +78,30 @@ function spawnReadinessProbe(clientPath: string): WindowsReadinessSpawnResult { } /** - * Production-identity readiness probe for the Windows backend (RFC §6.4). + * Production-identity readiness probe for the Windows backend (RFC §6.4). This + * is the *spawning* entry point, called by the Runtime Host composition warmer; + * it may block on `spawnSync`, so it must never run on the operation hot path + * (use `readCachedWindowsReadiness` there). * * `existsSync` alone only proves the packaged launcher is present, not that the * OS can actually create the AppContainer identity, token, and Job on this * machine (AppContainer can be disabled by policy, the edition may not support * it, etc.). This runs the launcher's `--readiness-probe`, which stands up the - * real production identity and launches a throwaway confined child, and caches - * the result for the process lifetime — availability is stable per host, and - * the sync `SandboxBackend.isAvailable` contract must stay cheap after the - * first call. Anything other than a clean exit 0 fails closed (a non-zero - * status, a spawn error, or an external timeout that leaves `status === null`). + * real production identity and launches a throwaway confined child. A fresh, + * unexpired cache entry short-circuits the spawn; a positive result is cached + * permanently and a negative one only for `WINDOWS_READINESS_NEGATIVE_TTL_MS`, + * so one transient failure re-probes on the next warmer rather than disabling + * the sandbox until restart. Anything other than a clean exit 0 fails closed (a + * non-zero status, a spawn error, or an external timeout that leaves + * `status === null`). */ export function probeWindowsReadiness( clientPath: string, spawn: WindowsReadinessSpawn = spawnReadinessProbe, + now: () => number = Date.now, ): boolean { const cached = windowsReadinessCache.get(clientPath); - if (cached !== undefined) return cached; + if (cached !== undefined && now() < cached.expiresAt) return cached.available; let available = false; if (existsSync(clientPath)) { try { @@ -86,12 +111,28 @@ export function probeWindowsReadiness( available = false; } } - windowsReadinessCache.set(clientPath, available); + const expiresAt = available ? Infinity : now() + WINDOWS_READINESS_NEGATIVE_TTL_MS; + windowsReadinessCache.set(clientPath, { available, expiresAt }); return available; } +/** + * Strictly cache-only readiness read for the synchronous `isAvailable` hot path. + * It never spawns: it returns the warmed result when a fresh entry exists and + * fails closed (`false`) otherwise — including when a negative entry has expired + * — so the event loop is never blocked on `spawnSync`. The composition warmer + * (`probeWindowsReadiness`) is responsible for (re)populating the cache. + */ +export function readCachedWindowsReadiness( + clientPath: string, + now: () => number = Date.now, +): boolean { + const cached = windowsReadinessCache.get(clientPath); + return cached !== undefined && now() < cached.expiresAt ? cached.available : false; +} + function createWindowsReadinessProbe(clientPath: string): () => boolean { - return () => probeWindowsReadiness(clientPath); + return () => readCachedWindowsReadiness(clientPath); } function builtinWindowsBackend( @@ -132,11 +173,12 @@ export function isBuiltinFilesystemWorkerSandboxAvailable( if (platform === 'darwin') return true; if (platform === 'win32') { // Single source of truth for Windows availability: file presence only - // discovers the launcher path; the memoized readiness probe decides - // availability. Runtime Host composition calls this at startup, which warms - // `windowsReadinessCache`, so the backend's later synchronous `isAvailable` - // on the operation hot path hits the cache instead of spawning. File - // presence is no longer a second availability authority. + // discovers the launcher path; the readiness probe decides availability. + // Runtime Host composition calls this at startup (and again as the cache + // expires), which warms `windowsReadinessCache` via the spawning probe. The + // backend's synchronous `isAvailable` on the operation hot path is strictly + // cache-only (`readCachedWindowsReadiness`) and never spawns. File presence + // is no longer a second availability authority. const clientPath = builtinWindowsClientPath(platform, resourcesPath); return clientPath !== undefined && probeWindowsReadiness(clientPath); } From 8b03f9f6176f6679bbd1171e5d9f712db3de65cb Mon Sep 17 00:00:00 2001 From: liugddx Date: Tue, 18 Aug 2026 18:01:52 +0800 Subject: [PATCH 5/9] fix(windows-sandbox): scope readiness recovery and quarantine claims to what the code enforces MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hqhq1025's round-3 review is factually correct on both points; this converges the claims rather than building the heavier machinery, matching the preview's fail-closed scope. Blocker 1 (TTL recovery overclaim): the filesystem worker is published once when a composition is built, so a negative readiness result is not recovered within a running host — the 60s negative TTL only bounds how long a stale negative poisons the module cache so the *next* composition build re-probes. De-claim the RFC (EN+zh §6.4/§6.5) and default-sandbox-manager.ts comments from 're-probes on the next composition rather than disabling until restart' to honest new-composition/restart-scoped recovery, and mark an active running-host readiness retry with dynamic worker publication as a deferred gate. Add a test asserting the hot path never self-recovers a negative; only an explicit re-probe (a new composition build) can. Blocker 2 (ineffective preserve/keep_on_drop): create_readiness deletes the fixed identity unconditionally next cycle, so preserve() only deferred deletion by one probe and never durably quarantined. Remove preserve()/keep_on_drop and the unsettled out-param; the settlement-failure surfacing in the match stays, so an unsettled probe still fails closed (reports unavailable). Document that cleanup relies on the kill-on-close Job's tree termination and a zero-filesystem-root identity, and mark durable quarantine (or unique probe identities plus a reconciliation ledger) as a deferred gate (RFC §6.5). Co-Authored-By: Claude Fable 5 Generated-by: Claude Fable 5 --- docs/architecture/windows-sandbox-rfc-v1.md | 34 ++++++-- .../windows-sandbox-rfc-v1.zh-CN.md | 6 +- .../launcher/src/windows_launcher.rs | 85 +++++++------------ .../__tests__/default-sandbox-manager.test.ts | 33 +++++++ .../src/sandbox/default-sandbox-manager.ts | 43 ++++++---- 5 files changed, 120 insertions(+), 81 deletions(-) diff --git a/docs/architecture/windows-sandbox-rfc-v1.md b/docs/architecture/windows-sandbox-rfc-v1.md index 343bd9afbf..958ef3437a 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.md +++ b/docs/architecture/windows-sandbox-rfc-v1.md @@ -175,11 +175,16 @@ Lexical prefix checks are never authorization evidence. validation, so no production launch can ever resolve to the profile the probe deletes and recreates. The whole delete→create→probe→settle→drop cycle is serialized across processes by a DACL-hardened per-user named mutex — the same primitive the ACL ledger uses — so concurrent probes - cannot delete each other's live registration. When the probe cannot prove its Job empty it - preserves the registration for a later lease-holder to reclaim rather than deleting an identity a - descendant may still hold. On the consumer side a negative availability result is cached only for a - bounded TTL, so a single transient failure re-probes on the next composition rather than disabling - the sandbox until the Runtime Host restarts; a positive result is cached for the process lifetime.)_ + cannot delete each other's live registration. When the probe cannot prove its Job drained it fails + closed (reports unavailable) for that cycle; the fixed readiness identity is not durably + quarantined — cleanup relies on the kill-on-close Job's tree termination, and because the probe + grants zero filesystem roots a surviving child inherits no ACE authority. On the consumer side a + negative availability result is cached only for a bounded TTL, which bounds how long one transient + failure poisons the module cache: the *next composition build* re-probes. It is not a running-host + retry — the filesystem worker is published once when a composition is built, so a host that already + resolved availability negative recovers only on a new composition or a Runtime Host restart; a + positive result is cached for the process lifetime. Durable quarantine of an unsettled identity and + active running-host readiness recovery are deferred gates — see §6.5.)_ - Launcher signature, version, and digest are verified against packaged metadata. _(Later gate: the per-launch request digest is recomputed and enforced in-broker today; verifying the launcher binary's signature and version against packaged metadata is deferred with Phase 3 signing — see @@ -220,8 +225,10 @@ Enforced in the preview slice: - a dedicated, cross-process-serialized readiness profile lifecycle (§6.4): the probe profile lives in a namespace disjoint from production, its reserved `requestId` is rejected by validation, a DACL-hardened per-user named mutex serializes its delete→create→probe→drop cycle, an unsettled - probe preserves rather than deletes its registration, and negative availability is cached with a - bounded TTL so one transient failure does not disable the sandbox until restart; + probe fails closed rather than claiming a clean boundary (relying on the kill-on-close Job and a + zero-authority identity for cleanup, not durable quarantine), and negative availability is cached + with a bounded TTL so one transient failure does not poison the module cache past that window — the + next composition build re-probes, rather than the running host recovering in place; - fail-closed capability outcomes with no unsandboxed fallback for `auto`/`require` (§6.4). Designed but deferred as later gates (not enforced in the preview slice): @@ -244,6 +251,19 @@ Designed but deferred as later gates (not enforced in the preview slice): validation primitives; a multi-process race test that spawns real concurrent probes on a live Windows host is deferred as disproportionate for a throwaway diagnostic probe and inherently flaky in CI. The serialization primitive itself, not an end-to-end race harness, is the enforced contract. +- Durable quarantine of an unsettled readiness identity (§6.4). When a probe cannot prove its Job + drained it fails closed for that cycle, and the next probe deletes and recreates the fixed + identity under the lease. Residual risk is bounded — the readiness child is `cmd.exe /c exit 0` + granted zero filesystem roots, so a hypothetically-surviving child spawns nothing and inherits no + ACE authority, and the kill-on-close Job terminates the tree — but the identity is not durably + quarantined. Durable quarantine (or unique per-probe identities plus an orphan/reconciliation + ledger) is deferred. +- Active running-host readiness recovery (§6.4). A negative availability result is bounded by a TTL + so it does not poison the module cache past that window, and the *next composition build* re-probes. + A running Runtime Host does not actively re-probe or hot-publish the filesystem worker — the worker + is composed once when a candidate is built — so recovery from a transient negative in an + already-running host is scoped to a new composition build or a restart. An active readiness + retry with dynamic worker publication is deferred. Deferral narrows readiness richness and desktop-layer defense-in-depth, not the enforcement boundary: an unavailable, drifted, or failed backend still fails closed, and a restricted managed diff --git a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md index 1ec9831142..4f69efcc34 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md +++ b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md @@ -135,7 +135,7 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 ### 6.4 能力与失败 - readiness 必须在生产 identity/token/Job/desktop/handle/filesystem/offline network 下启动真实 probe； _(已实现:预览版的 `--readiness-probe` 会真正建立 AppContainer identity/token 与 kill-on-close Job 并启动一个抛弃式受限子进程,宿主无法创建或强制边界时 fail closed,而非仅凭二进制存在即注册;private desktop 与完整的按 profile filesystem/offline network 策略尚未在 readiness 阶段演练 —— 见 §6.5。)_ -- readiness probe 的抛弃式 profile 生命周期必须隔离且 fail closed； _(已实现:probe profile 位于专属 `maka.readiness.` 命名空间,与生产 `maka.sandbox.` 命名空间结构性不相交,其保留的 `requestId` 被 launch validation 拒绝,任何生产启动都无法解析到 probe 删除并重建的那个 profile;整个 delete→create→probe→settle→drop 生命周期由一个 DACL 加固的按用户命名互斥量跨进程串行——与 ACL ledger 复用同一原语——使并发 probe 不会互删对方的 active 注册;当 probe 无法证明其 Job 清空时,保留该注册交由后续持锁者 reclaim,而非删除一个 descendant 可能仍持有的 identity;消费侧对负可用性结果只按有界 TTL 缓存,一次瞬时失败会在下次 composition 重探,而非把沙箱禁用到 Runtime Host 重启,正结果则按进程生命周期缓存。)_ +- readiness probe 的抛弃式 profile 生命周期必须隔离且 fail closed； _(已实现:probe profile 位于专属 `maka.readiness.` 命名空间,与生产 `maka.sandbox.` 命名空间结构性不相交,其保留的 `requestId` 被 launch validation 拒绝,任何生产启动都无法解析到 probe 删除并重建的那个 profile;整个 delete→create→probe→settle→drop 生命周期由一个 DACL 加固的按用户命名互斥量跨进程串行——与 ACL ledger 复用同一原语——使并发 probe 不会互删对方的 active 注册;当 probe 无法证明其 Job 清空时按该周期 fail closed(报告不可用),固定的 readiness identity 并不被持久隔离——清理依赖 kill-on-close Job 的整树终止,且因该 probe 不授任何 filesystem root,一个假设存活的子进程也继承不到任何 ACE 权限;消费侧对负可用性结果只按有界 TTL 缓存,以限制一次瞬时失败毒化 module 缓存的时长:由**下一次 composition 构建**重探,而非运行中的宿主原地恢复——filesystem worker 在 composition 构建时一次性发布,故一个已判负的宿主只在新 composition 或 Runtime Host 重启时恢复,正结果则按进程生命周期缓存。未证清空 identity 的持久隔离,以及运行中宿主的主动 readiness 恢复,均为后续门禁——见 §6.5。)_ - launcher signature/version/digest 必须与 package metadata 一致； _(后续门禁：每次启动的 request digest 目前已在 broker 内重算并强制；对照打包 metadata 校验 launcher 二进制的 signature 与 version 随 Phase 3 签名一并暂缓 —— 见 §6.5。)_ - setup 缺失、identity drift、ACL state 损坏、网络策略无效、文件系统不支持、helper 不匹配、probe 失败都返回 stable typed unavailable reason； _(后续门禁:readiness probe 目前把每种失败收敛为单一 fail-closed 布尔,统一以 @@ -157,7 +157,7 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 - 仅通过 `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` 继承声明的 handle（§6.3）； - 封闭、排序后的 allowlist 环境（§6.3）； - 生产 identity readiness probe（§6.4）:`--readiness-probe` 真正建立 AppContainer identity/token 与 kill-on-close Job 并启动抛弃式受限子进程,使可用性在宿主无法创建边界时 fail closed,而非仅凭打包二进制存在; -- 专属且跨进程串行的 readiness profile 生命周期（§6.4）:probe profile 位于与生产不相交的命名空间,其保留 `requestId` 被 validation 拒绝,一个 DACL 加固的按用户命名互斥量串行其 delete→create→probe→drop 生命周期,未证清空的 probe 保留而非删除其注册,负可用性按有界 TTL 缓存,使一次瞬时失败不会把沙箱禁用到重启; +- 专属且跨进程串行的 readiness profile 生命周期（§6.4）:probe profile 位于与生产不相交的命名空间,其保留 `requestId` 被 validation 拒绝,一个 DACL 加固的按用户命名互斥量串行其 delete→create→probe→drop 生命周期,未证清空的 probe 按周期 fail closed 而非宣称边界干净(清理依赖 kill-on-close Job 与零权限 identity,而非持久隔离),负可用性按有界 TTL 缓存以限制一次瞬时失败毒化 module 缓存的时长——由下一次 composition 构建重探,而非运行中宿主原地恢复; - fail-closed capability check，绝不 unsandboxed fallback（§6.4）。 **已设计但作为后续门禁暂缓（预览切片尚未强制）：** @@ -172,6 +172,8 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 互斥量名、命名空间与 validation 原语的单元测试;在真实 Windows 宿主上 spawn 多个并发 probe 的多进程 竞态测试暂缓——对一个抛弃式诊断探针不成比例且在 CI 中天然 flaky。被强制的契约是串行化原语本身,而非 端到端竞态 harness。 +- 未证清空的 readiness identity 的持久隔离（§6.4）:probe 无法证明其 Job 清空时按该周期 fail closed,下一次 probe 在锁下删除并重建那个固定 identity。残余风险有界——readiness 子进程是被授零 filesystem root 的 `cmd.exe /c exit 0`,一个假设存活的子进程既不 spawn 任何东西也继承不到任何 ACE 权限,且 kill-on-close Job 会终止整树——但该 identity 未被持久隔离。持久隔离(或每次 probe 用唯一 identity 加 orphan/对账 ledger)暂缓。 +- 运行中宿主的主动 readiness 恢复（§6.4）:负可用性结果由 TTL 限时,使其不会长时间毒化 module 缓存,并由**下一次 composition 构建**重探。运行中的 Runtime Host 不会主动重探或热发布 filesystem worker——worker 在候选构建时一次性组装——故已判负的运行中宿主对瞬时负结果的恢复被限定到新 composition 构建或重启。带动态 worker 发布的主动 readiness 重试暂缓。 暂缓收窄的是 readiness 丰富度与 desktop 层的 defense-in-depth，而非强制边界本身：backend 不可用、identity drift 或启动失败仍然 fail closed，受限 managed profile 也绝不回退到宿主执行。 diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher.rs b/experiments/windows-sandbox/launcher/src/windows_launcher.rs index c0df97a6f9..4a683bd465 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher.rs @@ -195,28 +195,26 @@ pub fn readiness_probe() -> Result { // without a persistent ledger and without relying on process // destruction to run `Drop`. The lease above makes this reclaim safe: // no other process can hold the profile while we delete and recreate it. - let mut profile = match AppContainerProfile::create_readiness() { + let profile = match AppContainerProfile::create_readiness() { Ok(profile) => profile, Err(error) => { CloseHandle(job); return Err(error); } }; - let mut unsettled = false; - let result = probe_appcontainer_child(&cmd_path, job, profile.sid, &mut unsettled); + let result = probe_appcontainer_child(&cmd_path, job, profile.sid); // Closing the last Job handle is the kernel backstop after the probe has // already explicitly terminated and drained its child (see // `probe_appcontainer_child`); it terminates any straggler the drain - // did not reap. + // did not reap. When the child cannot be proven drained the probe + // returns an error, so availability fails closed for this cycle. The + // fixed readiness identity is not durably quarantined: cleanup relies on + // the kill-on-close Job's tree termination, and because the probe grants + // zero filesystem roots a hypothetically-surviving child inherits no ACE + // authority. Durable quarantine of an unsettled identity — or unique + // probe identities plus a reconciliation ledger — is a deferred gate + // (RFC §6.5). CloseHandle(job); - if unsettled { - // The Job was not proven empty, so a descendant may still hold this - // AppContainer identity. Preserve the registration rather than - // deleting a profile that could still be in use; the next - // lease-holding `create_readiness` best-effort reclaims it — safe - // because the lease serializes that reclaim against any live holder. - profile.preserve(); - } drop(profile); result } @@ -230,7 +228,6 @@ unsafe fn probe_appcontainer_child( cmd_path: &str, job: HANDLE, app_container_sid: *mut c_void, - unsettled: &mut bool, ) -> Result { let executable = wide(cmd_path); // Build the command line directly: cmd.exe's `/c` switch must stay @@ -370,17 +367,15 @@ unsafe fn probe_appcontainer_child( CloseHandle(process.hProcess); } - // Report whether the identity is proven drained. A settlement failure means - // the Job was not confirmed empty, so a descendant may still hold this - // AppContainer profile; the caller must preserve the registration instead of - // deleting one that could still be in use. This is independent of `verify`: - // a verification failure whose Job *did* drain cleanly leaves nothing behind - // and the profile is safe to delete. - *unsettled = settled.is_err(); - // A verification/exit failure is the primary diagnosis; a settlement failure - // is itself a fail-closed signal (something still holds the identity), so it - // is surfaced when the probe otherwise succeeded. + // is itself a fail-closed signal (the Job was not confirmed empty, so a + // descendant may still hold this AppContainer identity), so it is surfaced + // when the probe otherwise succeeded. Either outcome reports unavailable + // rather than claiming a clean boundary; the readiness identity is not + // preserved for reuse — its profile carries no filesystem roots, so a + // surviving child inherits no ACE authority, and the kill-on-close Job is the + // cleanup backstop. Durable quarantine of an unsettled identity is a deferred + // gate (RFC §6.5). match (verify, settled) { (Err(error), _) => Err(error), (Ok(_), Err(cleanup)) => Err(cleanup), @@ -465,22 +460,9 @@ unsafe fn sid_string(sid: *mut c_void) -> Result { struct AppContainerProfile { sid: *mut c_void, name: Vec, - /// When set, `Drop` skips `DeleteAppContainerProfile`. The readiness probe - /// sets it when its Job could not be proven empty: a descendant may still - /// hold this identity, so the registration is preserved for a later - /// lease-holding `create_readiness` to reclaim rather than deleted while - /// possibly in use. Production launches never set it (each has a unique - /// identity that no live launch reuses). - keep_on_drop: bool, } impl AppContainerProfile { - /// Preserve this profile's registration on drop instead of deleting it. - /// Called only when the readiness probe could not prove its Job empty. - fn preserve(&mut self) { - self.keep_on_drop = true; - } - unsafe fn create(request_id: &str) -> Result { let name = appcontainer_profile_name(request_id); let display_name = wide("Maka Windows Sandbox"); @@ -505,11 +487,7 @@ impl AppContainerProfile { result as u32 )); } - Ok(Self { - sid, - name, - keep_on_drop: false, - }) + Ok(Self { sid, name }) } /// Create the throwaway readiness profile under a fixed, self-reconciling @@ -547,11 +525,7 @@ impl AppContainerProfile { result as u32 )); } - Ok(Self { - sid, - name, - keep_on_drop: false, - }) + Ok(Self { sid, name }) } } @@ -569,16 +543,15 @@ impl Drop for AppContainerProfile { fn drop(&mut self) { unsafe { FreeSid(self.sid); - if !self.keep_on_drop { - // The request-derived name is never reused by another live - // launch. Best-effort deletion keeps the user profile store - // bounded; a crash can leave this registration behind, but a - // future request has a different SID and cannot inherit its ACL - // authority. Skipped when `keep_on_drop` is set: the readiness - // probe could not prove its Job empty, so the identity may still - // be held and must be preserved for a later reclaim. - DeleteAppContainerProfile(self.name.as_ptr()); - } + // The request-derived name is never reused by another live launch. + // Best-effort deletion keeps the user profile store bounded; a crash + // can leave this registration behind, but a future request has a + // different SID and cannot inherit its ACL authority. The readiness + // probe instead uses a fixed name and reclaims it under a lease on + // the next cycle (`create_readiness`); an unsettled probe fails + // closed rather than durably quarantining its identity — that + // quarantine is a deferred gate (RFC §6.5). + DeleteAppContainerProfile(self.name.as_ptr()); } } } diff --git a/packages/runtime/src/__tests__/default-sandbox-manager.test.ts b/packages/runtime/src/__tests__/default-sandbox-manager.test.ts index 4c76109222..ad8e3d828e 100644 --- a/packages/runtime/src/__tests__/default-sandbox-manager.test.ts +++ b/packages/runtime/src/__tests__/default-sandbox-manager.test.ts @@ -269,4 +269,37 @@ describe('readCachedWindowsReadiness', () => { assert.equal(readCachedWindowsReadiness(clientPath, clock), false); }); }); + + it('recovers a negative only on a new composition build (spawning warmer), never on the running hot path', async () => { + await withFakeLauncher((clientPath) => { + let now = 4_000; + const clock = () => now; + let calls = 0; + // Available now, but the first probe caught a transient failure. + const spawn: WindowsReadinessSpawn = () => { + calls += 1; + return calls === 1 ? { status: 1 } : { status: 0 }; + }; + + // A composition build warms a negative entry. + assert.equal(probeWindowsReadiness(clientPath, spawn, clock), false); + + // Model a running Runtime Host: the worker was published once at build + // time and the hot path is all that runs afterwards. Advancing well past + // the TTL never flips it — a running composition does not self-recover, + // and the read never spawns (calls stays at 1). + for (let i = 0; i < 5; i += 1) { + now += 60_000; + assert.equal(readCachedWindowsReadiness(clientPath, clock), false); + } + assert.equal(calls, 1); + + // Recovery is scoped to the next composition build: a fresh spawning probe + // (a new composition, or a Runtime Host restart) re-evaluates and, now that + // the host is healthy, publishes the worker. + assert.equal(probeWindowsReadiness(clientPath, spawn, clock), true); + assert.equal(readCachedWindowsReadiness(clientPath, clock), true); + assert.equal(calls, 2); + }); + }); }); diff --git a/packages/runtime/src/sandbox/default-sandbox-manager.ts b/packages/runtime/src/sandbox/default-sandbox-manager.ts index 80f2e31add..50928308c0 100644 --- a/packages/runtime/src/sandbox/default-sandbox-manager.ts +++ b/packages/runtime/src/sandbox/default-sandbox-manager.ts @@ -35,10 +35,15 @@ const WINDOWS_READINESS_PROBE_TIMEOUT_MS = 15_000; /** * A negative readiness result may be transient (a spawn timeout under load, an * antivirus scan briefly holding the launcher, a momentary AppContainer service - * hiccup), so it is cached only for this window before the next warmer re-probes. - * A single blip must not disable the Windows sandbox until the Runtime Host - * restarts. Positive results are cached permanently: host capability does not - * regress within a process lifetime. + * hiccup). Caching it for only this window bounds how long one blip poisons the + * module-scoped cache: a *later* composition build re-probes instead of + * inheriting a stale negative. It does not make an already-running host recover + * — the filesystem worker is published once, when a composition is built (see + * `isBuiltinFilesystemWorkerSandboxAvailable` and RFC §6.4), so a composition + * that resolved availability negative stays without the worker for its lifetime; + * recovery is scoped to the next composition build or a Runtime Host restart. + * Positive results are cached permanently: host capability does not regress + * within a process lifetime. */ const WINDOWS_READINESS_NEGATIVE_TTL_MS = 60_000; @@ -52,10 +57,10 @@ interface WindowsReadinessCacheEntry { * Availability is a property of the host and the launcher binary, not of any one * backend instance, so the probe result is cached at module scope keyed by the * resolved client path. Two backends built for the same launcher share the one - * probe; distinct paths (tests, side-by-side installs) stay independent. Entries - * carry an expiry so a transient negative result re-probes (see - * `WINDOWS_READINESS_NEGATIVE_TTL_MS`) instead of poisoning availability for the - * process lifetime. + * probe; distinct paths (tests, side-by-side installs) stay independent. A + * negative entry carries a bounded expiry so it does not poison availability for + * the whole process lifetime — the *next composition build* re-probes (see + * `WINDOWS_READINESS_NEGATIVE_TTL_MS`); this is not an in-composition retry. */ const windowsReadinessCache = new Map(); @@ -90,9 +95,12 @@ function spawnReadinessProbe(clientPath: string): WindowsReadinessSpawnResult { * real production identity and launches a throwaway confined child. A fresh, * unexpired cache entry short-circuits the spawn; a positive result is cached * permanently and a negative one only for `WINDOWS_READINESS_NEGATIVE_TTL_MS`, - * so one transient failure re-probes on the next warmer rather than disabling - * the sandbox until restart. Anything other than a clean exit 0 fails closed (a - * non-zero status, a spawn error, or an external timeout that leaves + * so a transient failure does not poison the module cache past that window — the + * *next composition build* re-probes. This is not a running-host retry: the + * warmer runs at composition-build time and the worker is published once, so a + * host that already resolved availability negative recovers only on a new + * composition or a restart (RFC §6.4). Anything other than a clean exit 0 fails + * closed (a non-zero status, a spawn error, or an external timeout that leaves * `status === null`). */ export function probeWindowsReadiness( @@ -174,11 +182,14 @@ export function isBuiltinFilesystemWorkerSandboxAvailable( if (platform === 'win32') { // Single source of truth for Windows availability: file presence only // discovers the launcher path; the readiness probe decides availability. - // Runtime Host composition calls this at startup (and again as the cache - // expires), which warms `windowsReadinessCache` via the spawning probe. The - // backend's synchronous `isAvailable` on the operation hot path is strictly - // cache-only (`readCachedWindowsReadiness`) and never spawns. File presence - // is no longer a second availability authority. + // Runtime Host composition calls this once when it builds (and again only + // when a *new* composition is built), warming `windowsReadinessCache` via the + // spawning probe. It is not re-run when a cache entry expires, so recovery + // from a transient negative is scoped to the next composition build or a + // restart (RFC §6.4). The backend's synchronous `isAvailable` on the + // operation hot path is strictly cache-only (`readCachedWindowsReadiness`) + // and never spawns. File presence is no longer a second availability + // authority. const clientPath = builtinWindowsClientPath(platform, resourcesPath); return clientPath !== undefined && probeWindowsReadiness(clientPath); } From 04892c7ddf500d131594577efb85db44fc7dcb60 Mon Sep 17 00:00:00 2001 From: liugddx Date: Tue, 18 Aug 2026 19:15:29 +0800 Subject: [PATCH 6/9] fix(windows-sandbox): disable cmd AutoRun in the readiness probe The readiness probe launched its throwaway child as `"" /c exit 0` without `/d`, so cmd.exe ran the machine-wide `Software\Microsoft\Command Processor\AutoRun` value before the payload. Because AutoRun is machine-constant, a host that ships an AutoRun which exits non-zero would make *every* probe report failure, and one that ships a blocking AutoRun would hang the child until the Job drain times out -- either way the Windows sandbox fails closed on that machine on every startup, independent of whether the OS can actually stand up the boundary. The probe would then be measuring the host's shell customization instead of the sandbox. Route the command line through a pure `readiness_probe_command_line` helper that emits `"" /d /c exit 0`, matching the `/d` the production launch path already passes, and add a unit test asserting `/d` precedes `/c`. Also correct the RFC (EN + zh): the readiness probe and its serialized profile lifecycle are added by this PR (#3161), not the merged #2961 preview slice -- tag those two bullets accordingly and note the AutoRun-disabled command line. Co-Authored-By: Claude Fable 5 Generated-by: Claude Fable 5 --- docs/architecture/windows-sandbox-rfc-v1.md | 21 +++++++++++-------- .../windows-sandbox-rfc-v1.zh-CN.md | 8 +++---- .../launcher/src/windows_launcher.rs | 19 +++++++++++++++-- .../launcher/src/windows_launcher_tests.rs | 14 +++++++++++++ 4 files changed, 47 insertions(+), 15 deletions(-) diff --git a/docs/architecture/windows-sandbox-rfc-v1.md b/docs/architecture/windows-sandbox-rfc-v1.md index 958ef3437a..74eeb89f8c 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.md +++ b/docs/architecture/windows-sandbox-rfc-v1.md @@ -203,11 +203,13 @@ Lexical prefix checks are never authorization evidence. The first product slice — the packaged Windows 11 x64 AppContainer backend in [#2961](https://github.com/maka-agent/maka-agent/pull/2961), merged 2026-08-17 — enforces a subset -of the guarantees above. This subsection aligns the documented guarantees with what the preview -actually ships so the RFC does not overclaim. The remaining guarantees are designed but explicitly -deferred as later gates, tracked by Phase 4 in [#2142](https://github.com/maka-agent/maka-agent/issues/2142). +of the guarantees above. This subsection aligns the documented guarantees with what the code +actually ships so the RFC does not overclaim. Bullets tagged `(#3161)` land in the readiness-probe +follow-up rather than the merged #2961 slice; the rest are enforced by #2961 today. The remaining +guarantees are designed but explicitly deferred as later gates, tracked by Phase 4 in +[#2142](https://github.com/maka-agent/maka-agent/issues/2142). -Enforced in the preview slice: +Enforced (merged in #2961 unless tagged otherwise): - default-deny filesystem with distinct read/write roots compiled from the exact profile (§6.1); - recursive reparse-point rejection and multi-hard-link rejection before ACL mutation (§5, §6.1); @@ -218,11 +220,12 @@ Enforced in the preview slice: - inheritance limited to declared stdio/protocol handles through `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` (§6.3); - a closed, sorted, allowlisted environment (§6.3); -- a production-identity readiness probe (§6.4): `--readiness-probe` stands up the real AppContainer - identity and token and a kill-on-close Job and launches a throwaway confined child, so - availability fails closed on hosts where the OS cannot create the boundary rather than on the - packaged binary's presence alone; -- a dedicated, cross-process-serialized readiness profile lifecycle (§6.4): the probe profile lives +- a production-identity readiness probe (§6.4) **(#3161)**: `--readiness-probe` stands up the real + AppContainer identity and token and a kill-on-close Job and launches a throwaway confined child + (`cmd.exe /d /c exit 0`, with AutoRun disabled so a host's shell customization cannot skew the + result), so availability fails closed on hosts where the OS cannot create the boundary rather than + on the packaged binary's presence alone; +- a dedicated, cross-process-serialized readiness profile lifecycle (§6.4) **(#3161)**: the probe profile lives in a namespace disjoint from production, its reserved `requestId` is rejected by validation, a DACL-hardened per-user named mutex serializes its delete→create→probe→drop cycle, an unsettled probe fails closed rather than claiming a clean boundary (relying on the kill-on-close Job and a diff --git a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md index 4f69efcc34..2ddf8947b5 100644 --- a/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md +++ b/docs/architecture/windows-sandbox-rfc-v1.zh-CN.md @@ -145,9 +145,9 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 ### 6.5 预览实现状态（2026-08-17） -首个预览切片 [#2961](https://github.com/maka-agent/maka-agent/pull/2961) 已于 2026-08-17 合并，强制上述保证的一个子集。本节把文档与已交付切片对齐，使 RFC 不 overclaim：§6.3/§6.4 中尚未强制的保证在此显式标为后续门禁。 +首个预览切片 [#2961](https://github.com/maka-agent/maka-agent/pull/2961) 已于 2026-08-17 合并，强制上述保证的一个子集。本节把文档与已交付代码对齐，使 RFC 不 overclaim：§6.3/§6.4 中尚未强制的保证在此显式标为后续门禁。标注 `(#3161)` 的条目落在 readiness-probe 后续 PR，而非已合并的 #2961 切片；其余条目由 #2961 当前强制。 -**预览切片已强制：** +**已强制（未标注者由 #2961 合并强制）：** - 默认拒绝文件系统，读/写 grant 分离（§6.1）； - ACL 修改前拒绝 reparse point 与多硬链接对象（§5/§6.1）； @@ -156,8 +156,8 @@ Maka 外已失陷的同用户进程。sandboxed code 从第一条指令开始按 - 创建时原子附加、close 时杀整棵树的 kill-on-close Job（§6.3）； - 仅通过 `PROC_THREAD_ATTRIBUTE_HANDLE_LIST` 继承声明的 handle（§6.3）； - 封闭、排序后的 allowlist 环境（§6.3）； -- 生产 identity readiness probe（§6.4）:`--readiness-probe` 真正建立 AppContainer identity/token 与 kill-on-close Job 并启动抛弃式受限子进程,使可用性在宿主无法创建边界时 fail closed,而非仅凭打包二进制存在; -- 专属且跨进程串行的 readiness profile 生命周期（§6.4）:probe profile 位于与生产不相交的命名空间,其保留 `requestId` 被 validation 拒绝,一个 DACL 加固的按用户命名互斥量串行其 delete→create→probe→drop 生命周期,未证清空的 probe 按周期 fail closed 而非宣称边界干净(清理依赖 kill-on-close Job 与零权限 identity,而非持久隔离),负可用性按有界 TTL 缓存以限制一次瞬时失败毒化 module 缓存的时长——由下一次 composition 构建重探,而非运行中宿主原地恢复; +- 生产 identity readiness probe（§6.4）**(#3161)**:`--readiness-probe` 真正建立 AppContainer identity/token 与 kill-on-close Job 并启动抛弃式受限子进程（`cmd.exe /d /c exit 0`,以 `/d` 关闭 AutoRun 使宿主 shell 定制不能扭曲结果）,使可用性在宿主无法创建边界时 fail closed,而非仅凭打包二进制存在; +- 专属且跨进程串行的 readiness profile 生命周期（§6.4）**(#3161)**:probe profile 位于与生产不相交的命名空间,其保留 `requestId` 被 validation 拒绝,一个 DACL 加固的按用户命名互斥量串行其 delete→create→probe→drop 生命周期,未证清空的 probe 按周期 fail closed 而非宣称边界干净(清理依赖 kill-on-close Job 与零权限 identity,而非持久隔离),负可用性按有界 TTL 缓存以限制一次瞬时失败毒化 module 缓存的时长——由下一次 composition 构建重探,而非运行中宿主原地恢复; - fail-closed capability check，绝不 unsandboxed fallback（§6.4）。 **已设计但作为后续门禁暂缓（预览切片尚未强制）：** diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher.rs b/experiments/windows-sandbox/launcher/src/windows_launcher.rs index 4a683bd465..c4555c76d2 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher.rs @@ -220,6 +220,21 @@ pub fn readiness_probe() -> Result { } } +/// Command line for the throwaway readiness child: `"" /d /c exit 0`. +/// +/// `/d` is load-bearing, not cosmetic. Without it cmd.exe runs the +/// `HKLM`/`HKCU\Software\Microsoft\Command Processor\AutoRun` value before the +/// `/c` payload. A machine whose AutoRun exits non-zero would make every probe +/// report failure; one whose AutoRun blocks would hang the child until the Job +/// drain times out — either way the sandbox fails closed on *every* startup, +/// because AutoRun is machine-constant. `/d` disables AutoRun so the probe +/// measures the sandbox boundary, not the host's shell customization. The +/// production launch path already passes `/d` before `/c`; this keeps the +/// diagnostic probe on the same contract. +pub(crate) fn readiness_probe_command_line(cmd_path: &str) -> String { + format!("\"{cmd_path}\" /d /c exit 0") +} + /// Launch the throwaway readiness child under the AppContainer token and Job. /// Mirrors `create_appcontainer_child` but carries no stdio/handle inheritance /// (nothing is relayed) and inherits the parent environment, since the child @@ -230,10 +245,10 @@ unsafe fn probe_appcontainer_child( app_container_sid: *mut c_void, ) -> Result { let executable = wide(cmd_path); - // Build the command line directly: cmd.exe's `/c` switch must stay + // Build the command line directly: cmd.exe's `/d`/`/c` switches must stay // unquoted, so the per-token quoting used for arbitrary workloads would // make cmd treat "/c" as a program to run and exit non-zero. - let mut command = wide(&format!("\"{cmd_path}\" /c exit 0")); + let mut command = wide(&readiness_probe_command_line(cmd_path)); // Run from System32 rather than inheriting the launcher's cwd: the probe // grants no filesystem roots, so an inherited working directory would be // default-denied and cmd.exe would fail to initialize. System32 is readable diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs index 59377031d5..e5c12c0d0a 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs @@ -6,6 +6,7 @@ mod tests { use crate::protocol::{LaunchRequest, NetworkMode, RESERVED_READINESS_REQUEST_ID}; use crate::windows_launcher::{ appcontainer_profile_name, appcontainer_readiness_profile_name, environment_block, + readiness_probe_command_line, }; fn valid_launch_request(request_id: &str) -> LaunchRequest { @@ -178,4 +179,17 @@ mod tests { assert!(name.ends_with(sid)); assert!(!name.contains("AclLedger")); } + + #[test] + fn readiness_command_line_disables_autorun_before_exit() { + // `/d` must precede `/c` so cmd.exe skips the machine-constant + // `Command Processor\AutoRun` value. Without it, a host whose AutoRun + // exits non-zero (or blocks) would make the readiness probe fail closed + // on every startup, disabling the sandbox regardless of the boundary. + let line = readiness_probe_command_line("C:\\Windows\\System32\\cmd.exe"); + assert_eq!(line, "\"C:\\Windows\\System32\\cmd.exe\" /d /c exit 0"); + let d = line.find("/d").expect("/d present"); + let c = line.find("/c").expect("/c present"); + assert!(d < c, "/d must come before /c: {line}"); + } } From a59c2554e9a7e0e58d92da2fec9d7cc6f258f0b8 Mon Sep 17 00:00:00 2001 From: liugddx Date: Tue, 18 Aug 2026 22:25:10 +0800 Subject: [PATCH 7/9] fix(windows-sandbox): reject a squatted pre-existing lock by owner The named-lock comment claimed a squatted Global\ name "fails closed at acquisition", but that only held for a squatter with a restrictive DACL: CreateMutexW ignores the supplied security descriptor when the name already exists, so a local user pre-creating the predictable mutex name with a *permissive* DACL would hand the launcher an attacker-owned arbitration object -- readiness and ACL-ledger operations would then block on a mutex the squatter can hold forever (availability DoS), while the code read as if the DACL had rejected them. Close the gap where it is checkable: when CreateMutexW reports ERROR_ALREADY_EXISTS (also the normal same-user contention path), read the existing object's owner via GetSecurityInfo before any wait and require it to be the current user or SYSTEM. Ownership is the one property a permissive squatter cannot forge -- re-owning an object to another SID requires SeTakeOwnership/SeRestore, which standard users do not hold. Any other owner fails closed with an explicit squatted-mutex error instead of blocking. The 10-way concurrency smoke (same-user contention) passes unchanged; the cross-user negative path cannot be exercised by an unprivileged CI runner and is enforced by the owner check itself. Co-Authored-By: Claude Fable 5 Generated-by: Claude Fable 5 --- .../launcher/src/acl_ledger.rs | 79 ++++++++++++++++--- .../launcher/src/windows_launcher.rs | 2 +- 2 files changed, 71 insertions(+), 10 deletions(-) diff --git a/experiments/windows-sandbox/launcher/src/acl_ledger.rs b/experiments/windows-sandbox/launcher/src/acl_ledger.rs index a78c8d0a38..3d092f04cb 100644 --- a/experiments/windows-sandbox/launcher/src/acl_ledger.rs +++ b/experiments/windows-sandbox/launcher/src/acl_ledger.rs @@ -10,14 +10,17 @@ use std::process::Command; use serde::{Deserialize, Serialize}; use sha2::{Digest, Sha256}; use windows_sys::Win32::Foundation::{ - CloseHandle, HANDLE, INVALID_HANDLE_VALUE, LocalFree, WAIT_ABANDONED, WAIT_OBJECT_0, - WAIT_TIMEOUT, + CloseHandle, ERROR_ALREADY_EXISTS, GetLastError, HANDLE, INVALID_HANDLE_VALUE, LocalFree, + WAIT_ABANDONED, WAIT_OBJECT_0, WAIT_TIMEOUT, }; use windows_sys::Win32::Security::Authorization::{ - ConvertStringSecurityDescriptorToSecurityDescriptorW, SDDL_REVISION_1, + ConvertStringSecurityDescriptorToSecurityDescriptorW, GetSecurityInfo, SDDL_REVISION_1, + SE_KERNEL_OBJECT, }; use windows_sys::Win32::Security::Isolation::DeleteAppContainerProfile; -use windows_sys::Win32::Security::{PSECURITY_DESCRIPTOR, SECURITY_ATTRIBUTES}; +use windows_sys::Win32::Security::{ + OWNER_SECURITY_INFORMATION, PSECURITY_DESCRIPTOR, SECURITY_ATTRIBUTES, +}; use windows_sys::Win32::Storage::FileSystem::{ BY_HANDLE_FILE_INFORMATION, CreateFileW, FILE_ATTRIBUTE_REPARSE_POINT, FILE_FLAG_BACKUP_SEMANTICS, FILE_FLAG_OPEN_REPARSE_POINT, FILE_SHARE_DELETE, FILE_SHARE_READ, @@ -27,7 +30,7 @@ use windows_sys::Win32::System::Threading::{CreateMutexW, ReleaseMutex, WaitForS use crate::broker_pipe_security::pipe_security_sddl; use crate::protocol::LaunchRequest; -use crate::windows_launcher::{appcontainer_profile_name, current_user_sid_string}; +use crate::windows_launcher::{appcontainer_profile_name, current_user_sid_string, sid_string}; pub(crate) const LEDGER_VERSION: u8 = 2; const ACL_MUTEX_TIMEOUT_MS: u32 = 30_000; @@ -39,7 +42,10 @@ const ACL_MUTEX_TIMEOUT_MS: u32 = 30_000; /// a live lease and would recover grants that are still in use. The names are /// scoped by the owning user's SID and the objects carry an explicit /// SYSTEM+user-only DACL, so another user can neither open them nor learn -/// anything from them; a squatted name fails closed at acquisition. +/// anything from them. A squatted name fails closed at acquisition: a +/// restrictive squat is rejected by its own DACL, and a permissive squat is +/// rejected by the pre-existing-owner check in `LedgerLock::try_acquire` +/// (the DACL we pass is ignored for an object that already exists). pub(crate) fn acl_mutex_name(user_sid: &str) -> String { format!(r"Global\Maka.WindowsSandbox.AclLedger.v2.{user_sid}") } @@ -232,9 +238,15 @@ impl LedgerLock { fn try_acquire(name: &str, user_sid: &str, timeout_ms: u32) -> Result, String> { // Machine-wide named objects are creatable by any local user, so the // lock is born with an explicit SYSTEM+owner-only DACL instead of the - // default token DACL. If another principal squatted the name first, - // opening it fails and the launch fails closed rather than sharing an - // arbitration object with an untrusted owner. + // default token DACL. That DACL only protects a mutex *we* created: + // `CreateMutexW` ignores the supplied security descriptor when the name + // already exists and simply opens the existing object. A restrictive + // squatter is rejected by its own DACL (open fails, we fail closed), + // but a *permissive* squatter would hand us an attacker-owned + // arbitration object we would then block on. So when the object + // pre-existed (`ERROR_ALREADY_EXISTS` — also the normal same-user + // contention path), the owner is verified to be the current user or + // SYSTEM before any wait; anything else fails closed. let sddl = pipe_security_sddl(user_sid) .map_err(|error| format!("invalid ACL lock owner SID: {error:?}"))?; let descriptor = lock_security_descriptor(&sddl)?; @@ -244,6 +256,8 @@ impl LedgerLock { attributes.bInheritHandle = 0; let name = wide(name); let handle = unsafe { CreateMutexW(&attributes, 0, name.as_ptr()) }; + // Read the last error before any other call can clobber it. + let already_exists = !handle.is_null() && unsafe { GetLastError() } == ERROR_ALREADY_EXISTS; let create_error = if handle.is_null() { Some(last_error("CreateMutexW(ACL ledger mutex)")) } else { @@ -253,6 +267,12 @@ impl LedgerLock { if let Some(error) = create_error { return Err(error); } + if already_exists { + if let Err(error) = validate_existing_lock_owner(handle, user_sid) { + unsafe { CloseHandle(handle) }; + return Err(error); + } + } let wait = unsafe { WaitForSingleObject(handle, timeout_ms) }; if wait == WAIT_OBJECT_0 || wait == WAIT_ABANDONED { return Ok(Some(Self { handle })); @@ -265,6 +285,47 @@ impl LedgerLock { } } +/// Owner check for a named lock that existed before this process created it. +/// The pre-existing object keeps whatever security descriptor its creator gave +/// it, so ownership is the one property a permissive squatter cannot forge: +/// re-owning an object to another SID requires SeTakeOwnership/SeRestore, which +/// standard users do not hold. The legitimate creators are another process of +/// the same user (normal contention) or SYSTEM; any other owner means the name +/// was squatted and arbitration must fail closed instead of blocking on an +/// attacker-held mutex. +fn validate_existing_lock_owner(handle: HANDLE, user_sid: &str) -> Result<(), String> { + const SYSTEM_SID: &str = "S-1-5-18"; + let mut owner: *mut std::ffi::c_void = std::ptr::null_mut(); + let mut descriptor: PSECURITY_DESCRIPTOR = std::ptr::null_mut(); + let status = unsafe { + GetSecurityInfo( + handle, + SE_KERNEL_OBJECT, + OWNER_SECURITY_INFORMATION, + &mut owner, + std::ptr::null_mut(), + std::ptr::null_mut(), + std::ptr::null_mut(), + &mut descriptor, + ) + }; + if status != 0 { + return Err(format!( + "GetSecurityInfo(pre-existing lock owner) failed with error {status}" + )); + } + let rendered = unsafe { sid_string(owner) }; + unsafe { LocalFree(descriptor as *mut std::ffi::c_void) }; + let rendered = rendered?; + if rendered.eq_ignore_ascii_case(user_sid) || rendered == SYSTEM_SID { + return Ok(()); + } + Err(format!( + "pre-existing lock is owned by {rendered}, not the current user or SYSTEM; \ + refusing to arbitrate on a squatted mutex" + )) +} + fn lock_security_descriptor(sddl: &str) -> Result { let mut descriptor: PSECURITY_DESCRIPTOR = std::ptr::null_mut(); let sddl_wide = wide(sddl); diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher.rs b/experiments/windows-sandbox/launcher/src/windows_launcher.rs index c4555c76d2..29b5b6756a 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher.rs @@ -459,7 +459,7 @@ pub fn current_user_sid_string() -> Result { } } -unsafe fn sid_string(sid: *mut c_void) -> Result { +pub(crate) unsafe fn sid_string(sid: *mut c_void) -> Result { let mut value = null_mut(); if unsafe { ConvertSidToStringSidW(sid, &mut value) } == 0 { return Err(last_error("ConvertSidToStringSidW(AppContainer)")); From 0df509bc3208430a125ebabbbcbcea9cced2e954 Mon Sep 17 00:00:00 2001 From: liugddx Date: Tue, 18 Aug 2026 23:20:29 +0800 Subject: [PATCH 8/9] fix(windows-sandbox): accept the token default-owner SID for pre-existing locks The squatted-lock owner check rejected locks owned by BUILTIN\Administrators (S-1-5-32-544), which broke on elevated hosts: an elevated administrator token stamps the Administrators group -- not the user SID -- as the default owner on objects it creates, so a mutex this very code created earlier on the elevated CI runner failed the {user, SYSTEM} check and the W0 protocol lane went red (unsettled_launch_preserves_grants_and_quarantines_the_ledger). Accept the process token's default-owner SID (TokenOwner) as a third legitimate owner. This stays inside the threat model: only an elevated administrator can create objects owned by Administrators, and RFC S1/S5 explicitly does not defend against administrators. Standard-user squatters are still rejected -- they cannot forge either the user SID or the Administrators owner. Adds a unit test pinning the helper to {user SID, S-1-5-32-544} so both elevation states stay covered. Co-Authored-By: Claude Fable 5 Generated-by: Claude Fable 5 --- .../launcher/src/acl_ledger.rs | 26 +++++++++--- .../launcher/src/windows_launcher.rs | 41 ++++++++++++++++++- .../launcher/src/windows_launcher_tests.rs | 18 +++++++- 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/experiments/windows-sandbox/launcher/src/acl_ledger.rs b/experiments/windows-sandbox/launcher/src/acl_ledger.rs index 3d092f04cb..8b28ee1ece 100644 --- a/experiments/windows-sandbox/launcher/src/acl_ledger.rs +++ b/experiments/windows-sandbox/launcher/src/acl_ledger.rs @@ -30,7 +30,9 @@ use windows_sys::Win32::System::Threading::{CreateMutexW, ReleaseMutex, WaitForS use crate::broker_pipe_security::pipe_security_sddl; use crate::protocol::LaunchRequest; -use crate::windows_launcher::{appcontainer_profile_name, current_user_sid_string, sid_string}; +use crate::windows_launcher::{ + appcontainer_profile_name, current_token_owner_sid_string, current_user_sid_string, sid_string, +}; pub(crate) const LEDGER_VERSION: u8 = 2; const ACL_MUTEX_TIMEOUT_MS: u32 = 30_000; @@ -290,9 +292,16 @@ impl LedgerLock { /// it, so ownership is the one property a permissive squatter cannot forge: /// re-owning an object to another SID requires SeTakeOwnership/SeRestore, which /// standard users do not hold. The legitimate creators are another process of -/// the same user (normal contention) or SYSTEM; any other owner means the name -/// was squatted and arbitration must fail closed instead of blocking on an -/// attacker-held mutex. +/// the same user (normal contention) or SYSTEM — where "the same user" must be +/// judged by the token's *default owner* SID, not only the user SID: an +/// elevated administrator's token stamps `BUILTIN\Administrators` as owner on +/// objects it creates, so a lock this very code created earlier on an elevated +/// host (e.g. a CI runner) is owned by S-1-5-32-544, and rejecting it would +/// deny our own locks. Accepting the token-owner SID stays inside the threat +/// model: whoever can create objects owned by Administrators is an elevated +/// administrator, which RFC §1/§5 explicitly does not defend against. Any +/// other owner means the name was squatted and arbitration must fail closed +/// instead of blocking on an attacker-held mutex. fn validate_existing_lock_owner(handle: HANDLE, user_sid: &str) -> Result<(), String> { const SYSTEM_SID: &str = "S-1-5-18"; let mut owner: *mut std::ffi::c_void = std::ptr::null_mut(); @@ -320,8 +329,15 @@ fn validate_existing_lock_owner(handle: HANDLE, user_sid: &str) -> Result<(), St if rendered.eq_ignore_ascii_case(user_sid) || rendered == SYSTEM_SID { return Ok(()); } + // Elevated hosts: our own creations are owned by the token's default-owner + // SID (typically BUILTIN\Administrators), not the user SID. + let token_owner = current_token_owner_sid_string()?; + if rendered.eq_ignore_ascii_case(&token_owner) { + return Ok(()); + } Err(format!( - "pre-existing lock is owned by {rendered}, not the current user or SYSTEM; \ + "pre-existing lock is owned by {rendered}, not the current user \ + ({user_sid}), this token's default owner ({token_owner}), or SYSTEM; \ refusing to arbitrate on a squatted mutex" )) } diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher.rs b/experiments/windows-sandbox/launcher/src/windows_launcher.rs index 29b5b6756a..f27766bd0e 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher.rs @@ -19,7 +19,8 @@ use windows_sys::Win32::Security::{ CreateRestrictedToken, DISABLE_MAX_PRIVILEGE, DuplicateTokenEx, EqualSid, FreeSid, GetTokenInformation, IsTokenRestricted, LUA_TOKEN, SECURITY_ATTRIBUTES, SECURITY_CAPABILITIES, SecurityImpersonation, TOKEN_ALL_ACCESS, TOKEN_APPCONTAINER_INFORMATION, TOKEN_DUPLICATE, - TOKEN_QUERY, TOKEN_USER, TokenAppContainerSid, TokenIsAppContainer, TokenPrimary, TokenUser, + TOKEN_OWNER, TOKEN_QUERY, TOKEN_USER, TokenAppContainerSid, TokenIsAppContainer, TokenOwner, + TokenPrimary, TokenUser, }; use windows_sys::Win32::Storage::FileSystem::{ CreateFileW, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_EXISTING, @@ -459,6 +460,44 @@ pub fn current_user_sid_string() -> Result { } } +/// The SID this process's token stamps as *owner* on objects it creates. For a +/// standard user this is the user SID, but for an elevated administrator the +/// token's default owner is typically the `BUILTIN\Administrators` group +/// (S-1-5-32-544) — so an object legitimately created by an earlier run of this +/// same code is NOT necessarily owned by the user SID. Lock-squat validation +/// must accept this SID or it rejects our own locks on elevated hosts (e.g. CI +/// runners). +pub(crate) fn current_token_owner_sid_string() -> Result { + unsafe { + let mut token = null_mut(); + if OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) == 0 { + return Err(last_error("OpenProcessToken(token owner SID)")); + } + let mut required = 0; + GetTokenInformation(token, TokenOwner, null_mut(), 0, &mut required); + if required == 0 { + CloseHandle(token); + return Err(last_error("GetTokenInformation(TokenOwner size)")); + } + let words = (required as usize).div_ceil(size_of::()); + let mut storage = vec![0usize; words]; + if GetTokenInformation( + token, + TokenOwner, + storage.as_mut_ptr() as *mut c_void, + required, + &mut required, + ) == 0 + { + CloseHandle(token); + return Err(last_error("GetTokenInformation(TokenOwner)")); + } + CloseHandle(token); + let owner = &*(storage.as_ptr() as *const TOKEN_OWNER); + sid_string(owner.Owner) + } +} + pub(crate) unsafe fn sid_string(sid: *mut c_void) -> Result { let mut value = null_mut(); if unsafe { ConvertSidToStringSidW(sid, &mut value) } == 0 { diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs index e5c12c0d0a..b1350d1ed0 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs @@ -5,7 +5,8 @@ mod tests { use crate::acl_ledger::readiness_mutex_name; use crate::protocol::{LaunchRequest, NetworkMode, RESERVED_READINESS_REQUEST_ID}; use crate::windows_launcher::{ - appcontainer_profile_name, appcontainer_readiness_profile_name, environment_block, + appcontainer_profile_name, appcontainer_readiness_profile_name, + current_token_owner_sid_string, current_user_sid_string, environment_block, readiness_probe_command_line, }; @@ -180,6 +181,21 @@ mod tests { assert!(!name.contains("AclLedger")); } + #[test] + fn token_owner_sid_is_a_valid_lock_owner_candidate() { + // The lock-squat check accepts {user SID, SYSTEM, token default owner}. + // The token owner is what our own creations are stamped with: the user + // SID for a standard user, BUILTIN\Administrators (S-1-5-32-544) for an + // elevated token — the CI runner case that must not self-reject. + let owner = current_token_owner_sid_string().expect("token owner SID"); + let user = current_user_sid_string().expect("user SID"); + assert!(owner.starts_with("S-1-5-"), "unexpected owner SID: {owner}"); + assert!( + owner.eq_ignore_ascii_case(&user) || owner == "S-1-5-32-544", + "token owner {owner} is neither the user SID {user} nor Administrators" + ); + } + #[test] fn readiness_command_line_disables_autorun_before_exit() { // `/d` must precede `/c` so cmd.exe skips the machine-constant From 02f2776de28e1d8c366f74b01b681d88f5d53fb3 Mon Sep 17 00:00:00 2001 From: liugddx Date: Wed, 19 Aug 2026 08:11:52 +0800 Subject: [PATCH 9/9] fix(windows-sandbox): pin an elevation-independent owner on named locks The owner check accepted {user SID, SYSTEM, this token's default owner}, but the default owner differs between elevation states: a mutex created by an *elevated* instance is owned by BUILTIN\Administrators, while a concurrent *non-elevated* instance of the same user resolves its own token owner to the user SID -- so it rejected the elevated instance's legitimate lock and readiness/ACL-ledger acquisition failed closed whenever elevated and unelevated Maka processes overlapped. Make the lock's identity elevation-independent at the source: the lock security descriptor now pins an explicit owner (O: -- always an assignable owner for the user's own token, elevated or not) ahead of the protected DACL, so every lock this code creates carries the same owner in every elevation state. Validation of a pre-existing lock accepts exactly {user SID, SYSTEM, BUILTIN\Administrators} -- the last for locks created by builds that predate the pinning during side-by-side overlap, and safe because only an elevated administrator (outside the RFC S1/S5 threat model) can create Administrators-owned objects. The per-token default-owner query is deleted. Adds a lock_sddl unit test asserting the pinned owner precedes the protected DACL. Co-Authored-By: Claude Fable 5 Generated-by: Claude Fable 5 --- .../launcher/src/acl_ledger.rs | 57 +++++++++++-------- .../launcher/src/acl_ledger_tests.rs | 24 ++++++++ .../launcher/src/windows_launcher.rs | 41 +------------ .../launcher/src/windows_launcher_tests.rs | 18 +----- 4 files changed, 58 insertions(+), 82 deletions(-) diff --git a/experiments/windows-sandbox/launcher/src/acl_ledger.rs b/experiments/windows-sandbox/launcher/src/acl_ledger.rs index 8b28ee1ece..5eacac7531 100644 --- a/experiments/windows-sandbox/launcher/src/acl_ledger.rs +++ b/experiments/windows-sandbox/launcher/src/acl_ledger.rs @@ -30,9 +30,7 @@ use windows_sys::Win32::System::Threading::{CreateMutexW, ReleaseMutex, WaitForS use crate::broker_pipe_security::pipe_security_sddl; use crate::protocol::LaunchRequest; -use crate::windows_launcher::{ - appcontainer_profile_name, current_token_owner_sid_string, current_user_sid_string, sid_string, -}; +use crate::windows_launcher::{appcontainer_profile_name, current_user_sid_string, sid_string}; pub(crate) const LEDGER_VERSION: u8 = 2; const ACL_MUTEX_TIMEOUT_MS: u32 = 30_000; @@ -249,8 +247,7 @@ impl LedgerLock { // pre-existed (`ERROR_ALREADY_EXISTS` — also the normal same-user // contention path), the owner is verified to be the current user or // SYSTEM before any wait; anything else fails closed. - let sddl = pipe_security_sddl(user_sid) - .map_err(|error| format!("invalid ACL lock owner SID: {error:?}"))?; + let sddl = lock_sddl(user_sid)?; let descriptor = lock_security_descriptor(&sddl)?; let mut attributes: SECURITY_ATTRIBUTES = unsafe { std::mem::zeroed() }; attributes.nLength = size_of::() as u32; @@ -287,23 +284,36 @@ impl LedgerLock { } } +/// Security descriptor for a named lock. Beyond the SYSTEM+user-only DACL, the +/// owner is pinned explicitly to the user SID: without `O:`, the object owner +/// comes from the creating token's *default* owner, which is the user SID for a +/// standard token but `BUILTIN\Administrators` for an elevated one — so the +/// same user's elevated and non-elevated processes would create locks with +/// different owners and each reject the other's legitimate lock. Pinning the +/// owner makes the lock's identity elevation-independent (the user SID is +/// always an assignable owner for the user's own token, elevated or not). +pub(crate) fn lock_sddl(user_sid: &str) -> Result { + let dacl = pipe_security_sddl(user_sid) + .map_err(|error| format!("invalid ACL lock owner SID: {error:?}"))?; + Ok(format!("O:{user_sid}{dacl}")) +} + /// Owner check for a named lock that existed before this process created it. /// The pre-existing object keeps whatever security descriptor its creator gave /// it, so ownership is the one property a permissive squatter cannot forge: /// re-owning an object to another SID requires SeTakeOwnership/SeRestore, which -/// standard users do not hold. The legitimate creators are another process of -/// the same user (normal contention) or SYSTEM — where "the same user" must be -/// judged by the token's *default owner* SID, not only the user SID: an -/// elevated administrator's token stamps `BUILTIN\Administrators` as owner on -/// objects it creates, so a lock this very code created earlier on an elevated -/// host (e.g. a CI runner) is owned by S-1-5-32-544, and rejecting it would -/// deny our own locks. Accepting the token-owner SID stays inside the threat -/// model: whoever can create objects owned by Administrators is an elevated -/// administrator, which RFC §1/§5 explicitly does not defend against. Any -/// other owner means the name was squatted and arbitration must fail closed -/// instead of blocking on an attacker-held mutex. +/// standard users do not hold. Legitimate owners are exactly three: the current +/// user SID (what `lock_sddl` pins at creation, in any elevation state), +/// SYSTEM, and `BUILTIN\Administrators` — the last because locks created by +/// builds that predate the owner pinning (or by other elevated tooling of this +/// user) carry the elevated token's default owner, and whoever can create +/// Administrators-owned objects is an elevated administrator, which RFC §1/§5 +/// explicitly does not defend against. Any other owner means the name was +/// squatted and arbitration must fail closed instead of blocking on an +/// attacker-held mutex. fn validate_existing_lock_owner(handle: HANDLE, user_sid: &str) -> Result<(), String> { const SYSTEM_SID: &str = "S-1-5-18"; + const ADMINISTRATORS_SID: &str = "S-1-5-32-544"; let mut owner: *mut std::ffi::c_void = std::ptr::null_mut(); let mut descriptor: PSECURITY_DESCRIPTOR = std::ptr::null_mut(); let status = unsafe { @@ -326,19 +336,16 @@ fn validate_existing_lock_owner(handle: HANDLE, user_sid: &str) -> Result<(), St let rendered = unsafe { sid_string(owner) }; unsafe { LocalFree(descriptor as *mut std::ffi::c_void) }; let rendered = rendered?; - if rendered.eq_ignore_ascii_case(user_sid) || rendered == SYSTEM_SID { - return Ok(()); - } - // Elevated hosts: our own creations are owned by the token's default-owner - // SID (typically BUILTIN\Administrators), not the user SID. - let token_owner = current_token_owner_sid_string()?; - if rendered.eq_ignore_ascii_case(&token_owner) { + if rendered.eq_ignore_ascii_case(user_sid) + || rendered == SYSTEM_SID + || rendered == ADMINISTRATORS_SID + { return Ok(()); } Err(format!( "pre-existing lock is owned by {rendered}, not the current user \ - ({user_sid}), this token's default owner ({token_owner}), or SYSTEM; \ - refusing to arbitrate on a squatted mutex" + ({user_sid}), SYSTEM, or Administrators; refusing to arbitrate on a \ + squatted mutex" )) } diff --git a/experiments/windows-sandbox/launcher/src/acl_ledger_tests.rs b/experiments/windows-sandbox/launcher/src/acl_ledger_tests.rs index 63ebdae5d4..c91d522394 100644 --- a/experiments/windows-sandbox/launcher/src/acl_ledger_tests.rs +++ b/experiments/windows-sandbox/launcher/src/acl_ledger_tests.rs @@ -520,4 +520,28 @@ mod tests { assert_eq!(root["readRecursive"], false); assert_eq!(root["writeRecursive"], true); } + + #[test] + fn lock_sddl_pins_an_elevation_independent_owner() { + // Without an explicit `O:` clause the object owner comes from the + // creating token's *default* owner — the user SID for a standard token + // but BUILTIN\Administrators for an elevated one — so the same user's + // elevated and non-elevated processes would create locks with different + // owners and each reject the other's legitimate lock. Pinning the owner + // keeps the lock identity stable across elevation states. + let sid = "S-1-5-21-11111111-22222222-33333333-1001"; + let sddl = crate::acl_ledger::lock_sddl(sid).expect("lock SDDL"); + assert!( + sddl.starts_with(&format!("O:{sid}D:P")), + "owner must be pinned before the protected DACL: {sddl}" + ); + assert!( + sddl.contains("(A;;GA;;;SY)"), + "SYSTEM retains access: {sddl}" + ); + assert!( + sddl.contains(&format!("(A;;GA;;;{sid})")), + "the owning user retains access: {sddl}" + ); + } } diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher.rs b/experiments/windows-sandbox/launcher/src/windows_launcher.rs index f27766bd0e..29b5b6756a 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher.rs @@ -19,8 +19,7 @@ use windows_sys::Win32::Security::{ CreateRestrictedToken, DISABLE_MAX_PRIVILEGE, DuplicateTokenEx, EqualSid, FreeSid, GetTokenInformation, IsTokenRestricted, LUA_TOKEN, SECURITY_ATTRIBUTES, SECURITY_CAPABILITIES, SecurityImpersonation, TOKEN_ALL_ACCESS, TOKEN_APPCONTAINER_INFORMATION, TOKEN_DUPLICATE, - TOKEN_OWNER, TOKEN_QUERY, TOKEN_USER, TokenAppContainerSid, TokenIsAppContainer, TokenOwner, - TokenPrimary, TokenUser, + TOKEN_QUERY, TOKEN_USER, TokenAppContainerSid, TokenIsAppContainer, TokenPrimary, TokenUser, }; use windows_sys::Win32::Storage::FileSystem::{ CreateFileW, FILE_SHARE_READ, FILE_SHARE_WRITE, OPEN_EXISTING, @@ -460,44 +459,6 @@ pub fn current_user_sid_string() -> Result { } } -/// The SID this process's token stamps as *owner* on objects it creates. For a -/// standard user this is the user SID, but for an elevated administrator the -/// token's default owner is typically the `BUILTIN\Administrators` group -/// (S-1-5-32-544) — so an object legitimately created by an earlier run of this -/// same code is NOT necessarily owned by the user SID. Lock-squat validation -/// must accept this SID or it rejects our own locks on elevated hosts (e.g. CI -/// runners). -pub(crate) fn current_token_owner_sid_string() -> Result { - unsafe { - let mut token = null_mut(); - if OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token) == 0 { - return Err(last_error("OpenProcessToken(token owner SID)")); - } - let mut required = 0; - GetTokenInformation(token, TokenOwner, null_mut(), 0, &mut required); - if required == 0 { - CloseHandle(token); - return Err(last_error("GetTokenInformation(TokenOwner size)")); - } - let words = (required as usize).div_ceil(size_of::()); - let mut storage = vec![0usize; words]; - if GetTokenInformation( - token, - TokenOwner, - storage.as_mut_ptr() as *mut c_void, - required, - &mut required, - ) == 0 - { - CloseHandle(token); - return Err(last_error("GetTokenInformation(TokenOwner)")); - } - CloseHandle(token); - let owner = &*(storage.as_ptr() as *const TOKEN_OWNER); - sid_string(owner.Owner) - } -} - pub(crate) unsafe fn sid_string(sid: *mut c_void) -> Result { let mut value = null_mut(); if unsafe { ConvertSidToStringSidW(sid, &mut value) } == 0 { diff --git a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs index b1350d1ed0..e5c12c0d0a 100644 --- a/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs +++ b/experiments/windows-sandbox/launcher/src/windows_launcher_tests.rs @@ -5,8 +5,7 @@ mod tests { use crate::acl_ledger::readiness_mutex_name; use crate::protocol::{LaunchRequest, NetworkMode, RESERVED_READINESS_REQUEST_ID}; use crate::windows_launcher::{ - appcontainer_profile_name, appcontainer_readiness_profile_name, - current_token_owner_sid_string, current_user_sid_string, environment_block, + appcontainer_profile_name, appcontainer_readiness_profile_name, environment_block, readiness_probe_command_line, }; @@ -181,21 +180,6 @@ mod tests { assert!(!name.contains("AclLedger")); } - #[test] - fn token_owner_sid_is_a_valid_lock_owner_candidate() { - // The lock-squat check accepts {user SID, SYSTEM, token default owner}. - // The token owner is what our own creations are stamped with: the user - // SID for a standard user, BUILTIN\Administrators (S-1-5-32-544) for an - // elevated token — the CI runner case that must not self-reject. - let owner = current_token_owner_sid_string().expect("token owner SID"); - let user = current_user_sid_string().expect("user SID"); - assert!(owner.starts_with("S-1-5-"), "unexpected owner SID: {owner}"); - assert!( - owner.eq_ignore_ascii_case(&user) || owner == "S-1-5-32-544", - "token owner {owner} is neither the user SID {user} nor Administrators" - ); - } - #[test] fn readiness_command_line_disables_autorun_before_exit() { // `/d` must precede `/c` so cmd.exe skips the machine-constant