From a68d3ac90bfa04220bba19056c94bf5ffa4afaa8 Mon Sep 17 00:00:00 2001 From: pratikbin <68642400+pratikbin@users.noreply.github.com> Date: Wed, 26 Aug 2026 12:52:05 +0530 Subject: [PATCH] fix(sandbox): honor --cwd/--env after the box name urfave/cli stops parsing flags at the first positional argument, so every flag written after the sandbox name is discarded in silence: process run --cwd /workspace -- pwd body: {"cmd":"pwd"} cwd lost process run --cwd /workspace -- pwd body: {"cmd":"pwd","cwd":"/workspace"} cwd sent The process commands already carried raw-argv fallbacks for exactly this problem (processBoolFlag, processIntFlag, processStringFlag, rawProcessFlagValue). That is why --pty survived after the box name and --cwd did not: the cwd, cmd, and env reads never used them. Wire cwd and cmd to the existing processStringFlag. Add processStringSliceFlag and rawProcessFlagValues, because --env is repeatable and needs every occurrence, not just the first. Both raw readers stop at "--", so a sandbox command can never inject a flag. A test covers that. This is the same class of bug as #66, which hand-rolled parsing for the tunnel command. Two commands still carry it: sandbox sync reads --local/--remote/--mode with no fallback, and sandbox exec loses --stream. Left alone here, because the root fix is one argv reorder before parsing and it touches every command. Verified against a live sandbox: old, flag after box: "cwd": "/root" new, flag after box: "cwd": "/workspace" new, flag before box: "cwd": "/workspace" new, no flag: "cwd": "/root" --- cmd/sandbox/process.go | 46 ++++++++++++++++++--- cmd/sandbox/process_flags_test.go | 68 +++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 cmd/sandbox/process_flags_test.go diff --git a/cmd/sandbox/process.go b/cmd/sandbox/process.go index 8b90ed8..61ff4ff 100644 --- a/cmd/sandbox/process.go +++ b/cmd/sandbox/process.go @@ -327,13 +327,13 @@ func runProcessShell(c *cli.Context) error { if err != nil { return err } - envs, err := parseEnvFlags(c.StringSlice("env")) + envs, err := parseEnvFlags(processStringSliceFlag(c, subcommand, "env")) if err != nil { return err } req := api.ProcessCreateRequest{ - Cmd: c.String("cmd"), - Cwd: strings.TrimSpace(c.String("cwd")), + Cmd: processStringFlag(c, subcommand, "cmd"), + Cwd: strings.TrimSpace(processStringFlag(c, subcommand, "cwd")), Env: envs, PTY: ptyOptionsFromFlags(c, !output.IsJSON(c) && !processBoolFlag(c, subcommand, "no-attach")), } @@ -671,7 +671,7 @@ func processClientSandboxAndOptionalProcess(c *cli.Context) (*api.SandboxClient, func processCreateRequestFromCLI(c *cli.Context, shellMode bool) (api.ProcessCreateRequest, error) { subcommand := c.Command.Name - envs, err := parseEnvFlags(c.StringSlice("env")) + envs, err := parseEnvFlags(processStringSliceFlag(c, subcommand, "env")) if err != nil { return api.ProcessCreateRequest{}, err } @@ -683,7 +683,7 @@ func processCreateRequestFromCLI(c *cli.Context, shellMode bool) (api.ProcessCre req := api.ProcessCreateRequest{ Cmd: cmd, Args: args, - Cwd: strings.TrimSpace(c.String("cwd")), + Cwd: strings.TrimSpace(processStringFlag(c, subcommand, "cwd")), Env: envs, } if processBoolFlag(c, subcommand, "pty") { @@ -1575,6 +1575,14 @@ func processInt64Flag(c *cli.Context, subcommand, name string) int64 { return v } +// Repeatable flags need every occurrence, not just the first. +func processStringSliceFlag(c *cli.Context, subcommand, name string) []string { + if v := c.StringSlice(name); len(v) > 0 { + return v + } + return rawProcessFlagValues(subcommand, name) +} + func processDurationFlag(c *cli.Context, subcommand, name string) time.Duration { if v := c.Duration(name); v != 0 { return v @@ -1608,6 +1616,34 @@ func rawProcessFlagPresent(subcommand, name string) bool { return false } +func rawProcessFlagValues(subcommand, name string) []string { + start := processSubcommandArgIndex(subcommand) + if start < 0 { + return nil + } + target := "--" + name + prefix := target + "=" + var out []string + for i := start + 1; i < len(os.Args); i++ { + arg := os.Args[i] + if arg == "--" { + break + } + if strings.HasPrefix(arg, prefix) { + out = append(out, strings.TrimPrefix(arg, prefix)) + continue + } + if arg == target && i+1 < len(os.Args) { + next := os.Args[i+1] + if !strings.HasPrefix(next, "-") { + out = append(out, next) + i++ + } + } + } + return out +} + func rawProcessFlagValue(subcommand, name string) string { start := processSubcommandArgIndex(subcommand) if start < 0 { diff --git a/cmd/sandbox/process_flags_test.go b/cmd/sandbox/process_flags_test.go new file mode 100644 index 0000000..946d60f --- /dev/null +++ b/cmd/sandbox/process_flags_test.go @@ -0,0 +1,68 @@ +package sandbox + +import ( + "os" + "testing" +) + +// urfave/cli stops parsing flags at the first positional argument, so a flag +// written after the sandbox name never reaches the cli.Context. The raw helpers +// recover it from os.Args. Values after "--" belong to the sandbox command and +// must never be read as flags. + +func withArgs(t *testing.T, args []string) { + t.Helper() + saved := os.Args + os.Args = args + t.Cleanup(func() { os.Args = saved }) +} + +func TestRawProcessFlagValueAfterPositional(t *testing.T) { + withArgs(t, []string{"createos", "sandbox", "process", "start", "my-box", "--cwd", "/workspace", "--", "claude"}) + if got := rawProcessFlagValue("start", "cwd"); got != "/workspace" { + t.Fatalf("cwd = %q, want /workspace", got) + } +} + +func TestRawProcessFlagValueEqualsForm(t *testing.T) { + withArgs(t, []string{"createos", "sandbox", "process", "start", "my-box", "--cwd=/workspace", "--", "claude"}) + if got := rawProcessFlagValue("start", "cwd"); got != "/workspace" { + t.Fatalf("cwd = %q, want /workspace", got) + } +} + +func TestRawProcessFlagValueStopsAtDoubleDash(t *testing.T) { + withArgs(t, []string{"createos", "sandbox", "process", "run", "my-box", "--", "sh", "-c", "--cwd", "/evil"}) + if got := rawProcessFlagValue("run", "cwd"); got != "" { + t.Fatalf("cwd = %q, want empty: values after -- are the sandbox command", got) + } +} + +func TestRawProcessFlagValuesCollectsEveryOccurrence(t *testing.T) { + withArgs(t, []string{"createos", "sandbox", "process", "start", "my-box", "--env", "A=1", "--env=B=2", "--", "claude"}) + got := rawProcessFlagValues("start", "env") + want := []string{"A=1", "B=2"} + if len(got) != len(want) { + t.Fatalf("env = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Fatalf("env = %v, want %v", got, want) + } + } +} + +func TestRawProcessFlagValuesStopsAtDoubleDash(t *testing.T) { + withArgs(t, []string{"createos", "sandbox", "process", "run", "my-box", "--env", "A=1", "--", "env", "--env", "B=2"}) + got := rawProcessFlagValues("run", "env") + if len(got) != 1 || got[0] != "A=1" { + t.Fatalf("env = %v, want [A=1]", got) + } +} + +func TestRawProcessFlagValuesEmptyWhenAbsent(t *testing.T) { + withArgs(t, []string{"createos", "sandbox", "process", "start", "my-box", "--", "claude"}) + if got := rawProcessFlagValues("start", "env"); len(got) != 0 { + t.Fatalf("env = %v, want empty", got) + } +}