From 95945346567e2b54447fa2c6d06eaa848aa0f86e Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Fri, 5 Jun 2026 09:05:48 -0500 Subject: [PATCH 1/2] feat(extensions): let custom extensions reuse pup's client and formatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extensions are external executables in any language; today an author must re-implement HTTP+auth and output formatting to talk to Datadog. Expose pup's existing client and formatter through the runtime contract so an extension can shell out to the parent `pup` binary instead. - pup api now routes auth through client::apply_auth (made pub), gaining the per-endpoint OAuth/API-key fallback (e.g. /api/v2/api_keys now correctly uses API keys instead of failing with a bearer token), and renders responses via formatter::format_and_print so --output / agent mode are honored. - New `pup format` (alias `fmt`): reads JSON from stdin/--input and renders it through the shared formatter with optional agent-envelope metadata. - config::from_env reads PUP_OUTPUT/PUP_READ_ONLY/PUP_AUTO_APPROVE and useragent::is_agent_mode reads PUP_AGENT_MODE — the vars pup injects into extension subprocesses — so a child `pup` call inherits the parent's format and mode. This required changing --output from a defaulted String to Option (validated via resolve_output_format) so env-derived formats are no longer clobbered by the "json" default. - docs/EXTENSIONS.md documents the `pup api | pup format` reuse pattern. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/EXTENSIONS.md | 48 ++++++++++ src/client.rs | 9 +- src/commands/api.rs | 151 +++++++++++++++++++++++++++++--- src/commands/format.rs | 193 +++++++++++++++++++++++++++++++++++++++++ src/commands/mod.rs | 2 + src/config.rs | 61 ++++++++++++- src/main.rs | 97 +++++++++++++++++++-- src/useragent.rs | 18 +++- 8 files changed, 558 insertions(+), 21 deletions(-) create mode 100644 src/commands/format.rs diff --git a/docs/EXTENSIONS.md b/docs/EXTENSIONS.md index 3a378be5..1865e2a1 100644 --- a/docs/EXTENSIONS.md +++ b/docs/EXTENSIONS.md @@ -109,8 +109,12 @@ Pup refreshes the OAuth2 token if needed before passing it to the extension, so Variables not active in the current session are explicitly removed from the child environment to prevent stale credentials from leaking through the parent shell. +The `PUP_OUTPUT`, `PUP_AGENT_MODE`, `PUP_READ_ONLY`, and `PUP_AUTO_APPROVE` variables are read back by a child `pup` process. So if your extension shells out to `pup` (see below), those nested calls automatically inherit the format and mode the user selected on the parent command. + ### Example: using auth in a Python extension +The example below hand-rolls the HTTP call. If pup is on `PATH` (it is, when pup dispatched your extension), prefer shelling out to `pup api` and `pup format` instead — see [Reusing pup's client and formatter](#reusing-pups-client-and-formatter) below. + ```python #!/usr/bin/env python3 import os, requests @@ -131,6 +135,50 @@ print(resp.json()) Extensions written in Rust can use the `datadog-api-client` crate. The standard Datadog SDK env vars (`DD_API_KEY`, `DD_APP_KEY`, `DD_SITE`) are forwarded automatically, so most SDKs will work without any extra configuration. +## Reusing pup's client and formatter + +The examples above hand-roll HTTP and auth. You usually don't need to. Because pup is on `PATH` when it dispatched your extension, an extension in **any language** can shell out to the parent `pup` binary and reuse pup's request handling and output formatting directly — no auth, refresh, site-resolution, or table/CSV code of your own. + +### Make authenticated API calls with `pup api` + +`pup api ` reuses pup's full auth handler: it chooses OAuth bearer vs. API-key auth, applies the per-endpoint fallback for endpoints that don't accept OAuth (e.g. `/api/v2/api_keys`, fleet, cost), sets the branded User-Agent, and resolves the site. The extension already has a fresh `DD_ACCESS_TOKEN` (or the `DD_API_KEY`/`DD_APP_KEY` pair) in its environment, so these calls are authenticated automatically. + +```bash +#!/bin/bash +# GET as JSON, then extract fields. --silent suppresses pup's own rendering. +pup api v2/monitors --silent | jq -r '.[].name' + +# POST a typed body (-F coerces ints/bools/null; -f keeps raw strings). +pup api v2/tags/hosts/myhost -X POST -F source=web +``` + +### Render output with `pup format` + +`pup format` (alias `fmt`) reads a JSON document from stdin (or `--input FILE`) and prints it using the caller's output format and agent mode — the same JSON / YAML / table / CSV / TSV rendering and agent envelope every built-in command uses. Because pup forwards `PUP_OUTPUT` and a child `pup` reads it back, your extension inherits the format the user originally requested. + +```bash +#!/bin/bash +results='[{"id":1,"name":"alpha"},{"id":2,"name":"beta"}]' + +# Honor whatever -o the user passed to `pup my-tool`. +echo "$results" | pup format + +# Or force a specific format. +echo "$results" | pup format --output table + +# Populate the agent-mode envelope metadata. +echo "$results" | pup format --count 2 --command "my-tool list" +``` + +### Combine them + +A fully consistent extension that adds zero auth or formatting code: + +```bash +#!/bin/bash +pup api v2/monitors --silent | pup format +``` + ## Global Flags Pup's global flags (`--output`, `--yes`, `--agent`, `--read-only`, `--org`) are parsed by pup before dispatching to the extension. They are NOT passed as CLI arguments to the extension - instead, they are forwarded as environment variables (see the table above). diff --git a/src/client.rs b/src/client.rs index 4f00c3a5..b6f2715e 100644 --- a/src/client.rs +++ b/src/client.rs @@ -958,7 +958,14 @@ async fn raw_post_impl( parse_response_json(resp).await } -fn apply_auth( +/// Apply Datadog authentication headers to a request builder. +/// +/// Chooses between OAuth bearer and API-key/App-key auth based on `cfg` and the +/// per-endpoint requirements in [`requires_api_key_fallback`]: endpoints that do +/// not accept OAuth (see `OAUTH_EXCLUDED_ENDPOINTS`) force API-key auth even when +/// a bearer token is present. Exposed so the generic `pup api` passthrough reuses +/// the same auth routing as the typed clients. +pub fn apply_auth( mut req: reqwest::RequestBuilder, cfg: &Config, method: &str, diff --git a/src/commands/api.rs b/src/commands/api.rs index 80ba13f4..62fa93ee 100644 --- a/src/commands/api.rs +++ b/src/commands/api.rs @@ -85,12 +85,24 @@ pub async fn run( let method_upper = method.to_uppercase(); // Full URLs pass through; relative paths get the API base prepended. - let url = if endpoint.starts_with("http://") || endpoint.starts_with("https://") { + let is_absolute = endpoint.starts_with("http://") || endpoint.starts_with("https://"); + let url = if is_absolute { endpoint.to_string() } else { format!("{}{}", cfg.api_base_url(), normalize_path(endpoint)) }; + // Path used for per-endpoint auth routing. For relative endpoints this is the + // normalized API path; for absolute URLs we use the URL's path component so the + // OAuth-exclusion table (client::requires_api_key_fallback) still applies. + let auth_path = if is_absolute { + reqwest::Url::parse(&url) + .map(|u| u.path().to_string()) + .unwrap_or_else(|_| normalize_path(endpoint)) + } else { + normalize_path(endpoint) + }; + // POST, PUT, PATCH carry a body; GET/HEAD/DELETE use query params. let is_body_method = matches!(method_upper.as_str(), "POST" | "PUT" | "PATCH"); @@ -134,15 +146,10 @@ pub async fn run( .map_err(|_| anyhow::anyhow!("unsupported HTTP method: {method}"))?; let mut req = client.request(method_val, &url); - if let Some(token) = &cfg.access_token { - req = req.header("Authorization", format!("Bearer {token}")); - } else if let (Some(api_key), Some(app_key)) = (&cfg.api_key, &cfg.app_key) { - req = req - .header("DD-API-KEY", api_key.as_str()) - .header("DD-APPLICATION-KEY", app_key.as_str()); - } else { - bail!("authentication required: run 'pup auth login' or set DD_API_KEY and DD_APP_KEY"); - } + // Reuse the shared auth handler so `pup api` (and extensions that shell out to + // it) get the same OAuth-vs-API-key routing as the typed clients, including the + // per-endpoint OAuth-exclusion fallback. + req = crate::client::apply_auth(req, cfg, &method_upper, &auth_path)?; req = req .header("User-Agent", useragent::get()) @@ -198,7 +205,9 @@ pub async fn run( if !silent && !body_bytes.is_empty() { if let Ok(json) = serde_json::from_slice::(&body_bytes) { - println!("{}", serde_json::to_string_pretty(&json)?); + // Render through the shared formatter so `--output`/agent mode are + // honored, matching every other pup command. + crate::formatter::format_and_print(&json, &cfg.output_format, cfg.agent_mode, None)?; } else { print!("{}", String::from_utf8_lossy(&body_bytes)); } @@ -522,4 +531,124 @@ mod tests { assert!(result.is_err(), "expected error for malformed field"); cleanup_env(); } + + /// `pup api -o table` must render through the shared formatter without error, + /// proving the output now honors cfg.output_format instead of always JSON. + #[tokio::test] + async fn test_api_table_output() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let mut cfg = test_config(&server.url()); + cfg.output_format = crate::config::OutputFormat::Table; + let _mock = server + .mock("GET", "/api/v2/monitors") + .match_query(mockito::Matcher::Any) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"[{"id":1,"name":"Test"}]"#) + .create_async() + .await; + + let result = super::run( + &cfg, + "v2/monitors", + "GET", + &[], + &[], + &[], + None, + false, + false, + false, + ) + .await; + assert!(result.is_ok(), "api GET table failed: {:?}", result.err()); + cleanup_env(); + } + + /// OAuth-excluded endpoints (e.g. GET /api/v2/api_keys) must use API-key auth + /// even when a bearer token is present. This exercises the reuse of + /// client::apply_auth's per-endpoint fallback table. + #[tokio::test] + async fn test_api_oauth_excluded_uses_api_keys() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let mut cfg = test_config(&server.url()); + // Both a bearer token AND API keys are configured; the excluded endpoint + // must prefer the API keys. + cfg.access_token = Some("bearer-token".into()); + let _mock = server + .mock("GET", "/api/v2/api_keys") + .match_query(mockito::Matcher::Any) + .match_header("DD-API-KEY", "test-api-key") + .match_header("DD-APPLICATION-KEY", "test-app-key") + .match_header("authorization", mockito::Matcher::Missing) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"data":[]}"#) + .create_async() + .await; + + let result = super::run( + &cfg, + "v2/api_keys", + "GET", + &[], + &[], + &[], + None, + false, + true, + false, + ) + .await; + assert!( + result.is_ok(), + "expected API-key auth on OAuth-excluded endpoint: {:?}", + result.err() + ); + cleanup_env(); + } + + /// An absolute http(s):// endpoint must still consult the OAuth-exclusion + /// table via its URL path — exercising the `is_absolute` auth_path branch. + #[tokio::test] + async fn test_api_absolute_url_oauth_excluded_uses_api_keys() { + let _lock = lock_env().await; + let mut server = mockito::Server::new_async().await; + let mut cfg = test_config(&server.url()); + cfg.access_token = Some("bearer-token".into()); + let _mock = server + .mock("GET", "/api/v2/api_keys") + .match_query(mockito::Matcher::Any) + .match_header("DD-API-KEY", "test-api-key") + .match_header("authorization", mockito::Matcher::Missing) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"data":[]}"#) + .create_async() + .await; + + // Pass the fully-qualified URL, not a relative path. + let absolute = format!("{}/api/v2/api_keys", server.url()); + let result = super::run( + &cfg, + &absolute, + "GET", + &[], + &[], + &[], + None, + false, + true, + false, + ) + .await; + assert!( + result.is_ok(), + "expected API-key auth on absolute OAuth-excluded URL: {:?}", + result.err() + ); + cleanup_env(); + } } diff --git a/src/commands/format.rs b/src/commands/format.rs new file mode 100644 index 00000000..6bc4cac9 --- /dev/null +++ b/src/commands/format.rs @@ -0,0 +1,193 @@ +use anyhow::{Context, Result}; +use serde_json::Value; +use std::io::Read; + +use crate::config::Config; +use crate::formatter::{self, Metadata}; + +/// Render JSON through pup's formatter. +/// +/// Reads a JSON document from stdin (default) or `--input FILE`, then prints it +/// using the configured output format (`--output`, `$DD_OUTPUT`/`$PUP_OUTPUT`) and +/// agent mode. This lets an extension in any language produce JSON and reuse pup's +/// table/yaml/csv/tsv rendering and agent envelope instead of reimplementing them. +/// +/// The optional metadata flags populate the agent-mode envelope and are ignored +/// for non-agent, non-JSON formats. +pub fn run( + cfg: &Config, + input: Option<&str>, + count: Option, + command: Option, + next_action: Option, +) -> Result<()> { + let raw = read_input(input, std::io::stdin().lock())?; + render(cfg, &raw, count, command, next_action) +} + +/// Read the JSON document from a file (`Some(path)` other than `"-"`) or from the +/// provided reader (`None` or `"-"`, i.e. stdin). The reader is a parameter so the +/// stdin path can be tested without touching the process's real stdin. +fn read_input(input: Option<&str>, mut reader: impl Read) -> Result { + match input { + Some(path) if path != "-" => std::fs::read_to_string(path) + .map_err(|e| anyhow::anyhow!("failed to read --input {path:?}: {e}")), + _ => { + let mut buf = String::new(); + reader + .read_to_string(&mut buf) + .context("failed to read JSON from stdin")?; + Ok(buf) + } + } +} + +/// Parse `raw` as JSON and print it through the shared formatter. +fn render( + cfg: &Config, + raw: &str, + count: Option, + command: Option, + next_action: Option, +) -> Result<()> { + if raw.trim().is_empty() { + anyhow::bail!("no JSON input provided (pipe JSON to stdin or pass --input FILE)"); + } + + let value: Value = serde_json::from_str(raw).context("input is not valid JSON")?; + + // Only build a metadata envelope when at least one field is supplied; otherwise + // pass None so the output matches `pup api` / other commands with no metadata. + let meta = if count.is_some() || command.is_some() || next_action.is_some() { + Some(Metadata { + count, + truncated: false, + command, + next_action, + }) + } else { + None + }; + + formatter::format_and_print(&value, &cfg.output_format, cfg.agent_mode, meta.as_ref()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::OutputFormat; + use crate::test_support::write_temp_json; + use std::io::Cursor; + + #[test] + fn test_read_input_from_stdin_when_none() { + // input None → read from the provided reader (stdin in production). + let got = read_input(None, Cursor::new(b"[1,2,3]".to_vec())).unwrap(); + assert_eq!(got, "[1,2,3]"); + } + + #[test] + fn test_read_input_from_stdin_when_dash() { + // input "-" → read from the provided reader (stdin in production). + let got = read_input(Some("-"), Cursor::new(b"{\"a\":1}".to_vec())).unwrap(); + assert_eq!(got, "{\"a\":1}"); + } + + #[test] + fn test_read_input_from_file_ignores_reader() { + let path = write_temp_json("pup_format_read_input.json", r#"{"from":"file"}"#); + // A non-"-" path reads the file, not the reader. + let got = read_input( + path.to_str(), + Cursor::new(b"{\"from\":\"reader\"}".to_vec()), + ); + std::fs::remove_file(&path).ok(); + assert_eq!(got.unwrap(), r#"{"from":"file"}"#); + } + + #[test] + fn test_render_stdin_table() { + // The primary documented path: JSON piped via stdin, rendered as a table. + let cfg = cfg_with(OutputFormat::Table, false); + let raw = read_input(None, Cursor::new(b"[{\"id\":1}]".to_vec())).unwrap(); + let result = render(&cfg, &raw, None, None, None); + assert!(result.is_ok(), "stdin render failed: {:?}", result.err()); + } + + fn cfg_with(format: OutputFormat, agent_mode: bool) -> Config { + Config { + api_key: None, + app_key: None, + access_token: None, + site: "datadoghq.com".into(), + site_explicit: false, + org: None, + output_format: format, + auto_approve: false, + agent_mode, + read_only: false, + } + } + + #[test] + fn test_run_reads_file_input_json() { + let path = write_temp_json("pup_format_input.json", r#"[{"id":1,"name":"x"}]"#); + let cfg = cfg_with(OutputFormat::Json, false); + let result = run(&cfg, path.to_str(), None, None, None); + std::fs::remove_file(&path).ok(); + assert!( + result.is_ok(), + "format from file failed: {:?}", + result.err() + ); + } + + #[test] + fn test_run_table_format_from_file() { + let path = write_temp_json("pup_format_table.json", r#"[{"id":1,"name":"x"}]"#); + let cfg = cfg_with(OutputFormat::Table, false); + let result = run(&cfg, path.to_str(), None, None, None); + std::fs::remove_file(&path).ok(); + assert!(result.is_ok(), "table format failed: {:?}", result.err()); + } + + #[test] + fn test_run_agent_envelope_with_metadata() { + let path = write_temp_json("pup_format_agent.json", r#"{"data":[]}"#); + let cfg = cfg_with(OutputFormat::Json, true); + let result = run(&cfg, path.to_str(), Some(0), Some("format".into()), None); + std::fs::remove_file(&path).ok(); + assert!(result.is_ok(), "agent envelope failed: {:?}", result.err()); + } + + #[test] + fn test_run_invalid_json_errors() { + let path = write_temp_json("pup_format_bad.json", "{not json"); + let cfg = cfg_with(OutputFormat::Json, false); + let result = run(&cfg, path.to_str(), None, None, None); + std::fs::remove_file(&path).ok(); + assert!(result.is_err(), "expected error for invalid JSON"); + } + + #[test] + fn test_run_empty_input_errors() { + let path = write_temp_json("pup_format_empty.json", " \n"); + let cfg = cfg_with(OutputFormat::Json, false); + let result = run(&cfg, path.to_str(), None, None, None); + std::fs::remove_file(&path).ok(); + assert!(result.is_err(), "expected error for empty input"); + } + + #[test] + fn test_run_missing_file_errors() { + let cfg = cfg_with(OutputFormat::Json, false); + let result = run( + &cfg, + Some("/nonexistent/pup-format/x.json"), + None, + None, + None, + ); + assert!(result.is_err(), "expected error for missing file"); + } +} diff --git a/src/commands/mod.rs b/src/commands/mod.rs index ef94112f..a8bb41c9 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -40,6 +40,8 @@ pub mod events; pub mod extension; pub mod feature_flags; pub mod fleet; +#[cfg(not(target_arch = "wasm32"))] +pub mod format; pub mod google_chat; pub mod hamr; pub mod idp; diff --git a/src/config.rs b/src/config.rs index 95e05b62..c39b990c 100644 --- a/src/config.rs +++ b/src/config.rs @@ -125,15 +125,29 @@ impl Config { site, site_explicit, org, - output_format: env_or("DD_OUTPUT", file_cfg.output) + // `PUP_OUTPUT` is the variable pup injects into extension subprocesses, + // so a child `pup` call inherits the parent's format. `DD_OUTPUT` (the + // user-facing variable) still wins when both are set. + // + // Ambient config (env vars, config file) degrades to JSON on an + // unparseable value rather than erroring — this is deliberate: a bad + // value here should not make every command fail. The explicit + // `--output` flag, by contrast, is validated and errors loudly + // (see `resolve_output_format` in main.rs). + output_format: env_or("DD_OUTPUT", None) + .or_else(|| env_or("PUP_OUTPUT", file_cfg.output)) .and_then(|s| s.parse().ok()) .unwrap_or(OutputFormat::Json), + // `PUP_AUTO_APPROVE` / `PUP_READ_ONLY` are injected into extension + // subprocesses so a child `pup` call inherits the parent's mode flags. auto_approve: env_bool("DD_AUTO_APPROVE") || env_bool("DD_CLI_AUTO_APPROVE") + || env_bool("PUP_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") + || env_bool("PUP_READ_ONLY") || file_cfg.read_only.unwrap_or(false), }; @@ -1020,6 +1034,51 @@ mod tests { std::env::remove_var("PUP_CONFIG_DIR"); } + /// PUP_* variables are what pup injects into extension subprocesses, so a + /// child `pup` invocation must inherit the parent's output format and mode. + #[test] + fn test_pup_env_inherited_by_child() { + let _guard = ENV_LOCK.blocking_lock(); + std::env::set_var("PUP_CONFIG_DIR", "/tmp/pup_test_nonexistent"); + std::env::set_var("DD_ACCESS_TOKEN", "test"); + for var in [ + "DD_OUTPUT", + "PUP_OUTPUT", + "DD_READ_ONLY", + "DD_CLI_READ_ONLY", + "PUP_READ_ONLY", + "DD_AUTO_APPROVE", + "DD_CLI_AUTO_APPROVE", + "PUP_AUTO_APPROVE", + ] { + std::env::remove_var(var); + } + + // PUP_OUTPUT drives the format when DD_OUTPUT is unset. + std::env::set_var("PUP_OUTPUT", "table"); + let cfg = Config::from_env().unwrap(); + assert_eq!(cfg.output_format, OutputFormat::Table); + + // DD_OUTPUT wins over PUP_OUTPUT when both are set. + std::env::set_var("DD_OUTPUT", "yaml"); + let cfg = Config::from_env().unwrap(); + assert_eq!(cfg.output_format, OutputFormat::Yaml); + std::env::remove_var("DD_OUTPUT"); + std::env::remove_var("PUP_OUTPUT"); + + // PUP_READ_ONLY / PUP_AUTO_APPROVE flip the mode flags. + std::env::set_var("PUP_READ_ONLY", "true"); + std::env::set_var("PUP_AUTO_APPROVE", "true"); + let cfg = Config::from_env().unwrap(); + assert!(cfg.read_only); + assert!(cfg.auto_approve); + + std::env::remove_var("PUP_READ_ONLY"); + std::env::remove_var("PUP_AUTO_APPROVE"); + std::env::remove_var("DD_ACCESS_TOKEN"); + std::env::remove_var("PUP_CONFIG_DIR"); + } + /// Per-org session sites: when DD_ORG is set and DD_SITE is not, the saved /// session's site should win over the default. site_explicit must remain /// false so subsequent --org overrides can still adjust it. diff --git a/src/main.rs b/src/main.rs index 0e2d7dbf..73f89855 100644 --- a/src/main.rs +++ b/src/main.rs @@ -41,9 +41,9 @@ use clap::{CommandFactory, FromArgMatches, Parser, Subcommand}; #[derive(Parser)] #[command(name = "pup", version = version::VERSION, about = "Datadog API CLI")] pub(crate) struct Cli { - /// Output format (json, table, yaml, csv) - #[arg(short, long, global = true, default_value = "json")] - output: String, + /// Output format (json, table, yaml, csv). Defaults to json, or $DD_OUTPUT / $PUP_OUTPUT when set. + #[arg(short, long, global = true)] + output: Option, /// Auto-approve destructive operations #[arg(short = 'y', long = "yes", global = true)] yes: bool, @@ -1372,6 +1372,32 @@ enum Commands { #[command(subcommand)] action: FleetActions, }, + /// Render JSON through pup's formatter + /// + /// Reads a JSON document from stdin (or --input FILE) and prints it using the + /// configured output format (--output, or $DD_OUTPUT / $PUP_OUTPUT) and agent + /// mode. Lets an extension in any language reuse pup's table/yaml/csv/tsv + /// rendering and agent envelope instead of reimplementing them. + /// + /// EXAMPLES: + /// pup api v2/monitors --silent | pup format --output table + /// echo '[{"id":1}]' | pup format --output csv + #[cfg(not(target_arch = "wasm32"))] + #[command(visible_alias = "fmt", verbatim_doc_comment)] + Format { + /// Read JSON from file, or use "-" (default) for stdin + #[arg(long, value_name = "FILE")] + input: Option, + /// Set metadata.count in the agent-mode envelope + #[arg(long, value_name = "N")] + count: Option, + /// Set metadata.command in the agent-mode envelope + #[arg(long, value_name = "STR")] + command: Option, + /// Set metadata.next_action in the agent-mode envelope + #[arg(long, value_name = "STR")] + next_action: Option, + }, /// Manage High Availability Multi-Region (HAMR) /// /// Manage Datadog High Availability Multi-Region (HAMR) connections. @@ -10829,6 +10855,52 @@ mod resolve_callback_port_tests { } } +/// Resolve the effective output format. An explicit `--output` flag wins and is +/// validated — a malformed value is a hard error rather than being silently +/// ignored. When the flag is absent, the format already resolved from env/config +/// in `Config::from_env` is kept, so `DD_OUTPUT` / `PUP_OUTPUT` (and the format an +/// extension inherits from its parent) survive. +fn resolve_output_format( + flag: Option<&str>, + resolved: config::OutputFormat, +) -> anyhow::Result { + match flag { + Some(s) => s + .parse() + .map_err(|e| anyhow::anyhow!("invalid --output value: {e}")), + None => Ok(resolved), + } +} + +#[cfg(test)] +mod resolve_output_format_tests { + use super::resolve_output_format; + use crate::config::OutputFormat; + + #[test] + fn explicit_flag_overrides_resolved() { + let got = resolve_output_format(Some("table"), OutputFormat::Json).unwrap(); + assert_eq!(got, OutputFormat::Table); + } + + #[test] + fn absent_flag_keeps_resolved() { + // No --output: the env/config-resolved format (e.g. an inherited + // PUP_OUTPUT) must survive instead of being clobbered by a default. + let got = resolve_output_format(None, OutputFormat::Yaml).unwrap(); + assert_eq!(got, OutputFormat::Yaml); + } + + #[test] + fn invalid_explicit_flag_errors() { + let got = resolve_output_format(Some("tabel"), OutputFormat::Json); + assert!( + got.is_err(), + "a malformed --output value must be a hard error" + ); + } +} + 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(); @@ -10929,10 +11001,10 @@ async fn main_inner() -> anyhow::Result<()> { let mut cfg = config::Config::from_env()?; - // Apply flag overrides - if let Ok(fmt) = cli.output.parse() { - cfg.output_format = fmt; - } + // Apply flag overrides. `--output` is only applied when explicitly set so the + // format resolved from DD_OUTPUT / PUP_OUTPUT in `from_env` survives when no + // flag is passed (extensions rely on inheriting the parent's format). + cfg.output_format = resolve_output_format(cli.output.as_deref(), cfg.output_format)?; if cli.yes { cfg.auto_approve = true; } @@ -14585,6 +14657,17 @@ async fn main_inner() -> anyhow::Result<()> { ) .await?; } + // --- Format --- + // No auth required: this only renders JSON the caller already has. + #[cfg(not(target_arch = "wasm32"))] + Commands::Format { + input, + count, + command, + next_action, + } => { + commands::format::run(&cfg, input.as_deref(), count, command, next_action)?; + } // --- Skills --- #[cfg(not(target_arch = "wasm32"))] Commands::Skills { action } => match action { diff --git a/src/useragent.rs b/src/useragent.rs index a20e1095..c6977446 100644 --- a/src/useragent.rs +++ b/src/useragent.rs @@ -92,7 +92,11 @@ pub fn detect_agent_info() -> AgentInfo { } pub fn is_agent_mode() -> bool { - is_env_truthy("FORCE_AGENT_MODE") || detect_agent_info().detected + // `PUP_AGENT_MODE` is injected into extension subprocesses so a child `pup` + // call inherits the parent's agent mode. + is_env_truthy("FORCE_AGENT_MODE") + || is_env_truthy("PUP_AGENT_MODE") + || detect_agent_info().detected } #[allow(dead_code)] @@ -164,6 +168,7 @@ mod tests { } } std::env::remove_var("FORCE_AGENT_MODE"); + std::env::remove_var("PUP_AGENT_MODE"); } #[test] @@ -296,6 +301,17 @@ mod tests { std::env::remove_var("FORCE_AGENT_MODE"); } + #[test] + fn test_is_agent_mode_via_pup_env() { + // PUP_AGENT_MODE is injected into extension subprocesses so a child `pup` + // call inherits the parent's agent mode. + let _guard = ENV_LOCK.blocking_lock(); + clear_all_agent_vars(); + std::env::set_var("PUP_AGENT_MODE", "true"); + assert!(is_agent_mode()); + std::env::remove_var("PUP_AGENT_MODE"); + } + #[test] fn test_is_agent_mode_via_detector() { let _guard = ENV_LOCK.blocking_lock(); From 8bb0bb89990bd0a14a9ea7ff6759c3fbdcbc5d95 Mon Sep 17 00:00:00 2001 From: Cody Lee Date: Fri, 5 Jun 2026 09:23:51 -0500 Subject: [PATCH 2/2] fix(api): don't send Datadog credentials to off-host absolute URLs `pup api` accepts absolute http(s):// endpoints. The previous change routed auth for absolute URLs through the OAuth-exclusion table, which meant a path like `https://evil.example/api/v2/api_keys` would match an excluded endpoint and exfiltrate the long-lived API keys to an arbitrary host (and the bearer token for any path). Add a credential-exfiltration guard: only relative paths and absolute URLs whose scheme + host + effective port match the configured Datadog API base (`cfg.api_base_url()`) receive Datadog credentials. Off-host absolute URLs are sent unauthenticated, with a stderr warning when credentials were configured. Comparing scheme prevents a cleartext `http://host:443` from riding an https config's credentials; host comparison is ASCII-case-insensitive; any URL parse failure fails closed. Tests cover the guard (case-insensitive host, userinfo `@` trick, http/port-80, http:443 scheme mismatch, custom datadoghq.eu site allow + cross-region reject) and the off-host request paths (both api-keys and bearer-only) asserting the authorization / DD-API-KEY / DD-APPLICATION-KEY headers are absent. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/commands/api.rs | 220 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 216 insertions(+), 4 deletions(-) diff --git a/src/commands/api.rs b/src/commands/api.rs index 62fa93ee..ef6de6d6 100644 --- a/src/commands/api.rs +++ b/src/commands/api.rs @@ -69,6 +69,25 @@ fn normalize_path(endpoint: &str) -> String { } } +/// Returns true when `url`'s scheme, host, and effective port all match the +/// configured Datadog API base (`cfg.api_base_url()`). Used as a credential- +/// exfiltration guard: an absolute URL pointing anywhere other than the configured +/// Datadog host must not receive Datadog credentials. Scheme is compared so a +/// cleartext `http://host:443` cannot ride the credentials of an `https` config, +/// and the host comparison is ASCII-case-insensitive (the `url` crate lowercases +/// hosts at parse time). Any parse failure fails closed (no credentials). +fn targets_configured_host(url: &str, cfg: &Config) -> bool { + let base = cfg.api_base_url(); + match (reqwest::Url::parse(url), reqwest::Url::parse(&base)) { + (Ok(u), Ok(b)) => { + u.scheme() == b.scheme() + && u.host_str() == b.host_str() + && u.port_or_known_default() == b.port_or_known_default() + } + _ => false, + } +} + #[allow(clippy::too_many_arguments)] pub async fn run( cfg: &Config, @@ -92,9 +111,17 @@ pub async fn run( format!("{}{}", cfg.api_base_url(), normalize_path(endpoint)) }; + // Only relative paths and absolute URLs that point at the configured Datadog + // host may carry Datadog credentials. An absolute URL to any other host is an + // SSRF / credential-exfiltration vector: without this guard, a path like + // `https://evil.example/api/v2/api_keys` would match the OAuth-exclusion table + // and leak the long-lived API keys to an arbitrary host. + let credentials_allowed = !is_absolute || targets_configured_host(&url, cfg); + // Path used for per-endpoint auth routing. For relative endpoints this is the - // normalized API path; for absolute URLs we use the URL's path component so the - // OAuth-exclusion table (client::requires_api_key_fallback) still applies. + // normalized API path; for absolute URLs on the Datadog host we use the URL's + // path component so the OAuth-exclusion table (client::requires_api_key_fallback) + // still applies. let auth_path = if is_absolute { reqwest::Url::parse(&url) .map(|u| u.path().to_string()) @@ -148,8 +175,21 @@ pub async fn run( // Reuse the shared auth handler so `pup api` (and extensions that shell out to // it) get the same OAuth-vs-API-key routing as the typed clients, including the - // per-endpoint OAuth-exclusion fallback. - req = crate::client::apply_auth(req, cfg, &method_upper, &auth_path)?; + // per-endpoint OAuth-exclusion fallback. Skipped for off-host absolute URLs so + // Datadog credentials are never sent to an arbitrary host (see above); the + // request is sent unauthenticated and the caller may add headers via -H. + if credentials_allowed { + req = crate::client::apply_auth(req, cfg, &method_upper, &auth_path)?; + } else if cfg.access_token.is_some() || cfg.api_key.is_some() { + eprintln!( + "warning: not sending Datadog credentials to non-Datadog host {:?}; \ + use -H to add headers explicitly", + reqwest::Url::parse(&url) + .ok() + .and_then(|u| u.host_str().map(str::to_string)) + .unwrap_or_else(|| url.clone()) + ); + } req = req .header("User-Agent", useragent::get()) @@ -651,4 +691,176 @@ mod tests { ); cleanup_env(); } + + /// `targets_configured_host` is the credential-exfiltration guard: only the + /// configured Datadog host (same host + effective port) is a match. + #[test] + fn test_targets_configured_host() { + let _guard = crate::test_utils::ENV_LOCK.blocking_lock(); + std::env::remove_var("PUP_MOCK_SERVER"); + let cfg = Config { + api_key: Some("k".into()), + app_key: Some("a".into()), + access_token: None, + site: "datadoghq.com".into(), + site_explicit: true, + org: None, + output_format: crate::config::OutputFormat::Json, + auto_approve: false, + agent_mode: false, + read_only: false, + }; + assert!(targets_configured_host( + "https://api.datadoghq.com/api/v2/monitors", + &cfg + )); + // Host comparison is ASCII-case-insensitive (url crate lowercases hosts). + assert!(targets_configured_host( + "https://API.DATADOGHQ.COM/api/v2/monitors", + &cfg + )); + // Different host: not a match. + assert!(!targets_configured_host( + "https://evil.example/api/v2/api_keys", + &cfg + )); + // userinfo `@` trick — real host is evil.example: not a match. + assert!(!targets_configured_host( + "https://api.datadoghq.com@evil.example/api/v2/api_keys", + &cfg + )); + // Same host, plain http (default port 80): not a match (no downgrade). + assert!(!targets_configured_host( + "http://api.datadoghq.com/api/v2/monitors", + &cfg + )); + // Same host, http but port 443: scheme still differs, so not a match — + // credentials must never travel cleartext. + assert!(!targets_configured_host( + "http://api.datadoghq.com:443/api/v2/monitors", + &cfg + )); + + // Custom site: the configured host changes accordingly. + let eu = Config { + site: "datadoghq.eu".into(), + ..cfg + }; + assert!(targets_configured_host( + "https://api.datadoghq.eu/api/v2/monitors", + &eu + )); + // Cross-region: US host is off-host for an EU config (region exfil guard). + assert!(!targets_configured_host( + "https://api.datadoghq.com/api/v2/monitors", + &eu + )); + } + + /// An absolute URL pointing at a non-Datadog host must receive NO Datadog + /// credentials, even on an OAuth-excluded path and even with creds configured. + #[tokio::test] + async fn test_api_offhost_absolute_url_omits_credentials() { + let _lock = lock_env().await; + // Configure for the real Datadog host, not the mock, so the mock URL is + // treated as a different (off-Datadog) host. + std::env::remove_var("PUP_MOCK_SERVER"); + let mut server = mockito::Server::new_async().await; + let cfg = Config { + api_key: Some("test-api-key".into()), + app_key: Some("test-app-key".into()), + access_token: Some("bearer-token".into()), + site: "datadoghq.com".into(), + site_explicit: true, + org: None, + output_format: crate::config::OutputFormat::Json, + auto_approve: false, + agent_mode: false, + read_only: false, + }; + let _mock = server + .mock("GET", "/api/v2/api_keys") + .match_query(mockito::Matcher::Any) + .match_header("authorization", mockito::Matcher::Missing) + .match_header("DD-API-KEY", mockito::Matcher::Missing) + .match_header("DD-APPLICATION-KEY", mockito::Matcher::Missing) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"{"data":[]}"#) + .create_async() + .await; + + // OAuth-excluded path on a non-Datadog host: must NOT leak the API keys. + let absolute = format!("{}/api/v2/api_keys", server.url()); + let result = super::run( + &cfg, + &absolute, + "GET", + &[], + &[], + &[], + None, + false, + true, + false, + ) + .await; + assert!( + result.is_ok(), + "off-host request should succeed unauthenticated: {:?}", + result.err() + ); + std::env::remove_var("PUP_MOCK_SERVER"); + } + + /// OAuth-only users (bearer token, no API keys) must not leak the bearer token + /// to an off-Datadog host either. + #[tokio::test] + async fn test_api_offhost_bearer_only_omits_token() { + let _lock = lock_env().await; + std::env::remove_var("PUP_MOCK_SERVER"); + let mut server = mockito::Server::new_async().await; + let cfg = Config { + api_key: None, + app_key: None, + access_token: Some("bearer-token".into()), + site: "datadoghq.com".into(), + site_explicit: true, + org: None, + output_format: crate::config::OutputFormat::Json, + auto_approve: false, + agent_mode: false, + read_only: false, + }; + let _mock = server + .mock("GET", "/api/v2/monitors") + .match_query(mockito::Matcher::Any) + .match_header("authorization", mockito::Matcher::Missing) + .with_status(200) + .with_header("content-type", "application/json") + .with_body(r#"[]"#) + .create_async() + .await; + + let absolute = format!("{}/api/v2/monitors", server.url()); + let result = super::run( + &cfg, + &absolute, + "GET", + &[], + &[], + &[], + None, + false, + true, + false, + ) + .await; + assert!( + result.is_ok(), + "off-host bearer-only request should omit the token: {:?}", + result.err() + ); + std::env::remove_var("PUP_MOCK_SERVER"); + } }