Skip to content
Merged
137 changes: 136 additions & 1 deletion crates/stella-cli/src/agent/goal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,8 @@ async fn run_goal_pipeline_turn(
// the read-only tool view, the verifier tuning, and the session
// calibration (keyed per model, so a cross-family verifier learns its
// own drift) into each assessment.
let read_only = stella_core::ports::ReadOnlyTools::new(&tools);
let scoped = VerifierScopedTools::new(&tools);
let read_only = stella_core::ports::ReadOnlyTools::new(&scoped);
let verifier_engine = Engine::with_sleeper(
verifier,
&read_only,
Expand Down Expand Up @@ -876,3 +877,137 @@ async fn run_goal_pipeline_turn(
tui::files_touched_panel(&files);
goal_result
}

/// The six tools the goal verifier's system prompt promises
/// (`stella_core::goal::VERIFIER_SYSTEM_PROMPT`), and the only ones it is
/// offered. Pinned against the prompt by a test below so the two cannot
/// drift.
const VERIFIER_TOOL_ALLOWLIST: &[&str] = &[
"read_file",
"grep",
"glob",
"explorations",
"ci_status",
"search_issues",
];

/// The goal verifier's tool surface (#1783): the session stack narrowed to
/// [`VERIFIER_TOOL_ALLOWLIST`] BEFORE the read-only view applies. The bare
/// `ReadOnlyTools` wrap admitted every schema claiming `read_only: true` —
/// ~25 tools including `web_fetch`/`web_search` (outbound HTTP from a role
/// that reads worker-influenced content: a prompt-injection egress channel)
/// and any MCP/custom tool that self-declares read-only. The pipeline's
/// verdict call carries zero tools; this is the goal loop's equivalent
/// posture: exactly what the prompt names, enforced at execution rather
/// than by prompt.
struct VerifierScopedTools<'a> {
inner: &'a dyn stella_core::ToolExecutor,
}

impl<'a> VerifierScopedTools<'a> {
fn new(inner: &'a dyn stella_core::ToolExecutor) -> Self {
Self { inner }
}
}

#[async_trait::async_trait]
impl stella_core::ToolExecutor for VerifierScopedTools<'_> {
fn schemas(&self) -> Vec<stella_protocol::ToolSchema> {
self.inner
.schemas()
.into_iter()
.filter(|schema| VERIFIER_TOOL_ALLOWLIST.contains(&schema.name.as_str()))
.collect()
}

async fn execute(&self, name: &str, input: &serde_json::Value) -> stella_protocol::ToolOutput {
if !VERIFIER_TOOL_ALLOWLIST.contains(&name) {
return stella_protocol::ToolOutput::Error {
message: format!(
"`{name}` is not available to the goal verifier: only the tools its \
instructions name ({}) may be called",
VERIFIER_TOOL_ALLOWLIST.join(", ")
),
};
}
self.inner.execute(name, input).await
}

// Forwarded, not zeroed (the trait doc's decorator rule): nothing on the
// allowlist dispatches sub-agents today, but a wrapper that drops spend
// is wrong the day that changes.
fn drain_sub_agent_spend_usd(&self) -> f64 {
self.inner.drain_sub_agent_spend_usd()
}
}

