Skip to content

Commit

Permalink
feat: configuration file support (#116)
Browse files Browse the repository at this point in the history
Co-authored-by: Ben Cotton <ben@kusari.dev>
  • Loading branch information
woodruffw and funnelfiasco authored Nov 5, 2024
1 parent 7f53688 commit 801dfb3
Show file tree
Hide file tree
Showing 12 changed files with 359 additions and 46 deletions.
68 changes: 68 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Configuration

!!! note

Configuration support was added in `v0.2.0`.

`zizmor` supports a small amount of configuration via [YAML] config files,
typically named `zizmor.yml`.

[YAML]: https://learnxinyminutes.com/docs/yaml/

## Precedence

`zizmor` will discover and load
configuration files in the following order of precedence:

1. Passed explicitly via `--config`, e.g. `--config my-config.yml`. When passed
explicitly, the config file does *not* need to be named `zizmor.yml`.
1. `${CWD}/.github/zizmor.yml`
1. `${CWD}/zizmor.yml`

For the last two discovery methods, `${CWD}` is the current working directory,
i.e. the directory that `zizmor` was executed from.

Only one configuration file is ever loaded. In other words: if both
`${CWD}/.github/zizmor.yml` and `${CWD}/zizmor.yml` exist, only the former
will be loaded, per the precedence rules above.

## Settings

### `rules`

#### `rules.<id>`

##### `rules.<id>.ignore`

_Type_: `array`

Per-audit ignore rules, where `id` is the audit's name, e.g.
[`template-injection`](./audits.md#template-injection).

Each member of `rules.<id>.ignore` is a *workflow rule*, formatted as follows:

```
filename.yml:<line>?:<column>?
```

where `filename.yml` is the base filename of the workflow, and `line` and
`column` are both optional 1-based values indicating the exact line-and-column
location to ignore. If one or both are absent, then the rule applies to the
entire file or entire line.

By example, here is a configuration file with two different audit ignore
rule groups:

```yaml title="zizmor.yml"
rules:
template-injection:
ignore:
# ignore line 100 in ci.yml, any column
- ci.yml:100
# ignore all lines and columns in tests.yml
- tests.yml
use-trusted-publishing:
ignore:
# ignore line 12, column 10 on pypi.yml
- pypi.yml:12:10
```
43 changes: 43 additions & 0 deletions docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,49 @@ zizmor --format sarif

See [Integration](#integration) for suggestions on when to use each format.

## Ignoring results

`zizmor`'s defaults are not always 100% right for every possible use case.

If you find that `zizmor` produces findings that aren't right for you
(and **aren't** false positives, which should be reported!), then you can
choose to *selectively ignore* results via a `zizmor.yml` configuration file.

Here's what a `zizmor.yml` file might look like:

```yaml title="zizmor.yml"
rules:
template-injection:
ignore:
- safe.yml
- somewhat-safe.yml:123
- one-exact-spot.yml:123:456
```
Concretely, this `zizmor.yml` configuration declares three ignore rules,
all for the [`template-injection`](./audits.md#template-injection) audit:

1. Ignore all findings in `safe.yml`, regardless of line/column location
2. Ignore *any* findings in `somewhat-safe.yml` that occur on line 123
3. Ignore *one* finding in `one-exact-spot.yml` that occurs on line 123, column 456

More generally, the filename ignore syntax is `workflow.yml:line:col`, where
`line` and `col` are both optional and 1-based (meaning `foo.yml:1:1`
is the start of the file, not `foo.yml:0:0`).

To pass a configuration to `zizmor`, you can either place `zizmor.yml`
somewhere where `zizmor` [will discover it], or pass it explicitly via
the `--config` argument. With `--config`, the file can be named anything:

```bash
zizmor --config my-zizmor-config.yml /dir/to/audit
```

[will discover it]: ./configuration.md#discovery

See [Configuration: `rules.<id>.ignore`](./configuration.md#rulesidignore) for
more details on writing ignore rules.

## Integration

### Use in GitHub Actions
Expand Down
1 change: 1 addition & 0 deletions mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ nav:
- "installation.md"
- "quickstart.md"
- "usage.md"
- "configuration.md"
- "audits.md"
- "development.md"
- External links:
Expand Down
2 changes: 1 addition & 1 deletion src/audit/artipacked.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ 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.config.pedantic {
if self.state.pedantic {
vulnerable_checkouts.push(step)
} else {
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/audit/impostor_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl WorkflowAudit for ImpostorCommit {
}

fn new(state: AuditState) -> Result<Self> {
if state.config.offline {
if state.offline {
return Err(anyhow!("offline audits only requested"));
}

Expand Down
2 changes: 1 addition & 1 deletion src/audit/known_vulnerable_actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ impl WorkflowAudit for KnownVulnerableActions {
where
Self: Sized,
{
if state.config.offline {
if state.offline {
return Err(anyhow!("offline audits only requested"));
}

Expand Down
2 changes: 1 addition & 1 deletion src/audit/ref_confusion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl WorkflowAudit for RefConfusion {
where
Self: Sized,
{
if state.config.offline {
if state.offline {
return Err(anyhow!("offline audits only requested"));
}

Expand Down
6 changes: 3 additions & 3 deletions src/audit/self_hosted_runner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use github_actions_models::{
use super::WorkflowAudit;

pub(crate) struct SelfHostedRunner {
pub(crate) _state: AuditState,
pub(crate) state: AuditState,
}

impl WorkflowAudit for SelfHostedRunner {
Expand All @@ -41,7 +41,7 @@ impl WorkflowAudit for SelfHostedRunner {
where
Self: Sized,
{
Ok(Self { _state: state })
Ok(Self { state })
}

fn audit<'w>(
Expand All @@ -50,7 +50,7 @@ impl WorkflowAudit for SelfHostedRunner {
) -> Result<Vec<crate::finding::Finding<'w>>> {
let mut results = vec![];

if !self._state.config.pedantic {
if !self.state.pedantic {
log::info!("skipping self-hosted runner checks");
return Ok(results);
}
Expand Down
199 changes: 199 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
use anyhow::{anyhow, Context as _, Result};
use serde::{de, Deserialize};
use std::{collections::HashMap, fs, num::NonZeroUsize, str::FromStr};

use crate::{finding::Finding, App};

#[derive(Clone, Debug, PartialEq)]
pub(crate) struct WorkflowRule {
/// The workflow filename.
pub(crate) filename: String,
/// The (1-based) line within [`Self::filename`] that the rule occurs on.
pub(crate) line: Option<usize>,
/// The (1-based) column within [`Self::filename`] that the rule occurs on.
pub(crate) column: Option<usize>,
}

impl FromStr for WorkflowRule {
type Err = anyhow::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
// A rule has three parts, delimited by `:`, two of which
// are optional: `foobar.yml:line:col`, where `line` and `col`
// are optional. `col` can only be provided if `line` is provided.
let parts = s.rsplitn(3, ':').collect::<Vec<_>>();
let mut parts = parts.iter().rev();

let filename = parts
.next()
.ok_or_else(|| anyhow!("rule is missing a filename component"))?;

if !filename.ends_with(".yml") && !filename.ends_with(".yaml") {
return Err(anyhow!("invalid workflow filename: {filename}"));
}

let line = parts
.next()
.map(|line| NonZeroUsize::from_str(line).map(|line| line.get()))
.transpose()
.with_context(|| "invalid line number component (must be 1-based)")?;
let column = parts
.next()
.map(|col| NonZeroUsize::from_str(col).map(|col| col.get()))
.transpose()
.with_context(|| "invalid column number component (must be 1-based)")?;

Ok(Self {
filename: filename.to_string(),
line,
column,
})
}
}

impl<'de> Deserialize<'de> for WorkflowRule {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let raw = String::deserialize(deserializer)?;
WorkflowRule::from_str(&raw).map_err(de::Error::custom)
}
}

#[derive(Clone, Debug, Deserialize)]
pub(crate) struct AuditRuleConfig {
ignore: Vec<WorkflowRule>,
}

/// Runtime configuration, corresponding to a `zizmor.yml` file.
#[derive(Clone, Debug, Default, Deserialize)]
pub(crate) struct Config {
rules: HashMap<String, AuditRuleConfig>,
}

impl Config {
pub(crate) fn new(app: &App) -> Result<Self> {
let config = match &app.config {
Some(path) => serde_yaml::from_str(&fs::read_to_string(path)?)?,
None => {
// If the user didn't pass a config path explicitly with
// `--config`, then we attempt to discover one relative to $CWD
// Our procedure is to first look for `$CWD/.github/zizmor.yml`,
// then `$CWD/zizmor.yml`, and then bail.
let cwd = std::env::current_dir()
.with_context(|| "config discovery couldn't access CWD")?;

let path = cwd.join(".github").join("zizmor.yml");
if path.is_file() {
serde_yaml::from_str(&fs::read_to_string(path)?)?
} else {
let path = cwd.join("zizmor.yml");
if path.is_file() {
serde_yaml::from_str(&fs::read_to_string(path)?)?
} else {
log::debug!("no config discovered; loading default");
Config::default()
}
}
}
};

log::debug!("loaded config: {config:?}");

Ok(config)
}

/// Returns `true` if this [`Config`] has an ignore rule for the
/// given finding.
pub(crate) fn ignores(&self, finding: &Finding<'_>) -> bool {
let Some(rule_config) = self.rules.get(finding.ident) else {
return false;
};

let ignores = &rule_config.ignore;

// If *any* location in the finding matches an ignore rule,
// we consider the entire finding ignored.
// This will hopefully minimize confusion when a finding spans
// multiple files, as the first location is the one a user will
// typically ignore, suppressing the rest in the process.
for loc in &finding.locations {
for rule in ignores.iter().filter(|i| i.filename == loc.symbolic.name) {
match rule {
// Rule has a line and (maybe) a column.
WorkflowRule {
line: Some(line),
column,
..
} => {
if *line == loc.concrete.location.start_point.row + 1
&& column.map_or(true, |col| {
col == loc.concrete.location.start_point.column + 1
})
{
return true;
} else {
continue;
}
}
// Rule has no line/col, so we match by virtue of the filename matching.
WorkflowRule {
line: None,
column: None,
..
} => return true,
_ => unreachable!(),
}
}
}

false
}
}

#[cfg(test)]
mod tests {
use std::str::FromStr;

use anyhow::Result;

use super::WorkflowRule;

#[test]
fn test_parse_workflow_rule() -> Result<()> {
assert_eq!(
WorkflowRule::from_str("foo.yml:1:2")?,
WorkflowRule {
filename: "foo.yml".into(),
line: Some(1),
column: Some(2)
}
);

assert_eq!(
WorkflowRule::from_str("foo.yml:123")?,
WorkflowRule {
filename: "foo.yml".into(),
line: Some(123),
column: None
}
);

assert!(WorkflowRule::from_str("foo.yml:0:0").is_err());
assert!(WorkflowRule::from_str("foo.yml:1:0").is_err());
assert!(WorkflowRule::from_str("foo.yml:0:1").is_err());
assert!(WorkflowRule::from_str("foo.yml:123:").is_err());
assert!(WorkflowRule::from_str("foo.yml::").is_err());
assert!(WorkflowRule::from_str("foo.yml::1").is_err());
assert!(WorkflowRule::from_str("foo::1").is_err());
assert!(WorkflowRule::from_str("foo.unrelated::1").is_err());
// TODO: worth dealing with?
// assert!(WorkflowRule::from_str(".yml:1:1").is_err());
assert!(WorkflowRule::from_str("::1").is_err());
assert!(WorkflowRule::from_str(":1:1").is_err());
assert!(WorkflowRule::from_str("1:1").is_err());

Ok(())
}
}
Loading

0 comments on commit 801dfb3

Please sign in to comment.