From 89c2c810fa4fb417ef96a969254a9e3523fbfd18 Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Wed, 11 Feb 2026 17:00:20 -0500 Subject: [PATCH 01/18] fix: support opt-out of variable validation --- apollo-router/src/configuration/mod.rs | 15 ++ apollo-router/src/configuration/mode.rs | 9 + .../src/services/supergraph/service.rs | 49 ++-- apollo-router/src/spec/field_type.rs | 53 +++- apollo-router/src/spec/query.rs | 4 +- apollo-router/src/spec/query/tests.rs | 228 +++++++++++++++++- .../supergraph_input_variables.graphql | 47 ++++ ...idation_mode_propagates_fully@enforce.snap | 15 ++ ...idation_mode_propagates_fully@missing.snap | 15 ++ ...validation_mode_propagates_fully@warn.snap | 13 + apollo-router/tests/integration/validation.rs | 77 ++++++ 11 files changed, 494 insertions(+), 31 deletions(-) create mode 100644 apollo-router/tests/fixtures/supergraph_input_variables.graphql create mode 100644 apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@enforce.snap create mode 100644 apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@missing.snap create mode 100644 apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@warn.snap diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index ab126e4d9b..b290dc8ec9 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -49,6 +49,7 @@ use self::subgraph::SubgraphConfiguration; use crate::ApolloRouterError; use crate::cache::DEFAULT_CACHE_CAPACITY; use crate::configuration::cooperative_cancellation::CooperativeCancellation; +use crate::configuration::mode::WarnOrEnforceMode; use crate::graphql; use crate::plugin::plugins; use crate::plugins::chaos; @@ -743,6 +744,10 @@ pub(crate) struct Supergraph { /// Log a message if the client closes the connection before the response is sent. /// Default: false. pub(crate) experimental_log_on_broken_pipe: bool, + + /// TODO(@caroline) docs + #[serde(default = "default_strict_variable_validation")] + pub(crate) strict_variable_validation: WarnOrEnforceMode, } const fn default_generate_query_fragments() -> bool { @@ -767,6 +772,7 @@ impl Supergraph { early_cancel: Option, experimental_log_on_broken_pipe: Option, insert_result_coercion_errors: Option, + strict_variable_validation: Option, ) -> Self { Self { listen: listen.unwrap_or_else(default_graphql_listen), @@ -781,6 +787,8 @@ impl Supergraph { early_cancel: early_cancel.unwrap_or_default(), experimental_log_on_broken_pipe: experimental_log_on_broken_pipe.unwrap_or_default(), enable_result_coercion_errors: insert_result_coercion_errors.unwrap_or_default(), + strict_variable_validation: strict_variable_validation + .unwrap_or_else(default_strict_variable_validation), } } } @@ -800,6 +808,7 @@ impl Supergraph { early_cancel: Option, experimental_log_on_broken_pipe: Option, insert_result_coercion_errors: Option, + strict_variable_validation: Option, ) -> Self { Self { listen: listen.unwrap_or_else(test_listen), @@ -814,6 +823,8 @@ impl Supergraph { early_cancel: early_cancel.unwrap_or_default(), experimental_log_on_broken_pipe: experimental_log_on_broken_pipe.unwrap_or_default(), enable_result_coercion_errors: insert_result_coercion_errors.unwrap_or_default(), + strict_variable_validation: strict_variable_validation + .unwrap_or_else(default_strict_variable_validation), } } } @@ -1506,6 +1517,10 @@ fn default_connection_shutdown_timeout() -> Duration { Duration::from_secs(60) } +fn default_strict_variable_validation() -> WarnOrEnforceMode { + WarnOrEnforceMode::Enforce +} + #[derive(Clone, Debug, Default, Error, Display, Serialize, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] pub(crate) enum BatchingMode { diff --git a/apollo-router/src/configuration/mode.rs b/apollo-router/src/configuration/mode.rs index 83285a6fe7..e4244ab3de 100644 --- a/apollo-router/src/configuration/mode.rs +++ b/apollo-router/src/configuration/mode.rs @@ -10,3 +10,12 @@ pub(crate) enum Mode { Measure, Enforce, } + +// Don't add a default here. Instead, Default should be implemented for +// individual cases of WarnOrEnforceMode. +#[derive(Debug, Clone, Copy, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub(crate) enum WarnOrEnforceMode { + Warn, + Enforce, +} diff --git a/apollo-router/src/services/supergraph/service.rs b/apollo-router/src/services/supergraph/service.rs index d316bf98b5..fc9d149088 100644 --- a/apollo-router/src/services/supergraph/service.rs +++ b/apollo-router/src/services/supergraph/service.rs @@ -25,6 +25,7 @@ use crate::Context; use crate::batching::BatchQuery; use crate::configuration::Batching; use crate::configuration::PersistedQueriesPrewarmQueryPlanCache; +use crate::configuration::mode::WarnOrEnforceMode; use crate::error::CacheResolverError; use crate::graphql; use crate::graphql::IntoGraphQLErrors; @@ -79,6 +80,7 @@ pub(crate) struct SupergraphService { query_planner_service: CachingQueryPlanner, execution_service: execution::BoxCloneService, schema: Arc, + strict_variable_validation: WarnOrEnforceMode, } #[buildstructor::buildstructor] @@ -88,11 +90,13 @@ impl SupergraphService { query_planner_service: CachingQueryPlanner, execution_service: execution::BoxCloneService, schema: Arc, + strict_variable_validation: WarnOrEnforceMode, ) -> Self { SupergraphService { query_planner_service, execution_service, schema, + strict_variable_validation, } } } @@ -122,22 +126,27 @@ impl Service for SupergraphService { let schema = self.schema.clone(); let context_cloned = req.context.clone(); - let fut = service_call(planning, self.execution_service.clone(), schema, req).or_else( - |error: BoxError| async move { - let errors = vec![ - crate::error::Error::builder() - .message(error.to_string()) - .extension_code("INTERNAL_SERVER_ERROR") - .build(), - ]; - - Ok(SupergraphResponse::infallible_builder() - .errors(errors) - .status_code(StatusCode::INTERNAL_SERVER_ERROR) - .context(context_cloned) - .build()) - }, - ); + let fut = service_call( + planning, + self.execution_service.clone(), + schema, + req, + self.strict_variable_validation, + ) + .or_else(|error: BoxError| async move { + let errors = vec![ + crate::error::Error::builder() + .message(error.to_string()) + .extension_code("INTERNAL_SERVER_ERROR") + .build(), + ]; + + Ok(SupergraphResponse::infallible_builder() + .errors(errors) + .status_code(StatusCode::INTERNAL_SERVER_ERROR) + .context(context_cloned) + .build()) + }); Box::pin(fut) } @@ -148,6 +157,7 @@ async fn service_call( execution_service: execution::BoxCloneService, schema: Arc, req: SupergraphRequest, + strict_variable_validation: WarnOrEnforceMode, // todo ) -> Result { let context = req.context; let body = req.supergraph_request.body(); @@ -306,7 +316,11 @@ async fn service_call( ); *response.response.status_mut() = StatusCode::NOT_ACCEPTABLE; Ok(response) - } else if let Some(err) = plan.query.validate_variables(body, &schema).err() { + } else if let Some(err) = plan + .query + .validate_variables(body, &schema, strict_variable_validation) + .err() + { let mut res = SupergraphResponse::new_from_graphql_response(err, context); *res.response.status_mut() = StatusCode::BAD_REQUEST; Ok(res) @@ -583,6 +597,7 @@ impl PluggableSupergraphServiceBuilder { .query_planner_service(query_planner_service.clone()) .execution_service(execution_service) .schema(schema.clone()) + .strict_variable_validation(configuration.supergraph.strict_variable_validation) .build(); let supergraph_service = diff --git a/apollo-router/src/spec/field_type.rs b/apollo-router/src/spec/field_type.rs index 792b686c54..9fc026e499 100644 --- a/apollo-router/src/spec/field_type.rs +++ b/apollo-router/src/spec/field_type.rs @@ -5,6 +5,7 @@ use serde::Serialize; use serde::de::Error as _; use super::query::parse_hir_value; +use crate::configuration::mode::WarnOrEnforceMode; use crate::json_ext::Value; use crate::json_ext::ValueExt; use crate::spec::Schema; @@ -123,6 +124,7 @@ fn validate_input_value( value: Option<&Value>, schema: &Schema, path: &JsonValuePath<'_>, + strict_variable_validation: WarnOrEnforceMode, // todo ) -> Result<(), InvalidInputValue> { let fmt_path = |var_path: &JsonValuePath<'_>| match var_path { JsonValuePath::Variable { .. } => format!("variable `{var_path}`"), @@ -161,12 +163,24 @@ fn validate_input_value( index: i, parent: path, }; - validate_input_value(inner_type, Some(x), schema, &path)? + validate_input_value( + inner_type, + Some(x), + schema, + &path, + strict_variable_validation, + )? } return Ok(()); } else { // For coercion from single value to list - return validate_input_value(inner_type, Some(value), schema, path); + return validate_input_value( + inner_type, + Some(value), + schema, + path, + strict_variable_validation, + ); } } }; @@ -217,7 +231,7 @@ fn validate_input_value( )) }; - let unknown = obj.keys().find_map(|k| { + let mut unknown_input_fields = obj.keys().filter_map(|k| { let k = k.as_str(); if !def.fields.contains_key(k) { Some(k) @@ -226,8 +240,18 @@ fn validate_input_value( } }); - if let Some(unknown) = unknown { - return Err(unknown_field(unknown)); + match strict_variable_validation { + WarnOrEnforceMode::Enforce => { + if let Some(field) = unknown_input_fields.next() { + return Err(unknown_field(field)); + } + } + WarnOrEnforceMode::Warn => { + let unknown_fields: Vec<&str> = unknown_input_fields.collect(); + if !unknown_fields.is_empty() { + tracing::warn!(variables = ?unknown_fields, "encountered unexpected variable(s)"); // consider just doing first? based on comment at top of fn + } + } } // Validate all fields present on def @@ -242,9 +266,21 @@ fn validate_input_value( .default_value .as_ref() .and_then(|v| parse_hir_value(v)); - validate_input_value(&field.ty, default.as_ref(), schema, &path) + validate_input_value( + &field.ty, + default.as_ref(), + schema, + &path, + strict_variable_validation, + ) } - value => validate_input_value(&field.ty, value, schema, &path), + value => validate_input_value( + &field.ty, + value, + schema, + &path, + strict_variable_validation, + ), } }) } @@ -264,8 +300,9 @@ impl FieldType { value: Option<&Value>, schema: &Schema, path: &JsonValuePath<'_>, + strict_variable_validation: WarnOrEnforceMode, ) -> Result<(), InvalidInputValue> { - validate_input_value(&self.0, value, schema, path) + validate_input_value(&self.0, value, schema, path, strict_variable_validation) } pub(crate) fn is_non_null(&self) -> bool { diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index a1c8f16bb7..deb2f4bdb0 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -24,6 +24,7 @@ use self::subselections::SubSelectionValue; use super::Fragment; use super::QueryHash; use crate::Configuration; +use crate::configuration::mode::WarnOrEnforceMode; use crate::error::FetchError; use crate::graphql::Error; use crate::graphql::Request; @@ -1035,6 +1036,7 @@ impl Query { &self, request: &Request, schema: &Schema, + strict_variable_validation: WarnOrEnforceMode, // todo ) -> Result<(), Response> { if LevelFilter::current() >= LevelFilter::DEBUG { let known_variables = self @@ -1078,7 +1080,7 @@ impl Query { let path = super::JsonValuePath::Variable { name: name.as_str(), }; - ty.validate_input_value(value, schema, &path) + ty.validate_input_value(value, schema, &path, strict_variable_validation) .err() .map(|message| { FetchError::ValidationInvalidTypeVariable { diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index 9ec2c6d2c1..474476b69a 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -2515,7 +2515,7 @@ fn reformat_response_unknown_typename() { .test(); } -macro_rules! run_validation { +macro_rules! run_validation_enforce_mode { ($schema:expr, $query:expr, $variables:expr $(,)?) => {{ let variables = match $variables { Value::Object(object) => object, @@ -2536,13 +2536,38 @@ macro_rules! run_validation { &Default::default(), ) .expect("could not parse query"); - query.validate_variables(&request, &schema) + query.validate_variables(&request, &schema, WarnOrEnforceMode::Enforce) + }}; +} + +macro_rules! run_validation_warn_mode { + ($schema:expr, $query:expr, $variables:expr $(,)?) => {{ + let variables = match $variables { + Value::Object(object) => object, + _ => unreachable!("variables must be an object"), + }; + let schema = Schema::parse(&$schema, &Default::default()).expect("could not parse schema"); + let request = Request::builder() + .variables(variables) + .query($query.to_string()) + .build(); + let query = Query::parse( + request + .query + .as_ref() + .expect("query has been added right above; qed"), + None, + &schema, + &Default::default(), + ) + .expect("could not parse query"); + query.validate_variables(&request, &schema, WarnOrEnforceMode::Warn) }}; } macro_rules! assert_validation { ($schema:expr, $query:expr, $variables:expr $(,)?) => {{ - let res = run_validation!( + let res = run_validation_enforce_mode!( with_supergraph_boilerplate($schema, "Query"), $query, $variables @@ -2553,7 +2578,7 @@ macro_rules! assert_validation { macro_rules! assert_validation_error { ($schema:expr, $query:expr, $variables:expr $(,)?) => {{ - let res = run_validation!( + let res = run_validation_enforce_mode!( with_supergraph_boilerplate($schema, "Query"), $query, $variables @@ -3009,7 +3034,200 @@ fn variable_validation() { } "#; - let res = run_validation!( + let res = run_validation_enforce_mode!( + schema, + "mutation foo($input: FooInput!) { + foo (input: $input) { + __typename + }}", + json!({"input":{}}) + ); + assert!(res.is_ok(), "validation should have succeeded: {res:?}"); +} + +#[test] +fn variable_validation_v2() { + let res = run_validation_warn_mode!( + with_supergraph_boilerplate( + "input MessageInput { + content: String + author: String + } + type Receipt { + id: ID! + } + type Query{ + send(message: MessageInput): Receipt}", + "Query" + ), + "query($msg: MessageInput) { + send(message: $msg) { + id + }}", + json!({"msg": { + "content": "Hello", + "author": "Me", + "unknownField": "unknown", + }}) + ); + assert!( + res.is_ok(), + "validation should have warned rather than failed" + ); + + // Tests if nested inputs are correctly validated + let res = run_validation_warn_mode!( + with_supergraph_boilerplate( + "input MessageInput { + content: String + author: String + canvas: [CanvasInput] + } + input CanvasInput { + input: Int + } + type Receipt { + id: ID! + } + type Query{ + send(message: MessageInput): Receipt}", + "Query" + ), + "query($msg: MessageInput) { + send(message: $msg) { + id + }}", + json!({"msg": { + "content": "Hello", + "author": "Me", + "canvas": [ + {"input": 3}, + {"input": 4}, + {"input": 5, "unknownField": "unknown"} + ], + }}) + ); + assert!( + res.is_ok(), + "validation should have warned rather than failed" + ); + + // Tests if nested inputs are correctly validated + let res = run_validation_warn_mode!( + with_supergraph_boilerplate( + " + input MessageInput { + content: String + author: String + canvas: [CanvasInput] + } + input CanvasInput { + input: Int! + } + type Receipt { + id: ID! + } + type Query { + send(message: MessageInput): Receipt + } + ", + "Query" + ), + "query($msg: MessageInput) { + send(message: $msg) { + id + } + }", + json!({"msg": { + "content": "Hello", + "author": "Me", + "canvas": [{"innput": 4}], + }}) + ); + assert!(res.is_err(), "validation should have failed"); + + // Tests if nested inputs are correctly validated + let res = run_validation_warn_mode!( + with_supergraph_boilerplate( + " + input MessageInput { + content: String + author: String + canvas: [CanvasInput] + } + input CanvasInput { + input: Int! + } + type Receipt { + id: ID! + } + type Query { + send(message: MessageInput): Receipt + } + ", + "Query" + ), + "query($msg: MessageInput) { + send(message: $msg) { + id + } + }", + json!({"msg": { + "content": "Hello", + "author": "Me", + "canvas": [{"input": 3, "innput": 4}], + }}) + ); + assert!( + res.is_ok(), + "validation should have warned rather than failed" + ); + + let schema = r#" + schema + @link(url: "https://specs.apollo.dev/link/v1.0") + @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) + { + query: Query + mutation: Mutation + } + directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA + directive @join__graph(name: String!, url: String!) on ENUM_VALUE + directive @join__type( graph: join__Graph! key: join__FieldSet extension: Boolean! = false resolvable: Boolean! = true isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR + + scalar join__FieldSet + scalar link__Import + + enum link__Purpose { + SECURITY + EXECUTION + } + + enum join__Graph { + TEST @join__graph(name: "test", url: "http://localhost:4001/graphql") + } + + type Mutation{ + foo(input: FooInput!): FooResponse! + } + type Query @join__type(graph: TEST){ + data: String + } + + input FooInput { + enumWithDefault: EnumWithDefault! = WEB + } + type FooResponse { + id: ID! + } + + enum EnumWithDefault { + WEB + MOBILE + } + "#; + + let res = run_validation_warn_mode!( schema, "mutation foo($input: FooInput!) { foo (input: $input) { diff --git a/apollo-router/tests/fixtures/supergraph_input_variables.graphql b/apollo-router/tests/fixtures/supergraph_input_variables.graphql new file mode 100644 index 0000000000..0f588af94f --- /dev/null +++ b/apollo-router/tests/fixtures/supergraph_input_variables.graphql @@ -0,0 +1,47 @@ +schema +@link(url: "https://specs.apollo.dev/link/v1.0") +@link(url: "https://specs.apollo.dev/inaccessible/v0.2", for: SECURITY) +@link(url: "https://specs.apollo.dev/join/v0.2", for: EXECUTION) +{ + query: Query +} + +directive @join__field(graph: join__Graph!, requires: join__FieldSet, provides: join__FieldSet, type: String, external: Boolean, override: String, usedOverridden: Boolean) repeatable on FIELD_DEFINITION | INPUT_FIELD_DEFINITION +directive @join__graph(name: String!, url: String!) on ENUM_VALUE +directive @join__type(graph: join__Graph!, key: join__FieldSet, extension: Boolean! = false, resolvable: Boolean! = true) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR +directive @join__implements( + graph: join__Graph! + interface: String! +) repeatable on OBJECT | INTERFACE + +directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA +directive @inaccessible on FIELD_DEFINITION | OBJECT | INTERFACE | UNION | ARGUMENT_DEFINITION | SCALAR | ENUM | ENUM_VALUE | INPUT_OBJECT | INPUT_FIELD_DEFINITION + +scalar join__FieldSet +scalar link__Import +enum link__Purpose { + SECURITY + EXECUTION +} + +enum join__Graph { + TEST @join__graph(name: "test", url: "http://localhost:4001/graphql") +} + + +input MessageInput @join__type(graph: TEST) { + content: String + author: String + canvas: [CanvasInput] +} +input CanvasInput @join__type(graph: TEST) { + input: Int! +} +type Receipt @join__type(graph: TEST) { + id: ID! +} +type Query @join__type(graph: TEST) { + send(message: MessageInput): Receipt +} + + diff --git a/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@enforce.snap b/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@enforce.snap new file mode 100644 index 0000000000..ce6e07d195 --- /dev/null +++ b/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@enforce.snap @@ -0,0 +1,15 @@ +--- +source: apollo-router/tests/integration/validation.rs +expression: response_body +--- +{ + "errors": [ + { + "extensions": { + "code": "VALIDATION_INVALID_TYPE_VARIABLE", + "name": "msg" + }, + "message": "unknown field input value at `$msg.canvas[0].innput` found for GraphQL type `input CanvasInput @join__type(graph: TEST) {\n input: Int!\n}\n`" + } + ] +} diff --git a/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@missing.snap b/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@missing.snap new file mode 100644 index 0000000000..ce6e07d195 --- /dev/null +++ b/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@missing.snap @@ -0,0 +1,15 @@ +--- +source: apollo-router/tests/integration/validation.rs +expression: response_body +--- +{ + "errors": [ + { + "extensions": { + "code": "VALIDATION_INVALID_TYPE_VARIABLE", + "name": "msg" + }, + "message": "unknown field input value at `$msg.canvas[0].innput` found for GraphQL type `input CanvasInput @join__type(graph: TEST) {\n input: Int!\n}\n`" + } + ] +} diff --git a/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@warn.snap b/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@warn.snap new file mode 100644 index 0000000000..d4790044f4 --- /dev/null +++ b/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@warn.snap @@ -0,0 +1,13 @@ +--- +source: apollo-router/tests/integration/validation.rs +expression: response_body +--- +{ + "data": null, + "errors": [ + { + "message": "Subgraph errors redacted", + "path": [] + } + ] +} diff --git a/apollo-router/tests/integration/validation.rs b/apollo-router/tests/integration/validation.rs index fa5ed5cdcf..91901a879a 100644 --- a/apollo-router/tests/integration/validation.rs +++ b/apollo-router/tests/integration/validation.rs @@ -1,6 +1,12 @@ +use std::path::PathBuf; + use apollo_router::_private::create_test_service_factory_from_yaml; +use serde_json::json; use tower::ServiceExt; +use crate::integration::IntegrationTest; +use crate::integration::common::Query; + #[tokio::test] async fn test_supergraph_validation_errors_are_passed_on() { create_test_service_factory_from_yaml( @@ -205,3 +211,74 @@ async fn test_lots_of_validation_errors() { ); assert!(errors.len() <= 100, "should return limited error count"); } + +#[rstest::rstest] +#[case(Some("enforce"), true, false)] +#[case(Some("warn"), false, true)] +#[case(None, true, false)] +#[tokio::test(flavor = "multi_thread")] +async fn variable_validation_mode_propagates_fully( + #[case] strict_variable_validation: Option<&str>, + #[case] response_should_be_error: bool, + #[case] logs_should_contain_warning: bool, +) { + let mut settings = insta::Settings::clone_current(); + settings.set_snapshot_suffix(format!( + "{}", + strict_variable_validation.unwrap_or("missing") + )); + settings.set_sort_maps(true); + let _guard = settings.bind_to_scope(); + + let mut config = json!({"supergraph": {}}); + if let Some(strict_variable_validation) = strict_variable_validation { + config["supergraph"] = json!({ "strict_variable_validation": strict_variable_validation }); + } + + let mut router = IntegrationTest::builder() + .config(&serde_yaml::to_string(&config).unwrap()) + .supergraph(PathBuf::from( + "tests/fixtures/supergraph_input_variables.graphql", + )) + .build() + .await; + + router.start().await; + router.assert_started().await; + + // Execute a query to trigger all the callbacks + let (_trace_id, response) = router + .execute_query( + Query::builder() + .body(json!({ + "query": "query($msg: MessageInput) { send(message: $msg) { id } }", + "variables": { + "msg": { + "content": "Hello", + "author": "Me", + "canvas": [{"input": 4, "innput": 4}], + } + } + })) + .build(), + ) + .await; + + assert_eq!( + response.status().is_client_error(), + response_should_be_error + ); + let response_body: serde_json::Value = + serde_json::from_slice(response.text().await.unwrap().as_bytes()).unwrap(); + insta::assert_json_snapshot!(response_body); + + router.read_logs(); + const VALIDATION_MESSAGE: &str = "encountered unexpected variable(s)"; + if logs_should_contain_warning { + router.assert_log_contained(VALIDATION_MESSAGE); + } else { + router.assert_log_not_contained(VALIDATION_MESSAGE); + } + + router.graceful_shutdown().await; +} From db883ccb8d568ccddb99333d774dd0057b9d5e3f Mon Sep 17 00:00:00 2001 From: Chidimma O Date: Thu, 12 Feb 2026 09:18:02 -0800 Subject: [PATCH 02/18] convert run_validation_warn_mode from macro to function --- apollo-router/src/spec/query/tests.rs | 81 ++++++++++++++------------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index 474476b69a..1d812a1fef 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -2540,29 +2540,31 @@ macro_rules! run_validation_enforce_mode { }}; } -macro_rules! run_validation_warn_mode { - ($schema:expr, $query:expr, $variables:expr $(,)?) => {{ - let variables = match $variables { - Value::Object(object) => object, - _ => unreachable!("variables must be an object"), - }; - let schema = Schema::parse(&$schema, &Default::default()).expect("could not parse schema"); - let request = Request::builder() - .variables(variables) - .query($query.to_string()) - .build(); - let query = Query::parse( - request - .query - .as_ref() - .expect("query has been added right above; qed"), - None, - &schema, - &Default::default(), - ) - .expect("could not parse query"); - query.validate_variables(&request, &schema, WarnOrEnforceMode::Warn) - }}; +fn run_validation_warn_mode( + schema: String, + query: &str, + variables: serde_json_bytes::Value, +) -> Result<(), Response> { + let variables = match variables { + Value::Object(object) => object, + _ => unreachable!("variables must be an object"), + }; + let schema = Schema::parse(&schema, &Default::default()).expect("could not parse schema"); + let request = Request::builder() + .variables(variables) + .query(query.to_string()) + .build(); + let query = Query::parse( + request + .query + .as_ref() + .expect("query has been added right above; qed"), + None, + &schema, + &Default::default(), + ) + .expect("could not parse query"); + query.validate_variables(&request, &schema, WarnOrEnforceMode::Warn) } macro_rules! assert_validation { @@ -3046,8 +3048,9 @@ fn variable_validation() { } #[test] -fn variable_validation_v2() { - let res = run_validation_warn_mode!( +fn variable_validation_warn_mode() { + // Tests validation of variable fields + let res = run_validation_warn_mode( with_supergraph_boilerplate( "input MessageInput { content: String @@ -3058,7 +3061,7 @@ fn variable_validation_v2() { } type Query{ send(message: MessageInput): Receipt}", - "Query" + "Query", ), "query($msg: MessageInput) { send(message: $msg) { @@ -3068,7 +3071,7 @@ fn variable_validation_v2() { "content": "Hello", "author": "Me", "unknownField": "unknown", - }}) + }}), ); assert!( res.is_ok(), @@ -3076,7 +3079,7 @@ fn variable_validation_v2() { ); // Tests if nested inputs are correctly validated - let res = run_validation_warn_mode!( + let res = run_validation_warn_mode( with_supergraph_boilerplate( "input MessageInput { content: String @@ -3091,7 +3094,7 @@ fn variable_validation_v2() { } type Query{ send(message: MessageInput): Receipt}", - "Query" + "Query", ), "query($msg: MessageInput) { send(message: $msg) { @@ -3105,7 +3108,7 @@ fn variable_validation_v2() { {"input": 4}, {"input": 5, "unknownField": "unknown"} ], - }}) + }}), ); assert!( res.is_ok(), @@ -3113,7 +3116,7 @@ fn variable_validation_v2() { ); // Tests if nested inputs are correctly validated - let res = run_validation_warn_mode!( + let res = run_validation_warn_mode( with_supergraph_boilerplate( " input MessageInput { @@ -3131,7 +3134,7 @@ fn variable_validation_v2() { send(message: MessageInput): Receipt } ", - "Query" + "Query", ), "query($msg: MessageInput) { send(message: $msg) { @@ -3142,12 +3145,12 @@ fn variable_validation_v2() { "content": "Hello", "author": "Me", "canvas": [{"innput": 4}], - }}) + }}), ); assert!(res.is_err(), "validation should have failed"); // Tests if nested inputs are correctly validated - let res = run_validation_warn_mode!( + let res = run_validation_warn_mode( with_supergraph_boilerplate( " input MessageInput { @@ -3165,7 +3168,7 @@ fn variable_validation_v2() { send(message: MessageInput): Receipt } ", - "Query" + "Query", ), "query($msg: MessageInput) { send(message: $msg) { @@ -3176,7 +3179,7 @@ fn variable_validation_v2() { "content": "Hello", "author": "Me", "canvas": [{"input": 3, "innput": 4}], - }}) + }}), ); assert!( res.is_ok(), @@ -3227,13 +3230,13 @@ fn variable_validation_v2() { } "#; - let res = run_validation_warn_mode!( - schema, + let res = run_validation_warn_mode( + schema.to_string(), "mutation foo($input: FooInput!) { foo (input: $input) { __typename }}", - json!({"input":{}}) + json!({"input":{}}), ); assert!(res.is_ok(), "validation should have succeeded: {res:?}"); } From 18b5fe2ef2bee611c851b1447271970b49f3b713 Mon Sep 17 00:00:00 2001 From: Chidimma O Date: Thu, 12 Feb 2026 09:40:58 -0800 Subject: [PATCH 03/18] Convert validation test macros to functions --- apollo-router/src/spec/query/tests.rs | 352 +++++++++++++------------- 1 file changed, 175 insertions(+), 177 deletions(-) diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index 1d812a1fef..40a90cf190 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -2515,29 +2515,31 @@ fn reformat_response_unknown_typename() { .test(); } -macro_rules! run_validation_enforce_mode { - ($schema:expr, $query:expr, $variables:expr $(,)?) => {{ - let variables = match $variables { - Value::Object(object) => object, - _ => unreachable!("variables must be an object"), - }; - let schema = Schema::parse(&$schema, &Default::default()).expect("could not parse schema"); - let request = Request::builder() - .variables(variables) - .query($query.to_string()) - .build(); - let query = Query::parse( - request - .query - .as_ref() - .expect("query has been added right above; qed"), - None, - &schema, - &Default::default(), - ) - .expect("could not parse query"); - query.validate_variables(&request, &schema, WarnOrEnforceMode::Enforce) - }}; +fn run_validation_enforce_mode( + schema: String, + query: &str, + variables: serde_json_bytes::Value, +) -> Result<(), Response> { + let variables = match variables { + Value::Object(object) => object, + _ => unreachable!("variables must be an object"), + }; + let schema = Schema::parse(&schema, &Default::default()).expect("could not parse schema"); + let request = Request::builder() + .variables(variables) + .query(query.to_string()) + .build(); + let query = Query::parse( + request + .query + .as_ref() + .expect("query has been added right above; qed"), + None, + &schema, + &Default::default(), + ) + .expect("could not parse query"); + query.validate_variables(&request, &schema, WarnOrEnforceMode::Enforce) } fn run_validation_warn_mode( @@ -2567,30 +2569,26 @@ fn run_validation_warn_mode( query.validate_variables(&request, &schema, WarnOrEnforceMode::Warn) } -macro_rules! assert_validation { - ($schema:expr, $query:expr, $variables:expr $(,)?) => {{ - let res = run_validation_enforce_mode!( - with_supergraph_boilerplate($schema, "Query"), - $query, - $variables - ); - assert!(res.is_ok(), "validation should have succeeded: {:?}", res); - }}; +fn assert_validation(schema: &str, query: &str, variables: serde_json_bytes::Value) -> () { + let res = run_validation_enforce_mode( + with_supergraph_boilerplate(schema, "Query"), + query, + variables, + ); + assert!(res.is_ok(), "validation should have succeeded: {:?}", res); } -macro_rules! assert_validation_error { - ($schema:expr, $query:expr, $variables:expr $(,)?) => {{ - let res = run_validation_enforce_mode!( - with_supergraph_boilerplate($schema, "Query"), - $query, - $variables - ); - assert!(res.is_err(), "validation should have failed"); - }}; +fn assert_validation_error(schema: &str, query: &str, variables: serde_json_bytes::Value) -> () { + let res = run_validation_enforce_mode( + with_supergraph_boilerplate(schema, "Query"), + query, + variables, + ); + assert!(res.is_err(), "validation should have failed"); } #[test] -fn variable_validation() { +fn variable_validation_enforce_mode() { let schema = r#" type Query { int(a: Int): String @@ -2604,304 +2602,304 @@ fn variable_validation() { } "#; // https://spec.graphql.org/June2018/#sec-Int - assert_validation!(schema, "query($foo:Int){int(a:$foo)}", json!({})); - assert_validation_error!(schema, "query($foo:Int!){int(a:$foo)}", json!({})); - assert_validation!(schema, "query($foo:Int=1){int(a:$foo)}", json!({})); - assert_validation!(schema, "query($foo:Int!=1){int(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:Int){int(a:$foo)}", json!({})); + assert_validation_error(schema, "query($foo:Int!){int(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:Int=1){int(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:Int!=1){int(a:$foo)}", json!({})); // When expected as an input type, only integer input values are accepted. - assert_validation!(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":2})); - assert_validation!( + assert_validation(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":2})); + assert_validation( schema, "query($foo:Int){int(a:$foo)}", - json!({ "foo": i32::MAX }) + json!({ "foo": i32::MAX }), ); - assert_validation!( + assert_validation( schema, "query($foo:Int){int(a:$foo)}", - json!({ "foo": i32::MIN }) + json!({ "foo": i32::MIN }), ); // All other input values, including strings with numeric content, must raise a query error indicating an incorrect type. - assert_validation_error!(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":"2"})); - assert_validation_error!(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":2.0})); - assert_validation_error!(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":"str"})); - assert_validation_error!(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":true})); - assert_validation_error!(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":{}})); + assert_validation_error(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":"2"})); + assert_validation_error(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":2.0})); + assert_validation_error(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":"str"})); + assert_validation_error(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":true})); + assert_validation_error(schema, "query($foo:Int){int(a:$foo)}", json!({"foo":{}})); // If the integer input value represents a value less than -231 or greater than or equal to 231, a query error should be raised. - assert_validation_error!( + assert_validation_error( schema, "query($foo:Int){int(a:$foo)}", - json!({ "foo": i32::MAX as i64 + 1 }) + json!({ "foo": i32::MAX as i64 + 1 }), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:Int){int(a:$foo)}", - json!({ "foo": i32::MIN as i64 - 1 }) + json!({ "foo": i32::MIN as i64 - 1 }), ); // https://spec.graphql.org/draft/#sec-Float.Input-Coercion - assert_validation!(schema, "query($foo:Float){float(a:$foo)}", json!({})); - assert_validation_error!(schema, "query($foo:Float!){float(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:Float){float(a:$foo)}", json!({})); + assert_validation_error(schema, "query($foo:Float!){float(a:$foo)}", json!({})); // When expected as an input type, both integer and float input values are accepted. - assert_validation!(schema, "query($foo:Float){float(a:$foo)}", json!({"foo":2})); - assert_validation!( + assert_validation(schema, "query($foo:Float){float(a:$foo)}", json!({"foo":2})); + assert_validation( schema, "query($foo:Float){float(a:$foo)}", - json!({"foo":2.0}) + json!({"foo":2.0}), ); // double precision floats are valid - assert_validation!( + assert_validation( schema, "query($foo:Float){float(a:$foo)}", - json!({"foo":1600341978193i64}) + json!({"foo":1600341978193i64}), ); - assert_validation!( + assert_validation( schema, "query($foo:Float){float(a:$foo)}", - json!({"foo":1600341978193f64}) + json!({"foo":1600341978193f64}), ); // All other input values, including strings with numeric content, // must raise a request error indicating an incorrect type. - assert_validation_error!( + assert_validation_error( schema, "query($foo:Float){float(a:$foo)}", - json!({"foo":"2.0"}) + json!({"foo":"2.0"}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:Float){float(a:$foo)}", - json!({"foo":"2"}) + json!({"foo":"2"}), ); // https://spec.graphql.org/June2018/#sec-String - assert_validation!(schema, "query($foo:String){str(a:$foo)}", json!({})); - assert_validation_error!(schema, "query($foo:String!){str(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:String){str(a:$foo)}", json!({})); + assert_validation_error(schema, "query($foo:String!){str(a:$foo)}", json!({})); // When expected as an input type, only valid UTF‐8 string input values are accepted. - assert_validation!( + assert_validation( schema, "query($foo:String){str(a:$foo)}", - json!({"foo": "str"}) + json!({"foo": "str"}), ); // All other input values must raise a query error indicating an incorrect type. - assert_validation_error!( + assert_validation_error( schema, "query($foo:String){str(a:$foo)}", - json!({"foo":true}) + json!({"foo":true}), ); - assert_validation_error!(schema, "query($foo:String){str(a:$foo)}", json!({"foo": 0})); - assert_validation_error!( + assert_validation_error(schema, "query($foo:String){str(a:$foo)}", json!({"foo": 0})); + assert_validation_error( schema, "query($foo:String){str(a:$foo)}", - json!({"foo": 42.0}) + json!({"foo": 42.0}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:String){str(a:$foo)}", - json!({"foo": {}}) + json!({"foo": {}}), ); // https://spec.graphql.org/June2018/#sec-Boolean - assert_validation!(schema, "query($foo:Boolean){bool(a:$foo)}", json!({})); - assert_validation_error!(schema, "query($foo:Boolean!){bool(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:Boolean){bool(a:$foo)}", json!({})); + assert_validation_error(schema, "query($foo:Boolean!){bool(a:$foo)}", json!({})); // When expected as an input type, only boolean input values are accepted. // All other input values must raise a query error indicating an incorrect type. - assert_validation!( + assert_validation( schema, "query($foo:Boolean!){bool(a:$foo)}", - json!({"foo":true}) + json!({"foo":true}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:Boolean!){bool(a:$foo)}", - json!({"foo":"true"}) + json!({"foo":"true"}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:Boolean!){bool(a:$foo)}", - json!({"foo": 0}) + json!({"foo": 0}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:Boolean!){bool(a:$foo)}", - json!({"foo": "no"}) + json!({"foo": "no"}), ); - assert_validation!(schema, "query($foo:Boolean=true){bool(a:$foo)}", json!({})); - assert_validation!(schema, "query($foo:Boolean!=true){bool(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:Boolean=true){bool(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:Boolean!=true){bool(a:$foo)}", json!({})); // https://spec.graphql.org/June2018/#sec-ID - assert_validation!(schema, "query($foo:ID){id(a:$foo)}", json!({})); - assert_validation_error!(schema, "query($foo:ID!){id(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:ID){id(a:$foo)}", json!({})); + assert_validation_error(schema, "query($foo:ID!){id(a:$foo)}", json!({})); // When expected as an input type, any string (such as "4") or integer (such as 4) // input value should be coerced to ID as appropriate for the ID formats a given GraphQL server expects. - assert_validation!(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": 4})); - assert_validation!(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": "4"})); - assert_validation!( + assert_validation(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": 4})); + assert_validation(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": "4"})); + assert_validation( schema, "query($foo:String){str(a:$foo)}", - json!({"foo": "str"}) + json!({"foo": "str"}), ); - assert_validation!( + assert_validation( schema, "query($foo:String){str(a:$foo)}", - json!({"foo": "4.0"}) + json!({"foo": "4.0"}), ); // Any other input value, including float input values (such as 4.0), must raise a query error indicating an incorrect type. - assert_validation_error!(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": 4.0})); - assert_validation_error!(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": true})); - assert_validation_error!(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": {}})); + assert_validation_error(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": 4.0})); + assert_validation_error(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": true})); + assert_validation_error(schema, "query($foo:ID){id(a:$foo)}", json!({"foo": {}})); // https://spec.graphql.org/June2018/#sec-Type-System.List - assert_validation!(schema, "query($foo:[Int]){intList(a:$foo)}", json!({})); - assert_validation!(schema, "query($foo:[Int!]){intList(a:$foo)}", json!({})); - assert_validation!( + assert_validation(schema, "query($foo:[Int]){intList(a:$foo)}", json!({})); + assert_validation(schema, "query($foo:[Int!]){intList(a:$foo)}", json!({})); + assert_validation( schema, "query($foo:[Int!]){intList(a:$foo)}", - json!({ "foo": null }) + json!({ "foo": null }), ); - assert_validation!( + assert_validation( schema, "query($foo:[Int]){intList(a:$foo)}", - json!({"foo":1}) + json!({"foo":1}), ); - assert_validation!( + assert_validation( schema, "query($foo:[String]){strList(a:$foo)}", - json!({"foo":"bar"}) + json!({"foo":"bar"}), ); - assert_validation!( + assert_validation( schema, "query($foo:[[Int]]){intListList(a:$foo)}", - json!({"foo":1}) + json!({"foo":1}), ); - assert_validation!( + assert_validation( schema, "query($foo:[[Int]]){intListList(a:$foo)}", - json!({"foo":[[1], [2, 3]]}) + json!({"foo":[[1], [2, 3]]}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:[Int]){intList(a:$foo)}", - json!({"foo":"str"}) + json!({"foo":"str"}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:[Int]){intList(a:$foo)}", - json!({"foo":{}}) + json!({"foo":{}}), ); - assert_validation_error!(schema, "query($foo:[Int]!){intList(a:$foo)}", json!({})); - assert_validation_error!( + assert_validation_error(schema, "query($foo:[Int]!){intList(a:$foo)}", json!({})); + assert_validation_error( schema, "query($foo:[Int!]){intList(a:$foo)}", - json!({"foo":[1, null]}) + json!({"foo":[1, null]}), ); - assert_validation!( + assert_validation( schema, "query($foo:[Int]!){intList(a:$foo)}", - json!({"foo":[]}) + json!({"foo":[]}), ); - assert_validation!( + assert_validation( schema, "query($foo:[Int]){intList(a:$foo)}", - json!({"foo":[1,2,3]}) + json!({"foo":[1,2,3]}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:[Int]){intList(a:$foo)}", - json!({"foo":["f","o","o"]}) + json!({"foo":["f","o","o"]}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:[Int]){intList(a:$foo)}", - json!({"foo":["1","2","3"]}) + json!({"foo":["1","2","3"]}), ); - assert_validation!( + assert_validation( schema, "query($foo:[String]){strList(a:$foo)}", - json!({"foo":["1","2","3"]}) + json!({"foo":["1","2","3"]}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:[String]){strList(a:$foo)}", - json!({"foo":[1,2,3]}) + json!({"foo":[1,2,3]}), ); - assert_validation!( + assert_validation( schema, "query($foo:[Int!]){intList(a:$foo)}", - json!({"foo":[1,2,3]}) + json!({"foo":[1,2,3]}), ); - assert_validation_error!( + assert_validation_error( schema, "query($foo:[Int!]){intList(a:$foo)}", - json!({"foo":[1,null,3]}) + json!({"foo":[1,null,3]}), ); - assert_validation!( + assert_validation( schema, "query($foo:[Int]){intList(a:$foo)}", - json!({"foo":[1,null,3]}) + json!({"foo":[1,null,3]}), ); // https://spec.graphql.org/June2018/#sec-Input-Objects - assert_validation!( + assert_validation( "input Foo{ y: String } type Query { x(foo: Foo): String }", "query($foo:Foo){x(foo: $foo)}", - json!({}) + json!({}), ); - assert_validation!( + assert_validation( "input Foo{ y: String } type Query { x(foo: Foo): String }", "query($foo:Foo){x(foo: $foo)}", - json!({"foo":{}}) + json!({"foo":{}}), ); - assert_validation_error!( + assert_validation_error( "input Foo{ y: String } type Query { x(foo: Foo): String }", "query($foo:Foo){x(foo: $foo)}", - json!({"foo":1}) + json!({"foo":1}), ); - assert_validation_error!( + assert_validation_error( "input Foo{ y: String } type Query { x(foo: Foo): String }", "query($foo:Foo){x(foo: $foo)}", - json!({"foo":"str"}) + json!({"foo":"str"}), ); - assert_validation_error!( + assert_validation_error( "input Foo{x:Int!} type Query { x(foo: Foo): String }", "query($foo:Foo){x(foo: $foo)}", - json!({"foo":{}}) + json!({"foo":{}}), ); - assert_validation!( + assert_validation( "input Foo{x:Int!} type Query { x(foo: Foo): String }", "query($foo:Foo){x(foo: $foo)}", - json!({"foo":{"x":1}}) + json!({"foo":{"x":1}}), ); - assert_validation!( + assert_validation( "scalar Foo type Query { x(foo: Foo): String }", "query($foo:Foo!){x(foo: $foo)}", - json!({"foo":{}}) + json!({"foo":{}}), ); - assert_validation!( + assert_validation( "scalar Foo type Query { x(foo: Foo): String }", "query($foo:Foo!){x(foo: $foo)}", - json!({"foo":1}) + json!({"foo":1}), ); - assert_validation_error!( + assert_validation_error( "scalar Foo type Query { x(foo: Foo): String }", "query($foo:Foo!){x(foo: $foo)}", - json!({}) + json!({}), ); - assert_validation!( + assert_validation( "input Foo{bar:Bar!} input Bar{x:Int!} type Query { x(foo: Foo): String }", "query($foo:Foo){x(foo: $foo)}", - json!({"foo":{"bar":{"x":1}}}) + json!({"foo":{"bar":{"x":1}}}), ); - assert_validation!( + assert_validation( "enum Availability{AVAILABLE} type Product{availability:Availability! name:String} type Query{products(availability: Availability!): [Product]!}", "query GetProductsByAvailability($availability: Availability!){products(availability: $availability) {name}}", - json!({"availability": "AVAILABLE"}) + json!({"availability": "AVAILABLE"}), ); - assert_validation!( + assert_validation( "input MessageInput { content: String author: String @@ -2918,10 +2916,10 @@ fn variable_validation() { }) { id }}", - json!({"availability": "AVAILABLE"}) + json!({"availability": "AVAILABLE"}), ); - assert_validation!( + assert_validation( "input MessageInput { content: String author: String @@ -2938,10 +2936,10 @@ fn variable_validation() { json!({"msg": { "content": "Hello", "author": "Me" - }}) + }}), ); - assert_validation_error!( + assert_validation_error( "input MessageInput { content: String author: String @@ -2959,11 +2957,11 @@ fn variable_validation() { "content": "Hello", "author": "Me", "unknownField": "unknown", - }}) + }}), ); // Tests if nested inputs are correctly validated - assert_validation_error!( + assert_validation_error( "input MessageInput { content: String author: String @@ -2989,7 +2987,7 @@ fn variable_validation() { {"input": 4}, {"input": 5, "unknownField": "unknown"} ], - }}) + }}), ); let schema = r#" @@ -3036,13 +3034,13 @@ fn variable_validation() { } "#; - let res = run_validation_enforce_mode!( - schema, + let res = run_validation_enforce_mode( + schema.to_string(), "mutation foo($input: FooInput!) { foo (input: $input) { __typename }}", - json!({"input":{}}) + json!({"input":{}}), ); assert!(res.is_ok(), "validation should have succeeded: {res:?}"); } From 1b6237676615ab7f365984f12d3fb1a9bb4a7f5c Mon Sep 17 00:00:00 2001 From: Chidimma O Date: Thu, 12 Feb 2026 10:25:53 -0800 Subject: [PATCH 04/18] Update supergraph yaml snippet with strict_variable_validation --- docs/shared/config/supergraph.mdx | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/shared/config/supergraph.mdx b/docs/shared/config/supergraph.mdx index 289f8660cc..8d3ea41c6d 100644 --- a/docs/shared/config/supergraph.mdx +++ b/docs/shared/config/supergraph.mdx @@ -37,6 +37,7 @@ supergraph: experimental_plans_limit: null experimental_reuse_query_plans: false warmed_up_queries: null + strict_variable_validation: enabled ``` From f1b8570e76abb2c3744a93a7f82808b10fd4206a Mon Sep 17 00:00:00 2001 From: Chidimma O Date: Thu, 12 Feb 2026 10:27:55 -0800 Subject: [PATCH 05/18] Update full router yaml snippet with strict_variable_validation --- docs/shared/router-yaml-complete.mdx | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/shared/router-yaml-complete.mdx b/docs/shared/router-yaml-complete.mdx index 73147e74e5..1353c58322 100644 --- a/docs/shared/router-yaml-complete.mdx +++ b/docs/shared/router-yaml-complete.mdx @@ -357,6 +357,7 @@ supergraph: experimental_plans_limit: null experimental_reuse_query_plans: false warmed_up_queries: null + strict_variable_validation: enabled telemetry: apollo: batch_processor: From fce1933ba2ef19b761af3716d686a4fd1dcf76fa Mon Sep 17 00:00:00 2001 From: Chidimma O Date: Thu, 12 Feb 2026 10:30:26 -0800 Subject: [PATCH 06/18] Fix incorrect strict_variable_validation mode name --- docs/shared/config/supergraph.mdx | 2 +- docs/shared/router-yaml-complete.mdx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/shared/config/supergraph.mdx b/docs/shared/config/supergraph.mdx index 8d3ea41c6d..b54fd21408 100644 --- a/docs/shared/config/supergraph.mdx +++ b/docs/shared/config/supergraph.mdx @@ -37,7 +37,7 @@ supergraph: experimental_plans_limit: null experimental_reuse_query_plans: false warmed_up_queries: null - strict_variable_validation: enabled + strict_variable_validation: enforce ``` diff --git a/docs/shared/router-yaml-complete.mdx b/docs/shared/router-yaml-complete.mdx index 1353c58322..f0b8285808 100644 --- a/docs/shared/router-yaml-complete.mdx +++ b/docs/shared/router-yaml-complete.mdx @@ -357,7 +357,7 @@ supergraph: experimental_plans_limit: null experimental_reuse_query_plans: false warmed_up_queries: null - strict_variable_validation: enabled + strict_variable_validation: enforce telemetry: apollo: batch_processor: From 1e86ff2fdf9ba57ce0b82ca2e5f3152ce41c7162 Mon Sep 17 00:00:00 2001 From: Chidimma O Date: Thu, 12 Feb 2026 10:48:08 -0800 Subject: [PATCH 07/18] Remove superflous test and update comments --- apollo-router/src/spec/query/tests.rs | 60 ++------------------------- 1 file changed, 3 insertions(+), 57 deletions(-) diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index 40a90cf190..42333bf823 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -3076,7 +3076,7 @@ fn variable_validation_warn_mode() { "validation should have warned rather than failed" ); - // Tests if nested inputs are correctly validated + // Tests if nested unknown input fields are caught let res = run_validation_warn_mode( with_supergraph_boilerplate( "input MessageInput { @@ -3113,7 +3113,7 @@ fn variable_validation_warn_mode() { "validation should have warned rather than failed" ); - // Tests if nested inputs are correctly validated + // Tests if misspelled field names are caught let res = run_validation_warn_mode( with_supergraph_boilerplate( " @@ -3147,7 +3147,7 @@ fn variable_validation_warn_mode() { ); assert!(res.is_err(), "validation should have failed"); - // Tests if nested inputs are correctly validated + // Tests if a misspelled field is caught even when the correct field is present let res = run_validation_warn_mode( with_supergraph_boilerplate( " @@ -3183,60 +3183,6 @@ fn variable_validation_warn_mode() { res.is_ok(), "validation should have warned rather than failed" ); - - let schema = r#" - schema - @link(url: "https://specs.apollo.dev/link/v1.0") - @link(url: "https://specs.apollo.dev/join/v0.3", for: EXECUTION) - { - query: Query - mutation: Mutation - } - directive @link(url: String, as: String, for: link__Purpose, import: [link__Import]) repeatable on SCHEMA - directive @join__graph(name: String!, url: String!) on ENUM_VALUE - directive @join__type( graph: join__Graph! key: join__FieldSet extension: Boolean! = false resolvable: Boolean! = true isInterfaceObject: Boolean! = false) repeatable on OBJECT | INTERFACE | UNION | ENUM | INPUT_OBJECT | SCALAR - - scalar join__FieldSet - scalar link__Import - - enum link__Purpose { - SECURITY - EXECUTION - } - - enum join__Graph { - TEST @join__graph(name: "test", url: "http://localhost:4001/graphql") - } - - type Mutation{ - foo(input: FooInput!): FooResponse! - } - type Query @join__type(graph: TEST){ - data: String - } - - input FooInput { - enumWithDefault: EnumWithDefault! = WEB - } - type FooResponse { - id: ID! - } - - enum EnumWithDefault { - WEB - MOBILE - } - "#; - - let res = run_validation_warn_mode( - schema.to_string(), - "mutation foo($input: FooInput!) { - foo (input: $input) { - __typename - }}", - json!({"input":{}}), - ); - assert!(res.is_ok(), "validation should have succeeded: {res:?}"); } #[test] From 320b9342ddd1ef01fb1b07ac895e105effe32f8c Mon Sep 17 00:00:00 2001 From: Chidimma O Date: Thu, 12 Feb 2026 11:21:55 -0800 Subject: [PATCH 08/18] Add explanation of strict_variable_validation config --- docs/source/routing/configuration/yaml.mdx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/docs/source/routing/configuration/yaml.mdx b/docs/source/routing/configuration/yaml.mdx index 6182b956c7..cdda2bec3e 100644 --- a/docs/source/routing/configuration/yaml.mdx +++ b/docs/source/routing/configuration/yaml.mdx @@ -476,13 +476,25 @@ traffic_shaping: By default, the router compresses subgraph requests by generating fragment definitions based on the shape of the subgraph operation. In many cases this significantly reduces the size of the query sent to subgraphs. -You can explicitly opt-out of this behavior by specifying `supergraph.generate_query_fragments`: +You can explicitly opt out of this behavior by specifying `supergraph.generate_query_fragments`: ```yaml supergraph: generate_query_fragments: false ``` +#### Variable Validation Modes + +By default, the router validates input variables strictly, meaning that each input object value will be validated against its type definition and any unknown fields will result in a request error. + +```yaml +supergraph: + strict_variable_validation: enforce +``` + +If your implementation requires the use of unknown fields on a defined type, you can opt out of stricter validation by specifying `strict_variable_validation: warn`. +In this case, the router will not error when encountering unknown fields, but will log the field for reference. + --- From 3dd74fe5faeff0042bc3b791c4e5f4649b549783 Mon Sep 17 00:00:00 2001 From: Chidimma O Date: Thu, 12 Feb 2026 12:14:41 -0800 Subject: [PATCH 09/18] Add changeset --- ...breaking_plane_drawers_comedy_projector.md | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .changesets/breaking_plane_drawers_comedy_projector.md diff --git a/.changesets/breaking_plane_drawers_comedy_projector.md b/.changesets/breaking_plane_drawers_comedy_projector.md new file mode 100644 index 0000000000..7d476810cf --- /dev/null +++ b/.changesets/breaking_plane_drawers_comedy_projector.md @@ -0,0 +1,32 @@ +### Add config to opt-out of stricter variable validation ([PR #8884](https://github.com/apollographql/router/pull/8884)) + +Variable validation will become **_stricter by default_** due to [PR#8821](https://github.com/apollographql/router/pull/8821). +This PR fixed a gap in variable validation whereby the presence of unknown fields on an input object variable were not causing a request error as they should have. + +This stricter validation **_may cause breakages_** for customers. + +To alleviate that potential pain point while customers update their variables to be compliant, this change introduces a router config option to retain the previous level of validation and issue a warning log instead of an error. + +> [!WARNING] +> If you need to opt out, you must set the config option to `warn` instead. + +Enabled: +```yaml +supergraph: + strict_variable_validation: enforce +``` + +Disabled: +```yaml +supergraph: + strict_variable_validation: warn +``` + +Docs have also been updated to reflect this change. + + +--- + +[ROUTER-1602]: https://apollographql.atlassian.net/browse/ROUTER-1602?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ + +By [@carodewig](https://github.com/carodewig) and [@conwuegb](https://github.com/conwuegb) in https://github.com/apollographql/router/pull/8884 From 2adb9cc5d6af3aa8c800ab2630c72bd7933bde70 Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Fri, 13 Feb 2026 15:04:48 -0500 Subject: [PATCH 10/18] doc: combine changesets for simplicity --- ...breaking_plane_drawers_comedy_projector.md | 32 ------------------- .../fix_task_crust_preacher_toothpaste.md | 19 +++++++++-- 2 files changed, 16 insertions(+), 35 deletions(-) delete mode 100644 .changesets/breaking_plane_drawers_comedy_projector.md diff --git a/.changesets/breaking_plane_drawers_comedy_projector.md b/.changesets/breaking_plane_drawers_comedy_projector.md deleted file mode 100644 index 7d476810cf..0000000000 --- a/.changesets/breaking_plane_drawers_comedy_projector.md +++ /dev/null @@ -1,32 +0,0 @@ -### Add config to opt-out of stricter variable validation ([PR #8884](https://github.com/apollographql/router/pull/8884)) - -Variable validation will become **_stricter by default_** due to [PR#8821](https://github.com/apollographql/router/pull/8821). -This PR fixed a gap in variable validation whereby the presence of unknown fields on an input object variable were not causing a request error as they should have. - -This stricter validation **_may cause breakages_** for customers. - -To alleviate that potential pain point while customers update their variables to be compliant, this change introduces a router config option to retain the previous level of validation and issue a warning log instead of an error. - -> [!WARNING] -> If you need to opt out, you must set the config option to `warn` instead. - -Enabled: -```yaml -supergraph: - strict_variable_validation: enforce -``` - -Disabled: -```yaml -supergraph: - strict_variable_validation: warn -``` - -Docs have also been updated to reflect this change. - - ---- - -[ROUTER-1602]: https://apollographql.atlassian.net/browse/ROUTER-1602?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ - -By [@carodewig](https://github.com/carodewig) and [@conwuegb](https://github.com/conwuegb) in https://github.com/apollographql/router/pull/8884 diff --git a/.changesets/fix_task_crust_preacher_toothpaste.md b/.changesets/fix_task_crust_preacher_toothpaste.md index 7c8395b4f7..6ab338efca 100644 --- a/.changesets/fix_task_crust_preacher_toothpaste.md +++ b/.changesets/fix_task_crust_preacher_toothpaste.md @@ -1,4 +1,4 @@ -### Fix Router's validation of ObjectValue variables ([PR #8821](https://github.com/apollographql/router/pull/8821)) +### Fix Router's validation of `ObjectValue` variables ([PR #8821](https://github.com/apollographql/router/pull/8821) and [PR #8884](https://github.com/apollographql/router/pull/8884)) This change addresses an issue in Router whereby invalid additional fields of an input object were able to pass variable validation because the fields of the object were not being properly checked. @@ -34,6 +34,19 @@ query($msg: MessageInput) { ``` This request would pass validation because the variable `msg` from the query was present in the input, however, the fields of `msg` from the input were not being validated against the `MessageInput` type. -[ROUTER-981]: https://apollographql.atlassian.net/browse/ROUTER-981?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ +> [!WARNING] +> If you need to opt out, you must set the `supergraph.strict_variable_validation` config option to `warn` instead. -By [@conwuegb](https://github.com/conwuegb) in https://github.com/apollographql/router/pull/8821 \ No newline at end of file +Enabled: +```yaml +supergraph: + strict_variable_validation: enforce +``` + +Disabled: +```yaml +supergraph: + strict_variable_validation: warn +``` + +By [@conwuegb](https://github.com/conwuegb) in https://github.com/apollographql/router/pull/8821 and https://github.com/apollographql/router/pull/8884 From cf0f48aaa34d1fe66f365d19f89856e3debceb0d Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Fri, 13 Feb 2026 15:19:18 -0500 Subject: [PATCH 11/18] chore: refactor to use Mode rather than a new *Mode --- .changesets/fix_task_crust_preacher_toothpaste.md | 4 ++-- apollo-router/src/configuration/mod.rs | 12 ++++++------ apollo-router/src/configuration/mode.rs | 9 --------- apollo-router/src/services/supergraph/service.rs | 8 ++++---- apollo-router/src/spec/field_type.rs | 13 +++++++------ apollo-router/src/spec/query.rs | 4 ++-- apollo-router/src/spec/query/tests.rs | 4 ++-- ...e_validation_mode_propagates_fully@measure.snap} | 0 apollo-router/tests/integration/validation.rs | 2 +- docs/source/routing/configuration/yaml.mdx | 2 +- 10 files changed, 25 insertions(+), 33 deletions(-) rename apollo-router/tests/integration/snapshots/{integration_tests__integration__validation__variable_validation_mode_propagates_fully@warn.snap => integration_tests__integration__validation__variable_validation_mode_propagates_fully@measure.snap} (100%) diff --git a/.changesets/fix_task_crust_preacher_toothpaste.md b/.changesets/fix_task_crust_preacher_toothpaste.md index 6ab338efca..6de105b49c 100644 --- a/.changesets/fix_task_crust_preacher_toothpaste.md +++ b/.changesets/fix_task_crust_preacher_toothpaste.md @@ -35,7 +35,7 @@ query($msg: MessageInput) { This request would pass validation because the variable `msg` from the query was present in the input, however, the fields of `msg` from the input were not being validated against the `MessageInput` type. > [!WARNING] -> If you need to opt out, you must set the `supergraph.strict_variable_validation` config option to `warn` instead. +> If you need to opt out, you must set the `supergraph.strict_variable_validation` config option to `measure` instead. Enabled: ```yaml @@ -46,7 +46,7 @@ supergraph: Disabled: ```yaml supergraph: - strict_variable_validation: warn + strict_variable_validation: measure ``` By [@conwuegb](https://github.com/conwuegb) in https://github.com/apollographql/router/pull/8821 and https://github.com/apollographql/router/pull/8884 diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index b290dc8ec9..b81b728b4a 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -49,7 +49,7 @@ use self::subgraph::SubgraphConfiguration; use crate::ApolloRouterError; use crate::cache::DEFAULT_CACHE_CAPACITY; use crate::configuration::cooperative_cancellation::CooperativeCancellation; -use crate::configuration::mode::WarnOrEnforceMode; +use crate::configuration::mode::Mode; use crate::graphql; use crate::plugin::plugins; use crate::plugins::chaos; @@ -747,7 +747,7 @@ pub(crate) struct Supergraph { /// TODO(@caroline) docs #[serde(default = "default_strict_variable_validation")] - pub(crate) strict_variable_validation: WarnOrEnforceMode, + pub(crate) strict_variable_validation: Mode, } const fn default_generate_query_fragments() -> bool { @@ -772,7 +772,7 @@ impl Supergraph { early_cancel: Option, experimental_log_on_broken_pipe: Option, insert_result_coercion_errors: Option, - strict_variable_validation: Option, + strict_variable_validation: Option, ) -> Self { Self { listen: listen.unwrap_or_else(default_graphql_listen), @@ -808,7 +808,7 @@ impl Supergraph { early_cancel: Option, experimental_log_on_broken_pipe: Option, insert_result_coercion_errors: Option, - strict_variable_validation: Option, + strict_variable_validation: Option, ) -> Self { Self { listen: listen.unwrap_or_else(test_listen), @@ -1517,8 +1517,8 @@ fn default_connection_shutdown_timeout() -> Duration { Duration::from_secs(60) } -fn default_strict_variable_validation() -> WarnOrEnforceMode { - WarnOrEnforceMode::Enforce +fn default_strict_variable_validation() -> Mode { + Mode::Enforce } #[derive(Clone, Debug, Default, Error, Display, Serialize, Deserialize, JsonSchema)] diff --git a/apollo-router/src/configuration/mode.rs b/apollo-router/src/configuration/mode.rs index e4244ab3de..83285a6fe7 100644 --- a/apollo-router/src/configuration/mode.rs +++ b/apollo-router/src/configuration/mode.rs @@ -10,12 +10,3 @@ pub(crate) enum Mode { Measure, Enforce, } - -// Don't add a default here. Instead, Default should be implemented for -// individual cases of WarnOrEnforceMode. -#[derive(Debug, Clone, Copy, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "snake_case")] -pub(crate) enum WarnOrEnforceMode { - Warn, - Enforce, -} diff --git a/apollo-router/src/services/supergraph/service.rs b/apollo-router/src/services/supergraph/service.rs index fc9d149088..1eb40ba84d 100644 --- a/apollo-router/src/services/supergraph/service.rs +++ b/apollo-router/src/services/supergraph/service.rs @@ -25,7 +25,7 @@ use crate::Context; use crate::batching::BatchQuery; use crate::configuration::Batching; use crate::configuration::PersistedQueriesPrewarmQueryPlanCache; -use crate::configuration::mode::WarnOrEnforceMode; +use crate::configuration::mode::Mode; use crate::error::CacheResolverError; use crate::graphql; use crate::graphql::IntoGraphQLErrors; @@ -80,7 +80,7 @@ pub(crate) struct SupergraphService { query_planner_service: CachingQueryPlanner, execution_service: execution::BoxCloneService, schema: Arc, - strict_variable_validation: WarnOrEnforceMode, + strict_variable_validation: Mode, } #[buildstructor::buildstructor] @@ -90,7 +90,7 @@ impl SupergraphService { query_planner_service: CachingQueryPlanner, execution_service: execution::BoxCloneService, schema: Arc, - strict_variable_validation: WarnOrEnforceMode, + strict_variable_validation: Mode, ) -> Self { SupergraphService { query_planner_service, @@ -157,7 +157,7 @@ async fn service_call( execution_service: execution::BoxCloneService, schema: Arc, req: SupergraphRequest, - strict_variable_validation: WarnOrEnforceMode, // todo + strict_variable_validation: Mode, ) -> Result { let context = req.context; let body = req.supergraph_request.body(); diff --git a/apollo-router/src/spec/field_type.rs b/apollo-router/src/spec/field_type.rs index 9fc026e499..00a359071a 100644 --- a/apollo-router/src/spec/field_type.rs +++ b/apollo-router/src/spec/field_type.rs @@ -5,7 +5,7 @@ use serde::Serialize; use serde::de::Error as _; use super::query::parse_hir_value; -use crate::configuration::mode::WarnOrEnforceMode; +use crate::configuration::mode::Mode; use crate::json_ext::Value; use crate::json_ext::ValueExt; use crate::spec::Schema; @@ -124,7 +124,7 @@ fn validate_input_value( value: Option<&Value>, schema: &Schema, path: &JsonValuePath<'_>, - strict_variable_validation: WarnOrEnforceMode, // todo + strict_variable_validation: Mode, ) -> Result<(), InvalidInputValue> { let fmt_path = |var_path: &JsonValuePath<'_>| match var_path { JsonValuePath::Variable { .. } => format!("variable `{var_path}`"), @@ -241,15 +241,16 @@ fn validate_input_value( }); match strict_variable_validation { - WarnOrEnforceMode::Enforce => { + Mode::Enforce => { if let Some(field) = unknown_input_fields.next() { return Err(unknown_field(field)); } } - WarnOrEnforceMode::Warn => { + Mode::Measure => { + // TODO(@caroline): increment counter let unknown_fields: Vec<&str> = unknown_input_fields.collect(); if !unknown_fields.is_empty() { - tracing::warn!(variables = ?unknown_fields, "encountered unexpected variable(s)"); // consider just doing first? based on comment at top of fn + tracing::warn!(variables = ?unknown_fields, "encountered unexpected variable(s)"); // TODO(@caroline): consider just doing first? based on comment at top of fn } } } @@ -300,7 +301,7 @@ impl FieldType { value: Option<&Value>, schema: &Schema, path: &JsonValuePath<'_>, - strict_variable_validation: WarnOrEnforceMode, + strict_variable_validation: Mode, ) -> Result<(), InvalidInputValue> { validate_input_value(&self.0, value, schema, path, strict_variable_validation) } diff --git a/apollo-router/src/spec/query.rs b/apollo-router/src/spec/query.rs index deb2f4bdb0..11ebfabc4d 100644 --- a/apollo-router/src/spec/query.rs +++ b/apollo-router/src/spec/query.rs @@ -24,7 +24,7 @@ use self::subselections::SubSelectionValue; use super::Fragment; use super::QueryHash; use crate::Configuration; -use crate::configuration::mode::WarnOrEnforceMode; +use crate::configuration::mode::Mode; use crate::error::FetchError; use crate::graphql::Error; use crate::graphql::Request; @@ -1036,7 +1036,7 @@ impl Query { &self, request: &Request, schema: &Schema, - strict_variable_validation: WarnOrEnforceMode, // todo + strict_variable_validation: Mode, ) -> Result<(), Response> { if LevelFilter::current() >= LevelFilter::DEBUG { let known_variables = self diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index 42333bf823..3def29cdf7 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -2539,7 +2539,7 @@ fn run_validation_enforce_mode( &Default::default(), ) .expect("could not parse query"); - query.validate_variables(&request, &schema, WarnOrEnforceMode::Enforce) + query.validate_variables(&request, &schema, Mode::Enforce) } fn run_validation_warn_mode( @@ -2566,7 +2566,7 @@ fn run_validation_warn_mode( &Default::default(), ) .expect("could not parse query"); - query.validate_variables(&request, &schema, WarnOrEnforceMode::Warn) + query.validate_variables(&request, &schema, Mode::Measure) } fn assert_validation(schema: &str, query: &str, variables: serde_json_bytes::Value) -> () { diff --git a/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@warn.snap b/apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@measure.snap similarity index 100% rename from apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@warn.snap rename to apollo-router/tests/integration/snapshots/integration_tests__integration__validation__variable_validation_mode_propagates_fully@measure.snap diff --git a/apollo-router/tests/integration/validation.rs b/apollo-router/tests/integration/validation.rs index 91901a879a..e5ff988803 100644 --- a/apollo-router/tests/integration/validation.rs +++ b/apollo-router/tests/integration/validation.rs @@ -214,7 +214,7 @@ async fn test_lots_of_validation_errors() { #[rstest::rstest] #[case(Some("enforce"), true, false)] -#[case(Some("warn"), false, true)] +#[case(Some("measure"), false, true)] #[case(None, true, false)] #[tokio::test(flavor = "multi_thread")] async fn variable_validation_mode_propagates_fully( diff --git a/docs/source/routing/configuration/yaml.mdx b/docs/source/routing/configuration/yaml.mdx index cdda2bec3e..6843101005 100644 --- a/docs/source/routing/configuration/yaml.mdx +++ b/docs/source/routing/configuration/yaml.mdx @@ -492,7 +492,7 @@ supergraph: strict_variable_validation: enforce ``` -If your implementation requires the use of unknown fields on a defined type, you can opt out of stricter validation by specifying `strict_variable_validation: warn`. +If your implementation requires the use of unknown fields on a defined type, you can opt out of stricter validation by specifying `strict_variable_validation: measure`. In this case, the router will not error when encountering unknown fields, but will log the field for reference. --- From 556f80cc30cfaf0f6fcbf9266e1a0da33a526ad9 Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Fri, 13 Feb 2026 16:25:26 -0500 Subject: [PATCH 12/18] chore: refactor, remove todos --- apollo-router/src/spec/field_type.rs | 36 +++++++++++++--------------- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/apollo-router/src/spec/field_type.rs b/apollo-router/src/spec/field_type.rs index 00a359071a..1aaa450eb2 100644 --- a/apollo-router/src/spec/field_type.rs +++ b/apollo-router/src/spec/field_type.rs @@ -1,3 +1,5 @@ +use std::iter::once; + use apollo_compiler::Name; use apollo_compiler::schema; use serde::Deserialize; @@ -231,26 +233,22 @@ fn validate_input_value( )) }; - let mut unknown_input_fields = obj.keys().filter_map(|k| { - let k = k.as_str(); - if !def.fields.contains_key(k) { - Some(k) - } else { - None - } - }); - - match strict_variable_validation { - Mode::Enforce => { - if let Some(field) = unknown_input_fields.next() { - return Err(unknown_field(field)); + let mut unknown_input_fields = obj + .keys() + .map(|k| k.as_str()) + .filter(|&k| !def.fields.contains_key(k)); + if let Some(unknown_input_field) = unknown_input_fields.next() { + match strict_variable_validation { + Mode::Enforce => { + return Err(unknown_field(unknown_input_field)); } - } - Mode::Measure => { - // TODO(@caroline): increment counter - let unknown_fields: Vec<&str> = unknown_input_fields.collect(); - if !unknown_fields.is_empty() { - tracing::warn!(variables = ?unknown_fields, "encountered unexpected variable(s)"); // TODO(@caroline): consider just doing first? based on comment at top of fn + Mode::Measure => { + let unknown_fields: Vec<&str> = once(unknown_input_field) + .chain(unknown_input_fields) + .collect(); + // NB: warning will be attached to the span via trace id, so you can figure out + // operation name from parent span + tracing::warn!(variables = ?unknown_fields, "encountered unexpected variable(s)"); } } } From ea0f4eda8cf00bbc709dc2646e343ec085753b3d Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Fri, 13 Feb 2026 16:25:39 -0500 Subject: [PATCH 13/18] chore: name tests in rstest --- apollo-router/tests/integration/validation.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apollo-router/tests/integration/validation.rs b/apollo-router/tests/integration/validation.rs index e5ff988803..571c215234 100644 --- a/apollo-router/tests/integration/validation.rs +++ b/apollo-router/tests/integration/validation.rs @@ -213,9 +213,9 @@ async fn test_lots_of_validation_errors() { } #[rstest::rstest] -#[case(Some("enforce"), true, false)] -#[case(Some("measure"), false, true)] -#[case(None, true, false)] +#[case::enforce(Some("enforce"), true, false)] +#[case::measure(Some("measure"), false, true)] +#[case::missing(None, true, false)] #[tokio::test(flavor = "multi_thread")] async fn variable_validation_mode_propagates_fully( #[case] strict_variable_validation: Option<&str>, From f334b157c67765f1be622f7f3d69d54423f9af1c Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Fri, 13 Feb 2026 17:28:57 -0500 Subject: [PATCH 14/18] chore: refactor tests for simplicity --- apollo-router/src/graphql/response.rs | 13 ++ apollo-router/src/spec/query/tests.rs | 224 +++++++------------------- 2 files changed, 73 insertions(+), 164 deletions(-) diff --git a/apollo-router/src/graphql/response.rs b/apollo-router/src/graphql/response.rs index b4bcdd7868..78c1f58397 100644 --- a/apollo-router/src/graphql/response.rs +++ b/apollo-router/src/graphql/response.rs @@ -254,6 +254,19 @@ impl From for Response { } } +#[cfg(test)] +impl Response { + pub(crate) fn errors_with_code<'a>(&'a self, code: &'a str) -> impl Iterator { + self.errors + .iter() + .filter(move |err| err.extension_code().is_some_and(|c| c == code)) + } + + pub(crate) fn contains_error_code(&self, code: &str) -> bool { + self.errors_with_code(code).next().is_some() + } +} + #[cfg(test)] mod tests { use serde_json::json; diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index 3def29cdf7..565e481920 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -2515,10 +2515,11 @@ fn reformat_response_unknown_typename() { .test(); } -fn run_validation_enforce_mode( +fn run_validation( schema: String, query: &str, variables: serde_json_bytes::Value, + mode: Mode, ) -> Result<(), Response> { let variables = match variables { Value::Object(object) => object, @@ -2539,50 +2540,25 @@ fn run_validation_enforce_mode( &Default::default(), ) .expect("could not parse query"); - query.validate_variables(&request, &schema, Mode::Enforce) -} - -fn run_validation_warn_mode( - schema: String, - query: &str, - variables: serde_json_bytes::Value, -) -> Result<(), Response> { - let variables = match variables { - Value::Object(object) => object, - _ => unreachable!("variables must be an object"), - }; - let schema = Schema::parse(&schema, &Default::default()).expect("could not parse schema"); - let request = Request::builder() - .variables(variables) - .query(query.to_string()) - .build(); - let query = Query::parse( - request - .query - .as_ref() - .expect("query has been added right above; qed"), - None, - &schema, - &Default::default(), - ) - .expect("could not parse query"); - query.validate_variables(&request, &schema, Mode::Measure) + query.validate_variables(&request, &schema, mode) } fn assert_validation(schema: &str, query: &str, variables: serde_json_bytes::Value) -> () { - let res = run_validation_enforce_mode( + let res = run_validation( with_supergraph_boilerplate(schema, "Query"), query, variables, + Mode::Enforce, ); assert!(res.is_ok(), "validation should have succeeded: {:?}", res); } fn assert_validation_error(schema: &str, query: &str, variables: serde_json_bytes::Value) -> () { - let res = run_validation_enforce_mode( + let res = run_validation( with_supergraph_boilerplate(schema, "Query"), query, variables, + Mode::Enforce, ); assert!(res.is_err(), "validation should have failed"); } @@ -3034,155 +3010,75 @@ fn variable_validation_enforce_mode() { } "#; - let res = run_validation_enforce_mode( + let res = run_validation( schema.to_string(), "mutation foo($input: FooInput!) { foo (input: $input) { __typename }}", json!({"input":{}}), + Mode::Enforce, ); assert!(res.is_ok(), "validation should have succeeded: {res:?}"); } #[test] -fn variable_validation_warn_mode() { - // Tests validation of variable fields - let res = run_validation_warn_mode( - with_supergraph_boilerplate( - "input MessageInput { - content: String - author: String - } - type Receipt { - id: ID! - } - type Query{ - send(message: MessageInput): Receipt}", - "Query", - ), - "query($msg: MessageInput) { - send(message: $msg) { - id - }}", - json!({"msg": { - "content": "Hello", - "author": "Me", - "unknownField": "unknown", - }}), - ); - assert!( - res.is_ok(), - "validation should have warned rather than failed" - ); - - // Tests if nested unknown input fields are caught - let res = run_validation_warn_mode( - with_supergraph_boilerplate( - "input MessageInput { +#[rstest::rstest] +#[case::top_level_unexpected_field( + json!({"content": "Hello", "canvas": [], "unknownField": "unknown"}), + Ok(()) +)] +#[case::nested_unexpected_field( + json!({"canvas": [{"input": 3}, {"input": 5, "unknownField": "unknown"}]}), + Ok(()) +)] +#[case::top_level_missing_field( + json!({}), + Err("VALIDATION_INVALID_TYPE_VARIABLE") +)] +#[case::nested_missing_field( + json!({"canvas": [{"unknownField": 3}, {"input": 4}]}), + Err("VALIDATION_INVALID_TYPE_VARIABLE") +)] +fn variable_validation_measure_mode( + #[case] msg_variables: Value, + #[case] expected_result: Result<(), &str>, +) { + let schema = " + input MessageInput { content: String - author: String - canvas: [CanvasInput] - } + canvas: [CanvasInput]! + } input CanvasInput { - input: Int - } - type Receipt { - id: ID! - } - type Query{ - send(message: MessageInput): Receipt}", - "Query", - ), - "query($msg: MessageInput) { - send(message: $msg) { - id - }}", - json!({"msg": { - "content": "Hello", - "author": "Me", - "canvas": [ - {"input": 3}, - {"input": 4}, - {"input": 5, "unknownField": "unknown"} - ], - }}), - ); - assert!( - res.is_ok(), - "validation should have warned rather than failed" - ); + input: Int! + } + type Query { + send(message: MessageInput): ID + }"; - // Tests if misspelled field names are caught - let res = run_validation_warn_mode( - with_supergraph_boilerplate( - " - input MessageInput { - content: String - author: String - canvas: [CanvasInput] - } - input CanvasInput { - input: Int! - } - type Receipt { - id: ID! - } - type Query { - send(message: MessageInput): Receipt - } - ", - "Query", - ), - "query($msg: MessageInput) { - send(message: $msg) { - id - } - }", - json!({"msg": { - "content": "Hello", - "author": "Me", - "canvas": [{"innput": 4}], - }}), + // Tests validation of variable fields + let result = run_validation( + with_supergraph_boilerplate(schema, "Query"), + "query($msg: MessageInput) { send(message: $msg) }", + json!({"msg": msg_variables}), + Mode::Measure, ); - assert!(res.is_err(), "validation should have failed"); - // Tests if a misspelled field is caught even when the correct field is present - let res = run_validation_warn_mode( - with_supergraph_boilerplate( - " - input MessageInput { - content: String - author: String - canvas: [CanvasInput] - } - input CanvasInput { - input: Int! - } - type Receipt { - id: ID! - } - type Query { - send(message: MessageInput): Receipt - } - ", - "Query", - ), - "query($msg: MessageInput) { - send(message: $msg) { - id - } - }", - json!({"msg": { - "content": "Hello", - "author": "Me", - "canvas": [{"input": 3, "innput": 4}], - }}), - ); - assert!( - res.is_ok(), - "validation should have warned rather than failed" - ); + match (result, expected_result) { + (Ok(()), Ok(())) => {} + (Err(response), Err(expected_code)) => { + assert!( + response.contains_error_code(expected_code), + "response = {response:?}" + ); + } + (Err(response), Ok(())) => { + panic!("expected validation to pass: response = {response:?}"); + } + (Ok(()), Err(code)) => { + panic!("expected validation to fail with code {code}"); + } + } } #[test] From 0c39399c5496ccaa5a2d709a91c3d140e6816168 Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:29:31 -0500 Subject: [PATCH 15/18] chore: fix linter --- apollo-router/src/spec/query/tests.rs | 5 +++-- apollo-router/tests/integration/operation_limits.rs | 5 ++--- apollo-router/tests/integration/validation.rs | 7 ++----- 3 files changed, 7 insertions(+), 10 deletions(-) diff --git a/apollo-router/src/spec/query/tests.rs b/apollo-router/src/spec/query/tests.rs index 565e481920..9d328ab9d5 100644 --- a/apollo-router/src/spec/query/tests.rs +++ b/apollo-router/src/spec/query/tests.rs @@ -2515,6 +2515,7 @@ fn reformat_response_unknown_typename() { .test(); } +#[allow(clippy::result_large_err)] fn run_validation( schema: String, query: &str, @@ -2543,7 +2544,7 @@ fn run_validation( query.validate_variables(&request, &schema, mode) } -fn assert_validation(schema: &str, query: &str, variables: serde_json_bytes::Value) -> () { +fn assert_validation(schema: &str, query: &str, variables: serde_json_bytes::Value) { let res = run_validation( with_supergraph_boilerplate(schema, "Query"), query, @@ -2553,7 +2554,7 @@ fn assert_validation(schema: &str, query: &str, variables: serde_json_bytes::Val assert!(res.is_ok(), "validation should have succeeded: {:?}", res); } -fn assert_validation_error(schema: &str, query: &str, variables: serde_json_bytes::Value) -> () { +fn assert_validation_error(schema: &str, query: &str, variables: serde_json_bytes::Value) { let res = run_validation( with_supergraph_boilerplate(schema, "Query"), query, diff --git a/apollo-router/tests/integration/operation_limits.rs b/apollo-router/tests/integration/operation_limits.rs index 818c80f8bf..1a494fffdd 100644 --- a/apollo-router/tests/integration/operation_limits.rs +++ b/apollo-router/tests/integration/operation_limits.rs @@ -7,7 +7,6 @@ use apollo_router::TestHarness; use apollo_router::graphql; use apollo_router::services::execution; use apollo_router::services::supergraph; -use serde_json::Value; use serde_json::json; use tower::BoxError; use tower::ServiceExt; @@ -304,7 +303,7 @@ limits: .build(); let (_, response) = router.execute_query(request.clone()).await; - let body: Value = response.json().await.unwrap(); + let body: serde_json::Value = response.json().await.unwrap(); assert!( body.get("errors").is_none(), "expected no errors with warn_only, got: {body:?}" @@ -315,7 +314,7 @@ limits: router.assert_reloaded().await; let (_, response) = router.execute_query(request).await; - let body: Value = response.json().await.unwrap(); + let body: serde_json::Value = response.json().await.unwrap(); let errors = body .get("errors") diff --git a/apollo-router/tests/integration/validation.rs b/apollo-router/tests/integration/validation.rs index 571c215234..9c58510540 100644 --- a/apollo-router/tests/integration/validation.rs +++ b/apollo-router/tests/integration/validation.rs @@ -223,10 +223,7 @@ async fn variable_validation_mode_propagates_fully( #[case] logs_should_contain_warning: bool, ) { let mut settings = insta::Settings::clone_current(); - settings.set_snapshot_suffix(format!( - "{}", - strict_variable_validation.unwrap_or("missing") - )); + settings.set_snapshot_suffix(strict_variable_validation.unwrap_or("missing")); settings.set_sort_maps(true); let _guard = settings.bind_to_scope(); @@ -236,7 +233,7 @@ async fn variable_validation_mode_propagates_fully( } let mut router = IntegrationTest::builder() - .config(&serde_yaml::to_string(&config).unwrap()) + .config(serde_yaml::to_string(&config).unwrap()) .supergraph(PathBuf::from( "tests/fixtures/supergraph_input_variables.graphql", )) From 2dbc80b1c34f5f676a0fb7edcce437fc0eafe646 Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:38:21 -0500 Subject: [PATCH 16/18] chore: docs to align with style guide --- docs/source/routing/configuration/yaml.mdx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/source/routing/configuration/yaml.mdx b/docs/source/routing/configuration/yaml.mdx index 6843101005..7947a9e923 100644 --- a/docs/source/routing/configuration/yaml.mdx +++ b/docs/source/routing/configuration/yaml.mdx @@ -476,23 +476,23 @@ traffic_shaping: By default, the router compresses subgraph requests by generating fragment definitions based on the shape of the subgraph operation. In many cases this significantly reduces the size of the query sent to subgraphs. -You can explicitly opt out of this behavior by specifying `supergraph.generate_query_fragments`: +Opt out of this behavior by specifying `supergraph.generate_query_fragments`: ```yaml supergraph: generate_query_fragments: false ``` -#### Variable Validation Modes +#### Variable validation modes -By default, the router validates input variables strictly, meaning that each input object value will be validated against its type definition and any unknown fields will result in a request error. +By default, the router validates input variables strictly. It validates each input object value against its type definition, and any unknown fields result in a request error. ```yaml supergraph: strict_variable_validation: enforce ``` -If your implementation requires the use of unknown fields on a defined type, you can opt out of stricter validation by specifying `strict_variable_validation: measure`. +If your implementation requires unknown fields on a defined type, you can opt out of stricter validation by specifying `strict_variable_validation: measure`. In this case, the router will not error when encountering unknown fields, but will log the field for reference. --- From afe9d3d070876c227b434ddf24653f6b7de97042 Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Tue, 17 Feb 2026 10:38:58 -0500 Subject: [PATCH 17/18] chore: document modes in code --- apollo-router/src/configuration/mod.rs | 4 +++- ...ter__configuration__tests__schema_generation.snap | 12 +++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index b81b728b4a..5404c704c2 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -745,7 +745,9 @@ pub(crate) struct Supergraph { /// Default: false. pub(crate) experimental_log_on_broken_pipe: bool, - /// TODO(@caroline) docs + /// Determines how to handle queries which include additional fields of an input object. + /// - `enforce` (default): rejects query + /// - `measure`: permits query and the logs unknown fields #[serde(default = "default_strict_variable_validation")] pub(crate) strict_variable_validation: Mode, } diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index c53e67fd17..7642d314c0 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -10788,6 +10788,15 @@ expression: "&schema" "warmed_up_queries": null }, "description": "Query planning options" + }, + "strict_variable_validation": { + "allOf": [ + { + "$ref": "#/definitions/Mode" + } + ], + "default": "enforce", + "description": "Determines how to handle queries which include additional fields of an input object.\n- `enforce` (default): rejects query\n- `measure`: permits query and the logs unknown fields" } }, "type": "object" @@ -12343,7 +12352,8 @@ expression: "&schema" "experimental_plans_limit": null, "experimental_reuse_query_plans": false, "warmed_up_queries": null - } + }, + "strict_variable_validation": "enforce" }, "description": "Configuration for the supergraph" }, From acd87c03c4db3440c9318afd978b33d67552bced Mon Sep 17 00:00:00 2001 From: carodewig <16093297+carodewig@users.noreply.github.com> Date: Tue, 17 Feb 2026 11:02:28 -0500 Subject: [PATCH 18/18] chore: remove unnecessary default default is specified in the new fn --- apollo-router/src/configuration/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 5404c704c2..836b64c5de 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -748,7 +748,6 @@ pub(crate) struct Supergraph { /// Determines how to handle queries which include additional fields of an input object. /// - `enforce` (default): rejects query /// - `measure`: permits query and the logs unknown fields - #[serde(default = "default_strict_variable_validation")] pub(crate) strict_variable_validation: Mode, }