From 88b2bf31a6888e54be6daafce43444560284d80e Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Wed, 18 Mar 2026 18:02:38 -0400 Subject: [PATCH] Add `fabro secret` CLI subcommands for managing ~/.fabro/.env Provides get/list/rm/set subcommands to manage secrets without manually editing the .env file. Extracts shared dotenv utilities into fabro-config::dotenv and refactors install.rs to use them. Co-Authored-By: Claude Opus 4.6 (1M context) --- lib/crates/fabro-cli/src/commands/mod.rs | 1 + lib/crates/fabro-cli/src/commands/secret.rs | 89 +++++++ lib/crates/fabro-cli/src/install.rs | 38 +-- lib/crates/fabro-cli/src/main.rs | 38 +++ lib/crates/fabro-cli/tests/scenario.rs | 123 ++++++++++ lib/crates/fabro-config/src/dotenv.rs | 247 ++++++++++++++++++++ lib/crates/fabro-config/src/lib.rs | 1 + 7 files changed, 502 insertions(+), 35 deletions(-) create mode 100644 lib/crates/fabro-cli/src/commands/secret.rs create mode 100644 lib/crates/fabro-config/src/dotenv.rs diff --git a/lib/crates/fabro-cli/src/commands/mod.rs b/lib/crates/fabro-cli/src/commands/mod.rs index 45de11efc..87e04821d 100644 --- a/lib/crates/fabro-cli/src/commands/mod.rs +++ b/lib/crates/fabro-cli/src/commands/mod.rs @@ -12,6 +12,7 @@ pub mod rewind; pub mod run; mod run_progress; pub mod runs; +pub mod secret; pub(crate) mod shared; pub mod ssh; pub mod validate; diff --git a/lib/crates/fabro-cli/src/commands/secret.rs b/lib/crates/fabro-cli/src/commands/secret.rs new file mode 100644 index 000000000..f74920ae1 --- /dev/null +++ b/lib/crates/fabro-cli/src/commands/secret.rs @@ -0,0 +1,89 @@ +use anyhow::{bail, Result}; +use clap::Args; + +use fabro_config::dotenv; + +#[derive(Args)] +pub struct SecretGetArgs { + /// Name of the secret + pub key: String, +} + +#[derive(Args)] +pub struct SecretListArgs { + /// Show values alongside keys + #[arg(long)] + pub show_values: bool, +} + +#[derive(Args)] +pub struct SecretRmArgs { + /// Name of the secret to remove + pub key: String, +} + +#[derive(Args)] +pub struct SecretSetArgs { + /// Name of the secret + pub key: String, + /// Value to store + pub value: String, +} + +pub fn get_command(args: &SecretGetArgs) -> Result<()> { + let path = dotenv::env_file_path()?; + match dotenv::get_env_value(&path, &args.key)? { + Some(value) => { + println!("{value}"); + Ok(()) + } + None => bail!("secret not found: {}", args.key), + } +} + +pub fn list_command(args: &SecretListArgs) -> Result<()> { + let path = dotenv::env_file_path()?; + let contents = match std::fs::read_to_string(&path) { + Ok(c) => c, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()), + Err(e) => bail!("failed to read {}: {e}", path.display()), + }; + let pairs = dotenv::parse_env(&contents); + for (key, value) in pairs { + if args.show_values { + println!("{key}={value}"); + } else { + println!("{key}"); + } + } + Ok(()) +} + +pub fn rm_command(args: &SecretRmArgs) -> Result<()> { + let path = dotenv::env_file_path()?; + let contents = match std::fs::read_to_string(&path) { + Ok(c) => c, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + bail!("secret not found: {}", args.key) + } + Err(e) => bail!("failed to read {}: {e}", path.display()), + }; + // Check the key actually exists before removing + let pairs = dotenv::parse_env(&contents); + if !pairs.iter().any(|(k, _)| k == &args.key) { + bail!("secret not found: {}", args.key); + } + let updated = dotenv::remove_env_key(&contents, &args.key); + dotenv::write_env_file(&path, &updated)?; + eprintln!("Removed {}", args.key); + Ok(()) +} + +pub fn set_command(args: &SecretSetArgs) -> Result<()> { + let path = dotenv::env_file_path()?; + let existing = std::fs::read_to_string(&path).unwrap_or_default(); + let merged = dotenv::merge_env(&existing, &[(&args.key, &args.value)]); + dotenv::write_env_file(&path, &merged)?; + eprintln!("Set {}", args.key); + Ok(()) +} diff --git a/lib/crates/fabro-cli/src/install.rs b/lib/crates/fabro-cli/src/install.rs index 3183cae1f..294206016 100644 --- a/lib/crates/fabro-cli/src/install.rs +++ b/lib/crates/fabro-cli/src/install.rs @@ -200,38 +200,11 @@ ca = "~/.fabro/certs/ca.crt" } // --------------------------------------------------------------------------- -// .env merge +// .env merge (delegates to fabro_config::dotenv) // --------------------------------------------------------------------------- fn merge_env(existing: &str, new_vars: &[(&str, &str)]) -> String { - let mut result_lines: Vec = Vec::new(); - let mut handled_keys: std::collections::HashSet<&str> = std::collections::HashSet::new(); - - for line in existing.lines() { - if let Some(eq_pos) = line.find('=') { - let key = line[..eq_pos].trim(); - if !key.is_empty() && !key.starts_with('#') { - if let Some((_, new_val)) = new_vars.iter().find(|(k, _)| *k == key) { - result_lines.push(format!("{key}={new_val}")); - handled_keys.insert(key); - continue; - } - } - } - result_lines.push(line.to_string()); - } - - for (key, val) in new_vars { - if !handled_keys.contains(*key) { - result_lines.push(format!("{key}={val}")); - } - } - - let mut result = result_lines.join("\n"); - if !result.ends_with('\n') { - result.push('\n'); - } - result + fabro_config::dotenv::merge_env(existing, new_vars) } // --------------------------------------------------------------------------- @@ -577,12 +550,7 @@ fn write_env_file(arc_dir: &Path, env_pairs: &[(String, String)], s: &Styles) -> .map(|(k, v)| (k.as_str(), v.as_str())) .collect(); let merged = merge_env(&existing, &refs); - std::fs::write(&env_path, &merged)?; - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&env_path, std::fs::Permissions::from_mode(0o600))?; - } + fabro_config::dotenv::write_env_file(&env_path, &merged)?; eprintln!( " {}", s.dim.apply_to(format!("Wrote {}", env_path.display())) diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index e35eb8855..103977da2 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -135,6 +135,11 @@ enum Command { #[command(subcommand)] command: SkillCommand, }, + /// Manage secrets in ~/.fabro/.env + Secret { + #[command(subcommand)] + command: SecretCommand, + }, /// Rewind a workflow run to an earlier checkpoint Rewind(commands::rewind::RewindArgs), /// Fork a workflow run from an earlier checkpoint into a new run @@ -208,6 +213,19 @@ enum RepoCommand { Deinit, } +#[derive(Subcommand)] +enum SecretCommand { + /// Get a secret value + Get(commands::secret::SecretGetArgs), + /// List secret names + #[command(alias = "ls")] + List(commands::secret::SecretListArgs), + /// Remove a secret + Rm(commands::secret::SecretRmArgs), + /// Set a secret value + Set(commands::secret::SecretSetArgs), +} + #[derive(Subcommand)] enum SkillCommand { /// Install a built-in skill @@ -470,6 +488,12 @@ async fn main_inner() -> (String, Result<()>) { PrCommand::Merge(_) => "pr merge", PrCommand::Close(_) => "pr close", }, + Command::Secret { command } => match command { + SecretCommand::Get(_) => "secret get", + SecretCommand::List(_) => "secret list", + SecretCommand::Rm(_) => "secret rm", + SecretCommand::Set(_) => "secret set", + }, Command::Rewind(_) => "rewind", Command::Fork(_) => "fork", Command::Workflow { command } => match command { @@ -837,6 +861,20 @@ async fn main_inner() -> (String, Result<()>) { } } } + Command::Secret { command } => match command { + SecretCommand::Get(args) => { + commands::secret::get_command(&args)?; + } + SecretCommand::List(args) => { + commands::secret::list_command(&args)?; + } + SecretCommand::Rm(args) => { + commands::secret::rm_command(&args)?; + } + SecretCommand::Set(args) => { + commands::secret::set_command(&args)?; + } + }, Command::Rewind(args) => { let styles = fabro_util::terminal::Styles::detect_stderr(); commands::rewind::run(&args, &styles)?; diff --git a/lib/crates/fabro-cli/tests/scenario.rs b/lib/crates/fabro-cli/tests/scenario.rs index ce278c57e..5c6f262bd 100644 --- a/lib/crates/fabro-cli/tests/scenario.rs +++ b/lib/crates/fabro-cli/tests/scenario.rs @@ -502,6 +502,129 @@ fn test_repo_init_help_does_not_show_skill() { ); } +// --------------------------------------------------------------------------- +// secret subcommand lifecycle +// --------------------------------------------------------------------------- + +#[test] +fn test_secret_lifecycle() { + let tmp = tempfile::tempdir().unwrap(); + + let secret = |args: &[&str]| -> assert_cmd::assert::Assert { + fabro().env("HOME", tmp.path()).args(args).assert() + }; + + // 1. set FOO=bar + secret(&["secret", "set", "FOO", "bar"]).success(); + + // 2. get FOO → stdout is "bar\n" + secret(&["secret", "get", "FOO"]).success().stdout("bar\n"); + + // 3. list → contains FOO + secret(&["secret", "list"]) + .success() + .stdout(predicates::str::contains("FOO")); + + // 4. update FOO + secret(&["secret", "set", "FOO", "updated"]).success(); + + // 5. get FOO → "updated\n" + secret(&["secret", "get", "FOO"]) + .success() + .stdout("updated\n"); + + // 6. rm FOO + secret(&["secret", "rm", "FOO"]).success(); + + // 7. get FOO → fails + secret(&["secret", "get", "FOO"]).failure(); +} + +#[test] +fn test_secret_list_show_values() { + let tmp = tempfile::tempdir().unwrap(); + + let secret = |args: &[&str]| -> assert_cmd::assert::Assert { + fabro().env("HOME", tmp.path()).args(args).assert() + }; + + secret(&["secret", "set", "A", "1"]).success(); + secret(&["secret", "set", "B", "2"]).success(); + + // Without --show-values: just keys + let out = secret(&["secret", "list"]).success(); + let stdout = String::from_utf8(out.get_output().stdout.clone()).unwrap(); + assert!(stdout.contains("A")); + assert!(stdout.contains("B")); + assert!(!stdout.contains("A=1")); + + // With --show-values: KEY=VALUE + secret(&["secret", "list", "--show-values"]) + .success() + .stdout(predicates::str::contains("A=1")) + .stdout(predicates::str::contains("B=2")); +} + +#[test] +fn test_secret_list_alias_ls() { + let tmp = tempfile::tempdir().unwrap(); + + fabro() + .env("HOME", tmp.path()) + .args(["secret", "set", "X", "y"]) + .assert() + .success(); + + fabro() + .env("HOME", tmp.path()) + .args(["secret", "ls"]) + .assert() + .success() + .stdout(predicates::str::contains("X")); +} + +#[test] +fn test_secret_get_missing_key() { + let tmp = tempfile::tempdir().unwrap(); + + fabro() + .env("HOME", tmp.path()) + .args(["secret", "get", "NOPE"]) + .assert() + .failure() + .stderr(predicates::str::contains("secret not found")); +} + +#[test] +fn test_secret_rm_missing_key() { + let tmp = tempfile::tempdir().unwrap(); + + fabro() + .env("HOME", tmp.path()) + .args(["secret", "rm", "NOPE"]) + .assert() + .failure() + .stderr(predicates::str::contains("secret not found")); +} + +#[test] +fn test_secret_value_with_equals() { + let tmp = tempfile::tempdir().unwrap(); + + fabro() + .env("HOME", tmp.path()) + .args(["secret", "set", "URL", "https://x.com?a=1&b=2"]) + .assert() + .success(); + + fabro() + .env("HOME", tmp.path()) + .args(["secret", "get", "URL"]) + .assert() + .success() + .stdout("https://x.com?a=1&b=2\n"); +} + // --------------------------------------------------------------------------- // Standalone tests (no sandbox parametrization) // --------------------------------------------------------------------------- diff --git a/lib/crates/fabro-config/src/dotenv.rs b/lib/crates/fabro-config/src/dotenv.rs new file mode 100644 index 000000000..da97ae6f9 --- /dev/null +++ b/lib/crates/fabro-config/src/dotenv.rs @@ -0,0 +1,247 @@ +//! Shared utilities for reading and writing `.env` files (`~/.fabro/.env`). + +use std::path::{Path, PathBuf}; + +use anyhow::{bail, Context, Result}; + +/// Return the path to `~/.fabro/.env`. +pub fn env_file_path() -> Result { + let home = dirs::home_dir().context("could not determine home directory")?; + Ok(home.join(".fabro").join(".env")) +} + +/// Parse an env file's contents into `(key, value)` pairs, skipping comments +/// and blank lines. Values are split on the first `=` only. +pub fn parse_env(contents: &str) -> Vec<(String, String)> { + let mut pairs = Vec::new(); + for line in contents.lines() { + let trimmed = line.trim(); + if trimmed.is_empty() || trimmed.starts_with('#') { + continue; + } + if let Some(eq_pos) = trimmed.find('=') { + let key = trimmed[..eq_pos].trim().to_string(); + let value = trimmed[eq_pos + 1..].to_string(); + if !key.is_empty() { + pairs.push((key, value)); + } + } + } + pairs +} + +/// Merge `new_vars` into an existing env file's contents, preserving comments, +/// blank lines, and ordering. Existing keys are updated in place; new keys are +/// appended. The returned string always ends with a newline. +pub fn merge_env(existing: &str, new_vars: &[(&str, &str)]) -> String { + let mut result_lines: Vec = Vec::new(); + let mut handled_keys: std::collections::HashSet<&str> = std::collections::HashSet::new(); + + for line in existing.lines() { + if let Some(eq_pos) = line.find('=') { + let key = line[..eq_pos].trim(); + if !key.is_empty() && !key.starts_with('#') { + if let Some((_, new_val)) = new_vars.iter().find(|(k, _)| *k == key) { + result_lines.push(format!("{key}={new_val}")); + handled_keys.insert(key); + continue; + } + } + } + result_lines.push(line.to_string()); + } + + for (key, val) in new_vars { + if !handled_keys.contains(*key) { + result_lines.push(format!("{key}={val}")); + } + } + + let mut result = result_lines.join("\n"); + if !result.ends_with('\n') { + result.push('\n'); + } + result +} + +/// Remove a key from an env file's raw contents. Comments and blank lines are +/// preserved. Returns the new file contents. +pub fn remove_env_key(contents: &str, key_to_remove: &str) -> String { + let mut result_lines: Vec = Vec::new(); + for line in contents.lines() { + if let Some(eq_pos) = line.find('=') { + let key = line[..eq_pos].trim(); + if !key.is_empty() && !key.starts_with('#') && key == key_to_remove { + continue; + } + } + result_lines.push(line.to_string()); + } + let mut result = result_lines.join("\n"); + if !result.ends_with('\n') { + result.push('\n'); + } + result +} + +/// Write content to the env file, creating parent directories if needed. +/// Sets file permissions to 0600 on Unix. +pub fn write_env_file(path: &Path, contents: &str) -> Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent) + .with_context(|| format!("failed to create directory {}", parent.display()))?; + } + std::fs::write(path, contents) + .with_context(|| format!("failed to write {}", path.display()))?; + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600))?; + } + Ok(()) +} + +/// Read the value of a single key from the env file. Returns `None` if the +/// file doesn't exist or the key isn't found. +pub fn get_env_value(path: &Path, key: &str) -> Result> { + let contents = match std::fs::read_to_string(path) { + Ok(c) => c, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(e) => bail!("failed to read {}: {e}", path.display()), + }; + let pairs = parse_env(&contents); + Ok(pairs.into_iter().find(|(k, _)| k == key).map(|(_, v)| v)) +} + +#[cfg(test)] +mod tests { + use super::*; + + // -- parse_env -- + + #[test] + fn parse_env_basic() { + let pairs = parse_env("FOO=bar\nBAZ=qux\n"); + assert_eq!( + pairs, + vec![("FOO".into(), "bar".into()), ("BAZ".into(), "qux".into())] + ); + } + + #[test] + fn parse_env_skips_comments_and_blanks() { + let pairs = parse_env("# comment\n\nFOO=bar\n\n# another\n"); + assert_eq!(pairs, vec![("FOO".into(), "bar".into())]); + } + + #[test] + fn parse_env_values_with_equals() { + let pairs = parse_env("URL=https://example.com?a=1&b=2\n"); + assert_eq!( + pairs, + vec![("URL".into(), "https://example.com?a=1&b=2".into())] + ); + } + + // -- merge_env -- + + #[test] + fn merge_env_replaces_existing() { + let result = merge_env("FOO=old\nBAR=keep\n", &[("FOO", "new"), ("BAZ", "added")]); + assert!(result.contains("FOO=new")); + assert!(result.contains("BAR=keep")); + assert!(result.contains("BAZ=added")); + } + + #[test] + fn merge_env_empty_existing() { + let result = merge_env("", &[("FOO", "bar"), ("BAZ", "qux")]); + assert!(result.contains("FOO=bar")); + assert!(result.contains("BAZ=qux")); + } + + #[test] + fn merge_env_preserves_comments_and_blanks() { + let existing = "# A comment\n\nFOO=old\n# Another\nBAR=keep\n"; + let result = merge_env(existing, &[("FOO", "new")]); + assert!(result.contains("# A comment")); + assert!(result.contains("# Another")); + assert!(result.contains("FOO=new")); + assert!(result.contains("BAR=keep")); + } + + #[test] + fn merge_env_full_scenario() { + let result = merge_env("FOO=old\nBAR=keep", &[("FOO", "new"), ("BAZ", "added")]); + assert_eq!(result, "FOO=new\nBAR=keep\nBAZ=added\n"); + } + + // -- remove_env_key -- + + #[test] + fn remove_env_key_removes_target() { + let result = remove_env_key("FOO=bar\nBAZ=qux\n", "FOO"); + assert!(!result.contains("FOO")); + assert!(result.contains("BAZ=qux")); + } + + #[test] + fn remove_env_key_preserves_others() { + let result = remove_env_key("A=1\nB=2\nC=3\n", "B"); + assert!(result.contains("A=1")); + assert!(!result.contains("B=2")); + assert!(result.contains("C=3")); + } + + #[test] + fn remove_env_key_handles_missing_key() { + let original = "FOO=bar\n"; + let result = remove_env_key(original, "MISSING"); + assert_eq!(result, original); + } + + #[test] + fn remove_env_key_preserves_comments() { + let result = remove_env_key("# keep me\nFOO=bar\nBAZ=qux\n", "FOO"); + assert!(result.contains("# keep me")); + assert!(result.contains("BAZ=qux")); + assert!(!result.contains("FOO")); + } + + // -- write_env_file -- + + #[test] + fn write_env_file_creates_and_writes() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("sub").join(".env"); + write_env_file(&path, "KEY=val\n").unwrap(); + let content = std::fs::read_to_string(&path).unwrap(); + assert_eq!(content, "KEY=val\n"); + } + + // -- get_env_value -- + + #[test] + fn get_env_value_found() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join(".env"); + std::fs::write(&path, "FOO=bar\nBAZ=qux\n").unwrap(); + assert_eq!(get_env_value(&path, "FOO").unwrap(), Some("bar".into())); + assert_eq!(get_env_value(&path, "BAZ").unwrap(), Some("qux".into())); + } + + #[test] + fn get_env_value_not_found() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join(".env"); + std::fs::write(&path, "FOO=bar\n").unwrap(); + assert_eq!(get_env_value(&path, "MISSING").unwrap(), None); + } + + #[test] + fn get_env_value_file_missing() { + let tmp = tempfile::tempdir().unwrap(); + let path = tmp.path().join("nonexistent"); + assert_eq!(get_env_value(&path, "FOO").unwrap(), None); + } +} diff --git a/lib/crates/fabro-config/src/lib.rs b/lib/crates/fabro-config/src/lib.rs index b5b338bd4..a26798d7b 100644 --- a/lib/crates/fabro-config/src/lib.rs +++ b/lib/crates/fabro-config/src/lib.rs @@ -1,4 +1,5 @@ pub mod cli; +pub mod dotenv; pub mod hook; pub mod mcp; pub mod project;