From f38b3c977cd4cde8adf7f8b482032842f27dc48e Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Tue, 9 Jun 2026 19:07:42 -0700 Subject: [PATCH 1/4] feat(reborn): accept inline attachment uploads on the WebChat v2 send path (#4644) End-to-end ingress wiring for #4644: a browser can now attach files to a WebChat v2 message, the bytes land in project storage, and the user message persists attachment references. This is the call site for the land_inbound_attachments bridge. Flow (no src/ changes): - DTO: WebUiSendMessageRequest gains `attachments: Vec` (mime_type, filename, base64). decode_attachments() validates MIME against the shared format registry (Track 1), decodes base64, and enforces the v1 budgets (5 MiB/file, 10 MiB total, 10 files max). Kept separate from into_command so the serializable command never carries raw bytes. - Facade: RebornServices::submit_turn decodes attachments, lands them through a new InboundAttachmentLander port (using the stable per-message external_event_id for the storage path), and builds MessageContent::with_attachments before accept_inbound_message. With no lander wired, an attachment-bearing message is rejected (503) rather than silently dropped. - Port impl: composition's ProjectScopedAttachmentLander writes through the project-scoped workspace ScopedFilesystem (the same authority the agent's file tools resolve through) via land_inbound_attachments, and is wired onto the facade in build_webui_services when a local runtime is present. - Body limit: the send_message route descriptor goes from 1 MiB to 14 MiB to carry base64 of the 10 MiB decoded cap; the descriptor/body-limit contract tests and the composition CLAUDE.md are updated to match. Tests: - decode_attachments: metadata/kind/bytes, MIME normalization, unsupported MIME, malformed base64, per-file/total oversize, too-many, empty. - ProjectScopedAttachmentLander: lands + returns ref with storage_key; read-only workspace mount maps to an internal error. - Facade (test through the caller): submit_turn lands the attachment and the accepted user message carries the ref with storage_key; attachments without a wired lander are rejected with ServiceUnavailable. New dep edges product_workflow/composition -> ironclaw_attachments (accepted by the reborn dependency-boundary test). Deferred: model-visibility (content_parts / extracted_text / project_path), the memory index note, and the static frontend "+ button" upload UI. --- Cargo.lock | 3 + crates/ironclaw_product_workflow/Cargo.toml | 2 + crates/ironclaw_product_workflow/src/lib.rs | 20 +-- .../src/reborn_services.rs | 64 +++++++- .../src/webui_inbound.rs | 116 +++++++++++++ .../tests/reborn_services_contract.rs | 150 ++++++++++++++++- .../tests/webui_inbound_contract.rs | 133 ++++++++++++++- crates/ironclaw_reborn_composition/CLAUDE.md | 6 +- crates/ironclaw_reborn_composition/Cargo.toml | 1 + .../src/attachment_landing.rs | 153 ++++++++++++++++++ crates/ironclaw_reborn_composition/src/lib.rs | 1 + .../src/runtime.rs | 15 ++ .../ironclaw_reborn_composition/src/webui.rs | 5 + .../src/webui_body_limit.rs | 15 +- crates/ironclaw_webui_v2/src/descriptors.rs | 8 +- .../tests/webui_v2_descriptors_contract.rs | 2 +- 16 files changed, 659 insertions(+), 35 deletions(-) create mode 100644 crates/ironclaw_reborn_composition/src/attachment_landing.rs diff --git a/Cargo.lock b/Cargo.lock index 2e162d58471..90518518c67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4823,9 +4823,11 @@ name = "ironclaw_product_workflow" version = "0.1.0" dependencies = [ "async-trait", + "base64 0.22.1", "chrono", "futures", "ironclaw_approvals", + "ironclaw_attachments", "ironclaw_auth", "ironclaw_authorization", "ironclaw_common", @@ -4970,6 +4972,7 @@ dependencies = [ "http 1.4.1", "http-body-util", "ironclaw_approvals", + "ironclaw_attachments", "ironclaw_auth", "ironclaw_authorization", "ironclaw_capabilities", diff --git a/crates/ironclaw_product_workflow/Cargo.toml b/crates/ironclaw_product_workflow/Cargo.toml index 163093ede2b..8542e034e9c 100644 --- a/crates/ironclaw_product_workflow/Cargo.toml +++ b/crates/ironclaw_product_workflow/Cargo.toml @@ -19,9 +19,11 @@ test-support = ["ironclaw_product_adapters/test-support"] [dependencies] async-trait = "0.1" +base64 = "0.22" chrono = { version = "0.4", features = ["serde"] } futures = "0.3" ironclaw_common = { path = "../ironclaw_common", version = "0.4.1" } +ironclaw_attachments = { path = "../ironclaw_attachments", version = "0.1.0" } ironclaw_approvals = { path = "../ironclaw_approvals", version = "0.1.0" } ironclaw_authorization = { path = "../ironclaw_authorization", version = "0.1.0" } ironclaw_auth = { path = "../ironclaw_auth", version = "0.1.0" } diff --git a/crates/ironclaw_product_workflow/src/lib.rs b/crates/ironclaw_product_workflow/src/lib.rs index 0fa893560f0..dec05520d42 100644 --- a/crates/ironclaw_product_workflow/src/lib.rs +++ b/crates/ironclaw_product_workflow/src/lib.rs @@ -144,15 +144,15 @@ pub use reborn_services::{ AUTOMATION_RUN_HISTORY_DEFAULT_PAGE_SIZE, AUTOMATION_RUN_HISTORY_MAX_PAGE_SIZE, AutomationListRequest, AutomationProductFacade, CodexLoginStart, ConnectableChannelsProductFacade, ExtensionCredentialSetupService, - ExtensionCredentialStatusRequest, ExtensionCredentialSubmitRequest, LlmActiveSelection, - LlmConfigService, LlmConfigServiceError, LlmConfigSnapshot, LlmModelsResult, LlmProbeRequest, - LlmProbeResult, LlmProviderView, NearAiAuthProvider, NearAiLoginRequest, NearAiLoginStart, - NearAiWalletLoginRequest, NearAiWalletLoginResult, OperatorLogsService, - OperatorServiceLifecycleService, OperatorStatusService, OutboundPreferencesProductFacade, - ProductAgentBoundCaller, RebornAutomationInfo, RebornAutomationRecentRunInfo, - RebornAutomationRecentRunStatus, RebornAutomationRunStatus, RebornAutomationSource, - RebornAutomationState, RebornCancelRunResponse, RebornChannelConnectAction, - RebornChannelConnectStrategy, RebornConnectableChannelInfo, + ExtensionCredentialStatusRequest, ExtensionCredentialSubmitRequest, InboundAttachmentLander, + LlmActiveSelection, LlmConfigService, LlmConfigServiceError, LlmConfigSnapshot, + LlmModelsResult, LlmProbeRequest, LlmProbeResult, LlmProviderView, NearAiAuthProvider, + NearAiLoginRequest, NearAiLoginStart, NearAiWalletLoginRequest, NearAiWalletLoginResult, + OperatorLogsService, OperatorServiceLifecycleService, OperatorStatusService, + OutboundPreferencesProductFacade, ProductAgentBoundCaller, RebornAutomationInfo, + RebornAutomationRecentRunInfo, RebornAutomationRecentRunStatus, RebornAutomationRunStatus, + RebornAutomationSource, RebornAutomationState, RebornCancelRunResponse, + RebornChannelConnectAction, RebornChannelConnectStrategy, RebornConnectableChannelInfo, RebornConnectableChannelListResponse, RebornCreateThreadResponse, RebornDeleteThreadRequest, RebornDeleteThreadResponse, RebornExtensionActionResponse, RebornExtensionCredentialSetup, RebornExtensionInfo, RebornExtensionListResponse, RebornExtensionOnboardingPayload, @@ -192,7 +192,7 @@ pub use reborn_services::{ pub use webui_inbound::{ WebUiAuthenticatedCaller, WebUiCancelReason, WebUiCancelRunRequest, WebUiCreateThreadRequest, - WebUiGateResolution, WebUiInboundCommand, WebUiInboundValidationCode, + WebUiGateResolution, WebUiInboundAttachment, WebUiInboundCommand, WebUiInboundValidationCode, WebUiInboundValidationError, WebUiListAutomationsRequest, WebUiListThreadsRequest, WebUiResolveGateRequest, WebUiSendMessageRequest, WebUiSetupExtensionRequest, }; diff --git a/crates/ironclaw_product_workflow/src/reborn_services.rs b/crates/ironclaw_product_workflow/src/reborn_services.rs index d60a022eb6f..adb181993a9 100644 --- a/crates/ironclaw_product_workflow/src/reborn_services.rs +++ b/crates/ironclaw_product_workflow/src/reborn_services.rs @@ -12,6 +12,7 @@ use std::{ use async_trait::async_trait; use chrono::Utc; +use ironclaw_attachments::InboundAttachment; use ironclaw_auth::{ AuthProductScope, AuthProviderId, CredentialAccountId, CredentialAccountProjection, CredentialAccountUpdateBinding, ProviderScope, @@ -22,9 +23,10 @@ use ironclaw_product_adapters::{ ProjectionSubscriptionRequest, }; use ironclaw_threads::{ - AcceptInboundMessageRequest, AcceptedInboundMessageReplay, EnsureThreadRequest, MessageContent, - MessageStatus, ReplayAcceptedInboundMessageRequest, SessionThreadError, SessionThreadRecord, - SessionThreadService, ThreadHistory, ThreadHistoryRequest, ThreadMessageId, ThreadScope, + AcceptInboundMessageRequest, AcceptedInboundMessageReplay, AttachmentRef, EnsureThreadRequest, + MessageContent, MessageStatus, ReplayAcceptedInboundMessageRequest, SessionThreadError, + SessionThreadRecord, SessionThreadService, ThreadHistory, ThreadHistoryRequest, ThreadMessageId, + ThreadScope, }; use ironclaw_turns::{ AcceptedMessageRef, GateRef, GetRunStateRequest, IdempotencyKey, ResumeTurnPrecondition, @@ -1271,11 +1273,30 @@ pub trait RebornServicesApi: Send + Sync { } } +/// Lands inbound attachment bytes into durable, agent-accessible storage and +/// returns the transcript references to persist on the user message. +/// +/// Injected by host composition, which owns the project-scoped filesystem +/// authority. `message_id` is a stable per-message id (the idempotency key) +/// used only to disambiguate the storage path; the implementation writes +/// through the same `MountView` the agent's file tools resolve through, so +/// landed bytes are readable by `file_read`/`list_dir` in later turns. +#[async_trait] +pub trait InboundAttachmentLander: Send + Sync { + async fn land( + &self, + thread_scope: &ThreadScope, + message_id: &str, + attachments: Vec, + ) -> Result, RebornServicesError>; +} + /// Default facade implementation composed at the WebUI boundary. #[derive(Clone)] pub struct RebornServices { thread_service: Arc, turn_coordinator: Arc, + inbound_attachments: Option>, event_stream: Option>, lifecycle_facade: Arc, automation_facade: Arc, @@ -1302,6 +1323,7 @@ impl RebornServices { Self { thread_service, turn_coordinator, + inbound_attachments: None, event_stream: None, lifecycle_facade: Arc::new(UnsupportedLifecycleProductFacade::new_static( "reborn_lifecycle_facade_unwired", @@ -1330,6 +1352,17 @@ impl RebornServices { self } + /// Wire the port that lands inbound attachment bytes into project storage. + /// Without it, a send-message carrying attachments is rejected rather than + /// silently dropping the files. + pub fn with_inbound_attachments( + mut self, + inbound_attachments: Arc, + ) -> Self { + self.inbound_attachments = Some(inbound_attachments); + self + } + pub fn with_llm_config_service(mut self, llm_config: Arc) -> Self { self.llm_config = Some(llm_config); self @@ -1611,6 +1644,9 @@ impl RebornServicesApi for RebornServices { caller: WebUiAuthenticatedCaller, request: WebUiSendMessageRequest, ) -> Result { + // Decode + budget inline attachment bytes before the request is + // consumed into the (bytes-free, serializable) command. + let attachments = request.decode_attachments()?; let command = request.into_command(caller)?; let WebUiInboundCommand::SendMessage { scope, @@ -1683,6 +1719,26 @@ impl RebornServicesApi for RebornServices { } } } else { + // Land attachment bytes (if any) into project storage before the + // message is accepted, recording each as a transcript reference. + // Uses the stable per-message external_event_id for the storage + // path so a retry re-lands at the same deterministic location. + let message_content = if attachments.is_empty() { + MessageContent::text(content.clone()) + } else { + let lander = self.inbound_attachments.as_ref().ok_or_else(|| { + RebornServicesError::from_status_kind( + RebornServicesErrorCode::Unavailable, + RebornServicesErrorKind::ServiceUnavailable, + 503, + false, + ) + })?; + let refs = lander + .land(&thread_scope, &external_event_id, attachments) + .await?; + MessageContent::with_attachments(content.clone(), refs) + }; let accepted = self .thread_service .accept_inbound_message(AcceptInboundMessageRequest { @@ -1692,7 +1748,7 @@ impl RebornServicesApi for RebornServices { source_binding_id: Some(source_binding_id.clone()), reply_target_binding_id: Some(source_binding_id.clone()), external_event_id: Some(external_event_id), - content: MessageContent::text(content.clone()), + content: message_content, }) .await .map_err(map_thread_error)?; diff --git a/crates/ironclaw_product_workflow/src/webui_inbound.rs b/crates/ironclaw_product_workflow/src/webui_inbound.rs index 1f9972d6cbe..4664edf14c5 100644 --- a/crates/ironclaw_product_workflow/src/webui_inbound.rs +++ b/crates/ironclaw_product_workflow/src/webui_inbound.rs @@ -4,6 +4,7 @@ //! into canonical Reborn commands without depending on WebUI route handlers, //! product adapters, protocol auth evidence, WASM, or adapter registries. +use ironclaw_attachments::InboundAttachment; use ironclaw_host_api::{AgentId, ProjectId, TenantId, ThreadId, UserId}; use ironclaw_turns::{ CancelRunRequest, GateRef, IdempotencyKey, SanitizedCancelReason, TurnActor, TurnRunId, @@ -16,6 +17,13 @@ const CLIENT_ACTION_ID_MAX_BYTES: usize = 256; const USER_MESSAGE_TEXT_MAX_BYTES: usize = 64 * 1024; const GATE_REF_MAX_BYTES: usize = 256; const CREDENTIAL_REF_MAX_BYTES: usize = 512; +/// Inline-attachment budgets, mirroring the v1 web gateway: at most +/// `MAX_INLINE_ATTACHMENTS` files, `MAX_INLINE_ATTACHMENT_BYTES` decoded bytes +/// per file, and `MAX_INLINE_TOTAL_ATTACHMENT_BYTES` decoded bytes total. +const MAX_INLINE_ATTACHMENTS: usize = 10; +const MAX_INLINE_ATTACHMENT_BYTES: usize = 5 * 1024 * 1024; +const MAX_INLINE_TOTAL_ATTACHMENT_BYTES: usize = 10 * 1024 * 1024; +const ATTACHMENT_FILENAME_MAX_BYTES: usize = 256; /// Authenticated WebUI caller after route auth has already completed. /// @@ -70,6 +78,20 @@ pub struct WebUiCreateThreadRequest { pub requested_thread_id: Option, } +/// One inline attachment in a browser send-message body. +/// +/// `data_base64` is the base64-encoded file bytes; `mime_type` is validated +/// against the shared attachment format registry. This is the only place raw +/// upload bytes enter the workflow — they are decoded, budgeted, and landed in +/// storage, never carried on the (serializable) inbound command. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] +pub struct WebUiInboundAttachment { + pub mime_type: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub filename: Option, + pub data_base64: String, +} + /// Browser body for WebUI send-message mutation. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] pub struct WebUiSendMessageRequest { @@ -79,6 +101,100 @@ pub struct WebUiSendMessageRequest { pub thread_id: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub content: Option, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub attachments: Vec, +} + +fn normalize_attachment_mime(raw: &str) -> String { + raw.split(';') + .next() + .unwrap_or(raw) + .trim() + .to_ascii_lowercase() +} + +impl WebUiSendMessageRequest { + /// Validate and decode the inline attachments into bytes-bearing + /// [`InboundAttachment`]s ready for landing. + /// + /// Enforces the per-file / per-message / count budgets and rejects + /// unsupported MIME types (per the shared format registry) and malformed + /// base64 with a stable validation error. Kept separate from + /// [`Self::into_command`] so the serializable command never carries raw + /// bytes. + pub fn decode_attachments( + &self, + ) -> Result, WebUiInboundValidationError> { + use base64::Engine; + + if self.attachments.len() > MAX_INLINE_ATTACHMENTS { + return Err(WebUiInboundValidationError::new( + "attachments", + WebUiInboundValidationCode::TooLong, + )); + } + + let mut decoded = Vec::with_capacity(self.attachments.len()); + let mut total_bytes = 0usize; + for attachment in &self.attachments { + let mime = normalize_attachment_mime(&attachment.mime_type); + if !ironclaw_common::is_supported_mime(&mime) { + return Err(WebUiInboundValidationError::new( + "attachments.mime_type", + WebUiInboundValidationCode::InvalidValue, + )); + } + + let bytes = base64::engine::general_purpose::STANDARD + .decode(attachment.data_base64.as_bytes()) + .map_err(|_| { + WebUiInboundValidationError::new( + "attachments.data_base64", + WebUiInboundValidationCode::InvalidValue, + ) + })?; + if bytes.len() > MAX_INLINE_ATTACHMENT_BYTES { + return Err(WebUiInboundValidationError::new( + "attachments", + WebUiInboundValidationCode::TooLong, + )); + } + total_bytes = total_bytes.saturating_add(bytes.len()); + if total_bytes > MAX_INLINE_TOTAL_ATTACHMENT_BYTES { + return Err(WebUiInboundValidationError::new( + "attachments", + WebUiInboundValidationCode::TooLong, + )); + } + + let filename = attachment + .filename + .as_deref() + .map(str::trim) + .filter(|name| !name.is_empty()); + if let Some(name) = filename + && name.len() > ATTACHMENT_FILENAME_MAX_BYTES + { + return Err(WebUiInboundValidationError::new( + "attachments.filename", + WebUiInboundValidationCode::TooLong, + )); + } + + let fallback_extension = ironclaw_common::canonical_extension(&mime) + .unwrap_or("bin") + .to_string(); + decoded.push(InboundAttachment { + id: format!("webui-attachment-{}", decoded.len()), + kind: ironclaw_common::kind_for_mime(&mime), + mime_type: mime, + filename: filename.map(str::to_string), + fallback_extension, + bytes, + }); + } + Ok(decoded) + } } /// Browser body for WebUI cancel-run mutation. diff --git a/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs b/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs index 59f6c253342..1db9a62e18d 100644 --- a/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs +++ b/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs @@ -11,6 +11,7 @@ use std::{ use async_trait::async_trait; use chrono::Utc; +use ironclaw_attachments::InboundAttachment; use ironclaw_auth::{CredentialAccountId, CredentialAccountProjection}; use ironclaw_host_api::{AgentId, ApprovalRequestId, ProjectId, TenantId, ThreadId, UserId}; use ironclaw_product_adapters::{ @@ -23,7 +24,7 @@ use ironclaw_product_workflow::{ AUTOMATION_TRIGGER_THREAD_SOURCE_TAG, ApprovalInteractionDecision, ApprovalInteractionService, AuthInteractionDecision, AuthInteractionService, AutomationListRequest, AutomationProductFacade, CodexLoginStart, ExtensionCredentialSetupService, - ExtensionCredentialStatusRequest, ExtensionCredentialSubmitRequest, + ExtensionCredentialStatusRequest, ExtensionCredentialSubmitRequest, InboundAttachmentLander, LifecycleExtensionCredentialRequirement, LifecycleExtensionCredentialSetup, LifecycleExtensionOnboarding, LifecycleExtensionRuntimeKind, LifecycleExtensionSource, LifecycleExtensionSummary, LifecycleInstalledExtensionSummary, LifecyclePackageKind, @@ -61,10 +62,10 @@ use ironclaw_product_workflow::{ use ironclaw_threads::{ AcceptInboundMessageRequest, AcceptedInboundMessage, AcceptedInboundMessageReplay, AppendAssistantDraftRequest, AppendCapabilityDisplayPreviewRequest, - AppendToolResultReferenceRequest, ContextMessages, ContextWindow, CreateSummaryArtifactRequest, - EnsureThreadRequest, InMemorySessionThreadService, ListThreadsForScopeRequest, - ListThreadsForScopeResponse, LoadContextMessagesRequest, LoadContextWindowRequest, - MessageContent, MessageKind, MessageStatus, RedactMessageRequest, + AppendToolResultReferenceRequest, AttachmentKind, AttachmentRef, ContextMessages, + ContextWindow, CreateSummaryArtifactRequest, EnsureThreadRequest, InMemorySessionThreadService, + ListThreadsForScopeRequest, ListThreadsForScopeResponse, LoadContextMessagesRequest, + LoadContextWindowRequest, MessageContent, MessageKind, MessageStatus, RedactMessageRequest, ReplayAcceptedInboundMessageRequest, SessionThreadError, SessionThreadRecord, SessionThreadService, SummaryArtifact, ThreadHistory, ThreadHistoryRequest, ThreadMessageId, ThreadMessageRecord, ThreadScope, UpdateAssistantDraftRequest, @@ -7974,3 +7975,142 @@ async fn list_threads_skips_hidden_automation_threads_when_filling_page() { ); assert_eq!(second_page.next_cursor, None); } + +/// Test lander that records what it was asked to land and returns a ref per +/// attachment with a deterministic `storage_key`, so the facade test can assert +/// both that decode→land ran and that the returned refs reach the transcript. +#[derive(Default)] +struct RecordingLander { + landed: Mutex)>>, +} + +#[async_trait] +impl InboundAttachmentLander for RecordingLander { + async fn land( + &self, + _thread_scope: &ThreadScope, + message_id: &str, + attachments: Vec, + ) -> Result, RebornServicesError> { + let refs = attachments + .iter() + .enumerate() + .map(|(index, attachment)| AttachmentRef { + id: attachment.id.clone(), + kind: attachment.kind, + mime_type: attachment.mime_type.clone(), + filename: attachment.filename.clone(), + size_bytes: Some(attachment.bytes.len() as u64), + storage_key: Some(format!( + "/workspace/attachments/test/{message_id}-{index}-landed" + )), + extracted_text: None, + }) + .collect(); + self.landed + .lock() + .expect("lander mutex") + .push((message_id.to_string(), attachments)); + Ok(refs) + } +} + +#[tokio::test] +async fn submit_turn_lands_attachments_and_persists_refs_on_the_user_message() { + use base64::Engine; + + let threads: Arc = Arc::new(InMemorySessionThreadService::default()); + let coordinator = Arc::new(FakeTurnCoordinator::default()); + let lander = Arc::new(RecordingLander::default()); + let services = RebornServices::new(Arc::clone(&threads), coordinator.clone()) + .with_inbound_attachments(lander.clone()); + create_thread_for(&services, caller(), "thread-alpha").await; + + let pdf_b64 = base64::engine::general_purpose::STANDARD.encode(b"%PDF-1.7 body"); + services + .submit_turn( + caller(), + serde_json::from_value::(json!({ + "client_action_id": "send-att", + "thread_id": "thread-alpha", + "content": "see attached", + "attachments": [{ + "mime_type": "application/pdf", + "filename": "report.pdf", + "data_base64": pdf_b64, + }], + })) + .expect("request"), + ) + .await + .expect("submit succeeds"); + + // The lander was invoked with the decoded attachment bytes + metadata. + { + let landed = lander.landed.lock().expect("lander mutex"); + assert_eq!(landed.len(), 1); + assert_eq!(landed[0].1.len(), 1); + assert_eq!(landed[0].1[0].mime_type, "application/pdf"); + assert_eq!(landed[0].1[0].filename.as_deref(), Some("report.pdf")); + assert_eq!(landed[0].1[0].bytes, b"%PDF-1.7 body"); + } + + // The returned refs are persisted on the accepted user message. + let history = threads + .list_thread_history(ThreadHistoryRequest { + scope: thread_scope_for(&caller()), + thread_id: ThreadId::new("thread-alpha").unwrap(), + }) + .await + .expect("history"); + let user_message = history + .messages + .iter() + .find(|message| message.kind == MessageKind::User) + .expect("user message present"); + assert_eq!(user_message.content.as_deref(), Some("see attached")); + assert_eq!(user_message.attachments.len(), 1); + let attachment_ref = &user_message.attachments[0]; + assert_eq!(attachment_ref.kind, AttachmentKind::Document); + assert_eq!(attachment_ref.mime_type, "application/pdf"); + assert_eq!(attachment_ref.filename.as_deref(), Some("report.pdf")); + assert!( + attachment_ref + .storage_key + .as_deref() + .is_some_and(|key| key.ends_with("-landed")), + "expected landed storage_key, got {:?}", + attachment_ref.storage_key + ); +} + +#[tokio::test] +async fn submit_turn_rejects_attachments_when_no_lander_is_wired() { + use base64::Engine; + + let threads: Arc = Arc::new(InMemorySessionThreadService::default()); + let coordinator = Arc::new(FakeTurnCoordinator::default()); + // No `.with_inbound_attachments(...)`: a deployment without attachment + // support must reject rather than silently drop the files. + let services = RebornServices::new(threads, coordinator); + create_thread_for(&services, caller(), "thread-alpha").await; + + let pdf_b64 = base64::engine::general_purpose::STANDARD.encode(b"%PDF-1.7"); + let err = services + .submit_turn( + caller(), + serde_json::from_value::(json!({ + "client_action_id": "send-att", + "thread_id": "thread-alpha", + "content": "see attached", + "attachments": [{ + "mime_type": "application/pdf", + "data_base64": pdf_b64, + }], + })) + .expect("request"), + ) + .await + .expect_err("attachments without a lander must be rejected"); + assert_eq!(err.kind, RebornServicesErrorKind::ServiceUnavailable); +} diff --git a/crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs b/crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs index bfada67fbcd..c414ceee815 100644 --- a/crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs +++ b/crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs @@ -1,10 +1,11 @@ //! Contract tests for route-independent WebUI inbound DTOs. +use base64::Engine; use ironclaw_host_api::{AgentId, ProjectId, TenantId, ThreadId, UserId}; use ironclaw_product_workflow::{ WebUiAuthenticatedCaller, WebUiCancelReason, WebUiCancelRunRequest, WebUiCreateThreadRequest, - WebUiGateResolution, WebUiInboundCommand, WebUiInboundValidationCode, WebUiResolveGateRequest, - WebUiSendMessageRequest, + WebUiGateResolution, WebUiInboundAttachment, WebUiInboundCommand, WebUiInboundValidationCode, + WebUiResolveGateRequest, WebUiSendMessageRequest, }; use ironclaw_turns::SanitizedCancelReason; use serde_json::json; @@ -265,6 +266,7 @@ fn command_serializes_with_stable_command_tag() { client_action_id: Some("send-1".to_string()), thread_id: Some("thread-alpha".to_string()), content: Some("hello".to_string()), + attachments: Vec::new(), }; let command = request.into_command(caller()).expect("valid command"); @@ -281,6 +283,7 @@ fn token_fields_reject_control_characters() { client_action_id: Some("send\n1".to_string()), thread_id: Some("thread-alpha".to_string()), content: Some("hello".to_string()), + attachments: Vec::new(), }; let err = request.into_command(caller()).expect_err("control char"); @@ -353,3 +356,129 @@ fn cancel_reason_serializes_as_snake_case() { json!("policy") ); } + +fn b64(bytes: &[u8]) -> String { + base64::engine::general_purpose::STANDARD.encode(bytes) +} + +fn send_with_attachments(attachments: Vec) -> WebUiSendMessageRequest { + WebUiSendMessageRequest { + client_action_id: Some("send-att".to_string()), + thread_id: Some("thread-alpha".to_string()), + content: Some("see attached".to_string()), + attachments, + } +} + +#[test] +fn decode_attachments_decodes_metadata_kind_and_bytes() { + let request = send_with_attachments(vec![ + WebUiInboundAttachment { + mime_type: "application/pdf".to_string(), + filename: Some("report.pdf".to_string()), + data_base64: b64(b"%PDF-1.7 body"), + }, + WebUiInboundAttachment { + // Uppercase + charset params normalize; kind derives from registry. + mime_type: "IMAGE/PNG; charset=binary".to_string(), + filename: None, + data_base64: b64(&[0x89, 0x50, 0x4E, 0x47]), + }, + ]); + + let decoded = request + .decode_attachments() + .expect("valid attachments decode"); + assert_eq!(decoded.len(), 2); + + assert_eq!(decoded[0].mime_type, "application/pdf"); + assert_eq!(decoded[0].filename.as_deref(), Some("report.pdf")); + assert_eq!(decoded[0].fallback_extension, "pdf"); + assert_eq!(decoded[0].bytes, b"%PDF-1.7 body"); + + assert_eq!(decoded[1].mime_type, "image/png"); + assert!(decoded[1].filename.is_none()); + assert_eq!(decoded[1].fallback_extension, "png"); +} + +#[test] +fn decode_attachments_rejects_unsupported_mime() { + let request = send_with_attachments(vec![WebUiInboundAttachment { + mime_type: "image/svg+xml".to_string(), + filename: None, + data_base64: b64(b""), + }]); + let err = request + .decode_attachments() + .expect_err("svg is unsupported"); + assert_eq!(err.field, "attachments.mime_type"); + assert_eq!(err.code, WebUiInboundValidationCode::InvalidValue); +} + +#[test] +fn decode_attachments_rejects_malformed_base64() { + let request = send_with_attachments(vec![WebUiInboundAttachment { + mime_type: "application/pdf".to_string(), + filename: None, + data_base64: "not valid base64!!!".to_string(), + }]); + let err = request.decode_attachments().expect_err("bad base64"); + assert_eq!(err.field, "attachments.data_base64"); + assert_eq!(err.code, WebUiInboundValidationCode::InvalidValue); +} + +#[test] +fn decode_attachments_rejects_per_file_oversize() { + let request = send_with_attachments(vec![WebUiInboundAttachment { + mime_type: "application/pdf".to_string(), + filename: None, + data_base64: b64(&vec![0u8; 5 * 1024 * 1024 + 1]), + }]); + let err = request.decode_attachments().expect_err("over per-file cap"); + assert_eq!(err.field, "attachments"); + assert_eq!(err.code, WebUiInboundValidationCode::TooLong); +} + +#[test] +fn decode_attachments_rejects_total_oversize() { + let three_mib = vec![0u8; 3 * 1024 * 1024]; + let request = send_with_attachments(vec![ + WebUiInboundAttachment { + mime_type: "application/pdf".to_string(), + filename: None, + data_base64: b64(&three_mib), + }; + 4 // 12 MiB total > 10 MiB cap + ]); + let err = request.decode_attachments().expect_err("over total cap"); + assert_eq!(err.field, "attachments"); + assert_eq!(err.code, WebUiInboundValidationCode::TooLong); +} + +#[test] +fn decode_attachments_rejects_too_many() { + let request = send_with_attachments(vec![ + WebUiInboundAttachment { + mime_type: "text/plain".to_string(), + filename: None, + data_base64: b64(b"x"), + }; + 11 // > MAX_INLINE_ATTACHMENTS (10) + ]); + let err = request + .decode_attachments() + .expect_err("too many attachments"); + assert_eq!(err.field, "attachments"); + assert_eq!(err.code, WebUiInboundValidationCode::TooLong); +} + +#[test] +fn decode_attachments_empty_is_ok() { + let request = send_with_attachments(Vec::new()); + assert!( + request + .decode_attachments() + .expect("empty decodes") + .is_empty() + ); +} diff --git a/crates/ironclaw_reborn_composition/CLAUDE.md b/crates/ironclaw_reborn_composition/CLAUDE.md index ead743655f7..563316aa2e9 100644 --- a/crates/ironclaw_reborn_composition/CLAUDE.md +++ b/crates/ironclaw_reborn_composition/CLAUDE.md @@ -88,8 +88,8 @@ Inbound order (outer → inner → handler): enforces it before auth runs (so an oversized payload never spends a bearer-validation step). Today: `create_thread`, product-auth OAuth start, manual-token setup/secret-submit, accounts list/select/recovery/ - refresh, and lifecycle cleanup — all 16 KiB; `send_message` 1 MiB; - `cancel_run` and `resolve_gate` 4 KiB; `get_timeline`, + refresh, and lifecycle cleanup — all 16 KiB; `send_message` 14 MiB + (text + base64 inline attachments); `cancel_run` and `resolve_gate` 4 KiB; `get_timeline`, `stream_events`, and product-auth OAuth callback `NoBody`. `BodyLimitPolicy` is an exhaustive `match`, so a new variant added upstream fails the build rather than silently disabling @@ -320,7 +320,7 @@ rows are inventoried here, not implemented in the current PR. - **Body limit** — descriptor-driven per-route via `webui_body_limit::enforce_body_limit`. Caps come from `ironclaw_webui_v2::webui_v2_routes()`: `create_thread` 16 KiB, - `send_message` 1 MiB, `cancel_run` / `resolve_gate` 4 KiB, + `send_message` 14 MiB, `cancel_run` / `resolve_gate` 4 KiB, `get_timeline` / `stream_events` `NoBody`. The outer `RequestBodyLimitLayer` at `config.max_body_bytes` (14 MiB default) is kept as defense in depth for paths that don't match any v2 diff --git a/crates/ironclaw_reborn_composition/Cargo.toml b/crates/ironclaw_reborn_composition/Cargo.toml index a0188e2f235..9db0f76331d 100644 --- a/crates/ironclaw_reborn_composition/Cargo.toml +++ b/crates/ironclaw_reborn_composition/Cargo.toml @@ -98,6 +98,7 @@ ironclaw_auth = { path = "../ironclaw_auth" } ironclaw_common = { path = "../ironclaw_common" } ironclaw_capabilities = { path = "../ironclaw_capabilities" } ironclaw_approvals = { path = "../ironclaw_approvals" } +ironclaw_attachments = { path = "../ironclaw_attachments" } ironclaw_authorization = { path = "../ironclaw_authorization" } ironclaw_conversations = { path = "../ironclaw_conversations" } ironclaw_event_projections = { path = "../ironclaw_event_projections" } diff --git a/crates/ironclaw_reborn_composition/src/attachment_landing.rs b/crates/ironclaw_reborn_composition/src/attachment_landing.rs new file mode 100644 index 00000000000..3f32e8fb171 --- /dev/null +++ b/crates/ironclaw_reborn_composition/src/attachment_landing.rs @@ -0,0 +1,153 @@ +//! Project-scoped inbound attachment landing for the WebUI v2 facade. +//! +//! Implements the [`InboundAttachmentLander`] port the facade calls before +//! accepting a user message: it writes attachment bytes through the +//! project-scoped workspace [`ScopedFilesystem`] — the same filesystem +//! authority the agent's file tools resolve through — and returns the +//! transcript references to persist. Going through that one authority is what +//! makes a landed attachment readable by `file_read`/`list_dir` at the recorded +//! `storage_key` in this and later turns. + +use std::sync::Arc; + +use async_trait::async_trait; +use ironclaw_attachments::{ + DEFAULT_PROJECT_MOUNT_ALIAS, InboundAttachment, land_inbound_attachments, +}; +use ironclaw_filesystem::{RootFilesystem, ScopedFilesystem}; +use ironclaw_product_workflow::{ + InboundAttachmentLander, RebornServicesError, RebornServicesErrorCode, RebornServicesErrorKind, +}; +use ironclaw_threads::{AttachmentRef, ThreadScope}; + +/// Lands inbound attachments through a project-scoped workspace filesystem. +pub(crate) struct ProjectScopedAttachmentLander { + filesystem: Arc>, + project_alias: String, +} + +impl ProjectScopedAttachmentLander { + pub(crate) fn new(filesystem: Arc>) -> Self { + Self { + filesystem, + project_alias: DEFAULT_PROJECT_MOUNT_ALIAS.to_string(), + } + } +} + +#[async_trait] +impl InboundAttachmentLander for ProjectScopedAttachmentLander { + async fn land( + &self, + thread_scope: &ThreadScope, + message_id: &str, + attachments: Vec, + ) -> Result, RebornServicesError> { + let scope = thread_scope.to_resource_scope(); + // Partition by UTC date so a project's attachments directory stays + // browsable; the rest of the path (message id + index + filename) makes + // each attachment uniquely addressable. + let date = chrono::Utc::now().format("%Y-%m-%d").to_string(); + land_inbound_attachments( + self.filesystem.as_ref(), + &scope, + &self.project_alias, + &date, + message_id, + attachments, + ) + .await + .map_err(|_| RebornServicesError { + code: RebornServicesErrorCode::Internal, + kind: RebornServicesErrorKind::Internal, + status_code: 500, + retryable: false, + field: None, + validation_code: None, + }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + use ironclaw_filesystem::InMemoryBackend; + use ironclaw_host_api::{ + AgentId, MountAlias, MountGrant, MountPermissions, MountView, TenantId, UserId, VirtualPath, + }; + use ironclaw_threads::AttachmentKind; + + fn workspace_fs(permissions: MountPermissions) -> Arc> { + let view = MountView::new(vec![MountGrant::new( + MountAlias::new(DEFAULT_PROJECT_MOUNT_ALIAS).unwrap(), + VirtualPath::new("/projects/workspace").unwrap(), + permissions, + )]) + .unwrap(); + Arc::new(ScopedFilesystem::with_fixed_view( + Arc::new(InMemoryBackend::new()), + view, + )) + } + + fn thread_scope() -> ThreadScope { + ThreadScope { + tenant_id: TenantId::new("tenant-test").unwrap(), + agent_id: AgentId::new("agent-test").unwrap(), + project_id: None, + owner_user_id: Some(UserId::new("user-test").unwrap()), + mission_id: None, + } + } + + #[tokio::test] + async fn lands_attachment_and_returns_ref_with_storage_key() { + let lander = + ProjectScopedAttachmentLander::new(workspace_fs(MountPermissions::read_write())); + let refs = lander + .land( + &thread_scope(), + "msg1", + vec![InboundAttachment { + id: "att-0".to_string(), + kind: AttachmentKind::Document, + mime_type: "application/pdf".to_string(), + filename: Some("report.pdf".to_string()), + fallback_extension: "pdf".to_string(), + bytes: b"%PDF-1.7".to_vec(), + }], + ) + .await + .expect("landing succeeds through a read-write workspace mount"); + assert_eq!(refs.len(), 1); + let storage_key = refs[0].storage_key.as_deref().expect("storage_key set"); + assert!( + storage_key.starts_with("/workspace/attachments/") + && storage_key.ends_with("-report.pdf"), + "unexpected storage key: {storage_key}" + ); + } + + #[tokio::test] + async fn read_only_workspace_mount_maps_to_internal_error() { + let lander = + ProjectScopedAttachmentLander::new(workspace_fs(MountPermissions::read_only())); + let err = lander + .land( + &thread_scope(), + "msg1", + vec![InboundAttachment { + id: "att-0".to_string(), + kind: AttachmentKind::Document, + mime_type: "application/pdf".to_string(), + filename: Some("report.pdf".to_string()), + fallback_extension: "pdf".to_string(), + bytes: b"%PDF".to_vec(), + }], + ) + .await + .expect_err("read-only workspace mount must fail closed"); + assert_eq!(err.code, RebornServicesErrorCode::Internal); + } +} diff --git a/crates/ironclaw_reborn_composition/src/lib.rs b/crates/ironclaw_reborn_composition/src/lib.rs index f16acffd79b..0bfb913b4bc 100644 --- a/crates/ironclaw_reborn_composition/src/lib.rs +++ b/crates/ironclaw_reborn_composition/src/lib.rs @@ -22,6 +22,7 @@ use std::sync::Arc; #[cfg(test)] mod approval_test_support; +mod attachment_landing; mod auth; #[cfg(test)] mod auth_dcr_tests; diff --git a/crates/ironclaw_reborn_composition/src/runtime.rs b/crates/ironclaw_reborn_composition/src/runtime.rs index a974ebf1da3..c9830e81409 100644 --- a/crates/ironclaw_reborn_composition/src/runtime.rs +++ b/crates/ironclaw_reborn_composition/src/runtime.rs @@ -1079,6 +1079,19 @@ impl RebornRuntime { self.skill_activation_source.clone() } + /// Project-scoped workspace filesystem the agent's file tools resolve + /// through. Used to land inbound attachment bytes at paths the agent can + /// later read back. `None` when no local runtime is composed. + pub(crate) fn webui_workspace_filesystem( + &self, + ) -> Option>> + { + self.services + .local_runtime + .as_ref() + .map(|rt| Arc::clone(&rt.workspace_filesystem)) + } + /// Test-only handle on the resource governor backing the budget /// accountant. Exposed under `test-support` so integration tests can /// assert ledger state after a `send_user_message` round-trip. @@ -5519,6 +5532,7 @@ mod tests { client_action_id: Some("send-webui-stream-message".to_string()), thread_id: Some(created.thread.thread_id.to_string()), content: Some("hello webui stream".to_string()), + attachments: Vec::new(), }, ) .await @@ -6559,6 +6573,7 @@ mod tests { client_action_id: Some("send-webui-skill-message".to_string()), thread_id: Some(created.thread.thread_id.to_string()), content: Some("$webui-helper please help".to_string()), + attachments: Vec::new(), }, ) .await diff --git a/crates/ironclaw_reborn_composition/src/webui.rs b/crates/ironclaw_reborn_composition/src/webui.rs index e0208760306..21ec0fd4cb6 100644 --- a/crates/ironclaw_reborn_composition/src/webui.rs +++ b/crates/ironclaw_reborn_composition/src/webui.rs @@ -86,6 +86,11 @@ pub(crate) fn build_webui_services_with_connectable_channels( ) .with_approval_interactions(runtime.webui_approval_interaction_service()) .with_auth_interactions(runtime.webui_auth_interaction_service()); + if let Some(workspace_filesystem) = runtime.webui_workspace_filesystem() { + api = api.with_inbound_attachments(Arc::new( + crate::attachment_landing::ProjectScopedAttachmentLander::new(workspace_filesystem), + )); + } if let Some(skill_activation_source) = runtime.webui_skill_activation_source() { let activation_recorder = Arc::clone(&skill_activation_source); let activation_clearer = skill_activation_source; diff --git a/crates/ironclaw_reborn_composition/src/webui_body_limit.rs b/crates/ironclaw_reborn_composition/src/webui_body_limit.rs index 90148de9397..97eb5e41e9d 100644 --- a/crates/ironclaw_reborn_composition/src/webui_body_limit.rs +++ b/crates/ironclaw_reborn_composition/src/webui_body_limit.rs @@ -2,7 +2,7 @@ //! surface. //! //! `ironclaw_webui_v2::webui_v2_routes()` carries a [`BodyLimitPolicy`] -//! per route (16 KiB for `create_thread`, 1 MiB for `send_message`, 4 +//! per route (16 KiB for `create_thread`, 14 MiB for `send_message`, 4 //! KiB for `cancel_run`/`resolve_gate`, `NoBody` for the read / //! streaming routes). The v2 crate's CLAUDE.md designates enforcement //! as host-composition responsibility; this module is that enforcement. @@ -150,7 +150,7 @@ pub(crate) async fn enforce_body_limit( return too_large_for(route.policy); } - // Cast is safe: the largest descriptor cap is 1 MiB and the + // Cast is safe: the largest descriptor cap is 14 MiB and the // workspace targets are 64-bit platforms. `usize::try_from` returns // an error only on 32-bit platforms with a > 4 GiB cap, which the // v2 descriptors do not declare. @@ -170,7 +170,7 @@ pub(crate) async fn enforce_body_limit( // Buffer with the descriptor cap as the hard limit. `to_bytes` // reads up to `limit` bytes and returns an error if the body // produced more. Buffering is acceptable because every v2 route - // caps at ≤ 1 MiB and the v2 handlers already buffer via + // caps at ≤ 14 MiB and the v2 handlers already buffer via // `Json` downstream. let (parts, body) = request.into_parts(); let buffered = match axum::body::to_bytes(body, max_bytes_usize).await { @@ -272,9 +272,10 @@ mod tests { descriptors.len(), "every descriptor produced a RouteBodyLimit entry", ); - // Locks in the descriptor contract: send_message must be 1 MiB, - // get_timeline and stream_events must be NoBody. A regression - // that flips these would trip here before reaching production. + // Locks in the descriptor contract: send_message must be 14 MiB + // (text + base64 inline attachments), get_timeline and + // stream_events must be NoBody. A regression that flips these + // would trip here before reaching production. let send = state .routes .iter() @@ -282,7 +283,7 @@ mod tests { .expect("send_message route"); assert!(matches!( send.policy, - ResolvedBodyPolicy::Limited { max_bytes } if max_bytes == 1024 * 1024, + ResolvedBodyPolicy::Limited { max_bytes } if max_bytes == 14 * 1024 * 1024, )); let timeline = state .routes diff --git a/crates/ironclaw_webui_v2/src/descriptors.rs b/crates/ironclaw_webui_v2/src/descriptors.rs index fe1cade82f1..b4d3d5bf319 100644 --- a/crates/ironclaw_webui_v2/src/descriptors.rs +++ b/crates/ironclaw_webui_v2/src/descriptors.rs @@ -244,9 +244,11 @@ fn send_message_descriptor() -> IngressRouteDescriptor { NetworkMethod::Post, WEBUI_V2_PATTERN_SEND_MESSAGE, mutation_policy( - // Message bodies carry user content. 1 MiB is the same cap the - // existing turn admission layer enforces. - body_limit_kib(1024), + // Message bodies carry user text plus optional base64-encoded inline + // attachments. 14 MiB matches the gateway-wide body budget and covers + // base64 of the 10 MiB decoded per-message attachment cap (the facade + // enforces the 5 MiB-per-file / 10 MiB-total decoded budgets). + body_limit_kib(14 * 1024), mutation_rate_limit(), AuditTraceClass::UserAction, AllowedEffectPath::TurnCoordinator, diff --git a/crates/ironclaw_webui_v2/tests/webui_v2_descriptors_contract.rs b/crates/ironclaw_webui_v2/tests/webui_v2_descriptors_contract.rs index 1c6b01d1749..e54da843e52 100644 --- a/crates/ironclaw_webui_v2/tests/webui_v2_descriptors_contract.rs +++ b/crates/ironclaw_webui_v2/tests/webui_v2_descriptors_contract.rs @@ -112,7 +112,7 @@ fn expected_table() -> Vec { listener_class: ListenerClass::LocalGateway, auth_schemes: &[IngressAuthScheme::BearerToken], scope_source: IngressScopeSource::AuthenticatedCaller, - body_limit: body_limit_kib(1024), + body_limit: body_limit_kib(14 * 1024), rate_limit_max: 60, rate_limit_window_seconds: 60, rate_limit_scope: RateLimitScope::PerCaller, From 6e19c6ad3011fbd40d99300f5660d63aad9cd9f7 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Thu, 11 Jun 2026 12:24:07 -0700 Subject: [PATCH 2/4] refactor(reborn): route attachment-landing errors through canonical constructors Reuse the error type's own constructors instead of open-coding status/kind: - submit_turn's no-lander path now calls RebornServicesError::service_unavailable(false) instead of an inline from_status_kind(Unavailable, ServiceUnavailable, 503, false). - Add a public RebornServicesError::internal() so host-composition adapters stop hand-rolling the full struct literal (which silently drifts if a field is added); ProjectScopedAttachmentLander uses it. - The lander no longer discards the underlying AttachmentLandingError: it logs it (warn) before mapping to the sanitized 500, so an operator can tell a misconfigured read-only mount from a write failure. - decode_attachments derives the per-attachment id from enumerate() rather than the decoded-accumulator length. --- .../src/reborn_services.rs | 16 ++++----- .../src/reborn_services/error.rs | 8 +++++ .../src/webui_inbound.rs | 12 +++---- .../tests/reborn_services_contract.rs | 3 +- .../tests/webui_inbound_contract.rs | 4 +-- .../src/attachment_landing.rs | 33 ++++++++----------- .../src/local_dev_mounts.rs | 2 +- 7 files changed, 37 insertions(+), 41 deletions(-) diff --git a/crates/ironclaw_product_workflow/src/reborn_services.rs b/crates/ironclaw_product_workflow/src/reborn_services.rs index adb181993a9..b567a02fe30 100644 --- a/crates/ironclaw_product_workflow/src/reborn_services.rs +++ b/crates/ironclaw_product_workflow/src/reborn_services.rs @@ -25,8 +25,8 @@ use ironclaw_product_adapters::{ use ironclaw_threads::{ AcceptInboundMessageRequest, AcceptedInboundMessageReplay, AttachmentRef, EnsureThreadRequest, MessageContent, MessageStatus, ReplayAcceptedInboundMessageRequest, SessionThreadError, - SessionThreadRecord, SessionThreadService, ThreadHistory, ThreadHistoryRequest, ThreadMessageId, - ThreadScope, + SessionThreadRecord, SessionThreadService, ThreadHistory, ThreadHistoryRequest, + ThreadMessageId, ThreadScope, }; use ironclaw_turns::{ AcceptedMessageRef, GateRef, GetRunStateRequest, IdempotencyKey, ResumeTurnPrecondition, @@ -1726,14 +1726,10 @@ impl RebornServicesApi for RebornServices { let message_content = if attachments.is_empty() { MessageContent::text(content.clone()) } else { - let lander = self.inbound_attachments.as_ref().ok_or_else(|| { - RebornServicesError::from_status_kind( - RebornServicesErrorCode::Unavailable, - RebornServicesErrorKind::ServiceUnavailable, - 503, - false, - ) - })?; + let lander = self + .inbound_attachments + .as_ref() + .ok_or_else(|| RebornServicesError::service_unavailable(false))?; let refs = lander .land(&thread_scope, &external_event_id, attachments) .await?; diff --git a/crates/ironclaw_product_workflow/src/reborn_services/error.rs b/crates/ironclaw_product_workflow/src/reborn_services/error.rs index ab37c2c6968..76b4ca0ce3d 100644 --- a/crates/ironclaw_product_workflow/src/reborn_services/error.rs +++ b/crates/ironclaw_product_workflow/src/reborn_services/error.rs @@ -91,6 +91,14 @@ impl RebornServicesError { Self::from_status(RebornServicesErrorCode::Internal, 500, false) } + /// Sanitized internal (500) error for host-composition adapters that + /// implement workflow ports from outside this crate. The struct fields are + /// public, but new construction sites should route through one constructor + /// rather than hand-rolling the status/kind pairing. + pub fn internal() -> Self { + Self::from_status(RebornServicesErrorCode::Internal, 500, false) + } + pub(super) fn service_unavailable(retryable: bool) -> Self { Self::from_status_kind( RebornServicesErrorCode::Unavailable, diff --git a/crates/ironclaw_product_workflow/src/webui_inbound.rs b/crates/ironclaw_product_workflow/src/webui_inbound.rs index 4664edf14c5..11b6d7ec34b 100644 --- a/crates/ironclaw_product_workflow/src/webui_inbound.rs +++ b/crates/ironclaw_product_workflow/src/webui_inbound.rs @@ -136,7 +136,7 @@ impl WebUiSendMessageRequest { let mut decoded = Vec::with_capacity(self.attachments.len()); let mut total_bytes = 0usize; - for attachment in &self.attachments { + for (index, attachment) in self.attachments.iter().enumerate() { let mime = normalize_attachment_mime(&attachment.mime_type); if !ironclaw_common::is_supported_mime(&mime) { return Err(WebUiInboundValidationError::new( @@ -181,15 +181,13 @@ impl WebUiSendMessageRequest { )); } - let fallback_extension = ironclaw_common::canonical_extension(&mime) - .unwrap_or("bin") - .to_string(); + // `kind` and the fallback filename extension are derived from + // `mime_type` inside the landing bridge, so the DTO carries only the + // raw upload fields here. decoded.push(InboundAttachment { - id: format!("webui-attachment-{}", decoded.len()), - kind: ironclaw_common::kind_for_mime(&mime), + id: format!("webui-attachment-{index}"), mime_type: mime, filename: filename.map(str::to_string), - fallback_extension, bytes, }); } diff --git a/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs b/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs index 1db9a62e18d..3ad58cbc83b 100644 --- a/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs +++ b/crates/ironclaw_product_workflow/tests/reborn_services_contract.rs @@ -7997,7 +7997,8 @@ impl InboundAttachmentLander for RecordingLander { .enumerate() .map(|(index, attachment)| AttachmentRef { id: attachment.id.clone(), - kind: attachment.kind, + // The real bridge derives kind from the MIME type; mirror that. + kind: ironclaw_common::kind_for_mime(&attachment.mime_type), mime_type: attachment.mime_type.clone(), filename: attachment.filename.clone(), size_bytes: Some(attachment.bytes.len() as u64), diff --git a/crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs b/crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs index c414ceee815..b212f631815 100644 --- a/crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs +++ b/crates/ironclaw_product_workflow/tests/webui_inbound_contract.rs @@ -391,14 +391,14 @@ fn decode_attachments_decodes_metadata_kind_and_bytes() { .expect("valid attachments decode"); assert_eq!(decoded.len(), 2); + // `kind`/`fallback_extension` are derived from `mime_type` inside the + // landing bridge, so the decoded DTO carries only the raw upload fields. assert_eq!(decoded[0].mime_type, "application/pdf"); assert_eq!(decoded[0].filename.as_deref(), Some("report.pdf")); - assert_eq!(decoded[0].fallback_extension, "pdf"); assert_eq!(decoded[0].bytes, b"%PDF-1.7 body"); assert_eq!(decoded[1].mime_type, "image/png"); assert!(decoded[1].filename.is_none()); - assert_eq!(decoded[1].fallback_extension, "png"); } #[test] diff --git a/crates/ironclaw_reborn_composition/src/attachment_landing.rs b/crates/ironclaw_reborn_composition/src/attachment_landing.rs index 3f32e8fb171..3df42f8ab1d 100644 --- a/crates/ironclaw_reborn_composition/src/attachment_landing.rs +++ b/crates/ironclaw_reborn_composition/src/attachment_landing.rs @@ -11,15 +11,13 @@ use std::sync::Arc; use async_trait::async_trait; -use ironclaw_attachments::{ - DEFAULT_PROJECT_MOUNT_ALIAS, InboundAttachment, land_inbound_attachments, -}; +use ironclaw_attachments::{InboundAttachment, land_inbound_attachments}; use ironclaw_filesystem::{RootFilesystem, ScopedFilesystem}; -use ironclaw_product_workflow::{ - InboundAttachmentLander, RebornServicesError, RebornServicesErrorCode, RebornServicesErrorKind, -}; +use ironclaw_product_workflow::{InboundAttachmentLander, RebornServicesError}; use ironclaw_threads::{AttachmentRef, ThreadScope}; +use crate::local_dev_mounts::WORKSPACE_ALIAS; + /// Lands inbound attachments through a project-scoped workspace filesystem. pub(crate) struct ProjectScopedAttachmentLander { filesystem: Arc>, @@ -30,7 +28,7 @@ impl ProjectScopedAttachmentLander { pub(crate) fn new(filesystem: Arc>) -> Self { Self { filesystem, - project_alias: DEFAULT_PROJECT_MOUNT_ALIAS.to_string(), + project_alias: WORKSPACE_ALIAS.to_string(), } } } @@ -57,13 +55,12 @@ impl InboundAttachmentLander for ProjectScopedAttachmentLande attachments, ) .await - .map_err(|_| RebornServicesError { - code: RebornServicesErrorCode::Internal, - kind: RebornServicesErrorKind::Internal, - status_code: 500, - retryable: false, - field: None, - validation_code: None, + .map_err(|error| { + // The user-facing error stays a sanitized 500; log the underlying + // landing failure (invalid mount path vs. write/permission denied) + // so an operator can tell a misconfigured mount from a full disk. + tracing::warn!(%error, message_id, "failed to land inbound attachments"); + RebornServicesError::internal() }) } } @@ -76,11 +73,11 @@ mod tests { use ironclaw_host_api::{ AgentId, MountAlias, MountGrant, MountPermissions, MountView, TenantId, UserId, VirtualPath, }; - use ironclaw_threads::AttachmentKind; + use ironclaw_product_workflow::RebornServicesErrorCode; fn workspace_fs(permissions: MountPermissions) -> Arc> { let view = MountView::new(vec![MountGrant::new( - MountAlias::new(DEFAULT_PROJECT_MOUNT_ALIAS).unwrap(), + MountAlias::new(WORKSPACE_ALIAS).unwrap(), VirtualPath::new("/projects/workspace").unwrap(), permissions, )]) @@ -111,10 +108,8 @@ mod tests { "msg1", vec![InboundAttachment { id: "att-0".to_string(), - kind: AttachmentKind::Document, mime_type: "application/pdf".to_string(), filename: Some("report.pdf".to_string()), - fallback_extension: "pdf".to_string(), bytes: b"%PDF-1.7".to_vec(), }], ) @@ -139,10 +134,8 @@ mod tests { "msg1", vec![InboundAttachment { id: "att-0".to_string(), - kind: AttachmentKind::Document, mime_type: "application/pdf".to_string(), filename: Some("report.pdf".to_string()), - fallback_extension: "pdf".to_string(), bytes: b"%PDF".to_vec(), }], ) diff --git a/crates/ironclaw_reborn_composition/src/local_dev_mounts.rs b/crates/ironclaw_reborn_composition/src/local_dev_mounts.rs index 0eff5464872..d4a98950a38 100644 --- a/crates/ironclaw_reborn_composition/src/local_dev_mounts.rs +++ b/crates/ironclaw_reborn_composition/src/local_dev_mounts.rs @@ -4,7 +4,7 @@ use ironclaw_host_api::{ HostApiError, MountAlias, MountGrant, MountPermissions, MountView, ResourceScope, VirtualPath, }; -const WORKSPACE_ALIAS: &str = "/workspace"; +pub(crate) const WORKSPACE_ALIAS: &str = "/workspace"; const WORKSPACE_TARGET: &str = "/projects/workspace"; const HOST_ALIAS: &str = "/host"; const HOST_TARGET: &str = "/projects/host"; From 3da460df93d2360a01eff64c0abf6214a9674e5c Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sat, 13 Jun 2026 16:03:27 -0700 Subject: [PATCH 3/4] feat(reborn): cap inbound attachment size at the WebChat v2 lander MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit land_inbound_attachments gained a required max_bytes bound (Track 6/#4670). The ProjectScopedAttachmentLander now owns that policy — set to DEFAULT_MAX_ATTACHMENT_BYTES — and passes it per landing. The send_message route's 14 MiB body cap is the primary gate; this is defense in depth so a single attachment can never land unbounded bytes. --- .../src/attachment_landing.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/ironclaw_reborn_composition/src/attachment_landing.rs b/crates/ironclaw_reborn_composition/src/attachment_landing.rs index 3df42f8ab1d..aca9cbc7f45 100644 --- a/crates/ironclaw_reborn_composition/src/attachment_landing.rs +++ b/crates/ironclaw_reborn_composition/src/attachment_landing.rs @@ -11,7 +11,9 @@ use std::sync::Arc; use async_trait::async_trait; -use ironclaw_attachments::{InboundAttachment, land_inbound_attachments}; +use ironclaw_attachments::{ + DEFAULT_MAX_ATTACHMENT_BYTES, InboundAttachment, land_inbound_attachments, +}; use ironclaw_filesystem::{RootFilesystem, ScopedFilesystem}; use ironclaw_product_workflow::{InboundAttachmentLander, RebornServicesError}; use ironclaw_threads::{AttachmentRef, ThreadScope}; @@ -22,6 +24,10 @@ use crate::local_dev_mounts::WORKSPACE_ALIAS; pub(crate) struct ProjectScopedAttachmentLander { filesystem: Arc>, project_alias: String, + /// Per-attachment size ceiling passed to the landing routine. The + /// `send_message` route's 14 MiB body cap is the primary gate; this is + /// defense in depth so a single attachment can never land unbounded bytes. + max_attachment_bytes: usize, } impl ProjectScopedAttachmentLander { @@ -29,6 +35,7 @@ impl ProjectScopedAttachmentLander { Self { filesystem, project_alias: WORKSPACE_ALIAS.to_string(), + max_attachment_bytes: DEFAULT_MAX_ATTACHMENT_BYTES, } } } @@ -53,6 +60,7 @@ impl InboundAttachmentLander for ProjectScopedAttachmentLande &date, message_id, attachments, + self.max_attachment_bytes, ) .await .map_err(|error| { From 0005893c29b4ad60d9d580c2ed9a16bc9a539687 Mon Sep 17 00:00:00 2001 From: Illia Polosukhin Date: Sat, 13 Jun 2026 16:25:37 -0700 Subject: [PATCH 4/4] refactor(reborn): use canonical mime normalizer; fix landing-path comment Address review: - Replace the local normalize_attachment_mime in webui_inbound with ironclaw_common::normalize_mime_type (identical logic, but the single canonical normalizer the registry/kind/transcription already share) so attachment MIME handling can't drift from the rest of the workspace. - Correct the attachment-landing comment in reborn_services: the external_event_id is the path's stable message segment, but the lander partitions by UTC day, so a retry crossing midnight UTC lands under a new directory (dangling earlier bytes). Idempotency is enforced at message acceptance, not by the storage path. --- .../ironclaw_product_workflow/src/reborn_services.rs | 8 ++++++-- crates/ironclaw_product_workflow/src/webui_inbound.rs | 10 +--------- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/crates/ironclaw_product_workflow/src/reborn_services.rs b/crates/ironclaw_product_workflow/src/reborn_services.rs index b567a02fe30..8e505edf0e0 100644 --- a/crates/ironclaw_product_workflow/src/reborn_services.rs +++ b/crates/ironclaw_product_workflow/src/reborn_services.rs @@ -1721,8 +1721,12 @@ impl RebornServicesApi for RebornServices { } else { // Land attachment bytes (if any) into project storage before the // message is accepted, recording each as a transcript reference. - // Uses the stable per-message external_event_id for the storage - // path so a retry re-lands at the same deterministic location. + // The stable per-message external_event_id is the path's message + // segment, so a same-day retry re-lands at the same path; the lander + // also partitions by UTC day, so a retry that crosses midnight UTC + // lands under the new day's directory (the earlier bytes are left + // addressable but unreferenced). Idempotency is enforced at message + // acceptance, not by the storage path. let message_content = if attachments.is_empty() { MessageContent::text(content.clone()) } else { diff --git a/crates/ironclaw_product_workflow/src/webui_inbound.rs b/crates/ironclaw_product_workflow/src/webui_inbound.rs index 11b6d7ec34b..7db1a4d1e2e 100644 --- a/crates/ironclaw_product_workflow/src/webui_inbound.rs +++ b/crates/ironclaw_product_workflow/src/webui_inbound.rs @@ -105,14 +105,6 @@ pub struct WebUiSendMessageRequest { pub attachments: Vec, } -fn normalize_attachment_mime(raw: &str) -> String { - raw.split(';') - .next() - .unwrap_or(raw) - .trim() - .to_ascii_lowercase() -} - impl WebUiSendMessageRequest { /// Validate and decode the inline attachments into bytes-bearing /// [`InboundAttachment`]s ready for landing. @@ -137,7 +129,7 @@ impl WebUiSendMessageRequest { let mut decoded = Vec::with_capacity(self.attachments.len()); let mut total_bytes = 0usize; for (index, attachment) in self.attachments.iter().enumerate() { - let mime = normalize_attachment_mime(&attachment.mime_type); + let mime = ironclaw_common::normalize_mime_type(&attachment.mime_type); if !ironclaw_common::is_supported_mime(&mime) { return Err(WebUiInboundValidationError::new( "attachments.mime_type",