Skip to content

Commit 96ddeca

Browse files
authored
fix: resolve EACCES error from incorrect bundled plugins directory
Fixes bundled_root() to resolve the bundled plugins directory relative to the executable path at runtime instead of using a compile-time CARGO_MANIFEST_DIR path that may be root-owned. Resolution order: standard FHS layout, adjacent layout, then dev/source-tree fallback. Includes proper tests for override, nonexistent, and auto-detection scenarios.
1 parent 271283c commit 96ddeca

1 file changed

Lines changed: 194 additions & 9 deletions

File tree

rust/crates/plugins/src/lib.rs

Lines changed: 194 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,8 +1050,59 @@ impl PluginManager {
10501050
Self { config }
10511051
}
10521052

1053+
/// Returns the default bundled plugins root directory.
1054+
///
1055+
/// Resolution order (first existing path wins):
1056+
/// 1. `<exe_dir>/../share/claw/plugins/bundled` — standard install layout
1057+
/// 2. `<exe_dir>/bundled` — simple relocated layout
1058+
/// 3. `CARGO_MANIFEST_DIR/bundled` — dev/source-tree fallback (only if it exists)
1059+
/// 4. `<exe_dir>/../share/claw/plugins/bundled` — canonical default even if missing
1060+
///
1061+
/// This avoids baking in a compile-time source-tree path that may be
1062+
/// inaccessible at runtime (e.g. a root-owned repo directory).
10531063
#[must_use]
10541064
pub fn bundled_root() -> PathBuf {
1065+
// Candidate 1: standard FHS install layout — <prefix>/bin/claw -> <prefix>/share/claw/plugins/bundled
1066+
if let Ok(exe_path) = std::env::current_exe() {
1067+
if let Some(exe_dir) = exe_path.parent() {
1068+
let share_path = exe_dir
1069+
.join("..")
1070+
.join("share")
1071+
.join("claw")
1072+
.join("plugins")
1073+
.join("bundled");
1074+
if share_path.exists() {
1075+
return share_path;
1076+
}
1077+
1078+
// Candidate 2: simple adjacent layout — <exe_dir>/bundled
1079+
let adjacent = exe_dir.join("bundled");
1080+
if adjacent.exists() {
1081+
return adjacent;
1082+
}
1083+
}
1084+
}
1085+
1086+
// Candidate 3: dev/source-tree fallback — only if the directory actually exists
1087+
let dev_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("bundled");
1088+
if dev_path.exists() {
1089+
return dev_path;
1090+
}
1091+
1092+
// Default (nothing found): return the canonical install path even if missing,
1093+
// so callers get an empty plugin list rather than a permission error.
1094+
if let Ok(exe_path) = std::env::current_exe() {
1095+
if let Some(exe_dir) = exe_path.parent() {
1096+
return exe_dir
1097+
.join("..")
1098+
.join("share")
1099+
.join("claw")
1100+
.join("plugins")
1101+
.join("bundled");
1102+
}
1103+
}
1104+
1105+
// Last resort fallback
10551106
PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("bundled")
10561107
}
10571108

@@ -1370,12 +1421,24 @@ impl PluginManager {
13701421
}
13711422

