Skip to content

Commit

Permalink
feat: add personas (woodruffw#226)
Browse files Browse the repository at this point in the history
  • Loading branch information
woodruffw authored Dec 3, 2024
1 parent 8c8fecb commit 7644555
Show file tree
Hide file tree
Showing 23 changed files with 383 additions and 108 deletions.
11 changes: 7 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
49 changes: 42 additions & 7 deletions docs/snippets/help.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
88 changes: 88 additions & 0 deletions docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
24 changes: 10 additions & 14 deletions src/audit/artipacked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>>> {
Expand Down Expand Up @@ -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 {
Expand All @@ -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()
Expand All @@ -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())
{
Expand All @@ -128,6 +123,7 @@ impl WorkflowAudit for Artipacked {
Self::finding()
.severity(Severity::High)
.confidence(Confidence::High)
.persona(persona)
.add_location(
checkout
.location()
Expand Down
1 change: 0 additions & 1 deletion src/audit/github_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
21 changes: 9 additions & 12 deletions src/audit/self_hosted_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand All @@ -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,
Expand All @@ -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>(
Expand All @@ -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;
Expand All @@ -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()])
Expand All @@ -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",
Expand All @@ -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()])
Expand All @@ -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()])
Expand Down
Loading

0 comments on commit 7644555

Please sign in to comment.