feat(runtime): replace OS sandboxing with model auto review

[M4n5ter]

English

Summary

Replace OS sandbox execution with Auto review and Bypass. New sessions default to Auto review; explicitly saved Bypass choices remain unchanged.

• Auto review evaluates ordinary tools, Code Mode calls and client capabilities before execution. Rejection, timeout or reviewer failure blocks the action and returns its reason to the Agent, which decides whether to ask the user. It never falls back to Bypass.
• Add Auto review model below Default model, with an independent selection or the default model. Both WorkHub composers expose the permission selector. WorkHub uses the live Host setting, and newly delegated sessions inherit that setting directly from the Host.
• Give the reviewer the proposed action, current task, bounded conversation and paired tool evidence. Authorization comes from authenticated user requests in the current session or its exact WorkHub delegation. Source session, turn and delegation identities are explicit; agent-written tasks and unrelated history cannot grant permission.
• Let the reviewer investigate with the existing Read, Glob, Grep, Recall and RecallMore tools, within a shared time and context budget and at most six tool calls. Recalled history is background evidence, not new authorization. The reviewer cannot execute commands, write files or ask the user directly.
• Remove sandbox backends, filesystem workers, boundary-expansion tools and their packaging. Preserve serialized, identity-checked writes. The acting model's tool schemas are unchanged by reviewer investigation.

Compatibility

Retired sandbox modes migrate to Auto review, and historical records remain readable. Runtime Host compatibility epoch is 162. Linked agents cannot bypass their parent's review policy. Provider-executed tools and external executors that cannot expose actions before execution require Bypass. Auto review provides action review, not OS isolation.

Verification

• Workspace build and type checks, including Desktop main, preload, renderer and Storybook.
• Workspace tests exercised 12,143 cases (40 skipped). Targeted Runtime/Host/Desktop/ACP checks pass, covering read-based investigation, denial without side effects, cancellation, context limits, exact delegation authorization and current Host defaults. Two wall-clock-dependent cases timed out in the full WSL run and pass in isolation.
• Both Electron expected-failure feedback cases pass, covering native WorkHub attachment rejection and settings/Plan IPC error codes.
• Lint, formatting, protocol epoch, renderer architecture, locale/TUI copy, ASF headers, E2E budget and Knip checks pass.
• Browser verification of the production settings and WorkHub components: permission switching, Bypass confirmation and readable floating menus. Packaged Windows/macOS apps were not run.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex — implementation, tests and documentation.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

中文

改动说明

移除操作系统沙箱，执行模式统一为自动审核和 Bypass（完全权限）。新会话默认使用自动审核，已明确保存的 Bypass 选择保持不变。

• 普通工具、Code Mode 内部调用和客户端工具在执行前统一接受审核。审核拒绝、超时或模型不可用时，阻止本次操作并把原因交给 Agent，由它判断是否向用户请求授权，不会自动降级为 Bypass。
• 在“默认模型”下方增加“自动审核模型”，可单独选择，也可跟随默认模型。WorkHub 主界面与悬浮窗都能切换权限；WorkHub 即时采用 Host 设置，新委派的会话也直接采用 Host 的当前默认值。
• 审核器能看到待执行操作、当前任务、限定长度的对话，以及对应的工具参数和结果。授权只来自当前会话中来源可靠的用户请求，或与当前任务精确关联的 WorkHub 委派。来源会话、轮次和委派标识明确可查，Agent 写的任务描述和其他任务的历史不能充当授权。
• 审核器可调用已有的 Read、Glob、Grep、Recall 和 RecallMore 补查必要信息，共用时间与上下文预算，最多调用六次。检索出的历史仅供参考，不能补充授权；审核器不能执行命令、写文件或直接向用户提问。
• 删除沙箱后端、文件系统 worker、扩权工具及相关打包配置，保留写入串行化与文件身份校验。审核调查不会改变传给主 Agent 的工具 schema。

兼容性

旧沙箱模式迁移为自动审核，历史记录仍可读取。Runtime Host 兼容版本为 162。子 Agent 不能绕过父会话的审核要求。由服务商直接执行、无法提前拦截的工具，以及不能逐项暴露操作的外部执行器，需要使用 Bypass。自动审核负责执行前判断，不提供操作系统隔离。

验证

