Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ path = "src/lib.rs"
[dependencies]
async-trait = "0.1.89"
base64 = "0.22.1"
bytes = "1.11"
keyring = { version = "3.6.1", optional = true, default-features = false }
open = { version = "5.3.2", optional = true }
rand = { version = "0.9", optional = true }
Expand Down
14 changes: 13 additions & 1 deletion docs/concepts.md
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ populate middleware:
| `--schema` | `schema` | `false` | Renders command schema instead of running business logic. |
| `--reason` | `reason` | empty | Reason passed to authorization. |
| `--timeout` | `timeout` | `0s` | Command deadline (e.g. `60s`, `5m`); default `0s` = no timeout. |
| `--debug` | `debug` | empty | Enables debug behavior for integrations that use it. |
| `--debug` | `debug` | empty | Enables debug components (comma-separated patterns: `*`, `transport`, `*,-auth`). `transport` dumps HTTP requests/responses to stderr. See [HTTP debug logging](#http-debug-logging). |
Comment thread
jpage-godaddy marked this conversation as resolved.
Outdated
| `--search` | `search` | empty | Searches command and guide documentation before command execution. |

Applications can add additional global flags through `CliConfig::register_flags` and copy parsed
Expand Down Expand Up @@ -589,6 +589,18 @@ The transport module provides a `reqwest`-based HTTP client with:
Auth injectors include bearer token, provider bearer, cookie, basic auth, API key, client
credentials, and no-op injectors.

### HTTP debug logging

The global `--debug` flag drives transport diagnostics through the `transport` component (`--debug`, `--debug transport`, or `--debug '*'`; `--debug '*,-transport'` keeps everything else but silences HTTP). `flags::debug_component_enabled` parses the comma-separated pattern.
Comment thread
jpage-godaddy marked this conversation as resolved.
Outdated

When `transport` is selected the engine publishes a process-wide `StderrTransportLogger` via `transport::set_default_transport_logger`. Every `HttpClient` built afterward inherits it as its default logger (mirroring `set_default_user_agent`), so command handlers get a curl-style request/response trace on stderr with **no per-command wiring**. A client that sets its own logger with `HttpClientBuilder::logger` still overrides the default. The logger is installed once, before the command handler runs, and shared by every client the handler builds, so all of a command's HTTP requests are logged.

Sensitive headers (`authorization`, `proxy-authorization`, `cookie`, `set-cookie`, `x-api-key`) are redacted by default. A CLI with its own secret-bearing headers — e.g. a custom API-key header an auth injector adds — registers them with `CliConfig::with_redacted_debug_headers`; matching is case-insensitive and additive (the built-in set is always redacted). Request and JSON/decode response bodies are printed in full; raw byte-download and streaming responses report only their size to avoid dumping large payloads.

For code that talks to `reqwest` directly and cannot use `HttpClient` (bare clients, or progenitor-generated clients that wrap their own `reqwest::Client`), `transport::debug_log_reqwest_request` and `transport::debug_log_reqwest_response` emit to the same global logger, so a single `--debug`-controlled trace can still cover those call sites.

Adopting `HttpClient` for a generated client is not always possible; a typed progenitor client should attach the helpers above through its own request/response hook instead. Other engine gaps that would let more bare-`reqwest` call sites migrate onto `HttpClient`: a per-request dynamic header hook (e.g. a generated `x-request-id`), an absolute-URL/no-auth request method (pre-signed uploads), an arbitrary-method escape hatch returning the raw response, and surfacing `x-request-id` from error responses into `transport::Error`.

## Contributor Model

The intended contributor workflow is:
Expand Down
83 changes: 83 additions & 0 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,12 @@ pub struct CliConfig {
/// the engine derives `name/version` from this config. See
/// [`CliConfig::user_agent_string`].
pub user_agent: Option<String>,
/// Extra HTTP header names to redact in `--debug transport` output, on top
/// of the built-in sensitive set (`authorization`, `proxy-authorization`,
/// `cookie`, `set-cookie`, `x-api-key`). Set CLI-specific secret-bearing
/// headers here — e.g. a custom API-key header an auth injector adds.
/// Populate via [`CliConfig::with_redacted_debug_headers`].
pub redacted_debug_headers: Vec<String>,
Comment thread
jpage-godaddy marked this conversation as resolved.
/// Optional authorization gatekeeper injected into middleware.
pub authz: Option<Arc<dyn Authorizer>>,
/// Optional audit recorder injected into middleware.
Expand Down Expand Up @@ -351,6 +357,28 @@ impl CliConfig {
self
}

/// Adds HTTP header names to redact in `--debug transport` output, on top of
/// the built-in sensitive set.
///
/// Use this for CLI-specific secret-bearing headers that are not standard
/// auth headers — for example a custom API-key header that an
/// [`AuthInjector`](crate::transport::AuthInjector) sets. Matching is
/// case-insensitive and additive: the built-in set is always redacted.
/// Calls accumulate. Names are trimmed and empty entries are dropped, so a
/// mistyped value with stray whitespace cannot silently disable redaction.
#[must_use]
pub fn with_redacted_debug_headers(
mut self,
names: impl IntoIterator<Item = impl Into<String>>,
) -> Self {
self.redacted_debug_headers
.extend(names.into_iter().filter_map(|name| {
let name = name.into().trim().to_owned();
(!name.is_empty()).then_some(name)
}));
self
}
Comment thread
jpage-godaddy marked this conversation as resolved.

/// Returns the outbound User-Agent string the CLI presents on HTTP requests.
///
/// Resolution order:
Expand Down Expand Up @@ -1403,6 +1431,7 @@ impl Cli {
};
let mut middleware = self.middleware.clone();
apply_global_flags(&mut middleware, &flags, command_timeout);
install_debug_transport_logger(&flags.debug, &self.config.redacted_debug_headers);
if let Err(err) = self.apply_config_flags(&matches, &mut middleware) {
return self.finish_run(render_cli_error(&middleware, &err, &self.config.app_id));
}
Expand Down Expand Up @@ -1490,6 +1519,7 @@ impl Cli {
}
};
apply_global_flags(&mut middleware, &flags, command_timeout);
install_debug_transport_logger(&flags.debug, &self.config.redacted_debug_headers);
if let Err(err) = self.apply_config_flags(&matches, &mut middleware) {
return self.finish_run(render_cli_error(&middleware, &err, &self.config.app_id));
}
Expand Down Expand Up @@ -2081,6 +2111,29 @@ fn apply_global_flags(middleware: &mut Middleware, flags: &GlobalFlags, timeout:
middleware.search = flags.search.clone();
}

/// Installs (or clears) the process-wide transport debug logger from the parsed
/// `--debug` pattern.
///
/// When `--debug` selects the `transport` component the engine publishes a
/// [`StderrTransportLogger`](crate::transport::StderrTransportLogger) — extended
/// with any [`CliConfig::with_redacted_debug_headers`] entries — which every
/// [`HttpClient`](crate::transport::HttpClient) built afterward picks up
/// automatically, with no per-command wiring. The logger is reset to a noop when
/// `transport` is not selected so the explicit setting always reflects the
/// current invocation rather than a stale process-global from an earlier one.
fn install_debug_transport_logger(debug: &str, extra_redacted: &[String]) {
let logger: Arc<dyn crate::transport::TransportLogger> =
if crate::debug_component_enabled(debug, "transport") {
Arc::new(
crate::transport::StderrTransportLogger::new()
.with_redacted_headers(extra_redacted.iter().cloned()),
)
} else {
Arc::new(crate::transport::NoopTransportLogger)
};
crate::transport::set_default_transport_logger(logger);
}

async fn run_with_timeout<F, T>(
timeout: Option<Duration>,
timeout_label: &str,
Expand Down Expand Up @@ -3002,6 +3055,36 @@ mod user_agent_tests {
"uatest/4.5.6"
);
}

#[test]
fn install_debug_transport_logger_tracks_the_debug_pattern() {
let _guard = crate::transport::client::TRANSPORT_LOGGER_TEST_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let _restore = crate::transport::client::RestoreDefaultTransportLogger;

// `transport` selected -> a real stderr logger is published.
install_debug_transport_logger("transport", &[]);
assert!(
format!("{:?}", crate::transport::default_transport_logger())
.starts_with("StderrTransportLogger")
);
Comment thread
Copilot marked this conversation as resolved.
Outdated

// Wildcard with transport excluded -> back to the noop.
install_debug_transport_logger("*,-transport", &[]);
assert_eq!(
format!("{:?}", crate::transport::default_transport_logger()),
"NoopTransportLogger"
);
Comment thread
jpage-godaddy marked this conversation as resolved.
Outdated

// Empty pattern -> noop.
install_debug_transport_logger("transport", &[]);
install_debug_transport_logger("", &[]);
assert_eq!(
format!("{:?}", crate::transport::default_transport_logger()),
"NoopTransportLogger"
);
Comment thread
Copilot marked this conversation as resolved.
Outdated
}
}