13721423
fn sync_bundled_plugins(&self) -> Result<(), PluginError> {
1424+
let explicit_root = self.config.bundled_root.is_some();
13731425
let bundled_root = self
13741426
.config
13751427
.bundled_root
13761428
.clone()
13771429
.unwrap_or_else(Self::bundled_root);
1378-
let bundled_plugins = discover_plugin_dirs(&bundled_root)?;
1430+
let bundled_plugins = match discover_plugin_dirs(&bundled_root) {
1431+
Ok(plugins) => plugins,
1432+
// When the bundled root is the auto-detected default and the directory is
1433+
// inaccessible (e.g. a root-owned source tree), treat it as empty rather
1434+
// than fatally failing. An explicit config override still surfaces errors.
1435+
Err(PluginError::Io(ref error))
1436+
if !explicit_root && error.kind() == std::io::ErrorKind::PermissionDenied =>
1437+
{
1438+
Vec::new()
1439+
}
1440+
Err(error) => return Err(error),
1441+
};
13791442
let mut registry = self.load_registry()?;
13801443
let mut changed = false;
13811444
let install_root = self.install_root();
@@ -2989,17 +3052,139 @@ mod tests {
29893052
fn default_bundled_root_loads_repo_bundles_as_installed_plugins() {
29903053
let _guard = env_guard();
29913054
let config_home = temp_dir("default-bundled-home");
2992-
let manager = PluginManager::new(PluginManagerConfig::new(&config_home));
3055+
3056+
// Use the repo bundled path explicitly so the test is reliable regardless
3057+
// of where the binary runs from.
3058+
let repo_bundled = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("bundled");
3059+
let mut config = PluginManagerConfig::new(&config_home);
3060+
config.bundled_root = Some(repo_bundled.clone());
3061+
let manager = PluginManager::new(config);
3062+
3063+
if repo_bundled.exists() {
3064+
let installed = manager
3065+
.list_installed_plugins()
3066+
.expect("bundled plugins should auto-install from repo path");
3067+
assert!(installed
3068+
.iter()
3069+
.any(|plugin| plugin.metadata.id == "example-bundled@bundled"));
3070+
assert!(installed
3071+
.iter()
3072+
.any(|plugin| plugin.metadata.id == "sample-hooks@bundled"));
3073+
}
3074+
3075+
let _ = fs::remove_dir_all(config_home);
3076+
}
3077+
3078+
#[test]
3079+
fn default_bundled_root_is_not_blindly_cargo_manifest_dir() {
3080+
// Verify that bundled_root() no longer unconditionally returns
3081+
// CARGO_MANIFEST_DIR/bundled. The returned path must either exist
3082+
// (a valid runtime or dev location was found) OR differ from the
3083+
// compile-time source path (a runtime-relative default was chosen).
3084+
let resolved = PluginManager::bundled_root();
3085+
let compile_time_path = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("bundled");
3086+
3087+
// If the compile-time path does not exist (e.g. installed binary running
3088+
// outside the source tree), the resolved path must NOT be the CARGO_MANIFEST_DIR
3089+
// path, because that would re-introduce the original bug.
3090+
if !compile_time_path.exists() {
3091+
assert_ne!(
3092+
resolved, compile_time_path,
3093+
"bundled_root() must not fall back to CARGO_MANIFEST_DIR when that path \
3094+
does not exist — this would regress the root-owned-dir permission bug"
3095+
);
3096+
}
3097+
// Either the path exists (dev scenario) or we got a runtime-relative path.
3098+
// Either way the function should not panic or return an obviously wrong value.
3099+
assert!(
3100+
!resolved.as_os_str().is_empty(),
3101+
"bundled_root() should return a non-empty path"
3102+
);
3103+
}
3104+
3105+
#[test]
3106+
fn override_bundled_root_is_used_exactly() {
3107+
let _guard = env_guard();
3108+
let config_home = temp_dir("override-bundled-home");
3109+
let bundled_root = temp_dir("override-bundled-root");
3110+
write_bundled_plugin(
3111+
&bundled_root.join("override-plugin"),
3112+
"override-plugin",
3113+
"1.0.0",
3114+
false,
3115+
);
3116+
3117+
let mut config = PluginManagerConfig::new(&config_home);
3118+
config.bundled_root = Some(bundled_root.clone());
3119+
let manager = PluginManager::new(config);
29933120

29943121
let installed = manager
29953122
.list_installed_plugins()
2996-
.expect("default bundled plugins should auto-install");
2997-
assert!(installed
2998-
.iter()
2999-
.any(|plugin| plugin.metadata.id == "example-bundled@bundled"));
3000-
assert!(installed
3001-
.iter()
3002-
.any(|plugin| plugin.metadata.id == "sample-hooks@bundled"));
3123+
.expect("override bundled_root should be used");
3124+
assert!(
3125+
installed
3126+
.iter()
3127+
.any(|plugin| plugin.metadata.id == "override-plugin@bundled"),
3128+
"only the override bundled root should be scanned, not CARGO_MANIFEST_DIR"
3129+
);
3130+
3131+
let _ = fs::remove_dir_all(config_home);
3132+
let _ = fs::remove_dir_all(bundled_root);
3133+
}
3134+
3135+
#[test]
3136+
fn explicit_nonexistent_bundled_root_does_not_fail() {
3137+
// When bundled_root is explicitly configured to a path that does not exist,
3138+
// plugin list should succeed with an empty bundled section rather than
3139+
// returning an error (discover_plugin_dirs treats NotFound as empty).
3140+
let _guard = env_guard();
3141+
let config_home = temp_dir("missing-bundled-home");
3142+
3143+
let nonexistent = temp_dir("nonexistent-bundled-XXXXXXXX");
3144+
assert!(
3145+
!nonexistent.exists(),
3146+
"test precondition: path must not exist"
3147+
);
3148+
3149+
let mut config = PluginManagerConfig::new(&config_home);
3150+
config.bundled_root = Some(nonexistent);
3151+
let manager = PluginManager::new(config);
3152+
3153+
// Should succeed with zero bundled plugins, not crash with ENOENT.
3154+
let result = manager.list_installed_plugins();
3155+
assert!(
3156+
result.is_ok(),
3157+
"nonexistent explicit bundled root should not fail: {result:?}"
3158+
);
3159+
let installed = result.unwrap();
3160+
assert!(
3161+
installed
3162+
.iter()
3163+
.all(|p| p.metadata.kind != PluginKind::Bundled),
3164+
"no bundled plugins should be installed when bundled root path does not exist"
3165+
);
3166+
3167+
let _ = fs::remove_dir_all(config_home);
3168+
}
3169+
3170+
#[test]
3171+
fn no_bundled_root_config_uses_auto_detection_without_panic() {
3172+
// When bundled_root is not set (None), auto-detection runs. The resolved
3173+
// path should either exist (dev environment) or be a runtime-relative path
3174+
// that doesn't cause a panic or EACCES crash.
3175+
let _guard = env_guard();
3176+
let config_home = temp_dir("auto-detect-bundled-home");
3177+
3178+
// No bundled_root set — forces auto-detection in bundled_root().
3179+
let config = PluginManagerConfig::new(&config_home);
3180+
let manager = PluginManager::new(config);
3181+
3182+
// Should not panic or return a hard IO error.
3183+
let result = manager.list_installed_plugins();
3184+
assert!(
3185+
result.is_ok(),
3186+
"auto-detected bundled root resolution must not fail: {result:?}"
3187+
);
30033188

30043189
let _ = fs::remove_dir_all(config_home);
30053190
}

0 commit comments

Comments
 (0)