fix(gateway): block services/schema spec disclosure via the op path (review-002 PRJ-16)
Internal/ACL-restricted op specs were readable through
POST /call {"operation":"services/schema","input":{"name":...}}
(the MCP call/batch tools identically): the outer-name pre-checks pass
(services/schema is External) and alkcall's services_schema_handler
projects any registered spec with no visibility/ACL check of its own
(alkcall CF-004 is the complete fix there).
- GatewayDispatch.invoke/invoke_streaming now apply the GET /schema
route's is-internal + access-control checks to the meta-op's inner
name input before dispatch (404 Internal / FORBIDDEN ACL), one
interception point covering /call, /batch, /subscribe and the MCP
call/batch tools; /publish cannot reach the Query-typed meta-op
- the visibility+ACL check is one shared fn (schema_disclosure_denial)
used by the HTTP /schema route, the dispatch guard, and the MCP
schema tool, so transports cannot drift
- when CF-004 lands, this guard remains as defense-in-depth (ADR-071)
Tests: dispatch-spine guard unit tests; /call 404 + 401/403 matrix,
/batch NOT_FOUND entry, /subscribe error event; MCP call/batch tools
via services/schema with an Internal inner name (mcp feature).
Verify: cargo test (405), --all-features (523), clippy default and
--all-features --all-targets -D warnings, fmt --check — all pass.
This commit is contained in:
+173
-13
@@ -34,7 +34,6 @@ use std::sync::Arc;
|
||||
use alkcall::core::auth::Identity;
|
||||
use alkcall::protocol::wire::{CallError, ResponseEnvelope};
|
||||
use alkcall::registry::registration::OperationRegistry;
|
||||
use alkcall::registry::spec::{AccessResult, Visibility};
|
||||
use rmcp::model::{
|
||||
CallToolRequestParams, CallToolResult, Implementation, JsonObject, ListToolsResult,
|
||||
PaginatedRequestParams, ServerCapabilities, ServerInfo, Tool,
|
||||
@@ -292,23 +291,16 @@ fn json_type_name(value: &Value) -> &'static str {
|
||||
/// Symmetric with HTTP `GET /schema` (PRJ-06): internal ops are
|
||||
/// invisible (NOT_FOUND regardless of caller), ACL-forbidden ops
|
||||
/// return FORBIDDEN. The MCP `schema` tool must never return the full
|
||||
/// spec of an op the caller could not call.
|
||||
/// spec of an op the caller could not call. The check is the shared
|
||||
/// `schema_disclosure_denial` (review-002 PRJ-16) so the HTTP route,
|
||||
/// the dispatch-spine `services/schema` guard, and this tool cannot
|
||||
/// drift.
|
||||
fn schema_visibility_and_access_denial(
|
||||
registry: &OperationRegistry,
|
||||
operation: &str,
|
||||
identity: Option<&Identity>,
|
||||
) -> Option<CallError> {
|
||||
let name = operation.strip_prefix('/').unwrap_or(operation);
|
||||
let registration = registry.registration(name)?;
|
||||
if registration.spec.visibility == Visibility::Internal {
|
||||
return Some(CallError::not_found(operation));
|
||||
}
|
||||
if let AccessResult::Forbidden(message) =
|
||||
registration.spec.access_control.check(identity, None, None)
|
||||
{
|
||||
return Some(CallError::forbidden(message));
|
||||
}
|
||||
None
|
||||
crate::gateway::schema_disclosure_denial(registry, operation, identity)
|
||||
}
|
||||
|
||||
fn map_search_response(response: ResponseEnvelope, query: Option<&str>) -> CallToolResult {
|
||||
@@ -1126,6 +1118,174 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn call_tool_via_services_schema_with_internal_name_returns_not_found() {
|
||||
let mut inner = OperationRegistry::new();
|
||||
inner
|
||||
.register(HandlerRegistration::new(
|
||||
internal_spec("secret/op"),
|
||||
HandlerKind::Once(make_echo_handler()),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
None,
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
let inner = Arc::new(inner);
|
||||
let mut dispatch_registry = OperationRegistry::new();
|
||||
dispatch_registry
|
||||
.register(HandlerRegistration::new(
|
||||
internal_spec("secret/op"),
|
||||
HandlerKind::Once(make_echo_handler()),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
None,
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
dispatch_registry
|
||||
.register(HandlerRegistration::new(
|
||||
services_list_spec(),
|
||||
HandlerKind::Once(services_list_handler(Arc::clone(&inner))),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
Some(ScopedPeerEnv::empty()),
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
dispatch_registry
|
||||
.register(HandlerRegistration::new(
|
||||
services_schema_spec(),
|
||||
HandlerKind::Once(services_schema_handler(Arc::clone(&inner))),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
Some(ScopedPeerEnv::empty()),
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
let gateway = ToMcpGateway::new(dispatch(Arc::new(dispatch_registry)));
|
||||
|
||||
let mut args = Map::new();
|
||||
args.insert(
|
||||
"operation".to_string(),
|
||||
Value::String("services/schema".to_string()),
|
||||
);
|
||||
args.insert(
|
||||
"input".to_string(),
|
||||
serde_json::json!({ "name": "secret/op" }),
|
||||
);
|
||||
let result = invoke_tool(&gateway, "call", Some(args), None).await;
|
||||
assert_eq!(
|
||||
result.is_error,
|
||||
Some(true),
|
||||
"the MCP call tool must deny a spec the schema tool denies (PRJ-16)"
|
||||
);
|
||||
let structured = result.structured_content.expect("structured error present");
|
||||
assert_eq!(
|
||||
structured.get("code"),
|
||||
Some(&Value::String("NOT_FOUND".to_string()))
|
||||
);
|
||||
assert!(
|
||||
structured.get("input_schema").is_none(),
|
||||
"the internal op's spec must not leak: {structured}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn batch_tool_via_services_schema_with_internal_name_yields_not_found_entry() {
|
||||
let mut inner = OperationRegistry::new();
|
||||
inner
|
||||
.register(HandlerRegistration::new(
|
||||
internal_spec("secret/op"),
|
||||
HandlerKind::Once(make_echo_handler()),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
None,
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
inner
|
||||
.register(HandlerRegistration::new(
|
||||
external_spec("echo/run", OperationType::Query, AccessControl::default()),
|
||||
HandlerKind::Once(make_echo_handler()),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
None,
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
let inner = Arc::new(inner);
|
||||
let mut dispatch_registry = OperationRegistry::new();
|
||||
dispatch_registry
|
||||
.register(HandlerRegistration::new(
|
||||
internal_spec("secret/op"),
|
||||
HandlerKind::Once(make_echo_handler()),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
None,
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
dispatch_registry
|
||||
.register(HandlerRegistration::new(
|
||||
services_list_spec(),
|
||||
HandlerKind::Once(services_list_handler(Arc::clone(&inner))),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
Some(ScopedPeerEnv::empty()),
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
dispatch_registry
|
||||
.register(HandlerRegistration::new(
|
||||
services_schema_spec(),
|
||||
HandlerKind::Once(services_schema_handler(Arc::clone(&inner))),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
Some(ScopedPeerEnv::empty()),
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
dispatch_registry
|
||||
.register(HandlerRegistration::new(
|
||||
external_spec("echo/run", OperationType::Query, AccessControl::default()),
|
||||
HandlerKind::Once(make_echo_handler()),
|
||||
OperationProvenance::Local,
|
||||
None,
|
||||
None,
|
||||
Capabilities::new(),
|
||||
))
|
||||
.unwrap();
|
||||
let gateway = ToMcpGateway::new(dispatch(Arc::new(dispatch_registry)));
|
||||
|
||||
let mut args = Map::new();
|
||||
args.insert(
|
||||
"calls".to_string(),
|
||||
serde_json::json!([
|
||||
{ "operation": "services/schema", "input": { "name": "secret/op" } },
|
||||
{ "operation": "services/schema", "input": { "name": "echo/run" } },
|
||||
]),
|
||||
);
|
||||
let result = invoke_tool(&gateway, "batch", Some(args), None).await;
|
||||
assert_eq!(result.is_error, Some(false));
|
||||
let structured = result.structured_content.expect("structured present");
|
||||
let results = structured
|
||||
.get("results")
|
||||
.and_then(Value::as_array)
|
||||
.expect("results array");
|
||||
assert_eq!(results.len(), 2);
|
||||
assert_eq!(results[0].get("isError"), Some(&Value::Bool(true)));
|
||||
assert_eq!(
|
||||
results[0]
|
||||
.get("error")
|
||||
.and_then(|e| e.get("code"))
|
||||
.cloned()
|
||||
.unwrap_or(Value::Null),
|
||||
Value::String("NOT_FOUND".to_string())
|
||||
);
|
||||
assert_eq!(results[1].get("isError"), Some(&Value::Bool(false)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn batch_over_cap_returns_invalid_input_without_dispatching() {
|
||||
let registry = full_registry_with_ops(vec![(
|
||||
|
||||
Reference in New Issue
Block a user