fix(permissions): match extension owners exactly (#10455)

This commit is contained in:
Jasper
2026-08-10 20:46:21 -06:00
committed by GitHub
parent dc77984832
commit bf332b9837
3 changed files with 50 additions and 22 deletions
+48 -18
View File
@@ -212,25 +212,40 @@ impl PermissionManager {
fs::write(&self.config_path, yaml_content).expect("Failed to write to permission.yaml"); fs::write(&self.config_path, yaml_content).expect("Failed to write to permission.yaml");
} }
/// Removes all entries where the principal name starts with the given extension name.
pub fn remove_extension(&self, extension_name: &str) { pub fn remove_extension(&self, extension_name: &str) {
let mut map = self.permission_map.write().unwrap(); let mut map = self.permission_map.write().unwrap();
for permission_config in map.values_mut() { for permission_config in map.values_mut() {
permission_config permission_config
.always_allow .always_allow
.retain(|p| !p.starts_with(extension_name)); .retain(|p| !Self::belongs_to_extension(p, extension_name));
permission_config permission_config
.ask_before .ask_before
.retain(|p| !p.starts_with(extension_name)); .retain(|p| !Self::belongs_to_extension(p, extension_name));
permission_config permission_config
.never_allow .never_allow
.retain(|p| !p.starts_with(extension_name)); .retain(|p| !Self::belongs_to_extension(p, extension_name));
} }
let yaml_content = let yaml_content =
serde_yaml::to_string(&*map).expect("Failed to serialize permission config"); serde_yaml::to_string(&*map).expect("Failed to serialize permission config");
fs::write(&self.config_path, yaml_content).expect("Failed to write to permission.yaml"); fs::write(&self.config_path, yaml_content).expect("Failed to write to permission.yaml");
} }
pub fn clear_permissions(&self) {
let mut map = self.permission_map.write().unwrap();
map.clear();
let yaml_content =
serde_yaml::to_string(&*map).expect("Failed to serialize permission config");
fs::write(&self.config_path, yaml_content).expect("Failed to write to permission.yaml");
}
fn belongs_to_extension(principal_name: &str, extension_name: &str) -> bool {
!extension_name.is_empty()
&& principal_name
.strip_prefix(extension_name)
.is_some_and(|suffix| suffix.starts_with("__"))
}
} }
#[cfg(test)] #[cfg(test)]
@@ -332,24 +347,39 @@ mod tests {
#[test] #[test]
fn test_remove_extension() { fn test_remove_extension() {
let (manager, _temp_dir) = create_test_permission_manager(); let (manager, _temp_dir) = create_test_permission_manager();
manager.update_user_permission("prefix__tool1", PermissionLevel::AlwaysAllow); manager.update_user_permission("git__status", PermissionLevel::AlwaysAllow);
manager.update_user_permission("nonprefix__tool2", PermissionLevel::AlwaysAllow); manager.update_user_permission("git__tool__with__delimiter", PermissionLevel::AskBefore);
manager.update_user_permission("prefix__tool3", PermissionLevel::AskBefore); manager.update_user_permission("github__delete_repo", PermissionLevel::NeverAllow);
manager.update_user_permission("gitlab__deploy", PermissionLevel::AskBefore);
manager.update_user_permission("__cli__ent____tool", PermissionLevel::NeverAllow);
// Remove entries starting with "prefix" manager.remove_extension("git");
manager.remove_extension("prefix");
let map = manager.permission_map.read().unwrap(); assert_eq!(manager.get_user_permission("git__status"), None);
let config = map.get(USER_PERMISSION).unwrap(); assert_eq!(
manager.get_user_permission("git__tool__with__delimiter"),
None
);
assert_eq!(
manager.get_user_permission("github__delete_repo"),
Some(PermissionLevel::NeverAllow)
);
assert_eq!(
manager.get_user_permission("gitlab__deploy"),
Some(PermissionLevel::AskBefore)
);
// Verify entries with "prefix" are removed manager.remove_extension("__cli__ent__");
assert!(!config.always_allow.contains(&"prefix__tool1".to_string())); assert_eq!(manager.get_user_permission("__cli__ent____tool"), None);
assert!(!config.ask_before.contains(&"prefix__tool3".to_string()));
// Verify other entries remain manager.remove_extension("");
assert!(config assert_eq!(
.always_allow manager.get_user_permission("github__delete_repo"),
.contains(&"nonprefix__tool2".to_string())); Some(PermissionLevel::NeverAllow)
);
manager.clear_permissions();
assert!(manager.get_permission_names().is_empty());
} }
#[test] #[test]
+1 -2
View File
@@ -300,8 +300,7 @@ impl Connection for AcpProviderConnection {
} }
fn reset_permissions(&self) { fn reset_permissions(&self) {
// "" matches all extensions, clearing all stored permission decisions self.permission_manager.clear_permissions();
self.permission_manager.remove_extension("");
} }
} }
+1 -2
View File
@@ -542,8 +542,7 @@ impl Connection for AcpServerConnection {
} }
fn reset_permissions(&self) { fn reset_permissions(&self) {
// "" matches all extensions, clearing all stored permission decisions self.permission_manager.clear_permissions();
self.permission_manager.remove_extension("");
} }
} }