#[cfg(test)]
mod verifier_tools_tests {
use super::*;
use stella_protocol::{ToolOutput, ToolSchema};

struct FakeStack;
#[async_trait::async_trait]
impl stella_core::ToolExecutor for FakeStack {
fn schemas(&self) -> Vec<ToolSchema> {
["read_file", "grep", "web_fetch", "mcp__srv__read", "bash"]
.into_iter()
.map(|name| ToolSchema {
name: name.into(),
description: String::new(),
input_schema: serde_json::json!({}),
// Everything claims read-only — the exact shape a
// self-declaring MCP tool or the web group presents.
read_only: true,
speculation_safe: false,
})
.collect()
}
async fn execute(&self, name: &str, _input: &serde_json::Value) -> ToolOutput {
ToolOutput::Ok {
content: format!("ran {name}"),
}
}
}

/// #1783's witness: the goal verifier is offered exactly what its
/// prompt names — a read-only claim alone (web egress, a self-declared
/// MCP read) no longer admits a tool, and execution is enforced, not
/// prompted.
#[tokio::test]
async fn the_goal_verifier_gets_only_the_tools_its_prompt_names() {
let stack = FakeStack;
let scoped = VerifierScopedTools::new(&stack);
let names: Vec<_> = scoped
.schemas()
.into_iter()
.map(|schema| schema.name)
.collect();
assert_eq!(names, vec!["read_file", "grep"]);
for denied in ["web_fetch", "mcp__srv__read", "bash", "web_search"] {
assert!(
scoped
.execute(denied, &serde_json::json!({}))
.await
.is_error(),
"{denied} must be refused at execution"
);
}
assert!(matches!(
scoped.execute("read_file", &serde_json::json!({})).await,
ToolOutput::Ok { .. }
));
}

/// The allowlist and the prompt must name the same six tools — the
/// drift this pins is exactly how the surface grew to ~25 unnoticed.
#[test]
fn the_allowlist_matches_what_the_prompt_promises() {
for name in VERIFIER_TOOL_ALLOWLIST {
assert!(
stella_core::goal::VERIFIER_SYSTEM_PROMPT.contains(name),
"allowlisted `{name}` is not named by the verifier prompt"
);
}
}
}
11 changes: 10 additions & 1 deletion crates/stella-cli/src/candidate_ws.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,16 @@ impl GitCandidateWorkspaces {
// (the workspace outlives every borrow). Custom tools re-root
// to `ws_root`, so their subprocesses run in the shadow.
let registry: Arc<dyn stella_core::ToolExecutor> = Arc::new(registry);
let witness_tools = WitnessToolExecutor::new(ws_root.clone(), registry.clone());
// The witness author's reads honor the operator's tool policy
// exactly like the worker's do (#1784): before this wrap, a
// `"tools": {"read_file": "off"}` switch reached every worker
// surface while the witness author kept reading through the
// raw registry — an unstated exemption from a setting that
// claims to govern the session's whole tool stack.
let witness_reads: Arc<dyn stella_core::ToolExecutor> = Arc::new(
crate::agent::PolicyToolSet::new_owned(registry.clone(), self.policy.clone()),
);
let witness_tools = WitnessToolExecutor::new(ws_root.clone(), witness_reads);
let native =
CustomToolSet::new_owned(registry, self.custom_tools.clone(), ws_root.clone());
// MCP: layer the candidate_safe-filtered session view on top
Expand Down
105 changes: 103 additions & 2 deletions crates/stella-cli/src/candidate_ws/witness_tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ impl ToolExecutor for WitnessToolExecutor {
.reads
.schemas()
.into_iter()
.filter(|schema| schema.name == "read_file")
.filter(|schema| schema.name == "read_file" || schema.name == "glob")
.collect();
schemas.push(ToolSchema {
name: "create_witness_test".into(),
Expand Down Expand Up @@ -188,6 +188,30 @@ impl ToolExecutor for WitnessToolExecutor {
}
self.reads.execute(name, input).await
}
// Discovery for the blind author (#1792): the repo listing in the
// prompt is truncated to its first 200 sorted paths, and an
// author that cannot see any `tests/` directory there had no
// legal move — `read_file` needs a path it can already name, and
// `create_witness_test` requires the parent to exist. `glob` is
// names-only, root-confined by the tool itself, and its results
// pass the same credential exclusion the read path enforces.
"glob" => {
if let Some(raw_path) = input.get("path").and_then(serde_json::Value::as_str)
&& normalized_candidate_path(raw_path).is_none()

@vercel vercel Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The witness author's glob path guard wrongly rejects the documented root values . and "", denying the one blind-discovery move the feature exists to enable.

Fix on Vercel

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Confirmed — traced rather than assumed. normalized_candidate_path(".") drops the CurDir component and returns None at (!parts.is_empty()).then(...); ("") returns None at the is_empty() guard. Both are documented root values: glob defaults its path to "." and its own comment says "./empty resolve to the root itself".

Root cause is that normalized_candidate_path's None conflates "escapes the root" with "is the root" — fine for read_file, wrong for a listing tool. names_candidate_root splits them, ruling out absolute/drive-qualified spellings first so / stays an escape.

Fix + witness (glob_accepts_the_root_spelled_out_as_well_as_omitted) pushed to wip/fix-1813-glob-root as 8fc4000a; details in the PR comment. Kept off this branch pending your call, since it is your PR.

{
return Self::denied(name, "the path must stay within the candidate root");
}
match self.reads.execute(name, input).await {
ToolOutput::Ok { content } => ToolOutput::Ok {
content: content
.lines()
.filter(|line| !is_credential_path(line.trim()))
.collect::<Vec<_>>()
.join("\n"),
},
error => error,
}
}
"create_witness_test" => self.create_test(input),
_ => Self::denied(
name,
Expand Down Expand Up @@ -385,7 +409,7 @@ mod tests {
.into_iter()
.map(|schema| schema.name)
.collect();
assert_eq!(names, vec!["read_file", "create_witness_test"]);
assert_eq!(names, vec!["glob", "read_file", "create_witness_test"]);
for denied in [
"write_file",
"edit_file",
Expand Down Expand Up @@ -543,6 +567,83 @@ mod tests {
assert!(root.path().join("tests/real_witness.rs").exists());
}

/// #1792's witness: the blind author can discover the tree by name —
/// `glob` is offered and executes root-confined, and its results pass
/// the same credential exclusion as the read path, so discovery never
/// becomes a map of the workspace's secrets.
#[tokio::test]
async fn glob_lets_the_author_discover_tests_without_leaking_credentials() {
let root = tempfile::tempdir().unwrap();
std::fs::create_dir(root.path().join("tests")).unwrap();
std::fs::write(root.path().join("tests/existing.rs"), "#[test] fn t() {}").unwrap();
std::fs::write(root.path().join(".env.local"), "secret").unwrap();
let tools = witness_executor(root.path()).await;

let output = tools
.execute("glob", &serde_json::json!({"pattern": "**/*"}))
.await;
let ToolOutput::Ok { content } = output else {
panic!("glob must be available to the witness author: {output:?}");
};
assert!(
content.contains("tests/existing.rs"),
"discovery must surface the test directory: {content}"
);
assert!(
!content.contains(".env.local"),
"credential paths must not appear in discovery output: {content}"
);

let escaped = tools
.execute(
"glob",
&serde_json::json!({"pattern": "*", "path": "../.."}),
)
.await;
assert!(
escaped.is_error(),
"an escaping search path must be refused"
);
}

/// #1784's witness: the operator's tool policy governs the witness
/// author's reads exactly as it governs the worker's. The executor takes
/// whatever read surface it is handed — the candidate workspace hands it
/// the policy-wrapped one — so a `read_file: off` switch removes both
/// the schema and the dispatch here, not just on the worker path.
#[tokio::test]
async fn the_tool_policy_reaches_the_witness_authors_reads() {
let root = tempfile::tempdir().unwrap();
std::fs::create_dir(root.path().join("tests")).unwrap();
std::fs::write(root.path().join("src.rs"), "source").unwrap();
let registry: Arc<dyn ToolExecutor> = Arc::new(
ToolRegistry::new_detected(root.path().to_path_buf(), RegistryOptions::default()).await,
);
let policied: Arc<dyn ToolExecutor> = Arc::new(crate::agent::PolicyToolSet::new_owned(
registry,
stella_tools::policy::ToolPolicy::from_switches([("read_file".to_string(), false)]),
));
let tools = WitnessToolExecutor::new(root.path().to_path_buf(), policied);

let names: Vec<_> = tools
.schemas()
.into_iter()
.map(|schema| schema.name)
.collect();
assert_eq!(
names,
vec!["glob", "create_witness_test"],
"a switched-off read_file must not be offered to the author (glob stays)"
);
assert!(
tools
.execute("read_file", &serde_json::json!({"path": "src.rs"}))
.await
.is_error(),
"and must not execute either"
);
}

#[cfg(unix)]
#[tokio::test]
async fn refuses_a_terminal_symlink() {
Expand Down
6 changes: 5 additions & 1 deletion crates/stella-core/src/goal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,11 @@ pub fn goal_round_turn_offset(round: usize) -> u32 {
u32::try_from(round.saturating_sub(1).saturating_mul(2)).unwrap_or(u32::MAX)
}

const VERIFIER_SYSTEM_PROMPT: &str = "You are an impartial verifier assessing whether a coding agent \
/// Public so the CLI's goal loop can pin its tool allowlist against the six
/// tools this prompt names (#1783): the prompt and the offered surface must
/// not drift apart, and the test that enforces that lives beside the
/// executor it guards.
pub const VERIFIER_SYSTEM_PROMPT: &str = "You are an impartial verifier assessing whether a coding agent \
has fully met a stated goal. Judge from EVIDENCE, never from claims: use your read-only \
tools (read_file, grep, glob, explorations, ci_status, search_issues) to verify the work \
directly whenever the transcript alone is not conclusive — read the changed files, check \
Expand Down
16 changes: 8 additions & 8 deletions crates/stella-pipeline/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,6 @@ const CONVERSATIONAL_SYSTEM_PROMPT: &str = "You are Stella, a careful software e
no test. Do not invent a task. If it fits, add one short line inviting \
them to describe a change, bug, or question about their codebase.";

/// Small fixed system prompt for the independent witness author.
const WITNESS_SYSTEM_PROMPT: &str = "You are a precise test author. You write minimal failing tests that pin down intended \
behavior. You never modify production code and never fix the problem yourself.";

/// Per-role request overrides for the pipeline's raw completion calls
/// (triage / verifier / guidance), resolved by the caller from
/// `agent_engine_config`. Every field is optional and falls through to the
Expand All @@ -183,10 +179,13 @@ pub struct RoleCallOverrides {
pub params: Option<stella_protocol::GenerationParams>,
}

/// The pipeline's per-role override set. Worker (and plan/witness, which
/// ride the worker's tier) is configured through
/// [`PipelineConfig::engine`] directly; only the two roles with their own
/// models get their own request shaping.
/// The pipeline's per-role override set. Worker (and plan, which rides the
/// worker's tier) is configured through [`PipelineConfig::engine`] directly;
/// only the two roles with their own models get their own request shaping.
/// The witness author/repair engines ride the verifier's model, so they take
/// the `verifier` row's shaping too (#1785) — everything except `prompt`,
/// which stays scoped to the raw verdict/guidance calls
/// (`Pipeline::witness_engine_config` says why).
#[derive(Debug, Clone, Default)]
pub struct PipelineRoleOverrides {
pub triage: RoleCallOverrides,
Expand Down Expand Up @@ -2871,6 +2870,7 @@ impl<'a> Pipeline<'a> {
// than being forwarded (§4.3).
let feedback = self
.airlock_forward(&verdict.reasoning, "verifier_reasoning", &sealed)
.map(|text| crate::verify::bound_forwarded_reasoning(&text))
.unwrap_or_else(|| redact(&sealed, DisclosureGrain::Symptom).message());
if let Err(abort) = self
.revise_candidate(engine, surface, budget, &feedback, total, &mut state)
Expand Down
Loading
Loading