#[cfg(test)]
Expand Down
65 changes: 64 additions & 1 deletion src/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,49 @@ fn collect_flag_names(command: &Command, visit: &mut impl FnMut(&Arg, String)) {
}
}

/// Reports whether a `--debug` pattern enables a named component.
///
/// The pattern is a comma-separated list of tokens applied left to right, so
/// later tokens override earlier ones:
///
/// - `*` enables every component; `-*` disables every component.
/// - `name` enables that component; `-name` disables it.
/// - whitespace around tokens is ignored and matching is case-insensitive.
///
/// An empty pattern enables nothing. Tokens that name other components are
/// ignored for the queried `component`.
///
/// # Examples
///
/// ```
/// use cli_engine::debug_component_enabled;
///
/// assert!(debug_component_enabled("*", "transport"));
/// assert!(debug_component_enabled("transport", "transport"));
/// assert!(!debug_component_enabled("*,-transport", "transport"));
/// assert!(debug_component_enabled("*,-auth", "transport"));
/// assert!(!debug_component_enabled("", "transport"));
/// ```
#[must_use]
pub fn debug_component_enabled(pattern: &str, component: &str) -> bool {
let component = component.trim().to_ascii_lowercase();
let mut enabled = false;
for raw in pattern.split(',') {
Comment thread
jpage-godaddy marked this conversation as resolved.
let token = raw.trim();
if token.is_empty() {
continue;
}
let (negated, name) = token
.strip_prefix('-')
.map_or((false, token), |rest| (true, rest));
let name = name.trim().to_ascii_lowercase();
if name == "*" || name == component {
enabled = !negated;
}
}
enabled
}

