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..d9940c7 --- /dev/null +++ b/cmd/root/usage_error.go @@ -0,0 +1,213 @@ +package root + +import ( + "errors" + "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, 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) { + 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) + } + 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) + } + 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 +// 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..b6de1ab --- /dev/null +++ b/cmd/root/usage_error_test.go @@ -0,0 +1,192 @@ +package root + +import ( + "errors" + "os" + "strings" + "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) + } + } +} + +// 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 + 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..1e9a426 100644 --- a/cmd/sandbox/fork.go +++ b/cmd/sandbox/fork.go @@ -1,7 +1,9 @@ package sandbox import ( + "context" "fmt" + "strconv" "strings" "github.com/pterm/pterm" @@ -21,12 +23,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)", @@ -46,6 +67,10 @@ func runFork(c *cli.Context) error { return fmt.Errorf("you're not signed in — run 'createos login' to get started") } + if count := forkCountFlag(c); 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() { @@ -69,6 +94,8 @@ func runFork(c *cli.Context) error { } func runForkByID(c *cli.Context, client *api.SandboxClient, ref, srcID string) error { + count := forkCountFlag(c) + req := api.SandboxForkReq{ StartPaused: c.Bool("paused"), } @@ -82,53 +109,220 @@ 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 } + 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) + } + } + 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 { + return err + } + 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) + } +} + +// 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) + } + 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 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" } - sb, err := waitForStatus(c.Context, client, view.ID, target) - if err != nil { - spinner.Fail("Fork did not finish") + + // 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 } - 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) + 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 +} - name := "" - if sb.Name != nil { - name = *sb.Name +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") } - spinner.Success(fmt.Sprintf("Forked into %s", refLabel(name, sb.ID))) - if sb.IP != nil && *sb.IP != "" { - fmt.Printf(" IP: %s\n", *sb.IP) + raw := rawProcessFlagValue("fork", "count") + if raw == "" { + return c.Int("count") } - if sb.IngressURLTemplate != "" { - fmt.Printf(" URL: %s\n", sb.IngressURLTemplate) + n, err := strconv.Atoi(raw) + if err != nil { + return c.Int("count") } - return nil + return n } 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..990c5b0 --- /dev/null +++ b/cmd/sandbox/lifecycle_test.go @@ -0,0 +1,406 @@ +package sandbox + +import ( + "archive/tar" + "bytes" + "context" + "errors" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "sync" + "sync/atomic" + "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) + } +} + +// 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 +// 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) + } + } +} + +// 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) + } +} 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} }