Skip to content

shadow_sentinel: check_tool_call DB reads have no timeout on hot tool-dispatch path #6269

Description

@bug-ops

Finding

ShadowSentinel::check_tool_call (crates/zeph-core/src/agent/shadow_sentinel.rs) runs synchronously before every high-risk tool execution (Shell/FileWrite/ExfilCapable/McpUnclassified) and directly awaits two DbPool reads with no timeout:

  • self.store.get_trajectory(...) (line 745)
  • self.store.get_tool_history(...) (line 776-781)

Per the CLAUDE.md "Await Discipline" contract, every external .await on the agent's hot path — including cross-process I/O — must have an explicit timeout so a slow/unresponsive dependency cannot stall the caller indefinitely. zeph-db supports a Postgres backend (network round-trip, connection-pool contention), and unlike the LLM probe call three lines below (tokio::time::timeout(timeout, self.provider.chat(&messages)), line 288), or the equivalent pattern already established elsewhere in zeph-core for hot-path DB reads (agent_access_impl.rs:119, agent_access_impl.rs:518, persistence/store.rs:204), these two calls are unbounded. A stalled Postgres connection would block dispatch of every high-risk tool call (shell, file write, exfil-capable MCP tools) for the session — the exact failure mode the LLM-isolation and fail-open design in this file already goes to great lengths to avoid for the probe LLM call itself, but not for the DB reads that feed it.

Location

crates/zeph-core/src/agent/shadow_sentinel.rs:740-801 (inside ShadowSentinel::check_tool_call)

Before

let mut trajectory: Vec<SentinelEvent> = match self
    .store
    .get_trajectory(&self.session_id, self.config.max_context_events)
    .await
{
    Ok(t) => t.into_iter().filter(|e| e.event_type != "probe_result").collect(),
    Err(e) => { /* ... */ vec![] }
};
...
match self
    .store
    .get_tool_history(qualified_tool_id, self.session_id.as_str(), self.config.max_context_events)
    .await
{
    Ok(history) => { /* ... */ }
    Err(e) => { /* ... */ }
}

After

let db_timeout = std::time::Duration::from_millis(self.config.probe_timeout_ms.min(2000));

let mut trajectory: Vec<SentinelEvent> = match tokio::time::timeout(
    db_timeout,
    self.store.get_trajectory(&self.session_id, self.config.max_context_events),
)
.await
{
    Ok(Ok(t)) => t.into_iter().filter(|e| e.event_type != "probe_result").collect(),
    Ok(Err(e)) => { /* existing warn + vec![] */ }
    Err(_) => { /* warn: trajectory load timed out, proceed without context */ vec![] }
};
// same pattern for get_tool_history

(exact constant/config field name is an implementation detail — could also reuse or add a dedicated probe_db_timeout_ms field on ShadowSentinelConfig)

Why

Both calls are on the pre-tool-execution hot path shared by every check_tool_call invocation, not on a background/fire-and-forget path (unlike persist_event/spawn_persist, which are correctly bounded and non-blocking). ShadowSentinel is explicitly documented as defence-in-depth that "must never block tool dispatch" (see spawn_persist doc comment), but that guarantee currently only covers the write side, not these two hot-path reads. This matches the codebase's own established pattern for hot-path DB reads (5s timeout in agent_access_impl.rs), so fixing this is a straightforward consistency change, not a novel design.

Metadata

Metadata

Assignees

Labels

P2High value, medium complexityarchitectureArchitecture improvementsbugSomething isn't working

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions