eblume/kingfisher
1
0
Fork 0
forked from mirrors/kingfisher
Code Pull requests Activity Actions

Merge pull request #370 from mongodb/development

Browse source
This commit is contained in:
Mick Grove 2026-04-30 12:34:21 -07:00 • committed by GitHub
commit b2811107a8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 158 additions and 5 deletions
Download patch file Download diff file Expand all files Collapse all files

52
src/direct_validate.rs
View file

@ -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 => {

111
tests/cli_validate_revoke.rs
View file

@ -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"))