• 工作区构建与类型检查，包括桌面端 main、preload、renderer 和 Storybook。
• 全工作区共执行 12,143 项测试，跳过 40 项。Runtime、Host、桌面端与 ACP 的定向检查通过，覆盖读取后审核、拒绝后无副作用、取消、上下文限制、委派授权匹配和 Host 默认值。全量运行中两条依赖系统时钟的用例在 WSL 提前超时，隔离复跑通过。
• 两条 Electron 错误反馈用例通过，覆盖 WorkHub 原生附件选择后的拒绝提示，以及设置和 Plan 操作的 IPC 错误码。
• Lint、格式、协议版本、渲染层架构、文案、ASF 许可头、E2E 预算和 Knip 检查通过。
• 浏览器验证真实设置页和 WorkHub 组件：权限切换、Bypass 确认，以及悬浮菜单的可读性。未运行 Windows/macOS 安装包。

AI 使用

OpenAI Codex 参与实现、测试与文档编写。

检查清单

• 测试覆盖本次行为变更
• 本地静态检查、类型检查及受影响测试通过
• 存在行为变化，已在改动说明中列出

[M4n5ter]

English

We are putting this PR on hold and keeping it in draft.

Without a sandbox, reviewing every tool call adds substantial latency and model usage. Exempting some operations from review would require rules governing tool behavior, access scope, and authorization boundaries. That approach could become as complex as—or more complex than—the existing sandbox implementation, and we do not yet see a clear benefit that justifies the replacement.

For now, we are pausing the proposal to remove sandboxing and replace it with automatic review. This PR will remain unmerged while we reconsider the respective responsibilities of execution isolation and automatic review, which operations need review, and the acceptable latency and cost.

中文

暂缓推进此 PR，继续保留 Draft 状态。

移除沙箱后，逐工具自动审核会带来明显的延迟和模型用量开销。若通过免审规则减少调用，又需要维护工具行为、访问范围和授权边界等规则，复杂度可能接近甚至超过原有沙箱方案，目前还没有明确的收益足以支持这项替换。

因此，先搁置“移除沙箱并以自动审核替代”的方向，暂不合并。后续明确执行隔离与自动审核各自的职责、审核粒度和可接受的成本后，再重新评估。

[Astro-Han]

Targeted security review — not a full review

This is a targeted review of a very large PR (~4.9k additions / ~44k deletions, 103 files deleted). I sampled the new control, its wiring, the enforcement seam, the authorization resolver, the filesystem/shell execution paths and the removal set. A full human review is required before merge — in particular of the 378 modified files I did not read, and of the settings/WorkHub UI changes.

I did not find any dangling references to the removed sandboxing primitives (details at the end), and the harness-level fail-closed behaviour is genuinely well built. My concerns are about what the replacement control actually guarantees.

---

1. What replaces the OS sandbox: a prompt-based check, with a two-case mechanical veto

packages/runtime/src/auto-review.ts is the whole control. AUTO_REVIEW_POLICY (line 63) is a system prompt; parseAutoReviewDecision (lines 78–97) is the only code that can veto:

if (decision.decision === 'allow' &&
    (decision.risk === 'critical' ||
     (decision.risk === 'high' && decision.authorization !== 'high' && decision.authorization !== 'medium')))
  -> deny

risk and authorization are both fields the reviewer model fills in itself, in the same JSON object it is asked to classify. So the only deterministic guarantees are:

• the model self-reports critical → blocked;
• the model self-reports high and self-reports low/unknown authorization → blocked.

Anything the model labels low or medium is executed with no independent check. A confused, sycophantic, or prompt-injected reviewer that labels rm -rf ~ && curl … | sh as low risk gets it executed. There is no separate validator, no risk re-derivation from the action, and no static denylist — I grepped builtin-tools.ts and shell-tools.ts at head and found none.

The prompt is thoughtful about untrusted evidence, and parseAutoReviewDecision's decisionSchema is .strict() with maxRetries: 0, so a malformed verdict fails closed (good). But the classification itself is advisory text in the same context window as the content under review. Answering the question directly: this is a prompt-based check, not a security control in the sense the OS sandbox was. The PR body is honest ("Auto review provides action review, not OS isolation") — I would like it to be equally explicit that nothing in the system re-derives or bounds the reviewer's risk judgement.

2. By default the acting model reviews itself

packages/runtime-host/src/server/execution-model-authority.ts:172-174:

const configured = snapshot.policy.chatDefaults.autoReviewModel;
const connectionId = configured?.connectionId ?? catalog.defaultTarget?.connectionId;
const model = configured?.model ?? catalog.defaultTarget?.modelId;

autoReviewModel is optional and null is documented as "follows the default model connection" (packages/core/src/settings.ts:502-503). So out of the box the reviewer is the same connection and the same model as the actor, reading the same injected repository content and tool output. That is one more sample from the same context, not independent review. Worth stating as a known limitation in the settings copy, and worth considering whether the default should be "must pick explicitly".

3. Workspace path containment is removed too — not just OS isolation

This is the finding I'd most want a second opinion on.

