Skip to content

fix(sandbox): honor --cwd/--env after the box name - #81

Merged
pratikbin merged 1 commit into
mainfrom
fix/process-string-flags
Aug 26, 2026
Merged

fix(sandbox): honor --cwd/--env after the box name#81
pratikbin merged 1 commit into
mainfrom
fix/process-string-flags

Conversation

@pratikbin

Copy link
Copy Markdown
Contributor

The bug

urfave/cli stops parsing flags at the first positional argument. Every flag written after the sandbox name is discarded, with no error.

Captured with CREATEOS_DEBUG=1:

command request body
process run <box> --cwd /workspace -- pwd {"cmd":"pwd"}cwd lost
process run --cwd /workspace <box> -- pwd {"cmd":"pwd","cwd":"/workspace"}

The control plane is not at fault. The CLI never sent the field.

Why --pty worked and --cwd did not

The process commands already carry raw-argv fallbacks for exactly this problem: processBoolFlag, processIntFlag, processInt64Flag, processStringFlag, and rawProcessFlagValue. Bool and int flags were recovered. The cwd, cmd, and env reads never used them.

The change

  • cwd and cmd now read through the existing processStringFlag.
  • New processStringSliceFlag and rawProcessFlagValues, because --env is repeatable and needs every occurrence, not just the first.
  • New cmd/sandbox/process_flags_test.go, 6 tests.

Both raw readers stop at --, so a sandbox command can never inject a flag. One test covers that:

process run my-box -- sh -c --cwd /evil    ->  cwd stays empty

Who this unblocks

The Herdr plugin (createos.sandbox) starts its agent with process start <box> --pty --cwd /workspace. The agent landed in /root and could not see the uploaded worktree. Confirmed with /proc/<pid>/cwd.

Verified against a live sandbox

CLI flag position effective cwd
old after box /root
new after box /workspace
new before box /workspace
new absent /root

Checks run: go build ./..., go vet, full go test ./..., golangci-lint 2.11.3 (the version pinned in .tool-versions, 0 issues), gosec (no findings in the changed file).

Still open, deliberately

This is the same class of bug as #66, which hand-rolled parsing for tunnel. Two commands still carry it:

  • cmd/sandbox/sync.go reads --local, --remote, --mode with no fallback, so sandbox sync <box> --local ... drops them and falls back to an interactive prompt.
  • sandbox exec <box> --stream loses --stream.

The per-command fallbacks are a workaround. The root fix is one argv reorder before urfave/cli parses, which would remove the need for all of them. That touches every command, so it is left for a separate decision.

urfave/cli stops parsing flags at the first positional argument, so
every flag written after the sandbox name is discarded in silence:

  process run <box> --cwd /workspace -- pwd
    body: {"cmd":"pwd"}                       cwd lost
  process run --cwd /workspace <box> -- 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"
@pratikbin
pratikbin merged commit 8c1f7ac into main Aug 26, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant