diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7cd4ee2..99511cb 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -287,6 +287,9 @@ site: datadoghq.com output: json verbose: false +# Safety +read_only: false + # Defaults default_from: 1h default_to: now diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index a950601..ff45d69 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -179,6 +179,7 @@ Available on all commands: --output string Output format: json, yaml, table (default: json) --verbose Enable verbose logging --yes Skip confirmation prompts +--read-only Block all write operations (create, update, delete) ``` ## Recent Enhancements diff --git a/docs/EXAMPLES.md b/docs/EXAMPLES.md index cf1545c..32a1d27 100644 --- a/docs/EXAMPLES.md +++ b/docs/EXAMPLES.md @@ -412,6 +412,16 @@ pup --verbose monitors list pup --yes monitors delete 12345678 ``` +### Read-Only Mode +```bash +# Block all write operations (create, update, delete) +pup --read-only monitors list +pup --read-only dashboards list + +# Also available via env var or config file +DD_READ_ONLY=true pup monitors list +``` + ## Common Workflows ### Monitoring Dashboard diff --git a/src/client.rs b/src/client.rs index ba859bd..e48d72b 100644 --- a/src/client.rs +++ b/src/client.rs @@ -512,6 +512,7 @@ mod tests { output_format: crate::config::OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, } } diff --git a/src/config.rs b/src/config.rs index d97f9ba..4d2067b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -13,6 +13,7 @@ pub struct Config { pub output_format: OutputFormat, pub auto_approve: bool, pub agent_mode: bool, + pub read_only: bool, } #[derive(Clone, Debug, PartialEq)] @@ -55,6 +56,7 @@ struct FileConfig { org: Option, output: Option, auto_approve: Option, + read_only: Option, } impl Config { @@ -85,6 +87,9 @@ impl Config { || env_bool("DD_CLI_AUTO_APPROVE") || file_cfg.auto_approve.unwrap_or(false), agent_mode: false, // set by caller from --agent flag or useragent detection + read_only: env_bool("DD_READ_ONLY") + || env_bool("DD_CLI_READ_ONLY") + || file_cfg.read_only.unwrap_or(false), }; Ok(cfg) @@ -108,6 +113,7 @@ impl Config { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, } } @@ -253,6 +259,7 @@ mod tests { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, } } @@ -480,4 +487,36 @@ mod tests { ); std::env::remove_var("__PUP_TEST_ENV_EMPTY__"); } + + #[test] + fn test_file_config_read_only() { + let yaml = "read_only: true\n"; + let fc: FileConfig = serde_yaml::from_str(yaml).unwrap(); + assert_eq!(fc.read_only, Some(true)); + } + + #[test] + fn test_read_only_from_env() { + let _guard = ENV_LOCK.lock().unwrap_or_else(|p| p.into_inner()); + std::env::remove_var("DD_READ_ONLY"); + std::env::remove_var("DD_CLI_READ_ONLY"); + std::env::set_var("PUP_CONFIG_DIR", "/tmp/pup_test_nonexistent"); + std::env::set_var("DD_ACCESS_TOKEN", "test"); + + let cfg = Config::from_env().unwrap(); + assert!(!cfg.read_only); + + std::env::set_var("DD_READ_ONLY", "true"); + let cfg = Config::from_env().unwrap(); + assert!(cfg.read_only); + std::env::remove_var("DD_READ_ONLY"); + + std::env::set_var("DD_CLI_READ_ONLY", "1"); + let cfg = Config::from_env().unwrap(); + assert!(cfg.read_only); + std::env::remove_var("DD_CLI_READ_ONLY"); + + std::env::remove_var("DD_ACCESS_TOKEN"); + std::env::remove_var("PUP_CONFIG_DIR"); + } } diff --git a/src/formatter.rs b/src/formatter.rs index 23669b8..fddca21 100644 --- a/src/formatter.rs +++ b/src/formatter.rs @@ -717,6 +717,7 @@ mod tests { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let data = serde_json::json!({"hello": "world"}); assert!(output(&cfg, &data).is_ok()); diff --git a/src/main.rs b/src/main.rs index 631f561..ebd31fb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -20,11 +20,11 @@ pub(crate) mod test_utils { pub static ENV_LOCK: Mutex<()> = Mutex::new(()); } -use clap::{CommandFactory, Parser, Subcommand}; +use clap::{CommandFactory, FromArgMatches, Parser, Subcommand}; #[derive(Parser)] #[command(name = "pup", version = version::VERSION, about = "Datadog API CLI")] -struct Cli { +pub(crate) struct Cli { /// Output format (json, table, yaml) #[arg(short, long, global = true, default_value = "json")] output: String, @@ -34,6 +34,9 @@ struct Cli { /// Enable agent mode #[arg(long, global = true)] agent: bool, + /// Block all write operations (create, update, delete) + #[arg(long, global = true)] + read_only: bool, /// Named org session (see 'pup auth login --org') #[arg(long, global = true)] org: Option, @@ -4738,25 +4741,10 @@ fn build_compact_agent_schema(cmd: &clap::Command) -> serde_json::Value { serde_json::Value::Object(root) } -fn build_command_schema(cmd: &clap::Command, parent_path: &str) -> serde_json::Value { - let mut obj = serde_json::Map::new(); - let name = cmd.get_name().to_string(); - let full_path = if parent_path.is_empty() { - name.clone() - } else { - format!("{parent_path} {name}") - }; - - obj.insert("name".into(), serde_json::json!(name)); - obj.insert("full_path".into(), serde_json::json!(full_path)); - - if let Some(about) = cmd.get_about() { - obj.insert("description".into(), serde_json::json!(about.to_string())); - } - - // Determine read_only based on command name — but only emit for leaf commands - // (commands with no subcommands), matching Go behavior - let is_write = name == "delete" +/// Returns true if a leaf subcommand name represents a write (mutating) operation. +/// Used by both the read-only runtime guard and the agent JSON schema. +pub(crate) fn is_write_command_name(name: &str) -> bool { + name == "delete" || name == "create" || name == "update" || name == "cancel" @@ -4769,6 +4757,11 @@ fn build_command_schema(cmd: &clap::Command, parent_path: &str) -> serde_json::V || name == "unarchive" || name == "activate" || name == "deactivate" + || name == "move" + || name == "link" + || name == "unlink" + || name == "configure" + || name == "upgrade" || name.starts_with("update-") || name.starts_with("create-") || name == "submit" @@ -4777,7 +4770,29 @@ fn build_command_schema(cmd: &clap::Command, parent_path: &str) -> serde_json::V || name == "register" || name == "unregister" || name.contains("delete") - || name.contains("patch"); + || name == "patch" + || name.starts_with("patch-") +} + +fn build_command_schema(cmd: &clap::Command, parent_path: &str) -> serde_json::Value { + let mut obj = serde_json::Map::new(); + let name = cmd.get_name().to_string(); + let full_path = if parent_path.is_empty() { + name.clone() + } else { + format!("{parent_path} {name}") + }; + + obj.insert("name".into(), serde_json::json!(name)); + obj.insert("full_path".into(), serde_json::json!(full_path)); + + if let Some(about) = cmd.get_about() { + obj.insert("description".into(), serde_json::json!(about.to_string())); + } + + // Determine read_only based on command name — but only emit for leaf commands + // (commands with no subcommands), matching Go behavior + let is_write = is_write_command_name(&name); // Flags (named --flags only, excluding positional args and globals) let flags: Vec = cmd @@ -4867,6 +4882,19 @@ async fn main() -> anyhow::Result<()> { main_inner().await } +pub(crate) fn get_leaf_subcommand_name(matches: &clap::ArgMatches) -> Option { + match matches.subcommand() { + Some((name, sub_matches)) => { + get_leaf_subcommand_name(sub_matches).or(Some(name.to_string())) + } + None => None, + } +} + +pub(crate) fn get_top_level_subcommand_name(matches: &clap::ArgMatches) -> Option { + matches.subcommand().map(|(name, _)| name.to_string()) +} + async fn main_inner() -> anyhow::Result<()> { // In agent mode, intercept --help to return a JSON schema instead of plain text. let args: Vec = std::env::args().collect(); @@ -4894,7 +4922,8 @@ async fn main_inner() -> anyhow::Result<()> { return Ok(()); } - let cli = Cli::parse(); + let matches = Cli::command().get_matches(); + let cli = Cli::from_arg_matches(&matches).unwrap_or_else(|e| e.exit()); let mut cfg = config::Config::from_env()?; // Apply flag overrides @@ -4922,6 +4951,26 @@ async fn main_inner() -> anyhow::Result<()> { } } + if cli.read_only { + cfg.read_only = true; + } + if cfg.read_only { + let top = get_top_level_subcommand_name(&matches); + let is_local_only = matches!(top.as_deref(), Some("auth") | Some("alias")); + if !is_local_only { + if let Some(leaf) = get_leaf_subcommand_name(&matches) { + if is_write_command_name(&leaf) { + anyhow::bail!( + "write operation '{}' is blocked in read-only mode \ + (--read-only flag, DD_READ_ONLY / DD_CLI_READ_ONLY env var, \ + or read_only: true in config file)", + leaf + ); + } + } + } + } + match cli.command { // --- Monitors --- Commands::Monitors { action } => { diff --git a/src/test_commands.rs b/src/test_commands.rs index 8bb63d6..d546244 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -8,6 +8,7 @@ //! its own mockito server, so there's no cross-test interference. use crate::config::{Config, OutputFormat}; +use clap::CommandFactory; use std::sync::Mutex; /// Global mutex to serialize tests that modify process-wide env vars. @@ -32,6 +33,7 @@ fn test_config(mock_url: &str) -> Config { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, } } @@ -391,6 +393,7 @@ async fn test_logs_search_with_oauth() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let _mock = mock_any(&mut server, "POST", r#"{"data": []}"#).await; @@ -692,6 +695,7 @@ async fn test_events_search_requires_api_keys() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let result = @@ -721,6 +725,7 @@ async fn test_api_get() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -754,6 +759,7 @@ async fn test_api_get_with_query() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -791,6 +797,7 @@ async fn test_api_post() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -823,6 +830,7 @@ async fn test_api_put() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -855,6 +863,7 @@ async fn test_api_patch() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -887,6 +896,7 @@ async fn test_api_delete() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -918,6 +928,7 @@ async fn test_api_error_response() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -950,6 +961,7 @@ async fn test_api_bearer_auth() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -980,6 +992,7 @@ async fn test_api_no_auth() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let result = crate::api::get(&cfg, "/api/v1/test", &[]).await; @@ -1006,6 +1019,7 @@ async fn test_api_empty_response() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -1038,6 +1052,7 @@ async fn test_api_server_error() { output_format: OutputFormat::Json, auto_approve: false, agent_mode: false, + read_only: false, }; let mock = server @@ -1912,3 +1927,100 @@ async fn test_apm_services_list() { crate::commands::apm::services_list(&cfg, "prod".into(), "1h".into(), "now".into()).await; cleanup_env(); } + +// ------------------------------------------------------------------------- +// Read-only mode +// ------------------------------------------------------------------------- + +#[test] +fn test_is_write_command_name_writes() { + assert!(crate::is_write_command_name("delete")); + assert!(crate::is_write_command_name("create")); + assert!(crate::is_write_command_name("update")); + assert!(crate::is_write_command_name("cancel")); + assert!(crate::is_write_command_name("trigger")); + assert!(crate::is_write_command_name("submit")); + assert!(crate::is_write_command_name("send")); + assert!(crate::is_write_command_name("move")); + assert!(crate::is_write_command_name("link")); + assert!(crate::is_write_command_name("unlink")); + assert!(crate::is_write_command_name("configure")); + assert!(crate::is_write_command_name("upgrade")); + assert!(crate::is_write_command_name("update-status")); + assert!(crate::is_write_command_name("create-page")); + assert!(crate::is_write_command_name("patch")); + assert!(crate::is_write_command_name("patch-deployment")); +} + +#[test] +fn test_is_write_command_name_reads() { + assert!(!crate::is_write_command_name("list")); + assert!(!crate::is_write_command_name("get")); + assert!(!crate::is_write_command_name("search")); + assert!(!crate::is_write_command_name("query")); + assert!(!crate::is_write_command_name("aggregate")); + assert!(!crate::is_write_command_name("status")); + assert!(!crate::is_write_command_name("dispatch")); +} + +#[test] +fn test_read_only_guard_blocks_write() { + let matches = crate::Cli::command() + .try_get_matches_from(["pup", "monitors", "delete", "12345"]) + .unwrap(); + let leaf = crate::get_leaf_subcommand_name(&matches).unwrap(); + assert!(crate::is_write_command_name(&leaf)); +} + +#[test] +fn test_read_only_guard_allows_read() { + let matches = crate::Cli::command() + .try_get_matches_from(["pup", "monitors", "list"]) + .unwrap(); + let leaf = crate::get_leaf_subcommand_name(&matches).unwrap(); + assert!(!crate::is_write_command_name(&leaf)); +} + +#[test] +fn test_read_only_guard_nested_read() { + let matches = crate::Cli::command() + .try_get_matches_from(["pup", "rum", "apps", "list"]) + .unwrap(); + let leaf = crate::get_leaf_subcommand_name(&matches).unwrap(); + assert!(!crate::is_write_command_name(&leaf)); +} + +#[test] +fn test_read_only_guard_nested_write() { + let matches = crate::Cli::command() + .try_get_matches_from([ + "pup", + "cases", + "jira", + "create-issue", + "123", + "--file", + "f.json", + ]) + .unwrap(); + let leaf = crate::get_leaf_subcommand_name(&matches).unwrap(); + assert!(crate::is_write_command_name(&leaf)); +} + +#[test] +fn test_read_only_guard_exempts_alias() { + let matches = crate::Cli::command() + .try_get_matches_from(["pup", "alias", "set", "foo", "logs search *"]) + .unwrap(); + let top = crate::get_top_level_subcommand_name(&matches); + assert_eq!(top.as_deref(), Some("alias")); +} + +#[test] +fn test_read_only_guard_exempts_auth() { + let matches = crate::Cli::command() + .try_get_matches_from(["pup", "auth", "login"]) + .unwrap(); + let top = crate::get_top_level_subcommand_name(&matches); + assert_eq!(top.as_deref(), Some("auth")); +}