* chore: remove code unreachable on every supported platform
The Security and code health job's advisory deadcode step reports 152
unreachable functions on main. Running the same check under GOOS=linux,
windows, and darwin shows 13 of those are false positives whose callers
live in windows build-tagged files, so only functions unreachable on all
three platforms are touched.
Functions with no references anywhere are deleted, including the
internal/reasoning package, which nothing imports. Functions that only
tests reference move into per-package export_test.go files so tests keep
their seams while the production binary sheds the code. Test call sites
of deleted convenience wrappers are ported to the live APIs (scoped tool
constructors, tools.DeferredSource, FinishReason checks).
Kept despite the report: cron.Store.Runs (only reader of runs.jsonl,
which production writes), and agenteval.TraceEventKeys and
MissingTraceEvents (used across packages by the trace contract test).
* docs(config): correct ValidateFile's errors.As claim
Message is a flattened string (.Error() already called on the parse
error), not a wrapped error, so callers can't recover the concrete
json error type via errors.As. Fix the comment to match.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
* chore(trace,skills): drop obsolete Sink adapters and fix stale doc references
The dead-code pass removed the Emit methods but left Sink, NDJSONSink, and
TextSink exported with a contract nothing implements; remove the types with
their contract. Also remove the stray blank line at EOF of trace.go flagged
by gofmt, and update skills package docs and HOW_ZERO_WORKS.md to stop
directing callers to DiscoveryRoots/Duplicates, which are now test seams.
Co-Authored-By: Claude Fable 5 <[email protected]>
* chore: clean up remaining review nits on dead code removal
Replace a pointless fmt.Errorf(...%w...).Error() with fmt.Sprintf
using %v, since the wrapped error was discarded immediately anyway.
Remove a comment block that was duplicated verbatim.
* docs(skills): drop remaining references to test-only List/Get
Load and MergedSkills comments still named List/Get as production
entry points after they moved to export_test.go. Point callers at
Load and ListFromRoots instead.
* chore: remove now-unreferenced export_test.go seams
execSessionManager.len() and likelySandboxDenied/sandboxDenialKind had
no remaining callers after the sandbox unify-command-execution rebase;
the production code they wrapped for tests no longer exists.
---------
Co-authored-by: Claude Sonnet 5 <[email protected]>
- Remove unused safeRequestedPrefix, safeShellCommandTokens
(internal/agent/command_prefix.go), and truncateHeadTail
(internal/tools/bash.go): each was a thin wrapper with no remaining
callers.
- Drop an ineffectual assignment in stringListArgWebSearch: every
return path after it already sets the named return explicitly.
- Simplify a negated-and condition in
partition_cache_stable_test.go via De Morgan's law.
- Use strings.Contains instead of strings.Index(...) < 0 in
edit_replacers.go.
- Fix error string style (ST1005): lowercase "unsafe redirect URL"
and drop the trailing period from webFetchPublicOnlyHint.
Skipped the errcheck findings on this pass (unchecked defer
Close()/Write() errors): those are stylistic, not correctness bugs,
and this repo's CI only runs go vet, not golangci-lint.
* fix(tools): block MSYS coreutils under Windows sandbox
Git for Windows usr\bin coreutils (head, grep, cat, ...) are MSYS PE binaries that fail with ACCESS_DENIED and cygheap/signal-pipe errors when Zero runs them in the write-restricted Windows sandbox. Detect and block these commands before execution, surface MSYS runtime failures from stderr, steer agents toward native Zero tools or require_escalated, and exclude MSYS-prone names from known-safe command tails.
Refs #458
* fix(tools): anchor the last MSYS failure heuristic to MSYS markers
CodeRabbit review feedback: the win32-error-5 plus terminating check
had no MSYS-specific anchor, so unrelated access-denied failures could
be mislabeled as windows_msys_sandbox. Require an MSYS marker like the
neighboring checks do.
Co-Authored-By: Claude Fable 5 <[email protected]>
* fix(tools): let escalation bypass the MSYS guard and make detection quote-aware
Addresses jatmn's review on PR #476:
- bash and exec_command called detectShellCommandIssue before resolving
sandbox_permissions into a command engine, so a require_escalated retry of
a blocked command was still hard-blocked by the same preflight check.
Resolve the command engine first and only bypass the windows_msys_sandbox
issue kind when that resolves to true unsandboxed execution; other issue
kinds (real cmd.exe syntax problems) still block regardless of escalation.
- Unify all MSYS-prone command name checks (MsysProneCommandName, the
preflight scan, and transitively command_prefix.go's known-safe guard)
behind one windowsMsysProneNames set. This fixes expr (in the name set but
missing from the old regex alternations, so never proactively flagged) and
ls (hit the older windows_shell_syntax branch first, so it got different
metadata than every other MSYS-prone name).
- Make the preflight scan quote-aware: split the command into cmd.exe-
operator-separated segments respecting double-quote grouping, and only
check the first word of each segment instead of scanning raw text anywhere.
Fixes false positives on quoted text like commit messages or PR comment
bodies that merely mention a coreutil name.
* test(tools): decouple MSYS-guard bypass tests from real command output
TestBashToolRequireEscalatedMsysGuard and
TestExecCommandRequireEscalatedBypassesMsysGuardAfterApproval asserted
shell_issue == "" to prove the require_escalated bypass worked. Once
bypassed, "cat somefile.txt" actually runs unsandboxed, and its real,
PATH-dependent output could trip the unrelated post-execution
detectShellOutputIssue heuristic, failing the assertion for reasons
unrelated to the preflight guard under test.
Assert on the exit_code "-1" sentinel (set only by shellIssueBlockResult)
instead, which directly reflects whether the preflight guard blocked the
command.
* fix(tools): anchor MSYS path detection to command position, fix prefix approval gap
Address jatmn's latest review on PR #476:
- shell_runtime.go: the explicit MSYS binary-path check (usr\bin\,
mingw64\bin\, msys-2.0.dll, cygwin1.dll) ran against the raw command
string, so a quoted mention (e.g. a commit message describing a
usr\bin\head.exe failure) was still hard-blocked. It now runs against
the first word of each operator-separated segment, same as the
coreutil-name check. windowsCommandSegments also stopped treating ;
as a command separator (cmd.exe does not split on ;, unlike bash) and
now respects cmd.exe's ^ escape character, so escaped operators like
echo ^| head are no longer misread as a real head invocation.
- command_prefix.go: proposedCommandPrefix could offer to approve a
single segment (e.g. "ps aux" in "ps aux | head -5") without checking
that every other segment was independently safe. Since approving a
prefix escalates the whole command to bypass the sandbox, an
MSYS-prone or otherwise unsafe segment could ride along uncovered.
It now only proposes a segment as the prefix when every other
segment in the command is known-safe on its own.
Added regression tests for quoted MSYS path mentions, caret-escaped
operators, semicolon text, and the prefix-approval gap.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
* fix(tools): treat redirection as a command boundary, stop using command text as output evidence
Addresses jatmn's latest review on PR #476:
- firstCommandWord only split on whitespace, so cmd.exe redirection
attached directly to a command name (head>out.txt, cat<in.txt,
grep>matches.txt) was read as one word and never matched against the
MSYS-prone name set. It now also stops at < and >.
- detectShellOutputIssue scanned command+output together, so a command
whose own argument text merely quoted an MSYS failure string (e.g. a
gh pr comment --body describing the bug) could be mislabeled after
running even though the real stdout/stderr was unrelated. Removed the
command parameter entirely from detectShellOutputIssue and both call
sites (bash.go, exec_command.go), so the command line can never be
used as post-execution evidence again.
* fix(tools): remove unused truncateHeadTail wrapper
CodeRabbit nitpick: truncateHeadTailWithTotal is called directly
everywhere, so this wrapper is dead code that trips golangci-lint's
unused check and fails CI.
---------
Co-authored-by: Claude Fable 5 <[email protected]>
* Align shell approval sandbox execution
Make approved shell command-prefix executions use the escalated sandbox path when denied-read policy allows it, so remembered approvals do not still hit the restrictive sandbox.
Allow auto sandbox mode to degrade to a direct command plan when native command wrapping is unavailable, while strict sandbox require continues to fail closed.
Update regression tests for degraded execution, prefix-approved shell calls, and unavailable native sandbox metadata.
Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/sandbox
Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/tools -run 'TestRegistryRunsWithDegradedUnavailableNativeSandbox|TestBashToolRunsWithDegradedUnavailableNativeSandbox|TestBashRequireEscalatedBypassesNativeSandboxAfterApproval|TestExecCommandRequireEscalatedBypassesNativeSandboxAfterApproval'
Tested: GOCACHE=/tmp/zero-go-cache go test ./... -run '^$'
Tested: git diff --check
* Align sandbox command approval behavior
Evaluate reusable shell approvals across plain command segments so prefix grants can cover commands with safe read-only tails without relying on command-specific sandbox exceptions.
Keep process inspection commands sandboxed by default; unsandboxed host access continues to require explicit escalation or a narrow command-prefix approval.
Improve shell tool guidance around sandboxed additional permissions, full escalation, justifications, and narrow reusable prefixes.
Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/tools ./internal/agent
Tested: env HOME=/tmp/zero-test-home-clean XDG_CONFIG_HOME=/tmp/zero-test-config-clean GOCACHE=/tmp/zero-go-cache go test ./...
* Normalize explicit sandbox backend platforms
Infer the platform from a caller-provided sandbox backend name before choosing the manager runtime platform. This keeps Linux, macOS, and Windows backend plans deterministic across host OSes when tests or callers provide a backend without an explicit Platform field.
Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/sandbox ./internal/tools
Tested: env HOME=/tmp/zero-test-home-clean XDG_CONFIG_HOME=/tmp/zero-test-config-clean GOCACHE=/tmp/zero-go-cache go test ./...
* Add persistent exec command sessions
Introduce exec_command and write_stdin tools backed by a shared session manager so long-running shell commands can remain alive and be polled or interrupted across tool calls.
Wire exec_command through the existing shell permission, network prompt, command-prefix approval, sandbox grant scope, specialist manifest, and prompt/documentation paths.
Keep web_fetch limited to public remote HTTP(S) URLs so localhost/private URLs go through shell commands where sandbox network permission applies.
Tested: git diff --cached --check
* Fix exec session lifecycle review issues
Add file-backed command prefix grants to the sandbox store and load them through the engine so approved prefixes skip future prompts while still running through sandbox policy checks.
Tighten reusable prefix validation for runner-style launchers and script paths, and classify find -delete as destructive.
Surface persistent prefix choices in the TUI and include approved prefixes in the run prompt context and sandbox grants list.
Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/sandbox ./internal/tui ./internal/cli
Tested: GOCACHE=/tmp/zero-go-cache go test ./...
Tested: git diff --check
Bash permission prompts can now expose a validated session-scoped prefix approval. Prefixes are parsed from simple shell commands or an explicit prefix_rule argument, with redirects, control operators, assignments, expansions, globs, and broad launcher prefixes rejected before the option is shown.
Approved prefixes are stored in the session sandbox engine and only skip future matching bash prompts; commands still execute through the existing sandbox backend.
Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/tui
Tested: GOCACHE=/tmp/zero-go-cache go test ./internal/agent ./internal/sandbox ./internal/tui ./internal/tools
Tested: GOCACHE=/tmp/zero-go-cache go test ./...
