Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: configuration file support #116

Merged
merged 6 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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