Skip to content

Commit

Permalink
fix: excessive-permissions: be less noisy on one-job workflows (#337)
Browse files Browse the repository at this point in the history
  • Loading branch information
woodruffw authored Dec 19, 2024
1 parent 53ccaec commit 82bbdb4
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 1 deletion.
14 changes: 13 additions & 1 deletion src/audit/excessive_permissions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use github_actions_models::{

use super::{audit_meta, WorkflowAudit};
use crate::{
finding::{Confidence, Severity},
finding::{Confidence, Persona, Severity},
AuditState,
};

Expand Down Expand Up @@ -56,12 +56,24 @@ impl WorkflowAudit for ExcessivePermissions {
workflow: &'w crate::models::Workflow,
) -> anyhow::Result<Vec<crate::finding::Finding<'w>>> {
let mut findings = vec![];

// Top-level permissions are a minor issue if there's only one
// job in the workflow, since they're equivalent to job-level
// permissions in that case. Emit only pedantic findings in
// that case.
let persona = if workflow.jobs.len() == 1 {
Persona::Pedantic
} else {
Persona::Regular
};

// Top-level permissions.
for (severity, confidence, note) in self.check_permissions(&workflow.permissions, None) {
findings.push(
Self::finding()
.severity(severity)
.confidence(confidence)
.persona(persona)
.add_location(
workflow
.location()
Expand Down
18 changes: 18 additions & 0 deletions tests/snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -316,3 +316,21 @@ fn cache_poisoning() -> Result<()> {

Ok(())
}

#[test]
fn excessive_permissions() -> Result<()> {
insta::assert_snapshot!(zizmor()
.workflow(workflow_under_test(
"excessive-permissions/issue-336-repro.yml"
))
.run()?);

insta::assert_snapshot!(zizmor()
.workflow(workflow_under_test(
"excessive-permissions/issue-336-repro.yml"
))
.args(["--pedantic"])
.run()?);

Ok(())
}
15 changes: 15 additions & 0 deletions tests/snapshots/snapshot__excessive_permissions-2.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: tests/snapshot.rs
expression: "zizmor().workflow(workflow_under_test(\"excessive-permissions/issue-336-repro.yml\")).args([\"--pedantic\"]).run()?"
snapshot_kind: text
---
error[excessive-permissions]: overly broad workflow or job-level permissions
--> @@INPUT@@:3:1
|
3 | / permissions:
4 | | contents: write
| |_________________^ contents: write is overly broad at the workflow level
|
= note: audit confidenceHigh

1 finding: 0 unknown, 0 informational, 0 low, 0 medium, 1 high
6 changes: 6 additions & 0 deletions tests/snapshots/snapshot__excessive_permissions.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: tests/snapshot.rs
expression: "zizmor().workflow(workflow_under_test(\"excessive-permissions/issue-336-repro.yml\")).run()?"
snapshot_kind: text
---
No findings to report. Good job! (1 suppressed)
12 changes: 12 additions & 0 deletions tests/test-data/excessive-permissions/issue-336-repro.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
on: push

permissions:
contents: write

jobs:
single:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683
with:
persist-credentials: false

0 comments on commit 82bbdb4

Please sign in to comment.