forked from mirrors/kingfisher
copilot fixes
This commit is contained in:
parent
cceab35ec1
commit
b89c952043
2 changed files with 69 additions and 14 deletions
|
|
@ -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(),
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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"))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue