Fix trust filter syntax reporting (#1023)

Connect trust filter syntax checks to the info GUI

This also adds a lint warning for too many indents.

Closes #1018

crates/daemon/src/conf/read.rs

@@ -17,8 +17,8 @@ fn lines_in_file(path: PathBuf) -> Result<Vec<String>, Error> {
     let reader = File::open(path)
         .map(BufReader::new)
         .map_err(|_| Error::General)?;
-    let lines = reader.lines().flatten().collect();
-    Ok(lines)
+    let lines: Vec<_> = reader.lines().collect();
+    Ok(lines.into_iter().flatten().collect())
 }
 
 pub fn file(path: PathBuf) -> Result<DB, Error> {

crates/pyo3/src/trust.rs

@@ -233,16 +233,18 @@ impl PyFilterInfo {
 
 pub(crate) fn filter_info(db: &filter::DB) -> Vec<PyFilterInfo> {
     let e = "e";
+    let w = "w";
     db.iter().fold(vec![], |mut acc, line| {
-        let message = match line {
-            Line::Invalid(s) => Some(format!("Invalid: {s}")),
-            Line::Malformed(s) => Some(format!("Malformed: {s}")),
-            Line::Duplicate(s) => Some(format!("Duplicated: {s}")),
+        let info = match line {
+            Line::Invalid(s) => Some((e, format!("Invalid: {s}"))),
+            Line::Malformed(s) => Some((e, format!("Malformed: {s}"))),
+            Line::Duplicate(s) => Some((e, format!("Duplicated: {s}"))),
+            Line::ValidWithWarning(_, m) => Some((w, m.to_owned())),
             _ => None,
         };
-        if let Some(message) = message {
+        if let Some((category, message)) = info {
             acc.push(PyFilterInfo {
-                category: e.to_owned(),
+                category: category.to_owned(),
                 message,
             });
         };

crates/trust/src/filter/db.rs

@@ -11,6 +11,7 @@ use std::slice::Iter;
 #[derive(Clone, Debug)]
 pub enum Line {
     Valid(String),
+    ValidWithWarning(String, String),
     Invalid(String),
     Malformed(String),
     Duplicate(String),
@@ -52,7 +53,7 @@ impl Display for Line {
         use Line::*;
 
         match self {
-            Valid(tok) => f.write_fmt(format_args!("{tok}")),
+            Valid(tok) | ValidWithWarning(tok, _) => f.write_fmt(format_args!("{tok}")),
             Invalid(tok) => f.write_fmt(format_args!("{tok}")),
             Malformed(txt) => f.write_str(txt),
             Duplicate(tok) => f.write_fmt(format_args!("{tok}")),

crates/trust/src/filter/parse.rs

@@ -19,6 +19,7 @@ use nom::IResult;
 use thiserror::Error;
 
 use crate::filter::parse::Dec::*;
+use crate::filter::Line;
 
 /// Errors that can occur in this module
 #[derive(Error, Debug)]
@@ -151,8 +152,8 @@ fn ignored_line(l: &str) -> bool {
     l.trim_start().starts_with('#') || l.trim().is_empty()
 }
 
-/// Parse a filter config from the conf lines
-pub fn parse(lines: &[&str]) -> Result<Decider, Error> {
+/// Parse a Decider from the conf lines
+pub fn decider(lines: &[&str]) -> Result<Decider, Error> {
     use Error::*;
 
     let mut decider = Decider::default();
@@ -205,6 +206,76 @@ pub fn parse(lines: &[&str]) -> Result<Decider, Error> {
     Ok(decider)
 }
 
+/// Parse Lines entries from the conf lines
+pub fn lines(input: Vec<String>) -> Vec<Line> {
+    let mut lines = vec![];
+
+    let mut prev_i = None;
+    let mut stack = vec![];
+
+    // process lines from the config, ignoring comments and empty lines
+    for (_, line) in input.iter().enumerate() {
+        if line.trim_start().starts_with('#') {
+            lines.push(Line::Comment(line.to_string()))
+        } else if line.is_empty() {
+            lines.push(Line::BlankLine);
+        } else {
+            match (prev_i, parse_entry(line)) {
+                // ensure root level starts with /
+                (_, Ok((0, (k, _)))) if !k.starts_with('/') => {
+                    lines.push(Line::Invalid("NonAbsRootElement".to_owned()))
+                }
+                // at the root level, anywhere in the conf
+                (prev, Ok((0, (k, _)))) => {
+                    if prev.is_some() {
+                        stack.clear();
+                    }
+                    lines.push(Line::Valid(line.to_string()));
+                    stack.push(PathBuf::from(k));
+                    prev_i = Some(0);
+                }
+                // fail if the first conf element is indented
+                (None, Ok((_, (_, _)))) => {
+                    lines.push(Line::Invalid("TooManyStartIndents".to_owned()))
+                }
+                // handle indentation
+                (Some(pi), Ok((i, (k, _)))) if i > pi => {
+                    let entry = if i - pi == 1 {
+                        Line::Valid(line.to_string())
+                    } else {
+                        Line::ValidWithWarning(
+                            line.to_string(),
+                            format!("Excessive indent, {} spaces", i - pi - 1),
+                        )
+                    };
+                    let p = stack.last().map(|l| l.join(k)).unwrap();
+                    lines.push(entry);
+                    stack.push(p);
+                    prev_i = Some(i);
+                }
+                // handle unindentation
+                (Some(pi), Ok((i, (k, _)))) if i < pi => {
+                    stack.truncate(i);
+                    let p = stack.last().map(|l| l.join(k)).unwrap();
+                    lines.push(Line::Valid(line.to_string()));
+                    stack.push(p);
+                    prev_i = Some(i);
+                }
+                // remaining at previous level
+                (Some(_), Ok((i, (k, _)))) => {
+                    stack.truncate(i);
+                    let p = stack.last().map(|l| l.join(k)).unwrap();
+                    lines.push(Line::Valid(line.to_string()));
+                    stack.push(p);
+                }
+                // propagate parse errors
+                (_, Err(_)) => lines.push(Line::Malformed(line.to_string())),
+            }
+        }
+    }
+    lines
+}
+
 fn parse_dec(i: &str) -> IResult<&str, Dec> {
     alt((map(tag("+"), |_| Inc(0)), map(tag("-"), |_| Exc(0))))(i)
 }
@@ -220,12 +291,14 @@ fn parse_entry(i: &str) -> Result<(usize, (&str, Dec)), Error> {
 
 #[cfg(test)]
 mod tests {
+    use std::default::Default;
+
     use assert_matches::assert_matches;
+
     use fapolicy_util::trimto::TrimTo;
-    use std::default::Default;
 
     use crate::filter::parse::Dec::*;
-    use crate::filter::parse::{parse, Decider, Error};
+    use crate::filter::parse::{decider, Decider, Error};
 
     // the first few tests are modeled after example config from the fapolicyd documentation
 
@@ -237,7 +310,7 @@ mod tests {
         |+ /"#
             .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(d.check("/"));
         assert!(!d.check("/usr/bin/some_binary1"));
         assert!(!d.check("/usr/bin/some_binary2"));
@@ -252,7 +325,7 @@ mod tests {
         |  - some_binary2"#
             .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(d.check("/"));
         assert!(!d.check("/usr/bin/some_binary1"));
         assert!(!d.check("/usr/bin/some_binary2"));
@@ -306,7 +379,7 @@ mod tests {
         |"#
         .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(d.check("/bin/foo"));
         assert!(!d.check("/usr/share/x.txt"));
         assert!(!d.check("/usr/include/x.h"));
@@ -333,7 +406,7 @@ mod tests {
         | - bar/baz"#
             .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert_matches!(d.dec("/"), Inc(3));
         assert_matches!(d.dec("/usr/bin/some_binary1"), Exc(1));
         assert_matches!(d.dec("/usr/bin/some_binary2"), Exc(2));
@@ -343,21 +416,21 @@ mod tests {
 
     #[test]
     fn too_many_indents() {
-        assert_matches!(parse(&[" + foo"]), Err(Error::TooManyStartIndents));
+        assert_matches!(decider(&[" + foo"]), Err(Error::TooManyStartIndents));
     }
 
     #[test]
     fn indentation_0_starts_with_slash() {
-        assert_matches!(parse(&["+ x"]), Err(Error::NonAbsRootElement));
+        assert_matches!(decider(&["+ x"]), Err(Error::NonAbsRootElement));
         assert_matches!(
-            parse(&["+ /", " - foo", "+ bar"]),
+            decider(&["+ /", " - foo", "+ bar"]),
             Err(Error::NonAbsRootElement)
         );
     }
 
     #[test]
     fn indentation_basic() -> Result<(), Error> {
-        let d = parse(&["+ /", " - b", "  + baz"])?;
+        let d = decider(&["+ /", " - b", "  + baz"])?;
         assert!(d.check("/a"));
         assert!(!d.check("/b"));
         assert!(d.check("/b/baz"));
@@ -367,7 +440,7 @@ mod tests {
 
     #[test]
     fn indentation_mix() -> Result<(), Error> {
-        let d = parse(&["+ /", "  - foo/bar", "   + baz"])?;
+        let d = decider(&["+ /", "  - foo/bar", "   + baz"])?;
         assert!(d.check("/"));
         assert!(d.check("/foo"));
         assert!(!d.check("/foo/bar"));
@@ -378,7 +451,7 @@ mod tests {
 
     #[test]
     fn indentation_nested() -> Result<(), Error> {
-        let d = parse(&["+ /", "- /foo/bar", "+ /foo/bar/baz"])?;
+        let d = decider(&["+ /", "- /foo/bar", "+ /foo/bar/baz"])?;
         assert!(d.check("/"));
         assert!(d.check("/foo"));
         assert!(!d.check("/foo/bar"));
@@ -389,15 +462,15 @@ mod tests {
 
     #[test]
     fn basic() -> Result<(), Error> {
-        let d = parse(&["+ /", "- /foo"])?;
+        let d = decider(&["+ /", "- /foo"])?;
         assert!(d.check("/"));
         assert!(!d.check("/foo"));
         Ok(())
     }
 
     #[test]
     fn wildcard_single() -> Result<(), Error> {
-        let d = parse(&["+ /", " - b?"])?;
+        let d = decider(&["+ /", " - b?"])?;
         assert!(d.check("/a"));
         assert!(d.check("/b"));
         assert!(!d.check("/bb"));
@@ -408,7 +481,7 @@ mod tests {
 
     #[test]
     fn wildcard_glob() -> Result<(), Error> {
-        let d = parse(&["+ /", " - b", " - b*"])?;
+        let d = decider(&["+ /", " - b", " - b*"])?;
         assert!(d.check("/a"));
         assert!(!d.check("/b"));
         assert!(!d.check("/bb"));
@@ -419,11 +492,11 @@ mod tests {
 
     #[test]
     fn parse_basic() -> Result<(), Error> {
-        let d = parse(&["+ /"])?;
+        let d = decider(&["+ /"])?;
         assert!(d.check("/"));
         assert!(d.check("/foo"));
 
-        let d = parse(&["+ /foo"])?;
+        let d = decider(&["+ /foo"])?;
         assert!(!d.check("/"));
         assert!(d.check("/foo"));
         Ok(())
@@ -440,7 +513,7 @@ mod tests {
         | "#
         .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(!d.check("/usr/foo/x"));
         assert!(d.check("/usr/foo/x.py"));
         assert!(!d.check("/usr/bar/x"));
@@ -462,7 +535,7 @@ mod tests {
         |+ /"#
             .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(d.check("/"));
         assert!(!d.check("/usr/bin/some_binary1"));
         assert!(!d.check("/usr/bin/some_binary2"));
@@ -475,7 +548,7 @@ mod tests {
         | - usr/bin/some_binary*"#
             .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(d.check("/"));
         assert!(!d.check("/usr/bin/some_binary1"));
         assert!(!d.check("/usr/bin/some_binary2"));
@@ -490,7 +563,7 @@ mod tests {
         |  + *.pl"#
             .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(d.check("/usr/bin/ls"));
         assert!(!d.check("/usr/share/something"));
         assert!(d.check("/usr/share/abcd.py"));
@@ -504,7 +577,7 @@ mod tests {
         |   + *.py"#
             .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(!d.check("/usr/share/abc"));
         // assert!(!d.check("/usr/share/abc.py"));
     }
@@ -517,7 +590,7 @@ mod tests {
         |   + *.py"#
             .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(!d.check("/usr/share/abc"));
         assert!(!d.check("/usr/share/abc.pl"));
         assert!(d.check("/usr/share/abc.py"));
@@ -535,7 +608,7 @@ mod tests {
         |- /z"#
             .trim_to('|');
 
-        let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
+        let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
         assert!(!d.check("/usr/share/xyz"));
         assert!(d.check("/usr/share/abc/def/foo.py"));
         assert!(!d.check("/tmp/x"));

crates/trust/src/filter/read.rs

@@ -7,40 +7,22 @@
  */
 
 use crate::filter::error::Error;
-use crate::filter::{Line, DB};
+use crate::filter::{parse, DB};
 use std::fs::File;
 use std::io::{BufRead, BufReader};
 
 pub fn file(path: &str) -> Result<DB, Error> {
     let reader = File::open(path)
         .map(BufReader::new)
         .map_err(|_| Error::General("Parse file".to_owned()))?;
-    lines(reader.lines().flatten().collect())
+    let r: Vec<_> = reader.lines().collect();
+    lines(r.into_iter().flatten().collect())
 }
 
 pub fn mem(txt: &str) -> Result<DB, Error> {
     lines(txt.split('\n').map(|s| s.to_string()).collect())
 }
 
 fn lines(src: Vec<String>) -> Result<DB, Error> {
-    let mut lines = vec![];
-    let mut skip_blank = true;
-
-    for s in src {
-        let s = s.trim_end();
-        if s.is_empty() {
-            if skip_blank {
-                continue;
-            }
-            lines.push(Line::BlankLine);
-            skip_blank = true;
-        } else if s.trim_start().starts_with('#') {
-            lines.push(Line::Comment(s.to_string()));
-            skip_blank = false;
-        } else {
-            lines.push(Line::Valid(s.to_owned()));
-            skip_blank = false;
-        }
-    }
-    Ok(lines.into())
+    Ok(parse::lines(src).into())
 }