fix(adapters): reject header params colliding with default_headers/credential headers (OAI-19)
build_request inserts header params first, then default_headers, then credential headers — HeaderMap::insert replaces, so a declared in: header parameter whose name matched a default or credential header silently never delivered the peer's value upstream. check_header_param_collisions runs at import (the assembly's HttpServiceConfig is visible there): Authorization on an authed namespace is rejected outright, a default_headers name match (compared case-insensitively) fails import naming both keys, and an ApiKey header_name match fails naming the credential header. Per the task's decision note: the import-time surface does see the adapter's config, so the loud point stays at import rather than first-call warn-once. Tests: Authorization+Bearer, X-Tenant/x-tenant case-insensitive default_headers, x-api-key/X-API-Key, and the no-collision clean path.
This commit is contained in:
@@ -27,8 +27,8 @@ use async_trait::async_trait;
|
|||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
|
|
||||||
use super::forward::{
|
use super::forward::{
|
||||||
bounded_join, forward, forward_stream, validate_path_template, HttpServiceConfig,
|
bounded_join, forward, forward_stream, validate_path_template, HttpAuthScheme,
|
||||||
GATEWAY_BODY_KEY, HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE,
|
HttpServiceConfig, GATEWAY_BODY_KEY, HEADER_PARAM_IN_MARKER, HEADER_PARAM_MARKER_VALUE,
|
||||||
};
|
};
|
||||||
use super::openapi_spec::{
|
use super::openapi_spec::{
|
||||||
collect_ignored_schema_keys, OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS,
|
collect_ignored_schema_keys, OpenAPISpec, Operation, Parameter, SUCCESS_RESPONSE_KEYS,
|
||||||
@@ -168,6 +168,11 @@ impl FromOpenAPI {
|
|||||||
) -> Result<Value, AdapterError> {
|
) -> Result<Value, AdapterError> {
|
||||||
let mut properties = serde_json::Map::new();
|
let mut properties = serde_json::Map::new();
|
||||||
let mut required = Vec::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
|
// OAI-13: path-item-level parameters merge into every operation
|
||||||
// under the path. The operation-level entries come second, so a
|
// under the path. The operation-level entries come second, so a
|
||||||
@@ -193,6 +198,7 @@ impl FromOpenAPI {
|
|||||||
}
|
}
|
||||||
match param.in_.as_str() {
|
match param.in_.as_str() {
|
||||||
"header" => {
|
"header" => {
|
||||||
|
self.check_header_param_collisions(¶m.name, &locator)?;
|
||||||
properties.insert(
|
properties.insert(
|
||||||
param.name.clone(),
|
param.name.clone(),
|
||||||
serde_json::json!({
|
serde_json::json!({
|
||||||
@@ -243,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<Value, AdapterError> {
|
fn build_output_schema(&self, op: &Operation) -> Result<Value, AdapterError> {
|
||||||
// Mirrors `detect_op_type`'s success-key sweep (OAI-06, OAI-13):
|
// Mirrors `detect_op_type`'s success-key sweep (OAI-06, OAI-13):
|
||||||
// a stream declared under a non-200/201 2XX key, the `2XX`
|
// a stream declared under a non-200/201 2XX key, the `2XX`
|
||||||
@@ -863,6 +923,116 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[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]
|
#[tokio::test]
|
||||||
async fn op_type_detection() {
|
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":{}}}}}}}}}"#;
|
let get_doc = r#"{"openapi":"3.0.0","info":{"title":"T","version":"1"},"paths":{"/g":{"get":{"operationId":"g","responses":{"200":{"content":{"application/json":{"schema":{}}}}}}}}}"#;
|
||||||
|
|||||||
Reference in New Issue
Block a user