mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-06 02:48:25 +00:00
fix(agent): avoid nested Bash for sandbox MCP scripts
This commit is contained in:
parent
2f84b67558
commit
7f436bf64c
1 changed files with 37 additions and 2 deletions
|
|
@ -2104,8 +2104,20 @@ const fn is_auth_error(err: &LlmError) -> bool {
|
|||
/// the current `$BASH` because the sandbox evaluates this string as non-login
|
||||
/// Bash and may resolve that executable outside `/bin` (for example on NixOS).
|
||||
fn sandbox_mcp_launch_script(command: &[String]) -> String {
|
||||
let cmd_str = shell::shell_join(command);
|
||||
let inner = format!("{cmd_str} > /tmp/mcp_server_stdout.log 2>/tmp/mcp_server_stderr.log");
|
||||
let command_source = match command {
|
||||
// Sandbox MCP `script` entries resolve to this exact argv shape. The
|
||||
// surrounding launcher is already the provider-selected Bash, so
|
||||
// evaluate the source in that process instead of PATH-resolving a
|
||||
// second interpreter. Grouping keeps the log redirections scoped to
|
||||
// the whole script, including multi-command and trailing-comment
|
||||
// forms.
|
||||
[interpreter, flag, source] if interpreter == "bash" && flag == "-c" => {
|
||||
format!("{{\n{source}\n}}")
|
||||
}
|
||||
_ => shell::shell_join(command),
|
||||
};
|
||||
let inner =
|
||||
format!("{command_source} > /tmp/mcp_server_stdout.log 2>/tmp/mcp_server_stderr.log");
|
||||
format!(
|
||||
"setsid \"$BASH\" -c {quoted} </dev/null >/dev/null 2>&1 &\necho $!",
|
||||
quoted = shell::shell_quote(&inner)
|
||||
|
|
@ -2180,6 +2192,29 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandbox_mcp_launch_wrapper_evaluates_scripts_in_the_selected_bash() {
|
||||
let source =
|
||||
"PATH=/mcp-only\nprintf 'starting server\\n'\nexec my-server --port 3100 # ready";
|
||||
let script =
|
||||
sandbox_mcp_launch_script(&["bash".to_string(), "-c".to_string(), source.to_string()]);
|
||||
|
||||
let wrapper_argument = script
|
||||
.strip_prefix("setsid \"$BASH\" -c ")
|
||||
.and_then(|rest| rest.strip_suffix(" </dev/null >/dev/null 2>&1 &\necho $!"))
|
||||
.expect("launch wrapper should have the canonical shape");
|
||||
let unwrapped = shlex::split(wrapper_argument).expect("wrapper argument should parse");
|
||||
|
||||
assert_eq!(unwrapped, vec![format!(
|
||||
"{{\n{source}\n}} > /tmp/mcp_server_stdout.log 2>/tmp/mcp_server_stderr.log"
|
||||
)]);
|
||||
assert!(
|
||||
!unwrapped[0].contains("bash -c"),
|
||||
"script entries must not PATH-resolve a nested Bash: {}",
|
||||
unwrapped[0]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn sandbox_mcp_launch_wrapper_quotes_arbitrary_argv() {
|
||||
// A quote or metacharacter in any argv element must not break out of
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue