mirror of
https://github.com/zizmorcore/zizmor.git
synced 2025-12-23 08:47:33 +00:00
Introduce a rule which suggests that permissions are documented (#1131)
Co-authored-by: William Woodruff <william@yossarian.net>
This commit is contained in:
John Blackbourn
GitHub
parent
4a92dfc412
commit
e0ec65a187
29 changed files with 633 additions and 9 deletions
|
|
@ -36,6 +36,7 @@ pub(crate) mod secrets_inherit;
|
|||
pub(crate) mod self_hosted_runner;
|
||||
pub(crate) mod stale_action_refs;
|
||||
pub(crate) mod template_injection;
|
||||
pub(crate) mod undocumented_permissions;
|
||||
pub(crate) mod unpinned_images;
|
||||
pub(crate) mod unpinned_uses;
|
||||
pub(crate) mod unredacted_secrets;
|
||||
|
|
|
|||
129
crates/zizmor/src/audit/undocumented_permissions.rs
Normal file
129
crates/zizmor/src/audit/undocumented_permissions.rs
Normal file
|
|
@ -0,0 +1,129 @@
|
|||
use github_actions_models::common::{Permission, Permissions};
|
||||
|
||||
use super::{Audit, AuditLoadError, Job, audit_meta};
|
||||
use crate::finding::location::Locatable as _;
|
||||
use crate::models::AsDocument;
|
||||
use crate::{
|
||||
AuditState,
|
||||
finding::{Confidence, Persona, Severity, location::SymbolicLocation},
|
||||
};
|
||||
|
||||
audit_meta!(
|
||||
UndocumentedPermissions,
|
||||
"undocumented-permissions",
|
||||
"permissions without explanatory comments"
|
||||
);
|
||||
|
||||
pub(crate) struct UndocumentedPermissions;
|
||||
|
||||
impl Audit for UndocumentedPermissions {
|
||||
fn new(_state: &AuditState) -> Result<Self, AuditLoadError>
|
||||
where
|
||||
Self: Sized,
|
||||
{
|
||||
Ok(Self)
|
||||
}
|
||||
|
||||
fn audit_workflow<'doc>(
|
||||
&self,
|
||||
workflow: &'doc crate::models::workflow::Workflow,
|
||||
_config: &crate::config::Config,
|
||||
) -> anyhow::Result<Vec<crate::finding::Finding<'doc>>> {
|
||||
let mut findings = vec![];
|
||||
|
||||
// Check workflow-level permissions
|
||||
if let Some(finding) = self.check_permissions_documentation(
|
||||
&workflow.permissions,
|
||||
workflow.location(),
|
||||
workflow,
|
||||
)? {
|
||||
findings.push(finding);
|
||||
}
|
||||
|
||||
// Check job-level permissions
|
||||
for job in workflow.jobs() {
|
||||
let (permissions, job_location) = match job {
|
||||
Job::NormalJob(job) => (&job.permissions, job.location()),
|
||||
Job::ReusableWorkflowCallJob(job) => (&job.permissions, job.location()),
|
||||
};
|
||||
|
||||
if let Some(finding) =
|
||||
self.check_permissions_documentation(permissions, job_location, workflow)?
|
||||
{
|
||||
findings.push(finding);
|
||||
}
|
||||
}
|
||||
|
||||
Ok(findings)
|
||||
}
|
||||
}
|
||||
|
||||
impl UndocumentedPermissions {
|
||||
fn check_permissions_documentation<'a>(
|
||||
&self,
|
||||
permissions: &'a Permissions,
|
||||
location: SymbolicLocation<'a>,
|
||||
workflow: &'a crate::models::workflow::Workflow,
|
||||
) -> anyhow::Result<Option<crate::finding::Finding<'a>>> {
|
||||
// Only check explicit permissions blocks
|
||||
let Permissions::Explicit(perms) = permissions else {
|
||||
return Ok(None);
|
||||
};
|
||||
|
||||
if perms.is_empty() {
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
// Check each individual permission for documentation
|
||||
let mut finding_builder = Self::finding()
|
||||
.severity(Severity::Low)
|
||||
.confidence(Confidence::High)
|
||||
.persona(Persona::Pedantic);
|
||||
|
||||
let base_location = location.clone().primary();
|
||||
let mut has_undocumented = false;
|
||||
|
||||
for (perm_name, perm) in perms {
|
||||
if perm_name == "contents" && *perm == Permission::Read {
|
||||
// Skip "contents: read" as it's common and self-explanatory
|
||||
continue;
|
||||
}
|
||||
|
||||
let individual_perm_location = base_location
|
||||
.clone()
|
||||
.with_keys(["permissions".into(), perm_name.as_str().into()]);
|
||||
|
||||
if !self.has_explanatory_comment(&individual_perm_location, workflow)? {
|
||||
finding_builder = finding_builder.add_location(
|
||||
individual_perm_location.annotated("needs an explanatory comment"),
|
||||
);
|
||||
has_undocumented = true;
|
||||
}
|
||||
}
|
||||
|
||||
// Only create a finding if there are actually undocumented permissions
|
||||
if has_undocumented {
|
||||
Ok(Some(finding_builder.build(workflow)?))
|
||||
} else {
|
||||
Ok(None)
|
||||
}
|
||||
}
|
||||
|
||||
fn has_explanatory_comment(
|
||||
&self,
|
||||
location: &SymbolicLocation,
|
||||
workflow: &crate::models::workflow::Workflow,
|
||||
) -> anyhow::Result<bool> {
|
||||
let document = workflow.as_document();
|
||||
|
||||
// Use the concretize API to get a Location with concrete Feature
|
||||
let concrete_location = location.clone().concretize(document)?;
|
||||
|
||||
// Check if there are any meaningful comments
|
||||
Ok(concrete_location
|
||||
.concrete
|
||||
.comments
|
||||
.iter()
|
||||
.any(|comment| comment.is_meaningful()))
|
||||
}
|
||||
}
|
||||
|
|
@ -304,6 +304,11 @@ static IGNORE_EXPR: LazyLock<Regex> =
|
|||
pub(crate) struct Comment<'doc>(&'doc str);
|
||||
|
||||
impl Comment<'_> {
|
||||
pub(crate) fn is_meaningful(&self) -> bool {
|
||||
let content = self.0.strip_prefix('#').unwrap_or(self.0).trim();
|
||||
!content.is_empty()
|
||||
}
|
||||
|
||||
pub(crate) fn ignores(&self, rule_id: &str) -> bool {
|
||||
// Extracts foo,bar from `# zizmor: ignore[foo,bar]`
|
||||
let Some(caps) = IGNORE_EXPR.captures(self.0) else {
|
||||
|
|
|
|||
|
|
@ -63,6 +63,7 @@ impl AuditRegistry {
|
|||
register_audit!(audit::self_hosted_runner::SelfHostedRunner);
|
||||
register_audit!(audit::known_vulnerable_actions::KnownVulnerableActions);
|
||||
register_audit!(audit::unpinned_uses::UnpinnedUses);
|
||||
register_audit!(audit::undocumented_permissions::UndocumentedPermissions);
|
||||
register_audit!(audit::insecure_commands::InsecureCommands);
|
||||
register_audit!(audit::github_env::GitHubEnv);
|
||||
register_audit!(audit::cache_poisoning::CachePoisoning);
|
||||
|
|
|
|||
|
|
@ -812,6 +812,74 @@ fn unpinned_images() -> Result<()> {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn undocumented_permissions() -> Result<()> {
|
||||
// Test with pedantic persona (should find issues)
|
||||
insta::assert_snapshot!(
|
||||
zizmor()
|
||||
.input(input_under_test("undocumented-permissions.yml"))
|
||||
.args(["--persona=pedantic"])
|
||||
.run()?
|
||||
);
|
||||
|
||||
// Test with regular persona (should not find issues)
|
||||
insta::assert_snapshot!(
|
||||
zizmor()
|
||||
.input(input_under_test("undocumented-permissions.yml"))
|
||||
.run()?
|
||||
);
|
||||
|
||||
// Test with properly documented permissions (should not find issues even with pedantic)
|
||||
insta::assert_snapshot!(
|
||||
zizmor()
|
||||
.input(input_under_test("undocumented-permissions/documented.yml"))
|
||||
.args(["--persona=pedantic"])
|
||||
.run()?
|
||||
);
|
||||
|
||||
// Test with only "contents: read" (should not trigger rule)
|
||||
insta::assert_snapshot!(
|
||||
zizmor()
|
||||
.input(input_under_test(
|
||||
"undocumented-permissions/contents-read-only.yml"
|
||||
))
|
||||
.args(["--persona=pedantic"])
|
||||
.run()?
|
||||
);
|
||||
|
||||
// Test with empty permissions (should not trigger rule)
|
||||
insta::assert_snapshot!(
|
||||
zizmor()
|
||||
.input(input_under_test(
|
||||
"undocumented-permissions/empty-permissions.yml"
|
||||
))
|
||||
.args(["--persona=pedantic"])
|
||||
.run()?
|
||||
);
|
||||
|
||||
// Test with contents: read plus other permissions (should trigger rule)
|
||||
insta::assert_snapshot!(
|
||||
zizmor()
|
||||
.input(input_under_test(
|
||||
"undocumented-permissions/contents-read-with-other.yml"
|
||||
))
|
||||
.args(["--persona=pedantic"])
|
||||
.run()?
|
||||
);
|
||||
|
||||
// Test with partially documented permissions (should ideally only flag undocumented ones)
|
||||
insta::assert_snapshot!(
|
||||
zizmor()
|
||||
.input(input_under_test(
|
||||
"undocumented-permissions/partially-documented.yml"
|
||||
))
|
||||
.args(["--persona=pedantic"])
|
||||
.run()?
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn unsound_condition() -> Result<()> {
|
||||
insta::assert_snapshot!(
|
||||
|
|
|
|||
|
|
@ -998,4 +998,4 @@ error[dangerous-triggers]: use of fundamentally insecure workflow trigger
|
|||
|
|
||||
= note: audit confidence → Medium
|
||||
|
||||
160 findings (74 suppressed): 0 unknown, 7 informational, 0 low, 29 medium, 50 high
|
||||
162 findings (76 suppressed): 0 unknown, 7 informational, 0 low, 29 medium, 50 high
|
||||
|
|
|
|||
|
|
@ -171,4 +171,4 @@ error[unpinned-uses]: unpinned action reference
|
|||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
78 findings (1 ignored, 59 suppressed): 0 unknown, 0 informational, 1 low, 0 medium, 17 high
|
||||
85 findings (1 ignored, 66 suppressed): 0 unknown, 0 informational, 1 low, 0 medium, 17 high
|
||||
|
|
|
|||
|
|
@ -53,4 +53,4 @@ error[cache-poisoning]: runtime artifacts potentially vulnerable to a cache pois
|
|||
= note: audit confidence → Low
|
||||
= note: this finding has an auto-fix
|
||||
|
||||
3 findings (3 fixable): 0 unknown, 0 informational, 0 low, 0 medium, 3 high
|
||||
4 findings (1 suppressed, 3 fixable): 0 unknown, 0 informational, 0 low, 0 medium, 3 high
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ expression: "zizmor().input(input_under_test(\"excessive-permissions/issue-336-r
|
|||
error[excessive-permissions]: overly broad permissions
|
||||
--> @@INPUT@@:6:3
|
||||
|
|
||||
6 | contents: write
|
||||
6 | contents: write # Needed for the workflow
|
||||
| ^^^^^^^^^^^^^^^ contents: write is overly broad at the workflow level
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
|
|
|||
|
|
@ -26,4 +26,4 @@ error[excessive-permissions]: overly broad permissions
|
|||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
3 findings: 1 unknown, 0 informational, 0 low, 0 medium, 2 high
|
||||
5 findings (2 suppressed): 1 unknown, 0 informational, 0 low, 0 medium, 2 high
|
||||
|
|
|
|||
|
|
@ -2,4 +2,4 @@
|
|||
source: tests/integration/snapshot.rs
|
||||
expression: "zizmor().input(input_under_test(\"excessive-permissions/workflow-default-perms-all-jobs-explicit.yml\")).run()?"
|
||||
---
|
||||
No findings to report. Good job! (1 suppressed)
|
||||
No findings to report. Good job! (3 suppressed)
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
---
|
||||
source: tests/integration/snapshot.rs
|
||||
source: crates/zizmor/tests/integration/snapshot.rs
|
||||
expression: "zizmor().input(input_under_test(\"excessive-permissions/issue-336-repro.yml\")).run()?"
|
||||
---
|
||||
No findings to report. Good job! (1 suppressed)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
source: crates/zizmor/tests/integration/snapshot.rs
|
||||
expression: "zizmor().input(input_under_test(\"undocumented-permissions.yml\")).run()?"
|
||||
---
|
||||
No findings to report. Good job! (5 suppressed)
|
||||
|
|
@ -0,0 +1,13 @@
|
|||
---
|
||||
source: crates/zizmor/tests/integration/snapshot.rs
|
||||
expression: "zizmor().input(input_under_test(\"undocumented-permissions/documented.yml\")).args([\"--persona=pedantic\"]).run()?"
|
||||
---
|
||||
error[excessive-permissions]: overly broad permissions
|
||||
--> @@INPUT@@:8:3
|
||||
|
|
||||
8 | packages: write # Needed to publish packages
|
||||
| ^^^^^^^^^^^^^^^ packages: write is overly broad at the workflow level
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
1 finding: 0 unknown, 0 informational, 0 low, 0 medium, 1 high
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
source: crates/zizmor/tests/integration/snapshot.rs
|
||||
expression: "zizmor().input(input_under_test(\"undocumented-permissions/contents-read-only.yml\")).args([\"--persona=pedantic\"]).run()?"
|
||||
---
|
||||
No findings to report. Good job!
|
||||
|
|
@ -0,0 +1,5 @@
|
|||
---
|
||||
source: crates/zizmor/tests/integration/snapshot.rs
|
||||
expression: "zizmor().input(input_under_test(\"undocumented-permissions/empty-permissions.yml\")).args([\"--persona=pedantic\"]).run()?"
|
||||
---
|
||||
No findings to report. Good job!
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
---
|
||||
source: crates/zizmor/tests/integration/snapshot.rs
|
||||
expression: "zizmor().input(input_under_test(\"undocumented-permissions/contents-read-with-other.yml\")).args([\"--persona=pedantic\"]).run()?"
|
||||
---
|
||||
error[excessive-permissions]: overly broad permissions
|
||||
--> @@INPUT@@:8:3
|
||||
|
|
||||
8 | issues: write
|
||||
| ^^^^^^^^^^^^^ issues: write is overly broad at the workflow level
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
help[undocumented-permissions]: permissions without explanatory comments
|
||||
--> @@INPUT@@:8:3
|
||||
|
|
||||
8 | issues: write
|
||||
| ^^^^^^^^^^^^^ needs an explanatory comment
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
2 findings: 0 unknown, 0 informational, 1 low, 0 medium, 1 high
|
||||
|
|
@ -0,0 +1,47 @@
|
|||
---
|
||||
source: crates/zizmor/tests/integration/snapshot.rs
|
||||
expression: "zizmor().input(input_under_test(\"undocumented-permissions/partially-documented.yml\")).args([\"--persona=pedantic\"]).run()?"
|
||||
---
|
||||
error[excessive-permissions]: overly broad permissions
|
||||
--> @@INPUT@@:7:3
|
||||
|
|
||||
7 | contents: write # Needed to create releases
|
||||
| ^^^^^^^^^^^^^^^ contents: write is overly broad at the workflow level
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
error[excessive-permissions]: overly broad permissions
|
||||
--> @@INPUT@@:9:3
|
||||
|
|
||||
9 | issues: write # Needed to update issue comments
|
||||
| ^^^^^^^^^^^^^ issues: write is overly broad at the workflow level
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
help[undocumented-permissions]: permissions without explanatory comments
|
||||
--> @@INPUT@@:8:3
|
||||
|
|
||||
8 | packages: read
|
||||
| ^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
help[undocumented-permissions]: permissions without explanatory comments
|
||||
--> @@INPUT@@:18:7
|
||||
|
|
||||
18 | actions: write
|
||||
| ^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
help[undocumented-permissions]: permissions without explanatory comments
|
||||
--> @@INPUT@@:31:7
|
||||
|
|
||||
31 | contents: write
|
||||
| ^^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
32 | issues: read
|
||||
| ^^^^^^^^^^^^ needs an explanatory comment
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
5 findings: 0 unknown, 0 informational, 3 low, 0 medium, 2 high
|
||||
|
|
@ -0,0 +1,55 @@
|
|||
---
|
||||
source: crates/zizmor/tests/integration/snapshot.rs
|
||||
expression: "zizmor().input(input_under_test(\"undocumented-permissions.yml\")).args([\"--persona=pedantic\"]).run()?"
|
||||
---
|
||||
error[excessive-permissions]: overly broad permissions
|
||||
--> @@INPUT@@:7:3
|
||||
|
|
||||
7 | contents: write
|
||||
| ^^^^^^^^^^^^^^^ contents: write is overly broad at the workflow level
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
help[undocumented-permissions]: permissions without explanatory comments
|
||||
--> @@INPUT@@:7:3
|
||||
|
|
||||
7 | contents: write
|
||||
| ^^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
8 | packages: read
|
||||
| ^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
help[undocumented-permissions]: permissions without explanatory comments
|
||||
--> @@INPUT@@:29:7
|
||||
|
|
||||
29 | contents: write
|
||||
| ^^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
30 | packages: write
|
||||
| ^^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
31 | actions: write
|
||||
| ^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
help[undocumented-permissions]: permissions without explanatory comments
|
||||
--> @@INPUT@@:44:7
|
||||
|
|
||||
44 | packages: write #
|
||||
| ^^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
45 | actions: write #
|
||||
| ^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
help[undocumented-permissions]: permissions without explanatory comments
|
||||
--> @@INPUT@@:56:7
|
||||
|
|
||||
56 | metadata: read
|
||||
| ^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
57 | packages: write #
|
||||
| ^^^^^^^^^^^^^^^ needs an explanatory comment
|
||||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
5 findings: 0 unknown, 0 informational, 4 low, 0 medium, 1 high
|
||||
|
|
@ -90,4 +90,4 @@ info[use-trusted-publishing]: prefer trusted publishing for authentication
|
|||
|
|
||||
= note: audit confidence → High
|
||||
|
||||
25 findings (16 ignored, 1 suppressed): 0 unknown, 8 informational, 0 low, 0 medium, 0 high
|
||||
26 findings (16 ignored, 2 suppressed): 0 unknown, 8 informational, 0 low, 0 medium, 0 high
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ on: push
|
|||
name: issue-336-repro
|
||||
|
||||
permissions:
|
||||
contents: write
|
||||
contents: write # Needed for the workflow
|
||||
|
||||
jobs:
|
||||
single:
|
||||
|
|
|
|||
|
|
@ -0,0 +1,75 @@
|
|||
name: Test Undocumented Permissions
|
||||
|
||||
on: [push, pull_request]
|
||||
|
||||
# This workflow-level permission block has no comment
|
||||
permissions:
|
||||
contents: write
|
||||
packages: read
|
||||
|
||||
jobs:
|
||||
test-job-1:
|
||||
name: Test Job 1
|
||||
runs-on: ubuntu-latest
|
||||
# This job's permissions block has a comment explaining why
|
||||
permissions:
|
||||
contents: read # Needed to read repo contents
|
||||
issues: write # Needed to write issue comments
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
||||
test-job-2:
|
||||
name: Test Job 2
|
||||
runs-on: ubuntu-latest
|
||||
# Missing individual permission comments
|
||||
permissions:
|
||||
contents: write
|
||||
packages: write
|
||||
actions: write
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
||||
test-job-3:
|
||||
name: Test Job 3 - Empty Comments
|
||||
runs-on: ubuntu-latest
|
||||
# Permissions have empty comments
|
||||
permissions:
|
||||
contents: read #
|
||||
packages: write #
|
||||
actions: write #
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
||||
test-job-4:
|
||||
name: Test Job 4 - Mixed Documentation
|
||||
runs-on: ubuntu-latest
|
||||
permissions:
|
||||
metadata: read
|
||||
packages: write #
|
||||
issues: write # Needed to post status updates
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
||||
test-job-5:
|
||||
name: Test Job 5 - read contents
|
||||
runs-on: ubuntu-latest
|
||||
# Only one `contents: read` permission, no comment needed
|
||||
permissions:
|
||||
contents: read
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
name: Contents Read Only Test
|
||||
|
||||
on: [push]
|
||||
|
||||
# This should NOT trigger the rule since contents: read is self-explanatory
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
test-job:
|
||||
name: Test Job
|
||||
runs-on: ubuntu-latest
|
||||
# This should also NOT trigger the rule
|
||||
permissions:
|
||||
contents: read
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
name: Contents Read With Other Permissions Test
|
||||
|
||||
on: [push]
|
||||
|
||||
# This SHOULD trigger the rule since it has more than just contents: read
|
||||
permissions:
|
||||
contents: read
|
||||
issues: write
|
||||
|
||||
jobs:
|
||||
test-job:
|
||||
name: Test Job
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
|
@ -0,0 +1,49 @@
|
|||
name: Test Documented Permissions
|
||||
|
||||
on: [push, pull_request]
|
||||
|
||||
# GitHub token permissions for all jobs in this workflow
|
||||
permissions:
|
||||
contents: read # Needed to fetch code
|
||||
packages: write # Needed to publish packages
|
||||
|
||||
jobs:
|
||||
test-job-1:
|
||||
name: Test Job 1
|
||||
runs-on: ubuntu-latest
|
||||
# Override workflow permissions for this specific job
|
||||
permissions:
|
||||
contents: write # Needed to create releases
|
||||
issues: write # Needed to create and update issues
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
||||
test-job-2:
|
||||
name: Test Job 2
|
||||
runs-on: ubuntu-latest
|
||||
# Specific permissions documented for admin operations
|
||||
permissions:
|
||||
contents: write # Needed for admin operations
|
||||
packages: write # Needed to publish packages
|
||||
actions: write # Needed to manage workflow runs
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
||||
test-job-3:
|
||||
name: Test Job 3
|
||||
runs-on: ubuntu-latest
|
||||
# Specific permissions documented for analysis job
|
||||
permissions:
|
||||
contents: read # Needed for analysis job
|
||||
metadata: read # Needed to read repository metadata
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
name: Empty Permissions Test
|
||||
|
||||
on: [push]
|
||||
|
||||
# This should NOT trigger the rule since there are no permissions to document
|
||||
permissions: {}
|
||||
|
||||
jobs:
|
||||
test-job:
|
||||
name: Test Job
|
||||
runs-on: ubuntu-latest
|
||||
# This should also NOT trigger the rule
|
||||
permissions: {}
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
|
@ -0,0 +1,37 @@
|
|||
name: Partially Documented Permissions Test
|
||||
|
||||
on: [push]
|
||||
|
||||
# Mixed documentation - some permissions have comments, others don't
|
||||
permissions:
|
||||
contents: write # Needed to create releases
|
||||
packages: read
|
||||
issues: write # Needed to update issue comments
|
||||
|
||||
jobs:
|
||||
test-job-1:
|
||||
name: Test Job 1
|
||||
runs-on: ubuntu-latest
|
||||
# Job with partial documentation
|
||||
permissions:
|
||||
contents: read # Needed to read repo contents
|
||||
actions: write
|
||||
packages: write # Needed to publish packages
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
||||
test-job-2:
|
||||
name: Test Job 2
|
||||
runs-on: ubuntu-latest
|
||||
# Job with no documentation at all
|
||||
permissions:
|
||||
contents: write
|
||||
issues: read
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
with:
|
||||
persist-credentials: false
|
||||
- run: echo "Test"
|
||||
|
|
@ -1092,6 +1092,51 @@ shell quoting/expansion rules.
|
|||
ISSUE_TITLE: ${{ github.event.issue.title }}
|
||||
```
|
||||
|
||||
## `undocumented-permissions`
|
||||
|
||||
| Type | Examples | Introduced in | Works offline | Enabled by default | Configurable |
|
||||
|----------|------------------|---------------|----------------|--------------------|--------------|
|
||||
| Workflow | [undocumented-permissions.yml] | v1.13.0 | ✅ | ❌ | ❌ |
|
||||
|
||||
[undocumented-permissions.yml]: https://github.com/zizmorcore/zizmor/blob/main/crates/zizmor/tests/integration/test-data/undocumented-permissions.yml
|
||||
|
||||
Detects explicit permissions blocks that lack explanatory comments.
|
||||
|
||||
This audit recommends adding comments to document the purpose of each permission
|
||||
in explicit permissions blocks. Well-documented permissions help prevent
|
||||
over-scoping and make workflows more maintainable by explaining why specific
|
||||
permissions are needed.
|
||||
|
||||
The audit does not flag `contents: read`, as this is a common, self-explanatory
|
||||
permission.
|
||||
|
||||
!!! note
|
||||
|
||||
This is a `--pedantic` only audit, as it focuses on code quality and
|
||||
maintainability rather than security vulnerabilities.
|
||||
|
||||
### Remediation
|
||||
|
||||
Add inline comments explaining why each permission is needed:
|
||||
|
||||
=== "Before :warning:"
|
||||
|
||||
```yaml title="undocumented-permissions.yml" hl_lines="2-4"
|
||||
permissions:
|
||||
contents: write
|
||||
packages: read
|
||||
issues: write
|
||||
```
|
||||
|
||||
=== "After :white_check_mark:"
|
||||
|
||||
```yaml title="undocumented-permissions.yml" hl_lines="2-4"
|
||||
permissions:
|
||||
contents: write # Needed to create releases and update files
|
||||
packages: read # Needed to read existing package metadata
|
||||
issues: write # Needed to create and update issue comments
|
||||
```
|
||||
|
||||
## `unpinned-images`
|
||||
|
||||
| Type | Examples | Introduced in | Works offline | Enabled by default | Configurable |
|
||||
|
|
|
|||
|
|
@ -9,6 +9,13 @@ of `zizmor`.
|
|||
|
||||
## Next (UNRELEASED)
|
||||
|
||||
### New Features 🌈
|
||||
|
||||
* **New audit**: [undocumented-permissions] detects explicit permission
|
||||
grants that lack an explanatory comment (#1131)
|
||||
|
||||
Many thanks to @johnbillion for proposing and implementing this audit!
|
||||
|
||||
### Enhancements 🌱
|
||||
|
||||
* `zizmor`'s configuration discovery behavior has been significantly refactored,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue