From 443602a30b89f10e006c0dbf0f81dc43c60bf4c6 Mon Sep 17 00:00:00 2001 From: Eric Wendland Date: Sat, 18 Jul 2026 16:15:13 +0200 Subject: [PATCH] Add safe configuration commands --- Cargo.lock | 1 + README.md | 22 +++- crates/geth-cli/src/lib.rs | 198 +++++++++++++++++++++++++++++++++- crates/geth-config/Cargo.toml | 1 + crates/geth-config/src/lib.rs | 181 +++++++++++++++++++++++++++++++ docs/roadmap.md | 11 ++ docs/user-workflows.md | 21 ++++ 7 files changed, 432 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 81a699d..8dcd678 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1573,6 +1573,7 @@ version = "0.1.0" dependencies = [ "directories", "serde", + "tempfile", "thiserror 2.0.18", "toml_edit", "url", diff --git a/README.md b/README.md index 751ebc2..cd128e3 100644 --- a/README.md +++ b/README.md @@ -305,8 +305,10 @@ listing and later approval/signing workflows. The current log is materialized from signed certificate requests, signed certificate imports, and signed revocation records, then reduced locally; it is not a mutable remote ACL blob. While the daemon is running, it also performs a background live-sync tick for -known peers. The default interval is 30 seconds and can be changed in -`config.toml` with `[sync] live_sync_enabled` and `live_sync_interval_ms`. +known peers. The default interval is 30 seconds and can be changed with the +validated configuration commands, for example +`geth config set sync.live_sync_interval_ms 10000` or +`geth config set sync.live_sync_enabled false`. Live-sync stores per-peer high-water cursors in local metadata so repeated ticks request only newer SSH certificate-flow and revocation log entries. Sync import preserves local metadata by rejecting conflicting records with ids that already @@ -430,6 +432,22 @@ grants, and bearer-secret access without scraping prose. ## Local State +Use `geth config path` to find the selected configuration file, `geth config +show` to inspect either the file or the effective built-in defaults, and `geth +config validate` before restarting a daemon after manual edits. The safe setter +supports these dotted keys: + +- `iroh.relay_mode` (`default`, `staging`, `disabled`, or `custom`) +- `iroh.relay_map` +- `iroh.local_discovery` (`true` or `false`) +- `sync.live_sync_enabled` (`true` or `false`) +- `sync.live_sync_interval_ms` (at least `100`) + +`geth config set ` preserves unrelated TOML and comments, validates +the complete prospective config, and reports that the daemon must be restarted. +Custom relay-map URL tables remain a deliberate manual TOML edit; validate them +with `geth config validate`. + If `GETH_HOME` is set, geth uses it. Otherwise it uses an OS-specific data directory. The bootstrap layout is: diff --git a/crates/geth-cli/src/lib.rs b/crates/geth-cli/src/lib.rs index fc00633..a08a331 100644 --- a/crates/geth-cli/src/lib.rs +++ b/crates/geth-cli/src/lib.rs @@ -2,7 +2,7 @@ use anyhow::{Context, Result, bail}; use base64::Engine; use clap::{Args, CommandFactory, FromArgMatches, Parser, Subcommand, ValueEnum}; use clap_complete::{Shell, generate}; -use geth_config::GethPaths; +use geth_config::{ConfigKey, GethConfig, GethPaths}; use geth_control::{ControlRequest, ControlResponse, SyncStreamStatus}; use geth_node::service::{ServiceInstallOptions, ServiceManager, ServiceReport}; use std::io::{Read, Write, stdout}; @@ -422,6 +422,11 @@ pub enum Command { #[command(subcommand)] command: DaemonCommand, }, + /// Inspect, validate, and safely edit daemon configuration + Config { + #[command(subcommand)] + command: ConfigCommand, + }, /// Show daemon, storage, Iroh, and backend health Status, /// Inspect or trigger peer synchronization @@ -526,6 +531,48 @@ pub enum GuideTopic { SmokeTest, } +#[derive(Debug, Subcommand)] +pub enum ConfigCommand { + /// Print the selected config.toml path + Path, + /// Print the selected config and its effective values + Show, + /// Parse and validate the complete selected config + Validate, + /// Safely update one supported setting and validate the result + Set { + #[arg(value_enum)] + key: ConfigKeyArg, + value: String, + }, +} + +#[derive(Clone, Copy, Debug, ValueEnum)] +pub enum ConfigKeyArg { + #[value(name = "iroh.relay_mode")] + IrohRelayMode, + #[value(name = "iroh.relay_map")] + IrohRelayMap, + #[value(name = "iroh.local_discovery")] + IrohLocalDiscovery, + #[value(name = "sync.live_sync_enabled")] + SyncLiveSyncEnabled, + #[value(name = "sync.live_sync_interval_ms")] + SyncLiveSyncIntervalMs, +} + +impl From for ConfigKey { + fn from(value: ConfigKeyArg) -> Self { + match value { + ConfigKeyArg::IrohRelayMode => Self::IrohRelayMode, + ConfigKeyArg::IrohRelayMap => Self::IrohRelayMap, + ConfigKeyArg::IrohLocalDiscovery => Self::IrohLocalDiscovery, + ConfigKeyArg::SyncLiveSyncEnabled => Self::SyncLiveSyncEnabled, + ConfigKeyArg::SyncLiveSyncIntervalMs => Self::SyncLiveSyncIntervalMs, + } + } +} + #[derive(Debug, Subcommand)] #[command(after_long_help = DAEMON_AFTER_HELP)] pub enum DaemonCommand { @@ -1541,6 +1588,8 @@ fn argument_help(path: &str, id: &str) -> Option<&'static str> { ("geth cas conflict resolve", "resolution") => { Some("Resolution: keep-local, accept-remote, keep-both, or manual") } + ("geth config set", "key") => Some("Supported dotted configuration key"), + ("geth config set", "value") => Some("New value for the selected setting"), ("geth node enroll request", "capabilities") => { Some("Requested RESOURCE=CAPABILITY pair; repeat to request more than one") } @@ -1678,6 +1727,9 @@ async fn run_inner(cli: Cli) -> Result<()> { Command::Guide { topic } => { print_guide(topic, cli.json || cli.jsonl)?; } + Command::Config { command } => { + run_config_command(&paths, command, cli.json || cli.jsonl)?; + } Command::Init { admin_key, signing_key, @@ -1840,6 +1892,124 @@ async fn run_inner(cli: Cli) -> Result<()> { Ok(()) } +fn run_config_command(paths: &GethPaths, command: ConfigCommand, json: bool) -> Result<()> { + let path = paths.config_file(); + match command { + ConfigCommand::Path => { + if json { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "type": "config-path", + "path": path, + "exists": path.exists(), + }))? + ); + } else { + println!("{}", path.display()); + } + } + ConfigCommand::Show => { + let exists = path.exists(); + let text = if exists { + std::fs::read_to_string(&path) + .with_context(|| format!("read config at {}", path.display()))? + } else { + GethConfig::default_toml().to_owned() + }; + let config = GethConfig::parse(&text) + .with_context(|| format!("validate config at {}", path.display()))?; + if json { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "type": "config", + "path": path, + "source": if exists { "file" } else { "built-in-defaults" }, + "raw": text, + "effective": config_json(&config), + }))? + ); + } else { + println!("# path: {}", path.display()); + if !exists { + println!("# source: built-in defaults (file does not exist yet)"); + } + print!("{text}"); + if !text.ends_with('\n') { + println!(); + } + } + } + ConfigCommand::Validate => { + let exists = path.exists(); + let config = GethConfig::load(&path) + .with_context(|| format!("validate config at {}", path.display()))?; + if json { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "type": "config-validation", + "path": path, + "valid": true, + "source": if exists { "file" } else { "built-in-defaults" }, + "effective": config_json(&config), + }))? + ); + } else if exists { + println!("valid config: {}", path.display()); + } else { + println!( + "valid built-in defaults; config file does not exist yet: {}", + path.display() + ); + } + } + ConfigCommand::Set { key, value } => { + let key = ConfigKey::from(key); + let config = GethConfig::set(&path, key, &value) + .with_context(|| format!("set {} in {}", key.as_str(), path.display()))?; + if json { + println!( + "{}", + serde_json::to_string_pretty(&serde_json::json!({ + "type": "config-updated", + "path": path, + "key": key.as_str(), + "value": value, + "restart_required": true, + "effective": config_json(&config), + }))? + ); + } else { + println!("updated {} in {}", key.as_str(), path.display()); + println!("restart the daemon for the change to take effect"); + } + } + } + Ok(()) +} + +fn config_json(config: &GethConfig) -> serde_json::Value { + let relay_maps = config + .iroh + .relay_maps + .iter() + .map(|(name, map)| (name.clone(), map.urls.clone())) + .collect::>(); + serde_json::json!({ + "iroh": { + "relay_mode": config.iroh.relay_mode.label(), + "relay_maps": relay_maps, + "local_discovery": config.iroh.local_discovery, + }, + "sync": { + "live_sync_enabled": config.sync.live_sync_enabled, + "live_sync_interval_ms": config.sync.live_sync_interval_ms, + }, + }) +} + async fn run_ephemeral_daemon(json: bool) -> Result<()> { let (home, paths, node) = create_ephemeral_node()?; if json { @@ -2724,6 +2894,7 @@ fn request_for_command(command: Command) -> Result { }, }, Command::Guide { .. } + | Command::Config { .. } | Command::Init { .. } | Command::Daemon { .. } | Command::Backup { .. } @@ -4853,6 +5024,31 @@ mod tests { )); } + #[test] + fn configuration_commands_parse_and_update_an_isolated_home() { + for command in ["path", "show", "validate"] { + let parsed = Cli::try_parse_from(["geth", "config", command]); + assert!(parsed.is_ok(), "config {command} should parse: {parsed:?}"); + } + let parsed = + Cli::try_parse_from(["geth", "config", "set", "sync.live_sync_interval_ms", "500"]); + assert!(parsed.is_ok(), "config set should parse: {parsed:?}"); + + let home = tempfile::tempdir().expect("temporary geth home"); + let paths = GethPaths::from_home(home.path()); + run_config_command( + &paths, + ConfigCommand::Set { + key: ConfigKeyArg::SyncLiveSyncIntervalMs, + value: "500".to_owned(), + }, + false, + ) + .expect("set config"); + let config = GethConfig::load(&paths.config_file()).expect("load config"); + assert_eq!(config.sync.live_sync_interval_ms, 500); + } + #[tokio::test] async fn ephemeral_daemon_rejects_an_explicit_persistent_home() { let parsed = Cli::try_parse_from([ diff --git a/crates/geth-config/Cargo.toml b/crates/geth-config/Cargo.toml index 0c8bd17..242b9bc 100644 --- a/crates/geth-config/Cargo.toml +++ b/crates/geth-config/Cargo.toml @@ -10,4 +10,5 @@ directories.workspace = true serde.workspace = true thiserror.workspace = true toml_edit.workspace = true +tempfile.workspace = true url.workspace = true diff --git a/crates/geth-config/src/lib.rs b/crates/geth-config/src/lib.rs index bf26342..64dc08f 100644 --- a/crates/geth-config/src/lib.rs +++ b/crates/geth-config/src/lib.rs @@ -1,5 +1,7 @@ use std::collections::BTreeMap; +use std::io::Write; use std::path::{Path, PathBuf}; +use std::str::FromStr; pub const DEFAULT_CONFIG_TOML: &str = r#"# geth local node config # Remote node-to-node communication is Iroh-only. @@ -124,6 +126,117 @@ pub enum ConfigError { InvalidLiveSyncEnabled(String), #[error("invalid sync live_sync_interval_ms value: {0}")] InvalidLiveSyncInterval(String), + #[error( + "unsupported config key `{0}`; supported keys: iroh.relay_mode, iroh.relay_map, iroh.local_discovery, sync.live_sync_enabled, sync.live_sync_interval_ms" + )] + UnsupportedKey(String), + #[error("invalid value `{value}` for {key}; expected {expected}")] + InvalidSetting { + key: &'static str, + value: String, + expected: &'static str, + }, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ConfigKey { + IrohRelayMode, + IrohRelayMap, + IrohLocalDiscovery, + SyncLiveSyncEnabled, + SyncLiveSyncIntervalMs, +} + +impl ConfigKey { + #[must_use] + pub const fn as_str(self) -> &'static str { + match self { + Self::IrohRelayMode => "iroh.relay_mode", + Self::IrohRelayMap => "iroh.relay_map", + Self::IrohLocalDiscovery => "iroh.local_discovery", + Self::SyncLiveSyncEnabled => "sync.live_sync_enabled", + Self::SyncLiveSyncIntervalMs => "sync.live_sync_interval_ms", + } + } + + fn set( + self, + document: &mut toml_edit::DocumentMut, + raw_value: &str, + ) -> Result<(), ConfigError> { + match self { + Self::IrohRelayMode => { + if !matches!(raw_value, "default" | "staging" | "disabled" | "custom") { + return Err(invalid_setting( + self, + raw_value, + "default, staging, disabled, or custom", + )); + } + document["iroh"]["relay_mode"] = toml_edit::value(raw_value); + } + Self::IrohRelayMap => { + if raw_value.trim().is_empty() { + return Err(invalid_setting( + self, + raw_value, + "a non-empty relay-map name", + )); + } + document["iroh"]["relay_map"] = toml_edit::value(raw_value); + } + Self::IrohLocalDiscovery => { + document["iroh"]["local_discovery"] = + toml_edit::value(parse_bool_setting(self, raw_value)?); + } + Self::SyncLiveSyncEnabled => { + document["sync"]["live_sync_enabled"] = + toml_edit::value(parse_bool_setting(self, raw_value)?); + } + Self::SyncLiveSyncIntervalMs => { + let interval = raw_value + .parse::() + .ok() + .filter(|interval| *interval >= 100) + .ok_or_else(|| { + invalid_setting(self, raw_value, "an integer of at least 100") + })?; + document["sync"]["live_sync_interval_ms"] = toml_edit::value(interval); + } + } + Ok(()) + } +} + +impl FromStr for ConfigKey { + type Err = ConfigError; + + fn from_str(value: &str) -> Result { + match value { + "iroh.relay_mode" => Ok(Self::IrohRelayMode), + "iroh.relay_map" => Ok(Self::IrohRelayMap), + "iroh.local_discovery" => Ok(Self::IrohLocalDiscovery), + "sync.live_sync_enabled" => Ok(Self::SyncLiveSyncEnabled), + "sync.live_sync_interval_ms" => Ok(Self::SyncLiveSyncIntervalMs), + other => Err(ConfigError::UnsupportedKey(other.to_owned())), + } + } +} + +fn invalid_setting(key: ConfigKey, value: &str, expected: &'static str) -> ConfigError { + ConfigError::InvalidSetting { + key: key.as_str(), + value: value.to_owned(), + expected, + } +} + +fn parse_bool_setting(key: ConfigKey, value: &str) -> Result { + match value { + "true" => Ok(true), + "false" => Ok(false), + _ => Err(invalid_setting(key, value, "true or false")), + } } #[derive(Clone, Debug, Default, PartialEq, Eq)] @@ -169,6 +282,28 @@ impl GethConfig { sync, }) } + + pub fn set(path: &Path, key: ConfigKey, value: &str) -> Result { + let text = if path.exists() { + std::fs::read_to_string(path)? + } else { + Self::default_toml().to_owned() + }; + let mut document = text.parse::()?; + key.set(&mut document, value)?; + let updated = document.to_string(); + let config = Self::parse(&updated)?; + + let parent = path.parent().unwrap_or_else(|| Path::new(".")); + std::fs::create_dir_all(parent)?; + let mut temporary = tempfile::NamedTempFile::new_in(parent)?; + temporary.write_all(updated.as_bytes())?; + temporary.as_file().sync_all()?; + temporary + .persist(path) + .map_err(|error| ConfigError::Io(error.error))?; + Ok(config) + } } #[derive(Clone, Debug, PartialEq, Eq)] @@ -467,4 +602,50 @@ mod tests { let error = GethConfig::parse("[iroh]\nlocal_discovery = \"yes\"\n").expect_err("error"); assert!(matches!(error, ConfigError::InvalidLocalDiscovery(_))); } + + #[test] + fn config_set_preserves_comments_and_validates_the_result() { + let home = tempfile::tempdir().expect("tempdir"); + let path = home.path().join("config.toml"); + std::fs::write( + &path, + "# operator note\n[iroh]\nrelay_mode = \"default\" # keep relays\n\n[sync]\nlive_sync_enabled = true\nlive_sync_interval_ms = 30000\n", + ) + .expect("write config"); + + let config = GethConfig::set(&path, ConfigKey::SyncLiveSyncIntervalMs, "750") + .expect("update config"); + assert_eq!(config.sync.live_sync_interval_ms, 750); + let updated = std::fs::read_to_string(&path).expect("read config"); + assert!(updated.contains("# operator note")); + assert!(updated.contains("relay_mode = \"default\" # keep relays")); + assert!(updated.contains("live_sync_interval_ms = 750")); + } + + #[test] + fn config_set_does_not_replace_a_valid_file_with_an_invalid_update() { + let home = tempfile::tempdir().expect("tempdir"); + let path = home.path().join("config.toml"); + let original = GethConfig::default_toml(); + std::fs::write(&path, original).expect("write config"); + + let error = GethConfig::set(&path, ConfigKey::IrohRelayMode, "custom") + .expect_err("custom mode needs an existing map"); + assert!(matches!(error, ConfigError::MissingRelayMapSelection)); + assert_eq!( + std::fs::read_to_string(path).expect("read config"), + original + ); + } + + #[test] + fn config_set_creates_a_default_config_when_missing() { + let home = tempfile::tempdir().expect("tempdir"); + let path = home.path().join("nested/config.toml"); + + let config = + GethConfig::set(&path, ConfigKey::IrohLocalDiscovery, "false").expect("create config"); + assert!(!config.iroh.local_discovery); + assert!(path.is_file()); + } } diff --git a/docs/roadmap.md b/docs/roadmap.md index f02a393..358cdfe 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -67,6 +67,17 @@ For deployment-readiness work that cuts across feature areas, see - `[x]` A recursive CLI test fails when a future argument is added without a description. +- `[x]` Add a safe configuration workflow. + Acceptance criteria: + - `[x]` `geth config path/show/validate` identify the selected file, expose + built-in defaults when it is absent, and report parse or semantic errors. + - `[x]` `geth config set` supports the common Iroh and live-sync settings, + preserves unrelated TOML and comments, and validates before replacement. + - `[x]` Updates use a same-directory temporary file and clearly report that a + daemon restart is required. + - `[x]` Tests cover comment preservation, invalid-update rollback, and + creation from defaults in an isolated temporary home. + - `[x]` Document task-oriented user stories. Acceptance criteria: - `[x]` Workflows cover disposable evaluation, persistent background use, diff --git a/docs/user-workflows.md b/docs/user-workflows.md index 9874168..be056a5 100644 --- a/docs/user-workflows.md +++ b/docs/user-workflows.md @@ -67,6 +67,27 @@ Set `RUST_LOG=geth_node=debug` when more daemon diagnostics are useful. Use `geth --home ...` to operate an isolated home without exporting an environment variable. +## Inspect And Change Configuration + +User story: as an operator, I want to find and validate the exact configuration +used by one geth home without guessing an OS-specific path or hand-editing +common boolean and interval settings. + +```sh +geth config path +geth config show +geth config set sync.live_sync_interval_ms 10000 +geth config validate +geth daemon stop +geth daemon start +``` + +`config show` reports built-in defaults when the file has not been created yet. +`config set` preserves unrelated TOML and comments and refuses to replace the +file if the complete prospective configuration is invalid. Success means +`config validate` identifies the selected path as valid. Configuration is +loaded at daemon startup, so restart the user service after changes. + ## Establish An Owner Trust Root User story: as the mesh owner, I want my first node rooted in an existing SSH