Add a write capability for a single command, plus gate and audit seams - #43
Open
AntoniTok wants to merge 10 commits into
Open
Add a write capability for a single command, plus gate and audit seams#43AntoniTok wants to merge 10 commits into
AntoniTok wants to merge 10 commits into
Conversation
added 9 commits
August 3, 2026 14:17
Until now the only thing that could refuse a write was a registered read-only mount root, which is a property of the stored tree. Add a second dimension that is a property of the handle instead: build a `WorkspaceFilesystem` or `SQLiteWorkspaceProvider` with `writable: false` and every mutating method on it reports EROFS. The capability is fixed at construction and cannot be changed, which is the point. A command is not one write. `rm -rf` on a populated tree issues hundreds of calls, so a decision taken per write would let the first forty land before the forty-first is refused, leaving a half-deleted tree behind. A capability that holds for as long as the handle exists cannot produce that state. It lives on the handle rather than on the `Database` because two commands can be in flight against one workspace and need not agree about write access. `resolveCache` documents that exactly one `Database` wraps each `SqlStorage`, so a second handle is not available as a place to put this, and a flag on the shared one would let a read-only command disarm a writable command running beside it. Each check now sits at the layer that owns its state. The mount mode stays next to the tree, where the sync apply path still sees it. The capability sits on the handle's own methods, where the flag is immutable and cannot be flipped between the check and the commit. Four provider methods reached SQLite without consulting any guard: `chmodSync`, `openWriteBufferSync`, `openWriteBufferForCreateSync`, and `releaseWriteBufferSync`. Under mount-root enforcement alone they were narrow. A whole-handle capability makes them the obvious way out, since a command driven through a FUSE mount reaches all four, so they are guarded too. A read-only provider also refuses to open a file for writing at all, the way opening for write on a read-only filesystem fails at the open rather than at the first write. `WorkspaceFilesystem.writeFile` becomes async. It returns a promise, so a guard that threw synchronously would escape a caller using `.catch()` rather than arriving as a rejection.
`applyChanges` and `applyChangesSync` take a `writable` option. With `writable: false` nothing is applied and every entry comes back in `skipped` with reason `no-write-access`. This is the second half of running one command without write access. A backend that writes this store directly is stopped before anything commits, by a filesystem handle built without the capability. A backend that keeps its own copy of the files cannot be stopped that way: it has already written to its own copy by the time the changes arrive, so they stop on the way in instead. That is after the fact rather than preventive, and the two copies diverge as a result. The skipped entries are what tells the caller so, rather than hiding it. The capability is checked ahead of the idempotence skip, so a read-only apply reports every entry it was handed instead of quietly dropping the ones that happened to match local state. It is also checked ahead of the mount-root guard, so an entry both would refuse reports the missing capability, which is the answer the caller asked for. `SkippedEntry` becomes a union discriminated on `reason`. The two refusals carry different information: a read-only mount names the mount that owns the path, and a missing capability has no mount to name because the apply had no write access wherever the entry pointed. Code that read `mountRoot` unconditionally no longer compiles, which is the intent for a field that is now conditional.
`ShellRPC.exec` takes a `writable` flag and `pullOnce` takes a matching option that it forwards to `applyChanges`. The flag on `exec` is what a caller sets; the option on `pullOnce` is what enforces it for a runner that keeps its own copy of the files. That runner writes to its copy before we hear about the change, so the refusal happens as the change comes back rather than at the write. The interface comment says so plainly instead of implying the flag prevents the write everywhere. Two details that are easy to get wrong in the pull path: The object fetch is skipped without write access. `stageBlob` writes to the local store directly, so fetching bytes for entries about to be refused would write through the missing capability, and pay for the transfer first. The fetch cursor still advances over refused entries. Refusing is discarding, not deferring. Holding the cursor back would hand the same writes to the next pull that does have write access, which would make the refusal a delay rather than a denial. The stream is drained either way, so the caller gets a full account of what was refused rather than a bare count.
Two hooks around a workspace action: `gate.check(action)` runs before it and can refuse it, `audit.record(action, outcome)` runs after and records what happened. Both default to no-ops. `WorkspaceOptions` takes them as `gate` and `audit`. These are deliberately not folded into the existing observer. That contract says an implementation must return the callback's result unchanged so the wrapping is invisible — observability that changes behaviour is a bug. A gate is the opposite, so it gets its own seam with the same shape and the opposite licence, rather than the observer contract being weakened to fit. The two hooks differ in what they can do, matching what they are for. The gate decides, runs first, and sees only the request. The audit records, runs last, and sees the outcome. An audit hook cannot deny anything and its errors are swallowed, because by the time it runs the action has already happened; the alternative is a failed log entry failing the caller's work. A gate may also allow an action with write access withdrawn, which is the answer a policy wants for a command it will run but not trust. Narrowing only: a decision cannot hand out access the action never asked for, or a read-only exec would depend on the gate behaving. A gate that throws propagates rather than being read as a refusal. A gate that could not reach a decision is not a gate that said no, and a caller that cannot distinguish them will eventually treat an outage as permission. `Workspace.fs` is gated too, via a `WorkspaceFilesystem` subclass. That surface writes to the local store without crossing the wire, so gating only `shell.exec` would leave an obvious way around it: deny the command, write the file directly. Reads stay ungated. It is a subclass rather than a wrapper so the handle is still a WorkspaceFilesystem for the mount and think surfaces that take one by type. Filesystem mutations are gated per call, where each call is the whole action. Commands are gated once for the command, and gate.ts explains why: `rm -rf` is hundreds of calls, so refusing partway leaves a half-deleted tree, and there is no safe place to suspend a running command to ask a human.
`ExecOptions.writable` and `WorkspaceRuntimeExecOptions.writable`, defaulting to true. The flag reaches the runner over RPC and travels with the command's own post-command pull, and `Workspace.pull` takes a matching option. The pull is the part that is easy to leave out and useless to omit. The bracket around a command pulls whatever the command produced; a read-only command whose pull still had write access would have exactly its unauthorised changes applied a moment after it finished. The flag travels with the command rather than being read from configuration so that overlapping commands cannot borrow each other's access. `shell.exec` is also where the gate is consulted, before the pre-exec push, so a refused command moves no data either. The audit hook fires on the spawn rather than on the exit. exec() returns a detached handle that the caller may never drain, so there is no later moment guaranteed to arrive; picking one would mean a command that is dropped is never audited. What the command went on to do is already on the observer's span and on ExecResult. The routed shell now forwards the backend id it resolved instead of dropping the caller's selector. A gate deciding whether to trust a command with write access wants to know which backend runs it, because that determines whether a refused write is prevented or only reported.
This is where `writable: false` stops being a request and starts
preventing writes. The backend forwards the flag to the shell, and the
shell asks the host for a workspace stub built without write access
rather than checking the flag itself.
Asking for a stub it cannot write through is the point. Every route
into the workspace for that command — the shell's builtins, the git
command, anything a subclass registers — goes through that one handle,
so all of them are refused together. A check at the call site would
have covered that call site and nothing else, and would have needed
repeating for every path added later.
`Workspace.stub({ writable: false })` is the seam, backed by
`fsWithAccess`, which hands out a second filesystem handle over the
same store. Two handles over one database is deliberate: commands
overlap, and a read-only one must not be able to disarm a writable one
running beside it. The capability cannot live on the database for that
reason, and cannot be a second Database at all — dofs assumes exactly
one Database wraps each SqlStorage.
The stub narrows its runtime too. A read-only stub that could still
spawn a writing command would not be read-only.
The tests run real just-bash against a real Workspace, through the
same `workspace.stub(options)` call production makes. The one that
matters is `find /workspace -mindepth 1 -delete` under
`writable: false`: every file survives. Its control runs the same
command with access and confirms the tree does get deleted, so the
first test cannot pass by the command being broken. A third asserts
the failure reaches stderr as a read-only error, so neither can pass
because the stub failed to load.
No `denied` list on ExecResult. Here the refusal is already visible
where it happened — the write fails inside the command, which sees the
error and reports it — and for a backend with its own copy of the
files it arrives in `skipped`. A third channel would duplicate the one
and need new wire surface for the other.
`ExecToolOptions.writable` is a host-supplied function that decides
whether a command may modify the workspace. Omitting it leaves every
command writable, which is the behaviour without it.
It is not part of the input schema, and that is the whole design. The
model must not classify its own command: the case this feature exists
for is the command mislabelled as read-only, and asking the model that
mislabelled it to declare the label would produce a flag that agrees
with the mistake every time. The host decides from something it
already trusts — an allowlist, a plan step, a human — and the model
finds out the same way it finds out about any other failure, by the
write failing. A test asserts the field stays out of the schema, since
adding it there would look like a convenience.
The resolver is allowed to be wrong, and fails safe when it is. A read
command classified read-only runs normally. A write command classified
read-only fails where it writes instead of writing.
The effective access is reported on the tool result. Without it a
read-only run looks like an arbitrary failure and the model's next move
is to run the same command again.
A gate denial arrives as a thrown error and is returned as a tool
result like every other failure, so the model can read the refusal
rather than the agent loop tearing down.
`createAITools({ readonly: true })` is left alone. It drops the exec
tool even when a shell is configured, and a test passes `shell`
specifically to assert that, so the behaviour is a decision rather
than an oversight. Letting a readonly toolset run read-only commands
is worth proposing separately; it should not ride along here.
`ModuleExecutionInput.writable` narrows the capability handed to a module execution. Intersected with the backend's configured `access` rather than replacing it. A backend registered read-only stays read-only however the call was made, and an execution asking to be read-only gets that on a read-write backend. Neither side can widen the other, which is the same rule the gate follows. Like the shell backend, this one shares the host store, so the refusal lands where the write is attempted. The bridge answers the refused capability call with an error payload and the generated guest shim throws it inside the module, so the module sees an ordinary filesystem error rather than a silent no-op. The tests assert the file does not appear, with a control that runs the same module with access and confirms it does. A third pins the intersection by asking a read-only backend for a writable execution.
A new chapter for per-command write access, the gate consulted before an action, and the audit hook notified after it, plus the surface changes in the runtime and tool chapters and a section in the package README. The chapter leads with why classifying commands is not the thing being attempted. Correct classification is not achievable; making a wrong classification safe is. That framing is what explains the rest of the design, including why the capability is per command rather than per write and why a gate cannot ask a human once the command is running. The per-backend enforcement table is the part worth reading twice. What `writable: false` costs a command is not the same everywhere: backends sharing the host store refuse the write where it happens, and a container writes to its own copy first and has the change refused on the way back. The second is weaker and leaves the two copies disagreeing. Documenting that plainly is better than a sentence implying the flag prevents writes everywhere. The `EROFS` row in the filesystem chapter said no code path throws it. One does now, so it describes the two cases that reach it.
AntoniTok
marked this pull request as draft
August 3, 2026 14:35
commit: |
ExecToolOptions.writable is a function of ExecToolInput, and ExecToolOptions.workspace is an ExecWorkspaceLike, but neither type left the package. A host writing the resolver the previous commit added had no way to name its argument, which is the one thing it has to do to use the option at all. Found by writing that resolver outside the package for the first time.
This was referenced Aug 3, 2026
AntoniTok
marked this pull request as ready for review
August 3, 2026 17:17
koreydmyers
approved these changes
Aug 3, 2026
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
2 participants
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.
An agent decides a command only reads, and it is wrong. An alias, a shell function, a
&&it did not parse, a script that writes a lockfile on the way to printing a version. The workspace is modified, nothing reports it, and the mistake surfaces later as whatever breaks next.Classifying commands correctly is not solvable. What is solvable is making a wrong classification safe to hold, so this adds a per-command write capability. A command believed to only read runs without write access, and if the belief was wrong the write fails instead of landing.
Alongside it are the two hooks asked for in review: a
gateconsulted before an action, with the metadata to decide on it, and anaudithook told afterwards what happened. Both default to no-ops.These are shaped like the existing observer but are deliberately not part of it. That contract says an implementation must return its callback's result unchanged, because observability that changes behavior is a bug. A gate exists to change behavior. Rather than weaken the observer contract to fit, this is a second seam with the same shape and the opposite license.
A gate can also allow an action with write access withdrawn, which is the useful answer for a command a policy will run but not trust. Narrowing only: it cannot grant access the caller never asked for. A gate that throws propagates rather than reading as a refusal, because a gate that could not reach a decision is not a gate that said no, and code that cannot tell those apart will eventually treat an outage as permission.
Two things worth disagreeing with if you see them differently.
The gate covers mutating
workspace.fscalls as well asshell.exec, which is more than was asked for. The reason is thatworkspace.fswrites to the store without crossing the wire, so a gate over commands alone has an obvious way around it: deny the command, write the file directly. The cost is that this closes the direct filesystem surface for every caller, not just agents.And what
writable: falseactually buys depends on the backend, which is documented rather than smoothed over.worker-shellandworker-javascriptshare the workspace store, so the command holds a handle built without the capability and the first write fails where it happens. A container has its own copy of the files and has already written to it by the time the host hears about the change, so there the change is refused on arrival and reported inresult.skipped. That is weaker, and it leaves the container's copy disagreeing with the workspace, which is a reason to discard the container rather than keep using it.The capability is per command rather than per write because
rm -rfis hundreds of calls and refusing partway leaves a half-deleted tree. The same reasoning rules out prompting a human mid-command: once it is running there is nowhere to suspend it that does not risk a partial result. Filesystem calls are gated individually, where each call is the whole action.At the tool layer the resolver is supplied by the host and is deliberately absent from the schema the model fills in. The case being defended against is the command the model mislabeled, so a label from that same model would agree with the mistake every time.
To see it work, the clearest case is a command that tries to empty the workspace:
That case is a test, paired with a control that runs the same command with access and confirms the tree does get deleted, so it cannot pass by the command being broken, and a third that pins the failure to a read-only error so it cannot pass because the workspace handle failed to load. The same pairing covers the module backend. Elsewhere the tests cover two handles over one store disagreeing about access, which is what keeps overlapping commands from borrowing each other's rights, a read-only sync applying nothing and staging no bytes, and the write capability staying out of the tool schema.
docs/20_approval.mdis new and covers the capability, both hooks, and the per-backend table. The runtime, filesystem, and tool chapters pick up the surface changes, and theEROFSrow in the filesystem chapter no longer claims nothing throws it.One thing left alone on purpose:
createAITools({ readonly: true })still drops the exec tool even when a shell is configured. Letting a read-only toolset run read-only commands is now possible and looks worth doing, but an existing test asserts the current behavior deliberately, so it belongs in its own change rather than riding along here.