Synced from monorepo
Changes: - Classify clipboard delivery confidence - Add durable session update append - Scope the xAI session bearer to first-party memory embedding endpoints - Persist subagent outputs to disk and bound long-lived agent state - Add MiniSweAgent:bash for mini-swe-agent parity - Revert taking local sessions off the persistent shell - Contextual tip recommending grok wrap on SSH sessions - Voice STT bearer from model BYOK env_key/api_key - Define exact website policies for sandbox - Gate unsafe shell environments - Shared pin hoist; single require_sha gate for marketplace plugins - Server-signed is-managed claim (closes sidecar-removal downgrade) - Optional require_sha pin for remote plugin installs - Show session title and last exchange in the exit resume hint - Gate shell output redirects - Warn when fail_closed is present but not a boolean - Add canonical text editing core (ratatui-textarea) - Keep execution state out of goal scratch - Add acknowledged persistence primitives - Inherit child network restrictions in sandbox - Fail closed when hook matchers fail to recompile - Add MCP setup preferences for plugin MCPs - Gate sourced shell scripts - Gate file-typed project hooks - grok wrap: restore terminal modes on child death - Harden owner-only permissions on auth and MCP credentials - Create crash dump files with owner-only permissions - Write the agent_id cache owner-only (0600) - SessionMetrics mode skips Mixpanel profile sync - Dashboard: slim live-tail peek - Yank full queued prompt text, not (+N lines) - Defeat clock-rollback on the signed managed-config cache - Stop early session/cancel from overtaking the prompt and wedging the turn slot - Self-heal a diverged agent entrypoint on startup - Add matched inference expectations in test-support - Add AuthSingleFlight cancel/successor gap tests - Remove consumer from external OTEL allowlist and pin scrub coverage - Enable /copy in minimal mode - Surface capacity and API-key detail on 429 errors - Single-flight interactive auth - Fix PageUp/PageDown skipping lines behind sticky prompt header
This commit is contained in:
grokkybara[bot]
parent
8adf9013a0
commit
98c3b2438a
225 changed files with 18836 additions and 7156 deletions
|
|
@ -276,7 +276,8 @@ async fn run(args: Args, cwd: PathBuf) -> anyhow::Result<()> {
|
|||
"Workspace server sandbox NOT active"
|
||||
};
|
||||
tracing::info!(
|
||||
profile = % profile_name, active, restrict_network =
|
||||
profile = % profile_name, active,
|
||||
restrict_network_at_known_linux_launches =
|
||||
xai_grok_sandbox::should_restrict_child_network(), "{status_msg}"
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -254,6 +254,14 @@ pub fn repo_config_kinds(cwd: &Path) -> Vec<&'static str> {
|
|||
collect_repo_config_kinds(cwd, false)
|
||||
}
|
||||
|
||||
fn path_present_or_uncertain(path: &Path) -> bool {
|
||||
match std::fs::symlink_metadata(path) {
|
||||
Ok(_) => true,
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => false,
|
||||
Err(_) => true,
|
||||
}
|
||||
}
|
||||
|
||||
/// Shared scanner behind [`repo_configs_present`] and [`repo_config_kinds`]. With
|
||||
/// `first_only` it returns immediately after the first marker (the gate's
|
||||
/// historical short-circuit); otherwise it collects every distinct kind.
|
||||
|
|
@ -346,7 +354,7 @@ fn collect_repo_config_kinds(cwd: &Path, first_only: bool) -> Vec<&'static str>
|
|||
// resolve trusted and run ungated. Presence mirrors discovery's "something to
|
||||
// gate" check.
|
||||
let hook_root = chain.git_root.as_deref().unwrap_or(cwd);
|
||||
if hook_root.join(".grok").join("hooks").is_dir()
|
||||
if path_present_or_uncertain(&hook_root.join(".grok").join("hooks"))
|
||||
|| hook_root.join(".cursor").join("hooks.json").is_file()
|
||||
{
|
||||
hit!("hooks");
|
||||
|
|
@ -610,6 +618,29 @@ mod tests {
|
|||
assert!(repo_configs_present(tmp.path()));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn repo_configs_present_detects_project_hooks_file() {
|
||||
let tmp = repo_tmp();
|
||||
let grok = tmp.path().join(".grok");
|
||||
std::fs::create_dir_all(&grok).unwrap();
|
||||
std::fs::write(grok.join("hooks"), "{}").unwrap();
|
||||
|
||||
assert!(repo_configs_present(tmp.path()));
|
||||
assert!(repo_config_kinds(tmp.path()).contains(&"hooks"));
|
||||
}
|
||||
|
||||
#[cfg(unix)]
|
||||
#[test]
|
||||
fn repo_configs_present_detects_dangling_project_hooks_symlink() {
|
||||
let tmp = repo_tmp();
|
||||
let grok = tmp.path().join(".grok");
|
||||
std::fs::create_dir_all(&grok).unwrap();
|
||||
std::os::unix::fs::symlink("missing-hooks", grok.join("hooks")).unwrap();
|
||||
|
||||
assert!(repo_configs_present(tmp.path()));
|
||||
assert!(repo_config_kinds(tmp.path()).contains(&"hooks"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn repo_configs_present_detects_project_hooks_from_subdir() {
|
||||
// Hooks live at the git root but the session is launched from a subdir;
|
||||
|
|
|
|||
|
|
@ -702,10 +702,11 @@ impl WorkspaceHandle {
|
|||
TrackingMode::AllDirty,
|
||||
hunk_cancel.clone(),
|
||||
);
|
||||
let result = self.create_session_with_tracker_and_viewer_ctx(
|
||||
let result = self.create_session_with_tracker_inner(
|
||||
session_id,
|
||||
session_cwd,
|
||||
hunk_tracker,
|
||||
Some(hunk_cancel.clone()),
|
||||
tool_config,
|
||||
capability,
|
||||
viewer_ctx,
|
||||
|
|
@ -738,7 +739,8 @@ impl WorkspaceHandle {
|
|||
)
|
||||
}
|
||||
/// Variant of [`create_session_with_tracker`](Self::create_session_with_tracker)
|
||||
/// that carries a session-bind viewer context.
|
||||
/// that carries a session-bind viewer context. The tracker is externally
|
||||
/// owned, so the session stores no cancel token for it.
|
||||
pub fn create_session_with_tracker_and_viewer_ctx(
|
||||
&self,
|
||||
session_id: impl Into<String>,
|
||||
|
|
@ -748,6 +750,32 @@ impl WorkspaceHandle {
|
|||
capability: CapabilityMode,
|
||||
viewer_ctx: Option<xai_tool_runtime::WorkspaceViewerContext>,
|
||||
system_notifications: bool,
|
||||
) -> WorkspaceResult<Arc<WorkspaceSession>> {
|
||||
self.create_session_with_tracker_inner(
|
||||
session_id,
|
||||
cwd,
|
||||
hunk_tracker,
|
||||
None,
|
||||
tool_config,
|
||||
capability,
|
||||
viewer_ctx,
|
||||
system_notifications,
|
||||
)
|
||||
}
|
||||
/// Shared creation body. `hunk_tracker_cancel` is `Some` only for
|
||||
/// workspace-spawned trackers, whose actor lifetime the session then
|
||||
/// owns; externally owned trackers pass `None`.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
fn create_session_with_tracker_inner(
|
||||
&self,
|
||||
session_id: impl Into<String>,
|
||||
cwd: std::path::PathBuf,
|
||||
hunk_tracker: HunkTrackerHandle,
|
||||
hunk_tracker_cancel: Option<tokio_util::sync::CancellationToken>,
|
||||
tool_config: Option<xai_grok_tools::registry::types::ToolServerConfig>,
|
||||
capability: CapabilityMode,
|
||||
viewer_ctx: Option<xai_tool_runtime::WorkspaceViewerContext>,
|
||||
system_notifications: bool,
|
||||
) -> WorkspaceResult<Arc<WorkspaceSession>> {
|
||||
let session_id = session_id.into();
|
||||
if session_id.is_empty() {
|
||||
|
|
@ -797,6 +825,7 @@ impl WorkspaceHandle {
|
|||
toolset,
|
||||
terminal_backend,
|
||||
hunk_tracker,
|
||||
hunk_tracker_cancel,
|
||||
viewer_ctx,
|
||||
system_notifications,
|
||||
system_notify_channel,
|
||||
|
|
@ -2739,7 +2768,7 @@ impl WorkspaceHandle {
|
|||
cwd.clone(),
|
||||
hunk_event_tx,
|
||||
TrackingMode::AllDirty,
|
||||
hunk_cancel,
|
||||
hunk_cancel.clone(),
|
||||
);
|
||||
let session = Arc::new(WorkspaceSession::new(
|
||||
config.agent_id.clone(),
|
||||
|
|
@ -2752,6 +2781,7 @@ impl WorkspaceHandle {
|
|||
toolset,
|
||||
terminal_backend,
|
||||
hunk_tracker,
|
||||
Some(hunk_cancel),
|
||||
inherited_viewer_ctx,
|
||||
false,
|
||||
None,
|
||||
|
|
@ -2759,9 +2789,11 @@ impl WorkspaceHandle {
|
|||
{
|
||||
let mut sessions = self.shared.sessions.write();
|
||||
if self.shared.activity_tracker.is_draining() {
|
||||
session.cancel_hunk_tracker();
|
||||
return Err(WorkspaceError::ShuttingDown);
|
||||
}
|
||||
if sessions.contains_key(&config.agent_id) {
|
||||
session.cancel_hunk_tracker();
|
||||
return Err(WorkspaceError::SessionAlreadyExists(config.agent_id));
|
||||
}
|
||||
sessions.insert(config.agent_id.clone(), session.clone());
|
||||
|
|
@ -2785,6 +2817,7 @@ impl WorkspaceHandle {
|
|||
drop(sessions);
|
||||
session.abort_system_notify_forwarder();
|
||||
session.shutdown_terminal_backend();
|
||||
session.cancel_hunk_tracker();
|
||||
self.shared.tool_defs_last_emit.remove(session_id);
|
||||
Ok(())
|
||||
}
|
||||
|
|
@ -5726,6 +5759,98 @@ pub(crate) mod tests {
|
|||
assert_backend_stops(&retained_backend).await;
|
||||
drop(retained_toolset);
|
||||
}
|
||||
async fn assert_hunk_tracker_stops(tracker: &xai_hunk_tracker::HunkTrackerHandle) {
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10);
|
||||
while !tracker.is_closed() {
|
||||
assert!(
|
||||
std::time::Instant::now() < deadline,
|
||||
"hunk-tracker actor must stop within the deadline despite live \
|
||||
handle clones"
|
||||
);
|
||||
tokio::time::sleep(std::time::Duration::from_millis(20)).await;
|
||||
}
|
||||
}
|
||||
/// `drop_session` cancels the workspace-spawned hunk-tracker actor even
|
||||
/// while a leaked `HunkTrackerHandle` clone keeps its command channel
|
||||
/// open. Rationale on `cancel_hunk_tracker`.
|
||||
#[tokio::test]
|
||||
async fn drop_session_cancels_workspace_spawned_hunk_tracker() {
|
||||
let handle = make_handle();
|
||||
let session = handle
|
||||
.create_session_with_config("doomed-ht", None, None, CapabilityMode::All, None, false)
|
||||
.expect("create session");
|
||||
let leaked_tracker = session.hunk_tracker().clone();
|
||||
assert!(
|
||||
!leaked_tracker.is_closed(),
|
||||
"precondition: the actor is alive while the session exists"
|
||||
);
|
||||
drop(session);
|
||||
handle.drop_session("doomed-ht", "doomed-ht").expect("drop");
|
||||
assert_hunk_tracker_stops(&leaked_tracker).await;
|
||||
}
|
||||
/// Same guarantee for the fork spawn site.
|
||||
#[tokio::test]
|
||||
async fn drop_session_cancels_forked_session_hunk_tracker() {
|
||||
let handle = make_handle();
|
||||
let child = handle
|
||||
.fork_session(fork_cfg_with(
|
||||
"child-ht",
|
||||
CapabilityMode::ReadWrite,
|
||||
None,
|
||||
Some("main"),
|
||||
))
|
||||
.await
|
||||
.expect("fork should succeed");
|
||||
let leaked_tracker = child.hunk_tracker().clone();
|
||||
assert!(
|
||||
!leaked_tracker.is_closed(),
|
||||
"precondition: the actor is alive while the session exists"
|
||||
);
|
||||
drop(child);
|
||||
handle.drop_session("child-ht", "child-ht").expect("drop");
|
||||
assert_hunk_tracker_stops(&leaked_tracker).await;
|
||||
}
|
||||
/// The inverse guarantee: a tracker bound via `create_session_with_tracker`
|
||||
/// is externally owned, so `drop_session` must NOT cancel it. The agent
|
||||
/// shares such trackers with the workspace session.
|
||||
#[tokio::test]
|
||||
async fn drop_session_leaves_externally_owned_hunk_tracker_alive() {
|
||||
let handle = make_handle();
|
||||
let cwd = handle.shared.root_cwd.clone();
|
||||
let (hunk_event_tx, _hunk_event_rx) = tokio::sync::mpsc::unbounded_channel();
|
||||
let owner_cancel = tokio_util::sync::CancellationToken::new();
|
||||
let tracker = HunkTrackerActor::spawn(
|
||||
"external-ht".to_string(),
|
||||
cwd.clone(),
|
||||
hunk_event_tx,
|
||||
TrackingMode::AllDirty,
|
||||
owner_cancel.clone(),
|
||||
);
|
||||
let session = handle
|
||||
.create_session_with_tracker(
|
||||
"external-ht",
|
||||
cwd,
|
||||
tracker.clone(),
|
||||
None,
|
||||
CapabilityMode::All,
|
||||
)
|
||||
.expect("create session");
|
||||
assert!(
|
||||
!tracker.is_closed(),
|
||||
"precondition: the actor is alive while the session exists"
|
||||
);
|
||||
drop(session);
|
||||
handle
|
||||
.drop_session("external-ht", "external-ht")
|
||||
.expect("drop");
|
||||
let _ = tracker.get_all_hunks().await;
|
||||
assert!(
|
||||
!tracker.is_closed(),
|
||||
"drop_session must not cancel an externally owned hunk tracker"
|
||||
);
|
||||
owner_cancel.cancel();
|
||||
assert_hunk_tracker_stops(&tracker).await;
|
||||
}
|
||||
/// Isolation matrix #5: a workspace process restart loses tasks (they are
|
||||
/// process state — physics), and what's pinned here is the recovery UX:
|
||||
/// the same session id recreates cleanly on the fresh process, the task
|
||||
|
|
|
|||
|
|
@ -1078,6 +1078,7 @@ impl ToolServerHandler for WorkspaceRpcHandler {
|
|||
if let Some(session) = sessions.remove(sid) {
|
||||
session.abort_system_notify_forwarder();
|
||||
session.shutdown_terminal_backend();
|
||||
session.cancel_hunk_tracker();
|
||||
}
|
||||
let empty = sessions.is_empty();
|
||||
let already_winding_down = self.workspace.activity_tracker().is_draining();
|
||||
|
|
|
|||
|
|
@ -14,7 +14,9 @@ use super::bash_command_splitting::{
|
|||
PlainCommand, is_wrapper_command, strip_wrapper_command, try_parse_shell,
|
||||
try_parse_word_only_commands_sequence, unwrap_wrappers,
|
||||
};
|
||||
use super::shell_access::{command_words_write_paths, command_write_paths_in_tree};
|
||||
use super::shell_access::{
|
||||
command_words_write_paths, command_write_paths_in_tree, is_safe_write_sink,
|
||||
};
|
||||
use super::types::AccessKind;
|
||||
|
||||
/// Classifier outcome for a single tool authorization.
|
||||
|
|
@ -408,7 +410,7 @@ fn classify_bash(cmd: &str) -> ClassifierVerdict {
|
|||
// (or any `env` option) can change which binary runs / how code resolves.
|
||||
// Read from the PARSED, quote-stripped tree so `env "LD_PRELOAD=..."` can't
|
||||
// hide the key.
|
||||
if sets_unsafe_env(tree.root_node(), cmd, &cmds) {
|
||||
if script_sets_unsafe_env(tree.root_node(), cmd, &cmds) {
|
||||
return ClassifierVerdict::Block;
|
||||
}
|
||||
// A routine command can still write an arbitrary destination via a redirect
|
||||
|
|
@ -744,7 +746,7 @@ fn explicit_launch_target<'a>(head: &str, inner: &'a [String]) -> LaunchTarget<'
|
|||
/// whose KEY is not in [`SAFE_ENV_KEYS`], or passes an option to `env` (which can
|
||||
/// run a string, clear, or unset the environment). Reads the PARSED tree so
|
||||
/// quoting (`env "LD_PRELOAD=..."`) can't hide a key from the check.
|
||||
fn sets_unsafe_env(root: Node<'_>, src: &str, cmds: &[PlainCommand]) -> bool {
|
||||
pub(crate) fn script_sets_unsafe_env(root: Node<'_>, src: &str, cmds: &[PlainCommand]) -> bool {
|
||||
// (a) Inline `KEY=val cmd` assignments are `variable_assignment` nodes
|
||||
// (stripped from PlainCommand words), so walk the tree for them.
|
||||
let mut stack = vec![root];
|
||||
|
|
@ -823,12 +825,6 @@ fn is_lone_wrapper(words: &[String]) -> bool {
|
|||
words.len() == 1 && is_wrapper_command(words)
|
||||
}
|
||||
|
||||
/// Safe write sinks: writing to these discards/echoes output rather than
|
||||
/// touching a real file. Exact match.
|
||||
fn is_safe_write_sink(path: &str) -> bool {
|
||||
matches!(path, "/dev/null" | "/dev/stdout" | "/dev/stderr")
|
||||
}
|
||||
|
||||
/// `find` is routine ONLY when it has no action primary that deletes, executes,
|
||||
/// or writes files. Operates on the already-unwrapped command words.
|
||||
fn find_is_read_only(words: &[String]) -> bool {
|
||||
|
|
|
|||
|
|
@ -152,7 +152,7 @@ pub(crate) fn is_setup_command(cmd: &[String]) -> bool {
|
|||
|
||||
matches!(
|
||||
cmd[0].as_str(),
|
||||
"cd" | "pushd" | "popd" | "export" | "unset" | "set" | "source" | "." | "sleep" | "timeout"
|
||||
"cd" | "pushd" | "popd" | "export" | "unset" | "set" | "sleep" | "timeout"
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -7,12 +7,15 @@ use chrono::Utc;
|
|||
use tokio::sync::{mpsc, oneshot};
|
||||
use xai_acp_lib::AcpAgentGatewaySender as GatewaySender;
|
||||
|
||||
use crate::permission::auto_mode::script_sets_unsafe_env;
|
||||
use crate::permission::bash_command_splitting::{
|
||||
all_commands_from_script, is_setup_command, unwrap_wrappers,
|
||||
is_setup_command, try_parse_shell, try_parse_word_only_commands_sequence, unwrap_wrappers,
|
||||
};
|
||||
use crate::permission::policy::CompiledPolicy;
|
||||
use crate::permission::prompter::{AcpPrompter, PromptOutcome};
|
||||
use crate::permission::shell_access::combine_decisions;
|
||||
use crate::permission::shell_access::{
|
||||
combine_decisions, command_write_paths_in_tree, is_safe_write_sink,
|
||||
};
|
||||
use crate::permission::state::{PermissionState, load_state_from_disk, persist_state};
|
||||
use crate::permission::types::{
|
||||
AccessKind, ClientType, Decision, EditPolicy, PermissionCommand, PermissionEvent, PromptPolicy,
|
||||
|
|
@ -339,8 +342,7 @@ fn matches_whitelist_prefix(segment_str: &str, allowed_prefix: &str) -> bool {
|
|||
matches_command_prefix(segment_str, allowed_prefix)
|
||||
}
|
||||
|
||||
/// Result of evaluating a bash script's segments against the current
|
||||
/// permission state.
|
||||
/// Ordinary command-segment outcome, before script-level effect floors.
|
||||
#[derive(Debug)]
|
||||
pub(crate) enum SegmentEvaluation {
|
||||
/// All non-setup segments safe/always-safe or on an allow-prefix.
|
||||
|
|
@ -348,7 +350,7 @@ pub(crate) enum SegmentEvaluation {
|
|||
AutoAllow { via_session_grant: bool },
|
||||
/// Disallow-prefix matched; reject without prompting.
|
||||
Reject(String),
|
||||
/// Needs a user decision (manager prompts once for the full script).
|
||||
/// One or more segments need a user decision.
|
||||
NeedsPrompts {
|
||||
#[allow(dead_code)]
|
||||
segments: Vec<String>,
|
||||
|
|
@ -360,34 +362,51 @@ pub(crate) enum SegmentEvaluation {
|
|||
Unparseable,
|
||||
}
|
||||
|
||||
/// Walk every command in a chained script and classify the script as a whole
|
||||
/// against the current permission state.
|
||||
///
|
||||
/// This is the per-segment replacement for the previous primary-only check,
|
||||
/// closing the auto-allow bypass where a safe / whitelisted primary command
|
||||
/// could smuggle a dangerous follow-on through `&&`, `||`, `;`, or `|`.
|
||||
pub(crate) fn evaluate_bash_segments(cmd: &str, state: &PermissionState) -> SegmentEvaluation {
|
||||
evaluate_bash_segments_inner(cmd, state, true)
|
||||
/// One request's parsed Bash authorization facts.
|
||||
#[derive(Debug)]
|
||||
struct BashEvaluation {
|
||||
segments: SegmentEvaluation,
|
||||
writes_real_file: bool,
|
||||
sets_unsafe_env: bool,
|
||||
exact_grant: bool,
|
||||
all_segments_granted: bool,
|
||||
}
|
||||
|
||||
/// Core of [`evaluate_bash_segments`]. When `honor_safe_lists` is `false`, the
|
||||
/// built-in safe / always-safe command lists are ignored, so only the user's
|
||||
/// explicit `allowed_bash_commands` grants auto-allow a segment. That mode lets
|
||||
/// an explicit "Always allow" grant satisfy an `ask` policy floor (ask once,
|
||||
/// then remember) without letting the floor be bypassed for commands the user
|
||||
/// never personally approved. Disallow and dangerous handling are identical in
|
||||
/// both modes.
|
||||
pub(crate) fn evaluate_bash_segments_inner(
|
||||
cmd: &str,
|
||||
state: &PermissionState,
|
||||
honor_safe_lists: bool,
|
||||
) -> SegmentEvaluation {
|
||||
let Some(segments) = all_commands_from_script(cmd) else {
|
||||
return SegmentEvaluation::Unparseable;
|
||||
/// Parse and classify one Bash request once, keeping ordinary segment outcome
|
||||
/// separate from the script-level real-file-write and unsafe-environment floors.
|
||||
fn evaluate_bash(cmd: &str, state: &PermissionState, honor_safe_lists: bool) -> BashEvaluation {
|
||||
let exact_grant = state.allowed_bash_commands.contains(cmd);
|
||||
let Some(tree) = try_parse_shell(cmd) else {
|
||||
return BashEvaluation {
|
||||
segments: SegmentEvaluation::Unparseable,
|
||||
writes_real_file: false,
|
||||
sets_unsafe_env: false,
|
||||
exact_grant,
|
||||
all_segments_granted: false,
|
||||
};
|
||||
};
|
||||
let writes_real_file = command_write_paths_in_tree(tree.root_node(), cmd)
|
||||
.into_iter()
|
||||
.any(|path| !is_safe_write_sink(&path));
|
||||
let segments = try_parse_word_only_commands_sequence(&tree, cmd);
|
||||
let sets_unsafe_env = script_sets_unsafe_env(
|
||||
tree.root_node(),
|
||||
cmd,
|
||||
segments.as_deref().unwrap_or_default(),
|
||||
);
|
||||
let Some(segments) = segments else {
|
||||
return BashEvaluation {
|
||||
segments: SegmentEvaluation::Unparseable,
|
||||
writes_real_file,
|
||||
sets_unsafe_env,
|
||||
exact_grant,
|
||||
all_segments_granted: false,
|
||||
};
|
||||
};
|
||||
let mut needs_prompt: Vec<String> = Vec::new();
|
||||
let mut any_dangerous = false;
|
||||
let mut via_session_grant = false;
|
||||
let mut all_segments_granted = true;
|
||||
for parsed in segments {
|
||||
let raw_words = parsed.words();
|
||||
// Peel wrapper commands like `timeout 30 …`, `env FOO=1 …`, `nice -n 5 …`
|
||||
|
|
@ -406,11 +425,23 @@ pub(crate) fn evaluate_bash_segments_inner(
|
|||
.iter()
|
||||
.find(|d| matches_whitelist_prefix(&s, d))
|
||||
{
|
||||
return SegmentEvaluation::Reject(format!(
|
||||
"User previously rejected `{d}` for this session"
|
||||
));
|
||||
return BashEvaluation {
|
||||
segments: SegmentEvaluation::Reject(format!(
|
||||
"User previously rejected `{d}` for this session"
|
||||
)),
|
||||
writes_real_file,
|
||||
sets_unsafe_env,
|
||||
exact_grant,
|
||||
all_segments_granted,
|
||||
};
|
||||
}
|
||||
|
||||
let matched_grant = state
|
||||
.allowed_bash_commands
|
||||
.iter()
|
||||
.any(|a| matches_whitelist_prefix(&s, a));
|
||||
all_segments_granted &= matched_grant;
|
||||
|
||||
// 2. Dangerous commands must be prompted even if a whitelist prefix
|
||||
// would otherwise match. This preserves the historical invariant
|
||||
// that `is_dangerous_command` took precedence over auto-allow.
|
||||
|
|
@ -422,10 +453,6 @@ pub(crate) fn evaluate_bash_segments_inner(
|
|||
|
||||
// 3. Auto-allow conditions. Built-in safe lists count only when
|
||||
// `honor_safe_lists` is set; an explicit user grant always counts.
|
||||
let matched_grant = state
|
||||
.allowed_bash_commands
|
||||
.iter()
|
||||
.any(|a| matches_whitelist_prefix(&s, a));
|
||||
let matched_safe = honor_safe_lists
|
||||
&& (is_safe_command_words(words) || is_always_safe_command_words(words));
|
||||
if matched_grant || matched_safe {
|
||||
|
|
@ -438,16 +465,37 @@ pub(crate) fn evaluate_bash_segments_inner(
|
|||
// 4. Otherwise: prompt for this segment.
|
||||
needs_prompt.push(s);
|
||||
}
|
||||
if needs_prompt.is_empty() {
|
||||
let segments = if needs_prompt.is_empty() {
|
||||
SegmentEvaluation::AutoAllow { via_session_grant }
|
||||
} else {
|
||||
SegmentEvaluation::NeedsPrompts {
|
||||
segments: needs_prompt,
|
||||
any_dangerous,
|
||||
}
|
||||
};
|
||||
BashEvaluation {
|
||||
segments,
|
||||
writes_real_file,
|
||||
sets_unsafe_env,
|
||||
exact_grant,
|
||||
all_segments_granted,
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn evaluate_bash_segments(cmd: &str, state: &PermissionState) -> SegmentEvaluation {
|
||||
evaluate_bash(cmd, state, true).segments
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn evaluate_bash_segments_inner(
|
||||
cmd: &str,
|
||||
state: &PermissionState,
|
||||
honor_safe_lists: bool,
|
||||
) -> SegmentEvaluation {
|
||||
evaluate_bash(cmd, state, honor_safe_lists).segments
|
||||
}
|
||||
|
||||
impl PermissionHandle {
|
||||
pub fn allow_all() -> Self {
|
||||
PermissionHandle::AllowAll
|
||||
|
|
@ -691,32 +739,42 @@ fn persisted_bash_auto_allows(
|
|||
(state.allow_bash_execute && yolo_pin.is_none()) || state.allowed_bash_commands.contains(cmd)
|
||||
}
|
||||
|
||||
fn bash_write_floor_requires_prompt(evaluation: Option<&BashEvaluation>) -> bool {
|
||||
evaluation.is_some_and(|evaluation| evaluation.writes_real_file && !evaluation.exact_grant)
|
||||
}
|
||||
|
||||
fn bash_unsafe_env_floor_requires_prompt(evaluation: Option<&BashEvaluation>) -> bool {
|
||||
evaluation.is_some_and(|evaluation| evaluation.sets_unsafe_env && !evaluation.exact_grant)
|
||||
}
|
||||
|
||||
fn bash_request_floor_requires_prompt(evaluation: Option<&BashEvaluation>) -> bool {
|
||||
bash_write_floor_requires_prompt(evaluation)
|
||||
|| bash_unsafe_env_floor_requires_prompt(evaluation)
|
||||
}
|
||||
|
||||
fn sandbox_may_auto_allow_bash(evaluation: Option<&BashEvaluation>, sandbox_active: bool) -> bool {
|
||||
sandbox_active && !bash_request_floor_requires_prompt(evaluation)
|
||||
}
|
||||
|
||||
/// Policy knobs for [`bash_grant_pre_decision`].
|
||||
#[derive(Clone, Copy)]
|
||||
struct BashGrantOpts {
|
||||
/// When false, only explicit `allowed_bash_commands` count as AutoAllow.
|
||||
honor_safe_lists: bool,
|
||||
/// When false, NeedsPrompts / Unparseable never auto-allow via grants.
|
||||
allow_blanket: bool,
|
||||
/// When true (pre-classifier), refuse approve-all on dangerous / unparseable
|
||||
/// scripts so the classifier still sees them. Exact full-script grants stick.
|
||||
conservative_blanket: bool,
|
||||
}
|
||||
|
||||
impl BashGrantOpts {
|
||||
/// Early short-circuit before auto classify.
|
||||
const PRE_CLASSIFIER: Self = Self {
|
||||
honor_safe_lists: true,
|
||||
allow_blanket: true,
|
||||
conservative_blanket: true,
|
||||
};
|
||||
/// Managed Ask floor with remember_tool_approvals (explicit grants only).
|
||||
const ASK_FLOOR_REMEMBER: Self = Self {
|
||||
honor_safe_lists: false,
|
||||
allow_blanket: false,
|
||||
conservative_blanket: false,
|
||||
};
|
||||
/// Post-classify bash arm; blanket only when auto did not force a prompt.
|
||||
fn post_classify(auto_forced_prompt: bool) -> Self {
|
||||
Self {
|
||||
honor_safe_lists: true,
|
||||
|
|
@ -730,54 +788,50 @@ fn grant_allow(reason: &'static str) -> Option<(Decision, &'static str)> {
|
|||
Some((Decision::Allow, reason))
|
||||
}
|
||||
|
||||
/// Shared bash grant path for pre-classifier short-circuit and post-classify arm.
|
||||
fn bash_grant_pre_decision(
|
||||
cmd: &str,
|
||||
evaluation: &BashEvaluation,
|
||||
state: &PermissionState,
|
||||
yolo_pin: Option<&'static str>,
|
||||
opts: BashGrantOpts,
|
||||
) -> Option<(Decision, &'static str)> {
|
||||
let eval = if opts.honor_safe_lists {
|
||||
evaluate_bash_segments(cmd, state)
|
||||
} else {
|
||||
evaluate_bash_segments_inner(cmd, state, false)
|
||||
};
|
||||
match eval {
|
||||
SegmentEvaluation::Reject(reason) => {
|
||||
Some((Decision::Reject(reason), reasons::SESSION_DENY))
|
||||
}
|
||||
SegmentEvaluation::AutoAllow { via_session_grant } => grant_allow(if via_session_grant {
|
||||
reasons::SESSION_GRANT
|
||||
} else {
|
||||
reasons::SAFE_COMMAND
|
||||
}),
|
||||
SegmentEvaluation::NeedsPrompts { any_dangerous, .. } => {
|
||||
if !opts.allow_blanket {
|
||||
return None;
|
||||
}
|
||||
if any_dangerous && opts.conservative_blanket {
|
||||
return None;
|
||||
}
|
||||
if persisted_bash_auto_allows(state, cmd, yolo_pin) {
|
||||
grant_allow(reasons::SESSION_GRANT)
|
||||
} else {
|
||||
if let SegmentEvaluation::Reject(reason) = &evaluation.segments {
|
||||
return Some((Decision::Reject(reason.to_owned()), reasons::SESSION_DENY));
|
||||
}
|
||||
if bash_request_floor_requires_prompt(Some(evaluation)) {
|
||||
return None;
|
||||
}
|
||||
match &evaluation.segments {
|
||||
SegmentEvaluation::Reject(_) => unreachable!(),
|
||||
SegmentEvaluation::AutoAllow { via_session_grant } => {
|
||||
if !opts.honor_safe_lists && !evaluation.all_segments_granted {
|
||||
None
|
||||
} else {
|
||||
grant_allow(if *via_session_grant {
|
||||
reasons::SESSION_GRANT
|
||||
} else {
|
||||
reasons::SAFE_COMMAND
|
||||
})
|
||||
}
|
||||
}
|
||||
SegmentEvaluation::NeedsPrompts { any_dangerous, .. } => {
|
||||
if !opts.allow_blanket || (*any_dangerous && opts.conservative_blanket) {
|
||||
None
|
||||
} else {
|
||||
persisted_bash_auto_allows(state, cmd, yolo_pin)
|
||||
.then_some((Decision::Allow, reasons::SESSION_GRANT))
|
||||
}
|
||||
}
|
||||
SegmentEvaluation::Unparseable => {
|
||||
if !opts.allow_blanket {
|
||||
return None;
|
||||
}
|
||||
// Conservative path: approve-all must not cover undecomposable scripts.
|
||||
let allowed = if opts.conservative_blanket {
|
||||
state.allowed_bash_commands.contains(cmd)
|
||||
} else {
|
||||
persisted_bash_auto_allows(state, cmd, yolo_pin)
|
||||
};
|
||||
if allowed {
|
||||
grant_allow(reasons::SESSION_GRANT)
|
||||
} else {
|
||||
None
|
||||
} else {
|
||||
let allowed = if opts.conservative_blanket {
|
||||
evaluation.exact_grant
|
||||
} else {
|
||||
persisted_bash_auto_allows(state, cmd, yolo_pin)
|
||||
};
|
||||
allowed.then_some((Decision::Allow, reasons::SESSION_GRANT))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -787,6 +841,7 @@ fn bash_grant_pre_decision(
|
|||
/// Caller must skip under policy/shell Ask floors.
|
||||
fn session_grant_pre_decision(
|
||||
access: &AccessKind,
|
||||
bash_evaluation: Option<&BashEvaluation>,
|
||||
state: &PermissionState,
|
||||
allow_edits_for_session: bool,
|
||||
static_domain_matcher: &DomainMatcher,
|
||||
|
|
@ -811,9 +866,13 @@ fn session_grant_pre_decision(
|
|||
}
|
||||
}
|
||||
AccessKind::Edit(_) if allow_edits_for_session => grant_allow(reasons::SESSION_GRANT),
|
||||
AccessKind::Bash(cmd) => {
|
||||
bash_grant_pre_decision(cmd, state, yolo_pin, BashGrantOpts::PRE_CLASSIFIER)
|
||||
}
|
||||
AccessKind::Bash(cmd) => bash_grant_pre_decision(
|
||||
cmd,
|
||||
bash_evaluation?,
|
||||
state,
|
||||
yolo_pin,
|
||||
BashGrantOpts::PRE_CLASSIFIER,
|
||||
),
|
||||
AccessKind::Read(_)
|
||||
| AccessKind::Grep { .. }
|
||||
| AccessKind::WebSearch(_)
|
||||
|
|
@ -1150,6 +1209,11 @@ fn spawn_permission_manager_with_pin(
|
|||
continue;
|
||||
}
|
||||
|
||||
let bash_evaluation = match &access {
|
||||
AccessKind::Bash(cmd) => Some(evaluate_bash(cmd, &state, true)),
|
||||
_ => None,
|
||||
};
|
||||
|
||||
// Evaluate managed policy (direct access + per-segment Bash command
|
||||
// rules + Bash shell-file args) up front so the YOLO/sandbox fast
|
||||
// paths below honor a deny or forced prompt.
|
||||
|
|
@ -1212,6 +1276,7 @@ fn spawn_permission_manager_with_pin(
|
|||
&& !shell_forced_prompt
|
||||
&& let Some((decision, reason)) = session_grant_pre_decision(
|
||||
&access,
|
||||
bash_evaluation.as_ref(),
|
||||
&state,
|
||||
allow_edits_for_session,
|
||||
&static_domain_matcher,
|
||||
|
|
@ -1232,8 +1297,12 @@ fn spawn_permission_manager_with_pin(
|
|||
// Policy deny already handled; forced Ask falls through unless
|
||||
// fast-path/classifier allows. Policy Ask still prompts below
|
||||
// unless auto fast-path/classifier decides first for non-forced
|
||||
// paths — we skip auto entirely when policy_forced_prompt.
|
||||
if auto_mode && !policy_forced_prompt && !shell_forced_prompt {
|
||||
// paths; policy and Bash request floors skip auto entirely.
|
||||
if auto_mode
|
||||
&& !policy_forced_prompt
|
||||
&& !shell_forced_prompt
|
||||
&& !bash_request_floor_requires_prompt(bash_evaluation.as_ref())
|
||||
{
|
||||
use crate::permission::auto_mode::{
|
||||
AutoFastPath, ClassifierVerdict, access_requires_user_interaction,
|
||||
auto_mode_fast_path,
|
||||
|
|
@ -1331,7 +1400,10 @@ fn spawn_permission_manager_with_pin(
|
|||
}
|
||||
|
||||
if matches!(&access, AccessKind::Bash(_))
|
||||
&& xai_grok_sandbox::should_auto_allow_bash()
|
||||
&& sandbox_may_auto_allow_bash(
|
||||
bash_evaluation.as_ref(),
|
||||
xai_grok_sandbox::should_auto_allow_bash(),
|
||||
)
|
||||
&& !policy_forced_prompt
|
||||
&& !auto_forced_prompt
|
||||
{
|
||||
|
|
@ -1348,8 +1420,8 @@ fn spawn_permission_manager_with_pin(
|
|||
// `policy_forced_prompt` is consumed by the MCP arm of the
|
||||
// pre-decision match: a policy `Ask` rule on an MCP tool
|
||||
// overrides the session allowlist and forces a re-prompt.
|
||||
// Other access kinds (Bash / Edit / WebFetch) keep their
|
||||
// legacy fall-through behavior.
|
||||
// Other access kinds keep their legacy fall-through behavior,
|
||||
// subject to Bash request floors.
|
||||
match policy_decision {
|
||||
Some(Decision::Ask) => {
|
||||
tracing::info!(
|
||||
|
|
@ -1358,6 +1430,15 @@ fn spawn_permission_manager_with_pin(
|
|||
"permission policy: ask rule matched, prompting user"
|
||||
);
|
||||
}
|
||||
Some(Decision::Allow)
|
||||
if bash_request_floor_requires_prompt(bash_evaluation.as_ref()) =>
|
||||
{
|
||||
tracing::info!(
|
||||
tool = ?tool_name,
|
||||
source = "policy",
|
||||
"permission policy allow deferred to Bash prompt floor"
|
||||
);
|
||||
}
|
||||
Some(decision) => {
|
||||
tracing::info!(
|
||||
tool = ?tool_name,
|
||||
|
|
@ -1426,7 +1507,9 @@ fn spawn_permission_manager_with_pin(
|
|||
}
|
||||
}
|
||||
AccessKind::Bash(cmd) => {
|
||||
if policy_forced_prompt {
|
||||
if bash_request_floor_requires_prompt(bash_evaluation.as_ref()) {
|
||||
None
|
||||
} else if policy_forced_prompt {
|
||||
// Ask floor: only explicit grants with remember on.
|
||||
// `!shell_file_forced_prompt` blocks bash grants from
|
||||
// satisfying a Read/Edit ask escalated from shell-file access.
|
||||
|
|
@ -1436,6 +1519,9 @@ fn spawn_permission_manager_with_pin(
|
|||
{
|
||||
bash_grant_pre_decision(
|
||||
cmd,
|
||||
bash_evaluation
|
||||
.as_ref()
|
||||
.expect("Bash access has evaluation"),
|
||||
&state,
|
||||
yolo_pin,
|
||||
BashGrantOpts::ASK_FLOOR_REMEMBER,
|
||||
|
|
@ -1446,6 +1532,9 @@ fn spawn_permission_manager_with_pin(
|
|||
} else {
|
||||
bash_grant_pre_decision(
|
||||
cmd,
|
||||
bash_evaluation
|
||||
.as_ref()
|
||||
.expect("Bash access has evaluation"),
|
||||
&state,
|
||||
yolo_pin,
|
||||
BashGrantOpts::post_classify(auto_forced_prompt),
|
||||
|
|
@ -1803,6 +1892,10 @@ mod tests {
|
|||
// ── Managed-policy pin: yolo clamp + persisted bash clamp ──
|
||||
|
||||
const PIN: &str = crate::permission::resolution::YOLO_PIN_REASON_REQUIREMENTS;
|
||||
const UNSAFE_GIT_STATUS: &str = concat!(
|
||||
"GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.fsmonitor ",
|
||||
"GIT_CONFIG_VALUE_0=/tmp/pwn git status"
|
||||
);
|
||||
|
||||
#[test]
|
||||
fn clamp_yolo_respects_pin() {
|
||||
|
|
@ -3196,6 +3289,67 @@ mod tests {
|
|||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sourced_script_prompts_once_in_ask_mode() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let cwd = AbsPathBuf::new(tmp.path().to_path_buf()).unwrap();
|
||||
let client = RecordingClient::default();
|
||||
let prompts = client.prompts.clone();
|
||||
let (mgr, _e) =
|
||||
manager_with_recording_client(&cwd, None, client, ClientType::Generic);
|
||||
|
||||
let d = tokio::time::timeout(
|
||||
std::time::Duration::from_secs(5),
|
||||
mgr.request(
|
||||
AccessKind::Bash("source ./setup.sh".into()),
|
||||
tool_call(),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
),
|
||||
)
|
||||
.await
|
||||
.expect("permission request must resolve, not hang");
|
||||
|
||||
assert_eq!(prompts.borrow().len(), 1, "sourced script must prompt once");
|
||||
assert!(matches!(d, Decision::Reject(_)), "got {d:?}");
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn sourced_script_dont_ask_denies_without_prompt() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let cwd = AbsPathBuf::new(tmp.path().to_path_buf()).unwrap();
|
||||
let mut config = crate::permission::types::PermissionConfig::new(vec![]);
|
||||
config.prompt_policy = PromptPolicy::Deny;
|
||||
let client = RecordingClient::default();
|
||||
let prompts = client.prompts.clone();
|
||||
let (mgr, _e) =
|
||||
manager_with_recording_client(&cwd, Some(config), client, ClientType::Generic);
|
||||
|
||||
let d = mgr
|
||||
.request(
|
||||
AccessKind::Bash("source ./setup.sh".into()),
|
||||
tool_call(),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(matches!(d, Decision::PolicyDeny(_)), "got {d:?}");
|
||||
assert!(prompts.borrow().is_empty(), "dontAsk must not prompt");
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
/// Chained unsafe segments must produce **one** permission prompt for the
|
||||
/// full script, not one prompt per segment. `evaluate_bash_segments` still
|
||||
/// decomposes for auto-allow/reject, but the interactive path no longer
|
||||
|
|
@ -3235,9 +3389,122 @@ mod tests {
|
|||
.await;
|
||||
}
|
||||
|
||||
/// Negative direction: with no policy rule, the same bash-safe `ls`
|
||||
/// auto-allows via bash-safety and is never prompted — proving the fix did
|
||||
/// not over-correct into prompting on plain auto-allowed commands.
|
||||
async fn run_bash_request(cmd: &str, policy: PromptPolicy) -> (Decision, usize) {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let cwd = AbsPathBuf::new(tmp.path().to_path_buf()).unwrap();
|
||||
let client = RecordingClient::default();
|
||||
let prompts = client.prompts.clone();
|
||||
let mut config = crate::permission::types::PermissionConfig::new(vec![]);
|
||||
config.prompt_policy = policy;
|
||||
let (mgr, _events) =
|
||||
manager_with_recording_client(&cwd, Some(config), client, ClientType::Generic);
|
||||
let decision = mgr
|
||||
.request(AccessKind::Bash(cmd.into()), tool_call(), None, None, None)
|
||||
.await;
|
||||
let count = prompts.borrow().len();
|
||||
(decision, count)
|
||||
}
|
||||
|
||||
async fn run_write_request(policy: PromptPolicy) -> (Decision, usize) {
|
||||
run_bash_request("cat payload > out", policy).await
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn real_file_write_prompts_once() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let (decision, prompts) = run_write_request(PromptPolicy::Ask).await;
|
||||
assert!(matches!(decision, Decision::Reject(_)));
|
||||
assert_eq!(prompts, 1);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn configured_bash_allow_does_not_cross_write_floor() {
|
||||
use crate::permission::types::{PatternMode, PermissionRule, RuleAction, ToolFilter};
|
||||
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let cwd = AbsPathBuf::new(tmp.path().to_path_buf()).unwrap();
|
||||
let config =
|
||||
crate::permission::types::PermissionConfig::new(vec![PermissionRule {
|
||||
action: RuleAction::Allow,
|
||||
tool: ToolFilter::Bash,
|
||||
pattern: Some("*".to_owned()),
|
||||
pattern_mode: PatternMode::Glob,
|
||||
}]);
|
||||
let client = RecordingClient::default();
|
||||
let prompts = client.prompts.clone();
|
||||
let (mgr, _events) =
|
||||
manager_with_recording_client(&cwd, Some(config), client, ClientType::Generic);
|
||||
for cmd in ["cat payload > out", UNSAFE_GIT_STATUS] {
|
||||
let decision = mgr
|
||||
.request(AccessKind::Bash(cmd.into()), tool_call(), None, None, None)
|
||||
.await;
|
||||
assert!(matches!(decision, Decision::Reject(_)), "{cmd}");
|
||||
}
|
||||
assert_eq!(prompts.borrow().len(), 2);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn real_file_write_dont_ask_rejects_without_prompt() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let (decision, prompts) = run_write_request(PromptPolicy::Deny).await;
|
||||
assert!(matches!(decision, Decision::PolicyDeny(_)));
|
||||
assert_eq!(prompts, 0);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn unsafe_environment_ask_and_dont_ask() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
local
|
||||
.run_until(async {
|
||||
let (decision, prompts) =
|
||||
run_bash_request(UNSAFE_GIT_STATUS, PromptPolicy::Ask).await;
|
||||
assert!(matches!(decision, Decision::Reject(_)));
|
||||
assert_eq!(prompts, 1);
|
||||
|
||||
let (decision, prompts) =
|
||||
run_bash_request(UNSAFE_GIT_STATUS, PromptPolicy::Deny).await;
|
||||
assert!(matches!(decision, Decision::PolicyDeny(_)));
|
||||
assert_eq!(prompts, 0);
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandbox_auto_allow_respects_real_file_write_floor() {
|
||||
let state = PermissionState::default();
|
||||
for cmd in ["cat payload > out", UNSAFE_GIT_STATUS] {
|
||||
assert!(!sandbox_may_auto_allow_bash(
|
||||
Some(&evaluate_bash(cmd, &state, true)),
|
||||
true,
|
||||
));
|
||||
}
|
||||
for cmd in [
|
||||
"cargo build > /dev/null",
|
||||
"cargo build 2>&1",
|
||||
"RUST_LOG=debug git status",
|
||||
] {
|
||||
assert!(
|
||||
sandbox_may_auto_allow_bash(Some(&evaluate_bash(cmd, &state, true)), true),
|
||||
"sandbox control: {cmd}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Negative direction: with no policy rule, bash-safe `ls` auto-allows
|
||||
/// without a prompt.
|
||||
#[tokio::test]
|
||||
async fn bash_safe_command_without_policy_auto_allows_without_prompt() {
|
||||
let local = tokio::task::LocalSet::new();
|
||||
|
|
@ -4434,6 +4701,29 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluate_sourced_scripts_need_prompt() {
|
||||
let state = PermissionState::default();
|
||||
for (cmd, expected) in [
|
||||
("source ./setup.sh", "source ./setup.sh"),
|
||||
(". ./setup.sh", ". ./setup.sh"),
|
||||
("cd repo && source ./setup.sh", "source ./setup.sh"),
|
||||
("timeout 5 source ./setup.sh", "source ./setup.sh"),
|
||||
] {
|
||||
match evaluate_bash_segments(cmd, &state) {
|
||||
SegmentEvaluation::NeedsPrompts { segments, .. } => {
|
||||
assert_eq!(segments, vec![expected.to_owned()]);
|
||||
}
|
||||
other => panic!("expected NeedsPrompts for `{cmd}`, got {other:?}"),
|
||||
}
|
||||
}
|
||||
|
||||
assert!(matches!(
|
||||
evaluate_bash_segments("cd repo && git status", &state),
|
||||
SegmentEvaluation::AutoAllow { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluate_all_safe_chain_auto_allows() {
|
||||
let state = PermissionState::default();
|
||||
|
|
@ -4456,6 +4746,152 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn real_file_writes_need_prompt() {
|
||||
let state = PermissionState::default();
|
||||
for cmd in [
|
||||
"cat payload > ~/.zshrc",
|
||||
"cat payload >> out",
|
||||
"sort -o out input",
|
||||
"cat payload > 3",
|
||||
"> out",
|
||||
] {
|
||||
assert!(
|
||||
evaluate_bash(cmd, &state, true).writes_real_file,
|
||||
"real-file write must set the floor: {cmd}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsafe_environment_detection_covers_script_forms() {
|
||||
let state = PermissionState::default();
|
||||
for (cmd, unsafe_env) in [
|
||||
(UNSAFE_GIT_STATUS, true),
|
||||
(
|
||||
concat!(
|
||||
"env GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=core.fsmonitor ",
|
||||
"GIT_CONFIG_VALUE_0=/tmp/pwn git status"
|
||||
),
|
||||
true,
|
||||
),
|
||||
(
|
||||
concat!(
|
||||
"set -a; GIT_CONFIG_COUNT=1; GIT_CONFIG_KEY_0=core.fsmonitor; ",
|
||||
"GIT_CONFIG_VALUE_0=/tmp/pwn; git status"
|
||||
),
|
||||
true,
|
||||
),
|
||||
("RUST_LOG=debug git status", false),
|
||||
] {
|
||||
let evaluation = evaluate_bash(cmd, &state, true);
|
||||
assert_eq!(evaluation.sets_unsafe_env, unsafe_env, "{cmd}");
|
||||
assert_eq!(
|
||||
bash_unsafe_env_floor_requires_prompt(Some(&evaluation)),
|
||||
unsafe_env,
|
||||
"{cmd}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsafe_env_floor_blocks_broad_grants_but_preserves_exact_decisions() {
|
||||
let cmd = UNSAFE_GIT_STATUS;
|
||||
for (grants, blanket, allowed) in [
|
||||
(vec!["git status"], false, false),
|
||||
(vec![], true, false),
|
||||
(vec![cmd], false, true),
|
||||
] {
|
||||
let state = PermissionState {
|
||||
allowed_bash_commands: grants.into_iter().map(str::to_owned).collect(),
|
||||
allow_bash_execute: blanket,
|
||||
..Default::default()
|
||||
};
|
||||
let evaluation = evaluate_bash(cmd, &state, true);
|
||||
assert!(evaluation.sets_unsafe_env);
|
||||
assert_eq!(
|
||||
bash_grant_pre_decision(
|
||||
cmd,
|
||||
&evaluation,
|
||||
&state,
|
||||
None,
|
||||
BashGrantOpts::PRE_CLASSIFIER,
|
||||
)
|
||||
.is_some(),
|
||||
allowed
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_floor_preserves_sinks_fd_dups_and_exact_decisions() {
|
||||
let state = PermissionState::default();
|
||||
for cmd in ["grep text file 2>/dev/null", "cargo check 2>&1"] {
|
||||
assert!(!evaluate_bash(cmd, &state, true).writes_real_file);
|
||||
}
|
||||
|
||||
let cmd = "cat payload > another-file";
|
||||
for (state, allowed) in [
|
||||
(
|
||||
PermissionState {
|
||||
allowed_bash_commands: HashSet::from(["cat".to_owned()]),
|
||||
..Default::default()
|
||||
},
|
||||
false,
|
||||
),
|
||||
(
|
||||
PermissionState {
|
||||
allow_bash_execute: true,
|
||||
..Default::default()
|
||||
},
|
||||
false,
|
||||
),
|
||||
(
|
||||
PermissionState {
|
||||
allowed_bash_commands: HashSet::from([cmd.to_owned()]),
|
||||
..Default::default()
|
||||
},
|
||||
true,
|
||||
),
|
||||
] {
|
||||
let evaluation = evaluate_bash(cmd, &state, true);
|
||||
assert_eq!(
|
||||
bash_grant_pre_decision(
|
||||
cmd,
|
||||
&evaluation,
|
||||
&state,
|
||||
None,
|
||||
BashGrantOpts::PRE_CLASSIFIER,
|
||||
)
|
||||
.is_some(),
|
||||
allowed
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ask_floor_requires_every_segment_to_be_granted() {
|
||||
let cmd = "cat README && git status";
|
||||
for (grants, allowed) in [(["cat", "unused"], false), (["cat", "git status"], true)] {
|
||||
let state = PermissionState {
|
||||
allowed_bash_commands: grants.into_iter().map(str::to_owned).collect(),
|
||||
..Default::default()
|
||||
};
|
||||
let evaluation = evaluate_bash(cmd, &state, true);
|
||||
assert_eq!(
|
||||
bash_grant_pre_decision(
|
||||
cmd,
|
||||
&evaluation,
|
||||
&state,
|
||||
None,
|
||||
BashGrantOpts::ASK_FLOOR_REMEMBER,
|
||||
)
|
||||
.is_some(),
|
||||
allowed
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluate_inner_without_safe_lists_ignores_builtin_safe_commands() {
|
||||
// `honor_safe_lists = false` (the `ask`-floor escape mode): a built-in
|
||||
|
|
|
|||
|
|
@ -225,6 +225,11 @@ pub(crate) fn command_write_paths_in_tree(root: Node<'_>, src: &str) -> Vec<Stri
|
|||
out
|
||||
}
|
||||
|
||||
/// Safe write sinks that do not touch a real file. Exact match.
|
||||
pub(crate) fn is_safe_write_sink(path: &str) -> bool {
|
||||
matches!(path, "/dev/null" | "/dev/stdout" | "/dev/stderr")
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
pub(crate) enum ShellFileMode {
|
||||
Read,
|
||||
|
|
@ -432,28 +437,32 @@ pub(crate) fn shell_redirect_targets(
|
|||
}
|
||||
|
||||
fn shell_redirect_one(node: Node<'_>, src: &str) -> Option<(Option<String>, ShellFileMode, bool)> {
|
||||
let mut mode = None;
|
||||
let mut redirect = None;
|
||||
for i in 0..node.child_count() {
|
||||
let kind = node.child(i)?.kind();
|
||||
// `<<`/`<<<` read from inline text, not a file.
|
||||
if kind.contains("<<") {
|
||||
return None;
|
||||
}
|
||||
if kind.contains('>') {
|
||||
mode = Some(ShellFileMode::Write);
|
||||
break;
|
||||
}
|
||||
if kind.contains('<') {
|
||||
mode = Some(ShellFileMode::Read);
|
||||
if kind.contains('>') || kind.contains('<') {
|
||||
redirect = Some(kind);
|
||||
break;
|
||||
}
|
||||
}
|
||||
let mode = mode?;
|
||||
let redirect = redirect?;
|
||||
let mode = if redirect.contains('>') {
|
||||
ShellFileMode::Write
|
||||
} else {
|
||||
ShellFileMode::Read
|
||||
};
|
||||
let duplicates_fd = matches!(redirect, ">&" | "<&");
|
||||
let dest = node.child_by_field_name("destination")?;
|
||||
match shell_node_arg(dest, src)? {
|
||||
ArgText::Literal(s) => {
|
||||
// Skip fd duplications (`>&1`) and empty targets.
|
||||
if s.is_empty() || s.starts_with('&') || s.bytes().all(|b| b.is_ascii_digit()) {
|
||||
if s.is_empty()
|
||||
|| s.starts_with('&')
|
||||
|| (duplicates_fd && (s == "-" || s.bytes().all(|b| b.is_ascii_digit())))
|
||||
{
|
||||
None
|
||||
} else {
|
||||
let ambiguous = shell_arg_is_ambiguous(&s);
|
||||
|
|
@ -1485,6 +1494,18 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn adversarial_fd_duplication_and_numeric_filenames() {
|
||||
let parsed = |cmd: &str| {
|
||||
let tree = try_parse_shell(cmd).expect("shell parses");
|
||||
command_write_paths_in_tree(tree.root_node(), cmd)
|
||||
};
|
||||
assert!(parsed("cat payload 2>&1").is_empty());
|
||||
assert!(parsed("cat payload 1>&-").is_empty());
|
||||
assert!(parsed("cat payload 0<&3").is_empty());
|
||||
assert_eq!(parsed("cat payload > 3"), vec!["3"]);
|
||||
}
|
||||
|
||||
/// An outer reader fed a substitution can't pin its operand (Ask); an inner
|
||||
/// literal read (incl. inside `<(…)`) is a hard deny.
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -57,6 +57,13 @@ pub struct WorkspaceSession {
|
|||
pub(crate) depth: u32,
|
||||
pub(crate) fork_budget: u32,
|
||||
pub(crate) hunk_tracker: HunkTrackerHandle,
|
||||
/// Cancel token for the workspace-spawned [`HunkTrackerActor`] backing
|
||||
/// [`Self::hunk_tracker`], fired on session teardown by
|
||||
/// [`Self::cancel_hunk_tracker`]. `None` when the tracker is externally
|
||||
/// owned (e.g. `create_session_with_tracker` / local shell mode).
|
||||
///
|
||||
/// [`HunkTrackerActor`]: xai_hunk_tracker::HunkTrackerActor
|
||||
pub(crate) hunk_tracker_cancel: Option<tokio_util::sync::CancellationToken>,
|
||||
pub(crate) file_state_tracker: Arc<FileStateTracker>,
|
||||
/// Per-turn hunk deltas keyed by `prompt_index`, captured at finalize and
|
||||
/// replayed on rewind (only when `workspace_rewind_hunks` is on). The live
|
||||
|
|
@ -154,6 +161,7 @@ impl WorkspaceSession {
|
|||
toolset: Arc<FinalizedToolset>,
|
||||
terminal_backend: crate::config::SessionTerminalBackend,
|
||||
hunk_tracker: HunkTrackerHandle,
|
||||
hunk_tracker_cancel: Option<tokio_util::sync::CancellationToken>,
|
||||
viewer_ctx: Option<WorkspaceViewerContext>,
|
||||
#[allow(dead_code)] system_notifications: bool,
|
||||
system_notify_channel: Option<(
|
||||
|
|
@ -177,6 +185,7 @@ impl WorkspaceSession {
|
|||
depth,
|
||||
fork_budget,
|
||||
hunk_tracker,
|
||||
hunk_tracker_cancel,
|
||||
file_state_tracker,
|
||||
hunk_checkpoints: Arc::new(tokio::sync::Mutex::new(HashMap::new())),
|
||||
git_checkpoints: crate::session::git::GitCheckpointStore::new(),
|
||||
|
|
@ -299,6 +308,15 @@ impl WorkspaceSession {
|
|||
pub(crate) fn shutdown_terminal_backend(&self) {
|
||||
self.terminal_backend.shutdown();
|
||||
}
|
||||
/// Cancel the workspace-spawned hunk-tracker actor, if this session owns
|
||||
/// one. Runs at the session drop chokepoints so the actor (which pins file
|
||||
/// contents in `file_states`) stops even while leaked handle clones hold
|
||||
/// its channel open.
|
||||
pub(crate) fn cancel_hunk_tracker(&self) {
|
||||
if let Some(token) = &self.hunk_tracker_cancel {
|
||||
token.cancel();
|
||||
}
|
||||
}
|
||||
/// Return the current resolved toolset (snapshot).
|
||||
pub fn toolset(&self) -> Arc<FinalizedToolset> {
|
||||
self.inner.read().toolset.clone()
|
||||
|
|
|
|||
|
|
@ -867,14 +867,17 @@ fn hook_registry_to_wire(
|
|||
serde_json::to_value(registry).map_err(|e| WorkspaceError::HubError(e.to_string()))?;
|
||||
serde_json::from_value(value).map_err(|e| WorkspaceError::HubError(e.to_string()))
|
||||
}
|
||||
/// Inverse of [`hook_registry_to_wire`]. The compiled `matcher` is absent from
|
||||
/// the wire (and from this result); callers recompile it via
|
||||
/// `HookRegistry::recompile_matchers`, exactly as the proxy path already did.
|
||||
/// Inverse of [`hook_registry_to_wire`]. Rebuilds compiled matchers via
|
||||
/// [`HookRegistry::recompile_matchers`] so invalid patterns fail closed
|
||||
/// (match nothing) rather than widening to match-all after the wire hop.
|
||||
fn wire_to_hook_registry(
|
||||
wire: &HookRegistryWire,
|
||||
) -> WorkspaceResult<xai_grok_hooks::discovery::HookRegistry> {
|
||||
let value = serde_json::to_value(wire).map_err(|e| WorkspaceError::HubError(e.to_string()))?;
|
||||
serde_json::from_value(value).map_err(|e| WorkspaceError::HubError(e.to_string()))
|
||||
let mut registry: xai_grok_hooks::discovery::HookRegistry =
|
||||
serde_json::from_value(value).map_err(|e| WorkspaceError::HubError(e.to_string()))?;
|
||||
registry.recompile_matchers();
|
||||
Ok(registry)
|
||||
}
|
||||
#[async_trait]
|
||||
impl WorkspaceOp for HookRegistryReq {
|
||||
|
|
|
|||
Loading…
Reference in a new issue