feat: add personas (#226)

This commit is contained in:
William Woodruff 2024-12-02 20:45:28 -05:00 committed by GitHub
parent 8c8fecb13f
commit 76445552fc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
23 changed files with 383 additions and 108 deletions

11
Cargo.lock generated
View file

@ -545,10 +545,11 @@ checksum = "07e28edb80900c19c28f1072f2e8aeca7fa06b23cd4169cefe1af5aa3260783f"
[[package]]
name = "github-actions-models"
version = "0.11.0"
version = "0.12.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1106863433991623eb597aea98e60623ce119dbc190c9c2b3166cdb75c751b1f"
checksum = "0102922a92566de8ff25ff79144d6b30516efe941bc34ff849f01b4979add8e2"
dependencies = [
"indexmap",
"serde",
"serde_yaml",
]
@ -865,12 +866,13 @@ dependencies = [
[[package]]
name = "indexmap"
version = "2.6.0"
version = "2.7.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "707907fe3c25f5424cce2cb7e1cbcafee6bdbe735ca90ef77c29e84591e5b9da"
checksum = "62f822373a4fe84d4bb149bf54e584a7f4abec90e072ed49cda0edea5b95471f"
dependencies = [
"equivalent",
"hashbrown",
"serde",
]
[[package]]
@ -2578,6 +2580,7 @@ dependencies = [
"env_logger",
"github-actions-models",
"human-panic",
"indexmap",
"indicatif",
"insta",
"itertools",

View file

@ -19,8 +19,9 @@ anyhow = "1.0.93"
clap = { version = "4.5.21", features = ["derive", "env"] }
clap-verbosity-flag = "3.0.0"
env_logger = "0.11.5"
github-actions-models = "0.11.0"
github-actions-models = "0.12.0"
human-panic = "2.0.1"
indexmap = "2.7.0"
indicatif = "0.17.9"
itertools = "0.13.0"
log = "0.4.22"

View file

@ -3,34 +3,69 @@ Static analysis for GitHub Actions
Usage: zizmor [OPTIONS] <INPUTS>...
Arguments:
<INPUTS>... The workflow filenames or directories to audit
<INPUTS>...
The workflow filenames or directories to audit
Options:
-p, --pedantic
Emit findings even when the context suggests an explicit security decision made by the user
Emit 'pedantic' findings.
This is an alias for --persona=pedantic.
--persona <PERSONA>
The persona to use while auditing
[default: regular]
Possible values:
- auditor: The "auditor" persona (false positives OK)
- pedantic: The "pedantic" persona (code smells OK)
- regular: The "regular" persona (minimal false positives)
-o, --offline
Only perform audits that don't require network access
-v, --verbose...
Increase logging verbosity
-q, --quiet...
Decrease logging verbosity
-n, --no-progress
Disable the progress bar. This is useful primarily when running with a high verbosity level, as the two will fight for stderr
--gh-token <GH_TOKEN>
The GitHub API token to use [env: GH_TOKEN=]
The GitHub API token to use
[env: GH_TOKEN=]
--format <FORMAT>
The output format to emit. By default, plain text will be emitted [possible values: plain, json, sarif]
The output format to emit. By default, plain text will be emitted
[default: plain]
[possible values: plain, json, sarif]
-c, --config <CONFIG>
The configuration file to load. By default, any config will be discovered relative to $CWD
--no-config
Disable all configuration loading
--no-exit-codes
Disable all error codes besides success and tool failure
--min-severity <MIN_SEVERITY>
Filter all results below this severity [possible values: unknown, informational, low, medium, high]
Filter all results below this severity
[possible values: unknown, informational, low, medium, high]
--min-confidence <MIN_CONFIDENCE>
Filter all results below this confidence [possible values: unknown, low, medium, high]
Filter all results below this confidence
[possible values: unknown, low, medium, high]
-h, --help
Print help
Print help (see a summary with '-h')
-V, --version
Print version

View file

@ -69,6 +69,94 @@ See [Integration](#integration) for suggestions on when to use each format.
All other exit codes are currently reserved.
## Using personas
!!! tip
`--persona=...` is available in `v0.7.0` and later.
`zizmor` comes with three pre-defined "personas," which dictate how
sensitive `zizmor`'s analyses are:
* The _regular persona_: the user wants high-signal, low-noise, actionable
security findings. This persona is best for ordinary local use as well as use
in most CI/CD setups, which is why it's the default.
!!! note
This persona can be made explicit with `--persona=regular`,
although this is not required.
* The _pedantic persona_, enabled by `--persona=pedantic`: the user wants
*code smells* in addition to regular, actionable security findings.
This persona is ideal for finding things that are a good idea
to clean up or resolve, but are likely not immediately actionable
security findings (or are actionable, but indicate a intentional
security decision by the workflow author).
For example, using the pedantic persona will flag the following
with an `unpinned-uses` finding, since it uses a symbolic reference
as its pin instead of a hashed pin:
```yaml
uses: actions/checkout@v3
```
produces:
```console
$ zizmor --pedantic tests/test-data/unpinned-uses.yml
help[unpinned-uses]: unpinned action reference
--> tests/test-data/unpinned-uses.yml:14:9
|
14 | - uses: actions/checkout@v3
| ------------------------- help: action is not pinned to a hash ref
|
= note: audit confidence → High
```
!!! tip
This persona can also be enabled with `--pedantic`, which is
an alias for `--persona=pedantic`.
* The _auditor persona_, enabled by `--persona=auditor`: the user wants
*everything* flagged by `zizmor`, including findings that are likely
to be false positives.
This persona is ideal for security auditors and code reviewers, who
want to go through `zizmor`'s findings manually with a fine-toothed comb.
Some audits, notably `self-hosted-runner`, *only* produce auditor-level
results. This is because these audits require runtime context that `zizmor`
lacks access to by design, meaning that their results are always
subject to false positives.
For example, with the default persona:
```console
$ zizmor tests/test-data/self-hosted.yml
🌈 completed self-hosted.yml
No findings to report. Good job! (1 suppressed)
```
and with `--persona=auditor`:
```console
$ zizmor --persona=auditor tests/test-data/self-hosted.yml
note[self-hosted-runner]: runs on a self-hosted runner
--> tests/test-data/self-hosted.yml:8:5
|
8 | runs-on: [self-hosted, my-ubuntu-box]
| ------------------------------------- note: self-hosted runner used here
|
= note: audit confidence → High
1 finding: 1 unknown, 0 informational, 0 low, 0 medium, 0 high
```
## Filtering results
There are two straightforward ways to filter `zizmor`'s results:

View file

@ -9,14 +9,12 @@ use itertools::Itertools;
use super::{audit_meta, WorkflowAudit};
use crate::{
finding::{Confidence, Finding, Severity},
finding::{Confidence, Finding, Persona, Severity},
state::AuditState,
};
use crate::{models::Workflow, utils::split_patterns};
pub(crate) struct Artipacked {
pub(crate) state: AuditState,
}
pub(crate) struct Artipacked;
audit_meta!(
Artipacked,
@ -47,8 +45,8 @@ impl Artipacked {
}
impl WorkflowAudit for Artipacked {
fn new(state: AuditState) -> Result<Self> {
Ok(Self { state })
fn new(_state: AuditState) -> Result<Self> {
Ok(Self)
}
fn audit<'w>(&self, workflow: &'w Workflow) -> Result<Vec<Finding<'w>>> {
@ -76,15 +74,11 @@ impl WorkflowAudit for Artipacked {
Some(EnvValue::Boolean(true)) => {
// If a user explicitly sets `persist-credentials: true`,
// they probably mean it. Only report if being pedantic.
if self.state.pedantic {
vulnerable_checkouts.push(step)
} else {
continue;
}
vulnerable_checkouts.push((step, Persona::Pedantic))
}
// TODO: handle expressions and literal strings here.
// persist-credentials is true by default.
_ => vulnerable_checkouts.push(step),
_ => vulnerable_checkouts.push((step, Persona::default())),
}
} else if uses.starts_with("actions/upload-artifact") {
let Some(EnvValue::String(path)) = with.get("path") else {
@ -102,11 +96,12 @@ impl WorkflowAudit for Artipacked {
if vulnerable_uploads.is_empty() {
// If we have no vulnerable uploads, then emit lower-confidence
// findings for just the checkout steps.
for checkout in vulnerable_checkouts {
for (checkout, persona) in vulnerable_checkouts {
findings.push(
Self::finding()
.severity(Severity::Medium)
.confidence(Confidence::Low)
.persona(persona)
.add_location(
checkout
.location()
@ -119,7 +114,7 @@ impl WorkflowAudit for Artipacked {
// Select only pairs where the vulnerable checkout precedes the
// vulnerable upload. There are more efficient ways to do this than
// a cartesian product, but this way is simple.
for (checkout, upload) in vulnerable_checkouts
for ((checkout, persona), upload) in vulnerable_checkouts
.into_iter()
.cartesian_product(vulnerable_uploads.into_iter())
{
@ -128,6 +123,7 @@ impl WorkflowAudit for Artipacked {
Self::finding()
.severity(Severity::High)
.confidence(Confidence::High)
.persona(persona)
.add_location(
checkout
.location()

View file

@ -172,7 +172,6 @@ mod tests {
("something | tee \"${$OTHER_ENV}\" # not $GITHUB_ENV", false),
] {
let audit_state = AuditState {
pedantic: false,
offline: false,
gh_token: None,
caches: Caches::new(),

View file

@ -2,11 +2,11 @@
//! which are frequently unsafe to use in public repositories
//! due to the potential for persistence between workflow runs.
//!
//! This audit is "pedantic" only, since zizmor can't detect
//! This audit is "auditor" only, since zizmor can't detect
//! whether self-hosted runners are ephemeral or not.
use crate::{
finding::{Confidence, Severity},
finding::{Confidence, Persona, Severity},
AuditState,
};
@ -18,9 +18,7 @@ use github_actions_models::{
use super::{audit_meta, WorkflowAudit};
pub(crate) struct SelfHostedRunner {
pub(crate) state: AuditState,
}
pub(crate) struct SelfHostedRunner;
audit_meta!(
SelfHostedRunner,
@ -29,11 +27,11 @@ audit_meta!(
);
impl WorkflowAudit for SelfHostedRunner {
fn new(state: AuditState) -> anyhow::Result<Self>
fn new(_state: AuditState) -> anyhow::Result<Self>
where
Self: Sized,
{
Ok(Self { state })
Ok(Self)
}
fn audit<'w>(
@ -42,11 +40,6 @@ impl WorkflowAudit for SelfHostedRunner {
) -> Result<Vec<crate::finding::Finding<'w>>> {
let mut results = vec![];
if !self.state.pedantic {
log::info!("skipping self-hosted runner checks");
return Ok(results);
}
for job in workflow.jobs() {
let Job::NormalJob(normal) = *job else {
continue;
@ -66,6 +59,7 @@ impl WorkflowAudit for SelfHostedRunner {
Self::finding()
.confidence(Confidence::High)
.severity(Severity::Unknown)
.persona(Persona::Auditor)
.add_location(
job.location()
.with_keys(&["runs-on".into()])
@ -82,6 +76,7 @@ impl WorkflowAudit for SelfHostedRunner {
Self::finding()
.confidence(Confidence::Low)
.severity(Severity::Unknown)
.persona(Persona::Auditor)
.add_location(
job.location().with_keys(&["runs-on".into()]).annotated(
"expression may expand into a self-hosted runner",
@ -101,6 +96,7 @@ impl WorkflowAudit for SelfHostedRunner {
Self::finding()
.confidence(Confidence::Low)
.severity(Severity::Unknown)
.persona(Persona::Auditor)
.add_location(
job.location()
.with_keys(&["runs-on".into()])
@ -114,6 +110,7 @@ impl WorkflowAudit for SelfHostedRunner {
Self::finding()
.confidence(Confidence::Low)
.severity(Severity::Unknown)
.persona(Persona::Auditor)
.add_location(
job.location()
.with_keys(&["runs-on".into()])

View file

@ -20,14 +20,12 @@ use github_actions_models::{
use super::{audit_meta, WorkflowAudit};
use crate::{
expr::{BinOp, Expr, UnOp},
finding::{Confidence, Severity},
finding::{Confidence, Persona, Severity},
state::AuditState,
utils::extract_expressions,
};
pub(crate) struct TemplateInjection {
pub(crate) state: AuditState,
}
pub(crate) struct TemplateInjection;
audit_meta!(
TemplateInjection,
@ -165,22 +163,27 @@ impl TemplateInjection {
&self,
run: &str,
job: &NormalJob,
) -> Vec<(String, Severity, Confidence)> {
) -> Vec<(String, Severity, Confidence, Persona)> {
let mut bad_expressions = vec![];
for expr in extract_expressions(run) {
let Ok(expr) = Expr::parse(expr.as_bare()) else {
let Ok(parsed) = Expr::parse(expr.as_bare()) else {
log::warn!("couldn't parse expression: {expr}", expr = expr.as_bare());
continue;
};
// Filter "safe" expressions (ones that might expand to code,
// but not arbitrary code) by default, unless we're operating
// in pedantic mode.
if Self::expr_is_safe(&expr) && !self.state.pedantic {
if Self::expr_is_safe(&parsed) {
// Emit a pedantic finding for all expressions, since
// all template injections are code smells, even if unexploitable.
bad_expressions.push((
expr.as_raw().into(),
Severity::Unknown,
Confidence::Unknown,
Persona::Pedantic,
));
continue;
}
for context in expr.contexts() {
for context in parsed.contexts() {
if context.starts_with("secrets.") {
// While not ideal, secret expansion is typically not exploitable.
continue;
@ -191,14 +194,29 @@ impl TemplateInjection {
// input's type. In the future, we should index back into
// the workflow's triggers and exclude input expansions
// from innocuous types, e.g. booleans.
bad_expressions.push((context.into(), Severity::High, Confidence::Low));
bad_expressions.push((
context.into(),
Severity::High,
Confidence::Low,
Persona::default(),
));
} else if context.starts_with("env.") {
// Almost never exploitable.
bad_expressions.push((context.into(), Severity::Low, Confidence::High));
bad_expressions.push((
context.into(),
Severity::Low,
Confidence::High,
Persona::default(),
));
} else if context.starts_with("github.event.") || context == "github.ref_name" {
// TODO: Filter these more finely; not everything in the event
// context is actually attacker-controllable.
bad_expressions.push((context.into(), Severity::High, Confidence::High));
bad_expressions.push((
context.into(),
Severity::High,
Confidence::High,
Persona::default(),
));
} else if context.starts_with("matrix.") || context == "matrix" {
if let Some(Strategy { matrix, .. }) = &job.strategy {
let matrix_is_static = match matrix {
@ -218,6 +236,7 @@ impl TemplateInjection {
context.into(),
Severity::Medium,
Confidence::Medium,
Persona::default(),
));
}
}
@ -229,6 +248,7 @@ impl TemplateInjection {
context.into(),
Severity::Informational,
Confidence::Low,
Persona::default(),
));
}
}
@ -239,11 +259,11 @@ impl TemplateInjection {
}
impl WorkflowAudit for TemplateInjection {
fn new(state: AuditState) -> anyhow::Result<Self>
fn new(_state: AuditState) -> anyhow::Result<Self>
where
Self: Sized,
{
Ok(Self { state })
Ok(Self)
}
fn audit_step<'w>(&self, step: &super::Step<'w>) -> anyhow::Result<Vec<super::Finding<'w>>> {
@ -266,12 +286,14 @@ impl WorkflowAudit for TemplateInjection {
StepBody::Run { run, .. } => (run, step.location().with_keys(&["run".into()])),
};
for (expr, severity, confidence) in self.injectable_template_expressions(script, step.job())
for (expr, severity, confidence, persona) in
self.injectable_template_expressions(script, step.job())
{
findings.push(
Self::finding()
.severity(severity)
.confidence(confidence)
.persona(persona)
.add_location(step.location_with_name())
.add_location(
script_loc.clone().annotated(format!(

View file

@ -1,19 +1,17 @@
use crate::finding::{Confidence, Severity};
use crate::finding::{Confidence, Persona, Severity};
use super::{audit_meta, AuditState, Finding, Step, WorkflowAudit};
pub(crate) struct UnpinnedUses {
state: AuditState,
}
pub(crate) struct UnpinnedUses;
audit_meta!(UnpinnedUses, "unpinned-uses", "unpinned action reference");
impl WorkflowAudit for UnpinnedUses {
fn new(state: AuditState) -> anyhow::Result<Self>
fn new(_state: AuditState) -> anyhow::Result<Self>
where
Self: Sized,
{
Ok(Self { state })
Ok(Self)
}
fn audit_step<'w>(&self, step: &Step<'w>) -> anyhow::Result<Vec<Finding<'w>>> {
@ -23,13 +21,18 @@ impl WorkflowAudit for UnpinnedUses {
return Ok(vec![]);
};
let (annotation, severity) = if uses.unpinned() {
let (annotation, severity, persona) = if uses.unpinned() {
(
"action is not pinned to a tag, branch, or hash ref",
Severity::Medium,
Persona::default(),
)
} else if uses.unhashed() {
(
"action is not pinned to a hash ref",
Severity::Low,
Persona::Pedantic,
)
} else if uses.unhashed() && self.state.pedantic {
("action is not pinned to a hash ref", Severity::Low)
} else {
return Ok(vec![]);
};
@ -38,6 +41,7 @@ impl WorkflowAudit for UnpinnedUses {
Self::finding()
.confidence(Confidence::High)
.severity(severity)
.persona(persona)
.add_location(
step.location()
.with_keys(&["uses".into()])

View file

@ -1,7 +1,8 @@
use std::{collections::HashMap, ops::Deref};
use std::ops::Deref;
use anyhow::Ok;
use github_actions_models::{common::EnvValue, workflow::job::StepBody};
use indexmap::IndexMap;
use super::{audit_meta, WorkflowAudit};
use crate::{
@ -28,7 +29,7 @@ audit_meta!(
);
impl UseTrustedPublishing {
fn pypi_publish_uses_manual_credentials(&self, with: &HashMap<String, EnvValue>) -> bool {
fn pypi_publish_uses_manual_credentials(&self, with: &IndexMap<String, EnvValue>) -> bool {
// `password` implies the step isn't using Trusted Publishing,
// but we also need to check `repository-url` to prevent false-positives
// on third-party indices.
@ -46,7 +47,7 @@ impl UseTrustedPublishing {
}
}
fn release_gem_uses_manual_credentials(&self, with: &HashMap<String, EnvValue>) -> bool {
fn release_gem_uses_manual_credentials(&self, with: &IndexMap<String, EnvValue>) -> bool {
match with.get("setup-trusted-publisher") {
Some(v) if v.to_string() == "true" => false,
// Anything besides `true` means to *not* use trusted publishing.
@ -58,7 +59,7 @@ impl UseTrustedPublishing {
fn rubygems_credential_uses_manual_credentials(
&self,
with: &HashMap<String, EnvValue>,
with: &IndexMap<String, EnvValue>,
) -> bool {
with.contains_key("api-token")
}

View file

@ -13,7 +13,32 @@ use crate::models::{Job, Step, Workflow};
pub(crate) mod locate;
// TODO: Traits + more flexible models here.
/// Represents the expected "persona" that would be interested in a given
/// finding. This is used to model the sensitivity of different use-cases
/// to false positives.
#[derive(
Copy, Clone, Debug, Default, Eq, Hash, Ord, PartialOrd, PartialEq, Serialize, ValueEnum,
)]
pub(crate) enum Persona {
/// The "auditor" persona (false positives OK).
///
/// This persona wants all results, including results that are likely
/// to be false positives.
Auditor,
/// The "pedantic" persona (code smells OK).
///
/// This persona wants findings that may or may not be problems,
/// but are potential "code smells".
Pedantic,
/// The "regular" persona (minimal false positives).
///
/// This persona wants actionable findings, and is sensitive to
/// false positives.
#[default]
Regular,
}
#[derive(
Copy, Clone, Debug, Default, Eq, Hash, Ord, PartialOrd, PartialEq, Serialize, ValueEnum,
@ -240,11 +265,12 @@ pub(crate) struct Location<'w> {
pub(crate) concrete: Feature<'w>,
}
/// A finding's "determination," i.e. its confidence and severity classifications.
/// A finding's "determination," i.e. its various classifications.
#[derive(Serialize)]
pub(crate) struct Determinations {
pub(crate) confidence: Confidence,
pub(crate) severity: Severity,
pub(super) persona: Persona,
}
#[derive(Serialize)]
@ -263,6 +289,7 @@ pub(crate) struct FindingBuilder<'w> {
url: &'static str,
severity: Severity,
confidence: Confidence,
persona: Persona,
locations: Vec<SymbolicLocation<'w>>,
}
@ -274,6 +301,7 @@ impl<'w> FindingBuilder<'w> {
url,
severity: Default::default(),
confidence: Default::default(),
persona: Default::default(),
locations: vec![],
}
}
@ -288,6 +316,11 @@ impl<'w> FindingBuilder<'w> {
self
}
pub(crate) fn persona(mut self, persona: Persona) -> Self {
self.persona = persona;
self
}
pub(crate) fn add_location(mut self, location: SymbolicLocation<'w>) -> Self {
self.locations.push(location);
self
@ -309,6 +342,7 @@ impl<'w> FindingBuilder<'w> {
determinations: Determinations {
confidence: self.confidence,
severity: self.severity,
persona: self.persona,
},
locations,
ignored: should_ignore,

View file

@ -5,7 +5,7 @@ use anyhow::{anyhow, Context, Result};
use audit::WorkflowAudit;
use clap::{Parser, ValueEnum};
use config::Config;
use finding::{Confidence, Severity};
use finding::{Confidence, Persona, Severity};
use indicatif::{ProgressBar, ProgressDrawTarget, ProgressStyle};
use owo_colors::OwoColorize;
use registry::{AuditRegistry, FindingRegistry, WorkflowRegistry};
@ -27,10 +27,16 @@ mod utils;
#[derive(Parser)]
#[command(about, version)]
struct App {
/// Emit findings even when the context suggests an explicit security decision made by the user.
#[arg(short, long)]
/// Emit 'pedantic' findings.
///
/// This is an alias for --persona=pedantic.
#[arg(short, long, group = "_persona")]
pedantic: bool,
/// The persona to use while auditing.
#[arg(long, group = "_persona", value_enum, default_value_t)]
persona: Persona,
/// Only perform audits that don't require network access.
#[arg(short, long)]
offline: bool,
@ -48,8 +54,8 @@ struct App {
gh_token: Option<String>,
/// The output format to emit. By default, plain text will be emitted
#[arg(long, value_enum)]
format: Option<OutputFormat>,
#[arg(long, value_enum, default_value_t)]
format: OutputFormat,
/// The configuration file to load. By default, any config will be
/// discovered relative to $CWD.
@ -77,8 +83,9 @@ struct App {
inputs: Vec<PathBuf>,
}
#[derive(Debug, Copy, Clone, ValueEnum)]
#[derive(Debug, Default, Copy, Clone, ValueEnum)]
pub(crate) enum OutputFormat {
#[default]
Plain,
Json,
Sarif,
@ -87,7 +94,12 @@ pub(crate) enum OutputFormat {
fn run() -> Result<ExitCode> {
human_panic::setup_panic!();
let app = App::parse();
let mut app = App::parse();
// `--pedantic` is a shortcut for `--persona=pedantic`.
if app.pedantic {
app.persona = Persona::Pedantic;
}
env_logger::Builder::new()
.filter_level(app.verbose.log_level_filter())
@ -203,12 +215,7 @@ fn run() -> Result<ExitCode> {
bar.finish_and_clear();
let format = match app.format {
None => OutputFormat::Plain,
Some(f) => f,
};
match format {
match app.format {
OutputFormat::Plain => render::render_findings(&workflow_registry, &results),
OutputFormat::Json => serde_json::to_writer_pretty(stdout(), &results.findings())?,
OutputFormat::Sarif => serde_json::to_writer_pretty(
@ -217,7 +224,7 @@ fn run() -> Result<ExitCode> {
)?,
};
if app.no_exit_codes || matches!(format, OutputFormat::Sarif) {
if app.no_exit_codes || matches!(app.format, OutputFormat::Sarif) {
Ok(ExitCode::SUCCESS)
} else {
Ok(results.into())

View file

@ -1,7 +1,7 @@
//! Enriching/context-bearing wrappers over GitHub Actions models
//! from the `github-actions-models` crate.
use std::{collections::hash_map, iter::Enumerate, ops::Deref, path::Path};
use std::{iter::Enumerate, ops::Deref, path::Path};
use crate::finding::{Route, SymbolicLocation};
use anyhow::{anyhow, Context, Result};
@ -177,7 +177,7 @@ impl<'w> Job<'w> {
/// An iterable container for jobs within a [`Workflow`].
pub(crate) struct Jobs<'w> {
parent: &'w Workflow,
inner: hash_map::Iter<'w, String, workflow::Job>,
inner: indexmap::map::Iter<'w, String, workflow::Job>,
}
impl<'w> Jobs<'w> {

View file

@ -6,7 +6,7 @@ use std::{collections::HashMap, path::Path, process::ExitCode};
use crate::{
audit::WorkflowAudit,
config::Config,
finding::{Confidence, Finding, Severity},
finding::{Confidence, Finding, Persona, Severity},
models::Workflow,
App,
};
@ -114,6 +114,8 @@ pub(crate) struct FindingRegistry<'a> {
config: &'a Config,
minimum_severity: Option<Severity>,
minimum_confidence: Option<Confidence>,
persona: Persona,
suppressed: Vec<Finding<'a>>,
ignored: Vec<Finding<'a>>,
findings: Vec<Finding<'a>>,
highest_seen_severity: Option<Severity>,
@ -125,6 +127,8 @@ impl<'a> FindingRegistry<'a> {
config,
minimum_severity: app.min_severity,
minimum_confidence: app.min_confidence,
persona: app.persona,
suppressed: Default::default(),
ignored: Default::default(),
findings: Default::default(),
highest_seen_severity: None,
@ -137,7 +141,9 @@ impl<'a> FindingRegistry<'a> {
// TODO: is it faster to iterate like this, or do `find_by_max`
// and then `extend`?
for finding in results {
if finding.ignored
if self.persona > finding.determinations.persona {
self.suppressed.push(finding);
} else if finding.ignored
|| self
.minimum_severity
.map_or(false, |min| min > finding.determinations.severity)
@ -160,15 +166,25 @@ impl<'a> FindingRegistry<'a> {
}
}
/// All non-filtered findings.
/// The total count of all findings, regardless of status.
pub(crate) fn count(&self) -> usize {
self.findings.len() + self.ignored.len() + self.suppressed.len()
}
/// All non-ignored and non-suppressed findings.
pub(crate) fn findings(&self) -> &[Finding<'a>] {
&self.findings
}
/// All filtered findings.
/// All ignored findings.
pub(crate) fn ignored(&self) -> &[Finding<'a>] {
&self.ignored
}
/// All persona-suppressed findings.
pub(crate) fn suppressed(&self) -> &[Finding<'a>] {
&self.suppressed
}
}
impl From<FindingRegistry<'_>> for ExitCode {

View file

@ -72,14 +72,28 @@ pub(crate) fn render_findings(registry: &WorkflowRegistry, findings: &FindingReg
println!();
}
let mut qualifiers = vec![];
if !findings.ignored().is_empty() {
qualifiers.push(format!(
"{nignored} ignored",
nignored = findings.ignored().len().bright_yellow()
));
}
if !findings.suppressed().is_empty() {
qualifiers.push(format!(
"{nsuppressed} suppressed",
nsuppressed = findings.suppressed().len().bright_yellow()
));
}
if findings.findings().is_empty() {
if findings.ignored().is_empty() {
if qualifiers.is_empty() {
println!("{}", "No findings to report. Good job!".green());
} else {
println!(
"{no_findings} ({nignored} ignored)",
"{no_findings} ({qualifiers})",
no_findings = "No findings to report. Good job!".green(),
nignored = findings.ignored().len().bright_yellow()
qualifiers = qualifiers.join(", ").bold(),
);
}
} else {
@ -96,8 +110,8 @@ pub(crate) fn render_findings(registry: &WorkflowRegistry, findings: &FindingReg
}
}
if findings.ignored().is_empty() {
let nfindings = findings.findings().len();
if qualifiers.is_empty() {
let nfindings = findings.count();
print!(
"{nfindings} finding{s}: ",
nfindings = nfindings.green(),
@ -105,9 +119,9 @@ pub(crate) fn render_findings(registry: &WorkflowRegistry, findings: &FindingReg
);
} else {
print!(
"{nfindings} findings ({nignored} ignored): ",
nfindings = (findings.findings().len() + findings.ignored().len()).green(),
nignored = findings.ignored().len().bright_yellow()
"{nfindings} findings ({qualifiers}): ",
nfindings = findings.count().green(),
qualifiers = qualifiers.join(", ").bold(),
);
}

View file

@ -9,7 +9,6 @@ use crate::{
#[derive(Clone)]
pub(crate) struct AuditState {
pub(crate) pedantic: bool,
pub(crate) offline: bool,
pub(crate) gh_token: Option<String>,
pub(crate) caches: Caches,
@ -19,7 +18,6 @@ impl AuditState {
pub(crate) fn new(app: &App) -> Self {
Self {
caches: Caches::new(),
pedantic: app.pedantic,
offline: app.offline,
gh_token: app.gh_token.clone(),
}

View file

@ -152,8 +152,8 @@ fn audit_use_trusted_publishing() -> anyhow::Result<()> {
fn audit_self_hosted() -> anyhow::Result<()> {
let auditable = workflow_under_test("self-hosted.yml");
// Note : self-hosted audit is pedantic
let cli_args = ["--pedantic", &auditable];
// Note: self-hosted audit is auditor-only
let cli_args = ["--persona=auditor", &auditable];
let execution = zizmor().args(cli_args).output()?;

View file

@ -70,11 +70,25 @@ pub(crate) fn zizmor() -> Zizmor {
Zizmor::new()
}
#[test]
fn artipacked() -> Result<()> {
insta::assert_snapshot!(zizmor()
.workflow(workflow_under_test("artipacked.yml"))
.args(["--persona=pedantic"])
.run()?);
insta::assert_snapshot!(zizmor()
.workflow(workflow_under_test("artipacked.yml"))
.run()?);
Ok(())
}
#[test]
fn self_hosted() -> Result<()> {
insta::assert_snapshot!(zizmor()
.workflow(workflow_under_test("self-hosted.yml"))
.args(["--pedantic"])
.args(["--persona=auditor"])
.run()?);
insta::assert_snapshot!(zizmor()

View file

@ -0,0 +1,14 @@
---
source: tests/snapshot.rs
expression: "zizmor().workflow(workflow_under_test(\"artipacked.yml\")).run()?"
snapshot_kind: text
---
warning[artipacked]: credential persistence through GitHub Actions artifacts
--> @@INPUT@@:13:9
|
13 | - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
| ---------------------------------------------------------------------------- does not set persist-credentials: false
|
= note: audit confidence → Low
2 findings (1 suppressed): 0 unknown, 0 informational, 0 low, 1 medium, 0 high

View file

@ -0,0 +1,25 @@
---
source: tests/snapshot.rs
expression: "zizmor().workflow(workflow_under_test(\"artipacked.yml\")).args([\"--persona=pedantic\"]).run()?"
snapshot_kind: text
---
warning[artipacked]: credential persistence through GitHub Actions artifacts
--> @@INPUT@@:13:9
|
13 | - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
| ---------------------------------------------------------------------------- does not set persist-credentials: false
|
= note: audit confidence → Low
warning[artipacked]: credential persistence through GitHub Actions artifacts
--> @@INPUT@@:18:9
|
18 | - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
| _________-
19 | | with:
20 | | persist-credentials: true
| |____________________________________- does not set persist-credentials: false
|
= note: audit confidence → Low
2 findings: 0 unknown, 0 informational, 0 low, 2 medium, 0 high

View file

@ -1,6 +1,6 @@
---
source: tests/snapshot.rs
expression: "zizmor(&workflow_under_test(\"self-hosted.yml\"), &[], false)?"
expression: "zizmor().workflow(workflow_under_test(\"self-hosted.yml\")).run()?"
snapshot_kind: text
---
No findings to report. Good job!
No findings to report. Good job! (1 suppressed)

View file

@ -1,6 +1,6 @@
---
source: tests/snapshot.rs
expression: "zizmor(Some(&workflow_under_test(\"unpinned-uses.yml\")), &[], false)?"
expression: "zizmor().workflow(workflow_under_test(\"unpinned-uses.yml\")).run()?"
snapshot_kind: text
---
warning[unpinned-uses]: unpinned action reference
@ -35,4 +35,4 @@ warning[unpinned-uses]: unpinned action reference
|
= note: audit confidence → High
4 findings: 0 unknown, 0 informational, 0 low, 4 medium, 0 high
5 findings (1 suppressed): 0 unknown, 0 informational, 0 low, 4 medium, 0 high

View file

@ -11,3 +11,10 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
pedantic:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # tag=v4.2.2
with:
persist-credentials: true