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
9 changes: 5 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -455,10 +455,11 @@ jobs:
# Config-heavy async test frames (same root cause as the daemon-startup
# stack overflow fixed in #5394); tests that build more than one Agent/
# ServeAgentDeps in the same frame (e.g.
# build_agent_factory_gates_trust_state_independently_per_session) can
# overflow the default per-test thread stack under instrumentation even
# though they pass in the non-instrumented `test` job. Give test threads
# a larger stack here rather than in every other job.
# build_agent_factory_gates_trust_state_independently_per_session, which as of
# #6699 spawns its own 32 MiB-stack thread and so no longer depends on this var
# itself) can overflow the default per-test thread stack under instrumentation
# even though they pass in the non-instrumented `test` job. Keep this set — it
# protects any other, unnamed multi-agent test that doesn't manage its own stack.
RUST_MIN_STACK: "33554432"
steps:
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7
Expand Down
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,24 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
- RC-4 (AutoSkill draft-name collisions with native tool IDs) is tracked separately in #6702
and intentionally out of scope here.

- `crates/zeph-core`/`src/serve/agent_factory.rs`: `cargo test --features full` could crash with
a genuine stack overflow (`SIGABRT`) on
`serve::agent_factory::tests::build_agent_factory_gates_trust_state_independently_per_session`
(issue #6699) — not the reporter's suspected `Config::default()` size growth, but
`VigilGate::try_new` (`crates/zeph-core/src/agent/vigil.rs`) recompiling the entire bundled
14-pattern injection bank via `regex::Regex::new()` on every `Agent` construction, the only one
of 7 consumers of `RAW_INJECTION_PATTERNS` that didn't already cache the compile behind a
`LazyLock` (matching `zeph-sanitizer`, `zeph-mcp`, `zeph-skills`). One bundled pattern triggers
`regex_automata`'s SIMD Teddy-prefilter builder, whose frame overflowed the default 2 MiB test
thread stack when reached ~25 frames deep through this test's two-`Agent`, `--features
full`-inflated (`AnyProvider`'s unboxed `Candle`/`Gonka`/`Cocoon` variants) construction chain.
Cached the bundled-pattern compile behind a process-wide `static ...: LazyLock<Vec<_>>` in
`vigil.rs` (a genuine perf fix independent of the crash — removes O(sessions) redundant
compilation) and gave the specific test a dedicated 32 MiB-stack thread (matching
`MAIN_THREAD_STACK_SIZE` in `src/main.rs`, #5394, and the `coverage` job's existing
`RUST_MIN_STACK: "33554432"` from commit `9a1efb89a`, which this test now also self-provides
rather than relying solely on the CI env var).

- `zeph-channels`: `MAX_RETRY_SECS` (the upper bound `send_with_retry` clamps a `Retry-After`
delay to) had no compile-time invariant guard (issue #6517). #6516 (closing #6496) filtered
the *parsed* `Retry-After` values so a negative/non-finite header or body field could no
Expand Down
27 changes: 23 additions & 4 deletions crates/zeph-core/src/agent/vigil.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
//! - Retry-safe block semantics so a poisoned page does not trigger a fetch retry loop.

use std::collections::HashSet;
use std::sync::LazyLock;

use regex::Regex;
use zeph_common::patterns::RAW_INJECTION_PATTERNS;
Expand All @@ -37,6 +38,24 @@ struct CompiledPattern {
regex: Regex,
}

struct BundledPattern {
name: &'static str,
regex: Regex,
}

/// Compiled bundled injection patterns, compiled once per process instead of once per
/// [`VigilGate::try_new`] call. `Regex::clone` is a cheap `Arc` bump, so every gate
/// construction reuses this compile.
static BUNDLED_PATTERNS: LazyLock<Vec<BundledPattern>> = LazyLock::new(|| {
RAW_INJECTION_PATTERNS
.iter()
.map(|(name, pat)| BundledPattern {
name,
regex: Regex::new(pat).expect("bundled patterns are valid"),
})
.collect()
});

#[non_exhaustive]
/// Action to take when VIGIL flags a tool output.
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
Expand Down Expand Up @@ -92,11 +111,11 @@ impl VigilGate {
pub fn try_new(config: VigilConfig) -> Result<Self, ConfigError> {
config.validate()?;

let mut patterns: Vec<CompiledPattern> = RAW_INJECTION_PATTERNS
let mut patterns: Vec<CompiledPattern> = BUNDLED_PATTERNS
.iter()
.map(|(name, pat)| CompiledPattern {
name: (*name).to_owned(),
regex: Regex::new(pat).expect("bundled patterns are valid"),
.map(|bp| CompiledPattern {
name: bp.name.to_owned(),
regex: bp.regex.clone(),
})
.collect();

Expand Down
41 changes: 39 additions & 2 deletions src/serve/agent_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1894,6 +1894,13 @@ mod tests {
}
}

/// Stack size for [`build_agent_factory_gates_trust_state_independently_per_session`]'s
/// dedicated test thread. Matches the `coverage` job's `RUST_MIN_STACK` in
/// `.github/workflows/ci.yml` (set for this same test under `-C instrument-coverage`,
/// commit `9a1efb89a`) — `Builder::stack_size` takes precedence over `RUST_MIN_STACK`, so
/// this must stay at least as large or it would silently shrink that protection back down.
const TEST_THREAD_STACK_SIZE: usize = 32 * 1024 * 1024;

/// R3 (SEC-H1 guardrail, #5973/#5977): `build_agent_factory` must give each session its
/// OWN `TrustGateExecutor` trust-state instance — not share one gated executor (and its
/// single mutable `effective_trust` atomic) across every `/sessions` agent built from the
Expand All @@ -1903,8 +1910,38 @@ mod tests {
/// A's), and asserts A's `bash` (`QUARANTINE_DENIED`) call is STILL Blocked. This test FAILS
/// if `ServeAgentDeps::tool_executor`/`build_agent_factory` reverts to gating once, eagerly,
/// in `assemble_serve_deps` instead of per session.
#[tokio::test]
async fn build_agent_factory_gates_trust_state_independently_per_session() {
///
/// Runs the test body on a dedicated large-stack thread instead of directly under
/// `#[tokio::test]`. This test keeps TWO full `Agent`s alive simultaneously plus a large
/// `Config` (see #6699): in an unoptimized build, the self-by-value setter chain in
/// `agent_setup.rs`/`agent_factory.rs` (~120 calls) that constructs each `Agent<C>`, combined
/// with `--features full`'s unboxed `AnyProvider` variants (`Candle`/`Gonka`/`Cocoon`),
/// consumes most of the default 2 MiB test-thread stack before a downstream `regex_automata`
/// SIMD prefilter build (`VigilGate::try_new`) is reached, overflowing it. Same pattern as
/// `MAIN_THREAD_STACK_SIZE` in `src/main.rs` (#5394) — a large/deep-frame condition, not
/// unbounded recursion.
#[test]
fn build_agent_factory_gates_trust_state_independently_per_session() {
let result = std::thread::Builder::new()
.name("agent-factory-trust-state-test".into())
.stack_size(TEST_THREAD_STACK_SIZE)
.spawn(|| {
tokio::runtime::Builder::new_current_thread()
.enable_all()
.build()
.expect("failed to build test runtime")
.block_on(
build_agent_factory_gates_trust_state_independently_per_session_body(),
);
})
.expect("failed to spawn test thread")
.join();
if let Err(e) = result {
std::panic::resume_unwind(e);
}
}

async fn build_agent_factory_gates_trust_state_independently_per_session_body() {
let memory = make_memory().await;
let cid_a = memory.sqlite().create_conversation().await.unwrap();
let cid_b = memory.sqlite().create_conversation().await.unwrap();
Expand Down
Loading