4 Commits

Include renames

Author	SHA1	Message	Date
euxaristia	da41c3a75b	fix(lock): prevent POSIX lock file overwrite and leak on Windows/Unix (#628) ... * fix(lock): prevent POSIX lock file overwrite and leak on Windows/Unix During lock reclamation, reclaiming a suspected stale lock renames the file aside and renames it back if not stale. On POSIX systems, the rename back silently overwrites any new lock file created in the gap. On Windows, it fails to rename back and leaks the stale file. Introduce a cross-platform restoreLockFile helper: - On POSIX: Use os.Link to link the file back (which atomic-fails if the target already exists), then remove the temporary link. - On Windows: Use os.Rename (which safely fails if the target exists). If restoring fails, delete the temporary file immediately to prevent leaks. * fix(lockutil): fix Windows build by using golang.org/x/sys/windows Addresses jatmn's follow-up review: the required Windows smoke check was failing because syscall.MoveFileEx is not exported by the standard library's syscall package on this platform, so internal/lockutil (and everything importing it: cron, daemon, hooks, oauth) failed to compile on Windows. Switched to golang.org/x/sys/windows, which does export MoveFileEx with the same signature, already a direct module dependency. Verified on a real Windows machine that the no-flags call still fails with ERROR_ALREADY_EXISTS when the destination exists, and that this satisfies errors.Is(err, os.ErrExist), which cron/daemon/hooks/oauth's reclaim paths all depend on to decide whether the sidelined lock was genuinely superseded (safe to discard) versus some other, unrelated failure (must be preserved). TestRestoreLockFile already covered both the successful-restore and fail-when-target-exists cases; it now passes on Windows. * fix(lockutil): route lock file deletes through RemoveLockFile with Windows retry loop * fix(lockutil): close restore split-brain and unify RemoveLockFile RestoreLockFile could fail with errors other than os.ErrExist (hardlink- incapable filesystems such as FAT or some FUSE/network mounts, ENOSPC, Windows sharing violations). Callers only clean up on os.ErrExist, so any other failure left the lock path missing with the live holder's lock stranded in the sidelined file; the next O_EXCL create then succeeded and two processes held the lock. RestoreLockFile now falls back to an O_EXCL copy in that case, which preserves the no-overwrite guarantee, and a successful restore with a failed sidelined-file cleanup reports nil instead of a failed restore. RemoveLockFile now has one cross-platform contract: removing an already missing file reports nil on POSIX too, matching Windows, and daemon release() drops its caller-side ErrNotExist filter. Tests: TestRestoreByCopy and TestRemoveLockFile run on all platforms; Windows-tagged tests pin the ERROR_ALREADY_EXISTS mapping to os.ErrExist and exercise the RemoveLockFile retry loop to success and to bounded exhaustion. The package doc comment documents the shared contract. * refactor(swarm): extract reclaimStaleLock to mirror other lock callers The stale-lock reclaim in acquireLock was inline, so its restore path (a holder rotated a fresh lock in the gap between the stale check and the rename) could not be regression-tested the way cron, daemon, hooks, and oauth test theirs. Extract it into reclaimStaleLock with the same shape as the other callers, behavior unchanged, and add the mirrored regression test. * test(hooks,oauth): add stale lock reclaim regression tests Mirror the cron reclaim test in the two packages that lacked coverage for reclaimStaleLock: a genuinely stale lock is reclaimed, a fresh lock (reacquired in the gap) is restored intact rather than stolen, and a missing lock reports no reclaim. daemon already had equivalent tests in lock_test.go. * fix(lock): fail closed and drop sidelined file when restore fails A non-os.ErrExist error from RestoreLockFile means both the no-replace restore and its O_EXCL copy fallback failed, so the lock path may be missing while the sidelined holder is live. Callers previously swallowed that error and their next O_EXCL create would succeed, breaking mutual exclusion. reclaimStaleLock in cron, daemon, hooks, oauth, and swarm now returns an error for that case and the acquire loops fail closed instead of re-acquiring. The sidelined .stale.* file is now removed on every restore failure, not only os.ErrExist: once the restore has failed it has no protocol function (release only consults the lock path), so keeping it only leaked files. Also replace the numeric errno literals in the Windows RemoveLockFile retry loop with windows.ERROR_SHARING_VIOLATION and windows.ERROR_ACCESS_DENIED. * refactor(lockutil): extract shared ReclaimStaleLock helper Consolidate the duplicated reclaimStaleLock functions from cron, hooks, oauth, and swarm into lockutil.ReclaimStaleLock. Each caller now passes an mtime-based isLive predicate to the shared helper, which owns the rename-aside, restore-if-live, cleanup, ErrExist handling, and fail-closed error propagation. Daemon keeps its own reclaimStaleLock because it uses PID-based liveness (different predicate shape and no staleAfter duration). The per-package reclaim regression tests are removed since the shared helper is thoroughly tested in lockutil/reclaim_test.go (dead lock reclaimed, live lock restored, missing lock, fail-closed on restore error, sidelined file dropped when new holder wins). * fix(lockutil): propagate non-contention rename failures from ReclaimStaleLock A rename-aside failure that is not a lost race previously mapped to the benign lost-race result, so acquirers spun until their deadline and reported a timeout instead of the real failure. Keep only a missing source and the Windows sharing-violation/access-denied contention errnos benign (neither Windows errno maps to os.ErrExist, and ERROR_SHARING_VIOLATION does not map to os.ErrPermission either, so the classification compares raw errnos) and return everything else so callers fail closed. Also convert daemon's private reclaimStaleLock to the shared helper so it picks up the same contract, and reword the caller error messages from "restore reclaimed" to "reclaim stale" since the error can now come from the rename aside as well. * fix(lockutil): publish restore-by-copy atomically and honor lost mailbox reclaims restoreByCopy created the destination lock file empty before copying into it, so a crash mid-copy could leave a live holder's lock permanently unrepresented at the lock path. Stage the copy under a private name and publish it with the platform's no-replace primitive instead. swarm's mailbox lock discarded the reclaimed boolean and always retried immediately, so a lost reclaim race (or Windows sharing-violation contention) could hot-spin past the lock timeout instead of falling through to the bounded wait like the other lock callers. * fix(lockutil): shrink the reclaim/restore window with a fast replacing rename Restoring a lock that turned out live went through the no-replace path (plus its copy fallback on some failures) first, which is slow enough that a fourth process's unrelated O_EXCL create could win lockPath while it was absent for the whole call. RestoreLockFile's no-replace contract would then report os.ErrExist and the caller silently dropped the sidelined lock, orphaning the still-running original holder. restoreLiveLock now tries a single replacing rename straight back first, falling back to the no-replace path only if that fails. It shrinks the window from a whole restore call down to roughly one syscall; it does not close it, since the fast path can silently overwrite a competing lock rather than detect it. A fully race-free reclaim would need an OS-level advisory lock checked non-destructively instead of by moving the file, which is a larger change than this.	2026-07-13 21:45:08 +08:00
euxaristia	8ea53841a5	fix(daemon): handle os.ErrPermission as collision during O_EXCL lock creation (#616) ... On Windows, a concurrent holder's os.Remove leaves the lock file in a 'delete pending' state, causing a racing O_EXCL create to fail with ERROR_ACCESS_DENIED (mapped to os.ErrPermission). Treat os.ErrPermission as lock contention / already-existing lock, matching the collision checks in cron, hooks, oauth, and securefile.	2026-07-10 10:07:50 +08:00
KRATOS	fbbfdbf3f2	Harden background daemon: stdout integrity, lifecycle, and single-instance lock (#274) ... * harden background daemon: stdout integrity, lifecycle, single-instance lock Deep review of internal/daemon (worker pool, launcher, control server, session fan-out, single-instance lock) found and fixes a stdout-integrity bug plus several lifecycle/locking hazards. Each fix has a regression test; the daemon suite passes under -race. Worker stdout integrity - Replace the capped bufio.Scanner (64 KiB) on a worker's stdout with an uncapped bufio.Reader: a legitimately large stream-json line from our own trusted worker is no longer silently dropped (the 1 MiB cap belongs to untrusted network frames, not this local pipe). - Surface a stdout read error as a run failure instead of swallowing it as a clean success, and kill the worker first so it can't block writing to the now-unread pipe and hang Wait. Control-server lifecycle - Track open connections and close them on Shutdown so a single idle/hostile client can no longer wedge drain (and SIGTERM cleanup) forever. - Guard the listener with a mutex and re-check the shutdown signal after bind so a shutdown requested in the bind window is not lost. - Honor shutdown during buffered-history replay too. Session fan-out - A slow subscriber that drops live lines now receives a namespaced gap notice when its channel drains, instead of silently losing output. - Hold the session lock across the (non-blocking) broadcast sends to close a latent send-on-closed-channel panic when a subscriber cancels mid-send. Single-instance lock - Reclaim a stale lock with an atomic rename-with-verify instead of a blind remove, so two instances starting at once cannot both reclaim and end up both holding the lock; a holder that reacquires in the gap is restored. Client + pool - Bound the control handshake with an I/O deadline so a peer that never replies can't wedge Dial/NewClientConn. - Key the active-worker map by a monotonic id rather than PID (the OS reuses PIDs the instant a worker exits). - Group-terminate on context cancel so a stuck worker's children are not orphaned. * daemon: add -race regression test for the Session.Line lock-hold invariant Session.Line holds s.mu across its non-blocking subscriber sends so cancel()/ finish() cannot close a subscriber channel mid-send (send-on-closed-channel panic). That invariant had no concurrency test. Add one that drives Line concurrently with Subscribe/cancel and finish; it panics with a data race if the lock-hold is reverted and passes with it (verified both ways under -race).	2026-06-20 22:21:28 +05:30
KRATOS	135e3c5657	feat(daemon): zero daemon — worker pool + session supervisor over a local control socket (#203) ... * feat(daemon): add `zero daemon` — worker pool + session supervisor over a local control socket Adds internal/daemon + `zero daemon start|stop|status|run|attach`: a long-running local service that supervises a pool of headless `zero exec` workers and routes multiple agent sessions to them over an owner-only Unix-domain control socket. Additive — interactive and one-shot exec are unchanged. Reuses zero's building blocks: - internal/background: ConfigureChildProcessGroup (process-group setup) + TerminateProcess (cross-platform terminate) for worker kill/drain. The pool does its own os/exec and uses background only for those helpers. - internal/streamjson: the worker speaks stream-json on stdout; the daemon forwards those lines to clients. - internal/cli exec: a worker is `zero exec --output-format stream-json`. Pieces (reimplemented in Go from a reference daemon): - protocol.go: framed control codec — 4-byte BE length + 1 kind byte, 1 MiB cap, JSON control messages, version negotiation. - pool.go + backoff.go: bounded worker pool — at-least-once dispatch with exponential backoff+jitter retry capped by MaxAttempts, EXIT_TEMPFAIL(75)/ EXIT_PERMANENT(76) handling, graceful drain + kill-timeout. zero's exec is one-shot, so workers are on-demand (one session per worker = the lease), not warm long-lived slots (a persistent worker mode is a follow-up). - session.go: SessionManager — sessionID->worker routing, lease (one active request per worker), queue when the pool is full, per-session output buffer for attach + metrics. - server.go + lock.go + socket.go + paths.go: single-instance PID-file lock with stale-lock (dead-PID) recovery, status file, owner-only socket, accept loop; cleans up lock/socket/status on exit. - launcher.go: production worker launcher. client.go: control-socket client. Security model: - Control socket is LOCAL-ONLY (AF_UNIX loopback file, never a TCP port) and owner-only: socket 0600 + parent dir 0700 on POSIX (Windows relies on the per-user profile ACL; a dedicated pipe ACL is a follow-up). - Workers stay sandboxed: the launcher SCRUBS the sandbox re-entrancy markers (ZERO_SANDBOXED / ZERO_SANDBOX_BACKEND) from each worker's env so the worker re-establishes its OWN sandbox instead of inheriting a pass-through plan, while propagating the rest of the policy config/env. The daemon never bypasses the sandbox. - Control frames over the 1 MiB cap are rejected before allocation. Robustness: a crashed worker never takes down the daemon (backoff+jitter retry, MaxAttempts cap, EXIT_PERMANENT stops retry); drain gives a grace window then force-kills stragglers; a stale lock from an unclean exit is reclaimed. Out of scope (follow-up): remote session-linking; warm pre-spawned persistent workers; Windows named-pipe ACL. Tests: protocol round-trip + oversize/unknown-kind rejection; backoff bounds; pool spawn/retry-backoff/permanent/cap/queue/drain; session lease/route/queue/ attach; lock single-instance + stale recovery; launcher env-scrub (incl. an end-to-end check that the spawned worker process receives scrubbed env); server e2e start->run->stream-json->status->attach->stop with a fake worker; CLI usage/validation/not-running paths. Live-verified start/status/stop (0600 socket, 0700 dir, file cleanup). Platform files behind explicit //go:build tags. * daemon: address review — help listing, extra-arg rejection, lock write error, bounded sessions - cli/app.go: list `daemon` in `zero --help` so the command is discoverable. - cli/daemon: `stop`/`status` reject extra arguments; `attach` rejects more than one session id (fail fast on typos instead of silently ignoring tokens). - daemon/lock: check the PID fmt.Fprintf error — a partial/failed write would leave a malformed lock file that another process reads as stale and wrongly reclaims, breaking the single-instance guarantee; on write failure remove the file and return the error. - daemon/session: bound the registry with MaxSessions (default 256) — evict the oldest FINISHED sessions once past the cap so a long-running daemon doesn't accumulate sessions without limit; running/queued sessions are never evicted, so attach-after-finish for recent sessions is preserved. Tests: CLI extra-arg rejection; finished-over-cap eviction + running-never-evicted. Gates: gofmt, vet, build, test (incl -race), staticcheck (no new), govulncheck (0), deadcode (no new), GOOS=linux+windows cross-compile.	2026-06-14 22:16:37 +05:30