From ee029e4f7fcfc109224117e6d2d60a1f691c8e21 Mon Sep 17 00:00:00 2001 From: "Andrei G." Date: Thu, 16 Jul 2026 17:58:51 +0200 Subject: [PATCH] fix(subagent): strip MessagePart::Image from transcript writes TranscriptWriter::append serialized the full Message verbatim to the sub-agent's JSONL transcript with no filtering. Sub-agents never call Agent::persist_message, so their transcripts bypassed the MessagePart::Image strip landed in #6307 for spec-072 C1 entirely. Extract the strip logic into a shared MessagePart::strip_images helper in zeph-llm and call it from both Agent::persist_message and TranscriptWriter::append, closing the defense-in-depth gap before #6240 (MCP image emission in tool-result processing) can make it reachable. Closes #6305 --- CHANGELOG.md | 12 ++ .../zeph-core/src/agent/persistence/store.rs | 19 ++-- crates/zeph-llm/src/provider.rs | 35 ++++++ crates/zeph-subagent/src/transcript.rs | 106 +++++++++++++++++- 4 files changed, 158 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 927a9ecb5..c28a0c77c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Security + +- **zeph-subagent**: `TranscriptWriter::append` now strips every `MessagePart::Image` from a + message's `parts` before serializing it to the sub-agent's `.jsonl` transcript file + (spec-072 §4 C1, #6305). Sub-agents never go through `Agent::persist_message`, so their + transcripts were a structurally separate persistence path that bypassed the strip landed in + #6307 entirely — currently not reachable in practice (no sub-agent turn produces an `Image` + part yet), but closes the defense-in-depth gap before #6240 (MCP image emission inside + tool-result processing) can produce one. The strip logic is shared: `MessagePart::strip_images` + is now a public helper on `zeph-llm`'s `MessagePart`, and both `Agent::persist_message` and + `TranscriptWriter::append` call it instead of duplicating the filter. The caller's in-memory + `Message` is unaffected; only the persisted copy is stripped. ## [0.22.1] - 2026-07-15 ### Fixed diff --git a/crates/zeph-core/src/agent/persistence/store.rs b/crates/zeph-core/src/agent/persistence/store.rs index 0da4e3c3f..1d38f4199 100644 --- a/crates/zeph-core/src/agent/persistence/store.rs +++ b/crates/zeph-core/src/agent/persistence/store.rs @@ -21,12 +21,13 @@ impl Agent { /// When `true` and `guard_memory_writes` is enabled, only `SQLite` is written — the message /// is saved for conversation continuity but will not pollute semantic search (M2, D2). /// - /// `MessagePart::Image` parts are deliberately stripped before either persistence writer - /// below sees `parts` — they are ephemeral, current-turn-only content (spec-072 §4, C1) and - /// must never reach `SQLite` `parts_json`, the Qdrant embed path, or the durable JSONL session - /// log. This is a single, explicit strip point above both writers, not an omission; the - /// `parts` slice passed in by the caller is untouched, so the in-memory `Message` already - /// pushed via `push_message` keeps its `Image` parts for the current turn's provider request. + /// `MessagePart::Image` parts are deliberately stripped (via [`MessagePart::strip_images`]) + /// before either persistence writer below sees `parts` — they are ephemeral, current-turn-only + /// content (spec-072 §4, C1) and must never reach `SQLite` `parts_json`, the Qdrant embed path, + /// or the durable JSONL session log. This is a single, explicit strip point above both writers, + /// not an omission; the `parts` slice passed in by the caller is untouched, so the in-memory + /// `Message` already pushed via `push_message` keeps its `Image` parts for the current turn's + /// provider request. #[tracing::instrument(name = "core.persist.persist_message", skip_all, level = "debug")] pub(crate) async fn persist_message( &mut self, @@ -58,11 +59,7 @@ impl Agent { // C1 (spec-072 §4): strip Image parts once, above both persistence writers below. // Neither `sink.record_message` nor `PersistMessageRequest::from_borrowed`/ // `svc.persist_message` may see an unstripped `parts` slice — see the doc comment above. - let persisted_parts: Vec = parts - .iter() - .filter(|p| !matches!(p, MessagePart::Image(_))) - .cloned() - .collect(); + let persisted_parts: Vec = MessagePart::strip_images(parts); // INV-SP-1 (spec-068 §13): the durable event log must be appended and flushed before the // SQLite `messages` projection is written — the projection must never lead the log. A diff --git a/crates/zeph-llm/src/provider.rs b/crates/zeph-llm/src/provider.rs index 9cf750de3..2b64f3af8 100644 --- a/crates/zeph-llm/src/provider.rs +++ b/crates/zeph-llm/src/provider.rs @@ -338,6 +338,41 @@ impl MessagePart { None } } + + /// Return a cloned copy of `parts` with every `Image` entry removed. + /// + /// `Image` parts are ephemeral, current-turn-only vision input (spec-072 §4, C1) and must + /// never reach a persistence sink — `SQLite` `parts_json`, the Qdrant embed path, the durable + /// JSONL session log, or a sub-agent transcript file. Callers apply this once, immediately + /// above the persistence write, leaving the in-memory slice used for the current turn's + /// provider request untouched. + /// + /// # Examples + /// + /// ``` + /// use zeph_llm::provider::{ImageData, MessagePart}; + /// + /// let parts = vec![ + /// MessagePart::Text { + /// text: "hello".to_owned(), + /// }, + /// MessagePart::Image(Box::new(ImageData { + /// data: vec![0xFF, 0xD8, 0xFF, 0xE0], + /// mime_type: "image/jpeg".to_owned(), + /// })), + /// ]; + /// let stripped = MessagePart::strip_images(&parts); + /// assert_eq!(stripped.len(), 1); + /// assert!(!stripped.iter().any(|p| matches!(p, MessagePart::Image(_)))); + /// ``` + #[must_use] + pub fn strip_images(parts: &[MessagePart]) -> Vec { + parts + .iter() + .filter(|p| !matches!(p, MessagePart::Image(_))) + .cloned() + .collect() + } } #[derive(Clone, Serialize, Deserialize)] diff --git a/crates/zeph-subagent/src/transcript.rs b/crates/zeph-subagent/src/transcript.rs index 405e891fa..b3b51aa8d 100644 --- a/crates/zeph-subagent/src/transcript.rs +++ b/crates/zeph-subagent/src/transcript.rs @@ -18,7 +18,7 @@ use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; use serde::{Deserialize, Serialize}; -use zeph_llm::provider::Message; +use zeph_llm::provider::{Message, MessagePart}; use super::error::SubAgentError; use super::state::SubAgentState; @@ -111,6 +111,13 @@ impl TranscriptWriter { /// Append a single message as a JSON line and flush immediately. /// + /// `MessagePart::Image` parts are stripped (via [`MessagePart::strip_images`]) from the + /// persisted copy before serialization — they are ephemeral, current-turn-only vision input + /// (spec-072 §4, C1) and must never reach a transcript file on disk, mirroring the strip point + /// already enforced for `Agent::persist_message`'s `SQLite`/Qdrant/durable-JSONL writers. The + /// caller's `message` is untouched, so callers that hold onto it for the current turn's + /// provider request keep their `Image` parts. + /// /// Serialization is done on the caller's thread; the blocking write and flush /// are offloaded to `tokio::task::spawn_blocking` so the Tokio executor is not stalled. /// @@ -118,10 +125,12 @@ impl TranscriptWriter { /// /// Returns `io::Error` on serialization, write failure, lock poison, or thread-pool panic. pub async fn append(&self, seq: u32, message: &Message) -> io::Result<()> { + let mut persisted_message = message.clone(); + persisted_message.parts = MessagePart::strip_images(&persisted_message.parts); let entry = TranscriptEntry { seq, timestamp: utc_now(), - message: message.clone(), + message: persisted_message, }; let line = serde_json::to_string(&entry) .map_err(|e| io::Error::new(io::ErrorKind::InvalidData, e))?; @@ -455,7 +464,7 @@ fn epoch_to_parts(epoch: u64) -> (u32, u32, u32, u32, u32, u32) { #[cfg(test)] mod tests { use std::assert_matches; - use zeph_llm::provider::{Message, MessageMetadata, Role}; + use zeph_llm::provider::{ImageData, Message, MessageMetadata, MessagePart, Role}; use super::*; @@ -501,6 +510,97 @@ mod tests { assert_eq!(messages[1].content, "world"); } + /// #6305: `MessagePart::Image` must never reach the on-disk transcript — it is ephemeral, + /// current-turn-only vision input (spec-072 §4, C1), mirroring the strip already enforced + /// for `Agent::persist_message`'s `SQLite`/Qdrant/durable-JSONL writers. + #[tokio::test] + async fn append_strips_image_parts() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("test.jsonl"); + + let mut msg = test_message(Role::User, "look at this"); + msg.parts = vec![ + MessagePart::Text { + text: "look at this".to_owned(), + }, + MessagePart::Image(Box::new(ImageData { + data: vec![0xFFu8, 0xD8, 0xFF, 0xE0], + mime_type: "image/jpeg".to_owned(), + })), + ]; + + let writer = TranscriptWriter::new(&path).unwrap(); + writer.append(0, &msg).await.unwrap(); + + // The caller's own copy keeps the Image part for the current turn's provider request. + assert_eq!(msg.parts.len(), 2); + + let messages = TranscriptReader::load(&path).unwrap(); + assert_eq!(messages.len(), 1); + assert_eq!(messages[0].parts.len(), 1); + assert!(matches!(messages[0].parts[0], MessagePart::Text { .. })); + assert!( + !messages[0] + .parts + .iter() + .any(|p| matches!(p, MessagePart::Image(_))), + "transcript must not retain Image parts" + ); + + // The image payload must not appear anywhere in the file on disk either. + let raw = std::fs::read_to_string(&path).unwrap(); + assert!( + !raw.contains("mime_type") && !raw.contains("image/jpeg"), + "raw image payload leaked into transcript file" + ); + } + + #[tokio::test] + async fn append_preserves_non_image_parts() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("test.jsonl"); + + let mut msg = test_message(Role::Assistant, "used a tool"); + msg.parts = vec![ + MessagePart::Text { + text: "used a tool".to_owned(), + }, + MessagePart::ToolUse { + id: "call-1".to_owned(), + name: "search".to_owned(), + input: serde_json::json!({"query": "rust"}), + }, + ]; + + let writer = TranscriptWriter::new(&path).unwrap(); + writer.append(0, &msg).await.unwrap(); + + let messages = TranscriptReader::load(&path).unwrap(); + assert_eq!(messages.len(), 1); + assert_eq!(messages[0].parts.len(), 2); + assert!(matches!(messages[0].parts[0], MessagePart::Text { .. })); + assert!(matches!(messages[0].parts[1], MessagePart::ToolUse { .. })); + } + + #[tokio::test] + async fn append_empty_parts_unchanged() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("test.jsonl"); + + // Mirrors the `task_msg` / turn-generated-message call sites in `agent_loop.rs`, which + // always pass an empty `parts` vec — the strip must be a no-op for them. + let msg = test_message(Role::User, "plain task message"); + assert!(msg.parts.is_empty()); + + let writer = TranscriptWriter::new(&path).unwrap(); + writer.append(0, &msg).await.unwrap(); + + let messages = TranscriptReader::load(&path).unwrap(); + assert_eq!(messages.len(), 1); + assert!(messages[0].parts.is_empty()); + assert_eq!(messages[0].content, "plain task message"); + } + #[test] fn load_missing_file_no_meta_returns_empty() { let dir = tempfile::tempdir().unwrap();