add --pedantic mode

By default, we'll now ignore things
like explicit `persist-credentials: true`,
since they suggest that they user knows what they're
doing. However, in pedantic mode, these will still be flagged.

Signed-off-by: William Woodruff <william@yossarian.net>
This commit is contained in:
William Woodruff 2024-08-19 15:53:54 -04:00
parent 44e6dee80b
commit e7580168c0
No known key found for this signature in database
4 changed files with 38 additions and 10 deletions

View file

@ -4,10 +4,13 @@ use github_actions_models::{
};
use itertools::Itertools;
use crate::finding::{Confidence, Finding, JobIdentity, Severity, StepIdentity};
use crate::models::Workflow;
use crate::{
finding::{Confidence, Finding, JobIdentity, Severity, StepIdentity},
models::AuditOptions,
};
pub(crate) fn audit(workflow: &Workflow) -> Vec<Finding> {
pub(crate) fn audit(_options: &AuditOptions, workflow: &Workflow) -> Vec<Finding> {
let mut findings = vec![];
for (jobname, job) in workflow.jobs.iter() {
@ -29,11 +32,18 @@ pub(crate) fn audit(workflow: &Workflow) -> Vec<Finding> {
if uses.starts_with("actions/checkout") {
match with.get("persist-credentials") {
Some(EnvValue::Boolean(false)) => continue,
Some(EnvValue::Boolean(true)) => {
// If a user explicitly sets `persist-credentials: true`,
// they probably mean it. Only report if being pedantic.
if _options.pedantic {
vulnerable_checkouts.push(StepIdentity::new(stepno, step))
} else {
continue;
}
}
// TODO: handle expressions and literal strings here.
// persist-credentials is true by default.
Some(EnvValue::Boolean(true)) | _ => {
vulnerable_checkouts.push(StepIdentity::new(stepno, step))
}
_ => vulnerable_checkouts.push(StepIdentity::new(stepno, step)),
}
}

View file

@ -2,9 +2,9 @@ use github_actions_models::workflow::event::{BareEvent, OptionalBody};
use github_actions_models::workflow::Trigger;
use crate::finding::{Confidence, Finding, Severity};
use crate::models::Workflow;
use crate::models::{AuditOptions, Workflow};
pub(crate) fn audit(workflow: &Workflow) -> Vec<Finding> {
pub(crate) fn audit(_options: &AuditOptions, workflow: &Workflow) -> Vec<Finding> {
let trigger = &workflow.on;
let has_pull_request_target = match trigger {

View file

@ -2,6 +2,7 @@ use std::{io::stdout, path::PathBuf};
use anyhow::{anyhow, Result};
use clap::Parser;
use models::AuditOptions;
mod audit;
mod finding;
@ -10,16 +11,29 @@ mod models;
/// A tool to detect "ArtiPACKED"-type credential disclosures in GitHub Actions.
#[derive(Parser)]
struct Args {
/// Emit findings even when the context suggests an explicit security decision made by the user.
#[arg(short, long)]
pedantic: bool,
/// The workflow filename or directory to audit.
input: PathBuf,
}
impl From<&Args> for AuditOptions {
fn from(value: &Args) -> Self {
Self {
pedantic: value.pedantic,
}
}
}
fn main() -> Result<()> {
env_logger::init();
let args = Args::parse();
let mut workflow_paths = vec![];
let options = AuditOptions::from(&args);
let mut workflow_paths = vec![];
if args.input.is_file() {
workflow_paths.push(args.input);
} else if args.input.is_dir() {
@ -51,8 +65,8 @@ fn main() -> Result<()> {
for workflow_path in workflow_paths.iter() {
let workflow = models::Workflow::from_file(workflow_path)?;
// TODO: Proper abstraction for multiple audits here.
findings.extend(audit::artipacked::audit(&workflow));
findings.extend(audit::pull_request_target::audit(&workflow));
findings.extend(audit::artipacked::audit(&options, &workflow));
findings.extend(audit::pull_request_target::audit(&options, &workflow));
}
if !findings.is_empty() {

View file

@ -34,3 +34,7 @@ impl Workflow {
})
}
}
pub(crate) struct AuditOptions {
pub(crate) pedantic: bool,
}