audit: begin work on excessive_permissions (#13)

Cargo.toml

@@ -9,7 +9,7 @@ homepage = "https://github.com/woodruffw/zizmor"
 anyhow = "1.0.86"
 clap = { version = "4.5.16", features = ["derive", "env"] }
 env_logger = "0.11.5"
-github-actions-models = "0.5.0"
+github-actions-models = "0.6.0"
 itertools = "0.13.0"
 log = "0.4.22"
 regex = "1.10.6"

src/audit/artipacked.rs

@@ -7,14 +7,13 @@ use github_actions_models::{
 };
 use itertools::Itertools;
 
+use super::WorkflowAudit;
 use crate::{
     finding::{Confidence, Finding, Severity},
     models::AuditConfig,
 };
 use crate::{models::Workflow, utils::split_patterns};
 
-use super::WorkflowAudit;
-
 pub(crate) struct Artipacked<'a> {
     pub(crate) config: AuditConfig<'a>,
 }

src/audit/excessive_permissions.rs

@@ -0,0 +1,169 @@
+use std::{collections::HashMap, ops::Deref, sync::LazyLock};
+
+use github_actions_models::{
+    common::{BasePermission, Permission, Permissions},
+    workflow::Job,
+};
+
+use super::WorkflowAudit;
+use crate::{
+    finding::{Confidence, Severity},
+    models::AuditConfig,
+};
+
+// Subjective mapping of permissions to severities, when given `write` access.
+static KNOWN_PERMISSIONS: LazyLock<HashMap<&str, Severity>> = LazyLock::new(|| {
+    [
+        ("actions", Severity::High),
+        ("attestations", Severity::High),
+        ("checks", Severity::Medium),
+        ("contents", Severity::High),
+        ("deployments", Severity::High),
+        ("discussions", Severity::Medium),
+        ("id-token", Severity::High),
+        ("issues", Severity::High),
+        ("packages", Severity::High),
+        ("pages", Severity::High),
+        ("pull-requests", Severity::High),
+        ("repository-projects", Severity::Medium),
+        ("security-events", Severity::Medium),
+        // What does the write permission even do here?
+        ("statuses", Severity::Low),
+    ]
+    .into()
+});
+
+pub(crate) struct ExcessivePermissions<'a> {
+    pub(crate) _config: AuditConfig<'a>,
+}
+
+impl<'a> WorkflowAudit<'a> for ExcessivePermissions<'a> {
+    fn ident() -> &'static str
+    where
+        Self: Sized,
+    {
+        "excessive-permissions"
+    }
+
+    fn new(config: crate::models::AuditConfig<'a>) -> anyhow::Result<Self>
+    where
+        Self: Sized,
+    {
+        Ok(Self { _config: config })
+    }
+
+    fn audit<'w>(
+        &mut self,
+        workflow: &'w crate::models::Workflow,
+    ) -> anyhow::Result<Vec<crate::finding::Finding<'w>>> {
+        let mut findings = vec![];
+        // Top-level permissions.
+        for (severity, confidence, note) in self.check_permissions(&workflow.permissions, None) {
+            findings.push(
+                Self::finding()
+                    .severity(severity)
+                    .confidence(confidence)
+                    .add_location(workflow.key_location("permissions").annotated(note))
+                    .build(workflow)?,
+            )
+        }
+
+        for job in workflow.jobs() {
+            let Job::NormalJob(normal) = job.deref() else {
+                continue;
+            };
+
+            for (severity, confidence, note) in
+                self.check_permissions(&normal.permissions, Some(&workflow.permissions))
+            {
+                findings.push(
+                    Self::finding()
+                        .severity(severity)
+                        .confidence(confidence)
+                        .add_location(job.key_location("permissions").annotated(note))
+                        .build(workflow)?,
+                )
+            }
+        }
+
+        Ok(findings)
+    }
+}
+
+impl<'a> ExcessivePermissions<'a> {
+    fn check_permissions(
+        &self,
+        permissions: &Permissions,
+        parent: Option<&Permissions>,
+    ) -> Vec<(Severity, Confidence, String)> {
+        match permissions {
+            Permissions::Base(base) => match base {
+                // If no explicit permissions are specified, our behavior
+                // depends on the presence of a parent (workflow) permission
+                // specifier.
+                BasePermission::Default => match parent {
+                    // If there's a parent permissions block, this job inherits
+                    // from it and has nothing new to report.
+                    Some(_) => vec![],
+                    // If there's no parent permissions block, we're at the workflow
+                    // level and should report the default permissions as potentially
+                    // being too broad.
+                    None => vec![(
+                        Severity::Medium,
+                        Confidence::Low,
+                        "workflow uses default permissions, which may be excessive".into(),
+                    )],
+                },
+                BasePermission::ReadAll => vec![(
+                    Severity::Medium,
+                    Confidence::High,
+                    "uses read-all permissions, which may grant read access to more resources \
+                     than necessary"
+                        .into(),
+                )],
+                BasePermission::WriteAll => vec![(
+                    Severity::High,
+                    Confidence::High,
+                    "uses write-all permissions, which grants destructive access to repository \
+                     resources"
+                        .into(),
+                )],
+            },
+            Permissions::Explicit(perms) => match parent {
+                // In the general case, it's impossible to tell whether a
+                // job-level permission block is over-scoped.
+                Some(_) => vec![],
+                // Top-level permission-blocks should almost never contain
+                // write permissions.
+                None => {
+                    let mut results = vec![];
+
+                    for (name, perm) in perms {
+                        if *perm != Permission::Write {
+                            continue;
+                        }
+
+                        match KNOWN_PERMISSIONS.get(name.as_str()) {
+                            Some(sev) => results.push((
+                                *sev,
+                                Confidence::High,
+                                format!("{name}: write is overly broad at the workflow level; move to the job level"),
+                            )),
+                            None => {
+                                log::debug!("unknown permission: {name}");
+
+                                results.push((
+                                    Severity::Unknown,
+                                    Confidence::High,
+                                    format!("{name}: write is overly broad at the workflow level; move to the job level")
+                                ))
+                            },
+                        }
+                    }
+
+                    results
+                }
+            },
+        }
+    }
+}

