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
17 changes: 16 additions & 1 deletion crates/tui/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3640,13 +3640,28 @@ impl Config {
self.context.project_pack.unwrap_or(true)
}

/// Return whether shell execution is allowed. Defaults to `false`: shell
/// Return whether shell execution is allowed for noninteractive and
/// durable-task profiles. Defaults to `false`: in headless, app-server, and
/// background-task contexts there is no human to approve commands, so shell
/// access must be opted into explicitly (GHSA-72w5-pf8h-xfp4).
#[must_use]
pub fn allow_shell(&self) -> bool {
self.allow_shell.unwrap_or(false)
}

/// Return whether shell execution is allowed for an *interactive* TUI Agent
/// session. Defaults to `true`: the interactive composer always gates each
/// shell command behind an approval prompt, so the catalog can expose shell
/// by default while still preserving consent (GHSA-72w5-pf8h-xfp4). An
/// explicit `allow_shell = false` still hides shell tools. This is the
/// single source of truth for the interactive default; both startup
/// (`run_interactive`) and the durable Agent permission baseline read it so
/// the default cannot drift between them.
#[must_use]
pub fn interactive_allow_shell(&self) -> bool {
self.allow_shell.unwrap_or(true)
}

/// Whether ghost-text prompt suggestion is enabled (opt-in, default off).
pub fn prompt_suggestion_enabled(&self) -> bool {
self.prompt_suggestion.unwrap_or(false)
Expand Down
28 changes: 28 additions & 0 deletions crates/tui/src/config/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,34 @@ fn allow_shell_defaults_to_false_when_unset() {
);
}

// The interactive default is shell-on (approval-gated). Both interactive
// startup and the durable Agent permission baseline (app.rs) read this single
// method so the default cannot drift between launch modes; an explicit opt-out
// is still honored.
#[test]
fn interactive_allow_shell_defaults_to_true_but_honors_explicit_opt_out() {
let default_config = Config::default();
assert!(
default_config.interactive_allow_shell(),
"interactive Agent sessions expose shell by default so approvals can gate commands"
);

let opted_out = Config {
allow_shell: Some(false),
..Config::default()
};
assert!(
!opted_out.interactive_allow_shell(),
"explicit allow_shell = false still hides shell in interactive sessions"
);

let opted_in = Config {
allow_shell: Some(true),
..Config::default()
};
assert!(opted_in.interactive_allow_shell());
}

#[test]
fn prompt_suggestion_defaults_to_false() {
let config = Config::default();
Expand Down
25 changes: 20 additions & 5 deletions crates/tui/src/core/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1153,7 +1153,7 @@ impl Engine {
"decision": "approved",
"source": "composer_bang",
}));
Self::execute_tool_with_lock(
let mut result = Self::execute_tool_with_lock(
self.tool_exec_lock.clone(),
spec.supports_parallel(),
false,
Expand All @@ -1165,7 +1165,14 @@ impl Engine {
None,
None,
)
.await
.await;
if let Ok(tool_result) = result.as_mut() {
stamp_tool_result_approval(
tool_result,
ToolApprovalStamp::ApprovedByUser,
);
}
result
}
Ok(ApprovalResult::Denied) => {
emit_tool_audit(json!({
Expand All @@ -1192,7 +1199,7 @@ impl Engine {
.context()
.clone()
.with_elevated_sandbox_policy(policy);
Self::execute_tool_with_lock(
let mut result = Self::execute_tool_with_lock(
self.tool_exec_lock.clone(),
spec.supports_parallel(),
false,
Expand All @@ -1204,7 +1211,14 @@ impl Engine {
None,
Some(elevated_context),
)
.await
.await;
if let Ok(tool_result) = result.as_mut() {
stamp_tool_result_approval(
tool_result,
ToolApprovalStamp::ApprovedWithPolicy,
);
}
result
}
Err(err) => Err(err),
}
Expand Down Expand Up @@ -3874,12 +3888,13 @@ use self::approval::{ApprovalDecision, ApprovalResult, UserInputDecision};
#[cfg(test)]
use self::dispatch::should_parallelize_tool_batch;
use self::dispatch::{
ParallelToolResult, ParallelToolResultEntry, ToolExecGuard, ToolExecOutcome,
ParallelToolResult, ParallelToolResultEntry, ToolApprovalStamp, ToolExecGuard, ToolExecOutcome,
ToolExecutionBatch, ToolExecutionPlan, caller_allowed_for_tool, caller_type_for_tool_use,
final_tool_input, format_tool_error, malformed_tool_arguments_error,
malformed_tool_arguments_input, mcp_tool_approval_description, mcp_tool_is_parallel_safe,
mcp_tool_is_read_only, parse_parallel_tool_calls, parse_tool_input,
plan_tool_execution_batches, should_force_update_plan_first, should_stop_after_plan_tool,
stamp_tool_result_approval,
};
#[cfg(test)]
use self::lsp_hooks::edited_paths_for_tool;
Expand Down
54 changes: 54 additions & 0 deletions crates/tui/src/core/engine/dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,60 @@ pub(super) struct ParallelToolResult {
pub(super) results: Vec<ParallelToolResultEntry>,
}

#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub(super) enum ToolApprovalStamp {
ApprovedByUser,
ApprovedWithPolicy,
}

impl ToolApprovalStamp {
fn decision(self) -> &'static str {
match self {
Self::ApprovedByUser => "approved_by_user",
Self::ApprovedWithPolicy => "approved_with_policy",
}
}

fn model_visible_note(self) -> &'static str {
match self {
Self::ApprovedByUser => {
"[approval] This tool call required approval and was approved by the user before execution."
}
Self::ApprovedWithPolicy => {
"[approval] This tool call required approval and was approved by the user with an adjusted execution policy before execution."
}
}
}
}

pub(super) fn stamp_tool_result_approval(result: &mut ToolResult, approval: ToolApprovalStamp) {
let approval_metadata = json!({
"required": true,
"decision": approval.decision(),
"model_visible": true,
});
let metadata = result.metadata.get_or_insert_with(|| json!({}));
if let Some(object) = metadata.as_object_mut() {
object.insert("approval".to_string(), approval_metadata);
} else {
let prior = std::mem::replace(metadata, json!({}));
if let Some(object) = metadata.as_object_mut() {
object.insert("_prior".to_string(), prior);
object.insert("approval".to_string(), approval_metadata);
}
}

let note = approval.model_visible_note();
if result.content.starts_with("[approval] ") {
return;
}
if result.content.is_empty() {
result.content = note.to_string();
} else {
result.content = format!("{note}\n\n{}", result.content);
}
}

// Hold the lock guard for the duration of a tool execution.
// The inner guards are held for RAII purposes (dropped when the guard is dropped).
pub(super) enum ToolExecGuard<'a> {
Expand Down
40 changes: 40 additions & 0 deletions crates/tui/src/core/engine/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1441,6 +1441,46 @@ fn tool_exec_outcome_tracks_duration() {
assert!(outcome.started_at.elapsed().as_nanos() > 0);
}

#[test]
fn approval_stamp_makes_user_approval_model_visible() {
let mut result = ToolResult::success("stdout");

stamp_tool_result_approval(&mut result, ToolApprovalStamp::ApprovedByUser);

assert!(
result
.content
.starts_with("[approval] This tool call required approval"),
"{}",
result.content
);
assert!(
result
.content
.contains("approved by the user before execution")
);
assert!(result.content.ends_with("stdout"));

let metadata = result.metadata.expect("approval metadata");
assert_eq!(metadata["approval"]["required"], true);
assert_eq!(metadata["approval"]["decision"], "approved_by_user");
assert_eq!(metadata["approval"]["model_visible"], true);
}

#[test]
fn approval_stamp_preserves_existing_metadata() {
let mut result = ToolResult::success("ok").with_metadata(json!({
"summary": "kept"
}));

stamp_tool_result_approval(&mut result, ToolApprovalStamp::ApprovedWithPolicy);

let metadata = result.metadata.expect("metadata");
assert_eq!(metadata["summary"], "kept");
assert_eq!(metadata["approval"]["decision"], "approved_with_policy");
assert!(result.content.contains("adjusted execution policy"));
}

#[test]
fn core_native_tools_stay_loaded_in_yolo_mode() {
let always_load = HashSet::new();
Expand Down
24 changes: 18 additions & 6 deletions crates/tui/src/core/engine/turn_loop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2247,10 +2247,11 @@ impl Engine {
continue;
}

// Handle approval flow: returns (result_override, context_override)
let (result_override, context_override): (
// Handle approval flow: returns (result_override, context_override, approval_stamp)
let (result_override, context_override, approval_stamp): (
Option<Result<ToolResult, ToolError>>,
Option<crate::tools::ToolContext>,
Option<ToolApprovalStamp>,
) = if plan.approval_required {
emit_tool_audit(json!({
"event": "tool.approval_required",
Expand Down Expand Up @@ -2295,7 +2296,7 @@ impl Engine {
"decision": "approved",
"caller": caller_type_for_tool_use(tool_caller.as_ref()),
}));
(None, None)
(None, None, Some(ToolApprovalStamp::ApprovedByUser))
}
Ok(ApprovalResult::Denied) => {
emit_tool_audit(json!({
Expand All @@ -2310,6 +2311,7 @@ impl Engine {
"Tool '{tool_name}' denied by user"
)))),
None,
None,
)
}
Ok(ApprovalResult::RetryWithPolicy(policy)) => {
Expand All @@ -2324,12 +2326,16 @@ impl Engine {
let elevated_context = tool_registry.map(|r| {
r.context().clone().with_elevated_sandbox_policy(policy)
});
(None, elevated_context)
(
None,
elevated_context,
Some(ToolApprovalStamp::ApprovedWithPolicy),
)
}
Err(err) => (Some(Err(err)), None),
Err(err) => (Some(Err(err)), None, None),
}
} else {
(None, None)
(None, None, None)
};

// Per-tool snapshot for surgical undo (#384): capture workspace
Expand Down Expand Up @@ -2369,6 +2375,12 @@ impl Engine {
.await
};

if let Some(approval_stamp) = approval_stamp
&& let Ok(tool_result) = result.as_mut()
{
stamp_tool_result_approval(tool_result, approval_stamp);
}

// #500: spill outsized tool outputs to disk before the
// result fans out to the model context and the UI cell.
// Both consumers see the same artifact reference block +
Expand Down
36 changes: 34 additions & 2 deletions crates/tui/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1420,7 +1420,7 @@ async fn main() -> Result<()> {
};

// Default: Interactive TUI
// --yolo starts in YOLO mode (auto-approve; shell if allow_shell=true)
// --yolo starts in YOLO mode (auto-approve; shell enabled)
run_interactive(&cli, &config, resume_session_id, None).await
}

Expand Down Expand Up @@ -6409,6 +6409,10 @@ fn normalize_windows_config_path_str(path: &str) -> String {
normalized.to_ascii_lowercase()
}

fn interactive_tui_allow_shell(yolo: bool, config: &Config) -> bool {
yolo || config.interactive_allow_shell()
}

async fn run_interactive(
cli: &Cli,
config: &Config,
Expand Down Expand Up @@ -6513,7 +6517,7 @@ async fn run_interactive(
workspace,
config_path: cli.config.clone(),
config_profile: cli.profile.clone(),
allow_shell: yolo || config.allow_shell(),
allow_shell: interactive_tui_allow_shell(yolo, config),
use_alt_screen,
use_mouse_capture,
use_bracketed_paste,
Expand Down Expand Up @@ -8820,6 +8824,34 @@ mod terminal_mode_tests {
}
}

#[cfg(test)]
mod interactive_startup_tests {
use super::*;

#[test]
fn interactive_tui_defaults_agent_shell_to_approval_gated_on() {
let default_config = Config::default();
assert!(
interactive_tui_allow_shell(false, &default_config),
"interactive Agent mode should expose shell tools by default so approvals can gate commands"
);

let disabled = Config {
allow_shell: Some(false),
..Config::default()
};
assert!(
!interactive_tui_allow_shell(false, &disabled),
"explicit allow_shell=false still hides shell tools"
);

assert!(
interactive_tui_allow_shell(true, &disabled),
"YOLO forces shell access for its no-guardrails contract"
);
}
}

#[cfg(test)]
mod project_config_tests {
use super::*;
Expand Down
15 changes: 10 additions & 5 deletions crates/tui/src/tui/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2490,18 +2490,23 @@ impl App {
// Durable Agent-era permission baseline (#3386). Plan/Auto/YOLO derive
// from and restore to this. When the user starts in Auto or YOLO the
// live shell flag is force-enabled below, so the baseline shell value is
// taken from config (the pre-mode surface) rather than the live mirror;
// otherwise it mirrors the resolved `allow_shell` option. Trust is
// never part of the Agent baseline (it is YOLO-only authority). Approval
// mirrors the configured policy.
// taken from the interactive default (the pre-mode Agent surface) rather
// than the YOLO-forced live mirror; otherwise it mirrors the resolved
// `allow_shell` option, which already carries that same interactive
// default. Using `interactive_allow_shell()` here keeps the Agent
// baseline identical regardless of launch mode, so a YOLO/Auto -> Agent
// downshift exposes shell (approval-gated) exactly as documented, while
// an explicit `allow_shell = false` still hides it. Trust is never part
// of the Agent baseline (it is YOLO-only authority). Approval mirrors the
// configured policy.
let configured_approval_mode = config
.approval_policy
.as_deref()
.and_then(ApprovalMode::from_config_value)
.unwrap_or_default();
let mode_prefs = ModeSessionPrefs {
agent_allow_shell: if matches!(initial_mode, AppMode::Auto | AppMode::Yolo) {
config.allow_shell()
config.interactive_allow_shell()
} else {
allow_shell
},
Expand Down
Loading
Loading