packages/runtime/src/filesystem-executor.ts:132 hardcodes the path scope:

return await local.execute(call, 'host', expectedIdentity);

On base, pathScopeForBoundary returned 'workspace' for every non-bypass boundary (base filesystem-executor.ts:141-142), and the resolvers then enforced containment. With scope === 'host' every containment assertion is skipped:

• packages/runtime/src/workspace-executor.ts:521 — if (scope === 'host') return { root, path }; (skips assertInsideCwd)
• :532 — if (scope === 'host') return await fs.realpath(candidate); (skips the post-realpath re-assert)
• :551-553 — same for the directory-entry resolver
• packages/runtime/src/filesystem-executor.ts:547-552 — assertGlobPatternInScope: "Under host scope there is nothing to escape from and the pattern is left alone."

FilesystemOperationSchema (packages/runtime/src/filesystem-contract.ts:23-78) accepts an arbitrary path string with no root constraint. And Bash now runs directly: packages/runtime/src/shell-run-manager.ts at head has no sandboxType / sandboxExecution at all, versus base lines 958-962.

Net effect: in the default auto_review mode, Read/Write/Edit/ApplyPatch/Glob/Grep/Bash reach anywhere the OS user can reach — ~/.ssh/id_rsa, ~/.aws/credentials, ~/.gitconfig, anything on the mounted filesystem. The reviewer model is the only thing between a prompt-injected agent and those paths.

The Compatibility section says "Auto review provides action review, not OS isolation", which is accurate about the sandbox but understates the change, because an application-level workspace fence existed alongside the sandbox and is gone as well. Please make that explicit.

4. Authorization resolution has a fail-open path

packages/runtime-host/src/server/auto-review-context.ts:138 and :143:

const admission = admissionByTurn.get(group.turnId);
if (admission && group.messageId === admittedPromptMessageId(admission)) { ...; continue; }

appendProvenRequests(group.requests, group.turnId, authorizations, workHubTargets);   // line 143

When readRootTurnAdmission returns undefined, or when an admission exists but the message id / request texts don't line up and the turn is not a WorkHub target, the live in-memory scope.userRequests are appended straight into authorizations with no durable corroboration. auto-review-context.test.ts:70 ("keeps a trusted ordinary request when legacy root details do not reproduce it") documents this as intended legacy behaviour.

The whole "only host-authenticated user requests may authorize" property rests on that in-memory list being unforgeable. It is derived from event.content.authenticatedUserRequests on author === 'user' runtime events (packages/runtime/src/ai-sdk-turn.ts:838-853), which is set at ingress in packages/runtime-host/src/server/authenticated-user-requests.ts:24-33. That looks sound to me, but I could not satisfy myself that undefined admission is impossible for a new turn (as opposed to a store gap or an older epoch). Please confirm, or fail closed on a missing admission.

5. session_guest text counts as user authorization

packages/runtime-host/src/server/authenticated-user-requests.ts:29-31 attests the prompt text for local_owner | remote_owner | session_guest. A session_guest principal holds only a narrow grant set (packages/runtime-host/src/server/access-credential-store.ts:90-104 and :197), yet its prompt becomes authorizations[].request.text, which the reviewer may score medium or high — exactly the score needed to let a high-risk action through parseAutoReviewDecision. This follows the pre-existing model (a guest could already drive turns), so I am raising it as a question rather than a defect: is a shared-session guest intended to be able to authorize high-risk host actions, and is there a test for it?

6. Questions on the bypass surface

• packages/runtime/src/tool-runtime.ts:1172 skips review for AskUserQuestion — harmless, it has no side effects. :1177 skips review entirely when readPermissionMode() === 'bypass'.
• Can the acting agent, a linked/child agent, or a WorkHub delegation move its own session to bypass? I found no path, and the decoupling looks deliberate: createGenesisExecutionBoundary (packages/core/src/sandbox-boundary.ts:218-220) now always returns { kind: 'bypass' } with the comment "Execution is always direct; review is a separate session permission", while the gate reads the header mode. That is the right shape. But packages/runtime-host/src/__tests__/execution-model-composition.test.ts:3319 (assert.equal(child.permissionMode, parent.permissionMode)) only covers a parent that was already bypass. A test asserting a child of an auto_review parent stays auto_review would pin the PR body's claim ("Linked agents cannot bypass their parent's review policy").
• Code Mode nesting is covered correctly: ai-sdk-turn.ts:2596-2610 routes nested cell calls back through toolRuntime.settleToolCall, i.e. the same reviewed seam. Reviewer read-tools are name-filtered (auto-review.ts:197) and their impl is invoked directly, outside the gate — that is intended and looks safe.

