Skip to content

Commit

Permalink
audit registry abstraction
Browse files Browse the repository at this point in the history
This saves us a small amount of duplication, and sets us up
to handle `--offline` and other state tweaking flags correctly.

Signed-off-by: William Woodruff <[email protected]>
  • Loading branch information
woodruffw committed Sep 11, 2024
1 parent fc4e60a commit d8af234
Show file tree
Hide file tree
Showing 8 changed files with 75 additions and 33 deletions.
4 changes: 0 additions & 4 deletions src/audit/artipacked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,6 @@ impl<'a> WorkflowAudit<'a> for Artipacked<'a> {
}

fn audit<'w>(&mut self, workflow: &'w Workflow) -> Result<Vec<Finding<'w>>> {
log::debug!("audit: {} evaluating {}", Self::ident(), &workflow.filename);

let mut findings = vec![];

for job in workflow.jobs() {
Expand Down Expand Up @@ -143,8 +141,6 @@ impl<'a> WorkflowAudit<'a> for Artipacked<'a> {
}
}

log::debug!("audit: {} completed {}", Self::ident(), &workflow.filename);

Ok(findings)
}
}
12 changes: 9 additions & 3 deletions src/audit/excessive_permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,17 +136,23 @@ impl<'a> ExcessivePermissions<'a> {
Some(sev) => results.push((
*sev,
Confidence::High,
format!("{name}: write is overly broad at the workflow level; move to the job level"),
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")
format!(
"{name}: write is overly broad at the workflow level; \
move to the job level"
),
))
},
}
}
}

Expand Down
4 changes: 0 additions & 4 deletions src/audit/impostor_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,6 @@ impl<'a> WorkflowAudit<'a> for ImpostorCommit<'a> {
}

fn audit<'w>(&mut self, workflow: &'w Workflow) -> Result<Vec<Finding<'w>>> {
log::debug!("audit: {} evaluating {}", Self::ident(), &workflow.filename);

let mut findings = vec![];

for job in workflow.jobs() {
Expand Down Expand Up @@ -197,8 +195,6 @@ impl<'a> WorkflowAudit<'a> for ImpostorCommit<'a> {
}
}

log::debug!("audit: {} completed {}", Self::ident(), &workflow.filename);

Ok(findings)
}
}
4 changes: 0 additions & 4 deletions src/audit/pull_request_target.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ impl<'a> WorkflowAudit<'a> for PullRequestTarget<'a> {
}

fn audit<'w>(&mut self, workflow: &'w Workflow) -> Result<Vec<Finding<'w>>> {
log::debug!("audit: {} evaluating {}", Self::ident(), &workflow.filename);

let trigger = &workflow.on;

let has_pull_request_target = match trigger {
Expand All @@ -44,8 +42,6 @@ impl<'a> WorkflowAudit<'a> for PullRequestTarget<'a> {
);
}

log::debug!("audit: {} completed {}", Self::ident(), &workflow.filename);

Ok(findings)
}
}
4 changes: 0 additions & 4 deletions src/audit/ref_confusion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ impl<'a> WorkflowAudit<'a> for RefConfusion<'a> {
&mut self,
workflow: &'w crate::models::Workflow,
) -> anyhow::Result<Vec<crate::finding::Finding<'w>>> {
log::debug!("audit: {} evaluating {}", Self::ident(), &workflow.filename);

let mut findings = vec![];

for job in workflow.jobs() {
Expand Down Expand Up @@ -119,8 +117,6 @@ impl<'a> WorkflowAudit<'a> for RefConfusion<'a> {
}
}

log::debug!("audit: {} completed {}", Self::ident(), &workflow.filename);

Ok(findings)
}
}
3 changes: 0 additions & 3 deletions src/audit/use_trusted_publishing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ impl<'a> WorkflowAudit<'a> for UseTrustedPublishing<'a> {
&mut self,
workflow: &'w crate::models::Workflow,
) -> anyhow::Result<Vec<crate::finding::Finding<'w>>> {
log::debug!("audit: {} evaluating {}", Self::ident(), &workflow.filename);

let mut findings = vec![];

for job in workflow.jobs() {
Expand Down Expand Up @@ -121,7 +119,6 @@ impl<'a> WorkflowAudit<'a> for UseTrustedPublishing<'a> {
}
}

log::debug!("audit: {} completed {}", Self::ident(), &workflow.filename);
Ok(findings)
}
}
44 changes: 33 additions & 11 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ use anyhow::{anyhow, Result};
use audit::WorkflowAudit;
use clap::{Parser, ValueEnum};
use models::AuditConfig;
use registry::Registry;

mod audit;
mod finding;
mod github_api;
mod models;
mod registry;
mod sarif;
mod utils;

Expand Down Expand Up @@ -91,20 +93,40 @@ fn main() -> Result<()> {
workflows.push(models::Workflow::from_file(workflow_path)?);
}

let mut registry = Registry::new();

macro_rules! register_audit {
($rule:path) => {{
// HACK: https://github.com/rust-lang/rust/issues/48067
use $rule as base;
match base::new(config) {
Ok(audit) => registry.register_workflow_audit(base::ident(), Box::new(audit)),
Err(e) => log::warn!("{audit} is being skipped: {e}", audit = base::ident()),
}
}};
}

register_audit!(audit::artipacked::Artipacked);
register_audit!(audit::excessive_permissions::ExcessivePermissions);
register_audit!(audit::pull_request_target::PullRequestTarget);
register_audit!(audit::impostor_commit::ImpostorCommit);
register_audit!(audit::ref_confusion::RefConfusion);
register_audit!(audit::use_trusted_publishing::UseTrustedPublishing);
register_audit!(audit::template_injection::TemplateInjection);
register_audit!(audit::hardcoded_container_credentials::HardcodedContainerCredentials);

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)?,
&mut audit::use_trusted_publishing::UseTrustedPublishing::new(config)?,
&mut audit::template_injection::TemplateInjection::new(config)?,
&mut audit::hardcoded_container_credentials::HardcodedContainerCredentials::new(config)?,
];
for workflow in workflows.iter() {
for audit in audits.iter_mut() {
for (name, audit) in registry.iter_workflow_audits() {
log::info!(
"performing {name} on {workflow}",
workflow = &workflow.filename
);
results.extend(audit.audit(workflow)?);
log::info!(
"completed {name} on {workflow}",
workflow = &workflow.filename
);
}
}

Expand Down
33 changes: 33 additions & 0 deletions src/registry.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
//! Functionality for registering and managing the lifecycles of
//! audits.

use std::collections::HashMap;

use crate::audit::WorkflowAudit;

pub(crate) struct Registry<'config> {
pub(crate) workflow_audits: HashMap<&'static str, Box<dyn WorkflowAudit<'config> + 'config>>,
}

impl<'config> Registry<'config> {
pub(crate) fn new() -> Self {
Self {
workflow_audits: Default::default(),
}
}

pub(crate) fn register_workflow_audit(
&mut self,
ident: &'static str,
audit: Box<dyn WorkflowAudit<'config> + 'config>,
) {
self.workflow_audits.insert(ident, audit);
}

pub(crate) fn iter_workflow_audits(
&mut self,
) -> std::collections::hash_map::IterMut<'_, &str, Box<dyn WorkflowAudit<'config> + 'config>>
{
self.workflow_audits.iter_mut()
}
}

0 comments on commit d8af234

Please sign in to comment.