fix(permissions): rg exec flags cannot ride an allowlist, and argv is decoded
Port of upstream grok-build 3794978 (snap6 "Reject ripgrep --hostname-bin
on always-safe and Auto routine paths"), widened where review showed the
same hole one layer down.
- `rg --pre` / `--hostname-bin` execute a program per file. The safe
list refused only `--pre`, and the Auto-mode routine classifier had
no rg flag check at all. One shared `rg_has_unsafe_flag` now refuses
both flags (bare or `=VALUE`) on a lowercased-basename head, scanning
every argument: rg's grammar (`--replace -- --pre`, `-ne --`) makes a
partial arity model bypassable, so a literal search for the flag name
costs a prompt instead of the auto-allow.
- The shell splitter handed allowlists undecoded source: a concatenation
(`--host""name-bin`, `''--hostname-bin`, `--p're'`) kept its quotes,
and double-quoted escapes (`"--host\<newline>name-bin"`) survived.
`PlainCommand` words are now the decoded argv (bash double-quote
rules, whole-node text minus the quotes so literal newlines and the
whitespace tree-sitter folds into the closing quote survive), with a
per-word `expandable` flag for unquoted glob/brace/tilde/backslash
syntax the shell would still expand (`--{pre,pre}`, a glob that can
match a file named `--pre`); the rg guard treats those as unsafe.
- Every prefix-based decision matches per argv element through one
matcher (built-in safe list, always-safe list, dangerous list, Auto
routine prefixes): a decoded word with inner whitespace (`rg `,
`rg /../payload`, one word spelled `git status`) can no longer pose
as a prefix. Saved grants match token for word (a grant saved from a
spaced argument prompts again). Saved denies keep the pre-0.1.15
check verbatim on a `legacy_words` spelling (raw content and raw
concatenations, wrappers peeled independently, space-boundary prefix)
so no deny narrows on upgrade.
Claude-Session: https://claude.ai/code/session_01RUqV8HVGXcAETyYbZ2tPQ4
This commit is contained in:
1 parent
aa9a5dbe72
commit
fb3fe89d33
3 files changed
+525
-129
No files matched your search
@@ -11,7 +11,7 @@ use std::sync::Arc;
|
||||
use tree_sitter::Node;
|
||||
|
||||
use super::bash_command_splitting::{
|
||||
PlainCommand, is_wrapper_command, strip_wrapper_command, try_parse_shell,
|
||||
PlainCommand, is_wrapper_command, matches_prefix_words, 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};
|
||||
@@ -424,7 +424,11 @@ fn classify_bash(cmd: &str) -> ClassifierVerdict {
|
||||
// (`cargo`/`make`/`pytest`/`python`/`node`, `npm test`/`run`, `uv run
|
||||
// <routine>`) execute project-controlled code; this heuristic is a fail-closed
|
||||
// FALLBACK and the real safety boundary is the LLM side-query + managed policy.
|
||||
if !cmds.is_empty() && cmds.iter().all(|c| bash_command_is_routine(c.words())) {
|
||||
if !cmds.is_empty()
|
||||
&& cmds
|
||||
.iter()
|
||||
.all(|c| bash_command_is_routine(c.words(), c.expandable()))
|
||||
{
|
||||
return ClassifierVerdict::Allow;
|
||||
}
|
||||
ClassifierVerdict::Block
|
||||
@@ -438,10 +442,11 @@ fn classify_bash(cmd: &str) -> ClassifierVerdict {
|
||||
/// classified by [`package_manager_subcommand_is_routine`]: a fail-closed
|
||||
/// SAFE-subcommand allowlist (build/test/dep-management Allow; explicit launchers
|
||||
/// re-classified; remote / arbitrary-exec / unknown → Block).
|
||||
fn bash_command_is_routine(words: &[String]) -> bool {
|
||||
fn bash_command_is_routine(words: &[String], expandable: &[bool]) -> bool {
|
||||
// Peel canonical (quote-aware) wrappers: env [NAME=VALUE], timeout, nice,
|
||||
// stdbuf, ionice, chrt (incl. path-qualified).
|
||||
let inner = unwrap_wrappers(words);
|
||||
let inner_expandable = &expandable[words.len() - inner.len()..];
|
||||
// A bare wrapper (e.g. `env` printing the environment) or a command that was
|
||||
// only env assignments → routine.
|
||||
if inner.is_empty() || is_lone_wrapper(inner) {
|
||||
@@ -454,7 +459,7 @@ fn bash_command_is_routine(words: &[String]) -> bool {
|
||||
.to_ascii_lowercase();
|
||||
// Package managers: fail-closed safe-subcommand allowlist (None = not a
|
||||
// package manager → fall through to the generic find/prefix checks).
|
||||
if let Some(routine) = package_manager_subcommand_is_routine(&head, inner) {
|
||||
if let Some(routine) = package_manager_subcommand_is_routine(&head, inner, inner_expandable) {
|
||||
return routine;
|
||||
}
|
||||
// `find` is routine only without a filesystem-mutating primary.
|
||||
@@ -490,6 +495,10 @@ fn bash_command_is_routine(words: &[String]) -> bool {
|
||||
if head == "gh" {
|
||||
return gh_subcommand_is_read_only(inner);
|
||||
}
|
||||
// `rg --pre`/`--hostname-bin` execute a program per file; never routine.
|
||||
if head == "rg" && super::manager::rg_has_unsafe_flag(inner, inner_expandable) {
|
||||
return false;
|
||||
}
|
||||
// Branch switches and stash workflow are routine; a path operand turns
|
||||
// `checkout` into a working-tree discard, so those forms fail closed.
|
||||
if head == "git" {
|
||||
@@ -500,11 +509,9 @@ fn bash_command_is_routine(words: &[String]) -> bool {
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
let joined = inner.join(" ").to_ascii_lowercase();
|
||||
ROUTINE_PREFIXES.iter().any(|p| {
|
||||
let base = p.trim();
|
||||
joined == base || (joined.starts_with(base) && joined[base.len()..].starts_with(' '))
|
||||
})
|
||||
ROUTINE_PREFIXES
|
||||
.iter()
|
||||
.any(|p| matches_prefix_words(inner, p, true))
|
||||
}
|
||||
|
||||
const BRANCH_SWITCH_BENIGN_FLAGS: &[&str] = &[
|
||||
@@ -650,7 +657,11 @@ fn gh_subcommand_is_read_only(inner: &[String]) -> bool {
|
||||
/// fetch-and-run via [`is_remote_launcher`], explicit launchers via
|
||||
/// [`explicit_launch_target`] (which re-classifies the inner command after
|
||||
/// re-checking its writes/env), and everything else against a per-tool allowlist.
|
||||
fn package_manager_subcommand_is_routine(prog: &str, inner: &[String]) -> Option<bool> {
|
||||
fn package_manager_subcommand_is_routine(
|
||||
prog: &str,
|
||||
inner: &[String],
|
||||
expandable: &[bool],
|
||||
) -> Option<bool> {
|
||||
if !matches!(
|
||||
prog,
|
||||
"uv" | "uvx" | "npm" | "npx" | "pnpm" | "yarn" | "rustup"
|
||||
@@ -672,7 +683,10 @@ fn package_manager_subcommand_is_routine(prog: &str, inner: &[String]) -> Option
|
||||
return Some(
|
||||
!command_env_is_unsafe(launched)
|
||||
&& !launched_writes_nonsink(launched)
|
||||
&& bash_command_is_routine(launched),
|
||||
&& bash_command_is_routine(
|
||||
launched,
|
||||
&expandable[inner.len() - launched.len()..],
|
||||
),
|
||||
);
|
||||
}
|
||||
LaunchTarget::NotLauncher => {}
|
||||
@@ -2495,6 +2509,46 @@ mod tests {
|
||||
] {
|
||||
assert_eq!(v(cmd), ClassifierVerdict::Block, "`{cmd}` must block");
|
||||
}
|
||||
for cmd in [
|
||||
"rg --pre ./pre.sh TODO .",
|
||||
"rg --pre=cat TODO .",
|
||||
"rg --hostname-bin=./payload needle",
|
||||
"rg --hostname-bin ./payload needle",
|
||||
"env rg --hostname-bin ./payload needle",
|
||||
"RG --hostname-bin ./payload needle",
|
||||
"timeout 5 RG --pre ./payload needle .",
|
||||
"/usr/bin/rg --pre=cat needle .",
|
||||
"rg --replace -- --pre ./payload needle .",
|
||||
"rg --replace -e --hostname-bin ./payload needle .",
|
||||
"rg -ne -- --hostname-bin ./payload .",
|
||||
"rg -- --hostname-bin .",
|
||||
"rg --host\"\"name-bin ./payload needle .",
|
||||
"rg --p're' ./payload needle .",
|
||||
"rg --{hostname-bin,hostname-bin}=./payload needle file.rs",
|
||||
"rg --{pre,pre}=./payload needle file.rs",
|
||||
"rg --hostname-b[i]n ./payload needle file.rs",
|
||||
"rg ''--hostname-bin ./payload needle file.rs",
|
||||
"rg {--hostname-bin,--hostname-bin}=./payload needle file.rs",
|
||||
"rg \"--host\\\nname-bin\" ./payload needle file.rs",
|
||||
"rg needle *",
|
||||
"env \"rg\"\" \" needle file.rs",
|
||||
"env rg\" /../payload\" needle file.rs",
|
||||
"\"git status\"",
|
||||
] {
|
||||
assert_eq!(v(cmd), ClassifierVerdict::Block, "`{cmd}` must block");
|
||||
}
|
||||
for cmd in [
|
||||
"rg -n pattern .",
|
||||
"rg --pre-glob '*.pdf' pattern .",
|
||||
"rg -e needle --replace hostname .",
|
||||
"rg -n \"foo bar\" .",
|
||||
"rg -n 'foo bar' src",
|
||||
"rg -g '*.rs' needle .",
|
||||
"ls *.rs",
|
||||
"grep \"foo bar\" file.txt",
|
||||
] {
|
||||
assert_eq!(v(cmd), ClassifierVerdict::Allow, "`{cmd}` must be routine");
|
||||
}
|
||||
for cmd in [
|
||||
"git checkout",
|
||||
"git checkout --detach main",
|
||||
|
||||
@@ -16,6 +16,8 @@ pub struct BashCommandHighlights {
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct PlainCommand {
|
||||
words: Vec<String>,
|
||||
expandable: Vec<bool>,
|
||||
legacy: Vec<String>,
|
||||
span_start: usize,
|
||||
span_end: usize,
|
||||
}
|
||||
@@ -25,6 +27,106 @@ impl PlainCommand {
|
||||
pub fn words(&self) -> &[String] {
|
||||
&self.words
|
||||
}
|
||||
/// Per word: an unquoted segment still carries glob, brace, tilde or
|
||||
/// backslash syntax the shell would expand or decode into other argv.
|
||||
pub fn expandable(&self) -> &[bool] {
|
||||
&self.expandable
|
||||
}
|
||||
/// The spelling the pre-0.1.15 parser produced (string content with its
|
||||
/// escapes, concatenations as raw source); saved deny rules from that
|
||||
/// era are matched against it as well.
|
||||
pub fn legacy_words(&self) -> &[String] {
|
||||
&self.legacy
|
||||
}
|
||||
}
|
||||
|
||||
/// `pattern` (`git status`) matches when its whitespace-separated words equal
|
||||
/// the leading argv elements. Matching per element keeps a decoded word with
|
||||
/// inner whitespace (`rg `, `rg /../x`) from posing as a safe prefix.
|
||||
pub(crate) fn matches_prefix_words(words: &[String], pattern: &str, ignore_case: bool) -> bool {
|
||||
let pat: Vec<&str> = pattern.split_whitespace().collect();
|
||||
!pat.is_empty()
|
||||
&& words.len() >= pat.len()
|
||||
&& pat.iter().zip(words).all(|(p, w)| {
|
||||
if ignore_case {
|
||||
w.eq_ignore_ascii_case(p)
|
||||
} else {
|
||||
w == p
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
/// Unquoted text the shell would still expand or decode before exec:
|
||||
/// globs, brace lists, tilde, backslash escapes.
|
||||
pub(crate) fn unquoted_may_expand(text: &str) -> bool {
|
||||
text.contains(['*', '?', '[', ']', '{', '}', '~', '\\'])
|
||||
}
|
||||
|
||||
/// Bash's double-quote decoding: a backslash escapes only `$`, `` ` ``,
|
||||
/// `"`, `\\` and newline (`\<newline>` is removed); any other backslash
|
||||
/// stays literal.
|
||||
fn decode_double_quoted(raw: &str) -> String {
|
||||
let mut out = String::with_capacity(raw.len());
|
||||
let mut chars = raw.chars().peekable();
|
||||
while let Some(c) = chars.next() {
|
||||
if c != '\\' {
|
||||
out.push(c);
|
||||
continue;
|
||||
}
|
||||
match chars.peek() {
|
||||
Some('\n') => {
|
||||
chars.next();
|
||||
}
|
||||
Some(&escaped) if matches!(escaped, '$' | '`' | '"' | '\\') => {
|
||||
out.push(escaped);
|
||||
chars.next();
|
||||
}
|
||||
_ => out.push('\\'),
|
||||
}
|
||||
}
|
||||
out
|
||||
}
|
||||
|
||||
/// Decoded content of a plain double-quoted string: `"`, any number of
|
||||
/// `string_content` parts, `"`. An expansion child rejects the string. The
|
||||
/// text is the node's own source minus the two quote characters:
|
||||
/// tree-sitter leaves literal newlines in the gaps between
|
||||
/// `string_content` children and may fold whitespace into the closing
|
||||
/// quote token, so neither the children nor the token bounds are the text.
|
||||
fn simple_string_content(node: Node, src: &str) -> Option<String> {
|
||||
let count = node.child_count();
|
||||
if count < 2
|
||||
|| node.child(0)?.kind() != "\""
|
||||
|| node.child(count - 1)?.kind() != "\""
|
||||
|| (1..count - 1).any(|i| {
|
||||
node.child(i)
|
||||
.is_none_or(|part| part.kind() != "string_content")
|
||||
})
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let text = node.utf8_text(src.as_bytes()).ok()?;
|
||||
Some(decode_double_quoted(
|
||||
text.strip_prefix('"')?.strip_suffix('"')?,
|
||||
))
|
||||
}
|
||||
|
||||
/// What the old parser stored for a double-quoted string: its
|
||||
/// `string_content` text with escapes intact, or the raw source when the
|
||||
/// content is split (the old parser rejected those, so nothing was saved).
|
||||
fn legacy_string_spelling(node: Node, src: &str) -> Option<String> {
|
||||
let text = match node.child_count() {
|
||||
3 => node.child(1)?.utf8_text(src.as_bytes()).ok()?,
|
||||
_ => node.utf8_text(src.as_bytes()).ok()?,
|
||||
};
|
||||
Some(text.to_owned())
|
||||
}
|
||||
|
||||
fn raw_string_content<'a>(node: Node<'a>, src: &'a str) -> Option<&'a str> {
|
||||
node.utf8_text(src.as_bytes())
|
||||
.ok()?
|
||||
.strip_prefix('\'')?
|
||||
.strip_suffix('\'')
|
||||
}
|
||||
|
||||
/// Parse the provided bash source using tree-sitter-bash, returning a Tree on
|
||||
@@ -426,6 +528,8 @@ fn parse_plain_command_from_node(cmd: Node, src: &str) -> Option<PlainCommand> {
|
||||
return None;
|
||||
}
|
||||
let mut words = Vec::new();
|
||||
let mut expandable = Vec::new();
|
||||
let mut legacy = Vec::new();
|
||||
let mut cursor = cmd.walk();
|
||||
|
||||
let mut span_start: Option<usize> = None;
|
||||
@@ -450,6 +554,8 @@ fn parse_plain_command_from_node(cmd: Node, src: &str) -> Option<PlainCommand> {
|
||||
}
|
||||
span_end = Some(word_node.end_byte());
|
||||
|
||||
expandable.push(unquoted_may_expand(&text));
|
||||
legacy.push(text.clone());
|
||||
words.push(text);
|
||||
}
|
||||
"word" | "number" => {
|
||||
@@ -460,55 +566,62 @@ fn parse_plain_command_from_node(cmd: Node, src: &str) -> Option<PlainCommand> {
|
||||
}
|
||||
span_end = Some(child.end_byte());
|
||||
|
||||
expandable.push(unquoted_may_expand(&text));
|
||||
legacy.push(text.clone());
|
||||
words.push(text);
|
||||
}
|
||||
"string" => {
|
||||
// Allow only simple double-quoted strings with plain content.
|
||||
if child.child_count() == 3
|
||||
&& child.child(0)?.kind() == "\""
|
||||
&& child.child(1)?.kind() == "string_content"
|
||||
&& child.child(2)?.kind() == "\""
|
||||
{
|
||||
let content_node = child.child(1)?;
|
||||
let text = content_node.utf8_text(src.as_bytes()).ok()?.to_owned();
|
||||
// Only a plain double-quoted string; interpolation rejects.
|
||||
let text = simple_string_content(child, src)?;
|
||||
|
||||
if span_start.is_none() {
|
||||
// Highlight whole quoted string in the original script,
|
||||
// so span uses the outer node.
|
||||
span_start = Some(child.start_byte());
|
||||
}
|
||||
span_end = Some(child.end_byte());
|
||||
|
||||
words.push(text);
|
||||
} else {
|
||||
// Reject complex / interpolated strings
|
||||
return None;
|
||||
if span_start.is_none() {
|
||||
// Highlight whole quoted string in the original script,
|
||||
// so span uses the outer node.
|
||||
span_start = Some(child.start_byte());
|
||||
}
|
||||
span_end = Some(child.end_byte());
|
||||
|
||||
expandable.push(false);
|
||||
legacy.push(legacy_string_spelling(child, src)?);
|
||||
words.push(text);
|
||||
}
|
||||
"raw_string" => {
|
||||
let raw_string = child.utf8_text(src.as_bytes()).ok()?;
|
||||
let stripped = raw_string
|
||||
.strip_prefix('\'')
|
||||
.and_then(|s| s.strip_suffix('\''));
|
||||
{
|
||||
let s = stripped?;
|
||||
if span_start.is_none() {
|
||||
span_start = Some(child.start_byte());
|
||||
}
|
||||
span_end = Some(child.end_byte());
|
||||
|
||||
words.push(s.to_owned());
|
||||
let text = raw_string_content(child, src)?.to_owned();
|
||||
if span_start.is_none() {
|
||||
span_start = Some(child.start_byte());
|
||||
}
|
||||
span_end = Some(child.end_byte());
|
||||
|
||||
expandable.push(false);
|
||||
legacy.push(text.clone());
|
||||
words.push(text);
|
||||
}
|
||||
"concatenation" => {
|
||||
// Handle concatenation nodes (e.g., {} in find -exec)
|
||||
let text = child.utf8_text(src.as_bytes()).ok()?.to_owned();
|
||||
// Decode the segments (`--host""name-bin` → `--hostname-bin`)
|
||||
// so allowlists compare what the shell hands the program.
|
||||
let mut text = String::new();
|
||||
let mut may_expand = false;
|
||||
let mut parts = child.walk();
|
||||
for part in child.named_children(&mut parts) {
|
||||
match part.kind() {
|
||||
"word" | "number" => {
|
||||
let t = part.utf8_text(src.as_bytes()).ok()?;
|
||||
may_expand |= unquoted_may_expand(t);
|
||||
text.push_str(t);
|
||||
}
|
||||
"string" => text.push_str(&simple_string_content(part, src)?),
|
||||
"raw_string" => text.push_str(raw_string_content(part, src)?),
|
||||
_ => return None,
|
||||
}
|
||||
}
|
||||
|
||||
if span_start.is_none() {
|
||||
span_start = Some(child.start_byte());
|
||||
}
|
||||
span_end = Some(child.end_byte());
|
||||
|
||||
expandable.push(may_expand);
|
||||
legacy.push(child.utf8_text(src.as_bytes()).ok()?.to_owned());
|
||||
words.push(text);
|
||||
}
|
||||
_ => return None,
|
||||
@@ -520,6 +633,8 @@ fn parse_plain_command_from_node(cmd: Node, src: &str) -> Option<PlainCommand> {
|
||||
|
||||
Some(PlainCommand {
|
||||
words,
|
||||
expandable,
|
||||
legacy,
|
||||
span_start,
|
||||
span_end,
|
||||
})
|
||||
@@ -783,7 +898,7 @@ mod tests {
|
||||
highlighted_words: vec![
|
||||
"python3".to_owned(),
|
||||
"-c".to_owned(),
|
||||
"some text\\\" here ".to_owned()
|
||||
"some text\" here ".to_owned()
|
||||
],
|
||||
suffix: vec![
|
||||
"|".to_owned(),
|
||||
@@ -878,8 +993,7 @@ mod tests {
|
||||
highlighted_words: vec![
|
||||
"python".to_owned(),
|
||||
"-c".to_owned(),
|
||||
"\"\n import os\n os.environ.get(\"something\")\n \""
|
||||
.to_owned()
|
||||
"\n import os\n os.environ.get(something)\n ".to_owned()
|
||||
],
|
||||
suffix: vec![],
|
||||
})
|
||||
@@ -1245,3 +1359,64 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod decoded_word_tests {
|
||||
use super::all_commands_from_script;
|
||||
|
||||
fn parse(script: &str) -> (Vec<String>, Vec<bool>) {
|
||||
let cmds = all_commands_from_script(script).expect("word-only script");
|
||||
let cmd = &cmds[0];
|
||||
(cmd.words().to_vec(), cmd.expandable().to_vec())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn concatenations_decode_to_what_the_shell_passes() {
|
||||
for (script, decoded) in [
|
||||
("rg --host\"\"name-bin x", "--hostname-bin"),
|
||||
("rg ''--hostname-bin x", "--hostname-bin"),
|
||||
("rg --p're' x", "--pre"),
|
||||
("rg --pre=\"a b\" x", "--pre=a b"),
|
||||
("rg \"--host\\\nname-bin\" x", "--hostname-bin"),
|
||||
("rg --host\"\\\nname-bin\" x", "--hostname-bin"),
|
||||
("rg \"a\\\"b\\$c\\d\" x", "a\"b$c\\d"),
|
||||
("rg \"foo\nbar\" x", "foo\nbar"),
|
||||
("rg \" two words \" x", " two words "),
|
||||
("rg \" \" x", " "),
|
||||
("rg \"foo\n \" x", "foo\n "),
|
||||
("env \"rg\"\" \" needle x", "rg "),
|
||||
] {
|
||||
let (words, expandable) = parse(script);
|
||||
assert_eq!(words[1], decoded, "{script}");
|
||||
assert!(!expandable[1], "{script}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unquoted_expansion_syntax_is_flagged_and_quoted_is_not() {
|
||||
let (words, expandable) = parse("rg --{pre,pre}=x {--pre,--pre} *.rs -g '*.rs' \"?\"");
|
||||
assert_eq!(words[1], "--{pre,pre}=x");
|
||||
assert_eq!(
|
||||
expandable,
|
||||
vec![false, true, true, true, false, false, false]
|
||||
);
|
||||
let (_, expandable) = parse("cat ~/notes.txt");
|
||||
assert!(expandable[1]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn legacy_spelling_keeps_escapes_and_raw_concatenations() {
|
||||
let cmds = all_commands_from_script("rg \"\\\"api_key\\\"\" pri\"vate\" x").unwrap();
|
||||
assert_eq!(cmds[0].words(), ["rg", "\"api_key\"", "private", "x"]);
|
||||
assert_eq!(
|
||||
cmds[0].legacy_words(),
|
||||
["rg", "\\\"api_key\\\"", "pri\"vate\"", "x"]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_quoted_newline_in_the_head_is_not_the_bare_program() {
|
||||
let (words, _) = parse("env \"r\ng\" needle file.rs");
|
||||
assert_eq!(words[1], "r\ng");
|
||||
}
|
||||
}
|
||||
@@ -8,7 +8,8 @@ use kigi_acp_lib::AcpAgentGatewaySender as GatewaySender;
|
||||
use tokio::sync::{mpsc, oneshot};
|
||||
|
||||
use crate::permission::bash_command_splitting::{
|
||||
all_commands_from_script, is_setup_command, unwrap_wrappers,
|
||||
all_commands_from_script, is_setup_command, matches_prefix_words, unquoted_may_expand,
|
||||
unwrap_wrappers,
|
||||
};
|
||||
use crate::permission::policy::CompiledPolicy;
|
||||
use crate::permission::prompter::{AcpPrompter, PromptOutcome};
|
||||
@@ -143,75 +144,98 @@ fn mcp_pre_decision(
|
||||
|
||||
/// True when `words` is an `rg` invocation that enables a preprocessor.
|
||||
///
|
||||
/// `rg --pre COMMAND` (or `--pre=COMMAND`) runs `COMMAND <file>` for every
|
||||
/// searched file, so it can execute arbitrary programs. It must not ride the
|
||||
/// built-in safe-command auto-allow (unlike a pipeline, `--pre` stays one
|
||||
/// bash segment whose primary is still `rg`).
|
||||
///
|
||||
/// Deliberately does **not** match `--pre-glob`, which only filters when a
|
||||
/// preprocessor runs and does not itself spawn processes.
|
||||
fn rg_has_pre_flag(words: &[String]) -> bool {
|
||||
if words.first().map(String::as_str) != Some("rg") {
|
||||
/// `rg` options that execute a program per searched file: `--pre COMMAND`
|
||||
/// runs it as a preprocessor, `--hostname-bin COMMAND` to print hostnames.
|
||||
/// Both accept `--flag=VALUE`; `--pre-glob` only filters and is not listed.
|
||||
pub(crate) const RG_UNSAFE_FLAGS: &[&str] = &["--pre", "--hostname-bin"];
|
||||
|
||||
/// Whether an `rg` invocation carries a flag from [`RG_UNSAFE_FLAGS`], so
|
||||
/// it must not ride the built-in safe-command auto-allow (unlike a
|
||||
/// pipeline, the flag stays one bash segment whose primary is still `rg`).
|
||||
/// The head is matched by lowercased basename like Auto's routine check.
|
||||
/// Every later word is inspected, on purpose: rg's option grammar
|
||||
/// (`--replace -- --pre …`, `-ne -- --hostname-bin …`) makes any partial
|
||||
/// arity model bypassable, so a literal search for the flag name costs a
|
||||
/// prompt instead of the auto-allow. Words are the parser's decoded text;
|
||||
/// one the shell would still expand (`--{pre,pre}`, a glob that could
|
||||
/// match a file named `--pre`) counts as unsafe too.
|
||||
pub(crate) fn rg_has_unsafe_flag(words: &[String], expandable: &[bool]) -> bool {
|
||||
let head = words.first().map(|w| {
|
||||
w.rsplit(['/', '\\'])
|
||||
.next()
|
||||
.unwrap_or(w)
|
||||
.to_ascii_lowercase()
|
||||
});
|
||||
if head.as_deref() != Some("rg") {
|
||||
return false;
|
||||
}
|
||||
words
|
||||
.iter()
|
||||
.any(|w| w == "--pre" || w.starts_with("--pre="))
|
||||
words.iter().zip(expandable).skip(1).any(|(w, may_expand)| {
|
||||
let name = w.split_once('=').map_or(w.as_str(), |(name, _)| name);
|
||||
*may_expand || RG_UNSAFE_FLAGS.contains(&name)
|
||||
})
|
||||
}
|
||||
|
||||
/// Check whether the command words (already parsed by tree-sitter) match one of
|
||||
/// the known safe command prefixes.
|
||||
fn is_safe_command_words(words: &[String]) -> bool {
|
||||
fn is_safe_command_words(words: &[String], expandable: &[bool]) -> bool {
|
||||
if words.is_empty() {
|
||||
return false;
|
||||
}
|
||||
if rg_has_pre_flag(words) {
|
||||
if rg_has_unsafe_flag(words, expandable) {
|
||||
return false;
|
||||
}
|
||||
let joined = words.join(" ");
|
||||
is_safe_command_words_str(&joined)
|
||||
SAFE_COMMAND_PREFIXES
|
||||
.iter()
|
||||
.any(|p| matches_prefix_words(words, p, false))
|
||||
}
|
||||
|
||||
fn matches_command_prefix(cmd: &str, pattern: &str) -> bool {
|
||||
cmd == pattern || (cmd.starts_with(pattern) && cmd.as_bytes().get(pattern.len()) == Some(&b' '))
|
||||
}
|
||||
|
||||
/// Shared prefix check used by both the tree-sitter path and the fallback path.
|
||||
/// Built-in safe prefixes shared by the argv path and the display-string path.
|
||||
const SAFE_COMMAND_PREFIXES: &[&str] = &[
|
||||
"ls",
|
||||
"cat",
|
||||
"pwd",
|
||||
"date",
|
||||
"git status",
|
||||
"git branch",
|
||||
"git log",
|
||||
"git diff",
|
||||
"git ls-files",
|
||||
"git show",
|
||||
"git rev-parse",
|
||||
"cargo check",
|
||||
"whoami",
|
||||
"hostname",
|
||||
"uptime",
|
||||
"grep",
|
||||
"rg",
|
||||
"kubectl get",
|
||||
"kubectl logs",
|
||||
"kubectl describe",
|
||||
"ps",
|
||||
"bin/explorer ls",
|
||||
"head",
|
||||
"tail",
|
||||
"wc",
|
||||
"sort",
|
||||
"uniq",
|
||||
"tr",
|
||||
"cut",
|
||||
];
|
||||
|
||||
/// Display-string form of [`SAFE_COMMAND_PREFIXES`] (scope suggestions).
|
||||
fn is_safe_command_words_str(cmd: &str) -> bool {
|
||||
matches_command_prefix(cmd, "ls")
|
||||
|| matches_command_prefix(cmd, "cat")
|
||||
|| matches_command_prefix(cmd, "pwd")
|
||||
|| matches_command_prefix(cmd, "date")
|
||||
|| matches_command_prefix(cmd, "git status")
|
||||
|| matches_command_prefix(cmd, "git branch")
|
||||
|| matches_command_prefix(cmd, "git log")
|
||||
|| matches_command_prefix(cmd, "git diff")
|
||||
|| matches_command_prefix(cmd, "git ls-files")
|
||||
|| matches_command_prefix(cmd, "git show")
|
||||
|| matches_command_prefix(cmd, "git rev-parse")
|
||||
|| matches_command_prefix(cmd, "cargo check")
|
||||
|| matches_command_prefix(cmd, "whoami")
|
||||
|| matches_command_prefix(cmd, "hostname")
|
||||
|| matches_command_prefix(cmd, "uptime")
|
||||
|| matches_command_prefix(cmd, "grep")
|
||||
|| matches_command_prefix(cmd, "rg")
|
||||
|| matches_command_prefix(cmd, "kubectl get")
|
||||
|| matches_command_prefix(cmd, "kubectl logs")
|
||||
|| matches_command_prefix(cmd, "kubectl describe")
|
||||
|| matches_command_prefix(cmd, "ps")
|
||||
|| matches_command_prefix(cmd, "bin/explorer ls")
|
||||
|| matches_command_prefix(cmd, "head")
|
||||
|| matches_command_prefix(cmd, "tail")
|
||||
|| matches_command_prefix(cmd, "wc")
|
||||
|| matches_command_prefix(cmd, "sort")
|
||||
|| matches_command_prefix(cmd, "uniq")
|
||||
|| matches_command_prefix(cmd, "tr")
|
||||
|| matches_command_prefix(cmd, "cut")
|
||||
SAFE_COMMAND_PREFIXES
|
||||
.iter()
|
||||
.any(|p| matches_command_prefix(cmd, p))
|
||||
// CWE-863: `tee` is not in the safe-command list — it writes stdin
|
||||
// to arbitrary files, enabling pipelines like `cat data | tee /target` to
|
||||
// bypass edit permissions.
|
||||
//
|
||||
// `rg --pre` is excluded at the words level via [`rg_has_pre_flag`] — the
|
||||
// `rg --pre` is excluded at the words level via [`rg_has_unsafe_flag`] — the
|
||||
// string form here cannot see flag structure reliably after join.
|
||||
}
|
||||
|
||||
@@ -254,25 +278,17 @@ const ALWAYS_SAFE_COMMANDS: &[&str] = &[
|
||||
/// auto-approve via the always-safe primary alone — every non-setup segment
|
||||
/// must independently pass this (or the broader `is_safe_command_words`,
|
||||
/// or a user whitelist) check.
|
||||
fn is_always_safe_command_words(words: &[String]) -> bool {
|
||||
fn is_always_safe_command_words(words: &[String], expandable: &[bool]) -> bool {
|
||||
if words.is_empty() {
|
||||
return false;
|
||||
}
|
||||
if rg_has_pre_flag(words) {
|
||||
if rg_has_unsafe_flag(words, expandable) {
|
||||
return false;
|
||||
}
|
||||
|
||||
let joined = words.join(" ");
|
||||
|
||||
// CWE-183: use matches_command_prefix to require a word boundary after
|
||||
// the safe prefix, preventing e.g. "tr" from matching "truncate".
|
||||
for safe_pattern in ALWAYS_SAFE_COMMANDS {
|
||||
if matches_command_prefix(&joined, safe_pattern) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
// CWE-183: whole-word match, so "tr" cannot match "truncate".
|
||||
ALWAYS_SAFE_COMMANDS
|
||||
.iter()
|
||||
.any(|p| matches_prefix_words(words, p, false))
|
||||
}
|
||||
|
||||
/// Default always-allow whitelist scope (word count) for a parsed command.
|
||||
@@ -291,7 +307,9 @@ pub fn default_always_allow_scope(words: &[String]) -> usize {
|
||||
if words.is_empty() {
|
||||
return 0;
|
||||
}
|
||||
if is_safe_command_words(words) {
|
||||
// Display words carry no parser flags; expansion syntax is conservative.
|
||||
let expandable: Vec<bool> = words.iter().map(|w| unquoted_may_expand(w)).collect();
|
||||
if is_safe_command_words(words, &expandable) {
|
||||
if is_safe_command_words_str(&words[0]) {
|
||||
return 1;
|
||||
}
|
||||
@@ -314,26 +332,35 @@ pub fn default_always_allow_scope(words: &[String]) -> usize {
|
||||
/// `is_dangerous_command` script-level check, but applied to every
|
||||
/// segment in a chain instead of only the start of the script.
|
||||
fn is_dangerous_command_words(words: &[String]) -> bool {
|
||||
if words.is_empty() {
|
||||
return false;
|
||||
}
|
||||
let joined = words.join(" ");
|
||||
matches_command_prefix(&joined, "rm")
|
||||
|| matches_command_prefix(&joined, "chmod")
|
||||
|| matches_command_prefix(&joined, "chown")
|
||||
|| matches_command_prefix(&joined, "chgrp")
|
||||
|| matches_command_prefix(&joined, "chattr")
|
||||
|| matches_command_prefix(&joined, "pkill")
|
||||
|| matches_command_prefix(&joined, "kill")
|
||||
|| matches_command_prefix(&joined, "killall")
|
||||
|| matches_command_prefix(&joined, "git push")
|
||||
[
|
||||
"rm", "chmod", "chown", "chgrp", "chattr", "pkill", "kill", "killall", "git push",
|
||||
]
|
||||
.iter()
|
||||
.any(|p| matches_prefix_words(words, p, false))
|
||||
}
|
||||
|
||||
/// Whitelist matching helper. Uses `matches_command_prefix` so that user
|
||||
/// allow/deny entries enforce a word boundary after the prefix — preventing
|
||||
/// the "git" entry from matching "gitleaks" (CWE-183).
|
||||
fn matches_whitelist_prefix(segment_str: &str, allowed_prefix: &str) -> bool {
|
||||
matches_command_prefix(segment_str, allowed_prefix)
|
||||
/// A saved rule is the selected argv elements joined by one space, so it
|
||||
/// matches when some argv prefix joins back to it exactly: `rg private key`
|
||||
/// covers both `rg "private key"` and `rg private key`, while a padded
|
||||
/// program name (`rg `) never joins to `rg`.
|
||||
fn matches_saved_deny(words: &[String], legacy_inner: &[String], rule: &str) -> bool {
|
||||
// The legacy branch is the pre-0.1.15 check verbatim (wrappers peeled on
|
||||
// the legacy spelling, space-boundary prefix on its joined text), so no
|
||||
// deny saved by an older build narrows on upgrade.
|
||||
(1..=words.len()).any(|k| words[..k].join(" ") == rule)
|
||||
|| matches_command_prefix(&legacy_inner.join(" "), rule)
|
||||
}
|
||||
|
||||
/// A grant matches only token for word: the rule's tokens must equal the
|
||||
/// leading argv elements one by one, so neither one word spelled
|
||||
/// `git status` nor `./filter -n` absorbing a granted `-n` can claim a
|
||||
/// grant. A grant saved from a spaced argument therefore prompts again;
|
||||
/// that is the safe direction for an allow.
|
||||
fn matches_saved_grant(words: &[String], rule: &str) -> bool {
|
||||
matches_prefix_words(words, rule, false)
|
||||
}
|
||||
|
||||
/// Result of evaluating a bash script's segments against the current
|
||||
@@ -392,6 +419,8 @@ pub(crate) fn evaluate_bash_segments_inner(
|
||||
// such as `timeout 30 rm -rf /tmp/foo` would be treated as a benign
|
||||
// `timeout` invocation and silently auto-allowed.
|
||||
let words = unwrap_wrappers(raw_words);
|
||||
let expandable = &parsed.expandable()[raw_words.len() - words.len()..];
|
||||
let legacy_inner = unwrap_wrappers(parsed.legacy_words());
|
||||
if is_setup_command(words) {
|
||||
continue;
|
||||
}
|
||||
@@ -401,7 +430,7 @@ pub(crate) fn evaluate_bash_segments_inner(
|
||||
if let Some(d) = state
|
||||
.disallowed_bash_commands
|
||||
.iter()
|
||||
.find(|d| matches_whitelist_prefix(&s, d))
|
||||
.find(|d| matches_saved_deny(words, legacy_inner, d))
|
||||
{
|
||||
return SegmentEvaluation::Reject(format!(
|
||||
"User previously rejected `{d}` for this session"
|
||||
@@ -422,9 +451,10 @@ pub(crate) fn evaluate_bash_segments_inner(
|
||||
let matched_grant = state
|
||||
.allowed_bash_commands
|
||||
.iter()
|
||||
.any(|a| matches_whitelist_prefix(&s, a));
|
||||
.any(|a| matches_saved_grant(words, a));
|
||||
let matched_safe = honor_safe_lists
|
||||
&& (is_safe_command_words(words) || is_always_safe_command_words(words));
|
||||
&& (is_safe_command_words(words, expandable)
|
||||
|| is_always_safe_command_words(words, expandable));
|
||||
if matched_grant || matched_safe {
|
||||
if matched_grant {
|
||||
via_session_grant = true;
|
||||
@@ -3616,7 +3646,7 @@ mod tests {
|
||||
|
||||
/// Test shim: pure rename of `is_always_safe_primary_command`.
|
||||
fn is_always_safe_primary_command(words: &[String]) -> bool {
|
||||
is_always_safe_command_words(words)
|
||||
is_always_safe_command_words(words, &vec![false; words.len()])
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -3673,6 +3703,57 @@ mod tests {
|
||||
assert!(!is_safe_command("rg --pre cat pattern ."));
|
||||
assert!(!is_safe_command("rg --pre=/bin/cat pattern ."));
|
||||
assert!(!is_safe_command("rg -n --pre ./wrapper pattern"));
|
||||
// --hostname-bin runs COMMAND to print the hostname — same exec hole.
|
||||
assert!(!is_safe_command("rg --hostname-bin=./payload needle"));
|
||||
assert!(!is_safe_command("rg --hostname-bin ./payload needle"));
|
||||
// rg's option grammar lets a real flag follow a `--` or `-e` that is
|
||||
// itself a value; the scan therefore never trusts those positions,
|
||||
// and a literal search for the flag name falls back to a prompt.
|
||||
assert!(!is_safe_command("rg --replace -- --pre ./payload needle ."));
|
||||
assert!(!is_safe_command(
|
||||
"rg --replace -e --hostname-bin ./payload needle ."
|
||||
));
|
||||
assert!(!is_safe_command("rg -ne -- --hostname-bin ./payload ."));
|
||||
assert!(!is_safe_command("rg -e needle --hostname-bin ./payload ."));
|
||||
assert!(!is_safe_command("rg -- --hostname-bin ."));
|
||||
assert!(!is_safe_command("RG --pre ./payload needle ."));
|
||||
assert!(!is_safe_command("/usr/bin/rg --pre=cat needle ."));
|
||||
// Quote concatenation keeps its source text through the parser; the
|
||||
// shell would decode it into the flag.
|
||||
assert!(!is_safe_command("rg --host\"\"name-bin ./payload needle ."));
|
||||
assert!(!is_safe_command("rg --p're' ./payload needle ."));
|
||||
assert!(!is_safe_command("rg --hos\\tname-bin ./payload needle ."));
|
||||
assert!(!is_safe_command(
|
||||
"rg --{hostname-bin,hostname-bin}=./payload needle file.rs"
|
||||
));
|
||||
assert!(!is_safe_command("rg --{pre,pre}=./payload needle file.rs"));
|
||||
assert!(!is_safe_command(
|
||||
"rg --hostname-b[i]n ./payload needle file.rs"
|
||||
));
|
||||
assert!(!is_safe_command(
|
||||
"rg ''--hostname-bin ./payload needle file.rs"
|
||||
));
|
||||
assert!(!is_safe_command(
|
||||
"rg {--hostname-bin,--hostname-bin}=./payload needle file.rs"
|
||||
));
|
||||
assert!(!is_safe_command(
|
||||
"rg \"--host\\\nname-bin\" ./payload needle file.rs"
|
||||
));
|
||||
// Decoded words keep their argv boundary: a padded or slashed program
|
||||
// name is not `rg`, and a single word is not a two-word prefix.
|
||||
assert!(!is_safe_command("env \"rg\"\" \" needle file.rs"));
|
||||
assert!(!is_safe_command("env rg\" /../payload\" needle file.rs"));
|
||||
assert!(!is_safe_command("\"git status\""));
|
||||
assert!(is_safe_command("grep \"foo bar\" file.txt"));
|
||||
assert!(!is_safe_command("rg needle *"));
|
||||
assert!(is_safe_command("rg -n \"foo bar\" ."));
|
||||
assert!(is_safe_command("rg -g '*.rs' needle ."));
|
||||
assert!(is_safe_command("ls *.rs"));
|
||||
let unsafe_words: Vec<String> = "rg --hostname-bin=./payload needle"
|
||||
.split(' ')
|
||||
.map(String::from)
|
||||
.collect();
|
||||
assert_eq!(default_always_allow_scope(&unsafe_words), 2);
|
||||
assert!(!is_safe_command(
|
||||
"rg --pre-glob '*.pdf' --pre pdftotext pattern"
|
||||
));
|
||||
@@ -4183,6 +4264,92 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn saved_rules_match_argv_prefixes_not_joined_text() {
|
||||
let mut state = PermissionState::default();
|
||||
state
|
||||
.disallowed_bash_commands
|
||||
.insert("rg private key".to_string());
|
||||
for cmd in ["rg \"private key\" file.rs", "rg private key file.rs"] {
|
||||
assert!(
|
||||
matches!(
|
||||
evaluate_bash_segments(cmd, &state),
|
||||
SegmentEvaluation::Reject(_)
|
||||
),
|
||||
"`{cmd}` must hit the saved deny"
|
||||
);
|
||||
}
|
||||
// Denies saved by the pre-0.1.15 parser keep their raw spelling and
|
||||
// the old space-boundary prefix semantics.
|
||||
let mut state = PermissionState::default();
|
||||
for rule in [
|
||||
"rg \\\"api_key\\\"",
|
||||
"rg pri\"vate\"",
|
||||
"rg secret",
|
||||
"en\"v\" rg",
|
||||
] {
|
||||
state.disallowed_bash_commands.insert(rule.to_string());
|
||||
}
|
||||
for cmd in [
|
||||
"rg \"\\\"api_key\\\"\" file.rs",
|
||||
"rg pri\"vate\" file.rs",
|
||||
"rg \"secret phrase\" file.rs",
|
||||
"env rg \"secret phrase\" file.rs",
|
||||
"env en\"v\" rg needle file.rs",
|
||||
] {
|
||||
assert!(
|
||||
matches!(
|
||||
evaluate_bash_segments(cmd, &state),
|
||||
SegmentEvaluation::Reject(_)
|
||||
),
|
||||
"`{cmd}` must hit its legacy deny"
|
||||
);
|
||||
}
|
||||
assert!(!matches!(
|
||||
evaluate_bash_segments("rg secretive file.rs", &state),
|
||||
SegmentEvaluation::Reject(_)
|
||||
));
|
||||
let mut state = PermissionState::default();
|
||||
state.allowed_bash_commands.insert("rg".to_string());
|
||||
state.allowed_bash_commands.insert("git status".to_string());
|
||||
assert!(matches!(
|
||||
evaluate_bash_segments("git status --short", &state),
|
||||
SegmentEvaluation::AutoAllow { .. }
|
||||
));
|
||||
assert!(
|
||||
!matches!(
|
||||
evaluate_bash_segments("env \"rg\"\" \" needle file.rs", &state),
|
||||
SegmentEvaluation::AutoAllow { .. }
|
||||
),
|
||||
"a padded program name is not the granted `rg`"
|
||||
);
|
||||
// A grant matches token for word: neither one executable word nor a
|
||||
// spaced argument absorbing a granted token can claim it.
|
||||
let mut state = PermissionState::default();
|
||||
state.allowed_bash_commands.insert("git status".to_string());
|
||||
state
|
||||
.allowed_bash_commands
|
||||
.insert("rg --pre ./filter -n".to_string());
|
||||
for cmd in [
|
||||
"env git\" status\" --short",
|
||||
"env rg\" --pre ./filter -n\" needle file.rs",
|
||||
"rg --pre ./filter\" -n\" needle file.rs",
|
||||
"env rg --pre ./filter\" -n\" needle file.rs",
|
||||
] {
|
||||
assert!(
|
||||
!matches!(
|
||||
evaluate_bash_segments(cmd, &state),
|
||||
SegmentEvaluation::AutoAllow { .. }
|
||||
),
|
||||
"`{cmd}` must not ride a multiword grant"
|
||||
);
|
||||
}
|
||||
assert!(matches!(
|
||||
evaluate_bash_segments("rg --pre ./filter -n needle file.rs", &state),
|
||||
SegmentEvaluation::AutoAllow { .. }
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn evaluate_disallow_segment_rejects_whole_script() {
|
||||
// Disallow on any segment short-circuits with a Reject for the
|
||||
|
||||
Reference in new issue
Block a user