refactor(runtime): make the execution boundary the sole file-path authority - #2087
Merged
Merged
Conversation
…hority The built-in file tools each carried the same worker-versus-executor branch, and the fallback executor hard-coded a session-cwd containment that no permission profile declares. Managed sessions never saw it because they route through the sandboxed worker; a bypass boundary skips the worker, so the undeclared rule became the only arbiter and made "full access" stricter than ask mode, which grants :slash_tmp outright — while Bash under the same boundary wrote anywhere on the host. Introduce one boundary-driven FilesystemExecutor: managed goes to the worker, bypass runs host-local with host path scope, external and a missing boundary stay workspace-scoped. Path containment becomes a scope parameter the caller derives from the boundary rather than a policy each executor invents, so the isolated headless adapter can reject host scope instead of silently degrading. The six file tools now hold one call each, and the glob pattern check moves next to the scope it enforces. Closes #2083
Review of the first commit found the concurrency test proved nothing: one write(2) per file is already atomic, so its "the survivor is whole" assertion held with no lock at all, and the lock key it named was never asserted. Fixing the test exposed the real seam problem. The tools were taking the lock themselves, so each write tool canonicalised the cwd once for the key and the executor canonicalised it again for the operation — two snapshots of one decision, and lock plumbing in a layer that should only name an operation. The lock now lives with the authority that resolves the path: execute() holds it for the mutating operations and nothing else knows a lock exists. The test now instruments the backend and asserts no overlap plus a sequenced outcome, so removing the lock fails it; a second test pins the key across spellings directly. Also covers Grep under bypass, bypass with a worker wired, worker image decoding, and the adapter's other host-scope entry point. Drops the unused filesystem injection option, un-exports the internal scope helper, stops FormatJson leaking the backend discriminator into its result, and rewords the Glob and FormatJson descriptions that still promised a containment the boundary now decides. Refs #2083
Astro-Han
force-pushed
the
refactor/filesystem-authority-single-boundary
branch
from
August 4, 2026 06:23
4317fc5 to
49cc1e4
Compare
Astro-Han
marked this pull request as ready for review
August 4, 2026 06:29
sunheyi6
added a commit
to sunheyi6/maka-agent
that referenced
this pull request
Aug 4, 2026
) - builtin-tools: restore filesystemWorkerForExecution (removed by apache#2087) for the ApplyPatch adapter; import withFileWriteLock; pass workspace scope to resolveExistingPath - filesystem-executor: host-local backend handles the ApplyPatch lstat/delete protocol extensions so the discriminated switch stays exhaustive - biome format pass
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The built-in file tools each carried the same worker-versus-executor branch, and the fallback executor hard-coded a session-cwd containment that no permission profile declares. Managed sessions never met that rule because they route through the sandboxed worker; a bypass boundary skips the worker, so the undeclared rule became the only arbiter — and it is stricter than every declared profile. "Full access" therefore rejected
Writeto/tmp, whichaskmode allows outright via:slash_tmp, whileBashunder that same bypass boundary wrote anywhere on the host.This makes the
ExecutionBoundarythe only authority over where the file tools may reach:FilesystemExecutor(packages/runtime/src/filesystem-executor.ts) composes the backends behind a single decision —managed→ the sandboxed worker,bypass→ host-local execution with host path scope,externaland a missing boundary → workspace-scoped local execution, never a silent host fallback.scopeparameter the caller derives from the boundary instead of a policy each executor invents. The isolated headless adapter keeps its own containment as a transport invariant and now rejects host scope explicitly rather than degrading.execute()holds it for mutating operations, so the cwd is canonicalised once per call and no tool knows a lock exists.filesystemWorkerForExecutionand the duplicated branch in all six file tools are gone; each tool holds one call. The glob pattern check moved next to the scope it enforces, and theWrite,GlobandFormatJsondescriptions no longer promise a containment the boundary now decides.Closes #2083
Verification
packages/runtime/src/__tests__/filesystem-authority.test.ts(11 cases) drives the tools against a real filesystem across boundaries: bypass reaches outside the cwd (Write/Read/Edit/FormatJson/Glob/Grep) and bypasses a wired worker, every other boundary does not, symlink escapes stay rejected under workspace scope, absolute glob patterns are refused unless bypass, managed routes to the worker and never to the host backend, managed without a worker failsrequires_bypass, worker image bytes arrive decoded, and one file takes one write lock however its path is spelled.d3b0bc8(including the reported symptom), and the 3 that encode unchanged behavior pass — so the suite tracks the defect rather than the implementation.withFileWriteLockremoved it fails. Its first version did not — a plain concurrent-write race passes without any lock, because onewrite(2)per file is already atomic — so it now instruments the backend for overlap and asserts a sequenced outcome, with the key equality pinned separately.@maka/runtime3108 passed / 0 failed,@maka/headless1339/0,@maka/runtime-host628/0,maka-agent468/0,@maka/core772/0.npm run typecheck,npm run lint,npm run format:checkall clean.Review focus
The behavior change is deliberate and user-facing in one direction: under "full access" the file tools now reach the whole host, matching what
Bashalready does in that mode and whatdanger-full-accessdeclares. A missing boundary stays workspace-scoped, so embedders that never opted into a boundary are unaffected.One accepted drift: under a managed boundary an absolute or
..Glob pattern is now refused by the worker's own check ("must stay inside its search root") instead of the deleted tool-level message ("must stay inside session cwd"). The rejection is unchanged; only the wording is, and the worker's is the more accurate of the two.