mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-10 13:11:31 -10:00
fix/opencode-session-header
17
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 cairn-internandcairn-intern
|
a3852bac59 |
fix(update): keep Windows failed-restore state in trusted recovery records (#979)
* fix(update): keep Windows failed-restore state in trusted recovery records A failed restore currently records the last verified binary only as a .deletable .keep marker in the install directory. Write an unresolved identity-bound record to the existing per-user update-recovery store on that failure path too, and consult it during preflight so deleting the marker cannot silence the tamper refusal. Fixes #868 * fix(update): keep a fail-closed signal if unresolved recovery records cannot be written Ignoring a failed appendUnresolvedRecoveryRecord left only the sibling .keep marker. An install-directory writer can delete that marker, after which preflight finds no unresolved state and a retry can overwrite the last verified recovery copy. Relocate that copy to a .recovery name when the per-user record cannot be persisted so promotion still refuses. Fixes #868 * fix(update): do not relocate the live binary when failed-restore compensation cannot record persistFailedRestoreSignal relocated through the handle when the per-user unresolved record could not be written, without checking that the object was still the aside copy. A restore that had already put the verified bytes back at targetPath then had that binary moved out of the executable path. Verify identity with verifyPromotedTarget before relocating. Unreadable recovery-state JSON now fails closed instead of looking like an empty queue. The per-user vs per-machine split is documented: a second account still relies on the install-directory .keep marker. Fixes #868 * fix(update): durably commit recovery state on Windows --------- Co-authored-by: cairn-intern <[email protected]> |
||
|
HazyLabandcl-ment
|
deba3adfca |
security(update): prevent tar extraction from traversing escaping symlinks (#943)
* security(update): prevent tar extraction from traversing escaping symlinks (fixes #920) When extracting tar archives, lexical path cleaning alone does not prevent subsequent file entries from being written through a previously extracted directory symlink that resolves outside destDir (Zip/Tar Slip). This introduces verifyNoSymlinkEscape in safeExtractPath, which recursively inspects existing path components between destDir and target using os.Lstat and filepath.EvalSymlinks to reject entries attempting to write through escaping symlinks. * test(update): exercise chained symlink escape during file extract The previous case was a single escaping symlink already rejected by the existing Linkname check. Seed a mid -> outside chain under destDir and extract a regular file through it so verifyNoSymlinkEscape is what fails, with no write outside destDir. * fix(update): resolve destDir symlinks and permit valid dangling symlinks in archive * docs(update): document POSIX-only scope for verifyNoSymlinkEscape * internal/update: test extracting files through directory symlinks * fix(update): reject dangling symlinks instead of lexical fallback EvalSymlinks failure used filepath.Clean on the link text, which does not preserve already-traversed links. A dangling member is now an error. * fix(update): resolve dangling symlink chains from the physical parent * fix(update): authorize tar paths through OpenRoot and reject outbound symlink entries * fix(update): resolve intermediate symlink parents and cap Root link depth Walk each path component from the physical parent, reject a ninth link, and look up release binaries through OpenRoot as regular files only. --------- Co-authored-by: cl-ment <[email protected]> |
||
|
Vasanth T
|
01c3498205 |
feat(update): recognise Homebrew installs and add mise, go install docs (#910)
* feat(update): recognise Homebrew installs and add mise, go install docs
* fix(update): match the keg shape, correct the docs, cover Apply and cross-target
* test(update): build the release fixture from the running platform
* fix(update): require a Homebrew receipt before refusing self-update
Review found the keg check was shape-only, and it was right: a directory
tree a user built themselves can have exactly a keg's shape. Classifying
one as Homebrew refuses a `zero upgrade` that would have worked
perfectly, on a machine Homebrew has never touched.
Two changes, cheap one first. The shape rule now requires the full
<prefix>/Cellar/<formula>/<version>/bin/<binary> depth rather than one
segment fewer, which already excludes the exact path the review raised.
Then, only for a path that still looks like a keg, one Stat for
INSTALL_RECEIPT.json — the file Homebrew writes into every keg and
nothing else does.
The receipt is what the old comment argued against, on the grounds that
it would put filesystem I/O on a path that runs at every version check.
That is no longer true of it: the Stat happens only after the shape test
has already matched, so an ordinary install still reaches no filesystem
here at all.
A directory named INSTALL_RECEIPT.json is not a receipt, which a bare
Stat would have accepted silently.
NOT taken: the same review asked for the Homebrew branch to be moved
ahead of the cross-target one in upgradeGuidance. That ordering is
deliberate and already pinned by
TestUpgradeGuidanceKeepsCrossTargetAnswerForHomebrew, whose comment
predicted exactly this reading — "it reads like an ordering bug until you
see which question is being answered". Homebrew is a property of the
binary on THIS machine; when the check was asked about a different
target the answer belongs to that other machine, and `brew upgrade zero`
would change this one instead. The reasoning is now recorded at the
branch as well as in the test, so the next reader finds it in both
places.
The install docs claimed detection was the Cellar path alone, which
undersold it, and stated the self-update rule absolutely in one paragraph
before excepting mise in the next. Both say the same thing now.
* test(update): give the Homebrew fixtures a receipt, and stop gating the receipt rule on the host
The previous commit made a receipt the thing that distinguishes a keg from any
directory sitting under a path containing "Cellar". Two fixtures still built a
keg without one, so detection correctly answered standalone and both failed:
TestDetectInstallMethodRecognisesAHomebrewKeg keg path: got "standalone", want "homebrew"
TestApplyRefusesToUpdateAHomebrewKeg refusal does not name the command that works:
download release archive: dial tcp: lookup example.test
The second is worth reading twice: with detection returning standalone, Apply
stopped refusing and went to the network, so the failure surfaced as a DNS error
rather than as a missing refusal. Both fixtures write a receipt now.
The reason this reached CI at all is the second half. Every test of the receipt
rule passed runtime.GOOS to isHomebrewPath, which already takes the target as a
parameter, so all of them skipped on Windows and the rule could only ever break
somewhere else. They pass an explicit target now and run everywhere. The two
that exercise DetectInstallMethod stay host-gated, because those are about the
wired entry point rather than the rule, but the rule underneath them is no
longer invisible here.
Verified the receipt path arithmetic on both non-Windows targets rather than
trusting the host-gated tests: isHomebrewPath is false before the receipt is
written and true after, with the receipt at the keg directory hasHomebrewReceipt
actually reads.
|
||
|
|
cabfeefc4e |
security(update): stage binary replacement at an unpredictable, exclusive path (#751)
* security(update): stage binary replacement at an unpredictable, exclusive path Fixes #742. installBinary staged downloaded release bytes at the fixed <target>.new path and opened it with O_CREATE|O_WRONLY|O_TRUNC. In an installation directory writable by a lower-privileged process, that process could pre-create <target>.new as a hard link or reparse point to another file the (possibly elevated) updater can write; the truncating open then overwrote that unintended file with verified Zero executable bytes. stagingFilePath now generates a cryptographically random suffix so the path can't be targeted in advance, and platform-specific createStagingFile opens it exclusively without following a pre-existing link: O_CREATE|O_EXCL on POSIX (which POSIX guarantees fails on a pre-existing path, symlink or not, without resolving it), and CreateFile with CREATE_NEW|FILE_FLAG_OPEN_REPARSE_POINT plus a post-open GetFileInformationByHandle check on Windows (not a reparse point, not a directory, single hard link). replace_windows.go's rename-swap sequence is unchanged: the dangerous operation was always the truncating open of the staged path, not the .old rename/removal that follows. Co-Authored-By: Claude Sonnet 5 <[email protected]> * security(update): bind the binary swap to the staged object, not its pathname Addresses the two open findings on #751. [P1] The staged file is no longer promoted by pathname. Randomizing the name and creating it exclusively stops PRE-creation, but under #742's model — a lower-privileged principal that can write in the installation directory — the name can be observed after the fact and the entry replaced between the write and the swap, so the updater would install the substituted file. installBinary now keeps the creating handle open and promotes through it: - Windows renames through the handle itself (SetFileInformationByHandle/FileRenameInfo, hence the added DELETE access), so there is no second pathname lookup to win. replaceBinary is gone; promote owns the aside-rename of the running binary and its restore-on-failure retry. - POSIX has no rename-by-descriptor, so staging moves into a private directory created next to the target by os.MkdirTemp (mode 0700, random name, created exclusively). An attacker who can write in the installation directory cannot create, replace, or list entries inside it. promote additionally sets the executable bit through the handle (os.Chmod would re-resolve the path) and fails closed if the entry ever stops naming the object it wrote. [P2] Staging cleanup now covers every failure path: the removal defer is registered before the copy, and stageBinary discards the object if the copy fails, so a mid-write error (ENOSPC, unreadable source) no longer leaks a release-sized file that the next attempt will not reuse. CleanupStaleBinary also sweeps leftovers from a hard crash — staging files on Windows, staging directories on POSIX, where it used to be a no-op — skipping anything younger than an hour so a concurrent update is never disturbed. Tests: substitution-race regression per platform (Windows asserts the verified bytes are installed despite a replaced staging entry; POSIX asserts the private 0700 directory and that promote refuses a substituted entry), success controls, cleanup-on-failure, and the crash-leftover sweep. The forced-staging-failure test for helper refresh now uses a stageBinary seam, because the staging location can no longer be occupied from outside — which is the point of the fix. * security(update): close remaining updater staging races Address jatmn's review findings on PR #751: - Windows promote() now verifies targetPath is actually reachable after a reported-successful handle rename before trusting it. Some Windows versions have been observed accepting SetFileInformationByHandle against a handle whose directory entry was substituted out from under it without the object actually moving, which let promote report success while targetPath was left missing entirely. renameFileByHandle is now a package var so this is covered by a deterministic regression test rather than relying on reproducing the exact trigger condition. - When a promotion failure's restore-to-.old also fails (a writable-parent attacker occupying targetPath with a lock MOVEFILE_REPLACE_EXISTING can't get past), that combination is now wrapped in ErrTargetPossiblyTampered instead of reading like an ordinary failed update, with a best-effort MOVEFILE_DELAY_UNTIL_REBOOT fallback so an admin-context updater can still recover the original at next boot. - POSIX promote() now binds its final rename to a directory descriptor opened when the staging directory is created (unix.Renameat), not a pathname. The 0700 staging directory protects its contents from a writable-parent principal, but not its own directory entry — that principal could rename the staging directory aside and recreate a look-alike with an attacker file at the same basename in the gap between the identity check and the rename, which a plain os.Rename would silently follow. - Stale-cleanup on both platforms now matches the exact generated artifact shape (POSIX: prefix + os.MkdirTemp's all-digit suffix; Windows: prefix + 32 lowercase hex chars + ".new") instead of a loose prefix/suffix, so a user's own similarly-named file or directory is never swept up. Both also re-check identity immediately before deleting to shrink the window a writable-parent principal could swap the checked path in. - copyFrom now copies in chunks and refreshes the POSIX staging directory's mtime between them, since writing into an already-created file never touches the parent directory's own mtime — a large or slow copy could previously look abandoned to a concurrent update's cleanup sweep while still in progress. Verified on real Windows and Linux (via WSL), plus cross-compiled build/vet checks for darwin/linux-arm64. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(update): address staging review findings * fix(update): verify promoted object identity safely Bind POSIX staging creation and both rename endpoints to directory descriptors, clean only identity-matched empty directories, and verify Windows promotion by file identity rather than path spelling. Preserve unverifiable crash leftovers and add adversarial cleanup, hard-link, and reparse-ancestor coverage. Amp-Thread-ID: https://ampcode.com/threads/T-019fa019-27da-72ec-8e6d-5d43f127b6a1 Co-authored-by: Pierre Bruno <[email protected]> * fix(update): preserve promotion recovery signals Amp-Thread-ID: https://ampcode.com/threads/T-019fa7a0-1223-701d-9529-48ba5d7cf8c8 Co-authored-by: Pierre Bruno <[email protected]> * fix(update): keep the recovery copy a failed restore promised Two error-path follow-ups from the #742 staging review. When a promotion fails and restoring the original also fails, the error tells the operator their original is preserved at <binary>.old — and then the next Apply deleted it, because a present target read as proof the copy was stale. What occupies the target in exactly that case is the bytes the updater could not verify, so cleanup was erasing the one it could. A failed restore now marks the copy, cleanup honors the mark, and a successful promotion clears it. The POSIX staging flow could also leave an empty .zero-stage-* directory behind when the stat immediately after mkdirat failed. Nothing knows that name yet, so it would have stayed for good; it is now removed best-effort with rmdir semantics, which cannot touch anything but the empty directory just created. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * fix(update): stop the recovery copy from being destroyed by the next attempt The marker added in d9523ba kept cleanup from deleting <target>.old, but four paths still undermined the promise it was meant to make. Promotion destroyed the copy it was preserving. Skipping the pre-rename cleanup was not enough: os.Rename uses MOVEFILE_REPLACE_EXISTING on Windows, so renaming the running binary aside overwrote the last verified copy with the unverified bytes the earlier failure left at the target — and a promotion that then failed could only move those unverified bytes back. Promotion now refuses while the tamper state is unresolved. Only the operator can say whether the file at the target is theirs, so the error names both moves that end it: restore the copy, or delete the marker to accept what is installed. The marker itself was a predictable link-following truncate write. Its path is fixed, so a writer in the install directory could pre-create it as a hard link or reparse point and have the elevated updater write through it. It is now created with the same CREATE_NEW + FILE_FLAG_OPEN_REPARSE_POINT + fresh-regular- file check as a staging file, and never truncates; an existing name is treated as already-marked rather than an object to open. A marker that could not be written left the promise unqualified while the next run's cleanup deleted exactly the file it named. That failure now rides in the error, telling the operator to copy it somewhere safe now. Helper refreshes downgraded tampering to a warning. An ordinary helper failure still warns — a stale helper is better than a failed update — but a helper whose path may hold unverified content fails the apply, because helpers are resolved from the install directory and executed by the sandbox runner, and reporting Applied: true would hand the operator a success while a sibling executable is suspect. ErrTargetPossiblyTampered moved to apply.go so callers on every platform can test for it without build tags. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * fix(update): keep the recovery copy when its marker cannot be established Surfacing a failed marker write told the operator to hurry; it did not stop the next run from deleting the file they were being told to save. When no marker can be established, the copy is now moved to an unpredictable sibling name that routine cleanup never touches — CleanupStaleBinary only ever removes the exact "<target>.old" — and the error names where it went. Telling the operator to act now remains the fallback for when even that move fails. The marker check is conservative in the same direction: only a definite "not there" allows the copy to be deleted. An Lstat that fails for any other reason leaves the question open, and deleting is irreversible while keeping costs one stale file. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * fix(update): preserve Windows recovery state across retries Amp-Thread-ID: https://ampcode.com/threads/T-019faf74-99d4-75cf-ac7a-661c308240ef Co-authored-by: Amp <[email protected]> * fix(update): watch every recovery path and never orphan a partial marker A failed second-or-later update marks the randomized aside it used, but promotion only consulted the canonical <target>.old marker, so a retry proceeded over an unverified target while the verified copy sat marked on a path the refusal logic did not watch. Promotion now enumerates every recovery candidate — canonical and randomized asides — and refuses while any of them is marked, naming each recovery path and its marker. The missing-target refusal had the same single-path blind spot: with a stale canonical .old from an earlier successful update plus the aside from an interrupted attempt, it named only the canonical path, which can be the wrong binary. It now names the single actual candidate or refuses the ambiguous layout outright until the operator resolves it. A failed marker write could also leave a partial .keep behind while the recovery copy was relocated elsewhere; later refusals then named a location that no longer held the verified bytes. markOldBinaryPreserved now removes the entry it created before reporting a write/close failure, and conservatively reports success when that removal (or the state check) fails so oldPath stays authoritative. keepUnmarkedRecoveryCopy is handle-bound end to end: it pins the recovery copy with a no-delete-sharing, no-reparse open, verifies it is a single-link regular file, renames through the handle with ReplaceIfExists false, and confirms identity at the destination, failing closed with ErrTargetPossiblyTampered on any race. Finally, the error for an unestablishable marker still promised that a later update would otherwise remove the copy, but CleanupStaleBinary is now a no-op; the operator is told that a manual copy is required. * fix(update): tolerate the fail-closed outcome of a fully-unlinked substitution TestPromoteInstallsTheStagedObjectNotTheStagedPath deletes the staging file's only directory entry and recreates it as an "attacker" file, then required promote to succeed with the verified bytes installed. CI's windows-latest runner permits that delete (this workstation's exclusive share mode blocks it, so the path went unexercised here); on that runner, renaming the now fully-unlinked staging handle back into existence is not something every Windows build honors, so verifyPromotedTarget's post- rename identity check finds nothing at targetPath, promote fails, and restoreOriginalBinary moves the pre-update binary back — a safe, fail- closed outcome, not a security regression. The test now accepts either outcome after a real substitution: promote succeeding with the verified bytes, or promote failing as long as the attacker's substituted bytes were never installed. Any promote failure that occurs without substitution having actually happened still fails the test outright. * fix(update): surface unverified recovery path, match .old case-insensitively keepUnmarkedRecoveryCopy discarded the destination path when the rename to it succeeded but the post-rename identity verification failed, leaving restoreOriginalBinary's error telling the operator to save oldPath after it had already been vacated. Return the attempted kept path alongside the verification error so the caller can still name it. existingRecoveryPaths compared "<target>.old" filenames case-sensitively, but NTFS is case-insensitive/case-preserving, so a recovery or marker file spelled e.g. "zero.exe.OLD" could be silently skipped by every caller's fail-closed check. Fold both sides before matching. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(update): bound Windows recovery cleanup Amp-Thread-ID: https://ampcode.com/threads/T-019fbdec-f8dc-71f9-abdd-ea044a902b9a Co-authored-by: Pierre Bruno <[email protected]> * fix(update): secure Windows recovery cleanup Amp-Thread-ID: https://ampcode.com/threads/T-019fbec7-a912-7462-8938-1b4a8d95f1f3 Co-authored-by: Pierre Bruno <[email protected]> * fix(update): terminate Windows rename paths Amp-Thread-ID: https://ampcode.com/threads/T-019fbec7-a912-7462-8938-1b4a8d95f1f3 Co-authored-by: Pierre Bruno <[email protected]> * test(update): allow namespaced recovery copies Amp-Thread-ID: https://ampcode.com/threads/T-019fbec7-a912-7462-8938-1b4a8d95f1f3 Co-authored-by: Pierre Bruno <[email protected]> * fix(update): bind Windows cleanup to handles, drop dead cleanup API Failure-path staging cleanup on Windows re-resolved the staging pathname after the exclusive handle was closed, so a principal who can write in the installation directory could substitute that entry in the gap and have the updater delete a file of their choosing. Removal is now requested through the handle (FileDispositionInfo) before it is released, and nothing is removed by name afterwards; the same change covers the staging-file verification failure path and the partial recovery marker. CleanupStaleBinary has had no production caller since bounded, identity-bound cleanup moved into promote, so the exported no-op and the tests that could only pass vacuously against it are removed. The POSIX link-regression tests now drive createStagingFileAt, the primitive createStagedBinary actually uses, instead of a path-taking helper kept alive only for them. Also documents the fail-closed Windows recovery states in docs/UPDATE.md, including the refusal a planted .old/.keep pair can cause. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * fix(update): bind Windows recovery to handles and bound its state Addresses the 2026-08-02 review round on #751. Keep the recovery restore bound to the object promote moved aside instead of its pathname: promote now opens the target with delete access and no delete sharing before renaming it aside, and restoreOriginalBinary renames that same handle back (retrying through the handle, never re-resolving the source name), so a directory writer cannot swap the aside entry during the promotion gap and have the substitute installed. Run the recovery-state checks as a standalone-install precondition, under the same lock promotion uses, so "already up to date" can no longer report success while a .keep/.recovery state is unresolved. The in-promotion check stays to close the race. Make the cleanup record a queue: entries are only retired once their object is actually gone, so a copy held open by a scanner is retried by a later update rather than leaking, while records for paths that vanished or that this updater could never have created are retired instead of accumulating. Also from review of the above: - delete the pathname restore chain that lost its production callers (restoreOriginalBinary's pathname form, openIdentityFile, recordRecoveryCleanup) and point their tests at the live handle path - name the relocation path in the error when the post-move verification fails, since oldPath is already vacated by then - scope the recovery docs to Windows and describe POSIX replace semantics Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --------- Co-authored-by: Claude Sonnet 5 <[email protected]> Co-authored-by: Amp <[email protected]> |
||
|
|
1057d74512 |
docs(readme,cli): document /new & zero upgrade, add Discord link, and sync help text (#489)
* docs(readme): document /new, zero upgrade, and add Discord link Add the /new slash command and zero upgrade command to the README's command references, expand docs/UPDATE.md to cover the --apply install flow, and link the project Discord server. * docs(update): escape pipe in --endpoint table cell CodeRabbit flagged the unescaped | in the --endpoint placeholder breaking the Markdown table layout. * docs(readme-zh): bring /new and zero upgrade in line with README.md README_ZH.md had picked up the Discord link but was still missing the /new slash command and zero upgrade entries added to README.md, letting the translation drift out of sync. * cli: add upgrade to root help output * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <[email protected]> * docs: make required update flag explicit in common commands zero update errors without --check or --apply, so the bare form in the common-commands examples was misleading. Addresses the Copilot finding from the PR #489 review. * docs: use a permanent (non-expiring) Discord invite in the READMEs * docs(cli): match upgrade help summary to download/verify/install wording Copilot flagged the root zero --help summary for upgrade as less specific than the download/verify/install phrasing already used in README.md and docs/UPDATE.md for the same command. Co-Authored-By: Claude Sonnet 5 <[email protected]> * docs: use discord.gg short link for the community invite discord.com/invite/<code> and discord.gg/<code> resolve to the same invite; discord.gg is the canonical short form to publish. Co-Authored-By: Claude Sonnet 5 <[email protected]> * fix(update,readme-zh): keep npm --json stdout clean, add ZH Discussions route `zero upgrade --json` (and `zero update --apply --json`) on an npm-managed install wired npm's own stdout directly to the process stdout, so its progress output landed ahead of the JSON result and made it unparseable. Route npm's stdout into a buffer when JSON output is requested, only surfacing it in the error message if the install fails. README_ZH.md's Community section only pointed to Discord, unlike the English README, which reserves Discord for real-time chat and routes Q&A/ideas/sharing to GitHub Discussions per SUPPORT.md/CONTRIBUTING.md policy. Bring the Chinese section in line with the same split. Addresses the outstanding findings from jatmn's review on PR #489. Co-Authored-By: Claude Sonnet 5 <[email protected]> * docs(update): qualify npm-managed JSON output Document the npm subprocess output caveat instead of promising a single parseable JSON document for apply operations. Drop the JSON-specific updater plumbing and tests so the PR remains focused on documentation and help discovery. Amp-Thread-ID: https://ampcode.com/threads/T-019fa55a-8bcd-71bc-9bde-9b4c28b78b38 Co-authored-by: Pierre Bruno <[email protected]> * docs(update): align upgrade guidance Amp-Thread-ID: https://ampcode.com/threads/T-019fa7a0-1223-701d-9529-48ba5d7cf8c8 Co-authored-by: Pierre Bruno <[email protected]> * fix(update): keep the upgrade recommendation about this machine `zero update --check --target windows-x64` answers a question about a different machine, but the new call to action told the user to run `zero upgrade`, which only ever installs onto this one — replacing the target-specific download guidance the flag used to print. The recommendation is now conditional: a current-platform check keeps it, and a cross-target check gets the manual instruction plus a note naming the machine `zero upgrade` would have touched instead. Also align two flag contracts with what the code does: `zero update --help` described --repo/--endpoint as check-only although apply honors them, and UPDATE.md did not mention that both are ignored on an npm-managed apply, which takes its release from the npm registry rather than GitHub. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * fix(update): do not send a custom-source check to the default source `zero upgrade` is a fresh invocation: it does not inherit the --repo/--endpoint of the check that suggested it, so recommending it bare after a custom-source check told the user to install from a repository they never asked about. The check now records the flag it was given and the guidance repeats it, naming where a bare upgrade would install from instead. ZERO_UPDATE_RELEASE_URL is deliberately not recorded: every Check reads it from the environment, including the one inside Apply, so a bare upgrade already follows it and telling the user to repeat it would be wrong. Two smaller edges from the same review: a cross-target check on a host with no published release target no longer ends by describing what `zero upgrade` does there, since it does not work there at all; and the flag table and help text now carry the npm caveat the prose bullet already had, so docs and help agree. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> * fix(update): keep source guidance accurate Amp-Thread-ID: https://ampcode.com/threads/T-019fafa2-7d9d-75bc-8801-6d0efc8db1df Co-authored-by: Pierre Bruno <[email protected]> --------- Co-authored-by: Copilot Autofix powered by AI <[email protected]> Co-authored-by: Vasanthdev2004 <[email protected]> Co-authored-by: Claude Sonnet 5 <[email protected]> Co-authored-by: Amp <[email protected]> |
||
|
|
583e6530bd |
fix(lint): resolve static analysis baseline (#769)
* fix(lint): resolve static analysis baseline * fix(lint): clear linux staticcheck findings Amp-Thread-ID: https://ampcode.com/threads/T-019fa018-b2ee-7442-8a31-80b2cf9dd270 Co-authored-by: Pierre Bruno <[email protected]> --------- Co-authored-by: Amp <[email protected]> |
||
|
euxaristiaandClaude Sonnet 5
|
97ef90d27a |
chore: remove code unreachable on every supported platform (#706)
* 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]> |
||
|
PierrunoYT
|
1fc9b2d25c |
fix(update): clearer error on unsupported release platform (android/termux) (#603)
* fix(update): clearer error on unsupported release platform (android/termux) zero update/upgrade fails on Android/Termux because GOOS "android" has no published release archive. Replace the generic "unsupported release platform: android" error with a clear, actionable message explaining that no release builds are published for the platform, listing the supported platforms, and suggesting a rebuild from source. The command still fails gracefully (clean stderr message, exit 1, no panic/stack trace) rather than crashing. Fixes #598 * fix(update): address review feedback on unsupported-platform error - Condense the explanatory comment for the unsupported GOOS branch. - Replace the Unicode em dash with plain ASCII punctuation. - Align internal/release.ReleasePlatform's error message with the updater's so both flows give identical guidance. * fix(update): npm-based upgrade guidance for platforms without release assets Address review feedback: the previous message implied Android/Termux was an unsupported platform and told users to rebuild from source. Termux runs zero fine via the npm wrapper; it just has no self-updating release archive. Point users at 'npm update -g @gitlawb/zero', the documented Termux install/upgrade path, instead. |
||
|
euxaristia
|
ce9cb91295 |
fix(update): support safe symlink extraction during updates (#575)
* fix(update): support safe symlink extraction during updates Allow extracting symlinks from TarGz and Zip update archives to support Unix npm shim symlinks. Prevent path traversal vulnerabilities by rejecting absolute link targets and verifying that relative link targets do not escape the destination directory. * revert(update): keep zip symlink entries rejected Addresses jatmn's review: the stated bug (standalone update extraction broken on Unix/macOS) is about .tar.gz archives; the zip path handles Windows release archives. Adding zip symlink support alongside the tar.gz fix reversed the existing zip symlink-rejection hardening (TestExtractZipRejectsSymlinkEntry) and failed Windows CI, since filepath.IsAbs does not reliably reject a slash-rooted target like "/some/other/path" on Windows (which needs a drive letter or UNC prefix to be absolute). Reverted extractZip's symlink handling back to unconditional rejection and removed the two zip-symlink tests that covered it. The tar.gz symlink support and its tests (the actual fix for the reported bug) are unchanged. |
||
|
PierrunoYTandCopilot Autofix powered by AI
|
008bc9b3f3 |
fix: address bugs found in a multi-agent codebase audit (#481)
* 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]> |
||
|
PierrunoYTandVasanthdev2004
|
5f36349c18 |
feat(update): add zero upgrade command to apply self-updates (#461)
* 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]> |
||
|
|
6fb0231df8 |
TUI scrollback re-architecture, Lime chat surface, and deep-audit fixes (#158)
* docs(design): add Lime TUI prototype and rebuild spec
* tui: remove the zeroline skin system
One design remains in the tree ahead of the Lime rebuild: the zeroline
package, its TUI view/skin plumbing (Options.Skin/ThemeVariant/ThemeDark,
model skin branches, theme picker), and the zeroline CLI subcommand are
gone. /theme keeps its shell-only message.
* tui: replace the cyan palette with the Lime token table
theme.go is rebuilt around the Lime design tokens (docs/design/
zero_tui_lime.html): near-black panels, one lime accent, solid tint
stand-ins for the prototype's rgba overlays. Existing render sites map
onto the new role styles (ink/line/userPrompt/toolName/toolArg) without
layout changes; no hex exists outside theme.go.
* tui: build the Lime chat-surface components
Title bar with brand badge + context window, empty state replacing the
deleted splash (block-art 0, tagline, 1-3 starter suggestions), stream
blocks (user prompt gutter, muted interim with cursor, final-answer
accent rail, done line, bordered notes), tool cards with diff/read/
bash/grep bodies and MiniDot run spinner, borderless composer with
run-state hints, and the grouped status line. Final answers are marked
at append time; resolved tool calls collapse into their result cards.
* tui: fix Stage 2 review findings
Adversarial review of the chat-surface commit confirmed 15 issues, all
fixed here:
- wrapPlainText sliced wide runes by rune count: CJK/emoji prose could
panic the TUI or emit lines ~2x the measure; split at width instead
- a cancelled run's streamingText leaked into the next turn's interim
block; cancelRun now clears it
- orphaned tool-call cards (cancelled/errored turns) re-animated with
the spinner on any later run; the glyph is now scoped to the call's
own runID
- starter-suggestion digit capture was active while the chips were off
screen (/clear mid-run); the guard now mirrors the render condition
- prompt submits while a deferred Ctrl+C quit is draining could start a
run the quit would orphan; submission is blocked while exiting
- the running placeholder advertised ctrl+c (which quits) as the
interrupt key; it now says esc
- read-card parser expected 'N: text' but read_file emits 'N | text',
so the gutter and L-range never rendered on real output
- toolCard borders were one column short of the body rows
- diff tint bands now span the full row; NEW FILE renders green on
addBg; preamble/no-newline lines are unnumbered meta and no longer
shift hunk numbering
- card bodies and system notes paint the panel surface; the head gains
the separate faintest arg column (grep target + pattern)
- title-bar overflow now truncates the minimal candidate instead of
rendering the deleted design's 'ZERO … READY' line
- untracked the stray snake-game.html (left on disk, ignored)
* tui: restyle the interactive surfaces for Lime
Permission prompts render as the amber permission card (PERMISSION
badge, risk readout, key chips); resolved prompts collapse to one line
(allowed once/always/denied · tool) and unprompted allows fold into the
tool card's [auto] tag. Ask-user and spec-review prompts share the card
language with line borders. Autocomplete and picker overlays move onto
the panel/selection tints with the accent ❯ marker; model-picker rows
gain the provider dot and ctx · KEY_ENV metadata from the registry and
provider catalog. /resume renders its list as stacked session cards
(id + age, title, meta). Key handling is untouched everywhere.
* tui: adaptive width tiers, doc updates, final polish
Width tiers re-evaluate from the live width: ≥100 renders the full
spec; 80–99 drops the tool-arg column, header ctx, and the status
'interactive' group; 58–79 drops the diff/read gutters, bares the
badge, and keeps provider+tokens+mode; <58 renders a single-segment
header, rail-less cards, and a mode-only status line. Table-driven
tests cover {58,70,80,100,120} plus a frame-wide invariant that no
emitted line exceeds the terminal at any tier. Card bodies expand tabs
so width math holds. Dead splash-chip machinery (startupCommandNames,
startupOrder) is gone; README's TUI section now describes Lime.
* tui: fix final-review findings across Stages 3-4
A second adversarial review confirmed 14 issues, all addressed:
- permission collapse and tool-card pairing are now run-scoped (rcKey):
providers with repeating synthetic ToolCallIDs (Gemini gemini_tool_N)
could attribute a decision or result to a different run's call
- rehydrated always-grants no longer mislabel as 'allowed once'
(GrantMatched fallback)
- the width invariant now actually holds everywhere: empty-state
tagline/hint, ask-user rows, permission detail blobs, pending image
chips, and the sessions trailing hint are wrapped or fitted; the
invariant test renders the empty state and all of those rows at
widths 24-120
- every card (permission, ask-user, spec-review, sessions, notes)
drops side borders on tiny terminals, not just tool cards
- /resume card fields are sanitized against separator bytes from
user-controlled titles
- the picker's selected row paints its gap so the selBg band is solid
- dead splash helpers (normalizedStartupWidth, countLines, nonEmpty,
unreachable rowWelcome branch) and stale /theme comments are gone
- README no longer claims transcript scrolling keys or Tab-accept
behavior that never existed; chatWidth documents its 24-col floor
Tag tui-lime-s4 moves to this commit.
* audit: deep-audit report, TUI scrollback re-architecture, and cross-module fixes
TUI: settled-row flush frontier prints history to native terminal
scrollback (tea.Println, ordered + ack-serialized); View() renders only
the live tail. Full diffs (400-line flush cap) and OSC 8 file:// links
land in history; streamed interim text, cancellation markers, ask_user
exchanges, and composer input history are preserved; run-scoped dedup
keys fix Gemini's repeating tool-call ids; /exit, Ctrl+C-after-Esc, and
cross-session late flushes no longer orphan or contaminate checkpoints.
Cross-module: zero search panic + offset mapping, usage report
robustness, 529 retry, SSE keep-alive watchdog, registry corrections,
update --check on dev builds, MCP ping + mcp list --json secret
redaction, sandbox.network config knob, doctor apiKey presence, atomic
config writes, C1-safe notifications, rune-safe truncation everywhere,
gofmt + vet CI gates, dead glamour dependency dropped.
Full audit report: docs/audit/2026-06-10-deep-audit.md (175 findings).
Co-Authored-By: Claude Fable 5 <[email protected]>
* tui: hide malformed ask_user tool errors
* fix tui malformed tool retries
* fix max-turn finalization
* upgrade tui agent guardrails
* fix coderabbit guardrail findings
---------
Co-authored-by: Claude Fable 5 <[email protected]>
Co-authored-by: Vasanthdev2004 <[email protected]>
|
||
|
Anandan
|
2d41e24a5e | Polish update target output (#99) | ||
|
Anandan
|
699df010e2 |
feat(update): verify release metadata by target (#97)
* Add update check target selection * Reject empty update target flag |
||
|
Anandan
|
3e4add95ab |
feat(update): expose release check target options (#96)
* Expose update check target options * Reject non-positive update timeouts |
||
|
Anandan
|
04808f6abd | Verify update release assets (#84) | ||
|
Vasanth T
|
43c256068a |
feat: make Go runtime the app path (#71)
* feat: make go runtime the app path * fix: harden update semver parsing |