From 6976c9e8a46daaa0ed02a7b072f880bfc9c3eafe Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 29 Oct 2026 16:10:23 -0600 Subject: [PATCH 1/7] Gather the command tests into their module Co-Authored-By: Claude Opus 5.5 Co-authored-by: Claude --- client/src/commands.rs | 263 ++++++++++++++++++++--------------------- 1 file changed, 131 insertions(+), 132 deletions(-) diff --git a/client/src/commands.rs b/client/src/commands.rs index 92d341a7..24a36b46 100644 --- a/client/src/commands.rs +++ b/client/src/commands.rs @@ -255,138 +255,6 @@ impl ClientArgs { } } -/// A rack-wide target can't combine with `--binary` or `--file`. -#[tokio::test] -async fn output_needs_target() { - use crate::cli::Cli; - - let client = - Client::new_with_client("http://[::1]:1", reqwest::Client::new(), Default::default()); - let output = JobOutput::try_parse_from([ - "job-stdout", - "--binary", - "sea-say-sting-palm-tunnel-festival-pull-bid", - ]) - .unwrap(); - let err = job_output( - &mut Cli::default(), - &client, - &"*".parse::().unwrap(), - Stdout, - output, - ) - .await - .unwrap_err(); - assert!(matches!(err, CommandError::OutputNeedsTarget)); -} - -/// The watch settles only after the sled set is stable and every sled -/// is terminal for consecutive polls. -#[test] -fn watch_settling() { - use chrono::Utc; - - fn sled(serial: &str) -> BaseboardId { - BaseboardId { - part_number: "913-0000019".to_owned(), - serial_number: serial.to_owned(), - } - } - let job_id: JobId = "sea-say-sting-palm-tunnel-festival-pull-bid" - .parse() - .unwrap(); - let terminal = JobStatus::Cancelled { - job_id, - time_cancelled: Utc::now(), - actor: KeyId::random(), - }; - let running = JobStatus::Started { - job_id, - time_started: Utc::now(), - }; - - let mut settling = Settling::default(); - let mut status = JobStatusMap::new(); - assert!(!settling.done(None, None, &status), "empty never settles"); - - status.insert(sled("A"), terminal.clone()); - assert!(!settling.done(None, None, &status), "new sled resets"); - assert!(!settling.done(None, None, &status), "first quiet poll"); - assert!( - !settling.done(None, None, &status), - "quiet but too young for gossip" - ); - assert!(settling.done(None, None, &status), "old enough, quiet"); - - let mut settling = Settling::default(); - status.insert(sled("B"), running.clone()); - assert!(!settling.done(None, None, &status), "running sled holds"); - status.insert(sled("B"), terminal.clone()); - assert!( - !settling.done(None, Some(2), &status), - "the rack count must hold" - ); - assert!( - settling.done(None, Some(2), &status), - "the rack count held twice" - ); - - let known = BTreeSet::from([sled("A"), sled("B")]); - let mut settling = Settling::default(); - assert!( - settling.done(Some(&known), None, &status), - "known sleds settle on the first terminal poll" - ); - status.insert(sled("B"), running); - assert!( - !settling.done(Some(&known), None, &status), - "a running known sled holds" - ); - status.insert(sled("B"), terminal); - let missing = BTreeSet::from([sled("A"), sled("C")]); - let mut settling = Settling::default(); - assert!( - !settling.done(Some(&missing), None, &status), - "a missing known sled holds" - ); -} - -/// Anything the target grammar accepts stays a target. -#[test] -fn target_arg() { - assert!(matches!("*".parse(), Ok(TargetArg::Target(_)))); - assert!(matches!("14".parse(), Ok(TargetArg::Target(_)))); - assert!(matches!( - "913-0000019:BRM42220030".parse(), - Ok(TargetArg::Target(_)) - )); - assert!(matches!( - "brm42220030".parse(), - Ok(TargetArg::Abbreviated(sleds)) - if matches!(sleds.as_slice(), [SledArg::Serial(s)] if s == "brm42220030") - )); - assert!(matches!( - "14,brm42220030".parse(), - Ok(TargetArg::Abbreviated(sleds)) - if matches!(sleds.as_slice(), [SledArg::Sled(SledId::Cubby(14)), SledArg::Serial(_)]) - )); - assert!(matches!( - "913-0000019:BRM42220036,brm42220030".parse(), - Ok(TargetArg::Abbreviated(sleds)) - if matches!(sleds.as_slice(), [SledArg::Sled(SledId::Baseboard(_)), SledArg::Serial(_)]) - )); - assert!("n!ot,a:target".parse::().is_err()); - assert!("*,brm42220030".parse::().is_err()); -} - -/// [`ClientArgs`] must satisfy Clap's internal consistency asserts -/// (unique shorts per subcommand, valid references). -#[test] -fn client_args() { - use clap::CommandFactory as _; - ClientArgs::command().debug_assert(); -} - /// Arguments for `job-stdout` and `job-stderr`. #[derive(Clone, Debug, Parser)] pub struct JobOutput { @@ -2876,7 +2744,138 @@ impl From> for CommandError { #[cfg(test)] mod tests { + use chrono::Utc; + use clap::CommandFactory as _; + use super::*; + use crate::cli::Cli; + + /// A rack-wide target can't combine with `--binary` or `--file`. + #[tokio::test] + async fn output_needs_target() { + let client = + Client::new_with_client("http://[::1]:1", reqwest::Client::new(), Default::default()); + let output = JobOutput::try_parse_from([ + "job-stdout", + "--binary", + "sea-say-sting-palm-tunnel-festival-pull-bid", + ]) + .unwrap(); + let err = job_output( + &mut Cli::default(), + &client, + &"*".parse::().unwrap(), + Stdout, + output, + ) + .await + .unwrap_err(); + assert!(matches!(err, CommandError::OutputNeedsTarget)); + } + + /// The watch settles only after the sled set is stable and every sled + /// is terminal for consecutive polls. + #[test] + fn watch_settling() { + fn sled(serial: &str) -> BaseboardId { + BaseboardId { + part_number: "913-0000019".to_owned(), + serial_number: serial.to_owned(), + } + } + let job_id: JobId = "sea-say-sting-palm-tunnel-festival-pull-bid" + .parse() + .unwrap(); + let terminal = JobStatus::Cancelled { + job_id, + time_cancelled: Utc::now(), + actor: KeyId::random(), + }; + let running = JobStatus::Started { + job_id, + time_started: Utc::now(), + }; + + let mut settling = Settling::default(); + let mut status = JobStatusMap::new(); + assert!(!settling.done(None, None, &status), "empty never settles"); + + status.insert(sled("A"), terminal.clone()); + assert!(!settling.done(None, None, &status), "new sled resets"); + assert!(!settling.done(None, None, &status), "first quiet poll"); + assert!( + !settling.done(None, None, &status), + "quiet but too young for gossip" + ); + assert!(settling.done(None, None, &status), "old enough, quiet"); + + let mut settling = Settling::default(); + status.insert(sled("B"), running.clone()); + assert!(!settling.done(None, None, &status), "running sled holds"); + status.insert(sled("B"), terminal.clone()); + assert!( + !settling.done(None, Some(2), &status), + "the rack count must hold" + ); + assert!( + settling.done(None, Some(2), &status), + "the rack count held twice" + ); + + let known = BTreeSet::from([sled("A"), sled("B")]); + let mut settling = Settling::default(); + assert!( + settling.done(Some(&known), None, &status), + "known sleds settle on the first terminal poll" + ); + status.insert(sled("B"), running); + assert!( + !settling.done(Some(&known), None, &status), + "a running known sled holds" + ); + status.insert(sled("B"), terminal); + let missing = BTreeSet::from([sled("A"), sled("C")]); + let mut settling = Settling::default(); + assert!( + !settling.done(Some(&missing), None, &status), + "a missing known sled holds" + ); + } + + /// Anything the target grammar accepts stays a target. + #[test] + fn target_arg() { + assert!(matches!("*".parse(), Ok(TargetArg::Target(_)))); + assert!(matches!("14".parse(), Ok(TargetArg::Target(_)))); + assert!(matches!( + "913-0000019:BRM42220030".parse(), + Ok(TargetArg::Target(_)) + )); + assert!(matches!( + "brm42220030".parse(), + Ok(TargetArg::Abbreviated(sleds)) + if matches!(sleds.as_slice(), [SledArg::Serial(s)] if s == "brm42220030") + )); + assert!(matches!( + "14,brm42220030".parse(), + Ok(TargetArg::Abbreviated(sleds)) + if matches!(sleds.as_slice(), [SledArg::Sled(SledId::Cubby(14)), SledArg::Serial(_)]) + )); + assert!(matches!( + "913-0000019:BRM42220036,brm42220030".parse(), + Ok(TargetArg::Abbreviated(sleds)) + if matches!(sleds.as_slice(), [SledArg::Sled(SledId::Baseboard(_)), SledArg::Serial(_)]) + )); + assert!("n!ot,a:target".parse::().is_err()); + assert!("*,brm42220030".parse::().is_err()); + } + + /// [`ClientArgs`] must satisfy Clap's internal consistency asserts + /// (unique shorts per subcommand, valid references). + #[test] + fn client_args() { + ClientArgs::command().debug_assert(); + } #[test] fn login_refused_blames_build_skew() { From de8b29e6d54073e506b4a0877ee2567ae88d8e53 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Thu, 29 Oct 2026 20:08:41 -0600 Subject: [PATCH 2/7] Read option defaults from a config file The client now reads `sush.toml` from the XDG config directories, or the file named by `--config`. Keys are option names, and tables are profiles selected by `--profile`. The command line overrides the environment, which overrides the file; of two conflicting options, the one from the weaker source is dropped. Rename `--permslip` to `--permslip-key`. Co-authored-by: Claude --- Cargo.lock | 29 +++ Cargo.toml | 2 + client/Cargo.toml | 4 +- client/src/cli.rs | 8 +- client/src/commands.rs | 62 +++-- client/src/config.rs | 530 +++++++++++++++++++++++++++++++++++++++++ client/src/context.rs | 2 +- client/src/lib.rs | 10 + client/src/main.rs | 7 +- client/src/repl.rs | 26 +- 10 files changed, 645 insertions(+), 35 deletions(-) create mode 100644 client/src/config.rs diff --git a/Cargo.lock b/Cargo.lock index 86dad8b4..e612a3bf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3404,6 +3404,12 @@ dependencies = [ "thiserror 1.0.69", ] +[[package]] +name = "quick-error" +version = "1.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" + [[package]] name = "quinn" version = "0.11.11" @@ -3926,6 +3932,18 @@ version = "1.0.23" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cf54715a573b99ac80df0bc206da022bcd442c974952c7b9720069370852e21f" +[[package]] +name = "rusty-fork" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc6bf79ff24e648f6da1f8d1f011e9cac26491b619e6b9280f2b47f1774e6ee2" +dependencies = [ + "fnv", + "quick-error", + "tempfile", + "wait-timeout", +] + [[package]] name = "rustyline" version = "18.0.0" @@ -4858,6 +4876,7 @@ dependencies = [ "reqwest", "rustix 1.1.4", "rustls", + "rusty-fork", "rustyline", "serde", "serde_json", @@ -4876,6 +4895,7 @@ dependencies = [ "tokio-fd", "tokio-rustls", "tokio-tungstenite", + "toml 1.1.4+spec-1.1.0", "webbrowser", "x509-cert", "xdg 3.0.0", @@ -5734,6 +5754,15 @@ version = "0.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5c3082ca00d5a5ef149bb8b555a72ae84c9c59f7250f013ac822ac2e49b19c64" +[[package]] +name = "wait-timeout" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "09ac3b126d3914f9849036f826e054cbabdc8519970b8998ddaf3b5bd3c65f11" +dependencies = [ + "libc", +] + [[package]] name = "waitgroup" version = "0.1.2" diff --git a/Cargo.toml b/Cargo.toml index 95c0816e..94d047f8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -53,6 +53,7 @@ rlimit = "0.10" rumors = { git = "https://github.com/oxidecomputer/rumors", rev = "bf2ba264e2360eab1ba5f06148ea3ef3050825c8" } rustix = { version = "1", features = ["fs", "process", "pty", "termios"] } rustls = "0.23" +rusty-fork = "0.3" rustyline = { git = "https://github.com/kkawakam/rustyline", rev = "5ab22f2f8d65e85f0b925360437ece8eeeb6db92", features = ["signal-hook", "termios"] } schemars = { version = "0.8", features = ["chrono"] } serde = "1" @@ -76,6 +77,7 @@ tokio-stream = "0.1" tokio-fd = "0.3" tokio-tungstenite = "0.28" tokio-util = "0.7" +toml = "1" webbrowser = "1" x509-cert = { version = "0.2", features = ["pem", "std"] } xdg = "3" diff --git a/client/Cargo.toml b/client/Cargo.toml index 59d2e133..92cb1724 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -27,7 +27,7 @@ base64.workspace = true bytes.workspace = true bytesize.workspace = true chrono.workspace = true -clap.workspace = true +clap = { workspace = true, features = ["string"] } ed25519-dalek.workspace = true futures.workspace = true http.workspace = true @@ -62,9 +62,11 @@ tokio.workspace = true tokio-fd.workspace = true tokio-rustls.workspace = true tokio-tungstenite.workspace = true +toml.workspace = true webbrowser = { workspace = true, optional = true } x509-cert.workspace = true xdg.workspace = true [dev-dependencies] +rusty-fork.workspace = true tempfile.workspace = true diff --git a/client/src/cli.rs b/client/src/cli.rs index 98228c1b..a2069ce2 100644 --- a/client/src/cli.rs +++ b/client/src/cli.rs @@ -28,7 +28,6 @@ use serde_json::{json, to_string as to_json_string, to_string_pretty as to_json_ use sled_hardware_types::BaseboardId; use x509_cert::Certificate; use x509_cert::der::Encode as _; -use xdg::BaseDirectories; use sush_common::authn::Identity; use sush_common::jobs::{ @@ -42,9 +41,8 @@ use sush_common::version::VersionInfo; use crate::commands::{CommandError, GlobalArgs}; use crate::context::{CommandContext, OutputFormat, StartedDisplay, StatusDisplayStyle}; -use crate::{AuthzSigner, quiet_write}; +use crate::{AuthzSigner, base_directories, quiet_write}; -pub(crate) const PREFIX: &str = "sush"; const SESSION_FILE_NAME: &str = "session.json"; const SESSION_FILE_VERSION: u32 = 1; const CLIENT_NONCE_FILE_NAME: &str = "client-nonce.json"; @@ -84,7 +82,7 @@ impl Cli { /// Without persistence, every one-shot command would need a fresh /// `session attach`. pub fn load_session(&mut self) { - let path = match BaseDirectories::with_prefix(PREFIX).place_state_file(SESSION_FILE_NAME) { + let path = match base_directories().place_state_file(SESSION_FILE_NAME) { Ok(path) => path, Err(error) => { eprintln!("❗ The session will not persist: {error}"); @@ -106,7 +104,7 @@ impl Cli { Err(error) => eprintln!("❗ Ignoring the saved session: {error}"), } self.session_file = Some(path); - match BaseDirectories::with_prefix(PREFIX).place_state_file(CLIENT_NONCE_FILE_NAME) { + match base_directories().place_state_file(CLIENT_NONCE_FILE_NAME) { Ok(path) => self.client_nonce_file = Some(path), Err(error) => eprintln!("❗ The client nonce will not persist: {error}"), } diff --git a/client/src/commands.rs b/client/src/commands.rs index 24a36b46..92db06e3 100644 --- a/client/src/commands.rs +++ b/client/src/commands.rs @@ -7,6 +7,7 @@ //! May be executed via either the main CLI or the interactive REPL. use std::collections::BTreeSet; +use std::env::args_os; use std::fs::{File, OpenOptions, read}; use std::io::ErrorKind; use std::io::{Seek as _, SeekFrom, Write as _, stdin}; @@ -19,7 +20,7 @@ use std::time::Duration; use async_recursion::async_recursion; use bytesize::ByteSize; -use clap::{Parser, Subcommand}; +use clap::{CommandFactory as _, FromArgMatches as _, Parser, Subcommand}; use futures::future::{Fuse, FusedFuture as _}; use futures::stream; use futures::{FutureExt as _, StreamExt as _}; @@ -58,6 +59,7 @@ use sush_common::targets::{SledId, Target}; use sush_common::version::VersionInfo; use crate::ByteStream; +use crate::config::{ConfigError, parse_and_merge_config}; use crate::context::{Authz, CommandContext, OutputFormat, StartedDisplay, StatusDisplayStyle}; use crate::identity::{IdentityError, SshAgentConnection}; use crate::interactive::interactive_job; @@ -74,6 +76,7 @@ use crate::{AuthzSigner, Client, Error as ClientError}; #[cfg(feature = "permslip")] pub const PERMSLIP_URL: &str = "PERMSLIP_URL"; pub const SSH_AUTH_SOCK: &str = "SSH_AUTH_SOCK"; +pub const SUSH_CONFIG: &str = "SUSH_CONFIG"; pub const SUSH_JOB_ID: &str = "SUSH_JOB_ID"; pub const SUSH_KEY_ID: &str = "SUSH_KEY_ID"; pub const SUSH_MAX_CPU: &str = "SUSH_MAX_CPU"; @@ -83,6 +86,7 @@ pub const SUSH_MAX_FSIZE: &str = "SUSH_MAX_FSIZE"; pub const SUSH_PERMSLIP_KEY: &str = "SUSH_PERMSLIP_KEY"; #[cfg(feature = "permslip")] pub const SUSH_PERMSLIP_SSH: &str = "SUSH_PERMSLIP_SSH"; +pub const SUSH_PROFILE: &str = "SUSH_PROFILE"; pub const SUSH_OUTPUT_FORMAT: &str = "SUSH_OUTPUT_FORMAT"; pub const SUSH_NEXUS: &str = "SUSH_NEXUS"; pub const SUSH_NEXUS_RESOLVE: &str = "SUSH_NEXUS_RESOLVE"; @@ -142,6 +146,16 @@ impl LimitArgs { /// Optional global arguments, i.e., ones that apply to every command. #[derive(Clone, Debug, Default, Parser)] pub struct GlobalArgs { + /// Configuration file, defaults to XDG config `sush/sush.toml` + #[arg(long, env = SUSH_CONFIG, value_name = "TOML")] + #[clap(global = true)] + pub config: Option, + + /// Configuration profile + #[arg(long, env = SUSH_PROFILE, value_name = "NAME")] + #[clap(global = true)] + pub profile: Option, + /// Output format #[arg(long, env = SUSH_OUTPUT_FORMAT, @@ -167,8 +181,6 @@ pub struct GlobalArgs { pub url: Option, /// Nexus URL to tunnel through instead of a tech port URL - // Not env OXIDE_HOST: clap applies conflict rules to env-sourced - // values, so the oxide CLI's variable would poison --url. #[arg(long, env = SUSH_NEXUS, conflicts_with_all = ["url", "offline"])] #[clap(global = true)] pub nexus: Option, @@ -249,6 +261,18 @@ pub struct ClientArgs { } impl ClientArgs { + /// Parse the process arguments, taking defaults from the + /// configuration file. Exits on a parse error, like clap. + pub fn parse_and_merge_config() -> Result { + match parse_and_merge_config(Self::command(), args_os()) { + Ok(matches) => { + Ok(Self::from_arg_matches(&matches).unwrap_or_else(|error| error.exit())) + } + Err(CommandError::Clap(error)) => error.exit(), + Err(error) => Err(error), + } + } + pub async fn execute(&mut self, ctx: &mut impl CommandContext) -> Result<(), CommandError> { ctx.set_globals(self.globals.clone()); self.command.clone().execute(ctx).await @@ -417,10 +441,10 @@ pub enum SessionCommand { /// Use `permslip` to sign sessions and jobs with this key name. #[arg(short, long, env = SUSH_PERMSLIP_KEY, value_name = "KEY_NAME")] - permslip: Option, + permslip_key: Option, /// The `permslip` server to contact for signing. - #[arg(long, env = PERMSLIP_URL, requires = "permslip", value_name = "URL")] + #[arg(long, env = PERMSLIP_URL, value_name = "URL")] permslip_url: Option, }, @@ -446,11 +470,11 @@ pub enum SessionCommand { /// Use `permslip` to sign sessions and jobs with this key name. #[cfg(feature = "permslip")] #[arg(short, long, env = SUSH_PERMSLIP_KEY, value_name = "KEY_NAME")] - permslip: Option, + permslip_key: Option, /// The `permslip` server to contact for signing. #[cfg(feature = "permslip")] - #[arg(long, env = PERMSLIP_URL, requires = "permslip", value_name = "URL")] + #[arg(long, env = PERMSLIP_URL, value_name = "URL")] permslip_url: Option, }, @@ -590,11 +614,11 @@ pub struct JobStartArgs { /// Use `permslip` to sign job requests with this key name. #[cfg(feature = "permslip")] #[arg(short, long, env = SUSH_PERMSLIP_KEY, value_name = "KEY_NAME")] - permslip: Option, + permslip_key: Option, /// The `permslip` server to contact for signing. #[cfg(feature = "permslip")] - #[arg(long, env = PERMSLIP_URL, requires = "permslip", value_name = "URL")] + #[arg(long, env = PERMSLIP_URL, value_name = "URL")] permslip_url: Option, /// Terminal type for interactive jobs. @@ -973,7 +997,7 @@ async fn session( nonce, seq, client_nonce, - permslip, + permslip_key, permslip_url, }, Some(client), @@ -999,7 +1023,7 @@ async fn session( let Some(permslip_url) = permslip_url else { return session_start_params(ctx, client).await; }; - let Some(permslip_key) = permslip else { + let Some(permslip_key) = permslip_key else { return Err(CommandError::MissingKeyName); }; let (baseboard_id, nonce) = with_login(ctx, client, async || { @@ -1033,7 +1057,7 @@ async fn session( SessionCommand::Create { baseboard_id, nonce, - permslip, + permslip_key, permslip_url, }, _, @@ -1041,7 +1065,7 @@ async fn session( let Some(permslip_url) = permslip_url else { return Err(CommandError::MissingPermslipUrl); }; - let Some(permslip_key) = permslip else { + let Some(permslip_key) = permslip_key else { return Err(CommandError::MissingKeyName); }; let (session_id, nonce, seq) = @@ -1202,7 +1226,10 @@ async fn job( #[cfg(feature = "permslip")] ( JobCommand::Start { - start_args: JobStartArgs { permslip: None, .. }, + start_args: + JobStartArgs { + permslip_key: None, .. + }, .. }, Some(_), @@ -1213,7 +1240,7 @@ async fn job( JobCommand::Start { start_args: ref start_args @ JobStartArgs { - permslip: Some(ref key_name), + permslip_key: Some(ref key_name), permslip_url: Some(ref permslip_url), .. }, @@ -2503,6 +2530,8 @@ pub enum CommandError { Clap(#[from] clap::Error), #[error("{0}")] Client(String), + #[error("Configuration error: {0}")] + Config(#[from] ConfigError), #[error("{0}")] Der(#[from] x509_cert::der::Error), #[error("{0}")] @@ -2563,7 +2592,7 @@ pub enum CommandError { #[error("Missing job command, try `--help`")] MissingCommand, #[cfg(feature = "permslip")] - #[error("Missing signing key name, try `--permslip`")] + #[error("Missing signing key name, try `--permslip-key`")] MissingKeyName, #[cfg(feature = "permslip")] #[error("Missing permslip URL, try `--permslip-url` or setting `PERMSLIP_URL`")] @@ -2745,7 +2774,6 @@ impl From> for CommandError { #[cfg(test)] mod tests { use chrono::Utc; - use clap::CommandFactory as _; use super::*; use crate::cli::Cli; diff --git a/client/src/config.rs b/client/src/config.rs new file mode 100644 index 00000000..aefac61d --- /dev/null +++ b/client/src/config.rs @@ -0,0 +1,530 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +//! The client configuration file, e.g., `~/.config/sush/sush.toml`. +//! +//! The options specified in the config file provide defaults for +//! the client command options. Option values cascade, as in CSS: +//! the command line overrides the environment, which overrides the +//! config file, which overrides the built-in defaults. When two +//! conflicting options come from different sources, the cascade +//! drops the one from the weaker source. To run the cascade, we +//! take `UnmergedArgs` from the environment and command line, +//! and *merge* that with the parsed config options; see [`parse_with_config`]. +//! +//! Each key in the config file is the long name of a command option +//! with a (non-secret) environment variable. Each table is a profile, +//! and `--profile` selects one to apply. Top-level keys apply to +//! every profile, and the selected profile overrides the top-level. + +use std::cmp::Ordering; +use std::collections::{BTreeMap, BTreeSet}; +use std::ffi::OsString; +use std::fs::read_to_string; +use std::io; +use std::path::{Path, PathBuf}; + +use clap::parser::ValueSource; +use clap::{Arg, ArgMatches, Command, CommandFactory as _, FromArgMatches as _}; +use thiserror::Error; +use toml::{Table, Value}; + +use crate::base_directories; +use crate::commands::{ClientArgs, CommandError, GlobalArgs}; + +const FILE_NAME: &str = "sush.toml"; + +/// One profile's settings, ready to become option defaults. +#[derive(Debug, Default)] +pub struct ConfigOptions { + values: BTreeMap>, +} + +impl ConfigOptions { + /// Read `path`, or the default file if it exists. + pub fn load(path: Option, profile: Option) -> Result { + let path = path.or_else(|| base_directories().find_config_file(FILE_NAME)); + let Some(path) = path else { + return match profile { + Some(profile) => Err(ConfigError::NoFile(profile)), + None => Ok(Self::default()), + }; + }; + let text = read_to_string(&path).map_err(|error| ConfigError::Read { + path: path.clone(), + error, + })?; + Self::from_toml(path, &text, profile) + } + + fn from_toml(path: PathBuf, text: &str, profile: Option) -> Result { + let table = toml::from_str::(text).map_err(|error| ConfigError::Parse { + path: path.clone(), + error, + })?; + let valid_keys = all_keys(&ClientArgs::command()); + let convert = |table| convert_table(table, &valid_keys, &path); + + let mut top_level = Table::new(); + let mut profiles = BTreeMap::new(); + for (key, value) in table { + match value { + Value::Table(profile) => { + profiles.insert(key, convert(profile)?); + } + value => { + top_level.insert(key, value); + } + } + } + + let mut values = convert(top_level)?; + let profile = profile.or_else(|| values.get("profile")?.first().cloned()); + if let Some(profile) = profile { + let selected = profiles + .remove(&profile) + .ok_or(ConfigError::UnknownProfile { path, profile })?; + values.extend(selected); + } + Ok(Self { values }) + } + + fn values_for(&self, arg: &Arg) -> Option<&Vec> { + arg_keys(arg).find_map(|key| self.values.get(key)) + } +} + +/// Convert the entries of `table` to option values. Fails on +/// any key not in `valid_keys` or any value of the wrong type, +/// reporting `path` as the source. +fn convert_table( + table: Table, + valid_keys: &BTreeSet, + path: &Path, +) -> Result>, ConfigError> { + table + .into_iter() + .map(|(key, value)| { + if !valid_keys.contains(&key) { + return Err(ConfigError::UnknownKey { + path: path.to_owned(), + key, + valid_keys: valid_keys.iter().cloned().collect(), + }); + } + let Some(values) = arg_values(value) else { + return Err(ConfigError::InvalidValue { + path: path.to_owned(), + key, + }); + }; + Ok((key, values)) + }) + .collect() +} + +/// Parse `words` with `command`, merging in defaults from `options`. +pub fn parse_with_config( + command: Command, + words: I, + options: &ConfigOptions, +) -> Result +where + I: IntoIterator, + T: Into, +{ + UnmergedArgs::parse(command, words)?.merge(options) +} + +/// Parse `words` with `command`, merging in the config file and +/// profile they select. +pub fn parse_and_merge_config(command: Command, words: I) -> Result +where + I: IntoIterator, + T: Into, +{ + let unmerged = UnmergedArgs::parse(command, words)?; + let GlobalArgs { + config, profile, .. + } = GlobalArgs::from_arg_matches(&unmerged.matches)?; + let options = ConfigOptions::load(config, profile)?; + Ok(unmerged.merge(&options)?) +} + +struct UnmergedArgs { + command: Command, + words: Vec, + matches: ArgMatches, +} + +impl UnmergedArgs { + fn parse(command: Command, words: I) -> Result + where + I: IntoIterator, + T: Into, + { + let words = words.into_iter().map(Into::into).collect::>(); + let matches = command + .clone() + .ignore_errors(true) // wait for merge to resolve conflicts + .disable_help_flag(true) // wait for merge so defaults show + .disable_help_subcommand(true) // ditto + .try_get_matches_from(&words)?; + Ok(Self { + command, + words, + matches, + }) + } + + fn merge(self, options: &ConfigOptions) -> Result { + let Self { + command, + words, + matches, + } = self; + let losers = compute_merge_losers(options, &command, &matches); + let command = map_command_tree(command, &|command| { + command.mut_args(|arg| { + if losers.contains(arg.get_id().as_str()) { + arg.env(None).default_value(None) + } else if let Some(values) = options.values_for(&arg) { + arg.default_values(values.clone()) + } else { + arg + } + }) + }); + command.try_get_matches_from(words) + } +} + +/// List the config file keys that can set `arg`. +fn arg_keys(arg: &Arg) -> impl Iterator { + let configurable = arg.get_env().is_some() && !arg.is_hide_env_values_set(); + arg.get_long() + .into_iter() + .chain(arg.get_all_aliases().unwrap_or_default()) + .filter(move |_| configurable) +} + +/// Collect the config file keys valid anywhere in `command`. +fn all_keys(command: &Command) -> BTreeSet { + let mut keys = command + .get_arguments() + .flat_map(arg_keys) + .map(str::to_owned) + .collect::>(); + for subcommand in command.get_subcommands() { + keys.extend(all_keys(subcommand)); + } + keys +} + +fn compute_merge_losers( + options: &ConfigOptions, + command: &Command, + matches: &ArgMatches, +) -> BTreeSet { + let mut losers = BTreeSet::new(); + let source = |arg: &Arg| { + matches + .value_source(arg.get_id().as_str()) + .filter(|&source| source != ValueSource::DefaultValue) + .or_else(|| options.values_for(arg).map(|_| ValueSource::DefaultValue)) + }; + for arg in command.get_arguments() { + for other in command.get_arg_conflicts_with(arg) { + if let (Some(a), Some(b)) = (source(arg), source(other)) { + match a.cmp(&b) { + Ordering::Equal => (), // neither loses + Ordering::Less => { + losers.insert(arg.get_id().to_string()); + } + Ordering::Greater => { + losers.insert(other.get_id().to_string()); + } + } + } + } + } + if let Some((name, matches)) = matches.subcommand() + && let Some(subcommand) = command.find_subcommand(name) + { + losers.extend(compute_merge_losers(options, subcommand, matches)); + } + losers +} + +fn map_command_tree(command: Command, f: &impl Fn(Command) -> Command) -> Command { + f(command).mut_subcommands(|subcommand| map_command_tree(subcommand, f)) +} + +fn arg_values(value: Value) -> Option> { + let scalar = |value| match value { + Value::String(value) => Some(value), + Value::Integer(value) => Some(value.to_string()), + Value::Boolean(value) => Some(value.to_string()), + _ => None, + }; + match value { + Value::Array(values) => values.into_iter().map(scalar).collect(), + value => scalar(value).map(|value| vec![value]), + } +} + +#[derive(Debug, Error)] +pub enum ConfigError { + #[error("can't read {path}: {error}", path = path.display())] + Read { path: PathBuf, error: io::Error }, + #[error("can't parse {path}: {error}", path = path.display())] + Parse { + path: PathBuf, + error: toml::de::Error, + }, + #[error("no profile `{profile}` in {path}", path = path.display())] + UnknownProfile { path: PathBuf, profile: String }, + #[error("no {FILE_NAME} found for profile `{0}`")] + NoFile(String), + #[error( + "`{key}` in {path} must be a string, integer, boolean, or array", + path = path.display() + )] + InvalidValue { path: PathBuf, key: String }, + #[error( + "unknown key `{key}` in {path}, try one of: {}", + valid_keys.join(", "), + path = path.display() + )] + UnknownKey { + path: PathBuf, + key: String, + valid_keys: Vec, + }, +} + +#[cfg(test)] +mod test { + use std::env::{remove_var, set_var}; + use std::fs::write; + + use rusty_fork::rusty_fork_test; + use tempfile::NamedTempFile; + + use super::*; + use crate::commands::{SUSH_CONFIG, SUSH_NEXUS, SUSH_PROFILE, SUSH_PROXY_ROOT, SUSH_URL}; + use crate::context::OutputFormat; + + /// Parse `words` with the config file `text`, after unsetting every + /// option's environment variable and then setting those in `env`. + fn parse(text: &str, env: &[(&str, &str)], words: &str) -> Result { + // SAFETY: `rusty_fork_test!` runs each test alone in its own + // process, so no other thread reads the environment while this + // function writes it. + fn unset(command: &Command) { + for arg in command.get_arguments() { + if let Some(var) = arg.get_env() { + unsafe { remove_var(var) } + } + } + for subcommand in command.get_subcommands() { + unset(subcommand); + } + } + unset(&ClientArgs::command()); + let file = NamedTempFile::new().unwrap(); + write(file.path(), text).unwrap(); + unsafe { set_var(SUSH_CONFIG, file.path()) } + for (var, value) in env { + unsafe { set_var(var, value) } + } + let words = ["sush"].into_iter().chain(words.split_whitespace()); + parse_and_merge_config(ClientArgs::command(), words) + } + + fn globals(text: &str, env: &[(&str, &str)], words: &str) -> GlobalArgs { + GlobalArgs::from_arg_matches(&parse(text, env, words).unwrap()).unwrap() + } + + rusty_fork_test! { + /// The file's `profile` key selects a profile, and `--profile` or + /// `SUSH_PROFILE` overrides that choice. + #[test] + fn profiles() { + let text = r#" + profile = "a" + [a] + rack = "a" + [b] + rack = "b" + "#; + let rack = |env, words| globals(text, env, words).rack; + assert_eq!(rack(&[], "version").as_deref(), Some("a")); + assert_eq!(rack(&[], "--profile b version").as_deref(), Some("b")); + assert_eq!( + rack(&[(SUSH_PROFILE, "b")], "version").as_deref(), + Some("b") + ); + assert!(matches!( + parse(text, &[], "--profile c version"), + Err(CommandError::Config(ConfigError::UnknownProfile { .. })) + )); + } + + /// Each source overrides the ones below it: the command line, the + /// environment, the selected profile, the file's top level, and the + /// built-in default. + #[test] + fn precedence() { + let text = r#" + profile = "dublin" + rack = "top" + ssh-auth-sock = "top" + proxy-root = "file.pem" + [dublin] + ssh-auth-sock = "profile" + url = "https://file" + nexus-root = ["a.pem", "b.pem"] + "#; + let env = [(SUSH_URL, "https://env"), (SUSH_PROXY_ROOT, "env.pem")]; + let globals = globals(text, &env, "--proxy-root cli.pem version"); + assert_eq!( + globals.output, + Some(OutputFormat::Text), + "the built-in default applies when nothing else sets the option" + ); + assert_eq!( + globals.rack.as_deref(), + Some("top"), + "the top level applies when the profile does not set the option" + ); + assert_eq!( + globals.ssh_auth_sock.as_deref(), + Some("profile"), + "the profile overrides the top level" + ); + assert_eq!( + globals.nexus_roots, + [PathBuf::from("a.pem"), PathBuf::from("b.pem")], + "the profile sets every element of an array" + ); + assert_eq!( + globals.url.as_deref(), + Some("https://env"), + "the environment overrides the profile" + ); + assert_eq!( + globals.proxy_roots, + [PathBuf::from("cli.pem")], + "the command line overrides the environment and the top level" + ); + } + + /// The file can set a subcommand's options, and the command line can + /// override one of them while the others keep the file's values. + #[cfg(feature = "permslip")] + #[test] + fn subcommand_options() { + let text = r#" + permslip-key = "Key" + permslip-url = "https://permslip" + "#; + let start = |words| { + let matches = parse(text, &[], words).unwrap(); + let start = matches + .subcommand_matches("job") + .and_then(|job| job.subcommand_matches("start")) + .unwrap(); + let get = |id| start.get_one::(id).cloned(); + (get("permslip_key"), get("permslip_url")) + }; + assert_eq!( + start("job start uptime"), + (Some("Key".to_owned()), Some("https://permslip".to_owned())) + ); + assert_eq!( + start("job start --permslip-url https://other uptime"), + (Some("Key".to_owned()), Some("https://other".to_owned())) + ); + } + + /// Of two conflicting options, the one from the stronger source wins. + #[test] + fn conflicts() { + let reach = |text, env, words| { + let GlobalArgs { + offline, + url, + nexus, + .. + } = globals(text, env, words); + (offline, url, nexus) + }; + let url = || Some("https://url".to_owned()); + let nexus = || Some("https://nexus".to_owned()); + + let file_url = r#"url = "https://url""#; + assert_eq!( + reach(file_url, &[], "--nexus https://nexus version"), + (false, None, nexus()), + "the command line beats the file" + ); + assert_eq!( + reach(file_url, &[], "--offline version"), + (true, None, None), + "offline beats a configured url" + ); + assert_eq!( + reach(file_url, &[(SUSH_NEXUS, "https://nexus")], "version"), + (false, None, nexus()), + "the environment beats the file" + ); + assert_eq!( + reach( + "", + &[(SUSH_NEXUS, "https://nexus")], + "--url https://url version" + ), + (false, url(), None), + "the command line beats the environment" + ); + assert!( + parse("", &[], "--nexus https://nexus --url https://url version").is_err(), + "conflicts from one source are errors" + ); + + let output = |text, words| globals(text, &[], words).output_format(); + let file_json = r#"output = "json""#; + assert_eq!( + output(file_json, "version"), + Some(OutputFormat::Json), + "the file beats the built-in default" + ); + assert_eq!( + output(file_json, "--text version"), + Some(OutputFormat::Text), + "the command line beats the file" + ); + } + + /// The file rejects keys for secrets, keys for options without an + /// environment variable, and keys that name no option. + #[test] + fn invalid_keys() { + for (text, bad) in [ + (r#"nexus-token = "secret""#, "nexus-token"), + ("[rack]\nurll = \"https://url\"", "urll"), + ("force = true", "force"), + ] { + assert!( + matches!( + parse(text, &[], "version"), + Err(CommandError::Config(ConfigError::UnknownKey { key, .. })) if key == bad + ), + "the file rejects `{bad}`" + ); + } + } + } +} diff --git a/client/src/context.rs b/client/src/context.rs index b958841a..49036559 100644 --- a/client/src/context.rs +++ b/client/src/context.rs @@ -61,7 +61,7 @@ impl Authz { /// What kind of output to emit on standard output. Standard input /// and standard error may be used with all formats for interactive /// communication with a human user (if there is one). -#[derive(Clone, Copy, Debug, Default, ValueEnum)] +#[derive(Clone, Copy, Debug, Default, PartialEq, ValueEnum)] pub enum OutputFormat { /// UTF-8 encoded text #[default] diff --git a/client/src/lib.rs b/client/src/lib.rs index 298f4a2d..d6b28174 100644 --- a/client/src/lib.rs +++ b/client/src/lib.rs @@ -11,9 +11,18 @@ use reqwest::header::AUTHORIZATION; use sush_common::authn::{AuthnError, BoundCredentials}; use sush_common::keys::KeyId; use tokio::sync::{Mutex as AsyncMutex, MutexGuard as AsyncMutexGuard}; +use xdg::BaseDirectories; use crate::context::Authz; +/// The client's name. +pub(crate) const PREFIX: &str = "sush"; + +/// The client's XDG directories. +pub(crate) fn base_directories() -> BaseDirectories { + BaseDirectories::with_prefix(PREFIX) +} + /// Like `print!`, but exit quietly if stdout is a closed pipe. #[macro_export] macro_rules! qprint { @@ -55,6 +64,7 @@ pub fn quiet_write(bytes: &[u8]) -> io::Result<()> { pub mod agent; pub mod cli; pub mod commands; +pub mod config; pub mod context; pub mod identity; pub mod interactive; diff --git a/client/src/main.rs b/client/src/main.rs index f2581007..0d07e94a 100644 --- a/client/src/main.rs +++ b/client/src/main.rs @@ -7,7 +7,6 @@ use std::io::stdout; use std::process::ExitCode; -use clap::Parser as _; use rustix::termios::{OptionalActions, OutputModes, Termios, isatty, tcgetattr, tcsetattr}; use sush_client::cli::Cli; @@ -42,7 +41,11 @@ async fn main() -> ExitCode { let saved = no_expand_tabs(); let mut cli = Cli::default(); cli.load_session(); - let code = match ClientArgs::parse().execute(&mut cli).await { + let result = match ClientArgs::parse_and_merge_config() { + Ok(mut args) => args.execute(&mut cli).await, + Err(error) => Err(error), + }; + let code = match result { Ok(()) => ExitCode::SUCCESS, Err(error) => { cli.command_error(&error); diff --git a/client/src/repl.rs b/client/src/repl.rs index 628bbf31..b4e7b16a 100644 --- a/client/src/repl.rs +++ b/client/src/repl.rs @@ -12,13 +12,12 @@ use std::ffi::OsString; use std::fs::{Permissions, set_permissions}; use std::os::unix::fs::PermissionsExt as _; -use clap::Parser; +use clap::{CommandFactory as _, FromArgMatches as _, Parser}; use rustyline::DefaultEditor; use rustyline::error::ReadlineError; use shlex::split as split_command; use sled_hardware_types::BaseboardId; use x509_cert::Certificate; -use xdg::BaseDirectories; use sush_common::authn::Identity; use sush_common::jobs::{ @@ -29,13 +28,14 @@ use sush_common::keys::{KeyId, SshPublicKey}; use sush_common::targets::{SledId, SledVersion}; use sush_common::version::VersionInfo; -use crate::cli::{Cli, PREFIX}; +use crate::cli::Cli; use crate::commands::{ ClientCommand, CommandError, GlobalArgs, SSH_AUTH_SOCK, SUSH_JOB_ID, SUSH_KEY_ID, SUSH_OUTPUT_FORMAT, SUSH_URL, }; +use crate::config::{ConfigOptions, parse_with_config}; use crate::context::{CommandContext, OutputFormat, StartedDisplay, StatusDisplayStyle}; -use crate::{AuthzSigner, Client}; +use crate::{AuthzSigner, Client, PREFIX, base_directories}; const HISTORY_FILE: &str = "history.txt"; @@ -47,7 +47,12 @@ struct ReplCommandParser { } impl ReplCommandParser { - fn try_parse(words: I, ctx: &mut C) -> Result + /// Parse `words`, taking defaults from `options`. + fn try_parse( + ctx: &mut C, + words: I, + options: &ConfigOptions, + ) -> Result where I: IntoIterator, T: Into + Clone, @@ -59,7 +64,8 @@ impl ReplCommandParser { { ctx.pre_parse_hook(command); } - Ok(Self::try_parse_from(words)?) + let matches = parse_with_config(Self::command(), words, options)?; + Ok(Self::from_arg_matches(&matches)?) } } @@ -77,8 +83,8 @@ impl Repl { ) -> Result<(), CommandError> { self.set_globals(args.clone()); self.cli.load_session(); - let xdg = BaseDirectories::with_prefix(PREFIX); - let history_file = xdg + let options = ConfigOptions::load(args.config, args.profile)?; + let history_file = base_directories() .place_state_file(HISTORY_FILE) .map_err(|error| CommandError::io(HISTORY_FILE, error))?; let mut rl = DefaultEditor::new()?; @@ -99,7 +105,7 @@ impl Repl { let Some(words) = split_command(&command) else { perr!("❌ Invalid quoting in command"); }; - let command = match ReplCommandParser::try_parse(&words, &mut self) { + let command = match ReplCommandParser::try_parse(&mut self, &words, &options) { Ok(ReplCommandParser { command }) => command, Err(err) => { self.command_error(&err); @@ -182,6 +188,8 @@ impl CommandContext for Repl { nexus_roots: _, nexus_resolve: _, rack: _, + config: _, + profile: _, } = args; if json { output = Some(OutputFormat::Json); From 9ebe369a90dbc03369f134239d57aca3c6e60223 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 30 Oct 2026 11:32:07 -0600 Subject: [PATCH 3/7] Fix broken `set` command, add `unset` Co-authored-by: Claude --- Cargo.lock | 1 + client/Cargo.toml | 1 + client/src/commands.rs | 114 +++++++++++------ client/src/config.rs | 272 +++++++++++++++++++++++++++++++++++++++-- client/src/context.rs | 11 +- client/src/repl.rs | 194 ++++++++++++++--------------- 6 files changed, 450 insertions(+), 143 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e612a3bf..5b65277f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4887,6 +4887,7 @@ dependencies = [ "slog", "sprockets-tls", "ssh-key", + "strum 0.27.2", "sush-api", "sush-common", "tempfile", diff --git a/client/Cargo.toml b/client/Cargo.toml index 92cb1724..1bf01499 100644 --- a/client/Cargo.toml +++ b/client/Cargo.toml @@ -55,6 +55,7 @@ shlex.workspace = true signature.workspace = true sled-hardware-types.workspace = true ssh-key.workspace = true +strum.workspace = true sush-api = { path = "../api" } sush-common = { path = "../common" } thiserror.workspace = true diff --git a/client/src/commands.rs b/client/src/commands.rs index 92db06e3..1cd0a0dc 100644 --- a/client/src/commands.rs +++ b/client/src/commands.rs @@ -59,7 +59,7 @@ use sush_common::targets::{SledId, Target}; use sush_common::version::VersionInfo; use crate::ByteStream; -use crate::config::{ConfigError, parse_and_merge_config}; +use crate::config::{ConfigError, SuppliedOptions, parse_and_merge_config}; use crate::context::{Authz, CommandContext, OutputFormat, StartedDisplay, StatusDisplayStyle}; use crate::identity::{IdentityError, SshAgentConnection}; use crate::interactive::interactive_job; @@ -166,12 +166,12 @@ pub struct GlobalArgs { pub output: Option, /// Shortcut for `--output=json` - #[arg(short, long, default_value_t = false, conflicts_with = "text")] + #[arg(short, long, default_value_t = false, conflicts_with_all = ["text", "output"])] #[clap(global = true)] pub json: bool, /// Shortcut for `--output=text` - #[arg(short, long, default_value_t = false, conflicts_with = "json")] + #[arg(short, long, default_value_t = false, conflicts_with_all = ["json", "output"])] #[clap(global = true)] pub text: bool, @@ -234,6 +234,11 @@ pub struct GlobalArgs { #[arg(short, long, env = SUSH_KEY_ID)] #[clap(global = true)] pub ssh_key_id: Option, + + /// Options explicitly supplied on the command line. For REPL + /// bookkeeping only. + #[arg(skip)] + pub supplied: SuppliedOptions, } impl GlobalArgs { @@ -246,6 +251,16 @@ impl GlobalArgs { self.output } } + + /// Point these options at an open tunnel's local `url` instead of + /// Nexus. The REPL re-enters `run` for each command and parses its + /// supplied options each time, so they must name the tunnel too; + /// otherwise each command would open a tunnel of its own. + fn use_tunnel(&mut self, url: &str) { + self.url = Some(url.to_owned()); + self.nexus = None; + self.supplied.set(&Self::command(), "url", vec![url.into()]); + } } #[derive(Debug, Parser)] @@ -264,9 +279,13 @@ impl ClientArgs { /// Parse the process arguments, taking defaults from the /// configuration file. Exits on a parse error, like clap. pub fn parse_and_merge_config() -> Result { - match parse_and_merge_config(Self::command(), args_os()) { + let command = Self::command(); + match parse_and_merge_config(command.clone(), args_os()) { Ok(matches) => { - Ok(Self::from_arg_matches(&matches).unwrap_or_else(|error| error.exit())) + let mut args = + Self::from_arg_matches(&matches).unwrap_or_else(|error| error.exit()); + args.globals.supplied = SuppliedOptions::from_matches(&command, &matches); + Ok(args) } Err(CommandError::Clap(error)) => error.exit(), Err(error) => Err(error), @@ -284,7 +303,7 @@ impl ClientArgs { pub struct JobOutput { /// The job whose output should be fetched. #[clap(env = SUSH_JOB_ID)] - job_id: JobId, + job_id: Option, /// Job output is binary, not UTF-8 encoded text. #[arg(short, long, default_value_t = false, conflicts_with = "file")] @@ -357,12 +376,22 @@ pub enum ClientCommand { command: JobCommand, }, - /// Arguments for subsequent interactive commands. + /// Set options for later commands. + /// + /// Each option replaces any earlier or conflicting one. Also reload + /// the config file and show the current options. Set { #[clap(flatten)] args: GlobalArgs, }, + /// Unset options supplied earlier. + Unset { + /// The long names of the options to unset, without the leading `--`. + #[arg(required = true, value_name = "OPTION")] + options: Vec, + }, + /// Start an interactive REPL. #[clap(alias = "repl")] Shell, @@ -507,7 +536,7 @@ pub enum JobCommand { Stop { /// The job to abort. #[clap(env = SUSH_JOB_ID)] - job_id: JobId, + job_id: Option, }, /// Decline to run a job, burning its ID and advancing the session. @@ -520,7 +549,7 @@ pub enum JobCommand { Status { /// The job whose status should be fetched. #[clap(env = SUSH_JOB_ID)] - job_id: JobId, + job_id: Option, /// Show full per-sled status instead of one line per sled. #[arg(short, long)] @@ -557,7 +586,7 @@ pub enum JobCommand { Attach { /// The interactive job to attach to. #[clap(env = SUSH_JOB_ID)] - job_id: JobId, + job_id: Option, /// The sled to attach to. #[arg(short = 'T', long, default_value = "*")] @@ -642,9 +671,7 @@ impl JobStartArgs { } impl ClientCommand { - /// Interactive jobs forward SIGINT to the job, watches use it to - /// stop the job or end the watch, and the REPL turns it into a - /// fresh prompt. + /// Whether a command handles SIGINT itself. fn handles_sigint(&self) -> bool { matches!( self, @@ -655,6 +682,11 @@ impl ClientCommand { ) } + /// Whether a command contacts the server. + fn needs_server(&self) -> bool { + !matches!(self, Self::Set { .. } | Self::Unset { .. } | Self::Quit) + } + /// Run the command, letting an interrupt cancel it unless the /// command handles SIGINT itself. #[async_recursion(?Send)] @@ -676,7 +708,7 @@ impl ClientCommand { // The tunnel stands in for the proxy until the command ends. let _tunnel = match args.nexus.as_ref() { - Some(nexus) => { + Some(nexus) if self.needs_server() => { let rack = args.rack.as_ref().ok_or(CommandError::MissingRack)?; let token = args .nexus_token @@ -685,17 +717,14 @@ impl ClientCommand { let tunnel = Tunnel::start(nexus, rack, token, &args.nexus_roots, args.nexus_resolve) .await?; - args.url = Some(tunnel.url.clone()); - // The repl re-enters run per command; commands must - // reuse this tunnel, not build their own. - args.nexus = None; + args.use_tunnel(&tunnel.url); Some(tunnel) } - None => None, + _ => None, }; let client = match args.url.as_ref() { - Some(url) => { + Some(url) if self.needs_server() => { let roots = if args.proxy_roots.is_empty() { tls::platform_roots()? } else { @@ -715,7 +744,7 @@ impl ClientCommand { )) } } - None => None, + _ => None, }; match (self, client) { (ClientCommand::Cert { command }, Some(client)) => cert(ctx, &client, command).await, @@ -743,13 +772,12 @@ impl ClientCommand { (ClientCommand::Job { command }, client) => job(ctx, &client, command).await, - (ClientCommand::Set { args }, _) => { - ctx.set_globals(args); - Ok(()) - } + (ClientCommand::Set { args }, _) => ctx.set_options(args.supplied), - (ClientCommand::Shell, client) => { - Repl::default().run(args.clone(), client).await?; + (ClientCommand::Unset { options }, _) => ctx.unset_options(&options), + + (ClientCommand::Shell, _) => { + Repl::default().run(args.clone()).await?; Ok(()) } @@ -1289,6 +1317,7 @@ async fn job( } (JobCommand::Stop { job_id }, Some(client)) => { + let job_id = job_id_or_last(ctx, job_id)?; job_stop(ctx, client, &job_id, None).await?; ctx.job_stopped(&job_id); Ok(()) @@ -1323,6 +1352,7 @@ async fn job( } (JobCommand::Status { job_id, full, wait }, Some(client)) => { + let job_id = job_id_or_last(ctx, job_id)?; let style = if full { StatusDisplayStyle::Full } else { @@ -1346,6 +1376,7 @@ async fn job( } (JobCommand::Attach { job_id, target }, Some(client)) => { + let job_id = job_id_or_last(ctx, job_id)?; let target = match &target { TargetArg::Abbreviated(sleds) => match sleds.as_slice() { [SledArg::Serial(serial)] => { @@ -1457,7 +1488,7 @@ async fn job_start( Ok(()) => (), Err(CommandError::NotFound(_)) => { let output = JobOutput { - job_id, + job_id: Some(job_id), binary: true, file: None, force: false, @@ -2006,6 +2037,13 @@ struct Chunk(Range, ByteStream); /// A promise of some bytes for a range of output. type FutureChunk<'a> = dyn Future> + Send + 'a; +/// Take `job_id`, or else the last job `ctx` knows of. +fn job_id_or_last(ctx: &impl CommandContext, job_id: Option) -> Result { + job_id + .or_else(|| ctx.last_job_id()) + .ok_or(CommandError::MissingJobId) +} + /// Download job output. async fn job_output( ctx: &mut impl CommandContext, @@ -2014,11 +2052,12 @@ async fn job_output( stream: JobOutputStream, args: JobOutput, ) -> Result<(), CommandError> { + let job_id = job_id_or_last(ctx, args.job_id)?; let target = match target { TargetArg::Target(target) => target.clone(), TargetArg::Abbreviated(sleds) => { if let [SledArg::Serial(serial)] = sleds.as_slice() { - let baseboard = resolve_serial(ctx, client, &args.job_id, serial).await?; + let baseboard = resolve_serial(ctx, client, &job_id, serial).await?; return job_output_from(ctx, client, &baseboard, stream, args).await; } resolve_target_arg(client, target).await? @@ -2033,12 +2072,9 @@ async fn job_output( } // Fetch output from every sled with a recorded status. - let status = job_status_map(ctx, client, &args.job_id, None).await?; + let status = job_status_map(ctx, client, &job_id, None).await?; if status.is_empty() { - return Err(CommandError::NotFound(format!( - "Job `{}` not found", - args.job_id - ))); + return Err(CommandError::NotFound(format!("Job `{job_id}` not found"))); } for baseboard in status.keys() { if let Err(error) = job_output_from(ctx, client, baseboard, stream, args.clone()).await { @@ -2062,6 +2098,8 @@ async fn job_output_from( parallel, }: JobOutput, ) -> Result<(), CommandError> { + let job_id = job_id_or_last(ctx, job_id)?; + // Fetch job status for output length and hash. let status = job_status_map(ctx, client, &job_id, Some(target)).await?; @@ -2591,6 +2629,14 @@ pub enum CommandError { MissingClientNonce, #[error("Missing job command, try `--help`")] MissingCommand, + #[error("`{0}` only works in the REPL, try `sush repl`")] + ReplOnly(&'static str), + #[error("Unknown option `{0}`, try `set --help`")] + UnknownOption(String), + #[error("Option `{0}` was not set in the REPL")] + OptionNotSet(String), + #[error("Missing job ID, try passing one or setting `SUSH_JOB_ID`")] + MissingJobId, #[cfg(feature = "permslip")] #[error("Missing signing key name, try `--permslip-key`")] MissingKeyName, diff --git a/client/src/config.rs b/client/src/config.rs index aefac61d..f339ef45 100644 --- a/client/src/config.rs +++ b/client/src/config.rs @@ -20,6 +20,7 @@ use std::cmp::Ordering; use std::collections::{BTreeMap, BTreeSet}; +use std::env::var_os; use std::ffi::OsString; use std::fs::read_to_string; use std::io; @@ -27,23 +28,24 @@ use std::path::{Path, PathBuf}; use clap::parser::ValueSource; use clap::{Arg, ArgMatches, Command, CommandFactory as _, FromArgMatches as _}; +use strum::Display; use thiserror::Error; use toml::{Table, Value}; -use crate::base_directories; use crate::commands::{ClientArgs, CommandError, GlobalArgs}; +use crate::{PREFIX, base_directories}; const FILE_NAME: &str = "sush.toml"; /// One profile's settings, ready to become option defaults. -#[derive(Debug, Default)] +#[derive(Clone, Debug, Default)] pub struct ConfigOptions { values: BTreeMap>, } impl ConfigOptions { /// Read `path`, or the default file if it exists. - pub fn load(path: Option, profile: Option) -> Result { + fn load(path: Option, profile: Option) -> Result { let path = path.or_else(|| base_directories().find_config_file(FILE_NAME)); let Some(path) = path else { return match profile { @@ -58,6 +60,17 @@ impl ConfigOptions { Self::from_toml(path, &text, profile) } + /// Load the config file and profile that `supplied` and the + /// environment select. + pub fn from_supplied(supplied: &SuppliedOptions) -> Result { + let words = supplied.command_line(); + let matches = parse_with_config(GlobalArgs::command(), words, &Self::default())?; + let GlobalArgs { + config, profile, .. + } = GlobalArgs::from_arg_matches(&matches)?; + Ok(Self::load(config, profile)?) + } + fn from_toml(path: PathBuf, text: &str, profile: Option) -> Result { let table = toml::from_str::
(text).map_err(|error| ConfigError::Parse { path: path.clone(), @@ -93,6 +106,70 @@ impl ConfigOptions { fn values_for(&self, arg: &Arg) -> Option<&Vec> { arg_keys(arg).find_map(|key| self.values.get(key)) } + + /// Describe each option that has a value from the command line, + /// the environment, or this config, as `key = value (source)`. + /// The values of global options come from `matches`. Subcommand + /// options take their values from the environment or this config. + pub fn describe(&self, matches: &ArgMatches) -> Vec { + let command = ClientArgs::command(); + let mut described = BTreeSet::new(); + let mut lines = Vec::new(); + for arg in all_arguments(&command) { + let Some(long_name) = arg.get_long() else { + continue; + }; + if !described.insert(long_name) { + continue; + } + let value = match matches.try_get_raw(arg_id(arg)) { + Ok(_) => self.parsed_value(arg, matches), + Err(_) => self.unparsed_value(arg), + }; + if let Some((values, source)) = value { + lines.push(format!("{long_name} = {values} ({source})")); + } + } + lines + } + + /// Find the value of `arg` in `matches`, and its source. + fn parsed_value(&self, arg: &Arg, matches: &ArgMatches) -> Option<(String, OptionSource)> { + let source = match matches.value_source(arg_id(arg))? { + ValueSource::CommandLine => OptionSource::CommandLine, + ValueSource::EnvVariable => OptionSource::Environment, + _ if self.values_for(arg).is_none() => return None, + _ => OptionSource::ConfigFile, + }; + Some((display_values(arg, matches), source)) + } + + /// Find the value of a configurable `arg` in the environment or + /// this config, and its source. + fn unparsed_value(&self, arg: &Arg) -> Option<(String, OptionSource)> { + if !is_configurable(arg) { + return None; + } + match (arg.get_env().and_then(var_os), self.values_for(arg)) { + (Some(value), _) => Some(( + value.to_string_lossy().into_owned(), + OptionSource::Environment, + )), + (None, Some(values)) => Some((values.join(", "), OptionSource::ConfigFile)), + (None, None) => None, + } + } +} + +/// Where an option's value comes from. +#[derive(Clone, Copy, Debug, Display)] +enum OptionSource { + #[strum(to_string = "command line")] + CommandLine, + #[strum(to_string = "environment")] + Environment, + #[strum(to_string = "config file")] + ConfigFile, } /// Convert the entries of `table` to option values. Fails on @@ -200,9 +277,15 @@ impl UnmergedArgs { } } +/// An option is configurable if it has an environment variable +/// whose value is not a secret. +fn is_configurable(arg: &Arg) -> bool { + arg.get_env().is_some() && !arg.is_hide_env_values_set() +} + /// List the config file keys that can set `arg`. fn arg_keys(arg: &Arg) -> impl Iterator { - let configurable = arg.get_env().is_some() && !arg.is_hide_env_values_set(); + let configurable = is_configurable(arg); arg.get_long() .into_iter() .chain(arg.get_all_aliases().unwrap_or_default()) @@ -211,15 +294,20 @@ fn arg_keys(arg: &Arg) -> impl Iterator { /// Collect the config file keys valid anywhere in `command`. fn all_keys(command: &Command) -> BTreeSet { - let mut keys = command - .get_arguments() + all_arguments(command) + .into_iter() .flat_map(arg_keys) .map(str::to_owned) - .collect::>(); + .collect() +} + +/// Collect the arguments of `command` and all its subcommands. +fn all_arguments(command: &Command) -> Vec<&Arg> { + let mut args = command.get_arguments().collect::>(); for subcommand in command.get_subcommands() { - keys.extend(all_keys(subcommand)); + args.extend(all_arguments(subcommand)); } - keys + args } fn compute_merge_losers( @@ -261,6 +349,112 @@ fn map_command_tree(command: Command, f: &impl Fn(Command) -> Command) -> Comman f(command).mut_subcommands(|subcommand| map_command_tree(subcommand, f)) } +/// Options explicitly supplied on the command line, by long name, with +/// their values (none for a flag). +#[derive(Clone, Debug, Default)] +pub struct SuppliedOptions(BTreeMap>); + +impl SuppliedOptions { + /// Collect the options of `command` that `matches` shows were + /// supplied on the command line. + pub fn from_matches(command: &Command, matches: &ArgMatches) -> Self { + let mut supplied = BTreeMap::new(); + for arg in command.get_arguments() { + let Some(long_name) = arg.get_long() else { + continue; + }; + if matches.value_source(arg_id(arg)) != Some(ValueSource::CommandLine) { + continue; + } + let values = match arg.get_action().takes_values() { + true => matches + .get_raw(arg_id(arg)) + .into_iter() + .flatten() + .map(ToOwned::to_owned) + .collect(), + false => Vec::new(), + }; + supplied.insert(long_name.to_owned(), values); + } + Self(supplied) + } + + /// Supply the option of `command` named `long_name`, replacing any + /// supplied with that name or one that conflicts with it. + pub fn set(&mut self, command: &Command, long_name: &str, values: Vec) { + for conflicting in conflicting_long_names(command, long_name) { + self.0.remove(&conflicting); + } + self.0.insert(long_name.to_owned(), values); + } + + /// Stop supplying the option named `long_name`. Returns false + /// if that option was not supplied. + pub fn remove(&mut self, long_name: &str) -> bool { + self.0.remove(long_name).is_some() + } + + /// Supply each of the options in `other`, as by [`Self::set`]. + pub fn update(&mut self, command: &Command, other: &Self) { + for (long_name, values) in &other.0 { + self.set(command, long_name, values.clone()); + } + } + + /// Write these options as a command line. + pub fn command_line(&self) -> Vec { + let mut words = vec![OsString::from(PREFIX)]; + for (long_name, values) in &self.0 { + if values.is_empty() { + words.push(OsString::from(format!("--{long_name}"))); + } + for value in values { + let mut word = OsString::from(format!("--{long_name}=")); + word.push(value); + words.push(word); + } + } + words + } +} + +/// List the long names of the options of `command` that conflict with +/// the option named `long_name`. +fn conflicting_long_names(command: &Command, long_name: &str) -> Vec { + let mut names = Vec::new(); + for arg in command.get_arguments() { + for other in command.get_arg_conflicts_with(arg) { + if arg.get_long() == Some(long_name) { + names.extend(other.get_long().map(str::to_owned)); + } + if other.get_long() == Some(long_name) { + names.extend(arg.get_long().map(str::to_owned)); + } + } + } + names +} + +/// Show the values of `arg` in `matches`, hiding secrets. +fn display_values(arg: &Arg, matches: &ArgMatches) -> String { + if arg.is_hide_env_values_set() { + return "(hidden)".to_owned(); + } + if !arg.get_action().takes_values() { + return matches.get_flag(arg_id(arg)).to_string(); + } + let values = matches.get_raw(arg_id(arg)).into_iter().flatten(); + let values = values + .map(|value| value.to_string_lossy()) + .collect::>(); + values.join(", ") +} + +fn arg_id(arg: &Arg) -> &str { + arg.get_id().as_str() +} + fn arg_values(value: Value) -> Option> { let scalar = |value| match value { Value::String(value) => Some(value), @@ -308,6 +502,7 @@ pub enum ConfigError { mod test { use std::env::{remove_var, set_var}; use std::fs::write; + use std::iter::once; use rusty_fork::rusty_fork_test; use tempfile::NamedTempFile; @@ -347,6 +542,65 @@ mod test { GlobalArgs::from_arg_matches(&parse(text, env, words).unwrap()).unwrap() } + /// Collect the global options supplied in `words`, ignoring the + /// environment. + fn supplied(words: &str) -> SuppliedOptions { + let command = map_command_tree(GlobalArgs::command(), &|command| { + command.mut_args(|arg| arg.env(None)) + }); + let words = once("sush").chain(words.split_whitespace()); + let matches = command.clone().try_get_matches_from(words).unwrap(); + SuppliedOptions::from_matches(&command, &matches) + } + + fn command_line(supplied: &SuppliedOptions) -> Vec { + let words = supplied.command_line().into_iter(); + words.map(|word| word.into_string().unwrap()).collect() + } + + /// Supplied options come back out as a command line with a flag, a + /// value, and a repeated value, and that command line parses back to + /// the same options. + #[test] + fn supplied_command_line() { + let options = supplied("--offline --rack r --nexus-root a.pem --nexus-root b.pem"); + let words = [ + "sush", + "--nexus-root=a.pem", + "--nexus-root=b.pem", + "--offline", + "--rack=r", + ]; + assert_eq!(command_line(&options), words); + assert_eq!(command_line(&supplied(&words[1..].join(" "))), words); + } + + /// Supplying an option replaces any supplied option that conflicts + /// with it, in either direction, and keeps the rest. + #[test] + fn supplied_update() { + let command = GlobalArgs::command(); + let mut options = supplied("--nexus https://nexus --rack r"); + options.update(&command, &supplied("--url https://url")); + assert_eq!( + command_line(&options), + ["sush", "--rack=r", "--url=https://url"], + "url replaces nexus, which declares the conflict" + ); + options.update(&command, &supplied("--offline")); + assert_eq!( + command_line(&options), + ["sush", "--offline", "--rack=r"], + "offline replaces url" + ); + options.update(&command, &supplied("--rack s")); + assert_eq!( + command_line(&options), + ["sush", "--offline", "--rack=s"], + "a new value replaces the old" + ); + } + rusty_fork_test! { /// The file's `profile` key selects a profile, and `--profile` or /// `SUSH_PROFILE` overrides that choice. diff --git a/client/src/context.rs b/client/src/context.rs index 49036559..b91b9f44 100644 --- a/client/src/context.rs +++ b/client/src/context.rs @@ -24,6 +24,7 @@ use sush_common::version::VersionInfo; use crate::AuthzSigner; use crate::commands::{CommandError, GlobalArgs}; +use crate::config::SuppliedOptions; /// Authorization state: the credentials that authenticated us, and the /// ephemeral key that binds each request we make. @@ -116,7 +117,15 @@ pub trait CommandContext: Clone + Send + Sync { fn set_output_format(&mut self, output: OutputFormat); fn get_globals(&self) -> GlobalArgs; fn set_globals(&mut self, _args: GlobalArgs) {} - fn pre_parse_hook(&mut self, _command: &str) {} + fn set_options(&mut self, _options: SuppliedOptions) -> Result<(), CommandError> { + Err(CommandError::ReplOnly("set")) + } + fn unset_options(&mut self, _long_names: &[String]) -> Result<(), CommandError> { + Err(CommandError::ReplOnly("unset")) + } + fn last_job_id(&self) -> Option { + None + } // Errors fn command_error(&mut self, error: &CommandError); diff --git a/client/src/repl.rs b/client/src/repl.rs index b4e7b16a..c6b7cfbd 100644 --- a/client/src/repl.rs +++ b/client/src/repl.rs @@ -7,12 +7,11 @@ //! Inherits most of its behavior from the (non-interactive) CLI. use std::collections::BTreeSet; -use std::env; use std::ffi::OsString; use std::fs::{Permissions, set_permissions}; use std::os::unix::fs::PermissionsExt as _; -use clap::{CommandFactory as _, FromArgMatches as _, Parser}; +use clap::{ArgMatches, CommandFactory as _, FromArgMatches as _, Parser}; use rustyline::DefaultEditor; use rustyline::error::ReadlineError; use shlex::split as split_command; @@ -29,13 +28,10 @@ use sush_common::targets::{SledId, SledVersion}; use sush_common::version::VersionInfo; use crate::cli::Cli; -use crate::commands::{ - ClientCommand, CommandError, GlobalArgs, SSH_AUTH_SOCK, SUSH_JOB_ID, SUSH_KEY_ID, - SUSH_OUTPUT_FORMAT, SUSH_URL, -}; -use crate::config::{ConfigOptions, parse_with_config}; +use crate::commands::{ClientCommand, CommandError, GlobalArgs}; +use crate::config::{ConfigOptions, SuppliedOptions, parse_with_config}; use crate::context::{CommandContext, OutputFormat, StartedDisplay, StatusDisplayStyle}; -use crate::{AuthzSigner, Client, PREFIX, base_directories}; +use crate::{AuthzSigner, PREFIX, base_directories}; const HISTORY_FILE: &str = "history.txt"; @@ -48,42 +44,48 @@ struct ReplCommandParser { impl ReplCommandParser { /// Parse `words`, taking defaults from `options`. - fn try_parse( - ctx: &mut C, - words: I, - options: &ConfigOptions, - ) -> Result + fn try_parse(words: I, options: &ConfigOptions) -> Result where I: IntoIterator, - T: Into + Clone, - C: CommandContext, + T: Into, { - let mut words = words.into_iter().peekable(); - if let Some(command) = words.peek() - && let Ok(command) = command.clone().into().as_os_str().try_into() + let command = Self::command(); + let matches = parse_with_config(command.clone(), words, options)?; + let mut parsed = Self::from_arg_matches(&matches)?; + if let ClientCommand::Set { args } = &mut parsed.command + && let Some((name, matches)) = matches.subcommand() + && let Some(set) = command.find_subcommand(name) { - ctx.pre_parse_hook(command); + args.supplied = SuppliedOptions::from_matches(set, matches); } - let matches = parse_with_config(Self::command(), words, options)?; - Ok(Self::from_arg_matches(&matches)?) + Ok(parsed) } } /// Interactive REPL context. #[derive(Clone, Debug, Default)] pub struct Repl { + /// The underlying CLI context. Most methods forward to this. cli: Cli, + + /// Configuration options for the selected profile. + config: ConfigOptions, + + /// Global options explicitly supplied on the command line. + supplied: SuppliedOptions, + + /// The default for job ID arguments. + last_job_id: Option, } impl Repl { - pub async fn run( - mut self, - args: GlobalArgs, - _client: Option, - ) -> Result<(), CommandError> { - self.set_globals(args.clone()); + pub async fn run(mut self, args: GlobalArgs) -> Result<(), CommandError> { self.cli.load_session(); - let options = ConfigOptions::load(args.config, args.profile)?; + self.config = ConfigOptions::from_supplied(&args.supplied)?; + self.supplied = args.supplied; + let (globals, _) = parse_globals(&self.supplied, &self.config)?; + self.cli.set_globals(globals); + self.show_options()?; let history_file = base_directories() .place_state_file(HISTORY_FILE) .map_err(|error| CommandError::io(HISTORY_FILE, error))?; @@ -105,7 +107,7 @@ impl Repl { let Some(words) = split_command(&command) else { perr!("❌ Invalid quoting in command"); }; - let command = match ReplCommandParser::try_parse(&mut self, &words, &options) { + let command = match ReplCommandParser::try_parse(&words, &self.config) { Ok(ReplCommandParser { command }) => command, Err(err) => { self.command_error(&err); @@ -129,31 +131,52 @@ impl Repl { Ok(()) } - /// Get the default job ID from the `SUSH_JOB_ID` environment variable. - #[allow(unused)] - fn default_job_id(&self) -> Option { - env::var(SUSH_JOB_ID) - .ok() - .and_then(|job_id| job_id.parse().ok()) + fn apply_options(&mut self, supplied: SuppliedOptions) -> Result<(), CommandError> { + let config = ConfigOptions::from_supplied(&supplied)?; + let old = self.get_globals(); + let (globals, _) = parse_globals(&supplied, &config)?; + if (&old.ssh_auth_sock, &old.ssh_key_id) != (&globals.ssh_auth_sock, &globals.ssh_key_id) { + self.set_credentials(None); + self.authz_signer().clear_claim(); + } + self.supplied = supplied; + self.config = config; + self.cli.set_globals(globals); + self.show_options() } - /// (Un)set the `SUSH_JOB_ID` environment variable as a default for - /// job ID arguments. - fn set_job_id(&mut self, job_id: Option) { - if let Some(job_id) = job_id { - unsafe { env::set_var(SUSH_JOB_ID, job_id.to_string()) } - } else { - unsafe { env::remove_var(SUSH_JOB_ID) } + /// Print each global option that has a value, and its source. + fn show_options(&self) -> Result<(), CommandError> { + let (_, matches) = parse_globals(&self.supplied, &self.config)?; + for line in self.config.describe(&matches) { + eprintln!("✅ {line}"); } + Ok(()) } - /// Make a prompt indicating online/offline status. + /// Make a prompt indicating the profile and online/offline status. fn prompt(&self, args: &GlobalArgs) -> String { + let profile = match &args.profile { + Some(profile) => format!(" [{profile}]"), + None => String::new(), + }; let offline = if args.url.is_some() { "" } else { " (offline)" }; - format!("{PREFIX}{offline}# ") + format!("{PREFIX}{profile}{offline}# ") } } +/// Parse the global options in `supplied`, as if typed on the command +/// line, over the environment and `options`. +fn parse_globals( + supplied: &SuppliedOptions, + options: &ConfigOptions, +) -> Result<(GlobalArgs, ArgMatches), CommandError> { + let matches = parse_with_config(GlobalArgs::command(), supplied.command_line(), options)?; + let mut globals = GlobalArgs::from_arg_matches(&matches)?; + globals.supplied = supplied.clone(); + Ok((globals, matches)) +} + /// Most of these methods simply punt to those of [`Cli`] for display. /// But we do update local (ephemeral) state in response to some commands. impl CommandContext for Repl { @@ -171,58 +194,31 @@ impl CommandContext for Repl { self.cli.get_globals() } - fn set_globals(&mut self, mut args: GlobalArgs) { - let GlobalArgs { - mut output, - json, - text, - mut url, - offline, - ssh_auth_sock, - ssh_key_id, - #[cfg(feature = "permslip")] - permslip_ssh: _, - proxy_roots: _, - nexus: _, - nexus_token: _, - nexus_roots: _, - nexus_resolve: _, - rack: _, - config: _, - profile: _, - } = args; - if json { - output = Some(OutputFormat::Json); - } else if text { - output = Some(OutputFormat::Text); - } - if offline { - url = None; - } - if ssh_auth_sock.is_some() || ssh_key_id.is_some() { - self.set_credentials(None); - self.authz_signer().clear_claim(); - } + fn last_job_id(&self) -> Option { + self.last_job_id + } - macro_rules! set_or_unset { - ($arg:ident, $var:expr, $name:literal) => {{ - if let Some(arg) = $arg { - unsafe { env::set_var($var, arg.to_string()) } - args.$arg = Some(arg.clone()); - eprintln!("✅ {} set to `{}`", $name, arg); - } else { - unsafe { env::remove_var($var) } - args.$arg = None; - eprintln!("✅ {} unset", $name); - } - }}; - } - set_or_unset!(output, SUSH_OUTPUT_FORMAT, "Output format"); - set_or_unset!(url, SUSH_URL, "Server URL"); - set_or_unset!(ssh_auth_sock, SSH_AUTH_SOCK, "SSH agent socket"); - set_or_unset!(ssh_key_id, SUSH_KEY_ID, "SSH key ID"); + fn set_options(&mut self, options: SuppliedOptions) -> Result<(), CommandError> { + let mut supplied = self.supplied.clone(); + supplied.update(&GlobalArgs::command(), &options); + self.apply_options(supplied) + } - self.cli.set_globals(args); + fn unset_options(&mut self, long_names: &[String]) -> Result<(), CommandError> { + let command = GlobalArgs::command(); + let mut supplied = self.supplied.clone(); + for long_name in long_names { + if !command + .get_arguments() + .any(|arg| arg.get_long() == Some(long_name)) + { + return Err(CommandError::UnknownOption(long_name.to_owned())); + } + if !supplied.remove(long_name) { + return Err(CommandError::OptionNotSet(long_name.to_owned())); + } + } + self.apply_options(supplied) } // Errors @@ -326,12 +322,12 @@ impl CommandContext for Repl { sleds: Option<&BTreeSet>, display: StartedDisplay, ) { - self.set_job_id(Some(job.job_id().to_owned())); + self.last_job_id = Some(*job.job_id()); self.cli.job_started(job, sleds, display); } fn job_stopped(&mut self, job_id: &JobId) { - self.set_job_id(Some(job_id.to_owned())); + self.last_job_id = Some(*job_id); self.cli.job_stopped(job_id); } @@ -347,7 +343,7 @@ impl CommandContext for Repl { output: &[u8], binary: bool, ) { - self.set_job_id(Some(job_id.to_owned())); + self.last_job_id = Some(*job_id); self.cli.job_output(job_id, target, stream, output, binary); } @@ -358,7 +354,7 @@ impl CommandContext for Repl { stage: &str, total: u64, ) { - self.set_job_id(Some(job_id.to_owned())); + self.last_job_id = Some(*job_id); self.cli.job_output_started(job_id, stream, stage, total); } @@ -416,7 +412,7 @@ impl CommandContext for Repl { } fn job_status(&mut self, job_id: &JobId, status: &JobStatusMap, style: StatusDisplayStyle) { - self.set_job_id(Some(job_id.to_owned())); + self.last_job_id = Some(*job_id); self.cli.job_status(job_id, status, style); } From b3b8e88068755d34f9ce7863de21674a750cc6ab Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 30 Oct 2026 11:55:36 -0600 Subject: [PATCH 4/7] Start the REPL by default Co-authored-by: Claude --- client/src/commands.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/client/src/commands.rs b/client/src/commands.rs index 1cd0a0dc..5826818c 100644 --- a/client/src/commands.rs +++ b/client/src/commands.rs @@ -272,7 +272,7 @@ pub struct ClientArgs { globals: GlobalArgs, #[clap(subcommand)] - command: ClientCommand, + command: Option, } impl ClientArgs { @@ -294,7 +294,7 @@ impl ClientArgs { pub async fn execute(&mut self, ctx: &mut impl CommandContext) -> Result<(), CommandError> { ctx.set_globals(self.globals.clone()); - self.command.clone().execute(ctx).await + self.command.clone().unwrap_or_default().execute(ctx).await } } @@ -345,7 +345,7 @@ fn parallel_chunks(s: &str) -> Result { } /// Support Shell job management command -#[derive(Clone, Debug, Subcommand)] +#[derive(Clone, Debug, Default, Subcommand)] pub enum ClientCommand { /// Job-signing certificate management. Cert { @@ -392,8 +392,9 @@ pub enum ClientCommand { options: Vec, }, - /// Start an interactive REPL. + /// Start an interactive REPL (the default). #[clap(alias = "repl")] + #[default] Shell, /// Report client and sled build versions. From b17589b656a20d2ddb0075d3af50eb12b76a0fdf Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 30 Oct 2026 13:07:54 -0600 Subject: [PATCH 5/7] Rework online command handling Co-authored-by: Claude --- client/src/commands.rs | 252 ++++++++++++++++++++++++++++------------- client/src/config.rs | 17 +-- client/src/context.rs | 10 +- client/src/repl.rs | 21 +++- client/src/tunnel.rs | 1 + 5 files changed, 208 insertions(+), 93 deletions(-) diff --git a/client/src/commands.rs b/client/src/commands.rs index 5826818c..a92c5135 100644 --- a/client/src/commands.rs +++ b/client/src/commands.rs @@ -111,6 +111,36 @@ const JOB_STOP_RETRY_INTERVAL: Duration = Duration::from_millis(100); #[cfg(feature = "permslip")] const SIGNING_UPDATE_INTERVAL: Duration = Duration::from_millis(100); +/// Properties of client commands. +trait ClientCommandProperties { + /// Whether a command contacts the server. + fn is_online(&self) -> bool { + true + } + + /// Whether a command handles SIGINT itself. + fn handles_sigint(&self) -> bool { + false + } +} + +/// An omitted subcommand has the properties of its default. +impl ClientCommandProperties for Option { + fn is_online(&self) -> bool { + match self { + Some(command) => command.is_online(), + None => T::default().is_online(), + } + } + + fn handles_sigint(&self) -> bool { + match self { + Some(command) => command.handles_sigint(), + None => T::default().handles_sigint(), + } + } +} + /// Process limits for job execution. #[derive(Clone, Debug, Parser)] pub struct LimitArgs { @@ -251,16 +281,6 @@ impl GlobalArgs { self.output } } - - /// Point these options at an open tunnel's local `url` instead of - /// Nexus. The REPL re-enters `run` for each command and parses its - /// supplied options each time, so they must name the tunnel too; - /// otherwise each command would open a tunnel of its own. - fn use_tunnel(&mut self, url: &str) { - self.url = Some(url.to_owned()); - self.nexus = None; - self.supplied.set(&Self::command(), "url", vec![url.into()]); - } } #[derive(Debug, Parser)] @@ -358,10 +378,6 @@ pub enum ClientCommand { Iam { #[clap(subcommand)] command: Option, - - /// Shortcut for `iam available` - #[arg(short = 'l')] - available: bool, }, /// Session management. @@ -418,10 +434,13 @@ pub enum CertCommand { }, } +impl ClientCommandProperties for CertCommand {} + #[derive(Clone, Debug, Default, Subcommand)] pub enum IdentityCommand { /// List available local SSH agent identities. /// Does not communicate with the server. + #[command(short_flag = 'l')] Available, /// List SSH identities registered with the server. @@ -432,6 +451,12 @@ pub enum IdentityCommand { Login, } +impl ClientCommandProperties for IdentityCommand { + fn is_online(&self) -> bool { + !matches!(self, Self::Available) + } +} + #[derive(Clone, Debug, Subcommand)] pub enum SessionCommand { /// Grant a key attach access to this session's interactive jobs. @@ -524,6 +549,8 @@ impl Default for SessionCommand { } } +impl ClientCommandProperties for SessionCommand {} + #[derive(Clone, Debug, Subcommand)] pub enum JobCommand { /// Sign and start a job. @@ -606,6 +633,12 @@ pub enum JobCommand { }, } +impl ClientCommandProperties for JobCommand { + fn handles_sigint(&self) -> bool { + matches!(self, Self::Start { .. } | Self::Attach { .. }) + } +} + #[derive(Clone, Debug, Parser)] pub struct JobStartArgs { #[clap(flatten)] @@ -671,23 +704,32 @@ impl JobStartArgs { } } -impl ClientCommand { - /// Whether a command handles SIGINT itself. - fn handles_sigint(&self) -> bool { - matches!( - self, - Self::Shell - | Self::Job { - command: JobCommand::Start { .. } | JobCommand::Attach { .. }, - } - ) +impl ClientCommandProperties for ClientCommand { + fn is_online(&self) -> bool { + match self { + Self::Cert { command } => command.is_online(), + Self::Iam { command } => command.is_online(), + Self::Session { command } => command.is_online(), + Self::Job { command } => command.is_online(), + Self::Shell | Self::Version | Self::Set { .. } | Self::Unset { .. } | Self::Quit => { + false + } + } } - /// Whether a command contacts the server. - fn needs_server(&self) -> bool { - !matches!(self, Self::Set { .. } | Self::Unset { .. } | Self::Quit) + fn handles_sigint(&self) -> bool { + match self { + Self::Cert { command } => command.handles_sigint(), + Self::Iam { command } => command.handles_sigint(), + Self::Session { command } => command.handles_sigint(), + Self::Job { command } => command.handles_sigint(), + Self::Shell => true, + Self::Version | Self::Set { .. } | Self::Unset { .. } | Self::Quit => false, + } } +} +impl ClientCommand { /// Run the command, letting an interrupt cancel it unless the /// command handles SIGINT itself. #[async_recursion(?Send)] @@ -702,67 +744,26 @@ impl ClientCommand { } async fn run(self, ctx: &mut impl CommandContext) -> Result<(), CommandError> { - let mut args = ctx.get_globals().to_owned(); + let args = ctx.get_globals(); if let Some(output) = args.output_format() { ctx.set_output_format(output); } - // The tunnel stands in for the proxy until the command ends. - let _tunnel = match args.nexus.as_ref() { - Some(nexus) if self.needs_server() => { - let rack = args.rack.as_ref().ok_or(CommandError::MissingRack)?; - let token = args - .nexus_token - .as_ref() - .ok_or(CommandError::MissingNexusToken)?; - let tunnel = - Tunnel::start(nexus, rack, token, &args.nexus_roots, args.nexus_resolve) - .await?; - args.use_tunnel(&tunnel.url); - Some(tunnel) - } - _ => None, - }; - - let client = match args.url.as_ref() { - Some(url) if self.needs_server() => { - let roots = if args.proxy_roots.is_empty() { - tls::platform_roots()? - } else { - let mut roots = Vec::new(); - for path in &args.proxy_roots { - let pem = read(path).map_err(|err| CommandError::io(path, err))?; - roots.push(Certificate::from_pem(&pem)?); - } - roots - }; - { - let (url, resolve) = tls::descope_url(url)?; - Some(Client::new_with_client( - &url, - tls::client(roots, resolve)?, - ctx.authz_signer(), - )) - } - } - _ => None, + let handle = match self.is_online() { + true => ctx.client_handle(&args).await?, + false => None, }; + let client = handle.as_ref().map(|handle| handle.client.clone()); match (self, client) { (ClientCommand::Cert { command }, Some(client)) => cert(ctx, &client, command).await, - (ClientCommand::Iam { command, available }, Some(client)) => { + (ClientCommand::Iam { command }, client) => { iam( ctx, - &client, + client.as_ref(), &args.ssh_auth_sock, &args.ssh_key_id, - command.unwrap_or_else(|| { - if available { - IdentityCommand::Available - } else { - IdentityCommand::default() - } - }), + command.unwrap_or_default(), ) .await } @@ -782,8 +783,12 @@ impl ClientCommand { Ok(()) } - (ClientCommand::Version, client) => { - let (server, sleds) = match client { + (ClientCommand::Version, _) => { + let handle = ctx.client_handle(&args).await.unwrap_or_else(|error| { + ctx.command_error(&error); + None + }); + let (server, sleds) = match handle.as_ref().map(|handle| &handle.client) { Some(client) => ( Some(client.version().send().await?.into_inner()), client @@ -806,6 +811,56 @@ impl ClientCommand { } } +/// A client and the tunnel it goes through, if any. +/// Dropping the handle closes the tunnel. +#[derive(Debug)] +pub struct ClientHandle { + pub client: Client, + _tunnel: Option, +} + +impl ClientHandle { + /// Make a client and maybe a tunnel, or nothing if offline. + pub async fn from_args( + args: &GlobalArgs, + signer: AuthzSigner, + ) -> Result, CommandError> { + let (url, tunnel) = match &args.nexus { + Some(nexus) => { + let rack = args.rack.as_ref().ok_or(CommandError::MissingRack)?; + let token = args + .nexus_token + .as_ref() + .ok_or(CommandError::MissingNexusToken)?; + let tunnel = + Tunnel::start(nexus, rack, token, &args.nexus_roots, args.nexus_resolve) + .await?; + (Some(tunnel.url.clone()), Some(tunnel)) + } + None => (args.url.clone(), None), + }; + let Some(url) = url else { + return Ok(None); + }; + let roots = if args.proxy_roots.is_empty() { + tls::platform_roots()? + } else { + let mut roots = Vec::new(); + for path in &args.proxy_roots { + let pem = read(path).map_err(|err| CommandError::io(path, err))?; + roots.push(Certificate::from_pem(&pem)?); + } + roots + }; + let (url, resolve) = tls::descope_url(&url)?; + let client = Client::new_with_client(&url, tls::client(roots, resolve)?, signer); + Ok(Some(Self { + client, + _tunnel: tunnel, + })) + } +} + async fn authenticate( ctx: &mut impl CommandContext, client: &Client, @@ -914,7 +969,7 @@ where async fn iam( ctx: &mut impl CommandContext, - client: &Client, + client: Option<&Client>, ssh_auth_sock: &Option, ssh_key_id: &Option, command: IdentityCommand, @@ -939,6 +994,7 @@ async fn iam( } IdentityCommand::List => { + let client = client.ok_or(CommandError::Offline)?; let identities = with_login(ctx, client, async || client.identities().send().await) .await? .into_inner(); @@ -949,6 +1005,7 @@ async fn iam( } IdentityCommand::Login => { + let client = client.ok_or(CommandError::Offline)?; let identity = with_login(ctx, client, async || client.iam().body(None).send().await) .await? .into_inner(); @@ -2820,6 +2877,8 @@ impl From> for CommandError { #[cfg(test)] mod tests { + use std::iter::once; + use chrono::Utc; use super::*; @@ -2952,6 +3011,41 @@ mod tests { ClientArgs::command().debug_assert(); } + /// Only commands that contact the server are online, and only the + /// REPL, `job start`, and `job attach` handle SIGINT. `sush` with no + /// command starts the REPL. + #[test] + fn command_properties() { + let command = |words: &str| { + let words = once("sush").chain(words.split_whitespace()); + let args = ClientArgs::try_parse_from(words).unwrap(); + args.command.unwrap_or_default() + }; + for words in ["iam", "iam list", "job status"] { + assert!(command(words).is_online(), "`{words}` is online"); + } + for words in [ + "", + "iam -l", + "iam available", + "set --rack r", + "unset rack", + "version", + "quit", + ] { + assert!(!command(words).is_online(), "`{words}` is offline"); + } + for words in ["", "job start uptime", "job attach"] { + assert!(command(words).handles_sigint(), "`{words}` handles SIGINT"); + } + for words in ["job status", "iam", "session", "version"] { + assert!( + !command(words).handles_sigint(), + "`{words}` leaves SIGINT alone" + ); + } + } + #[test] fn login_refused_blames_build_skew() { let refused = || CommandError::Client(String::from("Authentication required, try `iam`")); diff --git a/client/src/config.rs b/client/src/config.rs index f339ef45..42f11b9f 100644 --- a/client/src/config.rs +++ b/client/src/config.rs @@ -380,25 +380,20 @@ impl SuppliedOptions { Self(supplied) } - /// Supply the option of `command` named `long_name`, replacing any - /// supplied with that name or one that conflicts with it. - pub fn set(&mut self, command: &Command, long_name: &str, values: Vec) { - for conflicting in conflicting_long_names(command, long_name) { - self.0.remove(&conflicting); - } - self.0.insert(long_name.to_owned(), values); - } - /// Stop supplying the option named `long_name`. Returns false /// if that option was not supplied. pub fn remove(&mut self, long_name: &str) -> bool { self.0.remove(long_name).is_some() } - /// Supply each of the options in `other`, as by [`Self::set`]. + /// Supply each of the options of `command` in `other`, replacing any + /// supplied with the same name or one that conflicts with it. pub fn update(&mut self, command: &Command, other: &Self) { for (long_name, values) in &other.0 { - self.set(command, long_name, values.clone()); + for conflicting in conflicting_long_names(command, long_name) { + self.0.remove(&conflicting); + } + self.0.insert(long_name.clone(), values.clone()); } } diff --git a/client/src/context.rs b/client/src/context.rs index b91b9f44..c44f68ad 100644 --- a/client/src/context.rs +++ b/client/src/context.rs @@ -23,7 +23,7 @@ use sush_common::targets::{SledId, SledVersion}; use sush_common::version::VersionInfo; use crate::AuthzSigner; -use crate::commands::{CommandError, GlobalArgs}; +use crate::commands::{ClientHandle, CommandError, GlobalArgs}; use crate::config::SuppliedOptions; /// Authorization state: the credentials that authenticated us, and the @@ -140,6 +140,14 @@ pub trait CommandContext: Clone + Send + Sync { // Session management fn authz_signer(&self) -> AuthzSigner; + #[expect(async_fn_in_trait, reason = "commands run as non-`Send` futures")] + async fn client_handle( + &mut self, + args: &GlobalArgs, + ) -> Result>, CommandError> { + let handle = ClientHandle::from_args(args, self.authz_signer()).await?; + Ok(handle.map(Arc::new)) + } fn get_credentials(&self) -> Option { self.authz_signer().get() } diff --git a/client/src/repl.rs b/client/src/repl.rs index c6b7cfbd..48f87ccb 100644 --- a/client/src/repl.rs +++ b/client/src/repl.rs @@ -10,6 +10,7 @@ use std::collections::BTreeSet; use std::ffi::OsString; use std::fs::{Permissions, set_permissions}; use std::os::unix::fs::PermissionsExt as _; +use std::sync::Arc; use clap::{ArgMatches, CommandFactory as _, FromArgMatches as _, Parser}; use rustyline::DefaultEditor; @@ -28,7 +29,7 @@ use sush_common::targets::{SledId, SledVersion}; use sush_common::version::VersionInfo; use crate::cli::Cli; -use crate::commands::{ClientCommand, CommandError, GlobalArgs}; +use crate::commands::{ClientCommand, ClientHandle, CommandError, GlobalArgs}; use crate::config::{ConfigOptions, SuppliedOptions, parse_with_config}; use crate::context::{CommandContext, OutputFormat, StartedDisplay, StatusDisplayStyle}; use crate::{AuthzSigner, PREFIX, base_directories}; @@ -76,6 +77,9 @@ pub struct Repl { /// The default for job ID arguments. last_job_id: Option, + + /// The client handle each command reuses until the options change. + client_handle: Option>, } impl Repl { @@ -141,6 +145,7 @@ impl Repl { } self.supplied = supplied; self.config = config; + self.client_handle = None; self.cli.set_globals(globals); self.show_options() } @@ -160,7 +165,8 @@ impl Repl { Some(profile) => format!(" [{profile}]"), None => String::new(), }; - let offline = if args.url.is_some() { "" } else { " (offline)" }; + let online = args.url.is_some() || args.nexus.is_some(); + let offline = if online { "" } else { " (offline)" }; format!("{PREFIX}{profile}{offline}# ") } } @@ -244,6 +250,17 @@ impl CommandContext for Repl { self.cli.authz_signer() } + async fn client_handle( + &mut self, + args: &GlobalArgs, + ) -> Result>, CommandError> { + if self.client_handle.is_none() { + let handle = ClientHandle::from_args(args, self.authz_signer()).await?; + self.client_handle = handle.map(Arc::new); + } + Ok(self.client_handle.clone()) + } + fn session_id(&self) -> Option { self.cli.session_id() } diff --git a/client/src/tunnel.rs b/client/src/tunnel.rs index d3871524..dd75a505 100644 --- a/client/src/tunnel.rs +++ b/client/src/tunnel.rs @@ -81,6 +81,7 @@ struct Target { /// A running tunnel. Dropping it closes the listener and every /// connection in flight. +#[derive(Debug)] pub struct Tunnel { /// The loopback URL standing in for the sush proxy. pub url: String, From ea6629c0755ce47fdb8443a2a4007577bea7cc21 Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Fri, 30 Oct 2026 15:04:53 -0600 Subject: [PATCH 6/7] Update README for recent changes Co-authored-by: Claude --- README.md | 82 ++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 665d1fcb..44ae14ff 100644 --- a/README.md +++ b/README.md @@ -70,28 +70,44 @@ Aug 09 03:41:44.594 INFO listening, local_addr: 0.0.0.0:44444 > On NixOS you will have to pass the `--insecure-disable-path-isolation` flag to > the server, otherwise spawning commands will fail. -In another terminal, point the client at the server and start it up -in REPL mode. Signing job requests needs a client built with the -`permslip` feature and a reachable Permission Slip instance holding -your signing key: +In another terminal, point the client at the server and start it up: ``` $ export PERMSLIP_URL="https://your-permslip-server.example" $ export SUSH_PERMSLIP_KEY="Your Signing Key" $ export SUSH_URL="http://localhost:44444" -$ cargo run --bin=sush --features permslip -- repl - Running `target/debug/sush repl` -✅ Output format set to `text` -✅ Server URL set to `http://localhost:44444` -✅ SSH agent socket set to `/run/user/1000/ssh-agent.sock` -✅ SSH key ID unset +$ cargo run --bin=sush --features permslip + Running `target/debug/sush` +✅ url = http://localhost:44444 (environment) +✅ ssh-auth-sock = /run/user/1000/ssh-agent.sock (environment) sush# ``` -We can explicitly start a session with `session start`, or we can -let `job start` implicitly start one for us. The server automatically -sets some environment variables for job processes, so as a simple -test try running the command `echo $SUSH_JOB_ID`: +With no command, `sush` starts its interactive REPL (aka shell). +Try `help` for a list of commands, or any `sush` sub-command, +such as `session`, `job`, etc. The REPL offers some convenient +features for running multiple jobs, such as transparent use of +the previous job ID, Nexus tunnel reuse, command history, etc. +Exit the REPL with `quit` or Ctrl-D. + +You can begin a session with `session start`. You may be prompted +to touch your authentication token (e.g., YubiKey) and to authenticate +to the signing service: + +``` +sush# session start +👋 Please confirm user presence to sign with key `excite-cabbage-reject-wave-learn-police-that-glad` +🔑 Open this URL in your browser to authenticate to the signing service: +https://your-permslip-server.example/google/07c339c687e9ac919af7ab2ac5025881 +✅ Created session + Session ID: slush-olympic-universe-ill-total-risk-host-suggest + Signer nonce: private-else-nut-erosion-trend-tube-token-man + Sequence number: borrow-session-iron-author +``` + +Now you're ready to run a job. The server automatically sets some +environment variables for job processes, so as a simple test try +running the command `echo $SUSH_JOB_ID`: ``` sush# job start --wait "echo $SUSH_JOB_ID" @@ -134,5 +150,39 @@ of cubby numbers, serial numbers, and full baseboard IDs are also accepted. Interactive and streaming jobs must target exactly one sled, which defaults to the sled they are talking to. -The `version` command shows which versions of `sush` are running where -in the rack. See `help` for a list of other commands. +The `version` command shows which versions of `sush` are running on +which sleds, as well as a visual indication of their health. Green means +the `sush` server is running and connected to the internal gossip network; +yellow means the server couldn't connect to the gossip network; and red +means that no `sush` server is running there at all. + +## Configuration files and the options system + +The `sush` client reads a configuration file in TOML format from the +standard XDG directories, e.g., `~/.config/sush/sush.toml`, or wherever +you point `--config` to. In such files, the keys are long option names, +and the values provide defaults for those options. + +Top-level options provide global defaults, while tables in the config file +let you organize sets of options into named *profiles*, e.g., for different +environments. You can choose a profile via a top-level `profile` key, or +the `SUSH_PROFILE` environment variable, or the `--profile` global option: + +```toml +permslip-url = "https://your-permslip-server.example" +permslip-key = "Your Signing Key" +profile = "local" + +[local] +url = "http://localhost:44444" +permslip-key = "Local Signing Key" + +[remote] +url = "https://somewhere-else.example" +``` + +Configuration files provide defaults for options in such a way that +environment variables override them, and explicit command line options +override those. You can see which options support environment variables +with `--help`; any of those may also be set in the config file, except +for ones whose values are secrets, such as `SUSH_NEXUS_TOKEN`. From 83256e8153de1f5a545455e114d0af50d39a339d Mon Sep 17 00:00:00 2001 From: Alex Plotnick Date: Sat, 31 Oct 2026 11:55:44 -0600 Subject: [PATCH 7/7] Catch malformed server URLs earlier Co-authored-by: Claude --- client/src/tls.rs | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/client/src/tls.rs b/client/src/tls.rs index 952b512c..b876eba3 100644 --- a/client/src/tls.rs +++ b/client/src/tls.rs @@ -15,6 +15,7 @@ use std::time::Duration; use attest_data::{DICE_TCB_INFO, DiceTcbInfo}; use ed25519_dalek::{Signature, VerifyingKey}; +use reqwest::Url; use rustls::client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}; use rustls::pki_types::{CertificateDer, ServerName, UnixTime}; use rustls::version::TLS13; @@ -56,11 +57,16 @@ pub enum ProxyTlsError { /// Placeholder host for scoped link-local URLs. const SCOPED_HOST: &str = "link-local.sush"; -/// Split a `%zone` scope out of a bracketed IPv6 host: URLs cannot -/// carry zone IDs, so the address maps to [`SCOPED_HOST`] and the -/// zone becomes the scope ID of the resolved socket address. A -/// numeric zone starting with `25` needs the RFC 6874 `%25` prefix. +/// Replace a host with a zone ID, like `[fe80::1%sw0tp0]`, with +/// [`SCOPED_HOST`], returning the host's address scoped to that zone. +/// Fails if the URL does not parse. pub fn descope_url(url: &str) -> Result<(String, Option), ProxyTlsError> { + let (descoped, resolve) = split_zone(url)?; + Url::parse(&descoped).map_err(|_| ProxyTlsError::Url(url.to_string()))?; + Ok((descoped, resolve)) +} + +fn split_zone(url: &str) -> Result<(String, Option), ProxyTlsError> { let (Some(open), Some(close)) = (url.find('['), url.find(']')) else { return Ok((url.to_string(), None)); }; @@ -331,5 +337,7 @@ mod test { assert!(descope_url("https://[fe80::1%0]:1").is_err()); assert!(descope_url("https://[fe80::1%nosuchif0]:1").is_err()); assert!(descope_url("https://[fe80::1%3]:notaport").is_err()); + assert!(descope_url("https://[]:12352").is_err()); + assert!(descope_url("not a url").is_err()); } }