src/audit/hardcoded_container_credentials.rs

@@ -8,13 +8,12 @@ use github_actions_models::{
     },
 };
 
+use super::WorkflowAudit;
 use crate::{
     finding::{Confidence, Severity},
     models::AuditConfig,
 };
 
-use super::WorkflowAudit;
-
 pub(crate) struct HardcodedContainerCredentials<'a> {
     pub(crate) _config: AuditConfig<'a>,
 }

src/audit/impostor_commit.rs

@@ -10,17 +10,16 @@ use std::{
     ops::Deref,
 };
 
+use anyhow::Result;
+use github_actions_models::workflow::{job::StepBody, Job};
+
+use super::WorkflowAudit;
 use crate::{
     finding::{Confidence, Finding, Severity},
     github_api::{self, Branch, ComparisonStatus, Tag},
     models::{AuditConfig, Uses, Workflow},
 };
 
-use anyhow::Result;
-use github_actions_models::workflow::{job::StepBody, Job};
-
-use super::WorkflowAudit;
-
 pub const IMPOSTOR_ANNOTATION: &str = "uses a commit that doesn't belong to the specified org/repo";
 
 pub(crate) struct ImpostorCommit<'a> {

src/audit/mod.rs

@@ -1,10 +1,12 @@
+use anyhow::Result;
+
 use crate::{
     finding::{Finding, FindingBuilder},
     models::{AuditConfig, Workflow},
 };
-use anyhow::Result;
 
 pub(crate) mod artipacked;
+pub(crate) mod excessive_permissions;
 pub(crate) mod hardcoded_container_credentials;
 pub(crate) mod impostor_commit;
 pub(crate) mod pull_request_target;

src/audit/pull_request_target.rs

