1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
use super::{Audit, AuditLoadError, Job, audit_meta};
use crate::finding::location::Locatable as _;
use crate::{
audit::AuditError,
config::Config,
finding::{Confidence, Finding, Persona, Severity},
models::workflow::Workflow,
state::AuditState,
};
use github_actions_models::workflow::Concurrency;
pub(crate) struct ConcurrencyLimits;
audit_meta!(
ConcurrencyLimits,
"concurrency-limits",
"insufficient job-level concurrency limits"
);
#[async_trait::async_trait]
impl Audit for ConcurrencyLimits {
fn new(_state: &AuditState) -> Result<Self, AuditLoadError> {
Ok(Self)
}
async fn audit_workflow<'doc>(
&self,
workflow: &'doc Workflow,
_config: &Config,
) -> Result<Vec<Finding<'doc>>, AuditError> {
let mut findings = vec![];
if workflow.is_reusable_only() {
// If a workflow is reusable-only, then we expect its calling workflow
// to manage its concurrency settings. Attempting to manage concurrency
// in the called workflow results in weird bugs like deadlocks and
// premature cancellations.
// See: <https://github.com/zizmorcore/zizmor/issues/1511>
// See: <https://github.com/orgs/community/discussions/30708>
return Ok(findings);
}
match &workflow.concurrency {
Some(Concurrency::Bare(_)) => {
findings.push(
Self::finding()
.confidence(Confidence::High)
.severity(Severity::Low)
.persona(Persona::Pedantic)
.add_location(
workflow
.location()
.primary()
.with_keys(["concurrency".into()])
.annotated("workflow concurrency is missing cancel-in-progress"),
)
.build(workflow)?,
);
}
None => {
for job in workflow.jobs() {
let Job::NormalJob(job) = job else {
continue;
};
match &job.concurrency {
Some(Concurrency::Bare(_)) => {
findings.push(
Self::finding()
.confidence(Confidence::High)
.severity(Severity::Low)
.persona(Persona::Pedantic)
.add_location(
job.location()
.primary()
.with_keys(["concurrency".into()])
.annotated(
"job concurrency is missing cancel-in-progress",
),
)
.build(workflow)?,
);
}
None => {
findings.push(
Self::finding()
.confidence(Confidence::High)
.severity(Severity::Low)
.persona(Persona::Pedantic)
.add_location(
workflow
.location()
.primary()
.annotated("missing concurrency setting"),
)
.build(workflow)?,
);
}
// NOTE: Per #1302, we don't nag the user if they've explicitly set
// `cancel-in-progress: false` or similar. This is like with the
// artipacked audit, where `persist-credentials: true` is seen as
// a positive signal of user intent.
_ => {}
}
}
}
// NOTE: Per #1302, we don't nag the user if they've explicitly set
// `cancel-in-progress: false` or similar. This is like with the
// artipacked audit, where `persist-credentials: true` is seen as
// a positive signal of user intent.
_ => {}
}
Ok(findings)
}
}