From 773a67dca564fff1baca1884d3780261b12171fa Mon Sep 17 00:00:00 2001 From: Pierre Rouanet Date: Thu, 10 Sep 2026 17:27:12 +0200 Subject: [PATCH] duckctl: scp, with `:path` for the robot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `duckctl ssh` already resolves the robot's address and hands the terminal over; copying a file off a robot still meant `scp radxa@$(duckctl ip):…`, which is the substitution `ssh` exists to remove. The robot side is `scp`'s own `host:path` with the host left out, so `duckctl scp report.md :/tmp/` sends one up and `duckctl scp :/var/log/robotd.log .` brings one down. The rewrite is textual and only ever looks at the first character, which is what keeps `-r`, `-P 2222`, several sources and a path with a colon in the middle of it working without this tool having to understand any of them. Both directions, then, out of one command that answers to the name people reach for — rather than a `push` and a `pull` that nobody typing `scp` would find. A copy with no `:` anywhere in it is refused before the radio is turned on. `scp` performs that copy happily, locally, and nothing in its output says the robot was never involved; refusing it after the scan would also charge eight seconds for the diagnosis. The `exec` that makes `ssh` become `ssh` is now `become_program`, shared by both, so the progress meter, the key prompts and the exit status are `scp`'s own for the same reason they were ssh's. Co-Authored-By: Claude Opus 5 (1M context) --- docs/robot/dev-push.md | 2 +- docs/robot/duckctl.md | 25 +++++ duckctl/src/main.rs | 216 +++++++++++++++++++++++++++++++++++------ 3 files changed, 213 insertions(+), 30 deletions(-) diff --git a/docs/robot/dev-push.md b/docs/robot/dev-push.md index 074929b1..7f552b22 100644 --- a/docs/robot/dev-push.md +++ b/docs/robot/dev-push.md @@ -333,7 +333,7 @@ This should not have been necessary, so it is worth reading the journal for why | | | |---|---| | `DUCK_ROBOT` | The robot, by name. Its address is found over Bluetooth and cached. | -| `DUCK_BOARD_USER` | The ssh user on the board, for the name path. Default `radxa`. `duckctl ssh` reads it too. | +| `DUCK_BOARD_USER` | The ssh user on the board, for the name path. Default `radxa`. `duckctl ssh` and `duckctl scp` read it too. | | `DUCK_PIN` | The robot's pairing PIN, if it is not the factory `000000`. Read by `duckctl`. | | `DUCK_BOARD_CACHE` | Where resolved addresses are cached. Default `~/.cache/duck/boards`. | | `DUCK_BOARD` | The board, by address, instead of an argument. `radxa@192.168.1.42`. | diff --git a/docs/robot/duckctl.md b/docs/robot/duckctl.md index 8daa9f5c..0e4cccc2 100644 --- a/docs/robot/duckctl.md +++ b/docs/robot/duckctl.md @@ -89,6 +89,31 @@ the exit status are ssh's own. The account is `--user`, else `DUCK_BOARD_USER` f — the variable [`dev-push.sh`](dev-push.md) reads, so a laptop set up for pushing is set up for this — else `radxa`. Words after `--` run on the robot instead of opening a shell. +Files go the same way: + +```bash +duckctl scp report.md :/tmp/ +``` + +```bash +duckctl scp :/var/log/robotd.log . +``` + +A path starting with `:` is on the robot — `scp`'s own `host:path` with the host left out, since the +host is the part this finds for you. Everything else reaches `scp` as typed, `-r` and the rest of +its flags included, and the progress meter and the exit status are `scp`'s own. The account resolves +the way `ssh`'s does. + +This tool's own flags come first, before the paths: + +```bash +duckctl --name ducky scp -r logs/ :/tmp/ +``` + +A copy with no `:` anywhere in it is refused before the scan, because it is a local-to-local copy +that no robot is party to and nothing in `scp`'s output would say so. A local file that really is +named `:foo` is `./:foo`. + A robot bonded to this machine often stops advertising the service to it, and then `ip` connects and asks `net.status` instead. That is slower and needs the PIN, and it always answers. `--verbose` says which of the two happened. diff --git a/duckctl/src/main.rs b/duckctl/src/main.rs index b0b57ee9..c5cfd98e 100644 --- a/duckctl/src/main.rs +++ b/duckctl/src/main.rs @@ -430,27 +430,11 @@ fn deliver(command: &Command, address: &str) -> Result<(), Box { let user = ssh_user(user.as_deref(), std::env::var("DUCK_BOARD_USER").ok()); - let argv = ssh_argv(&user, address, command); - eprintln!("ssh {}", argv.join(" ")); - let mut ssh = std::process::Command::new("ssh"); - ssh.args(&argv); - // Become ssh rather than run it: the terminal is then ssh's from here on — its - // prompts, its exit status, its handling of a dropped link — and nothing of this - // process is left behind to be `Ctrl-C`d separately. The radio was released above, - // before this was called, so there is nothing to clean up. - #[cfg(unix)] - { - use std::os::unix::process::CommandExt; - let e = ssh.exec(); - Err(format!("could not run ssh: {e}").into()) - } - #[cfg(not(unix))] - { - let status = ssh - .status() - .map_err(|e| format!("could not run ssh: {e}"))?; - std::process::exit(status.code().unwrap_or(1)); - } + become_program("ssh", &ssh_argv(&user, address, command)) + } + Command::Scp { user, paths } => { + let user = ssh_user(user.as_deref(), std::env::var("DUCK_BOARD_USER").ok()); + become_program("scp", &scp_argv(&user, address, paths)) } Command::Open { print, port } => { let url = console_url(address, *port); @@ -467,12 +451,13 @@ fn deliver(command: &Command, address: &str) -> Result<(), Box Err("this command does not resolve an address".into()), } } -/// Which account `ssh` logs into: the flag, else a non-empty `DUCK_BOARD_USER`, else `radxa`. +/// Which account `ssh` and `scp` log into: the flag, else a non-empty `DUCK_BOARD_USER`, else +/// `radxa`. /// /// Empty is unset — `DUCK_BOARD_USER= duckctl ssh` reads as "not set", the same rule `DUCK_ROBOT` /// follows, because a variable emptied to switch it off must not become an ssh login of `@host`. @@ -494,6 +479,76 @@ fn ssh_argv(user: &str, address: &str, command: &[String]) -> Vec { argv } +/// What `scp` is refused for, checked **before** the radio is turned on. +/// +/// Two of them, and neither could be left to `scp`: its usage error cannot mention the rule that +/// is actually being broken. A copy with no `:` anywhere in it is the one worth catching — it is a +/// local-to-local copy, `scp` performs it happily, and nothing in the output says the robot was +/// never involved. Here rather than in `scp_argv` because the alternative is charging eight +/// seconds of scanning for a robot the command was never going to touch. +fn scp_refusal(paths: &[String]) -> Result<(), String> { + if !paths.iter().any(|path| path.starts_with(':')) { + return Err(format!( + "no path on the robot in `{}`\nA leading `:` is the robot: `duckctl scp report.md \ + :/tmp/` sends a file up, `duckctl scp :/var/log/robotd.log .` brings one down. \ + Without one this is a local-to-local copy that has nothing to do with a robot.", + paths.join(" "), + )); + } + if paths.len() < 2 { + return Err(format!( + "`{}` is a source with no destination. `scp` takes both: `duckctl scp \ + :/var/log/robotd.log .`", + paths[0], + )); + } + Ok(()) +} + +/// `scp`'s arguments: every `:path` pointed at the robot, everything else as typed. +/// +/// The rewrite is textual and deliberately narrow — a leading `:` becomes `user@address:` and +/// nothing else is touched — so `-r`, `-P`, a local path, and a path with a colon in the middle of +/// it all reach `scp` exactly as they were typed. `scp` itself decides what they mean. +fn scp_argv(user: &str, address: &str, paths: &[String]) -> Vec { + paths + .iter() + .map(|path| match path.strip_prefix(':') { + Some(remote) => format!("{user}@{address}:{remote}"), + None => path.clone(), + }) + .collect() +} + +/// Hand the terminal to `ssh` or `scp` and never come back. +/// +/// Become the program rather than run it: the terminal is then its from here on — its prompts, +/// its progress meter, its exit status, its handling of a dropped link — and nothing of this +/// process is left behind to be `Ctrl-C`d separately. The radio was released before this was +/// called, so there is nothing to clean up. +/// +/// The command line is echoed first, on stderr, because a tool that resolved the address for you +/// still owes you the address it resolved — and it is the line to paste when the next copy needs +/// a flag this does not pass. +fn become_program(program: &str, argv: &[String]) -> Result<(), Box> { + eprintln!("{program} {}", argv.join(" ")); + let mut child = std::process::Command::new(program); + child.args(argv); + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + let e = child.exec(); + Err(format!("could not run {program}: {e}").into()) + } + #[cfg(not(unix))] + { + let status = child + .status() + .map_err(|e| format!("could not run {program}: {e}"))?; + std::process::exit(status.code().unwrap_or(1)); + } +} + /// Where the console is, given where the robot is. /// /// Plain `http`, because a robot on a LAN has no certificate to offer and `ws://` from an `https` @@ -869,6 +924,33 @@ enum Command { )] command: Vec, }, + /// Copy files to or from the robot with `scp`. + /// + /// A path that starts with `:` is on the robot: `duckctl scp report.md :/tmp/` sends one up, + /// `duckctl scp :/var/log/robotd.log .` brings one down. That is `scp`'s own `host:path` with + /// the host left out, because the host is the thing this tool exists to find. Everything else + /// — local paths, `-r`, any other `scp` flag — is passed through as typed, and then this + /// process *becomes* `scp`, so the progress meter, the key prompts and the exit status are + /// `scp`'s own. + /// + /// `duckctl`'s own flags come before the paths — `duckctl --name ducky scp -r logs/ :/tmp/` — + /// and `--` ends them for anything after that this tool would otherwise read as its own. A + /// local file that really is named `:foo` is `./:foo`. + /// + /// The user resolves the way `ssh`'s does: `--user`, else `DUCK_BOARD_USER`, else `radxa`. + Scp { + /// The account on the robot. Without it, `DUCK_BOARD_USER`; without that, `radxa`. + #[arg(long, value_name = "USER")] + user: Option, + /// What to copy, `scp`-style, with a leading `:` for a path on the robot. + #[arg( + trailing_var_arg = true, + allow_hyphen_values = true, + required = true, + value_name = "PATH" + )] + paths: Vec, + }, /// Open the robot's console in a browser. /// /// The page `mediad` serves: the camera, and the controls a WebRTC peer is allowed to drive. @@ -1261,18 +1343,25 @@ async fn run() -> Result<(), Box> { let target = Target::new(cli.name.clone(), std::env::var("DUCK_ROBOT").ok()); let pin = resolve_pin(cli.pin.clone(), std::env::var("DUCK_PIN").ok()); + // Before the scan rather than after it: a copy that names no path on the robot is wrong on its + // own terms, and finding the robot first would charge eight seconds for the privilege. + if let Command::Scp { paths, .. } = &cli.command { + scp_refusal(paths)?; + } + // `scan` shares the discovery below and then stops, because a listing and a search look for the // same thing and differ only in what they do with it. It connects to nothing at all: that is // what makes it the safe command to reach for when a robot cannot be reached, and it is also why // it can only report what an advertisement carries. let list_only = matches!(cli.command, Command::Scan); - // `ip`, `open` and `ssh` want one field out of an advertisement, so they read it the way `scan` does — - // and unlike `scan` they connect after all when no advertisement carried one. Cheap read first, - // call second: without the fallback these two commands would fail on exactly the laptops that - // use them most, because a robot bonded to this Mac often stops advertising the service to it. + // `ip`, `open`, `ssh` and `scp` want one field out of an advertisement, so they read it the way + // `scan` does — and unlike `scan` they connect after all when no advertisement carried one. + // Cheap read first, call second: without the fallback these commands would fail on exactly the + // laptops that use them most, because a robot bonded to this Mac often stops advertising the + // service to it. let resolving = matches!( cli.command, - Command::Ip | Command::Open { .. } | Command::Ssh { .. } + Command::Ip | Command::Open { .. } | Command::Ssh { .. } | Command::Scp { .. } ); let manager = Manager::new().await?; @@ -1940,7 +2029,7 @@ fn request_line(command: &Command) -> Result<(String, Duration), Box { + Command::Ip | Command::Open { .. } | Command::Ssh { .. } | Command::Scp { .. } => { ("net.status", serde_json::json!({}), REPLY_TIMEOUT) } Command::Version => ( @@ -2858,6 +2947,75 @@ mod tests { assert_eq!(ssh_user(None, None), "radxa"); } + /// A leading `:` is the robot, in either operand, and everything else reaches `scp` as typed. + #[test] + fn scp_points_colon_paths_at_the_robot_and_leaves_the_rest_alone() { + let up = scp_argv("radxa", "192.168.10.136", &paths(&["report.md", ":/tmp/"])); + assert_eq!(up, ["report.md", "radxa@192.168.10.136:/tmp/"]); + + let down = scp_argv( + "pierre", + "192.168.10.136", + &paths(&[":/var/log/robotd.log", "."]), + ); + assert_eq!(down, ["pierre@192.168.10.136:/var/log/robotd.log", "."]); + + // Flags, several sources, and a bare `:` for the home directory — all of it passes + // through, because the rewrite only ever looks at the first character. + let many = scp_argv("radxa", "192.168.10.136", &paths(&["-r", "a", "b:c", ":"])); + assert_eq!(many, ["-r", "a", "b:c", "radxa@192.168.10.136:"]); + } + + /// The two refusals `scp`'s own usage error could not have explained: a copy that names no + /// path on the robot, and a source with nothing to copy it to. + #[test] + fn scp_refuses_a_copy_the_robot_has_nothing_to_do_with() { + let local = scp_refusal(&paths(&["a", "b"])).expect_err("neither side is the robot"); + assert!(local.contains("leading `:`"), "names the rule: {local}"); + assert!(local.contains("duckctl scp"), "shows the shape: {local}"); + + let lonely = scp_refusal(&paths(&[":/tmp/x"])).expect_err("a source with no destination"); + assert!( + lonely.contains("destination"), + "says what is missing: {lonely}" + ); + + scp_refusal(&paths(&["report.md", ":/tmp/"])).expect("a copy that touches the robot"); + scp_refusal(&paths(&["-r", ":/tmp/logs", "."])).expect("a flag is not an operand"); + } + + /// `scp` takes the same `--user` as `ssh`, its paths are trailing, and it needs at least one. + #[test] + fn scp_parses_its_user_and_requires_a_path() { + let cli = Cli::try_parse_from(["duckctl", "scp", "--user", "pierre", "x", ":/tmp/"]) + .expect("parses"); + let Command::Scp { user, paths } = &cli.command else { + panic!("not an scp command"); + }; + assert_eq!(user.as_deref(), Some("pierre")); + assert_eq!(paths, &["x".to_owned(), ":/tmp/".to_owned()]); + + // `--` ends `duckctl`'s flags, so an `scp` flag of the same shape is not mistaken for one. + let cli = Cli::try_parse_from(["duckctl", "scp", "--", "-r", "logs/", ":/tmp/"]) + .expect("a flag for scp parses after `--`"); + let Command::Scp { paths, .. } = &cli.command else { + panic!("not an scp command"); + }; + assert_eq!( + paths, + &["-r".to_owned(), "logs/".to_owned(), ":/tmp/".to_owned()] + ); + + assert!( + Cli::try_parse_from(["duckctl", "scp"]).is_err(), + "scp with nothing to copy" + ); + } + + fn paths(paths: &[&str]) -> Vec { + paths.iter().map(|path| (*path).to_owned()).collect() + } + #[test] fn a_robot_broadcasts_where_it_is() { let (properties, duck) = advertised(