Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ ignore = [
"RUSTSEC-2026-0021",
# rustls-webpki CRL distributionPoint matching — 0.102.8 pinned by libsql transitive dep
"RUSTSEC-2026-0049",
# rustls-webpki URI name constraint bypass — 0.102.8 pinned by libsql transitive dep;
# patched in >=0.103.12 but libsql 0.6.0 requires rustls 0.22 which pins 0.102.x
"RUSTSEC-2026-0098",
# rand unsoundness with custom logger calling rand::rng() during reseed — we don't use this pattern;
# revisit/remove by 2026-06-30, or when transitive deps (tower, nanoid, phf_generator) release rand ≥0.9.3 compat
"RUSTSEC-2026-0097",
Expand Down
4 changes: 4 additions & 0 deletions src/bridge/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,7 @@ pub use router::{

#[cfg(feature = "libsql")]
pub use router::reset_engine_state;

// Exposed for caller-level testing of the cross-user thread_id guard
#[cfg(test)]
pub(crate) use router::handle_mission_notification;
18 changes: 12 additions & 6 deletions src/bridge/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2906,7 +2906,9 @@ fn interpret_message_event(role: &str, content_preview: &str) -> Option<&'static
/// its empty context) say "I haven't sent you a digest". Recording an
/// `Agent` entry tagged with the mission's thread id keeps the v2 history
/// consistent with what the user actually saw.
async fn handle_mission_notification(
// pub(crate) for #[cfg(test)] re-export in mod.rs; the module itself
// is private so this has no production visibility beyond router.rs.
pub(crate) async fn handle_mission_notification(
notif: &ironclaw_engine::MissionNotification,
channels: &std::sync::Arc<crate::channels::ChannelManager>,
sse: Option<&Arc<SseManager>>,
Expand All @@ -2926,12 +2928,16 @@ async fn handle_mission_notification(

for channel_name in &notif.notify_channels {
// Send via channel broadcast (proactive, no incoming message required)
let mut response = OutgoingResponse::text(&full_text);
// Only attach the mission owner's thread_id when the recipient IS the
// owner. When notify_user routes to a different user, omit the thread
// so the gateway's broadcast() fallback resolves the recipient's own
// assistant thread — avoids leaking the owner's thread_id cross-user.
if broadcast_user == notif.user_id {
response = response.in_thread(notif.thread_id.to_string());
}
if let Err(e) = channels

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Medium Severity — No test for the cross-user thread_id guard

The if broadcast_user == notif.user_id condition is a security-sensitive guard that prevents leaking the owner's mission thread_id to a different recipient. Per .claude/rules/testing.md ("Test Through the Caller"), when a predicate gates a side effect with computed inputs, a caller-level test is required.

Suggestion: Add an integration test that exercises handle_mission_notification (or the broadcast path) with a MissionNotification where notify_user = Some("other-user"), and verify the response sent to the channel does NOT carry the owner's thread_id. Separately test the notify_user == None case to verify thread_id IS attached.

.broadcast(
channel_name,
broadcast_user,
OutgoingResponse::text(&full_text),
)
.broadcast(channel_name, broadcast_user, response)
.await
{
debug!(
Expand Down
26 changes: 22 additions & 4 deletions src/channels/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -756,10 +756,28 @@ impl Channel for GatewayChannel {
let thread_id = match response.thread_id {
Some(tid) => tid,
None => {
return Err(ChannelError::MissingRoutingTarget {
name: "gateway".to_string(),
reason: "broadcast() requires a thread_id on the response".to_string(),
});
// Proactive broadcasts (mission notifications, self-repair,
// extension activation) don't always have a thread context.
// Route to the user's assistant conversation so the message
// appears in a known location instead of being rejected.
match self.state.store.as_ref() {
Some(store) => store
.get_or_create_assistant_conversation(user_id, "gateway")
.await
.map(|id| id.to_string())
.map_err(|e| ChannelError::SendFailed {
name: "gateway".to_string(),
reason: format!(
"broadcast() has no thread_id and assistant thread lookup failed: {e}"
),
})?,
None => {
return Err(ChannelError::MissingRoutingTarget {
name: "gateway".to_string(),
reason: "broadcast() has no thread_id and no DB to resolve assistant thread".to_string(),
});
}
}
}
};
self.state.sse.broadcast_for_user(
Expand Down
Loading
Loading