* fix: address bugs found in a multi-agent codebase audit
Five parallel review agents each swept a different area (self-update,
TUI keybindings, agent/session handling, sandbox/shell safety, and
auth/credential storage) and surfaced nine concrete defects, all fixed
here with regression tests:
- sandbox: grant_scope path comparisons were case-sensitive, letting a
persisted deny grant be bypassed by spelling a Windows path with
different case
- securefile/oauth: createSecretFile's Windows ERROR_ACCESS_DENIED
handling was only applied to the oauth token store, not the shared
credstore backend; both also masked the real lock-contention error
behind a misleading "file does not exist" timeout
- cli: firstUsableProvider ignored OAuth logins when picking a
fallback provider, forcing unnecessary re-onboarding for an
already-authenticated user
- sessions: exec prompt summarization truncated on a raw byte offset,
which could split a multi-byte UTF-8 rune and embed invalid UTF-8
into a resumed session's prompt
- tui: configurable keybindings had no collision detection, so a
remapped chord could silently make a hardcoded shortcut (or another
configured binding) permanently unreachable
- tools: the Windows shell-syntax pre-flight check wasn't quote-aware
and could block harmless commands whose quoted arguments merely
contained operator-like text
- update: checksum verification didn't cross-check the archive
filename it verified against the one being extracted; extractZip
didn't reject symlink-mode entries like extractTarGz already did;
replaceBinary's restore-on-failure rename had no retry, risking a
permanently missing binary on a transient Windows file lock
* fix: close remaining PR 481 review findings
* Potential fix for pull request finding
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
* fix(tools): drop stale ; test case after rebasing onto main's MSYS check
Rebasing fix/bug-hunt-findings onto main pulled in #476, which replaced
this file's regex-based windows_shell_syntax detection with a
segment/word-anchored check and deliberately stopped treating ; as a
command separator on Windows (cmd.exe doesn't split on ; the way bash
does). detectShellCommandIssue itself came out of the rebase unchanged
from main (our quote-masking version was fully superseded by #476's
more thorough segment-based approach), but the quoted-content test
this PR added still asserted the old bash-style semantics for
`echo "foo" ; ls -la`, which now correctly returns no issue since it's
a single echo invocation with literal arguments under cmd.exe, not a
real ls invocation.
---------
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
* feat(update): add zero upgrade command to apply self-updates
Extends the existing `zero update --check` (which only reported an
available release) with an actual install step:
- `zero update --apply` / `zero upgrade` download, sha256-verify, and
install the latest release.
- npm installs delegate to `npm install -g @gitlawb/zero@latest`.
- Standalone installs download the release archive, verify its
checksum, extract it (zip-slip guarded), and atomically replace the
running binary plus any pre-existing optional sandbox helpers. On
Windows, the running exe is renamed aside (NTFS allows this even
while it's executing) since it can't be overwritten directly; the
leftover ".old" file is cleaned up on a later invocation.
Closes#459.
* fix(update): correct macOS expectations in apply test
The optional-helper assertion in TestApplyStandaloneUpdateReplacesBinary
assumed every non-Windows platform has a linux-style "zero-seccomp"
helper to refresh, but applyStandaloneUpdate correctly ships no optional
helpers on macOS (matching scripts/postinstall.mjs). This made the test
fail on macOS CI. Only assert the helper refresh on linux/windows.
* fix(update): address CodeRabbit review findings on zero upgrade
- Reject --target when combined with --apply/upgrade: applying installs
onto the current machine, so a mismatched --target could replace the
running binary with an incompatible-arch build.
- Bound archive/checksum downloads with a dedicated timeout separate
from the release-metadata check timeout, so a stalled connection
can't hang zero upgrade forever.
- Surface a failed rollback in the Windows replaceBinary path instead
of swallowing it, so a botched install is reported rather than
silently leaving the binary missing.
- Run CleanupStaleBinary regardless of whether an update is available,
so a leftover Windows ".old" file isn't stuck waiting for a future
upgrade that may never come.
- Report optional sandbox-helper refresh failures as warnings on
ApplyResult (surfaced in text and JSON output) instead of discarding
them silently.
* fix(oauth): retry secret-lock on Windows delete-pending race to unblock CI
TestLoadOrCreateSecretConcurrentConverges was failing this PR's Windows
smoke job with "create token secret lock: ...Access is denied." — a
pre-existing flake in internal/oauth, not in the upgrade code. On Windows
a concurrent holder's os.Remove leaves the .lock in a delete-pending
state, so an O_EXCL create races it with ERROR_ACCESS_DENIED
(os.ErrPermission) rather than ErrExist; createSecretFile only treated
ErrExist as contention and hard-failed otherwise.
Treat os.ErrPermission as retryable too, mirroring acquireFileLock in
lock.go. This is the same root-cause fix as #445 (diagnosis by
@gnanam1990); folded in here to make this PR's CI green. Verified on
Windows: the concurrent-converge test passes 30/30.
---------
Co-authored-by: Vasanthdev2004 <[email protected]>
