Skip to content

Commit 336a973

Browse files
jw3jw3
authored
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
1 parent 25f65db commit 336a973

File tree

5 files changed

+115
-57
lines changed

5 files changed

+115
-57
lines changed

‎crates/daemon/src/conf/read.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ fn lines_in_file(path: PathBuf) -> Result<Vec<String>, Error> {
1717
let reader = File::open(path)
1818
.map(BufReader::new)
1919
.map_err(|_| Error::General)?;
20-
let lines = reader.lines().flatten().collect();
21-
Ok(lines)
20+
let lines: Vec<_> = reader.lines().collect();
21+
Ok(lines.into_iter().flatten().collect())
2222
}
2323

2424
pub fn file(path: PathBuf) -> Result<DB, Error> {

‎crates/pyo3/src/trust.rs

+8-6
Original file line numberDiff line numberDiff line change
@@ -233,16 +233,18 @@ impl PyFilterInfo {
233233

234234
pub(crate) fn filter_info(db: &filter::DB) -> Vec<PyFilterInfo> {
235235
let e = "e";
236+
let w = "w";
236237
db.iter().fold(vec![], |mut acc, line| {
237-
let message = match line {
238-
Line::Invalid(s) => Some(format!("Invalid: {s}")),
239-
Line::Malformed(s) => Some(format!("Malformed: {s}")),
240-
Line::Duplicate(s) => Some(format!("Duplicated: {s}")),
238+
let info = match line {
239+
Line::Invalid(s) => Some((e, format!("Invalid: {s}"))),
240+
Line::Malformed(s) => Some((e, format!("Malformed: {s}"))),
241+
Line::Duplicate(s) => Some((e, format!("Duplicated: {s}"))),
242+
Line::ValidWithWarning(_, m) => Some((w, m.to_owned())),
241243
_ => None,
242244
};
243-
if let Some(message) = message {
245+
if let Some((category, message)) = info {
244246
acc.push(PyFilterInfo {
245-
category: e.to_owned(),
247+
category: category.to_owned(),
246248
message,
247249
});
248250
};

‎crates/trust/src/filter/db.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ use std::slice::Iter;
1111
#[derive(Clone, Debug)]
1212
pub enum Line {
1313
Valid(String),
14+
ValidWithWarning(String, String),
1415
Invalid(String),
1516
Malformed(String),
1617
Duplicate(String),
@@ -52,7 +53,7 @@ impl Display for Line {
5253
use Line::*;
5354

5455
match self {
55-
Valid(tok) => f.write_fmt(format_args!("{tok}")),
56+
Valid(tok) | ValidWithWarning(tok, _) => f.write_fmt(format_args!("{tok}")),
5657
Invalid(tok) => f.write_fmt(format_args!("{tok}")),
5758
Malformed(txt) => f.write_str(txt),
5859
Duplicate(tok) => f.write_fmt(format_args!("{tok}")),

‎crates/trust/src/filter/parse.rs

+99-26
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use nom::IResult;
1919
use thiserror::Error;
2020

2121
use crate::filter::parse::Dec::*;
22+
use crate::filter::Line;
2223

2324
/// Errors that can occur in this module
2425
#[derive(Error, Debug)]
@@ -151,8 +152,8 @@ fn ignored_line(l: &str) -> bool {
151152
l.trim_start().starts_with('#') || l.trim().is_empty()
152153
}
153154

154-
/// Parse a filter config from the conf lines
155-
pub fn parse(lines: &[&str]) -> Result<Decider, Error> {
155+
/// Parse a Decider from the conf lines
156+
pub fn decider(lines: &[&str]) -> Result<Decider, Error> {
156157
use Error::*;
157158

158159
let mut decider = Decider::default();
@@ -205,6 +206,76 @@ pub fn parse(lines: &[&str]) -> Result<Decider, Error> {
205206
Ok(decider)
206207
}
207208

209+
/// Parse Lines entries from the conf lines
210+
pub fn lines(input: Vec<String>) -> Vec<Line> {
211+
let mut lines = vec![];
212+
213+
let mut prev_i = None;
214+
let mut stack = vec![];
215+
216+
// process lines from the config, ignoring comments and empty lines
217+
for (_, line) in input.iter().enumerate() {
218+
if line.trim_start().starts_with('#') {
219+
lines.push(Line::Comment(line.to_string()))
220+
} else if line.is_empty() {
221+
lines.push(Line::BlankLine);
222+
} else {
223+
match (prev_i, parse_entry(line)) {
224+
// ensure root level starts with /
225+
(_, Ok((0, (k, _)))) if !k.starts_with('/') => {
226+
lines.push(Line::Invalid("NonAbsRootElement".to_owned()))
227+
}
228+
// at the root level, anywhere in the conf
229+
(prev, Ok((0, (k, _)))) => {
230+
if prev.is_some() {
231+
stack.clear();
232+
}
233+
lines.push(Line::Valid(line.to_string()));
234+
stack.push(PathBuf::from(k));
235+
prev_i = Some(0);
236+
}
237+
// fail if the first conf element is indented
238+
(None, Ok((_, (_, _)))) => {
239+
lines.push(Line::Invalid("TooManyStartIndents".to_owned()))
240+
}
241+
// handle indentation
242+
(Some(pi), Ok((i, (k, _)))) if i > pi => {
243+
let entry = if i - pi == 1 {
244+
Line::Valid(line.to_string())
245+
} else {
246+
Line::ValidWithWarning(
247+
line.to_string(),
248+
format!("Excessive indent, {} spaces", i - pi - 1),
249+
)
250+
};
251+
let p = stack.last().map(|l| l.join(k)).unwrap();
252+
lines.push(entry);
253+
stack.push(p);
254+
prev_i = Some(i);
255+
}
256+
// handle unindentation
257+
(Some(pi), Ok((i, (k, _)))) if i < pi => {
258+
stack.truncate(i);
259+
let p = stack.last().map(|l| l.join(k)).unwrap();
260+
lines.push(Line::Valid(line.to_string()));
261+
stack.push(p);
262+
prev_i = Some(i);
263+
}
264+
// remaining at previous level
265+
(Some(_), Ok((i, (k, _)))) => {
266+
stack.truncate(i);
267+
let p = stack.last().map(|l| l.join(k)).unwrap();
268+
lines.push(Line::Valid(line.to_string()));
269+
stack.push(p);
270+
}
271+
// propagate parse errors
272+
(_, Err(_)) => lines.push(Line::Malformed(line.to_string())),
273+
}
274+
}
275+
}
276+
lines
277+
}
278+
208279
fn parse_dec(i: &str) -> IResult<&str, Dec> {
209280
alt((map(tag("+"), |_| Inc(0)), map(tag("-"), |_| Exc(0))))(i)
210281
}
@@ -220,12 +291,14 @@ fn parse_entry(i: &str) -> Result<(usize, (&str, Dec)), Error> {
220291

221292
#[cfg(test)]
222293
mod tests {
294+
use std::default::Default;
295+
223296
use assert_matches::assert_matches;
297+
224298
use fapolicy_util::trimto::TrimTo;
225-
use std::default::Default;
226299

227300
use crate::filter::parse::Dec::*;
228-
use crate::filter::parse::{parse, Decider, Error};
301+
use crate::filter::parse::{decider, Decider, Error};
229302

230303
// the first few tests are modeled after example config from the fapolicyd documentation
231304

@@ -237,7 +310,7 @@ mod tests {
237310
|+ /"#
238311
.trim_to('|');
239312

240-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
313+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
241314
assert!(d.check("/"));
242315
assert!(!d.check("/usr/bin/some_binary1"));
243316
assert!(!d.check("/usr/bin/some_binary2"));
@@ -252,7 +325,7 @@ mod tests {
252325
| - some_binary2"#
253326
.trim_to('|');
254327

255-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
328+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
256329
assert!(d.check("/"));
257330
assert!(!d.check("/usr/bin/some_binary1"));
258331
assert!(!d.check("/usr/bin/some_binary2"));
@@ -306,7 +379,7 @@ mod tests {
306379
|"#
307380
.trim_to('|');
308381

309-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
382+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
310383
assert!(d.check("/bin/foo"));
311384
assert!(!d.check("/usr/share/x.txt"));
312385
assert!(!d.check("/usr/include/x.h"));
@@ -333,7 +406,7 @@ mod tests {
333406
| - bar/baz"#
334407
.trim_to('|');
335408

336-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
409+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
337410
assert_matches!(d.dec("/"), Inc(3));
338411
assert_matches!(d.dec("/usr/bin/some_binary1"), Exc(1));
339412
assert_matches!(d.dec("/usr/bin/some_binary2"), Exc(2));
@@ -343,21 +416,21 @@ mod tests {
343416

344417
#[test]
345418
fn too_many_indents() {
346-
assert_matches!(parse(&[" + foo"]), Err(Error::TooManyStartIndents));
419+
assert_matches!(decider(&[" + foo"]), Err(Error::TooManyStartIndents));
347420
}
348421

349422
#[test]
350423
fn indentation_0_starts_with_slash() {
351-
assert_matches!(parse(&["+ x"]), Err(Error::NonAbsRootElement));
424+
assert_matches!(decider(&["+ x"]), Err(Error::NonAbsRootElement));
352425
assert_matches!(
353-
parse(&["+ /", " - foo", "+ bar"]),
426+
decider(&["+ /", " - foo", "+ bar"]),
354427
Err(Error::NonAbsRootElement)
355428
);
356429
}
357430

358431
#[test]
359432
fn indentation_basic() -> Result<(), Error> {
360-
let d = parse(&["+ /", " - b", " + baz"])?;
433+
let d = decider(&["+ /", " - b", " + baz"])?;
361434
assert!(d.check("/a"));
362435
assert!(!d.check("/b"));
363436
assert!(d.check("/b/baz"));
@@ -367,7 +440,7 @@ mod tests {
367440

368441
#[test]
369442
fn indentation_mix() -> Result<(), Error> {
370-
let d = parse(&["+ /", " - foo/bar", " + baz"])?;
443+
let d = decider(&["+ /", " - foo/bar", " + baz"])?;
371444
assert!(d.check("/"));
372445
assert!(d.check("/foo"));
373446
assert!(!d.check("/foo/bar"));
@@ -378,7 +451,7 @@ mod tests {
378451

379452
#[test]
380453
fn indentation_nested() -> Result<(), Error> {
381-
let d = parse(&["+ /", "- /foo/bar", "+ /foo/bar/baz"])?;
454+
let d = decider(&["+ /", "- /foo/bar", "+ /foo/bar/baz"])?;
382455
assert!(d.check("/"));
383456
assert!(d.check("/foo"));
384457
assert!(!d.check("/foo/bar"));
@@ -389,15 +462,15 @@ mod tests {
389462

390463
#[test]
391464
fn basic() -> Result<(), Error> {
392-
let d = parse(&["+ /", "- /foo"])?;
465+
let d = decider(&["+ /", "- /foo"])?;
393466
assert!(d.check("/"));
394467
assert!(!d.check("/foo"));
395468
Ok(())
396469
}
397470

398471
#[test]
399472
fn wildcard_single() -> Result<(), Error> {
400-
let d = parse(&["+ /", " - b?"])?;
473+
let d = decider(&["+ /", " - b?"])?;
401474
assert!(d.check("/a"));
402475
assert!(d.check("/b"));
403476
assert!(!d.check("/bb"));
@@ -408,7 +481,7 @@ mod tests {
408481

409482
#[test]
410483
fn wildcard_glob() -> Result<(), Error> {
411-
let d = parse(&["+ /", " - b", " - b*"])?;
484+
let d = decider(&["+ /", " - b", " - b*"])?;
412485
assert!(d.check("/a"));
413486
assert!(!d.check("/b"));
414487
assert!(!d.check("/bb"));
@@ -419,11 +492,11 @@ mod tests {
419492

420493
#[test]
421494
fn parse_basic() -> Result<(), Error> {
422-
let d = parse(&["+ /"])?;
495+
let d = decider(&["+ /"])?;
423496
assert!(d.check("/"));
424497
assert!(d.check("/foo"));
425498

426-
let d = parse(&["+ /foo"])?;
499+
let d = decider(&["+ /foo"])?;
427500
assert!(!d.check("/"));
428501
assert!(d.check("/foo"));
429502
Ok(())
@@ -440,7 +513,7 @@ mod tests {
440513
| "#
441514
.trim_to('|');
442515

443-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
516+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
444517
assert!(!d.check("/usr/foo/x"));
445518
assert!(d.check("/usr/foo/x.py"));
446519
assert!(!d.check("/usr/bar/x"));
@@ -462,7 +535,7 @@ mod tests {
462535
|+ /"#
463536
.trim_to('|');
464537

465-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
538+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
466539
assert!(d.check("/"));
467540
assert!(!d.check("/usr/bin/some_binary1"));
468541
assert!(!d.check("/usr/bin/some_binary2"));
@@ -475,7 +548,7 @@ mod tests {
475548
| - usr/bin/some_binary*"#
476549
.trim_to('|');
477550

478-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
551+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
479552
assert!(d.check("/"));
480553
assert!(!d.check("/usr/bin/some_binary1"));
481554
assert!(!d.check("/usr/bin/some_binary2"));
@@ -490,7 +563,7 @@ mod tests {
490563
| + *.pl"#
491564
.trim_to('|');
492565

493-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
566+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
494567
assert!(d.check("/usr/bin/ls"));
495568
assert!(!d.check("/usr/share/something"));
496569
assert!(d.check("/usr/share/abcd.py"));
@@ -504,7 +577,7 @@ mod tests {
504577
| + *.py"#
505578
.trim_to('|');
506579

507-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
580+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
508581
assert!(!d.check("/usr/share/abc"));
509582
// assert!(!d.check("/usr/share/abc.py"));
510583
}
@@ -517,7 +590,7 @@ mod tests {
517590
| + *.py"#
518591
.trim_to('|');
519592

520-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
593+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
521594
assert!(!d.check("/usr/share/abc"));
522595
assert!(!d.check("/usr/share/abc.pl"));
523596
assert!(d.check("/usr/share/abc.py"));
@@ -535,7 +608,7 @@ mod tests {
535608
|- /z"#
536609
.trim_to('|');
537610

538-
let d = parse(&al.split('\n').collect::<Vec<&str>>()).unwrap();
611+
let d = decider(&al.split('\n').collect::<Vec<&str>>()).unwrap();
539612
assert!(!d.check("/usr/share/xyz"));
540613
assert!(d.check("/usr/share/abc/def/foo.py"));
541614
assert!(!d.check("/tmp/x"));

‎crates/trust/src/filter/read.rs

+4-22
Original file line numberDiff line numberDiff line change
@@ -7,40 +7,22 @@
77
*/
88

99
use crate::filter::error::Error;
10-
use crate::filter::{Line, DB};
10+
use crate::filter::{parse, DB};
1111
use std::fs::File;
1212
use std::io::{BufRead, BufReader};
1313

1414
pub fn file(path: &str) -> Result<DB, Error> {
1515
let reader = File::open(path)
1616
.map(BufReader::new)
1717
.map_err(|_| Error::General("Parse file".to_owned()))?;
18-
lines(reader.lines().flatten().collect())
18+
let r: Vec<_> = reader.lines().collect();
19+
lines(r.into_iter().flatten().collect())
1920
}
2021

2122
pub fn mem(txt: &str) -> Result<DB, Error> {
2223
lines(txt.split('\n').map(|s| s.to_string()).collect())
2324
}
2425

2526
fn lines(src: Vec<String>) -> Result<DB, Error> {
26-
let mut lines = vec![];
27-
let mut skip_blank = true;
28-
29-
for s in src {
30-
let s = s.trim_end();
31-
if s.is_empty() {
32-
if skip_blank {
33-
continue;
34-
}
35-
lines.push(Line::BlankLine);
36-
skip_blank = true;
37-
} else if s.trim_start().starts_with('#') {
38-
lines.push(Line::Comment(s.to_string()));
39-
skip_blank = false;
40-
} else {
41-
lines.push(Line::Valid(s.to_owned()));
42-
skip_blank = false;
43-
}
44-
}
45-
Ok(lines.into())
27+
Ok(parse::lines(src).into())
4628
}

0 commit comments

Comments
 (0)