fix(plugins): preserve uninstall retries and bound mock private paths (#2073)

Related #1947

Uninstall keeps a plugin registered but stopped until its private data, secrets and package are cleaned up; a failure is shown and Uninstall can be retried, also after a restart (a durable cleanup record outside the package restores the target). While cleanup is pending, enable, reload, consent, permission changes, reinstall, replacement and rollback are refused. Secret-deletion errors are reported, and cleanup never follows a symlinked ancestor outside plugin storage. The SDK mock checks private-path components and rejects traversal independently of filesystem grants.

Co-authored-by: arnaud-ritti <[email protected]>
This commit is contained in:
Arnaud Ritti authored and GitHub committed 2026-11-08 08:29:19 +08:00
1 parent a2599a6f79
commit 99102d8a40
73 files changed
+794 -49

No files matched your search

+221
View File
@@ -24,6 +24,134 @@ pub const ACTIVE_FILE: &str = "active";
/// `<root>/previous`: the version an update replaced (for rollback).
pub const PREVIOUS_FILE: &str = "previous";
#[derive(serde::Serialize, serde::Deserialize)]
pub(super) struct PendingUninstall {
pub id: String,
pub name: String,
pub version: String,
pub flat: bool,
pub secrets: Vec<String>,
#[serde(default)]
pub error: Option<String>,
}
impl PendingUninstall {
fn validate(&self) -> io::Result<()> {
if self.id.is_empty()
|| self.id.starts_with('.')
|| Path::new(&self.id).components().count() != 1
|| !matches!(Path::new(&self.id).components().next(), Some(std::path::Component::Normal(_)))
{
return Err(io::ErrorKind::InvalidData.into());
}
version_dir(&self.version)?;
Ok(())
}
pub(super) fn package_dir(&self, root: &Path) -> PathBuf {
let dir = root.join(&self.id);
if self.flat { dir } else { dir.join(&self.version) }
}
}
pub(super) fn check_not_symlink(path: &Path) -> io::Result<()> {
match std::fs::symlink_metadata(path) {
Ok(meta) => {
#[cfg(windows)]
let reparse = {
use std::os::windows::fs::MetadataExt;
meta.file_attributes() & windows_sys::Win32::Storage::FileSystem::FILE_ATTRIBUTE_REPARSE_POINT != 0
};
#[cfg(not(windows))]
let reparse = false;
if meta.file_type().is_symlink() || reparse {
Err(io::Error::other("refusing symlink in plugin cleanup path"))
} else {
Ok(())
}
}
Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()),
Err(e) => Err(e),
}
}
pub(super) fn check_cleanup_parent(root: &Path, path: &Path) -> io::Result<()> {
let relative = path.strip_prefix(root).map_err(io::Error::other)?;
let mut parent = root.to_path_buf();
for part in relative.parent().unwrap_or(Path::new("")).components() {
if !matches!(part, std::path::Component::Normal(_)) {
return Err(io::Error::other("invalid plugin cleanup path"));
}
parent.push(part);
check_not_symlink(&parent)?;
}
Ok(())
}
fn uninstall_file(root: &Path, id: &str) -> PathBuf {
root.join(".uninstall").join(format!("{id}.json"))
}
pub(super) fn remember_uninstall(root: &Path, pending: &PendingUninstall, dir: &Path) -> io::Result<()> {
use std::io::Write;
pending.validate()?;
if pending.package_dir(root) != dir {
return Err(io::ErrorKind::InvalidData.into());
}
let file = uninstall_file(root, &pending.id);
check_not_symlink(file.parent().unwrap())?;
std::fs::create_dir_all(file.parent().unwrap())?;
check_not_symlink(&file)?;
let mut out = tempfile::NamedTempFile::new_in(file.parent().unwrap())?;
out.write_all(&serde_json::to_vec(pending)?)?;
out.as_file().sync_all()?;
out.persist(file).map(|_| ()).map_err(|e| e.error)
}
pub(super) fn pending_uninstalls(root: &Path) -> Vec<PendingUninstall> {
if check_not_symlink(&root.join(".uninstall")).is_err() {
return Vec::new();
}
let Ok(entries) = std::fs::read_dir(root.join(".uninstall")) else { return Vec::new() };
entries
.flatten()
.filter_map(|entry| {
let path = entry.path();
if path.extension().is_none_or(|ext| ext != "json") {
return None;
}
let read = || -> io::Result<PendingUninstall> {
check_not_symlink(&path)?;
let pending: PendingUninstall = serde_json::from_slice(&std::fs::read(&path)?)?;
pending.validate()?;
if path != uninstall_file(root, &pending.id) {
return Err(io::Error::from(io::ErrorKind::InvalidData));
}
Ok(pending)
};
match read() {
Ok(pending) => Some(pending),
Err(e) => {
tracing::warn!(path = %path.display(), "cannot recover pending plugin uninstall: {e}");
None
}
}
})
.collect()
}
pub(super) fn uninstall_pending(root: &Path, id: &str) -> io::Result<bool> {
uninstall_file(root, id).try_exists()
}
pub(super) fn forget_uninstall(root: &Path, id: &str) -> io::Result<()> {
check_not_symlink(&root.join(".uninstall"))?;
match std::fs::remove_file(uninstall_file(root, id)) {
Err(e) if e.kind() == io::ErrorKind::NotFound => Ok(()),
result => result,
}
}
fn nonce() -> String {
let n = SystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_nanos()).unwrap_or_default();
format!("{n:x}-{}", std::process::id())
@@ -219,6 +347,99 @@ pub fn recover(dir: &Path) {
mod tests {
use super::*;
#[test]
fn pending_uninstall_records_reject_paths_outside_the_package() {
let tmp = tempfile::tempdir().unwrap();
let mut pending = PendingUninstall {
id: "p".into(),
name: "Plugin".into(),
version: "1.0.0".into(),
flat: false,
secrets: vec!["key".into()],
error: None,
};
remember_uninstall(tmp.path(), &pending, &tmp.path().join("p/1.0.0")).unwrap();
assert_eq!(pending_uninstalls(tmp.path())[0].secrets, ["key"]);
for (id, version) in [
("../outside", "1"),
("/outside", "1"),
(".data", "p"),
("other", "1"),
("p", "../outside"),
("p", "1/nested"),
] {
pending.id = id.into();
pending.version = version.into();
std::fs::write(uninstall_file(tmp.path(), "p"), serde_json::to_vec(&pending).unwrap()).unwrap();
assert!(pending_uninstalls(tmp.path()).is_empty(), "{id}/{version}");
}
pending.id = "invalid-1-0-0".into();
assert!(remember_uninstall(tmp.path(), &pending, &tmp.path().join("other/1.0.0")).is_err());
forget_uninstall(tmp.path(), "p").unwrap();
assert!(pending_uninstalls(tmp.path()).is_empty());
}
#[cfg(unix)]
#[test]
fn pending_uninstall_records_do_not_follow_symlinks() {
use std::os::unix::fs::symlink;
let tmp = tempfile::tempdir().unwrap();
let outside = tempfile::tempdir().unwrap();
let pending = PendingUninstall {
id: "p".into(),
name: "Plugin".into(),
version: "1.0.0".into(),
flat: false,
secrets: Vec::new(),
error: None,
};
let record = outside.path().join("p.json");
std::fs::write(&record, "untouched").unwrap();
let parent = tmp.path().join(".uninstall");
symlink(outside.path(), &parent).unwrap();
assert!(remember_uninstall(tmp.path(), &pending, &tmp.path().join("p/1.0.0")).is_err());
assert!(forget_uninstall(tmp.path(), "p").is_err());
assert!(pending_uninstalls(tmp.path()).is_empty());
std::fs::remove_file(&parent).unwrap();
std::fs::create_dir(&parent).unwrap();
symlink(&record, uninstall_file(tmp.path(), "p")).unwrap();
assert!(remember_uninstall(tmp.path(), &pending, &tmp.path().join("p/1.0.0")).is_err());
assert!(pending_uninstalls(tmp.path()).is_empty());
forget_uninstall(tmp.path(), "p").unwrap();
symlink(&record, parent.join("p.tmp")).unwrap();
remember_uninstall(tmp.path(), &pending, &tmp.path().join("p/1.0.0")).unwrap();
assert_eq!(pending_uninstalls(tmp.path()).len(), 1);
assert_eq!(std::fs::read_to_string(&record).unwrap(), "untouched");
}
#[cfg(windows)]
#[test]
fn pending_uninstall_records_do_not_follow_junctions() {
let tmp = tempfile::tempdir().unwrap();
let outside = tempfile::tempdir().unwrap();
let record = outside.path().join("p.json");
std::fs::write(&record, "untouched").unwrap();
let status = std::process::Command::new("cmd")
.args(["/C", "mklink", "/J"])
.arg(tmp.path().join(".uninstall"))
.arg(outside.path())
.status()
.unwrap();
assert!(status.success());
let pending = PendingUninstall {
id: "p".into(),
name: "Plugin".into(),
version: "1.0.0".into(),
flat: false,
secrets: Vec::new(),
error: None,
};
assert!(remember_uninstall(tmp.path(), &pending, &tmp.path().join("p/1.0.0")).is_err());
assert!(forget_uninstall(tmp.path(), "p").is_err());
assert!(pending_uninstalls(tmp.path()).is_empty());
assert_eq!(std::fs::read_to_string(record).unwrap(), "untouched");
}
#[test]
fn failed_swap_keeps_the_old_version() {
let tmp = tempfile::tempdir().unwrap();
+152 -12
View File
@@ -115,6 +115,7 @@ struct Links {
}
struct Meta {
uninstalling: bool,
status: PluginStatus,
error: Option<String>,
warnings: Vec<String>,
@@ -142,6 +143,7 @@ struct Meta {
impl Meta {
fn new() -> Self {
Self {
uninstalling: false,
status: PluginStatus::Disabled,
error: None,
warnings: Vec::new(),
@@ -492,6 +494,47 @@ impl PluginHost {
/// Instantiates no Wasm: code runs on its activation events.
pub fn load_all(&self) {
let _guard = self.lifecycle.lock();
for pending in install::pending_uninstalls(&self.plugins_dir) {
if self.plugins.read().contains_key(&pending.id) {
continue;
}
let dir = pending.package_dir(&self.plugins_dir);
let mut manifest = placeholder_manifest(&dir);
manifest.id = pending.id.clone();
manifest.name = pending.name;
manifest.version = pending.version;
manifest.contributes.insert(
providers::SECRETS_POINT.into(),
toml::Value::Array(
pending
.secrets
.into_iter()
.map(|name| {
toml::Value::Table(toml::Table::from_iter([("name".into(), toml::Value::String(name))]))
})
.collect(),
),
);
let mut meta = Meta::new();
meta.status = PluginStatus::Failed;
meta.uninstalling = true;
meta.error = Some(
pending.error.unwrap_or_else(|| "Uninstall incomplete; retry Uninstall to finish cleanup.".into()),
);
self.plugins.write().insert(
pending.id,
Arc::new(LoadedPlugin {
pkg: Package::new(manifest, dir),
icon: None,
invalid: false,
source: PluginSource::Installed,
dev_root: None,
meta: Mutex::new(meta),
batches: Arc::default(),
runtime: Mutex::new(None),
}),
);
}
let dirs = package_dirs(&self.plugins_dir);
self.migrate_grants(&dirs);
for dir in dirs {
@@ -757,6 +800,7 @@ impl PluginHost {
pub fn set_enabled(&self, id: &str, enabled: bool) -> IdeResult<()> {
let _guard = self.lifecycle.lock();
let plugin = self.get(id)?;
self.check_not_uninstalling(&plugin)?;
{
let mut state = self.state.lock();
if enabled {
@@ -780,6 +824,7 @@ impl PluginHost {
pub fn reload(&self, id: &str) -> IdeResult<PluginInfo> {
let _guard = self.lifecycle.lock();
let plugin = self.get(id)?;
self.check_not_uninstalling(&plugin)?;
let (dir, source, root) = (plugin.dir().to_path_buf(), plugin.source, plugin.dev_root.clone());
self.unload(id);
let new_id = self.load_dir(&dir, source, root);
@@ -841,6 +886,11 @@ impl PluginHost {
let _guard = self.lifecycle.lock();
let (pkg, set) = self.validate_package(src)?;
let id = pkg.id().to_owned();
if let Some(plugin) = self.plugins.read().get(&id) {
self.check_not_uninstalling(plugin)?;
} else {
self.check_uninstall_pending(&id)?;
}
if let Some(existing) = self.plugins.read().get(&id)
&& existing.source != PluginSource::Installed
{
@@ -915,6 +965,13 @@ impl PluginHost {
/// Makes `previous` the active version of `id` again and drops `failed`
/// (its directory, its consent). Called with the plugin unloaded or not.
fn roll_back(&self, id: &str, previous: &str, failed: &str, grant: Option<ledger::Grant>) {
let ready = match self.get(id) {
Ok(plugin) => self.check_not_uninstalling(&plugin),
Err(_) => self.check_uninstall_pending(id),
};
if ready.is_err() {
return;
}
let root = self.plugins_dir.join(id);
self.unload(id);
if let Err(e) = install::activate_version(&root, previous) {
@@ -969,6 +1026,7 @@ impl PluginHost {
pub fn roll_back_update(&self, id: &str) -> IdeResult<PluginInfo> {
let _guard = self.lifecycle.lock();
let plugin = self.get(id)?;
self.check_not_uninstalling(&plugin)?;
if plugin.source != PluginSource::Installed {
return Err(IdeError::invalid("only installed plugins have versions to roll back to"));
}
@@ -987,6 +1045,7 @@ impl PluginHost {
pub fn grant(&self, id: &str, consent: &Consent) -> IdeResult<PluginInfo> {
let _guard = self.lifecycle.lock();
let plugin = self.get(id)?;
self.check_not_uninstalling(&plugin)?;
if plugin.invalid {
return Err(IdeError::invalid("the plugin's manifest is invalid"));
}
@@ -1007,6 +1066,7 @@ impl PluginHost {
pub fn set_capability(&self, id: &str, capability: &str, granted: bool) -> IdeResult<PluginInfo> {
let _guard = self.lifecycle.lock();
let plugin = self.get(id)?;
self.check_not_uninstalling(&plugin)?;
let cap: Capability = capability.parse().map_err(IdeError::invalid)?;
if !plugin.manifest().declares(&cap) && !plugin.meta.lock().declared_extra.contains(&cap) {
return Err(IdeError::invalid(format!("plugin {id:?} does not declare {cap}")));
@@ -1045,6 +1105,13 @@ impl PluginHost {
.map_err(|e| IdeError::new(ide_core::error::ErrorKind::PermissionDenied, e))
}
fn check_uninstall_pending(&self, id: &str) -> IdeResult<()> {
if install::uninstall_pending(&self.plugins_dir, id)? {
return Err(IdeError::unavailable(std::io::Error::from(std::io::ErrorKind::ResourceBusy).to_string()));
}
Ok(())
}
pub fn uninstall(&self, id: &str) -> IdeResult<()> {
let _guard = self.lifecycle.lock();
let plugin = self.get(id)?;
@@ -1063,25 +1130,65 @@ impl PluginHost {
.flatten()
.filter_map(|s| s.get("name")?.as_str().map(str::to_owned))
.collect();
self.unload(id);
let mut pending = install::PendingUninstall {
id: id.to_owned(),
name: plugin.manifest().name.clone(),
version: plugin.manifest().version.clone(),
flat: plugin.dir() == self.plugins_dir.join(id),
secrets: secrets.clone(),
error: None,
};
// A broken manifest has no trusted identity to journal; keep its existing removal path.
if !plugin.invalid {
install::remember_uninstall(&self.plugins_dir, &pending, plugin.dir())?;
}
// Retain registration for retry, but prevent concurrent on-demand activation.
{
let _runtime = plugin.runtime.lock();
let mut meta = plugin.meta.lock();
meta.uninstalling = true;
meta.status = PluginStatus::Disabled;
}
self.stop(&plugin);
let cleanup = || -> std::io::Result<()> {
install::check_cleanup_parent(&self.plugins_dir, &self.shared.data_dir.join(id))?;
install::check_cleanup_parent(&self.plugins_dir, plugin.dir())?;
install::check_not_symlink(&self.shared.data_dir.join(id))?;
for file in ["secrets.json", "secrets.tmp"] {
install::check_not_symlink(&self.shared.data_dir.join(id).join(file))?;
}
let store = self.shared.secrets.read().clone();
for name in &secrets {
if let Err(e) = store.set(id, name, None) {
tracing::warn!(plugin = %id, "cannot delete secret {name:?}: {e}");
return Err(std::io::Error::other(format!("cannot delete secret {name:?}: {e}")));
}
}
// Keep the installed package and registration available until cleanup succeeds.
let data = self.shared.data_dir.join(id);
match std::fs::remove_dir_all(&data) {
Ok(()) => {}
Err(e) if e.kind() == std::io::ErrorKind::NotFound && !data.exists() => {}
Err(e) => return Err(e),
}
// Every version (`<id>/<version>`), or the flat directory of an older install.
let target = self.plugins_dir.join(id);
match std::fs::remove_dir_all(&target) {
Err(e) if e.kind() == std::io::ErrorKind::NotFound && !target.exists() => Ok(()),
result => result,
}
};
if let Err(e) = cleanup() {
pending.error = Some(e.to_string());
if !plugin.invalid
&& let Err(record_error) = install::remember_uninstall(&self.plugins_dir, &pending, plugin.dir())
{
tracing::warn!(plugin = %id, "cannot save uninstall error: {record_error}");
}
self.set_failed(&plugin, e.to_string());
self.changed();
return Err(e.into());
}
// Every version (`<id>/<version>`), or the flat directory of an older install.
let root = self.plugins_dir.join(id);
std::fs::remove_dir_all(if root.is_dir() { root } else { plugin.dir().to_path_buf() })?;
self.updates.lock().remove(id);
// Its storage goes with it.
match std::fs::remove_dir_all(self.shared.data_dir.join(id)) {
Ok(()) => {}
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
Err(e) => return Err(e.into()),
}
{
let mut ledger = self.ledger.lock();
ledger.remove(&plugin.subject());
@@ -1095,6 +1202,12 @@ impl PluginHost {
}
}
self.save_state();
if let Err(e) = install::forget_uninstall(&self.plugins_dir, id) {
self.set_failed(&plugin, e.to_string());
self.changed();
return Err(e.into());
}
self.unload(id);
self.changed();
Ok(())
}
@@ -2066,7 +2179,17 @@ impl PluginHost {
fn load_dir(&self, dir: &Path, source: PluginSource, dev_root: Option<PathBuf>) -> String {
let (manifest, error) = match Manifest::load(dir) {
Ok(m) => (m, None),
Err(e) => (placeholder_manifest(dir), Some(e)),
Err(e) => {
let mut manifest = placeholder_manifest(dir);
// Installed directory names retain identity when discovery metadata is damaged.
if source == PluginSource::Installed
&& let Ok(relative) = dir.strip_prefix(&self.plugins_dir)
&& let Some(name) = relative.components().next().and_then(|p| p.as_os_str().to_str())
{
manifest.id = name.to_owned();
}
(manifest, Some(e))
}
};
let invalid = error.is_some();
let id = manifest.id.clone();
@@ -2116,9 +2239,25 @@ impl PluginHost {
self.stop(&plugin);
}
fn check_not_uninstalling(&self, plugin: &LoadedPlugin) -> IdeResult<()> {
if plugin.meta.lock().uninstalling {
return Err(IdeError::unavailable("Uninstall incomplete; retry Uninstall to finish cleanup."));
}
if plugin.source == PluginSource::Installed {
self.check_uninstall_pending(plugin.id())?;
}
Ok(())
}
/// Validates, checks grants and commits the contributions; activates code
/// whose activation event already fired.
fn start(&self, plugin: &Arc<LoadedPlugin>) {
if let Err(e) = self.check_not_uninstalling(plugin) {
if !plugin.meta.lock().uninstalling {
self.set_failed(plugin, e.message);
}
return;
}
self.registry.dispose_plugin(plugin.id());
self.clear_provider_diagnostics(plugin.id());
if plugin.source == PluginSource::Installed
@@ -2178,6 +2317,7 @@ impl PluginHost {
/// first if needed. Returns whether it was activated now.
fn ensure_active(&self, plugin: &Arc<LoadedPlugin>, client: Option<&str>) -> IdeResult<(Arc<Actor>, bool)> {
let mut runtime = plugin.runtime.lock();
self.check_not_uninstalling(plugin)?;
if let Some(actor) = runtime.as_ref() {
if actor.is_alive() {
return Ok((actor.clone(), false));
+299
View File
@@ -834,6 +834,305 @@ fn private_directory_survives_host_restart_and_inactive_uninstall() {
assert!(!dir.exists());
}
#[test]
fn private_uninstall_cleanup_failure_is_retryable() {
for fail_data in [true, false] {
let f = fixture(Limits::default());
let id = "test.private-dir";
f.host.install_from_dir(&sample("private-dir"), &Consent::default()).unwrap();
f.run(id, "roundtrip", None).unwrap();
let dir = PathBuf::from(f.run(id, "directory", None).unwrap().unwrap());
let target = if fail_data { dir.parent().unwrap().to_path_buf() } else { f.plugins_dir.join(id) };
let backup = target.with_extension("blocked-backup");
// A regular file deterministically makes remove_dir_all fail, even as root.
std::fs::rename(&target, &backup).unwrap();
std::fs::write(&target, "controlled cleanup failure").unwrap();
assert!(f.host.uninstall(id).is_err());
assert!(f.host.info_of(id).is_ok(), "failed uninstall must retain its target");
assert_eq!(f.status(id), PluginStatus::Failed);
assert!(f.command_titles(id).is_empty());
std::fs::remove_file(&target).unwrap();
std::fs::rename(&backup, &target).unwrap();
if fail_data {
assert!(f.installed(id).join("plugin.toml").exists());
f.host.shutdown();
let host = PluginHost::new(
f.plugins_dir.clone(),
Arc::new(|_: Option<&str>, _: PluginEvent| {}),
Limits::default(),
)
.unwrap();
host.load_all();
host.uninstall(id).unwrap();
} else {
f.host.uninstall(id).unwrap();
}
assert!(!dir.exists());
assert!(!f.plugins_dir.join(id).exists());
}
}
#[test]
fn private_uninstall_recovers_after_package_metadata_is_removed() {
for remove_entire_package in [false, true] {
let f = fixture(Limits::default());
let id = "test.private-dir";
f.install(&sample("private-dir"));
f.run(id, "roundtrip", None).unwrap();
let package = f.installed(id);
let root = f.plugins_dir.join(id);
let backup = root.with_extension("blocked-backup");
std::fs::rename(&root, &backup).unwrap();
std::fs::write(&root, "controlled package cleanup failure").unwrap();
assert!(f.host.uninstall(id).is_err());
std::fs::remove_file(&root).unwrap();
std::fs::rename(&backup, &root).unwrap();
// Reproduce the metadata loss a partial recursive deletion can leave behind.
if remove_entire_package {
std::fs::remove_dir_all(&root).unwrap();
} else {
std::fs::remove_file(root.join("active")).unwrap();
std::fs::remove_file(package.join("plugin.toml")).unwrap();
}
if remove_entire_package {
// An interrupted cleanup (and older records) has no saved failure yet.
let record = f.plugins_dir.join(".uninstall").join(format!("{id}.json"));
let mut value: serde_json::Value = serde_json::from_slice(&std::fs::read(&record).unwrap()).unwrap();
value.as_object_mut().unwrap().remove("error");
std::fs::write(record, serde_json::to_vec(&value).unwrap()).unwrap();
}
f.host.shutdown();
let host =
PluginHost::new(f.plugins_dir.clone(), Arc::new(|_: Option<&str>, _: PluginEvent| {}), Limits::default())
.unwrap();
host.load_all();
assert!(host.info_of(id).is_ok(), "partial package cleanup must retain a restartable uninstall target");
assert!(host.info_of(id).unwrap().error.is_some());
assert_eq!(host.info_of(id).unwrap().status, PluginStatus::Failed);
assert!(host.reload(id).is_err());
assert!(host.set_enabled(id, true).is_err());
assert_eq!(
host.install_from_dir(&sample("private-dir"), &Consent::all()).unwrap_err().kind,
ErrorKind::Unavailable
);
host.uninstall(id).unwrap();
assert!(host.info_of(id).is_err());
assert!(!root.exists());
assert!(!f.plugins_dir.join(".uninstall").join(format!("{id}.json")).exists());
host.install_from_dir(&sample("private-dir"), &Consent::all()).unwrap();
host.uninstall(id).unwrap();
assert!(!root.exists());
}
}
#[test]
fn private_uninstall_cleans_up_a_package_with_a_damaged_manifest() {
let f = fixture(Limits::default());
let id = "test.private-dir";
f.install(&sample("private-dir"));
f.run(id, "roundtrip", None).unwrap();
let dir = PathBuf::from(f.run(id, "directory", None).unwrap().unwrap());
let package = f.installed(id);
f.host.shutdown();
std::fs::write(package.join("plugin.toml"), "broken = [").unwrap();
let host =
PluginHost::new(f.plugins_dir.clone(), Arc::new(|_: Option<&str>, _: PluginEvent| {}), Limits::default())
.unwrap();
host.load_all();
assert_eq!(host.info_of(id).unwrap().status, PluginStatus::Failed);
host.uninstall(id).unwrap();
assert!(!dir.exists());
assert!(!f.plugins_dir.join(id).exists());
let broken = f.plugins_dir.join("Broken Package");
std::fs::create_dir(&broken).unwrap();
std::fs::write(broken.join("plugin.toml"), "broken = [").unwrap();
let other = f.plugins_dir.join("invalid-broken-package");
std::fs::create_dir(&other).unwrap();
std::fs::write(other.join("keep.txt"), "untouched").unwrap();
host.load_all();
host.uninstall("Broken Package").unwrap();
assert!(!broken.exists());
assert_eq!(std::fs::read_to_string(other.join("keep.txt")).unwrap(), "untouched");
}
#[test]
fn private_uninstall_cannot_be_restarted_through_lifecycle_actions() {
let f = fixture(Limits::default());
let id = "test.private-dir";
f.install(&sample("private-dir"));
f.run(id, "roundtrip", None).unwrap();
let dir = PathBuf::from(f.run(id, "directory", None).unwrap().unwrap());
let target = dir.parent().unwrap();
let backup = target.with_extension("blocked-backup");
std::fs::rename(target, &backup).unwrap();
std::fs::write(target, "controlled cleanup failure").unwrap();
assert!(f.host.uninstall(id).is_err());
let error = f.host.info_of(id).unwrap().error.unwrap();
for restart in [false, true] {
let recovered;
let host = if restart {
f.host.shutdown();
recovered = PluginHost::new(
f.plugins_dir.clone(),
Arc::new(|_: Option<&str>, _: PluginEvent| {}),
Limits::default(),
)
.unwrap();
recovered.load_all();
&recovered
} else {
&f.host
};
assert_eq!(host.info_of(id).unwrap().error.as_deref(), Some(error.as_str()));
assert!(host.set_enabled(id, true).is_err());
assert!(host.set_enabled(id, false).is_err());
assert!(host.reload(id).is_err());
assert!(host.grant(id, &Consent::default()).is_err());
assert!(host.set_capability(id, "project.read", true).is_err());
assert!(host.install_from_dir(&sample("private-dir"), &Consent::default()).is_err());
assert!(host.roll_back_update(id).is_err());
host.startup_finished();
assert_eq!(host.info_of(id).unwrap().status, PluginStatus::Failed);
assert!(host.run_command(id, "roundtrip", None, Invocation::default()).is_err());
if restart {
std::fs::remove_file(target).unwrap();
std::fs::rename(&backup, target).unwrap();
host.uninstall(id).unwrap();
assert!(!dir.exists());
}
}
}
#[test]
fn private_uninstall_retains_failed_secret_cleanup_for_retry() {
struct FailingSecrets;
impl SecretStore for FailingSecrets {
fn get(&self, _: &str, _: &str) -> Option<String> {
None
}
fn set(&self, _: &str, _: &str, _: Option<&str>) -> Result<(), String> {
Err("keychain locked".into())
}
}
let f = fixture(Limits::default());
let id = "test.private-dir";
let pkg = f._tmp.path().join("private-with-secret");
copy_dir(&sample("private-dir"), &pkg);
let manifest = pkg.join("plugin.toml");
let text = std::fs::read_to_string(&manifest).unwrap().replace("required = []", "required = [\"secrets\"]");
std::fs::write(
manifest,
format!("{text}\n[[contributes.secrets]]\nname = \"cleanup-test\"\nlabel = \"Cleanup test\"\n"),
)
.unwrap();
f.install(&pkg);
f.host.set_secret_store(Arc::new(FailingSecrets));
assert!(f.host.uninstall(id).is_err());
assert!(f.host.info_of(id).unwrap().error.unwrap().contains("keychain locked"));
assert!(f.installed(id).join("plugin.toml").exists());
f.host.shutdown();
let host =
PluginHost::new(f.plugins_dir.clone(), Arc::new(|_: Option<&str>, _: PluginEvent| {}), Limits::default())
.unwrap();
host.load_all();
assert!(host.info_of(id).unwrap().error.unwrap().contains("keychain locked"));
host.uninstall(id).unwrap();
assert!(host.info_of(id).is_err());
}
#[cfg(unix)]
#[test]
fn private_uninstall_does_not_follow_cleanup_symlinks() {
use std::os::unix::fs::symlink;
for relative in [".data", ".data/test.private-dir", "test.private-dir"] {
let f = fixture(Limits::default());
let id = "test.private-dir";
f.install(&sample("private-dir"));
f.run(id, "roundtrip", None).unwrap();
let target = f.plugins_dir.join(relative);
let backup = target.with_extension("blocked-backup");
std::fs::rename(&target, &backup).unwrap();
let outside = f._tmp.path().join("outside");
copy_dir(&backup, &outside);
std::fs::write(outside.join("keep.txt"), "untouched").unwrap();
symlink(&outside, &target).unwrap();
assert!(f.host.uninstall(id).is_err(), "{relative}");
assert_eq!(std::fs::read_to_string(outside.join("keep.txt")).unwrap(), "untouched");
std::fs::remove_file(&target).unwrap();
std::fs::rename(&backup, &target).unwrap();
f.host.uninstall(id).unwrap();
assert!(f.host.info_of(id).is_err());
assert_eq!(std::fs::read_to_string(outside.join("keep.txt")).unwrap(), "untouched");
}
let f = fixture(Limits::default());
let id = "test.private-dir";
f.install(&sample("private-dir"));
let outside = f._tmp.path().join("outside");
std::fs::create_dir(&outside).unwrap();
std::fs::write(outside.join("keep.txt"), "untouched").unwrap();
let nested = f.plugins_dir.join(".data").join(id).join("nested");
std::fs::create_dir_all(nested.parent().unwrap()).unwrap();
symlink(&outside, nested).unwrap();
f.host.uninstall(id).unwrap();
assert_eq!(std::fs::read_to_string(outside.join("keep.txt")).unwrap(), "untouched");
for file in ["secrets.json", "secrets.tmp"] {
let f = fixture(Limits::default());
f.install(&sample("private-dir"));
let outside = f._tmp.path().join("outside-secret");
std::fs::write(&outside, "untouched").unwrap();
let target = f.plugins_dir.join(".data").join(id).join(file);
std::fs::create_dir_all(target.parent().unwrap()).unwrap();
symlink(&outside, &target).unwrap();
assert!(f.host.uninstall(id).is_err());
assert_eq!(std::fs::read_to_string(&outside).unwrap(), "untouched");
std::fs::remove_file(target).unwrap();
f.host.uninstall(id).unwrap();
assert_eq!(std::fs::read_to_string(&outside).unwrap(), "untouched");
}
}
#[test]
fn private_uninstall_blocks_inactive_reactivation_during_cleanup() {
struct BlockingSecrets {
entered: std::sync::Barrier,
release: std::sync::Barrier,
}
impl SecretStore for BlockingSecrets {
fn get(&self, _: &str, _: &str) -> Option<String> {
None
}
fn set(&self, _: &str, _: &str, _: Option<&str>) -> Result<(), String> {
self.entered.wait();
self.release.wait();
Ok(())
}
}
let f = fixture(Limits::default());
let pkg = f._tmp.path().join("private-with-secret");
copy_dir(&sample("private-dir"), &pkg);
let manifest = pkg.join("plugin.toml");
let text = std::fs::read_to_string(&manifest).unwrap().replace("required = []", "required = [\"secrets\"]");
std::fs::write(
manifest,
format!("{text}\n[[contributes.secrets]]\nname = \"cleanup-test\"\nlabel = \"Cleanup test\"\n"),
)
.unwrap();
f.install(&pkg);
assert_eq!(f.status("test.private-dir"), PluginStatus::Inactive);
let store = Arc::new(BlockingSecrets { entered: std::sync::Barrier::new(2), release: std::sync::Barrier::new(2) });
f.host.set_secret_store(store.clone());
let result = std::thread::scope(|scope| {
let uninstall = scope.spawn(|| f.host.uninstall("test.private-dir"));
store.entered.wait();
let result = f.run("test.private-dir", "roundtrip", None);
store.release.wait();
uninstall.join().unwrap().unwrap();
result
});
assert!(result.is_err(), "plugin must remain stopped during uninstall");
}
#[cfg(unix)]
#[test]
fn private_paths_work_with_open_location_and_approved_process_arguments() {
Binary file not shown.
@@ -1 +1 @@
d9cf3708e558ce4931376b5f8c5b3d9d0771d65100ba2ac2c9d1a5a5cf13e62f
da346bd256a61b971c09632465b187c580809cb7f16faa2588d22c6369cba600
Binary file not shown.
@@ -1 +1 @@
606d02b53a0f51e41b5accb209750d2c768adf56190fe667caa2f90173e84d91
002f86e29e9bf4f5f4adeeedd2889d492dbf41b60900c03c25cd84d6f3d226fb
Binary file not shown.
@@ -1 +1 @@
2bf7c821d304b184c4b02540d170b7ec06e6a40b62436f61bfb97836e39dabab
a2815b273e270d473a80c3d416a41d9d7a44bbc7c87b8aae6668c734e89bac5c
Binary file not shown.
@@ -1 +1 @@
296e84611d0b89399944c98972f8c98378fce49368a16944646f1ed8c1d172a1
88bead9074bf6f2dfa2ceb0756b010b7f3b322e3bd977858f2fac97c840fd56e
Binary file not shown.
@@ -1 +1 @@
5da8fd5058502bf5a4f7a96352e3337cb7fef0d8478ebab335adc7959c027d8b
75145b486069cfb44429127ecb11ad863b9c6a88eff3205248c6ce5cf157d516
Binary file not shown.
@@ -1 +1 @@
dc42bf8cb4af2720ce83f4a5dd1bffdc6315ae7f041b84a2500ee9f735c73677
85d822e6ff64e6a5528d0b09d5ad3817ef77ea0f8a4832337c8958a002179755
Binary file not shown.
@@ -1 +1 @@
f1cb247d29f80c7313432cd660d0c26d50691a64a3d0cae0bc6b3ba09915dbe7
badc9dc4fa12739850a5935b3d13e9a223fa7e348af785c5e164a32c5dda2989
Binary file not shown.
@@ -1 +1 @@
ef09a1ce4ef3a5a28aee01861c7de00bbac568175e880b7c87222a1a0aee3da4
52565fa3ed2d76a4f61f7e870bb74124ca901a4efbc12025039c69aaf9a97844
Binary file not shown.
@@ -1 +1 @@
35647c498dc439bcf5a135d5c21223150977f89511c7bbef652997c95ebc0f59
dcff32b4f59f5f9ad868fd6cb94a8e8e38e4ff4258b70ed3cb0cfd394ae632a3
Binary file not shown.
@@ -1 +1 @@
3463e906eff16c16822848fc02cbf5c8c2282f0ae7849b3a9246b4671eb6b297
91988c62e885b8792028710a460fc47300de19b96bf9ed23748709e5cdcc5cc1
Binary file not shown.
@@ -1 +1 @@
a1361137766e4912126cfc5f6eed3942f455bc9b31e4fbce658df613130ccba3
ebc79bf4f9847003dca269a2870f1ca8d4362102a644ca220dc630c3040fbdf7
Binary file not shown.
@@ -1 +1 @@
791a13024ba10672d4e93d759d232443abc5e2cf9f97cacfe3d1ce93b0d4eeed
fb262393a3c0149719b460bc39d8df656a7e8a52f51257c2e1188e0e620bb3eb
Binary file not shown.
@@ -1 +1 @@
7976a4b3f67e536a24bbdbc9fd6cf23a102669a8aa039c02f9d956329300ff36
78959a803256b5555deadb8952e7410cc661c83dd1046fcf71e2143c2e33d524
Binary file not shown.
@@ -1 +1 @@
9fce102b05a2d11a09b632a9e20185b994a8ab71ca86969754aedef10a4d4985
d16118dd417be01dd23cc2e8693c3d5f5f12b679b72e3c5c5991370fa0b3bccb
Binary file not shown.
@@ -1 +1 @@
3c9cd0d3c519dccf414268cafe5342374009341b38c0ab061e72a9b27976e8c7
c73610ba50bc540f6719865a4864ec647f1e4b5983fc254e886e7c99d3b552f8
Binary file not shown.
@@ -1 +1 @@
1c09ba923ca3b59df4144095c6aba6c03981fc2dc5d208bda3ba0cfa4c4cde51
46d77e5912893a8a70150944b26c9f41f23ffcccaa6666a245a0203fd836f77b
Binary file not shown.
@@ -1 +1 @@
bddfc4fff3e25c611525ebfdee3fb7ab3e4890c394c3970da78f80a12a7a645d
135f065b9832ba8c86ca0fb5ea4b30ae6a4b38c00f178215ce288fee737acbb6
Binary file not shown.
@@ -1 +1 @@
5e1afb744ed0a230648d3dd9c2a9cee8cf7fc53db2f944a1ac580799b7e9ef96
c6f9b2167494532cc201279f2ac4a9c642ac09571197d7101e773c87b0ab19d5
Binary file not shown.
@@ -1 +1 @@
6daecc5ace3661fac8b158d3bde0ecb13b366d8918e28aa846f10858c5c57442
b7e055bda20c99de9385a6c90ee1e2ae57fb9288b84b8c71e3eb9cb536869fc6
Binary file not shown.
@@ -1 +1 @@
6c669b7893f975728d8f7677e9720743b655860383d8a196eb6db76620e28fe1
c61cbdcbb2af15cffdd75f8bc5bd411d849b80cb0cd19227a7632b2770e21f58
Binary file not shown.
@@ -1 +1 @@
cb791f264a947897336ada01f56f81e7bade9a046c701908dfff9852e0cdf15d
c79eefe6730dc50e4c013b80cc1493c89974d9bbcbc7f4278e6136f20dbbffc3
Binary file not shown.
@@ -1 +1 @@
33adfc97d25a9777d3215488dd96dc41d772ed0c71621fb5c78e3c766031d6ae
0dfe2f704cf0ad9b2ab06cfb92a190a19bcf18fe3d5da473a6f05b16002a76ff
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
978b1af84eae4b6ac381e1991241a11d6a416f3f45856bc0ad985ed59435b67d
344df18074d7171c42ae2c245e6ea2beeb7119ed6bf74b76503d241513ddcdba
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
5125e2e47c9adf8043d82fb60b9810fd3edcef272ff4a6c41b2450e19617d8cc
b5f99458a32d54373661e8f61569b1e675e19594275036153b8c52fea74d8f69
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
4fc2c8ca94ee5e45999c68285a124f535cde6f612aac319f04526acceb639339
b6f765692f944b63cb9ec0749a49c9d3be077b0eab709f148ffb0fb058ff1829
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
8cd633605b641e53cb27b61ce7648ab161671a334e6c5fcaba4a7c27c332612b
27a37336df957085a73504c5957033f1b178bcc47c190ffcd234ebad2d900817
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
48525c9df30fce0a443dadbfb85dd6f69e56dadbe9c529cfe4274ef24ac19a3b
99f00d65801c278a6d8a4e4466d0bf71738233c849dfcaf44902d60a557b8757
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
de4355361ad2f4b0950669ca41b17752bc7b67fc24a5eb40a2ffc7e75fe62441
0ace6d1b7f9f3cac0d23f92660d97d6eb6c79174203471970f3e4ca9f7d9b531
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
cba6d696b5984ba62619982042e496c39afcc2e97d8093872a9c7e0522d30b8c
9241c310c638a6a2e5e90e32c0651c76184498d193ad344f762f33a301bcaaab
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
a735b01070abd15165d221f994d712e199a4468f50c637d5c704d13639f2643a
e484774db29b214fb7247c6a403a7c9dbfaa030b0315c353826bf44982f82d61
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
01d43147d97f29798681450bfc75a1ef9cd578ede283012aecc09665234e9bce
ec03327641cc9f32d9205718847939981b3ec5e74e4aa2b800ba1598bccdb70f
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
924d3e7316d6d6e4dd8c371a1485df3215d9b2e71c27e7c683edb613fd1bee1a
baf14df1362b15a0022456d3ea927b9fe07533dabf1fb717888ea98179152464
Binary file not shown.
+1 -1
View File
@@ -1 +1 @@
3ff5852da7219a878efa8dc4d47b69b314d7ccfb1403c2ff2522aaeaba5ea9e2
5c96088e3f31ee74669bb4481fee6c95f51a9d875ad9bed9123f3e0de1a6ead5
+43 -4
View File
@@ -520,10 +520,16 @@ pub mod current {
/// under it: `/a` covers `/a/b` but not `/ab`.
const PLUGIN_DIR: &str = "/openidea/plugin-data/mock/files";
fn require_fs(s: &State, kind: &str, path: &str) -> Result<(), Error> {
if (path == PLUGIN_DIR || path.starts_with(&format!("{PLUGIN_DIR}/")))
fn is_private_path(path: &str) -> bool {
(path == PLUGIN_DIR || path.starts_with(&format!("{PLUGIN_DIR}/")))
&& !path.split(['/', '\\']).any(|part| part == "..")
{
}
fn require_fs(s: &State, kind: &str, path: &str) -> Result<(), Error> {
if path.split(['/', '\\']).any(|part| part == "..") {
return Err(Error::Denied(format!("{kind}:{path}")));
}
if is_private_path(path) {
return Ok(());
}
let prefix = format!("{kind}:");
@@ -1268,7 +1274,7 @@ pub mod current {
pub fn open_location(path: &str, line: u32, column: u32) -> Result<(), Error> {
with(|s| {
if require_fs(s, "fs.read", path).is_err() || !path.starts_with(PLUGIN_DIR) {
if !is_private_path(path) {
require(s, "project.read")?;
}
s.locations.push((path.into(), line, column));
@@ -1785,6 +1791,39 @@ mod tests {
}
}
#[test]
fn private_directory_boundary_and_traversal_ignore_explicit_grants() {
let dir = host::fs::plugin_dir().unwrap();
let adjacent = format!("{dir}-other");
let traversal = format!("{dir}/../other");
let mock = MockHost::install().with_capabilities(&[
&format!("fs.read:{adjacent}"),
&format!("fs.write:{adjacent}"),
&format!("fs.read:{dir}"),
&format!("fs.write:{dir}"),
]);
let file = format!("{adjacent}/fake.txt");
mock.add_external_file(&file, "adjacent");
assert_eq!(host::fs::read_file(&file).unwrap(), "adjacent");
host::fs::write_file(&file, "updated").unwrap();
assert!(host::fs::stat(&file).is_ok());
assert!(host::fs::read_dir(&adjacent).is_ok());
assert_eq!(host::open_location(&file, 0, 0), Err(Error::Denied("project.read".into())));
for path in [dir.clone(), format!("{dir}/fake.txt")] {
host::open_location(&path, 0, 0).unwrap();
}
let file = format!("{traversal}/fake.txt");
mock.add_external_file(&file, "escaped");
assert!(host::fs::read_file(&file).is_err());
assert!(host::fs::read_bytes(&file).is_err());
assert!(host::fs::write_file(&file, "x").is_err());
assert!(host::fs::write_bytes(&file, b"x").is_err());
assert!(host::fs::stat(&file).is_err());
assert!(host::fs::read_dir(&traversal).is_err());
assert!(host::open_location(&file, 0, 0).is_err());
assert_eq!(mock.opened_locations().len(), 2);
}
#[test]
fn fs_write_needs_a_grant_of_its_own() {
// `fs.read` does not let a plugin write.
@@ -723,6 +723,44 @@ describe("PluginsPage", () => {
expect(notes.at(-1)).toMatchObject({ title: "Uninstalled Hello" });
});
it("keeps failed cleanup visible and lets Uninstall retry", async () => {
const list = backend.list;
const uninstall = backend.uninstall;
let failed = false;
vi.spyOn(backend, "list").mockImplementation(async () => {
const snapshot = await list();
if (failed) {
snapshot.plugins = snapshot.plugins.map((p) =>
p.id === "hello" ? { ...p, status: "failed", error: "Private data is locked" } : p,
);
}
return snapshot;
});
const remove = vi.spyOn(backend, "uninstall").mockImplementation(async (id) => {
if (!failed) {
failed = true;
throw new Error("Private data is locked");
}
await uninstall(id);
});
const user = userEvent.setup();
render(<PluginsPage />);
await screen.findByRole("heading", { level: 2 });
await user.click(screen.getByRole("button", { name: "Uninstall…" }));
await user.click(screen.getByRole("button", { name: "Confirm uninstall Hello" }));
await screen.findByText("Private data is locked");
expect(notes.at(-1)).toMatchObject({
level: "error",
title: "Could not uninstall Hello",
body: "Private data is locked",
});
expect(screen.getByRole("heading", { level: 2 })).toHaveTextContent("Hello");
await user.click(screen.getByRole("button", { name: "Confirm uninstall Hello" }));
await waitFor(() => expect(screen.getByRole("heading", { level: 2 })).not.toHaveTextContent("Hello"));
expect(remove).toHaveBeenCalledTimes(2);
expect(notes.at(-1)).toMatchObject({ title: "Uninstalled Hello" });
});
it("shows What's New from the change notes", async () => {
const user = userEvent.setup();
setPluginsBackend(
+4
View File
@@ -384,6 +384,10 @@
"Retract": "Retract",
"Couldn't retract the message": "Couldn't retract the message",
"The agent is still stopping. Try again once it has stopped.": "The agent is still stopping. Try again once it has stopped.",
"Uninstall incomplete; retry Uninstall to finish cleanup.": "Uninstall incomplete; retry Uninstall to finish cleanup.",
"refusing symlink in plugin cleanup path": "refusing symlink in plugin cleanup path",
"invalid plugin cleanup path": "invalid plugin cleanup path",
"cannot delete secret {0}: {1}": "cannot delete secret {0}: {1}",
"Invalid content editor configuration": "Invalid content editor configuration",
"No save action is configured for this tab": "No save action is configured for this tab",
"Content editor action timed out": "Content editor action timed out",
+4
View File
@@ -6738,6 +6738,10 @@
"Retract": "撤回",
"Couldn't retract the message": "无法撤回消息",
"The agent is still stopping. Try again once it has stopped.": "智能体仍在停止中。请在停止后重试。",
"Uninstall incomplete; retry Uninstall to finish cleanup.": "卸载未完成;请重试卸载以完成清理。",
"refusing symlink in plugin cleanup path": "拒绝清理路径中包含符号链接的插件",
"invalid plugin cleanup path": "插件清理路径无效",
"cannot delete secret {0}: {1}": "无法删除密钥 {0}:{1}",
"Invalid content editor configuration": "无效的内容编辑器配置",
"No save action is configured for this tab": "此标签页未配置保存操作",
"Content editor action timed out": "内容编辑器操作超时",