* test(tui): pin the cwd in tests that assert text is absent from the view
Nine tests built their model with an empty Options.Cwd, so newModel took
the package directory and the title bar showed the developer's real
branch and checkout path. Each then asserted that some text was absent
from the whole view, and failed wherever that branch or path happened to
contain it: TestSuggestionOverlayRenders on a branch with /model in its
name, TestSuggestionOverlayCapsRowsWithoutMoreText on a branch or path
containing "more" or "compact", the provider wizard tests on "offline",
"chatgpt" or "gpt-4.1", and so on.
They now pin Options.Cwd to isolatedCwd, a path that does not exist, so
no branch is read and the path is the same on every machine. A guard
test checks that isolatedCwd stays outside any git checkout.
* test(tui): make isolatedCwd fully qualified and absent by construction
The fixed /workspace/zero path could exist on some machine, a dev
container for one, and newModel would then read whatever is there. On
Windows it was also only drive-relative: \workspace\zero is not a fully
qualified path.
isolatedCwd is now a child of the home TestMain isolates that nothing
creates, so it is fully qualified, absent and outside any checkout on
every platform, and the title bar shows ~/not-a-checkout everywhere. A
t.TempDir path would have put the user name and the test name into the
view. The guard test checks all three properties.
* test(tui): clear the branch in view-test models instead of trusting the cwd
isolatedCwd is a child of the isolated home, which lives under the
temporary directory. With TMPDIR inside a checkout, gitBranch walks up
from that absent path to the checkout and puts its branch in the title
bar, so the guard failed on an ordinary branch and a colliding branch
broke the nine view tests again.
newViewTestModel now builds those models: it sets the isolated cwd and
clears the branch after construction, since no path can promise there
is no checkout above it. The guard keeps only what a path can promise
(fully qualified and absent), and a new test puts the home inside a
checkout on feat/model-picker and checks that neither that branch nor
the real checkout path reaches the view.
* test(tui): assert no checkout path reaches the view in the enclosing-checkout test
The test checked the branch strings and that the isolated cwd shows,
but a view could show both the isolated cwd and a real checkout path
without failing it. It now also asserts that neither the enclosing
checkout nor the directory the tests run from appears in the view.
The variable turns off both the models.dev fetch and the cached overlay,
and four tests in modelsdev_test.go inherited it from the shell. With it
set, TestRefreshModelsDevCacheRejectsBadBodyWithoutClobbering and
TestDefaultModelEntriesAppliesFreshCache fail, and
TestCachedModelsDevProvidersIgnoresStaleCache and
TestCachedModelsDevProvidersRequiresOptIn pass without reaching the
check they are named for: the nil they expect comes from the variable.
Clear it in each of them, as the two sibling refresh tests already do.
GHSA-5r4p-p66f-jhc7 (moderate): hono's serveStatic decodes the request
path a second time, so middleware on static paths can be bypassed.
Versions 4.12.4 to 4.13.10 are affected and main pins 4.13.9 through
tuistory, which release packaging installs as a local-control helper.
npm update takes hono to 4.13.13, inside every range that asks for it.
package.json is unchanged and nothing else in the lock moves.
* fix(oauth): use the crash-safe secret store and sync token publications
The encrypted-file OAuth backend carried its own AES-GCM secret implementation that had drifted from internal/securefile: a crash during first login left <secret>.lock behind and every later Save timed out instead of reclaiming it, and the secret and token blobs were published without fsync.\n\nUse securefile.Crypter with an opt-in publication directory so the secret still publishes through the sandbox-denied .secret.publish directory, add file and directory barriers to the encrypted token blob, and extend securefile with the publication directory plus destination-directory sync.\n\nMove the concurrent-creation and wrong-sized-secret coverage to securefile, add a stale-lock regression through the OAuth store, and test publication-dir use, sync ordering, sync-failure preservation, and retry after a failed secret sync. The stale-lock test fails on the previous code.
* test(oauth): cover post-rename directory-sync failures
The existing failure tests inject a file sync error before the rename. Add coverage for a destination-directory barrier failure after the token or secret has been atomically replaced: the write must report the error and the newly published bytes must remain readable. Also cover the corresponding securefile secret path, including its expected size.
* fix(securefile): never clobber a newer lock when restoring a reclaimed one
reclaimStaleLock could rename a lock that had just been replaced, then restore the quarantined copy with os.Rename, replacing a third holder's new lock. Both holders could then publish different secrets for the same file. Restore with O_CREATE|O_EXCL and discard the quarantined copy when the destination is already held.
Extend the directory-sync failure test to prove a later caller actually loads the published secret, not just its size. Add restore tests for the free and already-held cases.
govulncheck now reports GO-2026-6809 (crypto/x509 distrusts compromised
intermediate certificates) against Go 1.26.9, reachable through
daemon.WriteFrame, so Security & Code Health fails on main and on every
open PR. It is fixed in 1.26.10.
CI and release builds read the toolchain from go.mod. The stated
source-build minimum in README.md, README_ZH.md and docs/INSTALL.md
moves with it.
runExec starts a background models.dev download unless
ZERO_DISABLE_MODELS_FETCH is set. TestRunExecImageAttachmentHistory moved
the user directories into a TempDir by hand but left the fetch on, so a
5.7 MB download landed in the TempDir while its cleanup ran, and the
macOS runner failed with "unlinkat ... directory not empty". The two
older image tests had no isolation and fetched into the real user cache.
Use isolateCLIUserState in all three, which also points the cache path
into the fixture and turns the fetch off.
* fix(tools): grep stops where a file's line breaks stop
scanGrepFile treated EOF after a trailing line break - and an empty file -
as the START of another line, and handed the matcher an empty line one
number past the real content. Every pattern that matches an empty string
then reported a match on a line no reader sees:
a.txt holds "x\n"; grep pattern "^$"
-> a.txt:2: (grep -c '' GNU-style reports no such line)
same file, pattern ".*", count mode
-> "2 matches found" (one real line, one phantom)
an empty file, pattern "^$"
-> a.txt:1: (the file has no lines at all)
read_file and the file tracker's countLines already state the rule the
scan violated: "the trailing newline does not open a line", and an empty
file has no lines. The scan now stops there too, so anchored and
empty-matching patterns can only match the lines the content carries.
Real matches, numbering, count-per-hit accounting for non-empty patterns,
file mode, glob and symlink handling are unchanged.
* test(tools): make the grep line-number parity test catch the phantom
The parity test matched only "needle", a pattern that cannot match the
empty line the old EOF branch invented, so its absence-of-line-4
assertion passed on the code being fixed. The pattern now also matches an
empty line, and the expected output is asserted exactly: on the pre-fix
scanner the same run reported
grep = "a.txt:2: needle two\na.txt:4: ",
want exactly the line read_file numbers 2
so the phantom is visible right beside the real line, and the test is
about the numbering claim it states. Also gofmt-aligns the case table
that made the CI formatting check fail.
Two credentials glued end to end share one recovering property: the second
one's trigger prefix is where redaction can re-anchor. For fixed-length
bodies that anchor is exact (AKIA{16} stops at the next key), which is the
case the anchored end-chaining always covered. For unbounded bodies it
isn't: github's [A-Za-z0-9]{36,} body eats the literal "ghp" of the token
glued behind it, so the first match ends PAST the second trigger, the scan
that re-anchors at the span end finds nothing there, and the second
credential leaves the output as +_bbbb... / .eyJzdWIi... plaintext:
RedactString("ghp_aaa..36a..ghp_bbb..36b..")
-> "[REDACTED]_bbbb......" (second token's body printed in full)
RedactString(jwtA+jwtB)
-> "[REDACTED].eyJzdWIiOiI5ODc2...SA" (second token's payload+sig)
The same holds across shapes (a github key directly feeding "sk-ant-..."
through its body) and for three tokens in one run.
redactAdjacent now follows the run a match starts: at each boundary it
re-binds the match at the EARLIEST trigger occurrence inside the span that
stills validates as its shape (the head before the trigger must match the
anchored pattern exactly, and the run chained from the trigger must reach
past the span, so a mid-body lookalike and a mid-span cut that recovers
nothing leave the span alone), and continues under whichever shape matched
the swallowed trigger. Candidate examination is capped at
maxTriggerOccurrences per span so body spam of trigger lookalikes costs
what the span costs, and the first validating cut wins.
End-chaining for fixed-length bodies, single-credential shapes, custom
replacement templates, and the split path's announced no-op-for-contiguous
text guarantee are all unchanged; the regression tests pin the glued pairs
that previously leaked plus the AWS chaining and lone-token cases.
The paragraph describing publishAdapterLaunch appeared twice: once above
launchedOnce, where it described the wrong closure, and once above
publishAdapterLaunch itself. Keep the second.
#1149 was opened before #1105 moved the module to github.com/Twigpine/zero
and merged right after it, so its two imports of internal/gitsafe still
used the old path. Nothing provides github.com/Gitlawb/zero any more, so
main stopped building at 2ffe1831.
* fix(sessions): preserve image attachment summaries on resume
Persist TUI-compatible attachment counts for ACP and exec user events.
Restore readable markers in live history, session replay and resumed
prompts without storing or resending historical image data.
Cover image-only and mixed prompts, restart recovery, malformed metadata
and the aborted-turn boundary with regression tests.
Fixes#1135
* fix(workspaceindex): follow the current module path
Use github.com/Twigpine/zero/internal/gitsafe in the ignore scanner and its
tests. The upstream module rename preceded the scanner's gitsafe change,
leaving these two imports on the old path and preventing the rebased tree
from compiling.
* fix(sessions): count only images accepted by the model
Take ACP and exec attachment counts after the vision capability gate,
matching the TUI's persisted attachment semantics. Update the ignored-image
regressions to require zero saved images and no historical image marker.
Refs #1135
main moved the module to github.com/Twigpine/zero in #1105. Rename this
branch's imports to match, including the two in internal/workspaceindex
that main itself still carries until #1153 lands.
* fix(workspaceindex): run Scan's git calls through gitsafe
Scan asks git for ignored paths on every turn with its own gitCommand, which
turned off core.fsmonitor and dropped the repository-location variables but
still let a clean filter or diff driver from the repository's own config run
during status. It now goes through gitsafe, like the TUI and the scratch-file
warning: hooks are off as well, and a repository whose config defines a filter
or diff command takes the non-git path. The status call also passes
--ignore-submodules=dirty.
Scan makes two git calls, so gitsafe gains Check, which reads the repository
config once and builds several commands from it. Command is now Check plus one
command, and each command gets its own copy of the environment.
* test(workspaceindex): pin Scan's fallback when gitsafe refuses the repository
A repository whose own config defines a filter or diff command is refused by
gitsafe, and Scan then takes the non-git path. Scan reaches gitsafe.Check
through a package variable now, so a test can stand in for such a repository
without configuring one that names a command, and
TestScanFallsBackWhenGitsafeRefusesTheRepository checks both sides: with git
consulted, .gitignore decides and the fixed list does not apply; refused, the
fixed list applies and .gitignore is not read.
Update the Go module path, imports, install scripts, docs, workflows,
packaging and action defaults from Gitlawb/zero to Twigpine/zero. The npm
scope (already @twigpine/zero on main, with its @gitlawb/zero migration
notes), LICENSE, action author, the OpenGateway provider and historical
CHANGELOG links are unchanged.
The X-AIMLAPI-Integration-Repo header keeps Gitlawb/zero: it is what
AIMLAPI sees for attribution, so changing it needs a maintainer's yes.
docs/INSTALL.md keeps `go install github.com/Gitlawb/zero/...`, because
every tagged release so far declares that module path. The Homebrew
formula comment names the Twigpine/homebrew-tap tap.
Co-authored-by: Claude Opus 5.5 <[email protected]>
* feat(sandbox): support self-exec pattern to eliminate separate helper binary
Linux sandboxing previously required distributing a separate 'zero-linux-sandbox'
binary and looking it up on PATH.
This wires an internal '__sandbox-helper' entrypoint directly in cmd/zero/main.go
and configures findLinuxSandboxHelperCommand to fallback to os.Executable(),
enabling single-binary zero distributions with fully functional sandboxing.
* fix(sandbox): prefer PATH helper before self-exec fallback
Colocated zero-linux-sandbox stays first. A standalone helper on PATH
is used next. Self-exec of the main zero binary is only the last
fallback, so an installed helper is not ignored.
* feat(sandbox): clean formatting in main.go and add self-exec unit tests
* feat(sandbox): insert __sandbox-helper verb in bwrap plan for self-exec binary
* fix(sandbox): drive Linux selection from the self-exec helper resolver
Availability and command-plan construction now share the same helper
resolution and keep the __sandbox-helper prefix on the backend, so a
main-only install with bwrap reaches native sandboxing.
* fix(sandbox): self-exec only the zero main binary, not test binaries
* fix(sandbox): carry standalone vs self-exec identity into the inner bwrap plan
A versioned physical helper (zero-linux-sandbox-v1) must not receive
__sandbox-helper. The inner plan uses an explicit SelfExec flag, threaded
from argv, instead of guessing from filepath.Base.
* docs(install): describe self-exec as the last helper fallback
Self-execution runs only when neither a colocated nor a PATH
zero-linux-sandbox is available.
* docs(install): state colocated, PATH, then self-exec helper order
---------
Co-authored-by: cl-ment <[email protected]>
An open, empty stream blocks Wait forever, which is the defect these tests
pin, so against the old Close they hung until the test binary timed out
instead of failing. Wait now runs behind a bound that reports the stream
open and closes it so the waiting goroutine returns.
A beforeTool hook that vetoed while carrying an enforcement notice put the
notice on two channels: Dispatch appended it to DispatchOutcome.Notices and
blockReason folded it into Reason as well. The one consumer made up for it
by dropping every notice whose text appeared in the reason, which also
dropped an earlier hook's disclosure whenever the blocking hook's output
quoted it. Reason now carries the hook's words only, and the veto result
takes every notice from the typed slice, the same owner hookMessage already
defers to.
Runtime.Close read the disclosure stream field directly. That raced a first
StartupDisclosureStream call, and a Close that came before any call closed
nothing, so a later caller got a fresh, open stream subscribed to launches
from a runtime that was already gone. Close now publishes through the same
Once, as an empty stream when nobody had asked for one, and closes it.
Tests: the launch-state and veto tests assert the typed slice and that the
reason carries no copy; the veto result is checked for the blocker's own
notice and for an earlier notice the reason quotes; a command that launched
and failed keeps its disclosure; the never-launched MCP check reads the
carried notices instead of the error text, which never included them.
The ACP agent spelled its own tool-result payload and wrote result.Output.
After the output/notice split that field is the undecorated text, and the
payload had no notices field, so the enforcement disclosure was dropped at
the moment of persisting. The live tool_call_update for the same result
showed it, and session/load then replayed the sandboxed command as if
nothing had constrained it. Fixing the live translation could not reach
this: the text was already gone on disk.
The payload builder moves from internal/tui to internal/agent, beside
ToolResult, because it now has three writers and only one of them is a
terminal. The TUI keeps its unexported helpers as thin delegates, exec calls
the agent function directly, and ACP persists through the same function. A
TUI resume of an ACP-written session therefore gets the typed notices and the
undecorated card body like any other.
The ACP reader restores the disclosure as what it was, typed notices beside
undecorated text, so toolCallResult composes it exactly once. The stored
output already has the notices composed in, so tools gains the inverse of
WithEnforcementNotices next to it; text that was never decorated passes
through unchanged and still gets its notice. displayPreview is deliberately
not read there: it is a card body, and the live ACP update sends the model
text.
The regression runs a real turn through the production OnToolResult
callback, loads the session in a fresh agent, and requires the replayed
update to equal the live one with the notice exactly once, including for a
command that printed nothing. Reported by jatmn.
The previous version used an unsupported sandbox level, which is refused in the
level switch, so moving the join down to just above that switch still counted
one call and the test passed. A DenyRead profile on a restricted-token tier is
refused before the switch, so a join below any check now reads as not having
happened.
Two problems with the ownership this branch added, both mine.
The handle was closed in a defer. This process is a MEMBER of the job it
creates, so closing the last handle to it is the kill: the defer fired during
runWindowsSandboxCommand's own return and terminated the helper before its exit
code could be handed back. Every sandboxed command reported 0 however it really
ended, refusals included. Measured three ways in one process: no job exits 42,
join and hold exits 42, join and close exits 0. The handle is now held for the
process lifetime, which is what the guarantee was built on in the first place:
process exit is what releases it.
The regression tested it from inside the process that owns it. It called
joinWindowsChildKillJob in the test binary and then closed the handle, so it
terminated the test binary. `go test` printed "ok" and exited 0 with the
assertions never reached, and the 51 tests that had not run yet were silently
dropped: 315 of them completed where skipping this one test gave 366. Everything
is driven out of process now, through the test binary re-executed as a stand-in
for the command helper, parked in the create-to-resume window with a real
suspended child on the real production join.
On top of that the coverage review asked for:
- cancellation and deadline expiry both driven through execution.Runner's
captured path over the sandbox Engine, the wiring hooks and plugins use, with
the call required to return bounded AND the child required to be gone
- the control without the job, which reproduces the orphan and the blocked
caller, so the two above cannot pass for an unrelated reason
- the exit code of the production runner, and that it takes ownership before it
refuses a command
- cleanup that owns the child independently of every assertion
Reported by @jatmn.
Keeps the typed enforcement notices and the Base* accessors this branch
introduced, and carries main's FileDiffs from #999 alongside them in both
ToolResult constructions; the ACP translation keeps composing the disclosure
through ModelOutput while building main's content slice.
Creating the child suspended is what makes the launch report trustworthy: the
fact is published before the child can execute, so an absent report means
nothing ran. It also creates a state the helper's ordinary unwind does not
cover. Between CreateProcessAsUser and ResumeThread the child exists, holds the
inherited stdin/stdout/stderr handles, and can run nothing.
Hook and plugin commands reach the helper through Engine.CommandContext, which
uses the default exec.CommandContext cancellation, so a timeout or a cancel
kills the helper outright. It never reaches terminateSuspendedWindowsChild, its
defers, or the resume. Windows does not terminate a child because its parent
died, so the suspended process survives holding the pipe write ends and a
parent still reading them waits on a process that will never write and never
exit.
The helper now joins a job object with JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE
before any child can exist, so every child created afterwards inherits
membership at creation and there is no window where one is alive and unowned.
Terminating the helper closes the last handle and the kernel does the killing,
which is the only mechanism that survives the helper being killed rather than
unwound. A failure to create the job is not fatal: it costs the guarantee, not
the sandbox.
Deliberately not fixed with exec.Cmd.WaitDelay in the parent. That releases the
caller while the orphan keeps running and keeps the pipes, trading a visible
hang for an invisible leak; it is a reasonable backstop, not the fix.
The regression drives the real kernel: join the job, create a suspended child,
close the last handle the way a terminated helper would, and require the child
to be gone. With a plain job instead, the child outlives it.
Reported by @jatmn.
Since #1006 a Windows profile with denyRead is rejected before setup or token creation on both restricted-token tiers, so there is no run left for the notice to describe. Keeping it meant a plan could carry a note saying reads are denied and writes unconfined while the runner refused to start that plan at all: two statements about one fact, disagreeing.
Removed: windowsDenyReadWarnings and its host indirection, denyReadDiagnosticWarnings and the append into the execution request warnings, the plan-notes append in the command-plan funnel, and the two predicates that only served it (windowsRestrictedTokenWillRun, willBuildWindowsRestrictedToken). The three test files that pinned the producer go with it.
CommandPlan.Notes now has no producer in the tree. The channel from plan notes into Enforcement.Notices stays, since it is what hooks, plugins and MCP read, and the end-to-end pin that reached it through the real producer is replaced by a fixture-plan test so deleting that one line still fails somewhere. The comment in loop.go that justified skipping the rebudget on the strength of that one producer now says so.
Closes the last part of #869, which #1006 settled by refusal.
The unwind test asserted the report was removed on a resume failure, which
is the contract this branch just replaced: absence is what a normal cleanup
leaves too, so it does not revoke a launch a live reader already saw. It
now asserts an explicit false, with a companion for the case where the
retraction write itself fails and the file is discarded after all.
The report is published before ResumeThread, so the fact is readable while
the child has executed nothing. A live poll landing in that window latched
it, and the latch outlived the file: the terminal read finds the report
cleaned away and restores what was observed, so a child that never became
runnable was reported as launched and its write-jail trade disclosed as if
it had been made.
Deleting the report on that path cannot fix it, because deletion is also
what a normal cleanup does and the restore exists for exactly that. The
helper now retracts with an explicit false, which is the one answer that
outranks a latch, and the manager repeats the observation while the command
runs instead of caching it. Silence still changes nothing, so a genuine
launch still survives its own cleanup.
The report is published before ResumeThread so the inherited-pipe race is
closed, and that ordering stays. But a failure between the publish and the
resume reaped a process that had executed nothing while leaving a report on disk
saying a child launched. AppliedEnforcementNotices gates on that report and
ResolveChildLaunched treats it as authoritative, so the operator would have been
told a write-jail trade applied to a child that never became runnable.
The comment above the publish already claimed the stronger invariant, that every
failure between creation and resume leaves "no child launched" true for the
parent. The code held it only for failures before the write.
publishThenResume now owns the sequence: the published flag it returns is true
only when the child actually resumed, so a resume failure hands the deferred
close a false and the report is removed. The docstring on the terminate helper
now distinguishes the pre-publish path, where nothing was written, from the
post-publish pre-resume path, where the record has to be taken back.
Reported by jatmn.
beforeTool was moved onto the typed EnforcementNotices slice and afterTool was
left folding its notices into the prose feedback, so the same fact had two
writers on the normal tool tail: the typed slice, which every surface composes
through ModelOutput and HumanDisplay, and the hook feedback block appended to the
body. Both carry the identical fixed deny_read string, so a hook running under
the same token shape as the tool it follows made the model see the disclosure
twice, and a bash or exec card show it in the amber furniture and again in the
body. Neither half existed on the merge base; this branch introduced both.
hookMessage now returns the hook's own stdout or stderr and nothing else.
dispatchAfterTool returns its notices alongside that output, and the loop merges
them into the result through the same finalization beforeTool uses, with the same
order and dedupe. A veto's Reason still carries its notice inline, because that
field is prose that reaches a person on its own.
Ordinary afterTool validator output is untouched: a formatter diff or vet warning
still arrives as feedback in the body, which is what an afterTool hook asked for.
A silent hook that exists only to disclose a token trade now reaches the typed
slice instead of becoming model input.
The hooks tests that asserted the disclosure through hookMessage now assert it on
Notices and additionally that it does NOT ride along in Messages, so the property
they protected is unchanged and its carrier moved with it.
Reported by jatmn.
A successful beforeTool hook's disclosure was folded into result.Output as hook
prose. That reached the provider, which reads the output, and reached nothing
else.
Every interactive surface builds its enforcement furniture from the typed
EnforcementNotices slice, and for an edit or a write the card renders
Display.Preview instead of Output. So on exactly the results where something was
written, the operator saw the diff and no disclosure at all, collapsed and
expanded, and the session payload persisted that same omission for the restored
card. The model was told the token had been weakened and the person was not.
The notice now merges into the typed slice at one finalization point that the
normal path, the hook veto and the retry denial all return through, so no return
path can reopen the loss. Ordering is the hook's disclosures ahead of the tool's
own, exact repeats dropped, so a surface rendering the slice shows each one once.
Nothing is written into Output as well: decoration has one owner per surface, or
the disclosure appears twice. ModelOutput, HumanDisplay, the card renderer and
session serialization keep the ownership they already had.
Hook notices are third-party text on a path that bypasses the registry's
redaction boundary, so the merge scrubs them the way appendHookFeedback did while
they travelled as prose, and reports it so Redacted stays accurate.
The joiner that folded notices in with afterTool feedback is gone with its last
caller. afterTool output was never enforcement data and still arrives as prose.
Reported by jatmn.
Three positive-path tests skipped when the manager came back with an unwrapped
plan. Nothing about those fixtures is an environment prerequisite: they pin GOOS,
an available native backend, command wrapping and an explicit executable, so an
unwrapped result is not a machine that cannot run the test, it is the producer
whose disclosure they exist to check having gone away. CI would have reported all
of the deny_read disclosure coverage passing after the real route disappeared,
and the hand-built predicate tests would have stayed green with it because they
start from CommandPlan{Wrapped: true}.
They now fail through one shared assertion that names the wrapping state, the
target backend and the enforcement level.
The fixtures were also still reading the real machine in one place.
windowsSandboxInitialized stats the per-host setup marker and BuildCommandPlan
consults it, so a box where `zero sandbox setup` has run resolved these requests
at native enforcement while every CI runner got unelevated. The plan came out
wrapped either way, which is why nothing failed, but the level being asserted
against was whichever machine ran it. Both fixtures now pin it.
Reported by jatmn.
The gate is redundant today: every route that reaches StartupNotices has already
settled the decision positive through the handshake confirmation, so removing it
left every test passing. Unfalsifiable code is a stop signal, so it is pinned
directly rather than left as an unchecked claim.
It stays because the redundancy is on the safe side. An edit that moves or loses
the confirmation makes this return nothing rather than announce a confinement on
the strength of the helper having started.
connectAndList reads client.StartupNotices() on the success path and hands the
result straight to the disclosure sources, with no launch check anywhere in
between. For a wrapped plan those notices were recorded because cmd.Start
returned, which is the HELPER starting: the one thing the report exists because
it does not prove. The answer happened to be right, since a completed handshake
does imply the child ran, but by coincidence rather than by rule, while the
failure path beside it was already asking the adapter.
Found by falsifying: dropping the handshake confirmation left every test passing,
because the success disclosure never consulted the decision at all.
StartupNotices now goes through the same decision as the sink and the error, so
one rule covers all three carriers.
The ordering test also asserts the failure disclosure travels through the ERROR,
not only the sink. Registration merges the two, so a sink-only assertion passed
while the decision was taken before the adapter settled and the failure the
operator reads carried nothing.
ChildLaunchTracker gets its own tests, because the rule that a negative read is
not remembered is invisible through the MCP paths: they each ask once, at a point
where the answer is already terminal. Pinned where the rule lives instead.
The cross-process launch report has three states, not two: not settled yet,
settled with no child, and child created. An absent file, the empty file the
adapter opens before launching, a half-written one and a decode error are all the
first state. Collapsing them to "no child" is a claim about a process that may be
running at that instant, and every consumer was making that collapse on its own
and compensating for the timing separately, which is why fixing one presentation
site kept exposing the next.
Three changes, one contract.
The Windows helper now creates the sandboxed child with CREATE_SUSPENDED and
resumes it only after the report is published. The child inherits the MCP pipes,
so created runnable it could answer initialize, emit a malformed response or close
stdout before the helper was next scheduled, and a parent reading the report then
saw the empty file and cached "no child" about a server already running
unconfined. It also closes the second manifestation: publishing could fail AFTER a
runnable child had begun making external changes, and reaping it afterwards does
not undo the work it did. A suspended child has executed nothing, so every failure
between creation and resume terminates a process that never ran and the missing
report is then true.
ChildLaunchTracker is the settlement rule, at the adapter boundary where every
consumer can share it. It caches a launch and never an absence: a negative read
stays provisional until the answer is terminal. Two things make it terminal.
Confirm, when the consumer observed the child itself, and Settle, when the adapter
process has exited. Settle runs from Cleanup, before the report file is deleted,
so reading the evidence and destroying it are one step rather than a race the
consumer has to win.
The MCP client uses both. A successful initialize response is Confirm: the adapter
speaks no MCP and the child is created suspended, so a well-formed response can
only have come from the requested server. On the failing exit the decision now
comes after Close instead of before it, because Close waits out the adapter and
cleanup settles with the report still on disk. Asking first asked an adapter that
may be between creating the child and recording it, and the connect timeout ends
the attempt exactly there.
The existing fixture writes the finished report during PrepareExecution, before
the helper command starts, so every ordering above is over before the parent
looks. The new tests drive a real helper process that publishes at a controlled
point: after answering the handshake, after failing it, and not at all.
For an adapter-owned launch, cmd.Start proves only that the sandbox helper
started. It can then fail setup-marker validation, ACL application, network
validation, token construction, or CreateProcessAsUser without ever creating
the requested MCP server. The launch sink made that distinction; the
initialize-error path did not, and carried the planned notices out
unconditionally. The operator was told a server had run without write
confinement when no server had run at all.
The adapter-gated path was also dead on that same route. client.Close runs the
plan cleanup, which deletes the report file, and it ran before
publishAdapterLaunch read it, so a wrapped server that really did launch and
then failed its handshake published nothing either. Two competing definitions
of applied enforcement, and both were wrong in opposite directions.
connectStdio now resolves the launch fact once, through
execution.ResolveChildLaunched, memoized, and reads it before Close so the
evidence still exists. The sink and the error carrier consume that one answer.
The success path is unchanged: a completed handshake is independent proof the
child existed, and gating it on a mis-written report would suppress a true
disclosure. Direct stdio and bwrap are unaffected, since an unwrapped plan
resolves as launched exactly as before.
Regressions drive RegisterTools with no ClientFactory, so the real connectStdio
runs, and with a preparer whose cleanup deletes the report the way the Windows
plan does. A helper reporting no child, and one that wrote no report at all,
disclose nothing while still being recorded as skipped; a helper that reports
the child discloses exactly once.
The Windows helper publishes childLaunched immediately after
CreateProcessAsUser creates the restricted child, and only then waits for it.
ProcessManager read that report exclusively in the post-Wait goroutine, so for
the entire live lifetime of a retained wrapped session the report was the zero
value. The first exec_command reply and every write_stdin poll resolved
Launched=false and disclosed nothing, while the fact sat readable on disk. A
watcher, or a retained session nobody polls to completion, would never be told
the write jail had been traded away.
The launch fact is a monotonic lifecycle transition, not terminal process data.
managedProcess now observes it once, latches it, and hands it out on live
results as well as the terminal one. markDone will not overwrite a latched
launch with a terminal read, because the plan's cleanup has already removed the
report file by then on some orderings.
Only the positive is promoted. An absent, partial, or undecodable report, and a
helper that failed before it created the child, all leave the live result
exactly as before: not confirmed, nothing disclosed. Latching false or
surfacing a read error from the live read would let a mid-flight poll rewrite a
running command into a setup failure. Direct commands and bwrap are untouched:
the read is gated on the plan being adapter-owned, so an unwrapped plan does no
extra work.
Regressions drive the real ProcessManager with a real child that publishes the
way the helper does and then stays alive: a live start and a live poll both
disclose, a helper that reported no child stays silent, and a report published
mid-flight is observed on the next poll.
Delivering DispatchOutcome.Messages for a successful beforeTool hook fixed the
silent disclosure and overshot. hookMessage builds that slice by folding the
enforcement notice together with the hook's ordinary stdout, or stderr when
stdout is empty, because afterTool validators want both. So every successful
hook's routine logging, large diagnostics, and whatever text the hook happened
to process became a standing input channel into the next model request. main is
silent for a successful hook.
DispatchOutcome now carries Notices separately: only the disclosures, one entry
per notice, accumulated across every hook that ran. Messages keeps its old
meaning and its old consumer.
The notices are also accumulated BEFORE the veto short-circuit, because they
describe something that already happened. Dispatch stops at the first veto, so
a hook that ran under the weakened token ahead of the vetoing one used to leave
its disclosure in the audit record and nowhere else. Two exits after the hook
runs now share one finalization helper with the normal tail: the veto result,
and a denied, cancelled or ungrantable unsandboxed retry. The blocking hook's
own notices are already inside its reason, so they are not repeated.
The regression that expected `go version` stdout to reach the provider was
locking in the wrong behaviour and is replaced. One hook run now emits both a
notice and ordinary output, and the test asserts the notice arrives exactly
once while the output stays silent, driven through Run and asserted on what the
provider received. A second test covers the veto path. Falsified by delivering
Messages again, by returning the veto result without the notices, and by
dropping the accumulation ahead of the veto.
Smoke (windows-latest) failed lint on cc85f2ee: SA1019, runtime.GOROOT has been
deprecated since Go 1.24. The fallback was copied from an older test and was
never needed here, since the test cannot run without a go binary anyway. Skipping
when one is not on PATH is the honest answer.
executeToolCall inspected the beforeTool DispatchOutcome only when Blocked was
true, so a hook that ran fine and produced output put it in the audit record and
on no surface anyone could see. A beforeTool process that ran under the weakened
DenyRead token said so to nobody; only vetoes and afterTool feedback reached the
model.
Its messages now ride out on the tool result, the same delivery afterTool
feedback already uses, ahead of that feedback and without displacing it. Blank
messages contribute nothing, so a run with no hook output stays silent rather
than appending an empty header, and veto behaviour is untouched.
The regression drives the real Run loop with a real hook process and asserts on
what the provider received on the next turn; dropping the capture fails it on
that assertion. A unit test on the joining helper passed with the capture
removed, so it could not have caught this.
sessionStart and sessionEnd still discard their outcomes. Routing those needs a
delivery surface that does not exist in agent.Options today, and choosing one is
a product decision rather than a mechanical fix, so it is raised on the PR
instead of invented here.
Joining the pump at stop bounded writes to the pump's lifetime but said nothing
about the overlap. Headless and interactive startup keep writing plugin, trust,
peer, provider, trace and validation output to the same caller-supplied writer
for the whole time the pump is live, and Run accepts an arbitrary io.Writer: a
plain bytes.Buffer corrupts under concurrent use, and even a concurrency-safe
terminal writer interleaves logical lines. A mutex private to the pump could not
fix that, because the foreground writes do not go through it.
reportMCPStartupDisclosures now returns a guarded view of the caller's writer
alongside stop, and both startup paths adopt it for the rest of startup, so the
pump and the foreground path take the same lock. Machine-readable stdout is
untouched and the stop-before-TUI boundary is unchanged.
The regression is deterministic rather than hopeful: a writer that parks inside
Write holds the foreground message there while the late launch resolves and the
pump tries to print, and it counts concurrent entries. Removing the lock from the
guarded writer fails it on that count; the test also asserts stop drains the
final notice exactly once and that the foreground message is not lost.
The adapter-owned child-launch contract was interpreted only by
Runner.ExecuteCaptured. bash, exec_command/ProcessManager and durable stdio MCP
each copied a subset of the prepared state and decided launch independently, so
the same false disclosure survived in every path the earlier fix did not touch:
a Windows helper that starts and then fails during marker, ACL, network, SID,
token or CreateProcessAsUser setup still promoted the planned DenyRead notice
even though no restricted child ever existed.
ResolveChildLaunched is now the single answer: the adapter's report wins in both
directions, an adapter that owns the fact and stays silent means not launched,
and anything else keeps the caller's own observation, which is correct for a
direct command and for bwrap. The captured runner, bash, and the exec_command
conversion all call it. ProcessManager carries the ownership bit into
ProcessResult so the retained write_stdin shape has it too, where the helper can
be returned before it has even attempted the inner launch. connectStdio keeps
prepared.Report and the ownership bit and no longer publishes at cmd.Start for a
wrapped plan; it publishes once the adapter confirms, on both ways the attempt
can end, which is also where an attempt abandoned at the connect timeout lands.
Separately, the helper no longer drops ownership of a child it created. The
report file is claimed BEFORE CreateProcessAsUser, so a failure to obtain the
side channel happens while there is still nothing to own, and a failure to
publish afterwards terminates and reaps the child instead of returning while it
runs with nobody waiting on it and the parent free to start a second one. A
report that was never published is removed, so a truncated file cannot be read
back as a launch.
The regression drives the production conversion rather than the shared helper:
asserting the launch again fails it on three property assertions naming the
notice it wrongly disclosed.
For a Windows restricted-token plan the command the runner starts is the sandbox
helper, not the requested executable, so exec.Cmd.Process becomes non-nil as soon
as the ordinary helper process starts. Setup-marker validation, unelevated ACL
application, network-policy validation, capability and offline SID construction,
restricted-token creation and CreateProcessAsUser all happen inside that helper
and can each return without creating the requested process. On those paths
AppliedEnforcementNotices still reported that reads were denied as requested,
when the only thing that ran was the unsandboxed adapter.
The fact belongs to whoever sees the transition. AdapterReport gains
ChildLaunched, which the helper writes to the adapter-owned report file at the
moment CreateProcessAsUser succeeds, and the runner believes over its own
observation. A plan whose adapter owns the fact is marked, and silence from that
adapter now means not launched rather than falling back to the wrapper's start,
so a missing report cannot be read as proof that enforcement applied. The mark is
per adapter, not plan.Wrapped: a bwrap plan is wrapped too and reports only
denials, and treating its silence as "no child" would drop the disclosure from
every successful Linux sandbox run.
Direct, unwrapped commands are unchanged, since the process the runner starts is
the requested one. The report file is created with O_EXCL under the per-user temp
directory, so a name another local user pre-created makes the helper fail rather
than supply the fact the parent reads back.
The regression drives both sides: a helper that runs and never creates the child
discloses nothing, an adapter that owns the fact and says nothing discloses
nothing, a restricted child that starts and then exits non-zero discloses exactly
once, and a direct command keeps its own observation. Ignoring the reported fact,
or removing the fail-closed branch, fails it on the notice it wrongly disclosed.
ReportStartupDisclosures took the CLI's presentation function and invoked it
from whichever goroutine resolved the launch. For a server still inside
cmd.Start when registration gave up, that is the abandoned connect goroutine,
so the write to stderr happened off the output owner's goroutine and could
arrive after runExec had returned or after Bubble Tea had taken the alt screen.
The regression added with it demonstrated the bug rather than catching it: it
raced bytes.Buffer.String under -race.
The runtime owns the fact that a process started; it does not own anyone's
writer. StartupDisclosureStream carries the disclosures as typed events and the
caller drains them on the goroutine that owns stderr: the set known at
registration is printed before the reporter returns, keeping server order, and
later arrivals are printed by a single pump. The returned stop ends delivery,
joins the pump and flushes what is queued, so no write outlives the caller's
ownership. A disclosure arriving after stop is dropped, which is the defined
answer to what close does with pending delivery.
Headless defers stop so it runs before the runtime is closed. The interactive
path calls it on the line before deps.runTUI, so a launch resolving later is
dropped instead of writing raw text over the alt screen. Runtime.Close also
ends delivery, so a launch that resolves after the runtime is gone has nowhere
defined to land.
Removing the join reintroduces the original data race; removing the close lets
a post-stop launch write to the abandoned writer, failing the new regression on
the bytes it wrote.
Two findings, one contract: a fact that outlives the consumer that first
looked for it.
The retained launch sink stopped a late cmd.Start from being lost, but a value
nobody re-reads is still a lost disclosure. Both production reporters sampled
StartupDisclosures once, right after RegisterTools returned, and a stdio
attempt abandoned at the connect timeout could still be inside Start at that
moment. The process then ran under the reduced write confinement, the reaper
closed its client, and the operator saw the skipped-server warning and never
the disclosure. The previous regression masked this by polling
StartupDisclosures by hand after release rather than exercising a reporter.
The sink is now an event as well as a value. Runtime.ReportStartupDisclosures
reports servers whose notices were known at commit immediately, in server
order, and subscribes every other server's sink; a subscriber whose launch has
already happened runs before subscribe returns, otherwise it runs from
publishLaunch, and either way exactly once. Servers that never start never
publish, so prepare, pipe and Start failures stay silent, and network servers
contribute nothing. reportMCPStartupDisclosures uses the push form and keeps
the pull form for a runtime that lacks it. The new test drives the REAL
reporter against a REAL runtime: reporting before release prints nothing,
release prints the notice exactly once. Reverting the reporter to pull-only
fails it with "disclosed 0 time(s), want exactly 1".
The headless session writers built their own payload with decorated output
only, while the TUI writer stored typed notices and an undecorated body. Both
append to the same store the TUI resumes from, so a CLI-written long collapsed
result restored into the TUI had no body to carry the decorated text and no
notice furniture to draw it: the disclosure the run had shown was gone.
tui.ToolResultSessionPayload is now the one owner and the CLI delegates to it;
the one field the CLI had added on its own, truncated, moves into the shared
shape so it is not lost. A tui test restores the CLI's exact bytes into a
collapsed card and asserts the notice exactly once; a cli test asserts the
delegation, since reverting it leaves the tui test green and fails the cli one
on the notice, the body and the truncated flag.
The settle grace is only another timeout. When it expires, registration reaps
the abandoned attempt in the background and returns, but the process can still
be inside cmd.Start at that moment. It then starts under the reduced write
confinement, publishes to its sink, and nobody is left who can say so: the
reaper closes the late client, and Runtime had already frozen its disclosures
into a snapshot taken during the serial commit. Startup reported such a server
only as skipped.
Registration is bounded and a launch is not, so the two cannot share a
lifetime. The sink already carries the authoritative fact and outlives the
attempt; Runtime now retains it per server and StartupDisclosures reads through
it instead of copying out of it. A server whose notices were known at commit
never re-reads its sink, and entries stay in server order, so repeated reads
cannot duplicate or reorder anything.
Nothing else moves: pre-launch failures still publish nothing and stay silent,
network servers still launch no process, and the reaper still closes the late
client.
The existing regression releases its launch at 120ms, deliberately inside the
250ms grace, so it only proved the grace covered that interval. The new test
releases strictly after the bound, asserts the disclosure was legitimately
absent beforehand, and asserts a second read does not duplicate it. Reverting
StartupDisclosures to the snapshot fails the new test and leaves the in-grace
one passing, which is the point: the old shape could not see this.
The enforcement disclosure reached the card in two forms: typed
EnforcementNotices, which the card renders as its own furniture, and
ModelOutput, which has the notice composed into the text. toolResultDetail
returned the undecorated Display.Preview when one existed and fell back to the
decorated ModelOutput when one did not, so every result without a preview drew
the warning twice: once in the notice lines and once at the top of the body.
That is every bash and exec card and every error card. The existing regression
used a rich-preview edit result, which selects the one representation that
masks the path.
Give the body one owner. toolResultDetail now returns the base text, and the
card decorates once. agent.ToolResult gains BaseModelOutput and BaseDisplay
mirroring tools.Result, so a surface that renders the typed notice has a
canonical accessor to build from rather than re-deriving it, and ModelOutput
and HumanDisplay are expressed in terms of them so the base is computed once.
The durable path had the same mismatch: the payload stores the decorated output
beside the typed notices, and restoration used that output as the body whenever
no distinct preview was stored. The undecorated body is now always written when
it differs, and restoration keys on the field being PRESENT rather than
non-empty, because a command that printed nothing under an enforced profile has
an empty body and a real notice.
Tests cover live and restored, success and error, and assert that the notice
and the underlying output each appear exactly once. Reverting toolResultDetail
alone fails all of them.
connectStdio publishes only once cmd.Start has returned, and the timeout branch
sampled the sink the instant it fired. Those interleave: the sample reads
empty, the result commits with no notice, and the background reaper closes the
late client without being able to amend a commit that has already happened. A
process that started under reduced write confinement is then never represented
in StartupDisclosures.
Wait briefly for the abandoned attempt to say whether it had started, before
concluding it had not. cancel() has already fired, so an attempt that never
reached Start fails fast and the grace costs nothing; only one that did start
can still be inside Start, and it publishes on the way out. A test asserts that
the never-started case is not delayed, so the grace cannot quietly become a
startup cost.
The window itself is microseconds wide and cannot be hit from a test seam: an
attempt to widen it with a slow second server failed, because every per-server
goroutine returns at its own timeout and nothing holds wg.Wait open. So the
tests drive the contract instead, a start that lands after the timeout but
inside the grace, which is the case the synchronization exists to catch.
The serial phase also re-reads the sink for an index whose result carried no
notices, as a second net that costs nothing.
execExecutionOutcome is shared between exec_command and bash, and it set
Launched unconditionally. That is true for exec_command, where a start failure
returns an errorResult before an execution outcome is ever built, and it is not
true for bash, which hands EVERY Run error to the same conversion: a missing
executable and a context cancelled before os.StartProcess both arrive here with
a prepared plan and no child.
So a bash command that never created a process reported the DenyRead token
trade as applied. Measured before the fix: Launched=true, ChildLaunched()=true,
and one applied enforcement notice for a command that did not exist.
Observe it where it is known instead. exec.Cmd sets Process only once
os.StartProcess has succeeded, so bash captures that at the Run boundary and
threads it through withBashExecution; exec_command sets it true at its own call
site, with the reason written down rather than assumed.
The regressions drive the real tool rather than constructing an outcome, since
the bug was exactly that the constructed shape and the real one disagreed.
A stdio server that started and then hung in initialize or tools/list was
abandoned at the connect timeout and recorded as skipped, with nothing said
about the confinement its process ran under. The notices left connectStdio
only on the returned client or the returned error, and an abandoned attempt
produces neither before the serial commit phase is over. The reaper that
collects it later runs after that phase, so it cannot contribute without
breaking the deterministic ordering the phase exists to provide.
Publish the launch fact at Start instead, on a sink carried in the context, and
read it in the timeout branch. Launch and connection usability are separate
facts with separate lifetimes, which is the distinction that was missing.
Carried on the context rather than in the factory signature so an injected or
third-party factory that knows nothing about it still works and simply
discloses nothing.
A timeout before Start stays silent, since the sink is only ever published to
after Start returns. That placement is load-bearing, so it is pinned by a test
that drives the real connectStdio with an executable that cannot start: moving
the call one line up leaves the registry-level tests green while every failed
launch begins claiming the trade.
currentUserSIDForTest swallowed the GetTokenUser error and returned empty,
and its one caller guarded on the result being non-empty. On a machine where
the call fails, the assertion ran against nothing and the test reported a
pass.
The check exists to catch the restricted token keying itself to the very SID
it has to be stricter than, which is the whole point of the shape, so failing
to read the prerequisite is a failure rather than a silent skip. Confirmed
both directions with a simulated unreadable SID: the old shape passes, this
one fails naming what it could not read.
The notices are prepended to the model view on the way out, so a disclosed
result costs more context than result.Output alone. The outcome diagnostics
measured the bare output, which undercounts every disclosed call, and the
undercount scales with the notice rather than being fixed slack. A short
command output measures 13 bytes against the 114 the model is handed.
Measure the canonical text instead. ModelView stays the bare output on
purpose: ModelOutput prepends the notices itself, so storing them here would
send them twice, and a test pins that too.
The notice was prepended to ModelOutput, which toolResultRowText carries into
row.text, and toolCardHead is handed row.text. But the head renders the action
and the target, so the notice went nowhere. Every result with a rich preview,
which is every edit and write card, rendered with no disclosure at all,
collapsed or expanded.
Resume lost it too. The session payload carried the notice only inside the
"output" string, and the restored card is rebuilt from displayPreview, which
never had it.
Carry the notices as their own field on the row, persist and restore them
alongside changedFiles, and render them above the body on all three card
paths. Shown collapsed as well as expanded: a trade the operator has to expand
a card to discover has not been disclosed.
Kept out of row.detail deliberately. That field is parsed as a diff by the
files panel and rendered line by line by the file view, so prefixing it with
the notice would have been the shorter fix and would have corrupted both. A
test pins the diff stats against exactly that.
The render cache keys on the notices for the same reason. It distinguished
them already, but only through row.text, and that incidental coupling is what
hid the notice from the card to begin with.
BackendPlan derived the DenyRead warning from request.Backend and the
requested profile. request.Backend is always the AVAILABLE backend, so on a
Windows host it stays the restricted-token backend with NativeIsolation set
even when the resolution disables sandboxing outright.
With deny_read configured and --sandbox forbid, the plan resolves to
enforcement disabled and target none, builds no token and enforces no read
rule, and `zero sandbox policy` still reported that the write jail had been
traded for read denial. The reassuring half was the false one: it claimed
reads were denied as requested on a run that denies nothing.
The execution path already keyed this on the resolved plan through
windowsRestrictedTokenWillRun. Split the request-side half of that predicate
into willBuildWindowsRestrictedToken and reuse it for the diagnostic, so both
describe the plan that will run rather than the backend that happens to be
installed.
plan.Wrapped stays with the execution caller rather than moving into the
shared predicate. It is the produced execution state and the request cannot
speak for it, so folding it in would buy symmetry by handing the execution
path back a bug the request-side checks alone cannot catch. A test pins that
split.
addSandboxMeta writes the plan's notices at plan time, before anything runs, and
finalizeToolOutcome promoted them into the user-visible disclosure
unconditionally. That claims a token trade for a command that may never have
started, which is the same substitution the hooks and plugins paths already
stopped making.
The promotion now comes from the execution outcome when there is one, so it
follows the recorded launch state and the planned notices together. The plan
metadata is untouched and stays as diagnostics, since what was intended is still
worth having in the record, and a tool with no execution outcome still promotes
from metadata rather than silently losing its disclosure.
execExecutionOutcome states that its outcomes describe a started process rather
than leaving it to be inferred: a command that could not be started returns an
error result before reaching it.
connectStdio records the notices once cmd.Start returns, which is the right
moment, but the initialize failure path closes and discards the client. The
client was the only carrier, so a server that started, did filesystem work and
then failed its handshake told the operator it was unavailable and never that it
had already run without the write jail.
A launched process is a fact about the past: once Start has succeeded the
disclosure is true whatever the handshake does next. The failure now carries it
out, and registration recovers it from the error, so the fact no longer dies
with the connection it was attached to. A connect that never launched still
discloses nothing.
OutcomeKind is not a launch-state field, and reading it as one was wrong in both
directions. The adapter report is read AFTER Run, so a child that really ran and
then produced an unreadable report is rewritten to a setup failure and the
disclosure was dropped although it applied. And a context already cancelled
before os.StartProcess still selects a cancellation, so the disclosure was
claimed for a process that never existed.
ExecuteCaptured now records whether an OS process was created, taken from the
only thing that knows: exec.Cmd sets Process only once os.StartProcess has
succeeded. That is false for a missing executable and for a context cancelled
before Start, and true for anything that ran, including a later timeout or
cancellation. Report decoding can now fail after launch without rewriting the
historical launch fact.
The plugins test that asserted the kind decides was encoding the defect, so it
now expresses the recorded-fact contract instead, including the two shapes the
kind gets wrong.
`zero exec` registers workspace MCP servers through the same sandbox-backed
runner interactive startup uses, so a stdio server here can launch under the
weakened token and serve the whole run. Only the TUI reported the disclosure, so
every text, JSON, stream-JSON and --list-tools caller was told nothing about the
enforcement trade for a process that was already running.
Reported immediately after registration, before --list-tools and before the
first result, since both return early. On stderr, which is where the
skipped-server and trust notices already go, so stdout framing is untouched; the
regression asserts the JSON and stream-JSON output still parses.
The test isolates HOME, APPDATA, LOCALAPPDATA and the XDG roots. Without that it
builds a sandbox engine against the real config dir, triggers the one-time grant
migration there, and the notice surfaces on a later test's stderr, failing
whichever test happens to assert an empty one. That moves between runs and reads
as flakiness rather than as contamination.
Two defects in the disclosure collection, both found by jatmn.
The append ran inside the per-server goroutine, so it raced the shared slice
header: entries could be lost or overwritten, and whichever survived were
ordered by completion time rather than by server. The comment directly above
promises that the concurrent phase touches no shared state and that the serial
phase is therefore deterministic, and this broke both halves of it. The notices
now travel on the indexed connectResult and are committed in the serial loop, in
server order. Reproduced with 32 simultaneous servers under -race before the fix.
The disclosure was also reachable only through the client, so a server that
started, did filesystem work, and then failed initialize or tools/list lost the
fact when that path closed the client and returned nil. The operator was told the
server was unavailable and not that the process had already run without the write
jail. connectAndList returns the notices separately now, so they survive the
failure that discards the client. A factory error still discloses nothing,
because nothing launched.
A hook audit record kept an exit code, stdout and stderr, and the notice is
deliberately in none of those. Once the dispatch result was gone, nothing could
tell an audit or recovery reader that a hook had run under the weakened DenyRead
token. AuditResult carries the notices typed and omitempty, so historical
records read back unchanged and an ordinary hook writes what it wrote before.
An MCP stdio server's launch is the same shape one level up. connectStdio
received the prepared command and kept only the command and its cleanup, so a
server started under the weakened token served the whole session with nothing
able to say so, and no later tool result could recover it because the fact
describes startup rather than any response. The client keeps the applied
enforcement, recorded after Start returns so a prepare failure or a missing
executable claims nothing, registration collects it per server, and startup
states it once next to the skipped-server warnings. Network servers launch no
local process and report nothing.
Enforcement.Notices is PLANNED: it describes the shape a command was prepared to
run under, and planning is not proof that anything ran. Hooks copied the field
straight out, so a sandbox setup failure or a missing executable told the
operator the write jail had been traded away for a child that never existed.
Outcome.AppliedEnforcementNotices now makes that call once, where the outcome
kind is known, and hooks and plugins both use it; the plugin-local copy of the
rule is gone. A new pre-launch outcome kind is classified in one place instead
of being disclosed by whichever consumer was not updated.
MCP tools/call serialized Result.Output directly. That was a complete value
before this branch and is not one now: Output holds the undecorated base text
and ModelOutput is the model-facing projection. An affected Windows command
reached an MCP client with its ordinary output and no statement about the token
shape it ran under.
Making agent.ToolResult store the undecorated model text plus the typed notices
was right, but I only audited the consumers that render to a terminal. Three
others read the raw field and lost the disclosure the moment that change
landed.
ACP sends the tool result straight to its client, so an ACP client saw the
output with the warning removed, on the one surface that has no other way to
learn the sandbox narrowed what the command could do.
Both headless session writers persisted the raw field, and replay reads that
value directly into the transcript without rebuilding a ToolResult, so a
warning visible during the original run vanished from resumed and compacted
context with nothing failing to say so. Those two writers also spelled the same
payload separately and had already drifted, since the stream writer used the
accessor; they now share one helper.
The rule is that presentation and durable consumers both go through
ModelOutput, because the accessor is the only thing that composes the text with
the notices.
executeToolCall copied the already-rendered ModelOutput/HumanDisplay into
agent.ToolResult while also copying the typed EnforcementNotices slice, so the
same disclosure lived in two places with no contract between them. It renders
once today only because the outcome arrives finalized and the agent accessor
then reads Outcome.ModelView rather than the stored field, which also means the
stored field disagreed with the outcome it came from. A result reaching the
accessor without a finalized outcome would have shown the notice twice.
Split the undecorated base out into BaseModelOutput/BaseDisplay and have the
projection store that. Decoration now happens in exactly one place, the
accessors, and the stored text agrees with the finalized outcome.
Every other notice assertion in this PR hands a constructor a Notices slice and
checks it comes out the other side. That proves the consumers and never the
producer: deleting the one line in EnforcementFor that puts plan.Notes into
Enforcement.Notices left every notice test in the repo green, and that line is
the entire reason hooks, plugins and MCP see anything at all.
This starts from a plan the manager built rather than a literal, so the chain
from profile through plan.Notes to Enforcement.Notices is covered end to end,
with a silent-plan case so it cannot be satisfied by a field that is never
empty. It fails with the projection removed.
Two more places the notice was dropped, both the same shape as the last round:
one path assembles the result and another path, taken under different
circumstances, rebuilds it from fewer fields.
A plugin that timed out or was cancelled took invoke's error branch, which
constructed a result from status, output and metadata alone. The child had
already launched under the non-WRITE_RESTRICTED token, so the disclosure was
still true of it, and the model saw only the timeout.
The launched-or-not question is answered once now, in
execPluginCommandWithExecution where the outcome kind is known, rather than at
each constructor. A setup failure or a missing executable started nothing and
carries no notice; everything past launch does, however it ended. Every return
in invoke now carries whatever that decision produced, so the disclosure cannot
depend on which branch runs.
A vetoing beforeTool hook took the blocking branch, which builds
DispatchOutcome.Reason through blockReason and returns immediately, never
reaching hookMessage. Reason is the field the agent turns into the
model-visible result, so a hook that blocked an action while running without
write confinement said only that it blocked. blockReason composes the notices
now; blockCause keeps the wording it had.
No double render: blockedByHookResult reads Reason only, and the advisory path
reads Messages only, so the two channels stay separate.
The hook regression drives Dispatch rather than calling blockReason with a
hand-built commandResult, because a test that assembles the shape it expects
proves the consumer and not the producer. Both fail with their fix reverted.
Two halves of the same disclosure, neither of which reached a user.
The predicate keyed on request.CommandWrapped, read as "something already
wrapped this, so we are re-entrant". That is the opposite of what the field
means: BuildExecutionRequest sets it TRUE for exactly the native and unelevated
requests that buildPlatformCommandPlan then routes to
windowsRestrictedTokenCommandPlan. So the notice was suppressed on every plan
that builds the restricted token and fired on none of them. Every real
file_system.deny_read execution got the non-WRITE_RESTRICTED token and was told
nothing.
It keys on the produced plan's Wrapped state now, which is the resulting
execution state and cannot be read backwards: the direct plan sets it false, the
restricted-token plan sets it true, and both arrive through the same funnel.
The old test passed because its hand-built request left CommandWrapped false,
which is a shape no real execution has, and the whole cluster around it did the
same by passing an empty CommandPlan. Those are rewritten to be plan-based, and
the new regression drives the manager so the request carries the state the
transition actually produces. One of its silent cases named the misreading
outright and is gone.
The second half: plugin and hook results discarded Outcome.Enforcement.Notices.
Both projections copied stdout, stderr and an exit code out of the structured
outcome and dropped the rest, so once the predicate above is fixed a plugin tool
or a hook runs under the weakened token and still says nothing. Both carry the
notices now, plugins onto Result.EnforcementNotices and hooks into the surfaced
message, prepended so a hook that prints nothing still discloses. Covered on
both paths with an assertion that the notice appears exactly once.
Three findings from review.
The disclosure went into Result.Meta and stopped there. That looked like the
established channel because sandbox_downgrade_reason travels the same way, and
it is not one: nothing in production reads those keys, ModelOutput and
HumanDisplay never consult Meta, and the durable history drops it. A Windows
user configuring deny_read could take the non-WRITE_RESTRICTED token, lose write
confinement, and see nothing but ordinary command output. It is a field on the
canonical result now, surfaced by both accessors, so every surface reads it
through one contract. Prepended rather than appended, because the output budget
trims from the end and a disclosure that survives only on short results is not
one. The metadata copy stays for integrations reading the result JSON.
Promoted at finalizeToolOutcome, the single seam every tool result crosses,
rather than at each construction site. Setting it where results are built would
have been a third hand-maintained projection of the same fact, which is how it
went missing from the generic adapter to begin with.
That generic adapter is the second finding. PrepareExecution built
execution.Enforcement by hand for the wrapper hooks, plugins and MCP processes
go through, while exec_command built the same struct by hand for the tool path,
so Notices reached one and not the other. Both go through EnforcementFor now,
which copies the slice defensively.
And the notice claimed a trade nobody had made. The predicate asked only about
the host, the backend and DenyRead, so a disabled sandbox or a re-entrant
command, both of which take the direct unwrapped plan while still carrying the
Windows backend and profile, were told the write jail was gone. Neither half was
true there: no restricted token is created and deny-read is not enforced either.
It is keyed on the resolved execution state now, with the disabled, degraded,
already-wrapped, no-platform-sandbox and no-backend cases covered as explicit
silent cases.
My own fixture from the previous round was one of the things that had to change:
it named the backend without the fields that make a plan wrapped, so it was
asserting against a request that would never have produced a token.
The warning this PR added was reachable only from BackendPlan, which is what
`zero sandbox policy` and `zero sandbox check` render. A real tool call takes a
different path: the resolved profile becomes a CommandPlan, and the Windows
runner picks the token shape from that profile alone. DenyRead being non-empty
drops WRITE_RESTRICTED, which is the shape #869 is about. So an operator could
approve file_system.deny_read for one command, lose the workspace write jail,
and never see the disclosure, because it lived on a diagnostic view they had no
reason to run.
The notice is derived in withSandboxExecutionMetadata rather than at each
caller. That is the single funnel every plan passes through, including the
Windows one, so an execution caller cannot be added that quietly misses it. It
travels on CommandPlan.Notes, reaches the tool boundary as the sandbox_notices
metadata key alongside the downgrade reason that already goes that way, and
reaches the typed execution path as Enforcement.Notices.
The policy and check warning stays as the diagnostic view.
Covered in both directions and at both layers: a plan resolved with DenyRead
carries the notice and an ordinary profile carries none, and the tool metadata
gains the key only when there is something to say. Dropping the derivation fails
the plan test, dropping the emission fails the metadata test.
The message advertised `--sandbox forbid`, an option that does not
exist, and it survived because nothing drove the branch. The text was
only ever correct by inspection, and inspection is what missed it.
Route the apply through a seam so a test can fail it, then assert what
the operator actually reads: the cause is still wrapped, the option that
does not exist never returns, and both surviving remedies are named.
Restoring the old wording fails the test on both counts.
Also assert the failure does not record the applied-plan marker. That
marker is what makes later commands skip the re-apply, so recording it
here would turn one refusal into a sandbox that silently stops applying
its ACLs entirely.
The unelevated ACL failure told the reader to re-run with `--sandbox
forbid`. There is no such option: SandboxPreferenceForbid is an internal
engine state with no flag behind it, so following the advice produced an
unknown option and left them stuck on a failure they had just been told
how to clear. A recovery instruction that does not work costs more than
no instruction, because the reader spends time discovering it is wrong.
Point at the real way out instead: turn the sandbox off in the user
config, which is honored from global config only. The elevated-setup
remedy beside it was already correct and stays.
Reported by jatmn against the same string on #640. It predates both
branches, having arrived with the unelevated fallback tier in #427.
The warning fired on any plan targeting the Windows backend, including one built
somewhere else, which broke the smoke job on macOS and ubuntu.
credentialDenyReadPaths returns empty ON Windows and populates itself from the
host everywhere else, so a Windows-targeted plan built on a Linux runner carries
that machine's credential paths and drew a warning about a token nothing would
ever build. TestSelectBackendChoosesPlatformAdapterWithFallback asserts a Windows
plan has no warnings, and it only ever builds Windows plans from another host.
Gating on the host is more accurate rather than merely convenient: this describes
a token the Windows command runner will build, and that runner only runs on
Windows. Behaviour on a real Windows host is unchanged, which is why local
testing missed it.
The host is read through a var so both sides stay testable anywhere, matching
windowsSandboxInitialized. Added a regression test for the case that actually
broke, and mutation-verified it: dropping the gate fails it for linux and darwin.
#865 removed the World SID from the WRITE_RESTRICTED token, which is what makes
the write jail hold: every principal carries Everyone, so while it was a
restricting SID the write half of the check passed for free on any
Everyone-writable path. That fix had no CI protection. The only test covering it
sits behind ZERO_SANDBOX_REAL_SMOKE=1 and no workflow sets that variable, so a
rebase or refactor restoring the unconditional World SID goes green. #640's
branch conflicts on exactly that hunk.
CreateRestrictedToken works unelevated against the caller's own token, so the
invariant can be checked in an ordinary unit test. Added four:
- the WRITE_RESTRICTED token must not carry the World SID (this fails against a
reverted #865, verified by mutation)
- neither token shape may carry Users, Authenticated Users, INTERACTIVE, BATCH,
Administrators, SYSTEM, SERVICE, NETWORK, or the user's own SID. #869 names
these as the ones that would reopen the same class of bypass
- the capability SID must be present, so a token with an empty list cannot pass
by having no keys at all
- the non-WRITE_RESTRICTED shape still carries the World SID, which documents the
open gap rather than asserting the end state. It skips with a note if that
stops being true, so whoever closes#869 is told to replace it
Second half: the trade was invisible. Setting denyRead selects the token shape
without WRITE_RESTRICTED, and nothing told the person who set it that they had
given up write confinement to get read-deny. The plan now carries a warning
saying so, keyed off the same field the runner reads and scoped to the Windows
restricted-token backend, so the default posture stays quiet. Zero never
populates denyRead on Windows itself, so this only reaches users who configured
it.
This does NOT close#869. Closing it needs a read-side grant that is not a
universal group (AppContainer, or the per-workspace principals in #808), which
is a different piece of work. What changes here is that the fixed shape can no
longer regress silently, and the unfixed shape no longer looks enforced.
Refs #869, #865, #612, #640
2026-09-09 11:25:01 +05:30
740 changed files with 11660 additions and 2295 deletions
iferr:=live.client.Call(ctx,MethodSessionPrompt,PromptParams{SessionID:created.SessionID,Prompt:[]ContentBlock{TextBlock("list the files")}},&PromptResult{});err!=nil{
t.Errorf("the hook's disclosure did not reach the typed field, so the card and the restored card show the diff and no disclosure: %v",result.EnforcementNotices)
}
// AND NOT IN THE BODY AS WELL. Decoration has one owner per surface; carrying
// it in both places renders it twice wherever the surface draws the slice.
// fakeTurnSession wraps an inner provider for streaming while counting the
Loaded 100 of 740 files, more files were not shown because too many files have changed in this diff.
Show more
Reference in new issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Vasanthdev2004