mirror of
https://github.com/aaif-goose/goose.git
synced 2026-07-03 14:10:03 +02:00
fix: remove defaults yaml (#8854)
This commit is contained in:
@@ -42,6 +42,8 @@ pub async fn run() -> Result<()> {
|
||||
boot_marker("main entered");
|
||||
crate::logging::setup_logging(Some("goosed"))?;
|
||||
|
||||
goose::security::set_security_defaults();
|
||||
|
||||
let settings = configuration::Settings::new()?;
|
||||
|
||||
let secret_key = std::env::var("GOOSE_SERVER__SECRET_KEY")
|
||||
|
||||
@@ -122,7 +122,6 @@ impl From<keyring::Error> for ConfigError {
|
||||
/// For goose-specific configuration, consider prefixing with "goose_" to avoid conflicts.
|
||||
pub struct Config {
|
||||
config_path: PathBuf,
|
||||
defaults_path: Option<PathBuf>,
|
||||
secrets: SecretStorage,
|
||||
guard: Mutex<()>,
|
||||
secrets_cache: Arc<Mutex<Option<HashMap<String, Value>>>>,
|
||||
@@ -142,16 +141,6 @@ impl Default for Config {
|
||||
|
||||
let config_path = config_dir.join(CONFIG_YAML_NAME);
|
||||
|
||||
let defaults_path = find_workspace_or_exe_root().and_then(|root| {
|
||||
let path = root.join("defaults.yaml");
|
||||
if path.exists() {
|
||||
tracing::info!("Found bundled defaults.yaml at: {:?}", path);
|
||||
Some(path)
|
||||
} else {
|
||||
None
|
||||
}
|
||||
});
|
||||
|
||||
let secrets = if env::var("GOOSE_DISABLE_KEYRING").is_ok()
|
||||
|| keyring_disabled_in_config(&config_path)
|
||||
{
|
||||
@@ -165,7 +154,6 @@ impl Default for Config {
|
||||
};
|
||||
Config {
|
||||
config_path,
|
||||
defaults_path,
|
||||
secrets,
|
||||
guard: Mutex::new(()),
|
||||
secrets_cache: Arc::new(Mutex::new(None)),
|
||||
@@ -284,7 +272,6 @@ impl Config {
|
||||
pub fn new<P: AsRef<Path>>(config_path: P, service: &str) -> Result<Self, ConfigError> {
|
||||
Ok(Config {
|
||||
config_path: config_path.as_ref().to_path_buf(),
|
||||
defaults_path: None,
|
||||
secrets: SecretStorage::Keyring {
|
||||
service: service.to_string(),
|
||||
},
|
||||
@@ -303,23 +290,6 @@ impl Config {
|
||||
) -> Result<Self, ConfigError> {
|
||||
Ok(Config {
|
||||
config_path: config_path.as_ref().to_path_buf(),
|
||||
defaults_path: None,
|
||||
secrets: SecretStorage::File {
|
||||
path: secrets_path.as_ref().to_path_buf(),
|
||||
},
|
||||
guard: Mutex::new(()),
|
||||
secrets_cache: Arc::new(Mutex::new(None)),
|
||||
})
|
||||
}
|
||||
|
||||
pub fn new_with_defaults<P1: AsRef<Path>, P2: AsRef<Path>, P3: AsRef<Path>>(
|
||||
config_path: P1,
|
||||
secrets_path: P2,
|
||||
defaults_path: P3,
|
||||
) -> Result<Self, ConfigError> {
|
||||
Ok(Config {
|
||||
config_path: config_path.as_ref().to_path_buf(),
|
||||
defaults_path: Some(defaults_path.as_ref().to_path_buf()),
|
||||
secrets: SecretStorage::File {
|
||||
path: secrets_path.as_ref().to_path_buf(),
|
||||
},
|
||||
@@ -369,9 +339,7 @@ impl Config {
|
||||
}
|
||||
|
||||
fn load(&self) -> Result<Mapping, ConfigError> {
|
||||
let mut values = self.load_raw()?;
|
||||
self.merge_missing_defaults(&mut values);
|
||||
Ok(values)
|
||||
self.load_raw()
|
||||
}
|
||||
|
||||
pub fn all_values(&self) -> Result<HashMap<String, Value>, ConfigError> {
|
||||
@@ -712,7 +680,6 @@ impl Config {
|
||||
/// This will attempt to get the value from (in order):
|
||||
/// 1. Environment variable with the uppercase key name
|
||||
/// 2. Configuration file (~/.config/goose/config.yaml)
|
||||
/// 3. Bundled defaults file (defaults.yaml in workspace root or executable directory)
|
||||
///
|
||||
/// The value will be deserialized into the requested type. This works with
|
||||
/// both simple types (String, i32, etc.) and complex types that implement
|
||||
@@ -738,24 +705,6 @@ impl Config {
|
||||
.and_then(|v| Ok(serde_yaml::from_value(v.clone())?))
|
||||
}
|
||||
|
||||
fn load_defaults(&self) -> Option<Mapping> {
|
||||
let path = self.defaults_path.as_ref()?;
|
||||
let content = std::fs::read_to_string(path).ok()?;
|
||||
parse_yaml_content(&content).ok()
|
||||
}
|
||||
|
||||
fn merge_missing_defaults(&self, values: &mut Mapping) {
|
||||
let Some(defaults) = self.load_defaults() else {
|
||||
return;
|
||||
};
|
||||
|
||||
for (key, default_value) in defaults {
|
||||
if !values.contains_key(&key) {
|
||||
values.insert(key, default_value);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Set a configuration value in the config file (non-secret).
|
||||
///
|
||||
/// This will immediately write the value to the config file. The value
|
||||
@@ -1889,101 +1838,14 @@ mod tests {
|
||||
Config::new_with_file_secrets(config_file.path(), secrets_file.path()).unwrap()
|
||||
}
|
||||
|
||||
fn new_test_config_with_defaults(defaults_content: &str) -> (Config, NamedTempFile) {
|
||||
let config_file = NamedTempFile::new().unwrap();
|
||||
let secrets_file = NamedTempFile::new().unwrap();
|
||||
let defaults_file = NamedTempFile::new().unwrap();
|
||||
std::fs::write(defaults_file.path(), defaults_content).unwrap();
|
||||
let config = Config::new_with_defaults(
|
||||
config_file.path(),
|
||||
secrets_file.path(),
|
||||
defaults_file.path(),
|
||||
)
|
||||
.unwrap();
|
||||
(config, defaults_file)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_defaults_fallback_when_key_not_in_config() -> Result<(), ConfigError> {
|
||||
let (config, _defaults) =
|
||||
new_test_config_with_defaults("SECURITY_PROMPT_ENABLED: true\nsome_key: default_val");
|
||||
|
||||
// Key only in defaults → returns defaults value
|
||||
let value: bool = config.get_param("SECURITY_PROMPT_ENABLED")?;
|
||||
assert!(value);
|
||||
|
||||
let value: String = config.get_param("some_key")?;
|
||||
assert_eq!(value, "default_val");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_full_precedence_env_over_config_over_defaults() -> Result<(), ConfigError> {
|
||||
let (config, _defaults) = new_test_config_with_defaults("my_key: from_defaults");
|
||||
|
||||
// Only defaults → returns defaults
|
||||
let value: String = config.get_param("my_key")?;
|
||||
assert_eq!(value, "from_defaults");
|
||||
|
||||
// Config file overrides defaults
|
||||
config.set_param("my_key", "from_config")?;
|
||||
let value: String = config.get_param("my_key")?;
|
||||
assert_eq!(value, "from_config");
|
||||
|
||||
// Env var overrides config file (and defaults)
|
||||
std::env::set_var("MY_KEY", "from_env");
|
||||
let value: String = config.get_param("my_key")?;
|
||||
assert_eq!(value, "from_env");
|
||||
std::env::remove_var("MY_KEY");
|
||||
|
||||
// After removing env var, config file value is back
|
||||
let value: String = config.get_param("my_key")?;
|
||||
assert_eq!(value, "from_config");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_no_defaults_file_behaves_as_before() {
|
||||
// Config without defaults (the normal open-source case)
|
||||
fn test_missing_key_returns_not_found() {
|
||||
let config = new_test_config();
|
||||
|
||||
let result: Result<String, ConfigError> = config.get_param("nonexistent_key");
|
||||
assert!(matches!(result, Err(ConfigError::NotFound(_))));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_defaults_not_persisted_on_write() -> Result<(), ConfigError> {
|
||||
let (config, _defaults) = new_test_config_with_defaults("default_key: default_value");
|
||||
|
||||
// Read a default value (should work)
|
||||
let value: String = config.get_param("default_key")?;
|
||||
assert_eq!(value, "default_value");
|
||||
|
||||
// Write a different key
|
||||
config.set_param("user_key", "user_value")?;
|
||||
|
||||
// Read config file directly - should NOT contain default_key
|
||||
let config_path = PathBuf::from(config.path());
|
||||
let file_content = std::fs::read_to_string(&config_path)?;
|
||||
assert!(
|
||||
!file_content.contains("default_key"),
|
||||
"Defaults should not be persisted to config file on write"
|
||||
);
|
||||
assert!(
|
||||
file_content.contains("user_key"),
|
||||
"User's key should be in config file"
|
||||
);
|
||||
|
||||
// But reading via get_param should still return the default
|
||||
let value: String = config.get_param("default_key")?;
|
||||
assert_eq!(value, "default_value");
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(unix)]
|
||||
fn test_secrets_file_created_with_restricted_permissions() -> Result<(), ConfigError> {
|
||||
|
||||
@@ -13,6 +13,29 @@ use scanner::PromptInjectionScanner;
|
||||
use std::sync::OnceLock;
|
||||
use uuid::Uuid;
|
||||
|
||||
fn set_default_if_not_exist(config: &Config, key: &str, default_env: &str) {
|
||||
if config.get_param::<bool>(key).is_ok() {
|
||||
return;
|
||||
}
|
||||
if let Ok(parsed) = config.get_param::<bool>(default_env) {
|
||||
let _ = config.set_param(key, parsed);
|
||||
}
|
||||
}
|
||||
|
||||
pub fn set_security_defaults() {
|
||||
let config = Config::global();
|
||||
set_default_if_not_exist(
|
||||
config,
|
||||
"SECURITY_PROMPT_ENABLED",
|
||||
"DEFAULT_SECURITY_PROMPT_ENABLED",
|
||||
);
|
||||
set_default_if_not_exist(
|
||||
config,
|
||||
"SECURITY_COMMAND_CLASSIFIER_ENABLED",
|
||||
"DEFAULT_SECURITY_COMMAND_CLASSIFIER_ENABLED",
|
||||
);
|
||||
}
|
||||
|
||||
pub struct SecurityManager {
|
||||
scanner: OnceLock<PromptInjectionScanner>,
|
||||
}
|
||||
@@ -29,6 +52,7 @@ pub struct SecurityResult {
|
||||
|
||||
impl SecurityManager {
|
||||
pub fn new() -> Self {
|
||||
set_security_defaults();
|
||||
Self {
|
||||
scanner: OnceLock::new(),
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user