diff --git a/src/direct_validate.rs b/src/direct_validate.rs index 508edec..c6914e5 100644 --- a/src/direct_validate.rs +++ b/src/direct_validate.rs @@ -595,10 +595,21 @@ pub async fn run_direct_validation( } } - // Execute validation based on type + // Execute validation based on type. Errors from the HTTP / gRPC + // pathways (DNS failure, SSRF preflight, request build, request + // execution, timeout) used to short-circuit the whole `validate` + // command via `?`, which left stdout empty and made downstream + // tools (and integration tests) unable to distinguish "no rule" + // from "validation attempted, infrastructure failed". Match the + // pattern used by AWS / GCP / raw branches below: surface the + // failure as a non-valid result with a generic `message`. The + // underlying error is intentionally NOT included in stdout or in + // debug logs because the rendered URL / headers / body can + // contain `{{ TOKEN }}` substituted to the secret (and any + // `--var` / `--arg` values). let mut result = match validation { Validation::Http(http_validation) => { - execute_http_validation( + match execute_http_validation( http_validation, &globals, &client, @@ -607,17 +618,48 @@ pub async fn run_direct_validation( args.retries, global_args.allow_internal_ips, ) - .await? + .await + { + Ok(r) => r, + Err(_e) => { + // Intentionally drop the underlying error: it can + // embed the rendered URL with `{{ TOKEN }}` + // substituted (i.e. the secret) or `--var` / + // `--arg` values. Logging it (even at debug) would + // leak credentials into stderr when -v is on. + debug!("HTTP validation failed"); + DirectValidationResult { + rule_id: String::new(), + rule_name: String::new(), + is_valid: false, + status_code: None, + message: "HTTP validation failed".to_string(), + } + } + } } Validation::Grpc(grpc_validation_cfg) => { - execute_grpc_validation( + match execute_grpc_validation( grpc_validation_cfg, &globals, &parser, timeout, global_args.allow_internal_ips, ) - .await? + .await + { + Ok(r) => r, + Err(_e) => { + debug!("gRPC validation failed"); + DirectValidationResult { + rule_id: String::new(), + rule_name: String::new(), + is_valid: false, + status_code: None, + message: "gRPC validation failed".to_string(), + } + } + } } Validation::AWS => { diff --git a/tests/cli_validate_revoke.rs b/tests/cli_validate_revoke.rs index ed018d2..49fa107 100644 --- a/tests/cli_validate_revoke.rs +++ b/tests/cli_validate_revoke.rs @@ -181,6 +181,117 @@ mod validate { .stdout(contains("Rule:").and(contains("Result:"))); } + /// HTTP infrastructure failures (DNS, SSRF preflight, connection + /// refused, etc.) must surface as a structured DirectValidationResult + /// rather than short-circuiting the validate command. Also verifies + /// the secret is not leaked into stdout via the error message. + #[test] + fn validate_http_failure_emits_structured_result() { + let secret = "ghp_redaction_test_secret_value_xyz"; + let assert = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher")) + .args([ + "validate", + "--rule", + "kingfisher.github.2", + secret, + "--format", + "json", + "--allow-internal-ips", + "--endpoint", + "github=http://127.0.0.1:1", + "--timeout", + "2", + "--retries", + "0", + "--no-update-check", + ]) + .assert(); + let output = assert.get_output(); + assert!(output.status.code().is_some_and(|code| code == 0 || code == 1)); + let stdout = String::from_utf8(output.stdout.clone()).expect("stdout should be UTF-8"); + let decoded: Value = serde_json::from_str(&stdout).expect("json should decode"); + assert_eq!(decoded.get("rule_id").and_then(|v| v.as_str()), Some("kingfisher.github.2")); + assert_eq!(decoded.get("is_valid").and_then(|v| v.as_bool()), Some(false)); + let message = decoded.get("message").and_then(|v| v.as_str()).unwrap_or(""); + assert!( + message.contains("HTTP validation failed"), + "message should explain the failure, got: {message}" + ); + // The CLI must never echo the user's secret back to stdout, even + // when the upstream validation fails. We emit a generic error + // message and only log the underlying detail at debug level. + assert!(!stdout.contains(secret), "secret must not appear in stdout output, got: {stdout}"); + } + + /// gRPC infrastructure failures must surface as a structured + /// DirectValidationResult and must not leak the secret into stdout. + /// Mirrors `validate_http_failure_emits_structured_result` for the + /// gRPC code path. + #[test] + fn validate_grpc_failure_emits_structured_result() { + let tmp = TempDir::new().unwrap(); + // Custom rule with gRPC validation pointing at an unreachable port + // so we deterministically trigger a connection failure rather than + // depending on any built-in provider's reachability. + fs::write( + tmp.path().join("custom_grpc_rule.yml"), + r#" +rules: + - name: Custom gRPC Rule + id: test.custom.grpc + pattern: "grpc_[a-z0-9]{8}" + validation: + type: Grpc + content: + request: + url: http://127.0.0.1:1/example.Service/Method + headers: + content-type: application/grpc + x-token: "{{ TOKEN }}" + response_matcher: + - report_response: true + - type: HeaderMatch + header: grpc-status + expected: + - "0" +"#, + ) + .unwrap(); + + let secret = "grpc_redaction_test_secret_xyz"; + let assert = Command::new(assert_cmd::cargo::cargo_bin!("kingfisher")) + .args([ + "validate", + "--rule", + "test.custom.grpc", + secret, + "--rules-path", + tmp.path().to_str().unwrap(), + "--no-builtins", + "--format", + "json", + "--allow-internal-ips", + "--timeout", + "2", + "--retries", + "0", + "--no-update-check", + ]) + .assert(); + let output = assert.get_output(); + assert!(output.status.code().is_some_and(|code| code == 0 || code == 1)); + let stdout = String::from_utf8(output.stdout.clone()).expect("stdout should be UTF-8"); + let decoded: Value = serde_json::from_str(&stdout).expect("json should decode"); + assert_eq!(decoded.get("rule_id").and_then(|v| v.as_str()), Some("test.custom.grpc")); + assert_eq!(decoded.get("is_valid").and_then(|v| v.as_bool()), Some(false)); + let message = decoded.get("message").and_then(|v| v.as_str()).unwrap_or(""); + assert!( + message.contains("gRPC validation failed"), + "message should explain the failure, got: {message}" + ); + assert!(!stdout.contains(secret), "secret must not appear in stdout output, got: {stdout}"); + } + #[test] fn validate_with_timeout() { Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"))