Pass sshv2 arguments through to ssh - #328
Conversation
Cobra removes the command path from an argv before handing it to a command, so `amika --output json sandbox sshv2 box` and `amika sandbox sshv2 --output json box` arrive identically as `[--output json box]`. Telling the two apart requires the original process argv, where the subcommand token still marks the boundary between the arguments amika interprets and the arguments it forwards to an underlying utility. `cliargs` recovers that boundary, and adds the getopt helpers needed to find an operand in an argv bound for ssh or scp so that a value such as the `6789:localhost:3010` in `-L 6789:localhost:3010 box` is not mistaken for the destination.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2224480bfe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // 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" |
There was a problem hiding this comment.
-P as an SSH option that consumes a value
When users pass OpenSSH's -P tag option, FirstOperand treats the tag as the sandbox name because P is absent from this set. For example, amika sandbox sshv2 -P deployment my-box queries the API for sandbox deployment instead of my-box and never invokes SSH. I confirmed against OpenSSH 9.6's ssh -h usage, which lists [-P tag]; include P among the value-taking option letters.
Useful? React with 👍 / 👎.
| for _, a := range args { | ||
| if len(a) == 0 || a[0] != '-' || a == "--" || a == "-" { | ||
| return false // first operand or end-of-options marker |
There was a problem hiding this comment.
When --help follows any option with a separate non-dash value, this loop mistakes that value for the destination and stops before reaching the help flag. Thus amika sandbox sshv2 -L 6789:localhost:3010 --help returns “missing sandbox name” instead of the command help, even though -L's value is not an operand. Account for value-taking SSH options while scanning the leading option region.
Useful? React with 👍 / 👎.
`amika sandbox sshv2` parsed its arguments with Cobra, which rejected ssh's own options before ssh ever saw them. Asking for a port forward with `sshv2 my-box -L 6789:localhost:3010` failed outright with `unknown shorthand flag: 'L' in -L`. The only way through was to smuggle the options past Cobra behind a `--`, which is neither discoverable nor documented. Arguments written after `sshv2` are now handed to the system ssh binary unchanged, and the sandbox name takes ssh's destination slot, so ssh's options work as they do with ssh itself. Amika's own flags go before the subcommand, which is where the argv split attributes them. `--help` is the exception and still prints amika's help, as does `amika help sandbox sshv2`. The `-t` flag is dropped, since ssh's own `-t` now reaches ssh directly rather than being shadowed by a Cobra flag of the same name. Existing invocations keep working, including the `--` form: ssh consumes the marker itself, so `sshv2 my-box -- uptime` still runs `uptime`. `amika scpv2` already forwarded its arguments to scp and is unchanged.
The help block explained how arguments reach ssh rather than how to use the command. Describe the grammar and the flag placement a caller needs, and drop the walkthrough of which token goes where.
2387ad5 to
ba326a0
Compare
1 participant
amika sandbox sshv2now mirrors the underlying ssh command: every argument written aftersshv2is handed to the system ssh binary unchanged, and the sandbox name takes ssh's destination slot.Previously those arguments were parsed by Cobra, which rejected ssh's own options before ssh ever saw them. Asking for a port forward failed outright:
The only way through was to smuggle the options past Cobra behind a
--, which is neither discoverable nor documented. They now work directly:Because arguments after the subcommand belong to ssh, amika's own flags go before it:
Cobra strips the command path before a command sees its arguments, so both placements of a flag arrive identically.
internal/cliargsrecovers the boundary from the process argv, which is what makes the rule positional.--helpis the exception and still prints amika's help, as doesamika help sandbox sshv2. The command defines no flags of its own, so ssh's-treaches ssh directly rather than being shadowed by a Cobra flag of the same name.Local (
-L) and dynamic (-D) forwarding are supported by the sandbox's SSH server. Remote forwarding (-R), agent forwarding (-A), and X11 forwarding are refused server-side.amika scpv2already forwarded its arguments to scp and is unchanged.Compatibility
Existing invocations keep working, including the
--form: ssh consumes the marker itself, soamika sandbox sshv2 my-box -- uptimestill runsuptime.Verification
Tests assert the exact argv the command hands to ssh, driving the real Cobra command through a seam over the exec, and cover port forwards,
-o, remote commands, flag placement on both sides of the subcommand, a missing sandbox name, and all three help paths.ssh's own argument grammar was confirmed against a local sshd echoing
$SSH_ORIGINAL_COMMAND: options are accepted before and after the destination, the first non-option token after the destination begins the remote command, and--is consumed by ssh rather than forwarded.make cifails only ininternal/amikad,internal/amikad/sshd, andinternal/amikad/state, which fail identically on a clean checkout ofmainon macOS ("amikad sensitive writes require Linux", "sensitive file path contains a symlink").