fix(adapters): input-schema enforcement + header/cookie params (OAI-02, OAI-03, OAI-07, OAI-09)
- build_request takes the op's input_schema and rejects undeclared input keys (INVALID_INPUT) before any outbound request is built; explicit `additionalProperties: true` opts into catch-all input; non-object inputs rejected (OAI-02) - in: header parameters are stamped `wire: header` in the generated input schema and sent as upstream request headers, not query params; in: cookie fails import with a clear error (OAI-03) - a spec parameter named `body` is rejected at import unconditionally (OAI-07) - FromJsonSchema::new returns Result and validates method/path template/ base_url at construction; registered visibility forced to Internal like from_openapi; module doc corrected (OAI-09) Verified: cargo test, cargo test --all-features, clippy (both feature sets, -D warnings), cargo fmt --check
This commit is contained in:
+244
-14
@@ -5,8 +5,17 @@
|
||||
//! One `HandlerRegistration` per call with a reqwest forwarding handler
|
||||
//! (the shared [`super::forward`] code path with `from_openapi`).
|
||||
//! Provenance is `FromJsonSchema` (leaf, `composition_authority: None`,
|
||||
//! `scoped_env: None`, `Internal` by default — ADR-015/022). `Sub` op
|
||||
//! type → `HandlerKind::Stream` expecting `text/event-stream` (ADR-049).
|
||||
//! `scoped_env: None`). The registered operation's visibility is forced
|
||||
//! to `Internal` (ADR-015: adapter-registered ops are composition
|
||||
//! material) — the caller's spec is not passed through verbatim.
|
||||
//! Construction validates the method, path template, and base URL
|
||||
//! eagerly (review 001 OAI-09), and the spec's declared path-template
|
||||
//! placeholders must be bound by the input schema. `Sub` op type →
|
||||
//! `HandlerKind::Stream` expecting `text/event-stream` (ADR-049).
|
||||
//!
|
||||
//! The spec's `input_schema` is the forwarding allow-list (review 001
|
||||
//! OAI-02, enforced in `super::forward`); see [`FromJsonSchema::new`]
|
||||
//! for the `wire: "header"` declaration (OAI-03).
|
||||
//!
|
||||
//! [ADR-066]: https://docs.rs/alkhttp (docs/architecture/decisions)
|
||||
|
||||
@@ -18,8 +27,9 @@ use alkcall::registry::context::OperationContext;
|
||||
use alkcall::registry::registration::{
|
||||
make_handler, make_streaming_handler, HandlerKind, HandlerRegistration, OperationProvenance,
|
||||
};
|
||||
use alkcall::registry::spec::{OperationSpec, OperationType};
|
||||
use alkcall::registry::spec::{OperationSpec, OperationType, Visibility};
|
||||
use async_trait::async_trait;
|
||||
use reqwest::Method;
|
||||
use serde_json::Value;
|
||||
|
||||
use super::forward::{forward, forward_stream, HttpServiceConfig};
|
||||
@@ -34,21 +44,143 @@ pub struct FromJsonSchema {
|
||||
}
|
||||
|
||||
impl FromJsonSchema {
|
||||
/// Register a caller-supplied [`OperationSpec`] backed by a single
|
||||
/// HTTP endpoint.
|
||||
///
|
||||
/// Construction validates eagerly (review 001 OAI-09): the HTTP
|
||||
/// method must parse, the path template must be well-formed (balanced
|
||||
/// `{}` placeholders, no empty name), and the base URL must parse as
|
||||
/// an HTTP(S) origin without userinfo — malformed values fail here
|
||||
/// instead of surfacing as a per-call `INTERNAL` error on first
|
||||
/// invoke. The registered operation's visibility is forced to
|
||||
/// `Internal` (ADR-015: adapter-registered ops are composition
|
||||
/// material; the caller's spec is not passed through verbatim).
|
||||
///
|
||||
/// The spec's `input_schema` is also the forwarding allow-list
|
||||
/// (review 001 OAI-02): input keys not declared in its `properties`
|
||||
/// are rejected at call time. Mark a property with
|
||||
/// `"wire": "header"` to route it as an upstream HTTPS header
|
||||
/// instead of a query parameter (review 001 OAI-03). Input key
|
||||
/// [`GATEWAY_BODY_KEY`](super::forward::GATEWAY_BODY_KEY) carries
|
||||
/// the request body.
|
||||
pub fn new(
|
||||
spec: OperationSpec,
|
||||
config: HttpServiceConfig,
|
||||
path_template: String,
|
||||
method: String,
|
||||
http_client: Arc<SharedHttpClient>,
|
||||
) -> Self {
|
||||
Self {
|
||||
) -> Result<Self, AdapterError> {
|
||||
validate_method(&method)?;
|
||||
validate_path_template(&path_template)?;
|
||||
Self::validate_base_url(&config.base_url)?;
|
||||
if spec_name_references_undeclared(&spec, &path_template) {
|
||||
return Err(AdapterError::SchemaParse {
|
||||
message: format!(
|
||||
"path template `{path_template}` references a placeholder the \
|
||||
operation's input_schema does not declare; the forwarded call \
|
||||
could never satisfy it"
|
||||
),
|
||||
});
|
||||
}
|
||||
if config.base_url.is_empty() {
|
||||
return Err(AdapterError::SchemaParse {
|
||||
message: "base_url must not be empty".into(),
|
||||
});
|
||||
}
|
||||
let spec = OperationSpec {
|
||||
visibility: Visibility::Internal,
|
||||
..spec
|
||||
};
|
||||
Ok(Self {
|
||||
spec,
|
||||
config,
|
||||
path_template,
|
||||
method,
|
||||
http_client,
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
fn validate_base_url(base_url: &str) -> Result<(), AdapterError> {
|
||||
let parsed = url::Url::parse(base_url).map_err(|e| AdapterError::SchemaParse {
|
||||
message: format!("invalid base_url `{base_url}`: {e}"),
|
||||
})?;
|
||||
let scheme = parsed.scheme();
|
||||
if scheme != "https" && scheme != "http" {
|
||||
return Err(AdapterError::SchemaParse {
|
||||
message: format!("base_url `{base_url}` must be an http(s) URL; `{scheme}` is not"),
|
||||
});
|
||||
}
|
||||
if !parsed.username().is_empty() || parsed.password().is_some() {
|
||||
return Err(AdapterError::SchemaParse {
|
||||
message: format!(
|
||||
"base_url `{base_url}` must not embed userinfo; credentials are \
|
||||
injected per-call from Capabilities"
|
||||
),
|
||||
});
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
fn validate_method(method: &str) -> Result<(), AdapterError> {
|
||||
if method.is_empty() {
|
||||
return Err(AdapterError::SchemaParse {
|
||||
message: "HTTP method must not be empty".into(),
|
||||
});
|
||||
}
|
||||
Method::from_bytes(method.to_ascii_uppercase().as_bytes()).map_err(|_| {
|
||||
AdapterError::SchemaParse {
|
||||
message: format!("invalid HTTP method `{method}`"),
|
||||
}
|
||||
})?;
|
||||
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
|
||||
.get("properties")
|
||||
.and_then(|p| p.as_object())
|
||||
.map(|p| p.keys().cloned().collect::<Vec<_>>())
|
||||
.unwrap_or_default();
|
||||
if properties.is_empty() {
|
||||
return false;
|
||||
}
|
||||
let mut rest = path_template;
|
||||
while let Some(start) = rest.find('{') {
|
||||
let Some(end_rel) = rest[start..].find('}') else {
|
||||
return false;
|
||||
};
|
||||
let name = &rest[start + 1..start + end_rel];
|
||||
if !properties.iter().any(|k| k == name) {
|
||||
return true;
|
||||
}
|
||||
rest = &rest[start + end_rel + 1..];
|
||||
}
|
||||
false
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
@@ -62,6 +194,7 @@ impl OperationAdapter for FromJsonSchema {
|
||||
let namespace = self.config.namespace.clone();
|
||||
let http_client = Arc::clone(&self.http_client);
|
||||
let op_type = self.spec.op_type;
|
||||
let input_schema = self.spec.input_schema.clone();
|
||||
|
||||
let error_status_codes: Vec<(u16, String)> = self
|
||||
.spec
|
||||
@@ -81,6 +214,7 @@ impl OperationAdapter for FromJsonSchema {
|
||||
let namespace = namespace.clone();
|
||||
let http_client = Arc::clone(&http_client);
|
||||
let error_status_codes = error_status_codes.clone();
|
||||
let input_schema = input_schema.clone();
|
||||
forward_stream(
|
||||
&http_client,
|
||||
&base_url,
|
||||
@@ -89,6 +223,7 @@ impl OperationAdapter for FromJsonSchema {
|
||||
&auth_scheme,
|
||||
&default_headers,
|
||||
&namespace,
|
||||
&input_schema,
|
||||
&error_status_codes,
|
||||
input,
|
||||
context,
|
||||
@@ -105,6 +240,7 @@ impl OperationAdapter for FromJsonSchema {
|
||||
let namespace = namespace.clone();
|
||||
let http_client = Arc::clone(&http_client);
|
||||
let error_status_codes = error_status_codes.clone();
|
||||
let input_schema = input_schema.clone();
|
||||
async move {
|
||||
forward(
|
||||
&http_client,
|
||||
@@ -114,6 +250,7 @@ impl OperationAdapter for FromJsonSchema {
|
||||
&auth_scheme,
|
||||
&default_headers,
|
||||
&namespace,
|
||||
&input_schema,
|
||||
&error_status_codes,
|
||||
input,
|
||||
context,
|
||||
@@ -239,7 +376,8 @@ mod tests {
|
||||
"/widgets".to_string(),
|
||||
"GET".to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
)
|
||||
.unwrap();
|
||||
let bundles = adapter.import().await.unwrap();
|
||||
assert_eq!(bundles.len(), 1);
|
||||
assert_eq!(bundles[0].spec.name, "svc/getWidget");
|
||||
@@ -256,7 +394,8 @@ mod tests {
|
||||
"/widgets".to_string(),
|
||||
"GET".to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
)
|
||||
.unwrap();
|
||||
let bundles = adapter.import().await.unwrap();
|
||||
assert!(matches!(bundles[0].handler, HandlerKind::Once(_)));
|
||||
}
|
||||
@@ -269,7 +408,8 @@ mod tests {
|
||||
"/stream".to_string(),
|
||||
"POST".to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
)
|
||||
.unwrap();
|
||||
let bundles = adapter.import().await.unwrap();
|
||||
assert!(matches!(bundles[0].handler, HandlerKind::Stream(_)));
|
||||
}
|
||||
@@ -282,7 +422,8 @@ mod tests {
|
||||
"/widgets".to_string(),
|
||||
"POST".to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
)
|
||||
.unwrap();
|
||||
let bundles = adapter.import().await.unwrap();
|
||||
assert!(matches!(bundles[0].handler, HandlerKind::Once(_)));
|
||||
}
|
||||
@@ -298,6 +439,7 @@ mod tests {
|
||||
&Some(HttpAuthScheme::Bearer),
|
||||
&HashMap::new(),
|
||||
"github",
|
||||
&serde_json::json!({"type": "object", "additionalProperties": true}),
|
||||
&serde_json::json!({"owner":"a","repo":"b"}),
|
||||
&ctx,
|
||||
)
|
||||
@@ -319,6 +461,7 @@ mod tests {
|
||||
&None,
|
||||
&HashMap::new(),
|
||||
"svc",
|
||||
&serde_json::json!({"type": "object", "additionalProperties": true}),
|
||||
&serde_json::json!({"id":42,"filter":"active"}),
|
||||
&ctx,
|
||||
)
|
||||
@@ -371,7 +514,8 @@ mod tests {
|
||||
"/data".to_string(),
|
||||
"GET".to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
)
|
||||
.unwrap();
|
||||
let bundles = adapter.import().await.unwrap();
|
||||
let registration = &bundles[0];
|
||||
let ctx = noop_context("req-10", Capabilities::new());
|
||||
@@ -401,7 +545,8 @@ mod tests {
|
||||
"/missing".to_string(),
|
||||
"GET".to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
)
|
||||
.unwrap();
|
||||
let bundles = adapter.import().await.unwrap();
|
||||
let registration = &bundles[0];
|
||||
let ctx = noop_context("req-11", Capabilities::new());
|
||||
@@ -427,7 +572,8 @@ mod tests {
|
||||
"/x".to_string(),
|
||||
"GET".to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
)
|
||||
.unwrap();
|
||||
let bundles = adapter.import().await.unwrap();
|
||||
let registration = &bundles[0];
|
||||
let ctx = noop_context("req-12", Capabilities::new());
|
||||
@@ -451,7 +597,8 @@ mod tests {
|
||||
"/stream".to_string(),
|
||||
"POST".to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
)
|
||||
.unwrap();
|
||||
let bundles = adapter.import().await.unwrap();
|
||||
let registration = &bundles[0];
|
||||
let ctx = noop_context("req-13", Capabilities::new());
|
||||
@@ -478,6 +625,7 @@ mod tests {
|
||||
&Some(HttpAuthScheme::Bearer),
|
||||
&HashMap::new(),
|
||||
"openai",
|
||||
&serde_json::json!({"type": "object", "additionalProperties": true}),
|
||||
&serde_json::json!({"body":{"prompt":"hi"}}),
|
||||
&ctx,
|
||||
)
|
||||
@@ -488,4 +636,86 @@ mod tests {
|
||||
);
|
||||
std::env::remove_var("OPENAI_API_KEY");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn malformed_method_path_template_and_base_url_fail_at_construction() {
|
||||
for (method, path_template, base_url, expected_fragment) in [
|
||||
("NOT/*A METHOD", "/x", "https://x", "invalid HTTP method"),
|
||||
("GET", "/x/{open", "https://x", "unterminated placeholder"),
|
||||
("GET", "/x/}", "https://x", "without a matching"),
|
||||
("GET", "/x", "https://u:p@x", "userinfo"),
|
||||
("GET", "/x", "ftp://x", "must be an http(s) URL"),
|
||||
("GET", "/x", "not a url", "invalid base_url"),
|
||||
] {
|
||||
let result = FromJsonSchema::new(
|
||||
test_spec("svc/x", OperationType::Query),
|
||||
test_config("svc", base_url),
|
||||
path_template.to_string(),
|
||||
method.to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
match result {
|
||||
Err(AdapterError::SchemaParse { message }) => {
|
||||
assert!(
|
||||
message.contains(expected_fragment),
|
||||
"message `{message}` lacks `{expected_fragment}`"
|
||||
);
|
||||
}
|
||||
Ok(_) => panic!(
|
||||
"malformed adapter config ({method}, {path_template}, {base_url}) must fail at construction"
|
||||
),
|
||||
Err(e) => panic!("expected SchemaParse, got {e}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn construction_forces_visibility_internal_and_validates_placeholder_binding() {
|
||||
let spec = OperationSpec::new(
|
||||
"svc/getWidget",
|
||||
OperationType::Query,
|
||||
Visibility::External,
|
||||
serde_json::json!({"type":"object","properties":{"id":{"type":"string"}}}),
|
||||
serde_json::json!({"type":"object"}),
|
||||
vec![],
|
||||
AccessControl::default(),
|
||||
None,
|
||||
);
|
||||
let adapter = FromJsonSchema::new(
|
||||
spec,
|
||||
test_config("svc", "https://api.example.com"),
|
||||
"/widgets/{id}".to_string(),
|
||||
"GET".to_string(),
|
||||
test_http_client(),
|
||||
)
|
||||
.expect("adapter builds");
|
||||
let bundles = futures::executor::block_on(adapter.import()).unwrap();
|
||||
assert_eq!(bundles[0].spec.visibility, Visibility::Internal);
|
||||
|
||||
let spec = OperationSpec::new(
|
||||
"svc/getWidget",
|
||||
OperationType::Query,
|
||||
Visibility::Internal,
|
||||
serde_json::json!({"type":"object","properties":{"widget_id":{"type":"string"}}}),
|
||||
serde_json::json!({"type":"object"}),
|
||||
vec![],
|
||||
AccessControl::default(),
|
||||
None,
|
||||
);
|
||||
let result = FromJsonSchema::new(
|
||||
spec,
|
||||
test_config("svc", "https://api.example.com"),
|
||||
"/widgets/{id}".to_string(),
|
||||
"GET".to_string(),
|
||||
test_http_client(),
|
||||
);
|
||||
match result {
|
||||
Err(AdapterError::SchemaParse { message }) => {
|
||||
assert!(message.contains("placeholder"), "message was: {message}");
|
||||
assert!(message.contains("{id}"), "message was: {message}");
|
||||
}
|
||||
Ok(_) => panic!("unbound placeholder must fail at construction"),
|
||||
Err(e) => panic!("expected SchemaParse, got {e}"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user