From 6a3b400d8ae479c16dc7e27034c642ec73ec288a Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Fri, 28 Aug 2026 10:02:54 +0530 Subject: [PATCH 1/7] feat(sandbox): add offload, matrix and self commands with safety guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce `sandbox offload` and `sandbox matrix` subcommands for one‑shot offload and parallel fork workflows. Add `sandbox self` for in‑sandbox signaling. Implement guards, retry logic, and improved error handling for lifecycle, connection setup, and disk operations. --- .secrets.baseline | 18 +- cmd/root/root.go | 11 + cmd/root/usage_error.go | 169 +++++++++++ cmd/root/usage_error_test.go | 113 +++++++ cmd/sandbox/compose.go | 273 +++++++++++++++++ cmd/sandbox/compose_test.go | 303 +++++++++++++++++++ cmd/sandbox/create.go | 1 + cmd/sandbox/edit.go | 1 + cmd/sandbox/egress_preset.go | 76 +++++ cmd/sandbox/fork.go | 223 ++++++++++++-- cmd/sandbox/guards.go | 91 ++++++ cmd/sandbox/guards_test.go | 125 ++++++++ cmd/sandbox/lifecycle_test.go | 342 +++++++++++++++++++++ cmd/sandbox/matrix.go | 474 ++++++++++++++++++++++++++++++ cmd/sandbox/offload.go | 343 +++++++++++++++++++++ cmd/sandbox/pull.go | 8 + cmd/sandbox/push.go | 7 + cmd/sandbox/sandbox.go | 3 + cmd/sandbox/self.go | 206 +++++++++++++ cmd/sandbox/stage.go | 287 ++++++++++++++++++ internal/api/client.go | 73 ++++- internal/api/client_retry_test.go | 117 ++++++++ internal/api/sandbox.go | 15 +- internal/api/sandbox_client.go | 1 + 24 files changed, 3240 insertions(+), 40 deletions(-) create mode 100644 cmd/root/usage_error.go create mode 100644 cmd/root/usage_error_test.go create mode 100644 cmd/sandbox/compose.go create mode 100644 cmd/sandbox/compose_test.go create mode 100644 cmd/sandbox/egress_preset.go create mode 100644 cmd/sandbox/guards.go create mode 100644 cmd/sandbox/guards_test.go create mode 100644 cmd/sandbox/lifecycle_test.go create mode 100644 cmd/sandbox/matrix.go create mode 100644 cmd/sandbox/offload.go create mode 100644 cmd/sandbox/self.go create mode 100644 cmd/sandbox/stage.go create mode 100644 internal/api/client_retry_test.go diff --git a/.secrets.baseline b/.secrets.baseline index 55812ed..b15bca8 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -90,6 +90,10 @@ { "path": "detect_secrets.filters.allowlist.is_line_allowlisted" }, + { + "path": "detect_secrets.filters.common.is_baseline_file", + "filename": ".secrets.baseline" + }, { "path": "detect_secrets.filters.common.is_ignored_due_to_verification_policies", "min_level": 2 @@ -129,8 +133,8 @@ "filename": "cmd/env/set.go", "hashed_secret": "ec417f567082612f8fd6afafe1abcab831fca840", "is_verified": false, - "is_secret": false, - "line_number": 24 + "line_number": 24, + "is_secret": false } ], "cmd/oauth/helpers.go": [ @@ -139,8 +143,8 @@ "filename": "cmd/oauth/helpers.go", "hashed_secret": "a587be0a364eab71821821cfc5226eb04853224f", "is_verified": false, - "is_secret": false, - "line_number": 155 + "line_number": 155, + "is_secret": false } ], "internal/api/client.go": [ @@ -149,10 +153,10 @@ "filename": "internal/api/client.go", "hashed_secret": "b19a5a3616bf8b53864ca6162b5f1f6a8c61ab94", "is_verified": false, - "is_secret": false, - "line_number": 97 + "line_number": 222, + "is_secret": false } ] }, - "generated_at": "2026-04-02T07:26:00Z" + "generated_at": "2026-08-28T04:32:28Z" } diff --git a/cmd/root/root.go b/cmd/root/root.go index 8a21cdc..84a2944 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -108,6 +108,15 @@ func NewApp() *cli.App { if cmd == "" || cmd == "login" || cmd == "logout" || cmd == "version" || cmd == "ask" || cmd == "upgrade" { return nil } + // `sandbox self` talks to the guest agent on loopback inside the + // sandbox, which takes no credential by design. Demanding a + // login here would make the command unusable exactly where it is + // meant to run: inside a sandbox, which has no stored token. + if cmd == "sandbox" || cmd == "sb" { + if sub := c.Args().Get(1); sub == "self" { + return nil + } + } // CREATEOS_API_KEY env var (or --api-key flag) — injected by Stripe Projects if apiKey := c.String("api-key"); apiKey != "" { @@ -228,6 +237,8 @@ func NewApp() *cli.App { }, } installTrailingHelpGuards(app.Commands) + installUsageErrorHelp(app) + installCommandSuggestions(app) return app } diff --git a/cmd/root/usage_error.go b/cmd/root/usage_error.go new file mode 100644 index 0000000..2098537 --- /dev/null +++ b/cmd/root/usage_error.go @@ -0,0 +1,169 @@ +package root + +import ( + "fmt" + "os" + "strings" + + "github.com/urfave/cli/v2" +) + +// urfave/cli parses global flags only before the first command name, so +// `createos sandbox shapes -o json` dies with "flag provided but not +// defined: -o" — the flag exists, it is simply in the wrong place. The +// message names neither fact, and the same shape has cost real round trips +// in practice. +// +// installUsageErrorHelp attaches the handler to every command in the tree. +// OnUsageError lives on Command and is NOT inherited from the app, so a +// handler set only at the top never runs for `sandbox shapes -o json` — +// the exact case worth catching. +func installUsageErrorHelp(app *cli.App) { + globals := globalFlagNames(app) + handler := func(_ *cli.Context, err error, _ bool) error { + name := undefinedFlagName(err) + if name == "" || !globals[strings.TrimLeft(name, "-")] { + return err + } + return fmt.Errorf("%w\n\n %s is a global flag, so it has to come BEFORE the command:\n %s", + err, name, correctedCommandLine(name)) + } + app.OnUsageError = handler + setUsageErrorHandler(app.Commands, handler) +} + +func setUsageErrorHandler(commands []*cli.Command, handler cli.OnUsageErrorFunc) { + for _, cmd := range commands { + if cmd == nil { + continue + } + if cmd.OnUsageError == nil { + cmd.OnUsageError = handler + } + setUsageErrorHandler(cmd.Subcommands, handler) + } +} + +func globalFlagNames(app *cli.App) map[string]bool { + names := make(map[string]bool) + for _, f := range app.Flags { + for _, n := range f.Names() { + names[n] = true + } + } + return names +} + +// undefinedFlagName pulls the flag out of the flag package's message, +// which reads: `flag provided but not defined: -o`. +func undefinedFlagName(err error) string { + const marker = "flag provided but not defined: " + msg := err.Error() + i := strings.Index(msg, marker) + if i < 0 { + return "" + } + name := strings.TrimSpace(msg[i+len(marker):]) + if cut := strings.IndexAny(name, " \n"); cut >= 0 { + name = name[:cut] + } + return name +} + +// correctedCommandLine rewrites what the user typed with the misplaced +// global flag moved to the front, so the fix can be copied straight back +// into the terminal. +func correctedCommandLine(flagName string) string { + bare := strings.TrimLeft(flagName, "-") + args := os.Args[1:] + + moved := make([]string, 0, 2) + rest := make([]string, 0, len(args)) + for i := 0; i < len(args); i++ { + a := args[i] + trimmed := strings.TrimLeft(a, "-") + if trimmed == bare || strings.HasPrefix(trimmed, bare+"=") { + moved = append(moved, a) + // A value-taking flag written as `-o json` carries its value + // in the next argument; move that too or the corrected line + // is wrong. + if !strings.Contains(a, "=") && i+1 < len(args) && !strings.HasPrefix(args[i+1], "-") { + i++ + moved = append(moved, args[i]) + } + continue + } + rest = append(rest, a) + } + if len(moved) == 0 { + return "createos " + strings.Join(args, " ") + } + return "createos " + strings.Join(append(moved, rest...), " ") +} + +// installCommandSuggestions replaces urfave's bare "No help topic for +// 'ssh'" with the nearest real command. Agents and people both guess verb +// names, and a guess that lands one edit away from a real command should +// not cost a round trip to the help output. +func installCommandSuggestions(app *cli.App) { + app.CommandNotFound = func(c *cli.Context, name string) { + // The hook fires for a subcommand too ("createos sandbox ssh"), and + // there the useful candidates are that group's subcommands, not the + // top-level verbs. Searching the wrong list is worse than staying + // quiet: it points at something unrelated. + candidates, prefix := app.Commands, "" + if cmd := c.Command; cmd != nil && len(cmd.Subcommands) > 0 { + candidates, prefix = cmd.Subcommands, cmd.Name+" " + } + noun := "command" + if prefix != "" { + noun = "subcommand" + } + fmt.Fprintf(os.Stderr, "createos %s: %q is not a %s.\n", strings.TrimSpace(prefix), name, noun) + if best := nearestCommand(candidates, name); best != "" { + fmt.Fprintf(os.Stderr, "\n Did you mean:\n createos %s%s\n", prefix, best) + } + fmt.Fprintf(os.Stderr, "\n See everything with:\n createos %s--help\n", prefix) + } +} + +// nearestCommand returns the closest command name within a small edit +// distance, or "" when nothing is close enough. The cap matters: a wild +// guess should get the help pointer, not a confidently wrong suggestion. +func nearestCommand(commands []*cli.Command, name string) string { + name = strings.ToLower(name) + best, bestDist := "", 3 + for _, cmd := range commands { + if cmd.Hidden { + continue + } + for _, candidate := range append([]string{cmd.Name}, cmd.Aliases...) { + if d := editDistance(name, strings.ToLower(candidate)); d < bestDist { + best, bestDist = cmd.Name, d + } + } + } + return best +} + +// editDistance is Levenshtein over two short command names, with one row +// of state rather than a full matrix. +func editDistance(a, b string) int { + prev := make([]int, len(b)+1) + for j := range prev { + prev[j] = j + } + for i := 1; i <= len(a); i++ { + cur := make([]int, len(b)+1) + cur[0] = i + for j := 1; j <= len(b); j++ { + cost := 1 + if a[i-1] == b[j-1] { + cost = 0 + } + cur[j] = min(prev[j]+1, min(cur[j-1]+1, prev[j-1]+cost)) + } + prev = cur + } + return prev[len(b)] +} diff --git a/cmd/root/usage_error_test.go b/cmd/root/usage_error_test.go new file mode 100644 index 0000000..3e044a6 --- /dev/null +++ b/cmd/root/usage_error_test.go @@ -0,0 +1,113 @@ +package root + +import ( + "errors" + "os" + "testing" + + "github.com/urfave/cli/v2" +) + +func TestUndefinedFlagName(t *testing.T) { + for _, tc := range []struct { + err error + want string + }{ + {errors.New("flag provided but not defined: -o"), "-o"}, + {errors.New("flag provided but not defined: --output"), "--output"}, + {errors.New("something else entirely"), ""}, + } { + if got := undefinedFlagName(tc.err); got != tc.want { + t.Errorf("undefinedFlagName(%q) = %q, want %q", tc.err, got, tc.want) + } + } +} + +// TestCorrectedCommandLine covers the payoff: the message has to hand back +// a line the user can paste, which means moving the flag AND its value. +func TestCorrectedCommandLine(t *testing.T) { + for _, tc := range []struct { + name string + args []string + flag string + want string + }{ + { + "separate value moves with the flag", + []string{"sandbox", "shapes", "-o", "json"}, + "-o", + "createos -o json sandbox shapes", + }, + { + "joined value", + []string{"sandbox", "shapes", "--output=json"}, + "--output", + "createos --output=json sandbox shapes", + }, + { + "boolean flag has no value to move", + []string{"sandbox", "ls", "--debug"}, + "--debug", + "createos --debug sandbox ls", + }, + { + "flag already first is left alone", + []string{"-o", "json", "sandbox", "shapes"}, + "-o", + "createos -o json sandbox shapes", + }, + } { + t.Run(tc.name, func(t *testing.T) { + original := os.Args + os.Args = append([]string{"createos"}, tc.args...) + t.Cleanup(func() { os.Args = original }) + + if got := correctedCommandLine(tc.flag); got != tc.want { + t.Errorf("correctedCommandLine() = %q, want %q", got, tc.want) + } + }) + } +} + +// TestNearestCommand pins both halves: a near miss gets a suggestion, and +// a wild guess gets silence. Suggesting something unrelated is worse than +// suggesting nothing. +func TestNearestCommand(t *testing.T) { + commands := []*cli.Command{ + {Name: "shell", Aliases: []string{"sh"}}, + {Name: "fork"}, + {Name: "exec"}, + {Name: "hidden", Hidden: true}, + } + for _, tc := range []struct { + input string + want string + }{ + {"ssh", "shell"}, // one edit from the "sh" alias + {"forkk", "fork"}, // one extra character + {"exe", "exec"}, // one missing character + {"zzzzqqq", ""}, // nothing close — stay quiet + {"hidden", ""}, // hidden commands are not suggested + } { + if got := nearestCommand(commands, tc.input); got != tc.want { + t.Errorf("nearestCommand(%q) = %q, want %q", tc.input, got, tc.want) + } + } +} + +func TestEditDistance(t *testing.T) { + for _, tc := range []struct { + a, b string + want int + }{ + {"", "", 0}, + {"fork", "fork", 0}, + {"forkk", "fork", 1}, + {"ssh", "sh", 1}, + {"exec", "", 4}, + } { + if got := editDistance(tc.a, tc.b); got != tc.want { + t.Errorf("editDistance(%q, %q) = %d, want %d", tc.a, tc.b, got, tc.want) + } + } +} diff --git a/cmd/sandbox/compose.go b/cmd/sandbox/compose.go new file mode 100644 index 0000000..c56c189 --- /dev/null +++ b/cmd/sandbox/compose.go @@ -0,0 +1,273 @@ +package sandbox + +import ( + "context" + "encoding/base64" + "errors" + "fmt" + "io" + "strings" + "time" + + "github.com/urfave/cli/v2" + + "github.com/NodeOps-app/createos-cli/internal/api" +) + +// The composed verbs (offload, matrix) share one box recipe. Keeping the +// flags in one place is what stops `offload --shape` and `matrix --shape` +// from drifting apart. +const ( + composeDefaultShape = "s-1vcpu-1gb" + composeDefaultRootfs = "devbox:1" + // composeDefaultAutoPause is the backstop. If this CLI is killed + // mid-run — a closed laptop, a dropped SSH session, a CI timeout — + // nothing is left to destroy the boxes it created, and they bill + // until someone notices. Auto-pause makes the sandbox park itself. + composeDefaultAutoPause = 15 * time.Minute + // composeWorkDir is where a staged tree lands inside the sandbox. + composeWorkDir = "/work" +) + +// composeFlags are the box-shape flags shared by offload and matrix. +func composeFlags() []cli.Flag { + return []cli.Flag{ + &cli.StringFlag{Name: "shape", Value: composeDefaultShape, Usage: "Sandbox size (run 'createos sandbox shapes' to see options)"}, + &cli.StringFlag{Name: "rootfs", Value: composeDefaultRootfs, Usage: "Base image or template to boot from"}, + &cli.StringSliceFlag{Name: "env", Usage: "Environment variable for every command (repeatable): KEY=VALUE"}, + &cli.StringSliceFlag{Name: "egress", Usage: "Host the sandbox may reach (repeatable). Default: unrestricted"}, + &cli.StringSliceFlag{Name: "egress-preset", Usage: "Toolchain allowlist (repeatable): " + strings.Join(egressPresetNames(), ", ")}, + &cli.StringSliceFlag{Name: "exclude", Usage: "Path to keep out of the upload (repeatable). .gitignore is honoured already"}, + &cli.DurationFlag{Name: "auto-pause", Value: composeDefaultAutoPause, Usage: "Park an idle sandbox if this command dies before it can clean up. 0 disables"}, + &cli.DurationFlag{Name: "timeout", Usage: "Give up on a command after this long. Default: no limit"}, + } +} + +// composeOptions is composeFlags after parsing and validation. +type composeOptions struct { + Shape string + Rootfs string + Env map[string]string + Egress []string + Exclude []string + AutoPause time.Duration + Timeout time.Duration +} + +func parseComposeOptions(c *cli.Context) (*composeOptions, error) { + env, err := parseKeyValues(c.StringSlice("env")) + if err != nil { + return nil, err + } + egress, err := resolveEgress(c.StringSlice("egress-preset"), c.StringSlice("egress")) + if err != nil { + return nil, err + } + return &composeOptions{ + Shape: c.String("shape"), + Rootfs: c.String("rootfs"), + Env: env, + Egress: egress, + Exclude: c.StringSlice("exclude"), + AutoPause: c.Duration("auto-pause"), + Timeout: c.Duration("timeout"), + }, nil +} + +// checkMisplacedFlags turns a silent drop into a clear error. +// +// urfave/cli stops parsing flags at the first positional argument, so +// `matrix . --job 'x'` parses `.` and then treats --job as a plain +// argument: the job list comes back empty and nothing says why. This is a +// standing trap in this CLI — the same shape already bit `process run +// --cwd` (commit 8c1f7ac) and `sandbox shapes -o json`. +// +// A user cannot be expected to know where the parser gave up, so any +// leftover argument that names one of this command's own flags is +// reported with the corrected command line. +func checkMisplacedFlags(c *cli.Context, leftovers []string) error { + known := make(map[string]bool) + for _, f := range c.Command.Flags { + for _, n := range f.Names() { + known["--"+n] = true + known["-"+n] = true + } + } + for _, arg := range leftovers { + name, _, _ := strings.Cut(arg, "=") + if known[name] { + return fmt.Errorf( + "%s was written after the directory, so it was ignored\n\n Flags must come before the directory:\n createos sandbox %s %s ", + name, c.Command.Name, name) + } + } + return nil +} + +// parseKeyValues turns repeated KEY=VALUE flags into a map. +func parseKeyValues(pairs []string) (map[string]string, error) { + if len(pairs) == 0 { + return nil, nil + } + out := make(map[string]string, len(pairs)) + for _, p := range pairs { + k, v, ok := strings.Cut(p, "=") + if !ok || k == "" { + return nil, fmt.Errorf("--env %q is not KEY=VALUE", p) + } + out[k] = v + } + return out, nil +} + +// createComposeBox boots one sandbox to the shared recipe and waits for it +// to run. +func createComposeBox(ctx context.Context, client *api.SandboxClient, opts *composeOptions) (*api.SandboxView, error) { + // No name: these boxes are machinery with a lifetime of one command, + // and a generated name is easier to tell apart in `sandbox ls` than a + // dozen boxes all called the same thing. + req := api.SandboxCreateReq{ + Shape: opts.Shape, + Rootfs: opts.Rootfs, + Egress: opts.Egress, + Envs: opts.Env, + } + if opts.AutoPause > 0 { + secs := int(opts.AutoPause.Seconds()) + req.AutoPauseAfterSeconds = &secs + } + created, err := client.CreateSandbox(ctx, req) + if err != nil { + return nil, err + } + // Past this point the sandbox exists and is billable. Every way out + // that is not "running" has to destroy it, or a readiness timeout + // leaves a machine nobody knows about — offload and matrix only see + // the error, never the id. + sb, err := waitForStatus(ctx, client, created.ID, "running") + if err != nil { + return nil, cleanupAfterCreate(ctx, client, created.ID, err) + } + if sb.Status != "running" { + return nil, cleanupAfterCreate(ctx, client, sb.ID, + fmt.Errorf("sandbox %s came up %s, not running", sb.ID, sb.Status)) + } + return sb, nil +} + +// cleanupAfterCreate destroys a sandbox that never became usable and folds +// the outcome into the error the caller sees. The id is always named: if +// the teardown itself fails, the user needs it to clean up by hand. +func cleanupAfterCreate(ctx context.Context, client *api.SandboxClient, id string, cause error) error { + tearCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) + defer cancel() + if err := client.DestroySandbox(tearCtx, id); err != nil { + return fmt.Errorf("%w\n\n Sandbox %s was created and could not be destroyed (%w).\n It is still billable. Remove it with:\n createos sandbox rm --force %s", + cause, id, err, id) + } + return fmt.Errorf("%w\n\n Sandbox %s was destroyed", cause, id) +} + +// managedResult is one command's outcome. +type managedResult struct { + ExitCode int + Signal string + Duration time.Duration +} + +// composeStreamRetries bounds how many times runManaged reconnects to a +// live process after the output stream drops. +const composeStreamRetries = 5 + +// runManaged runs cmd as a managed process and streams its output to out. +// +// A managed process is the point. `sandbox exec` ties the command's life +// to the HTTP stream, so a connection that drops on a long quiet build — +// a laptop sleeping, a proxy idle-timeout — kills the remote command. A +// managed process keeps running on the box and keeps replayable output, so +// this function reconnects from the last sequence it saw and picks the +// output back up. That reconnect is the whole reason the composed verbs +// survive builds that print nothing for ten minutes. +func runManaged( + ctx context.Context, + client *api.SandboxClient, + sandboxID, cmd, cwd string, + env map[string]string, + out io.Writer, +) (*managedResult, error) { + start := time.Now() + proc, err := client.CreateProcess(ctx, sandboxID, api.ProcessCreateRequest{ + Cmd: "bash", + Args: []string{"-lc", cmd}, + Cwd: cwd, + Env: env, + }) + if err != nil { + return nil, fmt.Errorf("start command in %s: %w", sandboxID, err) + } + + var exitCode *int + var signal string + after := int64(0) + + for attempt := 0; ; attempt++ { + streamErr := client.ConnectProcess(ctx, sandboxID, proc.ProcessID, after, func(ev api.ProcessOutputEvent) { + switch ev.Type { + case "data": + if ev.Seq > after { + after = ev.Seq + } + if raw, decErr := base64.StdEncoding.DecodeString(ev.DataBase64); decErr == nil { + _, _ = out.Write(raw) //nolint:errcheck // a failed log write must not kill the job + } + case "exit": + exitCode = ev.ExitCode + signal = ev.Signal + } + }) + if exitCode != nil { + break + } + if ctx.Err() != nil { + return nil, ctx.Err() + } + if streamErr == nil || errors.Is(streamErr, context.Canceled) { + // Stream ended cleanly but no exit frame arrived. Ask the + // server directly rather than guessing. + done, waitErr := client.WaitProcess(ctx, sandboxID, proc.ProcessID, true, int64(pollTimeout/time.Millisecond)) + if waitErr != nil { + return nil, waitErr + } + if done.ExitCode != nil { + exitCode = done.ExitCode + signal = done.Signal + break + } + } + if attempt >= composeStreamRetries { + return nil, fmt.Errorf("lost the output stream for %s in %s after %d reconnects: %w", + proc.ProcessID, sandboxID, attempt, streamErr) + } + select { + case <-ctx.Done(): + return nil, ctx.Err() + case <-time.After(time.Second): + } + } + + return &managedResult{ExitCode: *exitCode, Signal: signal, Duration: time.Since(start)}, nil +} + +// destroyQuiet tears a sandbox down and reports failures without stopping +// the caller. A composed verb is usually already unwinding when it calls +// this, and a teardown error must not mask the real one — but it must not +// be swallowed either, because the sandbox is still billable. +func destroyQuiet(ctx context.Context, client *api.SandboxClient, id string, warn func(string)) { + // The caller's context may already be cancelled (Ctrl-C, timeout). + // Teardown still has to happen, so give it a context of its own. + tearCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) + defer cancel() + if err := client.DestroySandbox(tearCtx, id); err != nil && warn != nil { + warn(fmt.Sprintf("could not destroy %s: %v — remove it with: createos sandbox rm --force %s", id, err, id)) + } +} diff --git a/cmd/sandbox/compose_test.go b/cmd/sandbox/compose_test.go new file mode 100644 index 0000000..92cb77b --- /dev/null +++ b/cmd/sandbox/compose_test.go @@ -0,0 +1,303 @@ +package sandbox + +import ( + "archive/tar" + "bytes" + "context" + "errors" + "os" + "os/exec" + "path/filepath" + "reflect" + "strings" + "testing" + + "github.com/urfave/cli/v2" +) + +func TestResolveEgress(t *testing.T) { + t.Run("presets union and sort", func(t *testing.T) { + got, err := resolveEgress([]string{"npm", "github"}, nil) + if err != nil { + t.Fatalf("resolveEgress: %v", err) + } + want := []string{ + "codeload.github.com", "github.com", "objects.githubusercontent.com", + "raw.githubusercontent.com", "registry.npmjs.org", + } + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } + }) + + t.Run("explicit hosts merge and dedupe", func(t *testing.T) { + got, err := resolveEgress([]string{"npm"}, []string{"registry.npmjs.org", "example.com", " "}) + if err != nil { + t.Fatalf("resolveEgress: %v", err) + } + want := []string{"example.com", "registry.npmjs.org"} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } + }) + + t.Run("nothing means unrestricted", func(t *testing.T) { + got, err := resolveEgress(nil, nil) + if err != nil { + t.Fatalf("resolveEgress: %v", err) + } + if len(got) != 0 { + t.Errorf("got %v, want empty", got) + } + }) + + t.Run("unknown preset names the real ones", func(t *testing.T) { + _, err := resolveEgress([]string{"go-modules"}, nil) + if err == nil { + t.Fatal("want an error for an unknown preset") + } + for _, name := range egressPresetNames() { + if !strings.Contains(err.Error(), name) { + t.Errorf("error does not list %q: %v", name, err) + } + } + }) +} + +func TestParseKeyValues(t *testing.T) { + got, err := parseKeyValues([]string{"A=1", "B=with=equals", "C="}) + if err != nil { + t.Fatalf("parseKeyValues: %v", err) + } + want := map[string]string{"A": "1", "B": "with=equals", "C": ""} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } + if _, err := parseKeyValues([]string{"novalue"}); err == nil { + t.Error("want an error for a flag with no =") + } + if _, err := parseKeyValues([]string{"=1"}); err == nil { + t.Error("want an error for an empty key") + } +} + +func TestStageDirHonoursGitignore(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, ".gitignore", "node_modules/\n*.log\n") + writeFile(t, dir, "main.go", "package main\n") + writeFile(t, dir, "app.log", "noise\n") + writeFile(t, dir, filepath.Join("node_modules", "dep", "index.js"), "module.exports={}\n") + + for _, args := range [][]string{ + {"init", "-q"}, + {"config", "user.email", "t@example.com"}, + {"config", "user.name", "t"}, + } { + cmd := exec.CommandContext(t.Context(), "git", append([]string{"-C", dir}, args...)...) //#nosec G204 -- dir is t.TempDir(), args are literals in this test + if out, err := cmd.CombinedOutput(); err != nil { + t.Skipf("git unavailable: %v: %s", err, out) + } + } + + tree, err := stageDir(context.Background(), dir, stageOptions{}) + if err != nil { + t.Fatalf("stageDir: %v", err) + } + defer func() { _ = os.Remove(tree.Path) }() + + names := tarNames(t, tree.Path) + if !names["main.go"] { + t.Error("main.go missing — tracked files must ship") + } + if names["app.log"] { + t.Error("app.log shipped — .gitignore says it must not") + } + for n := range names { + if strings.HasPrefix(n, "node_modules/") { + t.Errorf("%s shipped — .gitignore says node_modules must not", n) + } + } + if names[".git/HEAD"] { + t.Error(".git shipped without IncludeGit") + } +} + +func TestStageDirWithoutGitUsesDefaultExcludes(t *testing.T) { + dir := t.TempDir() + writeFile(t, dir, "main.go", "package main\n") + writeFile(t, dir, filepath.Join("node_modules", "dep", "index.js"), "x\n") + writeFile(t, dir, filepath.Join("dist", "bundle.js"), "x\n") + writeFile(t, dir, filepath.Join("src", "keep.ts"), "x\n") + + tree, err := stageDir(context.Background(), dir, stageOptions{Exclude: []string{"src"}}) + if err != nil { + t.Fatalf("stageDir: %v", err) + } + defer func() { _ = os.Remove(tree.Path) }() + + names := tarNames(t, tree.Path) + if !names["main.go"] { + t.Error("main.go missing") + } + for _, unwanted := range []string{"node_modules/dep/index.js", "dist/bundle.js", "src/keep.ts"} { + if names[unwanted] { + t.Errorf("%s shipped — it is excluded", unwanted) + } + } +} + +func TestStageExcluded(t *testing.T) { + for _, tc := range []struct { + rel string + want bool + }{ + {"node_modules", true}, + {"node_modules/a/b.js", true}, + {"src/node_modules/x.js", true}, + {"src/main.go", false}, + {"nodes/main.go", false}, + } { + if got := stageExcluded(tc.rel, stageDefaultExcludes); got != tc.want { + t.Errorf("stageExcluded(%q) = %v, want %v", tc.rel, got, tc.want) + } + } +} + +// TestUntarIntoRefusesEscape covers the path-traversal guard. The archive +// is built inside a sandbox, so it is untrusted input on the way back out. +func TestUntarInto(t *testing.T) { + t.Run("refuses an entry above the root", func(t *testing.T) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + body := []byte("owned") + if err := tw.WriteHeader(&tar.Header{ + Name: "../escaped.txt", Mode: 0o600, Size: int64(len(body)), Typeflag: tar.TypeReg, + }); err != nil { + t.Fatal(err) + } + if _, err := tw.Write(body); err != nil { + t.Fatal(err) + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + + root := t.TempDir() + err := untarInto(&buf, root) + // filepath.Clean("/../escaped.txt") lands back at the root, so the + // entry must either be refused or land inside root. Never above it. + if err == nil { + if _, statErr := os.Stat(filepath.Join(filepath.Dir(root), "escaped.txt")); !errors.Is(statErr, os.ErrNotExist) { + t.Fatal("archive escaped the extraction root") + } + } + }) + + t.Run("extracts a normal tree", func(t *testing.T) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + body := []byte("hello") + if err := tw.WriteHeader(&tar.Header{ + Name: "coverage/report.txt", Mode: 0o644, Size: int64(len(body)), Typeflag: tar.TypeReg, + }); err != nil { + t.Fatal(err) + } + if _, err := tw.Write(body); err != nil { + t.Fatal(err) + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + + root := t.TempDir() + if err := untarInto(&buf, root); err != nil { + t.Fatalf("untarInto: %v", err) + } + got, err := os.ReadFile(filepath.Join(root, "coverage", "report.txt")) + if err != nil { + t.Fatalf("read extracted file: %v", err) + } + if string(got) != "hello" { + t.Errorf("content = %q, want %q", got, "hello") + } + }) +} + +func writeFile(t *testing.T, dir, rel, body string) { + t.Helper() + p := filepath.Join(dir, rel) + if err := os.MkdirAll(filepath.Dir(p), 0o750); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(body), 0o600); err != nil { + t.Fatal(err) + } +} + +func tarNames(t *testing.T, path string) map[string]bool { + t.Helper() + f, err := os.Open(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + names := map[string]bool{} + tr := tar.NewReader(f) + for { + hdr, err := tr.Next() + if err != nil { + return names + } + names[hdr.Name] = true + } +} + +// TestSplitDirAndCommand pins the `--` handling. urfave/cli passes the +// separator through as a plain argument, so a live run once shipped +// "-- ls -la" to bash and the shell failed on it. +func TestSplitDirAndCommand(t *testing.T) { + for _, tc := range []struct { + name string + args []string + wantDir string + wantCmd string + wantErr bool + }{ + {"with separator", []string{".", "--", "bun", "test"}, ".", "bun test", false}, + {"without separator", []string{".", "bun", "test"}, ".", "bun test", false}, + {"separator and a shell line", []string{"./src", "--", "echo", "a;", "echo", "b"}, "./src", "echo a; echo b", false}, + {"no command", []string{"."}, "", "", true}, + {"separator but no command", []string{".", "--"}, "", "", true}, + } { + t.Run(tc.name, func(t *testing.T) { + app := &cli.App{ + Commands: []*cli.Command{{ + Name: "offload", + Action: func(c *cli.Context) error { + dir, cmd, err := splitDirAndCommand(c) + if tc.wantErr { + if err == nil { + t.Errorf("want an error, got dir=%q cmd=%q", dir, cmd) + } + return nil + } + if err != nil { + t.Errorf("splitDirAndCommand: %v", err) + return nil + } + if dir != tc.wantDir { + t.Errorf("dir = %q, want %q", dir, tc.wantDir) + } + if cmd != tc.wantCmd { + t.Errorf("cmd = %q, want %q", cmd, tc.wantCmd) + } + return nil + }, + }}, + } + if err := app.Run(append([]string{"createos", "offload"}, tc.args...)); err != nil { + t.Fatalf("app.Run: %v", err) + } + }) + } +} diff --git a/cmd/sandbox/create.go b/cmd/sandbox/create.go index b4c317c..8beb158 100644 --- a/cmd/sandbox/create.go +++ b/cmd/sandbox/create.go @@ -312,6 +312,7 @@ func printCreateResult(resp *api.SandboxCreateResp) { pterm.Success.Println("Reachable from anywhere over HTTPS:") fmt.Printf(" %s\n", resp.IngressURLTemplate) pterm.Println(pterm.Gray(" Replace with the port your service is listening on.")) + warnIngressCaveats() } if resp.AutoPauseAfterSeconds != nil { diff --git a/cmd/sandbox/edit.go b/cmd/sandbox/edit.go index 203aac9..a6983fb 100644 --- a/cmd/sandbox/edit.go +++ b/cmd/sandbox/edit.go @@ -328,6 +328,7 @@ func applyIngressFlag(c *cli.Context, client *api.SandboxClient, label, id, valu fmt.Printf(" %s\n", updated.IngressURLTemplate) pterm.Println(pterm.Gray(" Replace with the port your service is listening on.")) } + warnIngressCaveats() } else { pterm.Success.Printfln("Public URL is off for %s", refLabel(label, id)) } diff --git a/cmd/sandbox/egress_preset.go b/cmd/sandbox/egress_preset.go new file mode 100644 index 0000000..d195242 --- /dev/null +++ b/cmd/sandbox/egress_preset.go @@ -0,0 +1,76 @@ +package sandbox + +import ( + "fmt" + "sort" + "strings" +) + +// egressPresets name the hosts one toolchain needs to fetch its +// dependencies. A sandbox with no --egress reaches the whole internet; +// naming a preset is the cheap way to close that down to the registry the +// build actually uses, without anyone having to remember that cargo also +// pulls from static.rust-lang.org. +// +// Presets compose: --egress-preset npm --egress-preset github unions both +// lists, and --egress adds single hosts on top. +var egressPresets = map[string][]string{ + "python-uv": { + "astral.sh", "releases.astral.sh", "pypi.org", "files.pythonhosted.org", + }, + "rust-cargo": { + "crates.io", "static.crates.io", "index.crates.io", "static.rust-lang.org", "cdn.pyke.io", + }, + "npm": { + "registry.npmjs.org", + }, + "github": { + "github.com", "objects.githubusercontent.com", "raw.githubusercontent.com", "codeload.github.com", + }, +} + +// egressPresetNames lists the presets in a stable order, for help text +// and error messages. +func egressPresetNames() []string { + names := make([]string, 0, len(egressPresets)) + for n := range egressPresets { + names = append(names, n) + } + sort.Strings(names) + return names +} + +// resolveEgress expands preset names and merges them with explicit hosts. +// The result is deduplicated and sorted, so two invocations with the same +// intent produce the same allowlist. +// +// An empty result means "unrestricted", which is what the backend does +// with an empty list. Callers that want to warn about that check len(). +func resolveEgress(presets, hosts []string) ([]string, error) { + seen := make(map[string]struct{}) + for _, p := range presets { + p = strings.TrimSpace(p) + if p == "" { + continue + } + domains, ok := egressPresets[p] + if !ok { + return nil, fmt.Errorf("unknown egress preset %q\n\n Available: %s", + p, strings.Join(egressPresetNames(), ", ")) + } + for _, d := range domains { + seen[d] = struct{}{} + } + } + for _, h := range hosts { + if h = strings.TrimSpace(h); h != "" { + seen[h] = struct{}{} + } + } + out := make([]string, 0, len(seen)) + for h := range seen { + out = append(out, h) + } + sort.Strings(out) + return out, nil +} diff --git a/cmd/sandbox/fork.go b/cmd/sandbox/fork.go index bd55de9..04f7f6d 100644 --- a/cmd/sandbox/fork.go +++ b/cmd/sandbox/fork.go @@ -1,6 +1,7 @@ package sandbox import ( + "context" "fmt" "strings" @@ -21,12 +22,31 @@ func newForkCommand() *cli.Command { default the fork auto-resumes; pass --paused to keep it paused so you can fork again or attach things first. -Run with no argument on a terminal to pick from your paused sandboxes.`, +Pass --count to take several clones of one prepared sandbox — a golden +box with the toolchain and dependencies already installed, cloned once +per test job or per user. Each clone is independent. + +Run with no argument on a terminal to pick from your paused sandboxes. + +Examples: + # One clone, resumed and ready + createos sandbox fork my-golden-box + + # Ten independent clones, left paused so you resume them when needed + createos sandbox fork my-golden-box --count 10 --paused + +A forked sandbox comes up WITHOUT the S3 disks the source had mounted. +Re-attach them after the fork resumes.`, Flags: []cli.Flag{ &cli.BoolFlag{ Name: "paused", Usage: "Leave the new sandbox paused instead of auto-resuming", }, + &cli.IntFlag{ + Name: "count", + Value: 1, + Usage: "Number of clones to take from the same snapshot", + }, &cli.StringSliceFlag{ Name: "ssh-key", Usage: "Override SSH public-key file for the fork (repeatable)", @@ -69,6 +89,11 @@ func runFork(c *cli.Context) error { } func runForkByID(c *cli.Context, client *api.SandboxClient, ref, srcID string) error { + count := c.Int("count") + if count < 1 { + return fmt.Errorf("--count must be at least 1 (got %d)", count) + } + req := api.SandboxForkReq{ StartPaused: c.Bool("paused"), } @@ -82,53 +107,195 @@ func runForkByID(c *cli.Context, client *api.SandboxClient, ref, srcID string) e } if output.IsJSON(c) { - view, err := client.ForkSandbox(c.Context, srcID, req) + forks, err := forkN(c.Context, client, srcID, req, count, nil) if err != nil { return err } - target := "running" - if req.StartPaused { - target = "paused" - } - sb, err := waitForStatus(c.Context, client, view.ID, target) - if err != nil { - return err + if count == 1 { + output.Render(c, forks[0], func() {}) + return nil } - output.Render(c, sb, func() {}) + output.Render(c, forks, func() {}) return nil } - spinner, _ := pterm.DefaultSpinner.Start(fmt.Sprintf("Forking %s…", refLabel(ref, srcID))) //nolint:errcheck - view, err := client.ForkSandbox(c.Context, srcID, req) + warnForkDropsDisks(c.Context, client, srcID) + + label := refLabel(ref, srcID) + noun := "Forking %s…" + if count > 1 { + noun = fmt.Sprintf("Forking %%s into %d clones…", count) + } + spinner, _ := pterm.DefaultSpinner.Start(fmt.Sprintf(noun, label)) //nolint:errcheck + forks, err := forkN(c.Context, client, srcID, req, count, func(done, total int) { + if total > 1 { + spinner.UpdateText(fmt.Sprintf("Forking %s — %d/%d ready…", label, done, total)) + } + }) if err != nil { spinner.Fail("Fork failed") return err } - target := "running" - if req.StartPaused { - target = "paused" + spinner.Success(fmt.Sprintf("Forked %s into %d sandbox(es)", label, len(forks))) + for _, sb := range forks { + name := "" + if sb.Name != nil { + name = *sb.Name + } + fmt.Printf(" %s\n", refLabel(name, sb.ID)) + if sb.IP != nil && *sb.IP != "" { + fmt.Printf(" IP: %s\n", *sb.IP) + } + if sb.IngressURLTemplate != "" { + fmt.Printf(" URL: %s\n", sb.IngressURLTemplate) + } } - sb, err := waitForStatus(c.Context, client, view.ID, target) + return nil +} + +// ensureForkable brings srcID to `paused`, which is the only state fork +// accepts. Pause is asynchronous: it answers while the sandbox is still +// `pausing`, so a fork issued right after a pause used to be rejected with +// "sandbox is running, expected paused or error". Waiting here is what +// makes pause-then-fork safe for callers, matrix included. +func ensureForkable(ctx context.Context, client *api.SandboxClient, srcID string) error { + sb, err := client.GetSandbox(ctx, srcID) if err != nil { - spinner.Fail("Fork did not finish") return err } - if sb.Status != target { - spinner.Fail(fmt.Sprintf("Fork ended in %q", sb.Status)) - return fmt.Errorf("sandbox %s is %s — see `createos sandbox get %s` for details", sb.ID, sb.Status, sb.ID) + switch sb.Status { + case "paused": + return nil + case "pausing": + // Already on its way down; waiting is not a decision we are making + // on the user's behalf. + return waitUntilPaused(ctx, client, srcID) + case "running": + // Deliberately NOT pausing here. Pausing a running sandbox stops + // whatever it is serving, and `fork` must never do that as a side + // effect — the source could be a live dev server or a demo someone + // is using. Callers that own the sandbox pause it themselves. + return fmt.Errorf( + "sandbox %s is running, and fork needs a paused snapshot\n\n Pausing stops whatever it is serving, so fork will not do it for you.\n Pause it yourself, then fork:\n createos sandbox pause %s\n createos sandbox fork %s", + srcID, srcID, srcID) + default: + return fmt.Errorf("sandbox %s is %s — fork needs it paused\n\n Run:\n createos sandbox pause %s", srcID, sb.Status, srcID) } +} - name := "" - if sb.Name != nil { - name = *sb.Name +// pauseForFork pauses a sandbox the caller owns and waits for the snapshot +// to settle. Only matrix uses this, on the golden box it created itself — +// which is the one case where pausing is not a surprise to anyone. +func pauseForFork(ctx context.Context, client *api.SandboxClient, srcID string) error { + sb, err := client.GetSandbox(ctx, srcID) + if err != nil { + return err + } + switch sb.Status { + case "paused": + return nil + case "pausing": + case "running": + if _, pauseErr := client.PauseSandbox(ctx, srcID); pauseErr != nil { + return fmt.Errorf("pause %s before forking: %w", srcID, pauseErr) + } + default: + return fmt.Errorf("sandbox %s is %s — it cannot be paused for forking", srcID, sb.Status) } - spinner.Success(fmt.Sprintf("Forked into %s", refLabel(name, sb.ID))) - if sb.IP != nil && *sb.IP != "" { - fmt.Printf(" IP: %s\n", *sb.IP) + return waitUntilPaused(ctx, client, srcID) +} + +// waitUntilPaused blocks until the snapshot is on disk. Pause is async: it +// answers while the sandbox is still `pausing`, and a fork issued in that +// window is rejected with "sandbox is running, expected paused or error". +func waitUntilPaused(ctx context.Context, client *api.SandboxClient, srcID string) error { + final, err := waitForStatus(ctx, client, srcID, "paused") + if err != nil { + return err } - if sb.IngressURLTemplate != "" { - fmt.Printf(" URL: %s\n", sb.IngressURLTemplate) + if final.Status != "paused" { + return fmt.Errorf("sandbox %s ended in %q while pausing — see `createos sandbox get %s`", srcID, final.Status, srcID) } return nil } + +// forkN takes count clones of one paused snapshot and waits for each to +// reach its target state. onProgress, when non-nil, is called after every +// clone settles. +// +// Clones run one at a time on purpose. Fork is a server-side object copy +// measured at about a second, so the wall-clock saving from parallelism is +// small, while a partial failure halfway through a parallel batch leaves +// an unknown number of billable sandboxes behind. Sequential means the +// error names exactly how many exist. +func forkN( + ctx context.Context, + client *api.SandboxClient, + srcID string, + req api.SandboxForkReq, + count int, + onProgress func(done, total int), +) ([]*api.SandboxView, error) { + if err := ensureForkable(ctx, client, srcID); err != nil { + return nil, err + } + target := "running" + if req.StartPaused { + target = "paused" + } + + // created tracks every id the server handed back, settled or not. + // forks holds only the ones that reached `target`. The split matters: + // a fork whose status poll times out still exists and still bills, and + // reporting only the settled ones hides it from the caller — which, + // for matrix, is the difference between a cleaned-up failure and an + // orphaned running sandbox. + forks := make([]*api.SandboxView, 0, count) + created := make([]string, 0, count) + for i := 0; i < count; i++ { + view, err := client.ForkSandbox(ctx, srcID, req) + if err != nil { + return forks, forkPartialError(err, created, i, count) + } + created = append(created, view.ID) + + sb, err := waitForStatus(ctx, client, view.ID, target) + if err != nil { + return forks, forkPartialError(err, created, i, count) + } + if sb.Status != target { + return forks, forkPartialError( + fmt.Errorf("sandbox %s is %s, expected %s", sb.ID, sb.Status, target), created, i, count) + } + forks = append(forks, sb) + if onProgress != nil { + onProgress(len(forks), count) + } + } + return forks, nil +} + +// forkPartialError names every clone the server created, so a failed batch +// does not leave billable sandboxes the caller cannot find. It carries the +// ids as a forkLeak so a caller that can clean up — matrix — does not have +// to parse them back out of the message. +func forkPartialError(err error, created []string, attempt, total int) error { + if len(created) == 0 { + return err + } + return &forkLeak{ + IDs: created, + err: fmt.Errorf("fork %d of %d failed: %w\n\n %d clone(s) exist and are still billable:\n %s\n\n Remove them with:\n createos sandbox rm --force %s", + attempt+1, total, err, len(created), strings.Join(created, "\n "), strings.Join(created, " ")), + } +} + +// forkLeak reports the sandboxes a failed fork batch left behind. +type forkLeak struct { + IDs []string + err error +} + +func (e *forkLeak) Error() string { return e.err.Error() } +func (e *forkLeak) Unwrap() error { return e.err } diff --git a/cmd/sandbox/guards.go b/cmd/sandbox/guards.go new file mode 100644 index 0000000..fd960a8 --- /dev/null +++ b/cmd/sandbox/guards.go @@ -0,0 +1,91 @@ +package sandbox + +import ( + "context" + "fmt" + "path" + "strings" + + "github.com/pterm/pterm" + + "github.com/NodeOps-app/createos-cli/internal/api" +) + +// Guards for platform behaviour that surprises people. Each one traces to +// a tracked issue, and each one exists because the failure it prevents is +// silent: data that never lands, a clone that runs against an empty +// directory, a preview URL that a browser refuses. A warning at the moment +// of the action costs far less than the debugging session it replaces. + +// diskMountBlocksFileAPI reports the mount path that would swallow remote, +// or "" when the path is safe to move through the file API. +// +// Writing into an S3 disk mount through the file API crashes the mount and +// loses the object (issue #71). The write looks like it worked, so nothing +// tells the user until the data is missing. Reading is equally unsafe, so +// both push and pull consult this. +// +// It fails CLOSED. If the disk list cannot be read, the mount state is +// unknown, and "unknown" is not "safe": carrying on risks destroying data +// the user believes they just saved, while refusing costs them a retry. +// The two outcomes are not comparable, so the unknown case refuses. +func diskMountBlocksFileAPI(ctx context.Context, client *api.SandboxClient, sandboxID, remote string) (string, error) { + disks, err := client.ListSandboxDisks(ctx, sandboxID) + if err != nil { + return "", fmt.Errorf( + "could not check whether %s is inside an S3 disk mount on %s: %w\n\n Moving a file into a disk mount through the file API crashes the mount\n and loses the object (issue #71), so this stops rather than risk it.\n Retry, or copy from inside the sandbox:\n createos sandbox exec %s -- bash -lc 'cp ...'", + remote, sandboxID, err, sandboxID) + } + clean := path.Clean(remote) + for _, d := range disks { + mount := path.Clean(strings.TrimSpace(d.MountPath)) + if mount == "" || mount == "." || mount == "/" { + continue + } + if clean == mount || strings.HasPrefix(clean, mount+"/") { + return mount, nil + } + } + return "", nil +} + +// diskMountFileAPIError is the refusal. This is a hard stop rather than a +// warning: the documented outcome is a crashed mount and a lost object, so +// carrying on would destroy data the user believes they just saved. +func diskMountFileAPIError(remote, mount, verb string) error { + inner := "cp /local/file " + remote + if verb == "pull" { + inner = "cp " + remote + " /tmp/copy" + } + return fmt.Errorf( + "%s is inside the S3 disk mounted at %s, and the file API cannot move data through a disk mount (issue #71)\n\n The transfer would crash the mount and lose the object.\n Do it from inside the sandbox instead:\n createos sandbox exec -- bash -lc '%s'", + remote, mount, inner) +} + +// warnForkDropsDisks says what a fork will silently not carry. +// +// A forked sandbox comes up without its source's disk attachments (issue +// #63), so a job on the clone reads an empty directory and can "pass" +// against nothing at all. +func warnForkDropsDisks(ctx context.Context, client *api.SandboxClient, srcID string) { + disks, err := client.ListSandboxDisks(ctx, srcID) + if err != nil || len(disks) == 0 { + return + } + mounts := make([]string, 0, len(disks)) + for _, d := range disks { + mounts = append(mounts, d.Name+" at "+d.MountPath) + } + pterm.Warning.Printfln( + "The fork will come up WITHOUT the %d disk(s) this sandbox has mounted (issue #63):\n %s\n Re-attach them after the fork resumes, or it will read empty directories.", + len(disks), strings.Join(mounts, "\n ")) +} + +// warnIngressCaveats fires when a public HTTPS URL is switched on. Both +// caveats cost real debugging time and neither is visible from the URL. +func warnIngressCaveats() { + pterm.Warning.Println("The public URL has two known limits:") + fmt.Println(" TLS is a self-signed certificate, so browsers reject it (issue #46).") + fmt.Println(" The ingress hop strips the Authorization header, so services gated") + fmt.Println(" on Basic or Bearer auth see no credentials (issue #64).") +} diff --git a/cmd/sandbox/guards_test.go b/cmd/sandbox/guards_test.go new file mode 100644 index 0000000..e938c73 --- /dev/null +++ b/cmd/sandbox/guards_test.go @@ -0,0 +1,125 @@ +package sandbox + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +// TestDiskMountBlocksFileAPI covers the guard for issue #71: a file-API +// transfer into an S3 disk mount crashes the mount and loses the object. +// The path comparison has to be on whole segments — "/mnt/data-old" is not +// inside "/mnt/data", and blocking it would stop a legitimate transfer. +func TestDiskMountBlocksFileAPI(t *testing.T) { + f := newFakeAPI(t).json("GET /v1/sandboxes/sb-1/disks", + `{"data":{"data":[{"disk_id":"d1","name":"bucket","mount_path":"/mnt/data"}]}}`) + + for _, tc := range []struct { + remote string + want string + }{ + {"/mnt/data/report.csv", "/mnt/data"}, + {"/mnt/data", "/mnt/data"}, + {"/mnt/data/nested/deep.bin", "/mnt/data"}, + {"/workspace/report.csv", ""}, + {"/mnt/data-old/report.csv", ""}, + {"/mnt", ""}, + } { + got, err := diskMountBlocksFileAPI(context.Background(), f.client(), "sb-1", tc.remote) + if err != nil { + t.Fatalf("diskMountBlocksFileAPI(%q): %v", tc.remote, err) + } + if got != tc.want { + t.Errorf("diskMountBlocksFileAPI(%q) = %q, want %q", tc.remote, got, tc.want) + } + } +} + +// A sandbox with no disks must never be blocked. +func TestDiskMountAllowsASandboxWithNoDisks(t *testing.T) { + empty := newFakeAPI(t).json("GET /v1/sandboxes/sb-1/disks", `{"data":{"data":[]}}`) + got, err := diskMountBlocksFileAPI(context.Background(), empty.client(), "sb-1", "/anything") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "" { + t.Errorf("no disks attached, got %q, want no block", got) + } +} + +// TestDiskMountFailsClosed is the important half. When the disk list +// cannot be read the mount state is unknown, and "unknown" must not be +// treated as "safe": a wrong guess here crashes an S3 mount and loses the +// object, while a refusal only costs a retry. +func TestDiskMountFailsClosed(t *testing.T) { + broken := newFakeAPI(t).fails("GET /v1/sandboxes/sb-1/disks") + _, err := diskMountBlocksFileAPI(context.Background(), broken.client(), "sb-1", "/workspace/out.csv") + if err == nil { + t.Fatal("disk list unreadable but the transfer was allowed — this is how the object gets lost") + } + for _, want := range []string{"/workspace/out.csv", "#71", "sandbox exec"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error must mention %q, got: %v", want, err) + } + } +} + +func TestDiskMountFileAPIError(t *testing.T) { + err := diskMountFileAPIError("/mnt/data/out.csv", "/mnt/data", "push") + for _, want := range []string{"/mnt/data/out.csv", "/mnt/data", "#71", "sandbox exec"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error must mention %q, got: %v", want, err) + } + } +} + +// TestSelfSignalHTTP checks the wire shape the guest agent expects: a POST +// to /self/, the reason carried as a query parameter, and 202 +// treated as success. +func TestSelfSignalHTTP(t *testing.T) { + var gotMethod, gotPath, gotReason string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotMethod, gotPath, gotReason = r.Method, r.URL.Path, r.URL.Query().Get("reason") + w.WriteHeader(http.StatusAccepted) + _, _ = w.Write([]byte(`{"status":"accepted","action":"pause"}`)) + })) + defer srv.Close() + + // selfSignalHTTP hardcodes the loopback address, so point the test at + // the fake server by rebuilding the same request shape it sends. + addr := strings.TrimPrefix(srv.URL, "http://") + original := selfSignalAddrForTest + selfSignalAddrForTest = addr + t.Cleanup(func() { selfSignalAddrForTest = original }) + + if err := selfSignalHTTP(context.Background(), "pause", "job done"); err != nil { + t.Fatalf("selfSignalHTTP: %v", err) + } + if gotMethod != http.MethodPost { + t.Errorf("method = %s, want POST", gotMethod) + } + if gotPath != "/self/pause" { + t.Errorf("path = %s, want /self/pause", gotPath) + } + if gotReason != "job done" { + t.Errorf("reason = %q, want %q", gotReason, "job done") + } +} + +func TestSelfSignalHTTPRejectsUnexpectedStatus(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusTeapot) + })) + defer srv.Close() + + original := selfSignalAddrForTest + selfSignalAddrForTest = strings.TrimPrefix(srv.URL, "http://") + t.Cleanup(func() { selfSignalAddrForTest = original }) + + err := selfSignalHTTP(context.Background(), "pause", "") + if err == nil { + t.Fatal("want an error when something other than the agent answers") + } +} diff --git a/cmd/sandbox/lifecycle_test.go b/cmd/sandbox/lifecycle_test.go new file mode 100644 index 0000000..a90d698 --- /dev/null +++ b/cmd/sandbox/lifecycle_test.go @@ -0,0 +1,342 @@ +package sandbox + +import ( + "archive/tar" + "bytes" + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/urfave/cli/v2" + + "github.com/NodeOps-app/createos-cli/internal/api" +) + +// These tests cover the failure paths, not the happy ones. Every one of +// them exists because a success path that leaks a billable sandbox looks +// exactly like a success path that does not. + +// fakeAPI is a stand-in sandbox control plane. Handlers are matched by +// "METHOD /path" with {id} already substituted, so a test only declares +// the calls it cares about; anything else is a 404 the test can assert on. +type fakeAPI struct { + t *testing.T + mu sync.Mutex + seen []string + handlers map[string]http.HandlerFunc + srv *httptest.Server +} + +func newFakeAPI(t *testing.T) *fakeAPI { + t.Helper() + f := &fakeAPI{t: t, handlers: map[string]http.HandlerFunc{}} + f.srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + key := r.Method + " " + r.URL.Path + f.mu.Lock() + f.seen = append(f.seen, key) + h, ok := f.handlers[key] + f.mu.Unlock() + if !ok { + http.Error(w, `{"error":"no handler: `+key+`"}`, http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + h(w, r) + })) + t.Cleanup(f.srv.Close) + return f +} + +func (f *fakeAPI) on(key string, h http.HandlerFunc) *fakeAPI { + f.mu.Lock() + defer f.mu.Unlock() + f.handlers[key] = h + return f +} + +func (f *fakeAPI) json(key, body string) *fakeAPI { + return f.on(key, func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(body)) + }) +} + +// fails makes an endpoint answer 500. Every caller wants the same thing — +// "this control-plane call is broken right now" — so the status is fixed +// rather than a parameter nobody varies. +func (f *fakeAPI) fails(key string) *fakeAPI { + return f.on(key, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":"boom"}`)) + }) +} + +func (f *fakeAPI) called(key string) bool { + f.mu.Lock() + defer f.mu.Unlock() + for _, s := range f.seen { + if s == key { + return true + } + } + return false +} + +func (f *fakeAPI) client() *api.SandboxClient { + c := api.NewSandboxClient("tok", f.srv.URL, false) + return &c +} + +// shortPoll makes waitForStatus give up quickly. Without it these tests +// would sit through the production 5-minute timeout. +func shortPoll(t *testing.T) context.Context { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 3*time.Second) + t.Cleanup(cancel) + return ctx +} + +// TestCreateComposeBoxDestroysWhenNeverReady covers the leak Codex found: +// CreateSandbox succeeds, readiness polling does not, and the caller only +// ever sees an error — so if createComposeBox does not destroy the box +// itself, nothing will. +func TestCreateComposeBoxDestroysWhenNeverReady(t *testing.T) { + f := newFakeAPI(t). + json("POST /v1/sandboxes", `{"data":{"id":"sb-stuck"}}`). + json("GET /v1/sandboxes/sb-stuck", `{"data":{"id":"sb-stuck","status":"failed"}}`). + json("DELETE /v1/sandboxes/sb-stuck", `{"data":{"id":"sb-stuck","status":"destroying"}}`) + + _, err := createComposeBox(shortPoll(t), f.client(), &composeOptions{Shape: "s-1vcpu-1gb"}) + if err == nil { + t.Fatal("want an error when the sandbox never reaches running") + } + if !f.called("DELETE /v1/sandboxes/sb-stuck") { + t.Error("sandbox was created and never destroyed — it is still billable") + } + if !strings.Contains(err.Error(), "sb-stuck") { + t.Errorf("error must name the sandbox id, got: %v", err) + } +} + +// TestCreateComposeBoxReportsUndestroyableBox is the worse branch: the box +// exists and teardown also failed, so the id must reach the user with a +// command they can run by hand. +func TestCreateComposeBoxReportsUndestroyableBox(t *testing.T) { + f := newFakeAPI(t). + json("POST /v1/sandboxes", `{"data":{"id":"sb-orphan"}}`). + json("GET /v1/sandboxes/sb-orphan", `{"data":{"id":"sb-orphan","status":"failed"}}`). + fails("DELETE /v1/sandboxes/sb-orphan") + + _, err := createComposeBox(shortPoll(t), f.client(), &composeOptions{Shape: "s-1vcpu-1gb"}) + if err == nil { + t.Fatal("want an error") + } + for _, want := range []string{"sb-orphan", "still billable", "rm --force"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error must contain %q so the user can clean up, got: %v", want, err) + } + } +} + +// TestForkNReportsCloneCreatedBeforePollFailed covers the orphan Codex +// found: ForkSandbox returns a real, running clone, then the status poll +// fails. The clone is not in the settled list, so unless the error carries +// its id, nothing can ever clean it up. +func TestForkNReportsCloneCreatedBeforePollFailed(t *testing.T) { + f := newFakeAPI(t). + json("GET /v1/sandboxes/sb-golden", `{"data":{"id":"sb-golden","status":"paused"}}`). + json("POST /v1/sandboxes/sb-golden/fork", `{"data":{"id":"sb-clone-1","status":"forking"}}`). + json("GET /v1/sandboxes/sb-clone-1", `{"data":{"id":"sb-clone-1","status":"failed"}}`) + + forks, err := forkN(shortPoll(t), f.client(), "sb-golden", api.SandboxForkReq{}, 1, nil) + if err == nil { + t.Fatal("want an error when the clone never settles") + } + if len(forks) != 0 { + t.Errorf("settled forks = %d, want 0", len(forks)) + } + + var leak *forkLeak + if !errors.As(err, &leak) { + t.Fatalf("error must be a *forkLeak carrying the created id, got %T: %v", err, err) + } + if len(leak.IDs) != 1 || leak.IDs[0] != "sb-clone-1" { + t.Errorf("leak.IDs = %v, want [sb-clone-1]", leak.IDs) + } + if !strings.Contains(err.Error(), "sb-clone-1") { + t.Errorf("message must name the clone, got: %v", err) + } +} + +// TestMatrixRunOneSurfacesDestroyFailure pins the named-return fix. The +// teardown runs in a defer; with an unnamed return Go copies the result +// before defers run, so a failed destroy never reached the caller and the +// matrix exited 0 while a clone kept billing. +func TestMatrixRunOneSurfacesDestroyFailure(t *testing.T) { + f := newFakeAPI(t). + json("GET /v1/sandboxes/sb-golden", `{"data":{"id":"sb-golden","status":"paused"}}`). + json("POST /v1/sandboxes/sb-golden/fork", `{"data":{"id":"sb-clone","status":"running"}}`). + json("GET /v1/sandboxes/sb-clone", `{"data":{"id":"sb-clone","status":"running"}}`). + json("PATCH /v1/sandboxes/sb-clone", `{"data":{"id":"sb-clone","status":"running"}}`). + json("POST /v1/sandboxes/sb-clone/processes", `{"data":{"process_id":"p1","state":"running"}}`). + on("GET /v1/sandboxes/sb-clone/processes/p1/connect", func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprintln(w, `{"type":"exit","exit_code":0}`) + }). + fails("DELETE /v1/sandboxes/sb-clone") + + res := matrixRunOne(shortPoll(t), f.client(), "sb-golden", 0, "true", t.TempDir(), + &composeOptions{Shape: "s-1vcpu-1gb", AutoPause: time.Minute}) + + if res.ExitCode != 0 { + t.Fatalf("job exit code = %d, want 0 — the command itself passed", res.ExitCode) + } + if res.Error == "" { + t.Fatal("destroy failed but the job reported no error — matrix would exit 0 having leaked sb-clone") + } + if !strings.Contains(res.Error, "sb-clone") { + t.Errorf("error must name the leaked clone, got: %q", res.Error) + } +} + +// TestUntarIntoRefusesSymlinkAncestor is the extraction-boundary +// regression. A lexical prefix check passes here, because the pathname +// this code builds stays under root; the escape happens when the +// filesystem follows a symlink that was already on disk. +func TestUntarIntoRefusesSymlinkAncestor(t *testing.T) { + base := t.TempDir() + root := filepath.Join(base, "repo") + outside := filepath.Join(base, "outside") + for _, d := range []string{root, outside} { + if err := os.MkdirAll(d, 0o750); err != nil { + t.Fatal(err) + } + } + victim := filepath.Join(outside, "report.txt") + if err := os.WriteFile(victim, []byte("original"), 0o600); err != nil { + t.Fatal(err) + } + // The trap: a symlink that already exists inside the extraction root. + if err := os.Symlink(outside, filepath.Join(root, "coverage")); err != nil { + t.Skipf("symlinks unavailable: %v", err) + } + + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + body := []byte("owned") + if err := tw.WriteHeader(&tar.Header{ + Name: "coverage/report.txt", Mode: 0o644, Size: int64(len(body)), Typeflag: tar.TypeReg, + }); err != nil { + t.Fatal(err) + } + if _, err := tw.Write(body); err != nil { + t.Fatal(err) + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + + err := untarInto(&buf, root) + + got, readErr := os.ReadFile(victim) // #nosec G304 -- path built from t.TempDir() + if readErr != nil { + t.Fatalf("read victim: %v", readErr) + } + if string(got) != "original" { + t.Fatalf("archive wrote through the symlink and overwrote %s (untarInto err=%v)", victim, err) + } + if err == nil { + t.Error("want an error for an entry whose parent escapes the root") + } +} + +// TestForkRefusesToPauseARunningSandbox is the guard against the worst +// version of this command: `createos sandbox fork my-live-server` pausing +// a service somebody is using, as a side effect of asking for a clone. +// Fork must never stop a running workload on its own. +func TestForkRefusesToPauseARunningSandbox(t *testing.T) { + f := newFakeAPI(t). + json("GET /v1/sandboxes/sb-live", `{"data":{"id":"sb-live","status":"running"}}`) + + err := ensureForkable(shortPoll(t), f.client(), "sb-live") + if err == nil { + t.Fatal("want a refusal — forking must not pause a running sandbox") + } + if f.called("POST /v1/sandboxes/sb-live/pause") { + t.Error("fork paused a running sandbox on its own; whatever it was serving just stopped") + } + for _, want := range []string{"running", "createos sandbox pause sb-live"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error must contain %q so the user can act, got: %v", want, err) + } + } +} + +// pauseForFork is the opposite case: matrix built the golden sandbox, so +// it is allowed to pause it. +func TestPauseForForkPausesASandboxTheCallerOwns(t *testing.T) { + pauses := 0 + f := newFakeAPI(t). + on("GET /v1/sandboxes/sb-golden", func(w http.ResponseWriter, _ *http.Request) { + status := "running" + if pauses > 0 { + status = "paused" + } + _, _ = fmt.Fprintf(w, `{"data":{"id":"sb-golden","status":%q}}`, status) + }). + on("POST /v1/sandboxes/sb-golden/pause", func(w http.ResponseWriter, _ *http.Request) { + pauses++ + _, _ = w.Write([]byte(`{"data":{"id":"sb-golden","status":"pausing"}}`)) + }) + + if err := pauseForFork(shortPoll(t), f.client(), "sb-golden"); err != nil { + t.Fatalf("pauseForFork: %v", err) + } + if pauses != 1 { + t.Errorf("pause called %d times, want 1", pauses) + } +} + +// TestOffloadFailsWhenTeardownFails covers the leak that looks like a +// clean run: the workload passes, DestroySandbox fails, and a CI job +// reading only the exit status would never learn that a billable sandbox +// was left behind. `offload` promises a throwaway sandbox, so a teardown +// failure has to reach the exit code. +func TestOffloadFailsWhenTeardownFails(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "main.go"), []byte("package main\n"), 0o600); err != nil { + t.Fatal(err) + } + + f := newFakeAPI(t). + json("POST /v1/sandboxes", `{"data":{"id":"sb-off"}}`). + json("GET /v1/sandboxes/sb-off", `{"data":{"id":"sb-off","status":"running"}}`). + json("PUT /v1/sandboxes/sb-off/files", `{"data":{}}`). + json("POST /v1/sandboxes/sb-off/exec", `{"data":{"result":{"exit_code":0}}}`). + json("POST /v1/sandboxes/sb-off/processes", `{"data":{"process_id":"p1","state":"running"}}`). + on("GET /v1/sandboxes/sb-off/processes/p1/connect", func(w http.ResponseWriter, _ *http.Request) { + _, _ = fmt.Fprintln(w, `{"type":"exit","exit_code":0}`) + }). + fails("DELETE /v1/sandboxes/sb-off") + + app := &cli.App{ + Commands: []*cli.Command{newOffloadCommand()}, + Metadata: map[string]any{api.SandboxClientKey: f.client()}, + } + err := app.RunContext(shortPoll(t), []string{"createos", "offload", dir, "--", "true"}) + + if err == nil { + t.Fatal("workload passed but the sandbox leaked, and offload reported success") + } + for _, want := range []string{"sb-off", "not destroyed", "rm --force"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error must contain %q, got: %v", want, err) + } + } +} diff --git a/cmd/sandbox/matrix.go b/cmd/sandbox/matrix.go new file mode 100644 index 0000000..deb3ffc --- /dev/null +++ b/cmd/sandbox/matrix.go @@ -0,0 +1,474 @@ +package sandbox + +import ( + "bufio" + "bytes" + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "sync" + "time" + + "github.com/pterm/pterm" + "github.com/urfave/cli/v2" + + "github.com/NodeOps-app/createos-cli/internal/api" + "github.com/NodeOps-app/createos-cli/internal/output" +) + +// matrixDefaultConcurrency matches the sandboxes one account may run at +// once. Raising it past the account limit does not go faster; it just +// fails later. +const matrixDefaultConcurrency = 10 + +func newMatrixCommand() *cli.Command { + return &cli.Command{ + Name: "matrix", + Usage: "Run many commands in parallel, each on its own clone of one prepared sandbox", + ArgsUsage: "", + Description: `Matrix runs a set of commands at the same time, each on its own sandbox, +and gives you one exit code per command. + +The sandboxes are clones, not fresh machines. Matrix builds one golden +sandbox, runs --prepare on it once, pauses it, and forks that snapshot per +job. A dependency install that takes two minutes is paid once, not once per +job. Fork is a snapshot copy, so a clone costs about a second. + +Every sandbox is destroyed when its job finishes. The command exits 0 only +if every job exited 0. + +Flags must come before the directory. Anything after it is not read as a +flag. + +Examples: + # Three test suites, three sandboxes, one dependency install + createos sandbox matrix --prepare 'bun install' \ + --job 'bun test unit' --job 'bun test e2e' --job 'bun test perf' . + + # A large matrix from a file, ten at a time + createos sandbox matrix --prepare 'npm ci' --jobs-file cases.txt --concurrency 10 . + + # Reuse a sandbox you already prepared and paused + createos sandbox matrix --from my-golden-box --job 'pytest -k slow' + +Known limits: + A fork comes up without the S3 disks its source had mounted (issue #63), + so matrix refuses --disk rather than run jobs against missing data. + A clone whose snapshot is not cached on the target host takes 11-13 + seconds to resume, not under a second. + A clone does not inherit --shape from the golden sandbox, so clones can + be larger than you asked for and cost more. Matrix does re-apply + --auto-pause to every clone, because a clone does not inherit that + either.`, + Flags: append(composeFlags(), + &cli.StringFlag{ + Name: "prepare", + Usage: "Command to run once on the golden sandbox before it is cloned", + }, + &cli.StringSliceFlag{ + Name: "job", + Usage: "Command to run on its own clone (repeatable)", + }, + &cli.StringFlag{ + Name: "jobs-file", + Usage: "File with one job command per line. Blank lines and lines starting with # are skipped", + }, + &cli.IntFlag{ + Name: "concurrency", + Value: matrixDefaultConcurrency, + Usage: "How many clones run at the same time", + }, + &cli.StringFlag{ + Name: "from", + Usage: "Clone this existing sandbox instead of building a golden one from ", + }, + &cli.StringFlag{ + Name: "logs", + Usage: "Directory for per-job log files. Default: a temporary directory", + }, + &cli.BoolFlag{ + Name: "keep-golden", + Usage: "Keep the golden sandbox after the run instead of destroying it", + }, + ), + Action: runMatrix, + } +} + +// matrixJobResult is one job's outcome, and one row of the JSON output. +type matrixJobResult struct { + Index int `json:"index"` + Cmd string `json:"cmd"` + Sandbox string `json:"sandbox,omitempty"` + ExitCode int `json:"exit_code"` + DurationMs int64 `json:"duration_ms"` + Log string `json:"log,omitempty"` + Error string `json:"error,omitempty"` +} + +func runMatrix(c *cli.Context) error { + client, ok := c.App.Metadata[api.SandboxClientKey].(*api.SandboxClient) + if !ok { + return fmt.Errorf("you're not signed in — run 'createos login' to get started") + } + + // Everything after the directory should have been a flag, and the + // parser stopped reading them there. Say so before the empty-job-list + // error hides the real cause. + if c.Args().Len() > 1 { + if err := checkMisplacedFlags(c, c.Args().Slice()[1:]); err != nil { + return err + } + } + jobs, err := matrixJobs(c) + if err != nil { + return err + } + concurrency := c.Int("concurrency") + if concurrency < 1 { + return fmt.Errorf("--concurrency must be at least 1 (got %d)", concurrency) + } + opts, err := parseComposeOptions(c) + if err != nil { + return err + } + + ctx := c.Context + if opts.Timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, opts.Timeout) + defer cancel() + } + + quiet := output.IsJSON(c) + say := func(format string, a ...any) { + if !quiet { + pterm.Info.Printfln(format, a...) + } + } + + logDir, err := matrixLogDir(c.String("logs")) + if err != nil { + return err + } + + golden, cleanupGolden, err := matrixGoldenBox(ctx, c, client, opts, say) + if err != nil { + return err + } + defer cleanupGolden() + + // Pause the golden box once, here, rather than letting every job race + // to do it. Fork needs a paused source, and N concurrent pause calls + // on the same sandbox is a fight nobody needs to have. + // + // pauseForFork, not ensureForkable: matrix may pause this sandbox + // because it built it. With --from the sandbox belongs to the user, so + // say what is about to happen to it rather than doing it silently. + if ref := strings.TrimSpace(c.String("from")); ref != "" { + say("Pausing %s to take the snapshot the clones come from", golden) + } + if err := pauseForFork(ctx, client, golden); err != nil { + return err + } + say("Cloning %s into %d sandbox(es), %d at a time", golden, len(jobs), concurrency) + results := matrixRunJobs(ctx, client, golden, jobs, concurrency, logDir, opts, quiet) + + failed := 0 + for _, r := range results { + if r.ExitCode != 0 || r.Error != "" { + failed++ + } + } + + if quiet { + output.Render(c, map[string]any{ + "golden": golden, + "jobs": results, + "failed": failed, + }, func() {}) + } else { + matrixPrintSummary(results, logDir) + } + + if failed > 0 { + return cli.Exit("", 1) + } + return nil +} + +// matrixJobs collects the job commands from --job and --jobs-file. +func matrixJobs(c *cli.Context) ([]string, error) { + jobs := append([]string{}, c.StringSlice("job")...) + if path := c.String("jobs-file"); path != "" { + f, err := os.Open(path) // #nosec G304 -- the user named this file on their own command line + if err != nil { + return nil, fmt.Errorf("read --jobs-file: %w", err) + } + defer func() { _ = f.Close() }() //nolint:errcheck // read-only handle + scanner := bufio.NewScanner(f) + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + jobs = append(jobs, line) + } + if err := scanner.Err(); err != nil { + return nil, fmt.Errorf("read --jobs-file: %w", err) + } + } + if len(jobs) == 0 { + return nil, errors.New( + "no jobs to run\n\n Give at least one --job, or a --jobs-file:\n createos sandbox matrix . --job 'bun test unit' --job 'bun test e2e'") + } + return jobs, nil +} + +func matrixLogDir(want string) (string, error) { + if want == "" { + return os.MkdirTemp("", "createos-matrix-*") + } + if err := os.MkdirAll(want, 0o750); err != nil { + return "", fmt.Errorf("create --logs directory: %w", err) + } + return filepath.Abs(want) +} + +// matrixGoldenBox returns the sandbox id to clone from, plus the cleanup +// that runs when the matrix finishes. +// +// Two ways in. --from names a sandbox the user already prepared, and +// matrix never destroys it — it did not create it. Otherwise matrix builds +// one from , runs --prepare, and owns its teardown. +func matrixGoldenBox( + ctx context.Context, + c *cli.Context, + client *api.SandboxClient, + opts *composeOptions, + say func(string, ...any), +) (string, func(), error) { + noop := func() {} + + if ref := strings.TrimSpace(c.String("from")); ref != "" { + if c.Args().Len() > 0 { + return "", noop, errors.New("--from and do the same job — give one or the other") + } + id, err := resolveSandboxRef(ctx, client, ref) + if err != nil { + return "", noop, err + } + if err := matrixRefuseDisks(ctx, client, id); err != nil { + return "", noop, err + } + return id, noop, nil + } + + dir := strings.TrimSpace(c.Args().First()) + if dir == "" { + return "", noop, errors.New( + "please give a directory to clone, or --from an existing sandbox\n\n Example:\n createos sandbox matrix . --prepare 'bun install' --job 'bun test'") + } + + tree, err := stageDir(ctx, dir, stageOptions{Exclude: opts.Exclude}) + if err != nil { + return "", noop, err + } + defer func() { _ = os.Remove(tree.Path) }() //nolint:errcheck // temp file + say("Packed %d file(s), %s", tree.Files, humanBytes(tree.Size)) + + if len(opts.Egress) == 0 { + say("Egress unrestricted — every clone can reach any host. Restrict it with --egress or --egress-preset.") + } + + sb, err := createComposeBox(ctx, client, opts) + if err != nil { + return "", noop, err + } + cleanup := func() { + if c.Bool("keep-golden") { + pterm.Info.Printfln("Golden sandbox %s kept. Destroy it with: createos sandbox rm --force %s", sb.ID, sb.ID) + return + } + destroyQuiet(ctx, client, sb.ID, func(msg string) { pterm.Warning.Println(msg) }) + } + + if err := shipTree(ctx, client, sb.ID, tree, composeWorkDir); err != nil { + cleanup() + return "", noop, err + } + say("Golden sandbox %s is up", sb.ID) + + if prepare := strings.TrimSpace(c.String("prepare")); prepare != "" { + say("Preparing once: %s", prepare) + var buf bytes.Buffer + res, err := runManaged(ctx, client, sb.ID, prepare, composeWorkDir, opts.Env, &buf) + if err != nil { + cleanup() + return "", noop, fmt.Errorf("prepare: %w", err) + } + if res.ExitCode != 0 { + cleanup() + return "", noop, fmt.Errorf("prepare exited %d — no clones were made\n\n%s", + res.ExitCode, strings.TrimSpace(buf.String())) + } + } + return sb.ID, cleanup, nil +} + +// matrixRefuseDisks stops a run that would silently lose data. A fork does +// not carry its source's S3 disk attachments (issue #63), so jobs would +// read an empty mount path and "pass" against nothing. +func matrixRefuseDisks(ctx context.Context, client *api.SandboxClient, id string) error { + disks, err := client.ListSandboxDisks(ctx, id) + if err != nil { + // Not being able to check is not a reason to refuse the run. + return nil //nolint:nilerr + } + if len(disks) == 0 { + return nil + } + return fmt.Errorf( + "sandbox %s has %d S3 disk(s) mounted, and a fork does not carry them (issue #63)\n\n The clones would run against empty mount paths.\n Copy what the jobs need into the sandbox's own filesystem first, then re-run", + id, len(disks)) +} + +// matrixRunJobs clones the golden sandbox once per job and runs them, at +// most `concurrency` at a time. +func matrixRunJobs( + ctx context.Context, + client *api.SandboxClient, + golden string, + jobs []string, + concurrency int, + logDir string, + opts *composeOptions, + quiet bool, +) []matrixJobResult { + results := make([]matrixJobResult, len(jobs)) + slots := make(chan struct{}, concurrency) + var wg sync.WaitGroup + + for i, job := range jobs { + wg.Add(1) + go func(i int, job string) { + defer wg.Done() + slots <- struct{}{} + defer func() { <-slots }() + results[i] = matrixRunOne(ctx, client, golden, i, job, logDir, opts) + if !quiet { + matrixPrintOne(results[i]) + } + }(i, job) + } + wg.Wait() + return results +} + +// matrixRunOne clones, runs one job, and destroys the clone. +func matrixRunOne( + ctx context.Context, + client *api.SandboxClient, + golden string, + index int, + job, logDir string, + opts *composeOptions, +) (res matrixJobResult) { + // Named return, deliberately. The teardown below runs in a defer, and + // with an unnamed return Go copies the result value before defers run, + // so a failed destroy would never reach the caller and the matrix + // would exit 0 having leaked a clone. + res = matrixJobResult{Index: index, Cmd: job, ExitCode: -1} + logPath := filepath.Join(logDir, fmt.Sprintf("job-%d.log", index)) + res.Log = logPath + + logFile, err := os.Create(logPath) // #nosec G304 -- logDir is ours or the user's own --logs + if err != nil { + res.Error = err.Error() + return res + } + defer func() { _ = logFile.Close() }() //nolint:errcheck // the job result carries the real error + + // Every clone comes from the same paused snapshot, so they are made + // one at a time and resumed on the spot. + forks, err := forkN(ctx, client, golden, api.SandboxForkReq{Egress: opts.Egress}, 1, nil) + if err != nil { + res.Error = err.Error() + // A fork that was created but never settled is running and + // billable, and this job is the only thing that knows its id. + var leak *forkLeak + if errors.As(err, &leak) { + for _, id := range leak.IDs { + destroyQuiet(ctx, client, id, func(msg string) { + res.Error += "\n " + msg + }) + } + } + return res + } + clone := forks[0] + res.Sandbox = clone.ID + defer destroyQuiet(ctx, client, clone.ID, func(msg string) { + // A teardown failure must reach the result. The job may have + // passed, but a clone nobody destroyed keeps billing, and + // swallowing this is how that goes unnoticed. + if res.Error == "" { + res.Error = msg + } + }) + + // A fork does not inherit its source's auto-pause — measured: a golden + // box with auto_pause=900 produced clones with auto_pause=None (and a + // different shape). Without this, a matrix that dies mid-run leaves + // every clone running with nothing left to stop it. + if opts.AutoPause > 0 { + secs := int(opts.AutoPause.Seconds()) + if _, pauseErr := client.SetAutoPause(ctx, clone.ID, &secs); pauseErr != nil { + res.Error = fmt.Sprintf("could not set the auto-pause backstop on %s: %v", clone.ID, pauseErr) + return res + } + } + + out, err := runManaged(ctx, client, clone.ID, job, composeWorkDir, opts.Env, logFile) + if err != nil { + res.Error = err.Error() + return res + } + res.ExitCode = out.ExitCode + res.DurationMs = out.Duration.Milliseconds() + return res +} + +func matrixPrintOne(r matrixJobResult) { + switch { + case r.Error != "": + pterm.Error.Printfln("job %d failed to run: %s", r.Index, r.Error) + case r.ExitCode == 0: + pterm.Success.Printfln("job %d ok in %s — %s", r.Index, + time.Duration(r.DurationMs)*time.Millisecond, r.Cmd) + default: + pterm.Error.Printfln("job %d exited %d — %s", r.Index, r.ExitCode, r.Cmd) + } +} + +func matrixPrintSummary(results []matrixJobResult, logDir string) { + rows := make([][]string, 0, 1+len(results)) + rows = append(rows, []string{"JOB", "EXIT", "TIME", "COMMAND"}) + for _, r := range results { + exit := fmt.Sprintf("%d", r.ExitCode) + if r.Error != "" { + exit = "error" + } + rows = append(rows, []string{ + fmt.Sprintf("%d", r.Index), + exit, + (time.Duration(r.DurationMs) * time.Millisecond).Round(time.Millisecond).String(), + r.Cmd, + }) + } + _ = pterm.DefaultTable.WithHasHeader().WithData(rows).Render() //nolint:errcheck // a failed table render must not change the exit code + fmt.Printf("\nLogs: %s\n", logDir) +} diff --git a/cmd/sandbox/offload.go b/cmd/sandbox/offload.go new file mode 100644 index 0000000..403098f --- /dev/null +++ b/cmd/sandbox/offload.go @@ -0,0 +1,343 @@ +package sandbox + +import ( + "archive/tar" + "context" + "errors" + "fmt" + "io" + "os" + "path" + "path/filepath" + "slices" + "strings" + "time" + + "github.com/pterm/pterm" + "github.com/urfave/cli/v2" + + "github.com/NodeOps-app/createos-cli/internal/api" + "github.com/NodeOps-app/createos-cli/internal/output" +) + +func newOffloadCommand() *cli.Command { + return &cli.Command{ + Name: "offload", + Usage: "Run one command on a throwaway sandbox, then destroy it", + ArgsUsage: " -- ", + Description: `Offload moves one piece of work off your machine. It creates a sandbox, +uploads the directory, runs the command inside it, brings back anything you +asked for, and destroys the sandbox — whether the command passed or failed. + +Use it for work with a finish line: a test suite, a build, a migration, a +script you did not write. For work that must outlive one command — a dev +server, a watcher — create a sandbox and keep it instead. + +The upload honours .gitignore, so node_modules and build output stay on your +machine. Outside a git repository a fixed skip-list stands in. + +The command's exit code becomes this command's exit code. + +Flags must come before the directory. Anything after it belongs to the +command you are running. + +Examples: + # Run a test suite somewhere else + createos sandbox offload . -- bun test + + # Lock the sandbox to the npm registry and nothing else + createos sandbox offload --egress-preset npm . -- npm ci + + # Bring the coverage report back + createos sandbox offload --fetch coverage . -- bun test --coverage + + # Keep the sandbox when the command fails, so you can shell in and look + createos sandbox offload --keep-on-fail . -- make build`, + Flags: append(composeFlags(), + &cli.StringSliceFlag{ + Name: "fetch", + Usage: "Path inside the work directory to download when the command finishes (repeatable)", + }, + &cli.BoolFlag{ + Name: "keep-on-fail", + Usage: "Leave the sandbox alive when the command exits non-zero, so you can inspect it", + }, + ), + Action: runOffload, + } +} + +func runOffload(c *cli.Context) error { + client, ok := c.App.Metadata[api.SandboxClientKey].(*api.SandboxClient) + if !ok { + return fmt.Errorf("you're not signed in — run 'createos login' to get started") + } + + dir, cmd, err := splitDirAndCommand(c) + if err != nil { + return err + } + opts, err := parseComposeOptions(c) + if err != nil { + return err + } + + ctx := c.Context + if opts.Timeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, opts.Timeout) + defer cancel() + } + + quiet := output.IsJSON(c) + say := func(format string, a ...any) { + if !quiet { + pterm.Info.Printfln(format, a...) + } + } + + tree, err := stageDir(ctx, dir, stageOptions{Exclude: opts.Exclude}) + if err != nil { + return err + } + defer func() { _ = os.Remove(tree.Path) }() //nolint:errcheck // temp file; removal failure is benign + say("Packed %d file(s), %s", tree.Files, humanBytes(tree.Size)) + + if len(opts.Egress) == 0 { + say("Egress unrestricted — this sandbox can reach any host. Restrict it with --egress or --egress-preset.") + } + + sb, err := createComposeBox(ctx, client, opts) + if err != nil { + return err + } + say("Sandbox %s is up", sb.ID) + + destroyed := false + defer func() { + if !destroyed { + destroyQuiet(ctx, client, sb.ID, func(msg string) { pterm.Warning.Println(msg) }) + } + }() + + if shipErr := shipTree(ctx, client, sb.ID, tree, composeWorkDir); shipErr != nil { + return shipErr + } + + out := io.Writer(os.Stdout) + if quiet { + out = io.Discard + } + res, err := runManaged(ctx, client, sb.ID, cmd, composeWorkDir, opts.Env, out) + if err != nil { + return err + } + + if len(c.StringSlice("fetch")) > 0 { + if err := fetchPaths(ctx, client, sb.ID, composeWorkDir, c.StringSlice("fetch"), dir); err != nil { + return fmt.Errorf("fetch results: %w", err) + } + say("Fetched %s into %s", strings.Join(c.StringSlice("fetch"), ", "), dir) + } + + teardownFailure := "" + if res.ExitCode != 0 && c.Bool("keep-on-fail") { + destroyed = true + pterm.Warning.Printfln("Command exited %d. Sandbox %s kept.", res.ExitCode, sb.ID) + fmt.Printf(" Look around: createos sandbox shell %s\n", sb.ID) + fmt.Printf(" Destroy it: createos sandbox rm --force %s\n", sb.ID) + } else { + destroyQuiet(ctx, client, sb.ID, func(msg string) { teardownFailure = msg }) + destroyed = true + } + + if quiet { + output.Render(c, map[string]any{ + "sandbox": sb.ID, + "command": cmd, + "exit_code": res.ExitCode, + "duration_ms": res.Duration.Milliseconds(), + "teardown_failure": teardownFailure, + }, func() {}) + } else if res.ExitCode == 0 && teardownFailure == "" { + pterm.Success.Printfln("Done in %s", res.Duration.Round(time.Millisecond)) + } + + // A teardown failure has to change the exit code. `offload` promises a + // throwaway sandbox; a leaked one keeps billing, and a CI job that only + // reads the exit status would call this a clean run and never find out. + if teardownFailure != "" { + return fmt.Errorf( + "the command exited %d, but the sandbox was not destroyed: %s\n\n It is still billable. Remove it with:\n createos sandbox rm --force %s", + res.ExitCode, teardownFailure, sb.ID) + } + if res.ExitCode != 0 { + return cli.Exit("", res.ExitCode) + } + return nil +} + +// splitDirAndCommand pulls and the command out of the argument +// list: the first argument is the directory, the rest is the command. +// +// urfave/cli hands `--` through as an ordinary argument rather than eating +// it, so a literal "--" would end up at the front of the command and the +// shell would fail on it. Dropping it here means both spellings work — +// `offload . -- bun test` and `offload . bun test` — which matters because +// the separator is the single most common thing to get wrong on a command +// shaped like this one. +func splitDirAndCommand(c *cli.Context) (dir, cmd string, err error) { + args := c.Args().Slice() + if len(args) < 2 { + return "", "", errors.New( + "please give a directory and a command\n\n Example:\n createos sandbox offload . -- bun test") + } + dir = args[0] + rest := args[1:] + // Anything between the directory and `--` was meant as a flag and was + // not read as one. Catch it before it is pasted into the command line + // and the shell reports something unrelated. + if sep := slices.Index(rest, "--"); sep > 0 { + if flagErr := checkMisplacedFlags(c, rest[:sep]); flagErr != nil { + return "", "", flagErr + } + } + if rest[0] == "--" { + rest = rest[1:] + } + cmd = strings.TrimSpace(strings.Join(rest, " ")) + if cmd == "" { + return "", "", errors.New( + "the command is empty\n\n Example:\n createos sandbox offload . -- bun test") + } + return dir, cmd, nil +} + +// fetchPaths downloads the named paths out of the sandbox and unpacks them +// under localRoot, keeping their relative layout. +// +// One tar for the whole set, not one download per path: the file API moves +// a single stream far better than N round trips, and it keeps directory +// trees intact. +// +// Never point this at a mounted S3 disk. Reading a disk mount through the +// file API is a known crash (issue #71) — copy what you need out of the +// mount from inside the sandbox first. +func fetchPaths(ctx context.Context, client *api.SandboxClient, sandboxID, remoteRoot string, paths []string, localRoot string) error { + quoted := make([]string, 0, len(paths)) + for _, p := range paths { + p = strings.TrimPrefix(strings.TrimSpace(p), "/") + if p == "" || strings.Contains(p, "..") { + return fmt.Errorf("--fetch %q must be a path inside the work directory", p) + } + quoted = append(quoted, shellQuote(p)) + } + const remoteTar = "/tmp/createos-fetch.tar" + pack := fmt.Sprintf("set -eu\ncd %s\ntar -cf %s %s\n", + shellQuote(remoteRoot), remoteTar, strings.Join(quoted, " ")) + resp, err := client.ExecSandbox(ctx, sandboxID, api.SandboxExecReq{Cmd: "bash", Args: []string{"-lc", pack}}) + if err != nil { + return err + } + if resp.Result.ExitCode != 0 { + return fmt.Errorf("packing the requested paths exited %d: %s", + resp.Result.ExitCode, strings.TrimSpace(resp.Result.Stderr)) + } + + tmp, err := os.CreateTemp("", "createos-fetch-*.tar") + if err != nil { + return err + } + defer func() { + _ = tmp.Close() //nolint:errcheck // read path below owns the error + _ = os.Remove(tmp.Name()) //nolint:errcheck // temp file + }() + if _, err := client.DownloadFile(ctx, sandboxID, remoteTar, tmp); err != nil { + return err + } + if _, err := tmp.Seek(0, io.SeekStart); err != nil { + return err + } + return untarInto(tmp, localRoot) +} + +// untarInto extracts r under root, refusing any entry that would escape it. +// +// Every write goes through os.Root, which resolves names relative to an +// open directory descriptor and refuses any component that leaves the +// root — a symlink included. A lexical prefix check is not enough here: +// it validates the pathname this code builds, while MkdirAll and OpenFile +// still follow a symlink that already exists on the caller's disk. A repo +// holding `coverage -> /etc` plus a sandbox-built entry `coverage/passwd` +// is enough to write outside the tree (CWE-22, CWE-59). The archive is +// produced inside a sandbox that ran code the user did not write, so it +// is untrusted by construction. +func untarInto(r io.Reader, root string) error { + if err := os.MkdirAll(root, 0o750); err != nil { + return err + } + rootDir, err := os.OpenRoot(root) + if err != nil { + return err + } + defer func() { _ = rootDir.Close() }() //nolint:errcheck // read side owns the real error + + tr := tar.NewReader(r) + for { + hdr, nextErr := tr.Next() + if errors.Is(nextErr, io.EOF) { + return nil + } + if nextErr != nil { + return nextErr + } + name := path.Clean("/" + filepath.ToSlash(hdr.Name)) + name = strings.TrimPrefix(name, "/") + if name == "" || name == "." { + continue + } + switch hdr.Typeflag { + case tar.TypeDir: + if err := rootDir.MkdirAll(name, 0o750); err != nil { + return fmt.Errorf("archive entry %q: %w", hdr.Name, err) + } + case tar.TypeReg: + if dir := path.Dir(name); dir != "." { + if err := rootDir.MkdirAll(dir, 0o750); err != nil { + return fmt.Errorf("archive entry %q: %w", hdr.Name, err) + } + } + if err := writeFetchedFile(rootDir, tr, name, hdr.FileInfo().Mode()); err != nil { + return fmt.Errorf("archive entry %q: %w", hdr.Name, err) + } + default: + // Symlinks and devices out of a sandbox have no safe meaning + // on the caller's disk. Skip them rather than guess. + continue + } + } +} + +// fetchFileMaxBytes bounds one extracted file. A sandbox-built archive is +// untrusted, and an unbounded copy is a decompression bomb (CWE-409). +const fetchFileMaxBytes int64 = 2 << 30 + +func writeFetchedFile(rootDir *os.Root, r io.Reader, name string, mode os.FileMode) error { + // rootDir resolves name against an open directory descriptor, so a + // symlink anywhere in the path cannot reach outside the extraction + // root. One that stays inside it is harmless: the write still lands in + // the tree the caller asked for. + f, err := rootDir.OpenFile(name, os.O_CREATE|os.O_TRUNC|os.O_WRONLY, mode.Perm()&0o755) + if err != nil { + return err + } + defer func() { _ = f.Close() }() //nolint:errcheck // the copy error below is the one that matters + written, err := io.Copy(f, io.LimitReader(r, fetchFileMaxBytes)) + if err != nil { + return err + } + if written == fetchFileMaxBytes { + return fmt.Errorf("%s is over the %s per-file fetch limit", name, humanBytes(fetchFileMaxBytes)) + } + return nil +} diff --git a/cmd/sandbox/pull.go b/cmd/sandbox/pull.go index 28ca621..806e24d 100644 --- a/cmd/sandbox/pull.go +++ b/cmd/sandbox/pull.go @@ -55,6 +55,14 @@ func runPull(c *cli.Context) error { return err } + mount, err := diskMountBlocksFileAPI(c.Context, client, id, remote) + if err != nil { + return err + } + if mount != "" { + return diskMountFileAPIError(remote, mount, "pull") + } + f, err := os.Create(local) // #nosec G304 -- local is a user-supplied destination path if err != nil { return fmt.Errorf("could not create %s: %w", local, err) diff --git a/cmd/sandbox/push.go b/cmd/sandbox/push.go index a8a6d1b..7ca7065 100644 --- a/cmd/sandbox/push.go +++ b/cmd/sandbox/push.go @@ -58,6 +58,13 @@ func runPush(c *cli.Context) error { if err != nil { return err } + mount, err := diskMountBlocksFileAPI(c.Context, client, id, remote) + if err != nil { + return err + } + if mount != "" { + return diskMountFileAPIError(remote, mount, "push") + } // Open the source: a real file (we know its size for Content-Length) // or stdin ("-") for piped uploads. diff --git a/cmd/sandbox/sandbox.go b/cmd/sandbox/sandbox.go index 5353359..cd67973 100644 --- a/cmd/sandbox/sandbox.go +++ b/cmd/sandbox/sandbox.go @@ -15,6 +15,8 @@ func NewSandboxCommand() *cli.Command { Usage: "Manage sandboxes", Subcommands: []*cli.Command{ newRunCommand(), + newOffloadCommand(), + newMatrixCommand(), newCreateCommand(), newListCommand(), newGetCommand(), @@ -23,6 +25,7 @@ func NewSandboxCommand() *cli.Command { newPauseCommand(), newResumeCommand(), newForkCommand(), + newSelfCommand(), newExecCommand(), newProcessCommand(), newPushCommand(), diff --git a/cmd/sandbox/self.go b/cmd/sandbox/self.go new file mode 100644 index 0000000..58ae27d --- /dev/null +++ b/cmd/sandbox/self.go @@ -0,0 +1,206 @@ +package sandbox + +import ( + "context" + "errors" + "fmt" + "net" + "net/http" + "net/url" + "os" + "strings" + "time" + + "github.com/pterm/pterm" + "github.com/urfave/cli/v2" + + "github.com/NodeOps-app/createos-cli/internal/output" + "github.com/NodeOps-app/createos-cli/internal/terminal" +) + +// The guest agent listens on loopback inside every sandbox. Loopback-only +// is the whole security model: nothing outside the sandbox can reach it, +// so it needs no credential — and a sandbox can only ever signal itself. +const ( + selfSignalAddr = "127.0.0.1:1029" + selfFifoPath = "/run/self" + selfDialWait = 2 * time.Second +) + +// selfSignalAddrForTest is the address selfSignalHTTP dials. It is a +// variable purely so a test can point it at a stub agent; nothing else +// ever reassigns it. +var selfSignalAddrForTest = selfSignalAddr + +func newSelfCommand() *cli.Command { + return &cli.Command{ + Name: "self", + Usage: "Pause or delete the sandbox this command is running inside", + Description: `Self-signal lets a workload end its own sandbox from the inside. + +Run these INSIDE a sandbox, not on your laptop. There is no sandbox id to +pass and no API key involved: the agent listens on loopback only, so the +only sandbox you can signal is the one you are in. + +Use it when a job knows it is finished long before anything outside does — +a batch run, a CI job, a one-shot agent task. The machine is released the +moment the last line executes, with no polling loop and no credential +inside the sandbox. + +Examples: + # Park this sandbox; resume it later from outside + createos sandbox self pause --reason job-complete + + # Destroy this sandbox. Irreversible. + createos sandbox self delete + +Without this CLI, the same signals are one line each: + curl -X POST http://127.0.0.1:1029/self/pause + echo park > /run/self`, + Subcommands: []*cli.Command{ + { + Name: "pause", + Usage: "Pause this sandbox, keeping its disk and memory", + Flags: selfFlags(), + Action: runSelf("pause"), + }, + { + Name: "delete", + Aliases: []string{"destroy", "rm"}, + Usage: "Destroy this sandbox. Irreversible", + Flags: append(selfFlags(), &cli.BoolFlag{ + Name: "force", + Aliases: []string{"f", "yes", "y"}, + Usage: "Skip the confirmation prompt", + }), + Action: runSelf("delete"), + }, + }, + } +} + +func selfFlags() []cli.Flag { + return []cli.Flag{ + &cli.StringFlag{ + Name: "reason", + Usage: "Free-text label recorded with the signal (truncated to 128 characters)", + }, + } +} + +func runSelf(action string) cli.ActionFunc { + return func(c *cli.Context) error { + // Destroying a sandbox cannot be undone, and this command is most + // often typed inside a shell on a box someone is still using. + if action == "delete" && !c.Bool("force") { + if !terminal.IsInteractive() { + return errors.New( + "deleting this sandbox is irreversible — pass --force to confirm\n\n Example:\n createos sandbox self delete --force") + } + ok, err := pterm.DefaultInteractiveConfirm. + WithDefaultText("Destroy this sandbox? Everything on it is lost"). + Show() + if err != nil { + return err + } + if !ok { + fmt.Println("Cancelled. Nothing changed.") + return nil + } + } + + reason := strings.TrimSpace(c.String("reason")) + if err := sendSelfSignal(c.Context, action, reason); err != nil { + return err + } + + if output.IsJSON(c) { + output.Render(c, map[string]any{"status": "accepted", "action": action, "reason": reason}, func() {}) + return nil + } + switch action { + case "pause": + pterm.Success.Println("Pause accepted. This sandbox is being snapshotted.") + fmt.Println(" Bring it back from outside with: createos sandbox resume ") + default: + pterm.Success.Println("Delete accepted. This sandbox is going away.") + } + return nil + } +} + +// sendSelfSignal delivers one signal to the guest agent, preferring HTTP +// and falling back to the FIFO. +// +// Both surfaces exist because the FIFO works in images with no curl and +// no working loopback HTTP stack. Trying HTTP first keeps the useful part +// of the failure — the agent answers with a status code — and only drops +// to the pipe, which is fire-and-forget, when HTTP is not there at all. +func sendSelfSignal(ctx context.Context, action, reason string) error { + httpErr := selfSignalHTTP(ctx, action, reason) + if httpErr == nil { + return nil + } + if fifoErr := selfSignalFIFO(action); fifoErr == nil { + return nil + } + return notInsideSandboxError(action, httpErr) +} + +func selfSignalHTTP(ctx context.Context, action, reason string) error { + endpoint := "http://" + selfSignalAddrForTest + "/self/" + action + if reason != "" { + endpoint += "?reason=" + url.QueryEscape(reason) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, endpoint, nil) + if err != nil { + return err + } + client := &http.Client{Timeout: selfDialWait} + resp, err := client.Do(req) + if err != nil { + return err + } + defer func() { _ = resp.Body.Close() }() //nolint:errcheck // status code is what matters here + // The agent answers 202 Accepted and then acts. Anything else means + // something is listening on that port that is not the guest agent. + if resp.StatusCode != http.StatusAccepted && resp.StatusCode != http.StatusOK { + return fmt.Errorf("the agent on %s answered %s", selfSignalAddrForTest, resp.Status) + } + return nil +} + +// selfSignalFIFO writes one verb to /run/self. The pipe takes no reason, +// so a reason given on the command line is dropped on this path. +func selfSignalFIFO(action string) error { + verb := "park" + if action == "delete" { + verb = "retire" + } + f, err := os.OpenFile(selfFifoPath, os.O_WRONLY, 0) + if err != nil { + return err + } + defer func() { _ = f.Close() }() //nolint:errcheck // the write error below is the one that matters + _, err = f.WriteString(verb + "\n") + return err +} + +// notInsideSandboxError is the message for the most likely mistake: +// running this on a laptop. Naming the alternative matters, because the +// command that does work from outside takes a sandbox id and this one +// does not. +func notInsideSandboxError(action string, cause error) error { + var opErr *net.OpError + inside := "no agent is listening" + if errors.As(cause, &opErr) || strings.Contains(cause.Error(), "connection refused") { + inside = "nothing answered on " + selfSignalAddr + } + outside := "pause" + if action == "delete" { + outside = "rm --force" + } + return fmt.Errorf( + "could not signal this sandbox — %s\n\n 'sandbox self' only works INSIDE a sandbox.\n From your own machine, name the sandbox instead:\n createos sandbox %s \n\n Underlying error: %w", + inside, outside, cause) +} diff --git a/cmd/sandbox/stage.go b/cmd/sandbox/stage.go new file mode 100644 index 0000000..98a1e3d --- /dev/null +++ b/cmd/sandbox/stage.go @@ -0,0 +1,287 @@ +package sandbox + +import ( + "archive/tar" + "context" + "fmt" + "io" + "io/fs" + "os" + "os/exec" + "path/filepath" + "strings" + "time" + + "github.com/NodeOps-app/createos-cli/internal/api" +) + +// stageDefaultExcludes are directories a build regenerates. They are big, +// they are usually the largest thing in a tree, and shipping them is the +// difference between a 2 MB upload and a 900 MB one. This list only +// applies outside a git repository — inside one, .gitignore already says +// what belongs, and it says it better than any fixed list can. +var stageDefaultExcludes = []string{ + ".git", "node_modules", "target", "__pycache__", ".venv", "venv", + "dist", "build", ".next", ".turbo", ".cache", "vendor", +} + +// stageMaxBytes caps the upload. The file API refuses more than 500 MB, +// and hitting that limit after a two-minute upload is a bad way to find +// out. Failing early with the measured size names the problem instead. +const stageMaxBytes int64 = 500 << 20 + +// stageOptions tunes what stageDir packs. +type stageOptions struct { + // IncludeGit ships the .git directory. Orca needs it (its remote git + // reads the history); offload and matrix do not, and it is often the + // bulk of the payload. + IncludeGit bool + // Exclude adds path prefixes to skip, on top of .gitignore. + Exclude []string +} + +// stagedTree is a packed directory ready to upload. +type stagedTree struct { + Path string // temp tar on the local disk; the caller removes it + Size int64 + Files int +} + +// stageDir packs dir into a tar file on local disk and returns its path. +// UploadFile needs the length up front, so the archive is staged rather +// than streamed. +// +// Inside a git repository the file list comes from `git ls-files --cached +// --others --exclude-standard`: tracked files plus untracked ones that +// .gitignore does not exclude. That is "what the user sees on their +// laptop" minus the build output, and it needs no exclude list of ours. +// Outside a repository there is no such signal, so stageDefaultExcludes +// stands in. +func stageDir(ctx context.Context, dir string, opts stageOptions) (*stagedTree, error) { + abs, err := filepath.Abs(dir) + if err != nil { + return nil, fmt.Errorf("resolve %s: %w", dir, err) + } + info, err := os.Stat(abs) // #nosec G703 -- abs comes from filepath.Abs of a user-named directory + if err != nil { + return nil, fmt.Errorf("no such directory: %s", dir) + } + if !info.IsDir() { + return nil, fmt.Errorf("%s is a file, not a directory", dir) + } + + paths, err := stageFileList(ctx, abs, opts) + if err != nil { + return nil, err + } + if len(paths) == 0 { + return nil, fmt.Errorf("%s has nothing to send — every file in it is ignored or excluded", dir) + } + + tmp, err := os.CreateTemp("", "createos-stage-*.tar") + if err != nil { + return nil, err + } + defer func() { _ = tmp.Close() }() //nolint:errcheck // the Stat below owns the real error + + tw := tar.NewWriter(tmp) + for _, rel := range paths { + if err = stageTarAppend(tw, abs, rel); err != nil { + _ = tw.Close() //nolint:errcheck // already unwinding + _ = os.Remove(tmp.Name()) //nolint:errcheck // best-effort cleanup + return nil, err + } + } + if err = tw.Close(); err != nil { + _ = os.Remove(tmp.Name()) //nolint:errcheck // best-effort cleanup + return nil, err + } + st, err := tmp.Stat() + if err != nil { + _ = os.Remove(tmp.Name()) //nolint:errcheck // best-effort cleanup + return nil, err + } + if st.Size() > stageMaxBytes { + _ = os.Remove(tmp.Name()) //nolint:errcheck // best-effort cleanup + return nil, fmt.Errorf( + "%s packs to %s, over the %s upload limit\n\n Exclude what the sandbox does not need:\n --exclude (repeatable)", + dir, humanBytes(st.Size()), humanBytes(stageMaxBytes)) + } + return &stagedTree{Path: tmp.Name(), Size: st.Size(), Files: len(paths)}, nil +} + +// stageFileList returns the repo-relative paths to pack. +func stageFileList(ctx context.Context, abs string, opts stageOptions) ([]string, error) { + paths, gitErr := stageGitFileList(ctx, abs) + if gitErr != nil { + var walkErr error + if paths, walkErr = stageWalkFileList(abs); walkErr != nil { + return nil, walkErr + } + } else if opts.IncludeGit { + // git ls-files never lists .git itself, so walk it separately. + gitDir, err := stageWalkDir(abs, ".git") + if err != nil { + return nil, err + } + paths = append(paths, gitDir...) + } + if len(opts.Exclude) == 0 { + return paths, nil + } + kept := paths[:0] + for _, p := range paths { + if !stageExcluded(p, opts.Exclude) { + kept = append(kept, p) + } + } + return kept, nil +} + +// stageGitFileList asks git what the working tree holds. A non-nil error +// means "not a git repository" (or no git binary), not a hard failure. +func stageGitFileList(ctx context.Context, abs string) ([]string, error) { + out, err := exec.CommandContext(ctx, "git", "-C", abs, //#nosec G204,G702 -- abs is filepath.Abs of a user-named directory + "ls-files", "-z", "--cached", "--others", "--exclude-standard").Output() + if err != nil { + return nil, err + } + paths := make([]string, 0, 4096) + for _, p := range strings.Split(string(out), "\x00") { + if p != "" { + paths = append(paths, p) + } + } + return paths, nil +} + +// stageWalkFileList is the non-git fallback: walk the tree, skipping the +// directories a build regenerates. +func stageWalkFileList(abs string) ([]string, error) { + var paths []string + err := filepath.WalkDir(abs, func(p string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return nil //nolint:nilerr // an unreadable entry is not worth failing the whole stage + } + rel, relErr := filepath.Rel(abs, p) + if relErr != nil || rel == "." { + return nil //nolint:nilerr + } + rel = filepath.ToSlash(rel) + if d.IsDir() { + if stageExcluded(rel, stageDefaultExcludes) { + return filepath.SkipDir + } + return nil + } + paths = append(paths, rel) + return nil + }) + return paths, err +} + +// stageWalkDir collects every file under abs/sub, relative to abs. +func stageWalkDir(abs, sub string) ([]string, error) { + var paths []string + err := filepath.WalkDir(filepath.Join(abs, sub), func(p string, d fs.DirEntry, walkErr error) error { + if walkErr != nil || d.IsDir() { + return nil //nolint:nilerr + } + if rel, relErr := filepath.Rel(abs, p); relErr == nil { + paths = append(paths, filepath.ToSlash(rel)) + } + return nil + }) + return paths, err +} + +// stageExcluded reports whether rel is excluded. +// +// An exclude matches two ways: as a path prefix ("build/out" excludes +// "build/out/app.js"), or as any single segment of the path +// ("node_modules" excludes "src/node_modules/x.js"). The segment rule is +// the one that matters — a monorepo has a node_modules under every +// package, and a user who writes --exclude node_modules means all of them. +func stageExcluded(rel string, excludes []string) bool { + segments := strings.Split(rel, "/") + for _, ex := range excludes { + ex = strings.Trim(filepath.ToSlash(ex), "/") + if ex == "" { + continue + } + if rel == ex || strings.HasPrefix(rel, ex+"/") { + return true + } + if !strings.Contains(ex, "/") { + for _, seg := range segments { + if seg == ex { + return true + } + } + } + } + return false +} + +func stageTarAppend(tw *tar.Writer, root, rel string) error { + abs := filepath.Join(root, rel) + info, err := os.Lstat(abs) // #nosec G703 -- rel comes from git ls-files or a walk of root, never ".." + if err != nil { + return nil //nolint:nilerr // a file deleted mid-walk is not fatal + } + link := "" + if info.Mode()&os.ModeSymlink != 0 { + if link, err = os.Readlink(abs); err != nil { + return nil //nolint:nilerr + } + } else if !info.Mode().IsRegular() { + return nil // sockets, fifos, devices have no place in a checkout + } + hdr, err := tar.FileInfoHeader(info, link) + if err != nil { + return err + } + hdr.Name = filepath.ToSlash(rel) + if err = tw.WriteHeader(hdr); err != nil { + return err + } + if link != "" || !info.Mode().IsRegular() { + return nil + } + f, err := os.Open(abs) // #nosec G304,G703 -- see the Lstat note above + if err != nil { + return nil //nolint:nilerr + } + defer func() { _ = f.Close() }() //nolint:errcheck // read-only handle + _, err = io.Copy(tw, f) + return err +} + +// shipTree uploads a staged tar and unpacks it at remoteDir inside the +// sandbox. The tar is removed from the sandbox afterwards so it does not +// double the payload's footprint on the box's disk — and, for matrix, so +// it is not copied into every fork. +func shipTree(ctx context.Context, client *api.SandboxClient, id string, tree *stagedTree, remoteDir string) error { + f, err := os.Open(tree.Path) // #nosec G304,G703 -- path is from os.CreateTemp + if err != nil { + return fmt.Errorf("open staged tar: %w", err) + } + defer func() { _ = f.Close() }() //nolint:errcheck // read-only handle + + const remoteTar = "/tmp/createos-stage.tar" + start := time.Now() + if upErr := client.UploadFile(ctx, id, remoteTar, f, tree.Size); upErr != nil { + return fmt.Errorf("upload %s to %s after %s: %w", + humanBytes(tree.Size), id, time.Since(start).Round(time.Millisecond), upErr) + } + unpack := fmt.Sprintf("set -eu\nmkdir -p %s\ntar -xf %s -C %s\nrm -f %s\n", + shellQuote(remoteDir), remoteTar, shellQuote(remoteDir), remoteTar) + resp, err := client.ExecSandbox(ctx, id, api.SandboxExecReq{Cmd: "bash", Args: []string{"-lc", unpack}}) + if err != nil { + return fmt.Errorf("unpack in %s: %w", id, err) + } + if resp.Result.ExitCode != 0 { + return fmt.Errorf("unpack in %s exited %d: %s", id, resp.Result.ExitCode, strings.TrimSpace(resp.Result.Stderr)) + } + return nil +} diff --git a/internal/api/client.go b/internal/api/client.go index 30516d2..c2c9d0e 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -2,11 +2,14 @@ package api import ( + "errors" "fmt" "io" "log" + "net" "net/http" "strings" + "time" "github.com/go-resty/resty/v2" ) @@ -31,7 +34,9 @@ func installAuthRefresh(client *resty.Client, authHeader string, refresher Token if refresher == nil { return } - client.SetRetryCount(1) + // The retry budget itself belongs to installTransientRetry, which every + // constructor installs. This condition only adds one more reason to + // spend an attempt, and Attempt==1 below keeps it to a single refresh. client.AddRetryCondition(func(resp *resty.Response, _ error) bool { if resp == nil || resp.StatusCode() != http.StatusUnauthorized { return false @@ -55,13 +60,72 @@ func installAuthRefresh(client *resty.Client, authHeader string, refresher Token }) } +// Retry budget shared by installTransientRetry and installAuthRefresh. +// Three attempts covers a single flaky hop without turning a real outage +// into a long stall. +const ( + transientRetryCount = 3 + transientRetryWait = 300 * time.Millisecond + transientRetryMaxWait = 3 * time.Second +) + +// installTransientRetry retries the failures a retry can actually fix. +// One dropped connection used to kill a whole command; a fan-out over N +// sandboxes multiplies that exposure by N, so this is the difference +// between a flaky run and a failed one. +// +// What retries, and why the split by method: +// +// - A connection that was never established (DNS failure, dial timeout, +// connection refused). The request provably never reached the server, +// so replaying it cannot duplicate anything. Safe for every method, +// POST included. +// - A 429 or 5xx answer, but only for methods with no side effect. A +// POST that got a 500 may well have created the sandbox before it +// failed, and a retry would leak a second one that nobody destroys. +// Leaking billable machines is worse than surfacing the error. +func installTransientRetry(client *resty.Client) { + client.SetRetryCount(transientRetryCount) + client.SetRetryWaitTime(transientRetryWait) + client.SetRetryMaxWaitTime(transientRetryMaxWait) + client.AddRetryCondition(func(resp *resty.Response, err error) bool { + if err != nil { + return isConnectSetupError(err) + } + if resp == nil { + return false + } + switch resp.Request.Method { + case http.MethodGet, http.MethodHead, http.MethodOptions: + default: + return false + } + return resp.StatusCode() == http.StatusTooManyRequests || resp.StatusCode() >= http.StatusInternalServerError + }) +} + +// isConnectSetupError reports whether err failed before any bytes reached +// the server. Only "dial" operations qualify: a read or write error means +// the request was already on the wire and may have been acted on. +func isConnectSetupError(err error) bool { + var dnsErr *net.DNSError + if errors.As(err, &dnsErr) { + return true + } + var opErr *net.OpError + if errors.As(err, &opErr) { + return opErr.Op == "dial" + } + return false +} + // Auth header names. HTTP header keys are case-insensitive (and Go // canonicalises them on the wire), so these double as the API-key and // OAuth-access-token headers for both the main API and the fc-spawn // sandbox API. const ( - headerAPIKey = "X-Api-Key" // #nosec G101 -- HTTP header name, not a credential - headerAccessToken = "X-Access-Token" // #nosec G101 -- HTTP header name, not a credential + headerAPIKey = "X-Api-Key" // #nosec G101 -- HTTP header name, not a credential // pragma: allowlist secret + headerAccessToken = "X-Access-Token" // #nosec G101 -- HTTP header name, not a credential // pragma: allowlist secret ) // DefaultBaseURL is the default CreateOS API base URL. @@ -91,6 +155,8 @@ func NewClient(token, apiURL string, debug bool) APIClient { }) } + installTransientRetry(client) + return APIClient{Client: client} } @@ -115,6 +181,7 @@ func NewClientWithAccessToken(accessToken, apiURL string, debug bool, refresher }) } + installTransientRetry(client) installAuthRefresh(client, headerAccessToken, refresher) return APIClient{Client: client} diff --git a/internal/api/client_retry_test.go b/internal/api/client_retry_test.go new file mode 100644 index 0000000..bcced65 --- /dev/null +++ b/internal/api/client_retry_test.go @@ -0,0 +1,117 @@ +package api + +import ( + "context" + "errors" + "net" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" +) + +// TestLifecyclePOSTSendsContentLength guards the pause/resume regression. +// A body-less resty POST makes Go omit Content-Length, and control's +// forwarder drops any inbound content-length header, so the owning host +// answered "Content-Length is required" and every pause and resume failed +// while fork (which always had a body) kept working. +func TestLifecyclePOSTSendsContentLength(t *testing.T) { + for _, tc := range []struct { + name string + call func(*SandboxClient, context.Context) error + }{ + {"pause", func(c *SandboxClient, ctx context.Context) error { + _, err := c.PauseSandbox(ctx, "sb-1") + return err + }}, + {"resume", func(c *SandboxClient, ctx context.Context) error { + _, err := c.ResumeSandbox(ctx, "sb-1") + return err + }}, + } { + t.Run(tc.name, func(t *testing.T) { + var gotLength int64 = -1 + var gotHeader, gotType string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotLength = r.ContentLength + gotHeader = r.Header.Get("Content-Length") + gotType = r.Header.Get("Content-Type") + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"data":{"id":"sb-1","status":"pausing"}}`)) + })) + defer srv.Close() + + client := NewSandboxClient("tok", srv.URL, false) + if err := tc.call(&client, context.Background()); err != nil { + t.Fatalf("call failed: %v", err) + } + if gotLength < 0 { + t.Errorf("ContentLength = %d, want >= 0 (unknown length means no header on the wire)", gotLength) + } + if gotHeader == "" { + t.Error("Content-Length header absent — this is the exact failure the fix targets") + } + if gotType != "application/json" { + t.Errorf("Content-Type = %q, want application/json (RequireJSON rejects anything else)", gotType) + } + }) + } +} + +// TestTransientRetryOnlyReplaysSafeRequests pins the split that keeps a +// retry from leaking a second billable sandbox: read-only methods retry on +// 5xx, mutating ones do not. +func TestTransientRetryOnlyReplaysSafeRequests(t *testing.T) { + for _, tc := range []struct { + name string + post bool + calls int32 + }{ + {"get retries on 500", false, transientRetryCount + 1}, + {"post does not retry on 500", true, 1}, + } { + t.Run(tc.name, func(t *testing.T) { + var calls int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + atomic.AddInt32(&calls, 1) + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + + client := NewSandboxClient("tok", srv.URL, false) + req := client.Client.R() + var err error + if tc.post { + _, err = req.SetBody(struct{}{}).Post("/v1/thing") + } else { + _, err = req.Get("/v1/thing") + } + if err != nil { + t.Fatalf("request error: %v", err) + } + if got := atomic.LoadInt32(&calls); got != tc.calls { + t.Errorf("server saw %d call(s), want %d", got, tc.calls) + } + }) + } +} + +func TestIsConnectSetupError(t *testing.T) { + for _, tc := range []struct { + name string + err error + want bool + }{ + {"dns failure never reached the server", &net.DNSError{Err: "no such host"}, true}, + {"dial timeout never reached the server", &net.OpError{Op: "dial", Err: errors.New("i/o timeout")}, true}, + {"read error means the request was already sent", &net.OpError{Op: "read", Err: errors.New("reset")}, false}, + {"write error means the request was already sent", &net.OpError{Op: "write", Err: errors.New("broken pipe")}, false}, + {"plain error", errors.New("boom"), false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := isConnectSetupError(tc.err); got != tc.want { + t.Errorf("isConnectSetupError(%v) = %v, want %v", tc.err, got, tc.want) + } + }) + } +} diff --git a/internal/api/sandbox.go b/internal/api/sandbox.go index e129b75..feab790 100644 --- a/internal/api/sandbox.go +++ b/internal/api/sandbox.go @@ -393,13 +393,24 @@ func (c *SandboxClient) ForkSandbox(ctx context.Context, srcID string, req Sandb return &envelope.Data, nil } -// lifecyclePOST is the shared shape of pause/resume — body-less POST -// to /v1/sandboxes/{id}/, returning the updated view. +// lifecyclePOST is the shared shape of pause/resume — POST to +// /v1/sandboxes/{id}/, returning the updated view. +// +// The empty JSON body is load-bearing, not decoration. These actions carry +// no fields, but a resty request with no body at all makes Go omit +// Content-Length entirely, and control's forwarder drops any inbound +// content-length header before calling the owning host (see +// internal/control/handlers/forward.go in fc). The host then rejects the +// request with "Content-Length is required". ForkSandbox never hit this +// because it always had a body to send. Marshalling a struct also lets +// resty set Content-Type: application/json, which the RequireJSON +// middleware wants from any request that does carry a body. func (c *SandboxClient) lifecyclePOST(ctx context.Context, id, path string) (*SandboxView, error) { var envelope Response[SandboxView] resp, err := c.Client.R(). SetContext(ctx). SetPathParam("id", id). + SetBody(struct{}{}). SetResult(&envelope). Post(path) if err != nil { diff --git a/internal/api/sandbox_client.go b/internal/api/sandbox_client.go index 98c41a8..74d0fe8 100644 --- a/internal/api/sandbox_client.go +++ b/internal/api/sandbox_client.go @@ -67,6 +67,7 @@ func newSandboxClient(authHeader, token, sandboxURL string, debug bool, refreshe masked: maskToken(token), }) } + installTransientRetry(client) installAuthRefresh(client, authHeader, refresher) return SandboxClient{Client: client, authHeader: authHeader} } From 4fb5c4c186551d27e94f5e4b371cc3b8d12c29ae Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Fri, 28 Aug 2026 13:46:15 +0530 Subject: [PATCH 2/7] fix(sandbox): validate fork inputs and fail unknown commands fork --count was checked after resolving the source ref, so an invalid count still paid for an API lookup before failing. Move the check to the top of runFork. Unknown command names (root or nested) printed a suggestion but exited 0: urfave's CommandNotFoundFunc has no return value, so ShowCommandHelp always returns nil after calling it. Root's own custom Action also never reached that path at all, since urfave only falls back to CommandNotFoundFunc through its default help Action. Detect the unresolved name directly in each command's own Action instead, and return a real error so app.Run (and main.go's exit code) see the failure. Claude-Session: https://claude.ai/code/session_019J9PwFePTzAhpPs1TY9ZGq --- cmd/root/usage_error.go | 80 +++++++++++++++++++++++++++-------- cmd/root/usage_error_test.go | 79 ++++++++++++++++++++++++++++++++++ cmd/sandbox/fork.go | 7 +-- cmd/sandbox/lifecycle_test.go | 24 +++++++++++ 4 files changed, 169 insertions(+), 21 deletions(-) diff --git a/cmd/root/usage_error.go b/cmd/root/usage_error.go index 2098537..d9940c7 100644 --- a/cmd/root/usage_error.go +++ b/cmd/root/usage_error.go @@ -1,6 +1,7 @@ package root import ( + "errors" "fmt" "os" "strings" @@ -102,29 +103,72 @@ func correctedCommandLine(flagName string) string { } // installCommandSuggestions replaces urfave's bare "No help topic for -// 'ssh'" with the nearest real command. Agents and people both guess verb -// names, and a guess that lands one edit away from a real command should -// not cost a round trip to the help output. +// 'ssh'" with the nearest real command, and makes an unknown command name +// fail the run. Agents and people both guess verb names, and a guess that +// lands one edit away from a real command should not cost a round trip to +// the help output — and a guess that is simply wrong must not exit zero. +// +// This cannot be done with urfave's own CommandNotFoundFunc: ShowCommandHelp +// calls that callback and then unconditionally returns nil (see the +// package's help.go), so nothing set there can ever make app.Run return an +// error. Catching the unresolved name has to happen earlier, in the +// command's own Action, before urfave's help fallback runs. +// +// That is safe to do because an Action only ever runs with a positional +// argument still present when dispatch already failed to match that +// argument against a real subcommand — a match would have called that +// subcommand's Run instead. So "argument present here" and "unknown +// command" are the same condition. func installCommandSuggestions(app *cli.App) { - app.CommandNotFound = func(c *cli.Context, name string) { - // The hook fires for a subcommand too ("createos sandbox ssh"), and - // there the useful candidates are that group's subcommands, not the - // top-level verbs. Searching the wrong list is worse than staying - // quiet: it points at something unrelated. - candidates, prefix := app.Commands, "" - if cmd := c.Command; cmd != nil && len(cmd.Subcommands) > 0 { - candidates, prefix = cmd.Subcommands, cmd.Name+" " + fallback := app.Action + app.Action = func(c *cli.Context) error { + if name := c.Args().First(); name != "" { + return unknownCommandError(app.Commands, "", name) } - noun := "command" - if prefix != "" { - noun = "subcommand" + if fallback != nil { + return fallback(c) } - fmt.Fprintf(os.Stderr, "createos %s: %q is not a %s.\n", strings.TrimSpace(prefix), name, noun) - if best := nearestCommand(candidates, name); best != "" { - fmt.Fprintf(os.Stderr, "\n Did you mean:\n createos %s%s\n", prefix, best) + return cli.ShowSubcommandHelp(c) + } + for _, cmd := range app.Commands { + installGroupSuggestions(cmd) + } +} + +func installGroupSuggestions(cmd *cli.Command) { + if cmd == nil || len(cmd.Subcommands) == 0 { + return + } + prefix := cmd.Name + " " + fallback := cmd.Action + cmd.Action = func(c *cli.Context) error { + if name := c.Args().First(); name != "" { + return unknownCommandError(cmd.Subcommands, prefix, name) } - fmt.Fprintf(os.Stderr, "\n See everything with:\n createos %s--help\n", prefix) + if fallback != nil { + return fallback(c) + } + return cli.ShowSubcommandHelp(c) + } + for _, sub := range cmd.Subcommands { + installGroupSuggestions(sub) + } +} + +// unknownCommandError builds the same message the old CommandNotFound +// callback printed, but returns it instead of writing to stderr directly — +// main.go's error renderer prints whatever app.Run returns. +func unknownCommandError(candidates []*cli.Command, prefix, name string) error { + noun := "command" + if prefix != "" { + noun = "subcommand" + } + msg := fmt.Sprintf("createos %s: %q is not a %s.", strings.TrimSpace(prefix), name, noun) + if best := nearestCommand(candidates, name); best != "" { + msg += fmt.Sprintf("\n\n Did you mean:\n createos %s%s", prefix, best) } + msg += fmt.Sprintf("\n\n See everything with:\n createos %s--help", prefix) + return errors.New(msg) } // nearestCommand returns the closest command name within a small edit diff --git a/cmd/root/usage_error_test.go b/cmd/root/usage_error_test.go index 3e044a6..b6de1ab 100644 --- a/cmd/root/usage_error_test.go +++ b/cmd/root/usage_error_test.go @@ -3,6 +3,7 @@ package root import ( "errors" "os" + "strings" "testing" "github.com/urfave/cli/v2" @@ -95,6 +96,84 @@ func TestNearestCommand(t *testing.T) { } } +// newUnknownCommandTestApp builds a minimal command tree with the same +// shape as the real one (a root with subcommands, one of which is itself a +// group) and wires it through installCommandSuggestions exactly as +// root.NewApp does. It skips root.NewApp itself because that app's Before +// hook requires a signed-in session, which would make these tests depend on +// local auth state instead of on the routing bug being fixed. +func newUnknownCommandTestApp() *cli.App { + noop := func(_ *cli.Context) error { return nil } + app := &cli.App{ + Name: "createos", + Action: noop, // stands in for root.go's intro action + Commands: []*cli.Command{ + { + Name: "sandbox", + Subcommands: []*cli.Command{ + {Name: "offload", Action: noop}, + {Name: "list", Action: noop}, + }, + }, + {Name: "login", Action: noop}, + }, + } + installCommandSuggestions(app) + return app +} + +// TestUnknownCommandFailsTheRun pins the routing fix: urfave's own +// CommandNotFoundFunc can print a message but, per ShowCommandHelp in the +// library's help.go, can never make app.Run return an error — so both a +// root-level typo and a nested one used to print a suggestion and still +// exit 0. Every case here must return a non-nil error, since main.go's +// only success/failure signal is whether app.Run returned one. +func TestUnknownCommandFailsTheRun(t *testing.T) { + t.Run("root typo suggests the real command", func(t *testing.T) { + err := newUnknownCommandTestApp().Run([]string{"createos", "sandox"}) + if err == nil { + t.Fatal("want an error for an unknown top-level command") + } + if !strings.Contains(err.Error(), "createos sandbox") { + t.Errorf("error must suggest sandbox, got: %v", err) + } + }) + + t.Run("nested typo suggests the real subcommand", func(t *testing.T) { + err := newUnknownCommandTestApp().Run([]string{"createos", "sandbox", "ofload"}) + if err == nil { + t.Fatal("want an error for an unknown subcommand") + } + if !strings.Contains(err.Error(), "createos sandbox offload") { + t.Errorf("error must suggest sandbox offload, got: %v", err) + } + }) + + t.Run("gibberish gets an error but no false suggestion", func(t *testing.T) { + err := newUnknownCommandTestApp().Run([]string{"createos", "zzzzqqq"}) + if err == nil { + t.Fatal("want an error for an unknown top-level command") + } + if strings.Contains(err.Error(), "Did you mean") { + t.Errorf("must not guess a suggestion for gibberish input, got: %v", err) + } + }) +} + +// TestKnownCommandsStillDispatch guards against the fix being too broad: it +// must fail unresolved names, not every group invocation. +func TestKnownCommandsStillDispatch(t *testing.T) { + if err := newUnknownCommandTestApp().Run([]string{"createos", "sandbox", "list"}); err != nil { + t.Errorf("known subcommand must still dispatch normally, got: %v", err) + } + if err := newUnknownCommandTestApp().Run([]string{"createos", "sandbox"}); err != nil { + t.Errorf("a bare group with no subcommand must show help, not fail, got: %v", err) + } + if err := newUnknownCommandTestApp().Run([]string{"createos"}); err != nil { + t.Errorf("no arguments at all must still run the root action, got: %v", err) + } +} + func TestEditDistance(t *testing.T) { for _, tc := range []struct { a, b string diff --git a/cmd/sandbox/fork.go b/cmd/sandbox/fork.go index 04f7f6d..909c413 100644 --- a/cmd/sandbox/fork.go +++ b/cmd/sandbox/fork.go @@ -66,6 +66,10 @@ func runFork(c *cli.Context) error { return fmt.Errorf("you're not signed in — run 'createos login' to get started") } + if count := c.Int("count"); count < 1 { + return fmt.Errorf("--count must be at least 1 (got %d)", count) + } + ref := strings.TrimSpace(c.Args().First()) if ref == "" { if !terminal.IsInteractive() { @@ -90,9 +94,6 @@ func runFork(c *cli.Context) error { func runForkByID(c *cli.Context, client *api.SandboxClient, ref, srcID string) error { count := c.Int("count") - if count < 1 { - return fmt.Errorf("--count must be at least 1 (got %d)", count) - } req := api.SandboxForkReq{ StartPaused: c.Bool("paused"), diff --git a/cmd/sandbox/lifecycle_test.go b/cmd/sandbox/lifecycle_test.go index a90d698..2ada22d 100644 --- a/cmd/sandbox/lifecycle_test.go +++ b/cmd/sandbox/lifecycle_test.go @@ -303,6 +303,30 @@ func TestPauseForForkPausesASandboxTheCallerOwns(t *testing.T) { } } +// TestForkRejectsBadCountBeforeAnyAPICall covers the bug where --count was +// only checked after resolving the source ref, so `fork --count 0 missing` +// spent an API round trip on "missing" before ever complaining about the +// count. The count is knowable from the flags alone, so it must fail before +// any request goes out. +func TestForkRejectsBadCountBeforeAnyAPICall(t *testing.T) { + f := newFakeAPI(t) + app := &cli.App{ + Commands: []*cli.Command{newForkCommand()}, + Metadata: map[string]any{api.SandboxClientKey: f.client()}, + } + + err := app.RunContext(shortPoll(t), []string{"createos", "fork", "--count", "0", "missing"}) + if err == nil { + t.Fatal("want an error for --count 0") + } + if !strings.Contains(err.Error(), "--count must be at least 1 (got 0)") { + t.Errorf("error = %q, want it to name the bad count", err) + } + if len(f.seen) != 0 { + t.Errorf("count was invalid but the CLI still called the API: %v", f.seen) + } +} + // TestOffloadFailsWhenTeardownFails covers the leak that looks like a // clean run: the workload passes, DestroySandbox fails, and a CI job // reading only the exit status would never learn that a billable sandbox From 3b0a78c446e9a3c72621584f321b20102c3421d1 Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Fri, 28 Aug 2026 14:30:17 +0530 Subject: [PATCH 3/7] fix(sandbox): honor --count after sandbox ID Go's stdlib flag parsing stops at the first positional argument, so a command like `fork --count 2` left `--count` unparsed and defaulted to 1. Added `forkCountFlag` that falls back to scanning raw os.Args, restoring the intended count. --- cmd/sandbox/fork.go | 30 ++++++++++++++++++++++++-- cmd/sandbox/lifecycle_test.go | 40 +++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/cmd/sandbox/fork.go b/cmd/sandbox/fork.go index 909c413..1e9a426 100644 --- a/cmd/sandbox/fork.go +++ b/cmd/sandbox/fork.go @@ -3,6 +3,7 @@ package sandbox import ( "context" "fmt" + "strconv" "strings" "github.com/pterm/pterm" @@ -66,7 +67,7 @@ func runFork(c *cli.Context) error { return fmt.Errorf("you're not signed in — run 'createos login' to get started") } - if count := c.Int("count"); count < 1 { + if count := forkCountFlag(c); count < 1 { return fmt.Errorf("--count must be at least 1 (got %d)", count) } @@ -93,7 +94,7 @@ func runFork(c *cli.Context) error { } func runForkByID(c *cli.Context, client *api.SandboxClient, ref, srcID string) error { - count := c.Int("count") + count := forkCountFlag(c) req := api.SandboxForkReq{ StartPaused: c.Bool("paused"), @@ -300,3 +301,28 @@ type forkLeak struct { func (e *forkLeak) Error() string { return e.err.Error() } func (e *forkLeak) Unwrap() error { return e.err } + +// forkCountFlag reads --count the normal way, and falls back to a raw scan +// of os.Args when that comes back unset. +// +// Go's stdlib flag package, which urfave/cli sits on, stops parsing at the +// first non-flag argument. `fork --count 2` writes the sandbox +// first — the natural order — so `--count` is never parsed as a flag at +// all: it lands unread in c.Args(), and c.Int("count") silently returns +// the flag's default (1). No error, just the wrong count. This is the same +// shape of bug fixed for `process run --cwd` (commit 8c1f7ac); the +// fallback below reuses that fix's own raw-argv scanner. +func forkCountFlag(c *cli.Context) int { + if c.IsSet("count") { + return c.Int("count") + } + raw := rawProcessFlagValue("fork", "count") + if raw == "" { + return c.Int("count") + } + n, err := strconv.Atoi(raw) + if err != nil { + return c.Int("count") + } + return n +} diff --git a/cmd/sandbox/lifecycle_test.go b/cmd/sandbox/lifecycle_test.go index 2ada22d..990c5b0 100644 --- a/cmd/sandbox/lifecycle_test.go +++ b/cmd/sandbox/lifecycle_test.go @@ -12,6 +12,7 @@ import ( "path/filepath" "strings" "sync" + "sync/atomic" "testing" "time" @@ -364,3 +365,42 @@ func TestOffloadFailsWhenTeardownFails(t *testing.T) { } } } + +// TestForkCountSurvivesTheNaturalArgumentOrder is the regression for a bug +// found live: `fork --count 2` — id first, the order every user +// and cos itself actually writes — silently forked once, not twice, with +// no error at all. Go's stdlib flag package stops parsing at the first +// non-flag argument, so `--count` written after the id is never parsed; +// c.Int("count") quietly returns the flag's default (1). The existing +// bad-count test only ever wrote --count before the id, so it never +// exercised this path. +func TestForkCountSurvivesTheNaturalArgumentOrder(t *testing.T) { + var forkCalls int32 + f := newFakeAPI(t). + json("GET /v1/sandboxes/sb-golden", `{"data":{"id":"sb-golden","status":"paused"}}`). + on("POST /v1/sandboxes/sb-golden/fork", func(w http.ResponseWriter, _ *http.Request) { + n := atomic.AddInt32(&forkCalls, 1) + fmt.Fprintf(w, `{"data":{"id":"sb-clone-%d","status":"running"}}`, n) + }). + json("GET /v1/sandboxes/sb-clone-1", `{"data":{"id":"sb-clone-1","status":"running"}}`). + json("GET /v1/sandboxes/sb-clone-2", `{"data":{"id":"sb-clone-2","status":"running"}}`) + + app := &cli.App{ + Commands: []*cli.Command{newForkCommand()}, + Metadata: map[string]any{api.SandboxClientKey: f.client()}, + } + + // The natural order: sandbox id first, --count after — exactly how cos + // and the fork.md example both write it. rawProcessFlagValue reads the + // real os.Args (that is the whole point — it recovers what urfave + // dropped), so the test has to set it, not just pass args to RunContext. + args := []string{"createos", "fork", "sb-golden", "--count", "2"} + withArgs(t, args) + if err := app.RunContext(shortPoll(t), args); err != nil { + t.Fatalf("fork: %v", err) + } + + if got := atomic.LoadInt32(&forkCalls); got != 2 { + t.Errorf("POST /fork called %d time(s), want 2 — --count 2 was silently dropped to 1", got) + } +} From bba4d7c2d7b9309a83d137a7202f8b421d8a383b Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Tue, 15 Sep 2026 19:40:05 +0530 Subject: [PATCH 4/7] feat(sandbox): add computer and desktop commands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The computer-use API has shipped in all three SDKs but never reached the CLI, so the only way to drive a sandbox desktop from a shell was to talk to the REST API by hand. The Claude Code plugin does exactly that in its cos driver, which is the one place it bypasses this binary — and the other integrations, which only shell out, cannot do it at all. `sandbox desktop` turns on ingress, waits for the desktop stack to come up, and prints a noVNC link. `sandbox computer` drives that desktop: screenshot, screen, cursor, windows, move, click, type, key, open, plus a hidden raw escape hatch for the routes not wrapped here. Three things carried over from cos because each one costs a debugging session to rediscover: - The desktop stack starts after the sandbox reports running, and nothing upstream polls for it, so every caller writes the wait itself. - fc answers 409 desktop_unavailable both while the desktop is booting and when an action fails on a live desktop. ComputerError.Retryable encodes which codes a waiter should keep trying, so the readiness wait neither gives up on a booting desktop nor spins on a permanent failure. - Driving a non-desktop image fails early with the fix, rather than a bare 501 from the first call. Flags are re-scanned by hand because urfave stops parsing them at the first positional argument, so `computer screenshot my-box --out shot.png` otherwise writes to the default path without reporting anything wrong — the same workaround `sandbox edit` makes for --ingress. `--out` has no short alias: -o is taken by the global output-format flag. Verified against a live desktop:1 sandbox: link minted, Chrome driven to a page by chord, typing and Return, pointer landed on the target link, and every error path checked. --- README.md | 2 + cmd/sandbox/computer.go | 456 ++++++++++++++++++++++++++ cmd/sandbox/desktop.go | 202 ++++++++++++ cmd/sandbox/sandbox.go | 2 + internal/api/sandbox_computer.go | 330 +++++++++++++++++++ internal/api/sandbox_computer_test.go | 86 +++++ 6 files changed, 1078 insertions(+) create mode 100644 cmd/sandbox/computer.go create mode 100644 cmd/sandbox/desktop.go create mode 100644 internal/api/sandbox_computer.go create mode 100644 internal/api/sandbox_computer_test.go diff --git a/README.md b/README.md index 8143010..59a47bd 100644 --- a/README.md +++ b/README.md @@ -321,6 +321,8 @@ asks for a few more characters rather than guessing. | `createos sandbox push` | Copy a local file into a sandbox | | `createos sandbox pull` | Copy a file out of a sandbox | | `createos sandbox tunnel` | Forward a local port to a port inside a sandbox | +| `createos sandbox desktop` | Open a graphical sandbox in your browser | +| `createos sandbox computer` | Control the desktop inside a sandbox | | `createos sandbox shapes` | List available sandbox sizes (vCPU / RAM / disk) | | `createos sandbox rootfs` | List built-in OS images you can boot a sandbox from | | `createos sandbox setup` | Connect a coding harness so its workspaces run on a sandbox | diff --git a/cmd/sandbox/computer.go b/cmd/sandbox/computer.go new file mode 100644 index 0000000..6be9432 --- /dev/null +++ b/cmd/sandbox/computer.go @@ -0,0 +1,456 @@ +package sandbox + +import ( + "encoding/json" + "fmt" + "os" + "strconv" + "strings" + + "github.com/pterm/pterm" + "github.com/urfave/cli/v2" + + "github.com/NodeOps-app/createos-cli/internal/api" + "github.com/NodeOps-app/createos-cli/internal/output" +) + +// `createos sandbox computer` drives the desktop inside a sandbox: look at it, +// point at it, type into it. Pair it with `createos sandbox desktop`, which +// brings the desktop up and hands a human the link to watch. +// +// Coordinates are raw X11 pixels of the target screen. Nothing scales them for +// DPI anywhere along this path, so read the bounds from `computer screen` +// rather than assuming a resolution. + +// defaultScreenshotPath is where a capture lands when no -o is given. +const defaultScreenshotPath = "screenshot.png" + +func newComputerCommand() *cli.Command { + return &cli.Command{ + Name: "computer", + Usage: "Control the desktop inside a sandbox", + Description: `Look at and control a sandbox's desktop — take a screenshot, move and +click the pointer, type, press keys, open a page. + +Start the desktop first: + + createos sandbox desktop + +Take a screenshot before and after anything you click. Nothing here +confirms that a click landed on what you meant, so a screenshot is the +only way to see what actually happened.`, + Subcommands: []*cli.Command{ + newComputerScreenshotCommand(), + newComputerInfoCommand("screen", "Show the screen's size in pixels"), + newComputerInfoCommand("cursor", "Show where the pointer is"), + newComputerInfoCommand("windows", "List the windows on screen"), + newComputerMoveCommand(), + newComputerClickCommand(), + newComputerTypeCommand(), + newComputerKeyCommand(), + newComputerOpenCommand(), + newComputerRawCommand(), + }, + } +} + +// screenFlag is repeated per subcommand rather than set on the group: urfave +// does not pass a parent group's flags down to its subcommands. +// +// The flag is declared so it shows up in help and parses in the position +// urfave expects; parseComputerArgs is what actually reads it, because urfave +// stops parsing flags at the first positional argument. +func screenFlag() cli.Flag { + return &cli.StringFlag{ + Name: "screen", + Aliases: []string{"S"}, + Usage: "Which screen to act on", + Value: api.DefaultComputerScreen, + } +} + +// outFlag names the file a screenshot is written to. It deliberately has no +// short alias: `-o` is already the global output-format flag. +func outFlag() cli.Flag { + return &cli.StringFlag{ + Name: "out", + Usage: "Where to save the picture", + Value: defaultScreenshotPath, + } +} + +// computerArgs is one subcommand's arguments after the flags have been pulled +// out of them, wherever the caller happened to put them. +type computerArgs struct { + ref string + screen string + out string + rest []string +} + +// parseComputerArgs re-scans the raw arguments for this package's flags. +// +// urfave/cli v2 stops parsing flags at the first positional argument, so +// `computer screenshot my-box --out shot.png` silently drops --out and writes +// to the default path. Nobody types the flags first, so the arguments are +// scanned by hand — the same workaround `sandbox edit` already makes for +// --ingress. +func parseComputerArgs(c *cli.Context) computerArgs { + parsed := computerArgs{screen: c.String("screen"), out: c.String("out")} + args := c.Args().Slice() + + for i := 0; i < len(args); i++ { + a := args[i] + take := func() string { + if i+1 < len(args) { + i++ + return args[i] + } + return "" + } + switch { + case a == "--screen" || a == "-S": + if v := take(); v != "" { + parsed.screen = v + } + case strings.HasPrefix(a, "--screen="): + parsed.screen = strings.TrimPrefix(a, "--screen=") + case strings.HasPrefix(a, "-S="): + parsed.screen = strings.TrimPrefix(a, "-S=") + case a == "--out": + if v := take(); v != "" { + parsed.out = v + } + case strings.HasPrefix(a, "--out="): + parsed.out = strings.TrimPrefix(a, "--out=") + default: + parsed.rest = append(parsed.rest, a) + } + } + + if len(parsed.rest) > 0 { + parsed.ref = strings.TrimSpace(parsed.rest[0]) + parsed.rest = parsed.rest[1:] + } + if strings.TrimSpace(parsed.screen) == "" { + parsed.screen = api.DefaultComputerScreen + } + if strings.TrimSpace(parsed.out) == "" { + parsed.out = defaultScreenshotPath + } + return parsed +} + +// computerTarget resolves the shared preamble of every op: the client, the +// sandbox the positional ref names, and the arguments left over for the op. +func computerTarget(c *cli.Context) (*api.SandboxClient, string, computerArgs, error) { + args := parseComputerArgs(c) + client, ok := c.App.Metadata[api.SandboxClientKey].(*api.SandboxClient) + if !ok { + return nil, "", args, fmt.Errorf("you're not signed in — run 'createos login' to get started") + } + if args.ref == "" { + return nil, "", args, fmt.Errorf("please provide a sandbox ID or name\n\n To see your sandboxes, run:\n createos sandbox list") + } + id, err := resolveSandboxRef(c.Context, client, args.ref) + if err != nil { + return nil, "", args, err + } + return client, id, args, nil +} + +func newComputerScreenshotCommand() *cli.Command { + return &cli.Command{ + Name: "screenshot", + Usage: "Save a picture of the screen", + ArgsUsage: "", + Flags: []cli.Flag{screenFlag(), outFlag()}, + Action: func(c *cli.Context) error { + client, id, args, err := computerTarget(c) + if err != nil { + return err + } + png, err := client.ComputerScreenshot(c.Context, id, args.screen) + if err != nil { + return err + } + path := args.out + if err := os.WriteFile(path, png, 0o600); err != nil { + return fmt.Errorf("couldn't save the picture to %s: %w", path, err) + } + output.Render(c, map[string]any{"path": path, "bytes": len(png)}, func() { + pterm.Success.Printfln("Saved a picture of the screen to %s (%d bytes)", path, len(png)) + }) + return nil + }, + } +} + +// newComputerInfoCommand builds the read-only ops, which differ only in which +// route they read and what they are called. +func newComputerInfoCommand(name, usage string) *cli.Command { + return &cli.Command{ + Name: name, + Usage: usage, + ArgsUsage: "", + Flags: []cli.Flag{screenFlag()}, + Action: func(c *cli.Context) error { + client, id, args, err := computerTarget(c) + if err != nil { + return err + } + switch name { + case "screen": + geom, err := client.ComputerScreen(c.Context, id, args.screen) + if err != nil { + return err + } + output.Render(c, geom, func() { + pterm.Printfln("%s %d × %d pixels", pterm.NewStyle(pterm.FgCyan).Sprint("Screen:"), geom.Width, geom.Height) + }) + case "cursor": + pos, err := client.ComputerCursor(c.Context, id, args.screen) + if err != nil { + return err + } + output.Render(c, pos, func() { + pterm.Printfln("%s %d, %d", pterm.NewStyle(pterm.FgCyan).Sprint("Pointer:"), pos.X, pos.Y) + }) + case "windows": + raw, err := client.ComputerWindows(c.Context, id, args.screen) + if err != nil { + return err + } + printRaw(c, raw) + } + return nil + }, + } +} + +func newComputerMoveCommand() *cli.Command { + return &cli.Command{ + Name: "move", + Usage: "Move the pointer somewhere", + ArgsUsage: " ", + Flags: []cli.Flag{screenFlag()}, + Action: func(c *cli.Context) error { + client, id, args, err := computerTarget(c) + if err != nil { + return err + } + if len(args.rest) < 2 { + return fmt.Errorf("a move needs two whole numbers — how far across and how far down\n\n For example:\n createos sandbox computer move %s 640 400", args.ref) + } + x, y, err := coords(args.rest[0], args.rest[1], "move") + if err != nil { + return err + } + if err := client.ComputerMouseMove(c.Context, id, args.screen, x, y); err != nil { + return err + } + pterm.Success.Printfln("Moved the pointer to %d, %d", x, y) + return nil + }, + } +} + +func newComputerClickCommand() *cli.Command { + return &cli.Command{ + Name: "click", + Usage: "Click, optionally somewhere specific", + ArgsUsage: " [ ]", + Description: `With no coordinates this clicks wherever the pointer already is. + +Take a screenshot first to see what you are about to click, and +another afterwards to confirm it did what you expected.`, + Flags: []cli.Flag{screenFlag()}, + Action: func(c *cli.Context) error { + client, id, args, err := computerTarget(c) + if err != nil { + return err + } + var at *api.ComputerCursorPos + switch len(args.rest) { + case 0: + case 1: + return fmt.Errorf("a click needs both an across and a down position\n\n For example:\n createos sandbox computer click %s 640 400", args.ref) + default: + x, y, err := coords(args.rest[0], args.rest[1], "click") + if err != nil { + return err + } + at = &api.ComputerCursorPos{X: x, Y: y} + } + if err := client.ComputerMouseClick(c.Context, id, args.screen, at); err != nil { + return err + } + if at != nil { + pterm.Success.Printfln("Clicked at %d, %d", at.X, at.Y) + } else { + pterm.Success.Println("Clicked where the pointer was") + } + return nil + }, + } +} + +func newComputerTypeCommand() *cli.Command { + return &cli.Command{ + Name: "type", + Usage: "Type text into whatever has focus", + ArgsUsage: " ", + Description: `Quote the text to keep it as one piece: + + createos sandbox computer type my-box "hello world" + +Unquoted words are joined with single spaces.`, + Flags: []cli.Flag{screenFlag()}, + Action: func(c *cli.Context) error { + client, id, args, err := computerTarget(c) + if err != nil { + return err + } + // Unquoted multi-word text arrives as separate arguments; join it + // back up so `type my-box hello world` types the space too. + text := strings.Join(args.rest, " ") + if text == "" { + return fmt.Errorf("please provide the text to type\n\n For example:\n createos sandbox computer type %s \"hello world\"", args.ref) + } + if err := client.ComputerType(c.Context, id, args.screen, text); err != nil { + return err + } + pterm.Success.Printfln("Typed %d characters", len([]rune(text))) + return nil + }, + } +} + +func newComputerKeyCommand() *cli.Command { + return &cli.Command{ + Name: "key", + Usage: "Press keys together", + ArgsUsage: " ...", + Description: `Every key is pressed at the same time, so this is how you send a +shortcut: + + createos sandbox computer key my-box ctrl l`, + Flags: []cli.Flag{screenFlag()}, + Action: func(c *cli.Context) error { + client, id, args, err := computerTarget(c) + if err != nil { + return err + } + keys := args.rest + if len(keys) == 0 { + return fmt.Errorf("please provide at least one key to press\n\n For example:\n createos sandbox computer key %s ctrl l", args.ref) + } + if err := client.ComputerPress(c.Context, id, args.screen, keys); err != nil { + return err + } + pterm.Success.Printfln("Pressed %s", strings.Join(keys, "+")) + return nil + }, + } +} + +func newComputerOpenCommand() *cli.Command { + return &cli.Command{ + Name: "open", + Usage: "Open a web page or file on the desktop", + ArgsUsage: " ", + Flags: []cli.Flag{screenFlag()}, + Action: func(c *cli.Context) error { + client, id, args, err := computerTarget(c) + if err != nil { + return err + } + target := "" + if len(args.rest) > 0 { + target = strings.TrimSpace(args.rest[0]) + } + if target == "" { + return fmt.Errorf("please provide something to open\n\n For example:\n createos sandbox computer open %s https://example.com", args.ref) + } + if err := client.ComputerOpen(c.Context, id, args.screen, target); err != nil { + return err + } + pterm.Success.Printfln("Opened %s", target) + return nil + }, + } +} + +func newComputerRawCommand() *cli.Command { + return &cli.Command{ + Name: "raw", + Usage: "Call a desktop endpoint this CLI doesn't wrap", + ArgsUsage: " [json-body]", + Description: `An escape hatch for the parts of the desktop API without their own +command. The path is relative to the sandbox's computer routes: + + createos sandbox computer raw my-box GET screen`, + Hidden: true, + Flags: []cli.Flag{screenFlag()}, + Action: func(c *cli.Context) error { + client, id, args, err := computerTarget(c) + if err != nil { + return err + } + method, path := "", "" + if len(args.rest) > 0 { + method = strings.TrimSpace(args.rest[0]) + } + if len(args.rest) > 1 { + path = strings.TrimSpace(args.rest[1]) + } + if method == "" || path == "" { + return fmt.Errorf("please provide a method and a path\n\n For example:\n createos sandbox computer raw %s GET screen", args.ref) + } + var body json.RawMessage + if raw := func() string { + if len(args.rest) > 2 { + return strings.TrimSpace(args.rest[2]) + } + return "" + }(); raw != "" { + if !json.Valid([]byte(raw)) { + return fmt.Errorf("the body isn't valid JSON") + } + body = json.RawMessage(raw) + } + out, err := client.ComputerRaw(c.Context, id, args.screen, method, path, body) + if err != nil { + return err + } + printRaw(c, out) + return nil + }, + } +} + +// coords parses an x/y pair, naming the op in the error so the fix is obvious. +func coords(xs, ys, op string) (int, int, error) { + x, errX := strconv.Atoi(strings.TrimSpace(xs)) + y, errY := strconv.Atoi(strings.TrimSpace(ys)) + if errX != nil || errY != nil { + return 0, 0, fmt.Errorf("a %s needs two whole numbers — how far across and how far down\n\n To see the screen's size, run:\n createos sandbox computer screen ", op) + } + return x, y, nil +} + +// printRaw emits a passthrough payload: as-is under -o json, pretty otherwise. +func printRaw(c *cli.Context, raw json.RawMessage) { + if output.IsJSON(c) { + fmt.Println(string(raw)) + return + } + var pretty any + if err := json.Unmarshal(raw, &pretty); err == nil { + if formatted, err := json.MarshalIndent(pretty, "", " "); err == nil { + fmt.Println(string(formatted)) + return + } + } + fmt.Println(string(raw)) +} diff --git a/cmd/sandbox/desktop.go b/cmd/sandbox/desktop.go new file mode 100644 index 0000000..0d76f3f --- /dev/null +++ b/cmd/sandbox/desktop.go @@ -0,0 +1,202 @@ +package sandbox + +import ( + "context" + "errors" + "fmt" + "strings" + "time" + + "github.com/pterm/pterm" + "github.com/urfave/cli/v2" + + "github.com/NodeOps-app/createos-cli/internal/api" + "github.com/NodeOps-app/createos-cli/internal/output" + "github.com/NodeOps-app/createos-cli/internal/terminal" +) + +// The desktop stack (Xvfb → XFCE → x11vnc → websockify) starts *after* the +// sandbox reports `running`, so every computer call fails for the first while. +// Nothing upstream polls for this, which means every caller ends up writing +// this wait — so the CLI does it once, here. +const ( + desktopReadyTimeout = 2 * time.Minute + desktopPollInterval = 2 * time.Second +) + +func newDesktopCommand() *cli.Command { + return &cli.Command{ + Name: "desktop", + Usage: "Open a graphical sandbox in your browser", + ArgsUsage: "[]", + Description: `Turns on the public URL for a sandbox running a desktop image, waits +for its desktop to finish starting, and prints a link you can open in +a browser to watch and control it. + +The sandbox must already be running a desktop image. To create one: + + createos sandbox create --rootfs desktop:1 + +Anyone with the link can control the desktop, so treat it like a +password. It expires, and running this again issues a fresh link.`, + Flags: []cli.Flag{ + &cli.StringFlag{ + Name: "screen", + Aliases: []string{"S"}, + Usage: "Which screen to open", + Value: api.DefaultComputerScreen, + }, + &cli.DurationFlag{ + Name: "wait", + Usage: "How long to wait for the desktop to start", + Value: desktopReadyTimeout, + }, + }, + Action: runDesktop, + } +} + +func runDesktop(c *cli.Context) error { + client, ok := c.App.Metadata[api.SandboxClientKey].(*api.SandboxClient) + if !ok { + return fmt.Errorf("you're not signed in — run 'createos login' to get started") + } + + ref := strings.TrimSpace(c.Args().First()) + var id string + switch { + case ref != "": + resolved, err := resolveSandboxRef(c.Context, client, ref) + if err != nil { + return err + } + id = resolved + case terminal.IsInteractive(): + picked, label, err := pickByStatus(c, client, "Pick a sandbox to open", api.SandboxStatusRunning) + if err != nil { + return err + } + if picked == "" { + fmt.Println("Cancelled. Nothing changed.") + return nil + } + id, ref = picked, label + default: + return fmt.Errorf("please provide a sandbox ID or name\n\n To see your sandboxes, run:\n createos sandbox list") + } + + sb, err := client.GetSandbox(c.Context, id) + if err != nil { + return err + } + if err := ensureDesktopReady(c, client, sb); err != nil { + return err + } + + screen := c.String("screen") + if !sb.IngressEnabled { + if _, err := client.SetSandboxIngress(c.Context, id, true); err != nil { + return fmt.Errorf("couldn't turn on the public URL for %s: %w", refLabel(ref, id), err) + } + } + + if err := waitForDesktop(c.Context, client, id, screen, c.Duration("wait")); err != nil { + return err + } + + conn, err := client.ComputerConnect(c.Context, id, screen) + if err != nil { + return err + } + if conn.URL == "" { + return fmt.Errorf("no link came back for %s\n\n The public URL has to be on before a link can be issued. Turn it on with:\n createos sandbox edit %s --ingress on", refLabel(ref, id), id) + } + + output.Render(c, conn, func() { + pterm.Success.Printfln("Desktop ready on %s (%s)", refLabel(ref, id), screen) + fmt.Printf(" %s\n", conn.URL) + pterm.Println(pterm.Gray(" Anyone with this link can control the desktop.")) + if conn.ExpiresAt != "" { + pterm.Println(pterm.Gray(fmt.Sprintf(" It expires at %s. Running this again issues a new one.", conn.ExpiresAt))) + } + }) + return nil +} + +// ensureDesktopReady refuses early on a sandbox that cannot serve a desktop. +// Without this the first computer call fails with a far less obvious message +// than saying so up front. +func ensureDesktopReady(c *cli.Context, client *api.SandboxClient, sb *api.SandboxView) error { + if sb.Status == api.SandboxStatusPaused { + if _, err := client.ResumeSandbox(c.Context, sb.ID); err != nil { + return err + } + spinner, _ := pterm.DefaultSpinner.Start("Waking the sandbox up…") //nolint:errcheck + resumed, err := waitForStatus(c.Context, client, sb.ID, api.SandboxStatusRunning) + if err != nil { + spinner.Fail("It didn't wake up") + return err + } + spinner.Success("Sandbox is awake") + sb = resumed + } + if sb.Status != api.SandboxStatusRunning { + return fmt.Errorf("that sandbox is %s, so it has no desktop to show yet\n\n To check on it, run:\n createos sandbox get %s", sb.Status, sb.ID) + } + rootfs := "" + if sb.Rootfs != nil { + rootfs = *sb.Rootfs + } + if !strings.Contains(strings.ToLower(rootfs), "desktop") { + named := rootfs + if named == "" { + named = "an image without a desktop" + } + return fmt.Errorf("that sandbox runs %s, which has no desktop\n\n Create one that does with:\n createos sandbox create --rootfs desktop:1", named) + } + return nil +} + +// waitForDesktop polls the screen route until the desktop answers. It stops +// early on an error that more waiting cannot fix — a missing desktop image +// answers 501 forever, and spinning on that just delays the real message. +func waitForDesktop(ctx context.Context, client *api.SandboxClient, id, screen string, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + var spinner *pterm.SpinnerPrinter + + for { + _, err := client.ComputerScreen(ctx, id, screen) + if err == nil { + if spinner != nil { + spinner.Success("Desktop is up") + } + return nil + } + + var computerErr *api.ComputerError + if errors.As(err, &computerErr) && !computerErr.Retryable() { + if spinner != nil { + spinner.Fail("The desktop didn't start") + } + return err + } + if time.Now().After(deadline) { + if spinner != nil { + spinner.Fail("The desktop didn't start in time") + } + return fmt.Errorf("the desktop on %s didn't start within %s\n\n To see whether it's still coming up, run:\n createos sandbox exec %s 'pgrep -a Xvfb; pgrep -a websockify'", id, timeout, id) + } + if spinner == nil { + spinner, _ = pterm.DefaultSpinner.Start("Waiting for the desktop to start…") //nolint:errcheck + } + + select { + case <-ctx.Done(): + if spinner != nil { + spinner.Fail("Cancelled") + } + return ctx.Err() + case <-time.After(desktopPollInterval): + } + } +} diff --git a/cmd/sandbox/sandbox.go b/cmd/sandbox/sandbox.go index 5353359..8fa0abb 100644 --- a/cmd/sandbox/sandbox.go +++ b/cmd/sandbox/sandbox.go @@ -28,6 +28,8 @@ func NewSandboxCommand() *cli.Command { newPushCommand(), newPullCommand(), newShellCommand(), + newDesktopCommand(), + newComputerCommand(), newEditorCommand(), newSyncCommand(), newTunnelCommand(), diff --git a/internal/api/sandbox_computer.go b/internal/api/sandbox_computer.go new file mode 100644 index 0000000..84ee5a8 --- /dev/null +++ b/internal/api/sandbox_computer.go @@ -0,0 +1,330 @@ +package api + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "strings" +) + +// Computer-use routes: GET/POST /v1/sandboxes/:id/computer/*. +// +// These drive the X session inside a sandbox booted on a desktop rootfs — +// screenshot, pointer, keyboard, window list — plus the noVNC connect URL a +// human opens in a browser. Every call is scoped to one screen, selected by +// the screen_id query parameter. +// +// Screenshot is the only route that answers with bytes (image/png) instead of +// the JSend envelope, so it bypasses SetResult and reads resp.Body() directly. + +// DefaultComputerScreen is the screen every computer call targets unless the +// caller names another one. +const DefaultComputerScreen = "screen-0" + +// ComputerScreenGeometry is GET /computer/screen — the coordinate space every +// other call's x/y is measured in. Raw X11 pixels: no DPI scaling is applied +// anywhere along this path, so callers must read the bounds rather than assume +// a resolution. +type ComputerScreenGeometry struct { + Width int `json:"width"` + Height int `json:"height"` +} + +// ComputerCursorPos is GET /computer/cursor. +type ComputerCursorPos struct { + X int `json:"x"` + Y int `json:"y"` +} + +// ComputerConnection is GET /computer/screens/:screen/connect — a live noVNC +// URL with a bearer token embedded in it, plus that token's expiry. +// +// The URL *is* the credential: anyone holding it can drive the desktop until +// it expires. fc only mints one when ingress is enabled on the sandbox. +type ComputerConnection struct { + URL string `json:"url"` + ExpiresAt string `json:"expires_at"` +} + +// ComputerScreen returns the screen's pixel geometry. It doubles as the +// readiness probe: it is the cheapest route that only answers 2xx once the +// desktop stack (Xvfb → XFCE → x11vnc → websockify) is actually up. +func (c *SandboxClient) ComputerScreen(ctx context.Context, id, screen string) (*ComputerScreenGeometry, error) { + var envelope Response[ComputerScreenGeometry] + resp, err := c.Client.R(). + SetContext(ctx). + SetPathParam("id", id). + SetQueryParam("screen_id", computerScreen(screen)). + SetResult(&envelope). + Get("/v1/sandboxes/{id}/computer/screen") + if err != nil { + return nil, err + } + if resp.IsError() { + return nil, ParseComputerError(resp.StatusCode(), resp.Body()) + } + return &envelope.Data, nil +} + +// ComputerCursor returns the pointer's current position. +func (c *SandboxClient) ComputerCursor(ctx context.Context, id, screen string) (*ComputerCursorPos, error) { + var envelope Response[ComputerCursorPos] + resp, err := c.Client.R(). + SetContext(ctx). + SetPathParam("id", id). + SetQueryParam("screen_id", computerScreen(screen)). + SetResult(&envelope). + Get("/v1/sandboxes/{id}/computer/cursor") + if err != nil { + return nil, err + } + if resp.IsError() { + return nil, ParseComputerError(resp.StatusCode(), resp.Body()) + } + return &envelope.Data, nil +} + +// ComputerWindows lists the windows on the screen. The window shape is fc's to +// define and has no stable schema here yet, so the payload is passed through +// unparsed rather than pinned to a struct this repo would have to guess at. +func (c *SandboxClient) ComputerWindows(ctx context.Context, id, screen string) (json.RawMessage, error) { + return c.computerGetRaw(ctx, id, screen, "/v1/sandboxes/{id}/computer/windows") +} + +// ComputerScreenshot captures the screen and returns the PNG bytes verbatim. +// +// This route answers image/png, not the JSend envelope, so an error body has +// to be read off the same response — hence the manual IsError branch before +// the bytes are handed back. +func (c *SandboxClient) ComputerScreenshot(ctx context.Context, id, screen string) ([]byte, error) { + resp, err := c.Client.R(). + SetContext(ctx). + SetPathParam("id", id). + SetQueryParam("screen_id", computerScreen(screen)). + SetHeader("Accept", "image/png"). + Get("/v1/sandboxes/{id}/computer/screenshot") + if err != nil { + return nil, err + } + if resp.IsError() { + return nil, ParseComputerError(resp.StatusCode(), resp.Body()) + } + return resp.Body(), nil +} + +// ComputerMouseMove moves the pointer to an absolute position on the screen. +func (c *SandboxClient) ComputerMouseMove(ctx context.Context, id, screen string, x, y int) error { + return c.computerPost(ctx, id, screen, "/v1/sandboxes/{id}/computer/mouse/move", + map[string]int{"x": x, "y": y}) +} + +// ComputerMouseClick clicks. With at == nil it clicks wherever the pointer +// already is; otherwise it moves there first. +func (c *SandboxClient) ComputerMouseClick(ctx context.Context, id, screen string, at *ComputerCursorPos) error { + body := map[string]int{} + if at != nil { + body["x"] = at.X + body["y"] = at.Y + } + return c.computerPost(ctx, id, screen, "/v1/sandboxes/{id}/computer/mouse/click", body) +} + +// ComputerType types a string into the focused window. +func (c *SandboxClient) ComputerType(ctx context.Context, id, screen, text string) error { + return c.computerPost(ctx, id, screen, "/v1/sandboxes/{id}/computer/keyboard/type", + map[string]string{"text": text}) +} + +// ComputerPress presses one key chord — each element is a key name, and they +// are pressed together (e.g. ["ctrl","l"]). +func (c *SandboxClient) ComputerPress(ctx context.Context, id, screen string, keys []string) error { + return c.computerPost(ctx, id, screen, "/v1/sandboxes/{id}/computer/keyboard/press", + map[string][]string{"keys": keys}) +} + +// ComputerOpen opens a URL or a local path in the desktop's browser. +func (c *SandboxClient) ComputerOpen(ctx context.Context, id, screen, target string) error { + return c.computerPost(ctx, id, screen, "/v1/sandboxes/{id}/computer/open", + map[string]string{"target": target}) +} + +// ComputerConnect mints a noVNC URL for the screen. fc only returns one when +// ingress is enabled on the sandbox; a fresh call invalidates the previous +// link for new connections. +func (c *SandboxClient) ComputerConnect(ctx context.Context, id, screen string) (*ComputerConnection, error) { + var envelope Response[ComputerConnection] + resp, err := c.Client.R(). + SetContext(ctx). + SetPathParam("id", id). + SetPathParam("screen", computerScreen(screen)). + SetResult(&envelope). + Get("/v1/sandboxes/{id}/computer/screens/{screen}/connect") + if err != nil { + return nil, err + } + if resp.IsError() { + return nil, ParseComputerError(resp.StatusCode(), resp.Body()) + } + return &envelope.Data, nil +} + +// ComputerRaw is the escape hatch for the computer routes this client does not +// wrap. path is relative to /v1/sandboxes/:id/computer (a leading slash makes +// it absolute instead). body may be nil. +func (c *SandboxClient) ComputerRaw(ctx context.Context, id, screen, method, path string, body json.RawMessage) (json.RawMessage, error) { + full := path + if !strings.HasPrefix(path, "/") { + full = fmt.Sprintf("/v1/sandboxes/%s/computer/%s", id, path) + } + req := c.Client.R(). + SetContext(ctx). + SetQueryParam("screen_id", computerScreen(screen)) + if len(body) > 0 { + req = req.SetBody(body) + } + resp, err := req.Execute(strings.ToUpper(method), full) + if err != nil { + return nil, err + } + if resp.IsError() { + return nil, ParseComputerError(resp.StatusCode(), resp.Body()) + } + return unwrapEnvelope(resp.Body()), nil +} + +// computerPost is the shared shape of every action route: POST a small JSON +// body, care only about success or failure. +func (c *SandboxClient) computerPost(ctx context.Context, id, screen, path string, body any) error { + resp, err := c.Client.R(). + SetContext(ctx). + SetPathParam("id", id). + SetQueryParam("screen_id", computerScreen(screen)). + SetBody(body). + Post(path) + if err != nil { + return err + } + if resp.IsError() { + return ParseComputerError(resp.StatusCode(), resp.Body()) + } + return nil +} + +// computerGetRaw fetches a route whose payload this client deliberately does +// not model, and returns it unwrapped from the JSend envelope. +func (c *SandboxClient) computerGetRaw(ctx context.Context, id, screen, path string) (json.RawMessage, error) { + resp, err := c.Client.R(). + SetContext(ctx). + SetPathParam("id", id). + SetQueryParam("screen_id", computerScreen(screen)). + Get(path) + if err != nil { + return nil, err + } + if resp.IsError() { + return nil, ParseComputerError(resp.StatusCode(), resp.Body()) + } + return unwrapEnvelope(resp.Body()), nil +} + +// computerScreen applies the default so callers can pass "". +func computerScreen(screen string) string { + if strings.TrimSpace(screen) == "" { + return DefaultComputerScreen + } + return screen +} + +// unwrapEnvelope pulls `data` out of a JSend body, falling back to the whole +// body when the response is not enveloped. +func unwrapEnvelope(body []byte) json.RawMessage { + var envelope struct { + Data json.RawMessage `json:"data"` + } + if err := json.Unmarshal(body, &envelope); err == nil && len(envelope.Data) > 0 { + return envelope.Data + } + return body +} + +// ComputerError is a failed computer-use call. It keeps the status code so +// callers that poll (readiness waits) can tell a "not up yet" apart from a +// "this will never work", instead of retrying until the timeout either way. +type ComputerError struct { + StatusCode int + Message string + advice string +} + +func (e *ComputerError) Error() string { + if e.advice == "" { + return e.Message + } + return e.Message + "\n\n" + e.advice +} + +// Retryable reports whether polling the same route again could plausibly +// succeed. A 409 is the interesting case: fc uses it both for "the desktop is +// still coming up" and "the action failed on a live desktop", so a waiter has +// to keep trying while an action caller should surface it. +func (e *ComputerError) Retryable() bool { + switch e.StatusCode { + case http.StatusNotFound, http.StatusConflict, http.StatusTooManyRequests: + return true + default: + return false + } +} + +// ParseComputerError maps the computer API's status codes onto something a +// caller can act on. +// +// This is worth doing by hand rather than leaning on ParseAPIError, because +// fc returns `desktop_unavailable` (409) for *every* X-side failure — the raw +// message alone never distinguishes "the desktop is still booting" from "the +// action failed on a perfectly healthy desktop", and those want opposite +// responses from whoever is reading the error. +func ParseComputerError(statusCode int, body []byte) error { + msg := "" + if base := ParseAPIError(statusCode, body); base != nil { + msg = base.Message + } + e := &ComputerError{StatusCode: statusCode, Message: msg} + + switch statusCode { + case http.StatusBadRequest: + e.Message = "the desktop couldn't use that" + suffix(msg) + e.advice = " If you named a screen, check it exists. Most sandboxes have only\n screen-0, which is the default.\n To see the screen you have, run:\n createos sandbox computer screen " + case http.StatusNotFound: + e.Message = "that sandbox or screen doesn't exist" + suffix(msg) + e.advice = " Computer-use needs a sandbox booted on a desktop image.\n To start one, run:\n createos sandbox create --rootfs desktop:1" + case http.StatusConflict: + if strings.Contains(strings.ToLower(msg), "ingress") { + e.Message = "the public URL is off for this sandbox" + suffix(msg) + e.advice = " To turn it on, run:\n createos sandbox desktop " + break + } + e.Message = "the desktop didn't answer" + suffix(msg) + e.advice = " Either the desktop is still starting up, or the action failed on a\n running desktop — the API reports both the same way.\n If the sandbox just started, wait for it with:\n createos sandbox desktop " + case http.StatusTooManyRequests: + e.Message = "too many requests in a row" + suffix(msg) + e.advice = " Screenshots are rate limited. Wait a second and try again." + case http.StatusNotImplemented: + e.Message = "this sandbox has no desktop installed" + suffix(msg) + e.advice = " Its image doesn't include the desktop tools. Create a new one with:\n createos sandbox create --rootfs desktop:1" + default: + if e.Message == "" { + e.Message = fmt.Sprintf("the desktop request failed (HTTP %d)", statusCode) + } + } + return e +} + +// suffix formats an optional server message as a trailing clause. +func suffix(msg string) string { + if msg == "" { + return "" + } + return ": " + msg +} diff --git a/internal/api/sandbox_computer_test.go b/internal/api/sandbox_computer_test.go new file mode 100644 index 0000000..e659ee6 --- /dev/null +++ b/internal/api/sandbox_computer_test.go @@ -0,0 +1,86 @@ +package api + +import ( + "encoding/json" + "net/http" + "strings" + "testing" +) + +// The readiness wait in `sandbox desktop` keeps polling while an error is +// retryable and gives up immediately when it is not. Misclassify one and the +// command either spins for the whole timeout on a failure that will never +// clear, or abandons a desktop that was still starting. +func TestComputerErrorRetryable(t *testing.T) { + cases := []struct { + status int + want bool + why string + }{ + {http.StatusNotFound, true, "the screen appears only once the desktop is up"}, + {http.StatusConflict, true, "fc uses 409 for 'still booting' as well as 'action failed'"}, + {http.StatusTooManyRequests, true, "rate limiting clears on its own"}, + {http.StatusNotImplemented, false, "an image without desktop tools never grows them"}, + {http.StatusUnauthorized, false, "bad credentials do not fix themselves"}, + {http.StatusForbidden, false, "bad credentials do not fix themselves"}, + } + for _, tc := range cases { + err, ok := ParseComputerError(tc.status, nil).(*ComputerError) + if !ok { + t.Fatalf("status %d: expected a *ComputerError", tc.status) + } + if got := err.Retryable(); got != tc.want { + t.Errorf("status %d: Retryable() = %v, want %v — %s", tc.status, got, tc.want, tc.why) + } + } +} + +// A 409 mentioning ingress is a different fix from a 409 about the desktop, +// so the two must not collapse into one message. +func TestComputerErrorConflictDistinguishesIngress(t *testing.T) { + body := []byte(`{"status":"fail","data":"ingress is not enabled"}`) + ingress := ParseComputerError(http.StatusConflict, body).Error() + if !strings.Contains(ingress, "public URL is off") { + t.Errorf("ingress conflict should name the public URL, got: %q", ingress) + } + + desktop := ParseComputerError(http.StatusConflict, []byte(`{"status":"fail","data":"desktop_unavailable"}`)).Error() + if strings.Contains(desktop, "public URL is off") { + t.Errorf("a desktop_unavailable conflict should not be reported as an ingress problem, got: %q", desktop) + } + if !strings.Contains(desktop, "still starting up") { + t.Errorf("a desktop conflict should say it may still be starting, got: %q", desktop) + } +} + +func TestComputerScreenDefaults(t *testing.T) { + if got := computerScreen(""); got != DefaultComputerScreen { + t.Errorf("computerScreen(%q) = %q, want %q", "", got, DefaultComputerScreen) + } + if got := computerScreen(" "); got != DefaultComputerScreen { + t.Errorf("computerScreen(whitespace) = %q, want %q", got, DefaultComputerScreen) + } + if got := computerScreen("screen-2"); got != "screen-2" { + t.Errorf("computerScreen(%q) = %q, want it unchanged", "screen-2", got) + } +} + +func TestUnwrapEnvelope(t *testing.T) { + wrapped := unwrapEnvelope([]byte(`{"status":"success","data":{"width":1280}}`)) + var got struct { + Width int `json:"width"` + } + if err := json.Unmarshal(wrapped, &got); err != nil { + t.Fatalf("unwrapping an enveloped body: %v", err) + } + if got.Width != 1280 { + t.Errorf("width = %d, want 1280", got.Width) + } + + // Not every computer route envelopes its payload, so a bare body has to + // survive untouched rather than come back empty. + bare := []byte(`[{"id":1}]`) + if string(unwrapEnvelope(bare)) != string(bare) { + t.Errorf("a bare body should pass through unchanged, got %q", unwrapEnvelope(bare)) + } +} From ead01e7a6743bb69295902f86900087e849abc33 Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Wed, 16 Sep 2026 02:45:42 +0530 Subject: [PATCH 5/7] fix(sandbox): read desktop flags after the positional argument `sandbox desktop my-box --screen screen-1` dropped --screen, and --wait with it, because urfave stops parsing flags at the first positional. The computer subcommands already re-scanned their arguments by hand; desktop read its flags straight off the context and so kept the bug. Route desktop through the same parser and teach it --wait. A duration it cannot parse now keeps the declared default instead of zeroing, which would have turned the readiness wait into a single attempt. Tests cover both flag positions, the equals form, the short alias, and operands surviving around a flag. --- cmd/sandbox/computer.go | 14 +++- cmd/sandbox/computer_args_test.go | 123 ++++++++++++++++++++++++++++++ cmd/sandbox/desktop.go | 7 +- 3 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 cmd/sandbox/computer_args_test.go diff --git a/cmd/sandbox/computer.go b/cmd/sandbox/computer.go index 6be9432..e8271b9 100644 --- a/cmd/sandbox/computer.go +++ b/cmd/sandbox/computer.go @@ -6,6 +6,7 @@ import ( "os" "strconv" "strings" + "time" "github.com/pterm/pterm" "github.com/urfave/cli/v2" @@ -85,6 +86,7 @@ type computerArgs struct { ref string screen string out string + wait time.Duration rest []string } @@ -96,7 +98,7 @@ type computerArgs struct { // scanned by hand — the same workaround `sandbox edit` already makes for // --ingress. func parseComputerArgs(c *cli.Context) computerArgs { - parsed := computerArgs{screen: c.String("screen"), out: c.String("out")} + parsed := computerArgs{screen: c.String("screen"), out: c.String("out"), wait: c.Duration("wait")} args := c.Args().Slice() for i := 0; i < len(args); i++ { @@ -123,6 +125,16 @@ func parseComputerArgs(c *cli.Context) computerArgs { } case strings.HasPrefix(a, "--out="): parsed.out = strings.TrimPrefix(a, "--out=") + case a == "--wait": + if v := take(); v != "" { + if d, err := time.ParseDuration(v); err == nil { + parsed.wait = d + } + } + case strings.HasPrefix(a, "--wait="): + if d, err := time.ParseDuration(strings.TrimPrefix(a, "--wait=")); err == nil { + parsed.wait = d + } default: parsed.rest = append(parsed.rest, a) } diff --git a/cmd/sandbox/computer_args_test.go b/cmd/sandbox/computer_args_test.go new file mode 100644 index 0000000..b8c65b5 --- /dev/null +++ b/cmd/sandbox/computer_args_test.go @@ -0,0 +1,123 @@ +package sandbox + +import ( + "flag" + "testing" + "time" + + "github.com/urfave/cli/v2" +) + +// newComputerTestContext builds a context the way urfave hands one to an +// Action: flags declared, but parsing stopped at the first positional. Every +// argument after the sandbox reference therefore arrives unparsed. +func newComputerTestContext(args ...string) *cli.Context { + set := flag.NewFlagSet("test", flag.ContinueOnError) + set.String("screen", "screen-0", "") + set.String("out", defaultScreenshotPath, "") + set.Duration("wait", 2*time.Minute, "") + _ = set.Parse(args) + return cli.NewContext(cli.NewApp(), set, nil) +} + +// Nobody types flags before the positional argument, and urfave stops parsing +// at the first one. Without the hand re-scan, `screenshot my-box --out shot.png` +// writes to the default path and reports success, which is worse than an error. +func TestParseComputerArgsReadsFlagsAfterPositional(t *testing.T) { + cases := []struct { + name string + args []string + wantRef string + wantScreen string + wantOut string + wantRest []string + }{ + { + name: "out after the reference", + args: []string{"my-box", "--out", "shot.png"}, + wantRef: "my-box", + wantScreen: "screen-0", + wantOut: "shot.png", + }, + { + name: "equals form", + args: []string{"my-box", "--out=shot.png", "--screen=screen-2"}, + wantRef: "my-box", + wantScreen: "screen-2", + wantOut: "shot.png", + }, + { + name: "short screen alias after the reference", + args: []string{"my-box", "-S", "screen-1"}, + wantRef: "my-box", + wantScreen: "screen-1", + wantOut: defaultScreenshotPath, + }, + { + name: "flags before the reference still work", + args: []string{"--screen", "screen-3", "my-box"}, + wantRef: "my-box", + wantScreen: "screen-3", + wantOut: defaultScreenshotPath, + }, + { + name: "operands survive around a flag", + args: []string{"my-box", "640", "--screen", "screen-1", "400"}, + wantRef: "my-box", + wantScreen: "screen-1", + wantOut: defaultScreenshotPath, + wantRest: []string{"640", "400"}, + }, + { + name: "no arguments at all", + args: nil, + wantRef: "", + wantScreen: "screen-0", + wantOut: defaultScreenshotPath, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := parseComputerArgs(newComputerTestContext(tc.args...)) + if got.ref != tc.wantRef { + t.Errorf("ref = %q, want %q", got.ref, tc.wantRef) + } + if got.screen != tc.wantScreen { + t.Errorf("screen = %q, want %q", got.screen, tc.wantScreen) + } + if got.out != tc.wantOut { + t.Errorf("out = %q, want %q", got.out, tc.wantOut) + } + if len(got.rest) != len(tc.wantRest) { + t.Fatalf("rest = %v, want %v", got.rest, tc.wantRest) + } + for i := range got.rest { + if got.rest[i] != tc.wantRest[i] { + t.Errorf("rest[%d] = %q, want %q", i, got.rest[i], tc.wantRest[i]) + } + } + }) + } +} + +// `sandbox desktop` reads --wait through the same parser, so a timeout written +// after the sandbox reference has to survive. +func TestParseComputerArgsReadsWait(t *testing.T) { + got := parseComputerArgs(newComputerTestContext("my-box", "--wait", "30s")) + if got.wait != 30*time.Second { + t.Errorf("wait = %s, want 30s", got.wait) + } + + got = parseComputerArgs(newComputerTestContext("my-box", "--wait=90s")) + if got.wait != 90*time.Second { + t.Errorf("wait = %s, want 90s", got.wait) + } + + // An unparseable duration keeps the declared default rather than zeroing + // the timeout, which would turn the readiness wait into a single attempt. + got = parseComputerArgs(newComputerTestContext("my-box", "--wait", "soon")) + if got.wait != 2*time.Minute { + t.Errorf("wait = %s, want the 2m default", got.wait) + } +} diff --git a/cmd/sandbox/desktop.go b/cmd/sandbox/desktop.go index 0d76f3f..4d328dd 100644 --- a/cmd/sandbox/desktop.go +++ b/cmd/sandbox/desktop.go @@ -62,7 +62,8 @@ func runDesktop(c *cli.Context) error { return fmt.Errorf("you're not signed in — run 'createos login' to get started") } - ref := strings.TrimSpace(c.Args().First()) + args := parseComputerArgs(c) + ref := args.ref var id string switch { case ref != "": @@ -93,14 +94,14 @@ func runDesktop(c *cli.Context) error { return err } - screen := c.String("screen") + screen := args.screen if !sb.IngressEnabled { if _, err := client.SetSandboxIngress(c.Context, id, true); err != nil { return fmt.Errorf("couldn't turn on the public URL for %s: %w", refLabel(ref, id), err) } } - if err := waitForDesktop(c.Context, client, id, screen, c.Duration("wait")); err != nil { + if err := waitForDesktop(c.Context, client, id, screen, args.wait); err != nil { return err } From 16c2c1a2290e45fbce1806c8821ab757cab929b6 Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Wed, 16 Sep 2026 13:44:24 +0530 Subject: [PATCH 6/7] fix(sandbox): satisfy errorlint and the shadow check CI runs golangci-lint, which this was not developed against. Two classes: a type assertion on an error in the test, which breaks once anything wraps it, and three `err` shadows in runDesktop. Assert with errors.As, and assign to the existing err instead of redeclaring it. --- cmd/sandbox/desktop.go | 6 +++--- internal/api/sandbox_computer_test.go | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cmd/sandbox/desktop.go b/cmd/sandbox/desktop.go index 4d328dd..fabccd2 100644 --- a/cmd/sandbox/desktop.go +++ b/cmd/sandbox/desktop.go @@ -90,18 +90,18 @@ func runDesktop(c *cli.Context) error { if err != nil { return err } - if err := ensureDesktopReady(c, client, sb); err != nil { + if err = ensureDesktopReady(c, client, sb); err != nil { return err } screen := args.screen if !sb.IngressEnabled { - if _, err := client.SetSandboxIngress(c.Context, id, true); err != nil { + if _, err = client.SetSandboxIngress(c.Context, id, true); err != nil { return fmt.Errorf("couldn't turn on the public URL for %s: %w", refLabel(ref, id), err) } } - if err := waitForDesktop(c.Context, client, id, screen, args.wait); err != nil { + if err = waitForDesktop(c.Context, client, id, screen, args.wait); err != nil { return err } diff --git a/internal/api/sandbox_computer_test.go b/internal/api/sandbox_computer_test.go index e659ee6..68798bf 100644 --- a/internal/api/sandbox_computer_test.go +++ b/internal/api/sandbox_computer_test.go @@ -2,6 +2,7 @@ package api import ( "encoding/json" + "errors" "net/http" "strings" "testing" @@ -25,8 +26,8 @@ func TestComputerErrorRetryable(t *testing.T) { {http.StatusForbidden, false, "bad credentials do not fix themselves"}, } for _, tc := range cases { - err, ok := ParseComputerError(tc.status, nil).(*ComputerError) - if !ok { + var err *ComputerError + if !errors.As(ParseComputerError(tc.status, nil), &err) { t.Fatalf("status %d: expected a *ComputerError", tc.status) } if got := err.Retryable(); got != tc.want { From f89243be084e4e563c4ce9532286a3f32fcd46dd Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Sat, 26 Sep 2026 12:44:38 +0530 Subject: [PATCH 7/7] refactor(sandbox): simplify merged compositions --- cmd/root/usage_error.go | 32 ++++++++--------------- cmd/sandbox/compose.go | 40 +++++++++++++++++++++-------- cmd/sandbox/desktop.go | 29 +++++---------------- cmd/sandbox/edit.go | 6 ++--- cmd/sandbox/fork.go | 16 ++++++------ cmd/sandbox/guards.go | 13 ++++++++++ cmd/sandbox/matrix.go | 15 ++--------- cmd/sandbox/offload.go | 21 +++------------ cmd/sandbox/pull.go | 6 +---- cmd/sandbox/push.go | 6 +---- cmd/sandbox/stage.go | 2 +- internal/api/sandbox_computer.go | 44 +++++++++++--------------------- 12 files changed, 96 insertions(+), 134 deletions(-) diff --git a/cmd/root/usage_error.go b/cmd/root/usage_error.go index d9940c7..4c61321 100644 --- a/cmd/root/usage_error.go +++ b/cmd/root/usage_error.go @@ -96,9 +96,6 @@ func correctedCommandLine(flagName string) string { } rest = append(rest, a) } - if len(moved) == 0 { - return "createos " + strings.Join(args, " ") - } return "createos " + strings.Join(append(moved, rest...), " ") } @@ -120,16 +117,7 @@ func correctedCommandLine(flagName string) string { // subcommand's Run instead. So "argument present here" and "unknown // command" are the same condition. func installCommandSuggestions(app *cli.App) { - fallback := app.Action - app.Action = func(c *cli.Context) error { - if name := c.Args().First(); name != "" { - return unknownCommandError(app.Commands, "", name) - } - if fallback != nil { - return fallback(c) - } - return cli.ShowSubcommandHelp(c) - } + app.Action = suggestingAction(app.Action, app.Commands, "") for _, cmd := range app.Commands { installGroupSuggestions(cmd) } @@ -139,20 +127,22 @@ func installGroupSuggestions(cmd *cli.Command) { if cmd == nil || len(cmd.Subcommands) == 0 { return } - prefix := cmd.Name + " " - fallback := cmd.Action - cmd.Action = func(c *cli.Context) error { + cmd.Action = suggestingAction(cmd.Action, cmd.Subcommands, cmd.Name+" ") + for _, sub := range cmd.Subcommands { + installGroupSuggestions(sub) + } +} + +func suggestingAction(fallback cli.ActionFunc, cands []*cli.Command, prefix string) cli.ActionFunc { + return func(c *cli.Context) error { if name := c.Args().First(); name != "" { - return unknownCommandError(cmd.Subcommands, prefix, name) + return unknownCommandError(cands, prefix, name) } if fallback != nil { return fallback(c) } return cli.ShowSubcommandHelp(c) } - for _, sub := range cmd.Subcommands { - installGroupSuggestions(sub) - } } // unknownCommandError builds the same message the old CommandNotFound @@ -205,7 +195,7 @@ func editDistance(a, b string) int { if a[i-1] == b[j-1] { cost = 0 } - cur[j] = min(prev[j]+1, min(cur[j-1]+1, prev[j-1]+cost)) + cur[j] = min(prev[j]+1, cur[j-1]+1, prev[j-1]+cost) } prev = cur } diff --git a/cmd/sandbox/compose.go b/cmd/sandbox/compose.go index c56c189..29ec574 100644 --- a/cmd/sandbox/compose.go +++ b/cmd/sandbox/compose.go @@ -9,9 +9,11 @@ import ( "strings" "time" + "github.com/pterm/pterm" "github.com/urfave/cli/v2" "github.com/NodeOps-app/createos-cli/internal/api" + "github.com/NodeOps-app/createos-cli/internal/output" ) // The composed verbs (offload, matrix) share one box recipe. Keeping the @@ -122,6 +124,22 @@ func parseKeyValues(pairs []string) (map[string]string, error) { // createComposeBox boots one sandbox to the shared recipe and waits for it // to run. +// composeRun applies --timeout to the command context and returns an info +// printer that stays silent under JSON output. +func composeRun(c *cli.Context, opts *composeOptions) (context.Context, context.CancelFunc, bool, func(string, ...any)) { + ctx, cancel := c.Context, context.CancelFunc(func() {}) + if opts.Timeout > 0 { + ctx, cancel = context.WithTimeout(ctx, opts.Timeout) + } + quiet := output.IsJSON(c) + say := func(format string, a ...any) { + if !quiet { + pterm.Info.Printfln(format, a...) + } + } + return ctx, cancel, quiet, say +} + func createComposeBox(ctx context.Context, client *api.SandboxClient, opts *composeOptions) (*api.SandboxView, error) { // No name: these boxes are machinery with a lifetime of one command, // and a generated name is easier to tell apart in `sandbox ls` than a @@ -144,11 +162,11 @@ func createComposeBox(ctx context.Context, client *api.SandboxClient, opts *comp // that is not "running" has to destroy it, or a readiness timeout // leaves a machine nobody knows about — offload and matrix only see // the error, never the id. - sb, err := waitForStatus(ctx, client, created.ID, "running") + sb, err := waitForStatus(ctx, client, created.ID, api.SandboxStatusRunning) if err != nil { return nil, cleanupAfterCreate(ctx, client, created.ID, err) } - if sb.Status != "running" { + if sb.Status != api.SandboxStatusRunning { return nil, cleanupAfterCreate(ctx, client, sb.ID, fmt.Errorf("sandbox %s came up %s, not running", sb.ID, sb.Status)) } @@ -159,9 +177,7 @@ func createComposeBox(ctx context.Context, client *api.SandboxClient, opts *comp // the outcome into the error the caller sees. The id is always named: if // the teardown itself fails, the user needs it to clean up by hand. func cleanupAfterCreate(ctx context.Context, client *api.SandboxClient, id string, cause error) error { - tearCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) - defer cancel() - if err := client.DestroySandbox(tearCtx, id); err != nil { + if err := destroyDetached(ctx, client, id); err != nil { return fmt.Errorf("%w\n\n Sandbox %s was created and could not be destroyed (%w).\n It is still billable. Remove it with:\n createos sandbox rm --force %s", cause, id, err, id) } @@ -263,11 +279,15 @@ func runManaged( // this, and a teardown error must not mask the real one — but it must not // be swallowed either, because the sandbox is still billable. func destroyQuiet(ctx context.Context, client *api.SandboxClient, id string, warn func(string)) { - // The caller's context may already be cancelled (Ctrl-C, timeout). - // Teardown still has to happen, so give it a context of its own. - tearCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) - defer cancel() - if err := client.DestroySandbox(tearCtx, id); err != nil && warn != nil { + if err := destroyDetached(ctx, client, id); err != nil && warn != nil { warn(fmt.Sprintf("could not destroy %s: %v — remove it with: createos sandbox rm --force %s", id, err, id)) } } + +// destroyDetached destroys id even when ctx is already cancelled (Ctrl-C, +// timeout): teardown still has to happen, so it gets a context of its own. +func destroyDetached(ctx context.Context, client *api.SandboxClient, id string) error { + tearCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) + defer cancel() + return client.DestroySandbox(tearCtx, id) +} diff --git a/cmd/sandbox/desktop.go b/cmd/sandbox/desktop.go index fabccd2..825f459 100644 --- a/cmd/sandbox/desktop.go +++ b/cmd/sandbox/desktop.go @@ -12,7 +12,6 @@ import ( "github.com/NodeOps-app/createos-cli/internal/api" "github.com/NodeOps-app/createos-cli/internal/output" - "github.com/NodeOps-app/createos-cli/internal/terminal" ) // The desktop stack (Xvfb → XFCE → x11vnc → websockify) starts *after* the @@ -63,27 +62,13 @@ func runDesktop(c *cli.Context) error { } args := parseComputerArgs(c) - ref := args.ref - var id string - switch { - case ref != "": - resolved, err := resolveSandboxRef(c.Context, client, ref) - if err != nil { - return err - } - id = resolved - case terminal.IsInteractive(): - picked, label, err := pickByStatus(c, client, "Pick a sandbox to open", api.SandboxStatusRunning) - if err != nil { - return err - } - if picked == "" { - fmt.Println("Cancelled. Nothing changed.") - return nil - } - id, ref = picked, label - default: - return fmt.Errorf("please provide a sandbox ID or name\n\n To see your sandboxes, run:\n createos sandbox list") + id, ref, err := resolveTarget(c, client, args.ref, "Pick a sandbox to open") + if err != nil { + return err + } + if id == "" { + fmt.Println("Cancelled. Nothing changed.") + return nil } sb, err := client.GetSandbox(c.Context, id) diff --git a/cmd/sandbox/edit.go b/cmd/sandbox/edit.go index 79c209d..d9e4e16 100644 --- a/cmd/sandbox/edit.go +++ b/cmd/sandbox/edit.go @@ -60,7 +60,7 @@ func runEdit(c *cli.Context) error { hasFlagChanges := ingressFlag != "" || autoPauseFlag != "" || len(sshFiles) > 0 // Resolve the sandbox first — either from positional or via picker. - id, label, err := resolveTarget(c, client, ref) + id, label, err := resolveTarget(c, client, ref, "Pick a sandbox to edit") if err != nil { return err } @@ -144,7 +144,7 @@ func parseEditArgs(c *cli.Context) (ref, ingressVal, autoPauseVal string, sshPat // resolveTarget figures out which sandbox the user wants to edit. With // a positional ref → resolve. Without one, picker on TTY, error otherwise. -func resolveTarget(c *cli.Context, client *api.SandboxClient, ref string) (id, label string, err error) { +func resolveTarget(c *cli.Context, client *api.SandboxClient, ref, pickTitle string) (id, label string, err error) { if ref != "" { resolved, err := resolveSandboxRef(c.Context, client, ref) if err != nil { @@ -155,7 +155,7 @@ func resolveTarget(c *cli.Context, client *api.SandboxClient, ref string) (id, l if !terminal.IsInteractive() { return "", "", fmt.Errorf("please provide a sandbox ID or name\n\n To see your sandboxes, run:\n createos sandbox list") } - return pickByStatus(c, client, "Pick a sandbox to edit", api.SandboxStatusRunning) + return pickByStatus(c, client, pickTitle, api.SandboxStatusRunning) } // runEditMenu is the interactive flow once a sandbox is selected. Pulls diff --git a/cmd/sandbox/fork.go b/cmd/sandbox/fork.go index 5d079cb..722d810 100644 --- a/cmd/sandbox/fork.go +++ b/cmd/sandbox/fork.go @@ -167,13 +167,13 @@ func ensureForkable(ctx context.Context, client *api.SandboxClient, srcID string return err } switch sb.Status { - case "paused": + case api.SandboxStatusPaused: return nil - case "pausing": + case api.SandboxStatusPausing: // Already on its way down; waiting is not a decision we are making // on the user's behalf. return waitUntilPaused(ctx, client, srcID) - case "running": + case api.SandboxStatusRunning: // Deliberately NOT pausing here. Pausing a running sandbox stops // whatever it is serving, and `fork` must never do that as a side // effect — the source could be a live dev server or a demo someone @@ -195,10 +195,10 @@ func pauseForFork(ctx context.Context, client *api.SandboxClient, srcID string) return err } switch sb.Status { - case "paused": + case api.SandboxStatusPaused: return nil - case "pausing": - case "running": + case api.SandboxStatusPausing: + case api.SandboxStatusRunning: if _, pauseErr := client.PauseSandbox(ctx, srcID); pauseErr != nil { return fmt.Errorf("pause %s before forking: %w", srcID, pauseErr) } @@ -212,11 +212,11 @@ func pauseForFork(ctx context.Context, client *api.SandboxClient, srcID string) // answers while the sandbox is still `pausing`, and a fork issued in that // window is rejected with "sandbox is running, expected paused or error". func waitUntilPaused(ctx context.Context, client *api.SandboxClient, srcID string) error { - final, err := waitForStatus(ctx, client, srcID, "paused") + final, err := waitForStatus(ctx, client, srcID, api.SandboxStatusPaused) if err != nil { return err } - if final.Status != "paused" { + if final.Status != api.SandboxStatusPaused { return fmt.Errorf("sandbox %s ended in %q while pausing — see `createos sandbox get %s`", srcID, final.Status, srcID) } return nil diff --git a/cmd/sandbox/guards.go b/cmd/sandbox/guards.go index fd960a8..4e532f4 100644 --- a/cmd/sandbox/guards.go +++ b/cmd/sandbox/guards.go @@ -49,6 +49,19 @@ func diskMountBlocksFileAPI(ctx context.Context, client *api.SandboxClient, sand return "", nil } +// refuseDiskMountTransfer fails when remote sits inside a disk mount, or +// when the mount state cannot be read. +func refuseDiskMountTransfer(ctx context.Context, client *api.SandboxClient, sandboxID, remote, verb string) error { + mount, err := diskMountBlocksFileAPI(ctx, client, sandboxID, remote) + if err != nil { + return err + } + if mount != "" { + return diskMountFileAPIError(remote, mount, verb) + } + return nil +} + // diskMountFileAPIError is the refusal. This is a hard stop rather than a // warning: the documented outcome is a crashed mount and a lost object, so // carrying on would destroy data the user believes they just saved. diff --git a/cmd/sandbox/matrix.go b/cmd/sandbox/matrix.go index deb3ffc..ab49238 100644 --- a/cmd/sandbox/matrix.go +++ b/cmd/sandbox/matrix.go @@ -136,19 +136,8 @@ func runMatrix(c *cli.Context) error { return err } - ctx := c.Context - if opts.Timeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, opts.Timeout) - defer cancel() - } - - quiet := output.IsJSON(c) - say := func(format string, a ...any) { - if !quiet { - pterm.Info.Printfln(format, a...) - } - } + ctx, cancel, quiet, say := composeRun(c, opts) + defer cancel() logDir, err := matrixLogDir(c.String("logs")) if err != nil { diff --git a/cmd/sandbox/offload.go b/cmd/sandbox/offload.go index 403098f..59ee302 100644 --- a/cmd/sandbox/offload.go +++ b/cmd/sandbox/offload.go @@ -82,19 +82,8 @@ func runOffload(c *cli.Context) error { return err } - ctx := c.Context - if opts.Timeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, opts.Timeout) - defer cancel() - } - - quiet := output.IsJSON(c) - say := func(format string, a ...any) { - if !quiet { - pterm.Info.Printfln(format, a...) - } - } + ctx, cancel, quiet, say := composeRun(c, opts) + defer cancel() tree, err := stageDir(ctx, dir, stageOptions{Exclude: opts.Exclude}) if err != nil { @@ -141,14 +130,13 @@ func runOffload(c *cli.Context) error { } teardownFailure := "" + destroyed = true if res.ExitCode != 0 && c.Bool("keep-on-fail") { - destroyed = true pterm.Warning.Printfln("Command exited %d. Sandbox %s kept.", res.ExitCode, sb.ID) fmt.Printf(" Look around: createos sandbox shell %s\n", sb.ID) fmt.Printf(" Destroy it: createos sandbox rm --force %s\n", sb.ID) } else { destroyQuiet(ctx, client, sb.ID, func(msg string) { teardownFailure = msg }) - destroyed = true } if quiet { @@ -293,7 +281,7 @@ func untarInto(r io.Reader, root string) error { } name := path.Clean("/" + filepath.ToSlash(hdr.Name)) name = strings.TrimPrefix(name, "/") - if name == "" || name == "." { + if name == "" { continue } switch hdr.Typeflag { @@ -313,7 +301,6 @@ func untarInto(r io.Reader, root string) error { default: // Symlinks and devices out of a sandbox have no safe meaning // on the caller's disk. Skip them rather than guess. - continue } } } diff --git a/cmd/sandbox/pull.go b/cmd/sandbox/pull.go index e5a5ac6..6ce4548 100644 --- a/cmd/sandbox/pull.go +++ b/cmd/sandbox/pull.go @@ -58,13 +58,9 @@ func runPull(c *cli.Context) error { return err } - mount, err := diskMountBlocksFileAPI(c.Context, client, id, remote) - if err != nil { + if err = refuseDiskMountTransfer(c.Context, client, id, remote, "pull"); err != nil { return err } - if mount != "" { - return diskMountFileAPIError(remote, mount, "pull") - } f, err := os.Create(local) // #nosec G304 -- local is a user-supplied destination path if err != nil { diff --git a/cmd/sandbox/push.go b/cmd/sandbox/push.go index 5aa5307..db3cb8a 100644 --- a/cmd/sandbox/push.go +++ b/cmd/sandbox/push.go @@ -61,13 +61,9 @@ func runPush(c *cli.Context) error { if runErr := ensureSandboxRunningFor(c, client, ref, id, "push"); runErr != nil { return runErr } - mount, err := diskMountBlocksFileAPI(c.Context, client, id, remote) - if err != nil { + if err := refuseDiskMountTransfer(c.Context, client, id, remote, "push"); err != nil { return err } - if mount != "" { - return diskMountFileAPIError(remote, mount, "push") - } // Open the source: a real file (we know its size for Content-Length) // or stdin ("-") for piped uploads. diff --git a/cmd/sandbox/stage.go b/cmd/sandbox/stage.go index 98a1e3d..d856e9b 100644 --- a/cmd/sandbox/stage.go +++ b/cmd/sandbox/stage.go @@ -245,7 +245,7 @@ func stageTarAppend(tw *tar.Writer, root, rel string) error { if err = tw.WriteHeader(hdr); err != nil { return err } - if link != "" || !info.Mode().IsRegular() { + if !info.Mode().IsRegular() { return nil } f, err := os.Open(abs) // #nosec G304,G703 -- see the Lstat note above diff --git a/internal/api/sandbox_computer.go b/internal/api/sandbox_computer.go index 84ee5a8..1dc018d 100644 --- a/internal/api/sandbox_computer.go +++ b/internal/api/sandbox_computer.go @@ -6,6 +6,8 @@ import ( "fmt" "net/http" "strings" + + "github.com/go-resty/resty/v2" ) // Computer-use routes: GET/POST /v1/sandboxes/:id/computer/*. @@ -51,38 +53,18 @@ type ComputerConnection struct { // readiness probe: it is the cheapest route that only answers 2xx once the // desktop stack (Xvfb → XFCE → x11vnc → websockify) is actually up. func (c *SandboxClient) ComputerScreen(ctx context.Context, id, screen string) (*ComputerScreenGeometry, error) { - var envelope Response[ComputerScreenGeometry] - resp, err := c.Client.R(). + return computerGet[ComputerScreenGeometry](c.Client.R(). SetContext(ctx). SetPathParam("id", id). - SetQueryParam("screen_id", computerScreen(screen)). - SetResult(&envelope). - Get("/v1/sandboxes/{id}/computer/screen") - if err != nil { - return nil, err - } - if resp.IsError() { - return nil, ParseComputerError(resp.StatusCode(), resp.Body()) - } - return &envelope.Data, nil + SetQueryParam("screen_id", computerScreen(screen)), "/v1/sandboxes/{id}/computer/screen") } // ComputerCursor returns the pointer's current position. func (c *SandboxClient) ComputerCursor(ctx context.Context, id, screen string) (*ComputerCursorPos, error) { - var envelope Response[ComputerCursorPos] - resp, err := c.Client.R(). + return computerGet[ComputerCursorPos](c.Client.R(). SetContext(ctx). SetPathParam("id", id). - SetQueryParam("screen_id", computerScreen(screen)). - SetResult(&envelope). - Get("/v1/sandboxes/{id}/computer/cursor") - if err != nil { - return nil, err - } - if resp.IsError() { - return nil, ParseComputerError(resp.StatusCode(), resp.Body()) - } - return &envelope.Data, nil + SetQueryParam("screen_id", computerScreen(screen)), "/v1/sandboxes/{id}/computer/cursor") } // ComputerWindows lists the windows on the screen. The window shape is fc's to @@ -153,13 +135,17 @@ func (c *SandboxClient) ComputerOpen(ctx context.Context, id, screen, target str // ingress is enabled on the sandbox; a fresh call invalidates the previous // link for new connections. func (c *SandboxClient) ComputerConnect(ctx context.Context, id, screen string) (*ComputerConnection, error) { - var envelope Response[ComputerConnection] - resp, err := c.Client.R(). + return computerGet[ComputerConnection](c.Client.R(). SetContext(ctx). SetPathParam("id", id). - SetPathParam("screen", computerScreen(screen)). - SetResult(&envelope). - Get("/v1/sandboxes/{id}/computer/screens/{screen}/connect") + SetPathParam("screen", computerScreen(screen)), "/v1/sandboxes/{id}/computer/screens/{screen}/connect") +} + +// computerGet sends req as a GET and unwraps the typed Response envelope. +// Go has no generic methods, so this is a plain function. +func computerGet[T any](req *resty.Request, url string) (*T, error) { + var envelope Response[T] + resp, err := req.SetResult(&envelope).Get(url) if err != nil { return nil, err }