From c72539159712fa9ede48654dafdc8e8a754d7a41 Mon Sep 17 00:00:00 2001 From: Swaraj Rao Date: Mon, 2 Mar 2026 18:38:53 -0500 Subject: [PATCH 1/3] fix(schema): extract is_write_command_name and add 5 missing write names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract inline is_write logic from build_command_schema() into a standalone pub(crate) fn is_write_command_name() for reuse by the upcoming read-only runtime guard. - Added 5 missing write command names: move, link, unlink, configure, upgrade - Tightened patch check: name.contains("patch") → name == "patch" || name.starts_with("patch-") - Fixes agent schema JSON output where these commands were incorrectly marked read_only: true Co-Authored-By: Claude Opus 4.6 --- src/main.rs | 40 ++++++++++++++++++---------------------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/main.rs b/src/main.rs index 631f561..0755e72 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4738,6 +4738,23 @@ fn build_compact_agent_schema(cmd: &clap::Command) -> serde_json::Value { serde_json::Value::Object(root) } +/// 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" || name == "trigger" || name == "set" + || name == "add" || name == "remove" || name == "assign" + || name == "archive" || 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" || name == "send" || name == "import" + || name == "register" || name == "unregister" + || name.contains("delete") + || 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(); @@ -4756,28 +4773,7 @@ fn build_command_schema(cmd: &clap::Command, parent_path: &str) -> serde_json::V // 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" - || name == "create" - || name == "update" - || name == "cancel" - || name == "trigger" - || name == "set" - || name == "add" - || name == "remove" - || name == "assign" - || name == "archive" - || name == "unarchive" - || name == "activate" - || name == "deactivate" - || name.starts_with("update-") - || name.starts_with("create-") - || name == "submit" - || name == "send" - || name == "import" - || name == "register" - || name == "unregister" - || name.contains("delete") - || name.contains("patch"); + let is_write = is_write_command_name(&name); // Flags (named --flags only, excluding positional args and globals) let flags: Vec = cmd From 07691bde7b26f9748cabafbc69cefeb1d5c33be7 Mon Sep 17 00:00:00 2001 From: Swaraj Rao Date: Mon, 2 Mar 2026 18:39:25 -0500 Subject: [PATCH 2/3] feat(cli): add --read-only mode to block write operations Add a --read-only flag, DD_READ_ONLY / DD_CLI_READ_ONLY env vars, and read_only config file option that blocks all write (CUD) operations at runtime, allowing only read operations. - Added read_only field to Config and FileConfig structs - Added --read-only global CLI flag to Cli struct - Refactored main_inner() to use ArgMatches for subcommand introspection - Added get_leaf_subcommand_name() and get_top_level_subcommand_name() helpers - Runtime guard fires before auth validation (fail-fast) - auth and alias commands exempted (local-only state) - Updated all 14 Config struct literals in test_commands.rs - Added 10 new tests covering write detection, read commands, nested commands, and auth/alias exemptions - Updated COMMANDS.md, ARCHITECTURE.md, and EXAMPLES.md docs Co-Authored-By: Claude Opus 4.6 --- docs/ARCHITECTURE.md | 3 ++ docs/COMMANDS.md | 1 + docs/EXAMPLES.md | 10 +++++ src/config.rs | 39 ++++++++++++++++ src/main.rs | 43 ++++++++++++++++-- src/test_commands.rs | 104 +++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 197 insertions(+), 3 deletions(-) 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/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/main.rs b/src/main.rs index 0755e72..6950f01 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, @@ -4863,6 +4866,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(); @@ -4890,7 +4906,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 @@ -4918,6 +4935,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..7b058d9 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,92 @@ 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")); +} From 1c533dc7f0e1c1422f1c9fa5e283f4cef961f5f0 Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Mon, 2 Mar 2026 21:07:35 -0600 Subject: [PATCH 3/3] fix(tests): add missing read_only field to Config initializers and fix formatting Two test helpers (client.rs and formatter.rs) were missing the new `read_only: false` field added to `config::Config`, causing compile errors in CI. Also run cargo fmt to fix rustfmt violations in main.rs (is_write_command_name) and test_commands.rs (long array literal). Co-Authored-By: Claude Sonnet 4.6 (1M context) --- src/client.rs | 1 + src/formatter.rs | 1 + src/main.rs | 38 +++++++++++++++++++++++++++----------- src/test_commands.rs | 10 +++++++++- 4 files changed, 38 insertions(+), 12 deletions(-) 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/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 6950f01..ebd31fb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -4744,18 +4744,34 @@ fn build_compact_agent_schema(cmd: &clap::Command) -> serde_json::Value { /// 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" || name == "trigger" || name == "set" - || name == "add" || name == "remove" || name == "assign" - || name == "archive" || 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" || name == "send" || name == "import" - || name == "register" || name == "unregister" + name == "delete" + || name == "create" + || name == "update" + || name == "cancel" + || name == "trigger" + || name == "set" + || name == "add" + || name == "remove" + || name == "assign" + || name == "archive" + || 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" + || name == "send" + || name == "import" + || name == "register" + || name == "unregister" || name.contains("delete") - || name == "patch" || name.starts_with("patch-") + || name == "patch" + || name.starts_with("patch-") } fn build_command_schema(cmd: &clap::Command, parent_path: &str) -> serde_json::Value { diff --git a/src/test_commands.rs b/src/test_commands.rs index 7b058d9..d546244 100644 --- a/src/test_commands.rs +++ b/src/test_commands.rs @@ -1993,7 +1993,15 @@ fn test_read_only_guard_nested_read() { #[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"]) + .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));