* 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.
A snapshot whose identity could not be read at apply time failed the
rollback match the same way a replaced target does, and the operator was
told the target had been replaced. Both still leave the object alone;
the message now says which one happened. Reported by CodeRabbit.
The setup check read the marker's runtime root once to decide whether it
is current, then read the marker again to build the profile it
fingerprints. Reuse the first value, so a marker rewritten between the
two reads cannot put an unchecked root into the fingerprint. Reported by
CodeRabbit.
#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]>
* perf(redaction): add fast-path byte checks to eliminate regex allocations on clean strings
Evaluating cascading regular expressions across every log line, tool output,
and session event incurred ~23 µs and 51 heap allocations even on strings with
zero secrets.
This introduces fast-path substring checks before invoking expensive regex
substitutions, yielding a 70x speedup and 0 B/op heap allocation on clean
strings.
* test(redaction): lengthen ghp_ fixtures and cover conditional gates
GitHub classic tokens need 36 characters after ghp_. Benchmarks and
shape tests now use a matching fixture. A table-driven test checks each
fast-path gate, including near-misses and case-insensitive Authorization.
* perf(redaction): structure triggered patterns and avoid allocation in header case checks
* test(redaction): cover secret-header and URL-password gates
Keep the redaction marker literal after url.URL rewrites userinfo, so
callers still see [REDACTED] instead of a percent-encoded form.
* fix(redaction): unescape only the generated URL password
A path or query that already contains the encoded redaction marker
must stay encoded. Only the userinfo password is rewritten in the
clear [REDACTED] form.
* fix(redaction): re-escape URL username after parse
Username() is decoded. Re-encoding with url.User keeps control bytes
out of the redacted URL so a %0AAuthorization userinfo cannot inject
a header after the header pass.
* fix(redaction): fail closed on password URLs without a host
* test(redaction): isolate query-gate coverage from assign and text secrets
* fix(redaction): fold Unicode U+017F and U+212A in containsCaseInsensitive
Go (?i) regular expressions apply Unicode simple case folding, where U+017F (ſ) folds to 's' and U+212A (K) folds to 'k'. Teach containsCaseInsensitive to fold these runes to ASCII lowercase with zero allocations so case-insensitive header gates cannot be bypassed by Unicode-folded equivalents.
---------
Co-authored-by: cl-ment <[email protected]>
* feat(providers): add Concentrate provider preset
Register Concentrate (https://api.concentrate.ai/v1) as an
OpenAI-compatible catalog provider. Concentrate is already listed in the
README Partners section; this adds the provider itself.
- models.dev has no Concentrate provider, so the id `concentrate` and the
key name CONCENTRATE_API_KEY match OpenClaude's Concentrate
integration. `Concentrate AI`, `concentrate.ai` and `concentrate-ai`
resolve to it.
- Default model deepseek-v4-flash-0731: the dated id behind the
deepseek-v4-flash alias that OpenClaude uses. Concentrate's /v1/models
keeps the rolling name only in `aliases`, and onboarding keeps the
default selected only when it matches a live id, so an alias default
made setup land on the first model in the list (claude-fable-5).
- Curated models: deepseek-v4-flash-0731 (default), deepseek-v4-pro,
kimi-k3 and glm-5.3. Context windows (1,048,576 each), tool calling and
thinking support come from Concentrate's public /v1/models.
- Regression test: a Concentrate-shaped live list must contain the
preset's default model.
- README and README_ZH: key line and setup command.
* test(providers): prove the Concentrate default check runs on the live list
The regression test passed whenever the default model was in the result,
and the curated fallback also contains it. Assert that the mock served
/v1/models and that the result includes claude-fable-5, a model only the
live response lists, so a silent fall back to the curated catalog fails.
* fix(cli): let providers subcommands run with no active provider
`zero providers list`, `use`, `remove` and `rename` exited 3 with
`no active provider configured: active provider "" not found` as soon as the
environment supplied more than one provider and config.json had no active
provider: normalizeProvidersWithOptions only elects an active provider by
itself when there is exactly one, and resolveCommandCenterConfig treated the
resulting error as fatal, so the command stopped before doing anything. A
base URL without a key counts as a second provider too (applyProviderEnv
builds a profile from a base URL alone).
The resolver already returns the successfully normalized list alongside
ErrNoActiveProvider, and the interactive TUI already falls back to a usable
entry from it in runInteractiveTUIWithSetup. Do the same here instead of
failing, so these subcommands can answer for providers that only come from
the environment.
Also pin the four env-derived provider tests to a fixed environment and a
temporary user config. They set OPENAI_API_KEY but resolved through the real
resolver against the developer's own config.json and ambient provider
variables, so they failed on any machine that also exported another provider
variable and whose user config had no active provider.
Adds regression coverage for the two-env-provider case: `providers list`
must report every detected provider and `use`/`remove`/`rename` must give
their usual env-derived answer. Verified against the fix (pass) and with the
fix reverted (exit 3, `active provider "" not found`).
Fixes#1130
* refactor(cli): report the configured active provider, not an elected one
Provider-management subcommands answer for the SAVED configuration, while
the TUI's recovery path answers "what can I run with". Adopting
firstUsableProvider in resolveCommandCenterConfig conflated the two: with
two env-derived providers it made `providers current`/`list` present a
provider that `zero exec` would still refuse to run with, and a
base-URL-only profile failed `providers list` because the election demands
a credential. Return the resolver's partial config untouched instead, so
nothing is reported as active when nothing is selected.
Also removes the OAuth store read this path picked up with firstUsableProvider.
Adds assertions that `current` shows nothing selected, `list` marks nothing
active, `use --json` leaves activeProvider empty, plus a base-URL-only case.
* fix(cli): scope the no-active-provider exception to provider management
Returning the resolver's partial config made `zero config` exit 0 with a
partial summary, contradicting the resolver's documented headless contract:
`zero config` / `zero exec` still fail with the actionable message and only
the interactive TUI recovers from a missing active provider. Split the
helper so only issue #1130's commands — `providers list`/`current` and the
unpersisted `use`/`remove`/`rename` reporters — tolerate the state; `zero
config`, `detect`, `models` and `check` keep failing as before.
Also pin the OAuth store in providerEnvSetupDeps: summarizeConfig reads
OAuth login state through the default store, so a developer's stored login
could flip these assertions. Point ZERO_OAUTH_TOKENS_PATH at a temp file
and force ZERO_OAUTH_STORAGE=file so an ambient keyring setting cannot pull
the OS keychain in either.
* fix(workspaceindex): skip git-ignored and build-cache dirs in Scan only
Scan has a fixed file budget, and build output (Cargo target/, Python
virtualenvs, caches) could exhaust it before real source was reached:
repo-map reported truncation and ranked fingerprint files above code.
Inside a git work tree, Scan now asks git for the ignored set
(`git status --porcelain=v1 -z --ignored=matching .`), so the repo's own
.gitignore, nested ignore files, info/exclude and core.excludesFile all
apply, including names no fixed list anticipates (.locust_env/). Outside
a work tree, or when git is missing, older than 2.16, fails, or exceeds
5s, a fixed list of common build/cache directory names stands in.
ShouldSkipDir is unchanged: glob, grep, list_directory and path
autocomplete share it and must keep seeing target/, .venv/ and friends.
The lookup runs with GIT_OPTIONAL_LOCKS=0 so a per-turn scan never takes
.git/index.lock next to a concurrent git command.
Fixes#1108
* fix(workspaceindex): skip a rename's original-path field when parsing status
In `git status --porcelain=v1 -z`, a rename or copy record is followed by one
more NUL field holding the original path, with no XY prefix. The parser
inspected every field, so an original name beginning with "!! " read as an
ignored entry: after `git mv "!! a" b`, a tracked file "a" disappeared from
Scan.
Move the parsing into parseIgnoredStatus, which consumes that field for R or C
in either status column, and test it directly and end to end.
* fix(workspaceindex): disable repo fsmonitor and force untracked-files=normal for status
Scan's git status lookup must not run a repository's core.fsmonitor hook or fail
when status.showUntrackedFiles=no would reject --ignored=matching.
* fix(workspaceindex): ignore inherited git repository location
Git exports GIT_DIR and GIT_WORK_TREE to hooks. With those pointing at
another repository, Scan kept .locust_env/lib/x.py, which root's own
.gitignore excludes.
* fix(workspaceindex): quote the fsmonitor hook path in the regression
On Windows, sh treats an unquoted backslash path as escapes, so the hook
ran and the marker the test stats was never created. Require a status
without the override to create that marker first.
* fix(workspaceindex): pass the fsmonitor hook path to git with forward slashes
Smoke (windows-latest) failed TestGitIgnoredPathsDisablesRepoFsmonitor
because core.fsmonitor was set to a backslash script path. Git for Windows
hands that value to sh, which consumes the separators, so the hook never
runs and the marker is never created. The test now stores a quoted forward-slash
path. The product code is unchanged.
* fix(tui,cli): stop a repository's git config from running programs in Zero's own git calls
Zero runs git status and git diff on its own: the TUI's files sweep at
startup and after each command, its pull request diffstat, and the
scratch-file warning after a run. A repository's own config can name
programs those commands run (core.fsmonitor on every status, hooks when
the index is rewritten, clean filters and diff drivers assigned through
.gitattributes), so a workspace unpacked from an archive ran its own code
on the first turn, although Zero refuses the same workspace's project
hooks and plugins until the user trusts it.
internal/gitsafe builds those commands with core.fsmonitor and
core.hooksPath turned off and GIT_OPTIONAL_LOCKS=0, and drops the
repository-location variables git exports to hooks, using the same
environment for its config check and for the command. A repository whose
own config, or per-worktree config when extensions.worktreeConfig is on,
defines a filter or diff driver command is refused, and the callers
already treat that like a directory that is not a repository. The status
and diff calls also pass --ignore-submodules=dirty, because a submodule's
own config is not checked.
* test(gitsafe): keep the real-git setup off an inherited GIT_DIR
The setup helper in TestCommandAgainstRealGit ran git with the inherited
environment, so from a hook, or a shell that exports GIT_DIR or
GIT_WORK_TREE, git init and git config would act on the repository those
name instead of the temporary one. It now uses the same environment as
Command, and the test sets both variables to a decoy repository and checks
after each setup step that the decoy was not touched.
* fix(npm): wait for npm to apply each publish, and drop the one-time @gitlawb/zero steps
npm applies a publish asynchronously: `npm publish` returns once the upload
is accepted, and the version appears later. In the 0.10.0 release the
@twigpine/zero wrapper appeared 9 seconds after publish-packages.sh stopped
checking for it, and the last platform payload about ten minutes after its
upload. Two things went wrong:
- The wrapper became visible before four of its five platform payloads, so
installs in that window resolved aliases that 404 and fell back to the
GitHub-release download. Ordering the uploads does not order what npm
shows.
- The final `latest` check gave up after 25 seconds and failed the job even
though every publish landed, which also skipped the @gitlawb/zero steps
until a re-run.
publish-packages.sh now waits until npm shows every platform version before
it publishes the wrapper, then waits until `latest` is the wrapper version.
One deadline bounds the waits (ZERO_NPM_PUBLISH_WAIT_TIMEOUT, default 30
minutes). On timeout it fails before the wrapper goes out and says what npm
never showed. These lookups treat an error as "not yet", unlike the skip
decision, which still stops on anything but E404.
The test fake can now apply a publish only after a number of later lookups,
and records which aliased versions were missing when the wrapper was
published. Against the script as released in v0.10.0, the two new tests fail
the way the release did: the job exits 1 although everything landed, and the
wrapper goes out while its platform payload is not visible.
The two release steps that published 0.10.0 once more as @gitlawb/zero have
done their job and are removed; docs/NPM_PACKAGING.md and the comments that
pointed at them now describe what happened. The Go and wrapper handling of
the old name stays, since only a build installed under it can trigger it.
* fix(npm): validate the publish wait settings, and correct the legacy publishing claim
ZERO_NPM_PUBLISH_WAIT_TIMEOUT went into Bash arithmetic as typed. A leading
zero reads as octal (0600 waited 384 seconds), and 08 is an arithmetic error
that, in a script, does not stop it: Bash prints the error, leaves the
deadline unset and carries on, so the run only died at its first wait, after
the platform payloads were published. A negative timeout ended the wait
before it started, and a bad ZERO_NPM_PUBLISH_RETRY_DELAY only failed at the
first sleep, also after publishing began.
Both settings are now checked before anything is published: whole seconds
only, converted with 10# so a leading zero is decimal, bounded before the
conversion, and capped (timeout 86400, delay 600). A bad value fails with
the variable's name.
TestPublishPackagesReadsTheWaitTimeoutAsDecimal makes the script wait, which
is where the old code broke: against the reviewed script it publishes the
platform payload and then exits 1 on "08: value too great for base".
TestPublishPackagesRejectsAnInvalidWaitSetting covers -5, abc, 1e3, an
over-range and an overflowing timeout, and a non-numeric delay; the old
script published with five of them and crashed on the sixth with
"abc: unbound variable".
docs/NPM_PACKAGING.md said nothing could publish @gitlawb/zero again.
Removing the trusted publisher stops this workflow; a maintainer could
still publish by hand, and the doc now says that.
A session's lease file lived in the Store that took it, so once that Store
was unreachable the garbage collector closed the file, and closing it drops
the lock. That made TestForkAndChildHoldTheParentTheyAreCreatedFrom fail on
main whenever a collection ran before its dry-run prune. It can also happen
in an exec run: PrepareExec takes the calling or forked session's lease on a
Store the run never touches again, so the parent's lease can disappear part
way through and a prune in another terminal could remove it.
Leases now belong to the process, as Hold documents. Every Store holding a
session shares one lease file in a process-wide table, and the file is
unlocked and closed only when the last Store holding it releases it.
* feat(npm): publish as @twigpine/zero
The npm package moves from @gitlawb/zero to @twigpine/zero, and everything
that finds Zero's GitHub repository by name now names Twigpine/zero.
Package name
- package.json, the lockfile and release-please use @twigpine/zero.
`repository` names Twigpine/zero: npm checks provenance against it, so a
publish from this repo with the old URL fails with E422. The homepage is
zero.twigpine.com.
- The name lives in package.json only. build-platform-packages.mjs names the
wrapper, platform versions, alias names and alias targets from it, and
bin/zero.js reads its own package.json for the alias it resolves and for
every reinstall hint. The Go constant npmPackageName is held equal to
package.json by a test.
Repository defaults
- The update check (DefaultRepository), install.sh, install.ps1, the npm
wrapper's fallback downloader, and the GitHub Action's zero-repo default
all used Gitlawb/zero. That name now belongs to an unrelated GitHub org, and
these paths download and run what they find there, so they point at
Twigpine/zero. Docs, README and issue-template links follow. The Go module
path (and so `go install github.com/Gitlawb/zero/...`) is unchanged here.
Publishing
- The two inline publish steps become scripts/npm/publish-packages.sh, which
reads the name from the assembled payloads, refuses to start when the
wrapper's aliases do not match the platform payloads, skips versions
already on the registry (so re-running a half-finished job is safe), and
publishes with --ignore-scripts. The dist-tag rules are unchanged.
internal/npmwrapper/publish_test.go runs it against a fake registry.
Docs explain the switch for existing installs: uninstall @gitlawb/zero first,
because both packages own the `zero` bin and npm refuses the second (EEXIST).
* feat(npm): publish 0.10.0 once more as @gitlawb/zero to point users at the new name
Deprecating @gitlawb/zero alone would strand every existing npm install.
0.9.0's `zero upgrade` runs `npm install -g @gitlawb/zero@latest`; with
nothing newer under that name npm installs nothing, prints no deprecation
warning (it only warns when it installs something), and Zero then reports
"updated via npm to 0.10.0" while staying on 0.9.0.
So this release goes out once more under the old name, as a bridge:
- The publish job assembles the same verified artifacts a second time with
the new `build-platform-packages.mjs --package-name @gitlawb/zero` and
publishes them with publish-packages.sh, after the @twigpine/zero publish.
A gate runs it only while @gitlawb/zero's latest is still 0.9.0 (or is
this version, on a re-run), so it happens once.
- The build knows its name from the package.json beside it. The Go side
reports InstallMethodNpmLegacy: `zero upgrade` refuses with the switch
steps before any release check (so it works offline too), and
`zero update --check` adds them whether or not an update is out.
- bin/zero.js prints the same steps after zero exits (the TUI would hide a
notice printed first), only when stderr is a terminal, and not for
`update`/`upgrade`, which print it themselves.
- Updating from 0.9.0 now installs a real version, so npm also shows the
deprecation message.
The steps say to uninstall @gitlawb/zero before installing @twigpine/zero:
both own the `zero` bin, npm refuses the second install with EEXIST, and a
forced install loses the bin when the old package is later removed (checked
against npm 11.16).
docs/NPM_PACKAGING.md has the maintainer runbook: bootstrap and trust the new
package, retarget the old one's trusted publisher for the bridge, then
deprecate @gitlawb/zero, revoke its publisher, and delete the bridge steps.
* fix(npm): only skip a publish when npm says the version is not there
publish-packages.sh treated any failed `npm view <name>@<version>` as "not
published". A transient network, auth or registry error on the re-run that the
skip exists for therefore published a version that was already out, and npm
rejected it with EPUBLISHCONFLICT; with the registry unreachable it published
blind. npm 11 reports every kind of missing (no such package, no such version,
including platform versions) as E404, so only E404 now means "not published".
Any other failure is retried three times and then stops the job before it
publishes anything, with npm's own error shown.
The test fake now answers a missing version with E404 the way npm does (it
returned an empty success before), and can fail lookups with a network error.
The two new tests fail on the old lookup: one with EPUBLISHCONFLICT after
transient failures, the other by publishing while the registry is unreachable.
Also renames the unused retry variable in the legacy-name gate step to `_`
(shellcheck SC2034), matching the script.
shell-quote 1.9.0 is inside the range GHSA-pqg4-j6r4-53mv covers
(>= 1.8.4, < 1.11.0). It reaches the lock through tuistory,
@opentui/react and react-devtools-core, which asks for ^1.6.1, so
refreshing the lock moves it to 1.12.0 without touching package.json.
Release packaging installs the local-control helpers from this lock,
so the old version shipped in the helper tree.
* fix(acp): persist token usage events for ACP sessions
ACP turns never set agent.Options.OnUsage, so provider token counts were
dropped on the floor. zero exec and the TUI persist the same counts as
session usage events, and `zero usage report` totals those events, which
left every editor-driven ACP session at zero requests and zero tokens.
runTurn now queues each report through usage.EventUsagePayload, the same
writer exec uses for a non-escalating run, and commits it in the turn's
existing buffered batch under the session id. A hard run failure still
returns before persistTurn, so that turn's usage is dropped with its
history. StopCancelled stays a successful outcome and keeps the usage
already reported. session/load and session/resume still skip event types
other than messages, compaction, and tools, so the restored prompt and
the wire replay do not gain a usage channel. ACP has no model escalation,
so the payload has no "model" key and no prompt, tool argument, or other
raw input.
This sits beside the permission-mode field persisted on the session
(#1134). It does not change the ACP wire protocol, exec, the TUI, or the
report reader.
Refs #1118
* test(acp): pin cancel, resume, and payload shape for usage events
The persistence test showed that a successful turn records token counts,
and the hard-failure test showed that an aborted turn records nothing.
Neither one locked the two edges that are easy to get backwards.
A client cancel is a StopCancelled outcome, not a hard failure, so the
tokens the provider already reported stay in the session log. session/load
and session/resume read that same log; the replay and the next prompt
must still be the conversation, with no usage payload on the wire and no
token counts folded into the resumed prompt. The persisted JSON is also
checked to be token-count keys only, so a prompt or tool argument cannot
ride along in the usage record.
Refs #1118
* test(cli): pin sandbox policy golden temp roots
TestRunSandboxPolicyJSONGoldenIncludesManagerBaselineFields compared the
policy snapshot to a fixed golden after stripping only the temp roots
NewScope keeps. NewScope drops a candidate that sits inside an earlier one,
while the JSON (built with a nil scope) still lists it, so a TMPDIR inside
/tmp — or TMP inside TEMP on Windows — survived normalization and failed
the golden. Pin the variables that path reads (TMPDIR on Unix, TEMP and TMP
on Windows) and cover a nested temp dir that is not inside the earlier
candidate, so the normalizer removes it instead of leaving it in the JSON.
Refs #1121
* test(cli): run the golden against an ambient nested temp dir
The nested golden created its temp dir in the package working directory and
failed when that path sat inside /tmp, so a checkout under /tmp could not
run it and the test wrote into the source tree. Stage the issue's shape
first — TMPDIR inside /tmp, or TMP inside TEMP — then let the pin override
those variables. Directories come from t.TempDir. If the nested shape cannot
be built, skip.
Refs #1121
* test(cli): give the local adoption probe a test-sized timeout
TestLocalAdoptionSavesAndReloadsEligibleModel relied on the 400ms
defaultLocalProbeTimeout, which treats a slow probe as "runtime not running"
and silently drops the candidate; under -race on a busy runner one slow
loopback round trip can exceed it and empty the detection result. Pass a
5-second test deadline instead. The production default is unchanged and stays
covered by TestDetectLocalRuntimesAppliesDefaultTimeout.
Refs #1132
* test(cli): correct the timeout-coverage note on the adoption probe
TestDetectLocalRuntimesAppliesDefaultTimeout pins the probe timeout with an
explicit 20ms deadline, so it covers timeout enforcement rather than the
400ms production default. Reword the comment accordingly; no assertion or
production behavior changes.
Addresses the CodeRabbit finding on #1136.
ACP kept the permission mode in memory only: both mode doors called setMode,
registerSession always started at PermissionModeAuto, and nothing wrote the
mode to the session store, so session/load and session/resume returned a
plan/ask session as auto after a restart.
Save the mode next to the model on both doors, store first so a failed write
fails the request instead of leaving disk behind memory, and restore it through
registerSession. A session with no saved mode (written before this change)
keeps auto; a saved value outside auto/ask/plan never comes back weaker than
ask, and unsafe stays unreachable over ACP.
Refs #1125
* test(sessions): run the session lock test on Windows and add a cross-process one
TestSessionFileLockSerializesAcrossStores was skipped on Windows with
"flock semantics differ on windows", so the LockFileEx path in
filelock_windows.go was never exercised by a test on the platform that
uses it. LockFileEx locks are per handle, which is what the test relies
on, so the skip was unnecessary: remove it.
Also add a helper-process test (same pattern as lockutil's) proving the
session lock blocks a different process and is released by the OS when
the holder exits without unlocking.
Both fail on Windows when the lock is replaced by a no-op.
Fixes#1104
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
* test(sessions): keep the session lock alive in the helper process
The helper discarded the unlock closure returned by lockSession, which is the only reference to the open lock file/handle. GC finalization could close it while the helper waits on stdin, releasing the OS lock early and making the cross-process test pass for the wrong reason. Retain the closure and runtime.KeepAlive it after io.Copy, as the lockutil helper does.
---------
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
* test(tui): isolate user home, config and cache dirs for the TUI tests
newModel reads <user config>/zero/commands/*.md, and the package has 500+
newModel calls with no isolation, so a developer's own slash commands and
state leaked into the TUI tests (and tests could write to real user
directories), against the AGENTS.md "Hermetic tests" rule.
Add testutil.IsolateUserDirs, which points HOME, USERPROFILE, APPDATA,
LOCALAPPDATA and the XDG_* roots at a temp dir (covering Linux, macOS and
Windows), and call it from a TestMain in internal/tui. Per-test t.Setenv
overrides still win.
Refs #1103
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
* test(testutil): fail closed when user dir isolation cannot be applied
IsolateUserDirs now returns an error when any Setenv fails, rolling back
the variables it had already redirected, and its restore func reports
restoration failures. The TUI TestMain exits non-zero when isolation,
restoration or removal of the temp root fails, and the helper test
restores the inherited XDG_DATA_HOME via t.Setenv before clearing it.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
* test(testutil): cover partial-redirect rollback and compare physical paths
Make Setenv injectable so a test can let HOME be redirected and fail the
next variable, proving IsolateUserDirs rolls back a variable it really
changed (the NUL-root case fails on the first call and cannot).
Add testutil.PathWithin, which resolves the existing part of each path
with EvalSymlinks before the containment check, and use it in both the
helper test and the TUI hermetic test so symlinked temp dirs and Windows
short names cannot cause false results.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---------
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
The peer approval prompt's decide closure called runtimeMessageSink from
inside Update. The sink ends in program.Send, which blocks on bubbletea's
unbuffered message channel; the only reader is the event loop that is
running Update, so answering a held cross-session message hung the TUI.
Give pendingPermissionPrompt a decideCmd that returns a tea.Cmd yielding
the peerDecisionMsg, and have resolvePermissionWithReason return it
instead of calling the sink. Tests now drive the decision through the
returned command and a blocking sink, which hangs on the old code.
Fixes#1099
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
TestExecCommandForegroundServerReturnsSessionAndServesHTTP parsed the
helper's listening address from the first exec_command result only, with
a 500 ms yield. On a loaded Windows runner the helper had not printed its
address yet, so the test failed even though the session was running.
Poll the session with write_stdin until the address appears (20 s
deadline), and register the cleanup before the address check so the
server is stopped on early failure too.
Fixes#1097
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
SetNotify was kept in #1001 as a thin replace-form wrapper over
UpdateNotify for existing tests, but every product surface (the CLI
`zero config notify` and the TUI /notify picker) calls UpdateNotify's
merge form directly, so the advisory deadcode job reports it as
unreachable. Move the replace helper into writer_test.go as the
unexported setNotify, keep every test it served unchanged in behaviour,
and point the two CLI doc comments at UpdateNotify.
`make deadcode` no longer lists internal/config; the other advisory
findings in that job are pre-existing and untouched.
* fix(config): sync atomic config replacements
Persist temporary config contents before rename, then sync the parent directory on platforms that support it. Return persistence failures and cover ordering, cleanup, and pre/post-replacement failure states.
FixesGitlawb/zero#1087
Amp-Thread-ID: https://ampcode.com/threads/T-01a0dcd7-47fc-76eb-82d3-3f6c25384912
Co-authored-by: Pierre Bruno <[email protected]>
* fix(config): sync new config directories and relax unopenable dir sync
Sync each directory entry created by MkdirAll into its parent so a crash
cannot lose the config directory after a successful first write.
Treat a config directory that cannot be opened as best-effort durable,
matching the sessions store's syncDir; only a failed Sync or Close is
reported, since the rename has already replaced the file.
Check the temp file is closed with Seek instead of Stat: on Windows,
File.Stat on a closed file returns "The handle is invalid" rather than
os.ErrClosed.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(config): report directory-open failures except for write-only directories
syncConfigDir treated every os.Open failure as success, so a caller could
acknowledge a save whose directory barrier was never reached (Copilot
review, PR #1093). Keep the maintainer-requested best effort for a
directory the user can write but not read, as in the sessions store, and
return every other open failure.
TestSyncConfigDir now expects os.ErrNotExist for a missing directory on
non-Windows hosts; it fails on the previous code with
"missing directory: error = <nil>, want file does not exist".
* fix(config): create the config directory durably on the lock path and roll back failed barriers
Every exported mutator acquires the config lock before it writes, and the
lock file is a sibling of the config, so lockConfigFile's MkdirAll was what
created the config directory on a first write. writeConfigData then found
nothing missing and never synced the new entries into their parents, so a
locking mutator could report success without the directory creation being
durable (CodeRabbit review, PR #1093).
A failed parent sync also left the newly created directories in place: a
retry found nothing missing, skipped the barrier, and reported success.
Route both creation sites through one helper, ensureConfigDir. It creates
each missing component with os.Mkdir so it knows exactly what it created,
syncs each new entry into its parent deepest first, and on any barrier
failure removes only the directories this call created, deepest first,
with os.Remove so a directory another process has populated survives. A
failed rollback is joined into the error, never swallowed. A component
that appeared between the lookup and the Mkdir is not rolled back but its
parent is still synced.
Regression coverage:
- TestLockingMutatorSyncsCreatedDirectories drives UpdateNotify on a
missing nested directory, asserts the parent syncs happen before the
lock file exists, that a failed barrier rolls the directories back, and
that the retry re-runs the barrier. On the previous code it fails with
"error = <nil>, want injected sync failure" because the lock path
bypassed the barrier entirely.
- TestWriteConfigDataRollsBackOnlyCreatedDirectories asserts a failed
barrier removes the created directories but keeps a pre-existing
directory and its file, and that a directory populated by another
process is kept and the failed rollback reported. On the previous code
it fails with "must be removed after a failed parent sync: <nil>".
---------
Co-authored-by: Amp <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
Co-authored-by: Kevin Codex <[email protected]>
Register GMI Cloud (https://api.gmi-serving.com/v1) as an
OpenAI-compatible catalog provider and add it to the README Partners
section.
- The catalog id `gmicloud` matches the models.dev provider id, so the
remote model catalog resolves without a mapping. `gmi-cloud`,
`GMI Cloud` and `gmi` resolve to it.
- Reads GMI_API_KEY (the name in GMI's docs) and GMICLOUD_API_KEY (the
name models.dev uses).
- Defaults to Qwen/Qwen3.8-Max. The curated list is Qwen 3.8 Max and
Flash (GMI's partner models), then DeepSeek V4 Pro and Flash. All four
are served by GMI's /v1/models.
- README and README_ZH: key line, setup command, and the partner logo in
light and dark variants under docs/assets/.
Verified live with a GMI key: `zero providers check gmicloud
--connectivity` passes, and `zero exec` completes a read-then-write task
on both Qwen 3.8 models.
* feat(providers): opt-in ZERO_RESPONSE_HEADER_TIMEOUT override for slow first-byte deployments
The shared HTTP transport waits at most 120s for a response header. A local
model server that has to load a large model can take longer than that to
produce the first byte (measured 1-5 minutes on a throttled local Ollama), so
an otherwise-alive request is aborted.
Add ZERO_RESPONSE_HEADER_TIMEOUT, mirroring ZERO_STREAM_IDLE_TIMEOUT: a Go
duration or a bare number of seconds; "0", "off", "none" or "disabled" remove
the limit; an unparseable or non-positive value falls back to the default
instead of removing the limit. The default stays 120s, so nothing changes for
anyone who does not set the variable. The constant and the resolver sit next to
the idle-timeout equivalents in providerio.
Checked against a throttled local Ollama: ZERO_RESPONSE_HEADER_TIMEOUT=5s
fails at ~6.4s, =300s succeeds at ~223s (a request that would have hit the
120s ceiling).
Refs #1038
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_0142QEjA7eEFcdXTpcTmcAZk
* fix(providers): keep the 120s header timeout when bare seconds overflow
A value such as 36028797018963968 passes strconv.Atoi and wraps to zero
when multiplied by time.Second, which would disable the default. Values
that do not fit in time.Duration stay on the default.
* fix(providers): keep timeout defaults when bare seconds overflow
ResolveStreamIdleTimeout multiplied bare seconds by time.Second the same way the header timeout did, so a large count wrapped to zero and disabled the watchdog. Both resolvers now share a bounds-checked conversion.
* fix(providers): apply the header timeout when the shared client is taken
The stall-hardened transport baked ResponseHeaderTimeout in at package init, so a later ZERO_RESPONSE_HEADER_TIMEOUT never reached the real client and the 120s check depended on the env at init. Each resolved value now keeps its own transport, and a slow header proves that transport enforces it.
---------
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
Add three Trendshift badges under the tagline (#1 Go repository of the
day, #3 Go repository of the week, #3 on GitHub Trending) and a Partners
section listing Twigpine, Xiaomi MiMo, Novita AI, Concentrate, and Exa,
following the layout used in the OpenClaude README. README_ZH.md gets
the same badge row and a matching section.
The Novita, Concentrate, and Exa logos are vendored under docs/assets
with light and dark variants. The Twigpine and Xiaomi MiMo logos are
loaded from their own sites, as OpenClaude does; Twigpine switches to
its dark variant under a dark color scheme.
Reject starts when all slots are live or reserved, prune only completed history, and release reservations on launch failure. Keep live processes tracked even after failed termination. Add concurrent admission and real-process regressions for #1085.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0dcd5-eb93-74c9-a1ac-5b140edaf973
Co-authored-by: Amp <[email protected]>
* fix(security): harden shell flag parsing, write roots, and formatters
- sandbox: recognize POSIX short-option clusters in shellCommandFlag (ZERO-ESC-02)
Extract payload commands behind combined short flags (-ec, -lc, -xc) and long
--command in dashCPayload and shellDashCPayload, preventing destructive
subcommands from bypassing recursive AST inspection and auto-allowing execution.
- tools: anchor write_file and edit_file to *os.Root descriptor (ZERO-TRAV-01)
Use openScopedWriteRoot and commitRootedFileContents to traverse parents relative
to the root descriptor handle, eliminating residual TOCTOU directory swaps.
- tools: harden format_on_write against untrusted execution and secret leakage
Pass --no-config to prettier to disable dynamic JS config execution, reject
formatter binaries resolved inside the workspace (formatterBinaryInWriteRoots),
and scrub credential-bearing environment variables before running formatters.
* fix(security): stop shell flag parsing at dash-dash and verify root descriptor identity
Stop recursing into shell `-c` payloads for tokens that follow the `--`
separator, which marks positional operands rather than options, in both
shellDashCPayload (text detector) and dashCPayload (AST analyzer). This
prevents `bash -- -ec '...'` from being classified on a command the shell
never runs as a command string.
Verify the identity of the *os.Root descriptor returned by openScopedWriteRoot
against a pre-open os.Stat of the resolved root, closing the check-to-use
window opened by os.OpenRoot re-resolving the path. A root substituted for a
symlink or a different directory is refused and the descriptor closed.
Regression tests fail on the unfixed code:
- TestShellDashCPayloadStopsAtDashDash, TestDetectInteractiveCommandAllowsNonInteractive
- TestAnalyzeCommand (`bash -- -ec ...` must stay non-interactive/non-destructive)
- TestOpenScopedWriteRootRejectsSubstitutedRoot
Bureau: desk-pr-1066
* fix(tools): relax symlink resolution error handling on host formatters and clean up test directory
* fix(security): anchor write preimage on root descriptor and report skipped formatters
---------
Co-authored-by: hazyhaar <[email protected]>
* fix(tools): hint when Windows resolves POSIX paths under the workspace root
On Windows a leading-slash POSIX path is not absolute, so read tools joined
it under the workspace root and surfaced a raw GetFileAttributesEx error.
Append a Windows-only hint naming the host OS and the workspace root when such
a path is missing, so the model switches to a workspace-relative path instead
of retrying the same shape.
Guidance only: which path is opened, confinement errors, and write-path
resolution are unchanged. All read tools share the annotated resolver.
Refs #972
* test(tools): keep the Windows path hint wiring test hermetic
The cross-platform wiring test used a fixed "/home/..." request, but
resolveScopedReadPathForGOOS injects only the annotation GOOS: on Unix the
request is host-absolute, so root was ignored and the test read state it did
not own. Build the request from t.TempDir() instead - joined under root on
Windows, anchored in root on Unix - and resolve an existing file under that
tree to prove the resolution stayed in test-owned storage. The native Windows
end-to-end test is unchanged.
Addresses the P2 finding on #1063.
* fix(tools): reject mixed-separator UNC paths in the Windows hint
Windows treats both separators as valid, so a second leading separator in
either position (/\server\share, /\?\C:...) is the same UNC or device-style
prefix as //. looksLikePosixAbsolutePath excluded only the literal "//", so a
missing mixed-separator target was misreported as a POSIX path resolved under
the workspace root. Exclude either separator byte and cover both mixed forms
in the shape and annotation tables.
Addresses the P3 finding on #1063.
* feat(sessions): prune old sessions, never one another process has open
Store.Prune removes sessions last updated before a cutoff of at least a
day, and reports what it removed, what it kept and why, and what failed.
A dry run makes the same decisions and removes nothing.
A session holds no lock between writes, so nothing told a prune in one
terminal that another terminal still had the session open. Each session
now has a lease file: a process that writes a session, or loads it
through the rehydrated reads the TUI, exec --resume and ACP use to
continue one, holds a shared lock on it for its lifetime, and prune
takes it exclusively and leaves alone any session it cannot get. On
Windows the lease file is opened with FILE_SHARE_DELETE so a held lease
never blocks removing the directory.
Prune also keeps every ancestor of a session that stays, since Lineage
and Tree fail on a missing ancestor, removes descendants before their
ancestors, re-checks each session under its lease and write lock, and
removes the metadata first so a removal that fails part way leaves
nothing that looks like a session.
Part of #971.
* feat(cli): add zero sessions prune, with a retention setting read from user config only
`zero sessions prune --older-than 30d` removes sessions last updated
before the cutoff, `--dry-run` lists what it would remove and why it
keeps the rest, and `--json` prints the report. The cutoff is days or a
Go duration and never under a day. Without --older-than, prune uses
sessions.retentionDays; with neither it refuses and says how to set one,
so nothing is removed unless asked for.
sessions.retentionDays is read straight from the user config file.
Sessions belong to the user rather than to a repository, so a project's
.zero/config.json cannot choose what prune deletes, and no resolver
merge copies the setting from anywhere.
Closes#971.
* test(sessions): a dry run reports a session another process has open as kept
* test(sessions): pin the dry run for undated sessions and the lease a new session takes
* test(sessions): report every failure when a session is written while pruning
* docs(readme): list sessions prune in the Chinese README as well
* fix(sessions): a fork or child holds its parent, and is refused while prune holds it
A lease is taken best effort, so a fork whose parent prune held at that
moment went ahead: the rehydrated read in exec --fork could not hold the
parent, Fork read it anyway, and the fork was created under a parent
prune was removing. Fork and CreateChild now take the parent's lease
themselves before reading it, and refuse when prune holds it, so the
parent of a session being created is never removed under it.
* docs(sessions): say that nothing calls Release yet, and what it is for
* fix(sessions): let go of the lease before removing a pruned session's directory
lease.lock is deleted with the rest of the session while prune still
holds it. Where a delete only takes effect once the last handle closes
(Windows without POSIX delete semantics: older builds, or FAT and exFAT
volumes), prune's own handle kept the directory from being empty, so
the removal failed and left an empty directory behind. Prune now closes
the lease after session.lock is gone and before the directory goes.
* test(cli): an empty --older-than is a usage error, not the retention setting
* fix(sessions): refuse to continue a session prune holds, rather than read it without the lease
A resume that started while prune held the session could not take the
lease, read the session anyway, and went on to use one prune was free to
remove: a read changes nothing prune checks again. The rehydrated read
behind the TUI resume, exec --resume and --fork, and ACP session/load
and session/resume now fails with ErrPruning at that moment, and none of
the three falls back to a raw read on it. ACP load stays best effort
about a history it cannot read, but not about this one. Writes are
unchanged: a write lands under session.lock, where prune checks the
session again.
Prune takes the lease through the new Store.HoldExclusive, which the
TUI and ACP tests use to hold a session the way prune does.
* test(sessions): a lease that cannot be taken keeps the session, and does not stop a resume
Prune fails closed when it cannot take a session's lease at all, so a
process whose own hold fails the same way can carry on: prune cannot
remove the session either. Refusing there instead would stop resume
and fork wherever locking does not work, for people who never prune.
* fix(sessions): keep a parent forked after the plan, and refuse sessions removed under a caller
Three gaps jatmn found in the prune lease rules.
A fork made after prune's plan, by a process that has since exited,
was not in the plan, so its old parent was removed and the fork was
left under a missing ancestor. Once prune holds a parent exclusively it
now looks for sessions the plan did not see that name it as parent,
and keeps it if there is one.
Store.Create with a ParentSessionID, which exec --calling-session-id
and spec implementations use, did not hold the parent at all. It now
holds it the way Fork and CreateChild do, and all three refuse a
parent whose directory is still there without its metadata: prune
removes the metadata first and unlinks lease.lock after it, so a lease
taken at that moment lands on a fresh lease file and proves nothing.
A resume, exec --resume or --fork, or ACP load that picked a session a
moment before prune removed it could take a fresh lease and read the
session as empty. HoldToContinue now holds the picked session and
requires its metadata, and those callers go through it. The
rehydrated read on its own still reads a missing session as empty.
* fix(sessions): make a dry run decide at removal time like a real prune
A dry run appended every planned session to Removed without calling
pruneSession, so none of the checks made once a session is held for removal
ran: the second lease check, the re-read of the last update, and the scan for
a child created after the plan. On the same disk, a dry run could list a
session that a real run keeps.
pruneSession now takes a dry-run flag, makes every one of those checks under
the same locks, and stops before removing anything. The prune command reports
a session a dry run could not check as such, not as one it could not remove.
* test(cron): pass the history limit in the claim tests
#1094 added a limit argument to cron.Store.Runs. #1090 merged shortly
after from a branch that predated it, so its two new tests in
cron_claim_test.go still called Runs with only the job id and the
internal/cli test package stopped compiling on main:
internal/cli/cron_claim_test.go:111:38: not enough arguments in call to stores[0].Runs
have (string)
want (string, int)
That failed vet and every job that runs go test. Pass 0, which selects
the default MaxRunHistory cap, as the other callers in this package do.
* fix: bump Go to 1.26.9 for stdlib vulnerability fixes
govulncheck reports ten reachable standard-library vulnerabilities on
Go 1.26.6, all fixed in 1.26.9: GO-2026-6603, 6605, 6610, 6611, 6612,
6613 and 6617 in net/http, GO-2026-6608 in net/textproto, GO-2026-6607
in crypto/tls, and GO-2026-6604 in os (Windows only).
CI and release workflows read the toolchain from go.mod, so the go
directive is the only version to change. Update the stated source-build
minimum in the READMEs and install guide to match.
Login discovers the token endpoint (RFC 9728 / 8414) and may obtain a
client_id/client_secret through dynamic registration, but persisted none
of it. Refresh read only the config-file OAuthConfig, which is empty for
the standard `auth: oauth` setup, so every expired access token failed
with "no token endpoint configured for refresh" and required a new
`zero mcp oauth login`.
Store the discovered token endpoint (and whether it was server-advertised)
and the dynamically registered client on the token, and fall back to them
in Refresh. Explicit config still wins, and values already in config are
not copied into the store. A stored endpoint is re-validated before use:
the shared https/loopback rule always, and for a server-advertised
endpoint the same public-network policy and pinned client Login used. The
refreshed token keeps the stored fields so later refreshes keep working.
Fixes#1101
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
* fix(cron): preserve newer claims when overlapping runs complete
Let claimFire own schedule state and increment the current fire count under the store lock. Abort completion writes on read errors rather than restoring a stale snapshot.
Cover adjacent slots in both completion orders using independent stores, including failed executions, duplicate claim rejection, and unreadable completion state.
FixesGitlawb/zero#962
Amp-Thread-ID: https://ampcode.com/threads/T-01a0dc9c-8bc2-72cc-ad0b-6c64e3578df8
Co-authored-by: Pierre Bruno <[email protected]>
* fix(cron): avoid stale next-run summary on persist failure
Report the next run as unknown when completion state cannot be persisted, as requested in review. Extend the existing corrupt-state regression to verify the summary.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e862-0c9c-753c-9afc-45f9c1082c17
Co-authored-by: Pierre Bruno <[email protected]>
---------
Co-authored-by: Amp <[email protected]>
The pool stopped retrying only on exit 0 or ExitPermanent (76), so the
other codes `zero exec` uses to say a run is over (2 usage, 3 provider,
4 incomplete, 130 interrupted) were treated as crashes and retried up to
MaxAttempts with backoff. Exit 4 means the agent already ran, so each
retry repeated its side effects, re-billed the provider and appended a
second run to the same session stream.
Treat those four codes as final: return the worker's exit code with
ErrPermanent. Crash-type exits (1, signals) and launch/read errors still
retry.
Fixes#1100
Co-authored-by: Claude Sonnet 5.5 <[email protected]>
* fix(redaction): redact adjacent AWS keys glued end to end
Two AWS keys with no separator between them (AKIA...AKIA...) only had the
first redacted: the leading \b in the secret patterns sees a word character
on the left of the second key and misses it.
Apply each textSecretPattern via a helper that also retries at every redacted
span end with a \b-stripped, start-anchored variant of the pattern, so a
credential starting exactly where the previous one ended is still caught.
Chained runs (three or more glued credentials) are followed to the end. A
mid-word occurrence that does not abut a redacted span still does not match.
Extracted from #1075 per Vasanthdev2004's suggestion on #969; #1067 remains
the fix for the RedactString leak.
* Address CodeRabbit critical: bound redactAdjacent span-chaining loop
The outer loop re-read len(spans) while the inner loop appended chained
spans to the same slice, so appended spans were re-chained and the span
count grew exponentially on long glued-key runs (RedactString handles
untrusted content). Bound the loop to the original match count and add a
regression test with 64 glued AWS keys asserting 64 markers promptly.
* fix(cron): bound run history and read recent records from the tail
Retain the newest 1,000 outcomes under the job lock and publish complete history snapshots. Add bounded reverse reads and regression coverage for legacy logs, failures, and concurrent stores.
FixesGitlawb/zero#1086
Amp-Thread-ID: https://ampcode.com/threads/T-01a0dcd6-ad63-74d8-8396-45573c57dcec
Co-authored-by: Pierre Bruno <[email protected]>
* fix(cron): skip oversized legacy history records
Discard entire oversized lines during bounded tail reads so compaction can record new outcomes without weakening atomic publication. Cover exact size boundaries and oversized first, middle, and unterminated final records.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e861-ebee-7764-be1d-e67517cd914c
Co-authored-by: Pierre Bruno <[email protected]>
* fix(cron): append the outcome when the history log cannot be replaced
On Windows the atomic replace of runs.jsonl fails while another process
holds the log open, which dropped every outcome for as long as it stayed
open. Keep the replace, and only when it fails append the record on a
fresh line. The leading newline terminates any unterminated tail; the
blank line it may leave is already skipped by readRuns. Zero's own
readers take the job lock, so they never observe a partial append.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
* fix(cron): preserve the history DACL and append only on sharing violations
Compacting runs.jsonl renamed a temporary file over it, which on Windows
replaced a DACL applied to the log with the job directory's inherited
one. Publish through ReplaceWithRetry so the log keeps its descriptor.
Fall back to appending only when the replace fails with a Windows
sharing or lock violation, the error a reader holding the log open
produces. Any other failure, such as a persistent access denial, is
returned so a log that can never be replaced cannot grow past the
retention limit. fsutil.IsSharingOrLockViolation exposes the existing
classifier, gated to Windows because errno 32 and 33 mean EPIPE and EDOM
elsewhere.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
---------
Co-authored-by: Amp <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
* test(daemon): stop what a worker left running after it was reaped
A daemon worker is launched into its own process group, but Kill and the
Cancel hook pass its PID to background.TerminateProcess, which looks the
group up again with Getpgid when it signals. Once the worker has exited
and been waited, that lookup fails, only the dead PID is signalled, and a
child the worker left running survives. This pins the case with a shell
worker that backgrounds a sleep and exits: after the wait, Kill and the
Cancel hook must still stop the sleep.
Committed ahead of the fix so CI shows it failing on the current
behaviour on Linux and macOS.
* fix(daemon): stop worker process groups by their launch-time identity
Kill and the Cancel hook now go through background.TerminateCommandGroup,
which signals the group the worker was launched into (ConfigureChild-
ProcessGroup's Setpgid) instead of rediscovering it from the PID, so a
worker that has exited, or on Darwin one that is still a zombie, no longer
leaves its children running. It is TerminateCommand without the reap:
the pool waits on its workers itself and exec.Cmd calls Cancel from
inside Wait, so reaping there would be a second Wait.
On Windows it is the same tree kill, with the process handle pinned while
it runs. A command that has already been waited cannot be pinned; that
now reports os.ErrProcessDone, which exec.Cmd's Cancel contract treats
as nothing to cancel, instead of the unexported "already released" error.
Fixes#861
* fix(tui): show provider ownership in recent and favorite models
Display saved provider names in mixed-provider picker groups and include them in row sizing. Preserve provider-grouped rows and selection values.
FixesGitlawb/zero#840
Amp-Thread-ID: https://ampcode.com/threads/T-01a0d8e9-d346-71f9-bdac-4ac762d810a5
Co-authored-by: Pierre Bruno <[email protected]>
* fix(tui): label mixed-group rows with the provider Enter uses
Recent and Favorites rows labeled their owner from recent history, but
selection falls back to the active provider when that owner was renamed
or removed, so the label could name a provider that would not be used.
Derive the label from the same decision selection makes
(modelPickerSwitchOwner) and store it on the row as OwnerLabel.
Cap the owner prefix at 20 cells so an unbounded profile name cannot
consume the model name once the overlay reaches its 76-column maximum.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
* fix(tui): distinguish abbreviated model picker owners
Budget owner cues against the rendered row width, preserve model names, and distinguish abbreviated saved profiles with numbered suffix cues. Show the highlighted full owner as a wrapped key before selection. Cover shared-prefix and shared-suffix owners, narrow terminals, Favorites, and Enter routing.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e862-13e1-756e-9608-58d5fed96f05
Co-authored-by: Pierre Bruno <[email protected]>
* fix(tui): preserve model labels when owner cues cannot fit
Split owner cues onto a second line when the model needs the row width. Check the actually rendered row before suppressing the full owner key, so owner text hidden inside a clipped model label cannot hide that key.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e862-13e1-756e-9608-58d5fed96f05
Co-authored-by: Pierre Bruno <[email protected]>
* fix(tui): fit multiline model rows and mouse targets
Fit the model picker item window to its rendered viewport height and share that window with mouse hit-testing. Account for group headers and two-line rows so clicking either line selects the correct model and the keyboard-selected item remains visible.
Amp-Thread-ID: https://ampcode.com/threads/T-01a0e862-13e1-756e-9608-58d5fed96f05
Co-authored-by: Pierre Bruno <[email protected]>
---------
Co-authored-by: Amp <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
* fix(sandbox): say when a session's sandbox enforcement is degraded
Only `zero doctor` and `zero sandbox policy` reported a degraded sandbox, so
a TUI session or a `zero exec` run went ahead with the native sandbox
effectively off and nobody saw it. Both now say so once, before the first
prompt or tool call, naming the downgrade reason: exec on stderr next to
the grant migration notice, the TUI as a system notice when it opens.
The line comes from the session's own engine through Backend.BuildPlan,
the plan `zero sandbox policy` prints, so the notice and the command that
explains it cannot describe two different sandboxes. Only the degraded
level speaks: a disabled sandbox is the user's choice, and the unelevated
Windows tier still enforces the write jail.
Exec tests that expect a quiet stderr now compare against the notice this
host prints under the default policy, since they run on the host's own
backend and a Linux runner without the helper is degraded while macOS and
Windows are not.
Fixes#1041
* fix(sandbox): say so when the nesting markers skip wrapping
With ZERO_SANDBOXED and ZERO_SANDBOX_BACKEND set, BuildCommandPlan passes
every command through unwrapped on the word of an outer sandbox nothing
verifies (#727). The notice came from Backend.BuildPlan, which does not
model that path, so a native backend reported full enforcement and a
session started with the markers ran every command unwrapped in silence.
The engine's notice now mirrors the runner and names the markers and the
unverified outer sandbox. A disabled sandbox still says nothing.
hostSandboxNotice builds its expectation through the engine too, and the
tests that pin the notice clear the markers so a suite run inside a Zero
sandbox does not change their answer.
* fix(tui): say the startup notices again when a session is resumed
/resume rebuilds the transcript from an empty one, so the startup notices
went with the old transcript and the session it switched to ran under a
degraded sandbox without a word. The model keeps the notices and puts
them back first, before the resume summary. Rewind and compaction stay in
the same session and keep the notice in the scrollback above their
divider.
Also pins why the notice can be decided once, at startup: grants made
during a session only open the network or add allow and deny paths,
never the mode, and every mode but disabled already needs the platform
sandbox, so no grant reaches a degraded command plan the startup notice
missed, and a deny path the backend cannot enforce is refused instead.
* fix(tui): say the startup notices again when /new opens a session
/new cleared the transcript and added only its own note, so the new
session started under a degraded sandbox without a word, the same gap
/resume had. It now puts the notices back first, the way /resume does.
/clear stays in the same session and, like rewind and compaction, keeps
the notice in the scrollback above its divider.
* fix(modelregistry): start no background refresh when the fetch is turned off
The TUI and exec each started a goroutine that called
RefreshModelsDevCache, which reads ZERO_DISABLE_MODELS_FETCH and the
cache path when it runs. A test that turned the fetch off through
t.Setenv could finish and have the setting restored before that
goroutine ran, and the refresh would then fetch into the developer's
real cache. StartModelsDevRefresh reads the setting before starting
anything, so with the fetch off no refresh exists to outlive the test.
* test(cli): keep the degraded-notice tests out of the developer's user state
runExecWithSandbox and the TUI launch test left the sandbox grant store
and the models.dev cache at their defaults, so they read the
developer's sandbox-grants.json and could migrate it, and could refresh
the real models cache. Both now run under isolateCLIUserState, and both
join TestCLILaunchHelpersLeaveUserStateAlone so the isolation is pinned
for each of them.
* fix(cli): stop exec when the degraded-sandbox notice cannot be written
The notice goes out before any tool runs, and before MCP servers and
plugins start through the same sandbox, but a failed write to stderr
was ignored, so the run went ahead with reduced isolation and no
warning delivered. It now stops with exitCrash, the way the image and
reasoning-effort notices in the same function already do.
The lock pinned hono 4.12.27 and @hono/node-server 1.19.14 through
tuistory 0.10.0, and npm audit reports advisories against both. tuistory
already declares ^4.11.7 and ^1.19.9, so the patched releases fit the
existing ranges: hono 4.13.9 and @hono/node-server 1.19.17. package.json
is unchanged.
npm also re-syncs the lock's root entry with package.json, which main had
already changed without regenerating the lock: license MIT, android in
os, and no install script.
* fix(oauth): give every loopback callback server the bounded shutdown
The loopback OAuth callback server was written out three times: the shared
oauth.LoopbackListener, MCP's Login and OpenRouter's login. Each ended with
Shutdown under a one-second deadline, and Shutdown only closes idle
connections. A client that has sent part of a request header is not idle, so it
survived the deadline, and its goroutine and the Serve goroutine stayed alive
after the flow returned (#1028).
#1051 fixed that for LoopbackListener (a header read bound, force-close when
Shutdown runs out of time, and a join on Serve) and left the other two exactly
as they were, because each was its own copy.
The lifecycle now lives once, in oauth.CallbackServer, with #1051's behavior
unchanged: ReadHeaderTimeout of 5s, Shutdown within 1s, Close() on a missed
deadline, and Close returning only after Serve has. LoopbackListener uses it
through an unexported constructor that keeps its ConnState test hook, and MCP's
Login and OpenRouterLogin use StartCallbackServer, which both packages could
already import.
Each call site gets a regression test that stalls a raw client mid-header ahead
of the real callback and then requires the stalled connection to be closed once
the login returns. The ordering does the synchronising: the stalled client is
dialled before the real callback, Serve accepts in order, and the login cannot
finish until the real callback has been served, so no sleep is involved.
Fixes#1028
* test(oauth): pin the callback server's header bound and its join on Serve
Two of the three things CallbackServer does had no test that noticed their
removal, the same as when #1051 introduced them on LoopbackListener: the
five-second header read bound, and Close waiting for the Serve goroutine.
The header test stalls a client mid-header and never calls Close, so it can
only pass on ReadHeaderTimeout; the bound is a package var so the test can
shorten it. It synchronises on StateNew. Go reports StateActive only after a
request read returns, and a stalled header read returns only when the header
timeout fires, so a helper waiting for StateActive depended on the bound under
test and turned its removal into a SETUP INVALID instead of a failure.
The join test uses the case where the join matters: with no connections open,
Shutdown closes the listener and returns at once, so without the join Close can
return before Serve does. Without it the test failed 10 runs out of 10.
* fix(acp): publish sub-run sessions loaded over ACP as view-only
session/load registered a fully promptable session for any session kind,
including the child, side and spec sub-runs that every other resume path
refuses through sessions.IsResumableKind: the TUI, exec sessions, the exec CLI,
and ACP's own session/resume. So a sub-run could be opened and continued as a
standalone conversation through ACP and nowhere else, with whatever its parent
run relied on about that sub-run's lifecycle no longer true.
Load has a real render-only use, a client rebuilding a transcript it can scroll,
so it still loads those sessions and still replays their history. What changes
is that the session it publishes for a non-resumable kind is marked read-only,
and session/prompt refuses it with an invalid-params error that says why.
runTurn is reached only from session/prompt, so that one gate covers every way a
turn can run; set_mode and the config options only change settings.
The flag is decided at first registration and never changes. Resume refuses a
non-resumable kind before it registers anything, and session/new always creates
a standalone session, so the only path that can publish one of these ids is
load, which now publishes it read-only.
TestACPResumeAppliesStandaloneSessionKindPolicy claimed "session/load should
retain render-only access", but its prompt check ran before the load, so it only
proved the refused resume never registered the session. It now prompts after the
load as well. TestACPLoadPublishesSubRunsReadOnly checks every kind in both
directions, so a gate that refused everything would fail on the resumable kinds,
with a setup guard that the table agrees with IsResumableKind.
Fixes#1045
* test(acp): require resumable sessions to be promptable after load, not merely unrefused
TestACPLoadPublishesSubRunsReadOnly decided the outcome from the error text:
"refused as view-only" was false for any error other than the view-only one, so
a regular or fork session whose prompt failed for some other reason still
passed. With every writable prompt made to fail with an internal error, the old
test stayed green. Raised by CodeRabbit on the PR.
Resumable sessions now have to prompt successfully. For the sub-run kinds the
decision is the RPC code, and the message is checked only to tell the view-only
refusal apart from other invalid-params errors such as an unknown session id.
* fix(mcp): forward image tool results on the existing image channel
Recreated from Gitlawb/zero#1012 (approved but unmerged).
Original: euxaristia/zero:feat/823-forward-mcp-images
* test(mcp): pin the pre-decode encoded cap and MIME sniffing guards
Adds the two reviewer-requested tests for the untrusted-input boundary:
- TestOverCapEncodedPayloadIsRejectedBeforeDecoding: a payload one byte
over EncodedLen(MaxImageBytes) is rejected and the decoder is never
called, so a hostile server cannot force arbitrary decode work.
- TestDeclaredMimeTypeDisagreeingWithSniffedBytesIsDropped: an HTML
payload declared as image/png is sniffed from its bytes and dropped
rather than forwarded to the provider as an image.
Both mutations (removing the cap, trusting the declared mimeType) were
verified to turn their test red locally.
* fix(agent): retry mid-stream transport aborts before tool dispatch
Recreated from Gitlawb/zero#1014 (approved but unmerged).
Original: euxaristia/zero:fix/973-mid-stream-transport-retry
* Address CodeRabbit nit: add Unix ECONNABORTED needle to shouldReconnect
midStreamAbortNeedles claims to be a subset of shouldReconnect but listed
'software caused connection abort' (the Unix/macOS ECONNABORTED text),
which shouldReconnect did not match. Add the needle alongside the other
abort phrases, and cover it in TestShouldReconnectClassification.
* fix(search): redact every text field of session metadata, not ten of thirty
`zero search --json` prints Hit.Session straight to stdout and redactMetadata
covered ten of its string fields. Filling every string-kinded field of
sessions.Metadata with a credential and walking the result names twenty that
came back carrying it: SessionKind, Tag, RootSessionID, AgentName, TaskID,
SpawnedFromEventID, SpecID, SpecFilePath, SpecStatus, SpecDraftModelID,
SpecDraftReasoning, SpecUserComment, SpecRejectReason, SpecSourceSessionID,
SpecImplSessionID, and the five on the Goal (Objective, Status, StatusReason,
CreatedAt, UpdatedAt).
The ones carrying the most free text are the ones a hand-kept list is least
likely to reach: SpecUserComment and SpecRejectReason are review text the user
typed, SpecDraftReasoning is the model's own prose, and Goal.Objective and
Goal.StatusReason are however the goal was phrased.
Goal is redacted through a copy. Metadata is passed by value but Goal is a
pointer inside it, so redacting in place would edit the session the caller still
holds, and a search would quietly rewrite the live record it was only asked to
report on.
TestRedactMetadataCoversEveryTextField walks the struct rather than repeating
the list, so a string field added to Metadata or Goal later fails on the day it
lands. It carries a SETUP INVALID guard that the chosen secret is a shape
RedactString actually recognizes, since otherwise every leg would pass
vacuously, and a second guard that the walk visited any fields at all.
* test(search): make the metadata coverage guard recurse, so nesting cannot slip past it
The first version of the guard walked only the direct string fields of
sessions.Metadata and hand-allocated the Goal beside it. That left it with the
blind spot it exists to close: a nested struct or pointer field added later
would arrive empty, the walk would find nothing in it, and the test would pass
while the new field went out unredacted. Raised by CodeRabbit on the PR.
Filling is now recursive through structs, pointers and slices, allocating what
it finds nil, and the walk is its mirror. The test compares the two counts, so a
shape one traversal understands and the other does not fails as SETUP INVALID
rather than passing quietly.
Both helpers fail rather than skip on a kind they do not understand. A field the
guard cannot populate is a field it is not guarding, and saying so at the point
it is added is the whole value of having it.
`go test ./...` is red on a developer machine and green in CI, for two tests
that take an input from the machine rather than from the test.
TestResolveReportsExplicitMaxTurns left ResolveOptions.Env nil on four of its
five cases, and a nil Env means "read the real process environment" (envValue).
Any one of the ten variables the resolver reads is enough to break it. With
ANTHROPIC_BASE_URL exported, applyProviderEnv synthesizes a second, keyless
"anthropic" profile from a base URL on its own, the single-provider shortcut in
normalizeProviders no longer applies because there are now two, and Resolve
returns `no active provider configured: active provider "" not found` before the
test reaches its assertion. Measured one variable at a time against a clean
environment, all ten fail the same test: ANTHROPIC_BASE_URL, ANTHROPIC_API_KEY,
ANTHROPIC_MODEL, OPENAI_API_KEY, OPENAI_BASE_URL, OPENAI_MODEL, GEMINI_API_KEY,
GOOGLE_API_KEY, ZERO_PROVIDER and ZERO_MAX_TURNS.
The cases now state an empty environment instead of no environment, which is
what they meant, and the test exports three hostile values itself so it proves
that rather than relying on the machine it runs on being clean. Reverting either
changed case fails the test on a clean environment too, so this cannot regress
silently on CI.
TestAltScreenTranscriptScrollKeepsFooterFixed asserted the branch survives in
the title bar at width 90 while leaving Options.Cwd empty, so newModel took the
real working directory. The outcome is not a monotonic function of that path's
length, which is why this took a measurement rather than a guess. The title bar
picks the widest candidate that fits: workspace carries branch and path, cwdOnly
carries the path alone, compactLeft carries the branch alone. At width 90 with
this branch, a cwd of up to 52 bytes keeps both, 53 to 74 falls to path-only and
drops the branch, and 75 or more falls again to branch-only and restores it. The
test failed from a checkout whose package path is 74 bytes and passed from one
at 80; CI sits at 40. Cwd is now pinned, which also stops newModel reading the
real repository for a branch the test overwrites anyway.
No production code changes. The resolver's nil-Env contract and the header's
candidate ladder are both deliberate; only the tests were leaning on them.
* fix(redaction): redact credentials split by an invisible character
Every shape matcher in RedactString describes a contiguous run of body
characters, and none of their classes admits a control or format character.
One NUL or ESC dropped into the body of a key therefore ended the match and
both fragments were emitted untouched. Whoever read that output next saw the
original key back, because nothing about those characters was ever displayed:
a terminal eats the escape, a log viewer or JSON consumer strips the control
byte, and a copy-paste drops both.
The matchers now also run over a copy with those characters removed, and the
spans they find are mapped back so the replacement covers the original bytes,
separators included. Emitting the compacted copy would hand the caller a
different string than the one they passed in, and leaving a separator behind
would read as two secrets where there was one.
Written against the character classes rather than the two bytes in the report,
because every other Cc and Cf character behaves the same way: BEL, backspace,
DEL, the C1 controls, zero-width space and joiner, word joiner, BOM and soft
hyphen all split a key on main. Tab, newline and carriage return are
deliberately excluded and a test says so: they are real text structure, a
credential broken across a line is visible to whoever reads it rather than
hidden from them, and stripping them would let one match span a line break and
replace unrelated lines.
Text with no such character takes exactly the path it took before, decided by
a byte scan that allocates nothing, so the common case is unchanged at 51
allocations. Text that carries control characters and no credential pays for
one compacted copy and a second pattern pass.
Fixes#969
* test(redaction): assert the exact output around a malformed byte
The malformed-UTF8 legs only asked whether the key was gone, so a decoder that measured an invalid byte as the three bytes of U+FFFD still passed: the replacement covered the secret and moved its edges onto the bytes beside it. They now assert the whole string. One leg also records that an invalid byte is not one of the invisible separators, on the same ground as tab and newline: it decodes to U+FFFD and is drawn.
* fix(redaction): keep credential boundaries while matching across invisible characters
The split-credential pass compacted every Cc and Cf character out of the whole
string before matching, which removed delimiters as well as filler. Two things
followed. A whole credential written after a NUL lost the word boundary the
shape matchers open with, so "prefix<NUL>AKIA...<ESC>..." stopped redacting even
though the unsplit "prefix<NUL>AKIA..." has always redacted. And with the
delimiter between two credentials gone, the unbounded JWT shapes ran out of the
first key and through the header of the next, replacing both with one marker and
exposing the second token's payload and signature. The same reach consumed
ordinary prose written after a key.
Three changes, one rule: a separator is filler inside a credential and a
delimiter outside one, and the difference comes from the source rather than from
the compacted copy.
The pass now runs after the contiguous matchers instead of before them, over
what they left, and never across a replacement. A whole credential is claimed by
its own strict match, so neighbours stay separate and the delimiter between them
survives. The shape matchers used here drop their leading word boundary, and the
byte before the match's original start is what has to be a non-word byte, so a
key after a NUL redacts and a key glued to the end of a word still does not.
The offset table is gone with them. It carried a start and an end for every
source byte, which is sixteen bytes per byte of input on a 64-bit build, and
RedactString is handed whole command output and notification bodies with no
bound in front of it. Only the endpoints of actual matches are resolved now, in
one walk. On a 4 MiB input with a split key, the split path allocated 223.6 MB
before this change and 154.9 MB after, against 146.3 MB for the same text with
no separator in it.
Tests cover a delimiter in front of a split key against its unsplit verdict at
every kind of leading byte, each supported shape placed in front of a JWT with
each class of separator, text immediately behind a key, a caller-supplied
replacement made of word characters, and the allocation ceiling.
* fix(redaction): match split credentials with gap-tolerant shapes on the original text
CodeRabbit found a bypass in the compaction approach:
"xAKIA<NUL>AKIAIOSF<SOH>ODNN7EXAMPLE" leaked while its unsplit counterpart
redacts. With the separators removed first, the leftmost AWS match began at the
decoy AKIA, was then rejected for the "x" in front of it, and had already
consumed the start of the real key, so the real key was never considered. The
ordinary-prose version is any word ending in "sk" followed by a hyphen, "ask-"
or "task-", ahead of a split OpenAI key.
Resuming the scan one byte after each rejection would close that but is
quadratic for the greedy shapes on text like "ask-ask-ask-..." with one NUL in
it. So the boundary goes back inside the pattern instead. Each shape is rewritten
through regexp/syntax so a run of separators may sit between any two characters
it consumes, but never before the first, and matched against the ORIGINAL text
with its leading \b intact. RE2 then judges the word boundary against the real
neighbour in one linear pass, and there is no compaction and no offset mapping.
The separator class is generated from the same Unicode tables as
splitSecretSeparator.
Two further cases came out of rerunning the seeded differential against main:
- The shapes are matched independently and the union of their spans replaced.
Applied one after another, a split key's unbounded body ran on across a
separator into a following JWT, stopped at its first dot, and left the JWT
shape nothing to match, exposing its payload and signature.
- A second pass, ungated, catches a credential glued onto a split one, which
gains its word boundary only once the first is replaced. It is often whole
itself, so its region can hold no separator by then.
On 6,000 control-free inputs the output is byte-for-byte main's. On 6,000 with
invisible characters, 147 outputs still hold a known credential once those are
removed, against 2112 on main; 140 of the 147 leak on main too when the input is
unsplit, and the remaining 7 are the one limit now written into the header
comment: a split key whose first fragment is a complete credential on its own,
with another credential glued onto its tail.
* fix(redaction): keep two split credentials of one shape apart
Matches of one pattern never overlap, so when a split credential's body ran
on through the separator after it, a following credential of the same shape
could not start a match of its own. A JWT body stops at the next token's
first dot and a classic GitHub body at its underscore, so two split JWTs
side by side left the second one's payload and signature in the clear.
A match the OpenAI filter rejects spans what it ran into as well, so a
split key after kebab-case prose was rejected with the prose and printed.
When a match runs through a separator run into a position where the same
shape matches again and reaches at least as far, that credential is taken
on its own, and the first is cut at the separators when it is a complete
match by itself. Each piece is filtered on its own. Candidates are tried
latest first and only where the shape's literal prefix follows the
separators, at most eight per match, so the pass stays linear.
The header comment now says plainly that the rest of a key whose front
part is complete alone stays visible, and drops a stale sentence from
before the shapes were matched independently.
* fix(redaction): recognise the next credential when its prefix is split too
The run-on search looked for a shape's literal prefix contiguously after
the separators, so a token split inside its first few characters was never
seen as a new start and its payload leaked again. The prefix is now matched
with gaps allowed between its characters, the way the shapes are. The new
test splits the second token's header at every character, which also
pins that the bounded search is spent only where a credential can begin.
* fix(redaction): take one run-on step per match, not the whole chain
Following a run-on from the second credential into a third, and on to the
end of the chain, did that walk again from every match inside the chain,
which is quadratic: a megabyte of split JWTs back to back allocated about
seventeen gigabytes and ran for ten minutes. Each match now separates only
the credential it ran into. The scan's own next match starts inside that
credential and takes the following step, so a chain is still covered, and
in chains of three or more the markers after the first pair can join.
A test bounds the allocation on a chain of split JWTs; the linear pass uses
about forty times the input and the walk used far more.
* feat(notify): make permission-prompt alerts on by default and discoverable
The notify system existed but was silent unless the user hand-edited
config.json, with no UI surface to discover or change it.
- resolver: fall back to mode=both, focusMode=unfocused when the notify
block is missing or empty (Fixes#579)
- tui: add /notify slash command with popup picker, mirroring /theme;
explicit choices persist via config.SetNotify
- cli: add `zero config notify` to read/update/reset the preference
(--mode, --focus, --reset, --json)
- config: add SetNotify writer using the existing atomic-write helper,
validating against the same vocab the resolver accepts
The TUI effectiveTUINotifyMode default (empty -> both) now matches the
resolver. exec_test.go seeds notify.mode=off where a test asserted
silent stderr, which the old empty-default implicitly provided.
* fix(notify): apply review feedback from CodeRabbit
- cli: omitted --mode/--focus flags now preserve the current resolved
value instead of wiping it (--reset remains the only clearing path);
aligns the CLI with the TUI's mode-only preservation behavior
- tui: reject /notify inputs with more than two tokens instead of
silently accepting them
- tui: apply /notify choices to the live notifier via the new
notify.Notifier.Configure, so the change takes effect on the next
permission prompt in the same session (the previous message claimed
this but only the persisted value was updated)
- notify: add Notifier.Configure (mutex-guarded policy swap that
preserves sinks, focus state, and the writer)
- tests: mode-only/focus-only CLI preservation, live-notifier apply,
trailing-argument rejection, Configure immediate-effect + sink
retention
* fix(notify): read cfg under the lock in Notify
Configure (334c688) made cfg mutable at runtime, but Notify still read
n.cfg.Mode before acquiring n.mu — a data race with a concurrent
Configure. Move the mode check inside the critical section and copy cfg
to a local for all reads.
Regression test TestConfigureConcurrentWithNotify runs Configure
concurrently with Notify; verified it reports DATA RACE on the unfixed
code and passes after the fix (go test -race -count=5).
* fix(notify): keep defaults out of the resolver; preserve the user's own values
Addresses the maintainer review (Vasanthdev2004) on PR #1001. All three
findings share one root cause: the resolved config was treated as if it
were the user's choice.
- resolver: no longer defaults notify.mode/focusMode. The TUI's
effectiveTUINotifyMode already maps empty -> both on its own, so the
permission-prompt alert still works out of the box, while headless
`zero exec` stays byte-identical to base (no BEL/OSC-9 on stderr under
-o json), the exec empty-stderr fixture is restored, and the
ZERO_NOTIFY_WEBHOOK_URL sink is not armed by an implicit default.
- cli: `zero config notify` seeds omitted fields from the user's own
file (new config.UserNotify), never from the resolved view — a
project config's mode:off can no longer be copied into the user's
global config, and blank stays blank instead of pinning today's
default as an explicit choice. --reset remains the only clearing path.
- tui: /notify mode-only changes preserve the focus stored in the
user's own file (blank stays blank); the /notify picker enumerates the
full 4x3 mode x focus space so every valid pair is a row, the current
pair is always preselected, and Enter can never commit a setting the
user did not choose. State view reads the stored pair.
- tests: the three regressions from the review — exec writes nothing to
stderr on a clean run (fixture restored + resolver empty-default
test), a ProjectConfigPath test proving project notify cannot leak
into the user file, and Enter on an open picker from a pair outside
the old curated list (off, always) keeps the setting unchanged.
* fix(cli): zero config notify no longer requires a configured provider
The command resolved the full config (providers included) before
touching notification settings, so a fresh user with no provider hit
ErrNoActiveProvider and could not read, set, or reset their notify
preference — the exact first-run user this feature targets (CodeRabbit
review, PR #1001).
The command manages a user preference, so it now talks only to the
user's own config file (config.UserNotify / config.SetNotify) and never
runs config resolution. Display reports the user's stored values — a
project config that overrides notify for one repo is not shown here,
matching the write path's source-of-truth from the maintainer review.
* fix(notify): separate live session policy from persisted user preference
Addresses the review from jatmn (PR #1001). The three notification
states — stored user file, resolved startup pair, live session — are
distinct contracts and must not be conflated:
- handleNotifyCommand now keeps two focus values per update: the LIVE
focus (in-session, initialized from the resolved pair, so a project's
focus rule keeps applying) feeds the notifier's Configure and the
status line; the PERSISTED focus seeds from the user's own file
(config.UserNotify), so a mode-only /notify writes the mode while the
user's focus value — blank included — is preserved and project values
never leak into the global file.
- /notify list reports the live pair (model fields), not the stored
file, which can legitimately disagree under a project override.
- the /notify picker preselects the live pair; committing a row is an
explicit choice, so Enter on the preselected row keeps the setting.
- /notify list requires exactly one token, matching /theme, /effort,
and /style (rejects "/notify list junk").
- shell completions: config gained a notify child with its flags
(--mode/--focus/--reset/--json), plus a completion-tree assertion for
both contexts so new CLI surfaces cannot go stale.
- help/copy pass: the CLI help now describes the stored global
notification preference controlling BOTH the completion and
needs-input alerts, --reset as returning the TUI to its effective
default (headless unconfigured stays silent), lists all three focus
values in /notify usage, and drops the stale "resolver default"
wording from effectiveTUINotifyMode and the command doc.
- tests: contract-matrix coverage with the three states deliberately
disagreeing (blank user file + project-resolved off/focused) asserting
list/picker/preselection/Enter, mode-only and explicit-pair updates on
both live and persisted sides, and restart precedence; the SA4006
lint hits are resolved by asserting the returned live model.
lint-static: 0 issues. Affected suites green under -race; the only
remaining local failures are the known environment flakes
(TestRunAuthOpenRouterSavesMintedKey on macOS keychain under a
sanitized env, and TestLoadProviderCommand* 5s subprocess timeouts
under full-suite load — both pass in isolation and fail identically on
clean origin/main here).
* fix(notify): close the remaining review findings (isolation, races, fidelity)
Addresses the third review round (PR #1001) as one boundary pass:
- BTW isolation: /notify mutations and the picker are now behind the
existing btwCommandUnavailable guard (like /theme), because the side
conversation shares the parent's notifier pointer — an unblocked
/notify off inside BTW reconfigured the parent and wrote the global
file while only the side's display changed. /notify list stays
available. Regression: side-conversation attempt then return, checking
the parent's actual notification output and the stored preference.
- Serialization: new UpdateNotify runs read-merge-write as ONE
transaction under a cross-process advisory lock (flock/LockFileEx on a
.lock file beside the config, credstore's pattern), so two concurrent
partial updates (--mode off vs --focus always) cannot interleave and
silently undo each other's explicit change. The CLI and the TUI's
mode-only persistence both go through it; the TUI's stale pre-read of
the stored focus is gone (the merge reads under the lock). Verified
0/30 lost updates driving the real binary with two concurrent
processes per round; a goroutine-level regression test asserts both
explicit fields survive.
- Fidelity: notification writes now edit ONLY the notify member's bytes
(setNotifyJSONObject, on the existing byte-preserving JSON editor),
so unrelated values survive with their explicit presence intact —
including tools.deferThreshold: 0 and MCP disabled: false, which the
typed serializer's omitempty cannot round-trip, and keys FileConfig
does not model at all. A reset removes the notify member instead of
leaving a {} husk.
- Startup recovery: Resolve's ErrNoActiveProvider partial result now
carries the parsed non-provider fields (notify, sandbox, tools, ...),
and the setup-recovery branch that clears the resolved config keeps
the notify block — a stored explicit mode:off no longer resurrects
alerts through the provider-recovery path. Regression: stale
activeProvider + stored off/always through recovery asserts the
launched TUI receives the opt-out, not the both/unfocused default.
- docs nits: zero config --help now describes both alert classes; the
two remaining "resolver default" wordings updated.
lint-static: 0 issues. Affected packages green under -race (the only
local failure is the known TestRunAuthOpenRouterSavesMintedKey macOS
keychain flake, which fails identically on clean origin/main here).
* fix(config): notify reset removes every duplicate notify member
A hand-edited config may contain duplicate notify members (JSON decoders
tolerate them; the last occurrence wins). The reset path removed only
the final member, so an earlier block survived and became the effective
preference on the next decode — `zero config notify --reset` reported
success while the old value still applied (CodeRabbit review, PR #1001).
Loop the removal until no notify member remains, mirroring
setPetPreferenceJSON's duplicate handling. The replace path still edits
the LAST member, which is the one a decoder treats as effective, so an
earlier duplicate stays inert there.
Regression TestSetNotifyResetRemovesEveryDuplicateNotifyMember verifies
the reset-orphaned failure output on the pre-fix code and passes after;
it also pins the no-op reset and the last-member-replace behavior.
* fix(config): join UpdateNotify release error; restore main's test surface
CodeRabbit pass on the rebased head, three findings:
- UpdateNotify discarded the advisory-lock release error while every
other writer joins it into the result (errors.Join). A failed unlock
would leave later config updates blocked while the caller is told
success. Now named-result + deferred Join, matching SetPet et al.
- The rebase had resolved model_test.go by taking the pre-rebase file,
which silently dropped every test upstream added or changed since the
fork point (composer blink/cursor suite, beginRun run-details,
scrim-preserves-semantic-colors, session payload, Termux burst, ...).
Rebuilt the file from origin/main plus only this branch's own additions
(TestEffectiveTUINotifyMode), so main's newer assertions are back:
the stale scrim assertion now expects semantic ANSI preservation, and
the removed TestBeginRunResetsSidebarHidden stays removed exactly as
upstream had already replaced it with TestBeginRunKeepsRunDetailsClosed.
* fix(tui): restore pickerTheme's transcript append; pin the picker failure path
Rebase regression (Vasanthdev2004, PR #1001): resolving the choosePicker
conflict attached the shared transcript-append line to the new
pickerNotify case, so the pickerTheme case lost it — a theme choice
whose preference could not be saved fell out with the session theme
changed and nothing reported, while the /theme text path kept its line,
so no test went red. Restore main's line in the pickerTheme case.
New regression TestThemePickerAppendNoteOnFailedSave drives the exact
reported shape: UserConfigPath under a regular file (the write must
fail), a pickerTheme item, choosePicker, and the could-not-save note
asserted in the transcript. Verified it fails on the pre-fix code with
the reviewer's own repro output and passes after.
Also restores the TestEffectiveTUINotifyMode comment to the post-review
wording (the default lives in the TUI, not config.Resolve, so headless
runs stay silent); the model_test.go rebuild had pulled the older
pre-notify phrasing back in.
* test(tui): restore the default palette after the picker regression test
Vasanthdev2004 follow-up (PR #1001): handleThemeCommand applies its
theme to the package-level zeroTheme, so TestThemePickerAppendNoteOnFailedSave
left dracula installed and the next palette-sensitive test
(TestFocusedPermissionSelectedRowUsesSelectionTintNotBrandChip) read
dracula's selection tint instead of the default. One line, the package's
existing convention: defer applyTheme(themeDark, true), as every other
theme-committing test already does. Verified the tint test fails in
sequence before the defer and the full internal/tui suite passes under
-race after.
* fix(tui): label the effective-default notify mode as "(default)"
jatmn round (PR #1001): the TUI printed the effective default mode as a
bare "both" while the new CLI labeled the same configuration "mode:
(default)" — the two first-party surfaces contradicted each other, and a
user could not tell whether "both" was stored or effective. Focus
already labeled its built-in default ("unfocused (default)"); mode now
mirrors it.
The model keeps the RAW configured mode (notifyConfiguredMode) alongside
the effective one, so state output can distinguish "both because the user
chose both" from "both because nothing is configured" — clearing to the
raw value when an explicit choice is made. effectiveModeLabel renders the
(default) tag on the /notify list card and the /notify change line, the
same wording the CLI read path uses. No defaults are persisted; resolver
and headless behavior untouched.
Regression TestNotifyStateLabelsEffectiveDefaultMode pins the contract:
unconfigured shows "both (default)", explicit stored "both" renders bare,
an explicit /notify bell choice renders bare in both the change line and
later state. Verified it fails on the pre-fix code and passes after.
* fix(config): one lock authority; full-replace across duplicate notify keys
jatmn round (PR #1001), two findings plus one documented precedence:
- UpdateNotify took a SECOND lock primitive over the same config.json.lock.
On Windows its LockFileEx range did not overlap lockutil's, so notify
writes could race provider/theme/credential/MCP writes and silently
discard them; on every platform it also bypassed the established
no-follow path checks and the bounded wait. Delete filelock_unix.go /
filelock_windows.go entirely and use the existing lockConfigFileFn
authority every other mutator already shares. New regression
TestUpdateNotifyRacesExistingConfigMutatorWithoutLoss holds notify vs
SetTheme and requires both acknowledged edits to survive; the real
binary also races 30/30 clean cross-process on the shared lock.
- setNotifyJSONObject replaced only the last duplicate notify member, but
Go's decoder MERGES the fields of duplicate members into one struct, so
a replacement omitting a field inherited it from an earlier duplicate
({"notify":{"mode":"off"},"notify":{"focusMode":"focused"}} +
SetNotify({FocusMode:"always"}) left mode "off" effective). The
nonempty path now removes every duplicate and inserts the replacement
fresh — full replace semantics, nothing inherited. UpdateNotify's
partial merge is unaffected (it seeds omitted fields before the edit).
Regression TestSetNotifyReplacementInheritsNoFieldFromEarlierDuplicate,
verified failing on the pre-fix editor and passing after.
- zero config notify --reset combined with --mode/--focus now documents
its precedence: --reset wins, explicit values are ignored (the help
previously did not define the combination).
* fix(config): replace single notify member in place; reserve wipe for duplicates
Vasanthdev2004 review note (PR #1001, non-blocking): the duplicate-key fix
ran remove-and-reinsert for EVERY notify write, including the ordinary
single-member file — insertJSONMember appends after the last member, so
every /notify toggle moved the notify key to the end of the config object
and collapsed its formatting while everything around kept theirs.
Nothing was lost (member order carries no meaning), but a byte-preserving
write should not reformat what it need not touch.
setNotifyJSONObject now counts notify members first: exactly one is a
positional value replace (key stays where the user put it, surrounding
bytes untouched); two or more keeps the remove-every-duplicate-and-insert
path, where Go's field-merging decode would otherwise let an earlier
block leak fields into the replacement. Regression
TestSetNotifySingleMemberReplacedInPlace asserts exact bytes through an
in-place replace, verified failing on the over-eager version and passing
after; the duplicate regression and the whole config suite stay green.
The runtime root is a write root, so a sandboxed command can replace
cache, data, tmp or any of the listed grandchildren with a link to a host
directory. The fallback root is derived rather than minted, so the next
Zero process prepares that same tree unsandboxed, and it created and
chmodded every child by pathname after checking only the root and its
ancestors. It followed the planted link: a contained command could get
Zero to create npm inside a host directory and change that directory's
mode.
Everything below the root now goes through os.Root: each child is
created relative to the root's handle, which refuses any component that
leads out of it, a child that is a link at all (a junction on Windows) is
refused as an aliased component, and on Unix the mode is set on the
opened directory rather than on a name. The same path serves the
preferred cache root, which had the same pathname operations.
Reported by @jatmn.
The setup lock's release discarded both the unlock and the close error,
and elevated setup defers it after publishing the marker, so the operator
was told setup succeeded without anything knowing whether the lock had
been let go. The release now always runs both halves and returns whichever
failed, setup reports it and exits non-zero on top of the result it
already had, and the marker-only writer that holds the same lock returns
it too. The transaction order and the lock file name are unchanged.
Reported by @jatmn.
Commands and zero doctor read the runtime stamp without taking the setup
lock. The writer opened the existing stamp with FILE_OVERWRITE_IF, which
truncates at open time, and only then protected and wrote it, and rollback
deleted the stamp before recreating the previous one. A reader in either
interval saw an empty, partial or missing stamp on a setup that was still
valid, and a process that stopped there left it that way.
The new contents now go into a fresh file beside the stamp, created under
an unpredictable name with the protected DACL already in place, and that
file is renamed over the stamp through its own handle with POSIX
semantics. Rollback restores through the same publish instead of deleting
first. The validation read opens the stamp with FILE_SHARE_DELETE so a
publish can replace it while it is open; a reader without share-delete
gets a short bounded retry and then a failed setup with the previous stamp
whole and no replacement left behind.
Reported by @jatmn.
The Windows runner's TEMP is an 8.3 short path, so the raw t.TempDir()
spelling digested differently from the canonical one the record check
computes, and the test stopped at its own setup guard. Selection hands
the fallback a canonical workspace, and the test now does the same.
Resolve the stamp reader before the stamp is opened. FILE_OVERWRITE_IF
truncates at open time, so a reader that could not be established returned
an error with the previous stamp already emptied, or a new empty one left
behind. The pathname entry point now delegates to the handle writer instead
of carrying its own copy of the sequence.
Take the Windows user scope for the fallback runtime root from the token's
user SID rather than USERNAME. Setup and a later command derive that root in
two processes with independent environments, and a USERNAME that differed
between them made the command reject the root setup had recorded.
On Unix, report a created runtime directory that exists but cannot be
identified instead of treating every lookup failure as a clean undo, which
is the line the Windows build already draws.
Move runtimeTailNotOwned and windowsSameRuntimeRootPath beside their only,
Windows-only callers so they are no longer dead code on other platforms,
replace a deprecated filepath.HasPrefix in a Unix test, and bind the
junction test's planted flag to mklink's result so a failed mklink skips
instead of failing a correct acquisition.
With the type rule removed the guard-page legs still passed, because their short tails are stopped by the size rule as well. An entry in another layout can be long enough to hold the SID its would-be count claims, and then only its type says it is not a plain ACE. This drives the admission function with a whole SID at the plain offset for eight ACE types and admits only the two plain ones.
Two findings from jatmn, both on code this branch added.
The diagnostic that answers "would a command still select the recorded
runtime root?" spelled out the same steps as selection and was missing one.
os.UserCacheDir can succeed and still return a value that canonicalizes to
nothing, and selection fails closed on that with "user cache directory is
unavailable" while the diagnostic went on to derive candidates from the
empty root. Doctor could then report the recorded root as current or as
stale on a machine where every command fails before it reads a marker. Both
callers now take their inputs from resolveSandboxRuntimeRootInputs, so an
input one refuses is refused by the other with the same error. A sandbox
test drives both entry points through the resolver seam and requires
identical errors; a Windows doctor test sets LOCALAPPDATA to "." and
requires runtime-root-unresolved with the resolver cause.
The ACL attestation cast every DACL entry to the plain ACE layout and
compared whatever sat at the SID offset before looking at the entry's type.
Only ACCESS_ALLOWED_ACE and ACCESS_DENIED_ACE keep a SID there; an object
ACE has a flags word and GUIDs, and EqualSid is undefined for a structure
that is not a SID. An entry is now admitted on its header, type first and
then a size that can hold the SID its own count claims, before any byte past
the header is read. Other layouts are skipped, so an allow that is not read
is not credited and the worst case is one idempotent re-apply. The decision
is split from the filesystem read so it can be tested against an ACL built
in memory.
That test places each ACL against a PAGE_NOACCESS page with an entry whose
tail spells the start of the trustee's SID, so a reader that believes the
count walks into the guard page. Each leg runs in a child process, because
an access violation cannot be recovered and would take the package's other
tests down with it.
The fallback tests and the shared runtime fixture still had skips for a missing
user cache directory or an underivable root. With the cache and temp both owned
by the test those cannot be explained by the machine, and a skip there is how a
test passes without reaching its assertion. They are setup failures now, and
the recorded-fallback test owns its cache directory too.
The setup lock orders setups against each other and says nothing to an
eviction, which takes the runtime lease exclusively. This runs the control
against the real transaction: parked between its stamp and its marker, the
root reports in use, and it is free again once setup returns.
Five findings from review, at the boundaries between stages.
- The runtime stamp is named after the plan it attests. The runtime root is
chosen per workspace and the plan is specific to a sandbox home, so two homes
for one workspace shared one stamp file and each setup invalidated the other.
Each plan now has its own attestation, and a failed setup can only reach the
stamp of the plan it was applying. Eviction still takes them all.
- One setup at a time per sandbox home, through a lock file held from before the
snapshot until the marker is published or every compensation has run. The
shared runtime lease keeps eviction out but two setup helpers can both hold
it, so the stamp and marker of different setups could interleave, and a failed
setup could restore its snapshot over a later success.
- The native smoke test prepares its commands the way the planner does instead
of sending the bare profile to the command builder, which the elevated runner
refused before any probe ran. A portable test pins the agreement through the
real builders and parsers, starting from a bare profile.
- Test fixtures redirect TMPDIR as well as TMP and TEMP and check containment
before creating anything, so the fallback tests no longer write outside their
own temp on Unix or skip past their assertion.
- Stamp compensation classifies absence with isWindowsNotFound, which knows the
native statuses the no-follow open returns.
The elevated helper's transaction now runs in tests behind two seams (the
elevation check and the WFP install), so the regressions drive the real lock,
lease, snapshot, ACL apply with the riding stamp, marker and compensation, and
ask a later command prepared by the planner whether it validates.
* fix(providerio): close idle connections periodically to prevent HTTP/2 PROTOCOL_ERROR
OpenRouter and other HTTP/2 backends can silently reset or drop connections
that sit idle in Go's client connection pool across user turns. When a
subsequent turn reuses the stale connection, it fails on stream connect with
PROTOCOL_ERROR.
Launch a background idle connection closer on non-darwin platforms to evict
idle pooled connections on a 30-second interval, forcing fresh handshakes
after idle pauses while preserving connection reuse across fast agent turns.
Refs #1057
* test(providerio): poll for idle connection eviction with bounded deadline
Replace the fixed 60ms sleep in TestStartIdleConnCloserClosesIdleConnections
with a 2-second bounded polling loop to avoid test flakes under heavy CI
load or under the race detector.
Refs #1057
---------
Co-authored-by: euxaristia <[email protected]>
* feat(execution): add a stdin-only write for interactive processes
* feat(tui): attach overlay for interactive terminal sessions
* feat(tui): add /attach for interactive terminal sessions
* feat(tools): point the model at tty sessions for interactive prompts
* feat(tui): open the terminal automatically for tty exec sessions
* feat(tools): tell the model to keep polling while the user types
* feat(tui): give attached terminal sessions the full viewport
* feat(tools): route sudo through an escalated tty session
* fix(tui): route the auto-attach tick to its run
* fix(tui): defer auto-attach while another view owns the screen
* fix(tui): keep bracketed-paste framing for attached sessions
* fix(tools): let tty sessions run interactive commands
* fix(tui): capture the auto-attach baseline before the exec starts
* fix(mcp): reject a server-initiated request as a streamable-HTTP response
networkClient.request matched the decoded POST body to the in-flight call on
id alone, so a server that answered tools/call with a method-bearing frame
produced an empty, error-free result. Apply the same isRequestOrNotification
guard the SSE paths already use and return a protocol error naming the method.
Fixes#1050
Signed-off-by: Vladimir Babin <[email protected]>
* fix(mcp): describe the unexpected frame by method, not as a request
A method-bearing frame without an id is a notification, so the error
should not call it a request.
Signed-off-by: Vladimir Babin <[email protected]>
---------
Signed-off-by: Vladimir Babin <[email protected]>
Motivation: scripts/action-summary.mjs is invoked by the composite
action in action.yml on every action run, and ships with a 12-test
suite in scripts/action-summary.test.mjs. Neither the action-smoke
workflow nor the main Go-focused ci.yml executed that suite, and the
action-smoke workflow's paths filters did not even include those two
script files, so a change to the helper or its tests triggered no CI
job at all.
Approach: add scripts/action-summary.mjs and
scripts/action-summary.test.mjs to both the pull_request and push
paths filters in .github/workflows/zero-action-smoke.yml, and add a
Setup Node (actions/setup-node, node-version 24, matching the pin
already used in release-artifacts.yml) step followed by a
`node --test scripts/action-summary.test.mjs` step in the validate
job, ahead of the existing action.yml validation steps.
Validation:
- `node --test scripts/action-summary.test.mjs` passes locally: 12/12
tests pass (Node v22.23.1; repo engines field requires >=18, CI pins
24).
- Broke an assertion in a scratch copy of the test file outside the
repo and reran `node --test`: 1 test failed and the process exited
non-zero, confirming the new CI step actually gates on failures as
the issue requires.
- `python3 -c "import yaml; yaml.safe_load(...)"` confirms the edited
workflow YAML still parses.
- Confirmed .github/workflows/ci.yml has no Node step (Go-only
pipeline), so zero-action-smoke.yml is the correct integration
point.
- Base branch CI (upstream/main) is currently green.
Report: https://github.com/Gitlawb/zero/issues/1030
Assisted-by: claude-sonnet-5 (via Claude Code)
Signed-off-by: Pujitha Paladugu <[email protected]>
Co-authored-by: Pujitha Paladugu <[email protected]>
Motivation:
release-artifacts.yml's three actions/checkout steps (ordinary checkout,
workflow-dispatch requested-ref checkout, and the npm-publish job
checkout) and pr-auto-review.yml's single checkout step all leave
persist-credentials at its default, so the workflow token stays in the
job's local git config for every later step, including build/test/package
steps that never need it. ci.yml and zero-action-smoke.yml already
disable this. Neither modified file runs git push/commit/tag: the GitHub
Release publish step supplies its own explicit GH_TOKEN, and npm
publishing uses OIDC trusted publishing, so removing the persisted
credential changes no behavior.
Approach:
Add `persist-credentials: false` to each of the four checkout steps,
matching the exact convention already used in ci.yml/zero-action-smoke.yml.
Validation:
- go build ./... and go vet ./... pass.
- gofmt -l . reports nothing.
- go test ./internal/installtest/... passes.
- actionlint on both changed files reports only two pre-existing shellcheck
warnings in release-artifacts.yml (SC2016, SC2034) that are unrelated to
this diff and were confirmed present before the change as well (verified
by stashing and re-running actionlint against the original files).
- A live GitHub Actions run of the release/publish workflows was not
reproducible locally; this change only disables a default the workflows
never rely on, so no functional change is expected.
Report: https://github.com/Gitlawb/zero/issues/1029
Assisted-by: claude-sonnet-5 (via Claude Code)
Signed-off-by: Pujitha Paladugu <[email protected]>
Co-authored-by: Pujitha Paladugu <[email protected]>
* fix(oauth): force-close loopback callback connections past the shutdown deadline
Motivation:
LoopbackListener.Close shuts the callback server down with a 1-second
context and discards the result. http.Server.Shutdown only closes idle
connections, so a client that sends a partial request header and never
finishes it counts as active; Shutdown blocks out the full deadline,
times out, and Close returns while that connection and its Serve
goroutine remain open indefinitely. This is a loopback-only (127.0.0.1),
short-lived, single-use server, so the impact is a resource leak past
the end of the flow, not a remote vulnerability or crash.
Approach:
- add a ReadHeaderTimeout to the server so a stalled request header is
bounded on its own;
- follow a timed-out or otherwise failed Shutdown with server.Close()
to forcibly tear down any connection still open past the deadline;
- join the Serve goroutine (via a done channel) before Close returns,
so its completion is observable instead of left dangling.
internal/mcp/oauth.go and internal/provideroauth/openrouter.go build
their own loopback servers with the same pattern; they are left
untouched here to keep this change scoped to the file the issue's
reproduction demonstrates.
Validation:
- go build ./internal/oauth/... and go vet ./internal/oauth/...: clean.
- gofmt -l internal/oauth/: clean (make fmt-check clean repo-wide).
- go test ./internal/oauth/... -race -v -count=1: all pass, including
three new tests (TestLoopbackCloseForceClosesSlowHeaderConnection,
TestLoopbackCloseIsIdempotent,
TestLoopbackCloseReturnsPromptlyAfterSuccessfulCallback).
- Confirmed TestLoopbackCloseForceClosesSlowHeaderConnection fails
against the pre-fix loopback.go (with the new test kept) and passes
with the fix applied.
- Full repo go build ./..., go vet ./..., and go test ./... pass.
- Could not run make lint-static (golangci-lint) locally: it needs a
very large dependency download and the local environment ran out of
disk space fetching it. That check is continue-on-error (advisory)
in .github/workflows/ci.yml, not a required gate.
Report: https://github.com/Gitlawb/zero/issues/1028
Signed-off-by: Pujitha Paladugu <[email protected]>
Assisted-by: claude-sonnet-5 (via Claude Code)
* test(oauth): synchronize loopback regression tests instead of sleeping
TestLoopbackCloseForceClosesSlowHeaderConnection relied on a fixed
100ms sleep to let Serve accept a connection before sending a stalled
header; under load this could fail or silently skip the force-close
path it's meant to exercise. Use an http.Server.ConnState hook (via a
new unexported newLoopbackListener, set before Serve starts to avoid
a race) to wait for the real StateNew signal instead.
TestLoopbackCloseReturnsPromptlyAfterSuccessfulCallback measured
Close's elapsed time without joining its http.Get goroutine or
draining/closing the response body; join it first so the timing
starts from a connection that's actually back to idle.
Also register t.Cleanup(l.Close) in both tests so a Fatalf between
construction and the existing explicit Close doesn't leak the
listener's Serve goroutine.
Signed-off-by: Pujitha Paladugu <[email protected]>
---------
Signed-off-by: Pujitha Paladugu <[email protected]>
Co-authored-by: Pujitha Paladugu <[email protected]>
* feat(config): report whether the tool-turn budget was explicitly set
ResolvedConfig.MaxTurns is ambiguous today: a configured 80 and the
built-in default 80 are indistinguishable, so a caller that wants a
different default cannot tell "user chose this" from "nobody set it".
FileConfig now records at each merge gate (config files, provider
command, ZERO_MAX_TURNS, CLI overrides) when a source supplies a
positive maxTurns, and Resolve surfaces it as MaxTurnsSet.
* feat(exec): default headless runs to the turn-budget ceiling
The shared 80-turn default is sized for the TUI, where the user can
say "continue" and the run resumes. Headless exec has nobody to do
that, so benchmark evals truncated at 80 turns mid-implementation.
An unset budget now resolves to MaxTurnsCeiling for exec only; any
explicit source (flag, mode preset, ZERO_MAX_TURNS, config file) and
exec-profile budgets keep their precedence. The TUI default is
unchanged.
* test(cli,config): cover the headless turn-budget default
* refactor(cli): extract the headless turn-budget decision
* feat(tui): support an optional detail line on picker rows
Add a faint second line under picker items so richer surfaces can show
secondary metadata (project, model, size) without crowding the label.
The line renders on the selection band, joins the search haystack, and
only appears at medium width and above; detail rows halve the visible
window so the overlay keeps its height, and mouse hit-testing maps a
click on either line to the same item.
* feat(tui): show project, model, and size under /resume rows
Each session row now carries a detail line with the project directory
(~-contracted), the model that ran it, and its event count, plus a
status chip for forks and tagged sessions. The line also joins the
filter haystack, so searching by project path or model name works.
Narrow terminals keep the existing single-line list.
* feat(tui): show relative last-activity age in /resume rows
Replace the absolute timestamp column with relative age ("2h ago",
"12d ago") so recency reads at a glance; distant history still shows
the calendar date. Also adds coverage for the detail line: rendering
at wide vs narrow tiers, status chips, and filtering by model.
* refactor(tui): drop the event count from /resume detail rows
Event count is an internal metric — the detail line now carries just
project, model, and the status chip, which are the fields that
actually discriminate sessions.
* fix(tui): sanitize session metadata before it reaches picker rows
Persisted titles (--session-title, /rename) and other metadata can
legally contain newlines; a stray line would break the picker's fixed
row geometry and misalign mouse hit-testing. Reuse sanitizeCardField
for the label and detail line, and cover the detail-line click path.
* fix(tui): strip carriage returns from rendered session metadata
A \r in a persisted title or model id rewinds the cursor to column 0
and overwrites the row. Fold it into sanitizeCardField alongside the
existing newline handling, and cover it in the picker test.
* fix(tui): pad picker detail lines on the selection band
The detail line only painted its text, so the selection highlight
stopped at the end of the text instead of spanning the row like the
label line above it. Pad the remainder with the row surface.
* test(tui): cover carriage returns in sanitizeCardField
* fix(tui): drop all control runes from rendered session metadata
The picker detail line newly renders cwd, model id, and tag, and any
of them can legally carry ESC/CSI/OSC/BEL/BS/VT/NEL bytes — a project
directory name containing \x1b[2J would repaint the terminal when the
picker opens. sanitizeCardField now maps line separators and tabs to
spaces and drops every remaining control rune, matching the package's
other sanitizers; this also closes the same hole for titles, which
predates the detail line.
* fix(tui): include exactly-30-day sessions in the relative-age branch
* fix(sandbox): keep a derived ACL target inside the write root it came from
FILE_FLAG_OPEN_REPARSE_POINT refuses a reparse point at the final path
component and resolves every component above it, which is right for a path
the operator named and not enough for one this package derived. The
write-root carveouts are derived: <root>/.git/hooks and <root>/.git/config
are constructed from a root, and <root>/.git is a name an unprivileged
workspace writer can create before setup runs. mklink /J needs no
privilege, so a junction there had the apply open <junction-target>/hooks,
an ordinary directory with nothing wrong about its final component, and
zero sandbox setup wrote a deny ACE on it as Administrator, outside the
workspace.
A derived entry now carries the root it came from, and the apply requires
the object it finally holds to still live under it. The check is on the
handle rather than the name: GetFinalPathNameByHandle answers where the
open object actually is, and both sides go through it so the spelling
normalizes the same way. Materialization is checked before it creates,
since os.MkdirAll follows the same reparse points.
Deliberately not a refusal of every reparse point on the path. Above the
write root the path is the operator's, who may keep a workspace under a
junction or a mapped directory; only the tail the sandbox appended is held
strict.
Closes#1024
* fix(sandbox): reject the reparse ancestor itself before materializing
The pre-create check asked where the deepest existing ancestor lives, and
a junction answers with its own path: the open does not follow a
final-component reparse point, so <root>/.git came back as <root>/.git and
matched. os.MkdirAll does follow it. What disqualifies that ancestor is
that it IS a reparse point, not where it reports living.
Found by reverting the check and watching the test still pass, which said
the guard was doing nothing rather than that the test was weak. Pinned
against the function now, because through the whole apply the create is
made and then removed on the failure path, so the filesystem afterwards
looks identical either way. The plan wiring gets its own test for the same
reason: every apply-level case here hands the group an anchor directly, so
none of them would notice the builder never setting one.
* fix(sandbox): anchor only the carveouts that are under the root, and put the DACL back
Two from review, both right.
ReadOnlySubpaths is a profile field an operator can set to any path, and one
placed outside the write root is a configuration that works today; anchoring
it unconditionally turned that into a containment refusal. Only paths
lexically under the root are anchored now, which is where the derived
carveouts are anyway, and anything else keeps the final-component guard it
always had.
The successful-apply tests left their deny ACE in place, so t.TempDir could
fail to remove the tree. Restoring the snapshot exposed the sharper half:
the ACE denied the group the test runs as, which revoked its own WRITE_DAC
and left the rollback unable to reopen the target. The tests deny a group
this process is not a member of instead, which is what a capability SID is
in production, and roll back afterwards.
* test(sandbox): check the plan anchoring on every platform, out-of-root case included
The plan builder is cross-platform and its anchor tests were in a Windows-only
file, so nothing checked the wiring on Linux or macOS. Moved beside the other
BuildWindowsACLPlan tests, with the case review raised: an out-of-root
ReadOnlySubpath stays unanchored, and an in-root one is still held to its
write root.
* test(sandbox): give the cross-platform anchor tests native paths
Moving these beside the other BuildWindowsACLPlan tests made them run on
Linux and macOS, where their Windows path literals stop meaning what they
say: pathWithinRoot is filepath.Rel underneath, a backslash is an ordinary
character off Windows, and C:\workspace\.git\hooks is then one component
that is not under C:\workspace, so every anchor came back empty. The
neighbouring tests get away with such literals because they only compare
strings they built the same way and never ask whether one contains another.
Paths are built with filepath.Join from temp roots now, so the containment
question is asked in the separator the running platform actually uses.
windowsPathCarriesGrant read only the two inherit bits, so an allow ACE with
OBJECT_INHERIT|CONTAINER_INHERIT|NO_PROPAGATE_INHERIT satisfied the flag test
and had its whole mask credited. NO_PROPAGATE lets the ACE be inherited exactly
once: the immediate children carry it and their children do not. The runtime
tree's real consumers sit past that edge - cache/npm, cache/go-build and
data/go-mod, which sandboxRuntimeEnvironment points npm_config_cache, GOCACHE
and GOMODCACHE at - so attestation reported the grant adequate while those
writes were refused, and the unelevated path skipped the repair that would have
fixed it.
The skip is gated on needInherit so a file entry, where inherit flags are
meaningless, is unaffected, and the apply path never sets NO_PROPAGATE for an
AllowWrite entry, so a grant Zero itself wrote cannot start looking
insufficient.
Reported by @jatmn.
A command that opened the runtime lease while cleanup held it exclusively waited for the shared lock. Cleanup unlinked the lease name (marked it delete-pending on Windows) and compensated the tree before letting go, so the waiter woke on the old object, locked it, and got a lease that coordinated with nobody: the next cleanup created a fresh lease at the name, locked it unopposed, and reported the root unused while the command was running in it.
After either lock, both platforms now prove the locked object is still what the lease name resolves to under the retained parent: dev and inode against a no-follow fstatat on POSIX, volume serial and file index against a fresh no-follow open on Windows, where a name that is gone or delete-pending counts as moved on. A stale lock closes and the whole acquisition retries from tail creation, since cleanup has removed the leaf the contender was about to use; the exclusive side retries its open. Three stale objects in a row are reported. A stale lease is never undone by name, because the name is no longer that object. The created-directory ledger merges across attempts so a directory this run made on an earlier pass stays its own to compensate.
The regression drives the waiting-contender schedule through the production callers on both platforms, using the two seams as barriers and no sleeps: the contender must hold the current object, which the next cleanup proves by finding the root in use. Reported by @gnanam1990.
Brings in #1006 and the rest of main since the last rebase. Merged rather than rebased: fifty-three commits, and the marker-version comment conflicted on every replay.
One conflict: windows_setup.go keeps the marker schema at 6 with its history comments; main is at 5 with none.
The setup test #1006 added hands BuildWindowsSandboxSetupArgs straight to RunWindowsSandboxSetup as a string slice. On this branch the builder returns a WindowsSandboxSetupPlan so the creation ledger cannot be dropped, so the test now takes the plan, runs its Args, and registers its Rollback in cleanup, the same shape as the existing tests here.
The two carriers are each covered on their own now, but the interval where they
compose is the one the finding is actually about: the selection creates the lease
file, the step after it fails, so no lease object comes back and the builder takes
the early return that compensates only what the selection managed to report.
This drives BuildWindowsSandboxSetupArgs with that failure injected and asserts
nothing the invocation created is left behind, on both platforms. Falsified by
disabling the lease undo: the lease file survives, and with it every directory the
same invocation created, because it keeps their parent non-empty.
Both candidate roots are failed rather than only the preferred one, or the
selection relocates to the fallback and the build goes on to succeed.
Acquisition learned it had created a directory or a lease file at the create
itself, then published that fact only after every later step had also succeeded.
Between the two it held a fact nothing else did.
On Windows, createWindowsChildDirectory returns a handle to a directory that now
exists, and a failing handleRuntimeIdentity closed it and returned before the
ledger entry two lines below. On Unix the same gap sits under mkdirat, both for
the ownership refusal and the identity read. The lease files have the parallel
version: O_EXCL and FILE_CREATED each say this call made the file, and a failing
inspection, wrapping or lock returns no lease object, which was the only carrier
for that fact.
The consequence is not just litter. Setup returns an error while leaving
invocation-owned state behind, and where an ancestor WAS recorded, that
ancestor's compensation then fails on a child it cannot account for, so the
whole partial tree stays.
Each of those failures now undoes its own creation before returning: through the
handle on Windows, relative to the parent descriptor on Unix, never by
re-resolving the name, since the name is exactly what this descent refuses to
trust twice. Only a create by this call is undone, so a lease that was already
there still belongs to whoever is holding it, and components that completed stay
on the ledger for the caller to compensate.
createWindowsChildDirectory and both rooted lease opens now ask for DELETE, which
is what lets the undo go through the handle rather than the name.
Reported by jatmn.
Compensation took the exclusive cleanup lease to prove no command was holding
the runtime root, then gave that proof back before doing any of the work it was
proving anything about.
Two consequences. The lease was released before its own pathname was removed and
before the directory walk, so a command blocked on the shared lease was let in
mid-cleanup: it could take the old lease object, have the name unlinked out from
under it, and a later cleanup locking a fresh object at the same pathname would
then read the root as free while that command was still in it. The comment
justifying the early release said a file cannot be deleted on Windows while this
process holds it open, which is not true of a handle opened FILE_SHARE_DELETE,
and this one is: os.Remove succeeds and the name goes at once.
The second was worse and needed no race at all. An acquisition that came back
in-use appended "the lease and the tree it protects were left in place" to the
error list and then ran the directory walk anyway on the next statement. The
promise lived in the error string anyway. A contender holding the shared lease
had its root deleted and was told it had not been.
So the lease is now taken, held across the pathname removal and the whole walk,
and released at the end, and nothing is removed at all unless the acquisition
succeeded. Leaving a tree behind is recoverable by the next run; deleting a live
one is not. The lease pathname goes during the walk, immediately before the
directory that contains it: any earlier ends the exclusion everything after it
depends on, any later leaves that directory non-empty so it can never be
compensated.
Also removes the superseded pathname-based shared acquisition and both its
platform halves, which no longer had a caller. It opened the lease by full
pathname, so it followed a link planted at that name, and on Windows it opened
without FILE_SHARE_DELETE, which by itself stops cleanup from removing a lease
anyone is holding. A weaker door sitting beside the rooted one is how a later
change quietly takes it.
Reported by jatmn.
The untouched-tree case created the runtime root but not the lease beside it, so
nothing failed if compensation stopped asking whether the lease was ours. It is
the object two processes coordinate on, and removing one this invocation did not
create takes it out from under whoever did.
prepareSandboxRuntimeLeaseRecording reported the owned directories it created and
the two-result wrapper beside it dropped them. Every real selector and the setup
argument builder used the wrapper, so nothing in production ever saw that record,
and only tests called the recording form.
The consequence is a whole transaction with no undo. BuildWindowsSandboxSetupArgs
calls selectSandboxRuntimeRoot, which creates zero/runtime/v1 and the lease file
when they are not there, then releases the lease and serializes only the selected
pathname. The helper reacquires an already-existing tree so it records no
creation, and provisioning records only the leaf. Any failure before the marker
left the parents behind attested by nothing, and the sibling lease file kept v1
non-empty so even the leaf rollback could not finish.
The transaction now starts at the first mutating lease operation. The selector
returns the ledger, including on the error path where acquisition creates one
component and fails on the next, and the fallback branch appends rather than
replaces so a preferred attempt that created components before failing to lease
is not forgotten. BuildWindowsSandboxSetupArgs returns a plan rather than bare
args, so a caller cannot take the command line and leave the record behind, which
is exactly what happened last time. The CLI runs the undo when the helper does
not complete, on this side of the process boundary where the record lives.
The lease file is part of it. It is removed only when this acquisition created it,
which NtCreateFile and an O_EXCL open are what can say, and only after retaking it
exclusively. Taking it exclusively is not tidiness: it is the proof that no other
process is holding the tree about to be deleted. A holder means the lease and the
tree both stay and the caller is told, because reporting a clean undo while a tree
is still on disk sends the operator to retry against state they were told was
gone.
setupConsumerSID is a variable now. The interval between creating the tree and
returning the args is the whole reason the rollback exists, and the identity
lookup is the only step in it that can fail, so without that seam the compensation
path is unreachable from a test.
A command path takes no ledger on purpose and says so: a command is not a
transaction, publishes no marker, and what it creates is the tree it is about to
use.
Verified on Windows and on real Linux.
The fallback root moved from an atomically minted MkdirTemp parent to a
predictable /tmp/zero-u<uid>/runtime/v1/<digest>. The stable name is required,
because setup and every later process have to agree on one root without talking
to each other, but it also means another local account can name the first owned
component before this one does.
refuseAliasedRuntimeComponents answers about an ABSENT component by saying there
is nothing to alias, which is exactly the state a fresh fallback is in. After
that answer the code called os.MkdirAll on the parent and opened <root>.lease by
full pathname, and both follow a link planted in between. The guard was
authorizing writes whose destination it could not see, and the next check could
only report the problem after they had happened.
So the base is opened by name exactly once, and every component below it is
created or opened relative to a retained descriptor with O_NOFOLLOW, its
ownership and mode checked through fstat, and the lease opened relative to the
verified parent. A link at an owned component is an ELOOP or ENOTDIR from the
kernel rather than a redirection nobody noticed. The base itself keeps following
links: it belongs to the operator and is legitimately one on macOS, where /tmp
resolves through /private.
createRuntimeTailHandleRelative on this side was the same defect one layer up, an
os.Mkdir loop over full pathnames kept on the reasoning that the elevation
asymmetry the Windows descent closes does not apply here. True of the elevation,
false of the substitution. It goes through the same descent now, so there is one
implementation per platform rather than one protected platform.
Cleanup resolves the lease the same way for the same reason as on Windows: a link
at the lease name otherwise gives the shared holder and the exclusive holder two
different files, both locks succeed, and cleanup removes a root that is in use.
The two descent seams move to a file with no build tag. A seam defined beside one
of two implementations is how the other one quietly ends up untested.
Verified on real Linux, not only cross-compiled.
Falsifying turned up that an unknown third-party tag is unresolvable, so the old
pathname cleanup fails on it too, with ERROR_CANT_ACCESS_FILE rather than by
classifying anything. The case still pins that both sites refuse, and the reason
check separates a refusal from an accident, but only the symbolic-link cases show
the half that matters most: a pathname open succeeding on the target. Those need
the privilege, so their result has to be read in CI rather than locally.
FILE_OPEN_REPARSE_POINT says do not follow, not refuse. It returns a handle to
the LINK, and FILE_NON_DIRECTORY_FILE beside it excludes directories rather than
non-directory reparse objects, so a file symbolic link at <digest>.lease was
opened, wrapped and locked as though it were the lease. The flag was being read
as a guarantee it does not make.
That mattered because cleanup did not use the rooted opener at all. It called
os.OpenFile on the full pathname with no no-follow flag, followed the same
planted link, and locked its TARGET. Setup or a running command then held a
shared lock on the link while cleanup held an exclusive lock on the target, both
calls succeeded, and cleanup was free to RemoveAll a runtime root somebody was
still using. This is not the ancestor-junction case already fixed: the
substituted object is the final lease entry itself.
No-follow opening and object classification are two requirements. The lease
handle is now asked what it is before it is locked, which is the check the
directory descent in openWindowsChildNoFollow already made, and cleanup resolves
the name the way acquisition does: the same base, the same owned components, the
same no-follow opens, the same refusal. Mutual exclusion is a property of the
object, so both sides have to arrive at it the same way.
Cleanup opens and never creates the tree. A runtime root that is not there has no
lease to take, and rebuilding it in order to lock it would be inventing the thing
cleanup is about to remove.
The regression covers a file symbolic link at the exact lease name for both call
sites and asserts the link target is never locked. That needs
SeCreateSymbolicLinkPrivilege, so it skips on an ordinary account; a third-party
reparse tag needs only write access, and pins the same classification everywhere
for both sites. Controls prove a shared lease still makes cleanup report inUse
and then succeed after release, and that two holders can still share one
legitimate lease.
Setup reaches the temp fallback when the preferred cache-derived root cannot be
leased, and it records that concrete root in the marker. Command selection then
recognised the record only by deriving today's fallback from this process's
TEMP and comparing the two. A later IDE, service or terminal running with a
different TEMP matched neither of today's candidates, so the record was
ignored, selection produced a tree setup never provisioned, and the runner
rejected the marker as out of date. Re-running setup from the original
environment repeats the original answer and does not make the other
environment converge.
That is the second selection the persisted-root design existed to remove: the
record was being treated partly as history and partly as something reproducible
from the next process's environment.
The record is now recognised by its own shape, which is the question actually
being asked. It must have the owned runtime shape, its first component must be
the fallback's, and its leaf must be this workspace's fallback digest. Nothing
about its base is re-derived. The cache-derived digest is deliberately not
accepted, so a moved user cache still reports stale, which is a different
failure with a different remedy. Both call sites take it, or doctor and the
command disagree again.
The refusals that made the old comparison worth having are unchanged: a record
provisioned for another workspace has a different digest and is still refused,
and a record with no owned shape is refused outright.
snapshotWindowsSandboxRuntimeStamp reads the runtime root's identity and prior
stamp through one handle and closes it. applyWindowsACLPlanWithStamp then
resolved the same pathname again and mutated whatever answered. Neither stage
ever proved the two were the same object, so the transaction established "these
prior bytes belong to B" and wrote to A.
The root's owner can arrange that with ordinary directories and no privilege:
rename the real root aside, leave a plain directory at the predictable name for
the snapshot to read, and restore the original before the apply. Nothing is a
reparse point, so a no-follow open does not notice, and the setup lease is a
sibling of the root rather than the root entry itself.
The damage is not only a misplaced ACL. On a later failure, stamp compensation
compares what it finds against the snapshot's identity, refuses to restore, and
leaves this run's stamp on a directory whose published marker still describes
the previous successful setup, so a failed setup invalidates a good one.
The request now carries the identity the snapshot established, and the apply
verifies it before its first mutation rather than before the stamp, since the
ACL is the change that matters most. It fails closed: an identity that could not
be established refuses, because "we could not tell" is precisely the case the
guard exists for.
Two stamp tests built the request by hand without an identity and are updated to
build it the way setup does. The new regression substitutes the directory in the
one interval where a substitution can land, between the snapshot's close and the
apply's open, and asserts the substitute collects nothing and the prior stamp
survives byte for byte.
The first version of this test planted the junction before acquisition started,
which the old alias pre-check already refused, so it passed against the
implementation it was supposed to condemn. The defect is the check-then-use
window: os.MkdirAll and the pathname lease open both resolved the component
again after the check had passed.
A pre-create seam both implementations honour lets the junction land in exactly
that window.
Provisioning already descends from the fixed cache or TEMP base through
retained no-follow handles, because a predictable owned component is exactly
what an ordinary same-account process can replace with a junction. Lease
acquisition runs first and did neither: it checked the components for aliases,
then called os.MkdirAll on the parent by pathname and opened "<root>.lease" by
pathname. Both follow.
A junction dropped on zero, runtime or v1 between the check and either call put
elevated setup's first writes inside the caller's chosen target. Restoring the
component afterwards left the later handle-relative provisioning operating on
the legitimate tree, so no post-check saw it, and the rollback could not
compensate writes it had no record of. The lease file made it worse by being a
sibling of the runtime root rather than one of its owned components, so the
alias check never inspected that name at all.
Lease acquisition now splits the owned tail, creates only the base by name,
descends the components above the leaf from retained handles, and creates the
lease file relative to the deepest of them with FILE_OPEN_IF,
FILE_NON_DIRECTORY_FILE and FILE_OPEN_REPARSE_POINT. It refuses rather than
falling back when the path is not a runtime root, and it reports the
directories it created so a failure can undo them. The leaf stays with
provisioning, which records it: two owners for one directory is what the
accounting is meant to avoid.
Non-Windows keeps its previous behaviour, alias check included.
The existing lease tests pre-created the whole tree before every call, so no
test exercised the creation path, which is how this shipped green. The new
regressions plant a real junction, assert nothing lands in the redirected
target, and pin that acquisition records what it created and only that.
Three instances of one shape: the right fact was established, but only after
crossing the point where it had to become authoritative.
Runtime creation. createRuntimeDirRecording found the deepest component that
already existed with os.Stat and opened THAT by name, so existence chose the
trust boundary. On the ordinary second-workspace shape zero\runtime\v1 is
already there and only the digest is missing, so the single by-name open was v1,
one of the predictable components the rooted traversal exists to protect. A local
owner could swap it for a junction between the pre-check and that open, let
elevated setup create the digest beneath the redirected target, and restore the
original before the post-check, leaving a privileged creation recorded under a
pathname that no longer named it. The split now comes from the same inventory
that built the root: only the cache or TEMP directory above the owned tail is
opened by name, every owned component below it is reached from the retained
parent handle, and a root without that shape is refused rather than walked by
name.
Setup handoff. Zero never elevates: the operator elevates the terminal and the
helper is launched with a plain exec.Command, so the consumer SID was resolved
in a process that was already elevated and described the installer. Same-account
UAC hides that; alternate administrator credentials do not, and the stamp would
get its allow ACE for a token that can never read it. That case cannot be
detected from inside an already-elevated process, so it is excluded rather than
mis-provisioned: the helper now refuses when the carried identity is not its own
token, before anything is provisioned, and the comments no longer claim an
elevation boundary that does not exist.
Doctor. WindowsSandboxRecordedRuntimeRootIsCurrent answers current, stale, or
unresolved, and the third fell through to the pinned branch: a valid old marker
made doctor report healthy while BuildCommandPlan hit the same resolution failure
and stopped every command from launching. Unresolved is now its own warning
carrying the resolver cause, and the pinned validation profile is built only once
currentness has been established.
The regressions drive the production entry points. The recording test asserts the
single by-name open is the fixed base, which is the property; asserting only that
a swap was caught was not discriminating, because the mutated code refused for an
unrelated reason. Two rollback tests were using a root missing the leading owned
component, a shape production never builds, and now use the real one.
Doctor pinned the marker's runtime root onto the profile so the stamp check
could run without taking a lease. A command does not pin blindly: it derives
the current cache and fallback candidates and honours the marker only when its
root is one of them. Run setup with the user cache at A, relocate the cache so
commands derive B, and the stamped A tree stays behind. Doctor reused A,
constructed its expected plan, and reported healthy immediately before every
real command rejected A as not a current candidate and failed on the
out-of-date marker.
The marker's root is history, needed to validate the stamp without mutating
anything. It is not proof the root is still selectable, and doctor conflated
the two. WindowsSandboxRecordedRuntimeRootIsCurrent derives the same
candidates a command derives, through the same resolver, and applies the same
equality by calling pinnedSandboxRuntimeRoot itself, so doctor asks the
command's own question rather than a copy of it. Only a root a command would
still select is used to validate the marker; otherwise doctor reports the
setup out of date with the runtime root it found and the remedy. It still
takes no lease and creates nothing.
The regression records a marker with the cache resolver at A, flips the
resolver to B, and asserts the recorded root is no longer current while its
value is unchanged. Making the helper trust the marker unconditionally fails
it on that assertion. An earlier draft of this test planted the marker
without setting profile.Runtime, so it failed on its own setup guard and its
falsification proved nothing; that is why it is worth saying the failure is
on the property.
createRuntimeDirRecording guarded with Lstat before and after, but the
interval between them was still path-based: it found the deepest existing
ancestor with os.Stat and then created each missing component by opening its
parent BY NAME. Both follow a junction. A local user could replace the
predictable owned "zero" component with a junction after the pre-check, point
it at a directory of their choosing, let elevated setup create
runtime\v1\<hash> beneath that target, and restore the original before the
post-check. The post-check then saw an ordinary path, the plan applied
normally, and rollback reported the separately created target only as
identity-mismatched residue.
Pre and post pathname inspection was being used as authorization for an
elevated mutation. Now the deepest existing ancestor is the only thing opened
by name, and every missing component below it is addressed relative to its
parent's handle with FILE_OPEN_REPARSE_POINT: an existing component is opened
no-follow and refused if it is a link, and a missing one is created relative
to the parent handle and identified from the handle the create returned. No
name is resolved twice, so there is no interval for a swap to land in, and a
junction at any owned component is seen as the link it is. Redirected cache
and TEMP locations above the owned tail are still allowed; the restriction is
on the zero/runtime/v1/<hash> components Zero itself owns. Off Windows the
mkdir loop is unchanged.
A deterministic barrier between opening the base and touching the first owned
component lets a test plant a junction at exactly the vulnerable interval.
Driven with a real mklink /J: the descent refuses, records nothing, and the
junction target is untouched. The old by-name create against the same
junction returned nil and landed a directory under the target, which is the
hole verbatim. A control test creates the tail from handles and checks each
recorded identity against the directory the name resolves to afterwards.
Two rollback records were still assembled from facts that were never
established, so compensation was correct about the wrong thing.
The created-directory record took its identity from a pathname resolved after
creation. Win32 CreateFile cannot create a directory whatever disposition it is
given, so the CREATE_NEW attempt fell through to os.Mkdir plus a separate
reopen on EVERY real creation, and the documented creation-handle contract
never once held. The runtime parent belongs to the ordinary user, who can
rename the new directory away and put an ordinary one at the predictable name
in between; the ledger then recorded the substitute, and a failed setup could
delete it while the directory this run actually created kept the ACL and stamp
under a name nothing was tracking.
NtCreateFile with FILE_CREATE and FILE_DIRECTORY_FILE does create a directory
and returns the handle, so identity is read from the object created and there
is no second resolution to race. A collision still surfaces as a *PathError
carrying os.ErrExist, because the caller asks os.IsExist, which does not unwrap
a %w chain: wrapping alone would have turned a benign lost race into a hard
setup failure. An unidentifiable create now errors instead of manufacturing an
ownership record.
The stamp record collapsed every failure into proven absence. An encoding
failure, a root that would not open, an unreadable identity, a denied child
open and a short read all arrived as existed=false, exactly like a real
not-found. The writer uses FILE_OVERWRITE_IF and can replace an existing stamp
where the read was denied, and compensation for "did not exist" deletes the
current stamp and returns with nothing to restore, so a setup attempt that
REPORTED FAILURE destroyed the attestation of the previous successful setup.
The snapshot now returns three states. Only ERROR_FILE_NOT_FOUND and
ERROR_PATH_NOT_FOUND produce absent; a complete read produces present; anything
else is unknown and returns an error. Setup refuses on unknown before the ACL
and stamp are applied, and restore refuses to compensate an unproven prior
state, so the two halves of the guard sit on both sides of the mutation. The
zero value is unknown deliberately.
Tests cover the three states separately, an unreadable prior stamp left
byte-for-byte intact with the writer never reached, and a create/reopen barrier
that counts pathname resolutions. One honest limit on that barrier: it counts
resolutions through runtimeIdentityAfterCreate, which the non-Windows path uses
and which is documented as the pathname reopen. It would not catch a future
hand-rolled one.
Compensation verifies and mutates through a single handle now, but the records
it trusts were still built from separate pathname opens.
snapshotWindowsSandboxRuntimeStamp took the identity through a handle, closed
it, and then let os.ReadFile resolve the name again. A rename and substitution
in that interval pairs one directory's identity with another's stamp bytes, and
a rollback that correctly proves it holds the first writes the second's contents
into it, corrupting an attestation that predates this run. Reading the stamp as
a child of the identified handle removes the second resolution.
The creation ledger had the same split: os.Mkdir created the directory and
runtimeDirIdentity reopened the name, so the ledger could record a substituted
object this run never made. Rollback would then prove it held that object and
delete it, while the real one kept this run's ACL and stamp under a name nothing
was tracking. The identity now comes from the creation itself.
The lease stops cleanup selecting the root; it does not stop the parent's owner
renaming it, which is what makes both windows reachable.
The race needs an elevated installer against an unelevated renamer and is not
reproducible here, so the tests pin the contract instead: both halves of a
snapshot come back together and agree with the object at the path, and a created
directory's identity describes what was created.
Resolving the reader from the runtime leaf's owner does not name the token that
consumes the stamp, because the elevated helper CREATES that leaf when it is
absent. The owner is then commonly BUILTIN\Administrators, and the repair-identity
collision fix correctly folds the reader into the Administrators grant rather
than emitting a duplicate ACE.
Both steps are right on their own and wrong together. Production crosses a token
boundary that the test did not: a later UAC-filtered administrator carries
Administrators deny-only, and a standard user given alternate administrator
credentials is not in the group at all. Neither can satisfy an allow ACE, so the
protected stamp holds no enabled grant for the token that has to validate it,
ValidateWindowsSandboxSetupMarker fails immediately after setup reported
success, every restricted command stops before launch, and doctor calls the
setup unusable.
Resolve the consumer in the operator's shell instead, before elevation, and
carry it in as --consumer-sid. That is the same rule the runtime root already
follows two lines away: selected where the answer is knowable, not derived where
it is not.
The owner fallback stays for the paths that are not setup, notably rollback
recreating a stamp it just removed, where the leaf already exists and belongs to
whoever owns the install. A test pins that the carried identity outranks the
leaf owner, since that precedence is the entire difference between the two
designs.
The post-deletion probe distinguished only err == nil from err != nil, so every
Lstat failure took the success path. A second process holding a share-delete
handle leaves the entry delete-pending, and an access denial or sharing
violation from the probe then reported complete compensation for a directory
that is still there. A holder able to clear the disposition can make the
"removed" object visible again, after setup has already said the rollback
finished. Ordinary inspection failures were hidden the same way, and those
carry no evidence about the postcondition either.
Three outcomes rather than two: not-found proves absence and returns nil, a
successful stat means it is still present, and anything else is reported as
residue whose removal could not be verified.
The probe goes through a seam so a test can produce that third outcome, which
no real filesystem yields on demand. Reverting to the two-way check fails the
unverifiable case naming it as complete compensation.
The resolved stamp reader can BE one of the repair identities. A runtime root
created by an elevated process is commonly owned by BUILTIN\Administrators
rather than by the invoking user, which is what CI runners do.
Naming the same SID twice, once read-only as the reader and once GENERIC_ALL as
repair, let the narrower entry win: Administrators came back with mask
0x00130089 and could no longer rewrite the stamp. Setup would succeed and leave
an attestation nothing could replace.
Skip the reader entry when a repair identity already covers it, and pin it with
a test that resolves the reader to Administrators directly, so the case is
exercised on an unelevated box too. My own regression caught this, but only on
CI, because here the directory is owned by the ordinary user.
That is also why the existing assertion had to change: no-write is right for an
ordinary owner and wrong for one that is itself repair.
TestRollbackContinuesAfterACompensationFails injected its failure with a
snapshot pointing at an unwritable stamp path. That stopped meaning anything
once compensation began deriving the stamp from the verified root handle rather
than the recorded path: the restore wrote a stamp INTO the directory the
rollback then tries to remove, so the removal failed for a reason this test is
not about.
It passed on an unelevated box and failed on all three CI runners, because
whether that recreate succeeds depends on the token the tests run under.
Inject a root that is no longer the directory this run stamped, which is a shape
production produces and which writes nothing on any platform.
The identity check and the mutation used two different resolutions of the same
name: runtimeDirIdentity opened a handle, read the volume and file ID, closed
it, and then os.Remove or os.WriteFile resolved the pathname again. A rename
followed by a replacement in that interval makes the comparison true about one
object while the write or the delete lands on another. This is elevated
compensation, so a redirected pathname gives the mutation reach the replacer
does not have directly.
Open once, verify the identity on that handle, and perform the mutation through
it: the stamp relative to the directory handle, the created directory by
FileDispositionInfo on its own handle. No ancestor is re-resolved and there is
no interval to land in. A seam between the check and the mutation drives the
replacement in tests; with the old pathname resolution they fail, naming the
substitute.
Two things fell out of doing it properly.
The stamp restore now deletes and recreates through the ordinary writer rather
than overwriting. The stamp carries a protected DACL that withholds write, so
an in-place overwrite is denied under the token that wrote it, and only the
writer puts that DACL back on the replacement.
The reader ACE gains DELETE alongside read. Withholding write from the SANDBOX
is the real boundary and the capability SID has no ACE here at all. Withholding
it from the root owner is not one: they own the parent, so delete-then-create
forges a stamp exactly as well as an overwrite. What read-only actually cost
was rollback's ability to remove a stamp this run wrote.
A directory removal that reports success is verified rather than assumed, for
the reason recorded on the promote rename in #751.
Both identity capture sites discarded runtimeDirIdentity's success flag, so a
root that could not be opened was indistinguishable from one with no identity,
and both mutation sites read the empty string as "skip the check". Stamp
restoration then wrote to, or removed from, whatever answered to the pathname
afterwards, and directory rollback treated every reopened object as eligible
for removal. This is elevated compensation, so a pathname redirected after the
capture gives the mutation reach the replacer would not have directly.
Carry the flag and refuse: an identity that was never established is not
permission to mutate. The object is left alone and reported as residual state.
A root that is simply ABSENT stays quiet, because there is no object to confuse
and nothing of a previous run to put back; the created-directory rollback owns
that case. Without that distinction every fresh setup would report a
compensation failure, and a test pins it.
The test helper that builds created-directory records had the same discarded
flag, which is why the existing rollback tests passed against the fail-open
path. It now carries the identity through the way production does.
This does not close the check-then-mutate window itself: the identity is still
read on a handle that closes before the pathname operation. That half is
separate.
The stamp's DACL named WinCreatorOwnerSid at GENERIC_ALL. SetSecurityInfo does
substitute that placeholder even in a NO_INHERITANCE ACE, so a concrete SID did
land in the ACE, measured again on this head: ACE[0] carries the user SID at
mask 0x1f01ff and the readback succeeds. The mechanism was never the problem.
The identity it named was. It is whoever ran setup, and setup runs elevated.
When elevation comes from a different administrator account than the one that
later runs the command or zero doctor, the reader matches no ACE, and a reader
named in no ACE gets Access is denied on os.ReadFile. A successful setup then
hands over an attestation the launch gate and doctor cannot open.
Resolve the reader from the runtime root the stamp is created in, through the
directory handle rather than a pathname, so the grant is bound to the install
rather than to the elevation. Give it read only: nothing outside setup and
repair should be able to rewrite an attestation about the tree, and the stamp
write still succeeds because the handle was opened GENERIC_WRITE before the
DACL was applied.
A reader that could not be resolved is refused rather than protected with a
DACL naming nobody.
Owned depth is the fixed names plus the digest, so a root placed directly in
t.TempDir() put an owned component on /tmp. The Unix ownership guard then
correctly refused it, because /tmp belongs to root and the test runs as an
ordinary user. That is the guard working rather than an environment problem, and
it only appears off Windows: both ubuntu Smoke and Zero Review failed on it while
the Windows run was green.
The roots are built as <temp>/zero/runtime/v1/<leaf> now, so every component the
alias and ownership walk inspects was created by the test.
The unelevated caller took a lease only to learn which root wins and released it
at once, so nothing owned the selected root while the elevated helper provisioned
the tree, applied the ACL and stamp, installed network state and wrote the
marker. A command for another workspace scanning the same runtime parent excludes
only its own current root, so it could take this root's cleanup lease and
RemoveAll it mid-transaction, leaving setup to publish success for a pathname
that was gone. The helper now holds a shared lease on the root it was handed,
from before provisioning until after the marker write, and fails before any
persistent state is written if it cannot take one.
Compensation resolved pathnames again after the apply handles had closed, so a
rename-aside plus an ordinary directory at the same name let it strip a stamp
from the substitute, write another object's bytes onto it, and remove it as
though this run had created it, while the original kept the grant and the stamp.
The stamp snapshot and every created-directory record now carry the identity of
the object they describe and refuse a replacement, reporting the original as
residual instead.
os.SameFile cannot express that on Windows. A Windows fileStat loads its volume
serial and file index lazily, BY PATHNAME, at comparison time, so an identity
captured before a replacement and compared after reports the substitute as the
same object and the original as different: exactly backwards, and silently.
Measured rather than reasoned about. runtimeDirIdentity reads the identity
through a handle at capture time instead, with the Unix build using device and
inode so both platforms follow one rule.
Runtime preparation resolved the sandbox home from the ambient environment while
Windows platform planning resolves ZERO_WINDOWS_SANDBOX_HOME out of the command's
own spec.Env and hands that one to the runner for marker validation. A command
that explicitly selects home B, while the parent still points at home A, pinned
A's recorded root into the profile; the runner then loaded B's marker, saw a
different root, and rejected the command as out of date even though setup for B
was valid. The two homes need no different derivation rules to disagree, only
different valid selections from the same preferred/fallback pair. Selection now
takes the home the command asked for, and an empty one still resolves the ambient
environment because that is the only authority a caller without command context
has. The refusal of a root belonging to another workspace is unchanged.
Separately, plan construction creates directories, which is easy to miss because
it reads like naming: windowsSandboxProfileWithProvisionedRuntime provisions the
root it selects, and the simulated-Windows tests run on every platform. Nothing
redirected the cache, so the package wrote into the developer's real one. The
machine this was found on had 64 runtime directories and 9115 orphaned lease
files accumulated under the real runtime root. Redirected once for the package
rather than per test, because the leak was in the default and any new test that
builds a Windows plan would inherit it.
The grant check belongs beside the tier's other launch decision rather than
inside ValidateWindowsSandboxSetupMarker. That function compares what setup
intended with what this command wants, which is what its name asks and what
every consumer of it expects; folding a security-descriptor read into it made
`zero doctor` depend on real applied ACLs and report a freshly set-up machine
unhealthy.
Both tiers now attest in runWindowsSandboxCommand: the unelevated one reads the
descriptors and re-applies, the restricted-token one reads them and refuses,
because it cannot repeat an elevated provisioning. The refusal is covered
through the runner rather than only through the function, since a check that
reads correctly and is never called is the failure mode being avoided.
Three things the end-to-end pass over the runtime-root lifecycle turned up.
The elevated tier attested its runtime root with a stamp file. That proves the
directory was not removed and recreated under the same pathname and that setup
provisioned it for this configuration, and it says nothing about whether the
capability ACE still carries the permissions windowsACLAccess created or still
reaches descendants. An ordinary file survives an ACL edit untouched, so an
icacls reset, an inheritance change on a parent, or a security product rewriting
the DACL leaves a valid stamp over a runtime root the restricted child cannot
write. This is the finding already closed on the unelevated tier, still open on
the tier that runs under the restricted token. It reads the descriptors now, and
refuses with the action that fixes it rather than re-applying, because this tier
cannot repeat an elevated provisioning.
The grant check counted an INHERIT_ONLY allow ACE towards the directory's own
grant. Such an ACE grants descendants and grants the directory nothing, so the
child is still refused FILE_ADD_FILE on the runtime root while every inherit flag
the check looks for is present: the propagation half of the grant taken as
evidence for the whole of it.
Rollback checked object identity through one open and then resolved the name
again for the restore. The two opens are a check-then-use, and the fact
established is not the fact the write depends on. One open now, with the
identity read from the handle that gets mutated.
The attestation accepted any allow ACE naming the capability. That is weaker
than the contract the child depends on: windowsACLAccess grants
FILE_GENERIC_READ|WRITE|EXECUTE, and a directory entry carries
SUB_CONTAINERS_AND_OBJECTS_INHERIT so the grant reaches the TMP and cache
descendants the child actually writes.
So an ACE that still named the capability but had been reduced to a read-only
or metadata mask, or that no longer propagated, attested as healthy. Both
consumers skipped the idempotent re-apply and the restricted child reached a
marker-valid runtime root and got ACCESS_DENIED on its first write, which is
the silent unusable runtime the attestation was added to remove.
Attestation now unions the allow masks of the ACEs that actually propagate and
requires the plan's own mask, and a deny naming the capability over any
required bit fails closed rather than being ignored. The regression drives the
positive case plus four weakenings that all keep the SID: read+execute,
metadata only, full mask without inheritance, and containers-only inheritance.
Removing either half of the check fails the cases belonging to that half.
Scoping the fallback's first component to the user broke the shape the owned
tail is matched by, because the matcher compares that position against the one
fixed name. The fallback therefore stopped being recognised on Unix, which
silently costs it both protections that depend on the shape: the rooted
no-follow traversal falls back to opening the tree by name, and the
owned-component guard no longer sees it. TestTheTempFallbackRootKeepsTheOwnedShape
exists for exactly that reason and caught it on CI.
The first position now accepts either spelling, the fixed one the cache root
uses or the user-scoped one the fallback uses, and every component below it
stays fixed so a tree Zero does not own is still refused.
The scoped spelling does not exist on Windows, so the accepting branch could
not be exercised there at all and a break in it would only ever surface on
another platform's CI. The matcher reads the fallback names through a seam so
the shape test can drive both spellings, including a different user's scope,
on any platform.
The forward apply and its stamp go through one handle, so they are provably
about one object. Compensation runs later, after a network or marker failure,
and resolves those names again. Opening no-follow refuses a reparse point but
accepts an ordinary directory moved into the name since the handle closed, so a
rename plus a substitute made rollback restore the pre-apply DACL onto the
substitute and report success, while the moved original kept this run's
capability ACE. Setup claimed a completed rollback with the modified object
still reachable elsewhere, having also mutated a directory the forward
operation never touched.
The snapshot now records the object identity read from the open handle, which
is the last moment the name and the object are known to be the same thing, and
the restore refuses to act unless the name still answers to it. On a mismatch
the substitute is left alone and the error says the original is unrestored,
rather than reporting success.
Materialized targets are deliberately not identity-checked. A materialized
target is one this run created, and its plan routinely denies Everyone read,
which is what a protected metadata carve-out is, so the attributes identity
needs cannot be read back even by the owner: requiring it turned rollback of
every materialized directory into "Access is denied". Establishing identity
there means holding the apply handle open to the last failure point, which is a
larger change than this one and is called out rather than papered over.
The unelevated marker records a plan hash and entry count, both derived from
pathnames and entries. That says what should be granted; it cannot establish
that whatever directory currently answers to the name has received it.
The runtime root makes the gap reachable rather than theoretical, because it is
deterministic and disposable. Cleanup removes the tree, the next command's
parent recreates the same pathname with ordinary inherited permissions, and the
hash is unchanged, so the fast path skipped the apply and the WRITE_RESTRICTED
child could not write TMP or its language and package caches. Nothing failed to
say why.
The fast path now reads the security descriptor of each allow entry and takes
it only when the grant is actually present. That costs one descriptor read per
entry on a path that is about to create a process, and it covers every reason a
grant can be missing rather than only recreation by the parent: manual
deletion, eviction between commands, a restored backup. Any failure reads as
"not applied", because re-applying an existing grant is idempotent while
skipping an absent one produces a sandbox that silently cannot write.
The temp-derived runtime root scoped its LEAF to the user and left
zero/runtime/v1 fixed above it. Runtime preparation creates and
ownership-checks each of those at 0700, and on Unix os.TempDir() is shared
whenever TMPDIR is unset, so the first account to use the fallback created a
private directory every other account was then refused at: traversal fails on
the mode, and relaxing the mode fails the ownership guard instead. The
per-workspace digest never got the chance to separate them, and the fallback
became first-user-wins on a shared host. The MkdirTemp layout this replaced did
not have that problem, because each process sat under its own 0700 directory.
The scope moves to the shallowest component, so every ancestor the guards
create and validate is already inside one user's namespace. The workspace
digest stays the leaf, so setup and the command still derive the same root for
the same workspace.
Windows keeps the fixed names deliberately: its temp root already resolves
inside the user profile, and windowsSandboxRuntimeOwnedTail compares those
names to recognise a root built by an elevated setup running as another
account. The test pins both directions so neither platform's choice can drift
into the other.
The rollback test compared the full SDDL of the security descriptor before and
after, which carries the owner, the group and the control flags as well as the
access. It passed here and failed on the Windows runner with the three access
entries byte-identical on both sides: SetSecurityInfo sets
SE_DACL_AUTO_INHERITED when it writes a DACL, and GetNamedSecurityInfo did not
report the owner and group the same way there.
None of that grants anyone anything, and the test is about whether a capability
grant survived a failed setup, so compare the access-control entries instead.
The failure message now names the entry that survived rather than printing two
SDDL strings to diff by eye.
Three things, all on the elevated Windows setup path.
The capability ACL is committed by SetSecurityInfo before the ride-along stamp
is written, and the apply returns a rollback closure only on success, so a
stamp failure left the caller with nothing to compensate with. Setup reported
failure while leaving the grant on a pre-existing runtime root. The captured
descriptor is now restored through the same bound handle before the error goes
back, and a test hook makes that path reachable.
Runtime-root selection was best effort in BuildWindowsSandboxSetupArgs: on
failure setup carried on and wrote a marker recording no runtime root, having
already applied the ACLs. A later command selects a concrete root of its own,
finds no stamp on it and refuses, and re-running setup takes the same branch
again. That is the permanent brick the recorded-root contract exists to
prevent, reached by the one path allowed to skip it. Selection failure now
fails setup before any ACL or marker state is persisted.
The fallback-runtime test stubbed only the cache derivation, so the temp
derivation still read the process environment and the deterministic root landed
in the developer's real temp directory and stayed there. Both inputs are now
test owned, and the test asserts the redirect took rather than assuming it.
The runtime root grants the capability SID FILE_GENERIC_WRITE with
SUB_CONTAINERS_AND_OBJECTS_INHERIT, which is what lets a sandboxed command
write TMP, GOCACHE, GOMODCACHE and the package-manager caches beneath it. The
stamp is created inside that root, so it inherited the same grant. A restricted
command carrying the capability could open and overwrite the file attesting its
own setup after passing its own pre-launch validation. Its command would
continue, and every later elevated command and zero doctor would then reject
the altered plan hash until an Administrator re-ran setup: a sandboxed process
bricking the sandbox for everything after it.
Moving the attestation outside the tree is the other way to close it and is
worse. The stamp works precisely because it dies with the tree, which is how
eviction is detectable without reading an ACE. Keeping it inside with
inheritance switched off preserves that property.
The stamp now gets an explicit DACL naming only CREATOR OWNER, LocalSystem and
Administrators, applied with PROTECTED_DACL_SECURITY_INFORMATION. Protected
rather than merely explicit, because without that bit the inherited capability
ACE stays in the DACL beside whatever is set. Both writers do it, and both open
with WRITE_DAC now, which the regression caught: SetSecurityInfo needs it on the
handle and the first version failed with "Access is denied".
The regression grants a stand-in capability SID an inheritable write on a real
root, asserts an ordinary descendant DOES inherit it so the case is genuinely
exercised, then asserts the stamp neither grants that SID nor leaves its DACL
unprotected, and that setup can still read back what it wrote.
The stamp is supposed to prove that the directory a command later uses is the
one setup granted the capability ACE to, and writing it afterwards by pathname
cannot prove that however careful the second open is. The ACE went on through a
rooted handle, that handle closed, network setup ran, and only then did the
marker write re-open the name. A local process that can reach the user-owned
runtime tree removes the predictable root in that window and puts an ordinary
directory in its place. The re-open rejects a junction correctly, but an
ordinary replacement is not a reparse point and is not the ACL-bearing object
either, so it collected a valid-looking stamp. Marker validation then passed
over a directory with no capability ACE, and the next WRITE_RESTRICTED command
failed its cache and TMP writes while setup insisted it was current.
The stamp now rides along with the ACE. The marker is built before the apply so
the plan hash is known, the apply writes the stamp through the same handle it
just set the DACL on, and the marker write records the file and never names the
runtime tree again. The rollback snapshot moves ahead of the apply for the same
reason, so a failed setup restores what it found rather than its own artifact.
The regression drives the real apply and swaps the directory in the window
between the two, which is the only place the difference shows. A direct
handle-level test cannot see it: any name derived from a retained handle
resolves back to the same object, so a pathname write derived that way lands
correctly and the test passes either way. I had written that test first and it
survived the falsification, so it is gone rather than left looking like cover.
Also: the doctor stamp test was creating, removing and recreating a directory
under the developer's REAL user cache, because the runtime candidate is derived
from the process's actual cache directory. The workspace digest made a collision
with a live runtime tree unlikely rather than impossible, which is not the
standard for a test that calls RemoveAll. It redirects the cache to test-owned
storage now, and fails loudly if that ever stops working.
An audit of this branch turned up a regression this PR itself introduced, plus a
gap it left in doctor.
The temp fallback root used to come from os.MkdirTemp, which mints a random name
atomically at mode 0700 so no other local account can name it. Deriving it from
a digest of the workspace path bought stable cache reuse and gave the name away:
every component is computable, and on Linux os.TempDir() is the world-writable
/tmp whenever TMPDIR is unset. os.MkdirAll returns nil when Stat says the path
is already a directory and Stat follows links, so a link planted at an owned
component was accepted silently, the cache, data and tmp directories were built
inside whatever it pointed at, Chmod and Chtimes followed it too, and the root
was then handed to the backend as a write root (a read-write bind under bwrap)
with TMPDIR, GOCACHE, GOMODCACHE and the package-manager caches inside it. The
sandbox would have been granting the confined command write access to a
directory somebody else chose. Reproduced end to end before fixing.
The fallback digest now covers a per-user scope, and every component Zero owns
is checked for a link and for foreign ownership before anything is created
through it and again afterwards, on the shared path all three platforms take.
A link or a foreign owner is refused outright rather than relocated around,
because relocating leaves the link in place and moves to the next predictable
name; an ordinary file sitting where a component belongs is a broken machine
rather than a hostile one and still relocates, as before.
doctor built its profile with PermissionProfileFromPolicy, which never sets
Runtime, and validateWindowsSandboxRuntimeStamp returns nil early in that case.
So the one check that can tell an evicted runtime tree from a healthy one was
skipped and `zero doctor` reported a healthy sandbox on exactly the state the
stamp was added to detect. It reads the recorded root from the marker now, which
takes no lease and creates nothing.
One test in this branch was vacuous and is fixed here too: t.TempDir() names its
directory after the test, the subtest was called "link N levels above the leaf",
and strings.Contains(err.Error(), "link") was matching the path rather than the
reason, so all four subtests passed with the refusal deleted. Both that test and
the Windows traversal test now assert the sentinel and a distinctive phrase.
Three problems in the Windows setup-to-command contract.
Setup selected a runtime root, released the lease and recorded a plan hash over
pathnames, and a command later ran the same selector again. Selection consults a
lease, and a lease is a fact about one moment: with the cache root briefly
unusable setup provisioned and recorded the temp fallback, and once it freed up
the command selected the cache root instead. Two selections, two roots, a marker
that could never validate, and re-running setup did not help because setup was
equally free to pick the other one. The concrete root is recorded in the marker
now and the command consumes it. A recorded root is only honoured when it is one
of the two this workspace derives, so a root belonging to another workspace is
ignored, and a recorded root that cannot be leased fails with a message naming
the situation instead of relocating into a tree nothing provisioned.
The reparse-point checks inspected pathnames, and everything after them reopened
the tree by name. FILE_FLAG_OPEN_REPARSE_POINT governs only the final component,
so a junction planted at an owned ancestor after the last check was followed and
the elevated capability ACL landed on a leaf inside the attacker's directory.
Junctions need no privilege. A second check narrows that window and cannot close
it, so the owned tail is now walked one component at a time through NtCreateFile
relative to the handle above it, refusing a reparse point at each step, and the
resulting handle is what the ACL apply and the stamp write use. The stamp used
MkdirAll and a pathname write, which was the same hole again after the ACL had
been applied. The base above the owned components is still followed: a
redirected LOCALAPPDATA is ordinary configuration, not an attack.
Rollback returned as soon as the ACL restore reported an error, so the runtime
rollback never ran and the failure most likely to leave a machine in a strange
state was the one that skipped half the cleanup. Every compensation runs now and
the errors are joined. The stamp is part of the rollback record too: it lands
inside the runtime root before the marker is renamed into place, so a late
failure used to leave a root that was non-empty and therefore unremovable by
design. A stamp that was already there is restored rather than deleted, so a
machine whose previous setup succeeded does not start reporting itself broken.
The three P1s, and they were one defect: setup recorded what it INTENDED, a
fingerprint of a plan built from a root it merely derived, and never what it
actually provisioned.
A command derived the same cache root, failed to LEASE it, and silently
relocated to the temp fallback. Its plan then named a tree setup had never
provisioned, and the marker rejected it:
windows sandbox setup is out of date: permission roots or deny lists changed
(marker plan e0b1c3fec819, 2 entries; this command wants 8a75a38d0006, 2 entries)
blaming permissions for a runtime-root disagreement. Re-running setup could not
recover, because sandboxRuntimeRootFor rejects a candidate only for landing
inside the workspace, never for being unusable, so setup chose the same
unleasable root again. The only escapes were deleting the marker, which silently
drops WFP network enforcement, or turning the sandbox off, and the error
mentions neither. Setup and commands now select through one function, lease
attempt and fallback included, so a relocation is something they agree on.
The marker also could not tell whether the directory its pathnames resolve to
was still the one setup provisioned. cleanupSandboxRuntimeRoots evicts inactive
roots and the next run recreates the same pathname with ordinary inherited
permissions; the plan hash is over pathnames, so both marker checks reported
setup as current while the tree carried no capability ACE and a WRITE_RESTRICTED
token could not write TMP or GOCACHE. Setup now stamps the tree it provisioned,
alongside the marker and after the ACL has applied, and validation reads it. A
file inside the tree survives exactly as long as the tree does, so eviction is
detectable without needing to read an ACE.
Provisioning also created by pathname. Every component below the user cache root
is ours, a local user needs no privilege to create a junction, and planting one
at zero, runtime or v1 made the leaf land in their tree while
openWindowsACLTarget saw no reparse point ON THE LEAF and would have granted the
sandbox capability write access to a directory they control. Refused at every
component we own, before and after creation, and deliberately NOT above them:
a redirected LOCALAPPDATA is an ordinary configuration.
That last point caught a regression from the previous commit on this branch. Its
existence walk used os.Lstat, which reports a junction as not-a-directory, so a
redirected cache root was refused outright with "exists and is not a directory".
Existence follows links now; whether a link is acceptable is the separate
question above.
The marker schema is bumped, because a marker written by the previous version
has no stamp and requiring one without a bump would report every already-set-up
machine as broken rather than as out of date.
NOT VERIFIED HERE, said plainly: the elevated apply needs Administrator and this
machine is not. Everything above was exercised unelevated through the real
entry points, and the ACL write itself was not.
Two of the five findings from review. The three P1s are not addressed here and I
have said so on the PR rather than implying otherwise.
Setup materializes runtime roots before the network plan, the ACL apply, the
network apply and the marker write. Any of those can fail, and the rollback that
existed restored only ACL snapshots, so a run that reported failure still left
new persistent runtime directories behind. It could not have cleaned them up
even in principle, because provisioning returned nothing about what it had made.
Provisioning now records the components it actually created and hands back a
rollback, composed once at the top of the elevated path so no later failure can
forget it. It removes only what this run created, innermost first, and uses
os.Remove rather than os.RemoveAll: a directory that is not empty by then holds
something this run did not create, and removing it would turn a failed setup
into data loss. Refusing keeps the residue findable and reports it.
The provisioning test derived a real user-cache runtime path because it took a
bare t.TempDir() instead of the owned fixture the neighbouring tests use, so it
created and then RemoveAll'd a genuine ~/.cache/zero/runtime tree on every run
and failed outright on a read-only home. runtimeRootTestConfig now routes
through windowsRuntimeTestRoots, which redirects every derivation input before
any candidate is computed and refuses to run if a candidate escapes the owned
roots. Centralizing it there covers the other tests built on that config too.
The guard added with the provisioning tests measured a canonicalized
candidate against a raw t.TempDir(), so it fired on every machine where
those two spellings differ and passed on the one where they do not. macOS
resolves /var to /private/var and the CI Windows runner's profile has an
8.3 name, so both failed; this box has neither and stayed green.
That is the same one-sided comparison this PR fixes in
fallbackSandboxRuntimeRoot, written into the guard meant to protect
against it. Both sides go through canonicalSandboxWorkspaceRoot now.
Review found the setup marker depended on the caller's transient TEMP.
Setup run from one terminal recorded a plan containing that shell's
temp-derived candidate; a later parent process started by an IDE or a
service with a different TEMP built a different path, and every command
failed the marker's equality check while the cache runtime sat there
untouched and healthy. That is the outage in #881 arriving by another
route.
The cause was two derivations of the same thing. sandboxRuntimeRootFor
decides the tree the process uses; windowsSandboxRuntimeCandidates
separately decided the tree the marker fingerprints, and folded in BOTH
candidates so that whichever the command picked was covered. That bought
agreement at the price of putting os.TempDir() into a machine-wide
fingerprint.
This removes the second derivation rather than trying to keep two in
step. windowsSandboxRuntimeRoots pins to profile.Runtime.Root when the
profile has one, because by then prepareSandboxRuntime has chosen,
created and leased that tree, so the plan names exactly what the command
writes to, including after a lease-failure relocation. Callers with no
runtime yet (elevated setup, doctor) derive through sandboxRuntimeRootFor
itself, so setup and command cannot drift.
The lease-failure relocation in prepareSandboxRuntime is deliberately
left alone. It is load-bearing off Windows, where there is no marker and
relocation is inherently consistent, and removing it would trade this bug
for a cross-platform one.
Scope, since the finding is broader than what this closes: the runtime
root no longer tracks TEMP, but the full plan hash still does, because
PermissionProfileFromPolicy grants os.TempDir() as a write root when the
policy allows temp. That predates this PR. The test says so in a comment
rather than asserting a stability the code does not yet have.
Both new tests fail with the change reverted.
The new provisioning tests derived their runtime candidates from the real
os.UserCacheDir() and then cleared one to prove provisioning recreates it,
so every run deleted a directory under the developer's actual cache and
the tests failed outright on a read-only home. The test has no ownership
claim on that path, so it must not derive one.
windowsRuntimeTestRoots now stubs sandboxUserCacheDir and TMP/TEMP to
t.TempDir() before deriving anything, and refuses to run at all if a
derived candidate falls outside those owned roots, so a later change to
the derivation cannot quietly reintroduce this.
Measured against the real cache directory: the previous version added an
entry on every run, this one adds none.
Review found that the runtime roots this PR folds into the permission
profile were granted but never created. applyWindowsACLPlan materializes
only DenyRead targets, so an AllowWrite target that does not exist fails
the whole run with "windows ACL target does not exist" -- turning a clean
`zero sandbox setup` into a different outage than the one being fixed.
ensureWindowsSandboxRuntimeCandidates already existed for exactly this
reason and its doc comment names the failure, but the split from the
parent PR carried the function across without its call site.
Provisioning now sits with whoever computes the candidates, because the
two have to happen in the same environment:
- buildWindowsSandboxSetupACLPlan provisions, then builds the elevated
plan.
- windowsSandboxProfileWithProvisionedRuntime provisions, then returns
the command profile, and BuildCommandPlan calls it in the PARENT. The
unelevated tier applies its own plan inside the re-exec'd runner, where
TEMP points into the runtime tree, so the runner can derive neither the
paths nor the directories.
Also from review:
- doctor checked the marker against the bare profile, so `zero doctor`
reported a correctly prepared machine as out of date. It now folds in
the same roots setup and the command plan use.
- fallbackSandboxRuntimeRoot derived from a raw os.TempDir() while
pathWithinRoot compares spellings, so an aliased TEMP read as outside
the workspace. Canonicalized like the workspace and cache roots. This
closes short-name and symlink aliases, not junctions, which
EvalSymlinks returns unchanged; the comment says so rather than
implying the case is shut.
- windowsSandboxRuntimeCandidates deriving from the first workspace root
only is deliberate, not a defect: the marker compares plan hashes for
equality and a command presents one root, so widening it would put
entries in the marker no command reproduces. Documented and pinned by a
test, since the suggested fix would have reintroduced the outage.
Tests drive production entry points rather than the helpers behind them.
The previous round shipped tests that called the helpers directly and
stayed green with the call sites deleted, which is how the missing
provisioning survived. Each of the four new assertions was verified to
fail with its fix reverted.
Every exec_command on a Windows machine that had run `zero sandbox setup`
aborted with "windows sandbox setup is out of date: permission roots or
deny lists changed". File tools worked, only shell execution died, and
`zero doctor` reported the backend healthy throughout, so there was
nothing pointing at the cause. Reported in #881 with an accurate trace.
Setup fingerprinted the bare permission profile into the marker. Every
command arrived with the per-workspace runtime root already appended by
permissionProfileWithRuntime, so the plan hash the runner computed could
never match the one setup stored, and a marker written seconds earlier
was rejected forever.
Both sides now fold in the same runtime candidate set before the profile
is fingerprinted.
Three things this needs to get right, each of which broke it once.
BOTH candidates, not the one this process would pick.
sandboxRuntimeRootFor prefers the cache-derived root and falls back to
the temp-derived one when the cache sits inside the workspace, and that
choice is per process. Granting only one left a command that fell back
writing to a tree with no ACE on it.
The fallback has to be derived rather than minted. It used os.MkdirTemp
memoized in a process-global map, so the answer was private to whichever
process asked first: setup granted temp root A, the next command derived
root B, teardown cleaned a third. It is now a hash of the workspace and
creates nothing, so every process agrees without sharing state.
The runner cannot derive the candidates itself. It runs re-exec'd with
TEMP and TMP already pointed at the sandbox runtime temp, so os.TempDir()
there returns the redirected value and the temp-derived candidate comes
out rooted under the runtime tree. The profile is augmented in the parent
and passed down.
Split out of #808, which carries this fix among the Windows principal
work. That PR has open architectural questions and this does not, and
the bug is a user-visible outage on one platform, so it should not wait
on them.
On the tests, because one of them was not enough. The composition test
calls the helper directly and stays green even with the production call
site deleted, which is the same class of bug as the fix itself. The
added call-path test drives BuildCommandPlan and asserts the runtime
roots reach the runner's argv; reverting the runner call fails it with
the missing root named.
2026-09-02 11:56:23 +05:30
928 changed files with 41716 additions and 3530 deletions
echo "::error::dist-tag latest points at platform version ${latest} — a platform publish clobbered it. Fix the dist-tag before the wrapper publish." >&2
exit 1
;;
esac
echo "dist-tag latest is ${latest} (unchanged by platform publishes)."
* **cli:** let providers subcommands run with no active provider ([#1143](https://github.com/Twigpine/zero/issues/1143)) ([d1a0d04](https://github.com/Twigpine/zero/commit/d1a0d048ee2c1e7cbfb75160ec4c9f099f3c4472))
* **npm:** wait for npm to apply each publish, and drop the one-time @gitlawb/zero steps ([#1145](https://github.com/Twigpine/zero/issues/1145)) ([8e363fc](https://github.com/Twigpine/zero/commit/8e363fcaa5d51bf5b5b244692dc61bf8d4ae8799))
* **tui,cli:** stop a repository's git config from running programs in Zero's own git calls ([#1133](https://github.com/Twigpine/zero/issues/1133)) ([640d936](https://github.com/Twigpine/zero/commit/640d936bfbe00c4cd3017ca35fb7b5c15d91a1c5))
* **workspaceindex:** skip git-ignored and build-cache dirs in Scan only ([#1117](https://github.com/Twigpine/zero/issues/1117)) ([c88c77b](https://github.com/Twigpine/zero/commit/c88c77bf701039dca01d9786bf5648d779c949b3))
### Performance Improvements
* **redaction:** add fast-path byte checks to eliminate regex allocations on clean strings ([#945](https://github.com/Twigpine/zero/issues/945)) ([ce2fcff](https://github.com/Twigpine/zero/commit/ce2fcffaf10d8e8c8c5a13e06fd7809f09fc9867))
* **notify:** make permission-prompt alerts on by default and discoverable ([#1001](https://github.com/Twigpine/zero/issues/1001)) ([7f8e5f0](https://github.com/Twigpine/zero/commit/7f8e5f08bb83371ce16cc54c97ab032a010026cf))
* **npm:** publish as @twigpine/zero ([#1142](https://github.com/Twigpine/zero/issues/1142)) ([530563a](https://github.com/Twigpine/zero/commit/530563a0bb58d3cbadceb97b3ee234ef763124df))
* **sessions:** add zero sessions prune, which never removes a session another process has open ([#1089](https://github.com/Twigpine/zero/issues/1089)) ([23c27e0](https://github.com/Twigpine/zero/commit/23c27e0778e3af54c04b8749d1eda846d3253f4e))
### Bug Fixes
* **acp:** persist the session permission mode across restarts ([#1134](https://github.com/Twigpine/zero/issues/1134)) ([bb96206](https://github.com/Twigpine/zero/commit/bb96206c2781e846fd13437113eec5790afe9c81)), closes [#1125](https://github.com/Twigpine/zero/issues/1125)
* **cron:** bound run history and read recent records from the tail ([#1094](https://github.com/Twigpine/zero/issues/1094)) ([42eb6ab](https://github.com/Twigpine/zero/commit/42eb6ab5fbe4ae9037f1f4cd42a00243c456a22e))
* **daemon:** bound pending sessions and prune on completion ([#1091](https://github.com/Twigpine/zero/issues/1091)) ([8a68660](https://github.com/Twigpine/zero/commit/8a68660161b04ce9668363faa14ff72c9fbad223))
* **daemon:** do not retry workers that finished with a final exit code ([#1112](https://github.com/Twigpine/zero/issues/1112)) ([eefa978](https://github.com/Twigpine/zero/commit/eefa978f6183332b7416c70c25242bcdfba1a75c)), closes [#1100](https://github.com/Twigpine/zero/issues/1100)
* **daemon:** stop worker process groups by their launch-time identity ([#1088](https://github.com/Twigpine/zero/issues/1088)) ([ab018b3](https://github.com/Twigpine/zero/commit/ab018b3098b5cb103cf8f90b8698d32a6687bdf7)), closes [#861](https://github.com/Twigpine/zero/issues/861)
* **deps:** refresh the lockfile to patched hono and @hono/node-server ([#1080](https://github.com/Twigpine/zero/issues/1080)) ([aa52040](https://github.com/Twigpine/zero/commit/aa52040498e012c41d5e8f6027a91c18eff861d4))
* **deps:** refresh the lockfile to patched shell-quote ([#1131](https://github.com/Twigpine/zero/issues/1131)) ([c56a4c4](https://github.com/Twigpine/zero/commit/c56a4c4e6e598504ab417e8cc4f5ab8d69f1b42f))
* **execution:** reserve process capacity before launching ([#1092](https://github.com/Twigpine/zero/issues/1092)) ([bb3346a](https://github.com/Twigpine/zero/commit/bb3346a82883c19ed8815fe1e874f54950711d69))
* **mcp:** forward image tool results on the existing image channel ([#1077](https://github.com/Twigpine/zero/issues/1077)) ([0324c1d](https://github.com/Twigpine/zero/commit/0324c1d4103328eb90cb24a0b582dba335255b5c))
* **mcp:** refresh OAuth tokens for discovered endpoints and registered clients ([#1113](https://github.com/Twigpine/zero/issues/1113)) ([571af70](https://github.com/Twigpine/zero/commit/571af701bd3d8dfaf40f87b5b112b25e7def5b29)), closes [#1101](https://github.com/Twigpine/zero/issues/1101)
* **oauth:** give every loopback callback server the bounded shutdown ([#1079](https://github.com/Twigpine/zero/issues/1079)) ([5473301](https://github.com/Twigpine/zero/commit/547330197b20ece591e077e876a4b8ffd015cb6e))
* **providers:** send stable session headers to OpenCode ([#1060](https://github.com/Twigpine/zero/issues/1060)) ([99721c7](https://github.com/Twigpine/zero/commit/99721c762f37cd43ac511007a5f51d1846df959e))
* **redaction:** redact adjacent AWS keys glued end to end ([#1095](https://github.com/Twigpine/zero/issues/1095)) ([df67ede](https://github.com/Twigpine/zero/commit/df67edec13345b73a85327adc003cacc7bdd8009))
* **redaction:** redact credentials split by an invisible character ([#1067](https://github.com/Twigpine/zero/issues/1067)) ([e20b762](https://github.com/Twigpine/zero/commit/e20b7624261cef8336a8eb0fbdd44d536ad77fc2))
* **sandbox:** say when a session's sandbox enforcement is degraded ([#1082](https://github.com/Twigpine/zero/issues/1082)) ([6b5852d](https://github.com/Twigpine/zero/commit/6b5852dc18bb256af1f25745697c2f2e3ea6f112))
* **search:** redact every text field of session metadata, not ten of thirty ([#1073](https://github.com/Twigpine/zero/issues/1073)) ([9520f2b](https://github.com/Twigpine/zero/commit/9520f2bad7e09ce4a8d0d42be7ce77d2ae928ddd))
* **security:** harden shell flag parsing, write roots, and formatters ([#1066](https://github.com/Twigpine/zero/issues/1066)) ([ac5fe0d](https://github.com/Twigpine/zero/commit/ac5fe0d21849b0f74af428dd3ca12e3b80a8b7aa))
* **sessions:** keep a session lease for the life of the process, not of the Store ([#1144](https://github.com/Twigpine/zero/issues/1144)) ([883e5be](https://github.com/Twigpine/zero/commit/883e5bec24dac791d1d60618c7f4927f4659b687))
* **tools:** hint when Windows resolves POSIX paths under root ([#1063](https://github.com/Twigpine/zero/issues/1063)) ([0c6f179](https://github.com/Twigpine/zero/commit/0c6f179a8443d4dfdfbbefe5cc6caf11c2a525f0))
* **tui:** bound live dictation regions ([#985](https://github.com/Twigpine/zero/issues/985)) ([feb9927](https://github.com/Twigpine/zero/commit/feb992757c582c68f0397e070e206d4bf07071e1))
* **tui:** show provider ownership in Recent and Favorites ([#1083](https://github.com/Twigpine/zero/issues/1083)) ([1697b7a](https://github.com/Twigpine/zero/commit/1697b7a5ce948e781877cdbe9223adac62c3fde6))
* **tui:** stop held peer message Allow/Deny from deadlocking Update ([#1111](https://github.com/Twigpine/zero/issues/1111)) ([a195e88](https://github.com/Twigpine/zero/commit/a195e8848cec1a27b4fe1789ecc572b943c32186)), closes [#1099](https://github.com/Twigpine/zero/issues/1099)
* unbreak main CI (cron claim tests, Go 1.26.9) ([#1124](https://github.com/Twigpine/zero/issues/1124)) ([2c418fc](https://github.com/Twigpine/zero/commit/2c418fc8ff6763de16703128f372fe526fddd331))
* **acp:** add standard session list and resume ([#914](https://github.com/Gitlawb/zero/issues/914)) ([c1937df](https://github.com/Gitlawb/zero/commit/c1937dfac72e6ad0e5ade6e48e2d9c17d9c3e5d6))
* **acp:** make configured MCP tools callable ([#1003](https://github.com/Gitlawb/zero/issues/1003)) ([7f5e5af](https://github.com/Gitlawb/zero/commit/7f5e5af7bbdbe5bb4d91706146072d461c0234a4))
* **agent:** stop the tool-schema ratchet meaning a different thing per host ([#1017](https://github.com/Gitlawb/zero/issues/1017)) ([f909266](https://github.com/Gitlawb/zero/commit/f9092668a832ea73a69551f0ea2df4f6eaf356df))
* **background:** treat POSIX zombie leader as already-exited ([#981](https://github.com/Gitlawb/zero/issues/981)) ([1aacf38](https://github.com/Gitlawb/zero/commit/1aacf38b727d31b5ca09949316d6afa8292aec46))
* **config:** lock config read-modify-write across processes ([#960](https://github.com/Gitlawb/zero/issues/960)) ([b04fe96](https://github.com/Gitlawb/zero/commit/b04fe96e9ef3f2deda4fb2a972270300629079d2))
* **daemon:** publish status files atomically ([#949](https://github.com/Gitlawb/zero/issues/949)) ([9001421](https://github.com/Gitlawb/zero/commit/90014210d2ada2e3d61f4eca5087d3f35a3c4b8b))
* **installtxn:** put back an install a killed commit left behind ([#997](https://github.com/Gitlawb/zero/issues/997)) ([2837f5e](https://github.com/Gitlawb/zero/commit/2837f5efb38dcd3c69771b0ba31565f98a206120))
* let the configured forward proxy through the loopback guard ([#1025](https://github.com/Gitlawb/zero/issues/1025)) ([1e4db7c](https://github.com/Gitlawb/zero/commit/1e4db7c6643ca7e2c3d8e4b8b30371cfbe43561e))
* **lockutil:** use kernel-held advisory locks ([#950](https://github.com/Gitlawb/zero/issues/950)) ([6fe0d1e](https://github.com/Gitlawb/zero/commit/6fe0d1edcdbadea6d566800fbcca739c1cd54ea1))
* **mcp:** discover OAuth through protected resource metadata ([#991](https://github.com/Gitlawb/zero/issues/991)) ([1b5db17](https://github.com/Gitlawb/zero/commit/1b5db1765672820caac1684b168c9898b5ba3593))
* **mcp:** reject a server-initiated request as a streamable-HTTP response ([#1054](https://github.com/Gitlawb/zero/issues/1054)) ([2479d94](https://github.com/Gitlawb/zero/commit/2479d946c15dbf74ee3c04a637e97181d1786f08))
* **oauth:** force-close loopback callback connections past the shutdown deadline ([#1051](https://github.com/Gitlawb/zero/issues/1051)) ([176f5a2](https://github.com/Gitlawb/zero/commit/176f5a242b754a86ea4c7121a8dcc1c0d72cf2e4))
* **oauth:** Split an oversized keyring token blob across entries ([#1007](https://github.com/Gitlawb/zero/issues/1007)) ([275e248](https://github.com/Gitlawb/zero/commit/275e2486695338ce38193a6e3319f8b6a9a5c33c))
* **providerio:** close idle connections periodically to prevent HTTP/2 PROTOCOL_ERROR ([#1058](https://github.com/Gitlawb/zero/issues/1058)) ([b76c764](https://github.com/Gitlawb/zero/commit/b76c764b77926891aa67163e6605a2d8540aacaa))
* **providers:** register Atomic Chat as local OpenAI-compatible runtime ([#550](https://github.com/Gitlawb/zero/issues/550)) ([79fa142](https://github.com/Gitlawb/zero/commit/79fa1422ef1a3691b93342788453ebc284d57f9c))
* **redaction:** split camelCase keys in normalizeKey ([#994](https://github.com/Gitlawb/zero/issues/994)) ([08fbea7](https://github.com/Gitlawb/zero/commit/08fbea7a3221d2061d88038924f90a4cba9af1ac))
* **sandbox:** keep a derived ACL target inside the write root it came from ([#1040](https://github.com/Gitlawb/zero/issues/1040)) ([3b43c43](https://github.com/Gitlawb/zero/commit/3b43c43eca968c8619adadc81fcc1a17e220216a))
* **sandbox:** keep Windows restricted-token SIDs narrow and fail closed on DenyRead ([#1006](https://github.com/Gitlawb/zero/issues/1006)) ([0207f2b](https://github.com/Gitlawb/zero/commit/0207f2b2697149a73ecc89a5ab5db0bd665f1911))
* **sandbox:** Make WSL backend tests deterministic. ([#948](https://github.com/Gitlawb/zero/issues/948)) ([e59bf78](https://github.com/Gitlawb/zero/commit/e59bf78cabc2703678778b904c27c8bc8ef82bc8))
* **sandbox:** refcount temporary roots so one holder's cleanup cannot revoke another's ([#911](https://github.com/Gitlawb/zero/issues/911)) ([7acac7a](https://github.com/Gitlawb/zero/commit/7acac7acee65565e25089f2a7b5813c086ce65eb))
* **sessions:** carry an interrupted turn's work into the resume prompt ([#1016](https://github.com/Gitlawb/zero/issues/1016)) ([6ae8396](https://github.com/Gitlawb/zero/commit/6ae8396f0282a5f18a431f0c0ef1a272ecc2ca2b))
* **sessions:** render resume context fields in key order ([#1043](https://github.com/Gitlawb/zero/issues/1043)) ([da244bc](https://github.com/Gitlawb/zero/commit/da244bcbfcfa05e414567ac0acc3cf8ba745ad7d))
* **specialist:** keep the pinned model when a specialist is resumed ([#1009](https://github.com/Gitlawb/zero/issues/1009)) ([aadb4a2](https://github.com/Gitlawb/zero/commit/aadb4a27e9cdb41e774e8e1a8f23cf9c2fb50db2))
* **swarm:** stop original member before handoff ([#957](https://github.com/Gitlawb/zero/issues/957)) ([0021475](https://github.com/Gitlawb/zero/commit/0021475d4c58c91911098f414795512cf60a0b56))
* **tools:** an edit keeps the reads it did not disturb ([#908](https://github.com/Gitlawb/zero/issues/908)) ([1649008](https://github.com/Gitlawb/zero/commit/1649008506da0e675431dc056251a01891b15cc8))
* **tools:** make apply_patch tolerant and re-expose edit_file ([#956](https://github.com/Gitlawb/zero/issues/956)) ([2749447](https://github.com/Gitlawb/zero/commit/2749447cdce4ae5dd333538b36fa919b80ecc66d))
* **tools:** reject fuzzy edits with drifted content ([#983](https://github.com/Gitlawb/zero/issues/983)) ([8b3d1b1](https://github.com/Gitlawb/zero/commit/8b3d1b1597b04bfa6d4bc7f912abc13ca80a6298))
* **tools:** say when format-on-write ran out of time ([#1044](https://github.com/Gitlawb/zero/issues/1044)) ([6937a30](https://github.com/Gitlawb/zero/commit/6937a309cf00825572210a7610a1f3ea8b74c2f9))
* **tui:** move voice capture to Ctrl+Space ([#968](https://github.com/Gitlawb/zero/issues/968)) ([eeea330](https://github.com/Gitlawb/zero/commit/eeea3308fed38b37b203522624babaf2f30df91f))
* **update:** keep Windows failed-restore state in trusted recovery records ([#979](https://github.com/Gitlawb/zero/issues/979)) ([a3852ba](https://github.com/Gitlawb/zero/commit/a3852bac595e06c5d3ca9da22212b281b8796802))
iferr:=h.client.Call(ctx,MethodSessionLoad,LoadSessionParams{SessionID:meta.SessionID,Cwd:cwd,McpServers:[]McpServer{}},&LoadSessionResult{});err==nil||!strings.Contains(err.Error(),"locked by zero sessions prune"){
t.Errorf("session/load while prune holds the session: err = %v, want it refused",err)
}
iferr:=h.client.Call(ctx,MethodSessionResume,ResumeSessionParams{SessionID:meta.SessionID,Cwd:cwd,McpServers:[]McpServer{}},&ResumeSessionResult{});err==nil||!strings.Contains(err.Error(),"locked by zero sessions prune"){
t.Errorf("session/resume while prune holds the session: err = %v, want it refused",err)
constmaxTurnsAnswer="Agent reached maximum number of turns without a final answer."
constmaxTurnsFinalAnswerPrompt="You have reached the tool-turn limit. Do not call tools. Give a concise final answer now: summarize what you completed, what you found, and any remaining blockers."
// maxStreamStallRetries bounds how many times a turn that timed out (idle/stall)
// WITH NO OUTPUT yet is re-issued on a fresh connection before giving up. Only
// the no-output case is retried (a partial turn would duplicate), so this is a
// safe recovery for a stalled/dead pooled connection.
// OR hit a mid-stream transport abort (#973) with no answer text yet is re-issued
// on a fresh connection before giving up. Incomplete tool-call previews are
// allowed through the gate (they are never dispatched before the error return).
// The safety claim is only that visible answer prose is not duplicated on retry —
// not that every partial stream shape is excluded. Covers a stalled/dead pooled
// connection or a socket abort before tool dispatch.
//
// Set to 1 (not 2): each attempt can itself idle for the full stream timeout
// (~5min) before the stall is even detected, so 2 retries left an interactive
// session frozen for ~15min. One retry keeps the common single-hiccup recovery
// while bounding the worst case to ~2× the idle timeout.
// while bounding the worst case to ~2× the idle timeout. Mid-stream transport
// aborts share this bound (do not raise it for #973).
Content:"Image output from "+label+" was not sent because the current model does not support image input. Switch to a vision-capable model before retrying image delivery, including any images skipped by the tool's image budget.",
Loaded 100 of 928 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