@@ -2,11 +2,10 @@ use anyhow::Result;
 use github_actions_models::workflow::event::{BareEvent, OptionalBody};
 use github_actions_models::workflow::Trigger;
 
+use super::WorkflowAudit;
 use crate::finding::{Confidence, Finding, Severity};
 use crate::models::{AuditConfig, Workflow};
 
-use super::WorkflowAudit;
-
 pub(crate) struct PullRequestTarget<'a> {
     pub(crate) _config: AuditConfig<'a>,
 }

src/audit/ref_confusion.rs

@@ -11,14 +11,13 @@ use std::ops::Deref;
 use anyhow::Result;
 use github_actions_models::workflow::{job::StepBody, Job};
 
+use super::WorkflowAudit;
 use crate::{
     finding::{Confidence, Severity},
     github_api,
     models::{AuditConfig, Uses},
 };
 
-use super::WorkflowAudit;
-
 const REF_CONFUSION_ANNOTATION: &str =
     "uses a ref that's provided by both the branch and tag namespaces";
 

src/audit/template_injection.rs

@@ -9,14 +9,13 @@ use std::ops::Deref;
 
 use github_actions_models::workflow::{job::StepBody, Job};
 
+use super::WorkflowAudit;
 use crate::{
     finding::{Confidence, Severity},
     models::AuditConfig,
     utils::iter_expressions,
 };
 
-use super::WorkflowAudit;
-
 pub(crate) struct TemplateInjection<'a> {
     pub(crate) _config: AuditConfig<'a>,
 }

src/audit/use_trusted_publishing.rs

@@ -5,13 +5,12 @@ use github_actions_models::{
     workflow::{job::StepBody, Job},
 };
 
+use super::WorkflowAudit;
 use crate::{
     finding::{Confidence, Severity},
     models::AuditConfig,
 };
 
-use super::WorkflowAudit;
-
 const USES_MANUAL_CREDENTIAL: &str =
     "uses a manually-configured credential instead of Trusted Publishing";
 

src/finding/locate.rs

@@ -4,9 +4,8 @@
 use anyhow::{Ok, Result};
 use tree_sitter::{Language, Query, QueryCursor};
 
-use crate::models::Workflow;
-
 use super::{Feature, WorkflowLocation};
+use crate::models::Workflow;
 
 /// Captures an arbitrary top-level key within a YAML stream.
 const TOP_LEVEL_KEY: &str = r#"

src/finding/mod.rs

@@ -8,7 +8,7 @@ pub(crate) mod locate;
 
 // TODO: Traits + more flexible models here.
 
-#[derive(Default, Serialize)]
+#[derive(Copy, Clone, Default, Serialize)]
 pub(crate) enum Confidence {
     #[default]
     Unknown,
@@ -17,7 +17,7 @@ pub(crate) enum Confidence {
     High,
 }
 
-#[derive(Default, Serialize)]
+#[derive(Copy, Clone, Default, Serialize)]
 pub(crate) enum Severity {
     #[default]
     Unknown,

src/main.rs

@@ -94,6 +94,7 @@ fn main() -> Result<()> {
     let mut results = vec![];
     let audits: &mut [&mut dyn WorkflowAudit] = &mut [
         &mut audit::artipacked::Artipacked::new(config)?,
+        &mut audit::excessive_permissions::ExcessivePermissions::new(config)?,
         &mut audit::pull_request_target::PullRequestTarget::new(config)?,
         &mut audit::impostor_commit::ImpostorCommit::new(config)?,
         &mut audit::ref_confusion::RefConfusion::new(config)?,

src/models.rs

@@ -1,8 +1,8 @@
-use anyhow::{anyhow, Context, Ok, Result};
 use std::{collections::hash_map, iter::Enumerate, ops::Deref, path::Path};
-use tree_sitter::Tree;
 
+use anyhow::{anyhow, Context, Ok, Result};
 use github_actions_models::workflow;
+use tree_sitter::Tree;
 
 use crate::finding::WorkflowLocation;