From b89c95204342b59870a9bcd226d612f4bf1b2c0e Mon Sep 17 00:00:00 2001
From: Mick Grove <mick.grove@mongodb.com>
Date: Thu, 30 Apr 2026 11:28:45 -0700
Subject: [PATCH] copilot fixes

---
 src/direct_validate.rs       | 41 +++++++++++++++++++++++------------
 tests/cli_validate_revoke.rs | 42 ++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/src/direct_validate.rs b/src/direct_validate.rs
index c3a177a..ba53070 100644
--- a/src/direct_validate.rs
+++ b/src/direct_validate.rs
@@ -617,13 +617,22 @@ pub async fn run_direct_validation(
                 .await
                 {
                     Ok(r) => r,
-                    Err(e) => DirectValidationResult {
-                        rule_id: String::new(),
-                        rule_name: String::new(),
-                        is_valid: false,
-                        status_code: None,
-                        message: format!("HTTP validation error: {}", e),
-                    },
+                    Err(e) => {
+                        // Don't surface the underlying error: it can embed
+                        // the rendered URL with `{{ TOKEN }}` substituted
+                        // (i.e. the secret) or `--var` / `--arg` values.
+                        // Log the detail for debugging and emit a generic
+                        // message to stdout.
+                        debug!("HTTP validation failed: {e}");
+                        DirectValidationResult {
+                            rule_id: String::new(),
+                            rule_name: String::new(),
+                            is_valid: false,
+                            status_code: None,
+                            message: "HTTP validation failed (see debug logs for details)"
+                                .to_string(),
+                        }
+                    }
                 }
             }
             Validation::Grpc(grpc_validation_cfg) => {
@@ -637,13 +646,17 @@ pub async fn run_direct_validation(
                 .await
                 {
                     Ok(r) => r,
-                    Err(e) => DirectValidationResult {
-                        rule_id: String::new(),
-                        rule_name: String::new(),
-                        is_valid: false,
-                        status_code: None,
-                        message: format!("gRPC validation error: {}", e),
-                    },
+                    Err(e) => {
+                        debug!("gRPC validation failed: {e}");
+                        DirectValidationResult {
+                            rule_id: String::new(),
+                            rule_name: String::new(),
+                            is_valid: false,
+                            status_code: None,
+                            message: "gRPC validation failed (see debug logs for details)"
+                                .to_string(),
+                        }
+                    }
                 }
             }
 
diff --git a/tests/cli_validate_revoke.rs b/tests/cli_validate_revoke.rs
index ed018d2..f9de895 100644
--- a/tests/cli_validate_revoke.rs
+++ b/tests/cli_validate_revoke.rs
@@ -181,6 +181,48 @@ 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}");
+    }
+
     #[test]
     fn validate_with_timeout() {
         Command::new(assert_cmd::cargo::cargo_bin!("kingfisher"))