From 4e41e1d87eb3930af188490d6064f2902a024393 Mon Sep 17 00:00:00 2001 From: Karn Date: Mon, 10 Aug 2026 03:00:11 +0530 Subject: [PATCH] =?UTF-8?q?feat(cli):=20flue=20update=20=E2=80=94=20fetch,?= =?UTF-8?q?=20verify,=20swap,=20and=20restart=20the=20daemon?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The update story was advisory-only: the daemon asked GitHub every 12h, the sidebar showed a card with the brew/curl lines, and nothing self-updated — worse, installing a new binary left the old daemon serving, because flue enable deliberately never restarts a healthy one. flue update closes the loop: - resolves the latest release through release.go's existing fetch machinery (no second GitHub client) and compares semver; up to date is one line and exit 0, a dev build is refused with the from-source answer (git pull && make build) - a binary resolving into Homebrew's Caskroom is brew's to replace: brew upgrade karnstack/tap/flue runs when brew is on PATH, and is named when it is not — never a hand-swap of files brew owns - script/manual installs download flue_{version}_{os}_{arch}.tar.gz, verify sha256 against checksums.txt (install.sh's exact contract), and atomically rename the extracted binary over the running executable's resolved real path, mode preserved; an unwritable target refuses with a sudo hint before any download - service.Manager gains Restart — launchd bootout+bootstrap, systemd restart, both SIGTERM so sessions snapshot and revive — and the transcript's last line reports the version the restarted daemon actually answered with Co-Authored-By: Claude Fable 5 --- README.md | 1 + cmd/flue/enable_test.go | 14 + cmd/flue/main.go | 3 + cmd/flue/release.go | 9 +- cmd/flue/update.go | 422 +++++++++++++++++++++++++++ cmd/flue/update_test.go | 476 +++++++++++++++++++++++++++++++ internal/service/launchd.go | 20 ++ internal/service/launchd_test.go | 54 ++++ internal/service/service.go | 8 +- internal/service/systemd.go | 13 + internal/service/systemd_test.go | 31 ++ 11 files changed, 1048 insertions(+), 3 deletions(-) create mode 100644 cmd/flue/update.go create mode 100644 cmd/flue/update_test.go diff --git a/README.md b/README.md index 53e3f79..e2774d5 100644 --- a/README.md +++ b/README.md @@ -66,6 +66,7 @@ flue relay status # show the configured relay flue relay update # redeploy this release's relay; secret and pairings kept flue relay address # repoint this machine at a custom domain on the same relay flue serve # run the daemon in the foreground, no login service +flue update # download the newest release, swap this binary, restart the daemon flue version # print the version (also --version, -v) ``` diff --git a/cmd/flue/enable_test.go b/cmd/flue/enable_test.go index 4cbdd3f..b9d8d0e 100644 --- a/cmd/flue/enable_test.go +++ b/cmd/flue/enable_test.go @@ -16,9 +16,12 @@ import ( type fakeManager struct { st service.Status enableErr error + restartErr error warns []string // what Warnings reports after Enable + onRestart func() // runs inside Restart, before it reports back enableCalls int disableCalls int + restartCalls int statusCalls int } @@ -35,6 +38,17 @@ func (f *fakeManager) Disable() error { f.st = service.Status{} return nil } +func (f *fakeManager) Restart() error { + f.restartCalls++ + if f.restartErr != nil { + return f.restartErr + } + if f.onRestart != nil { + f.onRestart() + } + f.st = service.Status{Installed: true, Running: true} + return nil +} func (f *fakeManager) Status() (service.Status, error) { f.statusCalls++ return f.st, nil diff --git a/cmd/flue/main.go b/cmd/flue/main.go index 4ec4698..e54a1ce 100644 --- a/cmd/flue/main.go +++ b/cmd/flue/main.go @@ -84,6 +84,8 @@ func main() { err = cmdStatus() case "relay": err = cmdRelay(os.Args[2:]) + case "update": + err = cmdUpdate() case "version", "--version", "-v": err = cmdVersion() case "-h", "--help", "help": @@ -110,6 +112,7 @@ const usageText = `flue — your terminal, as a browser tab flue relay address URL repoint this machine at a custom domain on the same relay flue open [path] spawn a session in path and open it in the browser flue serve [--port N] [--open] run the daemon in the foreground + flue update download the newest release, swap this binary, restart the daemon flue version print the version (also --version, -v) ` diff --git a/cmd/flue/release.go b/cmd/flue/release.go index 6d5343b..a8f9cad 100644 --- a/cmd/flue/release.go +++ b/cmd/flue/release.go @@ -112,9 +112,14 @@ func (c *releaseChecker) refresh(ctx context.Context) { if err != nil { return } - c.latest, c.url = tag, url + c.latest, c.url = strings.TrimPrefix(tag, "v"), url } +// fetch asks GitHub for the latest stable release. The tag comes back raw — +// leading v and all — because it is the release's address: flue update builds +// download URLs under /releases/download// from it, exactly as +// install.sh does. Callers that want a version to compare or render trim the +// v themselves, as refresh does for the cache. func (c *releaseChecker) fetch(ctx context.Context) (tag, url string, err error) { res, err := c.get(ctx, releaseAPI) if err != nil { @@ -136,7 +141,7 @@ func (c *releaseChecker) fetch(ctx context.Context) (tag, url string, err error) if body.Draft || body.Pre { return "", "", fmt.Errorf("latest release is not a stable one") } - return strings.TrimPrefix(body.TagName, "v"), body.HTMLURL, nil + return body.TagName, body.HTMLURL, nil } // newer says whether `latest` is a later release than `current`. diff --git a/cmd/flue/update.go b/cmd/flue/update.go new file mode 100644 index 0000000..0323f73 --- /dev/null +++ b/cmd/flue/update.go @@ -0,0 +1,422 @@ +package main + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "net/http" + "net/url" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "time" + + "github.com/karnstack/flue/internal/daemon" + "github.com/karnstack/flue/internal/transport/local" +) + +// flue update is the loop the sidebar's advisory never closed: find the +// newest release, put its binary where this one is, and restart the daemon so +// the machine actually runs it. Before this command the update story ended at +// a card naming a brew line — and even a user who ran it was left with an old +// daemon serving their sessions, because nothing anywhere restarted it (flue +// enable deliberately never restarts a healthy daemon). + +const ( + // releaseDownloadBase is where a release's assets live, keyed by the raw + // tag. The contract is shared verbatim with .goreleaser.yaml and + // scripts/install.sh: archive flue_{version}_{os}_{arch}.tar.gz, the + // binary as a regular file named flue at the archive root, and + // checksums.txt in goreleaser's `sha256 filename` lines. + releaseDownloadBase = "https://github.com/karnstack/flue/releases/download/" + + // downloadTimeout bounds the archive download. releaseTimeout is sized + // for one small JSON document and would abandon a ~15MB archive on a + // slow link. + downloadTimeout = 5 * time.Minute + + // updateRestartWait is how long the restarted daemon gets to come back + // and identify itself before the update reports the restart as not + // having landed. Same figure as flue enable, for the same reason: the + // service manager has to fork, exec, and bind first. + updateRestartWait = enableWait + + // The download bounds. Backstops against a wedged or lying server in the + // same spirit as maxMintBytes: GitHub has been identified only by its + // hostname, and a checksum has not been verified yet while these apply. + maxChecksumsBytes = 1 << 20 + maxArchiveBytes = 256 << 20 + maxBinaryBytes = 512 << 20 +) + +// brewUpgradeCommand is the exact line the web sidebar's update card +// advertises; when brew owns the install, this command defers to it. +const brewUpgradeCommand = "brew upgrade karnstack/tap/flue" + +// updateTarget names the file flue update must replace: the running binary's +// real path, symlinks resolved. Deliberately the opposite of +// defaultServiceManager, which records the unresolved name so the plist +// survives upgrades — a service execs *through* a symlink, but a file swap +// has to land on the file itself. Renaming a new binary over the symlink +// would orphan the real file and quietly convert a managed install into an +// unmanaged one; resolving first means the swap replaces the bytes and every +// name that pointed at them still does. A package variable so the tests can +// point the updater at a temp-dir binary — under `go test` os.Executable is +// the test binary, which must never be swapped. +var updateTarget = func() (string, error) { + exe, err := os.Executable() + if err != nil { + return "", err + } + return filepath.EvalSymlinks(exe) +} + +// brewOnPath and runBrewUpgrade are the brew leg's seams: CI must never run a +// real brew, and most CI machines do not have one. +var brewOnPath = func() bool { + _, err := exec.LookPath("brew") + return err == nil +} + +var runBrewUpgrade = func(w io.Writer) error { + cmd := exec.Command("brew", "upgrade", "karnstack/tap/flue") + // brew's own transcript is the progress report; hide none of it. + cmd.Stdout = w + cmd.Stderr = w + return cmd.Run() +} + +func cmdUpdate() error { + return runUpdate(os.Stdout, version, newReleaseChecker(version)) +} + +// runUpdate resolves the newest release, replaces this binary with it (or +// hands the replacement to brew when brew owns the install), and restarts +// the daemon so the new build is the one serving. +// +// The checker is release.go's: the same fetch, the same semver comparison, +// and the same HTTP seam the daemon's 12-hourly check uses — no second +// GitHub client. +func runUpdate(w io.Writer, current string, c *releaseChecker) error { + if current == "dev" { + return errors.New("this is a from-source build, which corresponds to no release; update it with git pull && make build") + } + + ctx, cancel := context.WithTimeout(context.Background(), releaseTimeout) + tag, _, err := c.fetch(ctx) + cancel() + if err != nil { + return fmt.Errorf("look up the latest release: %w", err) + } + latest := strings.TrimPrefix(tag, "v") + if !newer(latest, current) { + fmt.Fprintf(w, "flue %s is already the newest release; nothing to do\n", current) + return nil + } + + target, err := updateTarget() + if err != nil { + return fmt.Errorf("locate the running binary: %w", err) + } + + if brewOwned(target) { + // Swapping a file under brew's roots would leave brew's bookkeeping + // believing the old version is installed — the next `brew upgrade` + // would clobber ours, and `brew uninstall` would half-work. brew is + // the owner, so brew does the swap; when it is somehow not on PATH, + // the command to run is the whole answer. + if !brewOnPath() { + return fmt.Errorf("this flue is Homebrew's (%s) but brew is not on PATH; upgrade it with: %s", target, brewUpgradeCommand) + } + fmt.Fprintf(w, "\n this install is Homebrew's, so brew does the swap:\n\n") + if err := runBrewUpgrade(w); err != nil { + return fmt.Errorf("%s: %w", brewUpgradeCommand, err) + } + fmt.Fprintf(w, "\n ✓ %s\n", brewUpgradeCommand) + } else { + if err := selfUpdate(w, c.get, tag, latest, target); err != nil { + return err + } + } + + return restartForUpdate(w, latest) +} + +// brewOwned reports whether path is a file Homebrew installed and owns. A +// cask install puts the binary at /Caskroom/flue//flue and +// links /bin/flue at it; the caller has already resolved symlinks, +// so the one substring covers both the Caskroom file and the bin link. +// Cellar is checked too so a formula install — should one ever exist — +// fails safe into brew's hands rather than getting its files swapped. +func brewOwned(path string) bool { + return strings.Contains(path, "/Caskroom/") || strings.Contains(path, "/Cellar/") +} + +// getter matches releaseChecker.get: one HTTP seam for everything the +// updater downloads, so the tests fake one function and no second GitHub +// client ever grows here. +type getter func(context.Context, string) (*http.Response, error) + +// selfUpdate downloads the release archive for this OS and architecture, +// verifies it against checksums.txt, and renames the extracted binary over +// target — the script/manual install path, mirroring install.sh's decisions. +// +// The order is deliberate. The staging file is created first, in target's own +// directory: that is the writability check (refuse before spending anyone's +// bandwidth) and it is what makes the final rename an atomic same-filesystem +// move, so no failure can leave target half-written. Renaming over a running +// executable is fine on unix — the running process keeps its inode — which +// is why this is a rename and never an in-place write: opening the running +// binary for writing is ETXTBSY on Linux and corruption anywhere it is not. +func selfUpdate(w io.Writer, get getter, tag, latest, target string) error { + info, err := os.Stat(target) + if err != nil { + return err + } + staged, err := os.CreateTemp(filepath.Dir(target), ".flue-update-") + if err != nil { + return permissionHint(err, target) + } + installed := false + defer func() { + if !installed { + staged.Close() + os.Remove(staged.Name()) + } + }() + + asset := fmt.Sprintf("flue_%s_%s_%s.tar.gz", latest, runtime.GOOS, runtime.GOARCH) + base := releaseDownloadBase + tag + "/" + ctx, cancel := context.WithTimeout(context.Background(), downloadTimeout) + defer cancel() + + archive, err := fetchAsset(ctx, get, base+asset, maxArchiveBytes) + if err != nil { + return fmt.Errorf("download %s: %w", asset, err) + } + sums, err := fetchAsset(ctx, get, base+"checksums.txt", maxChecksumsBytes) + if err != nil { + return fmt.Errorf("download checksums.txt: %w", err) + } + expected, err := checksumFor(sums, asset) + if err != nil { + return err + } + digest := sha256.Sum256(archive) + if got := hex.EncodeToString(digest[:]); got != expected { + return fmt.Errorf("sha256 mismatch for %s (expected %s, got %s); aborting before install", asset, expected, got) + } + fmt.Fprintf(w, "\n ✓ flue %s downloaded and verified\n", latest) + + bin, err := extractFlue(archive) + if err != nil { + return fmt.Errorf("extract flue from %s: %w", asset, err) + } + if _, err := staged.Write(bin); err != nil { + return err + } + // The mode the user (or install.sh, or sudo) gave the binary survives + // the swap; CreateTemp's 0600 is a staging mode, not an answer. + if err := staged.Chmod(info.Mode().Perm()); err != nil { + return err + } + if err := staged.Close(); err != nil { + return err + } + if err := os.Rename(staged.Name(), target); err != nil { + return permissionHint(err, target) + } + installed = true + fmt.Fprintf(w, " ✓ installed to %s\n", target) + return nil +} + +// permissionHint turns a permission refusal into instructions. Both call +// sites run before or instead of any modification to target — the staging +// file is elsewhere-named and the rename is atomic — so "re-run with sudo" +// is advice about a clean retry, never about digging out of a half-update. +func permissionHint(err error, target string) error { + if errors.Is(err, fs.ErrPermission) { + return fmt.Errorf("%s is not writable by you (%v); re-run as: sudo flue update", target, err) + } + return err +} + +// fetchAsset downloads one release asset, bounded. The get seam sends GitHub +// API headers, which the download endpoints ignore, and follows the redirect +// to the CDN that actually serves release assets. +func fetchAsset(ctx context.Context, get getter, url string, limit int64) ([]byte, error) { + res, err := get(ctx, url) + if err != nil { + return nil, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return nil, fmt.Errorf("github answered %d", res.StatusCode) + } + body, err := io.ReadAll(io.LimitReader(res.Body, limit+1)) + if err != nil { + return nil, err + } + if int64(len(body)) > limit { + return nil, fmt.Errorf("response exceeds %d bytes", limit) + } + return body, nil +} + +// checksumFor finds asset's digest in checksums.txt — goreleaser's +// `sha256 filename` lines, the same contract install.sh reads with +// awk '$2 == f'. Fields rather than a fixed offset, so the one-space and +// two-space spellings both parse. +func checksumFor(sums []byte, asset string) (string, error) { + for _, line := range strings.Split(string(sums), "\n") { + f := strings.Fields(line) + if len(f) == 2 && f[1] == asset { + return f[0], nil + } + } + return "", fmt.Errorf("checksums.txt has no entry for %s", asset) +} + +// extractFlue reads the one file the archive contract puts at the root. Only +// a regular file named flue counts; anything else in the archive — including +// anything path-shaped enough to be trying a traversal — is skipped, and the +// caller never writes any name the archive chose. +func extractFlue(archive []byte) ([]byte, error) { + gz, err := gzip.NewReader(bytes.NewReader(archive)) + if err != nil { + return nil, err + } + defer gz.Close() + tr := tar.NewReader(gz) + for { + hdr, err := tr.Next() + if errors.Is(err, io.EOF) { + return nil, errors.New("no flue binary at the archive root") + } + if err != nil { + return nil, err + } + if hdr.Typeflag != tar.TypeReg || filepath.Clean(hdr.Name) != "flue" { + continue + } + bin, err := io.ReadAll(io.LimitReader(tr, maxBinaryBytes+1)) + if err != nil { + return nil, err + } + if int64(len(bin)) > maxBinaryBytes { + return nil, fmt.Errorf("flue in the archive exceeds %d bytes", int64(maxBinaryBytes)) + } + return bin, nil + } +} + +// restartForUpdate is the half the old advisory story never had: a new +// binary on disk changes nothing until the process spawning shells is the +// new build. +// +// - Login service installed: restart it through the service manager and +// wait for a daemon that answers with the new version — the one line +// that proves the loop actually closed. +// - No service, daemon running: it was started by hand — flue serve in +// some terminal, or detached by flue open — and there is no stop command +// to bounce it cleanly from here; killing a foreground serve out from +// under the terminal that owns it would be a surprise, not a service. +// Say exactly what to run instead: SIGTERM is the graceful path +// (cmdServe snapshots sessions on the way out, and the next daemon +// revives them), so the two commands cost no session. +// - Nothing running: nothing to restart, and whatever starts the daemon +// next starts the new build. +func restartForUpdate(w io.Writer, latest string) error { + if mgr, err := newServiceManager(); err == nil { + if st, err := mgr.Status(); err == nil && st.Installed { + if err := mgr.Restart(); err != nil { + return fmt.Errorf("restart the login service: %w", err) + } + port, got, err := awaitVersion(updateRestartWait, latest) + if err != nil { + return err + } + fmt.Fprintf(w, " ✓ daemon restarted, running flue %s on 127.0.0.1:%d\n", got, port) + return nil + } + } + if port, ok := ourDaemon(); ok { + _, pid, _ := daemon.ReadRuntimeRecord() + fmt.Fprintf(w, " ! the daemon on 127.0.0.1:%d still runs the old build; restart it to finish:\n", port) + fmt.Fprintf(w, " kill %d && flue open\n", pid) + return nil + } + fmt.Fprintf(w, " no daemon running; the next flue open or flue enable starts flue %s\n", latest) + return nil +} + +// awaitVersion polls for a daemon of ours reporting version want — the same +// identity check awaitDaemon runs, plus the one question that matters after +// a swap: which build answered. The old daemon can still be shutting down +// when polling starts, so a daemon of the wrong version is something to wait +// through, not a failure; only the deadline decides. +func awaitVersion(wait time.Duration, want string) (port int, got string, err error) { + token, err := loadToken() + if err != nil { + return 0, "", fmt.Errorf("load auth token: %w", err) + } + deadline := time.Now().Add(wait) + for time.Now().Before(deadline) { + if p, ok := ourDaemon(); ok { + if v, err := daemonVersion(p, token); err == nil { + port, got = p, v + if v == want { + return port, got, nil + } + } + } + time.Sleep(50 * time.Millisecond) + } + if got != "" { + return 0, "", fmt.Errorf("the daemon came back running flue %s, not %s; run \"flue status\" to see what is installed where", got, want) + } + return 0, "", fmt.Errorf("the service was restarted but no daemon answered within %s; run \"flue status\" to see what it is doing", wait) +} + +// daemonVersion asks the daemon at port which build it is, via ReleasePath's +// Current field — the same authenticated read the sidebar's update card +// uses. Same transport habits as every other loopback read in this package: +// token in the header, status checked before decoding, response bounded. +func daemonVersion(port int, token string) (string, error) { + u := &url.URL{ + Scheme: "http", + Host: fmt.Sprintf("127.0.0.1:%d", port), + Path: daemon.ReleasePath, + } + req, err := http.NewRequest(http.MethodGet, u.String(), nil) + if err != nil { + return "", err + } + req.Header.Set(local.HeaderName, token) + resp, err := probeClient.Do(req) + if err != nil { + return "", err + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("daemon answered %s", resp.Status) + } + var body struct { + Current string `json:"current"` + } + if err := json.NewDecoder(io.LimitReader(resp.Body, maxMintBytes)).Decode(&body); err != nil { + return "", err + } + return body.Current, nil +} diff --git a/cmd/flue/update_test.go b/cmd/flue/update_test.go new file mode 100644 index 0000000..334e2e2 --- /dev/null +++ b/cmd/flue/update_test.go @@ -0,0 +1,476 @@ +package main + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "context" + "crypto/sha256" + "encoding/hex" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "runtime" + "strconv" + "strings" + "sync/atomic" + "testing" + "time" + + "github.com/karnstack/flue/internal/config" + "github.com/karnstack/flue/internal/daemon" + "github.com/karnstack/flue/internal/service" + "github.com/karnstack/flue/internal/session" + "github.com/karnstack/flue/internal/transport/local" +) + +// --- fakes and fixtures --- + +// flueArchive builds the release archive exactly as the contract promises: +// gzip over tar, one regular file named flue at the root. +func flueArchive(t *testing.T, bin []byte) []byte { + t.Helper() + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + if err := tw.WriteHeader(&tar.Header{ + Name: "flue", + Mode: 0o755, + Size: int64(len(bin)), + Typeflag: tar.TypeReg, + }); err != nil { + t.Fatalf("tar header: %v", err) + } + if _, err := tw.Write(bin); err != nil { + t.Fatalf("tar write: %v", err) + } + if err := tw.Close(); err != nil { + t.Fatalf("tar close: %v", err) + } + if err := gz.Close(); err != nil { + t.Fatalf("gzip close: %v", err) + } + return buf.Bytes() +} + +// fakeGitHub answers from a map of full URL to body — the latest-release +// JSON and the download assets through the one seam production uses. +func fakeGitHub(files map[string][]byte) func(context.Context, string) (*http.Response, error) { + return func(_ context.Context, url string) (*http.Response, error) { + if b, ok := files[url]; ok { + return &http.Response{StatusCode: 200, Body: io.NopCloser(bytes.NewReader(b))}, nil + } + return &http.Response{StatusCode: 404, Body: io.NopCloser(strings.NewReader("not found"))}, nil + } +} + +func releaseJSON(tag string) []byte { + return fmt.Appendf(nil, `{"tag_name":%q,"html_url":"https://github.com/karnstack/flue/releases/tag/%s"}`, tag, tag) +} + +// releaseFixture is a complete, verifiable fake release: the API answer plus +// this platform's archive and a checksums.txt that matches it. +func releaseFixture(t *testing.T, tag string, bin []byte) map[string][]byte { + t.Helper() + version := strings.TrimPrefix(tag, "v") + asset := fmt.Sprintf("flue_%s_%s_%s.tar.gz", version, runtime.GOOS, runtime.GOARCH) + archive := flueArchive(t, bin) + digest := sha256.Sum256(archive) + return map[string][]byte{ + releaseAPI: releaseJSON(tag), + releaseDownloadBase + tag + "/" + asset: archive, + releaseDownloadBase + tag + "/checksums.txt": fmt.Appendf(nil, "%s %s\n", hex.EncodeToString(digest[:]), asset), + } +} + +func updateChecker(current string, files map[string][]byte) *releaseChecker { + c := newReleaseChecker(current) + c.get = fakeGitHub(files) + return c +} + +func swapUpdateTarget(t *testing.T, fn func() (string, error)) { + t.Helper() + orig := updateTarget + updateTarget = fn + t.Cleanup(func() { updateTarget = orig }) +} + +func swapBrew(t *testing.T, onPath bool, run func(io.Writer) error) { + t.Helper() + origLook, origRun := brewOnPath, runBrewUpgrade + brewOnPath = func() bool { return onPath } + runBrewUpgrade = run + t.Cleanup(func() { brewOnPath, runBrewUpgrade = origLook, origRun }) +} + +// oldBinary writes a stand-in installed flue and points the updater at it. +func oldBinary(t *testing.T, mode os.FileMode) (dir, target string) { + t.Helper() + dir = t.TempDir() + target = filepath.Join(dir, "flue") + if err := os.WriteFile(target, []byte("the old build"), mode); err != nil { + t.Fatalf("write old binary: %v", err) + } + swapUpdateTarget(t, func() (string, error) { return target, nil }) + return dir, target +} + +// newVersionedDaemon is newTestDaemon with a version of the test's choosing, +// wired the way cmdServe wires a real one: the release checker is what +// serves ReleasePath's Current field, which is where flue update reads the +// restarted daemon's version from. Its get fails so no test asks GitHub. +func newVersionedDaemon(t *testing.T, token, ver string) int { + t.Helper() + srv := daemon.New(session.NewRegistry(time.Now), local.NewAuth(token, 0), uiHandler(), ver, daemon.Identity{}) + rc := newReleaseChecker(ver) + rc.get = func(context.Context, string) (*http.Response, error) { + return nil, errors.New("no network in tests") + } + srv.SetReleaseChecker(rc) + ts := httptest.NewServer(srv.Handler()) + t.Cleanup(ts.Close) + t.Cleanup(srv.Shutdown) + + u, err := url.Parse(ts.URL) + if err != nil { + t.Fatalf("parse test server URL %q: %v", ts.URL, err) + } + port, err := strconv.Atoi(u.Port()) + if err != nil { + t.Fatalf("parse port from %q: %v", ts.URL, err) + } + srv.SetAuth(local.NewAuth(token, port)) + return port +} + +// --- the refusals --- + +// TestRunUpdateRefusesADevBuild: a from-source build corresponds to no +// release, so there is nothing it could be updated *to* — and it must not +// even ask GitHub, because the answer could not change the refusal. +func TestRunUpdateRefusesADevBuild(t *testing.T) { + c := newReleaseChecker("dev") + var asked atomic.Bool + c.get = func(context.Context, string) (*http.Response, error) { + asked.Store(true) + return nil, errors.New("no network in tests") + } + + err := runUpdate(io.Discard, "dev", c) + if err == nil { + t.Fatal("runUpdate = nil for a dev build, want a refusal") + } + if !strings.Contains(err.Error(), "git pull") || !strings.Contains(err.Error(), "make build") { + t.Fatalf("refusal %q does not say how a from-source build updates", err) + } + if asked.Load() { + t.Fatal("a dev build asked GitHub about releases; the answer could not have mattered") + } +} + +// TestRunUpdateSaysAlreadyNewest: up to date is exit 0 and one line, and the +// binary is never located, let alone touched. +func TestRunUpdateSaysAlreadyNewest(t *testing.T) { + for _, current := range []string{"0.6.0", "0.7.0"} { // the tag, and ahead of it + t.Run(current, func(t *testing.T) { + c := updateChecker(current, map[string][]byte{releaseAPI: releaseJSON("v0.6.0")}) + swapUpdateTarget(t, func() (string, error) { + t.Error("an up-to-date flue went looking for the binary to replace") + return "", errors.New("refused by the test") + }) + + var out bytes.Buffer + if err := runUpdate(&out, current, c); err != nil { + t.Fatalf("runUpdate: %v", err) + } + if !strings.Contains(out.String(), "already the newest release") { + t.Fatalf("output does not say it is up to date:\n%s", out.String()) + } + }) + } +} + +// TestRunUpdateRefusesAChecksumMismatch is the security half of the download +// contract: an archive that does not match checksums.txt installs nothing, +// the old binary keeps its bytes, and no staging litter is left beside it. +func TestRunUpdateRefusesAChecksumMismatch(t *testing.T) { + dir, target := oldBinary(t, 0o755) + + files := releaseFixture(t, "v0.6.0", []byte("the new build")) + files[releaseDownloadBase+"v0.6.0/checksums.txt"] = fmt.Appendf(nil, + "%s flue_0.6.0_%s_%s.tar.gz\n", strings.Repeat("ab", 32), runtime.GOOS, runtime.GOARCH) + c := updateChecker("0.5.0", files) + + err := runUpdate(io.Discard, "0.5.0", c) + if err == nil { + t.Fatal("runUpdate = nil for a checksum mismatch, want a refusal") + } + if !strings.Contains(err.Error(), "sha256 mismatch") { + t.Fatalf("error %q does not name the mismatch", err) + } + got, rerr := os.ReadFile(target) + if rerr != nil || string(got) != "the old build" { + t.Fatalf("the installed binary changed on a refused update: %q, %v", got, rerr) + } + entries, rerr := os.ReadDir(dir) + if rerr != nil || len(entries) != 1 { + t.Fatalf("staging litter left behind: %v (want only the binary)", entries) + } +} + +// TestRunUpdateRefusesChecksumsWithoutOurEntry: a checksums.txt that has no +// line for this platform's asset proves nothing, so nothing installs. +func TestRunUpdateRefusesChecksumsWithoutOurEntry(t *testing.T) { + _, target := oldBinary(t, 0o755) + + files := releaseFixture(t, "v0.6.0", []byte("the new build")) + files[releaseDownloadBase+"v0.6.0/checksums.txt"] = []byte("deadbeef flue_0.6.0_plan9_mips.tar.gz\n") + c := updateChecker("0.5.0", files) + + err := runUpdate(io.Discard, "0.5.0", c) + if err == nil || !strings.Contains(err.Error(), "no entry") { + t.Fatalf("runUpdate error = %v, want the missing-entry refusal", err) + } + if got, _ := os.ReadFile(target); string(got) != "the old build" { + t.Fatalf("the installed binary changed on a refused update: %q", got) + } +} + +// --- the swap --- + +// TestRunUpdateSwapsTheBinary is the happy path end to end: download, +// verify, extract, and an atomic rename over the running binary's real path +// — with its mode preserved, the transcript in flue enable's voice, and no +// staging file left behind. +func TestRunUpdateSwapsTheBinary(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) // no service, no daemon + dir, target := oldBinary(t, 0o700) // an unusual mode, to watch it survive + swapManager(t, &fakeManager{}) // login service not installed + + newBin := []byte("the new build, byte for byte") + c := updateChecker("0.5.0", releaseFixture(t, "v0.6.0", newBin)) + + var out bytes.Buffer + if err := runUpdate(&out, "0.5.0", c); err != nil { + t.Fatalf("runUpdate: %v", err) + } + + got, err := os.ReadFile(target) + if err != nil { + t.Fatalf("read swapped binary: %v", err) + } + if !bytes.Equal(got, newBin) { + t.Fatalf("binary after the swap = %q, want the release's bytes", got) + } + info, err := os.Stat(target) + if err != nil { + t.Fatalf("stat swapped binary: %v", err) + } + if info.Mode().Perm() != 0o700 { + t.Fatalf("mode after the swap = %v, want the old binary's 0700 preserved", info.Mode().Perm()) + } + entries, err := os.ReadDir(dir) + if err != nil || len(entries) != 1 { + t.Fatalf("staging litter left beside the binary: %v", entries) + } + for _, want := range []string{ + "✓ flue 0.6.0 downloaded and verified", + "✓ installed to " + target, + "no daemon running", + } { + if !strings.Contains(out.String(), want) { + t.Errorf("transcript missing %q:\n%s", want, out.String()) + } + } +} + +// TestRunUpdateRefusesAnUnwritableTarget: no root-owned half-update, and the +// message says the fix. The staging file is created in the target's own +// directory precisely so this fails before any bytes are downloaded. +func TestRunUpdateRefusesAnUnwritableTarget(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("running as root: every directory is writable, so there is nothing to refuse") + } + dir, target := oldBinary(t, 0o755) + if err := os.Chmod(dir, 0o555); err != nil { + t.Fatalf("chmod: %v", err) + } + t.Cleanup(func() { _ = os.Chmod(dir, 0o755) }) + + var downloaded atomic.Bool + c := newReleaseChecker("0.5.0") + c.get = func(ctx context.Context, url string) (*http.Response, error) { + if url != releaseAPI { + downloaded.Store(true) + } + return fakeGitHub(map[string][]byte{releaseAPI: releaseJSON("v0.6.0")})(ctx, url) + } + + err := runUpdate(io.Discard, "0.5.0", c) + if err == nil { + t.Fatal("runUpdate = nil over an unwritable directory, want a refusal") + } + if !strings.Contains(err.Error(), "sudo flue update") { + t.Fatalf("refusal %q does not suggest sudo", err) + } + if downloaded.Load() { + t.Error("the archive was downloaded before the writability check refused") + } + if got, _ := os.ReadFile(target); string(got) != "the old build" { + t.Fatalf("the installed binary changed on a refused update: %q", got) + } +} + +// --- brew --- + +// TestRunUpdateHandsABrewInstallToBrew: a binary resolving into the Caskroom +// is brew's to replace — swapping it ourselves would corrupt brew's +// bookkeeping — so brew runs, no archive is downloaded, and the restart leg +// still happens (here: the nothing-running report). +func TestRunUpdateHandsABrewInstallToBrew(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + swapUpdateTarget(t, func() (string, error) { + return "/opt/homebrew/Caskroom/flue/0.5.0/flue", nil + }) + swapManager(t, &fakeManager{}) + + var brewRuns atomic.Int32 + swapBrew(t, true, func(w io.Writer) error { + brewRuns.Add(1) + fmt.Fprintln(w, "==> Upgrading karnstack/tap/flue") + return nil + }) + + var downloaded atomic.Bool + c := newReleaseChecker("0.5.0") + c.get = func(ctx context.Context, url string) (*http.Response, error) { + if url != releaseAPI { + downloaded.Store(true) + } + return fakeGitHub(map[string][]byte{releaseAPI: releaseJSON("v0.6.0")})(ctx, url) + } + + var out bytes.Buffer + if err := runUpdate(&out, "0.5.0", c); err != nil { + t.Fatalf("runUpdate: %v", err) + } + if n := brewRuns.Load(); n != 1 { + t.Fatalf("brew ran %d times, want 1", n) + } + if downloaded.Load() { + t.Error("an archive was downloaded for an install brew owns") + } + if !strings.Contains(out.String(), "✓ brew upgrade karnstack/tap/flue") { + t.Fatalf("transcript missing the brew checkmark:\n%s", out.String()) + } + if strings.Contains(out.String(), "installed to") { + t.Fatalf("transcript claims a file swap on the brew path:\n%s", out.String()) + } +} + +// TestRunUpdatePointsAtBrewWhenBrewIsMissing: Caskroom path, no brew on PATH +// — the one command that fixes it is the whole answer, and nothing is +// swapped by hand. +func TestRunUpdatePointsAtBrewWhenBrewIsMissing(t *testing.T) { + swapUpdateTarget(t, func() (string, error) { + return "/opt/homebrew/Caskroom/flue/0.5.0/flue", nil + }) + swapBrew(t, false, func(io.Writer) error { + t.Error("brew ran despite not being on PATH") + return nil + }) + c := updateChecker("0.5.0", map[string][]byte{releaseAPI: releaseJSON("v0.6.0")}) + + err := runUpdate(io.Discard, "0.5.0", c) + if err == nil { + t.Fatal("runUpdate = nil with a Caskroom binary and no brew, want a refusal") + } + if !strings.Contains(err.Error(), "brew upgrade karnstack/tap/flue") { + t.Fatalf("refusal %q does not name the brew command", err) + } +} + +// --- the restart --- + +// TestRunUpdateRestartsTheServiceAndReportsTheNewVersion closes the loop the +// command exists for: after the swap the service manager restarts the +// daemon, and the transcript's last line reports the version the *daemon* +// answered with — not the version this CLI hoped for. +func TestRunUpdateRestartsTheServiceAndReportsTheNewVersion(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + token, err := config.LoadOrCreateToken() + if err != nil { + t.Fatalf("LoadOrCreateToken: %v", err) + } + _, _ = oldBinary(t, 0o755) + + m := &fakeManager{st: service.Status{Installed: true, Running: true}} + m.onRestart = func() { + // The restarted daemon: the new build, discoverable the way a real + // one is — a runtime record naming a live process of ours. + port := newVersionedDaemon(t, token, "0.6.0") + if err := daemon.WriteRuntime(port); err != nil { + t.Errorf("WriteRuntime: %v", err) + } + } + swapManager(t, m) + + c := updateChecker("0.5.0", releaseFixture(t, "v0.6.0", []byte("the new build"))) + + var out bytes.Buffer + if err := runUpdate(&out, "0.5.0", c); err != nil { + t.Fatalf("runUpdate: %v", err) + } + if m.restartCalls != 1 { + t.Fatalf("Restart called %d times, want 1", m.restartCalls) + } + if !strings.Contains(out.String(), "✓ daemon restarted, running flue 0.6.0 on 127.0.0.1:") { + t.Fatalf("transcript missing the restarted-daemon line:\n%s", out.String()) + } +} + +// TestRunUpdateTellsTheUserAboutAStaleDaemon: no login service, but a daemon +// is up — it keeps running the old code after the swap, and there is no stop +// command to bounce it from here, so the transcript says exactly what to run. +func TestRunUpdateTellsTheUserAboutAStaleDaemon(t *testing.T) { + t.Setenv("XDG_CONFIG_HOME", t.TempDir()) + _, _ = oldBinary(t, 0o755) + swapManager(t, &fakeManager{}) // not installed + + port := newTestDaemon(t, "tok") + if err := daemon.WriteRuntime(port); err != nil { + t.Fatalf("WriteRuntime: %v", err) + } + + c := updateChecker("0.5.0", releaseFixture(t, "v0.6.0", []byte("the new build"))) + + var out bytes.Buffer + if err := runUpdate(&out, "0.5.0", c); err != nil { + t.Fatalf("runUpdate: %v", err) + } + for _, want := range []string{ + "still runs the old build", + fmt.Sprintf("kill %d && flue open", os.Getpid()), + } { + if !strings.Contains(out.String(), want) { + t.Errorf("transcript missing %q:\n%s", want, out.String()) + } + } +} + +// --- surface --- + +// TestUsageMentionsUpdate keeps flue update in the help text — a command +// nobody can discover may as well not exist, and the README's CLI table is +// kept in agreement with usageText by hand. +func TestUsageMentionsUpdate(t *testing.T) { + if !strings.Contains(usageText, "flue update") { + t.Fatalf("usage text does not mention %q:\n%s", "flue update", usageText) + } +} diff --git a/internal/service/launchd.go b/internal/service/launchd.go index a28020f..285a5f2 100644 --- a/internal/service/launchd.go +++ b/internal/service/launchd.go @@ -75,6 +75,26 @@ func (l *Launchd) Enable() error { return fmt.Errorf("launchctl bootstrap: %v: %s", err, out) } +// Restart stops the loaded job and bootstraps the plist on disk — the same +// bootout+bootstrap sequence Enable uses when the plist has drifted, chosen +// over `launchctl kickstart -k` on purpose: bootout tears the job down the +// graceful way (SIGTERM, then launchd's exit timeout), which is the signal +// cmdServe saves session snapshots on, while kickstart -k kills. A restart +// after an upgrade exists to carry live sessions onto the new build, so the +// graceful spelling is the only right one. +// +// The bootout error is tolerated for the reason Enable tolerates it: an +// unloaded label is not a failure to stop it, and the bootstrap — which reads +// the plist fresh, so it also converges any drift — is the call whose error +// matters. +func (l *Launchd) Restart() error { + _, _ = l.run.Run("launchctl", "bootout", l.serviceTarget()) + if out, err := l.run.Run("launchctl", "bootstrap", l.domainTarget(), l.unitPath()); err != nil { + return fmt.Errorf("launchctl bootstrap: %v: %s", err, out) + } + return nil +} + // Disable boots the agent out and removes the plist. Both halves tolerate // absence: a bootout of an unloaded label and a remove of a missing file are // what "already disabled" looks like, and that is a success. diff --git a/internal/service/launchd_test.go b/internal/service/launchd_test.go index 3e92a77..d6c3632 100644 --- a/internal/service/launchd_test.go +++ b/internal/service/launchd_test.go @@ -216,3 +216,57 @@ func TestLaunchdStatus(t *testing.T) { type errFake string func (e errFake) Error() string { return string(e) } + +// TestLaunchdRestartBootsOutAndBootstraps pins the graceful spelling: bootout +// (SIGTERM, the signal the daemon snapshots sessions on) and then bootstrap of +// the plist on disk — never kickstart -k, which kills. +func TestLaunchdRestartBootsOutAndBootstraps(t *testing.T) { + r := &fakeRunner{} + l, plist := newLaunchdUnderTest(t, r) + if err := l.Enable(); err != nil { + t.Fatalf("Enable: %v", err) + } + r.calls = nil + + if err := l.Restart(); err != nil { + t.Fatalf("Restart: %v", err) + } + if vs := strings.Join(r.verbs(), ","); vs != "bootout,bootstrap" { + t.Fatalf("verbs = %v, want [bootout bootstrap]", r.verbs()) + } + want := []string{"launchctl", "bootstrap", "gui/501", plist} + if got := r.calls[len(r.calls)-1]; strings.Join(got, " ") != strings.Join(want, " ") { + t.Fatalf("bootstrap call = %v, want %v", got, want) + } +} + +// TestLaunchdRestartToleratesAnUnloadedLabel: a service that is installed but +// not loaded has nothing to boot out, and that is not a failure to stop it — +// the bootstrap is what a restart is for. +func TestLaunchdRestartToleratesAnUnloadedLabel(t *testing.T) { + r := &fakeRunner{fail: map[string]error{"bootout": errFake("Boot-out failed: 5: Input/output error")}} + l, _ := newLaunchdUnderTest(t, r) + if err := l.Enable(); err != nil { + t.Fatalf("Enable: %v", err) + } + + if err := l.Restart(); err != nil { + t.Fatalf("Restart with nothing loaded: %v", err) + } +} + +func TestLaunchdRestartReportsABootstrapFailure(t *testing.T) { + r := &fakeRunner{ + fail: map[string]error{"bootstrap": errFake("exit status 5")}, + out: map[string]string{"bootstrap": "Bootstrap failed: 5: Input/output error"}, + } + l, _ := newLaunchdUnderTest(t, r) + + err := l.Restart() + if err == nil { + t.Fatal("Restart = nil when bootstrap failed, want the error") + } + if !strings.Contains(err.Error(), "bootstrap") { + t.Fatalf("Restart error = %q, want it to name the failing call", err) + } +} diff --git a/internal/service/service.go b/internal/service/service.go index 3b6d5ba..6bef81d 100644 --- a/internal/service/service.go +++ b/internal/service/service.go @@ -25,15 +25,21 @@ type Status struct { Running bool // the service manager reports it alive } -// Manager installs, removes, and inspects the flue login service. +// Manager installs, removes, restarts, and inspects the flue login service. // // - Enable converges: it rewrites the unit if it drifted, loads it if it is // not loaded, and starts it if it is dead — without restarting a healthy // daemon, whose sessions must survive a re-run of flue enable. // - Disable is idempotent: disabling what is not enabled is nil. +// - Restart bounces the daemon on purpose — the one thing Enable refuses to +// do — for the caller that has just replaced the binary and needs the +// running process to be the new one (flue update). Both implementations +// stop the daemon with SIGTERM, which is the graceful path cmdServe +// snapshots sessions on, so live sessions ride across the restart. type Manager interface { Enable() error Disable() error + Restart() error Status() (Status, error) } diff --git a/internal/service/systemd.go b/internal/service/systemd.go index e186b0a..f50b141 100644 --- a/internal/service/systemd.go +++ b/internal/service/systemd.go @@ -92,6 +92,19 @@ func (s *Systemd) Enable() error { // Warnings reports the advisories from the most recent Enable. func (s *Systemd) Warnings() []string { return s.warnings } +// Restart is systemd's own word for it: systemctl --user restart flue. The +// unit is stopped with SIGTERM — the graceful path cmdServe saves session +// snapshots on — and started from the unit file on disk, so a binary swapped +// since the last start is the one that execs. restart also starts a unit +// that happens to be dead, which is the convergence an update wants: the +// point is that the next running daemon is the new build. +func (s *Systemd) Restart() error { + if out, err := s.run.Run("systemctl", "--user", "restart", "flue"); err != nil { + return fmt.Errorf("systemctl --user restart flue: %v: %s", err, out) + } + return nil +} + // Disable stops and disables the unit, removes the file, and reloads. Every // systemctl failure is tolerated: on a machine with no user manager the file // removal is the whole operation, and "already disabled" is a success. diff --git a/internal/service/systemd_test.go b/internal/service/systemd_test.go index 5419c99..d3ca664 100644 --- a/internal/service/systemd_test.go +++ b/internal/service/systemd_test.go @@ -183,6 +183,37 @@ func TestSystemdStatus(t *testing.T) { } } +// TestSystemdRestartRunsTheExactCommand: systemctl --user restart flue, the +// spec's own spelling — SIGTERM to the old daemon, the unit file on disk for +// the new one. +func TestSystemdRestartRunsTheExactCommand(t *testing.T) { + r := &fakeRunner{} + s, _ := newSystemdUnderTest(t, r) + + if err := s.Restart(); err != nil { + t.Fatalf("Restart: %v", err) + } + if len(r.calls) != 1 || strings.Join(r.calls[0], " ") != "systemctl --user restart flue" { + t.Fatalf("calls = %v, want exactly [systemctl --user restart flue]", r.calls) + } +} + +func TestSystemdRestartReportsAFailure(t *testing.T) { + r := &fakeRunner{ + fail: map[string]error{"restart": errors.New("exit status 1")}, + out: map[string]string{"restart": "Failed to restart flue.service: Unit not found."}, + } + s, _ := newSystemdUnderTest(t, r) + + err := s.Restart() + if err == nil { + t.Fatal("Restart = nil when systemctl failed, want the error") + } + if !strings.Contains(err.Error(), "restart") { + t.Fatalf("Restart error = %q, want it to name the failing command", err) + } +} + func TestForPlatform(t *testing.T) { r := &fakeRunner{} if m, err := ForPlatform("darwin", "/x/flue", t.TempDir(), 501, r); err != nil || m == nil {