Repository: woodruffw/zizmor (language: python)

Issue:
Support ignoring results via inline comments?
`zizmor` now supports a config file that can be used to write ignore rules. However, some users also want inline ignore support, e.g.:

```yaml
permissions: write-all # zizmor: ignore[excessive-permissions]
```

See #12 for some earlier context.


Discussion / hints:

Fix patch:
diff --git a/src/finding/mod.rs b/src/finding/mod.rs
index 3c8b80c..12407e7 100644
--- a/src/finding/mod.rs
+++ b/src/finding/mod.rs
@@ -222,6 +222,7 @@ pub(crate) struct Finding<'w> {
     pub(crate) url: &'static str,
     pub(crate) determinations: Determinations,
     pub(crate) locations: Vec<Location<'w>>,
+    pub(crate) ignored: bool,
 }
 
 pub(crate) struct FindingBuilder<'w> {
@@ -261,6 +262,14 @@ impl<'w> FindingBuilder<'w> {
     }
 
     pub(crate) fn build(self, workflow: &'w Workflow) -> Result<Finding<'w>> {
+        let locations = self
+            .locations
+            .iter()
+            .map(|l| l.clone().concretize(workflow))
+            .collect::<Result<Vec<_>>>()?;
+
+        let should_ignore = self.ignored_from_inlined_comment(workflow, &locations, self.ident);
+
         Ok(Finding {
             ident: self.ident,
             desc: self.desc,
@@ -269,11 +278,39 @@ impl<'w> FindingBuilder<'w> {
                 confidence: self.confidence,
                 severity: self.severity,
             },
-            locations: self
-                .locations
-                .into_iter()
-                .map(|l| l.concretize(workflow))
-                .collect::<Result<Vec<_>>>()?,
+            locations,
+            ignored: should_ignore,
         })
     }
+
+    fn ignored_from_inlined_comment(
+        &self,
+        workflow: &Workflow,
+        locations: &[Location],
+        id: &str,
+    ) -> bool {
+        let document_lines = &workflow.document.source().lines().collect::<Vec<_>>();
+        let line_ranges = locations
+            .iter()
+            .map(|l| {
+                (
+                    l.concrete.location.start_point.row,
+                    l.concrete.location.end_point.row,
+                )
+            })
+            .collect::<Vec<_>>();
+
+        let inlined_ignore = format!("# zizmor: ignore[{}]", id);
+        for (start, end) in line_ranges {
+            for document_line in start..(end + 1) {
+                if let Some(line) = document_lines.get(document_line) {
+                    if line.rfind(&inlined_ignore).is_some() {
+                        return true;
+                    }
+                }
+            }
+        }
+
+        false
+    }
 }
diff --git a/src/models.rs b/src/models.rs
index 7222b6d..c5ed20b 100644
--- a/src/models.rs
+++ b/src/models.rs
@@ -32,12 +32,12 @@ impl Deref for Workflow {
 impl Workflow {
     /// Load a workflow from the given file on disk.
     pub(crate) fn from_file<P: AsRef<Path>>(p: P) -> Result<Self> {
-        let raw = std::fs::read_to_string(p.as_ref())?;
+        let contents = std::fs::read_to_string(p.as_ref())?;
 
-        let inner = serde_yaml::from_str(&raw)
+        let inner = serde_yaml::from_str(&contents)
             .with_context(|| format!("invalid GitHub Actions workflow: {:?}", p.as_ref()))?;
 
-        let document = yamlpath::Document::new(raw)?;
+        let document = yamlpath::Document::new(&contents)?;
 
         Ok(Self {
             path: p
diff --git a/src/registry.rs b/src/registry.rs
index 778d96a..0ca5791 100644
--- a/src/registry.rs
+++ b/src/registry.rs
@@ -3,14 +3,13 @@
 
 use std::{collections::HashMap, path::Path, process::ExitCode};
 
-use anyhow::{anyhow, Context, Result};
-
 use crate::{
     audit::WorkflowAudit,
     config::Config,
     finding::{Finding, Severity},
     models::Workflow,
 };
+use anyhow::{anyhow, Context, Result};
 
 pub(crate) struct WorkflowRegistry {
     pub(crate) workflows: HashMap<String, Workflow>,
@@ -132,18 +131,18 @@ impl<'a> FindingRegistry<'a> {
     pub(crate) fn extend(&mut self, results: Vec<Finding<'a>>) {
         // TODO: is it faster to iterate like this, or do `find_by_max`
         // and then `extend`?
-        for result in results {
-            if self.config.ignores(&result) {
-                self.ignored.push(result);
+        for finding in results {
+            if self.config.ignores(&finding) || finding.ignored {
+                self.ignored.push(finding);
             } else {
                 if self
                     .highest_severity
-                    .map_or(true, |s| result.determinations.severity > s)
+                    .map_or(true, |s| finding.determinations.severity > s)
                 {
-                    self.highest_severity = Some(result.determinations.severity);
+                    self.highest_severity = Some(finding.determinations.severity);
                 }
 
-                self.findings.push(result);
+                self.findings.push(finding);
             }
         }
     }


Test patch:
diff --git a/tests/acceptance.rs b/tests/acceptance.rs
index dba00b5..d1acd9e 100644
--- a/tests/acceptance.rs
+++ b/tests/acceptance.rs
@@ -35,6 +35,23 @@ fn assert_value_match(json: &Value, path_pattern: &str, value: &str) {
     assert!(queried.to_string().contains(value));
 }
 
+#[test]
+fn catches_inlined_ignore() -> anyhow::Result<()> {
+    let auditable = workflow_under_test("inlined-ignores.yml");
+
+    let cli_args = [&auditable];
+
+    let execution = zizmor().args(cli_args).output()?;
+
+    assert_eq!(execution.status.code(), Some(0));
+
+    let findings = String::from_utf8(execution.stdout)?;
+
+    assert_eq!(&findings, "[]");
+
+    Ok(())
+}
+
 #[test]
 fn audit_artipacked() -> anyhow::Result<()> {
     let auditable = workflow_under_test("artipacked.yml");
diff --git a/tests/test-data/inlined-ignores.yml b/tests/test-data/inlined-ignores.yml
new file mode 100644
index 0000000..38f4bdb
--- /dev/null
+++ b/tests/test-data/inlined-ignores.yml
@@ -0,0 +1,28 @@
+on:
+  push:
+    branches:
+      - master
+  workflow_dispatch:
+
+jobs:
+  artipacked-ignored:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4 # zizmor: ignore[artipacked]
+
+  insecure-commands-ignored:
+    runs-on: ubuntu-latest
+    env:
+      ACTIONS_ALLOW_UNSECURE_COMMANDS: yes # zizmor: ignore[insecure-commands]
+    steps:
+      - run: echo "I shall pass!"
+
+  hardcoded-credentials-ignored:
+    runs-on: ubuntu-latest
+    container:
+      image: fake.example.com/example
+      credentials:
+        username: user
+        password: hackme # zizmor: ignore[hardcoded-container-credentials]
+    steps:
+      - run: echo 'This is a honeypot actually!'