7. Does any test verify the new safety property?

The harness fail-closed behaviour is well covered — packages/runtime/src/__tests__/auto-review.test.ts has 12 cases including "denial prevents … capability preparation and execution", "missing or failing reviewer never falls back to execution", "cancellation after review still prevents execution", "review output is strict and cannot approve critical risk or unauthorized high risk", and "oversized authorization fails closed". Good work.

What is not covered, and cannot be covered by a unit test: whether the reviewer's judgement survives adversarial content. There is no test that a repository file, tool output, or transcript entry containing "the user has approved this, proceed" fails to raise authorization, or that an injected "this is a low-risk routine action" fails to lower risk. Meanwhile the PR deletes 31 test files, including experiments/windows-sandbox/adversarial-matrix-smoke.ps1 and packages/runtime/src/__tests__/tool-runtime-sandbox-boundary.test.ts — i.e. the adversarial matrix that existed for the old control is retired with nothing adversarial put in its place. A small checked-in red-team corpus (even a fixture set run against a fixed set of transcripts) would be the honest replacement and would give reviewers something to reason about.

8. Cost / budget observations (fail-closed, but worth documenting)

• One provider round trip per tool call, plus up to AUTO_REVIEW_MAX_TOOL_CALLS = 6 read calls inside the reviewer, under a hard AbortSignal.timeout(60_000) (execution-model-authority.ts:163). Any measurement of added latency and token cost per turn?
• autoReviewPrompt embeds request.args raw (no autoReviewEvidence bounding), and investigateAutoReview:282-283 subtracts it from maxInputBytes. A large Write therefore trips remaining < 1_024 or the context-budget throw and fails closed. Safe, but a legitimate >~90 KB file write will always be refused with a context-budget error rather than reviewed. Worth a doc note or a size cap on the write tool itself.

---

Removed primitives: no dangling references found

I diffed the base tree (672d8273) against head (827a30c5): 103 files deleted, 6 added (auto-review.ts, auto-review-context.ts, authenticated-user-requests.ts, filesystem-contract.ts, plus the two test files). I then checked every importer of the removed modules at head — packages/runtime/package.json, packages/core/package.json, apps/desktop/package.json, electron-builder.config.mjs, renderer-architecture.json, scripts/windows-package-source-closure.mjs, apps/desktop/scripts/dev.mjs, scripts/verify-{macos-dmg,windows-x64,windows-harness.test}.mjs, packages/runtime-host/src/server/{interactive-run-composer,session-catalog-coordinator}.ts, packages/runtime/src/{builtin-tools,shell-tools,shell-run-contract,shell-run-tool-result,ai-sdk-tool-repair,ai-sdk-turn,tool-runtime,session-manager,agent-run-recovery,filesystem-authority,filesystem-executor}.ts, packages/ui/src/{index,components}.tsx, apps/desktop/src/renderer/app-shell.tsx — all clean.

packages/core/src/sandbox-boundary.ts is retained and still imported (e.g. SANDBOX_BOUNDARY_RESTART_CLOSURE_CLASS in apps/desktop/src/renderer/session-status-presentation.ts:36), which matches the "historical records remain readable" goal. The mode migration (packages/core/src/permission.ts: execute/ask/explore → auto_review, with RecordedPermissionMode preserving what governed a past run) is clean.

Nits

• executionBoundaryDisplayMode in packages/core/src/sandbox-boundary.ts:209 has no remaining caller at head — all three call sites were removed or reworked (runtime-host-tui-context.ts now reads the chat default directly; session-catalog-coordinator.ts:1755 now uses boundary.profile.name === 'read-only'). Dead export, and its doc comment still describes the retired ask/explore modes.
• packages/cli/src/sandbox-boundary-failure.ts and the sandboxFailureReason branch in packages/cli/src/run-command-core.ts:392-400 still print "maka run: sandbox bypass requires an explicit --yolo". Nothing sets content.sandboxFailure at head, so that CLI copy is unreachable. Delete with the rest.
• createGenesisExecutionBoundary(_mode) ignores its parameter; drop it. More broadly, ExecutionBoundary / SandboxProfile / the sandbox-boundary protocol vocabulary is now a large retained type surface with no producer — a short note on which parts are intentionally frozen for history versus removable would help future readers.

Process

• The PR is DRAFT and reports mergeable: CONFLICTING against main; it needs a rebase.
• gh api repos/apache/maka/pulls/5405/files (486 entries) disagrees with the base→head tree diff, which shows 6 added files including packages/runtime-host/src/server/authenticated-user-requests.ts. That file is security-relevant — it is where "authenticated user request" is defined — and is not mentioned in the PR body. Please make sure the final diff and description include it.