From a392c000a7726c62c7fa7f6a14d10669deb0e845 Mon Sep 17 00:00:00 2001 From: Marc Schoolderman Date: Wed, 10 Jan 2024 12:35:38 +0100 Subject: [PATCH] include source file in syntax error struct --- src/sudo/mod.rs | 10 +++++-- src/sudoers/mod.rs | 62 ++++++++++++++++++++++++++++------------- src/sudoers/test/mod.rs | 2 +- src/visudo/mod.rs | 4 +-- 4 files changed, 54 insertions(+), 24 deletions(-) diff --git a/src/sudo/mod.rs b/src/sudo/mod.rs index e1e686aee..288dfabd8 100644 --- a/src/sudo/mod.rs +++ b/src/sudo/mod.rs @@ -46,8 +46,14 @@ impl PolicyPlugin for SudoersPolicy { let (sudoers, syntax_errors) = crate::sudoers::Sudoers::open(sudoers_path) .map_err(|e| Error::Configuration(format!("{e}")))?; - for crate::sudoers::Error(pos, error) in syntax_errors { - diagnostic::diagnostic!("{error}", sudoers_path @ pos); + for crate::sudoers::Error { + source, + location, + message, + } in syntax_errors + { + let path = source.as_deref().unwrap_or(sudoers_path); + diagnostic::diagnostic!("{message}", path @ location); } Ok(sudoers) diff --git a/src/sudoers/mod.rs b/src/sudoers/mod.rs index 45ff5243d..1f4093a41 100644 --- a/src/sudoers/mod.rs +++ b/src/sudoers/mod.rs @@ -25,7 +25,11 @@ const INCLUDE_LIMIT: u8 = 128; /// Export some necessary symbols from modules pub use ast::TextEnum; -pub struct Error(pub Option, pub String); +pub struct Error { + pub source: Option, + pub location: Option, + pub message: String, +} #[derive(Default)] pub struct Sudoers { @@ -587,12 +591,19 @@ fn analyze( } impl Sudoers { - fn include(&mut self, path: &Path, diagnostics: &mut Vec, count: &mut u8) { + fn include( + &mut self, + parent: &Path, + path: &Path, + diagnostics: &mut Vec, + count: &mut u8, + ) { if *count >= INCLUDE_LIMIT { - diagnostics.push(Error( - None, - format!("include file limit reached opening '{}'", path.display()), - )) + diagnostics.push(Error { + source: Some(parent.to_owned()), + location: None, + message: format!("include file limit reached opening '{}'", path.display()), + }) // FIXME: this will cause an error in `visudo` if we open a non-privileged sudoers file // that includes another non-privileged sudoer files. } else { @@ -609,7 +620,11 @@ fn analyze( e.to_string() }; - diagnostics.push(Error(None, message)) + diagnostics.push(Error { + source: Some(parent.to_owned()), + location: None, + message, + }) } } } @@ -641,6 +656,7 @@ fn analyze( } Sudo::Include(path) => self.include( + cur_path, &resolve_relative(cur_path, path), diagnostics, safety_count, @@ -648,18 +664,20 @@ fn analyze( Sudo::IncludeDir(path) => { if path.contains("%h") { - diagnostics.push(Error( - None, - format!("cannot open sudoers file {path}: percent escape %h in includedir is unsupported"))); + diagnostics.push(Error{ + source: Some(cur_path.to_owned()), + location: None, + message: format!("cannot open sudoers file {path}: percent escape %h in includedir is unsupported")}); continue; } let path = resolve_relative(cur_path, path); let Ok(files) = std::fs::read_dir(&path) else { - diagnostics.push(Error( - None, - format!("cannot open sudoers file {}", path.display()), - )); + diagnostics.push(Error { + source: Some(cur_path.to_owned()), + location: None, + message: format!("cannot open sudoers file {}", path.display()), + }); continue; }; let mut safe_files = files @@ -675,14 +693,16 @@ fn analyze( .collect::>(); safe_files.sort(); for file in safe_files { - self.include(file.as_ref(), diagnostics, safety_count) + self.include(cur_path, file.as_ref(), diagnostics, safety_count) } } }, - Err(basic_parser::Status::Fatal(pos, error)) => { - diagnostics.push(Error(Some(pos), error)) - } + Err(basic_parser::Status::Fatal(pos, message)) => diagnostics.push(Error { + source: Some(cur_path.to_owned()), + location: Some(pos), + message, + }), Err(_) => panic!("internal parser error"), } } @@ -756,7 +776,11 @@ fn sanitize_alias_table(table: &Vec>, diagnostics: &mut Vec) -> impl Visitor<'_, T> { fn complain(&mut self, text: String) { - self.diagnostics.push(Error(None, text)) + self.diagnostics.push(Error { + source: None, + location: None, + message: text, + }) } fn visit(&mut self, pos: usize) { diff --git a/src/sudoers/test/mod.rs b/src/sudoers/test/mod.rs index 21482cf84..7d0d0ca2e 100644 --- a/src/sudoers/test/mod.rs +++ b/src/sudoers/test/mod.rs @@ -382,7 +382,7 @@ fn gh676_percent_h_escape_unsupported() { ); assert_eq!(errs.len(), 1); assert_eq!( - errs[0].1, + errs[0].message, "cannot open sudoers file /etc/%h: percent escape %h in includedir is unsupported" ) } diff --git a/src/visudo/mod.rs b/src/visudo/mod.rs index 75e272024..ebe5169c6 100644 --- a/src/visudo/mod.rs +++ b/src/visudo/mod.rs @@ -107,7 +107,7 @@ fn check(file_arg: Option<&str>, perms: bool, owner: bool) -> io::Result<()> { } let mut stderr = io::stderr(); - for crate::sudoers::Error(_position, message) in errors { + for crate::sudoers::Error { message, .. } in errors { writeln!(stderr, "syntax error: {message}")?; } @@ -245,7 +245,7 @@ fn edit_sudoers_file( writeln!(stderr, "The provided sudoers file format is not recognized or contains syntax errors. Please review:\n")?; - for crate::sudoers::Error(_position, message) in errors { + for crate::sudoers::Error { message, .. } in errors { writeln!(stderr, "syntax error: {message}")?; }