From af18fed99e1c246584eb003bf0de369028387c3c Mon Sep 17 00:00:00 2001 From: Callum Reid Date: Tue, 4 Aug 2026 14:59:24 -0700 Subject: [PATCH] fix: treat a null metadata_key as absent in reports validation contains_key returns true for an explicit JSON null, but the field deserializes into an Option as None, so the guard disagreed with what was actually sent. Use the same predicate as validate_custom_dimensions. --- src/commands/reports.rs | 5 ++++- tests/cli_tests.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/commands/reports.rs b/src/commands/reports.rs index 9aa0751..7feacce 100644 --- a/src/commands/reports.rs +++ b/src/commands/reports.rs @@ -191,7 +191,10 @@ pub async fn execute(cmd: ReportCommands, client: &CovalClient, ctx: &OutputCont fn validate_metadata_key(input: &serde_json::Map) -> Result<()> { let is_metadata = input.get("compare_by").and_then(serde_json::Value::as_str) == Some("metadata"); - let has_metadata_key = input.contains_key("metadata_key"); + // An explicit null deserializes to None, so it is absent, not supplied. + let has_metadata_key = input + .get("metadata_key") + .is_some_and(|value| !value.is_null()); if is_metadata && !has_metadata_key { anyhow::bail!("--metadata-key is required when --compare-by is metadata"); diff --git a/tests/cli_tests.rs b/tests/cli_tests.rs index 6330661..0dcb0b5 100644 --- a/tests/cli_tests.rs +++ b/tests/cli_tests.rs @@ -3835,6 +3835,46 @@ fn test_reports_create_metadata_key_rejected_without_metadata_compare_by() { )); } +#[test] +fn test_reports_create_metadata_rejects_null_metadata_key() { + coval() + .arg("--api-key") + .arg("test_key") + .arg("reports") + .arg("create") + .arg("--name") + .arg("Bad Report") + .arg("--run-ids") + .arg("run1") + .arg("--compare-by") + .arg("metadata") + .arg("--input-json") + .arg(r#"{"metadata_key": null}"#) + .assert() + .failure() + .stderr(predicate::str::contains("--metadata-key is required")); +} + +#[test] +fn test_reports_create_allows_null_metadata_key_without_metadata_compare_by() { + coval() + .arg("--api-key") + .arg("test_key") + .arg("reports") + .arg("create") + .arg("--name") + .arg("Report") + .arg("--run-ids") + .arg("run1") + .arg("--compare-by") + .arg("run") + .arg("--input-json") + .arg(r#"{"metadata_key": null}"#) + .assert() + .failure() + .stderr(predicate::str::contains("--metadata-key can only be set").not()); +} + #[tokio::test] async fn test_traces_search_sends_structured_filters_and_preserves_cursor() { let mock_server = MockServer::start().await;