From 2ec02fd57810538b00575f3b0d80cb72b13c0dc1 Mon Sep 17 00:00:00 2001 From: "glm-5.3-flash" Date: Mon, 31 Aug 2026 07:18:53 +0000 Subject: [PATCH] feat(adapters): enforce advertised input schemas at call time (OAI-18, option a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Decision: advertise == enforce. The key allowlist (OAI-02) stays as the first gate with its established unknown-key message; a compiled leaf validator now runs second, so required/type/enum/pattern/bounds violations surface as INVALID_INPUT 422 naming the keyword — not as upstream round-trips. - new src/adapters/input_validation.rs: CompiledInputSchema compiles an op's input_schema once at import with the jsonschema crate (same 2020-12 dialect publish_schema uses) and validates peer input at call time; the compile-time copy is hardened closed-by-default (additionalProperties: false injected when absent) so the validator reproduces the allowlist's unknown-key semantics; explicit additionalProperties:true catch-all and schema values are preserved; the original spec value is never mutated - from_openapi/from_jsonschema import(): compile per registration, capture the validator in the handler closure (re-import recompiles — the closure capture is the invalidation story); a non-compilable input schema fails import loudly (AdapterError::SchemaParse naming the operation), matching the publish_schema fail-closed precedent - from_openapi generated input schemas now carry additionalProperties:false explicitly, so the /schema advert states the enforced rule and external schema-driven validators reach the same verdicts - forward/forward_stream/build_request gain an Option<&CompiledInputSchema> parameter; enforcement runs after the allowlist - round-trip test (review 002 Test-gap 10): the /schema-exported input_schema is compiled with the same validator and driven against build_request over a 10-input violation matrix — accept-sets exactly equal in both directions; the chain-test that lets advertise/enforce drift surface as a CI failure - ADR-066: new decision section (advertise==enforce) with the trust- boundary reasoning and the rejected option (b) rationale - module + enforce_input_schema docs updated to the two-gate shape cargo test --all-features 596 pass; clippy --all-features/-D warnings, fmt, doc gates clean. docs(tasks): mark review-002-fu-oai18-decision done --- .../066-from-jsonschema-as-http-adapter.md | 67 +++++ src/adapters/forward.rs | 216 ++++++++++++++-- src/adapters/from_jsonschema.rs | 50 +++- src/adapters/from_openapi.rs | 121 ++++++++- src/adapters/input_validation.rs | 243 ++++++++++++++++++ src/adapters/mod.rs | 1 + .../adapters/review-002-fu-oai18-decision.md | 2 +- 7 files changed, 678 insertions(+), 22 deletions(-) create mode 100644 src/adapters/input_validation.rs diff --git a/docs/architecture/decisions/066-from-jsonschema-as-http-adapter.md b/docs/architecture/decisions/066-from-jsonschema-as-http-adapter.md index 5ed3a79..7fc3007 100644 --- a/docs/architecture/decisions/066-from-jsonschema-as-http-adapter.md +++ b/docs/architecture/decisions/066-from-jsonschema-as-http-adapter.md @@ -149,6 +149,73 @@ error projection and the success sweep (SSE detection + output-schema selection), where the precedence order is: concrete 2XX statuses, then `2XX`, then `default`. +### Input-schema enforcement: advertise == enforce (review 002 OAI-18) + +**Decided: enforce the full input schema at call time.** Until this +decision, `build_request` enforced a **key allowlist** only (review 001 +OAI-02: unknown keys rejected), while `/schema` advertised the full +JSON Schema — `required`, `enum`, `pattern`, value types, bounds. The +advertised contract was broader than the defended one: a +`required: [id]` operation accepted `{}` and let the upstream surface +the violation as a remote 422; a `{"type": "string"}` property sent as +an object serialized as JSON text into the query string; +`enum`/`pattern`/`minimum` were never consulted. This is the same +advertise/enforce drift class review 002 filed as +[minor→major-class], and OAI-02's own rationale decides the direction: +peer **input** is never trusted, and the advertised schema is exactly +what a peer reads before crafting input. ADR-066's trust boundary +(specs are trusted *configuration*) answers whether the assembly's +schema must be defended against; it does not make peer input trusted, +so "the spec is configuration" does not justify enforcement-by-allowlist. +Scoping the advert instead (the rejected alternative) would have to +strip `required`/`enum`/`pattern` from **two** projection surfaces +(`to_openapi` and `to_mcp`) and degrades the contract for well-behaved +consumers. + +Concretely: + +- Each adapter's `import()` compiles the op's `input_schema` **once** + with the `jsonschema` crate (the same compiler and 2020-12 dialect + `PublishSchemaCache` already uses for `publish_schema`) and captures + the validator in the handler closure — compile once per registration, + no runtime cache-invalidation scheme needed, mirroring the + value-keyed cache only in spirit (the closure capture *is* the + invalidation story: re-import recompiles). +- The compile step **hardens** the schema copy it compiles: when + `additionalProperties` is absent, `additionalProperties: false` is + injected into the compiled copy, so the validator reproduces the + OAI-02 closed-by-default semantics. The spec value itself is never + mutated. +- `from_openapi`'s *generated* input schemas now state the rule in the + advert itself: every generated schema carries + `additionalProperties: false` explicitly, so a schema-driven external + consumer reaches the same verdicts the gateway enforces (pinned by + the advertised-vs-enforced round-trip test). +- The allowlist check runs **first** and is unchanged (its message + names the undeclared key and the declared set — better diagnosis + than the validator's generic additionalProperties error); the + compiled validator runs second. Unknown-key rejection therefore + keeps the established message, and leaf violations surface as + `INVALID_INPUT` 422 naming the violated keyword and input location + (ADR-023 shape) — the gateway rejects, never an upstream round-trip. +- The explicit `"additionalProperties": true` opt-in catch-all + (documented catch-all for open-shaped endpoints) is preserved + verbatim by the compiler; declared keys' constraints still bind + under it. +- An input schema that fails to compile fails **import** loudly + (`AdapterError::SchemaParse` naming the operation), matching the + `publish_schema` fail-closed precedent: an un-validatable input + contract must never register as an operation whose enforcement + silently degrades. + +Interaction with the gateway marker extensions: the +`HEADER_PARAM_IN_MARKER`-decorated properties and the `body` property +are peer-visible schema extensions that ride inside `properties`, so +the compiled validator treats them as ordinary properties (validated +against their embedded schema; the marker key itself is not a JSON +Schema keyword and is ignored by the compiler). The catch-all +semantics above keep open-shaped endpoints working. + ### Forwarding contract decisions (review 002 FWD-17/18/19) The shared forwarding core (`src/adapters/forward.rs`) used by both diff --git a/src/adapters/forward.rs b/src/adapters/forward.rs index a2584e1..9e7f176 100644 --- a/src/adapters/forward.rs +++ b/src/adapters/forward.rs @@ -77,13 +77,19 @@ //! missing key rather than sent unauthenticated to produce corrupted //! upstream 401s with no local diagnostic. //! -//! Input-schema enforcement (review 001 OAI-02): every input key must be -//! declared by the operation's `input_schema` or consumed by a path -//! template placeholder; undeclared keys are rejected with `INVALID_INPUT` -//! at call time. There is no pass-through knob — a request the forwarder -//! sends must match what `/schema` advertises, so peer-supplied input -//! cannot add upstream query parameters, headers, or craft a request body -//! the contract does not declare. Declared keys route by designation: a +//! Input-schema enforcement (review 001 OAI-02, review 002 OAI-18): +//! two call-time gates. The key allowlist rejects undeclared keys with +//! `INVALID_INPUT` — every input key must be declared by the operation's +//! `input_schema` or consumed by a path template placeholder, so +//! peer-supplied input cannot add upstream query parameters, headers, or +//! craft a request body the contract does not declare (no pass-through +//! knob). The compiled leaf validator (captured per registration at +//! `import()`, `CompiledInputSchema`) then enforces the schema the +//! gateway advertises — `required`, value types, `enum`, `pattern`, +//! bounds — as `INVALID_INPUT` naming the keyword, so advertise == +//! enforce and violations fail here instead of as upstream 422s; +//! non-compilable input schemas fail import loudly. Declared keys route +//! by designation: a //! property marked with the `HEADER_PARAM_IN_MARKER` marker key set to "header" is sent as a //! request header, the declared `GATEWAY_BODY_KEY` property becomes the //! request body, and every other declared key becomes an upstream query @@ -134,6 +140,7 @@ use reqwest::Method; use serde_json::Value; use url::Url; +use crate::adapters::input_validation::CompiledInputSchema; use crate::client::SharedHttpClient; /// Maximum size, in bytes, of a buffered upstream response body on any @@ -289,6 +296,7 @@ pub(crate) fn build_request( default_headers: &HashMap, namespace: &str, input_schema: &Value, + input_validator: Option<&CompiledInputSchema>, input: &Value, context: &OperationContext, ) -> Result<(Method, Url, Option, HeaderMap), CallError> { @@ -300,6 +308,9 @@ pub(crate) fn build_request( })?; enforce_input_schema(input_schema, inputs)?; + if let Some(validator) = input_validator { + validator.validate(input)?; + } let mut query_params: Vec<(String, String)> = Vec::new(); let mut header_params: Vec<(String, String)> = Vec::new(); @@ -452,19 +463,35 @@ fn type_name_of(value: &Value) -> &'static str { } } -/// Input-schema enforcement at call time (review 001 OAI-02). +/// Input-schema enforcement at call time (review 001 OAI-02, review +/// 002 OAI-18). /// -/// Every input key must be declared by the input schema's `properties` — -/// including the gateway body property, which `from_openapi` declares as -/// `body` whenever the operation has a requestBody, and including the -/// path-consumed placeholders, which `from_openapi` also declares. An -/// explicit `"additionalProperties": true` opts the operation into -/// catch-all input (JSON Schema semantics: the schema advertises that -/// extra properties are valid), which `from_jsonschema` callers can use -/// for open-shaped endpoints. Anything else undeclared is a rejected -/// `INVALID_INPUT` rather than a silently-added upstream query parameter, -/// so peer input like `{debug: true}` or `{impersonate_id: …}` cannot -/// decorate an upstream request the contract does not advertise. +/// Two gates run in order: +/// +/// 1. The **key allowlist** (this function): every input key must be +/// declared by the input schema's `properties` — including the +/// gateway body property, which `from_openapi` declares as `body` +/// whenever the operation has a requestBody, and including the +/// path-consumed placeholders, which `from_openapi` also declares. +/// An explicit `"additionalProperties": true` opts the operation +/// into catch-all input (JSON Schema semantics: the schema +/// advertises that extra properties are valid), which +/// `from_jsonschema` callers can use for open-shaped endpoints. +/// Anything else undeclared is a rejected `INVALID_INPUT` naming the +/// key and the declared set. +/// 2. The **compiled leaf validator** (`CompiledInputSchema`, OAI-18): +/// the input value is validated against the `input_schema` the +/// adapter compiled at import (hardened closed-by-default), so +/// `required`, value types, `enum`, `pattern`, and bounds are +/// enforced exactly as `/schema` advertises — violations are +/// `INVALID_INPUT` naming the keyword, never an upstream round-trip. +/// Adapters capture the validator at `import()`; a schema that +/// cannot compile fails import loudly (fail-closed). +/// +/// The allowlist-first ordering keeps the OAI-02 unknown-key message +/// (better diagnosis than the validator's generic +/// additionalProperties error); leaf enforcement then closes the +/// advertise/enforce drift OAI-18 filed. fn enforce_input_schema( input_schema: &Value, inputs: &serde_json::Map, @@ -931,6 +958,7 @@ pub(crate) async fn forward( namespace: &str, input_schema: &Value, error_status_codes: &[(u16, String)], + input_validator: Option<&CompiledInputSchema>, input: Value, context: OperationContext, ) -> ResponseEnvelope { @@ -944,6 +972,7 @@ pub(crate) async fn forward( default_headers, namespace, input_schema, + input_validator, &input, &context, ) { @@ -1067,6 +1096,7 @@ pub(crate) fn forward_stream( namespace: &str, input_schema: &Value, error_status_codes: &[(u16, String)], + input_validator: Option<&CompiledInputSchema>, input: Value, context: OperationContext, ) -> ResponseStream { @@ -1080,6 +1110,7 @@ pub(crate) fn forward_stream( default_headers, namespace, input_schema, + input_validator, &input, &context, ) { @@ -1484,6 +1515,7 @@ mod tests { &TestHashMap::new(), "svc", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &input, &ctx, )?; @@ -1647,6 +1679,7 @@ mod tests { &TestHashMap::new(), "svc", &schema, + None, &input, &ctx, ) @@ -1656,6 +1689,127 @@ mod tests { } } + /// OAI-18 (enforce leg): leaf constraints the compiled validator + /// enforces — `required`, value types, `enum`, `minimum` — reject + /// with `INVALID_INPUT` naming the violated keyword, so the gateway + /// rejects before an upstream round-trip would surface the + /// violation as a remote 422/400. + #[test] + fn compiled_input_schema_enforces_leaf_constraints_as_invalid_input() { + let ctx = noop_context(); + let schema = json!({ + "type": "object", + "properties": { + "id": {"type": "string"}, + "level": {"enum": ["low", "high"]}, + "n": {"minimum": 1}, + "tag": {"pattern": "^[a-z]+$"}, + }, + "required": ["id"], + }); + let validator = Some(CompiledInputSchema::compile(&schema).expect("schema compiles")); + for (input, keyword) in [ + (json!({}), "required"), + (json!({"id": 7}), "type"), + (json!({"id": "x", "level": "medium"}), "enum"), + (json!({"id": "x", "n": 0}), "minimum"), + (json!({"id": "x", "tag": "UPPER"}), "pattern"), + ] { + let err = build_request( + "https://api.example.com", + "/x", + "GET", + &None, + &TestHashMap::new(), + "svc", + &schema, + validator.as_ref(), + &input, + &ctx, + ) + .expect_err("violated leaf constraint must be rejected"); + assert_eq!(err.code, "INVALID_INPUT", "input was: {input}"); + assert!( + err.message.contains(&format!("[keyword: {keyword}")), + "input {input} must name the violated keyword `{keyword}`: {}", + err.message + ); + } + } + + /// OAI-18: absent the compiled validator (`None`, the shape direct + /// `build_request` callers may still use), enforcement is exactly + /// the key allowlist — the schema's leaf constraints are not + /// consulted. This pins the split so a future refactor cannot + /// silently make leaf enforcement depend on the parameter. + #[test] + fn without_a_compiled_validator_enforcement_stays_allowlist_only() { + let ctx = noop_context(); + let schema = json!({ + "type": "object", + "properties": {"id": {"type": "string"}}, + "required": ["id"], + }); + let (_, url, _, _) = build_request( + "https://api.example.com", + "/x", + "GET", + &None, + &TestHashMap::new(), + "svc", + &schema, + None, + &json!({}), + &ctx, + ) + .expect("allowlist-only enforcement accepts the empty input"); + assert_eq!(url.path(), "/x"); + } + + /// OAI-18: the explicit `additionalProperties: true` catch-all is + /// preserved by the compiled validator (the documented opt-in), and + /// the validator's schema for declared keys still binds: a + /// `{"type": "string"}` property sent as an object is rejected even + /// under the catch-all. + #[test] + fn catch_all_opt_in_keeps_working_under_the_compiled_validator() { + let ctx = noop_context(); + let schema = json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "additionalProperties": true, + }); + let validator = Some(CompiledInputSchema::compile(&schema).expect("schema compiles")); + build_request( + "https://api.example.com", + "/search", + "GET", + &None, + &TestHashMap::new(), + "svc", + &schema, + validator.as_ref(), + &json!({"debug": true}), + &ctx, + ) + .expect("the catch-all still accepts undeclared keys"); + let err = build_request( + "https://api.example.com", + "/search", + "GET", + &None, + &TestHashMap::new(), + "svc", + &schema, + validator.as_ref(), + &json!({"q": {"object": "value"}}), + &ctx, + ) + .expect_err("a declared property's type still binds under the catch-all"); + assert_eq!(err.code, "INVALID_INPUT"); + assert!(err.message.contains("[keyword: type"), "{}", err.message); + } + #[test] fn declared_body_and_header_params_route_off_the_query_string() { let ctx = noop_context(); @@ -1675,6 +1829,7 @@ mod tests { &TestHashMap::new(), "svc", &schema, + None, &json!({"q": "rust", "X-Trace-Id": "t-1", "body": {"page": 2}}), &ctx, ) @@ -1704,6 +1859,7 @@ mod tests { &TestHashMap::new(), "svc", &schema, + None, &input, &ctx, ) @@ -1728,6 +1884,7 @@ mod tests { &TestHashMap::new(), "svc", &schema, + None, &json!({"q": "rust", "debug": "true"}), &ctx, ) @@ -1771,6 +1928,7 @@ mod tests { &TestHashMap::new(), "svc", &schema, + None, &json!({"id": "x", "undeclared": "v"}), &ctx, ) @@ -2191,6 +2349,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), ctx, ) @@ -2274,6 +2433,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -2313,6 +2473,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -2347,6 +2508,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -2434,6 +2596,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -2516,6 +2679,7 @@ mod tests { &defaults, "svc", &serde_json::json!({"type": "object"}), + None, &json!({}), &noop_context(), ) @@ -2537,6 +2701,7 @@ mod tests { &TestHashMap::new(), "svc", &serde_json::json!({"type": "object"}), + None, &json!({}), &ctx, ); @@ -2570,6 +2735,7 @@ mod tests { &TestHashMap::new(), "svc", &serde_json::json!({"type": "object"}), + None, &json!({}), &noop_context(), ) @@ -2607,6 +2773,7 @@ mod tests { &TestHashMap::new(), "svc", &serde_json::json!({"type": "object"}), + None, &json!({}), &noop_context(), ) @@ -2664,6 +2831,7 @@ mod tests { &defaults, "svc", &serde_json::json!({"type": "object"}), + None, &json!({}), &noop_context(), ) @@ -2811,6 +2979,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -2869,6 +3038,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -3009,6 +3179,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -3061,6 +3232,7 @@ mod tests { "properties": {"id": {"type": "string"}} }), &[], + None, input, noop_context(), ); @@ -3104,6 +3276,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -3162,6 +3335,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -3189,6 +3363,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ) @@ -3221,6 +3396,7 @@ mod tests { "svc", &serde_json::json!({"type": "object"}), &[], + None, json!({}), noop_context(), ); @@ -3327,6 +3503,7 @@ mod tests { &TestHashMap::new(), "svc", &schema, + None, &json!({"X-Trace": "bad\u{0000}value"}), &ctx, ) @@ -3356,6 +3533,7 @@ mod tests { &TestHashMap::new(), "svc", &schema, + None, &json!({"bad header": "v"}), &ctx, ) diff --git a/src/adapters/from_jsonschema.rs b/src/adapters/from_jsonschema.rs index 208a007..7686e46 100644 --- a/src/adapters/from_jsonschema.rs +++ b/src/adapters/from_jsonschema.rs @@ -28,6 +28,8 @@ use alkcall::registry::registration::{ make_handler, make_streaming_handler, HandlerKind, HandlerRegistration, OperationProvenance, }; use alkcall::registry::spec::{OperationSpec, OperationType, Visibility}; + +use crate::adapters::input_validation::CompiledInputSchema; use async_trait::async_trait; use reqwest::Method; use serde_json::Value; @@ -163,6 +165,16 @@ fn spec_name_references_undeclared(spec: &OperationSpec, path_template: &str) -> #[async_trait] impl OperationAdapter for FromJsonSchema { async fn import(&self) -> Result, AdapterError> { + let input_validator = + CompiledInputSchema::compile(&self.spec.input_schema).map_err(|message| { + AdapterError::SchemaParse { + message: format!( + "operation `{}`: {message} (review 002 OAI-18: an un-validatable input \ + contract must fail import, not register unenforced)", + self.spec.name + ), + } + })?; let path_template = self.path_template.clone(); let method_upper = self.method.to_ascii_uppercase(); let auth_scheme = self.config.auth.clone(); @@ -192,6 +204,7 @@ impl OperationAdapter for FromJsonSchema { let http_client = Arc::clone(&http_client); let error_status_codes = error_status_codes.clone(); let input_schema = input_schema.clone(); + let input_validator = input_validator.clone(); forward_stream( &http_client, &base_url, @@ -202,6 +215,7 @@ impl OperationAdapter for FromJsonSchema { &namespace, &input_schema, &error_status_codes, + Some(&input_validator), input, context, ) @@ -218,6 +232,7 @@ impl OperationAdapter for FromJsonSchema { let http_client = Arc::clone(&http_client); let error_status_codes = error_status_codes.clone(); let input_schema = input_schema.clone(); + let input_validator = input_validator.clone(); async move { forward( &http_client, @@ -229,6 +244,7 @@ impl OperationAdapter for FromJsonSchema { &namespace, &input_schema, &error_status_codes, + Some(&input_validator), input, context, ) @@ -417,6 +433,7 @@ mod tests { &HashMap::new(), "github", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &serde_json::json!({"owner":"a","repo":"b"}), &ctx, ) @@ -439,6 +456,7 @@ mod tests { &HashMap::new(), "svc", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &serde_json::json!({"id":42,"filter":"active"}), &ctx, ) @@ -608,7 +626,7 @@ mod tests { ), ( serde_json::json!({"id": {"nested": "object"}}), - "must be a scalar", + "[keyword: type", ), ] { let base = spawn_echo_server(200, "data: {\"n\":1}\n\n", "text/event-stream").await; @@ -660,6 +678,7 @@ mod tests { &HashMap::new(), "openai", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &serde_json::json!({"body":{"prompt":"hi"}}), &ctx, ) @@ -678,6 +697,35 @@ mod tests { std::env::remove_var("OPENAI_API_KEY"); } + /// OAI-18 fail-closed precedent: an input schema that cannot + /// compile fails *import* loudly — never a registration whose + /// call-time enforcement silently degrades. + #[tokio::test] + async fn uncompilable_input_schema_fails_import_loudly() { + let mut spec = test_spec("svc/broken", OperationType::Query); + spec.input_schema = serde_json::json!({"type": "object", "required": "n"}); + let adapter = FromJsonSchema::new( + spec, + test_config("svc", "https://api.example.com"), + "/x".to_string(), + "GET".to_string(), + test_http_client(), + ) + .unwrap(); + let result = adapter.import().await; + match result { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("failed to compile"), + "message was: {message}" + ); + assert!(message.contains("svc/broken"), "message was: {message}"); + } + Ok(_) => panic!("an un-validatable input contract must fail import (OAI-18)"), + Err(e) => panic!("expected SchemaParse, got {e:?}"), + } + } + #[test] fn malformed_method_path_template_and_base_url_fail_at_construction() { for (method, path_template, base_url, expected_fragment) in [ diff --git a/src/adapters/from_openapi.rs b/src/adapters/from_openapi.rs index c6b0176..cda4a2e 100644 --- a/src/adapters/from_openapi.rs +++ b/src/adapters/from_openapi.rs @@ -33,6 +33,7 @@ use super::forward::{ use super::openapi_spec::{ collect_ignored_schema_keys, OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS, }; +use crate::adapters::input_validation::CompiledInputSchema; use crate::client::SharedHttpClient; fn unbound_placeholders(path_template: &str, input_schema: &Value) -> Vec { @@ -239,13 +240,21 @@ impl FromOpenAPI { } if properties.is_empty() { - return Ok(serde_json::json!({"type": "object"})); + return Ok(serde_json::json!({ + "type": "object", + "additionalProperties": false, + })); } + // OAI-18: the generated input schema advertises closed-by-default + // object shape explicitly (`additionalProperties: false`), so the + // /schema advert states the rule call time enforces and a + // schema-driven external validator reaches the same verdicts. Ok(serde_json::json!({ "type": "object", "properties": properties, "required": required, + "additionalProperties": false, })) } @@ -413,6 +422,15 @@ impl FromOpenAPI { AccessControl::default(), None, ); + let input_validator = CompiledInputSchema::compile(&input_schema).map_err(|message| { + AdapterError::SchemaParse { + message: format!( + "operation `{}`: {message} (review 002 OAI-18: an un-validatable \ + input contract must fail import, not register unenforced)", + spec.name + ), + } + })?; let path_template = path.to_string(); let method_upper = method.to_ascii_uppercase(); let auth_scheme = self.config.auth.clone(); @@ -464,6 +482,7 @@ impl FromOpenAPI { let http_client = Arc::clone(&http_client); let error_status_codes = error_status_codes.clone(); let input_schema = input_schema.clone(); + let input_validator = input_validator.clone(); forward_stream( &http_client, &base_url, @@ -474,6 +493,7 @@ impl FromOpenAPI { &namespace, &input_schema, &error_status_codes, + Some(&input_validator), input, context, ) @@ -490,6 +510,7 @@ impl FromOpenAPI { let http_client = Arc::clone(&http_client); let error_status_codes = error_status_codes.clone(); let input_schema = input_schema.clone(); + let input_validator = input_validator.clone(); async move { forward( &http_client, @@ -501,6 +522,7 @@ impl FromOpenAPI { &namespace, &input_schema, &error_status_codes, + Some(&input_validator), input, context, ) @@ -553,13 +575,16 @@ impl OperationAdapter for FromOpenAPI { mod tests { use super::*; use crate::adapters::forward::{build_request, HttpAuthScheme}; + use crate::adapters::input_validation::CompiledInputSchema; use crate::client::HttpClientConfig; + use alkcall::core::types::Capabilities; use alkcall::protocol::wire::ResponseEnvelope; use alkcall::registry::context::AbortPolicy; use alkcall::registry::env::OperationEnv; use futures::StreamExt; use reqwest::header::AUTHORIZATION; use reqwest::Method; + use serde_json::json; use std::collections::HashMap; use std::time::Duration; use tokio::io::{AsyncReadExt, AsyncWriteExt}; @@ -640,6 +665,94 @@ mod tests { }"# } + /// OAI-18 advertised-vs-enforced round-trip (review 002 Test-gap 10): + /// the chain-test that lets advertise/enforce drift surface as a + /// CI failure instead of surviving reviews. A doc is imported; the + /// `/schema`-exported `input_schema` (the exact value + /// `OperationSpec::input_schema` carries, projected verbatim by + /// `spec_to_json`) is compiled with the same runtime validator and + /// driven over a matrix of violating inputs; every input the + /// exported schema accepts must be accepted by `build_request`, and + /// every input it rejects must be rejected as `INVALID_INPUT` — + /// accept-sets exactly equal, both directions. + #[tokio::test] + async fn advertised_input_schema_accepts_exactly_what_build_request_accepts() { + let doc = r#"{ + "openapi": "3.0.0", + "info": { "title": "T", "version": "1" }, + "paths": { + "/items": { + "get": { + "operationId": "listItems", + "parameters": [ + {"name": "q", "in": "query", "schema": {"type": "string"}}, + {"name": "n", "in": "query", "schema": {"type": "integer", "minimum": 1}, + "required": true}, + {"name": "level", "in": "query", + "schema": {"type": "string", "enum": ["low", "high"]}} + ], + "responses": { + "200": {"content": {"application/json": {"schema": {}}}} + } + } + } + } + }"#; + let spec = OpenAPISpec::from_json(doc).unwrap(); + let imported = adapter(spec, config("items", "https://api.example.com", None)); + let bundles = imported.import().await.unwrap(); + assert_eq!(bundles.len(), 1); + let advertised_schema = bundles[0].spec.input_schema.clone(); + + for (input, expect_accepted) in [ + (json!({"n": 1}), true), + (json!({"n": 1, "q": "rust"}), true), + (json!({"n": 1, "level": "high"}), true), + (json!({}), false), + (json!({"q": "missing-n"}), false), + (json!({"n": 0}), false), + (json!({"n": 1, "level": "medium"}), false), + (json!({"n": 1, "debug": true}), false), + (json!({"n": 1, "q": {"object": 1}}), false), + (json!({"n": 1, "n": 2}), true), + ] { + let advertised_verdict = jsonschema::options() + .build(&advertised_schema) + .unwrap() + .is_valid(&input); + assert_eq!( + advertised_verdict, expect_accepted, + "advertised schema verdict mismatch for {input}" + ); + + let result = build_request( + "https://api.example.com", + "/items", + "GET", + &None, + &HashMap::new(), + "items", + &advertised_schema, + Some(&CompiledInputSchema::compile(&advertised_schema).unwrap()), + &input, + &noop_context("rt-1", Capabilities::new()), + ); + match (result, expect_accepted) { + (Ok(_), true) => {} + (Err(err), false) => { + assert_eq!(err.code, "INVALID_INPUT", "input was: {input}"); + } + (Ok(_), false) => { + panic!("enforcement accepted what the advertised schema rejects: {input}") + } + (Err(err), true) => panic!( + "enforcement rejected what the advertised schema accepts: {input} — {}", + err.message + ), + } + } + } + #[tokio::test] async fn import_minimal_doc_yields_one_registration() { let spec = OpenAPISpec::from_json(minimal_spec_json()).unwrap(); @@ -1585,6 +1698,7 @@ mod tests { &HashMap::new(), "github", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &serde_json::json!({"owner":"a","repo":"b"}), &ctx, ) @@ -1611,6 +1725,7 @@ mod tests { &HashMap::new(), "vastai", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &serde_json::json!({}), &ctx, ) @@ -1632,6 +1747,7 @@ mod tests { &HashMap::new(), "svc", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &serde_json::json!({"id":42,"filter":"active"}), &ctx, ) @@ -1848,6 +1964,7 @@ mod tests { &HashMap::new(), "openai", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &serde_json::json!({"body":{"prompt":"hi"}}), &ctx, ) @@ -2054,6 +2171,7 @@ mod tests { &HashMap::new(), "svc", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &serde_json::json!({}), &ctx, ) @@ -2093,6 +2211,7 @@ mod tests { &defaults, "svc", &serde_json::json!({"type": "object", "additionalProperties": true}), + None, &serde_json::json!({}), &ctx, ) diff --git a/src/adapters/input_validation.rs b/src/adapters/input_validation.rs new file mode 100644 index 0000000..f0d1215 --- /dev/null +++ b/src/adapters/input_validation.rs @@ -0,0 +1,243 @@ +//! Compile-once input-schema validators for the HTTP forwarding +//! adapters (review 002 OAI-18 — the enforce leg of the +//! advertise-vs-enforce decision). +//! +//! `/schema` advertises a full JSON Schema for every adapter-imported +//! operation (`required`, `enum`, `pattern`, value types, bounds), but +//! call-time enforcement was a key allowlist only (review 001 OAI-02): +//! a `required: [id]` operation accepted `{}`, a `{"type": "string"}` +//! property sent as an object serialized as JSON text into the query +//! string, and `enum`/`pattern`/`minimum` were never consulted. This +//! module closes the drift: the op's `input_schema` compiles **once at +//! import**, and peer input is validated against the compiled schema at +//! call time — advertise == enforce. +//! +//! # Closed-by-default hardening +//! +//! The adapters generate input schemas without +//! `additionalProperties`, relying on the allowlist for the +//! closed-by-default unknown-key rejection (OAI-02). Raw JSON Schema +//! semantics are permissive by default, so compiling the spec verbatim +//! would *weaken* enforcement. The compile step therefore injects +//! `"additionalProperties": false` into its copy of the schema when the +//! key is absent; an explicit `true` (the documented opt-in catch-all) +//! or an explicit schema value is preserved as written. The original +//! spec value is never mutated — the hardened copy exists only inside +//! the compiled validator. +//! +//! # Failure discipline +//! +//! A schema that fails to compile fails **import** loudly +//! (`AdapterError::SchemaParse`), matching the `publish_schema` +//! fail-closed precedent (review-001 follow-up): an un-validatable +//! input contract must never register as an operation that would then +//! either skip validation or fail closed per call. Compilation happens +//! once per registration; no runtime cache invalidation problem exists +//! because the validator is captured in the same closure as the +//! schema it was compiled from. + +use alkcall::protocol::wire::CallError; +use jsonschema::Validator; +use serde_json::Value; + +/// A call-time input validator compiled from an operation's +/// `input_schema`. `None`-free by construction: use +/// [`CompiledInputSchema::for_schema`] so operations without +/// properties-shaped input still validate against their declared +/// (possibly empty-object) schema. +#[derive(Clone)] +pub(crate) struct CompiledInputSchema { + validator: std::sync::Arc, +} + +impl CompiledInputSchema { + /// Compile `input_schema` for call-time enforcement. Fails with a + /// schema-diagnostics string when the schema is not compilable — + /// the caller (each adapter's `import`) turns that into a loud + /// `AdapterError::SchemaParse` naming the operation. + pub(crate) fn compile(input_schema: &Value) -> Result { + let hardened = harden_closed_by_default(input_schema); + let validator = jsonschema::options() + .build(&hardened) + .map_err(|error| format!("input schema failed to compile: {error}"))?; + Ok(Self { + validator: std::sync::Arc::new(validator), + }) + } + + /// Validate an input object against the compiled schema. `Ok(())` + /// means the input satisfies every leaf constraint the schema + /// advertises; `Err` carries an `INVALID_INPUT` `CallError` naming + /// the violated keyword and the input location. + pub(crate) fn validate(&self, input: &Value) -> Result<(), CallError> { + let error = match self.validator.validate(input) { + Ok(()) => return Ok(()), + Err(error) => error, + }; + let kind = error.kind(); + let keyword = kind.keyword().to_string(); + let instance_path = error.instance_path().to_string(); + let detail = error.to_string(); + Err(CallError::invalid_input(format!( + "input violates the operation's input schema: {detail} [keyword: {keyword}, \ + at: {instance_path}]; the gateway enforces the schema that /schema advertises" + ))) + } +} + +/// Return a hardened copy of the input schema for compilation: +/// `additionalProperties: false` is injected when the key is absent, so +/// the compiled validator keeps the OAI-02 closed-by-default semantics +/// the allowlist enforces today. `true` and explicit schema values pass +/// through unchanged (the documented catch-all opt-in). +fn harden_closed_by_default(input_schema: &Value) -> Value { + if input_schema.get("additionalProperties").is_some() { + return input_schema.clone(); + } + let mut hardened = input_schema.clone(); + if let Some(map) = hardened.as_object_mut() { + map.insert("additionalProperties".to_string(), Value::Bool(false)); + } + hardened +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn absent_additional_properties_is_hardened_to_false() { + let hardened = harden_closed_by_default(&json!({ + "type": "object", + "properties": {"id": {"type": "string"}}, + })); + assert_eq!( + hardened.get("additionalProperties"), + Some(&Value::Bool(false)), + "the compiled copy is closed by default" + ); + } + + #[test] + fn explicit_additional_properties_is_preserved() { + for value in [json!(true), json!({"type": "string"}), json!(false)] { + let schema = json!({ + "type": "object", + "properties": {"id": {"type": "string"}}, + "additionalProperties": value, + }); + let hardened = harden_closed_by_default(&schema); + assert_eq!(hardened.get("additionalProperties"), Some(&value)); + } + } + + #[test] + fn the_original_schema_is_never_mutated() { + let original = json!({ + "type": "object", + "properties": {"id": {"type": "string"}}, + }); + let snapshot = original.clone(); + let _ = CompiledInputSchema::compile(&original).expect("compiles"); + assert_eq!(original, snapshot, "compile must not mutate the spec value"); + } + + #[test] + fn required_is_enforced_at_call_time() { + let compiled = CompiledInputSchema::compile(&json!({ + "type": "object", + "properties": {"id": {"type": "string"}}, + "required": ["id"], + })) + .expect("compiles"); + let err = compiled + .validate(&json!({})) + .expect_err("the missing required key must be rejected"); + assert_eq!(err.code, "INVALID_INPUT"); + assert!(err.message.contains("required"), "message: {}", err.message); + } + + #[test] + fn value_type_mismatch_is_rejected() { + let compiled = CompiledInputSchema::compile(&json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + })) + .expect("compiles"); + let err = compiled + .validate(&json!({"q": {"deep": 1}})) + .expect_err("an object under a string property must be rejected"); + assert!(err.message.contains("type"), "message: {}", err.message); + } + + #[test] + fn unknown_key_is_rejected_by_the_hardened_schema() { + let compiled = CompiledInputSchema::compile(&json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + })) + .expect("compiles"); + let err = compiled + .validate(&json!({"debug": true})) + .expect_err("an undeclared key must be rejected"); + assert!( + err.message.contains("Additional properties") + || err.message.contains("additionalProperties"), + "message: {}", + err.message + ); + } + + #[test] + fn explicit_catch_all_keeps_accepting_unknown_keys() { + let compiled = CompiledInputSchema::compile(&json!({ + "type": "object", + "properties": {"q": {"type": "string"}}, + "additionalProperties": true, + })) + .expect("compiles"); + compiled + .validate(&json!({"debug": true})) + .expect("the documented catch-all opt-in must keep working"); + } + + #[test] + fn enum_and_minimum_are_enforced() { + let compiled = CompiledInputSchema::compile(&json!({ + "type": "object", + "properties": {"level": {"enum": ["low", "high"]}, "n": {"minimum": 1}}, + })) + .expect("compiles"); + for bad in [json!({"level": "medium"}), json!({"n": 0})] { + let err = compiled + .validate(&bad) + .expect_err("violated leaf constraints must be rejected"); + assert_eq!(err.code, "INVALID_INPUT"); + } + } + + #[test] + fn marked_and_body_properties_are_ordinary_properties() { + use crate::adapters::forward::{GATEWAY_BODY_KEY, HEADER_PARAM_IN_MARKER}; + let compiled = CompiledInputSchema::compile(&json!({ + "type": "object", + "properties": { + "region": {HEADER_PARAM_IN_MARKER: "header", "type": "string"}, + GATEWAY_BODY_KEY: {"type": "object"}, + }, + "required": [GATEWAY_BODY_KEY], + })) + .expect("compiles"); + let input = json!({"region": "eu", GATEWAY_BODY_KEY: {"prompt": "hi"}}); + compiled + .validate(&input) + .expect("marker-decorated and body properties validate as ordinary properties"); + } + + #[test] + fn uncompilable_schema_is_an_error_not_a_panicking_path() { + let result = CompiledInputSchema::compile(&json!({"required": "n"})); + assert!(result.is_err(), "a malformed schema fails compilation"); + } +} diff --git a/src/adapters/mod.rs b/src/adapters/mod.rs index 0d31424..7e3ad10 100644 --- a/src/adapters/mod.rs +++ b/src/adapters/mod.rs @@ -6,6 +6,7 @@ pub mod forward; pub mod from_jsonschema; pub mod from_openapi; +pub mod input_validation; pub mod openapi_spec; pub mod to_openapi; diff --git a/tasks/adapters/review-002-fu-oai18-decision.md b/tasks/adapters/review-002-fu-oai18-decision.md index 21ba57b..0a38ad0 100644 --- a/tasks/adapters/review-002-fu-oai18-decision.md +++ b/tasks/adapters/review-002-fu-oai18-decision.md @@ -1,7 +1,7 @@ --- id: review-002-fu-oai18-decision name: OAI-18 advertise-vs-enforce decision — enforce leaf constraints or scope the advert (OAI-18) -status: pending +status: done depends_on: [] scope: moderate risk: medium