diff --git a/docs/cli-reference.md b/docs/cli-reference.md index d321786f..102efc5a 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -43,7 +43,7 @@ Manage Docker-backed persistent sandboxes with bind mounts and named volumes. ### Global sandbox flags -These persistent flags apply to all `sandbox` subcommands (`create`, `list`, `connect`, `stop`, `start`, `delete`, `ssh`, `code`, `codev2`, `agent-send`): +These persistent flags apply to all `sandbox` subcommands (`create`, `list`, `connect`, `stop`, `start`, `delete`, `ssh`, `sshv2`, `code`, `codev2`, `agent-send`). For `sshv2` they must be written before the subcommand, since everything after it is forwarded to `ssh`: | Flag | Default | Description | | ---------- | ------- | -------------------------------- | @@ -314,6 +314,50 @@ amika sandbox codev2 my-sandbox --editor=codex `--editor` and `--path` have the same values and defaults as `sandbox code`. +### `amika sandbox sshv2` + +Open an SSH session to a remote sandbox over the beta direct WebSocket +transport. Remote sandboxes only; requires an SSH identity from +`amika secret ssh-keygen`. + +``` +amika sandbox sshv2 [ssh-options] [command...] +``` + +Use it like `ssh`: options go before the sandbox name, an optional command +after it. Every ssh option works, including port forwarding, and `sshv2` +defines no flags of its own. + +Amika's own flags go **before** `sshv2`: + +```bash +amika sandbox --remote sshv2 -N -L 8080:localhost:80 my-sandbox +``` + +`--help` is the one exception: `amika sandbox sshv2 --help` prints amika's +help, as does `amika help sandbox sshv2`. + +```bash +# Interactive shell +amika sandbox sshv2 my-sandbox + +# Run a command instead of opening a shell +amika sandbox sshv2 my-sandbox uptime + +# Forward local port 6789 to port 3010 inside the sandbox, without a shell +amika sandbox sshv2 -N -L 6789:localhost:3010 my-sandbox + +# SOCKS proxy on local port 1080 +amika sandbox sshv2 -N -D 1080 my-sandbox +``` + +Local (`-L`) and dynamic (`-D`) forwarding are supported. Remote forwarding +(`-R`), agent forwarding (`-A`), and X11 forwarding are not. + +`-o` after the subcommand is ssh's ssh_config option, so amika's +`-o`/`--output` is not available there; written before `sshv2` it is rejected, +as it is for `sandbox ssh` (see above). + ### `amika sandbox agent-send` Send a prompt to an AI agent CLI running inside a sandbox container. The message can be provided as a positional argument or piped via stdin. By default the command waits for the agent to finish and streams the response. diff --git a/go/cmd/amika/sandbox/command.go b/go/cmd/amika/sandbox/command.go index 1b6cd772..bf5f5813 100644 --- a/go/cmd/amika/sandbox/command.go +++ b/go/cmd/amika/sandbox/command.go @@ -67,7 +67,9 @@ func New() *cobra.Command { sandboxSSHCmd.Flags().BoolP("t", "t", false, "Force pseudo-terminal allocation (like ssh -t)") sandboxSSHCmd.Flags().Bool("revoke", false, "Revoke SSH access for the sandbox") sandboxSSHCmd.Flags().Bool("print", false, "Print the SSH connection string instead of connecting") - sandboxSSHV2Cmd.Flags().BoolP("t", "t", false, "Force pseudo-terminal allocation (like ssh -t)") + // sshv2 registers no flags of its own: it forwards everything after the + // subcommand to ssh, so ssh's own -t (and every other option) passes + // through untouched. sandboxCodeCmd.Flags().String("editor", "cursor", "Editor or agent to open: \"cursor\", \"claude\", or \"codex\"") sandboxCodeCmd.Flags().String("path", "", "Override the remote path to open (absolute, or relative to the sandbox workspace root)") sandboxCodeV2Cmd.Flags().String("editor", "cursor", "Editor or agent to open: \"cursor\", \"claude\", or \"codex\"") diff --git a/go/cmd/amika/sandbox/sandbox_ssh_v2.go b/go/cmd/amika/sandbox/sandbox_ssh_v2.go index 53a0ae9a..9c09cdcb 100644 --- a/go/cmd/amika/sandbox/sandbox_ssh_v2.go +++ b/go/cmd/amika/sandbox/sandbox_ssh_v2.go @@ -2,42 +2,122 @@ package sandboxcmd import ( "fmt" + "os" "github.com/gofixpoint/amika/go/internal/apiclient" "github.com/gofixpoint/amika/go/internal/basedir" + "github.com/gofixpoint/amika/go/internal/cliargs" "github.com/gofixpoint/amika/go/internal/output" "github.com/gofixpoint/amika/go/internal/runmode" "github.com/gofixpoint/amika/go/internal/ssh" "github.com/spf13/cobra" ) +// osArgs is a seam over the process argv, which the positional split needs to +// tell an amika flag written before "sshv2" from an ssh option written after +// it. Tests supply a synthetic argv instead of mutating the real one. +var osArgs = func() []string { return os.Args } + +// execSessionSSH is a seam around the exec of the system ssh binary, so tests +// can assert the argv the command would run without replacing the process. +var execSessionSSH = ssh.ExecSessionSSH + +// newSSHV2Client is a seam over the API client sshv2 resolves a sandbox +// through, narrowed to the two calls the command makes so tests can supply a +// stub instead of reaching the network. +var newSSHV2Client = func(target string) (sshV2Client, error) { + return getRemoteClient(target) +} + +// sshV2OwnValueFlags are the amika flags reaching sshv2 that take their value +// as a separate token. commandIndex skips those values so a sandbox named for +// the subcommand ("--remote-target sshv2") is not read as the subcommand. +var sshV2OwnValueFlags = map[string]bool{ + "--output": true, + "-o": true, + "--remote-target": true, +} + var sandboxSSHV2Cmd = &cobra.Command{ - Use: "sshv2 [flags] [-- ...]", + Use: "sshv2 [ssh-options] [command...]", Short: "SSH through the beta direct WebSocket transport", - Args: cobra.MinimumNArgs(1), + Long: `Open an SSH session to a remote sandbox over the beta direct WebSocket +transport. Requires an SSH identity from "amika secret ssh-keygen". + +Use it like ssh: options go before the sandbox name, an optional command +after it. Every ssh option works, including port forwarding. + +Amika's own flags go before "sshv2": + + amika sandbox --remote sshv2 -N -L 8080:localhost:80 my-sandbox + +Local (-L) and dynamic (-D) forwarding are supported. Remote forwarding (-R), +agent forwarding (-A), and X11 forwarding are not. + +Examples: + # Interactive shell + amika sandbox sshv2 my-sandbox + + # Run a command instead of opening a shell + amika sandbox sshv2 my-sandbox uptime + + # Forward local port 6789 to port 3010 inside the sandbox, no shell + amika sandbox sshv2 -N -L 6789:localhost:3010 my-sandbox + + # SOCKS proxy on local port 1080 + amika sandbox sshv2 -N -D 1080 my-sandbox`, + // Arguments after "sshv2" belong to ssh, so Cobra must not parse them: it + // would reject "-L" and friends as unknown flags. RunE therefore parses the + // amika-owned portion itself, after splitting the two apart by position. + DisableFlagParsing: true, + // The usage line already spells out where options go, and the trailing + // "[flags]" Cobra appends would suggest amika flags belong after the + // subcommand, which is exactly what they must not do. + DisableFlagsInUseLine: true, RunE: func(cmd *cobra.Command, args []string) error { - if runmode.Resolve(cmd) == runmode.Local { - return fmt.Errorf("direct WebSocket SSH requires a remote sandbox") + own, forward := cliargs.Split(osArgs(), args, cmd.Name(), sshV2OwnValueFlags) + // DisableFlagParsing bypasses Cobra's built-in help flag, so honor it + // here before anything else can fail on an incomplete command line. + if cliargs.HasHelpFlag(forward) || cliargs.HasHelpFlag(own) { + return cmd.Help() } - if err := runmode.RequireAuth(runmode.Remote, runmode.DefaultAuthChecker); err != nil { + // Cobra merges the parents' persistent flags into a command's own set + // inside ParseFlags, which DisableFlagParsing skips. Reading + // InheritedFlags forces that merge, so --local, --remote, + // --remote-target, and --output resolve as they do elsewhere. + _ = cmd.InheritedFlags() + if err := cmd.Flags().Parse(own); err != nil { return err } if err := output.RejectFlag(cmd); err != nil { return err } + if runmode.Resolve(cmd) == runmode.Local { + return fmt.Errorf("direct WebSocket SSH requires a remote sandbox") + } + // Locate the sandbox name before requiring auth, so an unusable command + // line is reported as the usage error it is rather than as a login + // prompt. + nameIdx := cliargs.FirstOperand(forward, cliargs.SSHArgLetters) + if nameIdx < 0 { + return fmt.Errorf("missing sandbox name; usage: amika sandbox sshv2 [ssh-options] [command...]") + } + if err := runmode.RequireAuth(runmode.Remote, runmode.DefaultAuthChecker); err != nil { + return err + } target, err := getRemoteTarget(cmd) if err != nil { return err } - client, err := getRemoteClient(target) + client, err := newSSHV2Client(target) if err != nil { return err } - sandbox, err := client.GetSandbox(args[0]) + sandbox, err := client.GetSandbox(forward[nameIdx]) if err != nil { return err } - alias, err := ssh.PrepareSessionTarget( + alias, err := prepareSessionTarget( basedir.New(""), client, sandbox.Name, @@ -46,8 +126,7 @@ var sandboxSSHV2Cmd = &cobra.Command{ if err != nil { return err } - forcePTY, _ := cmd.Flags().GetBool("t") - return ssh.ExecSessionSSH(alias, forcePTY, args[1:]) + return execSessionSSH(alias, ssh.BuildSessionSSHArgv(forward, nameIdx, alias)) }, } diff --git a/go/cmd/amika/sandbox/sandbox_ssh_v2_args_test.go b/go/cmd/amika/sandbox/sandbox_ssh_v2_args_test.go new file mode 100644 index 00000000..7da73eca --- /dev/null +++ b/go/cmd/amika/sandbox/sandbox_ssh_v2_args_test.go @@ -0,0 +1,266 @@ +package sandboxcmd + +// sandbox_ssh_v2_args_test.go covers how `amika sandbox sshv2` divides a +// command line between amika and the system ssh binary. The assertions are on +// the argv the command would exec, which is the whole contract: an argument +// written after "sshv2" reaches ssh untouched and in its original position, +// and one written before it does not reach ssh at all. + +import ( + "bytes" + "reflect" + "strings" + "sync" + "testing" + + "github.com/gofixpoint/amika/go/internal/apiclient" + "github.com/gofixpoint/amika/go/internal/basedir" + "github.com/gofixpoint/amika/go/internal/ssh" + "github.com/spf13/cobra" + "github.com/spf13/pflag" +) + +const testV2Alias = "my-box.sb_abc.app-amika-dev.amika" + +// sshV2Harness drives the real sshv2 command with every outward call replaced, +// and records the argv that would have been handed to ssh. +type sshV2Harness struct { + argv []string + alias string + ran bool +} + +// sandboxSSHV2Cmd is a package-level singleton and New() may be called only +// once per process, so the tests hang a minimal amika/sandbox tree around the +// real command object instead, built once and reused. +var ( + sshV2TreeOnce sync.Once + sshV2Root *cobra.Command + sshV2Parent *cobra.Command +) + +func sshV2TestTree() (*cobra.Command, *cobra.Command) { + sshV2TreeOnce.Do(func() { + sshV2Root = &cobra.Command{Use: "amika", SilenceUsage: true, SilenceErrors: true} + sshV2Root.PersistentFlags().StringP("output", "o", "text", "output format") + sshV2Parent = &cobra.Command{Use: "sandbox"} + sshV2Parent.PersistentFlags().Bool("local", false, "Only operate on local sandboxes") + sshV2Parent.PersistentFlags().Bool("remote", false, "Only operate on remote sandboxes") + sshV2Parent.PersistentFlags().String("remote-target", "", "Operate on a specific named remote target") + sshV2Root.AddCommand(sshV2Parent) + }) + return sshV2Root, sshV2Parent +} + +// newSSHV2Harness wires the seams for one invocation and returns the shared +// tree, so Cobra's routing and flag inheritance are exercised rather than +// simulated. +func newSSHV2Harness(t *testing.T, procArgs []string) (*cobra.Command, *sshV2Harness, *bytes.Buffer) { + t.Helper() + h := &sshV2Harness{} + + // AMIKA_API_KEY short-circuits RequireAuth, so the command reaches the + // argv-building step without a login. + t.Setenv("AMIKA_API_KEY", "test-key") + + prevArgs, prevExec, prevClient, prevPrepare := osArgs, execSessionSSH, newSSHV2Client, prepareSessionTarget + osArgs = func() []string { return procArgs } + execSessionSSH = func(alias string, argv []string) error { + h.ran, h.alias, h.argv = true, alias, argv + return nil + } + newSSHV2Client = func(string) (sshV2Client, error) { + return &stubV2SSHClient{sandbox: &apiclient.RemoteSandbox{Name: "my-box", ID: "sb_abc"}}, nil + } + prepareSessionTarget = func(basedir.Paths, ssh.SessionCreator, string, string) (string, error) { + return testV2Alias, nil + } + t.Cleanup(func() { + osArgs, execSessionSSH, newSSHV2Client, prepareSessionTarget = prevArgs, prevExec, prevClient, prevPrepare + }) + + root, parent := sshV2TestTree() + // Another test in this package calls New(), which re-parents the shared + // command; reattach it here so the tree is whatever this test expects. + if sandboxSSHV2Cmd.Parent() != parent { + parent.AddCommand(sandboxSSHV2Cmd) + } + // Force Cobra's persistent-flag merge, then clear what a previous Execute + // left behind: Cobra never resets flag values or their Changed state. Both + // the command's parse and its reads go through Flags(), so resetting there + // is what the command actually observes. + _ = sandboxSSHV2Cmd.InheritedFlags() + for _, name := range []string{"local", "remote", "remote-target", "output"} { + resetSSHV2Flag(t, sandboxSSHV2Cmd.Flags().Lookup(name)) + } + + out := &bytes.Buffer{} + root.SetOut(out) + root.SetErr(out) + // procArgs carries the binary path at index 0; Cobra wants only the rest. + root.SetArgs(procArgs[1:]) + return root, h, out +} + +// resetSSHV2Flag restores a flag to its declared default and clears Changed. +func resetSSHV2Flag(t *testing.T, f *pflag.Flag) { + t.Helper() + if f == nil { + return + } + if err := f.Value.Set(f.DefValue); err != nil { + t.Fatalf("reset flag %q: %v", f.Name, err) + } + f.Changed = false +} + +func TestSSHV2ForwardsArgsToSSH(t *testing.T) { + tests := []struct { + name string + procArgs []string + wantArgv []string + }{ + { + name: "bare name becomes the alias", + procArgs: []string{"amika", "sandbox", "sshv2", "my-box"}, + wantArgv: []string{testV2Alias}, + }, + { + // The point of the change: -L reaches ssh ahead of the destination, + // where ssh reads it as a client option. + name: "local port forward", + procArgs: []string{"amika", "sandbox", "sshv2", "-N", "-L", "6789:localhost:3010", "my-box"}, + wantArgv: []string{"-N", "-L", "6789:localhost:3010", testV2Alias}, + }, + { + name: "dynamic port forward", + procArgs: []string{"amika", "sandbox", "sshv2", "-N", "-D", "1080", "my-box"}, + wantArgv: []string{"-N", "-D", "1080", testV2Alias}, + }, + { + name: "ssh config option with a separate value", + procArgs: []string{"amika", "sandbox", "sshv2", "-o", "BatchMode=yes", "my-box"}, + wantArgv: []string{"-o", "BatchMode=yes", testV2Alias}, + }, + { + // -t no longer belongs to amika; it passes through to ssh. + name: "force pty", + procArgs: []string{"amika", "sandbox", "sshv2", "-t", "my-box", "top"}, + wantArgv: []string{"-t", testV2Alias, "top"}, + }, + { + name: "remote command stays after the destination", + procArgs: []string{"amika", "sandbox", "sshv2", "my-box", "uptime"}, + wantArgv: []string{testV2Alias, "uptime"}, + }, + { + name: "options and a remote command", + procArgs: []string{"amika", "sandbox", "sshv2", "-L", "6789:localhost:3010", "my-box", "ls", "-la"}, + wantArgv: []string{"-L", "6789:localhost:3010", testV2Alias, "ls", "-la"}, + }, + { + // An amika flag before the subcommand is consumed by amika. + name: "sandbox flag before the subcommand is not forwarded", + procArgs: []string{"amika", "sandbox", "--remote", "sshv2", "-N", "my-box"}, + wantArgv: []string{"-N", testV2Alias}, + }, + { + // The same spelling after the subcommand belongs to ssh, which is + // what makes the rule positional rather than name-based. + name: "output flag after the subcommand is forwarded", + procArgs: []string{"amika", "sandbox", "sshv2", "-o", "SendEnv=FOO", "my-box"}, + wantArgv: []string{"-o", "SendEnv=FOO", testV2Alias}, + }, + { + name: "end of options marker before the name", + procArgs: []string{"amika", "sandbox", "sshv2", "--", "my-box"}, + wantArgv: []string{"--", testV2Alias}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + root, h, out := newSSHV2Harness(t, tt.procArgs) + if err := root.Execute(); err != nil { + t.Fatalf("Execute: %v (output %q)", err, out.String()) + } + if !h.ran { + t.Fatal("ssh was never invoked") + } + if h.alias != testV2Alias { + t.Errorf("alias = %q, want %q", h.alias, testV2Alias) + } + if !reflect.DeepEqual(h.argv, tt.wantArgv) { + t.Errorf("ssh argv = %#v, want %#v", h.argv, tt.wantArgv) + } + }) + } +} + +func TestSSHV2AmikaFlagsBeforeSubcommand(t *testing.T) { + t.Run("output flag before the subcommand is rejected, not forwarded", func(t *testing.T) { + root, h, _ := newSSHV2Harness(t, []string{"amika", "--output", "json", "sandbox", "sshv2", "my-box"}) + err := root.Execute() + if err == nil { + t.Fatal("expected --output to be rejected") + } + if h.ran { + t.Errorf("ssh ran with argv %#v; --output should never reach it", h.argv) + } + }) + + t.Run("local flag before the subcommand is honored", func(t *testing.T) { + root, h, _ := newSSHV2Harness(t, []string{"amika", "sandbox", "--local", "sshv2", "my-box"}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "requires a remote sandbox") { + t.Fatalf("err = %v, want a remote-sandbox error", err) + } + if h.ran { + t.Error("ssh should not run for a local sandbox") + } + }) +} + +func TestSSHV2MissingName(t *testing.T) { + root, h, _ := newSSHV2Harness(t, []string{"amika", "sandbox", "sshv2", "-N", "-L", "6789:localhost:3010"}) + err := root.Execute() + if err == nil || !strings.Contains(err.Error(), "missing sandbox name") { + t.Fatalf("err = %v, want a missing-name error", err) + } + if h.ran { + t.Error("ssh should not run without a sandbox name") + } +} + +func TestSSHV2Help(t *testing.T) { + // --help is the documented exception to the forwarding rule, and the help + // subcommand must reach the same text. + for _, tt := range []struct { + name string + procArgs []string + }{ + {name: "help flag after the subcommand", procArgs: []string{"amika", "sandbox", "sshv2", "--help"}}, + {name: "short help flag", procArgs: []string{"amika", "sandbox", "sshv2", "-h"}}, + {name: "help subcommand", procArgs: []string{"amika", "help", "sandbox", "sshv2"}}, + } { + t.Run(tt.name, func(t *testing.T) { + root, h, out := newSSHV2Harness(t, tt.procArgs) + if err := root.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + if h.ran { + t.Errorf("ssh ran with argv %#v; --help must not reach it", h.argv) + } + got := out.String() + for _, want := range []string{ + "amika sandbox sshv2", + "Use it like ssh", + "-L 6789:localhost:3010", + } { + if !strings.Contains(got, want) { + t.Errorf("help output missing %q; got:\n%s", want, got) + } + } + }) + } +} diff --git a/go/internal/cliargs/cliargs.go b/go/internal/cliargs/cliargs.go new file mode 100644 index 00000000..a7b75a81 --- /dev/null +++ b/go/internal/cliargs/cliargs.go @@ -0,0 +1,128 @@ +// Package cliargs splits a command line between the arguments amika interprets +// itself and the arguments it forwards verbatim to an underlying utility such +// as ssh or scp. +// +// The split is positional, mirroring the underlying utility: arguments written +// before the subcommand name belong to amika, arguments written after it are +// forwarded. That distinction is not recoverable from the arguments Cobra hands +// a command, because Cobra removes the command path from the argv first. Both +// +// amika --output json sandbox sshv2 my-box +// amika sandbox sshv2 --output json my-box +// +// reach the sshv2 command as the identical slice +// +// ["--output", "json", "my-box"] +// +// so Split needs the original process argv, where the "sshv2" token still marks +// the boundary. Callers pass it in rather than reading os.Args here, so tests +// can exercise the split with a synthetic argv. +package cliargs + +import "strings" + +// SSHArgLetters are ssh's single-letter options that take their argument as a +// separate token (from ssh(1)). Locating an operand requires them: without +// them the "6789:localhost:3010" in "-L 6789:localhost:3010 my-box" would be +// mistaken for the destination. +const SSHArgLetters = "BbcDEeFIiJLlmOopQRSWw" + +// Split divides the arguments Cobra delivered to a leaf command into the +// amika-owned prefix and the suffix to forward to the underlying utility. +// +// cmdName is the leaf command's name as written on the command line, and +// valueFlags lists the amika flags that take their value as a separate token, +// so a value that happens to equal cmdName is not mistaken for the command +// itself. If cmdName does not appear in procArgs, nothing can be attributed by +// position and every argument is treated as amika's. +func Split(procArgs, leafArgs []string, cmdName string, valueFlags map[string]bool) (own, forward []string) { + idx := commandIndex(procArgs, cmdName, valueFlags) + if idx < 0 { + return leafArgs, nil + } + // Every token after the command name survives in leafArgs as a contiguous + // suffix: Cobra strips only the command path, which lies entirely before + // that point. So the count of trailing tokens locates the boundary without + // having to match tokens between the two slices. + n := len(procArgs) - (idx + 1) + if n > len(leafArgs) { + n = len(leafArgs) + } + return leafArgs[:len(leafArgs)-n], leafArgs[len(leafArgs)-n:] +} + +// commandIndex returns the position of the leaf command's name in the process +// argv, or -1. Element 0 is the binary path and is never considered. +func commandIndex(procArgs []string, cmdName string, valueFlags map[string]bool) int { + for i := 1; i < len(procArgs); i++ { + tok := procArgs[i] + if tok == cmdName { + return i + } + if valueFlags[tok] { + i++ // skip the flag's value, which may itself read as a command name + } + } + return -1 +} + +// FirstOperand returns the index of the first operand in a getopt-style argv: +// the first token that is neither an option nor an option's value. It returns +// -1 when the argv holds no operand. argLetters lists the utility's +// single-letter options that take a separate argument. +func FirstOperand(args []string, argLetters string) int { + for i := 0; i < len(args); i++ { + tok := args[i] + if tok == "--" { + // End of options: the next token is an operand, whatever it looks + // like. This mirrors "ssh -- -weird-host". + if i+1 < len(args) { + return i + 1 + } + return -1 + } + if len(tok) >= 2 && tok[0] == '-' { + if ConsumesNextArg(tok, argLetters) && i+1 < len(args) { + i++ + } + continue + } + // A bare "-" is an operand, as is any token not starting with "-". + return i + } + return -1 +} + +// ConsumesNextArg reports whether an option token takes the following argv +// token as its argument rather than an attached value. It mirrors getopt: in a +// bundled cluster such as "-Nl" only the first argument-taking letter takes a +// value, and it takes the following token only when nothing is attached after +// it ("-o" takes the next token, while "-oVALUE" and "-Nl root" do not). +func ConsumesNextArg(tok, argLetters string) bool { + if len(tok) < 2 || tok[0] != '-' || tok[1] == '-' { + return false // an operand, a bare "-", or a "--" end-of-options marker + } + for i := 1; i < len(tok); i++ { + if strings.IndexByte(argLetters, tok[i]) >= 0 { + return i == len(tok)-1 + } + } + return false +} + +// HasHelpFlag reports whether args requests help. Only a help flag written +// among the leading options counts: once the first operand or a "--" appears, +// later tokens belong to the underlying utility (for ssh, they form the remote +// command), so "sshv2 my-box --help" asks the sandbox for help rather than +// amika. +func HasHelpFlag(args []string) bool { + for _, a := range args { + if len(a) == 0 || a[0] != '-' || a == "--" || a == "-" { + return false // first operand or end-of-options marker + } + if a == "-h" || a == "--help" { + return true + } + } + return false +} diff --git a/go/internal/cliargs/cliargs_test.go b/go/internal/cliargs/cliargs_test.go new file mode 100644 index 00000000..7450f2e7 --- /dev/null +++ b/go/internal/cliargs/cliargs_test.go @@ -0,0 +1,159 @@ +package cliargs + +import ( + "reflect" + "testing" +) + +// amikaValueFlags mirrors the flags sshv2 passes to Split. +var amikaValueFlags = map[string]bool{"--output": true, "-o": true, "--remote-target": true} + +func TestSplit(t *testing.T) { + tests := []struct { + name string + procArgs []string + leafArgs []string + wantOwn []string + wantForward []string + }{ + { + name: "no flags", + procArgs: []string{"amika", "sandbox", "sshv2", "my-box"}, + leafArgs: []string{"my-box"}, + wantForward: []string{"my-box"}, + }, + { + // The pair of cases the split exists for: identical leafArgs, + // opposite meanings, told apart only by the process argv. + name: "amika flag before the subcommand is not forwarded", + procArgs: []string{"amika", "--output", "json", "sandbox", "sshv2", "my-box"}, + leafArgs: []string{"--output", "json", "my-box"}, + wantOwn: []string{"--output", "json"}, + wantForward: []string{"my-box"}, + }, + { + name: "same flag after the subcommand is forwarded", + procArgs: []string{"amika", "sandbox", "sshv2", "--output", "json", "my-box"}, + leafArgs: []string{"--output", "json", "my-box"}, + wantForward: []string{"--output", "json", "my-box"}, + }, + { + name: "sandbox flag before the subcommand", + procArgs: []string{"amika", "sandbox", "--remote", "sshv2", "-L", "6789:localhost:3010", "my-box"}, + leafArgs: []string{"--remote", "-L", "6789:localhost:3010", "my-box"}, + wantOwn: []string{"--remote"}, + wantForward: []string{"-L", "6789:localhost:3010", "my-box"}, + }, + { + name: "value-taking amika flag before the subcommand", + procArgs: []string{"amika", "sandbox", "--remote-target", "prod", "sshv2", "-N", "my-box"}, + leafArgs: []string{"--remote-target", "prod", "-N", "my-box"}, + wantOwn: []string{"--remote-target", "prod"}, + wantForward: []string{"-N", "my-box"}, + }, + { + // A flag value equal to the subcommand name must not be mistaken + // for the subcommand and shift the boundary. + name: "flag value equal to the subcommand name", + procArgs: []string{"amika", "sandbox", "--remote-target", "sshv2", "sshv2", "my-box"}, + leafArgs: []string{"--remote-target", "sshv2", "my-box"}, + wantOwn: []string{"--remote-target", "sshv2"}, + wantForward: []string{"my-box"}, + }, + { + name: "remote command after the name is forwarded", + procArgs: []string{"amika", "sandbox", "sshv2", "my-box", "uptime"}, + leafArgs: []string{"my-box", "uptime"}, + wantForward: []string{"my-box", "uptime"}, + }, + { + // Without the subcommand in the argv nothing can be placed, so + // everything stays with amika rather than leaking to the utility. + name: "subcommand absent from the process argv", + procArgs: []string{"amika", "sandbox"}, + leafArgs: []string{"my-box"}, + wantOwn: []string{"my-box"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + own, forward := Split(tt.procArgs, tt.leafArgs, "sshv2", amikaValueFlags) + if len(own) != len(tt.wantOwn) || (len(own) > 0 && !reflect.DeepEqual(own, tt.wantOwn)) { + t.Errorf("own = %#v, want %#v", own, tt.wantOwn) + } + if len(forward) != len(tt.wantForward) || (len(forward) > 0 && !reflect.DeepEqual(forward, tt.wantForward)) { + t.Errorf("forward = %#v, want %#v", forward, tt.wantForward) + } + }) + } +} + +func TestFirstOperand(t *testing.T) { + tests := []struct { + name string + args []string + want int + }{ + {name: "bare name", args: []string{"my-box"}, want: 0}, + {name: "boolean option first", args: []string{"-N", "my-box"}, want: 1}, + { + // The forwarding spec is -L's value, not the destination. + name: "option with a separate value", + args: []string{"-L", "6789:localhost:3010", "my-box"}, + want: 2, + }, + {name: "option with an attached value", args: []string{"-L6789:localhost:3010", "my-box"}, want: 1}, + {name: "bundled booleans", args: []string{"-tv", "my-box"}, want: 1}, + { + // Only the first argument-taking letter in a cluster takes the + // following token, and only when nothing is attached after it. + name: "bundled boolean then value-taking letter", + args: []string{"-Nl", "root", "my-box"}, + want: 2, + }, + {name: "ssh config option", args: []string{"-o", "BatchMode=yes", "my-box"}, want: 2}, + {name: "end of options marker", args: []string{"--", "-weird-name"}, want: 1}, + {name: "options only", args: []string{"-N", "-L", "6789:localhost:3010"}, want: -1}, + {name: "empty", args: nil, want: -1}, + {name: "trailing marker", args: []string{"--"}, want: -1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := FirstOperand(tt.args, SSHArgLetters); got != tt.want { + t.Errorf("FirstOperand(%#v) = %d, want %d", tt.args, got, tt.want) + } + }) + } +} + +func TestHasHelpFlag(t *testing.T) { + tests := []struct { + name string + args []string + want bool + }{ + {name: "long form", args: []string{"--help"}, want: true}, + {name: "short form", args: []string{"-h"}, want: true}, + {name: "after another option", args: []string{"-N", "--help"}, want: true}, + { + // Past the sandbox name the tokens are ssh's remote command, so + // --help is meant for the far side. + name: "after the operand", + args: []string{"my-box", "--help"}, + want: false, + }, + {name: "after end of options", args: []string{"--", "--help"}, want: false}, + {name: "absent", args: []string{"-N", "my-box"}, want: false}, + {name: "empty", args: nil, want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := HasHelpFlag(tt.args); got != tt.want { + t.Errorf("HasHelpFlag(%#v) = %v, want %v", tt.args, got, tt.want) + } + }) + } +} diff --git a/go/internal/ssh/ssh.go b/go/internal/ssh/ssh.go index eaede3ca..0dc2e76e 100644 --- a/go/internal/ssh/ssh.go +++ b/go/internal/ssh/ssh.go @@ -76,20 +76,31 @@ func DispatchSSH(client *apiclient.Client, name string, extraArgs []string, stdo return cmd.Run() } +// BuildSessionSSHArgv assembles the argv for the system ssh binary from the +// arguments forwarded to it, swapping the sandbox name at nameIdx for the +// managed v2 alias. Substituting in place rather than prepending the alias +// preserves ssh's own grammar, "ssh [options] destination [command]": options +// written before the sandbox name stay before the destination, where ssh reads +// them as client options, and anything after it stays after, where ssh reads it +// as the remote command. +func BuildSessionSSHArgv(forward []string, nameIdx int, alias string) []string { + argv := make([]string, 0, len(forward)) + argv = append(argv, forward[:nameIdx]...) + argv = append(argv, alias) + argv = append(argv, forward[nameIdx+1:]...) + return argv +} + // ExecSessionSSH replaces the current process with OpenSSH targeting a strict -// v2 alias whose ProxyCommand fetches a fresh session per dial. -func ExecSessionSSH(alias string, forcePTY bool, extraArgs []string) error { +// v2 alias whose ProxyCommand fetches a fresh session per dial. argv is the +// complete ssh argument list, as built by BuildSessionSSHArgv. +func ExecSessionSSH(alias string, argv []string) error { if _, err := ParseSessionAlias(alias); err != nil { return err } - sshArgs := []string{alias} - if forcePTY { - sshArgs = append([]string{"-t"}, sshArgs...) - } - sshArgs = append(sshArgs, extraArgs...) sshBin, err := exec.LookPath("ssh") if err != nil { return fmt.Errorf("ssh not found: %w", err) } - return syscall.Exec(sshBin, append([]string{"ssh"}, sshArgs...), os.Environ()) + return syscall.Exec(sshBin, append([]string{"ssh"}, argv...), os.Environ()) }