mirror of
https://github.com/Twigpine/zero.git
synced 2026-11-11 23:40:28 -10:00
* 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]>
58 lines
2.0 KiB
Go
58 lines
2.0 KiB
Go
//go:build windows
|
|
|
|
package update
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"time"
|
|
)
|
|
|
|
const (
|
|
restoreRenameRetryAttempts = 10
|
|
restoreRenameRetryDelay = 100 * time.Millisecond
|
|
)
|
|
|
|
// replaceBinary installs newPath over targetPath. Windows will not let a
|
|
// running executable be overwritten or deleted directly, but NTFS does allow
|
|
// renaming it aside — the same trick already used for locked config files in
|
|
// internal/cli/mcp_config.go's replaceMCPWritableConfigFile.
|
|
func replaceBinary(targetPath string, newPath string) error {
|
|
oldPath := targetPath + ".old"
|
|
_ = os.Remove(oldPath) // best-effort cleanup of a leftover from a previous upgrade
|
|
if err := os.Rename(targetPath, oldPath); err != nil {
|
|
return fmt.Errorf("rename running binary aside: %w", err)
|
|
}
|
|
if err := os.Rename(newPath, targetPath); err != nil {
|
|
// Retry the restore: a transient Windows file lock (antivirus/indexer
|
|
// scanning the just-renamed file, a lingering handle) can make a rename
|
|
// fail momentarily, and here failure means targetPath is left missing
|
|
// entirely rather than merely stale — worth a short retry to avoid that.
|
|
if restoreErr := renameWithRetry(oldPath, targetPath); restoreErr != nil {
|
|
return fmt.Errorf("install new binary: %w; additionally failed to restore the original binary: %v (original preserved at %s)", err, restoreErr, oldPath)
|
|
}
|
|
return fmt.Errorf("install new binary: %w", err)
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func renameWithRetry(oldPath string, newPath string) error {
|
|
var lastErr error
|
|
for attempt := 0; attempt < restoreRenameRetryAttempts; attempt++ {
|
|
if err := os.Rename(oldPath, newPath); err == nil {
|
|
return nil
|
|
} else {
|
|
lastErr = err
|
|
}
|
|
time.Sleep(restoreRenameRetryDelay)
|
|
}
|
|
return lastErr
|
|
}
|
|
|
|
// CleanupStaleBinary best-effort removes a "<path>.old" file left behind by a
|
|
// previous replaceBinary call once the old process holding it has exited.
|
|
// Callers should invoke this once at startup for the current executable.
|
|
func CleanupStaleBinary(targetPath string) {
|
|
_ = os.Remove(targetPath + ".old")
|
|
}
|