From 7a66d7069f3b6a75e65d45b7c64a617d204f73a7 Mon Sep 17 00:00:00 2001 From: hocine Date: Tue, 18 Feb 2025 09:53:06 +0100 Subject: [PATCH 1/3] add unit tests for SUserType and SGroupType --- rar-common/src/database/actor.rs | 65 +++++++++++++ rar-common/src/database/finder.rs | 143 +++++++++++++++++++++++++++-- rar-common/src/database/options.rs | 133 ++++++++++++++------------- rar-common/src/database/structs.rs | 128 ++++++++++++++++---------- rar-common/src/lib.rs | 5 +- 5 files changed, 349 insertions(+), 125 deletions(-) diff --git a/rar-common/src/database/actor.rs b/rar-common/src/database/actor.rs index 6c72e1d0..8d63b4f2 100644 --- a/rar-common/src/database/actor.rs +++ b/rar-common/src/database/actor.rs @@ -418,3 +418,68 @@ impl core::fmt::Display for SActor { } } } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_suser_type_creation() { + let user_by_id = SUserType::from(0); + let user_by_name = SUserType::from("testuser"); + + assert_eq!(user_by_id.to_string(), "0"); + assert_eq!(user_by_name.to_string(), "testuser"); + } + #[test] + fn test_fetch_id() { + let user = SUserType::from(0); + assert_eq!(user.fetch_id(), Some(0)); + + let group = SGroupType::from(0); + assert_eq!(group.fetch_id(), Some(0)); + } + #[test] + fn test_fetch_user() { + let user = SUserType::from("testuser"); + assert!(user.fetch_user().is_none()); + + let user_by_id = SUserType::from(1001); + assert!(user_by_id.fetch_user().is_some()); + } + + #[test] + fn test_sgroups_multiple() { + let groups = SGroups::from(vec![SGroupType::from(0), SGroupType::from(200)]); + + assert_eq!(groups.len(), 2); + assert!(!groups.is_empty()); + + if let SGroups::Multiple(group_list) = groups { + assert_eq!(group_list[0].to_string(), "0"); + assert_eq!(group_list[1].to_string(), "200"); + } else { + panic!("Expected SGroups::Multiple"); + } + } + + #[test] + fn test_fech_group() { + let group = SGroupType::from(0); + assert_eq!( + group.fetch_group(), + Some(Group::from_gid(0.into()).unwrap().unwrap()) + ); + + let group = SGroupType::from("root"); + assert_eq!( + group.fetch_group(), + Some(Group::from_name("root").unwrap().unwrap()) + ); + } + + #[test] + fn test_is_empty() { + let groups = SGroups::Multiple(vec![]); + assert!(groups.is_empty()); + } +} diff --git a/rar-common/src/database/finder.rs b/rar-common/src/database/finder.rs index 4c99e317..83dc38d8 100644 --- a/rar-common/src/database/finder.rs +++ b/rar-common/src/database/finder.rs @@ -355,8 +355,6 @@ impl Default for TaskMatch { } } - - pub trait TaskMatcher { fn matches( &self, @@ -434,7 +432,9 @@ fn evaluate_regex_cmd(role_args: String, commandline: String) -> Result for Rc> { .borrow() .env .as_ref() - .is_some_and(|env| !env.override_behavior.is_some_and(|b| b) || env.default_behavior == *behavior) + .is_some_and(|env| { + !env.override_behavior.is_some_and(|b| b) + || env.default_behavior == *behavior + }) // but the polcy deny it and the behavior is not the same as the default one // we return NoMatch // (explaination: if the behavior is the same as the default one, we don't override it) }) { - return Err(MatchError::NoMatch("The user wants to override the behavior but the policy deny it".to_string())); + return Err(MatchError::NoMatch( + "The user wants to override the behavior but the policy deny it".to_string(), + )); } // Processing setuid let setuid: Option = self.as_ref().borrow().cred.setuid.clone(); @@ -740,7 +745,9 @@ impl TaskMatcher for Rc> { Some(t.fallback.clone()) // Si l'utilisateur correspond au fallback, utiliser le fallback } else if t.sub.iter().any(|s| s.fetch_eq(user)) { // Si l'utilisateur est explicitement interdit dans `sub` - return Err(MatchError::NoMatch("L'utilisateur est interdit dans sub.".into())); + return Err(MatchError::NoMatch( + "L'utilisateur est interdit dans sub.".into(), + )); } else if t.add.iter().any(|s| s.fetch_eq(user)) { // Si l'utilisateur est explicitement autorisé dans `add` @@ -749,7 +756,10 @@ impl TaskMatcher for Rc> { // Aucun match explicite, appliquer le comportement par défaut match t.default { SetBehavior::None => { - return Err(MatchError::NoMatch("Aucun comportement par défaut applicable.".into())); // Aucun utilisateur par défaut + println!("Aucun comportement par défaut applicable."); + return Err(MatchError::NoMatch( + "Aucun comportement par défaut applicable.".into(), + )); // Aucun utilisateur par défaut } SetBehavior::All => { debug!("Tous les utilisateurs sont acceptés."); @@ -1183,6 +1193,32 @@ mod tests { .unwrap(); } + #[test] + fn test_find_from_envpath() { + let needle = PathBuf::from("ls"); + let result = find_from_envpath(&needle); + println!("{:?}", result); + assert_eq!(result, Some("/usr/bin/ls".into())); + } + + #[test] + fn test_find_from_envpath_absolute_path() { + // Avec un chemin absolu + let needle = PathBuf::from("/bin/ls"); + let result = find_from_envpath(&needle); + println!("{:?}", result); + assert_eq!(result, None); + } + + #[test] + fn test_find_from_envpath_not_found() { + // Avec un fichier qui n'existe pas dans le PATH. + let needle = PathBuf::from("no_path"); + let result = find_from_envpath(&needle); + println!("{:?}", result); + assert_eq!(result, None); + } + #[test] fn test_match_path() { let result = match_path(&"/bin/ls".to_string(), &"/bin/ls".to_string()); @@ -1363,7 +1399,46 @@ mod tests { uid: Some(SetuidMin { is_root: false }), gid: None } - ) + ); + let setuid: Option = Some("root".into()); + assert_eq!( + get_setuid_min(setuid.as_ref(), None, &security_min), + SetUserMin { + uid: Some(SetuidMin { is_root: true }), + gid: None, + } + ); + setgid = Some(SGroups::Multiple(vec![1.into(), 2.into()])); + assert_eq!( + get_setuid_min(setuid.as_ref(), setgid.as_ref(), &security_min), + SetUserMin { + uid: Some(SetuidMin { is_root: true }), + gid: Some(SetgidMin { + is_root: false, + nb_groups: 2, + }), + } + ); + setgid = Some(SGroups::Multiple(vec![])); + assert_eq!( + get_setuid_min(setuid.as_ref(), setgid.as_ref(), &security_min), + SetUserMin { + uid: Some(SetuidMin { is_root: true }), + gid: None, + } + ); + + setgid = Some(SGroups::Multiple(vec![0.into()])); + assert_eq!( + get_setuid_min(None, setgid.as_ref(), &security_min), + SetUserMin { + uid: None, + gid: Some(SetgidMin { + is_root: true, + nb_groups: 1, + }), + } + ); } #[test] @@ -1567,7 +1642,7 @@ mod tests { } #[test] - + fn test_setuid_fallback_valid() { // Configuration de test let config = setup_test_config(1); // Un seul rôle pour simplifier @@ -1614,7 +1689,7 @@ mod tests { } #[test] - + fn test_setuid_fallback_nonarg_valid() { // Configuration de test let config = setup_test_config(1); @@ -1963,6 +2038,54 @@ mod tests { println!("Test réussi : L'utilisateur spécifié correspond bien à l'ajout."); } + #[test] + fn test_setuid_none_add_invalid() { + // Configuration de test + let config = setup_test_config(1); // Un seul rôle pour simplifier + let role = setup_test_role(1, Some(config.as_ref().borrow().roles[0].clone()), None); + let task = role.as_ref().borrow().tasks[0].clone(); + // Ajout d'un acteur autorisé + role.as_ref() + .borrow_mut() + .actors + .push(SActor::user("root").build()); + + task.as_ref().borrow_mut().commands.default_behavior = Some(SetBehavior::All); + // Définition du `setuid` avec un `fallback` + let fallback_user = SUserType::from(get_non_root_uid()); + let chooser_struct = SSetuidSet { + fallback: fallback_user.clone(), + default: SetBehavior::None, + add: vec![SUserType::from("root")], // Ajout d'un utilisateur + sub: vec![], + }; + task.as_ref().borrow_mut().cred.setuid = Some(SUserChooser::ChooserStruct(chooser_struct)); + + // Création des credentials avec l'utilisateur correspondant à l'ajout + let cred = Cred { + user: User::from_name("root").unwrap().unwrap(), // Même nom que l'ajout + groups: vec![Group::from_name("root").unwrap().unwrap()], + ppid: Pid::from_raw(0), + tty: None, + }; + + // Commande de test + let command = vec!["/bin/ls".to_string(), "-l".to_string(), "-a".to_string()]; + // Exécution du match + let filter_matcher = FilterMatcher::builder().user("aitbelkacem").build(); + + let result = config.matches(&cred, &Some(filter_matcher), &command); + + // Vérification que le match est réussi + assert!(result.is_err()); + let result = result.unwrap_err(); + + // Vérification que l'erreur est bien de type `NoMatch` + assert!(result.is_no_match()); + + println!("Test réussi : L'utilisateur spécifié ne correspond pas "); + } + #[test] fn test_equal_settings() { let mut settings1 = ExecSettings::new(); diff --git a/rar-common/src/database/options.rs b/rar-common/src/database/options.rs index dc2ba294..4431bf59 100644 --- a/rar-common/src/database/options.rs +++ b/rar-common/src/database/options.rs @@ -24,7 +24,7 @@ use crate::rc_refcell; #[cfg(feature = "finder")] use super::finder::Cred; -use super::{FilterMatcher, deserialize_duration, is_default, serialize_duration}; +use super::{deserialize_duration, is_default, serialize_duration, FilterMatcher}; use super::{ lhs_deserialize, lhs_deserialize_envkey, lhs_serialize, lhs_serialize_envkey, @@ -633,75 +633,78 @@ impl OptStackBuilder { where ::Roles: opt_stack_builder::IsUnset, { - self.with_default().roles(roles.to_owned()) + self.with_default() + .roles(roles.to_owned()) .opt(roles.as_ref().borrow().options.to_owned()) } fn with_default(self) -> Self { - self.opt(Some(Opt::builder(Level::Default) - .root(SPrivileged::User) - .bounding(SBounding::Strict) - .path( - SPathOptions::builder(PathBehavior::Delete) - .add([ - "/usr/local/sbin", - "/usr/local/bin", - "/usr/sbin", - "/usr/bin", - "/sbin", - "/bin", - "/snap/bin", - ]) - .build(), - ) - .authentication(SAuthentication::Perform) - .env( - SEnvOptions::builder(EnvBehavior::Delete) - .keep([ - "HOME", - "USER", - "LOGNAME", - "COLORS", - "DISPLAY", - "HOSTNAME", - "KRB5CCNAME", - "LS_COLORS", - "PS1", - "PS2", - "XAUTHORY", - "XAUTHORIZATION", - "XDG_CURRENT_DESKTOP", - ]) - .unwrap() - .check([ - "COLORTERM", - "LANG", - "LANGUAGE", - "LC_*", - "LINGUAS", - "TERM", - "TZ", - ]) - .unwrap() - .delete([ - "PS4", - "SHELLOPTS", - "PERLLIB", - "PERL5LIB", - "PERL5OPT", - "PYTHONINSPECT", - ]) - .unwrap() - .build(), - ) - .timeout( - STimeout::builder() - .type_field(TimestampType::TTY) - .duration(Duration::minutes(5)) + self.opt(Some( + Opt::builder(Level::Default) + .root(SPrivileged::User) + .bounding(SBounding::Strict) + .path( + SPathOptions::builder(PathBehavior::Delete) + .add([ + "/usr/local/sbin", + "/usr/local/bin", + "/usr/sbin", + "/usr/bin", + "/sbin", + "/bin", + "/snap/bin", + ]) + .build(), + ) + .authentication(SAuthentication::Perform) + .env( + SEnvOptions::builder(EnvBehavior::Delete) + .keep([ + "HOME", + "USER", + "LOGNAME", + "COLORS", + "DISPLAY", + "HOSTNAME", + "KRB5CCNAME", + "LS_COLORS", + "PS1", + "PS2", + "XAUTHORY", + "XAUTHORIZATION", + "XDG_CURRENT_DESKTOP", + ]) + .unwrap() + .check([ + "COLORTERM", + "LANG", + "LANGUAGE", + "LC_*", + "LINGUAS", + "TERM", + "TZ", + ]) + .unwrap() + .delete([ + "PS4", + "SHELLOPTS", + "PERLLIB", + "PERL5LIB", + "PERL5OPT", + "PYTHONINSPECT", + ]) + .unwrap() + .build(), + ) + .timeout( + STimeout::builder() + .type_field(TimestampType::TTY) + .duration(Duration::minutes(5)) + .build(), + ) + .wildcard_denied(";&|") .build(), - ) - .wildcard_denied(";&|") - .build())) + )) } } diff --git a/rar-common/src/database/structs.rs b/rar-common/src/database/structs.rs index 2e1de1b1..a4aafbaf 100644 --- a/rar-common/src/database/structs.rs +++ b/rar-common/src/database/structs.rs @@ -711,7 +711,10 @@ mod tests { as_borrow, database::{ actor::SGroupType, - options::{EnvBehavior, PathBehavior, SAuthentication, SBounding, SEnvOptions, SPathOptions, SPrivileged, STimeout, TimestampType}, + options::{ + EnvBehavior, PathBehavior, SAuthentication, SBounding, SEnvOptions, SPathOptions, + SPrivileged, STimeout, TimestampType, + }, }, }; @@ -1090,59 +1093,92 @@ mod tests { #[test] fn test_serialize() { - let config = SConfig::builder().role( - SRole::builder("role1") - .actor(SActor::user("user1").build()) - .actor(SActor::group([SGroupType::from("group1"), SGroupType::from(1000)]).build()) - .task( - STask::builder("task1") - .purpose("purpose1".into()) - .cred( - SCredentials::builder() - .setuid(SUserChooser::ChooserStruct( - SSetuidSet::builder("user1", SetBehavior::All) - .add(["user2".into()]) - .sub(["user3".into()]) - .build(), - )) - .setgid(["setgid1"]) - .capabilities( - SCapabilities::builder(SetBehavior::All) - .add_cap(Cap::NET_BIND_SERVICE) - .sub_cap(Cap::SYS_ADMIN) - .build(), - ) - .build(), - ) - .commands( - SCommands::builder(SetBehavior::All) - .add(["cmd1".into()]) - .sub(["cmd2".into()]) - .build(), - ) + let config = SConfig::builder() + .role( + SRole::builder("role1") + .actor(SActor::user("user1").build()) + .actor( + SActor::group([SGroupType::from("group1"), SGroupType::from(1000)]).build(), + ) + .task( + STask::builder("task1") + .purpose("purpose1".into()) + .cred( + SCredentials::builder() + .setuid(SUserChooser::ChooserStruct( + SSetuidSet::builder("user1", SetBehavior::All) + .add(["user2".into()]) + .sub(["user3".into()]) + .build(), + )) + .setgid(["setgid1"]) + .capabilities( + SCapabilities::builder(SetBehavior::All) + .add_cap(Cap::NET_BIND_SERVICE) + .sub_cap(Cap::SYS_ADMIN) + .build(), + ) + .build(), + ) + .commands( + SCommands::builder(SetBehavior::All) + .add(["cmd1".into()]) + .sub(["cmd2".into()]) + .build(), + ) + .build(), + ) + .build(), + ) + .options(|opt| { + opt.path( + SPathOptions::builder(PathBehavior::Delete) + .add(["path_add"]) + .sub(["path_sub"]) + .build(), + ) + .env( + SEnvOptions::builder(EnvBehavior::Delete) + .override_behavior(true) + .keep(["keep_env"]) + .unwrap() + .check(["check_env"]) + .unwrap() + .build(), + ) + .root(SPrivileged::Privileged) + .bounding(SBounding::Ignore) + .authentication(SAuthentication::Skip) + .wildcard_denied("wildcards") + .timeout( + STimeout::builder() + .type_field(TimestampType::PPID) + .duration(Duration::minutes(5)) .build(), ) - .build(), - ).options( | opt | opt.path(SPathOptions::builder(PathBehavior::Delete).add(["path_add"]).sub(["path_sub"]).build()) - .env(SEnvOptions::builder(EnvBehavior::Delete).override_behavior(true).keep(["keep_env"]).unwrap().check(["check_env"]).unwrap().build()) - .root(SPrivileged::Privileged) - .bounding(SBounding::Ignore) - .authentication(SAuthentication::Skip) - .wildcard_denied("wildcards") - .timeout(STimeout::builder().type_field(TimestampType::PPID).duration(Duration::minutes(5)).build()) - .build() - ).build(); + .build() + }) + .build(); let config = serde_json::to_string_pretty(&config).unwrap(); println!("{}", config); } - #[test] fn test_serialize_operride_behavior_option() { - let config = SConfig::builder().options( | opt | opt.env(SEnvOptions::builder(EnvBehavior::Inherit).override_behavior(true).build()) - .build() - ).build(); + let config = SConfig::builder() + .options(|opt| { + opt.env( + SEnvOptions::builder(EnvBehavior::Inherit) + .override_behavior(true) + .build(), + ) + .build() + }) + .build(); let config = serde_json::to_string(&config).unwrap(); - assert_eq!(config,"{\"options\":{\"env\":{\"override_behavior\":true}}}"); + assert_eq!( + config, + "{\"options\":{\"env\":{\"override_behavior\":true}}}" + ); } } diff --git a/rar-common/src/lib.rs b/rar-common/src/lib.rs index 896f2891..68fe82c7 100644 --- a/rar-common/src/lib.rs +++ b/rar-common/src/lib.rs @@ -108,8 +108,7 @@ pub struct Settings { pub ldap: Option, } -#[derive(Serialize, Deserialize, Debug, Clone, Builder)] -#[derive(Default)] +#[derive(Serialize, Deserialize, Debug, Clone, Builder, Default)] pub struct RemoteStorageSettings { #[serde(skip_serializing_if = "Option::is_none")] #[builder(name = not_immutable,with = || false)] @@ -194,8 +193,6 @@ impl Default for Settings { } } - - pub fn save_settings(settings: Rc>) -> Result<(), Box> { let default_remote: RemoteStorageSettings = RemoteStorageSettings::default(); // remove immutable flag From f4fd3653f9dfd01bb6deddcfbef8e4c436cceb76 Mon Sep 17 00:00:00 2001 From: LeChatP Date: Tue, 18 Feb 2025 09:58:58 +0100 Subject: [PATCH 2/3] refactor: consolidate serialization and deserialization logic for path and env vars. More thorough testing of option management. --- rar-common/src/database/finder.rs | 31 +- rar-common/src/database/mod.rs | 180 ++++++- rar-common/src/database/options.rs | 721 ++++++++++++++++++++--------- rar-common/src/database/structs.rs | 173 ++++--- rar-common/src/lib.rs | 5 +- src/chsr/cli/mod.rs | 72 ++- src/chsr/cli/process/json.rs | 140 +++--- 7 files changed, 963 insertions(+), 359 deletions(-) diff --git a/rar-common/src/database/finder.rs b/rar-common/src/database/finder.rs index 4c99e317..b9d152fa 100644 --- a/rar-common/src/database/finder.rs +++ b/rar-common/src/database/finder.rs @@ -355,8 +355,6 @@ impl Default for TaskMatch { } } - - pub trait TaskMatcher { fn matches( &self, @@ -434,7 +432,9 @@ fn evaluate_regex_cmd(role_args: String, commandline: String) -> Result for Rc> { .borrow() .env .as_ref() - .is_some_and(|env| !env.override_behavior.is_some_and(|b| b) || env.default_behavior == *behavior) + .is_some_and(|env| { + !env.override_behavior.is_some_and(|b| b) + || env.default_behavior == *behavior + }) // but the polcy deny it and the behavior is not the same as the default one // we return NoMatch // (explaination: if the behavior is the same as the default one, we don't override it) }) { - return Err(MatchError::NoMatch("The user wants to override the behavior but the policy deny it".to_string())); + return Err(MatchError::NoMatch( + "The user wants to override the behavior but the policy deny it".to_string(), + )); } // Processing setuid let setuid: Option = self.as_ref().borrow().cred.setuid.clone(); @@ -740,7 +745,9 @@ impl TaskMatcher for Rc> { Some(t.fallback.clone()) // Si l'utilisateur correspond au fallback, utiliser le fallback } else if t.sub.iter().any(|s| s.fetch_eq(user)) { // Si l'utilisateur est explicitement interdit dans `sub` - return Err(MatchError::NoMatch("L'utilisateur est interdit dans sub.".into())); + return Err(MatchError::NoMatch( + "L'utilisateur est interdit dans sub.".into(), + )); } else if t.add.iter().any(|s| s.fetch_eq(user)) { // Si l'utilisateur est explicitement autorisé dans `add` @@ -749,7 +756,9 @@ impl TaskMatcher for Rc> { // Aucun match explicite, appliquer le comportement par défaut match t.default { SetBehavior::None => { - return Err(MatchError::NoMatch("Aucun comportement par défaut applicable.".into())); // Aucun utilisateur par défaut + return Err(MatchError::NoMatch( + "Aucun comportement par défaut applicable.".into(), + )); // Aucun utilisateur par défaut } SetBehavior::All => { debug!("Tous les utilisateurs sont acceptés."); @@ -1567,7 +1576,7 @@ mod tests { } #[test] - + fn test_setuid_fallback_valid() { // Configuration de test let config = setup_test_config(1); // Un seul rôle pour simplifier @@ -1614,7 +1623,7 @@ mod tests { } #[test] - + fn test_setuid_fallback_nonarg_valid() { // Configuration de test let config = setup_test_config(1); @@ -2023,7 +2032,7 @@ mod tests { let command = vec!["/bin/ls".to_string()]; let result = role.matches(&cred, &None, &command); assert!(result.is_ok()); - assert!(role.as_ref().borrow_mut()[0] + role.as_ref().borrow_mut()[0] .as_ref() .borrow_mut() .options @@ -2035,7 +2044,7 @@ mod tests { .as_mut() .unwrap() .add - .insert("/test".to_string())); + .replace(["/test".to_string()].iter().cloned().collect()); let result = role.matches(&cred, &None, &command); assert!(result.is_err()); } diff --git a/rar-common/src/database/mod.rs b/rar-common/src/database/mod.rs index d68035f1..9b993474 100644 --- a/rar-common/src/database/mod.rs +++ b/rar-common/src/database/mod.rs @@ -155,39 +155,58 @@ fn write_sconfig( } // deserialize the linked hash set -fn lhs_deserialize_envkey<'de, D>(deserializer: D) -> Result, D::Error> +fn lhs_deserialize_envkey<'de, D>( + deserializer: D, +) -> Result>, D::Error> where D: de::Deserializer<'de>, { - let v: Vec = Vec::deserialize(deserializer)?; - Ok(v.into_iter().collect()) + if let Ok(v) = Vec::::deserialize(deserializer) { + Ok(Some(v.into_iter().collect())) + } else { + Ok(None) + } } // serialize the linked hash set -fn lhs_serialize_envkey(value: &LinkedHashSet, serializer: S) -> Result +fn lhs_serialize_envkey( + value: &Option>, + serializer: S, +) -> Result where S: serde::Serializer, { - let v: Vec = value.iter().cloned().collect(); - v.serialize(serializer) + if let Some(v) = value { + let v: Vec = v.iter().cloned().collect(); + v.serialize(serializer) + } else { + serializer.serialize_none() + } } // deserialize the linked hash set -fn lhs_deserialize<'de, D>(deserializer: D) -> Result, D::Error> +fn lhs_deserialize<'de, D>(deserializer: D) -> Result>, D::Error> where D: de::Deserializer<'de>, { - let v: Vec = Vec::deserialize(deserializer)?; - Ok(v.into_iter().collect()) + if let Ok(v) = Vec::::deserialize(deserializer) { + Ok(Some(v.into_iter().collect())) + } else { + Ok(None) + } } // serialize the linked hash set -fn lhs_serialize(value: &LinkedHashSet, serializer: S) -> Result +fn lhs_serialize(value: &Option>, serializer: S) -> Result where S: serde::Serializer, { - let v: Vec = value.iter().cloned().collect(); - v.serialize(serializer) + if let Some(v) = value { + let v: Vec = v.iter().cloned().collect(); + v.serialize(serializer) + } else { + serializer.serialize_none() + } } pub fn is_default(t: &T) -> bool { @@ -201,7 +220,7 @@ where // hh:mm:ss format match value { Some(value) => serializer.serialize_str(&format!( - "{}:{}:{}", + "{:#02}:{:#02}:{:#02}", value.num_hours(), value.num_minutes() % 60, value.num_seconds() % 60 @@ -236,3 +255,138 @@ where let v: Vec = value.iter().map(|cap| cap.to_string()).collect(); v.serialize(serializer) } + +#[cfg(test)] +mod tests { + use super::*; + + struct LinkedHashSetTester(LinkedHashSet); + + impl<'de> Deserialize<'de> for LinkedHashSetTester { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Ok(Self( + lhs_deserialize_envkey(deserializer).map(|v| v.unwrap())?, + )) + } + } + + impl Serialize for LinkedHashSetTester { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + lhs_serialize_envkey(&Some(self.0.clone()), serializer) + } + } + + impl<'de> Deserialize<'de> for LinkedHashSetTester { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Ok(Self(lhs_deserialize(deserializer).map(|v| v.unwrap())?)) + } + } + + impl Serialize for LinkedHashSetTester { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + lhs_serialize(&Some(self.0.clone()), serializer) + } + } + + struct DurationTester(Duration); + + impl<'de> Deserialize<'de> for DurationTester { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Ok(Self( + deserialize_duration(deserializer).map(|v| v.unwrap())?, + )) + } + } + + impl Serialize for DurationTester { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + serialize_duration(&Some(self.0.clone()), serializer) + } + } + + #[test] + fn test_lhs_deserialize_envkey() { + let json = r#"["key1", "key2", "key3"]"#; + let deserialized: Option> = serde_json::from_str(json).unwrap(); + assert!(deserialized.is_some()); + let set = deserialized.unwrap(); + assert_eq!(set.0.len(), 3); + assert!(set.0.contains(&EnvKey::from("key1"))); + assert!(set.0.contains(&EnvKey::from("key2"))); + assert!(set.0.contains(&EnvKey::from("key3"))); + } + + #[test] + fn test_lhs_serialize_envkey() { + let mut set = LinkedHashSetTester(LinkedHashSet::new()); + set.0.insert(EnvKey::from("key1")); + set.0.insert(EnvKey::from("key2")); + set.0.insert(EnvKey::from("key3")); + let serialized = serde_json::to_string(&Some(set)).unwrap(); + assert_eq!(serialized, r#"["key1","key2","key3"]"#); + } + + #[test] + fn test_lhs_deserialize() { + let json = r#"["value1", "value2", "value3"]"#; + let deserialized: Option> = serde_json::from_str(json).unwrap(); + assert!(deserialized.is_some()); + let set = deserialized.unwrap(); + assert_eq!(set.0.len(), 3); + assert!(set.0.contains("value1")); + assert!(set.0.contains("value2")); + assert!(set.0.contains("value3")); + } + + #[test] + fn test_lhs_serialize() { + let mut set = LinkedHashSetTester(LinkedHashSet::new()); + set.0.insert("value1".to_string()); + set.0.insert("value2".to_string()); + set.0.insert("value3".to_string()); + let serialized = serde_json::to_string(&Some(set)).unwrap(); + assert_eq!(serialized, r#"["value1","value2","value3"]"#); + } + + #[test] + fn test_serialize_duration() { + let duration = Some(DurationTester(Duration::seconds(3661))); + let serialized = serde_json::to_string(&duration).unwrap(); + assert_eq!(serialized, r#""01:01:01""#); + } + + #[test] + fn test_deserialize_duration() { + let json = r#""01:01:01""#; + let deserialized: Option = serde_json::from_str(json).unwrap(); + assert!(deserialized.is_some()); + let duration = deserialized.unwrap(); + assert_eq!(duration.0.num_seconds(), 3661); + } + + #[test] + fn test_is_default() { + assert!(is_default(&0)); + assert!(is_default(&String::new())); + assert!(!is_default(&1)); + assert!(!is_default(&"non-default".to_string())); + } +} diff --git a/rar-common/src/database/options.rs b/rar-common/src/database/options.rs index dc2ba294..0dce33c4 100644 --- a/rar-common/src/database/options.rs +++ b/rar-common/src/database/options.rs @@ -24,7 +24,7 @@ use crate::rc_refcell; #[cfg(feature = "finder")] use super::finder::Cred; -use super::{FilterMatcher, deserialize_duration, is_default, serialize_duration}; +use super::{deserialize_duration, is_default, serialize_duration, FilterMatcher}; use super::{ lhs_deserialize, lhs_deserialize_envkey, lhs_serialize, lhs_serialize_envkey, @@ -97,21 +97,21 @@ pub struct SPathOptions { pub default_behavior: PathBehavior, #[serde( default, - skip_serializing_if = "LinkedHashSet::is_empty", + skip_serializing_if = "Option::is_none", deserialize_with = "lhs_deserialize", serialize_with = "lhs_serialize" )] - #[builder(default, with = |v : impl IntoIterator| { v.into_iter().map(|s| s.to_string()).collect() })] - pub add: LinkedHashSet, + #[builder(with = |v : impl IntoIterator| { v.into_iter().map(|s| s.to_string()).collect() })] + pub add: Option>, #[serde( default, - skip_serializing_if = "LinkedHashSet::is_empty", + skip_serializing_if = "Option::is_none", deserialize_with = "lhs_deserialize", serialize_with = "lhs_serialize", alias = "del" )] - #[builder(default, with = |v : impl IntoIterator| { v.into_iter().map(|s| s.to_string()).collect() })] - pub sub: LinkedHashSet, + #[builder(with = |v : impl IntoIterator| { v.into_iter().map(|s| s.to_string()).collect() })] + pub sub: Option>, #[serde(default)] #[serde(flatten)] #[builder(default)] @@ -164,28 +164,28 @@ pub struct SEnvOptions { pub set: HashMap, #[serde( default, - skip_serializing_if = "LinkedHashSet::is_empty", + skip_serializing_if = "Option::is_none", deserialize_with = "lhs_deserialize_envkey", serialize_with = "lhs_serialize_envkey" )] - #[builder(default, with = |v : impl IntoIterator| -> Result<_,String> { let mut res = LinkedHashSet::new(); for s in v { res.insert(EnvKey::new(s.to_string())?); } Ok(res)})] - pub keep: LinkedHashSet, + #[builder(with = |v : impl IntoIterator| -> Result<_,String> { let mut res = LinkedHashSet::new(); for s in v { res.insert(EnvKey::new(s.to_string())?); } Ok(res)})] + pub keep: Option>, #[serde( default, - skip_serializing_if = "LinkedHashSet::is_empty", + skip_serializing_if = "Option::is_none", deserialize_with = "lhs_deserialize_envkey", serialize_with = "lhs_serialize_envkey" )] - #[builder(default, with = |v : impl IntoIterator| -> Result<_,String> { let mut res = LinkedHashSet::new(); for s in v { res.insert(EnvKey::new(s.to_string())?); } Ok(res)})] - pub check: LinkedHashSet, + #[builder(with = |v : impl IntoIterator| -> Result<_,String> { let mut res = LinkedHashSet::new(); for s in v { res.insert(EnvKey::new(s.to_string())?); } Ok(res)})] + pub check: Option>, #[serde( default, - skip_serializing_if = "LinkedHashSet::is_empty", + skip_serializing_if = "Option::is_none", deserialize_with = "lhs_deserialize_envkey", serialize_with = "lhs_serialize_envkey" )] - #[builder(default, with = |v : impl IntoIterator| -> Result<_,String> { let mut res = LinkedHashSet::new(); for s in v { res.insert(EnvKey::new(s.to_string())?); } Ok(res)})] - pub delete: LinkedHashSet, + #[builder(with = |v : impl IntoIterator| -> Result<_,String> { let mut res = LinkedHashSet::new(); for s in v { res.insert(EnvKey::new(s.to_string())?); } Ok(res)})] + pub delete: Option>, #[serde(default, flatten)] #[builder(default)] pub _extra_fields: Map, @@ -376,8 +376,8 @@ impl Default for SPathOptions { fn default() -> Self { SPathOptions { default_behavior: PathBehavior::Inherit, - add: LinkedHashSet::new(), - sub: LinkedHashSet::new(), + add: None, + sub: None, _extra_fields: Map::default(), } } @@ -488,11 +488,17 @@ impl EnvSet for HashMap { } impl EnvSet for LinkedHashSet { - fn env_matches(&self, wildcarded: &EnvKey) -> bool { - match wildcarded.env_type { - EnvKeyType::Normal => self.contains(wildcarded), - EnvKeyType::Wildcarded => self.iter().any(|s| check_wildcarded(wildcarded, &s.value)), - } + fn env_matches(&self, needle: &EnvKey) -> bool { + self.iter().any(|s| match s.env_type { + EnvKeyType::Normal => s == needle, + EnvKeyType::Wildcarded => check_wildcarded(s, &needle.value), + }) + } +} + +impl EnvSet for Option> { + fn env_matches(&self, needle: &EnvKey) -> bool { + self.as_ref().map_or(false, |set| set.env_matches(needle)) } } @@ -575,6 +581,7 @@ pub struct OptStack { task: Option>>, } +#[cfg(not(tarpaulin_include))] impl OptStackBuilder { fn opt(mut self, opt: Option>>) -> Self { if let Some(opt) = opt { @@ -633,75 +640,78 @@ impl OptStackBuilder { where ::Roles: opt_stack_builder::IsUnset, { - self.with_default().roles(roles.to_owned()) + self.with_default() + .roles(roles.to_owned()) .opt(roles.as_ref().borrow().options.to_owned()) } fn with_default(self) -> Self { - self.opt(Some(Opt::builder(Level::Default) - .root(SPrivileged::User) - .bounding(SBounding::Strict) - .path( - SPathOptions::builder(PathBehavior::Delete) - .add([ - "/usr/local/sbin", - "/usr/local/bin", - "/usr/sbin", - "/usr/bin", - "/sbin", - "/bin", - "/snap/bin", - ]) - .build(), - ) - .authentication(SAuthentication::Perform) - .env( - SEnvOptions::builder(EnvBehavior::Delete) - .keep([ - "HOME", - "USER", - "LOGNAME", - "COLORS", - "DISPLAY", - "HOSTNAME", - "KRB5CCNAME", - "LS_COLORS", - "PS1", - "PS2", - "XAUTHORY", - "XAUTHORIZATION", - "XDG_CURRENT_DESKTOP", - ]) - .unwrap() - .check([ - "COLORTERM", - "LANG", - "LANGUAGE", - "LC_*", - "LINGUAS", - "TERM", - "TZ", - ]) - .unwrap() - .delete([ - "PS4", - "SHELLOPTS", - "PERLLIB", - "PERL5LIB", - "PERL5OPT", - "PYTHONINSPECT", - ]) - .unwrap() - .build(), - ) - .timeout( - STimeout::builder() - .type_field(TimestampType::TTY) - .duration(Duration::minutes(5)) + self.opt(Some( + Opt::builder(Level::Default) + .root(SPrivileged::User) + .bounding(SBounding::Strict) + .path( + SPathOptions::builder(PathBehavior::Delete) + .add([ + "/usr/local/sbin", + "/usr/local/bin", + "/usr/sbin", + "/usr/bin", + "/sbin", + "/bin", + "/snap/bin", + ]) + .build(), + ) + .authentication(SAuthentication::Perform) + .env( + SEnvOptions::builder(EnvBehavior::Delete) + .keep([ + "HOME", + "USER", + "LOGNAME", + "COLORS", + "DISPLAY", + "HOSTNAME", + "KRB5CCNAME", + "LS_COLORS", + "PS1", + "PS2", + "XAUTHORY", + "XAUTHORIZATION", + "XDG_CURRENT_DESKTOP", + ]) + .unwrap() + .check([ + "COLORTERM", + "LANG", + "LANGUAGE", + "LC_*", + "LINGUAS", + "TERM", + "TZ", + ]) + .unwrap() + .delete([ + "PS4", + "SHELLOPTS", + "PERLLIB", + "PERL5LIB", + "PERL5OPT", + "PYTHONINSPECT", + ]) + .unwrap() + .build(), + ) + .timeout( + STimeout::builder() + .type_field(TimestampType::TTY) + .duration(Duration::minutes(5)) + .build(), + ) + .wildcard_denied(";&|") .build(), - ) - .wildcard_denied(";&|") - .build())) + )) } } @@ -755,37 +765,45 @@ impl OptStack { #[cfg(feature = "finder")] fn calculate_path(&self) -> String { let path = self.get_final_path(); - let final_add = path - .add - .difference(&path.sub) - .fold("".to_string(), |mut acc, s| { - if !acc.is_empty() { - acc.insert(0, ':'); - } - acc.insert_str(0, s); - acc - }); - match path.default_behavior { - PathBehavior::Inherit | PathBehavior::Delete => final_add, - is_safe => std::env::vars() - .find_map(|(key, value)| if key == "PATH" { Some(value) } else { None }) - .unwrap_or(String::new()) - .split(':') - .filter(|s| { - !path.sub.contains(*s) && (!is_safe.is_keep_safe() || PathBuf::from(s).exists()) - }) - .fold(final_add, |mut acc, s| { + let default = LinkedHashSet::new(); + println!("path: {:?}", path); + if let Some(add) = path.add { + let final_add = add.difference(path.sub.as_ref().unwrap_or(&default)).fold( + "".to_string(), + |mut acc, s| { if !acc.is_empty() { - acc.push(':'); + acc.insert(0, ':'); } - acc.push_str(s); + acc.insert_str(0, s); acc - }), + }, + ); + match path.default_behavior { + PathBehavior::Inherit | PathBehavior::Delete => final_add, + is_safe => std::env::vars() + .find_map(|(key, value)| if key == "PATH" { Some(value) } else { None }) + .unwrap_or(String::new()) + .split(':') + .filter(|s| { + !path.sub.as_ref().unwrap_or(&default).contains(*s) + && (!is_safe.is_keep_safe() || PathBuf::from(s).exists()) + }) + .fold(final_add, |mut acc, s| { + if !acc.is_empty() { + acc.push(':'); + } + acc.push_str(s); + acc + }), + } + } else { + "".to_string() } } fn get_final_path(&self) -> SPathOptions { let mut final_behavior = PathBehavior::Delete; + let default = LinkedHashSet::new(); let final_add = rc_refcell!(LinkedHashSet::new()); // Cannot use HashSet as we need to keep order let final_sub = rc_refcell!(LinkedHashSet::new()); @@ -794,19 +812,21 @@ impl OptStack { let final_sub_clone = Rc::clone(&final_sub); if let Some(p) = opt.path.borrow().as_ref() { match p.default_behavior { - PathBehavior::Delete => { - final_add_clone.as_ref().replace(p.add.clone()); - } - PathBehavior::KeepSafe | PathBehavior::KeepUnsafe => { - final_sub_clone.as_ref().replace(p.sub.clone()); + PathBehavior::KeepSafe | PathBehavior::KeepUnsafe | PathBehavior::Delete => { + if let Some(add) = p.add.as_ref() { + final_add_clone.as_ref().replace(add.clone()); + } + if let Some(sub) = p.sub.as_ref() { + final_sub_clone.as_ref().replace(sub.clone()); + } } PathBehavior::Inherit => { if final_behavior.is_delete() { let union: LinkedHashSet = final_add_clone .as_ref() .borrow() - .union(&p.add) - .filter(|e| !p.sub.contains(*e)) + .union(p.add.as_ref().unwrap_or(&default)) + .filter(|e| !p.sub.as_ref().unwrap_or(&default).contains(*e)) .cloned() .collect(); final_add_clone.as_ref().borrow_mut().extend(union); @@ -815,8 +835,8 @@ impl OptStack { let union: LinkedHashSet = final_sub_clone .as_ref() .borrow() - .union(&p.sub) - .filter(|e| !p.add.contains(*e)) + .union(p.sub.as_ref().unwrap_or(&default)) + .filter(|e| !p.add.as_ref().unwrap_or(&default).contains(*e)) .cloned() .collect(); final_sub_clone.as_ref().borrow_mut().extend(union); @@ -854,6 +874,7 @@ impl OptStack { #[cfg(not(tarpaulin_include))] fn union_all_path(&self) -> SPathOptions { let mut final_behavior = PathBehavior::Delete; + let default = LinkedHashSet::new(); let final_add = rc_refcell!(LinkedHashSet::new()); // Cannot use HashSet as we need to keep order let final_sub = rc_refcell!(LinkedHashSet::new()); @@ -866,8 +887,8 @@ impl OptStack { let union = final_add_clone .as_ref() .borrow() - .union(&p.add) - .filter(|e| !p.sub.contains(*e)) + .union(p.add.as_ref().unwrap_or(&default)) + .filter(|e| !p.sub.as_ref().unwrap_or(&default).contains(*e)) .cloned() .collect(); // policy is to delete, so we add whitelist and remove blacklist @@ -878,8 +899,8 @@ impl OptStack { let union = final_sub_clone .as_ref() .borrow() - .union(&p.sub) - .filter(|e| !p.add.contains(*e)) + .union(p.sub.as_ref().unwrap_or(&default)) + .filter(|e| !p.add.as_ref().unwrap_or(&default).contains(*e)) .cloned() .collect(); //policy is to keep, so we remove blacklist and add whitelist @@ -890,8 +911,8 @@ impl OptStack { let union: LinkedHashSet = final_add_clone .as_ref() .borrow() - .union(&p.add) - .filter(|e| !p.sub.contains(*e)) + .union(p.add.as_ref().unwrap_or(&default)) + .filter(|e| !p.sub.as_ref().unwrap_or(&default).contains(*e)) .cloned() .collect(); final_add_clone.as_ref().borrow_mut().extend(union); @@ -900,8 +921,8 @@ impl OptStack { let union: LinkedHashSet = final_sub_clone .as_ref() .borrow() - .union(&p.sub) - .filter(|e| !p.add.contains(*e)) + .union(p.sub.as_ref().unwrap_or(&default)) + .filter(|e| !p.add.as_ref().unwrap_or(&default).contains(*e)) .cloned() .collect(); final_sub_clone.as_ref().borrow_mut().extend(union); @@ -946,6 +967,7 @@ impl OptStack { I: Iterator, { let env = self.get_final_env(opt_filter); + println!("env: {:?}", env); if env.default_behavior.is_keep() { warn!("Keeping environment variables is dangerous operation, it can lead to security vulnerabilities. Please consider using delete instead. @@ -1000,24 +1022,21 @@ impl OptStack { } fn get_final_env(&self, cmd_filter: Option) -> SEnvOptions { - let mut final_behavior = cmd_filter - .as_ref() - .and_then(|f| f.env_behavior) - .unwrap_or_default(); + let mut final_behavior = EnvBehavior::default(); let mut final_set = HashMap::new(); let mut final_keep = LinkedHashSet::new(); let mut final_check = LinkedHashSet::new(); let mut final_delete = LinkedHashSet::new(); - let overriden = cmd_filter - .as_ref() - .is_some_and(|f| f.env_behavior.is_some()); + let overriden_behavior = cmd_filter.as_ref().and_then(|f| f.env_behavior); self.iter_in_options(|opt| { if let Some(p) = opt.env.borrow().as_ref() { final_behavior = match p.default_behavior { - EnvBehavior::Delete => { + EnvBehavior::Delete | EnvBehavior::Keep => { // policy is to delete, so we add whitelist and remove blacklist final_keep = p .keep + .as_ref() + .unwrap_or(&LinkedHashSet::new()) .iter() .filter(|e| { !p.set.env_matches(e) @@ -1028,75 +1047,37 @@ impl OptStack { .collect(); final_check = p .check + .as_ref() + .unwrap_or(&LinkedHashSet::new()) .iter() .filter(|e| !p.set.env_matches(e) || !p.delete.env_matches(e)) .cloned() .collect(); - final_set = p.set.clone(); - debug!("check: {:?}", final_check); - if overriden { - final_behavior - } else { - p.default_behavior - } - } - EnvBehavior::Keep => { - //policy is to keep, so we remove blacklist and add whitelist final_delete = p .delete + .as_ref() + .unwrap_or(&LinkedHashSet::new()) .iter() - .filter(|e| { - !p.set.env_matches(e) - || !p.keep.env_matches(e) - || !p.check.env_matches(e) - }) - .cloned() - .collect(); - final_check = p - .check - .iter() - .filter(|e| !p.set.env_matches(e) || !p.keep.env_matches(e)) + .filter(|e| !p.set.env_matches(e) || !p.check.env_matches(e)) .cloned() .collect(); final_set = p.set.clone(); - if overriden { - final_behavior - } else { - p.default_behavior - } + debug!("check: {:?}", final_check); + p.default_behavior } EnvBehavior::Inherit => { - if final_behavior.is_delete() { - final_keep = final_keep - .union(&p.keep) - .filter(|e| { - !p.set.env_matches(e) - || !p.delete.env_matches(e) - || !p.check.env_matches(e) - }) - .cloned() - .collect(); - final_check = final_check - .union(&p.check) - .filter(|e| !p.set.env_matches(e) || !p.delete.env_matches(e)) - .cloned() - .collect(); - } else { - final_delete = final_delete - .union(&p.delete) - .filter(|e| { - !p.set.env_matches(e) - || !p.keep.env_matches(e) - || !p.check.env_matches(e) - }) - .cloned() - .collect(); - final_check = final_check - .union(&p.check) - .filter(|e| !p.set.env_matches(e) || !p.keep.env_matches(e)) - .cloned() - .collect(); - } + final_keep = final_keep + .union(p.keep.as_ref().unwrap_or(&LinkedHashSet::new())) + .cloned() + .collect(); + final_check = final_check + .union(p.check.as_ref().unwrap_or(&LinkedHashSet::new())) + .cloned() + .collect(); + final_delete = final_delete + .union(p.delete.as_ref().unwrap_or(&LinkedHashSet::new())) + .cloned() + .collect(); final_set.extend(p.set.clone()); debug!("check: {:?}", final_check); final_behavior @@ -1104,7 +1085,7 @@ impl OptStack { }; } }); - SEnvOptions::builder(final_behavior) + SEnvOptions::builder(overriden_behavior.unwrap_or(final_behavior)) .set(final_set) .keep(final_keep) .unwrap() @@ -1135,12 +1116,12 @@ impl OptStack { EnvBehavior::Delete => { // policy is to delete, so we add whitelist and remove blacklist final_keep = final_keep - .union(&p.keep) + .union(p.keep.as_ref().unwrap_or(&LinkedHashSet::new())) .filter(|e| !p.check.env_matches(e) || !p.delete.env_matches(e)) .cloned() .collect(); final_check = final_check - .union(&p.check) + .union(p.check.as_ref().unwrap_or(&LinkedHashSet::new())) .filter(|e| !p.delete.env_matches(e)) .cloned() .collect(); @@ -1149,12 +1130,12 @@ impl OptStack { EnvBehavior::Keep => { //policy is to keep, so we remove blacklist and add whitelist final_delete = final_delete - .union(&p.delete) + .union(p.delete.as_ref().unwrap_or(&LinkedHashSet::new())) .filter(|e| !p.keep.env_matches(e) || !p.check.env_matches(e)) .cloned() .collect(); final_check = final_check - .union(&p.check) + .union(p.check.as_ref().unwrap_or(&LinkedHashSet::new())) .filter(|e| !p.keep.env_matches(e)) .cloned() .collect(); @@ -1163,23 +1144,23 @@ impl OptStack { EnvBehavior::Inherit => { if final_behavior.is_delete() { final_keep = final_keep - .union(&p.keep) + .union(p.keep.as_ref().unwrap_or(&LinkedHashSet::new())) .filter(|e| !p.delete.env_matches(e) || !p.check.env_matches(e)) .cloned() .collect(); final_check = final_check - .union(&p.check) + .union(p.check.as_ref().unwrap_or(&LinkedHashSet::new())) .filter(|e| !p.delete.env_matches(e)) .cloned() .collect(); } else { final_delete = final_delete - .union(&p.delete) + .union(p.delete.as_ref().unwrap_or(&LinkedHashSet::new())) .filter(|e| !p.keep.env_matches(e) || !p.check.env_matches(e)) .cloned() .collect(); final_check = final_check - .union(&p.check) + .union(p.check.as_ref().unwrap_or(&LinkedHashSet::new())) .filter(|e| !p.keep.env_matches(e)) .cloned() .collect(); @@ -1286,10 +1267,23 @@ impl PartialEq for OptStack { fn eq(&self, other: &Self) -> bool { // we must assess that every option result in the same final result let path = self.get_final_path(); + let default = LinkedHashSet::new(); let other_path = other.get_final_path(); let res = path.default_behavior == other_path.default_behavior - && path.add.symmetric_difference(&other_path.add).count() == 0 - && path.sub.symmetric_difference(&other_path.sub).count() == 0 + && path + .add + .as_ref() + .unwrap_or(&default) + .symmetric_difference(other_path.add.as_ref().unwrap_or(&default)) + .count() + == 0 + && path + .sub + .as_ref() + .unwrap_or(&default) + .symmetric_difference(other_path.sub.as_ref().unwrap_or(&default)) + .count() + == 0 && self.get_root_behavior().1 == other.get_root_behavior().1 && self.get_bounding().1 == other.get_bounding().1 && self.get_wildcard().1 == other.get_wildcard().1 @@ -1307,8 +1301,18 @@ impl PartialEq for OptStack { path.default_behavior == other_path.default_behavior, path.add, other_path.add, - path.add.symmetric_difference(&other_path.add).count() == 0, - path.sub.symmetric_difference(&other_path.sub).count() == 0, + path.add + .as_ref() + .unwrap_or(&default) + .symmetric_difference(other_path.add.as_ref().unwrap_or(&default)) + .count() + == 0, + path.sub + .as_ref() + .unwrap_or(&default) + .symmetric_difference(other_path.sub.as_ref().unwrap_or(&default)) + .count() + == 0, self.get_root_behavior().1 == other.get_root_behavior().1, self.get_bounding().1 == other.get_bounding().1, self.get_wildcard().1 == other.get_wildcard().1, @@ -1522,7 +1526,10 @@ mod tests { let binding = OptStack::from_task(config.task("test", 1).unwrap()).to_opt(); let options = binding.as_ref().borrow(); let res = &options.env.as_ref().unwrap().keep; - assert!(res.contains(&EnvKey::from("env1"))); + assert!(res + .as_ref() + .unwrap_or(&LinkedHashSet::new()) + .contains(&EnvKey::from("env1"))); } // test to_opt() for OptStack @@ -1610,6 +1617,7 @@ mod tests { .build() }) .build(); + let default = LinkedHashSet::new(); let stack = OptStack::from_roles(config.clone()); let opt = stack.to_opt(); let global_options = opt.as_ref().borrow(); @@ -1618,7 +1626,14 @@ mod tests { PathBehavior::Delete ); assert!(hashset_vec_equal( - global_options.path.as_ref().unwrap().add.clone(), + global_options + .path + .as_ref() + .unwrap() + .add + .as_ref() + .unwrap_or(&default) + .clone(), vec!["path1"] )); assert_eq!( @@ -1626,7 +1641,14 @@ mod tests { EnvBehavior::Delete ); assert!(env_key_set_equal( - global_options.env.as_ref().unwrap().keep.clone(), + global_options + .env + .as_ref() + .unwrap() + .keep + .as_ref() + .unwrap_or(&LinkedHashSet::new()) + .clone(), vec![EnvKey::from("env2")] )); assert_eq!(global_options.root.unwrap(), SPrivileged::Privileged); @@ -1651,7 +1673,14 @@ mod tests { PathBehavior::Delete ); assert!(hashset_vec_equal( - role_options.path.as_ref().unwrap().add.clone(), + role_options + .path + .as_ref() + .unwrap() + .add + .as_ref() + .unwrap_or(&default) + .clone(), vec!["path1", "path2"] )); assert_eq!( @@ -1659,7 +1688,14 @@ mod tests { EnvBehavior::Delete ); assert!(env_key_set_equal( - role_options.env.as_ref().unwrap().keep.clone(), + role_options + .env + .as_ref() + .unwrap() + .keep + .as_ref() + .unwrap_or(&LinkedHashSet::new()) + .clone(), vec![EnvKey::from("env1")] )); assert_eq!(role_options.root.unwrap(), SPrivileged::Privileged); @@ -1681,7 +1717,14 @@ mod tests { PathBehavior::Delete ); assert!(hashset_vec_equal( - task_options.path.as_ref().unwrap().add.clone(), + task_options + .path + .as_ref() + .unwrap() + .add + .as_ref() + .unwrap_or(&default) + .clone(), vec!["path1", "path2", "path3"] )); assert_eq!( @@ -1689,7 +1732,14 @@ mod tests { EnvBehavior::Delete ); assert!(env_key_set_equal( - task_options.env.as_ref().unwrap().keep.clone(), + task_options + .env + .as_ref() + .unwrap() + .keep + .as_ref() + .unwrap_or(&LinkedHashSet::new()) + .clone(), vec![EnvKey::from("env1"), EnvKey::from("env3")] )); assert_eq!(task_options.root.unwrap(), SPrivileged::User); @@ -1913,4 +1963,251 @@ mod tests { assert!(result.get("env2").is_none()); assert_eq!(result.get("env4").unwrap(), "value4"); } + + #[test] + fn is_wildcard_env_key() { + assert!(!is_valid_env_name("TEST_.*")); + assert!(!is_valid_env_name("123")); + assert!(!is_valid_env_name("")); + assert!(is_regex("TEST_.*")); + } + + #[test] + fn test_wildcard_env() { + let config = SConfig::builder() + .role( + SRole::builder("test") + .task( + STask::builder(IdTask::Number(1)) + .options(|opt| { + opt.env( + SEnvOptions::builder(EnvBehavior::Delete) + .keep(["TEST_.*"]) + .unwrap() + .build(), + ) + .build() + }) + .build(), + ) + .build(), + ) + .build(); + let options = OptStack::from_task(config.task("test", 1).unwrap()); + let mut test_env = HashMap::new(); + test_env.insert("TEST_A".to_string(), "value1".to_string()); + test_env.insert("TEST_B".into(), "value2".into()); + test_env.insert("TESTaA".into(), "value3".into()); + let cred = Cred::builder().user_id(0).group_id(0).build(); + let result = options + .calculate_filtered_env(None, cred, test_env.into_iter()) + .unwrap(); + assert_eq!(result.get("TEST_A").unwrap(), "value1"); + assert_eq!(result.get("TEST_B").unwrap(), "value2"); + assert!(result.get("TESTaA").is_none()); + } + + #[test] + fn test_safe_path() { + let path = std::env::var("PATH").unwrap(); + std::env::set_var("PATH", "/sys:./proc:/tmp:/bin"); + let config = SConfig::builder() + .role( + SRole::builder("test") + .task( + STask::builder(IdTask::Number(1)) + .options(|opt| { + opt.path( + SPathOptions::builder(PathBehavior::KeepSafe) + .add(Vec::::new()) + .sub(["/sys"]) + .build(), + ) + .build() + }) + .build(), + ) + .build(), + ) + .build(); + let options = OptStack::from_task(config.task("test", 1).unwrap()); + let res = options.calculate_path(); + + assert_eq!(res, "/tmp:/bin"); + std::env::set_var("PATH", path); + } + + #[test] + fn test_unsafe_path() { + let path = std::env::var("PATH").unwrap(); + + let config = SConfig::builder() + .role( + SRole::builder("test") + .task( + STask::builder(IdTask::Number(1)) + .options(|opt| { + opt.path( + SPathOptions::builder(PathBehavior::KeepUnsafe) + .add(Vec::::new()) + .sub(["/sys"]) + .build(), + ) + .build() + }) + .build(), + ) + .build(), + ) + .build(); + let options = OptStack::from_task(config.task("test", 1).unwrap()); + std::env::set_var("PATH", "/sys:./proc:/tmp:/bin"); + let res = options.calculate_path(); + assert_eq!(res, "./proc:/tmp:/bin"); + std::env::set_var("PATH", path); + } + + #[test] + fn test_inherit_keep_path() { + let path = std::env::var("PATH").unwrap(); + let config = SConfig::builder() + .role( + SRole::builder("test") + .task( + STask::builder(IdTask::Number(1)) + .options(|opt| { + opt.path( + SPathOptions::builder(PathBehavior::Inherit) + .add(Vec::::new()) + .sub(["/sys"]) + .build(), + ) + .build() + }) + .build(), + ) + .options(|opt| { + opt.path( + SPathOptions::builder(PathBehavior::KeepSafe) + .add(Vec::::new()) + .sub(["/tmp"]) + .build(), + ) + .build() + }) + .build(), + ) + .build(); + let options = OptStack::from_task(config.task("test", 1).unwrap()); + std::env::set_var("PATH", "/sys:./proc:/tmp:/bin"); + let res = options.calculate_path(); + + assert_eq!(res, "/bin"); + std::env::set_var("PATH", path); + } + + #[test] + fn test_final_env_keep() { + let config = SConfig::builder() + .role( + SRole::builder("test") + .task( + STask::builder(IdTask::Number(1)) + .options(|opt| { + opt.env( + SEnvOptions::builder(EnvBehavior::Inherit) + .delete(["env1"]) + .unwrap() + .build(), + ) + .build() + }) + .build(), + ) + .options(|opt| { + opt.env( + SEnvOptions::builder(EnvBehavior::Inherit) + .delete(["env2"]) + .unwrap() + .build(), + ) + .build() + }) + .build(), + ) + .options(|opt| { + opt.env( + SEnvOptions::builder(EnvBehavior::Keep) + .delete(["env3"]) + .unwrap() + .build(), + ) + .build() + }) + .build(); + let options = OptStack::from_task(config.task("test", 1).unwrap()); + let test_env = [ + ("env1", "value1"), + ("env2", "value2"), + ("env3", "value3"), + ("env4", "value4"), + ("env5", "value5"), + ] + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())); + + let cred = Cred::builder().user_id(0).group_id(0).build(); + let result = options + .calculate_filtered_env(None, cred, test_env.into_iter()) + .unwrap(); + assert!(result.get("env1").is_none()); + assert!(result.get("env2").is_none()); + assert!(result.get("env3").is_none()); + assert_eq!(result.get("env4").unwrap(), "value4"); + assert_eq!(result.get("env5").unwrap(), "value5"); + } + + #[test] + fn test_opt_filter_env() { + let config = SConfig::builder() + .role( + SRole::builder("test") + .task( + STask::builder(IdTask::Number(1)) + .options(|opt| { + opt.env( + SEnvOptions::builder(EnvBehavior::Delete) + .delete(["envA"]) + .unwrap() + .override_behavior(true) + .build(), + ) + .build() + }) + .build(), + ) + .build(), + ) + .build(); + let options = OptStack::from_task(config.task("test", 1).unwrap()); + let test_env = [("envA", "value1"), ("envB", "value2"), ("envC", "value3")] + .iter() + .map(|(k, v)| (k.to_string(), v.to_string())); + + let cred = Cred::builder().user_id(0).group_id(0).build(); + let result = options + .calculate_filtered_env( + Some( + FilterMatcher::builder() + .env_behavior(EnvBehavior::Keep) + .build(), + ), + cred, + test_env.into_iter(), + ) + .unwrap(); + assert!(result.get("envA").is_none()); + assert_eq!(result.get("envB").unwrap(), "value2"); + assert_eq!(result.get("envC").unwrap(), "value3"); + } } diff --git a/rar-common/src/database/structs.rs b/rar-common/src/database/structs.rs index 2e1de1b1..1456a807 100644 --- a/rar-common/src/database/structs.rs +++ b/rar-common/src/database/structs.rs @@ -706,12 +706,16 @@ mod tests { use capctl::Cap; use chrono::Duration; + use linked_hash_set::LinkedHashSet; use crate::{ as_borrow, database::{ actor::SGroupType, - options::{EnvBehavior, PathBehavior, SAuthentication, SBounding, SEnvOptions, SPathOptions, SPrivileged, STimeout, TimestampType}, + options::{ + EnvBehavior, PathBehavior, SAuthentication, SBounding, SEnvOptions, SPathOptions, + SPrivileged, STimeout, TimestampType, + }, }, }; @@ -790,12 +794,28 @@ mod tests { let options = config.options.as_ref().unwrap().as_ref().borrow(); let path = options.path.as_ref().unwrap(); assert_eq!(path.default_behavior, PathBehavior::Delete); - assert!(path.add.front().is_some_and(|s| s == "path_add")); + let default = LinkedHashSet::new(); + assert!(path + .add + .as_ref() + .unwrap_or(&default) + .front() + .is_some_and(|s| s == "path_add")); let env = options.env.as_ref().unwrap(); assert_eq!(env.default_behavior, EnvBehavior::Delete); assert!(env.override_behavior.is_some_and(|b| b)); - assert!(env.keep.front().is_some_and(|s| s == "keep_env")); - assert!(env.check.front().is_some_and(|s| s == "check_env")); + assert!(env + .keep + .as_ref() + .unwrap_or(&LinkedHashSet::new()) + .front() + .is_some_and(|s| s == "keep_env")); + assert!(env + .check + .as_ref() + .unwrap_or(&LinkedHashSet::new()) + .front() + .is_some_and(|s| s == "check_env")); assert!(options.root.as_ref().unwrap().is_privileged()); assert!(options.bounding.as_ref().unwrap().is_ignore()); assert_eq!(options.authentication, Some(SAuthentication::Skip)); @@ -1035,11 +1055,27 @@ mod tests { let options = config.options.as_ref().unwrap().as_ref().borrow(); let path = options.path.as_ref().unwrap(); assert_eq!(path.default_behavior, PathBehavior::Delete); - assert!(path.add.front().is_some_and(|s| s == "path_add")); + let default = LinkedHashSet::new(); + assert!(path + .add + .as_ref() + .unwrap_or(&default) + .front() + .is_some_and(|s| s == "path_add")); let env = options.env.as_ref().unwrap(); assert_eq!(env.default_behavior, EnvBehavior::Delete); - assert!(env.keep.front().is_some_and(|s| s == "keep_env")); - assert!(env.check.front().is_some_and(|s| s == "check_env")); + assert!(env + .keep + .as_ref() + .unwrap() + .front() + .is_some_and(|s| s == "keep_env")); + assert!(env + .check + .as_ref() + .unwrap() + .front() + .is_some_and(|s| s == "check_env")); assert!(options.root.as_ref().unwrap().is_privileged()); assert!(options.bounding.as_ref().unwrap().is_ignore()); assert_eq!(options.authentication, Some(SAuthentication::Skip)); @@ -1090,59 +1126,92 @@ mod tests { #[test] fn test_serialize() { - let config = SConfig::builder().role( - SRole::builder("role1") - .actor(SActor::user("user1").build()) - .actor(SActor::group([SGroupType::from("group1"), SGroupType::from(1000)]).build()) - .task( - STask::builder("task1") - .purpose("purpose1".into()) - .cred( - SCredentials::builder() - .setuid(SUserChooser::ChooserStruct( - SSetuidSet::builder("user1", SetBehavior::All) - .add(["user2".into()]) - .sub(["user3".into()]) - .build(), - )) - .setgid(["setgid1"]) - .capabilities( - SCapabilities::builder(SetBehavior::All) - .add_cap(Cap::NET_BIND_SERVICE) - .sub_cap(Cap::SYS_ADMIN) - .build(), - ) - .build(), - ) - .commands( - SCommands::builder(SetBehavior::All) - .add(["cmd1".into()]) - .sub(["cmd2".into()]) - .build(), - ) + let config = SConfig::builder() + .role( + SRole::builder("role1") + .actor(SActor::user("user1").build()) + .actor( + SActor::group([SGroupType::from("group1"), SGroupType::from(1000)]).build(), + ) + .task( + STask::builder("task1") + .purpose("purpose1".into()) + .cred( + SCredentials::builder() + .setuid(SUserChooser::ChooserStruct( + SSetuidSet::builder("user1", SetBehavior::All) + .add(["user2".into()]) + .sub(["user3".into()]) + .build(), + )) + .setgid(["setgid1"]) + .capabilities( + SCapabilities::builder(SetBehavior::All) + .add_cap(Cap::NET_BIND_SERVICE) + .sub_cap(Cap::SYS_ADMIN) + .build(), + ) + .build(), + ) + .commands( + SCommands::builder(SetBehavior::All) + .add(["cmd1".into()]) + .sub(["cmd2".into()]) + .build(), + ) + .build(), + ) + .build(), + ) + .options(|opt| { + opt.path( + SPathOptions::builder(PathBehavior::Delete) + .add(["path_add"]) + .sub(["path_sub"]) + .build(), + ) + .env( + SEnvOptions::builder(EnvBehavior::Delete) + .override_behavior(true) + .keep(["keep_env"]) + .unwrap() + .check(["check_env"]) + .unwrap() + .build(), + ) + .root(SPrivileged::Privileged) + .bounding(SBounding::Ignore) + .authentication(SAuthentication::Skip) + .wildcard_denied("wildcards") + .timeout( + STimeout::builder() + .type_field(TimestampType::PPID) + .duration(Duration::minutes(5)) .build(), ) - .build(), - ).options( | opt | opt.path(SPathOptions::builder(PathBehavior::Delete).add(["path_add"]).sub(["path_sub"]).build()) - .env(SEnvOptions::builder(EnvBehavior::Delete).override_behavior(true).keep(["keep_env"]).unwrap().check(["check_env"]).unwrap().build()) - .root(SPrivileged::Privileged) - .bounding(SBounding::Ignore) - .authentication(SAuthentication::Skip) - .wildcard_denied("wildcards") - .timeout(STimeout::builder().type_field(TimestampType::PPID).duration(Duration::minutes(5)).build()) - .build() - ).build(); + .build() + }) + .build(); let config = serde_json::to_string_pretty(&config).unwrap(); println!("{}", config); } - #[test] fn test_serialize_operride_behavior_option() { - let config = SConfig::builder().options( | opt | opt.env(SEnvOptions::builder(EnvBehavior::Inherit).override_behavior(true).build()) - .build() - ).build(); + let config = SConfig::builder() + .options(|opt| { + opt.env( + SEnvOptions::builder(EnvBehavior::Inherit) + .override_behavior(true) + .build(), + ) + .build() + }) + .build(); let config = serde_json::to_string(&config).unwrap(); - assert_eq!(config,"{\"options\":{\"env\":{\"override_behavior\":true}}}"); + assert_eq!( + config, + "{\"options\":{\"env\":{\"override_behavior\":true}}}" + ); } } diff --git a/rar-common/src/lib.rs b/rar-common/src/lib.rs index 896f2891..68fe82c7 100644 --- a/rar-common/src/lib.rs +++ b/rar-common/src/lib.rs @@ -108,8 +108,7 @@ pub struct Settings { pub ldap: Option, } -#[derive(Serialize, Deserialize, Debug, Clone, Builder)] -#[derive(Default)] +#[derive(Serialize, Deserialize, Debug, Clone, Builder, Default)] pub struct RemoteStorageSettings { #[serde(skip_serializing_if = "Option::is_none")] #[builder(name = not_immutable,with = || false)] @@ -194,8 +193,6 @@ impl Default for Settings { } } - - pub fn save_settings(settings: Rc>) -> Result<(), Box> { let default_remote: RemoteStorageSettings = RemoteStorageSettings::default(); // remove immutable flag diff --git a/src/chsr/cli/mod.rs b/src/chsr/cli/mod.rs index edf76386..dbc0a0cf 100644 --- a/src/chsr/cli/mod.rs +++ b/src/chsr/cli/mod.rs @@ -41,6 +41,7 @@ where mod tests { use std::{env::current_dir, io::Write}; + use linked_hash_set::LinkedHashSet; use rar_common::{ database::{ actor::SActor, @@ -1340,6 +1341,7 @@ mod tests { debug!("{}", e); }) .is_ok_and(|b| b)); + let default = LinkedHashSet::new(); assert!(config.as_ref().borrow()[0].as_ref().borrow().tasks[0] .as_ref() .borrow() @@ -1352,6 +1354,8 @@ mod tests { .as_ref() .unwrap() .add + .as_ref() + .unwrap_or(&default) .contains(&"/usr/bin".to_string())); assert!(config.as_ref().borrow()[0].as_ref().borrow().tasks[0] .as_ref() @@ -1365,6 +1369,8 @@ mod tests { .as_ref() .unwrap() .add + .as_ref() + .unwrap_or(&default) .contains(&"/bin".to_string())); assert!(main( &Storage::JSON(config.clone()), @@ -1389,6 +1395,8 @@ mod tests { .as_ref() .unwrap() .add + .as_ref() + .unwrap_or(&default) .contains(&"/usr/bin".to_string())); assert!(!config.as_ref().borrow()[0].as_ref().borrow().tasks[0] .as_ref() @@ -1402,6 +1410,8 @@ mod tests { .as_ref() .unwrap() .add + .as_ref() + .unwrap_or(&default) .contains(&"/bin".to_string())); debug!("====="); assert!(main( @@ -1427,6 +1437,8 @@ mod tests { .as_ref() .unwrap() .add + .as_ref() + .unwrap_or(&default) .is_empty()); debug!("====="); assert!(main( @@ -1452,6 +1464,8 @@ mod tests { .as_ref() .unwrap() .add + .as_ref() + .unwrap_or(&default) .contains(&"/usr/bin".to_string())); assert!(config.as_ref().borrow()[0].as_ref().borrow().tasks[0] .as_ref() @@ -1465,6 +1479,8 @@ mod tests { .as_ref() .unwrap() .add + .as_ref() + .unwrap_or(&default) .contains(&"/bin".to_string())); assert_eq!( config.as_ref().borrow()[0].as_ref().borrow().tasks[0] @@ -1479,6 +1495,8 @@ mod tests { .as_ref() .unwrap() .add + .as_ref() + .unwrap_or(&default) .len(), 2 ); @@ -1518,6 +1536,8 @@ mod tests { .as_ref() .unwrap() .sub + .as_ref() + .unwrap_or(&default) .contains(&"/tmp".to_string())); assert!(main( &Storage::JSON(config.clone()), @@ -1558,6 +1578,8 @@ mod tests { .as_ref() .unwrap() .sub + .as_ref() + .unwrap_or(&default) .len(), 1 ); @@ -1573,6 +1595,8 @@ mod tests { .as_ref() .unwrap() .sub + .as_ref() + .unwrap_or(&default) .contains(&"/tmp".to_string())); assert!(!config.as_ref().borrow()[0].as_ref().borrow().tasks[0] .as_ref() @@ -1586,6 +1610,8 @@ mod tests { .as_ref() .unwrap() .sub + .as_ref() + .unwrap_or(&default) .contains(&"/usr/bin".to_string())); assert!(!config.as_ref().borrow()[0].as_ref().borrow().tasks[0] .as_ref() @@ -1599,6 +1625,8 @@ mod tests { .as_ref() .unwrap() .sub + .as_ref() + .unwrap_or(&default) .contains(&"/bin".to_string())); teardown("r_complete_t_t_complete_o_path_whitelist_add"); } @@ -1669,6 +1697,8 @@ mod tests { .as_ref() .unwrap() .keep + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); assert!(config.as_ref().borrow()[0].as_ref().borrow().tasks[0] .as_ref() @@ -1682,6 +1712,8 @@ mod tests { .as_ref() .unwrap() .keep + .as_ref() + .unwrap() .contains(&"VAR2".to_string().into())); assert_eq!( config.as_ref().borrow()[0].as_ref().borrow().tasks[0] @@ -1696,6 +1728,8 @@ mod tests { .as_ref() .unwrap() .keep + .as_ref() + .unwrap() .len(), 2 ); @@ -1746,6 +1780,8 @@ mod tests { .as_ref() .unwrap() .delete + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); assert!(config.as_ref().borrow()[0].as_ref().borrow().tasks[0] .as_ref() @@ -1759,6 +1795,8 @@ mod tests { .as_ref() .unwrap() .delete + .as_ref() + .unwrap() .contains(&"VAR2".to_string().into())); assert_eq!( config.as_ref().borrow()[0].as_ref().borrow().tasks[0] @@ -1773,6 +1811,8 @@ mod tests { .as_ref() .unwrap() .delete + .as_ref() + .unwrap() .len(), 2 ); @@ -2172,6 +2212,8 @@ mod tests { .as_ref() .unwrap() .keep + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); assert!( config.as_ref().borrow()[0].as_ref().borrow().tasks[0] @@ -2186,6 +2228,8 @@ mod tests { .as_ref() .unwrap() .keep + .as_ref() + .unwrap() .len() > 1 ); @@ -2212,6 +2256,8 @@ mod tests { .as_ref() .unwrap() .keep + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); debug!("====="); assert!(main( @@ -2237,6 +2283,8 @@ mod tests { .as_ref() .unwrap() .keep + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); assert_eq!( config.as_ref().borrow()[0].as_ref().borrow().tasks[0] @@ -2251,6 +2299,8 @@ mod tests { .as_ref() .unwrap() .keep + .as_ref() + .unwrap() .len(), 1 ); @@ -2288,7 +2338,7 @@ mod tests { .as_ref() .unwrap() .keep - .is_empty()); + .is_none()); teardown("r_complete_t_t_complete_o_env_whitelist_purge"); } #[test] @@ -2323,6 +2373,8 @@ mod tests { .as_ref() .unwrap() .delete + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); assert!(main( &Storage::JSON(config.clone()), @@ -2347,6 +2399,8 @@ mod tests { .as_ref() .unwrap() .delete + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); teardown("r_complete_t_t_complete_o_env_blacklist_add_MYVAR"); } @@ -2382,6 +2436,8 @@ mod tests { .as_ref() .unwrap() .delete + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); assert_eq!( config.as_ref().borrow()[0].as_ref().borrow().tasks[0] @@ -2396,6 +2452,8 @@ mod tests { .as_ref() .unwrap() .delete + .as_ref() + .unwrap() .len(), 1 ); @@ -2433,7 +2491,7 @@ mod tests { .as_ref() .unwrap() .delete - .is_empty()); + .is_none()); teardown("r_complete_t_t_complete_o_env_blacklist_purge"); } #[test] @@ -2468,6 +2526,8 @@ mod tests { .as_ref() .unwrap() .check + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); debug!("====="); assert!(main( @@ -2493,6 +2553,8 @@ mod tests { .as_ref() .unwrap() .check + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); debug!("====="); assert!(main( @@ -2518,6 +2580,8 @@ mod tests { .as_ref() .unwrap() .check + .as_ref() + .unwrap() .contains(&"MYVAR".to_string().into())); assert_eq!( config.as_ref().borrow()[0].as_ref().borrow().tasks[0] @@ -2532,6 +2596,8 @@ mod tests { .as_ref() .unwrap() .check + .as_ref() + .unwrap() .len(), 1 ); @@ -2559,7 +2625,7 @@ mod tests { .as_ref() .unwrap() .check - .is_empty()); + .is_none()); teardown("r_complete_t_t_complete_o_env_checklist_add_MYVAR"); } #[test] diff --git a/src/chsr/cli/process/json.rs b/src/chsr/cli/process/json.rs index 4fe165ca..0bf9d6d7 100644 --- a/src/chsr/cli/process/json.rs +++ b/src/chsr/cli/process/json.rs @@ -1,7 +1,7 @@ use std::{cell::RefCell, collections::HashMap, error::Error, ops::Deref, rc::Rc}; use linked_hash_set::LinkedHashSet; -use log::debug; +use log::{debug, warn}; use crate::cli::data::{InputAction, RoleType, SetListType, TaskType, TimeoutOpt}; @@ -598,14 +598,14 @@ pub fn env_set_policylist( opt.as_ref().borrow_mut().env = Some(SEnvOptions { default_behavior: options_env_policy, keep: if options_env_policy.is_delete() { - options_env.clone() + Some(options_env.clone()) } else { - LinkedHashSet::new() + None }, delete: if options_env_policy.is_keep() { - options_env.clone() + Some(options_env.clone()) } else { - LinkedHashSet::new() + None }, ..Default::default() }); @@ -665,19 +665,18 @@ pub fn path_set( ) -> Result> { debug!("chsr o path set"); perform_on_target_opt(rconfig, role_id, task_id, |opt: Rc>| { - let mut default_path = SPathOptions::default(); let mut binding = opt.as_ref().borrow_mut(); - let path = binding.path.as_mut().unwrap_or(&mut default_path); + let path = binding.path.get_or_insert(SPathOptions::default()); match setlist_type { Some(SetListType::White) => { - path.add = options_path.split(':').map(|s| s.to_string()).collect(); + path.add = Some(options_path.split(':').map(|s| s.to_string()).collect()); } Some(SetListType::Black) => { - path.sub = options_path.split(':').map(|s| s.to_string()).collect(); + path.sub = Some(options_path.split(':').map(|s| s.to_string()).collect()); } None => { path.default_behavior = PathBehavior::Delete; - path.add = options_path.split(':').map(|s| s.to_string()).collect(); + path.add = Some(options_path.split(':').map(|s| s.to_string()).collect()); } _ => unreachable!("Unknown setlist type"), } @@ -694,15 +693,18 @@ pub fn path_purge( ) -> Result> { debug!("chsr o path purge"); perform_on_target_opt(rconfig, role_id, task_id, |opt: Rc>| { - let mut default_path = SPathOptions::default(); let mut binding = opt.as_ref().borrow_mut(); - let path = binding.path.as_mut().unwrap_or(&mut default_path); + let path = binding.path.get_or_insert(SPathOptions::default()); match setlist_type { Some(SetListType::White) => { - path.add.clear(); + if let Some(add) = &mut path.add { + add.clear(); + } } Some(SetListType::Black) => { - path.sub.clear(); + if let Some(sub) = &mut path.sub { + sub.clear(); + } } _ => unreachable!("Unknown setlist type"), } @@ -720,22 +722,21 @@ pub fn env_whitelist_set( ) -> Result> { debug!("chsr o env whitelist set"); perform_on_target_opt(rconfig, role_id, task_id, |opt: Rc>| { - let mut default_env = SEnvOptions::default(); let mut binding = opt.as_ref().borrow_mut(); - let env = binding.env.as_mut().unwrap_or(&mut default_env); + let env = binding.env.get_or_insert(SEnvOptions::default()); match setlist_type { Some(SetListType::White) => { - env.keep = options_env.clone(); + env.keep = Some(options_env.clone()); } Some(SetListType::Black) => { - env.delete = options_env.clone(); + env.delete = Some(options_env.clone()); } Some(SetListType::Check) => { - env.check = options_env.clone(); + env.check = Some(options_env.clone()); } None => { env.default_behavior = EnvBehavior::Delete; - env.keep = options_env.clone(); + env.keep = Some(options_env.clone()); } _ => unreachable!("Unknown setlist type"), } @@ -816,6 +817,7 @@ pub fn path_setlist2( Some(SetListType::White) => match action { InputAction::Add => { path.add + .get_or_insert(LinkedHashSet::new()) .extend(options_path.split(':').map(|s| s.to_string())); } InputAction::Del => { @@ -824,20 +826,24 @@ pub fn path_setlist2( .split(':') .map(|s| s.to_string()) .collect::>(); - path.add = path - .add - .difference(&hashset) - .cloned() - .collect::>(); + if let Some(path) = &mut path.add { + *path = path + .difference(&hashset) + .cloned() + .collect::>(); + } else { + warn!("No path to remove from del list"); + } } InputAction::Set => { - path.add = options_path.split(':').map(|s| s.to_string()).collect(); + path.add = Some(options_path.split(':').map(|s| s.to_string()).collect()); } _ => unreachable!("Unknown action {:?}", action), }, Some(SetListType::Black) => match action { InputAction::Add => { path.sub + .get_or_insert(LinkedHashSet::new()) .extend(options_path.split(':').map(|s| s.to_string())); } InputAction::Del => { @@ -846,14 +852,17 @@ pub fn path_setlist2( .split(':') .map(|s| s.to_string()) .collect::>(); - path.sub = path - .sub - .difference(&hashset) - .cloned() - .collect::>(); + if let Some(path) = &mut path.sub { + *path = path + .difference(&hashset) + .cloned() + .collect::>(); + } else { + warn!("No path to remove from del list"); + } } InputAction::Set => { - path.sub = options_path.split(':').map(|s| s.to_string()).collect(); + path.sub = Some(options_path.split(':').map(|s| s.to_string()).collect()); } _ => unreachable!("Unknown action {:?}", action), }, @@ -902,36 +911,33 @@ pub fn env_setlist_add( let mut default_env = SEnvOptions::default(); let mut binding = opt.as_ref().borrow_mut(); let env = binding.env.as_mut().unwrap_or(&mut default_env); + match setlist_type { Some(SetListType::White) => match action { InputAction::Add => { if options_key_env.is_none() { return Err("Empty list".into()); } - env.keep.extend(options_key_env.as_ref().unwrap().clone()); + env.keep + .get_or_insert(LinkedHashSet::new()) + .extend(options_key_env.as_ref().unwrap().clone()); } InputAction::Del => { if options_key_env.is_none() { return Err("Empty list".into()); } - env.keep = env - .keep - .difference( - &options_key_env - .as_ref() - .unwrap() - .iter() - .cloned() - .collect::>(), - ) - .cloned() - .collect::>(); + if let Some(keep) = &mut env.keep { + *keep = keep + .difference(options_key_env.as_ref().unwrap()) + .cloned() + .collect::>(); + } } InputAction::Purge => { - env.keep = LinkedHashSet::new(); + env.keep = None; } InputAction::Set => { - env.keep = options_key_env.as_ref().unwrap().clone(); + env.keep = Some(options_key_env.as_ref().unwrap().clone()); } _ => unreachable!("Unknown action {:?}", action), }, @@ -940,23 +946,26 @@ pub fn env_setlist_add( if options_key_env.is_none() { return Err("Empty list".into()); } - env.delete.extend(options_key_env.as_ref().unwrap().clone()); + env.delete + .get_or_insert(LinkedHashSet::new()) + .extend(options_key_env.as_ref().unwrap().clone()); } InputAction::Del => { if options_key_env.is_none() { return Err("Empty list".into()); } - env.delete = env - .delete - .difference(options_key_env.as_ref().unwrap()) - .cloned() - .collect::>(); + if let Some(delete) = &mut env.delete { + *delete = delete + .difference(options_key_env.as_ref().unwrap()) + .cloned() + .collect::>(); + } } InputAction::Purge => { - env.delete = LinkedHashSet::new(); + env.delete = None; } InputAction::Set => { - env.delete = options_key_env.as_ref().unwrap().clone(); + env.delete = Some(options_key_env.as_ref().unwrap().clone()); } _ => unreachable!("Unknown action {:?}", action), }, @@ -965,23 +974,26 @@ pub fn env_setlist_add( if options_key_env.is_none() { return Err("Empty list".into()); } - env.check.extend(options_key_env.as_ref().unwrap().clone()); + env.check + .get_or_insert(LinkedHashSet::new()) + .extend(options_key_env.as_ref().unwrap().clone()); } InputAction::Del => { if options_key_env.is_none() { return Err("Empty list".into()); } - env.check = env - .check - .difference(options_key_env.as_ref().unwrap()) - .cloned() - .collect::>(); + if let Some(check) = &mut env.check { + *check = check + .difference(options_key_env.as_ref().unwrap()) + .cloned() + .collect::>(); + } } InputAction::Set => { - env.check = options_key_env.as_ref().unwrap().clone(); + env.check = Some(options_key_env.as_ref().unwrap().clone()); } InputAction::Purge => { - env.check = LinkedHashSet::new(); + env.check = None; } _ => unreachable!("Unknown action {:?}", action), }, @@ -1008,7 +1020,7 @@ pub fn env_setlist_add( }, None => match action { InputAction::Set => { - env.keep = options_key_env.as_ref().unwrap().clone(); + env.keep = Some(options_key_env.as_ref().unwrap().clone()); } _ => unreachable!("Unknown action {:?}", action), }, From 393107db3506f488b55baaa906425580c01b8562 Mon Sep 17 00:00:00 2001 From: LeChatP Date: Tue, 18 Feb 2025 10:13:16 +0100 Subject: [PATCH 3/3] fix tests on generic machine --- rar-common/src/database/actor.rs | 3 +-- rar-common/src/database/finder.rs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/rar-common/src/database/actor.rs b/rar-common/src/database/actor.rs index 8d63b4f2..d4f194c6 100644 --- a/rar-common/src/database/actor.rs +++ b/rar-common/src/database/actor.rs @@ -442,8 +442,7 @@ mod tests { fn test_fetch_user() { let user = SUserType::from("testuser"); assert!(user.fetch_user().is_none()); - - let user_by_id = SUserType::from(1001); + let user_by_id = SUserType::from(0); assert!(user_by_id.fetch_user().is_some()); } diff --git a/rar-common/src/database/finder.rs b/rar-common/src/database/finder.rs index 7f06721c..9c197f75 100644 --- a/rar-common/src/database/finder.rs +++ b/rar-common/src/database/finder.rs @@ -2071,7 +2071,7 @@ mod tests { // Commande de test let command = vec!["/bin/ls".to_string(), "-l".to_string(), "-a".to_string()]; // Exécution du match - let filter_matcher = FilterMatcher::builder().user("aitbelkacem").build(); + let filter_matcher = FilterMatcher::builder().user("nouser").build(); let result = config.matches(&cred, &Some(filter_matcher), &command);