From 51b21292481ba5671a37df3054597031bdb312f7 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Fri, 3 Jul 2026 23:48:07 +0000 Subject: [PATCH 1/5] feat(reborn): manifest-projected host-ingress route + fail-closed credential coherence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-derives the one still-valuable idea from the superseded earliest-reborn ingress stack (#5072/#5093/#5100/#5107, closed) fresh atop main's current `ironclaw_host_api::ingress` contract — as a single small, self-contained registry-only change, instead of rebasing a ~250-commit-stale 4-deep stack built on a `host_ingress_registry` crate main never adopted. A ProductAdapter manifest section may now declare `[[...host_ingress]]` routes, each carrying a full host-owned `IngressRouteDescriptor` (validated by host_api's own Deserialize — dotted route id, absolute path, and every policy invariant including the fail-closed floor that a `public_webhook` listener MUST require `webhook_signature`) plus the `credential_handles` that verify it. The registry does NOT re-own ingress route/policy vocabulary; it projects the descriptor and adds the binding host_api deliberately lacks — ingress credential coherence, enforced fail-closed: - every credential handle must be declared in `required_credentials` (mirrors the egress rule; ingress handles flow into the same declared set installation bindings validate against), - an auth-required route must name at least one verifying credential handle (no route nothing can authenticate), - route ids stay distinct within a section. This is exactly the seam PR #5107's review flagged as fail-open/TODO'd. The serve-layer generic mount (descriptor -> axum route + reused SharedSecretHeader/Hmac verifier) is the deliberate follow-up; main's PublicRouteMount + descriptor-fold middleware already generalize, so it is a small wiring change with no new crate. Tests: ingress credential-coherence matrix (undeclared handle, auth-required route missing credential, duplicate route id, happy projection) as focused unit tests in src/lib.rs; wire-path projection, the inherited fail-closed floor, and coherence-over-the-wire as integration tests in tests/manifest_ingestion.rs. fmt + clippy (all-features, --tests) clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../CLAUDE.md | 24 +- .../src/lib.rs | 251 +++++++++++++++++- .../tests/manifest_ingestion.rs | 69 +++++ 3 files changed, 342 insertions(+), 2 deletions(-) diff --git a/crates/ironclaw_product_adapter_registry/CLAUDE.md b/crates/ironclaw_product_adapter_registry/CLAUDE.md index e59a5b4f63a..60438b477a2 100644 --- a/crates/ironclaw_product_adapter_registry/CLAUDE.md +++ b/crates/ironclaw_product_adapter_registry/CLAUDE.md @@ -23,6 +23,23 @@ Owns ProductAdapter host-api projection contracts for IronClaw Reborn. - validate every egress credential handle is declared in `required_credentials`, - keep `(host, credential_handle)` pairs distinct. +- A section MAY declare `host_ingress` routes (`[[product_adapter..host_ingress]]`), + each carrying a full host-owned `ironclaw_host_api::IngressRouteDescriptor` + (validated by host_api's own `Deserialize` — dotted route id, absolute path, + and every policy invariant incl. the fail-closed floor that a + `public_webhook` listener MUST require `webhook_signature`) plus the + `credential_handles` that verify it. This crate does NOT re-own ingress + route/policy vocabulary — it only projects the descriptor and enforces + **ingress credential coherence**, which is fail-closed: + - every `credential_handles` entry must be declared in `required_credentials` + (mirrors the egress rule, so ingress handles flow into the same declared + set installation bindings validate against), + - an auth-required route (`IngressAuthPolicy::Required`) must name at least + one verifying credential handle, + - route ids stay distinct within a section. + Mounting these routes (descriptor → axum route + verifier) is the serve + layer's job, NOT this crate's — this crate still must not route webhooks, + resolve secret material, or bind HTTP ingress. - ProductAdapter runtime projection must keep the cross-write invariant at read time: every surfaced installation must remain valid against its registered manifest and current ProductAdapter sections. @@ -38,6 +55,11 @@ Owns ProductAdapter host-api projection contracts for IronClaw Reborn. mismatch, redacted health, and cross-write invariant maintenance. - Integration tests in `tests/manifest_ingestion.rs` cover manifest parsing, unknown-field rejection, inline-secret rejection, and egress - credential validation. + credential validation — plus host-ingress route projection over the wire, + the inherited host_api fail-closed floor (`public_webhook` without + `webhook_signature` rejected), and ingress credential coherence. The + ingress credential-coherence matrix (undeclared handle, auth-required route + missing a credential, duplicate route id, happy projection) has focused + unit coverage in `src/lib.rs` `mod tests`. - `cargo test -p ironclaw_architecture reborn_crate_dependency_boundaries_hold` pins crate dependency boundary. diff --git a/crates/ironclaw_product_adapter_registry/src/lib.rs b/crates/ironclaw_product_adapter_registry/src/lib.rs index 427e42a6f5d..00f8aed4e88 100644 --- a/crates/ironclaw_product_adapter_registry/src/lib.rs +++ b/crates/ironclaw_product_adapter_registry/src/lib.rs @@ -21,7 +21,9 @@ use ironclaw_extensions::{ HostApiManifestContext, HostApiManifestContract, HostApiMultiplicity, HostApiRefV2, ManifestSectionPath, ManifestSource, ManifestV2Error, }; -use ironclaw_host_api::{ExtensionId, HostPortCatalog}; +use ironclaw_host_api::{ + ExtensionId, HostPortCatalog, IngressAuthPolicy, IngressRouteDescriptor, IngressRouteId, +}; use ironclaw_product_adapters::{ AuthRequirement, DeclaredEgressTarget, EgressCredentialHandle, ProductAdapterCapabilities, ProductAdapterId, ProductCapabilityFlag, ProductSurfaceKind, @@ -68,6 +70,37 @@ pub fn product_adapter_sections( project_product_adapter_sections(record.raw_toml(), record.manifest()) } +/// A host-ingress route declared by a ProductAdapter manifest section, paired +/// with the credential handles that verify it. +/// +/// The route itself is the host-owned [`IngressRouteDescriptor`] vocabulary +/// (`ironclaw_host_api` owns route/policy validation, including the fail-closed +/// floor that a `PublicWebhook` listener must require `WebhookSignature`). That +/// descriptor deliberately carries **no** credential binding — host_api is +/// route/policy vocabulary only. The manifest layer is therefore where "which +/// credential handle verifies this route" is declared, and this crate makes it +/// credential-coherent against the section's `required_credentials` +/// (see [`ProductAdapterHostApiSection::validate`]). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct HostIngressRoute { + descriptor: IngressRouteDescriptor, + credential_handles: Vec, +} + +impl HostIngressRoute { + /// The host-owned, already-validated ingress route/policy descriptor. + pub fn descriptor(&self) -> &IngressRouteDescriptor { + &self.descriptor + } + + /// Credential handles that verify this route. Every handle is guaranteed to + /// be declared in the owning section's `required_credentials`, and an + /// auth-required route is guaranteed to name at least one. + pub fn credential_handles(&self) -> &[EgressCredentialHandle] { + &self.credential_handles + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct ProductAdapterHostApiSection { adapter_id: ProductAdapterId, @@ -77,6 +110,7 @@ pub struct ProductAdapterHostApiSection { auth_requirement: AuthRequirement, declared_egress: Vec, required_credentials: Vec, + host_ingress: Vec, } impl ProductAdapterHostApiSection { @@ -114,6 +148,14 @@ impl ProductAdapterHostApiSection { .into_iter() .map(|c| c.handle) .collect(); + let host_ingress = raw + .host_ingress + .into_iter() + .map(|route| HostIngressRoute { + descriptor: route.descriptor, + credential_handles: route.credential_handles, + }) + .collect(); let projected = Self { adapter_id, section, @@ -122,6 +164,7 @@ impl ProductAdapterHostApiSection { auth_requirement, declared_egress: raw.egress, required_credentials, + host_ingress, }; projected.validate()?; Ok(projected) @@ -149,6 +192,14 @@ impl ProductAdapterHostApiSection { &self.required_credentials } + /// Host-ingress routes this ProductAdapter section declares. Each carries a + /// host-owned [`IngressRouteDescriptor`] and its verifying credential + /// handles; the serve layer projects these into mounted routes. Empty for + /// sections that declare no ingress (the common case today). + pub fn host_ingress(&self) -> &[HostIngressRoute] { + &self.host_ingress + } + fn validate(&self) -> Result<(), RegistryError> { validate_auth_requirement(&self.auth_requirement)?; let mut required = BTreeSet::new(); @@ -172,6 +223,36 @@ impl ProductAdapterHostApiSection { return Err(RegistryError::DuplicateEgressTarget); } } + // Host-ingress credential coherence. Fail closed: an auth-required route + // must name at least one verifying credential handle, and every named + // handle must be declared in `required_credentials` (mirroring the + // egress rule above, so ingress handles flow into the same declared set + // installation bindings are validated against). Route ids stay distinct + // within a section so a mounted route can be addressed unambiguously. + let mut route_ids = BTreeSet::new(); + for route in &self.host_ingress { + if !route_ids.insert(route.descriptor.route_id().as_str()) { + return Err(RegistryError::DuplicateIngressRoute { + route_id: route.descriptor.route_id().clone(), + }); + } + let auth_required = matches!( + route.descriptor.policy().auth(), + IngressAuthPolicy::Required { .. } + ); + if auth_required && route.credential_handles.is_empty() { + return Err(RegistryError::IngressRouteMissingCredential { + route_id: route.descriptor.route_id().clone(), + }); + } + for handle in &route.credential_handles { + if !required.contains(handle) { + return Err(RegistryError::UndeclaredIngressCredentialHandle { + handle: handle.clone(), + }); + } + } + } Ok(()) } } @@ -344,6 +425,12 @@ pub enum RegistryError { DuplicateEgressTarget, #[error("egress references undeclared credential handle {handle}")] UndeclaredEgressCredentialHandle { handle: EgressCredentialHandle }, + #[error("host-ingress route references undeclared credential handle {handle}")] + UndeclaredIngressCredentialHandle { handle: EgressCredentialHandle }, + #[error("auth-required host-ingress route {route_id} declares no verifying credential handle")] + IngressRouteMissingCredential { route_id: IngressRouteId }, + #[error("duplicate host-ingress route {route_id}")] + DuplicateIngressRoute { route_id: IngressRouteId }, #[error("installation references unknown extension manifest {extension_id}")] UnknownManifest { extension_id: ExtensionId }, #[error("installation binds undeclared credential handle {handle}")] @@ -656,6 +743,22 @@ struct RawProductAdapterSection { required_credentials: Vec, #[serde(default)] egress: Vec, + #[serde(default)] + host_ingress: Vec, +} + +/// Manifest shape for a declared host-ingress route: the full host-owned +/// [`IngressRouteDescriptor`] (validated by `ironclaw_host_api`'s own +/// `Deserialize` — `deny_unknown_fields`, dotted route id, absolute path, and +/// all policy cross-field invariants) plus the credential handles that verify +/// it. Credential coherence against `required_credentials` is enforced in +/// [`ProductAdapterHostApiSection::validate`]. +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct RawHostIngressRoute { + descriptor: IngressRouteDescriptor, + #[serde(default)] + credential_handles: Vec, } #[derive(Debug, Deserialize)] @@ -707,3 +810,149 @@ impl RawProductAdapterAuth { Ok(requirement) } } + +#[cfg(test)] +mod tests { + //! Unit coverage for host-ingress credential coherence — the novel logic + //! this crate adds on top of host_api's already-validated ingress + //! descriptor. Descriptors are built in Rust (not TOML text) so these + //! cases are robust to serde renames; the wire path is covered end-to-end + //! in `tests/manifest_ingestion.rs`. + use super::*; + use ironclaw_host_api::{ + AllowedEffectPath, AuditTraceClass, BodyLimitPolicy, CorsPolicy, IngressAuthScheme, + IngressPolicy, IngressPolicyParts, IngressScopeSource, ListenerClass, NetworkMethod, + RateLimitPolicy, RateLimitScope, StreamingMode, WebSocketOriginPolicy, + }; + use serde::Serialize; + use std::num::{NonZeroU32, NonZeroU64}; + + /// A fail-closed public-webhook descriptor mirroring the values Slack's + /// `slack_events_policy()` uses, parameterized by route id. + fn webhook_descriptor(route_id: &str) -> IngressRouteDescriptor { + let policy = IngressPolicy::new(IngressPolicyParts { + listener_class: ListenerClass::PublicWebhook, + auth: IngressAuthPolicy::Required { + schemes: vec![IngressAuthScheme::WebhookSignature], + }, + scope_source: IngressScopeSource::HostResolved, + body_limit: BodyLimitPolicy::Limited { + max_bytes: NonZeroU64::new(262_144).expect("nonzero"), + }, + rate_limit: RateLimitPolicy::Limited { + scope: RateLimitScope::Global, + max_requests: NonZeroU32::new(600).expect("nonzero"), + window_seconds: NonZeroU32::new(60).expect("nonzero"), + }, + cors: CorsPolicy::NotApplicable, + websocket_origin: WebSocketOriginPolicy::NotApplicable, + streaming: StreamingMode::None, + audit: AuditTraceClass::PublicCallback, + effect_path: AllowedEffectPath::ProductWorkflow, + }) + .expect("policy validates"); + IngressRouteDescriptor::new( + route_id, + NetworkMethod::Post, + "/webhooks/telegram/updates", + policy, + ) + .expect("descriptor validates") + } + + #[derive(Serialize)] + struct RouteFixture { + descriptor: IngressRouteDescriptor, + credential_handles: Vec, + } + + /// Build a ProductAdapter section `toml::Value` with a valid base and the + /// given host-ingress routes, then run it through the real projection. + fn project(routes: Vec) -> Result { + let mut value: toml::Value = toml::from_str( + r#" +surface_kind = "external_channel" +[auth] +kind = "shared_secret_header" +header_name = "X-Telegram-Bot-Api-Secret-Token" +[capabilities] +flags = ["inbound_messages"] +[[required_credentials]] +handle = "telegram_bot_token" +"#, + ) + .expect("base section parses"); + let host_ingress = toml::Value::try_from(routes).expect("routes serialize"); + value + .as_table_mut() + .expect("section is a table") + .insert("host_ingress".to_string(), host_ingress); + + let extension_id = ExtensionId::new("telegram-v2").expect("extension id"); + let section = ManifestSectionPath::new("product_adapter.inbound").expect("section path"); + ProductAdapterHostApiSection::from_value(&extension_id, section, value) + } + + fn route(route_id: &str, credential_handles: &[&str]) -> RouteFixture { + RouteFixture { + descriptor: webhook_descriptor(route_id), + credential_handles: credential_handles.iter().map(|h| h.to_string()).collect(), + } + } + + #[test] + fn host_ingress_route_projects_descriptor_and_handles() { + let section = project(vec![route("telegram.updates", &["telegram_bot_token"])]) + .expect("valid section projects"); + assert_eq!(section.host_ingress().len(), 1); + let projected = §ion.host_ingress()[0]; + assert_eq!( + projected.descriptor().route_id().as_str(), + "telegram.updates" + ); + assert_eq!( + projected.descriptor().route_pattern().as_str(), + "/webhooks/telegram/updates" + ); + assert_eq!(projected.credential_handles().len(), 1); + assert_eq!( + projected.credential_handles()[0].as_str(), + "telegram_bot_token" + ); + } + + #[test] + fn host_ingress_undeclared_credential_handle_rejected() { + let err = project(vec![route("telegram.updates", &["not_declared_token"])]) + .expect_err("undeclared handle must reject"); + assert!( + matches!(err, RegistryError::UndeclaredIngressCredentialHandle { .. }), + "got {err:?}" + ); + } + + #[test] + fn host_ingress_auth_required_route_needs_credential() { + // Fail closed: an auth-required route with no verifying credential + // handle must reject, not mount a route nothing can authenticate. + let err = project(vec![route("telegram.updates", &[])]) + .expect_err("auth-required route without a credential must reject"); + assert!( + matches!(err, RegistryError::IngressRouteMissingCredential { .. }), + "got {err:?}" + ); + } + + #[test] + fn host_ingress_duplicate_route_id_rejected() { + let err = project(vec![ + route("telegram.updates", &["telegram_bot_token"]), + route("telegram.updates", &["telegram_bot_token"]), + ]) + .expect_err("duplicate route id must reject"); + assert!( + matches!(err, RegistryError::DuplicateIngressRoute { .. }), + "got {err:?}" + ); + } +} diff --git a/crates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rs b/crates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rs index 88bb8162950..f6f24f5bf5e 100644 --- a/crates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rs +++ b/crates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rs @@ -149,6 +149,75 @@ fn rejects_auth_header_injection_shape() { )); } +/// A valid public-webhook host-ingress route declaration in manifest wire form. +/// `{cred}` lets a test swap the verifying credential handle; `{scheme}` lets a +/// test swap the auth scheme to exercise host_api's fail-closed floor. +fn host_ingress_fragment(cred: &str, scheme: &str) -> String { + format!( + r#" +[[product_adapter.inbound.host_ingress]] +credential_handles = ["{cred}"] +descriptor = {{ route_id = "telegram.updates", method = "post", route_pattern = "/webhooks/telegram/updates", policy = {{ listener_class = "public_webhook", auth = {{ type = "required", schemes = ["{scheme}"] }}, scope_source = "host_resolved", body_limit = {{ type = "limited", max_bytes = 262144 }}, rate_limit = {{ type = "limited", scope = "global", max_requests = 600, window_seconds = 60 }}, cors = "not_applicable", websocket_origin = "not_applicable", streaming = "none", audit = "public_callback", effect_path = {{ type = "product_workflow" }} }} }} +"# + ) +} + +#[test] +fn parses_host_ingress_route_from_manifest() { + let record = parse(&manifest(&host_ingress_fragment( + "telegram_bot_token", + "webhook_signature", + ))) + .unwrap(); + let adapters = product_adapter_sections(&record).unwrap(); + let routes = adapters[0].host_ingress(); + assert_eq!(routes.len(), 1); + assert_eq!( + routes[0].descriptor().route_id().as_str(), + "telegram.updates" + ); + assert_eq!( + routes[0].descriptor().route_pattern().as_str(), + "/webhooks/telegram/updates" + ); + assert_eq!( + routes[0].credential_handles()[0].as_str(), + "telegram_bot_token" + ); +} + +#[test] +fn rejects_host_ingress_public_webhook_without_webhook_signature() { + // host_api's own fail-closed floor: a `public_webhook` listener MUST + // require `webhook_signature`. Declaring `bearer_token` instead must be + // rejected while projecting the manifest — the manifest layer cannot + // weaken the descriptor's built-in verification requirement. + let raw = manifest(&host_ingress_fragment("telegram_bot_token", "bearer_token")); + let err = parse(&raw).unwrap_err(); + assert!( + matches!(err, RegistryError::Manifest(_)), + "expected the host_api listener/auth invariant to reject, got {err:?}" + ); +} + +#[test] +fn rejects_host_ingress_credential_handle_not_declared_as_required() { + // Ingress credential coherence over the wire: a route may only be verified + // by a credential the section declares in `required_credentials`. + let raw = manifest(&host_ingress_fragment( + "undeclared_token", + "webhook_signature", + )); + let err = parse(&raw).unwrap_err(); + assert!( + matches!( + err, + RegistryError::UndeclaredIngressCredentialHandle { .. } | RegistryError::Manifest(_) + ), + "got {err:?}" + ); +} + #[test] fn rejects_real_derived_adapter_id_that_exceeds_limit() { let extension_id = "a".repeat(128); From 428bdc1551351cc99511aa0a699902c8ceb46ad0 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sat, 4 Jul 2026 00:45:24 +0000 Subject: [PATCH 2/5] review: reject public host-ingress routes that declare credentials; robustness + type nits Addresses bot review feedback on #5625: - gemini (security): a public (no-auth) host-ingress route declaring a credential handle is incoherent and misleading (a reader would assume it is authenticated). Add PublicIngressRouteHasCredential and reject it, making the coherence rule symmetric with the auth-required-needs-credential rule. Adds two unit tests (rejection + the valid no-credential complement). - gemini (types): store &IngressRouteId in the route-id dedup set instead of downgrading to &str. - copilot/coderabbit (tests): the fail-closed-floor wire test now accepts both RegistryError::Manifest(_) and ManifestSectionParse, so it pins the fail-closed behavior rather than the error-routing path. - copilot (naming): documented on HostIngressRoute::credential_handles why the shared EgressCredentialHandle newtype is reused for ingress (type-placement rule) and that its Display leaks no "egress" wording. cargo fmt + clippy (all-features, --tests) clean; registry tests green (6 unit incl. the 2 new public-route cases + 9 manifest + 13 contract). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/lib.rs | 124 +++++++++++++++--- .../tests/manifest_ingestion.rs | 10 +- 2 files changed, 112 insertions(+), 22 deletions(-) diff --git a/crates/ironclaw_product_adapter_registry/src/lib.rs b/crates/ironclaw_product_adapter_registry/src/lib.rs index 00f8aed4e88..95f79b7cc11 100644 --- a/crates/ironclaw_product_adapter_registry/src/lib.rs +++ b/crates/ironclaw_product_adapter_registry/src/lib.rs @@ -94,8 +94,15 @@ impl HostIngressRoute { } /// Credential handles that verify this route. Every handle is guaranteed to - /// be declared in the owning section's `required_credentials`, and an - /// auth-required route is guaranteed to name at least one. + /// be declared in the owning section's `required_credentials`; an + /// auth-required route names at least one, and a public (no-auth) route + /// names none. + /// + /// The handle type is [`EgressCredentialHandle`] — the single credential- + /// handle newtype `ironclaw_product_adapters` owns. It is reused here rather + /// than mirrored into an ingress-specific type (per the type-placement + /// rule); its `Display` renders only the handle string, so no "egress" + /// wording leaks into ingress error messages. pub fn credential_handles(&self) -> &[EgressCredentialHandle] { &self.credential_handles } @@ -223,27 +230,41 @@ impl ProductAdapterHostApiSection { return Err(RegistryError::DuplicateEgressTarget); } } - // Host-ingress credential coherence. Fail closed: an auth-required route - // must name at least one verifying credential handle, and every named - // handle must be declared in `required_credentials` (mirroring the - // egress rule above, so ingress handles flow into the same declared set - // installation bindings are validated against). Route ids stay distinct - // within a section so a mounted route can be addressed unambiguously. - let mut route_ids = BTreeSet::new(); + // Host-ingress credential coherence, fail closed. A route's declared + // verifying credentials must line up with whether it is actually + // authenticated, and every named handle must be declared in + // `required_credentials` (mirroring the egress rule above, so ingress + // handles flow into the same declared set installation bindings are + // validated against). Route ids stay distinct within a section so a + // mounted route can be addressed unambiguously. + let mut route_ids: BTreeSet<&IngressRouteId> = BTreeSet::new(); for route in &self.host_ingress { - if !route_ids.insert(route.descriptor.route_id().as_str()) { + let route_id = route.descriptor.route_id(); + if !route_ids.insert(route_id) { return Err(RegistryError::DuplicateIngressRoute { - route_id: route.descriptor.route_id().clone(), + route_id: route_id.clone(), }); } - let auth_required = matches!( - route.descriptor.policy().auth(), - IngressAuthPolicy::Required { .. } - ); - if auth_required && route.credential_handles.is_empty() { - return Err(RegistryError::IngressRouteMissingCredential { - route_id: route.descriptor.route_id().clone(), - }); + match route.descriptor.policy().auth() { + // An auth-required route with no verifying credential is a route + // nothing could authenticate — reject it. + IngressAuthPolicy::Required { .. } => { + if route.credential_handles.is_empty() { + return Err(RegistryError::IngressRouteMissingCredential { + route_id: route_id.clone(), + }); + } + } + // A public (no-auth) route is verified by nothing, so declaring a + // credential handle on it is incoherent and misleading — a reader + // would assume the route is authenticated by that credential. + IngressAuthPolicy::Public { .. } => { + if !route.credential_handles.is_empty() { + return Err(RegistryError::PublicIngressRouteHasCredential { + route_id: route_id.clone(), + }); + } + } } for handle in &route.credential_handles { if !required.contains(handle) { @@ -429,6 +450,10 @@ pub enum RegistryError { UndeclaredIngressCredentialHandle { handle: EgressCredentialHandle }, #[error("auth-required host-ingress route {route_id} declares no verifying credential handle")] IngressRouteMissingCredential { route_id: IngressRouteId }, + #[error( + "public host-ingress route {route_id} declares a verifying credential handle but is not authenticated" + )] + PublicIngressRouteHasCredential { route_id: IngressRouteId }, #[error("duplicate host-ingress route {route_id}")] DuplicateIngressRoute { route_id: IngressRouteId }, #[error("installation references unknown extension manifest {extension_id}")] @@ -821,8 +846,8 @@ mod tests { use super::*; use ironclaw_host_api::{ AllowedEffectPath, AuditTraceClass, BodyLimitPolicy, CorsPolicy, IngressAuthScheme, - IngressPolicy, IngressPolicyParts, IngressScopeSource, ListenerClass, NetworkMethod, - RateLimitPolicy, RateLimitScope, StreamingMode, WebSocketOriginPolicy, + IngressJustification, IngressPolicy, IngressPolicyParts, IngressScopeSource, ListenerClass, + NetworkMethod, RateLimitPolicy, RateLimitScope, StreamingMode, WebSocketOriginPolicy, }; use serde::Serialize; use std::num::{NonZeroU32, NonZeroU64}; @@ -860,6 +885,35 @@ mod tests { .expect("descriptor validates") } + /// A valid public (no-auth) route, mirroring the SSO login mount's policy + /// combination (LocalGateway + Public + PublicRoute + NoEffect). + fn public_descriptor(route_id: &str) -> IngressRouteDescriptor { + let policy = IngressPolicy::new(IngressPolicyParts { + listener_class: ListenerClass::LocalGateway, + auth: IngressAuthPolicy::Public { + justification: IngressJustification::new("ingress", "public test route") + .expect("justification"), + }, + scope_source: IngressScopeSource::PublicRoute, + body_limit: BodyLimitPolicy::Limited { + max_bytes: NonZeroU64::new(4096).expect("nonzero"), + }, + rate_limit: RateLimitPolicy::Limited { + scope: RateLimitScope::PerIp, + max_requests: NonZeroU32::new(60).expect("nonzero"), + window_seconds: NonZeroU32::new(60).expect("nonzero"), + }, + cors: CorsPolicy::SameOriginOnly, + websocket_origin: WebSocketOriginPolicy::NotApplicable, + streaming: StreamingMode::None, + audit: AuditTraceClass::PublicCallback, + effect_path: AllowedEffectPath::NoEffect, + }) + .expect("public policy validates"); + IngressRouteDescriptor::new(route_id, NetworkMethod::Post, "/public/callback", policy) + .expect("descriptor validates") + } + #[derive(Serialize)] struct RouteFixture { descriptor: IngressRouteDescriptor, @@ -955,4 +1009,32 @@ handle = "telegram_bot_token" "got {err:?}" ); } + + #[test] + fn host_ingress_public_route_must_not_declare_credentials() { + // Fail closed on the dual of the auth-required rule: a public (no-auth) + // route is verified by nothing, so declaring a credential handle on it + // is incoherent and would mislead a reader into assuming it is + // authenticated. + let err = project(vec![RouteFixture { + descriptor: public_descriptor("public.callback"), + credential_handles: vec!["telegram_bot_token".to_string()], + }]) + .expect_err("public route with a credential handle must reject"); + assert!( + matches!(err, RegistryError::PublicIngressRouteHasCredential { .. }), + "got {err:?}" + ); + } + + #[test] + fn host_ingress_public_route_without_credentials_projects() { + // The complement: a public route that declares no credentials is valid. + let section = project(vec![RouteFixture { + descriptor: public_descriptor("public.callback"), + credential_handles: vec![], + }]) + .expect("public route with no credentials projects"); + assert_eq!(section.host_ingress().len(), 1); + } } diff --git a/crates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rs b/crates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rs index f6f24f5bf5e..4062a319fb6 100644 --- a/crates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rs +++ b/crates/ironclaw_product_adapter_registry/tests/manifest_ingestion.rs @@ -194,8 +194,16 @@ fn rejects_host_ingress_public_webhook_without_webhook_signature() { // weaken the descriptor's built-in verification requirement. let raw = manifest(&host_ingress_fragment("telegram_bot_token", "bearer_token")); let err = parse(&raw).unwrap_err(); + // The invalid descriptor is rejected while deserializing the section, which + // may surface either as a stringified `Manifest` error (via the host-api + // contract validator) or as a typed `ManifestSectionParse` (via the final + // projection) depending on which projection runs first — accept both so the + // test pins the fail-closed behavior, not the error-routing path. assert!( - matches!(err, RegistryError::Manifest(_)), + matches!( + err, + RegistryError::Manifest(_) | RegistryError::ManifestSectionParse { .. } + ), "expected the host_api listener/auth invariant to reject, got {err:?}" ); } From 4bfdc6be0ab6604e075633d4b9de84a96c538a7e Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sat, 4 Jul 2026 00:21:19 +0000 Subject: [PATCH 3/5] feat(reborn): project Slack ingress routes from the manifest, delete the Rust policy literals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Makes the manifest-driven ingress contract from #5625 load-bearing: Slack's two inbound routes are now declared as data in the bundled extension manifest and projected into descriptors at serve time, instead of being hand-written Rust policy literals. This is the real example of usage the mechanism needed. - assets/slack/manifest.toml declares `[[product_adapter.inbound.host_ingress]]` for `slack.events` and `slack.commands`, each naming `slack_bot_token` as its verifying credential (fail-closed credential coherence, enforced by the registry). - slack_serve::slack_events_route_descriptors / slack_commands_route_descriptors now project their descriptor from the bundled manifest via a new generic helper (composition::host_ingress::bundled_host_ingress_descriptor), and the two hardcoded slack_events_policy() / slack_commands_policy() literals are deleted. - Only the declarative descriptor moved to the manifest. The axum handler and the HMAC verifier (behavior) stay in Rust — a manifest cannot carry behavior. The descriptor is validated by ironclaw_host_api on deserialize (dotted route id, absolute path, and the fail-closed floor that a public_webhook listener MUST require webhook_signature) and by ironclaw_product_adapter_registry for ingress credential coherence — so a manifest cannot declare a weaker route than the Rust literal did. Stacked on #5625 (needs ProductAdapterHostApiSection::host_ingress()). This also answers the "unused API" question on #5625: the accessor now has a production consumer that deletes per-channel Rust. Tests: two behavior-preserving equivalence guards assert the manifest-projected descriptor is byte-for-byte identical to the pre-migration literal (slack_{events,commands}_route_descriptor_matches_manifest_projection); the existing 377 Slack serve/e2e/handler tests still drive real signed and forged webhooks through the projected mounts (all pass). fmt + clippy (slack-v2-host-beta,webui-v2-beta,libsql; --tests) clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../assets/slack/manifest.toml | 12 ++ .../src/available_extensions.rs | 8 + .../src/host_ingress.rs | 66 ++++++ crates/ironclaw_reborn_composition/src/lib.rs | 2 + .../src/slack_serve.rs | 194 ++++++++---------- 5 files changed, 178 insertions(+), 104 deletions(-) create mode 100644 crates/ironclaw_reborn_composition/src/host_ingress.rs diff --git a/crates/ironclaw_first_party_extensions/assets/slack/manifest.toml b/crates/ironclaw_first_party_extensions/assets/slack/manifest.toml index b9205abc3b4..ec1ee88d505 100644 --- a/crates/ironclaw_first_party_extensions/assets/slack/manifest.toml +++ b/crates/ironclaw_first_party_extensions/assets/slack/manifest.toml @@ -35,3 +35,15 @@ handle = "slack_bot_token" [[product_adapter.inbound.egress]] host = "slack.com" credential_handle = "slack_bot_token" + +# Host-ingress routes projected by the serve layer (ironclaw_reborn_composition +# ::slack_serve). The route path/method/policy live here as data — the axum +# handler + HMAC verifier stay in Rust. Credential coherence: each route is +# verified by `slack_bot_token`, which is declared in required_credentials above. +[[product_adapter.inbound.host_ingress]] +credential_handles = ["slack_bot_token"] +descriptor = { route_id = "slack.events", method = "post", route_pattern = "/webhooks/slack/events", policy = { listener_class = "public_webhook", auth = { type = "required", schemes = ["webhook_signature"] }, scope_source = "host_resolved", body_limit = { type = "limited", max_bytes = 1048576 }, rate_limit = { type = "limited", scope = "global", max_requests = 12000, window_seconds = 60 }, cors = "not_applicable", websocket_origin = "not_applicable", streaming = "none", audit = "public_callback", effect_path = { type = "product_workflow" } } } + +[[product_adapter.inbound.host_ingress]] +credential_handles = ["slack_bot_token"] +descriptor = { route_id = "slack.commands", method = "post", route_pattern = "/webhooks/slack/commands", policy = { listener_class = "public_webhook", auth = { type = "required", schemes = ["webhook_signature"] }, scope_source = "host_resolved", body_limit = { type = "limited", max_bytes = 16384 }, rate_limit = { type = "limited", scope = "global", max_requests = 6000, window_seconds = 60 }, cors = "not_applicable", websocket_origin = "not_applicable", streaming = "none", audit = "public_callback", effect_path = { type = "product_workflow" } } } diff --git a/crates/ironclaw_reborn_composition/src/available_extensions.rs b/crates/ironclaw_reborn_composition/src/available_extensions.rs index bca219c5fc5..7fb764f688e 100644 --- a/crates/ironclaw_reborn_composition/src/available_extensions.rs +++ b/crates/ironclaw_reborn_composition/src/available_extensions.rs @@ -548,6 +548,14 @@ pub(crate) fn slack_manifest_digest() -> String { sha256_digest_token(SLACK_MANIFEST.as_bytes()) } +/// The bundled Slack extension manifest TOML. The serve layer projects the +/// Slack host-ingress route descriptors from this manifest rather than from +/// Rust literals (see `slack_serve::slack_events_route_descriptors`). +#[cfg(feature = "slack-v2-host-beta")] +pub(crate) fn slack_manifest_toml() -> &'static str { + SLACK_MANIFEST +} + pub(crate) fn nearai_mcp_manifest_toml_for_config( config: Option<&NearAiMcpBootstrapConfig>, ) -> Result { diff --git a/crates/ironclaw_reborn_composition/src/host_ingress.rs b/crates/ironclaw_reborn_composition/src/host_ingress.rs new file mode 100644 index 00000000000..dff17e20aa5 --- /dev/null +++ b/crates/ironclaw_reborn_composition/src/host_ingress.rs @@ -0,0 +1,66 @@ +//! Project host-ingress route descriptors from a bundled extension manifest. +//! +//! This is the read side of the manifest-driven ingress contract: an +//! extension's `manifest.toml` declares its inbound HTTP routes as data +//! (`[[product_adapter..host_ingress]]`), and this helper projects the +//! validated [`IngressRouteDescriptor`] the serve layer mounts. The route's +//! *policy/shape* is data in the manifest; the axum handler and the webhook +//! verifier (behavior) stay in the owning serve module. +//! +//! `ironclaw_product_adapter_registry` owns the parsing/validation and the +//! fail-closed credential coherence (every route names a verifying credential +//! declared in `required_credentials`); this module only selects a route by id +//! and hands back its descriptor. + +use ironclaw_extensions::ManifestSource; +use ironclaw_host_api::HostPortCatalog; +use ironclaw_host_api::ingress::IngressRouteDescriptor; +use ironclaw_product_adapter_registry::{ + parse_product_adapter_manifest_record, product_adapter_sections, +}; +use thiserror::Error; + +#[derive(Debug, Error)] +pub(crate) enum HostIngressProjectionError { + #[error("bundled manifest failed to project host-ingress routes: {reason}")] + Projection { reason: String }, + #[error("bundled manifest declares no host-ingress route {route_id}")] + RouteNotDeclared { route_id: String }, +} + +/// Project the [`IngressRouteDescriptor`] for `route_id` from a bundled +/// (host-compiled) extension manifest. +/// +/// The descriptor is validated by `ironclaw_host_api` on deserialize (dotted +/// route id, absolute path, and every policy invariant including the +/// fail-closed floor that a `public_webhook` listener must require +/// `webhook_signature`), and by the registry for ingress credential coherence. +/// Intended for compile-time bundled manifests, so callers may treat a failure +/// as a startup invariant violation. +pub(crate) fn bundled_host_ingress_descriptor( + manifest_toml: &str, + route_id: &str, +) -> Result { + let record = parse_product_adapter_manifest_record( + manifest_toml, + ManifestSource::HostBundled, + &HostPortCatalog::empty(), + None, + ) + .map_err(|error| HostIngressProjectionError::Projection { + reason: error.to_string(), + })?; + let sections = product_adapter_sections(&record).map_err(|error| { + HostIngressProjectionError::Projection { + reason: error.to_string(), + } + })?; + sections + .iter() + .flat_map(|section| section.host_ingress()) + .find(|route| route.descriptor().route_id().as_str() == route_id) + .map(|route| route.descriptor().clone()) + .ok_or_else(|| HostIngressProjectionError::RouteNotDeclared { + route_id: route_id.to_string(), + }) +} diff --git a/crates/ironclaw_reborn_composition/src/lib.rs b/crates/ironclaw_reborn_composition/src/lib.rs index 48f7eb6fba6..a9f8f4f4e28 100644 --- a/crates/ironclaw_reborn_composition/src/lib.rs +++ b/crates/ironclaw_reborn_composition/src/lib.rs @@ -103,6 +103,8 @@ mod trajectory_observer; pub use auth_prompt::{AuthChallengeProvider, AuthChallengeView, BlockedAuthFlowCanceller}; #[cfg(feature = "slack-v2-host-beta")] mod delivered_gate_routing; +#[cfg(feature = "slack-v2-host-beta")] +mod host_ingress; #[cfg(feature = "root-llm-provider")] mod provider_admin; #[cfg(feature = "root-llm-provider")] diff --git a/crates/ironclaw_reborn_composition/src/slack_serve.rs b/crates/ironclaw_reborn_composition/src/slack_serve.rs index 161329ef581..31b2277d8dd 100644 --- a/crates/ironclaw_reborn_composition/src/slack_serve.rs +++ b/crates/ironclaw_reborn_composition/src/slack_serve.rs @@ -6,7 +6,6 @@ //! `REBORN_SLACK_ENABLED`) and supplies a preconfigured native adapter runner. use std::future::Future; -use std::num::{NonZeroU32, NonZeroU64}; use std::pin::Pin; use std::sync::Arc; @@ -18,13 +17,7 @@ use axum::{ response::{IntoResponse, Response}, routing::post, }; -use ironclaw_host_api::NetworkMethod; -use ironclaw_host_api::ingress::{ - AllowedEffectPath, AuditTraceClass, BodyLimitPolicy, CorsPolicy, IngressAuthPolicy, - IngressAuthScheme, IngressPolicy, IngressPolicyParts, IngressRouteDescriptor, - IngressScopeSource, ListenerClass, RateLimitPolicy, RateLimitScope, StreamingMode, - WebSocketOriginPolicy, -}; +use ironclaw_host_api::ingress::IngressRouteDescriptor; use ironclaw_product_adapters::ProtocolAuthEvidence; use ironclaw_wasm_product_adapters::{ ImmediateAckWorkflowObserver, NativeProductAdapterRunner, RunnerError, WebhookProcessOutcome, @@ -53,15 +46,9 @@ mod handler_tests; pub const SLACK_EVENTS_PATH: &str = "/webhooks/slack/events"; const SLACK_EVENTS_ROUTE_ID: &str = "slack.events"; -const SLACK_EVENTS_BODY_LIMIT_BYTES: NonZeroU64 = NonZeroU64::new(1024 * 1024).unwrap(); // safety: 1 MiB is a non-zero literal. -const SLACK_EVENTS_MAX_REQUESTS: NonZeroU32 = NonZeroU32::new(12_000).unwrap(); // safety: 12,000 requests is a non-zero literal. -const SLACK_EVENTS_RATE_WINDOW_SECONDS: NonZeroU32 = NonZeroU32::new(60).unwrap(); // safety: 60 seconds is a non-zero literal. pub const SLACK_COMMANDS_PATH: &str = "/webhooks/slack/commands"; const SLACK_COMMANDS_ROUTE_ID: &str = "slack.commands"; -const SLACK_COMMANDS_BODY_LIMIT_BYTES: NonZeroU64 = NonZeroU64::new(16 * 1024).unwrap(); // safety: 16 KiB is a non-zero literal; slash-command forms are tiny. -const SLACK_COMMANDS_MAX_REQUESTS: NonZeroU32 = NonZeroU32::new(6_000).unwrap(); // safety: 6,000 requests is a non-zero literal. -const SLACK_COMMANDS_RATE_WINDOW_SECONDS: NonZeroU32 = NonZeroU32::new(60).unwrap(); // safety: 60 seconds is a non-zero literal. pub trait SlackEventsWebhookDispatcher: Send + Sync { fn verify_webhook_auth( @@ -252,44 +239,29 @@ pub fn slack_events_route_mount(state: SlackEventsRouteState) -> PublicRouteMoun } pub fn slack_events_route_descriptors() -> Vec { - let descriptor = IngressRouteDescriptor::new( - SLACK_EVENTS_ROUTE_ID, - NetworkMethod::Post, - SLACK_EVENTS_PATH, - slack_events_policy(), - ) - .expect("Slack events route descriptor must validate at startup"); // safety: route id/path are crate-local literals and policy is built by sibling helper. - vec![descriptor] + vec![bundled_slack_ingress_descriptor(SLACK_EVENTS_ROUTE_ID)] } -fn slack_events_policy() -> IngressPolicy { - IngressPolicy::new(IngressPolicyParts { - listener_class: ListenerClass::PublicWebhook, - auth: IngressAuthPolicy::Required { - schemes: vec![IngressAuthScheme::WebhookSignature], - }, - scope_source: IngressScopeSource::HostResolved, - body_limit: BodyLimitPolicy::Limited { - max_bytes: SLACK_EVENTS_BODY_LIMIT_BYTES, - }, - rate_limit: RateLimitPolicy::Limited { - // Coarse pre-auth abuse guard. Keep this well above the - // per-installation quota below: the route-level service adds a - // second post-verification bucket keyed by the resolved Slack - // tenant/installation, because Slack events can arrive from shared - // Slack egress pools and one workspace must not consume the budget - // for every tenant. - scope: RateLimitScope::Global, - max_requests: SLACK_EVENTS_MAX_REQUESTS, - window_seconds: SLACK_EVENTS_RATE_WINDOW_SECONDS, - }, - cors: CorsPolicy::NotApplicable, - websocket_origin: WebSocketOriginPolicy::NotApplicable, - streaming: StreamingMode::None, - audit: AuditTraceClass::PublicCallback, - effect_path: AllowedEffectPath::ProductWorkflow, +/// Project a Slack host-ingress route descriptor from the bundled Slack +/// extension manifest. +/// +/// The route's path/method/policy are declared as data in +/// `assets/slack/manifest.toml` (`[[product_adapter.inbound.host_ingress]]`) +/// and validated by `ironclaw_host_api` (incl. the fail-closed floor that a +/// `public_webhook` listener must require `webhook_signature`) plus +/// `ironclaw_product_adapter_registry` (ingress credential coherence). Only the +/// declarative descriptor lives in the manifest — the axum handler and the HMAC +/// verifier stay in this module. Panics if the bundled manifest does not +/// declare the route: `SLACK_MANIFEST` is a compile-time constant, so a missing +/// route is a build-time invariant violation, surfaced at startup. +fn bundled_slack_ingress_descriptor(route_id: &str) -> IngressRouteDescriptor { + crate::host_ingress::bundled_host_ingress_descriptor( + crate::available_extensions::slack_manifest_toml(), + route_id, + ) + .unwrap_or_else(|error| { + panic!("bundled Slack manifest must declare host-ingress route {route_id}: {error}") }) - .expect("Slack events ingress policy must validate") // safety: policy combines validated constants and host-resolved webhook-signature scope. } async fn slack_events_handler( @@ -536,41 +508,7 @@ pub fn slack_commands_route_mount(state: SlackCommandsRouteState) -> PublicRoute } pub fn slack_commands_route_descriptors() -> Vec { - let descriptor = IngressRouteDescriptor::new( - SLACK_COMMANDS_ROUTE_ID, - NetworkMethod::Post, - SLACK_COMMANDS_PATH, - slack_commands_policy(), - ) - .expect("Slack commands route descriptor must validate at startup"); // safety: route id/path are crate-local literals and policy is built by sibling helper. - vec![descriptor] -} - -fn slack_commands_policy() -> IngressPolicy { - IngressPolicy::new(IngressPolicyParts { - listener_class: ListenerClass::PublicWebhook, - auth: IngressAuthPolicy::Required { - schemes: vec![IngressAuthScheme::WebhookSignature], - }, - scope_source: IngressScopeSource::HostResolved, - body_limit: BodyLimitPolicy::Limited { - max_bytes: SLACK_COMMANDS_BODY_LIMIT_BYTES, - }, - rate_limit: RateLimitPolicy::Limited { - // Coarse pre-auth abuse guard, mirroring the events route. A second - // post-verification bucket keyed by the resolved installation is - // applied inside `SlackIngressService::resolve_command`. - scope: RateLimitScope::Global, - max_requests: SLACK_COMMANDS_MAX_REQUESTS, - window_seconds: SLACK_COMMANDS_RATE_WINDOW_SECONDS, - }, - cors: CorsPolicy::NotApplicable, - websocket_origin: WebSocketOriginPolicy::NotApplicable, - streaming: StreamingMode::None, - audit: AuditTraceClass::PublicCallback, - effect_path: AllowedEffectPath::ProductWorkflow, - }) - .expect("Slack commands ingress policy must validate") // safety: policy combines validated constants and host-resolved webhook-signature scope. + vec![bundled_slack_ingress_descriptor(SLACK_COMMANDS_ROUTE_ID)] } async fn slack_commands_handler( @@ -637,6 +575,48 @@ mod tests { use tower::ServiceExt; use super::*; + use ironclaw_host_api::NetworkMethod; + use ironclaw_host_api::ingress::{ + AllowedEffectPath, AuditTraceClass, BodyLimitPolicy, CorsPolicy, IngressAuthPolicy, + IngressAuthScheme, IngressPolicy, IngressPolicyParts, IngressScopeSource, ListenerClass, + RateLimitPolicy, RateLimitScope, StreamingMode, WebSocketOriginPolicy, + }; + use std::num::{NonZeroU32, NonZeroU64}; + + /// Rebuild the pre-migration Slack ingress descriptor literal, so the + /// manifest-projected descriptor can be asserted equal to it + /// (behavior-preserving migration guard). `window_seconds` is 60 for both + /// Slack routes. + fn expected_slack_descriptor( + route_id: &str, + path: &str, + body_limit: NonZeroU64, + max_requests: NonZeroU32, + ) -> IngressRouteDescriptor { + let policy = IngressPolicy::new(IngressPolicyParts { + listener_class: ListenerClass::PublicWebhook, + auth: IngressAuthPolicy::Required { + schemes: vec![IngressAuthScheme::WebhookSignature], + }, + scope_source: IngressScopeSource::HostResolved, + body_limit: BodyLimitPolicy::Limited { + max_bytes: body_limit, + }, + rate_limit: RateLimitPolicy::Limited { + scope: RateLimitScope::Global, + max_requests, + window_seconds: NonZeroU32::new(60).expect("nonzero"), + }, + cors: CorsPolicy::NotApplicable, + websocket_origin: WebSocketOriginPolicy::NotApplicable, + streaming: StreamingMode::None, + audit: AuditTraceClass::PublicCallback, + effect_path: AllowedEffectPath::ProductWorkflow, + }) + .expect("policy validates"); + IngressRouteDescriptor::new(route_id, NetworkMethod::Post, path, policy) + .expect("descriptor validates") + } #[derive(Clone)] struct FakeSlackDispatcher { @@ -1116,29 +1096,35 @@ mod tests { } #[test] - fn slack_events_route_uses_limited_body_policy() { - let descriptors = slack_events_route_descriptors(); - let [descriptor] = descriptors.as_slice() else { - panic!("expected exactly one Slack Events route descriptor") - }; - let BodyLimitPolicy::Limited { max_bytes } = descriptor.policy().body_limit() else { - panic!("Slack Events route should have a body limit") - }; - - assert_eq!(max_bytes, SLACK_EVENTS_BODY_LIMIT_BYTES); + fn slack_events_route_descriptor_matches_manifest_projection() { + // Behavior-preserving migration guard: the Slack events descriptor + // projected from the bundled manifest's `[[host_ingress]]` declaration + // equals the pre-migration Rust literal (1 MiB body, 12k req / 60s, + // public_webhook + webhook_signature). This is the load-bearing example + // that the manifest-driven ingress contract is real and used. + assert_eq!( + slack_events_route_descriptors(), + vec![expected_slack_descriptor( + SLACK_EVENTS_ROUTE_ID, + SLACK_EVENTS_PATH, + NonZeroU64::new(1024 * 1024).expect("nonzero"), + NonZeroU32::new(12_000).expect("nonzero"), + )] + ); } #[test] - fn slack_events_route_uses_global_rate_limit_scope() { - let descriptors = slack_events_route_descriptors(); - let [descriptor] = descriptors.as_slice() else { - panic!("expected exactly one Slack Events route descriptor") - }; - let RateLimitPolicy::Limited { scope, .. } = descriptor.policy().rate_limit() else { - panic!("Slack Events route should be rate limited") - }; - - assert_eq!(*scope, RateLimitScope::Global); + fn slack_commands_route_descriptor_matches_manifest_projection() { + // Same guard for the slash-commands route (16 KiB body, 6k req / 60s). + assert_eq!( + slack_commands_route_descriptors(), + vec![expected_slack_descriptor( + SLACK_COMMANDS_ROUTE_ID, + SLACK_COMMANDS_PATH, + NonZeroU64::new(16 * 1024).expect("nonzero"), + NonZeroU32::new(6_000).expect("nonzero"), + )] + ); } /// Caller-level coverage for the `/pair` slash command handler: signature From fd152f940babd8a4a82f969a967c86ff8af28eda Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sat, 4 Jul 2026 00:49:43 +0000 Subject: [PATCH 4/5] review: cache the Slack ingress descriptor projection with LazyLock Addresses gemini review feedback on #5626: project each Slack route descriptor from the bundled manifest exactly once (LazyLock) instead of re-parsing the manifest TOML on every slack_{events,commands}_route_descriptors() call. The manifest is a compile-time constant, so the projection is deterministic and safe to memoize for the process lifetime. Behavior-preserving: the equivalence guards and the full 377-test Slack serve suite still pass; clippy clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/slack_serve.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/crates/ironclaw_reborn_composition/src/slack_serve.rs b/crates/ironclaw_reborn_composition/src/slack_serve.rs index 31b2277d8dd..360f56e41ea 100644 --- a/crates/ironclaw_reborn_composition/src/slack_serve.rs +++ b/crates/ironclaw_reborn_composition/src/slack_serve.rs @@ -7,7 +7,7 @@ use std::future::Future; use std::pin::Pin; -use std::sync::Arc; +use std::sync::{Arc, LazyLock}; use axum::{ Json, Router, @@ -239,9 +239,15 @@ pub fn slack_events_route_mount(state: SlackEventsRouteState) -> PublicRouteMoun } pub fn slack_events_route_descriptors() -> Vec { - vec![bundled_slack_ingress_descriptor(SLACK_EVENTS_ROUTE_ID)] + vec![SLACK_EVENTS_DESCRIPTOR.clone()] } +/// The Slack events route descriptor, projected from the bundled manifest +/// exactly once on first use (the manifest is a compile-time constant, so the +/// projection is deterministic and cached for the process lifetime). +static SLACK_EVENTS_DESCRIPTOR: LazyLock = + LazyLock::new(|| bundled_slack_ingress_descriptor(SLACK_EVENTS_ROUTE_ID)); + /// Project a Slack host-ingress route descriptor from the bundled Slack /// extension manifest. /// @@ -508,9 +514,14 @@ pub fn slack_commands_route_mount(state: SlackCommandsRouteState) -> PublicRoute } pub fn slack_commands_route_descriptors() -> Vec { - vec![bundled_slack_ingress_descriptor(SLACK_COMMANDS_ROUTE_ID)] + vec![SLACK_COMMANDS_DESCRIPTOR.clone()] } +/// The Slack commands route descriptor, projected from the bundled manifest +/// exactly once on first use (see [`SLACK_EVENTS_DESCRIPTOR`]). +static SLACK_COMMANDS_DESCRIPTOR: LazyLock = + LazyLock::new(|| bundled_slack_ingress_descriptor(SLACK_COMMANDS_ROUTE_ID)); + async fn slack_commands_handler( State(state): State, headers: HeaderMap, From 27731cb6040f34061d3839028cbbd56a96bdc5f5 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sun, 5 Jul 2026 01:00:18 +0000 Subject: [PATCH 5/5] review: signing-secret verifier handle, descriptor-driven mounts, single manifest parse path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address PR #5626 review feedback: - manifest.toml: the ingress routes' credential_handles now name slack_signing_secret (declared in required_credentials) — the secret the runtime's HMAC webhook verifier is actually built from — instead of the outbound slack_bot_token. Descriptors expanded from one-line inline tables into multiline TOML tables so policy changes diff field-by-field. - slack_serve: both descriptors project from one manifest parse via a single LazyLock, and the axum mounts now build their route path from the projected descriptor (failing closed at projection time if a route ever declares a non-POST method), so what axum mounts cannot drift from what the manifest declares. - host_ingress: parsing reuses the same context as bundled extension installation (default host-port catalog + default host-API contract registry) instead of a second ingestion path with an empty catalog; projects all routes in one pass with a separate route-id selector. - host_ingress tests: cover the happy projection and the previously untested RouteNotDeclared branch. Co-Authored-By: Claude Fable 5 --- .../assets/slack/manifest.toml | 47 +++++- .../src/host_ingress.rs | 157 ++++++++++++++---- .../src/slack_serve.rs | 89 ++++++---- 3 files changed, 231 insertions(+), 62 deletions(-) diff --git a/crates/ironclaw_first_party_extensions/assets/slack/manifest.toml b/crates/ironclaw_first_party_extensions/assets/slack/manifest.toml index ec1ee88d505..a21e9de8547 100644 --- a/crates/ironclaw_first_party_extensions/assets/slack/manifest.toml +++ b/crates/ironclaw_first_party_extensions/assets/slack/manifest.toml @@ -32,6 +32,9 @@ flags = [ [[product_adapter.inbound.required_credentials]] handle = "slack_bot_token" +[[product_adapter.inbound.required_credentials]] +handle = "slack_signing_secret" + [[product_adapter.inbound.egress]] host = "slack.com" credential_handle = "slack_bot_token" @@ -39,11 +42,45 @@ credential_handle = "slack_bot_token" # Host-ingress routes projected by the serve layer (ironclaw_reborn_composition # ::slack_serve). The route path/method/policy live here as data — the axum # handler + HMAC verifier stay in Rust. Credential coherence: each route is -# verified by `slack_bot_token`, which is declared in required_credentials above. +# verified by `slack_signing_secret` (the secret behind the runtime's HMAC +# webhook verifier), declared in required_credentials above. `slack_bot_token` +# is the outbound egress credential and does not verify inbound routes. [[product_adapter.inbound.host_ingress]] -credential_handles = ["slack_bot_token"] -descriptor = { route_id = "slack.events", method = "post", route_pattern = "/webhooks/slack/events", policy = { listener_class = "public_webhook", auth = { type = "required", schemes = ["webhook_signature"] }, scope_source = "host_resolved", body_limit = { type = "limited", max_bytes = 1048576 }, rate_limit = { type = "limited", scope = "global", max_requests = 12000, window_seconds = 60 }, cors = "not_applicable", websocket_origin = "not_applicable", streaming = "none", audit = "public_callback", effect_path = { type = "product_workflow" } } } +credential_handles = ["slack_signing_secret"] + +[product_adapter.inbound.host_ingress.descriptor] +route_id = "slack.events" +method = "post" +route_pattern = "/webhooks/slack/events" + +[product_adapter.inbound.host_ingress.descriptor.policy] +listener_class = "public_webhook" +auth = { type = "required", schemes = ["webhook_signature"] } +scope_source = "host_resolved" +body_limit = { type = "limited", max_bytes = 1048576 } +rate_limit = { type = "limited", scope = "global", max_requests = 12000, window_seconds = 60 } +cors = "not_applicable" +websocket_origin = "not_applicable" +streaming = "none" +audit = "public_callback" +effect_path = { type = "product_workflow" } [[product_adapter.inbound.host_ingress]] -credential_handles = ["slack_bot_token"] -descriptor = { route_id = "slack.commands", method = "post", route_pattern = "/webhooks/slack/commands", policy = { listener_class = "public_webhook", auth = { type = "required", schemes = ["webhook_signature"] }, scope_source = "host_resolved", body_limit = { type = "limited", max_bytes = 16384 }, rate_limit = { type = "limited", scope = "global", max_requests = 6000, window_seconds = 60 }, cors = "not_applicable", websocket_origin = "not_applicable", streaming = "none", audit = "public_callback", effect_path = { type = "product_workflow" } } } +credential_handles = ["slack_signing_secret"] + +[product_adapter.inbound.host_ingress.descriptor] +route_id = "slack.commands" +method = "post" +route_pattern = "/webhooks/slack/commands" + +[product_adapter.inbound.host_ingress.descriptor.policy] +listener_class = "public_webhook" +auth = { type = "required", schemes = ["webhook_signature"] } +scope_source = "host_resolved" +body_limit = { type = "limited", max_bytes = 16384 } +rate_limit = { type = "limited", scope = "global", max_requests = 6000, window_seconds = 60 } +cors = "not_applicable" +websocket_origin = "not_applicable" +streaming = "none" +audit = "public_callback" +effect_path = { type = "product_workflow" } diff --git a/crates/ironclaw_reborn_composition/src/host_ingress.rs b/crates/ironclaw_reborn_composition/src/host_ingress.rs index dff17e20aa5..54b11afe76e 100644 --- a/crates/ironclaw_reborn_composition/src/host_ingress.rs +++ b/crates/ironclaw_reborn_composition/src/host_ingress.rs @@ -3,21 +3,23 @@ //! This is the read side of the manifest-driven ingress contract: an //! extension's `manifest.toml` declares its inbound HTTP routes as data //! (`[[product_adapter..host_ingress]]`), and this helper projects the -//! validated [`IngressRouteDescriptor`] the serve layer mounts. The route's +//! validated [`IngressRouteDescriptor`]s the serve layer mounts. The route's //! *policy/shape* is data in the manifest; the axum handler and the webhook //! verifier (behavior) stay in the owning serve module. //! -//! `ironclaw_product_adapter_registry` owns the parsing/validation and the -//! fail-closed credential coherence (every route names a verifying credential -//! declared in `required_credentials`); this module only selects a route by id -//! and hands back its descriptor. +//! Parsing deliberately reuses the same context as bundled extension +//! installation (`available_extensions::bundled_extension_package`): the +//! default host-port catalog and the default host-API contract registry. A +//! bundled manifest therefore cannot be installable but fail serve-time +//! projection (or vice versa) because the two paths diverged on parsing +//! context. `ironclaw_product_adapter_registry` owns the section +//! validation and the fail-closed credential coherence (every auth-required +//! route names a verifying credential declared in `required_credentials`); +//! this module only projects the descriptors and selects one by id. -use ironclaw_extensions::ManifestSource; -use ironclaw_host_api::HostPortCatalog; +use ironclaw_extensions::{ExtensionManifestRecord, ManifestSource}; use ironclaw_host_api::ingress::IngressRouteDescriptor; -use ironclaw_product_adapter_registry::{ - parse_product_adapter_manifest_record, product_adapter_sections, -}; +use ironclaw_product_adapter_registry::product_adapter_sections; use thiserror::Error; #[derive(Debug, Error)] @@ -28,39 +30,138 @@ pub(crate) enum HostIngressProjectionError { RouteNotDeclared { route_id: String }, } -/// Project the [`IngressRouteDescriptor`] for `route_id` from a bundled -/// (host-compiled) extension manifest. +/// Project every [`IngressRouteDescriptor`] a bundled (host-compiled) +/// extension manifest declares, in one parse. /// -/// The descriptor is validated by `ironclaw_host_api` on deserialize (dotted +/// Each descriptor is validated by `ironclaw_host_api` on deserialize (dotted /// route id, absolute path, and every policy invariant including the /// fail-closed floor that a `public_webhook` listener must require /// `webhook_signature`), and by the registry for ingress credential coherence. /// Intended for compile-time bundled manifests, so callers may treat a failure /// as a startup invariant violation. -pub(crate) fn bundled_host_ingress_descriptor( +pub(crate) fn bundled_host_ingress_descriptors( manifest_toml: &str, - route_id: &str, -) -> Result { - let record = parse_product_adapter_manifest_record( +) -> Result, HostIngressProjectionError> { + let host_ports = ironclaw_host_runtime::default_host_port_catalog().map_err(projection)?; + let contracts = + ironclaw_host_runtime::default_host_api_contract_registry().map_err(projection)?; + let record = ExtensionManifestRecord::from_toml_with_contracts( manifest_toml, ManifestSource::HostBundled, - &HostPortCatalog::empty(), + &host_ports, None, + &contracts, ) - .map_err(|error| HostIngressProjectionError::Projection { - reason: error.to_string(), - })?; - let sections = product_adapter_sections(&record).map_err(|error| { - HostIngressProjectionError::Projection { - reason: error.to_string(), - } - })?; - sections + .map_err(projection)?; + let sections = product_adapter_sections(&record).map_err(projection)?; + Ok(sections .iter() .flat_map(|section| section.host_ingress()) - .find(|route| route.descriptor().route_id().as_str() == route_id) .map(|route| route.descriptor().clone()) + .collect()) +} + +/// Select the descriptor for `route_id` from an already-projected set. +pub(crate) fn descriptor_for_route( + descriptors: &[IngressRouteDescriptor], + route_id: &str, +) -> Result { + descriptors + .iter() + .find(|descriptor| descriptor.route_id().as_str() == route_id) + .cloned() .ok_or_else(|| HostIngressProjectionError::RouteNotDeclared { route_id: route_id.to_string(), }) } + +fn projection(error: impl std::fmt::Display) -> HostIngressProjectionError { + HostIngressProjectionError::Projection { + reason: error.to_string(), + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const VALID_MANIFEST: &str = r#" +schema_version = "reborn.extension_manifest.v2" +id = "testext" +name = "Test Extension" +version = "0.1.0" +description = "Host-ingress projection fixture." +trust = "first_party_requested" + +[runtime] +kind = "first_party" +service = "test_service" + +[[host_api]] +id = "ironclaw.product_adapter/v1" +section = "product_adapter.inbound" + +[product_adapter.inbound] +surface_kind = "external_channel" + +[product_adapter.inbound.auth] +kind = "request_signature" +header_name = "X-Test-Signature" +timestamp_header_name = "X-Test-Timestamp" + +[product_adapter.inbound.capabilities] +flags = ["inbound_messages"] + +[[product_adapter.inbound.required_credentials]] +handle = "test_signing_secret" + +[[product_adapter.inbound.host_ingress]] +credential_handles = ["test_signing_secret"] + +[product_adapter.inbound.host_ingress.descriptor] +route_id = "testext.events" +method = "post" +route_pattern = "/webhooks/testext/events" + +[product_adapter.inbound.host_ingress.descriptor.policy] +listener_class = "public_webhook" +auth = { type = "required", schemes = ["webhook_signature"] } +scope_source = "host_resolved" +body_limit = { type = "limited", max_bytes = 1024 } +rate_limit = { type = "limited", scope = "global", max_requests = 60, window_seconds = 60 } +cors = "not_applicable" +websocket_origin = "not_applicable" +streaming = "none" +audit = "public_callback" +effect_path = { type = "product_workflow" } +"#; + + #[test] + fn bundled_host_ingress_descriptors_project_declared_routes() { + let descriptors = + bundled_host_ingress_descriptors(VALID_MANIFEST).expect("valid manifest projects"); + let descriptor = + descriptor_for_route(&descriptors, "testext.events").expect("declared route resolves"); + assert_eq!(descriptor.route_id().as_str(), "testext.events"); + assert_eq!( + descriptor.route_pattern().as_str(), + "/webhooks/testext/events" + ); + } + + #[test] + fn bundled_host_ingress_descriptor_rejects_missing_route_id() { + let descriptors = + bundled_host_ingress_descriptors(VALID_MANIFEST).expect("valid manifest projects"); + let error = descriptor_for_route(&descriptors, "testext.absent") + .expect_err("absent route id must be rejected"); + assert!( + matches!( + &error, + HostIngressProjectionError::RouteNotDeclared { route_id } + if route_id == "testext.absent" + ), + "expected RouteNotDeclared, got: {error}" + ); + } +} diff --git a/crates/ironclaw_reborn_composition/src/slack_serve.rs b/crates/ironclaw_reborn_composition/src/slack_serve.rs index 360f56e41ea..f51d3bc848a 100644 --- a/crates/ironclaw_reborn_composition/src/slack_serve.rs +++ b/crates/ironclaw_reborn_composition/src/slack_serve.rs @@ -17,6 +17,7 @@ use axum::{ response::{IntoResponse, Response}, routing::post, }; +use ironclaw_host_api::NetworkMethod; use ironclaw_host_api::ingress::IngressRouteDescriptor; use ironclaw_product_adapters::ProtocolAuthEvidence; use ironclaw_wasm_product_adapters::{ @@ -229,45 +230,76 @@ impl std::fmt::Debug for SlackEventsRouteState { } pub fn slack_events_route_mount(state: SlackEventsRouteState) -> PublicRouteMount { + let descriptor = SLACK_INGRESS_DESCRIPTORS.events.clone(); PublicRouteMount::new( Router::new() - .route(SLACK_EVENTS_PATH, post(slack_events_handler)) + .route( + descriptor.route_pattern().as_str(), + post(slack_events_handler), + ) .with_state(state.clone()), - slack_events_route_descriptors(), + vec![descriptor], ) .with_drain(Arc::new(state)) } pub fn slack_events_route_descriptors() -> Vec { - vec![SLACK_EVENTS_DESCRIPTOR.clone()] + vec![SLACK_INGRESS_DESCRIPTORS.events.clone()] } -/// The Slack events route descriptor, projected from the bundled manifest -/// exactly once on first use (the manifest is a compile-time constant, so the -/// projection is deterministic and cached for the process lifetime). -static SLACK_EVENTS_DESCRIPTOR: LazyLock = - LazyLock::new(|| bundled_slack_ingress_descriptor(SLACK_EVENTS_ROUTE_ID)); - -/// Project a Slack host-ingress route descriptor from the bundled Slack -/// extension manifest. +/// Both Slack host-ingress route descriptors, projected from the bundled Slack +/// extension manifest in a single parse on first use (the manifest is a +/// compile-time constant, so the projection is deterministic and cached for +/// the process lifetime). /// -/// The route's path/method/policy are declared as data in +/// The routes' path/method/policy are declared as data in /// `assets/slack/manifest.toml` (`[[product_adapter.inbound.host_ingress]]`) /// and validated by `ironclaw_host_api` (incl. the fail-closed floor that a /// `public_webhook` listener must require `webhook_signature`) plus /// `ironclaw_product_adapter_registry` (ingress credential coherence). Only the -/// declarative descriptor lives in the manifest — the axum handler and the HMAC -/// verifier stay in this module. Panics if the bundled manifest does not -/// declare the route: `SLACK_MANIFEST` is a compile-time constant, so a missing -/// route is a build-time invariant violation, surfaced at startup. -fn bundled_slack_ingress_descriptor(route_id: &str) -> IngressRouteDescriptor { - crate::host_ingress::bundled_host_ingress_descriptor( +/// declarative descriptors live in the manifest — the axum handlers and the +/// HMAC verifier stay in this module, and the mount functions build their +/// routes from these descriptors so what axum mounts cannot drift from what +/// the manifest declares. Panics if the bundled manifest does not declare a +/// route or declares it with a non-POST method: `SLACK_MANIFEST` is a +/// compile-time constant, so either is a build-time invariant violation, +/// surfaced at startup. +static SLACK_INGRESS_DESCRIPTORS: LazyLock = LazyLock::new(|| { + let descriptors = crate::host_ingress::bundled_host_ingress_descriptors( crate::available_extensions::slack_manifest_toml(), - route_id, ) .unwrap_or_else(|error| { - panic!("bundled Slack manifest must declare host-ingress route {route_id}: {error}") - }) + panic!("bundled Slack manifest must project host-ingress routes: {error}") + }); + SlackIngressDescriptors { + events: bundled_slack_post_descriptor(&descriptors, SLACK_EVENTS_ROUTE_ID), + commands: bundled_slack_post_descriptor(&descriptors, SLACK_COMMANDS_ROUTE_ID), + } +}); + +struct SlackIngressDescriptors { + events: IngressRouteDescriptor, + commands: IngressRouteDescriptor, +} + +fn bundled_slack_post_descriptor( + descriptors: &[IngressRouteDescriptor], + route_id: &str, +) -> IngressRouteDescriptor { + let descriptor = crate::host_ingress::descriptor_for_route(descriptors, route_id) + .unwrap_or_else(|error| { + panic!("bundled Slack manifest must declare host-ingress route {route_id}: {error}") + }); + // The mount functions wire their handlers with `post(...)`; fail closed at + // projection time if the manifest ever declares another method. + if descriptor.method() != NetworkMethod::Post { + panic!( + "bundled Slack manifest declares host-ingress route {route_id} with method {}, \ + but the serve layer mounts POST handlers", + descriptor.method() + ); + } + descriptor } async fn slack_events_handler( @@ -505,23 +537,22 @@ impl std::fmt::Debug for SlackCommandsRouteState { } pub fn slack_commands_route_mount(state: SlackCommandsRouteState) -> PublicRouteMount { + let descriptor = SLACK_INGRESS_DESCRIPTORS.commands.clone(); PublicRouteMount::new( Router::new() - .route(SLACK_COMMANDS_PATH, post(slack_commands_handler)) + .route( + descriptor.route_pattern().as_str(), + post(slack_commands_handler), + ) .with_state(state), - slack_commands_route_descriptors(), + vec![descriptor], ) } pub fn slack_commands_route_descriptors() -> Vec { - vec![SLACK_COMMANDS_DESCRIPTOR.clone()] + vec![SLACK_INGRESS_DESCRIPTORS.commands.clone()] } -/// The Slack commands route descriptor, projected from the bundled manifest -/// exactly once on first use (see [`SLACK_EVENTS_DESCRIPTOR`]). -static SLACK_COMMANDS_DESCRIPTOR: LazyLock = - LazyLock::new(|| bundled_slack_ingress_descriptor(SLACK_COMMANDS_ROUTE_ID)); - async fn slack_commands_handler( State(state): State, headers: HeaderMap,