From d1be303503bda1761a225141eb9cc9ef6cd7e4c6 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Fri, 14 Aug 2026 20:14:58 +0300 Subject: [PATCH 1/3] refactor: dedup helpers and drop narrator comments Anti-slop code pass. Extract resolveTargets (apply.go), snapshotCmd (qmp.go), progressNum (progressbar.go); delete joinSpace for strings.Join. LocalImages now returns an error instead of swallowing a non-ENOENT isos/ read, propagated to Images and pruneImages. Trim InstallData's misplaced Install docstring and the recipe-installer comments that restated the command below them. --- internal/cli/cli.go | 14 +----- internal/core/apply.go | 50 ++++++++++--------- internal/core/image.go | 16 ++++-- internal/core/images.go | 5 +- internal/core/prune.go | 7 ++- internal/installer/build.go | 6 --- internal/qemu/qmp.go | 28 +++++------ .../recipes/bundled/docker/install-arch.sh | 1 - .../recipes/bundled/docker/install-debian.sh | 4 -- .../recipes/bundled/docker/install-fedora.sh | 1 - .../bundled/tailscale/install-debian.sh | 1 - .../bundled/tailscale/install-fedora.sh | 1 - internal/recipes/scaffold.go | 2 - internal/tui/form.go | 4 +- internal/tui/list.go | 3 +- internal/tui/progressbar.go | 26 +++++----- mcp/tests/test_guards.py | 4 +- 17 files changed, 79 insertions(+), 94 deletions(-) diff --git a/internal/cli/cli.go b/internal/cli/cli.go index e66b8fc0..cf9f342c 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -18,6 +18,7 @@ import ( "bytes" "fmt" "io" + "strings" "time" "github.com/alecthomas/kong" @@ -165,18 +166,7 @@ func commandPath(ctx *kong.Context) string { parts = append(parts, t.Command.Name) } } - return joinSpace(parts) -} - -func joinSpace(parts []string) string { - out := "" - for i, p := range parts { - if i > 0 { - out += " " - } - out += p - } - return out + return strings.Join(parts, " ") } // Parse turns argv (excluding the "stoat" program name) into an Args, or a diff --git a/internal/core/apply.go b/internal/core/apply.go index 55179d3d..8f91a296 100644 --- a/internal/core/apply.go +++ b/internal/core/apply.go @@ -96,18 +96,9 @@ func PlanApply(name string, opts ApplyOpts) ([]ApplyPlan, error) { return nil, err } - targets := v.Recipes - if len(opts.Only) > 0 { - have := make(map[string]bool, len(v.Recipes)) - for _, r := range v.Recipes { - have[r] = true - } - for _, o := range opts.Only { - if !have[o] { - return nil, fmt.Errorf("%w: recipe %q is not one of %s's recipes", ErrRecipeNotApplicable, o, v.Name) - } - } - targets = opts.Only + targets, err := resolveTargets(v, opts.Only) + if err != nil { + return nil, err } explicit := make(map[string]bool, len(opts.Only)) for _, o := range opts.Only { @@ -129,24 +120,35 @@ func PlanApply(name string, opts ApplyOpts) ([]ApplyPlan, error) { return plan, nil } +// resolveTargets validates only against v's own recipes and returns the +// target list: only itself when non-empty, v.Recipes otherwise. Shared by +// PlanApply and applyLocked so a dry-run plan always targets the same +// recipes a real Apply would. +func resolveTargets(v *config.VM, only []string) ([]string, error) { + if len(only) == 0 { + return v.Recipes, nil + } + have := make(map[string]bool, len(v.Recipes)) + for _, r := range v.Recipes { + have[r] = true + } + for _, o := range only { + if !have[o] { + return nil, fmt.Errorf("%w: recipe %q is not one of %s's recipes", ErrRecipeNotApplicable, o, v.Name) + } + } + return only, nil +} + // applyLocked is Apply's body, run while Apply holds name's provision lock. func applyLocked(ctx context.Context, v *config.VM, opts ApplyOpts) error { if !qemu.Running(v) { return fmt.Errorf("%w: %s", ErrNotRunning, v.Name) } - targets := v.Recipes - if len(opts.Only) > 0 { - have := make(map[string]bool, len(v.Recipes)) - for _, r := range v.Recipes { - have[r] = true - } - for _, o := range opts.Only { - if !have[o] { - return fmt.Errorf("%w: recipe %q is not one of %s's recipes", ErrRecipeNotApplicable, o, v.Name) - } - } - targets = opts.Only + targets, err := resolveTargets(v, opts.Only) + if err != nil { + return err } if len(targets) == 0 { // Nothing to run is not an error. An explicit Apply on a VM with no diff --git a/internal/core/image.go b/internal/core/image.go index 5536e353..6b51d429 100644 --- a/internal/core/image.go +++ b/internal/core/image.go @@ -21,10 +21,13 @@ import ( // file would show up as selectable and let a VM build on a truncated // image. Downloads run minutes with no cancel key, so aborted ones, and // their .part files, are common. -func LocalImages() []string { +func LocalImages() ([]string, error) { entries, err := os.ReadDir(filepath.Join(config.Root(), "isos")) if err != nil { - return nil + if os.IsNotExist(err) { + return nil, nil + } + return nil, err } var out []string for _, e := range entries { @@ -32,7 +35,7 @@ func LocalImages() []string { out = append(out, e.Name()) } } - return out + return out, nil } // MatchLocal reports which local file, if any, satisfies catalog entry e. @@ -86,7 +89,7 @@ func MatchLocal(e iso.Entry, files []string) string { // image is everything Create needs to know about the file a Spec named. type image struct { - abs string // absolute path to the file on disk + abs string rel string // bare name under isos/; "" when abs lives elsewhere entry *iso.Entry // nil for BYO osName string @@ -126,7 +129,10 @@ func resolveImage(spec string) (image, error) { return image{abs: spec, backend: backend, osName: osName}, nil } - files := LocalImages() + files, err := LocalImages() + if err != nil { + return image{}, err + } for _, e := range iso.Catalog() { if e.ID != spec { continue diff --git a/internal/core/images.go b/internal/core/images.go index d7c2b2cc..25a0d0f5 100644 --- a/internal/core/images.go +++ b/internal/core/images.go @@ -60,7 +60,10 @@ func fileSize(file string) (int64, bool) { // resolveImage, so Images can never disagree with what a Spec naming a catalog // ID resolves to. func Images() ([]CatalogImage, error) { - files := LocalImages() + files, err := LocalImages() + if err != nil { + return nil, err + } matched := map[string]bool{} var out []CatalogImage diff --git a/internal/core/prune.go b/internal/core/prune.go index addc1270..91bb41b2 100644 --- a/internal/core/prune.go +++ b/internal/core/prune.go @@ -302,8 +302,13 @@ func pruneImages(dryRun bool) ([]PruneItem, error) { return nil, err } + files, err := LocalImages() + if err != nil { + return nil, err + } + var out []PruneItem - for _, f := range LocalImages() { + for _, f := range files { abs, err := filepath.Abs(filepath.Join(config.Root(), "isos", f)) if err != nil { continue diff --git a/internal/installer/build.go b/internal/installer/build.go index a2d94bbf..69c9c1f3 100644 --- a/internal/installer/build.go +++ b/internal/installer/build.go @@ -62,12 +62,6 @@ func (e *BuildError) Error() string { func (e *BuildError) Unwrap() error { return e.Err } -// Install copies srcPath into destDir as an executable, creating destDir if -// needed. -// -// It writes to a temp file and renames, rather than opening the destination -// for write. Replacing a running binary that way is atomic and avoids -// ETXTBSY, the error a reinstall over a running stoat would otherwise hit. // InstallData creates ~/.stoat subdirectories and copies bundled recipes from // the repo. Safe to call on reinstall: existing user files are not overwritten. func InstallData(repoDir, home string) error { diff --git a/internal/qemu/qmp.go b/internal/qemu/qmp.go index c0395dfa..f8dcb184 100644 --- a/internal/qemu/qmp.go +++ b/internal/qemu/qmp.go @@ -145,35 +145,31 @@ func (q *qmp) hmpChecked(cmd string) error { return nil } -// SnapshotSave takes a live snapshot of a RUNNING VM, capturing RAM as well as -// disk, so restoring resumes execution rather than rebooting. -func SnapshotSave(v *config.VM, tag string) error { +// snapshotCmd dials QMP, runs an HMP command that prints nothing on success, +// and closes the session. savevm, loadvm and delvm all share this shape. +func snapshotCmd(v *config.VM, hmp string) error { q, err := dialQMP(v) if err != nil { return err } defer q.Close() - return q.hmpChecked("savevm " + tag) + return q.hmpChecked(hmp) +} + +// SnapshotSave takes a live snapshot of a RUNNING VM, capturing RAM as well as +// disk, so restoring resumes execution rather than rebooting. +func SnapshotSave(v *config.VM, tag string) error { + return snapshotCmd(v, "savevm "+tag) } // SnapshotLoad restores a RUNNING VM to a snapshot. func SnapshotLoad(v *config.VM, tag string) error { - q, err := dialQMP(v) - if err != nil { - return err - } - defer q.Close() - return q.hmpChecked("loadvm " + tag) + return snapshotCmd(v, "loadvm "+tag) } // SnapshotDelete removes a snapshot from a RUNNING VM's disk. func SnapshotDelete(v *config.VM, tag string) error { - q, err := dialQMP(v) - if err != nil { - return err - } - defer q.Close() - return q.hmpChecked("delvm " + tag) + return snapshotCmd(v, "delvm "+tag) } // SnapshotInfo returns the raw "info snapshots" text from a RUNNING VM. diff --git a/internal/recipes/bundled/docker/install-arch.sh b/internal/recipes/bundled/docker/install-arch.sh index 6c8fd2a9..109b49a5 100755 --- a/internal/recipes/bundled/docker/install-arch.sh +++ b/internal/recipes/bundled/docker/install-arch.sh @@ -8,7 +8,6 @@ pacman -Sy --noconfirm docker docker-compose systemctl enable docker systemctl start docker -# Wait for daemon i=0 while [ $i -lt 30 ]; do docker info >/dev/null 2>&1 && break diff --git a/internal/recipes/bundled/docker/install-debian.sh b/internal/recipes/bundled/docker/install-debian.sh index 053e6390..3f1719ab 100755 --- a/internal/recipes/bundled/docker/install-debian.sh +++ b/internal/recipes/bundled/docker/install-debian.sh @@ -5,17 +5,14 @@ set -e export DEBIAN_FRONTEND=noninteractive -# Install prerequisites apt-get update apt-get install -y ca-certificates curl gnupg -# Add Docker's official GPG key install -m 0755 -d /etc/apt/keyrings curl -fsSL https://download.docker.com/linux/$(. /etc/os-release && echo "$ID")/gpg | \ gpg --dearmor -o /etc/apt/keyrings/docker.gpg chmod a+r /etc/apt/keyrings/docker.gpg -# Add the repository echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.gpg] \ https://download.docker.com/linux/$(. /etc/os-release && echo "$ID") \ $(. /etc/os-release && echo "$VERSION_CODENAME") stable" | \ @@ -27,7 +24,6 @@ apt-get install -y docker-ce docker-ce-cli containerd.io docker-compose-plugin systemctl enable docker systemctl start docker -# Wait for daemon i=0 while [ $i -lt 30 ]; do docker info >/dev/null 2>&1 && break diff --git a/internal/recipes/bundled/docker/install-fedora.sh b/internal/recipes/bundled/docker/install-fedora.sh index 7c281fde..13adfa23 100755 --- a/internal/recipes/bundled/docker/install-fedora.sh +++ b/internal/recipes/bundled/docker/install-fedora.sh @@ -13,7 +13,6 @@ dnf install -y docker-ce docker-ce-cli containerd.io docker-compose-plugin systemctl enable docker systemctl start docker -# Wait for daemon i=0 while [ $i -lt 30 ]; do docker info >/dev/null 2>&1 && break diff --git a/internal/recipes/bundled/tailscale/install-debian.sh b/internal/recipes/bundled/tailscale/install-debian.sh index a78461db..e1793f62 100755 --- a/internal/recipes/bundled/tailscale/install-debian.sh +++ b/internal/recipes/bundled/tailscale/install-debian.sh @@ -9,7 +9,6 @@ set -e export DEBIAN_FRONTEND=noninteractive -# Use Tailscale's official install script curl -fsSL https://tailscale.com/install.sh | sh systemctl enable tailscaled diff --git a/internal/recipes/bundled/tailscale/install-fedora.sh b/internal/recipes/bundled/tailscale/install-fedora.sh index 5d3fdd18..e3e1f25d 100755 --- a/internal/recipes/bundled/tailscale/install-fedora.sh +++ b/internal/recipes/bundled/tailscale/install-fedora.sh @@ -7,7 +7,6 @@ # you the one command to run yourself. set -e -# Use Tailscale's official install script curl -fsSL https://tailscale.com/install.sh | sh systemctl enable tailscaled diff --git a/internal/recipes/scaffold.go b/internal/recipes/scaffold.go index e2ec0fdf..74b05e28 100644 --- a/internal/recipes/scaffold.go +++ b/internal/recipes/scaffold.go @@ -90,13 +90,11 @@ func New(name, osName, _ string) (string, error) { return "", err } - // Write recipe.toml manifest := fmt.Sprintf(manifestTemplate, name, osName) if err := os.WriteFile(filepath.Join(recipeDir, "recipe.toml"), []byte(manifest), 0o644); err != nil { return "", err } - // Write install.sh setup, install := osSetup(osName) script := fmt.Sprintf(shellTemplate, name, osName, setup, install) if err := os.WriteFile(filepath.Join(recipeDir, "install.sh"), []byte(script), 0o755); err != nil { diff --git a/internal/tui/form.go b/internal/tui/form.go index 26163e68..895d95ad 100644 --- a/internal/tui/form.go +++ b/internal/tui/form.go @@ -163,8 +163,8 @@ var matchLocalImage = core.MatchLocal // core.CatalogImage carries an ID but not the richer iso.Entry fields // (Variant, SSHUser) the picker's rows and resolvedSSHUser need, so each row // is paired with its iso.Entry by ID here. The error core.Images returns is -// not surfaced: LocalImages, underneath it, already treats an unreadable -// isos/ dir as "no local files" rather than an error. +// discarded: the picker has no error channel, so a failed isos/ read shows an +// empty list, not a message. func buildImages() []imageOption { imgs, _ := core.Images() catalog := map[string]iso.Entry{} diff --git a/internal/tui/list.go b/internal/tui/list.go index a682bebe..b8ef47e9 100644 --- a/internal/tui/list.go +++ b/internal/tui/list.go @@ -274,6 +274,7 @@ func (m model) viewList() string { ci = m.cloudInit[cur.Name] } box = joinAccess(box, accessBox(cur, ci, m.ciProg, m.width), m.width) + sshAvailable := cur != nil && cur.State == core.StateRunning // column holds every piece to appContentWidth and stacks them left- // aligned. Without it, the box, the status line, and the footer would @@ -291,8 +292,6 @@ func (m model) viewList() string { parts = append(parts, l) } parts = append(parts, warnStyle.Render(m.status)) - v := m.current() - sshAvailable := v != nil && v.State == core.StateRunning parts = append(parts, renderFooter(listHelp{sshAvailable: sshAvailable}, m.width, m.showHelp)) return column(appContentWidth, parts...) } diff --git a/internal/tui/progressbar.go b/internal/tui/progressbar.go index 4dce081b..95c56f03 100644 --- a/internal/tui/progressbar.go +++ b/internal/tui/progressbar.go @@ -17,16 +17,20 @@ const ( provLabelMax = 18 ) -// progressLabel renders p as the bar plus a number: "N/M" when the source -// counted packages, "NN%" when it only gave a fraction. bar() already draws -// the track with the theme's colors; this adds the number and, when there -// is room, the package name. -func progressLabel(p Progress, width int) string { - num := fmt.Sprintf("%3.0f%%", p.Frac*100) +// progressNum renders p's count as "N/M" when the source counted packages, +// "NN%" when it only gave a fraction. +func progressNum(p Progress) string { if p.Total > 0 { - num = fmt.Sprintf("%d/%d", p.Done, p.Total) + return fmt.Sprintf("%d/%d", p.Done, p.Total) } - out := bar(p.Frac, width) + " " + dimStyle.Render(num) + return fmt.Sprintf("%3.0f%%", p.Frac*100) +} + +// progressLabel renders p as the bar plus a number. bar() already draws the +// track with the theme's colors; this adds the number and, when there is +// room, the package name. +func progressLabel(p Progress, width int) string { + out := bar(p.Frac, width) + " " + dimStyle.Render(progressNum(p)) if l := p.Label; l != "" { out += dimStyle.Render(" · " + ansi.Truncate(l, provLabelMax, "…")) } @@ -66,11 +70,7 @@ func renderProgressPanel(phase string, p Progress, hasProg bool, elapsed time.Du var f fields f.row("", "phase", dimStyle.Render(phase)) if hasProg { - num := fmt.Sprintf("%3.0f%%", p.Frac*100) - if p.Total > 0 { - num = fmt.Sprintf("%d/%d", p.Done, p.Total) - } - f.row("", "", bar(p.Frac, detailBarWidth)+" "+dimStyle.Render(num)) + f.row("", "", bar(p.Frac, detailBarWidth)+" "+dimStyle.Render(progressNum(p))) if p.Label != "" { f.row("", "package", p.Label) } diff --git a/mcp/tests/test_guards.py b/mcp/tests/test_guards.py index 10e1bd1b..a7b97f8b 100644 --- a/mcp/tests/test_guards.py +++ b/mcp/tests/test_guards.py @@ -299,8 +299,8 @@ def test_rate_limiter_allows_up_to_capacity(): # A tiny nonzero refill rate, not 0.0: RateLimiter.check divides by # refill_per_second when it needs to report a wait time, so an actual # 0.0 raises ZeroDivisionError instead of GuardRejection once the bucket - # is empty. See the report for this finding; a still-positive rate this - # small is effectively "never refills" within any of these tests. + # is empty. A still-positive rate this small is effectively "never + # refills" within any of these tests. rl = RateLimiter(capacity=3, refill_per_second=1e-9) rl.check("create", now=0.0) rl.check("create", now=0.0) From 7b303776d99b6793b165ed466ef7412400477c43 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Fri, 14 Aug 2026 20:15:00 +0300 Subject: [PATCH 2/3] docs: plain-language pass Anti-slop prose pass across 13 docs. Remove contrast constructions, drop load-bearing/additionally/crucially, convert the recipe-fixes spec headings to sentence case. --- docs/README.md | 9 ++- docs/concepts/access-and-auth.md | 11 ++-- docs/concepts/data-root.md | 5 +- docs/concepts/modes-and-backends.md | 11 ++-- docs/concepts/networking-and-sharing.md | 3 - docs/design/core-api.md | 4 +- docs/design/guest-subsystem.md | 6 +- docs/design/tui-migration-draft.md | 2 +- docs/recipe-authoring-spec.md | 5 +- docs/reference/cli.md | 4 +- docs/reference/json.md | 3 +- .../2026-08-10-recipe-system-fixes-design.md | 62 +++++++++---------- docs/troubleshooting.md | 8 +-- 13 files changed, 61 insertions(+), 72 deletions(-) diff --git a/docs/README.md b/docs/README.md index fc0ea6b3..6ca4cdb6 100644 --- a/docs/README.md +++ b/docs/README.md @@ -5,11 +5,10 @@ Alpine-first, and it manages each VM through a small `vm.toml` file instead of a system daemon or a virtualization stack like libvirt. stoat is for developers who want a disposable Linux VM (or a handful of -persistent ones) on their own machine, driven entirely from the terminal. What -sets it apart is what it *doesn't* need: no libvirt, no background daemon, no -database, just a single Go binary that shells out to `qemu-system-x86_64`, -`qemu-img`, and `ssh`, and a data root of plain files under `~/.stoat` (or -`$STOAT_HOME`). +persistent ones) on their own machine, driven entirely from the terminal. +stoat needs no libvirt, no background daemon, and no database, just a single +Go binary that shells out to `qemu-system-x86_64`, `qemu-img`, and `ssh`, and +a data root of plain files under `~/.stoat` (or `$STOAT_HOME`). New here? Start with [Installation](getting-started/installation.md), then [Your first VM](getting-started/first-vm.md). diff --git a/docs/concepts/access-and-auth.md b/docs/concepts/access-and-auth.md index 914f601d..68e273bd 100644 --- a/docs/concepts/access-and-auth.md +++ b/docs/concepts/access-and-auth.md @@ -85,20 +85,19 @@ builds a very similar argument list: same host-key options, same `-i id_stoat`, same `root@127.0.0.1` target for live VMs, but deliberately **omits `BatchMode=yes`**. `Args` (used for unattended provisioning) needs SSH to fail immediately rather than block waiting for input that will never come. -The interactive path hands the terminal to a real, attended `ssh` process, so -if key auth doesn't work, for instance a disk-mode VM that was installed +The interactive path hands the terminal to a real, attended `ssh` process. +If key auth doesn't work, for instance a disk-mode VM that was installed manually and never had the key provisioned, SSH is free to fall back to prompting for a password at the terminal, the same as running `ssh` by hand. ## Why host key checking is off Both paths set `StrictHostKeyChecking=no` and `UserKnownHostsFile=/dev/null`. -This is deliberate, not a shortcut: live Alpine VMs are rebuilt by stoat +This is deliberate. live Alpine VMs are rebuilt by stoat constantly, and even with the stable guest host key described above, disk and cloud VMs generate their own host keys independently. Strict checking would mean a stale or mismatched `known_hosts` entry breaking a connection to a VM -stoat just built, the kind of false alarm host-key checking exists to avoid, -not the kind it's meant to catch. +stoat just built: a false alarm, not an actual key mismatch. ## Passwords: console only, never over SSH @@ -136,7 +135,7 @@ the detail screen: and in the log line written when the VM starts, which you can read with `stoat logs`. -### Why a fixed default is not a compromise +### Why a fixed default is safe You end up at that console precisely when SSH *isn't* working, which is the worst possible moment to go and look a credential up. And it is no weaker diff --git a/docs/concepts/data-root.md b/docs/concepts/data-root.md index 318b126e..63df1fe9 100644 --- a/docs/concepts/data-root.md +++ b/docs/concepts/data-root.md @@ -40,7 +40,7 @@ than an error. └── meta-data ``` -A couple of things worth calling out about that tree: +Two facts about that tree: - `isos/` holds both plain ISOs (Alpine) and downloaded cloud images (Ubuntu/Debian/Fedora/Arch `.qcow2`/`.img` files). A `cloud` VM's @@ -81,8 +81,7 @@ Each VM directory holds one `vm.toml`. Every field: `vm.toml` is a plain TOML file and nothing stops you from editing it directly while the VM is stopped: stoat re-reads it fresh every time, there's no -cache to invalidate. That said, some fields are much safer to touch than -others: +cache to invalidate. Some fields are safer to edit than others: - **Safe-ish**: `ram`, `cpus`, `share`, `recipes` (as long as the filenames still exist under `recipes/`), `sshuser`. diff --git a/docs/concepts/modes-and-backends.md b/docs/concepts/modes-and-backends.md index 0d56ce96..f827fed0 100644 --- a/docs/concepts/modes-and-backends.md +++ b/docs/concepts/modes-and-backends.md @@ -10,13 +10,10 @@ related but not the same thing: what its start logic checks before launching a VM. - **Backend** (`vm.toml`'s `backend` field) records how the create form picked recipes and provisioning at creation time. Once the VM exists, - nothing at runtime dispatches on this field: it's informational, not - load-bearing. + nothing at runtime dispatches on this field: it's informational. -If you only remember one thing from this page: **`disk` mode gives you no -SSH access until you install an OS yourself and tell stoat so.** That single -fact trips up more people than everything else in this tool combined, so it -gets its own section below. +**`disk` mode gives you no SSH access until you install an OS yourself and +tell stoat so.** See the section below. ## The three modes @@ -27,7 +24,7 @@ into RAM: the root filesystem is an Alpine initramfs overlay, and nothing about it is written anywhere persistent. Stop or reboot the VM and you get a clean slate next time. -What makes `live` mode useful rather than just a toy: every time you start +What makes `live` mode useful: every time you start one, stoat builds a small Alpine overlay tarball (an "apkovl") and hands it to the guest as a fake FAT disk over `-virtfs`/vvfat. That overlay bakes in: diff --git a/docs/concepts/networking-and-sharing.md b/docs/concepts/networking-and-sharing.md index c8bb0372..6b36fe95 100644 --- a/docs/concepts/networking-and-sharing.md +++ b/docs/concepts/networking-and-sharing.md @@ -63,9 +63,6 @@ Whether the share actually gets *mounted* inside the guest depends on mode: ## Sharing a binary built on the host: the musl/glibc trap -This is the single most confusing failure mode around the share feature, so -it gets called out explicitly. - If you build a binary on a typical glibc-based Linux host (Arch, Fedora, Ubuntu, Debian) and drop it into your shared directory expecting to run it straight from `/mnt/host` inside an **Alpine** guest (the default for `live` diff --git a/docs/design/core-api.md b/docs/design/core-api.md index 2d9e8537..34d98da7 100644 --- a/docs/design/core-api.md +++ b/docs/design/core-api.md @@ -157,7 +157,7 @@ The TUI keeps its prompt and passes the answer. The MCP server requires the agen ## 7. Convenience operations -These are the "quick VM-based testing" features that make stoat pleasant rather than merely functional. All are cheap because of what QEMU and qcow2 already give us. +These are the "quick VM-based testing" features that make stoat pleasant to use. All are cheap because of what QEMU and qcow2 already give us. | Operation | Signature | Why it earns a slot | |---|---|---| @@ -282,7 +282,7 @@ The mechanism that does work is `security_model=mapped-xattr`, which stores *"ui ### 10.3 MCP tool taxonomy -Three classes, annotated honestly and, crucially, enforced independently of whether the client honours the annotation. +Three classes, annotated honestly and enforced independently of whether the client honours the annotation. | Class | Tools | Annotations | Server-side rule | |---|---|---|---| diff --git a/docs/design/guest-subsystem.md b/docs/design/guest-subsystem.md index 295f61b4..3cd1b1ff 100644 --- a/docs/design/guest-subsystem.md +++ b/docs/design/guest-subsystem.md @@ -358,11 +358,11 @@ These are requirements the TUI never forced, and each one is a real change: **Declarative creation.** One `Create(Spec)` call, no multi-step form state. Everything the form asks for either has a default or is in the `Spec`. -**Recipe metadata becomes load-bearing.** §5's `# stoat:os` / `# stoat:requires`, enforced at selection, is *optional politeness* for a human who can read a black screen and *mandatory* for an agent that cannot. `Create` returns `ErrRecipeNotApplicable` rather than producing a VM that boots broken. This is the strongest argument for the departure from prior art in §2. +**Recipe metadata now gates VM creation.** §5's `# stoat:os` / `# stoat:requires`, enforced at selection, is *optional politeness* for a human who can read a black screen and *mandatory* for an agent that cannot. `Create` returns `ErrRecipeNotApplicable` rather than producing a VM that boots broken. This is the strongest argument for the departure from prior art in §2. **Cancellation.** Every operation takes a `context.Context`. Downloads and boot-waits are long; an agent that abandons a task must be able to stop them. Today `esc` leaves a download goroutine running (a known open item). -### 9.5 Concurrency: new, and load-bearing +### 9.5 Concurrency: new, and required once a second caller exists A TUI has one user doing one thing. An MCP server plus a TUI plus a CLI can act **at the same time**, and two current mechanisms are not safe under that: @@ -407,6 +407,6 @@ If any of these needs logic that is not already in `core`, the layering is wrong ## 10. Risks - **This is a real refactor**, touching `qemu`, `cloudinit`, `apkovl`, `recipes`, `iso` and `tui`. The mitigation is that behaviour must not change except where a bug is named here: existing tests are the contract, and any test that needs updating is a signal to stop and check rather than to edit. -- **`Backend.Prepare`'s call frequency differs by backend** (apkovl every boot; cloudinit once, ever). That asymmetry is load-bearing: a cloud VM whose seed is rebuilt would have its instance identity change under it. The interface hides it, which is right, but the implementations must be explicit about it and tested for it. +- **`Backend.Prepare`'s call frequency differs by backend** (apkovl every boot; cloudinit once, ever). A cloud VM whose seed is rebuilt would have its instance identity change under it. The interface hides it, which is right, but the implementations must be explicit about it and tested for it. - **`cloud-init schema` may be absent** on the host (Arch does not install cloud-init by default). Validation must degrade to "not checked", never to "assumed valid". - **The merge-semantics change is silent when wrong.** Packages that quietly stop being installed look like a recipe bug, not a merge bug. Needs a direct test. diff --git a/docs/design/tui-migration-draft.md b/docs/design/tui-migration-draft.md index 045622dc..39ba200b 100644 --- a/docs/design/tui-migration-draft.md +++ b/docs/design/tui-migration-draft.md @@ -440,7 +440,7 @@ original and is fine), `vmlist.go`'s delegate and every width constant, `app.go`'s `View`/`newView`/`renderModal` compositing, `list.go brokenReason`, `access.go`'s `shortenPath`/`joinAccess`. -**Every width constant is load-bearing and has a story:** `listWidth = 60` +**Every width constant has a story:** `listWidth = 60` (sized to a running row), `formContentWidth = 72` (sized to fit 80 columns), `byoFileWidth = 24`, `imageMetaWidth = 11` (sized to `"13 (trixie)"`), `modalSizeWidth = 9` (sized to `"~66.0 MiB"`), `accessWidth = 40`, diff --git a/docs/recipe-authoring-spec.md b/docs/recipe-authoring-spec.md index d71d038f..b2671926 100644 --- a/docs/recipe-authoring-spec.md +++ b/docs/recipe-authoring-spec.md @@ -20,9 +20,8 @@ and survives upgrades. named `mything.alpine.sh` and it appears in the picker for Alpine VMs. No code required. Any "creator" competes with `$EDITOR` on a path that already works. -So the question is not "can we add a creator" but "what does a creator do that -`vim ~/.stoat/recipes/x.alpine.sh` doesn't". Three honest candidates below, -smallest first. +The question is what a creator does that `vim ~/.stoat/recipes/x.alpine.sh` +doesn't. Candidates below, smallest first. --- diff --git a/docs/reference/cli.md b/docs/reference/cli.md index 60894080..b7218267 100644 --- a/docs/reference/cli.md +++ b/docs/reference/cli.md @@ -399,7 +399,7 @@ broken vm: /home/user/.stoat/vms/oldvm partial download: /home/user/.stoat/isos/ubuntu-24.04.iso.part ``` -`--broken` additionally considers VMs whose `vm.toml` won't parse for removal; `--images` additionally considers downloaded images no VM refers to. Without either, `prune` only reports partial downloads by default (broken VMs and orphaned images need to be asked for explicitly). Printing an identical list for the dry run and the real run is deliberate: the two are meant to be readable as the same thing, one with the deletions actually applied. +`--broken` also considers VMs whose `vm.toml` won't parse for removal; `--images` also considers downloaded images no VM refers to. Without either, `prune` only reports partial downloads by default (broken VMs and orphaned images need to be asked for explicitly). Printing an identical list for the dry run and the real run is deliberate: the two are meant to be readable as the same thing, one with the deletions actually applied. **Exit codes:** 0 on success, including "nothing to prune"; 1 if pruning fails. @@ -567,7 +567,7 @@ A usage error (2) always prints both the specific complaint and the full usage t ## Scripting - **`-q`, `--quiet`, `--no-interactive`** are three names for the same flag, present on every subcommand. Where it has an effect, it suppresses the "in-progress" chatter (`starting work...`, `provisioning work...`, ...); final results and all errors print regardless of this flag. -- **`rm`** additionally treats `--no-interactive`/`-q`/`--json` as "there is no one to answer a confirmation prompt": without `-y` it refuses rather than blocking on stdin. +- **`rm`** also treats `--no-interactive`/`-q`/`--json` as "there is no one to answer a confirmation prompt": without `-y` it refuses rather than blocking on stdin. - **`--json`** is the machine-readable mode: one JSON object per line on stdout, errors included, implying `--quiet` and never prompting. See [json.md](json.md) for the wire format. - **`NO_COLOR`** (any non-empty value) disables ANSI color in `ls`'s output. - Color is also **disabled automatically whenever stdout is not a terminal** (checked via `os.ModeCharDevice`), so piping `stoat ls` into `awk`, `grep`, or a file never carries escape codes even without setting `NO_COLOR`. Only `ls`'s `STATE` column is ever colored. diff --git a/docs/reference/json.md b/docs/reference/json.md index 4b863f42..55f4cda7 100644 --- a/docs/reference/json.md +++ b/docs/reference/json.md @@ -48,8 +48,7 @@ Four rules make it work: Rule 3 is not a preference. A consumer that must merge two pipes to reconstruct one result will eventually interleave them wrong, and a naive `subprocess` read of two pipes in sequence deadlocks when either buffer fills. -The error envelope is the answer to the question that was asked, not a -diagnostic. +The error envelope answers the question that was asked. ## The envelope diff --git a/docs/specs/2026-08-10-recipe-system-fixes-design.md b/docs/specs/2026-08-10-recipe-system-fixes-design.md index e091a73b..75b20418 100644 --- a/docs/specs/2026-08-10-recipe-system-fixes-design.md +++ b/docs/specs/2026-08-10-recipe-system-fixes-design.md @@ -1,4 +1,4 @@ -# Recipe System Fixes +# Recipe system fixes Status: **draft** @@ -6,7 +6,7 @@ Status: **draft** This spec addresses six architectural issues in the recipe system. Items 1 and the cloudinit v2 conversion are coupled; the rest can land independently. -## Priority Order +## Priority order 1. Delete v1 metadata parser + wire cloudinit to v2 scripts (coupled) 2. Add dependency ordering @@ -17,7 +17,7 @@ This spec addresses six architectural issues in the recipe system. Items 1 and t --- -## 1. Delete v1 Metadata Parser + Cloudinit v2 Conversion +## 1. Delete v1 metadata parser + cloudinit v2 conversion **Can implement independently:** No — cloudinit's `Prepare()` and `clone.go` call `recipes.Read()` on v2 directory names, which fails. These must be fixed together. @@ -31,12 +31,12 @@ Additionally, the cloudinit backend is already broken for v2 recipes. `cloudinit Delete the v1 path, wire cloudinit to v2 scripts, and make `Manifest` the single source of truth. -### Files to Delete +### Files to delete - `internal/recipes/metadata.go` - `internal/recipes/metadata_test.go` -### Files to Modify +### Files to modify **`internal/core/apply.go`:** - Remove `ReadMetadata` calls @@ -69,7 +69,7 @@ Warning: Moved legacy recipe to .v1-removed/. Convert to v2 format. See d --- -## 2. Dependency Ordering +## 2. Dependency ordering **Can implement independently:** Yes @@ -77,7 +77,7 @@ Warning: Moved legacy recipe to .v1-removed/. Convert to v2 format. See d `v.Recipes` is a flat list run in array order. Recipes cannot declare "run after X". If devtools assumes Docker is present, the user must manually order them correctly. -### Manifest Change +### Manifest change ```toml name = "devtools" @@ -98,7 +98,7 @@ depends = ["docker"] - Error with the cycle path if found: `"cycle detected: devtools -> docker -> devtools"` - Auto-added dependencies must pass `MatchesVM` for the target VM's OS -### Topo-Sort +### Topo-sort **In `filterByRunMode`:** - Before filtering by run mode, topo-sort `targets` so dependencies come first @@ -106,7 +106,7 @@ depends = ["docker"] - Kahn's algorithm or DFS-based sort; either works for small N - **Cycle detection here too**: user can edit manifests on disk after add-time. Error if a cycle appears; do not silently fall back to array order. -### Dependency Satisfaction Rules +### Dependency satisfaction rules A dependency is satisfied when: - The dependency recipe ran earlier in this apply run, OR @@ -116,13 +116,13 @@ A dependency is **not** satisfied when: - The dependency is `run = "manual"` and was never applied — error: `"devtools depends on docker, which has never been applied (run = manual)"` - The dependency is filtered out by `ApplyOpts.Only` and was never applied — error: `"devtools depends on docker; add it to --recipe or apply it first"` -### TUI Behavior +### TUI behavior User adds "devtools" which depends on "docker": - If docker is already in `v.Recipes`: proceed - If docker is missing: run `CheckRecipes` on docker for this VM's OS. If it passes, auto-add and show: `"Added docker (required by devtools)"`. If it fails, error with the reason. -### CLI Behavior +### CLI behavior `stoat vm create --recipes devtools` errors if docker isn't also specified: ``` @@ -133,7 +133,7 @@ The CLI does not auto-add. Scripts must be explicit. --- -## 3. Cloudinit Post-Boot Path +## 3. Cloudinit post-boot path **Can implement independently:** Yes (after item 1) @@ -150,7 +150,7 @@ After item 1, cloudinit generates seeds with v2 scripts. `Apply()` still returns Remove `ErrAppliedAtBoot`. Allow `Apply()` to work on cloudinit VMs via SSH after first boot. Track what cloud-init ran via marker files. -### Marker Files +### Marker files **At VM creation (in cloudinit backend's seed generation):** @@ -162,7 +162,7 @@ runcmd: Each recipe writes a marker file on success. -### Apply Behavior +### Apply behavior **In `applyLocked` (after removing the `ErrAppliedAtBoot` check):** @@ -175,7 +175,7 @@ Each recipe writes a marker file on success. **Fallback:** If `/var/lib/stoat/.applied/` doesn't exist (old VM created before this feature, or cloud-init failed entirely), treat all recipes as pending. The first `Apply()` re-runs everything once; after that, state is tracked correctly. -### Files to Modify +### Files to modify **`internal/core/apply.go`:** - Remove the `backend.For(v).Name() == "cloudinit"` check that returns `ErrAppliedAtBoot` @@ -201,7 +201,7 @@ Cloudinit VMs behave like other VMs after first boot: --- -## 4. Dry-Run +## 4. Dry-run **Can implement independently:** Yes @@ -213,7 +213,7 @@ Cloudinit VMs behave like other VMs after first boot: Add `stoat apply --dry-run` and expose the filtering result. -### CLI Output +### CLI output **Human-readable (default):** ``` @@ -257,11 +257,11 @@ func PlanApply(v *config.VM, opts ApplyOpts) ([]ApplyPlan, error) --- -## 5. Single Reboot Behavior +## 5. Single reboot behavior **Can implement independently:** Yes (documentation only) -### Current Behavior +### Current behavior `reboot = true` in the manifest triggers one reboot after all recipes finish. The first recipe that declares `reboot = true` names the reboot in the log. Subsequent `reboot = true` recipes don't trigger additional reboots. @@ -271,7 +271,7 @@ Reboot only fires for disk-mode VMs (`apply.go:185` checks `v.Mode == "disk"`). Keep this behavior. Document it clearly. -### Documentation Update +### Documentation update In `docs/recipe-spec-v2.md`, add a section: @@ -291,13 +291,13 @@ and a reboot wipes everything. Live recipes that need a session restart should restart in place instead (e.g., `kill -HUP 1`). ``` -### Future Extension +### Future extension Per-recipe reboots (reboot after recipe A, then run recipe B) could be added later. Out of scope for this spec. --- -## 6. Stage Field Validation +## 6. Stage field validation **Can implement independently:** Yes @@ -305,7 +305,7 @@ Per-recipe reboots (reboot after recipe A, then run recipe B) could be added lat `stage = "install"` parses and validates, but install-stage recipe bodies are never executed. The field exists in the schema with no effect. -### Use Case +### Use case BYO ISO support will need install-stage hooks for custom partitioning, bootloader config, and installer automation. That feature does not exist yet. @@ -322,7 +322,7 @@ if m.Stage == "install" { } ``` -### Why Not Implement Now +### Why not implement now The cloudinit design has a bug: cloud-init's `bootcmd` runs *before* the `write_files` module, so the script file doesn't exist yet. `bootcmd` also runs before networking, so package installs fail. "Install stage" on a cloud image is conceptually unclear anyway — the system is already installed. @@ -334,9 +334,9 @@ When BYO ISO support is added, revisit this section. The implementation will lik --- -## Testing Strategy +## Testing strategy -### Unit Tests +### Unit tests - `TestParseManifestDependsField` — validates depends parsing - `TestCycleDetection` — catches A→B→A cycles at add-time @@ -347,14 +347,14 @@ When BYO ISO support is added, revisit this section. The implementation will lik - `TestPlanApplyStoppedVM` — works when VM is not running - `TestCloudInitMarkerDiscovery` — reads marker files, populates Applied -### Integration Tests +### Integration tests - Create a VM with depends, verify execution order - `--dry-run` outputs correct plan - cloudinit VM: apply after first boot, verify state tracking - TUI: add recipe with missing dep that fails OS check, verify error -### Manual Tests +### Manual tests - TUI: add recipe with missing dep, verify auto-add message - CLI: `--recipes devtools` without docker, verify error @@ -362,9 +362,9 @@ When BYO ISO support is added, revisit this section. The implementation will lik --- -## Implementation Notes +## Implementation notes -### Order Matters +### Order matters 1. **Item 1** first — deletes v1, fixes broken cloudinit backend 2. **Item 2** — dependency ordering, no blockers @@ -373,7 +373,7 @@ When BYO ISO support is added, revisit this section. The implementation will lik 5. **Item 5** — docs only 6. **Item 6** — validation only, can land anytime -### Backwards Compatibility +### Backwards compatibility - v1 flat-file recipes are swept to `.v1-removed/` with a warning; users must migrate - Existing v2 recipes without `depends` keep working (empty deps = no ordering constraint) diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 4fa6296b..fa2006ce 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -34,9 +34,9 @@ window, which stays reachable even if the guest locks up or loses its network. stoat: up: qemu failed to start: qemu-system-x86_64: OpenGL is not supported by display backend 'gtk' ``` -This one is not about OpenGL, and mesa and your GPU drivers are not the place -to look. stoat starts a VM with `-display gtk,gl=on` by default, and `gl=on` -is simply the first option QEMU rejects when it cannot open a window at all. +`gl=on` is the first option QEMU rejects when it can't open a window at all; +mesa and GPU drivers aren't involved. stoat starts a VM with +`-display gtk,gl=on` by default. With the same window and no `gl=on`, the same host says `gtk initialization failed` instead. @@ -185,7 +185,7 @@ membership actually takes effect. Confirm with `ls -l /dev/kvm` and `groups`. ## A live VM lost everything after a reboot -This isn't a bug, it's how live mode works. A live VM's root filesystem is a +Live mode works this way by design. A live VM's root filesystem is a `tmpfs`/`overlay` mount that only exists in RAM for that boot (`internal/apkovl/apkovl.go`'s doc comment, and the same detection every bundled recipe uses: `awk '$2 == "/" { print $3 }' /proc/mounts` reporting From 66d839f82b1d2c94c7980ecb8b5f9c36198cfeb7 Mon Sep 17 00:00:00 2001 From: NovusEdge Date: Fri, 4 Sep 2026 21:48:46 +0300 Subject: [PATCH 3/3] fix: apply JSON shape, MCP guards, sudo on cloud images apply --dry-run --json returns an object; applied is always a list. Recipe DTO gains reboot, depends, runtime. sshx wraps recipe, bootstrap, and share-mount scripts in sudo for a non-root ssh user and waits for cloud-init before the first recipe. MCP: plan_recipes tool, apply_recipes and restore marked destructive, check_flag_free guard, shared rate-limit bucket, wait and logs clamps. Debian docker keyring import runs non-interactive. Signed-off-by: NovusEdge --- docs/design/mcp-server.md | 21 +++- docs/reference/json.md | 45 +++++--- internal/cli/json_test.go | 10 ++ internal/cli/run_apply.go | 23 +++- internal/cli/wire/dto.go | 40 ++++++- internal/cli/wire/dto_test.go | 30 ++++- internal/core/apply.go | 18 ++- .../recipes/bundled/docker/install-debian.sh | 2 +- internal/sshx/sharemount.go | 2 +- internal/sshx/sshx.go | 33 +++++- .../__pycache__/__init__.cpython-314.pyc | Bin 846 -> 0 bytes .../__pycache__/client.cpython-314.pyc | Bin 9650 -> 0 bytes .../__pycache__/errors.cpython-314.pyc | Bin 4793 -> 0 bytes .../__pycache__/guards.cpython-314.pyc | Bin 10529 -> 0 bytes mcp/stoat_mcp/guards.py | 65 +++++++++-- mcp/stoat_mcp/server.py | 104 ++++++++++++++---- mcp/tests/test_guards.py | 50 +++++++++ mcp/tests/test_server.py | 67 ++++++++++- 18 files changed, 445 insertions(+), 65 deletions(-) delete mode 100644 mcp/stoat_mcp/__pycache__/__init__.cpython-314.pyc delete mode 100644 mcp/stoat_mcp/__pycache__/client.cpython-314.pyc delete mode 100644 mcp/stoat_mcp/__pycache__/errors.cpython-314.pyc delete mode 100644 mcp/stoat_mcp/__pycache__/guards.cpython-314.pyc diff --git a/docs/design/mcp-server.md b/docs/design/mcp-server.md index 70b91bce..8074d5f3 100644 --- a/docs/design/mcp-server.md +++ b/docs/design/mcp-server.md @@ -112,7 +112,8 @@ is enforced regardless of what the client does. | `check_vm_name` | Must match stoat's own name rules and resolve to a managed VM. No paths, no traversal, no empty. | | `check_host_path` | For `copy_to`/`copy_from` only. Resolve with `os.path.realpath` (after symlinks), require the prefix `~/.stoat/shared//`, reject anything else. | | `check_image_id` | Catalog IDs only. An absolute or relative path is rejected: §7.1 #4. | -| `rate_limit` | A token bucket per tool. The MCP spec makes rate limiting a server `MUST`. | +| `rate_limit` | A token bucket per tool, and a second one shared by every tool. Per-tool alone let a caller burst `capacity` times across each of ~20 tools. The MCP spec makes rate limiting a server `MUST`. | +| `check_flag_free` | Values splatted into argv as positionals (`forward` pairs, `check_recipes` names) must not start with `-`. `forward(pairs=["--clear"])` otherwise reached kong as the clear flag. | **Never exposed as tools at all** (§7.1): `share` as any parameter, BYO image paths, `recipe new`, `ssh-command`, and the global (no VM) `logs`. These are @@ -128,11 +129,23 @@ may ignore them. | Class | Tools | `readOnlyHint` | `destructiveHint` | |---|---|---|---| -| Read-only | `list_vms`, `vm_status`, `list_images`, `list_recipes`, `check_recipes`, `logs`, `doctor` | true | false | -| Mutating | `create`, `start`, `stop`, `apply_recipes`, `update`, `clone`, `snapshot`, `restore`, `forward`, `wait` | false | false | -| Destructive | `destroy`, `prune` | false | true | +| Read-only | `list_vms`, `vm_status`, `list_images`, `list_recipes`, `check_recipes`, `logs`, `doctor`, `plan_recipes` | true | false | +| Mutating | `create`, `start`, `stop`, `apply_recipes`, `update`, `clone`, `snapshot`, `forward`, `wait` | false | false | +| Destructive | `destroy`, `prune`, `restore` | false | true | | Execution | `exec`, `copy_to`, `copy_from` | false | true, `openWorldHint` true | +`plan_recipes` is `apply --dry-run`. It exists so an agent can read what an +apply would do before running one, which is the cheapest safety an agent gets +here. It is a separate tool rather than a `dry_run` flag on `apply_recipes`, +because one tool cannot honestly declare `readOnlyHint` both ways. + +`restore` is destructive. It discards everything written since the snapshot, +and only another snapshot taken later undoes that. + +`apply_recipes` runs arbitrary scripts inside the guest, so it checks +`allow_exec` exactly as `exec` does. A VM created with `--allow-exec=false` +refuses both. + Every schema sets `additionalProperties: false`, so an unexpected parameter is rejected rather than silently ignored (OWASP MCP guidance). diff --git a/docs/reference/json.md b/docs/reference/json.md index 55f4cda7..bfbe5f55 100644 --- a/docs/reference/json.md +++ b/docs/reference/json.md @@ -73,8 +73,8 @@ branching on `ok` and one branching on `"error" in obj` can never disagree. |---|---|---| | `result` | yes | the answer; exactly one per invocation, always last | | `progress` | no | a download's byte count (`pull`) | -| `stage` | no | a recipe boundary during `apply` / `provision` | -| `log` | no | one line of guest output during `apply` / `provision` | +| `stage` | no | a recipe boundary during `apply` | +| `log` | no | one line of guest output during `apply` | **A consumer MUST ignore any `type` it does not recognize.** New non-terminal types can be added without a version bump. @@ -112,7 +112,7 @@ bump the contract version. Do not write code that requires them. | `immutable_field` | `update` was asked to change a field that cannot change | | `disk_shrink` | a disk can only grow | | `cannot_reach` | `wait` was asked for a state this VM can never reach | -| `applied_at_boot` | a cloud VM's recipes already ran; `apply` has nothing to do | +| `applied_at_boot` | reserved; no command emits it. A cloud VM whose recipes ran at boot answers `ok:true` with `applied: []` | | `unknown_log` | bad `--which` | | `timeout` | the deadline expired | | `canceled` | the context was cancelled | @@ -188,9 +188,13 @@ Check {"name":"qemu-img","ok":false,"detail":"not found", PruneItem {"class":"orphaned_image","path":"/home/u/.stoat/isos/old.iso"} -Recipe {"name":"xfce","description":"XFCE desktop over SSH or at boot"} +Recipe {"name":"xfce","description":"XFCE desktop over SSH or at boot", + "reboot":false,"depends":[],"runtime":"sh"} RecipeIssue {"name":"docker","reason":"docker is not offered to debian/cloudinit"} + +ApplyPlan {"name":"xfce","action":"run","reason":"never applied", + "version":"1.2"} ``` `state` is one of `stopped`, `running`, `broken`. `error` appears only on a @@ -211,6 +215,16 @@ itself before calling. `Snapshot.size_display` and `created_display` are named that way because they are qemu's own formatted table output. They are opaque. Do not parse them. +`Recipe.reboot` says the guest needs a restart before that recipe's effect is +visible. A caller that runs `apply` and then waits for `reachable` can see the +sshd that is about to go down, so it must account for the reboot itself. +`Recipe.depends` names recipes that run first. `apply` orders the run on its +own, so a caller reads `depends` to report the order, never to sort by it. + +`ApplyPlan.action` is `run` or `skip`, and `reason` is human text whose wording +is not part of the contract. `version` is the recipe version already applied, +absent when the recipe never ran. + `Image.file` is a bare filename under the data root's `isos/`, never an absolute path. That is a guarantee, not an accident. @@ -284,8 +298,8 @@ so a leak fails the build rather than shipping. | `snapshot` (list) | `{"vm":"work","snapshots":[Snapshot,...]}` | | `snapshot` (act) | `{"vm":"work","tag":"clean","action":"restore"}` | | `prune` | `{"dry_run":true,"items":[PruneItem,...]}` | -| `apply` | `{"vm":"work","applied":["xfce"]}` | -| `provision` | `{"vm":"work","provisioned":true,"skipped_reason":""}` | +| `apply` | `{"vm":"work","applied":["xfce"],"skipped_reason":""}` | +| `apply --dry-run` | `{"vm":"work","dry_run":true,"plan":[ApplyPlan,...]}` | | `recipes` | `{"recipes":[Recipe,...]}` | | `check-recipes` | `{"applicable":false,"issues":[RecipeIssue,...]}` | | `recipe list` | `{"dir":"...","recipes":["xfce"]}`, see note below | @@ -302,7 +316,7 @@ actually run; use `recipe list` only to find a file to edit. | `logs` (no VM) | `{"lines":[...]}` (stoat's own log) | | `logs ` | `{"vm":"work","which":"console","lines":[...]}` | | `doctor` | `{"healthy":false,"checks":[Check,...]}` | -| `version` | `{"version":"1.2.3","contract":1}` | +| `version` | `{"version":"1.2.3","contract":2}` | | `help` | `{"usage":"..."}` | | `ssh` | **refused**, see below | @@ -319,9 +333,13 @@ Fields worth knowing about: - **`check-recipes.applicable`** is emitted explicitly even though it equals `issues == []`, because a consumer reading an empty list and guessing is a consumer that will one day guess wrong. -- **`provision.skipped_reason`** distinguishes "recipes ran" from "there was - nothing to run" without reading English. A cloud VM's recipes ran at first - boot, so `provisioned` is `false` with a reason. +- **`apply.skipped_reason`** distinguishes "recipes ran" from "there was + nothing to run" without reading English. It is `""` when the apply ran. A + cloud VM's recipes ran at first boot, and a second run already holding the + VM's lock is the other case; both come back with `applied: []` and a reason. + `applied` is always a list, never a bool. +- **`provision`** is a hidden alias of `apply` and reports `"cmd":"apply"`. + It has no `data` shape of its own. - **`doctor.healthy`**, not `ok`: the envelope already owns `ok`, and two differently-scoped `ok` fields one level apart is a trap. @@ -348,7 +366,8 @@ faking one would break the exactly-one-result guarantee everywhere. Use ## Streaming -`pull`, `apply` and `provision` emit non-terminal events before their result. +`pull` and `apply` emit non-terminal events before their result. `apply +--dry-run` emits none: it computes the plan host-side and runs nothing. ``` {"v":2,"type":"progress","cmd":"pull","data":{"id":"alpine-virt","done":41943040,"total":62914560,"percent":66}} @@ -358,8 +377,8 @@ faking one would break the exactly-one-result guarantee everywhere. Use `progress` fires only when the percentage changes, not per read. -`apply` and `provision` wrap each appended line of the recipe log as a `log` -event, and emit a `stage` event at each recipe boundary: +`apply` wraps each appended line of the recipe log as a `log` event, and emits +a `stage` event at each recipe boundary: ``` {"v":2,"type":"stage","cmd":"apply","data":{"recipe":"xfce"}} diff --git a/internal/cli/json_test.go b/internal/cli/json_test.go index 83e71451..7047c245 100644 --- a/internal/cli/json_test.go +++ b/internal/cli/json_test.go @@ -80,6 +80,8 @@ func TestJSONEnvelopeEveryCommand(t *testing.T) { // doctor exits 0 even when the host fails a check: it succeeded at // checking, and exit 1 means stoat failed to answer. {name: "doctor", argv: []string{"doctor"}, ok: true, exit: ExitOK}, + // The plan is computed host-side, so a stopped fixture VM answers it. + {name: "apply dry-run", argv: []string{"apply", "work", "--dry-run"}, ok: true, exit: ExitOK}, {name: "up unknown", argv: []string{"up", "nope"}, code: wire.CodeNotFound, exit: ExitFail}, {name: "down stopped", argv: []string{"down", "work"}, code: wire.CodeNotRunning, exit: ExitFail}, @@ -120,6 +122,14 @@ func TestJSONEnvelopeEveryCommand(t *testing.T) { if _, bad := res["error"]; bad { t.Errorf("ok result carries an error field: %v", res) } + // json.md's envelope table types data as an object. apply + // --dry-run shipped a bare array here, which broke a consumer + // that indexes data by field name. + if d, present := res["data"]; present { + if _, obj := d.(map[string]any); !obj { + t.Errorf("data is %T, want an object: %v", d, res) + } + } return } errObj, _ := res["error"].(map[string]any) diff --git a/internal/cli/run_apply.go b/internal/cli/run_apply.go index 5e8bdfdf..26c66f89 100644 --- a/internal/cli/run_apply.go +++ b/internal/cli/run_apply.go @@ -58,7 +58,10 @@ func runApply(a *Args, stdout, stderr io.Writer) int { // caller owns the error; this one exits clean rather than reporting // somebody else's concurrent apply as its own failure. if a.JSON { - return a.ok(stdout, map[string]any{"vm": a.VM, "applied": false, "skipped_reason": "an apply is already running"}) + // applied stays a list here. It named the recipes on the success + // path and false on this one, so a consumer iterating it hit a + // bool. skipped_reason is what distinguishes the two. + return a.ok(stdout, map[string]any{"vm": a.VM, "applied": []string{}, "skipped_reason": "an apply is already running"}) } fmt.Fprintf(stdout, "%s: an apply is already running\n", a.VM) return ExitOK @@ -68,22 +71,32 @@ func runApply(a *Args, stdout, stderr io.Writer) int { } if a.JSON { - return a.ok(stdout, map[string]any{"vm": a.VM, "applied": applied}) + if applied == nil { + applied = []string{} + } + return a.ok(stdout, map[string]any{"vm": a.VM, "applied": applied, "skipped_reason": ""}) } fmt.Fprintf(stdout, "%s: recipes applied\n", a.VM) return ExitOK } // runApplyDryRun prints core.PlanApply's plan and runs nothing. Human output -// is one line per recipe ("xfce (run, never applied)"); --json emits the plan -// array. The plan is computed host-side, so the VM need not be running. +// is one line per recipe ("xfce (run, never applied)"). The plan is computed +// host-side, so the VM need not be running. +// +// The JSON plan is wrapped in an object. json.md §2 says every result's +// `data` is an object, and a bare array broke that. func runApplyDryRun(a *Args, stdout, stderr io.Writer) int { plan, err := core.PlanApply(a.VM, core.ApplyOpts{Only: a.Only}) if err != nil { return a.fail(stdout, stderr, err) } if a.JSON { - return a.ok(stdout, plan) + return a.ok(stdout, map[string]any{ + "vm": a.VM, + "dry_run": true, + "plan": wire.FromApplyPlans(plan), + }) } for _, p := range plan { reason := p.Reason diff --git a/internal/cli/wire/dto.go b/internal/cli/wire/dto.go index 4f302157..d7c0f5a3 100644 --- a/internal/cli/wire/dto.go +++ b/internal/cli/wire/dto.go @@ -258,13 +258,26 @@ func FromPruneItems(ps []core.PruneItem) []PruneItem { } // Recipe is core.Recipe for the wire. +// +// reboot is the field a machine caller needs most: it decides whether "wait +// until reachable" after an apply answers about the guest before or after the +// restart. type Recipe struct { - Name string `json:"name"` - Description string `json:"description"` + Name string `json:"name"` + Description string `json:"description"` + Reboot bool `json:"reboot"` + Depends []string `json:"depends"` + Runtime string `json:"runtime"` } func FromRecipe(r core.Recipe) Recipe { - return Recipe{Name: r.Name, Description: r.Description} + return Recipe{ + Name: r.Name, + Description: r.Description, + Reboot: r.Reboot, + Depends: nonNil(r.Depends), + Runtime: r.Runtime, + } } func FromRecipes(rs []core.Recipe) []Recipe { @@ -275,6 +288,27 @@ func FromRecipes(rs []core.Recipe) []Recipe { return nonNil(out) } +// ApplyPlan is core.ApplyPlan for the wire: one recipe's entry in an +// `apply --dry-run`. +type ApplyPlan struct { + Name string `json:"name"` + Action string `json:"action"` + Reason string `json:"reason"` + Version string `json:"version,omitempty"` +} + +func FromApplyPlan(p core.ApplyPlan) ApplyPlan { + return ApplyPlan{Name: p.Name, Action: p.Action, Reason: p.Reason, Version: p.Version} +} + +func FromApplyPlans(ps []core.ApplyPlan) []ApplyPlan { + out := make([]ApplyPlan, len(ps)) + for i, p := range ps { + out[i] = FromApplyPlan(p) + } + return nonNil(out) +} + // RecipeIssue is core.RecipeIssue for the wire. type RecipeIssue struct { Name string `json:"name"` diff --git a/internal/cli/wire/dto_test.go b/internal/cli/wire/dto_test.go index a599a14b..c78ab82b 100644 --- a/internal/cli/wire/dto_test.go +++ b/internal/cli/wire/dto_test.go @@ -118,9 +118,35 @@ func TestHostCheckGolden(t *testing.T) { } func TestRecipeGolden(t *testing.T) { - r := core.Recipe{Name: "xfce", Description: "XFCE desktop environment"} + r := core.Recipe{ + Name: "xfce", + Description: "XFCE desktop environment", + Reboot: true, + Depends: []string{"devtools"}, + Runtime: "sh", + } got := marshal(t, FromRecipe(r)) - want := `{"name":"xfce","description":"XFCE desktop environment"}` + want := `{"name":"xfce","description":"XFCE desktop environment","reboot":true,"depends":["devtools"],"runtime":"sh"}` + if got != want { + t.Errorf("got %s\nwant %s", got, want) + } +} + +// TestRecipeNilDependsIsEmptyList pins the nil -> [] normalization this file's +// doc comment requires: a Python consumer iterating "depends" raises TypeError +// on null. +func TestRecipeNilDependsIsEmptyList(t *testing.T) { + got := marshal(t, FromRecipe(core.Recipe{Name: "xfce"})) + want := `{"name":"xfce","description":"","reboot":false,"depends":[],"runtime":""}` + if got != want { + t.Errorf("got %s\nwant %s", got, want) + } +} + +func TestApplyPlanGolden(t *testing.T) { + p := core.ApplyPlan{Name: "xfce", Action: "run", Reason: "never applied"} + got := marshal(t, FromApplyPlan(p)) + want := `{"name":"xfce","action":"run","reason":"never applied"}` if got != want { t.Errorf("got %s\nwant %s", got, want) } diff --git a/internal/core/apply.go b/internal/core/apply.go index 8f91a296..2ec6b2b6 100644 --- a/internal/core/apply.go +++ b/internal/core/apply.go @@ -475,6 +475,16 @@ func dependencyError(dependent, dep string, manifests map[string]recipes.Manifes type Recipe struct { Name string // recipe name, matches the directory name Description string // from recipe.toml + // Reboot says the guest needs a restart before this recipe's effect is + // visible. A caller that waits for "reachable" after an apply sees the + // pre-reboot sshd and reads it as done. + Reboot bool + // Depends names recipes that run before this one. Apply orders the run + // itself, so this is for a caller that reports the plan, not one that + // sorts. + Depends []string + // Runtime is the interpreter the script runs under: "sh" or "python3". + Runtime string } // RecipeFilter selects the recipes Recipes returns: the set @@ -498,7 +508,13 @@ func Recipes(f RecipeFilter) ([]Recipe, error) { var out []Recipe for _, m := range manifests { if recipes.MatchesVM(&m, f.OS) { - out = append(out, Recipe{Name: m.Name, Description: m.Description}) + out = append(out, Recipe{ + Name: m.Name, + Description: m.Description, + Reboot: m.Reboot, + Depends: m.Depends, + Runtime: m.Runtime, + }) } } return out, nil diff --git a/internal/recipes/bundled/docker/install-debian.sh b/internal/recipes/bundled/docker/install-debian.sh index 3f1719ab..71a80955 100755 --- a/internal/recipes/bundled/docker/install-debian.sh +++ b/internal/recipes/bundled/docker/install-debian.sh @@ -10,7 +10,7 @@ apt-get install -y ca-certificates curl gnupg install -m 0755 -d /etc/apt/keyrings curl -fsSL https://download.docker.com/linux/$(. /etc/os-release && echo "$ID")/gpg | \ - gpg --dearmor -o /etc/apt/keyrings/docker.gpg + gpg --batch --yes --no-tty --dearmor -o /etc/apt/keyrings/docker.gpg chmod a+r /etc/apt/keyrings/docker.gpg echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/docker.gpg] \ diff --git a/internal/sshx/sharemount.go b/internal/sshx/sharemount.go index bae14624..6c783048 100644 --- a/internal/sshx/sharemount.go +++ b/internal/sshx/sharemount.go @@ -106,7 +106,7 @@ func mountShares(ctx context.Context, v *config.VM, log io.Writer) { return } fmt.Fprintln(log, "\n=== mounting 9p shares ===") - cmd := exec.CommandContext(ctx, "ssh", Args(v, "sh", "-s")...) + cmd := exec.CommandContext(ctx, "ssh", Args(v, sudoWrap(v, []string{"sh", "-s"})...)...) cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) } cmd.WaitDelay = recipeShutdownGrace cmd.Stdin = strings.NewReader(shareMountScript(tags, guestFstabPath)) diff --git a/internal/sshx/sshx.go b/internal/sshx/sshx.go index 70945755..d890cf9c 100644 --- a/internal/sshx/sshx.go +++ b/internal/sshx/sshx.go @@ -72,6 +72,18 @@ func Args(v *config.VM, extra ...string) []string { return append(a, extra...) } +// sudoWrap prefixes remote with "sudo" for a non-root ssh user. Recipe bodies +// install packages, so they need root. Cloud images log in as an unprivileged +// account; cloudinit grants it passwordless sudo ("ALL=(ALL) NOPASSWD:ALL"). +// An apkovl or BYO-root image connects as root and gets no prefix. sudo reads +// the piped script body on stdin the same way the bare interpreter does. +func sudoWrap(v *config.VM, remote []string) []string { + if User(v) == "root" { + return remote + } + return append([]string{"sudo"}, remote...) +} + // CopyArgs returns the argv (excluding argv[0]) for scp between the host and // v's guest. It shares every connection setting Args does (see connOptions) // and differs only in the port flag, since scp's is capital -P. @@ -231,6 +243,23 @@ func Provision(ctx context.Context, v *config.VM) (err error) { return err } + // A cloud image runs cloud-init at first boot; it may still be installing + // packages when sshd comes up. A recipe that calls apt or dnf then races + // cloud-init for the package-manager lock and fails. Wait for cloud-init + // to finish first. The exit code is ignored on purpose: Alpine reports + // "degraded" over a missing fingerprint helper (harmless), and a real + // cloud-init failure surfaces when the recipe itself runs. apkovl and BYO + // images ship no cloud-init and skip this. + if v.Backend == "cloudinit" { + fmt.Fprintln(log, "waiting for cloud-init to finish…") + ci := exec.CommandContext(ctx, "ssh", Args(v, sudoWrap(v, []string{"cloud-init", "status", "--wait"})...)...) + ci.Cancel = func() error { return ci.Process.Signal(syscall.SIGTERM) } + ci.WaitDelay = recipeShutdownGrace + ci.Stdout = io.Discard + ci.Stderr = log + _ = ci.Run() + } + mountShares(ctx, v, log) for _, name := range v.Recipes { @@ -256,7 +285,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { if bootstrap := recipes.BootstrapScript(runtime, v.OS); bootstrap != "" { fmt.Fprintf(log, "ensuring %s is installed...\n", runtime) - bs := exec.CommandContext(ctx, "ssh", Args(v, "sh", "-s")...) + bs := exec.CommandContext(ctx, "ssh", Args(v, sudoWrap(v, []string{"sh", "-s"})...)...) bs.Cancel = func() error { return bs.Process.Signal(syscall.SIGTERM) } bs.WaitDelay = recipeShutdownGrace bs.Stdin = strings.NewReader(bootstrap) @@ -272,7 +301,7 @@ func Provision(ctx context.Context, v *config.VM) (err error) { } } - cmd := exec.CommandContext(ctx, "ssh", Args(v, recipes.InterpreterArgs(runtime)...)...) + cmd := exec.CommandContext(ctx, "ssh", Args(v, sudoWrap(v, recipes.InterpreterArgs(runtime))...)...) cmd.Cancel = func() error { return cmd.Process.Signal(syscall.SIGTERM) } cmd.WaitDelay = recipeShutdownGrace cmd.Stdin = strings.NewReader(body) diff --git a/mcp/stoat_mcp/__pycache__/__init__.cpython-314.pyc b/mcp/stoat_mcp/__pycache__/__init__.cpython-314.pyc deleted file mode 100644 index 8f9b54c7827c4142c123282f1022770fb7e6ab0c..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 846 zcmY*X!H&}~5OvZv?FM#%3n!$t!~rVW$NGOzT(Qdoy_P~kMn@n4m#I9_&OD_oV zH{AFGK7v~=9QXlX4_p~1T_mh%;<0Dmdo$xFM~6ParCzDkSC^0<*11{em+QKRzmH@> z40-EJoS`#tjXS8B+OhTBVSUgrje%#pLDMt`Ez>$C0h!dtjyagr9+Gy0eQ^lUMl#dQ zlXZ_WLC>OdszGf*QE81?0xctGbUJ*b>FbN}h$d1P#S#&br-|mSTq3DNP}fDeOT`xd0td1Q38se#XT+O;2PQSsqaZEYdchXC_$|M<*Mm(h01vdt8d*_JtjJq?lepTKJ{cS4yf;+fy}s zQ>t!_eP*g*jb}|0P1`S;ag6=7u#JNbHPm<2kTwYdX~!+=k#3NK@5DmL_a|Dv*tL?FZaNy Y|G7_`;5!+8BctE-<}LSAQypT!UmNQGIRF3v diff --git a/mcp/stoat_mcp/__pycache__/client.cpython-314.pyc b/mcp/stoat_mcp/__pycache__/client.cpython-314.pyc deleted file mode 100644 index ada35218abc7c8b6cd9d50cdda36f4f222c47b0c..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 9650 zcmbVSYfv0lcD_9?<^?l61ric1K$4MQuqD}O^|FzK1eO-zW)^uPq){`|z=&abw!23N zvhh;MCWY)&BxRHGxN1{Y%C0O1$ zJGXltXtA8Q3vS=Oeeb!CKKDE4e0TO$Rr(pOq2FORS}5-ra2ig2cNs2zcN=bg_ZS}BodXs9Uc=k(GkpDi!_V8e1_J#-BZ#~p zxd$rytBk7tkP+$+8(}IlswIz6BUKo+lGmuauJm|1m8BJ0po~!u#tx^d2Fwbw39Br(lT7JdJSaebm)pTlB z%%tSFA{vvjA!^yQj+`RuhKk3rWLnl{BYuD1HAR~>CX?w2aZ=F~F{z7*WZaMqB_WQ@ ziU~EYcWcTxUZvwo_f=g@N2U{^CPS1)lG9>*Qq`4oL`1LtaW$)0*r#c# zri-$sC9h$mX+9rK(X%N-RMOX!l$xQ5`$f?f6%l*IxSH0p)7D@yAVr&?!Of^*CYhm} zDrySVF|=%)`d08v5=n}3O-UPBIfbQ6rZJI}B46WEQ>MikO-)Z&Gt(3~K?_)tsw+Rr zqG^(vlAr`?q}2G7j#W_7qLSpxH)XcVa=l1UjOoW-nK ztwFW&l}sA6hACM$BBBJzG>N|Rr#rT(0KUl zz|c_NsZKGCat%^NEVioASHx#Sg&FJfO({B!Urz7|%BVh5E(rx{x@!yzFhvUxhdxXw z5&uE14We=#BaN#Gu2uXyq)4sCRSmC7Gv(TMm{x|bifn%Xz5={+{=E|-28Q*Fh=Y8{ zW15_XGO1}ySX8OyOj1{lK(Z3YdZ&_O3ba(gYSJ)t@mUfzGnq`$2hOOPK?COFpHnnd zoX9G=0kx5htX@#9j69oCBcxF z6w{efC*lG&KK;{0Ab|ZXtT!Vw&qml~$DyAxmOxoEdqtQOBtdd?3jhM=7gX2uXo`{5 z(xwyY{T{=xJXm~;Lnn!?=5yVX>a^0GRTP|k*I zgew*xGh}0Td6^^}w3j1;1=wT1N{rDGlu7fHx3Tg=zTYkzVFhVLNu5tf;+AD&%bn=C ztgqn4xAauN>w?sYFqaxC)RY_}EC|c8-&yE?QW#-vY!vmZYpAA>E8SrYT!s1>VMHiO z#|SX{l5}^W7Nn9oc;@~}Qgk&1*u+kOnwMF1X$K}Xne$P`ig^@^B1$n=C&5C29OFnJZtcFfKG*W3#Ky{vt~u6 z1!f2a2hcz>;gDGrMRH&Y6+q_zKp&&GR|BAwQGxPesVZhlt7Y5D4!`M8b+ZDNCJF2} zofC>-x-zhYrYEWMk$B{Ioc6El)!6y7X7y=|ZcsH&(`c+M?$IZ+Mlxl(0Yvf14u|Pg zu}E4nfm!fD>$D3*QKS822F8==M9k`5+l3Oo)v)2QaUKO(xaQ{Ejk%kz-FPiGwG`Sr z-~U-f=wYb(=FE*5UO2fN+H*g&=a<_1vwuDNLF2FY|8viWJfOe> z{`}!+zWoPFH6!`p$VzC-Lg- zRWA!x&!5xkF^~;=+l2yL!a6o1hIok?g2W8R<>Q7^5)7AAB{|`*xX52g35Hv8hv^R z4yYc~AORbgu&lQD^6>dV@f<9xUUV2DgBu{0a0B=xthe(yY!0!KQ*ude$lo-`Oz!9)cQ`CQ}H4l!?SPw6f*NH zDjX>xNq~@~g^k)63na6fjj(jF?1Gpk8T^ufTJH%kiCjaq4isvnr!amlT}h3XN^}~- z?%lgB<@Uy6$#l|)#mrD~epUgWTNCQevz3PC<%X_&L)W{3cPbZLe;in8iG1vBwx&eG zDGbS;(*MOW^3P?hy%YQWX-ltlp^HBJ7wwzhUBAYV+;Q?-X&|V?FI#^q*MEgaGSNeY zA=(w_R%XM1&qL+k#;zqf3J9KYj5tPJg~o?wVD9&Gw67?XjTY@1Y8I{4lNKn!ZA7`O zLJhbL&QcGh(h-U%Tyy_4qG-wS*)k{;^$Z+7otFUdxql-9$8^BCIHTYXfy9iKgzujw zZx6PFyd@X~6+r~~akfE-i09$tV#C7oA>g#!Y4~&GwCG|hnUkyzN6mj$ndJzIJ~ahK z45o)XV0d1wIf~hpP~Ae1Yo=LY0}t#>I>sY_4PfU* z-%vhb{TV)pr-L1!*xT)bO86|mbIIC-u93f@y-3MEN?t-TSKCK%Q$N5r1rNZ&LcZbV zswlR+2!!lJd@_6KTld~85T`@ku|kj85%)7|lzbklS#{&S>R~lIRx2pyWwpCkeU$Sv zf8$yJxs6+s@Bz@q@HH#f{{`C#M1YWSMOemN=ExO64K?kHT)iRGfAW$X5yy!0yinG9 z6PL5>Ev@wv*VErhPEZT39$RY0vyF{-hKk#}Bx)RA5Z*t7tGDUFZXMyJ< z4qL_<*LIWw2!4$=6mkcg-$QnS&+2RpY=&be`(bX=ev4x-Ghn=33+pbr;4SRz z@59)!9W3e@BD3>?S=Bvy1vwtMRExQy`7GSa+>&0_d@qckyToQvbs zmBrE(SsVZ0oTsac_`_(2+w@peAHO61p%4xPlj$;)>xNkYUQ7pPg-1|d#9@$B?I;uu zE;88~lOYtFGTmqj+Qmiwz3Jr)U0P0=6?QafR=~5ks>Bhcft^ZpxU@mE)rKe`!=gz@ zI_gY+$pJClL*Nk8+-EgiLjyy7W`#{ZYNt@!^vD^!PnZ=kE3B~GhN`}izQJg0NIHK; z>Kh(51J?6!wD(M(>9Zcroj+r`X5^&7y<-H9YAQtoOQke24W{cVqEC%nIc`%sg51Nf zOzTTjdV-Rdk(fbi^fOc?68dyiyf@h_*eIfWCp{C_f%l%NxN@exUi#IOjp2-LI-iac4 zSiAkr7jM6a+j8x$d$sUTnwM+Z@71)gRM)T6)jz7N2>TaYtKkM;^GfZOH;>#ta=&&j z+U0Bat~55k9r$_R7Mz{t<#5}*aNC_z55gS}8@A^XKbyYgT&ZpN(<7hMHLq;h`sU2- zncTIX{RDMux8x2zsBMFx-?8)7X?k6|?LqCH#Z!NK`PUa8NEh=5M?dZyMR_jzi=Myi zS$yH07nj=(<=YP39s9WMdu!WSQ~RpOw(fYl>8+-_6Zc;jc(3OpWBGji-uZa0X{jl( z@bc%O+MBaCX5aYT`=M6y!+f`We-$!V$Y2QmrHNE*@ z@8_XA_FC06zPb1I-rR+ynqA8^`|>sW7HbwxVu2pkH|A>J-tyL#I}M9lmzoYP z)j$8Raa-=>+h^W7b7x>tTWa3F)OcXCqOyiHOHG|i^<9sA?%JwV=B}(-4YP0q?|=7F zXeVO-d}z-dDev9$)oL3#m)|s{KM$JTYGo(cd=g`t3~>c^-Yv^cOm_# z>-m9J=LYZbyN1e^CO_dglItyyaHu2#gt^DM$t7hD6BJbA%AbV!Kdz~{LRAdUh@ z;q7*~SjIW*ss-0^6|Ti~Vcv1h?S~}EJ>opzxX$L?$itPcVCRC*I|1PyxgMPpipvGy z?G$H|N{Z9&#CLkmRZEv0z$1xj%Y9qr6V1`Ax|Mp<8sF2_>{%wgN#B)d{D3k89B+xy=Mw16P+R% zl@1W_{FDYX2%rs8VPI^25YbTCNlcQn=ETdWVDu*CEE*tujB|FGxI~>STEt&Mz__Dq z^~p$sB(p-7qryBJc5D`DUl0xyBk4A#b)yw*$OXDNEugHhS$@;Q7g)Dhig}x*IGU2D z#}e{!v&9BU1TTs_9XX2Q99YBS$XFHLf=r#1`m1@i_`>_g-aGa|`v)x#_6=|y8|281 zt_g5E+-2C#VV!K7HboNPHUp;jiZ5CH!%$LpX61(7xrU!Z&9A;7psr#EUjR@8wywW^ z9SFVZbcOw&*KJ#M%>D)v>OukuTgTVZIy2Mt>E}@--uYK%j^NW*YmvP=8RLpaV$SGu|h0KXk**hXmM|C z7VJ_GTyWkqLWysR3Qysiu!sl%A)R^<%X`=)Q1;jXV+-5P>ld9{hnxU14#^tMC4dLu zcK8`FDOD$+12_kzp_9C{eTvON4r9l2;tX6js z|L{~Ylc53(S4&Pz;uPCD7@m=54|8^{+@VG#`%sq7)NQUTF-zRpSG|>I{MjgCCBu!WV^~Zo294 zQ#V6InxEn%dLKc%b_99`<_k;?5Q1xd93}kZVz+g2p@9HkJmD&Rkt-(rlVJGf;TwnF zI68movq0#RQ0=P2>8^WNRde&|>sNDiOI14-oPev^?YZH1+m~zh+^^ZQa3Jq`ZZV4F zt4D6MUk$II{hfrEg^u~fDg0;-{^JplyeVise%3&41V1ywuN3@eNdg%qgp;y3CPSBOAy>$w zz|Saf2oE7B=ev;Jrd8g{m56u&tgs~$+laHl2w5}OL-fNCexNw6snh&4o_ODgf6md> zWeUd-3qmjs$tVJ~XhVng8rDMl2@*^9?1P0$@RHYJpq*pRWZU zJh$Xu?8_AR>>&H7C8IFQ<`JH$4bvF(-|b5#9!OlQ_N113)YEBa^26kdoh_gm#W) zSAL9~8SE<~R;}|EZV~x*qlrFYFHP@J%Qzp`-oRS`Dbs%s3HFU3tT{r0>&sRqwEcz! zK4rd7S>>m!`ZIRu9y|1Jtl>Y|@q6qznAQ4*JgX+IwRyg3rK)~Du!7S+9}2>VVseQ) y2Ns=o4*f+s-`KTiEr86(;vztCi*0mL=JiJ(hzcaug{)PLMchk+rmTHkMWnwUUvd2~CkBEiEp& z84e{&r#%z_3K)TpJ{su7z6Cu6J>}S=3Iyna=&h&T>`M7L_ItzCZ{0L4u{$$-^Y`Y> z`@Z-1>ao6}hUffsargC-rv05>cAsq0(6|4DMq4v9seP^)dR6}@B~uqPBPG*DT4s!l z%o9ygB57mOF=3FCx3X`Ga& zj8k%8K)a|J1B04z`m@tl(#gJZP)P?N9XcQ#Qqo~a&m52rE9qHC&mEASF-9(FW9Kcd zYg$E!K!mL6QtkRHOg1?Ccz&+N?7)}8vL$=P3G8rE@MSKzZ}Z8!Vc<_Row12xaVl^) z4Gg8_%aHMVZYWt8Fc}0M3G)ZNoGv^GL!6-l9}i7jRq=Icxf zZb_OD`GVWQitlb>2c?{a(QJkLT(~w{wp=g5UI)}G+QJGOP;VUbo!w1Y(^|!W$WXo| zBhor-anB>`1#td!>zG*9j!(HwYlWLM#R=AZFR&c$!~-~s+e*Y%tL3@2wdC=*mxhVR_n}PO zZ+SSov#}@0`+@1W;cDFPHk*;88RoLfJtyuZu^BdS7prl}R?3=~Y&Wn9Ezcd(lIBtn z$hbH1SAA$=dcjJZmu`~>k&FwNWOEOyWup)Z#{;;AWmh!SnX%Rvkt@(!aDCjgZ+Z4E zps3CyFZ%X7Xjp*NXPVRjt<>kEfRhe5WsTIJwy3?O8|gvKD|p8}9RSP7+@kirF7t5c zTwI)&fhCpEH~(3NYvBzy-E1be7p}vD9k%Yu1`AfNE?Zv6;l%ZZ8#4Uho&nVL7>rp|lS|RcRW|AeER5_1TbvzL&M>O(d2v?FOzd3j8h(^51=zw8!d>I=2duqrZA`Fh z3JZjUJLGu`A*Jq*TiiGVwJ9 zKZVnYq76K2^rwdiEREOgaWvYn+~=uxf2U~mdS)NBrQhcHzpgf zR>-= z`RwpBDjv>1rRwops`}=ns{Wi*TZ_L@W419zv%3^dZH?NnNze8jG(OPV+HX-O&wO@K zyMz8ky#$(HOqH}Z(^tCoy`bGb?vtI=bDQ~CquYMn?|QSQ-Tu(u+sn{l+tg<2DyzX& zvtiWYA}*Mw>$}o4#Tm>N=cpK=;yelr%Y@upeqLYPW16;Sg`sJNIgEXYp0>C|bH`A` zCl8q}*+iM`4fOuh*8W!L`+fc|g`tPj58ruo;?a#q?`;jg_FZA3b86^*PiOG_{nFE% zHgw^B34*VB#7mepmPs@r-S$--OU0Ua4WcInI*#HoGcKBDGjJjhj7fL;MPzy1o_-UY zBSHzjcW%Vfv8{DzZz*!K( zSW5&pfKKsIb_oa^!}uCv)Vgpbf!qS}MbiZ_GQq>hlZ*_c=xHD0FNbU;vOpt}bE1V& z4lctY*1S_UfMvFhRf)Q|(m9Ei zHW3;Ri>{x59HDH2RV8b3%P011S-xu{QnJup!7-E?U{4%LY+xx+F*GLjQVei}uE>Ye zL{~4O@iTNqfgm@T29)w*7(?S?!a)gHA-2T1P&%-#IE!9!feJ>&MHFMYP~M@~DRCRx zE*?I3Yln#D19XQpwXOZArL!Z?sCWd1LG^W7Ro~Q=P`QV{KJ&P=b>-u&;hWzTW;$nHyx-qBeeSE?gfmk6o5B3V6Jml23iJo@ z`z{HoxI!I=@e5-s>nnRF_@$9H{(Dc?^7{6h5FN@Ou66NyB!4)PJ!IqKQ{9Yr)(sKE z?8fF_>x2`PMReU58JV?8&hEI&Dj%l&qMIiJ%^@&BuI{sSLJrb52?QIKi}1$)05F@7 zQR`riN$G&%LxvbRPRTO>BrOlAbAlYoOOZeT=tT=yQ~3fV(nK06&*<)p90qwf@;yon zEOwJ`s6+_r2#^J3T945Z~u{v%L_pJT}ctjdaSW3dBZ?Hs%O`0d9pZ@qE-+k4;MeX8l7=$BFUUEWEjdx}pB z+Uc|Ri=83%Ro^apNgJWmcrI+q)J?@T{KFF{RM^cR?3QppKkQwS8f2q??;#e(qi<8{ zb0Fh4cJT^?D!a$WZXoD62B*{$2;2N)8DE4+xWor1zTK9*z+OcRT&6q|w}6Ogc^+Hh zh*Y}>R(40|1R$qrmXA-{Wf3%)V!8w?z+x*BEhOLcWl7;GnMEH$-GviW5>ShXW%O~# z8u&uOM=&jB7bUI*yD>P+}VQ;6vHGKDSVtj&s+Hxto<*T&$I6 zD`ve+?NYV2Se~g)oAb9SQ}L|_FJYZC&s<#qx`PQwO>QwpK zOeOB6Va7tOR;}HL`^z&%r9A!Vo*-YFGpDQbH{<^5?Ce6FW|-Hjm6_?dm&E4$M@F@F zGcHY)YigZJdFmsqLd#dC&2rtmHaAy~dlzaqYtY1;nY$6^>($xH+(JDrVA51&2C8Kj z=Am#rFf~`ZUNvUR_3B*BG%BAgR1NeNssFi2pp2|I&x5#Z&&Zk0*^2Nls6x;U= z9v41HvesV<6CLEC;U7>uP3yY;ES1wU-yhfXOTbCrP`jto^Flk<=^JhrJN+Z=QfKhB zE$viiXmU#%=o~xS?(Lko)b8&TPqy<(OUcwo)uCQhZ|^|6&?)q{v)#$`zLV?MhaU{@ nXsC8x?A1peytkvF+Bw&!Uwp8!qoLYiS-tS!(;qZc>Ja||*A52& diff --git a/mcp/stoat_mcp/__pycache__/guards.cpython-314.pyc b/mcp/stoat_mcp/__pycache__/guards.cpython-314.pyc deleted file mode 100644 index 23fc3552e71b24f97b45a2b59f76da6ae2340702..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 10529 zcmcgyYitx(macxfA9iDdc_h3pHbZd6?k0vLU`hFy%5-8d2}*qI+|Mp;2dBPBo4N-O1OLlfDm&1xgf?vMFr)``&0ul>%g zuI|Qx1nsV-)LqZ}IQP~)-}%lt_2XDmM8G!wn(4ORonctn3+=y=!)Zxu%UgRO4Uwc|EH z4D9hZ*H-i?$~20>^@7;2UdT3EO=1Y=VSXMHSBnu`Md>Q*u~y-_5!X$X>*lor`ikMs zsy*IHe=TA&u3LEh)uU^+3A+SwP0mx5r&aW}3mvTm8v+hz6ecT{V&qg!H7zyG#<8H%ZA3-r8`nL4vF$~2QuOSzbl z3TSOo$xcTs#k8cctP*F(CKY3v>6*e+lPTJ`Zlo0%jZ8>}oK;Md>EmoFBU#iC8#p@1 z(pgo}EGFxU*%=vA(r8rS5A%j@>1jR7CJK@vX_lgxkk87f+5|(H@j{km6hrA@nLT(TO?G8pI%U14+5G&9v)9<|XdCO4hMl*HKx4a~H3?57~HSrc9Ej5zlzQ$mwJ zq~N{iX)_@!raGY|a_M}RsTi2Rcur>j{EvIMKyR74)|J*Z%aGDmmuyJmmh<@Kc%;K) zN9#s*3fKNYyfhqRuVdC^QF%+@@3y$<&9g_kekyepcXjO>t)0eGU8B!$Pt!84EMiML~nwYnEwy7ByGPYh?Bw~)n2jfsa(5dCmN~jxdykGSXIT0XXJZZ*gJ)uT2|EdCWRhv zBjBjp=c;tZlLTPwLkCYR?XNF=F67EG$br(ma*j!XTAJwO{nkoi6e~A(gz}CbF6*@V zMEL^s@`=Gp>6J6rP#f{Nm@Ca7W@zP>6x@=oj1y0q$54IGNvxrch%|sj;OGbRY}&yU zixe2>L0T=bd{$C5mKQtc>y%!|0&`4&j01YIWJ%1>bt}#)x(l=$o2JqLaE%$zZpDZ| zPK|DxlBQZ}ksuLL%K)!21w{agRE%RVAU&-cBpxtn8feJc#?^~$0LYv~Sf>{-T#V0w zKgn%UG0+5|nVQ950Ca{jWvIY;rfT_u#WE^Ti+YKWWVyT*j}+HT=A0RVEQa!S0h1yZ zL+;IDJIi3QvWg^=yqFF^8h{n&QyFSGFQt`YD@R>tcJLfvXiC}QvlxSB;cd`lbF*YB zc~}KZIV@apliNpKDXoyrVu~!Kn5w?fY*wFA~&9JzfDh#4jsY)Z@bvwMaCtmc%;~il;qM8uWlC-p9kXqRRK%$yA{B-Ws zO+ciQw}VpFqGKqP97vryGH}ce043AFZG#Ym!{rWxl+kX&QcGV*P3BVcl8k3?ubqng zJ9aa|H>-ulb+hudqYF`XFUscqw<0$qw;F%b_*HCEX>)fuw!0MCz1R@FQhdMo%XjWK zY{dP!i}U_}xL8VvrOiX-*l;N{i~^Vb{C>l;bq|l0Husie$4a4NUp0jPYVt28XItNU z_gdTaj_VzB9iMn_g>Qy$58vOpudYg8Y4eG4>|`l)a9jqFeE3F}7~jx|&%MJX;T9*K}~%=UWw75`5vvQb=fQo;hid_Wxf@lMzG% zc0kQZ6VBT(!nk1&9Ac)UI5>uh&onnKe0Pu@xE7|x)AB&TZmxZa4maBQRZTRMR-|f$SjC4k`x0-^ZZX??2uJVz`Iv~o-ZV!;Mvbkkzv08TO$G8+S&qeaP#kfAUy zu$@ofbf&}A*vs)3So@xiI2#5>KHA^Mq;y)zgHEazs5xiIXo}NbHf8{r2%7-Ks=@Rr zjX6?cED|#d`MhpW?^C*QVI1(cALz!Q51JK8XLKEC1t@mdI*cAbytPtYreV1#ZhPJ> z2Av1RGYlO20)Sk-pr8vr5$rUkk)b3w0V1pdt6?s213L0veR*DyZ_0X9K4h+e2 z@nYDm#enwc+A+bCuu*cYVYwq>tj1d4*lN(~;K*u_duFVq9agSjgKGn^%_+dO9ZvNP z9C`g%s;}3;S|!Ebh(kTZ#vF*{{ijsfV0g|k;5tr#vCV|B%C(_|=(c;&ZS!kyZM?bh z*5==A=J>X|9NSY0?Qv0Z?R!7J)_r~7^?h^uK3RS1nVZktR_=G~zY|zEJb3r;VCjwF zg~Mm=9X`YHuXXm!)jmMqmKO>CUaaBYH^9HOR}Q^@XzoR)PZXb)%$ zHc$!;EJmAW`fEVsQd+`1AJO;iT>2-{cfS}Ag9#s~P=kY$-pQO}vcg0{Qba=jxEa=q zy~A`fK8W?+k2ii6cSdpR6c&vGr|9g5F;zq%vyDsiPzZVUKcL<0?j<^u+7I*N7rp$h zGo&gjN1ZCR#yGYnJyo1?xT8CCAz5YJ11D=7zsB$f{b+5)Hb@qC#a3hQ`>TCTY$l}J zDx9m99jx-^>Mc0xu4f>6JITUSHWvx=-Xwgd`X}eArE17l|9p^~o)w$h2l-#chlCQO z!W?#LOw`qLM&2q1A`lkN;TXI!XdRR+IRJbcPz^&jIGJNvX&T5^p|+q!N_L#sl3A&u z@CL}!XZ!L90T8%EcD|d4J6@cbfx^p)1CzOfiH=U9b?FT0GI#iM(gm~wA6m~(gG3U2 zqOOQ1Ldyw|fCs<=IL(MA@Qw(aR1LQw35Pa9F~)m|N9@SZ@aZGNspF>yjunHB1a{C7 z0c)BsOTq=aLD42vL)UEIgksr#@O>r9XpuA!6Hth zDa@_Wg$H~w{~1o;g2sfcJ1_5<4PQR=$?)&mK70Pt=Rb>o8o#rt+pl7ET1Zxd zrdXh1>0huMj_TUD3IGGAA?8G@mAJuUVWksL1`m3YpuwEZ!F>c38CMOHm>Ney1CARo zk9&9Al>-0~tW0Hez!ncJ^;8;xZ#R7W*eUL`1G$I?axM_bD+Wcbz?;StoDh+7qo9Om zvNBGL5Kk#6SP=tAWx^@ueZeo6$MlPlybk!{6bnd)$fpVylcJ1f0l-*S6A~Kgq$*{J zMRB&t+peNvXT-r$a-bcz9L``})FG!A5Fn*Et0Z@EjK^#oMJPft5ZeSLG%G=b3>!Qm z4i8bJ7hDnn1oVrU%o_@V0&&*1a?S~G+u+oau=Pc0Fpn+VrA(^XHXi%{H{{JSiVlN- zBGeI)CL4&vO**Da)TNwYzIOz|0h`>}zE0Dn|`hUE}Q$4X;vaEOLPtGL+Y zE>y^+Jg?B8hTrS1Gis zK0vXz9NSk4?fZ||hPl<{*mE}=U#SsM3@=47yx|DiX>R@S)sJ4k`uala_Pedy zZ~Ua(nkcW@HFNyyhE)i+%uOxC58jO*yubBet);nF<~Piv0nhGji`#bI2;=Pe_9dUU zseRe!r@C=sYC%%ohcJyi@vMRz7FAK?N;oz`nL1~3oLI4w zbs`q6r3(n_YH0#-iaa}NjWim^cI6@*?Reyo@Y(;`H4IS_4B4`2+lMz|w-Bsnvnjm) z6k--g71{yB79@-FN!!nT!|#c**&FH>R=hsLa}`V0+{V}KL%|n-n~2?wHHZoZ_J6MM z!xu8Y`CWzyB0&r@yxhs_k2GsII@<`-sy<=@(JGC~@=ZiE^`_?(Jb(kZHpuNqRMuJF zW{1!AlLe_gtM~<#*lZ}3LPpg}rR>&62jtx4gCP%OMp)dyE;lW1>%Je_vbeYBlK*d- zxcsO7e_oHDl+f|reVRU1Y~(NF5hwjHjqZD1iE#u4{to}@>_-CK4+QLp=;LsspC3Ml zkB+PF&OUk3nEj^ZUty+^HS~@gb4Xn-K(1Lvw)FVUCufK&UE0Wr`7wC=95XygUa&m%2k#E+lJq-O`j6g}h` z7GDAFHVdGU6gzNmofy=4T1@9bvn5Jp!2cfSa8-eP02L*BrsJn3oB=N#Dz2}AJP}YB zXqkr#iY*BFDHfm!q(T}aXTRu+?;5u~CNd>{!cjY%(}BvmrlyTvG-!0t?hx%rA8j8# zS>Q^^!+hWl$LLWX?M~41AeY;;{aML`Yd=~W@@KhEbLCyVchaTAiSqiB z?>Alwe$~=CJN(h;)zKTT-R}LTw?6M(I3V6VAb!~;ez{#Nw+vkhE=F5sk6u2o*t&jh z$KUqf=v~;}b9Z~smuq@1^?tRQEw1jE-#)*tlsJ5+^-lUuk=6+C!H8D_#rr1(Y zU_0i0fB(+>P-)8xm;6_n-fxL9T(dPR1&B7Va8E?Ut5DMPcU>P_btp=7=v5~y% z2@HRea1zKdvU<#W5;GVUwg?4Z-kXQ!7VrU+bFLzoEp?)*sO0oX$|xInmz313#`t;c z&vjJbdyX4=j`5g11?CXvLw*3?-^k!A=ehg9lNrQkI4BUEs(8{pRD6QI!*N=FG}%%0 zCORXW@o*Qk(}~?lPC(BgG?Y-KSh5orR4(G17P!3J&m}IWrn_=do;;bnLP>7wo@gZc z=E%Y$k9-g(hEAVy$fsmX0L36zz`8?FSgSt6MR|7eCNnEN>r zF;7aF@Yp4Rq6CjC;w-U>Z14~zFf|^nMDaqN{1_xIkc0+i%3!jHS0M7}d}n~cz%8fI z?8h+i36`qhJAle3MdaioYp=ioPV?^ooP>GZ3!-QVTx?{pIYPUiEObUVKc#7t5R^b? zguV6O1>ln*FA}0i*qe6iYp2Cm`+9qiok|TJ89sVEb@JFxhJY#LOxd0bwrA2|J5|q% z0s#3GY{Zx<%Zg^CQSjIJo4c@^5tg16wzOYA`|;UpqxYlzACF!go_%9py8rUYyYZ8k zdoLaNp!eg^d(r-qzyH6>(SDO6njfy}4$cR=gB++1l0gN*GI zHok#A)qq=U;4>e=z$b(Pj|%hj4-fNl*OCJiAF3V3!Ye8orrkJCr60vTJ9NN-)`JES zPeK9H#BRyw@pvA1BOd>^8w5}L?}hNc(l7dN!iN78+LmHMW9&-L2R)^AFP5V(&4iXB zLer`%Cq6h)TK`hH@#UHDw?TZ^^zBQ?JwM!uWXZ#I2)8~Q^lbEio?-Xx0ghMOg$>*8 PZA+HdkK7YlIZFKx7}5oj diff --git a/mcp/stoat_mcp/guards.py b/mcp/stoat_mcp/guards.py index bfbb176d..cc32ad4b 100644 --- a/mcp/stoat_mcp/guards.py +++ b/mcp/stoat_mcp/guards.py @@ -131,21 +131,31 @@ def check_host_path(path: str, vm: str, data_root: str | os.PathLike[str] | None class RateLimiter: - """A token bucket per tool name. + """A token bucket per tool name, plus one bucket every tool shares. The MCP spec makes rate limiting a server MUST. The numbers here are a starting point rather than a tuned value: generous enough that ordinary agent work never notices, tight enough that a runaway loop calling `create` a thousand times stops being the host's problem. + The shared bucket is what bounds the server as a whole. Per-tool buckets + alone let a caller burst `capacity` times against each of ~20 tools, so + the real ceiling was 20x the number anyone read off this class. + Not thread-safe on purpose; the stdio server is single-threaded, and a lock here would imply a concurrency story that does not exist. """ - def __init__(self, capacity: int = 30, refill_per_second: float = 0.5) -> None: - if capacity < 1: + def __init__( + self, + capacity: int = 30, + refill_per_second: float = 0.5, + total_capacity: int = 60, + total_refill_per_second: float = 2.0, + ) -> None: + if capacity < 1 or total_capacity < 1: raise ValueError("capacity must be at least 1, or nothing can ever run") - if refill_per_second <= 0: + if refill_per_second <= 0 or total_refill_per_second <= 0: # A bucket that never refills is a bucket that permanently bricks # the tool after `capacity` calls. Refusing at construction beats # discovering it in production, and it is why the message below @@ -153,19 +163,54 @@ def __init__(self, capacity: int = 30, refill_per_second: float = 0.5) -> None: raise ValueError("refill_per_second must be positive") self.capacity = capacity self.refill_per_second = refill_per_second + self.total_capacity = total_capacity + self.total_refill_per_second = total_refill_per_second self._buckets: dict[str, tuple[float, float]] = {} def check(self, tool: str, now: float | None = None) -> None: - """Consume one token for `tool`, raising if the bucket is empty.""" + """Consume one token for `tool` and one shared token. + + Both buckets are read before either is charged. A call refused by one + must not spend from the other: a hot tool hitting its own limit would + otherwise drain the shared bucket and starve every other tool. + """ now = time.monotonic() if now is None else now - tokens, last = self._buckets.get(tool, (float(self.capacity), now)) - tokens = min(self.capacity, tokens + (now - last) * self.refill_per_second) + tool_tokens = self._read(tool, self.capacity, self.refill_per_second, now, tool) + total_tokens = self._read( + "", self.total_capacity, self.total_refill_per_second, now, "the server" + ) + self._buckets[tool] = (tool_tokens - 1.0, now) + self._buckets[""] = (total_tokens - 1.0, now) + + def _read( + self, key: str, capacity: int, refill: float, now: float, subject: str + ) -> float: + """Return key's token count at `now`, raising when it is below one.""" + tokens, last = self._buckets.get(key, (float(capacity), now)) + tokens = min(capacity, tokens + (now - last) * refill) if tokens < 1.0: - wait = (1.0 - tokens) / self.refill_per_second + wait = (1.0 - tokens) / refill raise GuardRejection( - f"rate limit reached for {tool}; retry in about {wait:.0f}s" + f"rate limit reached for {subject}; retry in about {wait:.0f}s" ) - self._buckets[tool] = (tokens - 1.0, now) + return tokens + + +def check_flag_free(values: list[str], what: str) -> list[str]: + """Refuse a value that kong would read as a flag. + + `forward` and `check_recipes` splat their list arguments into argv as + positionals. `forward(pairs=["--clear"])` reached kong as the --clear + flag and wiped the VM's forwards, from a call that passed clear=False. + Nothing escapes the process (there is no shell), so this is argv + confusion, and refusing a leading dash closes it. + """ + for v in values: + if not isinstance(v, str) or not v.strip(): + raise GuardRejection(f"{what} contains an empty value") + if v.startswith("-"): + raise GuardRejection(f"{what} value {v!r} may not start with a dash") + return values def strip_forbidden(patch: dict[str, object]) -> dict[str, object]: diff --git a/mcp/stoat_mcp/server.py b/mcp/stoat_mcp/server.py index ba6eb0ec..a7b320ca 100644 --- a/mcp/stoat_mcp/server.py +++ b/mcp/stoat_mcp/server.py @@ -7,12 +7,16 @@ is a sign the layering is wrong (docs/design/mcp-server.md, "Layering"), and the fix belongs in the Go core, not here. -Four things are enforced independently of what an MCP client does, because +Five things are enforced independently of what an MCP client does, because the protocol guarantees none of them: rate limiting (RateLimiter.check, first thing every tool does), path confinement (guards.check_host_path), -name validation (guards.check_vm_name), and allow_exec (checked here, since -core.Exec deliberately does not enforce it and core is a library the TUI and -CLI also call without wanting that restriction). +name validation (guards.check_vm_name), argv-flag confusion +(guards.check_flag_free), and allow_exec (checked here, since core.Exec +deliberately does not enforce it and core is a library the TUI and CLI also +call without wanting that restriction). + +allow_exec covers apply_recipes too. A recipe body is arbitrary guest code, +so a VM that refuses exec refuses an apply as well. Deliberately absent, not merely gated, per docs/design/mcp-server.md §4: no `share` parameter on any tool, no bring-your-own image path, no @@ -22,6 +26,7 @@ from __future__ import annotations import functools +import subprocess import sys from collections.abc import Callable, Sequence from typing import Any, Literal, TypeVar @@ -31,7 +36,14 @@ from .client import Client, argv_for_bool from .errors import ContractMismatch, GuardRejection, StoatCrashed, StoatError -from .guards import RateLimiter, check_host_path, check_image_id, check_vm_name, strip_forbidden +from .guards import ( + RateLimiter, + check_flag_free, + check_host_path, + check_image_id, + check_vm_name, + strip_forbidden, +) mcp: FastMCP = FastMCP("stoat") @@ -89,6 +101,11 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: raise ToolError(f"{e.code}: {e.message}") from e except StoatCrashed as e: raise ToolError(f"stoat exited {e.returncode} without answering") from e + except subprocess.TimeoutExpired as e: + # Client.run kills the process in its own finally, so this is + # already cleaned up. Without this arm it surfaces as a bare + # traceback that fastmcp masks. + raise ToolError(f"stoat did not finish within {e.timeout:.0f}s") from e return wrapper # type: ignore[return-value] @@ -114,8 +131,16 @@ def _recipes_flag(recipes: Sequence[str] | None) -> list[str]: return ["--recipes", ",".join(recipes)] +# The stdio transport is single-threaded: one blocked tool blocks the server. +# `wait` is the only tool that blocks on purpose, so it is the only one that +# needs a ceiling a caller cannot raise. +MAX_WAIT_SECONDS = 600 + +MAX_LOG_LINES = 2000 + + def _require_exec_allowed(vm: str) -> None: - """Refuse exec/copy_to/copy_from on a VM created with --allow-exec=false. + """Refuse exec, copy and apply on a VM created with --allow-exec=false. core.Exec does not enforce this itself (core is a library the TUI and CLI also call, and a blanket refusal there would be the wrong layer); @@ -206,6 +231,7 @@ def list_recipes(os: str | None = None, backend: str | None = None) -> dict[str, def check_recipes( recipes: list[str], os: str, backend: str | None = None ) -> dict[str, Any]: + check_flag_free(recipes, "recipes") argv = ["check-recipes", *recipes, "--os", os, *_flag("backend", backend)] return get_client().run(*argv) @@ -215,13 +241,17 @@ def check_recipes( description=( "Tail one VM's log: its qemu console output (default), or its most " "recent recipe-apply log. Always scoped to a single named VM; there is " - "no way to read stoat's own global log through this tool. Read-only." + "no way to read stoat's own global log through this tool. 'n' is " + "capped at 2000 lines. Read-only." ), annotations={"readOnlyHint": True, "destructiveHint": False}, ) @_guarded("logs") def logs(vm: str, which: Literal["console", "apply"] = "console", n: int = 50) -> dict[str, Any]: vm = check_vm_name(vm) + # A console log grows without bound, and the whole tail comes back in one + # response. Clamping beats handing an agent a payload it cannot read. + n = max(1, min(n, MAX_LOG_LINES)) return get_client().run("logs", vm, "--which", which, "-n", str(n)) @@ -317,22 +347,49 @@ def stop(vm: str) -> dict[str, Any]: return get_client().run("down", vm) +@mcp.tool( + name="plan_recipes", + description=( + "Report what apply_recipes would do to a VM, without running anything: " + "one entry per recipe with 'run' or 'skip' and the reason. Computed on " + "the host, so it works on a stopped VM. Read-only." + ), + annotations={"readOnlyHint": True, "destructiveHint": False}, +) +@_guarded("plan_recipes") +def plan_recipes(vm: str, only: list[str] | None = None) -> dict[str, Any]: + vm = check_vm_name(vm) + return get_client().run(*_apply_argv(vm, only), "--dry-run") + + @mcp.tool( name="apply_recipes", description=( "Run a VM's own configured recipes over ssh (or, optionally, a subset " - "of them). Mutating: runs arbitrary recipe scripts inside the guest, " - "which is only reversible to whatever extent the recipe itself is." + "of them). Call plan_recipes first to see what this would do. Refused " + "on a VM created with --allow-exec=false, since a recipe is arbitrary " + "guest code. Mutating: runs arbitrary recipe scripts inside the guest, " + "which is only reversible to whatever extent the recipe itself is. " + "Reaches outside this process (openWorldHint)." ), - annotations={"readOnlyHint": False, "destructiveHint": False}, + annotations={"readOnlyHint": False, "destructiveHint": True, "openWorldHint": True}, ) @_guarded("apply_recipes") def apply_recipes(vm: str, only: list[str] | None = None) -> dict[str, Any]: vm = check_vm_name(vm) + # A recipe body is arbitrary guest code, so this is exec by another name + # and answers to the same opt-out. + _require_exec_allowed(vm) + return get_client().run(*_apply_argv(vm, only)) + + +def _apply_argv(vm: str, only: list[str] | None) -> list[str]: + """Shared argv for apply and its dry run: `apply [--only n]...`.""" + names = check_flag_free(list(only or []), "only") argv = ["apply", vm] - for name in only or []: + for name in names: argv += ["--only", name] - return get_client().run(*argv) + return argv @mcp.tool( @@ -420,10 +477,10 @@ def snapshot(vm: str, tag: str) -> dict[str, Any]: name="restore", description=( "Roll a VM's disk back to a previously saved snapshot tag, discarding " - "everything written since. Mutating and only reversible if another " + "everything written since. Destructive and only reversible if another " "snapshot was taken after the one being restored to." ), - annotations={"readOnlyHint": False, "destructiveHint": False}, + annotations={"readOnlyHint": False, "destructiveHint": True}, ) @_guarded("restore") def restore(vm: str, tag: str) -> dict[str, Any]: @@ -448,7 +505,7 @@ def forward(vm: str, pairs: list[str] | None = None, clear: bool = False) -> dic if clear: argv.append("--clear") else: - argv += list(pairs or []) + argv += check_flag_free(list(pairs or []), "pairs") return get_client().run(*argv) @@ -458,7 +515,8 @@ def forward(vm: str, pairs: list[str] | None = None, clear: bool = False) -> dic "Block until a VM reaches a state: reachable (sshd answering), applied " "(most recent recipe run finished), or stopped (qemu no longer " "running). The bound is 'timeout_seconds', a plain integer count of " - "seconds, not a duration string. Fails immediately, rather than waiting " + "seconds, not a duration string, capped at 600. Fails immediately, " + "rather than waiting " "out the timeout, for a state the VM can never reach. Mutating only in " "that it blocks the caller; it does not itself change the VM." ), @@ -471,6 +529,7 @@ def wait( timeout_seconds: int = 120, ) -> dict[str, Any]: vm = check_vm_name(vm) + timeout_seconds = max(1, min(timeout_seconds, MAX_WAIT_SECONDS)) argv = ["wait", vm, "--until", until, "--timeout", f"{timeout_seconds}s"] # The subprocess-level timeout must exceed stoat's own --timeout, or the # process gets killed before stoat has a chance to answer with "timeout" @@ -592,11 +651,14 @@ def _verify_cp_local(data: dict[str, Any], authorised_local: str) -> None: """Post-verify that `cp` acted on the exact path this server authorised. `cp` echoes back the resolved absolute local path it used (json.md's - `cp` row). Guard-time validation establishes that the path we ASKED for - was safe; this establishes that the path stoat actually ACTED on is the - same one, catching any divergence between the two resolvers (guard-time - uses os.path.realpath, stoat's own resolveLocal does not) rather than - trusting them to agree. + `cp` row). This catches a divergence between the two resolvers: the guard + uses os.path.realpath, stoat's own resolveLocal does not. + + It does not catch a component of the path swapped to a symlink between + the guard and scp's open, and it runs after the copy either way. The + defence there is QEMU's mapped-xattr, which stores a guest symlink as an + extended attribute instead of a real host one (mcp-server.md section 8). + That guard and this one are independent; neither covers the other. """ actual = data.get("local") if actual != authorised_local: diff --git a/mcp/tests/test_guards.py b/mcp/tests/test_guards.py index a7b97f8b..6297eab6 100644 --- a/mcp/tests/test_guards.py +++ b/mcp/tests/test_guards.py @@ -18,6 +18,7 @@ from stoat_mcp.errors import GuardRejection from stoat_mcp.guards import ( RateLimiter, + check_flag_free, check_host_path, check_image_id, check_vm_name, @@ -323,3 +324,52 @@ def test_rate_limiter_refills_over_time(): rl.check("create", now=0.1) # A full second later, one token has refilled. rl.check("create", now=1.0) + + +def test_rate_limiter_shared_bucket_bounds_every_tool_together(): + # Per-tool buckets alone let a caller burst capacity times against each + # of ~20 tools. The shared bucket is what caps the server as a whole. + rl = RateLimiter( + capacity=10, refill_per_second=1e-9, total_capacity=2, total_refill_per_second=1e-9 + ) + rl.check("list_vms", now=0.0) + rl.check("vm_status", now=0.0) + with pytest.raises(GuardRejection): + rl.check("doctor", now=0.0) + + +def test_rate_limiter_refused_tool_does_not_spend_a_shared_token(): + # One hot tool hitting its own limit must not starve every other tool. + rl = RateLimiter( + capacity=1, refill_per_second=1e-9, total_capacity=5, total_refill_per_second=1e-9 + ) + rl.check("create", now=0.0) + for _ in range(3): + with pytest.raises(GuardRejection): + rl.check("create", now=0.0) + rl.check("destroy", now=0.0) + rl.check("doctor", now=0.0) + + +# --------------------------------------------------------------------------- +# check_flag_free +# --------------------------------------------------------------------------- + + +def test_check_flag_free_rejects_a_kong_flag(): + # forward(pairs=["--clear"]) reached kong as the clear flag and wiped the + # VM's forwards, from a call that passed clear=False. + with pytest.raises(GuardRejection): + check_flag_free(["--clear"], "pairs") + + +def test_check_flag_free_rejects_a_short_flag_and_empty_values(): + with pytest.raises(GuardRejection): + check_flag_free(["-n"], "pairs") + with pytest.raises(GuardRejection): + check_flag_free([" "], "pairs") + + +def test_check_flag_free_passes_ordinary_values(): + assert check_flag_free(["8080:80", "2222:22"], "pairs") == ["8080:80", "2222:22"] + assert check_flag_free([], "pairs") == [] diff --git a/mcp/tests/test_server.py b/mcp/tests/test_server.py index 9dba7914..52b6e4e9 100644 --- a/mcp/tests/test_server.py +++ b/mcp/tests/test_server.py @@ -12,6 +12,7 @@ import asyncio import pytest +from fastmcp.exceptions import ToolError from stoat_mcp import server @@ -24,18 +25,25 @@ "check_recipes": {"readOnlyHint": True, "destructiveHint": False}, "logs": {"readOnlyHint": True, "destructiveHint": False}, "doctor": {"readOnlyHint": True, "destructiveHint": False}, + "plan_recipes": {"readOnlyHint": True, "destructiveHint": False}, "create": {"readOnlyHint": False, "destructiveHint": False}, "start": {"readOnlyHint": False, "destructiveHint": False}, "stop": {"readOnlyHint": False, "destructiveHint": False}, - "apply_recipes": {"readOnlyHint": False, "destructiveHint": False}, "update": {"readOnlyHint": False, "destructiveHint": False}, "clone": {"readOnlyHint": False, "destructiveHint": False}, "snapshot": {"readOnlyHint": False, "destructiveHint": False}, - "restore": {"readOnlyHint": False, "destructiveHint": False}, "forward": {"readOnlyHint": False, "destructiveHint": False}, "wait": {"readOnlyHint": False, "destructiveHint": False}, "destroy": {"readOnlyHint": False, "destructiveHint": True}, "prune": {"readOnlyHint": False, "destructiveHint": True}, + "restore": {"readOnlyHint": False, "destructiveHint": True}, + # A recipe body is arbitrary guest code, so apply_recipes carries exec's + # hints and exec's allow_exec check. + "apply_recipes": { + "readOnlyHint": False, + "destructiveHint": True, + "openWorldHint": True, + }, "exec": {"readOnlyHint": False, "destructiveHint": True, "openWorldHint": True}, "copy_to": {"readOnlyHint": False, "destructiveHint": True, "openWorldHint": True}, "copy_from": {"readOnlyHint": False, "destructiveHint": True, "openWorldHint": True}, @@ -124,3 +132,58 @@ def test_no_tool_description_contains_an_em_dash(): tools = _tools() for name, tool in tools.items(): assert "—" not in (tool.description or ""), name + + +class _FakeClient: + """Records the argv each tool builds and answers from a fixed dict.""" + + def __init__(self, answers: dict[str, object] | None = None) -> None: + self.calls: list[tuple[str, ...]] = [] + self.answers = answers or {} + + def run(self, *args: str, **_: object) -> dict[str, object]: + self.calls.append(args) + return dict(self.answers.get(args[0], {})) + + +@pytest.fixture +def fake_client(monkeypatch): + client = _FakeClient({"get": {"vm": {"allow_exec": True}}}) + monkeypatch.setattr(server, "get_client", lambda: client) + return client + + +def test_apply_recipes_refuses_a_vm_with_allow_exec_false(monkeypatch): + # A recipe body is arbitrary guest code. allow_exec=false blocked exec + # and copy while apply ran scripts as root, which made the opt-out a + # partial one. + client = _FakeClient({"get": {"vm": {"allow_exec": False}}}) + monkeypatch.setattr(server, "get_client", lambda: client) + with pytest.raises(ToolError): + server.apply_recipes("work") + assert all(c[0] != "apply" for c in client.calls) + + +def test_plan_recipes_runs_a_dry_run_and_needs_no_exec_permission(monkeypatch): + client = _FakeClient({"get": {"vm": {"allow_exec": False}}}) + monkeypatch.setattr(server, "get_client", lambda: client) + server.plan_recipes("work") + assert client.calls == [("apply", "work", "--dry-run")] + + +def test_wait_clamps_the_timeout(fake_client): + server.wait("work", timeout_seconds=10**6) + argv = fake_client.calls[-1] + assert argv[argv.index("--timeout") + 1] == f"{server.MAX_WAIT_SECONDS}s" + + +def test_logs_clamps_the_line_count(fake_client): + server.logs("work", n=10**6) + argv = fake_client.calls[-1] + assert argv[argv.index("-n") + 1] == str(server.MAX_LOG_LINES) + + +def test_forward_refuses_a_pair_that_kong_reads_as_a_flag(fake_client): + with pytest.raises(ToolError): + server.forward("work", pairs=["--clear"]) + assert fake_client.calls == []