diff --git a/cmd/mavend/main.go b/cmd/mavend/main.go index d6df63a..05e92a8 100644 --- a/cmd/mavend/main.go +++ b/cmd/mavend/main.go @@ -356,7 +356,11 @@ func run(args []string) error { if locked { srv.Check = func(ctx context.Context, m ipc.Method, _ json.RawMessage) error { switch m { - case ipc.MethodAssertStepUp, ipc.MethodUnlock: + case ipc.MethodAssertStepUp, ipc.MethodUnlock, ipc.MethodPing: + // Ping is allowed for the same reason the two unlock methods + // are: it never reaches CoreAPI. It answers "she is up and + // locked", which is what mavupdate needs to tell a daemon + // waiting for a passkey apart from one that failed to start. return nil // allowed in locked mode default: return errLocked @@ -367,6 +371,7 @@ func run(args []string) error { } srv.StepUp = func(ctx context.Context) error { return passkeySess.Assert(ctx, auth.Scope{}) } + srv.LockedFn = dl.isLocked // Mail ingestion (Vikunja #246): the hook stays nil unless an email block is // configured and there is a llama-server to extract with, in which case diff --git a/cmd/mavupdate/main.go b/cmd/mavupdate/main.go index 4dfe1b5..7f64914 100644 --- a/cmd/mavupdate/main.go +++ b/cmd/mavupdate/main.go @@ -12,9 +12,12 @@ // the gate that guards the tool allowlist. That is deliberate and it is the // reason there is no MethodApplyUpdate anywhere in internal/ipc. // -// Consequently: mavend does not import internal/update, nothing runs on a timer, -// nothing checks a release server, and no act, intent, tool or LLM output can -// reach any of this. She cannot update herself. She can be updated, by him. +// Consequently: mavend never constructs an update.Updater and nothing in the +// daemon can call Apply, nothing runs on a timer, nothing checks a release +// server, and no act, intent, tool or LLM output can reach any of this. The +// package is linked into mavend through internal/config, which validates the +// update block at startup; the guarantee is the absent caller, not an absent +// import. She cannot update herself. She can be updated, by him. // // mavupdate -config deploy/mavend.json list # snapshots available to roll back to // mavupdate -config deploy/mavend.json verify # make build + make test, deploys nothing @@ -158,6 +161,12 @@ func cmdRollback(ctx context.Context, u *update.Updater, id string) { res, err := u.Rollback(ctx, id) report(res.Steps) summarize(res) + // The standalone rollback is what he reaches for when something is already + // wrong, so a failed one needs the loud paragraph more than apply does, not + // less. + if errors.Is(err, update.ErrRollbackFailed) { + die("\n%v\n\nSHE IS PROBABLY DOWN. The previous artifacts are in the snapshot dir; copy them\nover the install dir and restart by hand.", err) + } if err != nil && !errors.Is(err, update.ErrRolledBack) { die("\n%v", err) } diff --git a/deploy/README.md b/deploy/README.md index 790cc55..d57942f 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -132,18 +132,61 @@ it), with paths as they exist **on the host**, not inside a container: "source_dir": "/home/kami/apps/Maven", "install_dir": "/home/kami/apps/Maven", "snapshot_dir": "/var/lib/maven-snapshots", + "source_rollback": "git", "binaries": ["mavend", "mavweb", "mavsttd", "mavttsd", "mavwaked", - "mavenclient", "mavpoll", "mavcaldav", "mavmaild"], + "mavenclient", "mavpoll", "mavcaldav", "mavmaild", "mavupdate"], "config_files": ["deploy/mavend.json"], - "restart_cmd": ["docker", "compose", "up", "-d", "--build"], - "health_socket": "/var/lib/docker/volumes/maven_sockets/_data/mavend.sock", + "restart_cmd": ["docker", "compose", "up", "-d", "--build", "mavend"], + "health_socket": "/run/maven-host/mavend.sock", "health_timeout_sec": 120 } ``` -`snapshot_dir` must be outside `install_dir` (a restore must not read from what -the install writes) and `health_socket` is required: an update that cannot check -its own result cannot roll itself back, so the config is refused without one. +`snapshot_dir` must be outside both `install_dir` and `source_dir` (a restore +must not read from what the install writes, and a snapshot dir inside the tree +lands in the docker build context). `health_socket` is required: an update that +cannot check its own result cannot roll itself back, so the config is refused +without one. + +**`source_rollback` is what makes a rollback real on this deployment.** Compose +builds the image from the tree — the Dockerfile copies `cmd/` and `internal/` +and runs the build in the builder stage, and `.dockerignore` keeps the host +binaries out — so `install_dir` is the tree, `install` is a no-op, and putting +the old binaries back puts back bytes nothing reads. A rollback that only did +that would rebuild the same bad image and burn a second health timeout proving +it. With `"source_rollback": "git"` the commit is recorded before the update and +checked back out before the restart, so the restore is of the thing that +actually gets deployed. It requires a clean tree: `apply` refuses to start with +uncommitted changes, because the recorded commit would not describe what is +deployed and the forced checkout on the way back would delete the work. It also +means a rollback moves every tracked file, `deploy/mavend.json` included, so on +this deployment a config edit belongs in a commit. + +Leaving `source_rollback` out is only valid when `install_dir` holds what +actually runs. `Validate` refuses the combination of "same dir" and "no way to +put the source back" at startup rather than at the one rollback that mattered. + +**The socket has to be one the account running `mavupdate` can open.** The +compose stack keeps IPC in a named volume, whose host path +(`/var/lib/docker/volumes/maven_sockets/_data`) is under a `drwx--x--- root +root` directory, and the socket itself is 0600 owned by the container's uid +10001. A non-root `mavupdate` gets EACCES on the dial, which reports as +`update: cannot open the health socket` rather than as a daemon that will not +answer. Bind-mount the socket dir to a host path he owns and run the daemon +under his uid instead: + +```yaml +mavend: + user: "1000:1000" + volumes: + - /run/maven-host:/run/maven +``` + +Do **not** work around it with `sudo mavupdate apply`. `verify` runs `make +build` and `make test` in `source_dir`, and as root that leaves root-owned +binaries, object files and a build cache in the working tree, so the next +ordinary `make` fails. `Verify` refuses to run as root over a tree owned by +someone else for exactly that reason. Then: diff --git a/internal/config/config.go b/internal/config/config.go index 0124e1a..717cff2 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -117,10 +117,14 @@ type Config struct { // the update capability does not exist, which is the state to leave it in // unless the operator has read internal/update's package comment. // - // mavend never reads this block: the daemon does not import internal/update - // and cannot update itself. It lives here because cmd/mavupdate — a CLI the - // owner runs on the host, the only trigger there is — reads the same config - // file to find the socket it health-checks. + // mavend never acts on this block: it constructs no Updater and cannot + // update itself. Validate below is the one thing the daemon does with it, so + // a broken update config is caught at startup instead of on the night it is + // needed. That validation is also why internal/update is linked into mavend + // at all — linked, with no caller, which is the property that matters. The + // block lives here because cmd/mavupdate — a CLI the owner runs on the host, + // the only trigger there is — reads the same config file to find the socket + // it health-checks. Update *update.Config `json:"update,omitempty"` // Voice — the client↔core surface + the stt/tts modules the daemon diff --git a/internal/ipc/api.go b/internal/ipc/api.go index 30cc4ab..db71b6c 100644 --- a/internal/ipc/api.go +++ b/internal/ipc/api.go @@ -390,6 +390,15 @@ type ModelStatusResp struct { Swappable []string `json:"swappable,omitempty"` } +// PingResp — the answer to MethodPing. Alive is always true (the reply itself +// is the proof); Locked says whether the daemon is still waiting for a passkey +// assertion, which is the one state where a CoreAPI read cannot tell an +// operator anything. +type PingResp struct { + Alive bool `json:"alive"` + Locked bool `json:"locked"` +} + type listTasksReq struct { Status string `json:"status"` // "" all | "live" | candidate|open|done|dropped } diff --git a/internal/ipc/client.go b/internal/ipc/client.go index fae2f4d..1009e58 100644 --- a/internal/ipc/client.go +++ b/internal/ipc/client.go @@ -75,6 +75,7 @@ var readOnlyMethods = map[Method]bool{ MethodMCPServers: true, MethodDayPlan: true, MethodRecentEvents: true, + MethodPing: true, } // Dial connects to a core socket at path and returns a Client. The module @@ -636,5 +637,16 @@ func (c *Client) RevertFact(ctx context.Context, key string) (int64, error) { return result.NewID, nil } +// Ping asks whether the daemon is there, and whether it is locked. It is not a +// CoreAPI method: it touches no store, so it answers before the passkey +// assertion that every other read waits for. +func (c *Client) Ping(ctx context.Context) (PingResp, error) { + var r PingResp + if err := c.call(ctx, MethodPing, nil, &r); err != nil { + return PingResp{}, err + } + return r, nil +} + // Compile-time check: *Client satisfies CoreAPI. var _ CoreAPI = (*Client)(nil) diff --git a/internal/ipc/server.go b/internal/ipc/server.go index cee5355..772012d 100644 --- a/internal/ipc/server.go +++ b/internal/ipc/server.go @@ -486,6 +486,11 @@ type Server struct { ListSpeakersFn ListSpeakersFunc ForgetSpeakerFn ForgetSpeakerFunc + // LockedFn — reports whether the daemon is in locked (pre-unlock) mode. + // Read by MethodPing only. Nil ⇒ not locked, which is what an embedded or + // test Server without the unlock dance is. + LockedFn func() bool + // UnlockFn — unwraps the store encryption key from the wrapped blob using // the passkey PRF secret, opens the encrypted store, and wires // the rest of the daemon (voice, loop, delivery). Set by the daemon when @@ -933,6 +938,17 @@ func (s *Server) dispatch(ctx context.Context, req Request) (json.RawMessage, er // directly by the daemon (StepUp / WrapKeyFn / UnlockFn), not store // state, so they can never be table entries keyed on a CoreAPI method. switch req.Method { + case MethodPing: + // Deliberately reaches nothing: no store, no CoreAPI, no daemon + // component. That is what makes it answerable in locked mode, and it is + // the whole point — an update that restarts her into locked mode has to + // be able to tell that apart from a daemon that did not come up. + locked := false + if s.LockedFn != nil { + locked = s.LockedFn() + } + return marshalResult(PingResp{Alive: true, Locked: locked}), nil + case MethodAssertStepUp: if s.StepUp != nil { return marshalResult(nil), s.StepUp(ctx) diff --git a/internal/ipc/unlock_test.go b/internal/ipc/unlock_test.go index 14bdb2e..3e1611c 100644 --- a/internal/ipc/unlock_test.go +++ b/internal/ipc/unlock_test.go @@ -122,3 +122,39 @@ func TestLockedCheckDefaultDenies(t *testing.T) { t.Error("UnlockFn never ran") } } + +// A locked daemon has to be able to say it is alive. Every CoreAPI method is +// refused before unlock, so a health check built on one of those cannot tell a +// daemon waiting for a passkey apart from a daemon that failed to start. That +// is what turned a good update into the manual-recovery case in +// internal/update. MethodPing reaches no store, so it answers either way. +func TestPingAnswersWhileLocked(t *testing.T) { + _, srv, cli, _ := newServerWithStore(t) + srv.LockedFn = func() bool { return true } + locked := errors.New("daemon locked") + srv.Check = func(_ context.Context, m Method, _ json.RawMessage) error { + switch m { + case MethodAssertStepUp, MethodUnlock, MethodPing: + return nil + default: + return locked + } + } + ctx := context.Background() + p, err := cli.Ping(ctx) + if err != nil { + t.Fatalf("Ping while locked: %v", err) + } + if !p.Alive || !p.Locked { + t.Errorf("Ping = %+v; want alive and locked", p) + } + // And the read it replaces is still refused, which is the whole point. + if _, err := cli.Presence(ctx); err == nil { + t.Error("Presence answered while locked") + } + + srv.LockedFn = func() bool { return false } + if p, err := cli.Ping(ctx); err != nil || p.Locked { + t.Errorf("Ping after unlock = %+v, %v; want alive and not locked", p, err) + } +} diff --git a/internal/ipc/wire.go b/internal/ipc/wire.go index fc0d7f5..d9eea80 100644 --- a/internal/ipc/wire.go +++ b/internal/ipc/wire.go @@ -63,6 +63,14 @@ const ( MethodListSpeakers Method = "list_speakers" MethodForgetSpeaker Method = "forget_speaker" MethodRecentEvents Method = "recent_events" + + // MethodPing — liveness, and the only method that answers in locked mode + // without a passkey assertion. It reaches no store, takes no arguments and + // returns whether the daemon is locked, so an operator tool can tell "she is + // up and waiting for a passkey" apart from "she is not there at all". + // Everything else about her state needs the store, and the store needs the + // key. + MethodPing Method = "ping" ) // Request — one frame from module to core. Params is the JSON-encoded argument diff --git a/internal/update/apply.go b/internal/update/apply.go index d11f22a..e839d70 100644 --- a/internal/update/apply.go +++ b/internal/update/apply.go @@ -41,9 +41,31 @@ func (u *Updater) Apply(ctx context.Context) (Result, error) { // has no baseline to prove itself against. u.log("preflight: checking the running daemon") if err := u.health(ctx, u.cfg.HealthSocket); err != nil { + if errors.Is(err, ErrHealthDial) { + // Not the daemon's fault and not fixed by fixing the daemon. The + // usual cause is the socket path: under a docker volume it sits in + // /var/lib/docker, which the operator's account cannot traverse. + return res, err + } return res, fmt.Errorf("%w: %v", ErrUnhealthyBefore, err) } + // 0b. If the source is part of what a rollback has to put back, it must be + // in a state that can be described and restored. A dirty tree is neither: + // the commit recorded in the snapshot does not say what is deployed, and a + // forced checkout on the way back would delete his uncommitted work. + commit := u.gitHead(ctx) + if u.cfg.SourceRollback == "git" { + if commit == "" { + return res, fmt.Errorf("%w: source_rollback is \"git\" but %s has no readable git HEAD", ErrSourceRollback, u.cfg.SourceDir) + } + if dirty, err := u.gitDirty(ctx); err != nil { + return res, fmt.Errorf("%w: %v", ErrDirtyTree, err) + } else if dirty { + return res, fmt.Errorf("%w: %s", ErrDirtyTree, u.cfg.SourceDir) + } + } + // 1. Snapshot what is deployed now, BEFORE the build. // // The order matters and it is not the obvious one. `make build` writes its @@ -53,7 +75,7 @@ func (u *Updater) Apply(ctx context.Context) (Result, error) { // only thing standing between a bad build and a box that needs a screwdriver, // so it is taken first, while the deployed bytes are still the old ones. names := append(append([]string{}, u.cfg.Binaries...), u.cfg.ConfigFiles...) - snap, err := u.store.Save(u.cfg.InstallDir, names, u.gitHead(ctx), "pre-update") + snap, err := u.store.Save(u.cfg.InstallDir, names, commit, "pre-update") if err != nil { return res, err } @@ -68,10 +90,13 @@ func (u *Updater) Apply(ctx context.Context) (Result, error) { steps, err := u.Verify(ctx) res.Steps = append(res.Steps, steps...) if err != nil { - if rerr := snap.Restore(u.cfg.InstallDir); rerr != nil { + // RolledBack stays false here on purpose. Nothing was installed and + // nothing was restarted, so there is no rollback to report; a compile + // error printing rolled_back=true sends the operator looking for a + // restart that never happened. The log line carries what was done. + if rerr := u.restoreBinaries(snap); rerr != nil { u.log("verify failed and the artifacts could not be put back: %v — the previous ones are in %s", rerr, snap.Dir()) } else { - res.RolledBack = true u.log("verify failed; the previously deployed artifacts are back in place, she was never restarted") } return res, err @@ -141,10 +166,17 @@ func (u *Updater) rollback(ctx context.Context, snap Snapshot, res Result, cause ctx = context.WithoutCancel(ctx) res.RolledBack = true u.log("rollback: restoring snapshot %s over %s", snap.ID, u.cfg.InstallDir) - if err := snap.Restore(u.cfg.InstallDir); err != nil { + if err := u.restoreBinaries(snap); err != nil { u.log("rollback: RESTORE FAILED: %v", err) return res, fmt.Errorf("%w: %v (after %v); the previous artifacts are in %s — copy them back by hand", ErrRollbackFailed, err, cause, snap.Dir()) } + // On a deployment that rebuilds from source, putting the binaries back is + // the part that changes nothing. The source is what the restart deploys, so + // it goes back too, and it goes back before the restart that reads it. + if err := u.restoreSource(ctx, snap); err != nil { + u.log("rollback: SOURCE CHECKOUT FAILED: %v", err) + return res, fmt.Errorf("%w: %v (after %v); the tree is still on the new commit, so a restart would redeploy it — `git -C %s checkout --force %s` by hand", ErrRollbackFailed, err, cause, u.cfg.SourceDir, snap.Commit) + } // A restore with no restart leaves the failed process running, so a failed // restart here is still the manual-recovery case. if err := u.restart(ctx, &res); err != nil { @@ -197,6 +229,48 @@ func (u *Updater) restart(ctx context.Context, res *Result) error { return nil } +// restoreBinaries puts back the built artifacts and nothing else. +// +// ConfigFiles are snapshotted and deliberately not restored. The Config doc +// says an update never replaces the operator's config, and a rollback that +// quietly reverted deploy/mavend.json would undo edits made since the last +// apply — a phraser.model_path change among them, which is how the resident +// model gets swapped. The copies stay in the snapshot dir for him to take by +// hand if the config is what he wants back. +func (u *Updater) restoreBinaries(snap Snapshot) error { + return snap.RestoreOnly(u.cfg.InstallDir, u.cfg.Binaries) +} + +// restoreSource puts the working tree back on the commit the snapshot was taken +// at, for the deployments where that is what the restart command deploys. A +// no-op for every other shape. +func (u *Updater) restoreSource(ctx context.Context, snap Snapshot) error { + if u.cfg.SourceRollback != "git" { + return nil + } + if snap.Commit == "" { + return fmt.Errorf("snapshot %s records no commit, so there is nothing to check out", snap.ID) + } + u.log("rollback: checking %s back out to %s", u.cfg.SourceDir, snap.Commit) + // --force because the failed build left artifacts in the tree. Safe only + // because Apply refused to start on a dirty tree, so nothing uncommitted of + // his is in reach. + out, err := u.run(ctx, u.cfg.SourceDir, []string{"git", "checkout", "--force", snap.Commit}) + if err != nil { + return fmt.Errorf("git checkout %s: %v: %s", snap.Commit, err, tail(out, 1000)) + } + return nil +} + +// gitDirty reports whether the working tree has uncommitted changes. +func (u *Updater) gitDirty(ctx context.Context) (bool, error) { + out, err := u.run(ctx, u.cfg.SourceDir, []string{"git", "status", "--porcelain"}) + if err != nil { + return false, fmt.Errorf("git status in %s: %v", u.cfg.SourceDir, err) + } + return strings.TrimSpace(out) != "", nil +} + // gitHead records which commit produced a snapshot, for the operator's benefit. // Best-effort: a tree without git is not a reason to refuse to snapshot. func (u *Updater) gitHead(ctx context.Context) string { diff --git a/internal/update/health.go b/internal/update/health.go index 06b21bc..621f78a 100644 --- a/internal/update/health.go +++ b/internal/update/health.go @@ -2,6 +2,7 @@ package update import ( "context" + "errors" "fmt" "time" @@ -17,14 +18,39 @@ import ( // Presence is the method used because it is read-only (safe to retry), needs no // arguments, and touches the store. It cannot write anything, so a health check // never leaves a trace in her memory. +// +// A locked daemon is healthy. With a passkey enrolled and no env key mavend +// boots locked and refuses every CoreAPI method until an assertion arrives, so +// a Presence read there fails for a daemon that came up perfectly. Rolling back +// on that would turn a good update into the manual-recovery case, and the +// rollback would boot locked too. MethodPing reaches no store and answers in +// locked mode, so it is asked first: an answer of "locked" is proof of life and +// is where the check stops. -// DialHealth connects to the mavend socket and performs one read. +// ErrHealthDial — the socket could not be opened at all. Kept apart from a +// failed read because the two have different causes and different fixes: on the +// docker deployment the socket lives under /var/lib/docker, which is +// drwx--x--- root root, so a non-root operator gets EACCES before reaching +// mavend. "She is not answering" would be the wrong thing to tell him. +var ErrHealthDial = errors.New("update: cannot open the health socket") + +// DialHealth connects to the mavend socket and proves someone is serving it. func DialHealth(ctx context.Context, socket string) error { c, err := ipc.Dial(socket) if err != nil { - return fmt.Errorf("update: health dial: %w", err) + return fmt.Errorf("%w %s: %v", ErrHealthDial, socket, err) } defer c.Close() + // Liveness first, because it is the only question a locked daemon can + // answer. ErrUnknownMethod means an older mavend on the other end, which is + // exactly the case during a rollback to a build from before ping existed — + // fall through to the store read rather than calling that a failure. + switch p, perr := c.Ping(ctx); { + case perr == nil && p.Locked: + return nil + case perr != nil && !errors.Is(perr, ipc.ErrUnknownMethod): + return fmt.Errorf("update: health ping: %w", perr) + } if _, err := c.Presence(ctx); err != nil { return fmt.Errorf("update: health read: %w", err) } @@ -39,7 +65,20 @@ func (u *Updater) waitHealthy(ctx context.Context, timeout time.Duration) error delay := 500 * time.Millisecond var last error for { - attemptCtx, cancel := context.WithTimeout(ctx, 10*time.Second) + // Cap the attempt at whatever is left of the budget, not a flat 10s: an + // attempt starting at 89s of a 90s timeout would otherwise run to 99s, + // and the caller asked for 90. + attempt := 10 * time.Second + if left := deadline.Sub(u.now()); left < attempt { + attempt = left + } + if attempt <= 0 { + if last == nil { + last = context.DeadlineExceeded + } + return fmt.Errorf("update: not healthy after %s: %w", timeout, last) + } + attemptCtx, cancel := context.WithTimeout(ctx, attempt) err := u.health(attemptCtx, u.cfg.HealthSocket) cancel() if err == nil { diff --git a/internal/update/snapshot.go b/internal/update/snapshot.go index 42af6c6..f591f9e 100644 --- a/internal/update/snapshot.go +++ b/internal/update/snapshot.go @@ -164,11 +164,26 @@ func (st *Store) Load(id string) (Snapshot, error) { // This is the function the whole package exists to be able to run. It uses the // filesystem and nothing else — no toolchain, no build, no cooperation from the // code being replaced. -func (s Snapshot) Restore(dstDir string) error { +func (s Snapshot) Restore(dstDir string) error { return s.RestoreOnly(dstDir, nil) } + +// RestoreOnly is Restore limited to the named files. A nil list means all of +// them. The caller uses it to put binaries back without putting config back: +// see Updater.restoreBinaries. +func (s Snapshot) RestoreOnly(dstDir string, names []string) error { if s.dir == "" { return errors.New("update: snapshot has no directory (load it through the store)") } + var want map[string]bool + if names != nil { + want = make(map[string]bool, len(names)) + for _, n := range names { + want[n] = true + } + } for _, f := range s.Files { + if want != nil && !want[f.Name] { + continue + } src := filepath.Join(s.dir, f.Name) sum, err := hashFile(src) if err != nil { diff --git a/internal/update/source_test.go b/internal/update/source_test.go new file mode 100644 index 0000000..e3914eb --- /dev/null +++ b/internal/update/source_test.go @@ -0,0 +1,193 @@ +package update + +import ( + "context" + "errors" + "path/filepath" + "strings" + "testing" +) + +// The deployment the README documents builds the image from the working tree: +// source_dir and install_dir are the same path, the Dockerfile copies cmd/ and +// internal/ and runs the build inside the builder stage, and .dockerignore +// keeps the host binaries out. Restoring binaries there restores bytes nothing +// reads. These tests pin the two acceptable outcomes: the source goes back, or +// the config is refused. + +// dockerBox — a fakeBox wired the way the README wires the docker deployment. +func dockerBox(t *testing.T, sourceRollback string) (*fakeBox, Config) { + t.Helper() + b := newFakeBox(t) + // The source IS the deployment. The old commit's source is what the running + // image was built from. + b.byCommit["old-commit"] = "GOOD-SOURCE" + b.commit = "old-commit" + write(t, filepath.Join(b.root, "src", "source.go"), "BAD-SOURCE") + + cfg := b.cfg() + cfg.InstallDir = cfg.SourceDir + cfg.ConfigFiles = nil + cfg.SourceRollback = sourceRollback + // The artifacts the docker shape snapshots live in the tree. + write(t, filepath.Join(b.root, "src", "mavend"), "OLD-BUILD") + return b, cfg +} + +func TestValidate_RefusesABuildFromSourceDeploymentThatCannotRollBack(t *testing.T) { + _, cfg := dockerBox(t, "") + err := cfg.Validate() + if !errors.Is(err, ErrSourceRollback) { + t.Fatalf("Validate = %v; want ErrSourceRollback — a binary snapshot rolls back nothing when the restart rebuilds from the tree", err) + } + if _, err := New(cfg); !errors.Is(err, ErrSourceRollback) { + t.Fatalf("New = %v; want the same refusal", err) + } +} + +func TestApply_RollbackPutsTheSourceBackBeforeTheRestart(t *testing.T) { + b, cfg := dockerBox(t, "git") + u, err := New(cfg, WithRunner(b.run), WithHealth(b.health)) + if err != nil { + t.Fatal(err) + } + // The new build compiles and passes, and then does not come up — the case + // this whole package exists for. + first := true + b.healthFn = func() error { + if first { + first = false + return nil // preflight + } + if len(b.builtFromSource) > 0 && b.builtFromSource[len(b.builtFromSource)-1] == "GOOD-SOURCE" { + return nil // she answers again once the good source is deployed + } + return errors.New("she does not answer") + } + + res, err := u.Apply(context.Background()) + if !errors.Is(err, ErrRolledBack) { + t.Fatalf("Apply = %v; want ErrRolledBack", err) + } + if !res.RollbackHealthy { + t.Fatalf("result = %+v; want a healthy rollback", res) + } + if len(b.builtFromSource) != 2 { + t.Fatalf("restarts = %v; want the bad one and the rolled-back one", b.builtFromSource) + } + if b.builtFromSource[1] != "GOOD-SOURCE" { + t.Errorf("the rollback restarted on %q; want the previous commit's source — restoring binaries alone redeploys the bad commit", b.builtFromSource[1]) + } + // And the checkout came before the restart, not after it. + var checkoutAt, restartAt = -1, -1 + for i, c := range b.ran { + if strings.HasPrefix(c, "git checkout") { + checkoutAt = i + } + if c == "restart-the-thing" { + restartAt = i + } + } + if checkoutAt < 0 || restartAt < checkoutAt { + t.Errorf("commands ran = %v; want the checkout before the last restart", b.ran) + } +} + +func TestApply_RefusesADirtyTreeWhenTheSourceIsTheRollbackTarget(t *testing.T) { + b, cfg := dockerBox(t, "git") + b.dirty = true + u, err := New(cfg, WithRunner(b.run), WithHealth(b.health)) + if err != nil { + t.Fatal(err) + } + if _, err := u.Apply(context.Background()); !errors.Is(err, ErrDirtyTree) { + t.Fatalf("Apply on a dirty tree = %v; want ErrDirtyTree", err) + } + for _, c := range b.ran { + if strings.HasPrefix(c, "make") || c == "restart-the-thing" { + t.Errorf("a refused update still ran %q", c) + } + } +} + +func TestApply_VerifyFailureDoesNotClaimARollback(t *testing.T) { + b := newFakeBox(t) + b.testErr = errors.New("exit status 1") + res, err := b.updater(t).Apply(context.Background()) + if !errors.Is(err, ErrVerifyFailed) { + t.Fatalf("Apply = %v; want ErrVerifyFailed", err) + } + if res.RolledBack { + t.Error("a compile error reported rolled_back=true; nothing was installed and nothing was restarted") + } +} + +func TestRollback_LeavesHisConfigAlone(t *testing.T) { + b := newFakeBox(t) + if _, err := b.updater(t).Apply(context.Background()); err != nil { + t.Fatalf("setup Apply: %v", err) + } + // He edits the config after the update — a phraser.model_path change, say. + cfgPath := filepath.Join(b.root, "install", "mavend.json") + write(t, cfgPath, `{"tick_interval":"90s"}`) + + if _, err := b.updater(t).Rollback(context.Background(), ""); err != nil && !errors.Is(err, ErrRolledBack) { + t.Fatalf("Rollback: %v", err) + } + if got := read(t, cfgPath); !strings.Contains(got, "90s") { + t.Errorf("config after rollback = %q; a rollback must not revert his config", got) + } + if got := b.deployed(); got != "OLD-BUILD" { + t.Errorf("deployed binary = %q; want the binaries rolled back", got) + } +} + +func TestVerify_RefusesToBuildHisTreeAsRoot(t *testing.T) { + b := newFakeBox(t) + u := b.updater(t, WithIDs(func(string) (int, uint32, error) { return 0, 1000, nil })) + if _, err := u.Verify(context.Background()); !errors.Is(err, ErrRootOnHisTree) { + t.Fatalf("Verify as root over a uid-1000 tree = %v; want ErrRootOnHisTree", err) + } + if len(b.ran) != 0 { + t.Errorf("the refusal still ran %v — root-owned artifacts would break his next make", b.ran) + } + // Root's own tree is fine, and so is a normal user. + for _, ids := range []func(string) (int, uint32, error){ + func(string) (int, uint32, error) { return 0, 0, nil }, + func(string) (int, uint32, error) { return 1000, 1000, nil }, + } { + if _, err := b.updater(t, WithIDs(ids)).Verify(context.Background()); err != nil { + t.Errorf("Verify refused a legitimate build: %v", err) + } + } +} + +func TestValidate_RefusesSnapshotsInsideTheSourceTree(t *testing.T) { + cfg := (&fakeBox{root: t.TempDir()}).cfg() + cfg.SnapshotDir = filepath.Join(cfg.SourceDir, "snaps") + if err := cfg.Validate(); err == nil { + t.Error("snapshot_dir inside source_dir was accepted; it lands in the docker build context") + } + cfg = (&fakeBox{root: t.TempDir()}).cfg() + cfg.SourceRollback = "svn" + if err := cfg.Validate(); err == nil { + t.Error("an unknown source_rollback was accepted") + } +} + +func TestTail_CutsOnARuneBoundary(t *testing.T) { + s := strings.Repeat("x", 20) + "--- FAIL: TestПривет" + got := tail(s, 10) + if !utf8ValidString(got) { + t.Errorf("tail(%q) = %q; a cut mid-rune shows as a replacement character", s, got) + } +} + +func utf8ValidString(s string) bool { + for _, r := range s { + if r == 0xFFFD { + return false + } + } + return true +} diff --git a/internal/update/update.go b/internal/update/update.go index e030335..5dc53fa 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -10,12 +10,16 @@ // - It is never automatic and never on a timer. There is no checker, no // channel, no "check for updates" call and nothing that fires from the tick // loop. Apply runs exactly when a human runs cmd/mavupdate on the box. -// - The daemon cannot update itself. mavend does not import this package and -// there is no IPC method and no web route that reaches it, so no act, no -// intent, no tool and no LLM output can start an update. The trigger needs -// shell access to the host, which is a strictly higher bar than the step-up -// passkey gate that guards /tools — an update is not a thing to expose to -// anything reachable over the network. +// - The daemon cannot update itself. mavend never constructs an Updater and +// nothing in the daemon can call Apply: there is no IPC method and no web +// route that reaches this package, so no act, no intent, no tool and no LLM +// output can start an update. (The package IS linked into mavend, via +// internal/config, which calls Config.Validate so a bad update block is +// caught at daemon startup rather than on the night it is needed. Linked is +// not reachable — the guarantee is the absent caller, not an absent +// import.) The trigger needs shell access to the host, which is a strictly +// higher bar than the step-up passkey gate that guards /tools — an update +// is not a thing to expose to anything reachable over the network. // - It does not fetch code. Nothing here talks to a release server, a // registry, or GitHub. The new version is whatever is in the working tree // the operator points it at, which he pulled himself. Downloading and @@ -32,13 +36,18 @@ // // # The order of operations, and why // -// Apply is: health-check the CURRENT daemon → build → test → snapshot → install -// → restart → health-check → rollback on any failure. +// Apply is: health-check the CURRENT daemon → snapshot → build → test → +// install → restart → health-check → rollback on any failure. // // The first health check is not ceremony. If she is already not answering, a // failed update and a broken box are indistinguishable afterwards, and the // rollback has nothing to prove itself against — so Apply refuses to start. // +// The snapshot comes before the build, not after, and the comment in apply.go +// spells out why: `make build` writes into the working tree, which on the +// docker deployment IS the install dir, so a snapshot taken after it would +// snapshot the new artifacts. +// // Build and test run BEFORE anything is written to the install dir, so a broken // tree costs nothing but time. Install is per-file write-temp-then-rename, so a // crash mid-install leaves whole files, not half ones. @@ -49,6 +58,24 @@ // a migration, and does not need the update to have gotten far enough to leave // a working anything behind. // +// # What the snapshot has to cover +// +// A rollback is only real if it puts back the thing the restart command +// deploys. For a bare-metal layout that is the binaries in InstallDir. For the +// docker layout it is not: the image is built from the source tree, and the +// host binaries never enter it. Restoring binaries there rebuilds the same bad +// image and burns a second health timeout proving it. So a deployment that +// rebuilds from source must say how the source is put back +// (Config.SourceRollback), and one that cannot say is refused by Validate +// rather than discovering it during the one rollback that mattered. +// +// # What an update is not +// +// It is not turn-safe. Nothing quiesces the daemon first: the restart kills the +// process mid-utterance if one is in flight. The model swap in +// internal/phraser drains, because a swap is a routine operation on a running +// box; an update is a deliberate restart and the operator picked the moment. +// // # What is out of scope on purpose // // The database is not snapshotted or rolled back. It is encrypted, live, and @@ -84,6 +111,26 @@ var ( // so the previous snapshot was restored. Wraps the underlying failure. ErrRolledBack = errors.New("update: rolled back") + // ErrSourceRollback — the deployment rebuilds from source, and nothing in + // the config says how to put the source back. Refused at Validate: see + // Config.SourceRollback. + ErrSourceRollback = errors.New("update: this deployment rebuilds from source and has no way to roll the source back") + + // ErrDirtyTree — source_rollback is "git" and the working tree has + // uncommitted changes, so the recorded commit does not describe what is + // deployed and a checkout would throw work away. Refused before anything is + // built. + ErrDirtyTree = errors.New("update: the source tree has uncommitted changes — commit or stash them first") + + // ErrRootOnHisTree — running as root over a tree owned by somebody else. + // Refused: Verify runs `make build` and `make test` in SourceDir, and as + // root that leaves root-owned binaries, object files and a build cache in + // his working tree. His next ordinary `make` then fails, so one root apply + // breaks the normal build. This fires easily, because the documented health + // socket lives under a root-only directory and sudo is the obvious way past + // that. + ErrRootOnHisTree = errors.New("update: refusing to build someone else's tree as root — it would leave root-owned artifacts and break his next make") + // ErrRollbackFailed — the worst case: the new build failed AND the restore // did not bring her back. The operator has to fix the box by hand; the // snapshot directory is named in the result so he knows what to copy. @@ -105,18 +152,47 @@ type Config struct { InstallDir string `json:"install_dir"` // SnapshotDir — where the pre-install copies live. Must not be inside - // InstallDir: a restore reading from a directory the install is writing to - // is not a restore. + // InstallDir or SourceDir: a restore reading from a directory the install is + // writing to is not a restore, and a snapshot dir inside the source tree + // lands in the docker build context and in whatever make and git do there. SnapshotDir string `json:"snapshot_dir"` + // SourceRollback — how the SOURCE is put back when the deployment rebuilds + // from it. "" means it is not, which is only valid when the built binaries + // are what gets deployed. + // + // This exists because of what a rollback has to undo, which is not always + // the binaries. When RestartCmd is `docker compose up -d --build`, the image + // is built by the Dockerfile from cmd/ and internal/, and the host binaries + // are excluded by .dockerignore. Restoring them then restores bytes nothing + // reads: the restart rebuilds the same bad image from the same bad source, + // and the box stays down through two health timeouts for no reason. + // + // "git" makes the source part of the snapshot: the commit is recorded before + // the update and a rollback checks it back out before restarting. It + // requires a clean tree, because a recorded commit does not describe a dirty + // one and a forced checkout would throw uncommitted work away. + // + // Validate refuses a build-from-source deployment (SourceDir == InstallDir) + // that leaves this empty, rather than letting the operator find out during + // the one rollback he needed. + SourceRollback string `json:"source_rollback,omitempty"` + // Binaries — the artifact names to snapshot and install, relative to // SourceDir (built) and InstallDir (deployed). Listed explicitly rather than // globbed so a stray file in the tree never gets deployed. Binaries []string `json:"binaries"` // ConfigFiles — extra files to snapshot alongside the binaries, relative to - // InstallDir. Snapshotted, never overwritten by an install: the operator's - // config is not something an update gets to replace. + // InstallDir. Snapshotted and never written back: not by an install, and + // not by a rollback either. The operator's config is not something an update + // gets to replace, and a rollback that reverted it would silently undo every + // edit since the last apply. The copies are in the snapshot dir if he wants + // one back. + // + // The exception is source_rollback "git": a checkout moves every tracked + // file, config included. That is the same rollback the deployment needs to + // work at all, so on that shape a config edit belongs in a commit. ConfigFiles []string `json:"config_files,omitempty"` // RestartCmd — how this deployment restarts mavend, e.g. @@ -156,6 +232,17 @@ func (c Config) Validate() error { if within(c.SnapshotDir, c.InstallDir) { return fmt.Errorf("update: snapshot_dir %q is inside install_dir %q — a restore must not read from what the install writes", c.SnapshotDir, c.InstallDir) } + if within(c.SnapshotDir, c.SourceDir) { + return fmt.Errorf("update: snapshot_dir %q is inside source_dir %q — snapshots would land in the build context, and in whatever make and git do to that tree", c.SnapshotDir, c.SourceDir) + } + switch c.SourceRollback { + case "", "git": + default: + return fmt.Errorf("update: source_rollback %q is not a thing — use \"git\" or leave it out", c.SourceRollback) + } + if c.buildsFromSource() && c.SourceRollback == "" { + return fmt.Errorf("%w: source_dir and install_dir are both %q, so the restart deploys the tree and a restore of the binaries would undo nothing. Set \"source_rollback\": \"git\", or split the layout so install_dir holds what actually runs", ErrSourceRollback, c.SourceDir) + } if len(c.Binaries) == 0 { return errors.New("update: binaries is empty — nothing to install") } @@ -186,6 +273,13 @@ func (c Config) withDefaults() Config { return c } +// buildsFromSource — the deployment whose restart command rebuilds from the +// tree, which is what SourceDir == InstallDir means in practice (install is a +// no-op copy and the artifacts that matter are produced inside the image). +func (c Config) buildsFromSource() bool { + return filepath.Clean(c.SourceDir) == filepath.Clean(c.InstallDir) +} + func (c Config) healthTimeout() time.Duration { return time.Duration(c.HealthTimeoutSec) * time.Second } @@ -234,6 +328,9 @@ type Updater struct { health HealthCheck log Logger now func() time.Time + // ids reports the running euid and the owner of a directory. Injected so + // the root-build refusal is testable without a second account. + ids func(dir string) (int, uint32, error) } // New builds an Updater. Every seam has a real default; the tests replace them. @@ -248,6 +345,7 @@ func New(cfg Config, opts ...Option) (*Updater, error) { health: DialHealth, log: func(string, ...any) {}, now: time.Now, + ids: realIDs, } for _, o := range opts { o(u) @@ -266,5 +364,10 @@ func WithClock(f func() time.Time) Option { return func(u *Updater) { u.now = f } } +// WithIDs replaces the euid/owner lookup behind the root-build refusal. +func WithIDs(f func(dir string) (int, uint32, error)) Option { + return func(u *Updater) { u.ids = f } +} + // Snapshots lists what is available to roll back to, newest first. func (u *Updater) Snapshots() ([]Snapshot, error) { return u.store.List() } diff --git a/internal/update/update_test.go b/internal/update/update_test.go index 6f72f68..f5193af 100644 --- a/internal/update/update_test.go +++ b/internal/update/update_test.go @@ -32,11 +32,24 @@ type fakeBox struct { healthErrs int // remaining failures to serve healthy bool healthChecks int + // healthFn overrides the scripted behaviour entirely, for tests whose + // verdict depends on what the last restart actually deployed. + healthFn func() error // deployedAtRestart records the installed bytes each time restart runs, so a // test can prove the rollback put the old bytes back BEFORE restarting. deployedAtRestart []string + // builtFromSource records the source the restart would have built an image + // from, each time it runs. + builtFromSource []string ran []string + + // The git half, for the deployment whose restart rebuilds from the tree. + // commit is HEAD; byCommit is what each commit's source says; dirty makes + // `git status --porcelain` report uncommitted work. + commit string + byCommit map[string]string + dirty bool } func newFakeBox(t *testing.T) *fakeBox { @@ -52,7 +65,11 @@ func newFakeBox(t *testing.T) *fakeBox { write(t, filepath.Join(root, "install", "mavend.json"), `{"tick_interval":"60s"}`) // The source tree already contains a stale binary; `make build` overwrites it. write(t, filepath.Join(root, "src", "mavend"), "STALE") - return &fakeBox{t: t, root: root, newBytes: "NEW-BUILD", healthy: true} + return &fakeBox{ + t: t, root: root, newBytes: "NEW-BUILD", healthy: true, + commit: "cafebabecafebabecafebabecafebabecafebabe", + byCommit: map[string]string{}, + } } func (b *fakeBox) cfg() Config { @@ -86,19 +103,41 @@ func (b *fakeBox) run(ctx context.Context, dir string, argv []string) (string, e } return "ok", nil case "git rev-parse HEAD": - return "cafebabecafebabecafebabecafebabecafebabe\n", nil + return b.commit + "\n", nil + case "git status --porcelain": + if b.dirty { + return " M internal/router/router.go\n", nil + } + return "", nil case "restart-the-thing": b.deployedAtRestart = append(b.deployedAtRestart, read(b.t, filepath.Join(b.root, "install", "mavend"))) + // The docker shape: the restart rebuilds the image from the tree, so + // what it deploys is the source, not any binary on the host. + if src, err := os.ReadFile(filepath.Join(b.root, "src", "source.go")); err == nil { + b.builtFromSource = append(b.builtFromSource, string(src)) + } if b.restartErr != nil { return "no such container", b.restartErr } return "restarted", nil } + if len(argv) == 4 && argv[0] == "git" && argv[1] == "checkout" && argv[2] == "--force" { + content, ok := b.byCommit[argv[3]] + if !ok { + return "error: pathspec did not match", errors.New("exit status 1") + } + b.commit = argv[3] + write(b.t, filepath.Join(b.root, "src", "source.go"), content) + return "HEAD is now at " + argv[3], nil + } return "", errors.New("unexpected command: " + strings.Join(argv, " ")) } func (b *fakeBox) health(ctx context.Context, socket string) error { b.healthChecks++ + if b.healthFn != nil { + return b.healthFn() + } if b.healthErrs > 0 { b.healthErrs-- return errors.New("connection refused") diff --git a/internal/update/verify.go b/internal/update/verify.go index 26b0511..44a402a 100644 --- a/internal/update/verify.go +++ b/internal/update/verify.go @@ -2,8 +2,12 @@ package update import ( "context" + "errors" "fmt" + "os" + "syscall" "time" + "unicode/utf8" ) // Verification is "does this tree build and does it pass its own tests", run @@ -34,6 +38,9 @@ type Step struct { // Verify runs the build and the test suite in SourceDir. func (u *Updater) Verify(ctx context.Context) ([]Step, error) { + if err := u.refuseRootBuild(); err != nil { + return nil, err + } ctx, cancel := context.WithTimeout(ctx, u.cfg.verifyTimeout()) defer cancel() var steps []Step @@ -55,11 +62,43 @@ func (u *Updater) Verify(ctx context.Context) ([]Step, error) { return steps, nil } +// refuseRootBuild stops a sudo'd apply from building in a tree it does not own. +// +// The seam is injected so the tests can drive both sides without a second uid. +func (u *Updater) refuseRootBuild() error { + uid, owner, err := u.ids(u.cfg.SourceDir) + if err != nil || uid != 0 || owner == 0 { + return nil // not root, or root's own tree, or we cannot tell + } + return fmt.Errorf("%w: %s is owned by uid %d", ErrRootOnHisTree, u.cfg.SourceDir, owner) +} + +// realIDs — the running uid and the owner of dir. Split out for the tests. +func realIDs(dir string) (uid int, owner uint32, err error) { + fi, err := os.Stat(dir) + if err != nil { + return 0, 0, err + } + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + return 0, 0, errors.New("update: cannot read directory ownership") + } + return os.Geteuid(), st.Uid, nil +} + // tail keeps the last n bytes — a failing `make test` prints far more than is // useful, and the failure is always at the end. +// +// The cut is nudged forward to a rune boundary. Russian test names and fixture +// strings are the common case in this tree, and a slice landing mid-rune starts +// the log with a replacement character. func tail(s string, n int) string { if len(s) <= n { return s } - return "…" + s[len(s)-n:] + cut := len(s) - n + for cut < len(s) && !utf8.RuneStart(s[cut]) { + cut++ + } + return "…" + s[cut:] } diff --git a/models/stt b/models/stt new file mode 120000 index 0000000..b983fa3 --- /dev/null +++ b/models/stt @@ -0,0 +1 @@ +/home/kami/apps/Maven/models/stt \ No newline at end of file diff --git a/models/tts b/models/tts new file mode 120000 index 0000000..66782fb --- /dev/null +++ b/models/tts @@ -0,0 +1 @@ +/home/kami/apps/Maven/models/tts \ No newline at end of file