fn arg_requires_value(arg: &Arg) -> bool {
match arg.get_action() {
ArgAction::Set | ArgAction::Append => arg
Expand All @@ -502,7 +545,27 @@ fn arg_requires_value(arg: &Arg) -> bool {

#[cfg(test)]
mod tests {
use super::{output_env_var, resolve_default_output_format};
use super::{debug_component_enabled, output_env_var, resolve_default_output_format};

#[test]
fn debug_component_matcher_handles_wildcards_and_negation() {
// Empty pattern enables nothing.
assert!(!debug_component_enabled("", "transport"));
// Wildcard enables everything.
assert!(debug_component_enabled("*", "transport"));
assert!(debug_component_enabled("*", "auth"));
// Bare name enables only that component.
assert!(debug_component_enabled("transport", "transport"));
assert!(!debug_component_enabled("transport", "auth"));
// Negation after a wildcard removes one component but keeps the rest.
assert!(!debug_component_enabled("*,-transport", "transport"));
assert!(debug_component_enabled("*,-auth", "transport"));
// `-*` disables everything; later tokens still win.
assert!(!debug_component_enabled("*,-*", "transport"));
assert!(debug_component_enabled("-*,transport", "transport"));
// Whitespace and case are ignored.
assert!(debug_component_enabled(" Transport , -auth ", "transport"));
}

#[test]
fn default_output_format_follows_env_override_then_tty() {
Expand Down
7 changes: 4 additions & 3 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,10 @@ pub use error::{
CliCoreError, DetailedError, ExitCoder, Result, exit_code_for_error, exit_code_for_exit_coder,
};
pub use flags::{
GlobalFlags, app_id_env_prefix, default_output_format, derive_bool_flags, derive_value_flags,
extract_command_path, extract_output_format, extract_search_query, global_flags_from_matches,
has_true_schema_flag, output_env_var, register_global_flags, resolve_default_output_format,
GlobalFlags, app_id_env_prefix, debug_component_enabled, default_output_format,
derive_bool_flags, derive_value_flags, extract_command_path, extract_output_format,
extract_search_query, global_flags_from_matches, has_true_schema_flag, output_env_var,
register_global_flags, resolve_default_output_format,
};
pub use guide::{GuideEntry, parse_guides, parse_guides_from_markdown};
pub use middleware::{
Expand Down
Loading