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(