tkmind/tkmind_go
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

revert: "feat: allowlist optionally for goosed (#1848)" (#2010)

Browse Source
This commit is contained in:
Salman Mohammed
2025-04-02 13:47:34 -04:00
committed by GitHub
parent 4846d15354
commit 5ece6592b3
4 changed files with 0 additions and 557 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose-server/ALLOWLIST.md
-46
View File
@@ -1,46 +0,0 @@
# Goose Extension Allowlist
The allowlist feature provides a security mechanism for controlling which MCP commands can be used by goose.
By default, goose will let you run any MCP via any command, which isn't always desired.
## How It Works
1. When enabled, Goose will only allow execution of commands that match entries in the allowlist
2. Commands not in the allowlist will be rejected with an error message
3. The allowlist is fetched from a URL specified by the `GOOSE_ALLOWLIST` environment variable and cached while running.
## Setup
Set the `GOOSE_ALLOWLIST` environment variable to the URL of your allowlist YAML file:
```bash
export GOOSE_ALLOWLIST=https://example.com/goose-allowlist.yaml
```
If this environment variable is not set, no allowlist restrictions will be applied (all commands will be allowed).
## Allowlist File Format
The allowlist file should be a YAML file with the following structure:
```yaml
extensions:
- id: extension-id-1
command: command-name-1
- id: extension-id-2
command: command-name-2
```
Example:
```yaml
extensions:
- id: slack
command: uvx mcp_slack
- id: github
command: uvx mcp_github
- id: jira
command: uvx mcp_jira
```
Note that the command should be the full command to launch the MCP (environment variables are provided for context by goose). Additional arguments will be rejected (to avoid injection attacks)
crates/goose-server/Cargo.toml
-1
View File
@@ -35,7 +35,6 @@ serde_yaml = "0.9.34"
axum-extra = "0.10.0"
utoipa = { version = "4.1", features = ["axum_extras"] }
dirs = "6.0.0"
reqwest = { version = "0.12.9", features = ["json", "rustls-tls", "blocking"], default-features = false }
[[bin]]
name = "goosed"
crates/goose-server/src/routes/extension.rs
-508
View File
@@ -1,7 +1,4 @@
use std::collections::HashMap;
use std::env;
use std::path::Path;
use std::sync::OnceLock;
use crate::state::AppState;
use axum::{extract::State, routing::post, Json, Router};
@@ -130,18 +127,6 @@ async fn add_extension(
env_keys,
timeout,
} => {
// Check allowlist for Stdio extensions
if !is_command_allowed(&cmd, &args) {
return Ok(Json(ExtensionResponse {
error: true,
message: Some(format!(
"Extension '{}' is not in the allowed extensions list. Command: '{} {}'. If you require access please ask your administrator to update the allowlist.",
args.join(" "),
cmd, args.join(" ")
)),
}));
}
let mut env_map = HashMap::new();
for key in env_keys {
match config.get_secret(&key) {
@@ -242,496 +227,3 @@ pub fn routes(state: AppState) -> Router {
.route("/extensions/remove", post(remove_extension))
.with_state(state)
}
/// Structure representing the allowed extensions from the YAML file
#[derive(Deserialize, Debug, Clone)]
struct AllowedExtensions {
extensions: Vec<ExtensionAllowlistEntry>,
}
/// Structure representing an individual extension entry in the allowlist
#[derive(Deserialize, Debug, Clone)]
struct ExtensionAllowlistEntry {
#[allow(dead_code)]
id: String,
command: String,
}
// Global cache for the allowed extensions
static ALLOWED_EXTENSIONS: OnceLock<Option<AllowedExtensions>> = OnceLock::new();
/// Fetches and parses the allowed extensions from the URL specified in GOOSE_ALLOWLIST env var
fn fetch_allowed_extensions() -> Option<AllowedExtensions> {
match env::var("GOOSE_ALLOWLIST") {
Err(_) => {
// Environment variable not set, no allowlist to enforce
None
}
Ok(url) => match reqwest::blocking::get(&url) {
Err(e) => {
eprintln!("Failed to fetch allowlist: {}", e);
None
}
Ok(response) if !response.status().is_success() => {
eprintln!("Failed to fetch allowlist, status: {}", response.status());
None
}
Ok(response) => match response.text() {
Err(e) => {
eprintln!("Failed to read allowlist response: {}", e);
None
}
Ok(text) => match serde_yaml::from_str::<AllowedExtensions>(&text) {
Ok(allowed) => Some(allowed),
Err(e) => {
eprintln!("Failed to parse allowlist YAML: {}", e);
None
}
},
},
},
}
}
/// Gets the cached allowed extensions or fetches them if not yet cached
fn get_allowed_extensions() -> &'static Option<AllowedExtensions> {
ALLOWED_EXTENSIONS.get_or_init(fetch_allowed_extensions)
}
/// Checks if a command is allowed based on the allowlist
fn is_command_allowed(cmd: &str, args: &[String]) -> bool {
is_command_allowed_with_allowlist(&make_full_cmd(cmd, args), get_allowed_extensions())
}
fn make_full_cmd(cmd: &str, args: &[String]) -> String {
// trim each arg string to remove any leading/trailing whitespace
let args_trimmed = args.iter().map(|arg| arg.trim()).collect::<Vec<&str>>();
format!("{} {}", cmd.trim(), args_trimmed.join(" ").trim())
}
/// Normalizes a command name by removing common executable extensions (.exe, .cmd, .bat)
/// This makes the allowlist more portable across different operating systems
fn normalize_command_name(cmd: &str) -> String {
cmd.replace(".exe", "")
.replace(".cmd", "")
.replace(".bat", "")
.to_string()
}
/// Implementation of command allowlist checking that takes an explicit allowlist parameter
/// This makes it easier to test without relying on global state
fn is_command_allowed_with_allowlist(
cmd: &str,
allowed_extensions: &Option<AllowedExtensions>,
) -> bool {
println!("\n\n\n\n\n\n------------\n\nChecking command: {}", cmd);
// Extract the first part of the command (before any spaces)
let first_part = cmd.split_whitespace().next().unwrap_or(cmd);
println!("First part: {}", first_part);
// Extract the base command name (last part of the path)
let cmd_base_with_ext = Path::new(first_part)
.file_name()
.and_then(|name| name.to_str())
.unwrap_or(first_part);
// Normalize the command name by removing extensions like .exe or .cmd
let cmd_base = normalize_command_name(cmd_base_with_ext);
// Special case: Always allow commands ending with "/goosed" or equal to "goosed"
// But still enforce that it's in the same directory as the current executable
if cmd_base == "goosed" {
// Only allow exact matches (no arguments)
if cmd == first_part {
// For absolute paths, check that it's in the same directory as the current executable
if (first_part.contains('/') || first_part.contains('\\'))
&& !first_part.starts_with("./")
{
let current_exe = std::env::current_exe().unwrap();
let current_exe_dir = current_exe.parent().unwrap();
let expected_path = current_exe_dir.join("goosed").to_str().unwrap().to_string();
// Normalize both paths before comparing
let normalized_cmd_path = normalize_command_name(first_part);
let normalized_expected_path = normalize_command_name(&expected_path);
if normalized_cmd_path == normalized_expected_path {
return true;
}
// If the path doesn't match, don't allow it
println!("Goosed not in expected directory: {}", cmd);
println!("Expected path: {}", expected_path);
return false;
} else {
// For non-path goosed or relative paths, allow it
return true;
}
}
return false;
}
println!("Allowed extensions: {:?}", allowed_extensions);
match allowed_extensions {
// No allowlist configured, allow all commands
None => true,
// Empty allowlist, allow all commands
Some(extensions) if extensions.extensions.is_empty() => true,
// Check against the allowlist
Some(extensions) => {
// Check that the command exists as a peer command to current executable directory
// Only apply this check if the command includes a path separator
let current_exe = std::env::current_exe().unwrap();
let current_exe_dir = current_exe.parent().unwrap();
let expected_path = current_exe_dir
.join(&cmd_base)
.to_str()
.unwrap()
.to_string();
// Normalize both paths before comparing
let normalized_cmd_path = normalize_command_name(first_part);
if (first_part.contains('/') || first_part.contains('\\'))
&& normalized_cmd_path != expected_path
{
println!("Command not in expected directory: {}", cmd);
return false;
}
//let cmd_to_check= cmd.replace(current_exe_dir.to_str(), "").to_string();
// remove current_exe_dir + "/" from the cmd to clean it up
let path_to_trim = format!("{}/", current_exe_dir.to_str().unwrap());
// now remove this to make it clean
let cmd_to_check = cmd.replace(&path_to_trim, "");
println!("Command to check: {}", cmd_to_check);
// Normalize the command before comparing with allowlist entries
let normalized_cmd = normalize_command_name(&cmd_to_check);
println!("Normalized command: {}", normalized_cmd);
extensions.extensions.iter().any(|entry| {
let normalized_entry = normalize_command_name(&entry.command);
normalized_cmd == normalized_entry
})
}
}
}
#[cfg(test)]
mod tests {
use super::*;
use std::env;
#[test]
fn test_normalize_command_name() {
// Test removing .exe extension
assert_eq!(normalize_command_name("goosed.exe"), "goosed");
assert_eq!(
normalize_command_name("/path/to/goosed.exe"),
"/path/to/goosed"
);
// Test removing .cmd extension
assert_eq!(normalize_command_name("script.cmd"), "script");
assert_eq!(
normalize_command_name("/path/to/script.cmd"),
"/path/to/script"
);
assert_eq!(normalize_command_name("batch.bat"), "batch");
assert_eq!(
normalize_command_name("/path/to/batch.bat thing"),
"/path/to/batch thing"
);
// Test with no extension
assert_eq!(normalize_command_name("goosed"), "goosed");
assert_eq!(normalize_command_name("/path/to/goosed"), "/path/to/goosed");
}
// Create a test allowlist with the given commands
fn create_test_allowlist(commands: &[&str]) -> Option<AllowedExtensions> {
if commands.is_empty() {
return Some(AllowedExtensions { extensions: vec![] });
}
let entries = commands
.iter()
.enumerate()
.map(|(i, cmd)| ExtensionAllowlistEntry {
id: format!("test-{}", i),
command: cmd.to_string(),
})
.collect();
Some(AllowedExtensions {
extensions: entries,
})
}
#[test]
fn test_make_full() {
assert_eq!(
make_full_cmd("uvx", &vec!["mcp_slack".to_string()]),
"uvx mcp_slack"
);
assert_eq!(
make_full_cmd("uvx", &vec!["mcp_slack ".to_string()]),
"uvx mcp_slack"
);
assert_eq!(
make_full_cmd(
"uvx",
&vec!["mcp_slack".to_string(), "--verbose".to_string()]
),
"uvx mcp_slack --verbose"
);
assert_eq!(
make_full_cmd(
"uvx",
&vec!["mcp_slack".to_string(), " --verbose".to_string()]
),
"uvx mcp_slack --verbose"
);
}
#[test]
fn test_command_allowed_when_matching() {
let allowlist = create_test_allowlist(&[
"uvx something",
"uvx mcp_slack",
"npx mcp_github",
"minecraft",
]);
// Test with exact command matches
assert!(is_command_allowed_with_allowlist(
"uvx something",
&allowlist
));
// Test with exact command matches
assert!(is_command_allowed_with_allowlist("minecraft", &allowlist));
assert!(is_command_allowed_with_allowlist(
"uvx mcp_slack",
&allowlist
));
assert!(is_command_allowed_with_allowlist(
"npx mcp_github",
&allowlist
));
// Get the current executable directory for reference
let current_exe = std::env::current_exe().unwrap();
let current_exe_dir = current_exe.parent().unwrap();
// Create a full path command that would be in the current executable directory
// For testing purposes, we'll use a direct path to the command in the allowlist
let full_path_cmd = current_exe_dir
.join("uvx my_mcp")
.to_str()
.unwrap()
.to_string();
// Create a test allowlist with the command name (without path)
let path_test_allowlist = create_test_allowlist(&["uvx my_mcp"]);
// This should be allowed because the path is correct and the base command matches
println!(
"Current executable directory: {}",
current_exe_dir.to_str().unwrap()
);
println!("Path test allowlist: {:?}", path_test_allowlist);
assert!(is_command_allowed_with_allowlist(
&full_path_cmd,
&path_test_allowlist
));
// Test with additional arguments - should NOT match because we require exact matches
assert!(!is_command_allowed_with_allowlist(
"uvx mcp_slack --verbose --flag=value",
&allowlist
));
// Test with a path that doesn't match the current directory - should fail
assert!(!is_command_allowed_with_allowlist(
"/Users/username/path/to/uvx mcp_slack",
&allowlist
));
// These should NOT match with exact matching
assert!(!is_command_allowed_with_allowlist(
"uvx other_command",
&allowlist
));
assert!(!is_command_allowed_with_allowlist(
"prefix_npx mcp_github",
&allowlist
));
}
#[test]
fn test_command_not_allowed_when_not_matching() {
let allowlist =
create_test_allowlist(&["uvx something", "uvx mcp_slack", "npx mcp_github"]);
// These should not be allowed
assert!(!is_command_allowed_with_allowlist(
"/Users/username/path/to/uvx_malicious",
&allowlist
));
assert!(!is_command_allowed_with_allowlist(
"unauthorized_command",
&allowlist
));
assert!(!is_command_allowed_with_allowlist("/bin/bash", &allowlist));
assert!(!is_command_allowed_with_allowlist(
"uvx unauthorized",
&allowlist
));
}
#[test]
fn test_all_commands_allowed_when_no_allowlist() {
// Empty allowlist should allow all commands
let empty_allowlist = create_test_allowlist(&[]);
assert!(is_command_allowed_with_allowlist(
"any_command_should_be_allowed",
&empty_allowlist
));
// No allowlist should allow all commands
assert!(is_command_allowed_with_allowlist(
"any_command_should_be_allowed",
&None
));
}
#[test]
fn test_goosed_special_case() {
// Create a restrictive allowlist that doesn't include goosed
let allowlist = create_test_allowlist(&["uvx mcp_slack"]);
// Get the current executable directory for goosed path testing
let current_exe = std::env::current_exe().unwrap();
let current_exe_dir = current_exe.parent().unwrap();
let goosed_path = current_exe_dir.join("goosed").to_str().unwrap().to_string();
let goosed_exe_path = current_exe_dir
.join("goosed.exe")
.to_str()
.unwrap()
.to_string();
// This should be allowed because it's goosed in the correct directory
assert!(is_command_allowed_with_allowlist(&goosed_path, &allowlist));
// This should also be allowed because it's goosed.exe in the correct directory
assert!(is_command_allowed_with_allowlist(
&goosed_exe_path,
&allowlist
));
// These should NOT be allowed because they're in the wrong directory
assert!(!is_command_allowed_with_allowlist(
"/usr/local/bin/goosed",
&allowlist
));
assert!(!is_command_allowed_with_allowlist(
"/Users/username/path/to/goosed",
&allowlist
));
// Commands with arguments should NOT be allowed - we require exact matches
assert!(!is_command_allowed_with_allowlist(
"/Users/username/path/to/goosed --flag value",
&allowlist
));
// Simple goosed without path should be allowed
assert!(is_command_allowed_with_allowlist("./goosed", &allowlist));
assert!(is_command_allowed_with_allowlist("goosed", &allowlist));
// These should NOT be allowed because they don't end with "/goosed"
assert!(!is_command_allowed_with_allowlist(
"/usr/local/bin/goosed-extra",
&allowlist
));
assert!(!is_command_allowed_with_allowlist(
"/usr/local/bin/not-goosed",
&allowlist
));
assert!(!is_command_allowed_with_allowlist(
"goosed-extra",
&allowlist
));
}
#[test]
fn test_fetch_allowed_extensions_from_url() {
// Start a mock server - we need to use a blocking approach since fetch_allowed_extensions is blocking
let server = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
let port = server.local_addr().unwrap().port();
let server_url = format!("http://127.0.0.1:{}", port);
let server_path = "/allowed_extensions.yaml";
// Define the mock response
let yaml_content = r#"extensions:
- id: slack
command: uvx mcp_slack
- id: github
command: uvx mcp_github
"#;
// Spawn a thread to handle the request
let handle = std::thread::spawn(move || {
let (stream, _) = server.accept().unwrap();
let mut buf_reader = std::io::BufReader::new(&stream);
let mut request_line = String::new();
std::io::BufRead::read_line(&mut buf_reader, &mut request_line).unwrap();
// Very simple HTTP response
let response = format!(
"HTTP/1.1 200 OK\r\nContent-Length: {}\r\nContent-Type: text/yaml\r\n\r\n{}",
yaml_content.len(),
yaml_content
);
let mut writer = std::io::BufWriter::new(&stream);
std::io::Write::write_all(&mut writer, response.as_bytes()).unwrap();
std::io::Write::flush(&mut writer).unwrap();
});
// Set the environment variable to point to our mock server
env::set_var("GOOSE_ALLOWLIST", format!("{}{}", server_url, server_path));
// Give the server a moment to start
std::thread::sleep(std::time::Duration::from_millis(100));
// Call the function that fetches from the URL
let allowed_extensions = fetch_allowed_extensions();
// Verify the result
assert!(allowed_extensions.is_some());
let extensions = allowed_extensions.unwrap();
assert_eq!(extensions.extensions.len(), 2);
assert_eq!(extensions.extensions[0].id, "slack");
assert_eq!(extensions.extensions[0].command, "uvx mcp_slack");
assert_eq!(extensions.extensions[1].id, "github");
assert_eq!(extensions.extensions[1].command, "uvx mcp_github");
// Clean up
env::remove_var("GOOSE_ALLOWLIST");
// Wait for the server thread to complete
handle.join().unwrap();
}
}