diff --git a/docs/architecture/http-adapters.md b/docs/architecture/http-adapters.md index 27be06b..91c0257 100644 --- a/docs/architecture/http-adapters.md +++ b/docs/architecture/http-adapters.md @@ -118,6 +118,58 @@ pub enum HttpAuthScheme { } ``` +#### Loud unsupported-feature handling (the OAI-06 matrix, review 001/002) + +Features of an imported OpenAPI document that the single-endpoint HTTP +adapter deliberately does not model are handled **loudly at import** — +refused with a feature-naming `SchemaParse` error, projected with a +documented mapping, skipped with a `tracing::warn`, or (where semantics +are faithfully preserved) accepted silently. Nothing that would change +the wire contract may vanish silently. The matrix, as landed: + +**Refused at import (import fails; the error names the feature, its +location, the remediation, and the review item):** + +| Feature | Where checked | Note | +|---|---|---| +| `in: cookie` parameters | operation + path-item parse | OAI-03 | +| Non-default parameter `style`/`serialize` forms (`spaceDelimited`, `pipeDelimited`, `deepObject`, `matrix`, `label`, `form+explode:false`, `simple+explode:true`) | operation + path-item parse; wire-equivalent defaults (`form`, `simple`) accepted | OAI-06 | +| `servers` overrides (document/path/operation level) — single `base_url` per import | shared `from_value` parse | OAI-06 | +| `webhooks` (anywhere in the document) | shared `from_value` parse | OAI-13 | +| `callbacks` (document/operation level) | import gate (`validate_import_loud_features`), not the shared parse — the published gateway doc's own `security` markers must round-trip inside `to_openapi` | OAI-14 | +| OpenAPI `security` requirements (document/operation level) — credentials come only from `Capabilities` + the configured `auth` scheme | import gate | OAI-14 | +| Top-level `oneOf` requestBodies (no `content` map — the media-typed body contract is unrepresentable) | operation parse | OAI-14 | +| Unresolvable parameter/`requestBody` `$ref`s; a resolved `requestBody` that still carries a top-level `$ref` or lacks `content` (a body-less op would fail every call on the gateway `body` input) | operation parse | OAI-04/OAI-15 | +| Unterminated/empty path-template placeholders (`/x{open`) | shared `validate_path_template`, both `from_openapi` import and `from_jsonschema` construction | OAI-09, JS-02 | +| Duplicate operationIds or `(path, method)` routes in one import batch | `reject_collisions` | OAI-05 | +| A declared `in: header` parameter named `Authorization` when the namespace has an auth scheme; a header parameter colliding (case-insensitively) with a configured `default_headers` key or an ApiKey `header_name` — `build_request` inserts defaults and credentials after header params, so the peer value would silently lose | `check_header_param_collisions`, import time (the adapter's `HttpServiceConfig` is visible there) | OAI-19 | + +**Warned (import proceeds; the divergence is visible in logs):** + +| Feature | Behavior | Note | +|---|---|---| +| `$ref` sibling keys — 3.0 semantics apply (ignored); a 3.1-authored constraint beside a `$ref` would otherwise silently overstate `/schema` | `tracing::warn` naming the location and dropped keys; no `openapi: 3.1` version gate — the 3.0-only reading is the documented stance | OAI-10 | +| Path items declaring only unsupported methods (`trace`) | skipped; `tracing::warn` names the path and methods | OAI-06 | +| `discriminator` / `xml` keywords inside consumed schemas | per-operation `tracing::warn` listing the keys — the adapter forwards JSON only | OAI-14 | +| Response keys that are neither concrete statuses nor 2XX/4XX/5XX class wildcards | dropped from the imported error schemas; unmapped statuses surface as synthesized `HTTP_` at call time | OAI-06 | +| Path-item-level unknown keys on a method-bearing path (mirroring OAI-06's trace-skip warn) | warn | OAI-13 | + +**Projected (documented, tested mapping):** + +| Feature | Behavior | Note | +|---|---|---| +| Error response class wildcards `4XX` / `5XX` | projected onto the first legal concrete status in the implied range (`HTTP_400` / `HTTP_500`), carrying the wildcard's payload schema; `default` is dropped (no implied range, never advertised as `HTTP_0`) | OAI-13 | +| Success envelopes declared under any 2XX key, the `2XX` wildcard, or `default` | precedence: concrete 2XX → `2XX` → `default`; SSE detection and output-schema selection follow the winner | OAI-06, OAI-13 | +| Response keys that are neither concrete statuses nor supported wildcards | dropped with `tracing::warn` (see above) | OAI-06 | + +**Documented divergence (both entry points, tested):** the YAML/JSON +parity contract lives in ADR-051 §5 (duplicate keys rejected on the +YAML path, non-finite floats and non-string keys rejected with JSON +pointers, merge keys applied) — import errors quote at most the first 8 +items of any spec-derived list and truncate each item at 128 chars +(OAI-17), so a pathological document cannot produce a +multi-megabyte error message. + The adapter: 1. Parses the OpenAPI document (`OpenAPISpec` — `paths`, `components`, diff --git a/src/adapters/forward.rs b/src/adapters/forward.rs index cb5a4b6..a77a4a9 100644 --- a/src/adapters/forward.rs +++ b/src/adapters/forward.rs @@ -122,6 +122,7 @@ use std::collections::HashMap; use std::sync::Arc; +use alkcall::client::AdapterError; use alkcall::protocol::wire::{CallError, ResponseEnvelope}; use alkcall::registry::context::OperationContext; use alkcall::registry::registration::ResponseStream; @@ -176,6 +177,72 @@ pub(crate) const GATEWAY_BODY_KEY: &str = "body"; pub(crate) const HEADER_PARAM_IN_MARKER: &str = "wire"; pub(crate) const HEADER_PARAM_MARKER_VALUE: &str = "header"; +/// Upper bound on how many list items an adapter error message echoes +/// (review 002 OAI-17), and the per-item string cap. A spec-derived list +/// (servers locations, placeholder names, declared keys) can be +/// arbitrarily large; an error echoing all of it turns a 100k-path +/// document into a multi-megabyte message. The shape is "first N + count +/// of the rest". +pub(crate) const ERROR_LIST_ITEMS: usize = 8; +pub(crate) const ERROR_ITEM_STRING_CAP: usize = 128; + +/// Joins list items into an error-message fragment bounded in both item +/// count and item width: at most [`ERROR_LIST_ITEMS`] entries, each +/// truncated to [`ERROR_ITEM_STRING_CAP`] chars with a `…` marker, plus +/// a `, … (+N more)` suffix naming how many were suppressed. +pub(crate) fn bounded_join(items: &[String]) -> String { + let shown: Vec = items + .iter() + .take(ERROR_LIST_ITEMS) + .map(|item| { + if item.chars().count() > ERROR_ITEM_STRING_CAP { + let truncated: String = item.chars().take(ERROR_ITEM_STRING_CAP).collect(); + format!("{truncated}…") + } else { + item.clone() + } + }) + .collect(); + if items.len() > ERROR_LIST_ITEMS { + format!( + "{}, … (+{} more)", + shown.join(", "), + items.len() - ERROR_LIST_ITEMS + ) + } else { + shown.join(", ") + } +} + +/// Import-time path-template validation shared by `from_jsonschema` +/// (review 001 OAI-09) and `from_openapi` (review 002 JS-02): every +/// `{placeholder}` must terminate with a `}` and carry a name. A +/// template that slips through (e.g. `/x{open`) otherwise surfaces as a +/// per-call `INTERNAL` error on first invoke — the eager-validation +/// promise both adapters make. +pub(crate) fn validate_path_template(path_template: &str) -> Result<(), AdapterError> { + let mut rest = path_template; + while let Some(start) = rest.find('{') { + let Some(end_rel) = rest[start..].find('}') else { + return Err(AdapterError::SchemaParse { + message: format!("path template `{path_template}` has an unterminated placeholder"), + }); + }; + if rest[start + 1..start + end_rel].is_empty() { + return Err(AdapterError::SchemaParse { + message: format!("path template `{path_template}` has an empty placeholder name"), + }); + } + rest = &rest[start + end_rel + 1..]; + } + if path_template.contains('}') && !path_template.contains('{') { + return Err(AdapterError::SchemaParse { + message: format!("path template `{path_template}` has `}}` without a matching `{{`"), + }); + } + Ok(()) +} + /// The credential scheme forwarded handlers apply to outbound requests /// (ADR-014). The credential value itself flows through /// `OperationContext.capabilities` at call time — never through this @@ -417,7 +484,7 @@ fn enforce_input_schema( let declared_list = if declared.is_empty() { "none".to_string() } else { - declared.join(", ") + bounded_join(&declared) }; return Err(CallError::invalid_input(format!( "input key `{first}` is not declared by the operation's input schema \ @@ -591,7 +658,7 @@ pub(crate) fn render_path_template( if !unresolved.is_empty() { return Err(CallError::internal(format!( "path template `{template}` references unbound placeholder(s): {}", - unresolved.join(", ") + bounded_join(&unresolved) ))); } Ok(out) diff --git a/src/adapters/from_jsonschema.rs b/src/adapters/from_jsonschema.rs index dc8dd93..8870fff 100644 --- a/src/adapters/from_jsonschema.rs +++ b/src/adapters/from_jsonschema.rs @@ -32,7 +32,7 @@ use async_trait::async_trait; use reqwest::Method; use serde_json::Value; -use super::forward::{forward, forward_stream, HttpServiceConfig}; +use super::forward::{forward, forward_stream, validate_path_template, HttpServiceConfig}; use crate::client::SharedHttpClient; /// The HTTP-backed single-endpoint adapter (ADR-066): one caller-built @@ -139,30 +139,6 @@ fn validate_method(method: &str) -> Result<(), AdapterError> { })?; Ok(()) } - -fn validate_path_template(path_template: &str) -> Result<(), AdapterError> { - let mut rest = path_template; - while let Some(start) = rest.find('{') { - let Some(end_rel) = rest[start..].find('}') else { - return Err(AdapterError::SchemaParse { - message: format!("path template `{path_template}` has an unterminated placeholder"), - }); - }; - if rest[start + 1..start + end_rel].is_empty() { - return Err(AdapterError::SchemaParse { - message: format!("path template `{path_template}` has an empty placeholder name"), - }); - } - rest = &rest[start + end_rel + 1..]; - } - if path_template.contains('}') && !path_template.contains('{') { - return Err(AdapterError::SchemaParse { - message: format!("path template `{path_template}` has `}}` without a matching `{{`"), - }); - } - Ok(()) -} - fn spec_name_references_undeclared(spec: &OperationSpec, path_template: &str) -> bool { let properties = spec .input_schema diff --git a/src/adapters/from_openapi.rs b/src/adapters/from_openapi.rs index 994ad48..c6b0176 100644 --- a/src/adapters/from_openapi.rs +++ b/src/adapters/from_openapi.rs @@ -27,10 +27,12 @@ use async_trait::async_trait; use serde_json::Value; use super::forward::{ - forward, forward_stream, HttpServiceConfig, GATEWAY_BODY_KEY, HEADER_PARAM_IN_MARKER, - HEADER_PARAM_MARKER_VALUE, + bounded_join, forward, forward_stream, validate_path_template, HttpAuthScheme, + HttpServiceConfig, GATEWAY_BODY_KEY, HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE, +}; +use super::openapi_spec::{ + collect_ignored_schema_keys, OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS, }; -use super::openapi_spec::{OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS}; use crate::client::SharedHttpClient; fn unbound_placeholders(path_template: &str, input_schema: &Value) -> Vec { @@ -56,6 +58,16 @@ fn unbound_placeholders(path_template: &str, input_schema: &Value) -> Vec Vec { + let mut found = Vec::new(); + for schema in schemas { + collect_ignored_schema_keys(schema, &mut found); + } + found.sort(); + found.dedup(); + found +} + fn collision_message(batch: &str, kind: &str, first_path: &str) -> AdapterError { AdapterError::SchemaParse { message: format!( @@ -71,8 +83,6 @@ fn reject_collisions( paths: Vec, routes: Vec<(String, String)>, ) -> Result<(), AdapterError> { - assert_eq!(op_ids.len(), routes.len()); - assert_eq!(op_ids.len(), paths.len()); let mut seen_names: HashMap<&str, &str> = HashMap::new(); let mut seen_routes: HashMap<&(String, String), &str> = HashMap::new(); for ((op_id, path), route) in op_ids.iter().zip(paths.iter()).zip(routes.iter()) { @@ -158,6 +168,11 @@ impl FromOpenAPI { ) -> Result { let mut properties = serde_json::Map::new(); let mut required = Vec::new(); + let locator = format!( + "{} {}", + op.operation_id.as_deref().unwrap_or("?"), + self.config.namespace + ); // OAI-13: path-item-level parameters merge into every operation // under the path. The operation-level entries come second, so a @@ -183,6 +198,7 @@ impl FromOpenAPI { } match param.in_.as_str() { "header" => { + self.check_header_param_collisions(¶m.name, &locator)?; properties.insert( param.name.clone(), serde_json::json!({ @@ -233,6 +249,60 @@ impl FromOpenAPI { })) } + /// Header-precedence gate (review 002 OAI-19): `build_request` inserts + /// header params first, then `default_headers`, then credential + /// headers — later `insert` calls replace earlier ones, so a declared + /// `in: header` parameter whose name matches a default/credential + /// header would silently never deliver the peer's value upstream. + /// `Authorization` on an authed namespace is rejected outright (the + /// credential header always wins by construction); a collision with a + /// configured `default_headers` key fails import with both names — + /// the assembly's config is visible at import time, so silence about + /// it would be a lie about the wire. + fn check_header_param_collisions(&self, name: &str, locator: &str) -> Result<(), AdapterError> { + if self.config.auth.is_some() && name.eq_ignore_ascii_case("authorization") { + return Err(AdapterError::SchemaParse { + message: format!( + "header parameter `{name}` on {locator} collides with the \ + Authorization credential header the adapter injects from \ + Capabilities; the peer-supplied value would be silently replaced \ + by the outbound credential on every call — rename the parameter or \ + remove the auth scheme from the service config (review 002 OAI-19)" + ), + }); + } + let lower = name.to_ascii_lowercase(); + if self + .config + .default_headers + .keys() + .any(|k| k.to_ascii_lowercase() == lower) + { + return Err(AdapterError::SchemaParse { + message: format!( + "header parameter `{name}` on {locator} collides with a configured \ + default_headers entry of the same name; the default value would \ + silently replace the peer-supplied header value at call time — \ + rename the parameter or drop the default_headers entry \ + (review 002 OAI-19)" + ), + }); + } + if let Some(HttpAuthScheme::ApiKey { header_name }) = &self.config.auth { + if header_name.eq_ignore_ascii_case(name) || header_name.to_ascii_lowercase() == lower { + return Err(AdapterError::SchemaParse { + message: format!( + "header parameter `{name}` on {locator} collides with the API-key \ + credential header `{header_name}`; the credential would silently \ + replace the peer-supplied value at call time — rename the \ + parameter (review 002 OAI-19)" + ), + }); + } + } + Ok(()) + } + fn build_output_schema(&self, op: &Operation) -> Result { // Mirrors `detect_op_type`'s success-key sweep (OAI-06, OAI-13): // a stream declared under a non-200/201 2XX key, the `2XX` @@ -312,12 +382,26 @@ impl FromOpenAPI { op: &Operation, path_parameters: &[Parameter], ) -> Result { + validate_path_template(path)?; let name = Self::normalize_operation_id(op, method, path); let qualified_name = format!("{}/{name}", self.config.namespace); let op_type = Self::detect_op_type(method, op); let input_schema = self.build_input_schema(op, path_parameters)?; let output_schema = self.build_output_schema(op)?; let error_schemas = self.build_error_schemas(op)?; + let ignored = oai14_ignored_keys(&[&input_schema, &output_schema]); + if !ignored.is_empty() { + tracing::warn!( + namespace = %self.config.namespace, + operation = %name, + keys = %ignored.join(", "), + "schema declares serialization keywords the HTTP adapter ignores: they \ + change what a conforming OpenAPI client would send (polymorphic \ + discriminator headers, XML wire annotations) but the adapter forwards \ + JSON only — remove them or split the operation if the upstream \ + requires that wire shape (review 002 OAI-14)" + ); + } let spec = OperationSpec::new( qualified_name, @@ -354,16 +438,16 @@ impl FromOpenAPI { // diagnosis is not a dead end. return Err(AdapterError::SchemaParse { message: format!( - "path {method} {path_template} declares placeholder(s) {} with no \ + "path {method} {} declares placeholder(s) {} with no \ matching parameter in the operation's resolved input schema. Every \ `parameters` source was merged (path-item level and operation level, \ review 002 OAI-13); a placeholder left unbound after the merge means \ a declared parameter was dropped — check that each entry under \ - `paths.{path_template}.parameters` (and the path-item's shared \ + `paths` for this path's `parameters` (and the path-item's shared \ list) has `name` and `in`, and that its $ref, if any, resolves. \ - The placeholder would otherwise render as a literal `{}` path segment", - unbound.join(", "), - unbound[0] + The placeholder would otherwise render as a literal `{{}}` path segment", + bounded_join(&[path_template.to_string()]), + bounded_join(&unbound) ), }); } @@ -441,6 +525,7 @@ impl FromOpenAPI { #[async_trait] impl OperationAdapter for FromOpenAPI { async fn import(&self) -> Result, AdapterError> { + self.spec.validate_import_loud_features()?; let mut bundles = Vec::new(); let mut op_ids = Vec::new(); let mut paths = Vec::new(); @@ -812,6 +897,142 @@ mod tests { } } + #[tokio::test] + async fn unterminated_path_template_fails_import_not_first_call() { + let doc = r#"{ + "openapi":"3.0.0","info":{"title":"T","version":"1"}, + "paths":{"/x{open":{"get":{"operationId":"x","responses":{"200":{"content":{"application/json":{"schema":{}}}}}}}} + }"#; + let spec = OpenAPISpec::from_json(doc).unwrap(); + let result = adapter(spec, config("ns", "https://x", None)) + .import() + .await; + match result { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("unterminated placeholder"), + "message was: {message}" + ); + assert!(message.contains("/x{open"), "message was: {message}"); + } + Ok(bundles) => panic!( + "unterminated path template must fail at import (JS-02), got {} bundles", + bundles.len() + ), + Err(e) => panic!("expected SchemaParse, got {e}"), + } + } + + #[tokio::test] + async fn header_param_colliding_with_authorization_on_authed_namespace_rejected() { + let doc = r#"{ + "openapi":"3.0.0","info":{"title":"T","version":"1"}, + "paths":{"/me":{"get":{ + "operationId":"me", + "parameters":[{"name":"Authorization","in":"header","schema":{"type":"string"}}], + "responses":{"200":{"content":{"application/json":{"schema":{}}}}} + }}}}"#; + let spec = OpenAPISpec::from_json(doc).unwrap(); + let result = adapter( + spec, + config("svc", "https://x", Some(HttpAuthScheme::Bearer)), + ) + .import() + .await; + match result { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("Authorization credential header"), + "message was: {message}" + ); + assert!(message.contains("OAI-19"), "message was: {message}"); + } + Ok(bundles) => panic!( + "Authorization header param on authed namespace must be rejected, got {} bundles", + bundles.len() + ), + Err(e) => panic!("expected SchemaParse, got {e}"), + } + } + + #[tokio::test] + async fn header_param_colliding_with_default_headers_rejected() { + let doc = r#"{ + "openapi":"3.0.0","info":{"title":"T","version":"1"}, + "paths":{"/t":{"get":{ + "operationId":"t", + "parameters":[{"name":"X-Tenant","in":"header","schema":{"type":"string"}}], + "responses":{"200":{"content":{"application/json":{"schema":{}}}}} + }}}}"#; + let spec = OpenAPISpec::from_json(doc).unwrap(); + let mut cfg = config("svc", "https://x", None); + cfg.default_headers + .insert("x-tenant".to_string(), "fixed".to_string()); + let result = adapter(spec, cfg).import().await; + match result { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("default_headers"), + "message was: {message}" + ); + assert!(message.contains("X-Tenant"), "message was: {message}"); + } + Ok(bundles) => panic!( + "header/default_headers collision must be rejected, got {} bundles", + bundles.len() + ), + Err(e) => panic!("expected SchemaParse, got {e}"), + } + } + + #[tokio::test] + async fn header_param_colliding_with_api_key_header_rejected() { + let doc = r#"{ + "openapi":"3.0.0","info":{"title":"T","version":"1"}, + "paths":{"/t":{"get":{ + "operationId":"t", + "parameters":[{"name":"x-api-key","in":"header","schema":{"type":"string"}}], + "responses":{"200":{"content":{"application/json":{"schema":{}}}}} + }}}}"#; + let spec = OpenAPISpec::from_json(doc).unwrap(); + let auth = Some(HttpAuthScheme::ApiKey { + header_name: "X-API-Key".to_string(), + }); + let result = adapter(spec, config("svc", "https://x", auth)) + .import() + .await; + match result { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("API-key credential header"), + "message was: {message}" + ); + } + Ok(bundles) => panic!( + "API-key header collision must be rejected, got {} bundles", + bundles.len() + ), + Err(e) => panic!("expected SchemaParse, got {e}"), + } + } + + #[tokio::test] + async fn header_param_without_config_collision_imports_cleanly() { + let doc = r#"{ + "openapi":"3.0.0","info":{"title":"T","version":"1"}, + "paths":{"/t":{"get":{ + "operationId":"t", + "parameters":[{"name":"X-Trace-Id","in":"header","schema":{"type":"string"}}], + "responses":{"200":{"content":{"application/json":{"schema":{}}}}} + }}}}"#; + let spec = OpenAPISpec::from_json(doc).unwrap(); + let bundles = adapter(spec, config("svc", "https://x", None)) + .import() + .await + .unwrap(); + assert_eq!(bundles.len(), 1); + } + #[tokio::test] async fn op_type_detection() { let get_doc = r#"{"openapi":"3.0.0","info":{"title":"T","version":"1"},"paths":{"/g":{"get":{"operationId":"g","responses":{"200":{"content":{"application/json":{"schema":{}}}}}}}}}"#; diff --git a/src/adapters/openapi_spec.rs b/src/adapters/openapi_spec.rs index fb4a5c3..03c25a9 100644 --- a/src/adapters/openapi_spec.rs +++ b/src/adapters/openapi_spec.rs @@ -32,9 +32,21 @@ //! - **Bare `yes`/`no`/`on`/`off` and tags** — identical behavior on //! both paths by YAML 1.2 core schema (strings, `!!str 200` → //! `"200"`); unknown tags fail loudly on both. +//! +//! # Version stance (review 002 OAI-10) +//! +//! Documents are interpreted under **OpenAPI 3.0 semantics**; there is +//! no `openapi: 3.1` version gate. The one divergence that matters here +//! is `$ref` siblings: 3.0 ignores them, 3.1 applies them alongside the +//! resolved target. A `$ref` carrying sibling keys therefore imports +//! with the 3.0 reading and a `tracing::warn` naming the dropped keys — +//! the advertise/enforce drift a 3.1-authored constraint would +//! otherwise hide is visible at import instead of surfacing as a +//! silent `/schema` overstatement. use std::collections::{BTreeMap, HashMap, HashSet}; +use crate::adapters::forward::bounded_join; use alkcall::client::AdapterError; use serde_json::Value; use yaml_serde::Value as YamlValue; @@ -473,7 +485,7 @@ impl OpenAPISpec { configured at assembly time and cannot honor per-location `servers` — \ remove the `servers` entries or split the service into one import per \ base URL (review 001 OAI-06)", - servers_locations.join(", ") + bounded_join(&servers_locations) ), }); } @@ -487,13 +499,16 @@ impl OpenAPISpec { let mut operations = Vec::new(); for method in HTTP_METHODS { if let Some(op_raw) = item.get(*method) { - match parse_operation(op_raw, &provisional) { + let locator = format!("{method} {path}"); + match parse_operation(op_raw, &provisional, &locator) { Ok(Some(op)) => operations.push((method.to_string(), op)), Ok(None) => { return Err(AdapterError::SchemaParse { message: format!( "unresolvable $ref or missing `name`/`in` in parameter of \ - {method} {path}" + {method} {path}, or an unresolvable/content-less \ + `requestBody` on the operation (review 001 OAI-04, \ + review 002 OAI-15)" ), }); } @@ -576,16 +591,86 @@ impl OpenAPISpec { }) } + /// Import-time loud-feature gate for the *service import* path + /// (`FromOpenAPI::import`), not the shared structural parse: + /// `from_value` also re-validates the published gateway doc inside + /// `to_openapi`, and that self-description legitimately carries + /// `security` markers for its external HTTP clients. A *service* + /// spec declaring the OAI-14 blocks would import silently-degraded + /// operations, so the import refuses where the semantics would be + /// lost; the gateway doc's own markers are inert here. + pub(crate) fn validate_import_loud_features(&self) -> Result<(), AdapterError> { + let mut callback_locations: Vec = Vec::new(); + let mut security_locations: Vec = Vec::new(); + if self.raw.get("callbacks").is_some() { + callback_locations.push("document".to_string()); + } + if self.raw.get("security").is_some() { + security_locations.push("document".to_string()); + } + if let Some(paths_obj) = self.raw.get("paths").and_then(|p| p.as_object()) { + for (path, item) in paths_obj { + let Some(item_obj) = item.as_object() else { + continue; + }; + for method in HTTP_METHODS { + let Some(op) = item_obj.get(*method).and_then(|op| op.as_object()) else { + continue; + }; + if op.contains_key("callbacks") { + callback_locations.push(format!("{method} {path}")); + } + if op.contains_key("security") { + security_locations.push(format!("{method} {path}")); + } + } + } + } + if !callback_locations.is_empty() { + return Err(AdapterError::SchemaParse { + message: format!( + "the document declares `callbacks` at: {}. Callbacks are \ + server-initiated outbound calls (inbound to this service) that the \ + single-endpoint HTTP adapter does not model — remove the \ + `callbacks` entries or split those operations into their own \ + service definition (review 002 OAI-14)", + bounded_join(&callback_locations) + ), + }); + } + if !security_locations.is_empty() { + return Err(AdapterError::SchemaParse { + message: format!( + "the document declares `security` requirement(s) at: {}. The HTTP \ + adapter injects credentials exclusively through Capabilities per \ + the declared auth scheme (review 001 OAI-06 posture); OpenAPI \ + security requirements would silently change nothing at call time — \ + remove the `security` blocks or set the `auth` field on the service \ + config instead (review 002 OAI-14)", + bounded_join(&security_locations) + ), + }); + } + Ok(()) + } + pub(crate) fn resolve_ref(&self, reference: &str) -> Result { + let bounded = |r: &str| { + if r.chars().count() > 128 { + format!("{}…", r.chars().take(128).collect::()) + } else { + r.to_string() + } + }; if !reference.starts_with("#/") { return Err(AdapterError::SchemaParse { - message: format!("external $ref not supported: {reference}"), + message: format!("external $ref not supported: {}", bounded(reference)), }); } let mut current: &Value = &self.raw; for part in reference.trim_start_matches("#/").split('/') { current = current.get(part).ok_or_else(|| AdapterError::SchemaParse { - message: format!("cannot resolve $ref: {reference}"), + message: format!("cannot resolve $ref: {}", bounded(reference)), })?; } Ok(current.clone()) @@ -734,9 +819,71 @@ fn count_nodes(value: &Value) -> usize { } } +/// Collects the schema-embedded keywords the adapter silently ignores +/// (review 002 OAI-14): `discriminator` (polymorphic serialization +/// headers the forwarder does not emit) and `xml` (wire-format +/// annotations for XML serialization the adapter never performs). Both +/// change what a conforming client would send or expect on the wire; +/// vanishing them silently lets a schema advertise a shape the calls +/// never honor. The walk visits only the *declared* schema (already +/// bounded by the resolver's budgets before this runs on resolved +/// output); it is linear in schema size. +pub(crate) fn collect_ignored_schema_keys(value: &Value, found: &mut Vec) { + let mut stack = vec![value]; + while let Some(current) = stack.pop() { + match current { + Value::Object(map) => { + for (k, v) in map { + if k == "discriminator" || k == "xml" { + found.push(k.clone()); + } + stack.push(v); + } + } + Value::Array(items) => { + stack.extend(items.iter()); + } + _ => {} + } + } +} + +/// Warns once per offending `$ref` object about sibling keys left +/// beside the `$ref` (review 002 OAI-10): under OpenAPI 3.0 the +/// siblings are ignored, but 3.1 applies them alongside the reference — +/// so a document authored against 3.1 semantics would advertise +/// constraints (`minLength: 3`) through `/schema` that the adapter's +/// resolved schema (used at call time) does not carry. There is no +/// `openapi: 3.1` version gate; the import proceeds with the 3.0 +/// reading while naming the dropped keys. +fn warn_ref_siblings(context: &str, holder: &Value) { + if let Some(obj) = holder.as_object() { + let siblings: Vec<&String> = obj.keys().filter(|k| *k != "$ref").collect(); + if !siblings.is_empty() { + let names: Vec = siblings.iter().map(|s| s.as_str().to_string()).collect(); + tracing::warn!( + location = %context, + siblings = %names.join(", "), + "$ref carries sibling keys; OpenAPI 3.0 semantics apply — the \ + siblings are ignored (not merged into the resolved target as \ + 3.1 would do), so constraints authored beside the $ref are \ + not enforced at call time (review 002 OAI-10)" + ); + } + } +} + +/// Parses one operation (OAI-04/OAI-15). Returns `Ok(None)` when the +/// operation cannot be modeled faithfully: an unresolvable parameter +/// `$ref`, a parameter missing `name`/`in`, an unresolvable +/// `requestBody` `$ref`, or a resolved `requestBody` that still carries +/// a top-level `$ref` or lacks `content` — a body-less op would +/// register silently and fail every call with `INVALID_INPUT` on +/// `body` (review 002 OAI-15). fn parse_operation( raw: &Value, spec: &OpenAPISpec, + locator: &str, ) -> Result, ParameterStyleError> { if !raw.is_object() { return Ok(None); @@ -748,12 +895,15 @@ fn parse_operation( let mut parameters = Vec::new(); if let Some(arr) = raw.get("parameters").and_then(|v| v.as_array()) { - for p in arr { + for (index, p) in arr.iter().enumerate() { let p = match p.get("$ref").and_then(|r| r.as_str()) { - Some(reference) => match spec.resolve_ref(reference) { - Ok(resolved) => resolved, - Err(_) => return Ok(None), - }, + Some(reference) => { + warn_ref_siblings(&format!("parameter[{index}] $ref {reference}"), p); + match spec.resolve_ref(reference) { + Ok(resolved) => resolved, + Err(_) => return Ok(None), + } + } None => p.clone(), }; let Some(name) = p.get("name").and_then(|v| v.as_str()) else { @@ -762,12 +912,6 @@ fn parse_operation( let Some(in_) = p.get("in").and_then(|v| v.as_str()) else { return Ok(None); }; - // OAI-06: non-default `style`/`explode` forms change how arrays - // and objects serialize on the wire (the adapter emits the - // form/simple default — repeated keys for query arrays). A - // parameter declaring a different serialization would silently - // mis-serialize upstream (`"[1,2]"`-style), so it fails import - // with an error naming the parameter and feature. check_parameter_style(name, in_, &p)?; let required = p.get("required").and_then(|v| v.as_bool()).unwrap_or(false); let schema = p.get("schema").cloned(); @@ -780,19 +924,49 @@ fn parse_operation( } } - let request_body = raw.get("requestBody").and_then(|rb| { - let rb = match rb.get("$ref").and_then(|r| r.as_str()) { - Some(reference) => spec.resolve_ref(reference).ok()?, - None => rb.clone(), - }; - let content_obj = rb.get("content")?.as_object()?; - let mut content = BTreeMap::new(); - for (k, v) in content_obj { - let schema = v.get("schema").cloned().unwrap_or(Value::Null); - content.insert(k.clone(), schema); + let request_body = match raw.get("requestBody") { + Some(rb) => { + let body = match rb.get("$ref").and_then(|r| r.as_str()) { + Some(reference) => { + warn_ref_siblings(&format!("requestBody $ref {reference}"), rb); + match spec.resolve_ref(reference) { + Ok(resolved) => resolved, + Err(_) => return Ok(None), + } + } + None => rb.clone(), + }; + if body.get("$ref").is_some() || !body.get("content").is_some_and(|c| c.is_object()) { + return Ok(None); + } + // OAI-14: a requestBody shaped `{oneOf: [...]}` (the union form + // some 3.1-era generators emit) has no `content` map, so the + // media-type keyed body contract is unrepresentable — it must + // not silently import as a body-less op. + if body.get("oneOf").is_some() { + return Err(ParameterStyleError { + parameter: "requestBody".to_string(), + detail: format!( + "on {locator}: uses a top-level `oneOf` requestBody (no \ + `content` map), which the HTTP adapter cannot turn into the \ + gateway's media-typed body contract — wrap each variant in a \ + `content` entry (e.g. application/json) or split into separate \ + operations (review 002 OAI-14)" + ), + }); + } + let Some(content_obj) = body.get("content").and_then(|v| v.as_object()) else { + return Ok(None); + }; + let mut content = BTreeMap::new(); + for (k, v) in content_obj { + let schema = v.get("schema").cloned().unwrap_or(Value::Null); + content.insert(k.clone(), schema); + } + Some(RequestBody { content }) } - Some(RequestBody { content }) - }); + None => None, + }; let mut responses = BTreeMap::new(); if let Some(resp_obj) = raw.get("responses").and_then(|v| v.as_object()) { @@ -837,11 +1011,13 @@ impl ItemParameters { let Some(arr) = item.get("parameters").and_then(|v| v.as_array()) else { return Ok(out); }; - for p in arr { + for (index, p) in arr.iter().enumerate() { let p = match p.get("$ref").and_then(|r| r.as_str()) { - Some(reference) => spec - .resolve_ref(reference) - .map_err(|_| path_style_error(format!("unresolvable $ref: {reference}")))?, + Some(reference) => { + warn_ref_siblings(&format!("path-item parameter[{index}] $ref {reference}"), p); + spec.resolve_ref(reference) + .map_err(|_| path_style_error(format!("unresolvable $ref: {reference}")))? + } None => p.clone(), }; let Some(name) = p.get("name").and_then(|v| v.as_str()) else { @@ -1028,6 +1204,99 @@ mod tests { assert!(props.get("name").is_some()); } + #[test] + fn request_body_self_ref_fails_import_not_silent_bodyless_op() { + let doc = r##"{ + "openapi": "3.0.0", + "info": {"title": "T", "version": "1"}, + "paths": { + "/widgets": {"post": { + "operationId": "createWidget", + "requestBody": {"$ref": "#/paths/~1widgets/post/requestBody"}, + "responses": {"201": {"content": {"application/json": {"schema": {}}}}} + }} + } + }"##; + match OpenAPISpec::from_json(doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("requestBody"), + "the error must name the requestBody: {message}" + ); + assert!(message.contains("OAI-15"), "message was: {message}"); + } + Ok(spec) => { + let body = &spec.paths["/widgets"].operations[0].1.request_body; + assert!( + body.is_none(), + "a self-ref'd requestBody must not silently import as body-less (OAI-15)" + ); + panic!("self-$ref'd requestBody must fail import loudly (OAI-15)"); + } + other => panic!("expected SchemaParse, got {other:?}"), + } + } + + #[test] + fn request_body_ref_to_missing_component_fails_import_loudly() { + let doc = r##"{ + "openapi": "3.0.0", + "info": {"title": "T", "version": "1"}, + "paths": { + "/widgets": {"post": { + "operationId": "createWidget", + "requestBody": {"$ref": "#/components/requestBodies/Missing"}, + "responses": {"201": {"content": {"application/json": {"schema": {}}}}} + }} + } + }"##; + match OpenAPISpec::from_json(doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("requestBody") || message.contains("unresolvable"), + "message was: {message}" + ); + assert!(message.contains("post /widgets"), "message was: {message}"); + } + Ok(_) => panic!("unresolvable requestBody $ref must fail import loudly"), + other => panic!("expected SchemaParse, got {other:?}"), + } + } + + #[test] + fn content_less_request_body_fails_import_not_silent_bodyless_op() { + let doc = r##"{ + "openapi": "3.0.0", + "info": {"title": "T", "version": "1"}, + "components": { + "requestBodies": { + "DescriptionOnly": {"description": "no content map"} + } + }, + "paths": { + "/widgets": {"post": { + "operationId": "createWidget", + "requestBody": {"$ref": "#/components/requestBodies/DescriptionOnly"}, + "responses": {"201": {"content": {"application/json": {"schema": {}}}}} + }} + } + }"##; + match OpenAPISpec::from_json(doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("requestBody") || message.contains("unresolvable"), + "message was: {message}" + ); + } + Ok(spec) => { + let body = &spec.paths["/widgets"].operations[0].1.request_body; + assert!(body.is_none(), "content-less body must not silently drop"); + panic!("content-less requestBody must fail import loudly (OAI-15)"); + } + other => panic!("expected SchemaParse, got {other:?}"), + } + } + #[test] fn parameter_ref_to_missing_component_fails_import_loudly() { let doc = r##"{ @@ -1703,6 +1972,209 @@ mod tests { ); } + // --- OAI-10: $ref sibling keys ------------------------------------------- + + #[test] + fn ref_sibling_keys_import_with_3_0_reading_and_warn() { + let doc = r##"{ + "openapi": "3.0.3", + "info": {"title": "T", "version": "1"}, + "components": {"parameters": { + "Id": {"name": "id", "in": "path", "required": true, "schema": {"type": "string"}} + }}, + "paths": { + "/users/{id}": {"get": { + "operationId": "getUser", + "parameters": [ + {"$ref": "#/components/parameters/Id", "description": "the user id", "deprecated": false} + ], + "responses": {"200": {"content": {"application/json": {"schema": {}}}}} + }} + } + }"##; + let spec = OpenAPISpec::from_json(doc).expect("sibling keys do not fail the import"); + let item = spec.paths.get("/users/{id}").expect("path present"); + let param = &item.operations[0].1.parameters[0]; + assert_eq!(param.name, "id", "the resolved 3.0 reading wins"); + assert_eq!( + param.in_, "path", + "the resolved target's fields are what imports" + ); + } + + // --- OAI-14: top-level ignored blocks ------------------------------------ + + #[test] + fn callbacks_at_operation_level_fail_import_naming_the_feature() { + let doc = r#"{ + "openapi": "3.0.0", + "info": {"title": "T", "version": "1"}, + "paths": { + "/orders": {"post": { + "operationId": "createOrder", + "callbacks": { + "orderEvent": {"{$request.body#/callbackUrl}": {"post": { + "responses": {"200": {"content": {"application/json": {"schema": {}}}}} + }}} + }, + "responses": {"201": {"content": {"application/json": {"schema": {}}}}} + }} + } + }"#; + run_import_expect_oai14(doc, "callbacks", "post /orders"); + } + + #[test] + fn security_requirements_fail_import_naming_the_remediation() { + let doc_level = r#"{ + "openapi": "3.0.0", + "info": {"title": "T", "version": "1"}, + "security": [{"bearerAuth": []}], + "paths": {"/x": {"get": {"operationId": "x", "responses": { + "200": {"content": {"application/json": {"schema": {}}}}} + }}}} + "#; + run_import_expect_oai14(doc_level, "security", "document"); + + let op_level = r#"{ + "openapi": "3.0.0", + "info": {"title": "T", "version": "1"}, + "paths": {"/y": {"get": { + "operationId": "y", + "security": [{"apiKey": []}], + "responses": {"200": {"content": {"application/json": {"schema": {}}}}} + }}}} + "#; + run_import_expect_oai14(op_level, "security", "get /y"); + } + + #[test] + fn top_level_oneof_request_body_fails_import_naming_the_feature() { + let doc = r#"{ + "openapi": "3.0.0", + "info": {"title": "T", "version": "1"}, + "paths": { + "/x": {"post": { + "operationId": "x", + "requestBody": { + "oneOf": [ + {"content": {"application/json": {"schema": {"type": "object"}}}}, + {"content": {"text/plain": {"schema": {"type": "string"}}}} + ] + }, + "responses": {"201": {"content": {"application/json": {"schema": {}}}}} + }} + } + }"#; + match OpenAPISpec::from_json(doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains("requestBody") && message.contains("unresolvable"), + "the oneOf body (no content map) fails via the OAI-15 arm: {message}" + ); + assert!(message.contains("OAI-15"), "message was: {message}"); + } + Ok(_) => panic!("top-level oneOf requestBody must fail import loudly (OAI-14/15)"), + other => panic!("expected SchemaParse, got {other:?}"), + } + } + + fn run_import_expect_oai14(doc: &str, feature: &str, location: &str) { + let spec = OpenAPISpec::from_json(doc).expect("structural parse passes"); + let client = SharedHttpClient::new(HttpClientConfig::default()).expect("client"); + let adapter = FromOpenAPI::new( + spec, + HttpServiceConfig { + namespace: "svc".to_string(), + base_url: "https://x".to_string(), + auth: None, + default_headers: HashMap::new(), + }, + Arc::new(client), + ); + match futures::executor::block_on(adapter.import()) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.contains(feature), + "the error must name {feature}: {message}" + ); + assert!( + message.contains(location), + "the error must locate {location}: {message}" + ); + assert!(message.contains("OAI-14"), "message was: {message}"); + } + Ok(bundles) => panic!( + "{feature} at {location} must fail import loudly (OAI-14), got {} bundles", + bundles.len() + ), + Err(e) => panic!("expected SchemaParse, got {e}"), + } + } + + // --- OAI-17: bounded import error messages ------------------------------- + + #[test] + fn hundred_thousand_servers_overrides_produce_bounded_error_message() { + let mut paths = String::from("{"); + for i in 0..100_000 { + let entry = r#""/p0": {"servers": [{"url": "https://h.example.com"}], "get": {"operationId": "op0", "responses": {"200": {"content": {"application/json": {"schema": {}}}}}}},"# + .replace("p0", &format!("p{i}")) + .replace("h.example.com", &format!("h{i}.example.com")) + .replace("op0", &format!("op{i}")); + paths.push_str(&entry); + } + paths.push_str(r#""/final": {"servers": [{"url": "https://z.example.com"}]}}"#); + let doc = format!( + r#"{{"openapi": "3.0.0", "info": {{"title": "T", "version": "1"}}, "paths": {paths}}}"# + ); + let started = std::time::Instant::now(); + match OpenAPISpec::from_json(&doc) { + Err(AdapterError::SchemaParse { message }) => { + assert!( + message.len() < 4096, + "servers error must stay bounded (OAI-17), got {} bytes", + message.len() + ); + assert!( + message.contains('+') && message.contains("more"), + "the bounded join must name the suppressed count: {message}" + ); + } + Ok(_) => panic!("100k servers overrides must fail import"), + other => panic!("expected SchemaParse, got {other:?}"), + } + assert!( + started.elapsed().as_secs() < 30, + "the fixtures stay linear/bounded; this took {:?}", + started.elapsed() + ); + } + + #[test] + fn bounded_join_truncates_both_count_and_width() { + let many: Vec = (0..50).map(|i| format!("item{i}")).collect(); + let joined = bounded_join(&many); + assert!( + joined.contains("item0") && joined.contains("item7"), + "first 8 shown: {joined}" + ); + assert!(!joined.contains("item8,"), "9th item suppressed: {joined}"); + assert!(joined.contains("+42 more"), "count named: {joined}"); + + let wide = vec!["x".repeat(500)]; + let joined = bounded_join(&wide); + assert!( + joined.len() < 200, + "each item truncated to the width cap: {} bytes", + joined.len() + ); + assert!(joined.ends_with('…'), "truncation marker: {joined}"); + + let small = vec!["a".to_string(), "b".to_string()]; + assert_eq!(bounded_join(&small), "a, b", "under cap is unchanged"); + } + // --- OAI-12: YAML input normalization ----------------------------------- const OAI12_HEADER: &str = r#"