Add Fix for known-vulnerable-actions audit rule (#1019)

Co-authored-by: William Woodruff <william@yossarian.net>
This commit is contained in:
Mostafa Moradian 2025-07-21 03:08:14 +02:00 committed by GitHub
parent d01fd3ab3f
commit 558bec2669
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
39 changed files with 1142 additions and 561 deletions

9
Cargo.lock generated
View file

@ -840,7 +840,7 @@ dependencies = [
[[package]]
name = "github-actions-models"
version = "0.31.0"
version = "0.32.0"
dependencies = [
"indexmap",
"serde",
@ -2503,7 +2503,7 @@ dependencies = [
[[package]]
name = "subfeature"
version = "0.0.2"
version = "0.0.3"
dependencies = [
"memchr",
"regex",
@ -3675,7 +3675,7 @@ checksum = "fdd20c5420375476fbd4394763288da7eb0cc0b8c11deed431a91562af7335d3"
[[package]]
name = "yamlpatch"
version = "0.2.0"
version = "0.3.0"
dependencies = [
"indexmap",
"insta",
@ -3684,13 +3684,14 @@ dependencies = [
"serde",
"serde_json",
"serde_yaml",
"subfeature",
"thiserror 2.0.12",
"yamlpath",
]
[[package]]
name = "yamlpath"
version = "0.24.0"
version = "0.25.0"
dependencies = [
"line-index",
"serde",

View file

@ -19,7 +19,7 @@ license = "MIT"
[workspace.dependencies]
anyhow = "1.0.98"
github-actions-expressions = { path = "crates/github-actions-expressions", version = "0.0.8" }
github-actions-models = { path = "crates/github-actions-models", version = "0.31.0" }
github-actions-models = { path = "crates/github-actions-models", version = "0.32.0" }
itertools = "0.14.0"
pest = "2.8.1"
pest_derive = "2.8.1"
@ -54,7 +54,7 @@ serde-sarif = "0.8.0"
serde_json = "1.0.140"
serde_json_path = "0.7.2"
serde_yaml = "0.9.34"
subfeature = { path = "crates/subfeature", version = "0.0.2" }
subfeature = { path = "crates/subfeature", version = "0.0.3" }
tar = "0.4.44"
terminal-link = "0.1.0"
thiserror = "2.0.12"
@ -66,8 +66,8 @@ tracing-subscriber = "0.3.19"
tree-sitter = "0.25.8"
tree-sitter-bash = "0.23.3"
tree-sitter-powershell = "0.25.7"
yamlpath = { path = "crates/yamlpath", version = "0.24.0" }
yamlpatch = { path = "crates/yamlpatch", version = "0.2.0" }
yamlpath = { path = "crates/yamlpath", version = "0.25.0" }
yamlpatch = { path = "crates/yamlpatch", version = "0.3.0" }
tree-sitter-yaml = "0.7.1"
[workspace.lints.clippy]

View file

@ -1,6 +1,6 @@
[package]
name = "github-actions-models"
version = "0.31.0"
version = "0.32.0"
description = "Unofficial, high-quality data models for GitHub Actions workflows, actions, and related components"
repository = "https://github.com/zizmorcore/zizmor/tree/main/crates/github-actions-models"
keywords = ["github", "ci"]

View file

@ -238,8 +238,8 @@ pub struct RepositoryUses {
pub repo: String,
/// The subpath to the action or reusable workflow, if present.
pub subpath: Option<String>,
/// The `@<ref>` that the `uses:` is pinned to, if present.
pub git_ref: Option<String>,
/// The `@<ref>` that the `uses:` is pinned to.
pub git_ref: String,
}
impl FromStr for RepositoryUses {
@ -255,8 +255,8 @@ impl FromStr for RepositoryUses {
// NOTE: Both git refs and paths can contain `@`, but in practice
// GHA refuses to run a `uses:` clause with more than one `@` in it.
let (path, git_ref) = match uses.rsplit_once('@') {
Some((path, git_ref)) => (path, Some(git_ref)),
None => (uses, None),
Some((path, git_ref)) => (path, git_ref),
None => return Err(UsesError(format!("missing `@<ref>` in {uses}"))),
};
let components = path.splitn(3, '/').collect::<Vec<_>>();
@ -268,7 +268,7 @@ impl FromStr for RepositoryUses {
owner: components[0].into(),
repo: components[1].into(),
subpath: components.get(2).map(ToString::to_string),
git_ref: git_ref.map(Into::into),
git_ref: git_ref.into(),
})
}
}
@ -368,16 +368,7 @@ where
let uses = step_uses(de)?;
match uses {
Uses::Repository(ref repo) => {
// Remote reusable workflows must be pinned.
if repo.git_ref.is_none() {
Err(custom_error::<D>(
"repo action must have `@<ref>` in reusable workflow",
))
} else {
Ok(uses)
}
}
Uses::Repository(_) => Ok(uses),
Uses::Local(ref local) => {
// Local reusable workflows cannot be pinned.
// We do this with a string scan because `@` *can* occur as
@ -456,7 +447,7 @@ mod tests {
];
for (val, expected) in vectors {
assert_eq!(val.csharp_trueish(), expected, "failed for {:?}", val);
assert_eq!(val.csharp_trueish(), expected, "failed for {val:?}");
}
}
@ -470,7 +461,7 @@ mod tests {
owner: "actions".to_owned(),
repo: "checkout".to_owned(),
subpath: None,
git_ref: Some("8f4b7f84864484a7bf31766abe9204da3cbe65b3".to_owned()),
git_ref: "8f4b7f84864484a7bf31766abe9204da3cbe65b3".to_owned(),
})),
),
(
@ -480,7 +471,7 @@ mod tests {
owner: "actions".to_owned(),
repo: "aws".to_owned(),
subpath: Some("ec2".to_owned()),
git_ref: Some("8f4b7f84864484a7bf31766abe9204da3cbe65b3".to_owned()),
git_ref: "8f4b7f84864484a7bf31766abe9204da3cbe65b3".to_owned(),
})),
),
(
@ -490,7 +481,7 @@ mod tests {
owner: "example".to_owned(),
repo: "foo".to_owned(),
subpath: Some("bar/baz/quux".to_owned()),
git_ref: Some("8f4b7f84864484a7bf31766abe9204da3cbe65b3".to_owned()),
git_ref: "8f4b7f84864484a7bf31766abe9204da3cbe65b3".to_owned(),
})),
),
(
@ -500,7 +491,7 @@ mod tests {
owner: "actions".to_owned(),
repo: "checkout".to_owned(),
subpath: None,
git_ref: Some("v4".to_owned()),
git_ref: "v4".to_owned(),
})),
),
(
@ -509,18 +500,15 @@ mod tests {
owner: "actions".to_owned(),
repo: "checkout".to_owned(),
subpath: None,
git_ref: Some("abcd".to_owned()),
git_ref: "abcd".to_owned(),
})),
),
(
// Valid: unpinned
// Invalid: unpinned
"actions/checkout",
Ok(Uses::Repository(RepositoryUses {
owner: "actions".to_owned(),
repo: "checkout".to_owned(),
subpath: None,
git_ref: None,
})),
Err(UsesError(
"missing `@<ref>` in actions/checkout".to_owned(),
)),
),
(
// Valid: Docker ref, implicit registry
@ -641,7 +629,7 @@ mod tests {
owner: "octo-org".to_owned(),
repo: "this-repo".to_owned(),
subpath: Some(".github/workflows/workflow-1.yml".to_owned()),
git_ref: Some("172239021f7ba04fe7327647b213799853a9eb89".to_owned()),
git_ref: "172239021f7ba04fe7327647b213799853a9eb89".to_owned(),
})),
),
(
@ -650,7 +638,7 @@ mod tests {
owner: "octo-org".to_owned(),
repo: "this-repo".to_owned(),
subpath: Some(".github/workflows/workflow-1.yml".to_owned()),
git_ref: Some("notahash".to_owned()),
git_ref: "notahash".to_owned(),
})),
),
(
@ -659,7 +647,7 @@ mod tests {
owner: "octo-org".to_owned(),
repo: "this-repo".to_owned(),
subpath: Some(".github/workflows/workflow-1.yml".to_owned()),
git_ref: Some("abcd".to_owned()),
git_ref: "abcd".to_owned(),
})),
),
// Invalid: remote reusable workflow without ref

View file

@ -2,7 +2,7 @@
name = "subfeature"
description = "Subfeature handling and manipulation APIs"
repository = "https://github.com/zizmorcore/zizmor/tree/main/crates/subfeature"
version = "0.0.2"
version = "0.0.3"
readme = "README.md"
authors.workspace = true

View file

@ -100,6 +100,11 @@ impl Span {
end: self.end + bias,
}
}
/// Returns the span as a range.
pub fn as_range(&self) -> std::ops::Range<usize> {
self.start..self.end
}
}
impl From<std::ops::Range<usize>> for Span {
@ -111,18 +116,18 @@ impl From<std::ops::Range<usize>> for Span {
}
}
impl From<Span> for std::ops::Range<usize> {
fn from(span: Span) -> Self {
span.start..span.end
}
}
/// Represents a "subfeature" of a symbolic location, such as a substring
/// within a YAML string.
#[derive(Serialize, Clone, Debug)]
pub struct Subfeature<'doc> {
pub(crate) after: usize,
pub(crate) fragment: Fragment<'doc>,
pub struct Subfeature<'a> {
/// A byte index after which the subfeature starts.
///
/// This is a fuzzy anchor: we know our subfeature starts
/// *somewhere* after this index, but we don't know exactly where it is
/// in the original feature due to parsed whitespace.
pub after: usize,
/// The fragment of the subfeature.
pub fragment: Fragment<'a>,
}
impl<'a> Subfeature<'a> {

View file

@ -1,6 +1,6 @@
[package]
name = "yamlpatch"
version = "0.2.0"
version = "0.3.0"
description = "Comment and format-preserving YAML patch operations"
repository = "https://github.com/zizmorcore/zizmor/tree/main/crates/yamlpatch"
keywords = ["yaml", "patch"]
@ -22,6 +22,7 @@ line-index.workspace = true
serde = { workspace = true, features = ["derive"] }
serde_json.workspace = true
serde_yaml.workspace = true
subfeature.workspace = true
thiserror.workspace = true
yamlpath.workspace = true

View file

@ -139,10 +139,18 @@ pub enum Op<'doc> {
/// which specifies that the rewrite should only occur on
/// the first match of `from` that occurs after the given byte index.
RewriteFragment {
from: Cow<'doc, str>,
from: subfeature::Subfeature<'doc>,
to: Cow<'doc, str>,
after: Option<usize>,
},
/// Replace a comment at the given path.
///
/// This operation replaces the entire comment associated with the feature
/// at the given path with the new comment.
///
/// The entire comment is replaced at once, and only one matching
/// comment is permitted. Features that don't have an associated comment
/// are ignored, while features with multiple comments will be rejected.
ReplaceComment { new: Cow<'doc, str> },
/// Replace the value at the given path
Replace(serde_yaml::Value),
/// Add a new key-value pair at the given path.
@ -208,7 +216,7 @@ fn apply_single_patch(
) -> Result<yamlpath::Document, Error> {
let content = document.source();
match &patch.operation {
Op::RewriteFragment { from, to, after } => {
Op::RewriteFragment { from, to } => {
let Some(feature) = route_to_feature_exact(&patch.route, document)? else {
return Err(Error::InvalidOperation(format!(
"no pre-existing value to patch at {route:?}",
@ -217,11 +225,7 @@ fn apply_single_patch(
};
let extracted_feature = document.extract(&feature);
let bias = match after {
Some(after) => *after,
None => 0,
};
let bias = from.after;
if bias > extracted_feature.len() {
return Err(Error::InvalidOperation(format!(
@ -229,19 +233,14 @@ fn apply_single_patch(
)));
}
let slice = &extracted_feature[bias..];
let (from_start, from_end) = match slice.find(from.as_ref()) {
Some(idx) => (idx + bias, idx + bias + from.len()),
None => {
return Err(Error::InvalidOperation(format!(
"no match for '{from}' in feature"
)));
}
let Some(span) = from.locate_within(extracted_feature) else {
return Err(Error::InvalidOperation(format!(
"no match for '{from:?}' in feature",
)));
};
let mut patched_feature = extracted_feature.to_string();
patched_feature.replace_range(from_start..from_end, to);
patched_feature.replace_range(span.as_range(), to);
// Finally, put our patch back into the overall content.
let mut patched_content = content.to_string();
@ -252,6 +251,32 @@ fn apply_single_patch(
yamlpath::Document::new(patched_content).map_err(Error::from)
}
Op::ReplaceComment { new } => {
let feature = route_to_feature_exact(&patch.route, document)?.ok_or_else(|| {
Error::InvalidOperation(format!(
"no existing feature at {route:?}",
route = patch.route
))
})?;
let comment_features = document.feature_comments(&feature);
let comment_feature = match comment_features.len() {
0 => return Ok(document.clone()),
1 => &comment_features[0],
_ => {
return Err(Error::InvalidOperation(format!(
"multiple comments found at {route:?}",
route = patch.route
)));
}
};
let mut result = content.to_string();
let span = comment_feature.location.byte_span;
result.replace_range(span.0..span.1, new);
yamlpath::Document::new(result).map_err(Error::from)
}
Op::Replace(value) => {
let feature = route_to_feature_pretty(&patch.route, document)?;

View file

@ -224,9 +224,8 @@ foo:
let operations = vec![Patch {
route: route!("foo", "bar"),
operation: Op::RewriteFragment {
from: "${{ foo }}".into(),
from: subfeature::Subfeature::new(0, "${{ foo }}"),
to: "${FOO}".into(),
after: None,
},
}];
@ -254,9 +253,8 @@ foo:
let operations = vec![Patch {
route: route!("foo", "bar"),
operation: Op::RewriteFragment {
from: "${{ foo }}".into(),
from: subfeature::Subfeature::new(0, "${{ foo }}"),
to: "${FOO}".into(),
after: None,
},
}];
@ -274,9 +272,11 @@ foo:
let operations = vec![Patch {
route: route!("foo", "bar"),
operation: Op::RewriteFragment {
from: "${{ foo }}".into(),
from: subfeature::Subfeature::new(
original.find("${{ foo }}").unwrap() + 1,
"${{ foo }}",
),
to: "${FOO}".into(),
after: original.find("${{ foo }}").map(|idx| idx + 1),
},
}];
@ -309,17 +309,15 @@ jobs:
Patch {
route: route!("jobs", "test", "steps", 0, "run"),
operation: Op::RewriteFragment {
from: "${{ foo }}".into(),
from: subfeature::Subfeature::new(0, "${{ foo }}"),
to: "${FOO}".into(),
after: None,
},
},
Patch {
route: route!("jobs", "test", "steps", 0, "run"),
operation: Op::RewriteFragment {
from: "${{ bar }}".into(),
from: subfeature::Subfeature::new(0, "${{ bar }}"),
to: "${BAR}".into(),
after: None,
},
},
];
@ -337,6 +335,87 @@ jobs:
"#);
}
/// `Operation::ReplaceComment` should replace the comment
/// at the given route with the new comment, without affecting
/// any YAML values or any other comments.
#[test]
fn test_replace_comment() {
let original = r#"
foo:
bar: baz # This is a comment
abc: def # Another comment
"#;
let document = yamlpath::Document::new(original).unwrap();
let operations = vec![Patch {
route: route!("foo", "bar"),
operation: Op::ReplaceComment {
new: "# Updated comment".into(),
},
}];
let result = apply_yaml_patches(&document, &operations).unwrap();
insta::assert_snapshot!(result.source(), @r"
foo:
bar: baz # Updated comment
abc: def # Another comment
");
}
/// `Operation::ReplaceComment` should fdo nothing if there is no comment
/// at the given route, and should not affect the YAML value.
#[test]
fn test_replace_comment_noop() {
let original = r#"
foo:
bar: baz
abc: def
"#;
let document = yamlpath::Document::new(original).unwrap();
let operations = vec![Patch {
route: route!("foo", "bar"),
operation: Op::ReplaceComment {
new: "# This comment does not exist".into(),
},
}];
let result = apply_yaml_patches(&document, &operations).unwrap();
insta::assert_snapshot!(result.source(), @r"
foo:
bar: baz
abc: def
");
}
/// `Operation::ReplaceComment` should fail if there are multiple comments
/// at the given route, as it's unclear which one to replace.
#[test]
fn test_replace_comment_fails_on_too_many_comments() {
let original = r#"
foo:
bar: baz # First comment
abc: def # Second comment
"#;
let document = yamlpath::Document::new(original).unwrap();
let operations = vec![Patch {
route: route!("foo"),
operation: Op::ReplaceComment {
new: "# This won't work".into(),
},
}];
let result = apply_yaml_patches(&document, &operations);
assert!(result.is_err());
}
#[test]
fn test_replace_empty_block_value() {
let original = r#"
@ -869,7 +948,7 @@ jobs:
- run: echo "test""#;
// Test with trailing newline (common in real files)
let original_with_newline = format!("{}\n", original);
let original_with_newline = format!("{original}\n");
let operations = vec![Patch {
route: route!("jobs", "test"),
@ -1022,16 +1101,14 @@ fn test_comment_boundary_issue() {
// Assert that there's content between the steps (whitespace and list marker)
assert!(
!content_between.is_empty(),
"There should be content between steps. Content between: {:?}",
content_between
"There should be content between steps. Content between: {content_between:?}"
);
// The content between is just whitespace and the list marker for step2
// yamlpath includes comments as part of the respective steps
assert!(
content_between.contains("- "),
"Should contain list marker for step2. Content between: {:?}",
content_between
"Should contain list marker for step2. Content between: {content_between:?}"
);
// Assert that step boundaries don't overlap
@ -1325,8 +1402,7 @@ fn test_debug_indentation_issue() {
// Assert that leading whitespace extraction includes the step content
assert!(
feature_with_ws.contains("name: Test step"),
"Step should contain the step name. Actual content: {:?}",
feature_with_ws
"Step should contain the step name. Actual content: {feature_with_ws:?}"
);
// Assert that the content includes the multiline run block
@ -1349,7 +1425,7 @@ fn test_debug_indentation_issue() {
if let Some(first_line) = feature_with_ws.lines().next() {
if let Some(_colon_pos) = first_line.find(':') {
let key_indent = &first_line[..first_line.len() - first_line.trim_start().len()];
let final_indent = format!("{} ", key_indent);
let final_indent = format!("{key_indent} ");
// Assert that indentation calculation works correctly
assert!(!final_indent.is_empty(), "Final indent should not be empty");
@ -1452,15 +1528,13 @@ jobs:
}
} else {
panic!(
"Env content should parse as a mapping. Actual content: {:?}",
env_content
"Env content should parse as a mapping. Actual content: {env_content:?}"
);
}
}
Err(e) => {
panic!(
"Env content should parse as valid YAML: {}. Actual content: {:?}",
e, env_content
"Env content should parse as valid YAML: {e}. Actual content: {env_content:?}"
);
}
}

View file

@ -1,6 +1,6 @@
[package]
name = "yamlpath"
version = "0.24.0"
version = "0.25.0"
description = "Format-preserving YAML feature extraction"
repository = "https://github.com/zizmorcore/zizmor/tree/main/crates/yamlpath"
readme = "README.md"

View file

@ -475,7 +475,7 @@ impl Document {
/// Given a [`Feature`], return all comments that span the same range
/// as the feature does.
pub fn feature_comments<'tree>(&'tree self, feature: &Feature<'tree>) -> Vec<&'tree str> {
pub fn feature_comments<'tree>(&'tree self, feature: &Feature<'tree>) -> Vec<Feature<'tree>> {
// To extract all comments for a feature, we trawl the entire tree's
// nodes and extract all comment nodes in the line range for the
// feature.
@ -502,11 +502,10 @@ impl Document {
fn trawl<'tree>(
node: &Node<'tree>,
source: &'tree str,
comment_id: u16,
start_line: usize,
end_line: usize,
) -> Vec<&'tree str> {
) -> Vec<Feature<'tree>> {
let mut comments = vec![];
let mut cur = node.walk();
@ -524,11 +523,11 @@ impl Document {
&& c.start_position().row >= start_line
&& c.end_position().row <= end_line
})
.map(|c| c.utf8_text(source.as_bytes()).unwrap()),
.map(|c| c.into()),
);
for child in node.children(&mut cur) {
comments.extend(trawl(&child, source, comment_id, start_line, end_line));
comments.extend(trawl(&child, comment_id, start_line, end_line));
}
comments
@ -536,7 +535,6 @@ impl Document {
trawl(
&self.tree.root_node(),
&self.source,
self.comment_id,
start_line,
end_line,
@ -906,7 +904,10 @@ bar: # outside
};
let feature = doc.query_pretty(&route).unwrap();
assert_eq!(
doc.feature_comments(&feature),
doc.feature_comments(&feature)
.iter()
.map(|f| doc.extract(f))
.collect::<Vec<_>>(),
&["# rootlevel", "# foo", "# bar", "# baz", "# quux"]
);
@ -920,7 +921,13 @@ bar: # outside
],
};
let feature = doc.query_pretty(&route).unwrap();
assert_eq!(doc.feature_comments(&feature), &["# quux"]);
assert_eq!(
doc.feature_comments(&feature)
.iter()
.map(|f| doc.extract(f))
.collect::<Vec<_>>(),
&["# quux"]
);
}
#[test]

View file

@ -385,9 +385,8 @@ impl BotConditions {
patches: vec![Patch {
route: if_route,
operation: Op::RewriteFragment {
from: spoofable_context_raw.into(),
from: subfeature::Subfeature::new(0, spoofable_context_raw),
to: safe_context.into(),
after: None,
},
}],
})

View file

@ -10,11 +10,12 @@ use github_actions_models::common::{RepositoryUses, Uses};
use super::{Audit, AuditLoadError, audit_meta};
use crate::{
finding::{Confidence, Finding, Severity},
finding::{Confidence, Finding, Fix, Severity, location::Routable as _},
github_api,
models::{StepCommon, action::CompositeStep, uses::RepositoryUsesExt as _, workflow::Step},
state::AuditState,
};
use yamlpatch::{Op, Patch};
pub(crate) struct KnownVulnerableActions {
client: github_api::Client,
@ -30,7 +31,7 @@ impl KnownVulnerableActions {
fn action_known_vulnerabilities(
&self,
uses: &RepositoryUses,
) -> Result<Vec<(Severity, String)>> {
) -> Result<Vec<(Severity, String, Option<String>)>> {
let version = match &uses.git_ref {
// If `uses` is pinned to a symbolic ref, we need to perform
// feats of heroism to figure out what's going on.
@ -48,7 +49,7 @@ impl KnownVulnerableActions {
//
// To handle all of the above, we convert the ref into a commit
// and then find the longest tag for that commit.
Some(version) if !uses.ref_is_commit() => {
version if !uses.ref_is_commit() => {
let Some(commit_ref) =
self.client
.commit_for_ref(&uses.owner, &uses.repo, version)?
@ -74,7 +75,7 @@ impl KnownVulnerableActions {
// tag matching that ref. In theory the action's repo could do
// something annoying like use branches for versions instead,
// which we should also probably support.
Some(commit_ref) => {
commit_ref => {
match self
.client
.longest_tag_for_commit(&uses.owner, &uses.repo, commit_ref)?
@ -87,12 +88,6 @@ impl KnownVulnerableActions {
None => return Ok(vec![]),
}
}
// No version means the action runs the latest default branch
// version. We could in theory query GHSA for this but it's
// unlikely to be meaningful.
// TODO: Maybe we need a separate (low-sev) audit for actions usage
// on @master/@main/etc?
None => return Ok(vec![]),
};
let vulns = self
@ -110,12 +105,113 @@ impl KnownVulnerableActions {
_ => Severity::Unknown,
};
results.push((severity, vuln.ghsa_id));
// Get the first patched version from the first vulnerability in the advisory
let first_patched_version = vuln
.vulnerabilities
.first()
.and_then(|v| v.first_patched_version.clone());
results.push((severity, vuln.ghsa_id, first_patched_version));
}
Ok(results)
}
/// Create a fix to upgrade to a specific non-vulnerable version
fn create_upgrade_fix<'doc>(
&self,
uses: &RepositoryUses,
target_version: String,
step: &impl StepCommon<'doc>,
) -> Result<Fix<'doc>> {
let mut uses_slug = format!("{}/{}", uses.owner, uses.repo);
if let Some(subpath) = &uses.subpath {
uses_slug.push_str(&format!("/{subpath}"));
}
let (bare_version, prefixed_version) = if let Some(bare) = target_version.strip_prefix('v')
{
(bare.into(), target_version)
} else {
let prefixed = format!("v{target_version}");
(target_version, prefixed)
};
match uses.ref_is_commit() {
// If `uses` is pinned to a commit, then we need two patches:
// one to change the `uses` clause to the new version,
// and another to replace any existing version comment.
true => {
// Annoying: GHSA will usually give us a fix version as `X.Y.Z`,
// but GitHub Actions are conventionally tagged as `vX.Y.Z`.
// We don't know whether a given action follows this
// convention or not, so we have to try both.
// We try the prefixed version first, since we expect it
// to be more common.
let (target_ref, target_commit) = self
.client
.commit_for_ref(&uses.owner, &uses.repo, &prefixed_version)
.map(|commit| commit.map(|commit| (&prefixed_version, commit)))
.or_else(|_| {
self.client
.commit_for_ref(&uses.owner, &uses.repo, &bare_version)
.map(|commit| commit.map(|commit| (&bare_version, commit)))
})?
.ok_or_else(|| {
anyhow!(
"Cannot resolve version {bare_version} to commit hash for {}/{}",
uses.owner,
uses.repo
)
})?;
let new_uses_value = format!("{uses_slug}@{target_commit}");
Ok(Fix {
title: format!("upgrade {uses_slug} to {target_ref}"),
key: step.location().key,
disposition: Default::default(),
patches: vec![
Patch {
route: step.route().with_key("uses"),
operation: Op::Replace(new_uses_value.into()),
},
Patch {
route: step.route().with_key("uses"),
operation: Op::ReplaceComment {
new: format!("# {target_ref}").into(),
},
},
],
})
}
// If `uses` is pinned to a symbolic ref, we only need to perform
// a single patch.
false => {
// Like above, we don't know a priori whether the new tag should be
// prefixed with `v` or not. Instead of trying to figure it out
// via the GitHub API, we match the style of the current `uses`
// clause.
let target_version_tag = if uses.git_ref.starts_with('v') {
prefixed_version
} else {
bare_version
};
let new_uses_value = format!("{uses_slug}@{target_version_tag}");
Ok(Fix {
title: format!("upgrade {uses_slug} to {target_version_tag}"),
key: step.location().key,
disposition: Default::default(),
patches: vec![Patch {
route: step.route().with_key("uses"),
operation: Op::Replace(new_uses_value.into()),
}],
})
}
}
}
fn process_step<'doc>(&self, step: &impl StepCommon<'doc>) -> Result<Vec<Finding<'doc>>> {
let mut findings = vec![];
@ -123,20 +219,36 @@ impl KnownVulnerableActions {
return Ok(findings);
};
for (severity, id) in self.action_known_vulnerabilities(uses)? {
findings.push(
Self::finding()
.confidence(Confidence::High)
.severity(severity)
.add_location(
step.location()
.primary()
.with_keys(["uses".into()])
.annotated(&id)
.with_url(format!("https://github.com/advisories/{id}")),
)
.build(step)?,
);
for (severity, id, first_patched_version) in self.action_known_vulnerabilities(uses)? {
let mut finding_builder = Self::finding()
.confidence(Confidence::High)
.severity(severity)
.add_location(
step.location()
.primary()
.with_keys(["uses".into()])
.annotated(&id)
.with_url(format!("https://github.com/advisories/{id}")),
);
// Add fix if available.
// TODO(ww): In principle we could have multiple findings on a single
// `uses:` clause, in which case our suggested fixes would potentially
// overlap and partially cancel each other out. The end result of this
// would be a lack of a single fixpoint, i.e. the user has to invoke
// `zizmor` multiple times to fix all vulnerabilities.
// To avoid that, we could probably collect each `first_patched_version`
// and only apply the highest one. This would be moderately annoying
// to do, since we'd have to decide which finding to attach that
// fix to.
if let Some(fix) = first_patched_version
.map(|patched_version| self.create_upgrade_fix(uses, patched_version, step))
.transpose()?
{
finding_builder = finding_builder.fix(fix);
}
findings.push(finding_builder.build(step)?);
}
Ok(findings)
@ -169,3 +281,561 @@ impl Audit for KnownVulnerableActions {
self.process_step(step)
}
}
#[cfg(test)]
mod tests {
use std::path::Path;
use insta::assert_snapshot;
use super::*;
use crate::{
models::{AsDocument, workflow::Workflow},
registry::InputKey,
};
// Helper function to create a test KnownVulnerableActions instance
fn create_test_audit() -> KnownVulnerableActions {
let config = crate::config::Config::default();
let state = crate::state::AuditState {
config: &config,
no_online_audits: false,
gh_client: Some(
github_api::Client::new(
&github_api::GitHubHost::Standard("github.com".to_string()),
&github_api::GitHubToken::new("fake").unwrap(),
Path::new("/tmp"),
)
.unwrap(),
),
gh_hostname: crate::github_api::GitHubHost::Standard("github.com".to_string()),
};
KnownVulnerableActions::new(&state).unwrap()
}
#[test]
fn test_fix_upgrade_actions_checkout() {
let workflow_content = r#"
name: Test Vulnerable Actions
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Checkout with old version
uses: actions/checkout@v2
- name: Another step
run: echo "hello"
"#;
let key = InputKey::local("test_checkout.yml", None::<&str>).unwrap();
let workflow = Workflow::from_string(workflow_content.to_string(), key).unwrap();
let job = workflow.jobs().next().unwrap();
let steps: Vec<_> = match job {
crate::models::workflow::Job::NormalJob(normal_job) => normal_job.steps().collect(),
_ => panic!("Expected normal job"),
};
let step = &steps[0];
// Test the fix directly
let uses = RepositoryUses {
owner: "actions".to_string(),
repo: "checkout".to_string(),
git_ref: "v2".to_string(),
subpath: None,
};
let audit = create_test_audit();
let fix = audit.create_upgrade_fix(&uses, "v4".into(), step).unwrap();
let fixed_document = fix.apply(workflow.as_document()).unwrap();
insta::assert_snapshot!(fixed_document.source(), @r#"
name: Test Vulnerable Actions
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Checkout with old version
uses: actions/checkout@v4
- name: Another step
run: echo "hello"
"#);
}
#[test]
fn test_fix_upgrade_actions_setup_node() {
let workflow_content = r#"
name: Test Node Setup
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Setup Node
uses: actions/setup-node@v1
with:
node-version: '18'
- name: Install dependencies
run: npm install
"#;
let key = InputKey::local("test_setup_node.yml", None::<&str>).unwrap();
let workflow = Workflow::from_string(workflow_content.to_string(), key).unwrap();
let job = workflow.jobs().next().unwrap();
let steps: Vec<_> = match job {
crate::models::workflow::Job::NormalJob(normal_job) => normal_job.steps().collect(),
_ => panic!("Expected normal job"),
};
let step = &steps[0];
// Test the fix directly
let uses = RepositoryUses {
owner: "actions".to_string(),
repo: "setup-node".to_string(),
git_ref: "v1".to_string(),
subpath: None,
};
let audit = create_test_audit();
let fix = audit.create_upgrade_fix(&uses, "v4".into(), step).unwrap();
let fixed_document = fix.apply(workflow.as_document()).unwrap();
insta::assert_snapshot!(fixed_document.source(), @r#"
name: Test Node Setup
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Setup Node
uses: actions/setup-node@v4
with:
node-version: '18'
- name: Install dependencies
run: npm install
"#);
}
#[test]
fn test_fix_upgrade_third_party_action() {
let workflow_content = r#"
name: Test Third Party Action
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Upload to codecov
uses: codecov/codecov-action@v1
with:
token: ${{ secrets.CODECOV_TOKEN }}
- name: Another step
run: echo "test"
"#;
let key = InputKey::local("test_third_party.yml", None::<&str>).unwrap();
let workflow = Workflow::from_string(workflow_content.to_string(), key).unwrap();
let job = workflow.jobs().next().unwrap();
let steps: Vec<_> = match job {
crate::models::workflow::Job::NormalJob(normal_job) => normal_job.steps().collect(),
_ => panic!("Expected normal job"),
};
let step = &steps[0];
// Test the fix directly
let uses = RepositoryUses {
owner: "codecov".to_string(),
repo: "codecov-action".to_string(),
git_ref: "v1".to_string(),
subpath: None,
};
let audit = create_test_audit();
let fix = audit.create_upgrade_fix(&uses, "v4".into(), step).unwrap();
let fixed_document = fix.apply(workflow.as_document()).unwrap();
insta::assert_snapshot!(fixed_document.source(), @r#"
name: Test Third Party Action
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Upload to codecov
uses: codecov/codecov-action@v4
with:
token: ${{ secrets.CODECOV_TOKEN }}
- name: Another step
run: echo "test"
"#);
}
#[test]
fn test_fix_upgrade_multiple_vulnerable_actions() {
let workflow_content = r#"
name: Test Multiple Vulnerable Actions
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2
- name: Setup Node
uses: actions/setup-node@v1
with:
node-version: '18'
- name: Cache dependencies
uses: actions/cache@v2
with:
path: ~/.npm
key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
- name: Install dependencies
run: npm install
"#;
let key = InputKey::local("test_multiple.yml", None::<&str>).unwrap();
let workflow = Workflow::from_string(workflow_content.to_string(), key).unwrap();
let job = workflow.jobs().next().unwrap();
let steps: Vec<_> = match job {
crate::models::workflow::Job::NormalJob(normal_job) => normal_job.steps().collect(),
_ => panic!("Expected normal job"),
};
// Apply fixes to each vulnerable action
let mut current_document = workflow.as_document().clone();
let audit = create_test_audit();
// Fix checkout action
let uses_checkout = RepositoryUses {
owner: "actions".to_string(),
repo: "checkout".to_string(),
git_ref: "v2".to_string(),
subpath: None,
};
let fix_checkout = audit
.create_upgrade_fix(&uses_checkout, "v4".into(), &steps[0])
.unwrap();
current_document = fix_checkout.apply(&current_document).unwrap();
// Fix setup-node action
let uses_node = RepositoryUses {
owner: "actions".to_string(),
repo: "setup-node".to_string(),
git_ref: "v1".to_string(),
subpath: None,
};
let fix_node = audit
.create_upgrade_fix(&uses_node, "v4".into(), &steps[1])
.unwrap();
current_document = fix_node.apply(&current_document).unwrap();
// Fix cache action
let uses_cache = RepositoryUses {
owner: "actions".to_string(),
repo: "cache".to_string(),
git_ref: "v2".to_string(),
subpath: None,
};
let fix_cache = audit
.create_upgrade_fix(&uses_cache, "v4".into(), &steps[2])
.unwrap();
current_document = fix_cache.apply(&current_document).unwrap();
insta::assert_snapshot!(current_document.source(), @r#"
name: Test Multiple Vulnerable Actions
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v4
- name: Setup Node
uses: actions/setup-node@v4
with:
node-version: '18'
- name: Cache dependencies
uses: actions/cache@v4
with:
path: ~/.npm
key: ${{ runner.os }}-node-${{ hashFiles('**/package-lock.json') }}
- name: Install dependencies
run: npm install
"#);
}
#[test]
fn test_fix_upgrade_action_with_subpath() {
let workflow_content = r#"
name: Test Action with Subpath
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Custom action
uses: owner/repo/subpath@v1
with:
param: value
"#;
let key = InputKey::local("test_subpath.yml", None::<&str>).unwrap();
let workflow = Workflow::from_string(workflow_content.to_string(), key).unwrap();
let job = workflow.jobs().next().unwrap();
let steps: Vec<_> = match job {
crate::models::workflow::Job::NormalJob(normal_job) => normal_job.steps().collect(),
_ => panic!("Expected normal job"),
};
let step = &steps[0];
// Test the fix with subpath
let uses = RepositoryUses {
owner: "owner".to_string(),
repo: "repo".to_string(),
git_ref: "v1".to_string(),
subpath: Some("subpath".to_string()),
};
let audit = create_test_audit();
let fix = audit.create_upgrade_fix(&uses, "v2".into(), step).unwrap();
let fixed_document = fix.apply(workflow.as_document()).unwrap();
insta::assert_snapshot!(fixed_document.source(), @r"
name: Test Action with Subpath
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Custom action
uses: owner/repo/subpath@v2
with:
param: value
");
}
#[test]
fn test_first_patched_version_priority() {
// This test verifies that first_patched_version is used when available
let workflow_content = r#"
name: Test First Patched Version Priority
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Vulnerable action
uses: actions/checkout@v2
"#;
let key = InputKey::local("test_first_patched.yml", None::<&str>).unwrap();
let workflow = Workflow::from_string(workflow_content.to_string(), key).unwrap();
let job = workflow.jobs().next().unwrap();
let steps: Vec<_> = match job {
crate::models::workflow::Job::NormalJob(normal_job) => normal_job.steps().collect(),
_ => panic!("Expected normal job"),
};
let step = &steps[0];
let uses = RepositoryUses {
owner: "actions".to_string(),
repo: "checkout".to_string(),
git_ref: "v2".to_string(),
subpath: None,
};
// Test that when first_patched_version is provided, it's used
let audit = create_test_audit();
let fix_with_patched_version = audit
.create_upgrade_fix(&uses, "v3.1.0".into(), step)
.unwrap();
let fixed_document = fix_with_patched_version
.apply(workflow.as_document())
.unwrap();
insta::assert_snapshot!(fixed_document.source(), @r#"
name: Test First Patched Version Priority
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Vulnerable action
uses: actions/checkout@v3.1.0
"#);
}
#[test]
fn test_fix_symbolic_ref() {
let workflow_content = r#"
name: Test Non-Commit Ref
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Tag pinned action
uses: actions/checkout@v2 # this comment stays
"#;
let key = InputKey::local("test_non_commit.yml", None::<&str>).unwrap();
let workflow = Workflow::from_string(workflow_content.to_string(), key).unwrap();
let job = workflow.jobs().next().unwrap();
let steps: Vec<_> = match job {
crate::models::workflow::Job::NormalJob(normal_job) => normal_job.steps().collect(),
_ => panic!("Expected normal job"),
};
let step = &steps[0];
let uses = RepositoryUses {
owner: "actions".to_string(),
repo: "checkout".to_string(),
git_ref: "v2".to_string(),
subpath: None,
};
let audit = create_test_audit();
let fix = audit.create_upgrade_fix(&uses, "v4".into(), step).unwrap();
let new_doc = fix.apply(workflow.as_document()).unwrap();
assert_snapshot!(new_doc.source(), @r"
name: Test Non-Commit Ref
on: push
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Tag pinned action
uses: actions/checkout@v4 # this comment stays
");
}
#[test]
fn test_offline_audit_state_creation() {
// Test that we can create an audit state without a GitHub token
let config = crate::config::Config::default();
let state = crate::state::AuditState {
config: &config,
no_online_audits: true,
gh_client: None,
gh_hostname: crate::github_api::GitHubHost::Standard("github.com".to_string()),
};
// This should fail because no GitHub token is provided
let audit_result = KnownVulnerableActions::new(&state);
assert!(audit_result.is_err());
}
#[cfg(feature = "gh-token-tests")]
#[test]
fn test_fix_commit_pin() {
// Test with real GitHub API - requires GH_TOKEN environment variable
let workflow_content = r#"
name: Test Commit Hash Pinning Real API
on: push
permissions: {}
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Commit pinned action
uses: actions/download-artifact@7a1cd3216ca9260cd8022db641d960b1db4d1be4 # v4.0.0
"#;
let key = InputKey::local("dummy.yml", None::<&str>).unwrap();
let workflow = Workflow::from_string(workflow_content.to_string(), key).unwrap();
let config = crate::config::Config::default();
let state = crate::state::AuditState {
config: &config,
no_online_audits: false,
gh_client: Some(
github_api::Client::new(
&github_api::GitHubHost::Standard("github.com".to_string()),
&github_api::GitHubToken::new(&std::env::var("GH_TOKEN").unwrap()).unwrap(),
Path::new("/tmp"),
)
.unwrap(),
),
gh_hostname: crate::github_api::GitHubHost::Standard("github.com".to_string()),
};
let audit = KnownVulnerableActions::new(&state).unwrap();
let input = workflow.into();
let findings = audit.audit(&input).unwrap();
assert_eq!(findings.len(), 1);
let new_doc = findings[0].fixes[0].apply(input.as_document()).unwrap();
assert_snapshot!(new_doc.source(), @r"
name: Test Commit Hash Pinning Real API
on: push
permissions: {}
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Commit pinned action
uses: actions/download-artifact@87c55149d96e628cc2ef7e6fc2aab372015aec85 # v4.1.3
");
}
// TODO: test_fix_commit_pin_subpath
#[cfg(feature = "gh-token-tests")]
#[test]
fn test_fix_commit_pin_no_comment() {
// Ensure that we don't rewrite a version comment
// if the `uses:` clause doesn't already have one.
let workflow_content = r#"
name: Test Commit Hash Pinning Real API
on: push
permissions: {}
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Commit pinned action
uses: actions/download-artifact@7a1cd3216ca9260cd8022db641d960b1db4d1be4
"#;
let key = InputKey::local("dummy.yml", None::<&str>).unwrap();
let workflow = Workflow::from_string(workflow_content.to_string(), key).unwrap();
let config = crate::config::Config::default();
let state = crate::state::AuditState {
config: &config,
no_online_audits: false,
gh_client: Some(
github_api::Client::new(
&github_api::GitHubHost::Standard("github.com".to_string()),
&github_api::GitHubToken::new(&std::env::var("GH_TOKEN").unwrap()).unwrap(),
Path::new("/tmp"),
)
.unwrap(),
),
gh_hostname: crate::github_api::GitHubHost::Standard("github.com".to_string()),
};
let audit = KnownVulnerableActions::new(&state).unwrap();
let input = workflow.into();
let findings = audit.audit(&input).unwrap();
assert_eq!(findings.len(), 1);
let new_doc = findings[0].fixes[0].apply(input.as_document()).unwrap();
assert_snapshot!(new_doc.source(), @r"
name: Test Commit Hash Pinning Real API
on: push
permissions: {}
jobs:
test:
runs-on: ubuntu-latest
steps:
- name: Commit pinned action
uses: actions/download-artifact@87c55149d96e628cc2ef7e6fc2aab372015aec85
");
}
}

View file

@ -287,9 +287,8 @@ impl TemplateInjection {
patches.push(Patch {
route: step.route().with_key("run"),
operation: Op::RewriteFragment {
from: raw.as_raw().to_string().into(),
from: subfeature::Subfeature::new(0, raw.as_raw()),
to: format!("${{{env_var}}}").into(),
after: None,
},
});
@ -628,9 +627,7 @@ mod tests {
.fixes
.iter()
.find(|f| f.title == expected_title)
.unwrap_or_else(|| {
panic!("Expected fix with title '{}' but not found", expected_title)
});
.unwrap_or_else(|| panic!("Expected fix with title '{expected_title}' but not found"));
fix.apply(document).unwrap()
}

View file

@ -162,7 +162,7 @@ impl<'doc> SymbolicLocation<'doc> {
(
extracted,
ConcreteLocation::from_span(subfeature_span.into(), document),
ConcreteLocation::from_span(subfeature_span.as_range(), document),
feature,
)
}
@ -194,7 +194,7 @@ impl<'doc> SymbolicLocation<'doc> {
comments: document
.feature_comments(&feature)
.into_iter()
.map(Comment)
.map(|f| Comment(document.extract(&f)))
.collect(),
},
})
@ -338,7 +338,7 @@ impl<'doc> Feature<'doc> {
) -> Self {
let contents = input.as_document().source();
let span = subfeature.locate_within(contents).unwrap().into();
let span = subfeature.locate_within(contents).unwrap().as_range();
Self::from_span(&span, input)
}

View file

@ -8,7 +8,6 @@ use std::{io::Read, ops::Deref, path::Path};
use anyhow::{Context, Result, anyhow};
use camino::Utf8Path;
use flate2::read::GzDecoder;
use github_actions_models::common::RepositoryUses;
use http_cache_reqwest::{
CACacheManager, Cache, CacheMode, CacheOptions, HttpCache, HttpCacheOptions,
};
@ -24,7 +23,7 @@ use tracing::instrument;
use crate::{
InputRegistry,
registry::{InputKey, InputKind},
registry::{InputKey, InputKind, RepoSlug},
utils::PipeSelf,
};
@ -36,7 +35,7 @@ pub(crate) enum GitHubHost {
}
impl GitHubHost {
pub(crate) fn from_clap(hostname: &str) -> Result<Self, String> {
pub(crate) fn new(hostname: &str) -> Result<Self, String> {
let normalized = hostname.to_lowercase();
// NOTE: ideally we'd do a full domain validity check here.
@ -66,7 +65,7 @@ impl GitHubHost {
pub(crate) struct GitHubToken(String);
impl GitHubToken {
pub(crate) fn from_clap(token: &str) -> Result<Self, String> {
pub(crate) fn new(token: &str) -> Result<Self, String> {
let token = token.trim();
if token.is_empty() {
return Err("GitHub token cannot be empty".into());
@ -350,7 +349,7 @@ impl Client {
#[tokio::main]
pub(crate) async fn fetch_workflows(
&self,
slug: &RepositoryUses,
slug: &RepoSlug,
registry: &mut InputRegistry,
) -> Result<()> {
let owner = &slug.owner;
@ -417,7 +416,7 @@ impl Client {
#[tokio::main]
pub(crate) async fn fetch_audit_inputs(
&self,
slug: &RepositoryUses,
slug: &RepoSlug,
registry: &mut InputRegistry,
) -> Result<()> {
let url = format!(
@ -537,6 +536,13 @@ pub(crate) struct Comparison {
pub(crate) struct Advisory {
pub(crate) ghsa_id: String,
pub(crate) severity: String,
pub(crate) vulnerabilities: Vec<Vulnerability>,
}
/// Represents a vulnerability within a GHSA advisory.
#[derive(Deserialize)]
pub(crate) struct Vulnerability {
pub(crate) first_patched_version: Option<String>,
}
/// Represents a file listing from GitHub's contents API.
@ -560,7 +566,7 @@ mod tests {
"https://selfhosted.example.com/api/v3",
),
] {
assert_eq!(GitHubHost::from_clap(host).unwrap().to_api_url(), expected);
assert_eq!(GitHubHost::new(host).unwrap().to_api_url(), expected);
}
}
@ -572,14 +578,14 @@ mod tests {
("gho_testtest", "gho_testtest"),
("gho_test\ntest", "gho_test\ntest"),
] {
assert_eq!(GitHubToken::from_clap(token).unwrap().0, expected);
assert_eq!(GitHubToken::new(token).unwrap().0, expected);
}
}
#[test]
fn test_github_token_err() {
for token in ["", " ", "\r", "\n", "\t", " "] {
assert!(GitHubToken::from_clap(token).is_err());
assert!(GitHubToken::new(token).is_err());
}
}
}

View file

@ -15,7 +15,6 @@ use clap_complete::Generator;
use clap_verbosity_flag::InfoLevel;
use config::Config;
use finding::{Confidence, Persona, Severity};
use github_actions_models::common::Uses;
use github_api::{GitHubHost, GitHubToken};
use ignore::WalkBuilder;
use indicatif::ProgressStyle;
@ -27,6 +26,8 @@ use tracing::{Span, info_span, instrument};
use tracing_indicatif::{IndicatifLayer, span_ext::IndicatifSpanExt};
use tracing_subscriber::{EnvFilter, layer::SubscriberExt as _, util::SubscriberInitExt as _};
use crate::registry::RepoSlug;
mod audit;
mod config;
mod finding;
@ -69,11 +70,11 @@ struct App {
offline: bool,
/// The GitHub API token to use.
#[arg(long, env, value_parser = GitHubToken::from_clap)]
#[arg(long, env, value_parser = GitHubToken::new)]
gh_token: Option<GitHubToken>,
/// The GitHub Server Hostname. Defaults to github.com
#[arg(long, env = "GH_HOST", default_value = "github.com", value_parser = GitHubHost::from_clap)]
#[arg(long, env = "GH_HOST", default_value = "github.com", value_parser = GitHubHost::new)]
gh_hostname: GitHubHost,
/// Perform only offline audits.
@ -420,8 +421,7 @@ fn collect_from_repo_slug(
state: &AuditState,
registry: &mut InputRegistry,
) -> Result<()> {
// Our pre-existing `uses: <slug>` parser does 90% of the work for us.
let Ok(Uses::Repository(slug)) = Uses::from_str(input) else {
let Ok(slug) = RepoSlug::from_str(input) else {
return Err(anyhow!(tips(
format!("invalid input: {input}"),
&[format!(
@ -433,14 +433,6 @@ fn collect_from_repo_slug(
)));
};
// We don't expect subpaths here.
if slug.subpath.is_some() {
return Err(anyhow!(tips(
"invalid GitHub repository reference",
&["pass owner/repo or owner/repo@ref"]
)));
}
let client = state.gh_client.as_ref().ok_or_else(|| {
anyhow!(tips(
format!("can't retrieve repository: {input}", input = input.green()),

View file

@ -335,38 +335,38 @@ mod tests {
#[test]
fn test_usage() {
let workflow = r#"
name: test_usage
on: push
jobs:
test_usage:
runs-on: ubuntu-latest
steps:
- uses: foo/bar # 0
name: test_usage
on: push
jobs:
test_usage:
runs-on: ubuntu-latest
steps:
- uses: foo/bar@v1 # 0
- uses: foo/bar@v1 # 1
- uses: foo/bar@v1 # 1
- uses: not/thesame # 2
with:
set-me: true
- uses: not/thesame@v1 # 2
with:
set-me: true
- uses: not/thesame # 3
- uses: not/thesame@v1 # 3
- uses: foo/bar # 4
with:
set-me: true
- uses: foo/bar@v1 # 4
with:
set-me: true
- uses: foo/bar # 5
with:
set-me: false
- uses: foo/bar@v1 # 5
with:
set-me: false
- uses: foo/bar # 6
with:
disable-cache: true
- uses: foo/bar@v1 # 6
with:
disable-cache: true
- uses: foo/bar # 7
with:
disable-cache: false
"#;
- uses: foo/bar@v1 # 7
with:
disable-cache: false
"#;
let workflow =
Workflow::from_string(workflow.into(), InputKey::local("dummy", None).unwrap())

View file

@ -75,7 +75,7 @@ impl RepositoryUsesPattern {
uses.owner.eq_ignore_ascii_case(owner)
&& uses.repo.eq_ignore_ascii_case(repo)
&& uses.subpath == *subpath
&& uses.git_ref.as_deref().is_some_and(|s| s == git_ref)
&& uses.git_ref.as_str() == git_ref
}
RepositoryUsesPattern::ExactPath {
owner,
@ -194,22 +194,19 @@ impl RepositoryUsesExt for RepositoryUses {
}
fn ref_is_commit(&self) -> bool {
match &self.git_ref {
Some(git_ref) => git_ref.len() == 40 && git_ref.chars().all(|c| c.is_ascii_hexdigit()),
None => false,
}
self.git_ref.len() == 40 && self.git_ref.chars().all(|c| c.is_ascii_hexdigit())
}
fn commit_ref(&self) -> Option<&str> {
match &self.git_ref {
Some(git_ref) if self.ref_is_commit() => Some(git_ref),
git_ref if self.ref_is_commit() => Some(git_ref),
_ => None,
}
}
fn symbolic_ref(&self) -> Option<&str> {
match &self.git_ref {
Some(git_ref) if !self.ref_is_commit() => Some(git_ref),
git_ref if !self.ref_is_commit() => Some(git_ref),
_ => None,
}
}
@ -226,7 +223,7 @@ impl UsesExt for Uses {
fn unpinned(&self) -> bool {
match self {
Uses::Docker(docker) => docker.hash.is_none() && docker.tag.is_none(),
Uses::Repository(repo) => repo.git_ref.is_none(),
Uses::Repository(_) => false,
// Local `uses:` are always unpinned; any `@ref` component
// is actually part of the path.
Uses::Local(_) => true,
@ -407,65 +404,54 @@ mod tests {
for (uses, pattern, matches) in [
// OK: case-insensitive, except subpath and tag
("actions/checkout@v3", "Actions/Checkout@v3", true),
("actions/checkout/foo", "actions/checkout/Foo", false),
("actions/checkout/foo@v3", "Actions/Checkout/foo", true),
("actions/checkout@v3", "actions/checkout@V3", false),
// NOT OK: owner/repo do not match
("actions/checkout@v3", "foo/checkout", false),
("actions/checkout@v3", "actions/bar", false),
// NOT OK: subpath does not match
("actions/checkout/foo", "actions/checkout", false),
("actions/checkout/foo@v3", "actions/checkout@v3", false),
// NOT OK: template is more specific than `uses:`
("actions/checkout", "actions/checkout@v3", false),
("actions/checkout/foo", "actions/checkout/foo@v3", false),
("actions/checkout@v3", "actions/checkout/foo@v3", false),
// owner/repo/subpath matches regardless of ref and casing
// but only when the subpath matches.
// the subpath must share the same case but might not be
// normalized
("actions/checkout/foo", "actions/checkout/foo", true),
("ACTIONS/CHECKOUT/foo", "actions/checkout/foo", true),
("actions/checkout/foo@v3", "actions/checkout/foo", true),
("ACTIONS/CHECKOUT/foo@v3", "actions/checkout/foo", true),
// TODO: See comment in `RepositoryUsesPattern::matches`
// ("ACTIONS/CHECKOUT/foo@v3", "actions/checkout/foo/", true),
// ("ACTIONS/CHECKOUT/foo@v3", "actions/checkout/foo//", true),
// ("ACTIONS/CHECKOUT//foo////@v3", "actions/checkout/foo", true),
("actions/checkout/FOO", "actions/checkout/foo", false),
("actions/checkout/foo/bar", "actions/checkout/foo", false),
// owner/repo matches regardless of ref and casing
// but does not match subpaths
("actions/checkout", "actions/checkout", true),
("ACTIONS/CHECKOUT", "actions/checkout", true),
("ACTIONS/CHECKOUT@v3", "actions/checkout", true),
("actions/checkout@v3", "actions/checkout", true),
("actions/checkout/foo@v3", "actions/checkout", false),
("actions/somethingelse", "actions/checkout", false),
("whatever/checkout", "actions/checkout", false),
("actions/somethingelse@v3", "actions/checkout", false),
("whatever/checkout@v3", "actions/checkout", false),
// owner/repo/* matches regardless of ref and casing
// including subpaths
// but does not match when owner diverges
("actions/checkout", "actions/checkout/*", true),
("ACTIONS/CHECKOUT", "actions/checkout/*", true),
("ACTIONS/CHECKOUT@v3", "actions/checkout/*", true),
("actions/checkout@v3", "actions/checkout/*", true),
("actions/checkout/foo@v3", "actions/checkout/*", true),
("actions/checkout/foo/bar@v3", "actions/checkout/*", true),
("someoneelse/checkout", "actions/checkout/*", false),
("someoneelse/checkout@v3", "actions/checkout/*", false),
// owner/* matches regardless of ref, casing, and subpath
// but rejects when owner diverges
("actions/checkout", "actions/*", true),
("ACTIONS/CHECKOUT", "actions/*", true),
("ACTIONS/CHECKOUT@v3", "actions/*", true),
("actions/checkout@v3", "actions/*", true),
("actions/checkout/foo@v3", "actions/*", true),
("someoneelse/checkout", "actions/*", false),
("someoneelse/checkout@v3", "actions/*", false),
// * matches everything
("actions/checkout", "*", true),
("actions/checkout@v3", "*", true),
("actions/checkout/foo@v3", "*", true),
("whatever/checkout", "*", true),
("whatever/checkout@v3", "*", true),
// exact matches
("actions/checkout@v3", "actions/checkout@v3", true),
("actions/checkout/foo@v3", "actions/checkout/foo@v3", true),
("actions/checkout/foo", "actions/checkout/foo@v3", false),
("actions/checkout/foo@v1", "actions/checkout/foo@v3", false),
] {
let Ok(Uses::Repository(uses)) = Uses::from_str(uses) else {
return Err(anyhow!("invalid uses: {uses}"));

View file

@ -5,11 +5,11 @@ use std::{
collections::{BTreeMap, btree_map},
fmt::Display,
process::ExitCode,
str::FromStr,
};
use anyhow::{Context, anyhow};
use camino::{Utf8Path, Utf8PathBuf};
use github_actions_models::common::RepositoryUses;
use indexmap::IndexMap;
use serde::Serialize;
use thiserror::Error;
@ -64,6 +64,49 @@ impl std::fmt::Display for InputKind {
}
}
/// A GitHub repository slug, i.e. `owner/repo[@ref]`.
#[derive(Debug)]
pub(crate) struct RepoSlug {
/// The owner of the repository.
pub(crate) owner: String,
/// The name of the repository.
pub(crate) repo: String,
/// An optional Git reference, e.g. a branch or tag name.
pub(crate) git_ref: Option<String>,
}
impl FromStr for RepoSlug {
type Err = anyhow::Error;
/// NOTE: This is almost exactly the same as
/// [`github_actions_models::common::RepositoryUses`],
/// except that we don't require a git ref and we forbid subpaths.
fn from_str(s: &str) -> Result<Self, Self::Err> {
let (path, git_ref) = match s.rsplit_once('@') {
Some((path, git_ref)) => (path, Some(git_ref)),
None => (s, None),
};
let components = path.splitn(2, '/').collect::<Vec<_>>();
match components.len() {
2 => Ok(Self {
owner: components[0].into(),
repo: components[1].into(),
git_ref: git_ref.map(|s| s.into()),
}),
x if x < 2 => Err(anyhow!(tips(
"invalid repo slug (too short)",
&["pass owner/repo or owner/repo@ref"]
))),
_ => Err(anyhow!(tips(
"invalid repo slug (too many parts)",
&["pass owner/repo or owner/repo@ref"]
))),
}
}
}
#[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, PartialOrd, Ord)]
pub(crate) struct LocalKey {
/// The path's nondeterministic prefix, if any.
@ -74,6 +117,7 @@ pub(crate) struct LocalKey {
#[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, PartialOrd, Ord)]
pub(crate) struct RemoteKey {
// TODO: Dedupe with RepoSlug above.
owner: String,
repo: String,
git_ref: Option<String>,
@ -126,7 +170,7 @@ impl InputKey {
}))
}
pub(crate) fn remote(slug: &RepositoryUses, path: String) -> Result<Self, InputError> {
pub(crate) fn remote(slug: &RepoSlug, path: String) -> Result<Self, InputError> {
if Utf8Path::new(&path).file_name().is_none() {
return Err(InputError::MissingName);
}
@ -464,7 +508,7 @@ impl<'a> FindingRegistry<'a> {
mod tests {
use std::str::FromStr;
use github_actions_models::common::Uses;
use crate::registry::RepoSlug;
use super::InputKey;
@ -474,9 +518,7 @@ mod tests {
assert_eq!(local.to_string(), "file:///foo/bar/baz.yml");
// No ref
let Uses::Repository(slug) = Uses::from_str("foo/bar").unwrap() else {
panic!()
};
let slug = RepoSlug::from_str("foo/bar").unwrap();
let remote = InputKey::remote(&slug, ".github/workflows/baz.yml".into()).unwrap();
assert_eq!(
remote.to_string(),
@ -484,9 +526,7 @@ mod tests {
);
// With a git ref
let Uses::Repository(slug) = Uses::from_str("foo/bar@v1").unwrap() else {
panic!()
};
let slug = RepoSlug::from_str("foo/bar@v1").unwrap();
let remote = InputKey::remote(&slug, ".github/workflows/baz.yml".into()).unwrap();
assert_eq!(
remote.to_string(),

View file

@ -444,7 +444,7 @@ impl<'a> ExtractedExpr<'a> {
// Returns the extracted expression exactly as it was extracted,
// including any fencing.
pub(crate) fn as_raw(&self) -> &str {
pub(crate) fn as_raw(&self) -> &'a str {
self.inner
}
}
@ -457,10 +457,7 @@ impl<'a> ExtractedExpr<'a> {
///
/// Adapted roughly from GitHub's `parseScalar`:
/// See: <https://github.com/actions/languageservices/blob/3a8c29c2d/workflow-parser/src/templates/template-reader.ts#L448>
fn extract_expression<'a>(
text: &'a str,
offset: usize,
) -> Option<(ExtractedExpr<'a>, Range<usize>)> {
fn extract_expression(text: &str, offset: usize) -> Option<(ExtractedExpr<'_>, Range<usize>)> {
let view = &text[offset..];
let start = view.find("${{")?;
@ -485,7 +482,7 @@ fn extract_expression<'a>(
}
/// Extract zero or more expressions from the given free-form text.
pub(crate) fn extract_expressions<'a>(text: &'a str) -> Vec<(ExtractedExpr<'a>, Range<usize>)> {
pub(crate) fn extract_expressions(text: &str) -> Vec<(ExtractedExpr<'_>, Range<usize>)> {
let mut exprs = vec![];
let mut offset = 0;
@ -508,9 +505,9 @@ pub(crate) fn extract_expressions<'a>(text: &'a str) -> Vec<(ExtractedExpr<'a>,
/// Unlike `extract_expressions`, this function performs some semantic
/// filtering over the raw input. For example, it skip ignore expressions
/// that are inside comments.
pub(crate) fn parse_expressions_from_input<'doc>(
input: &'doc AuditInput,
) -> Vec<(ExtractedExpr<'doc>, Range<usize>)> {
pub(crate) fn parse_expressions_from_input(
input: &AuditInput,
) -> Vec<(ExtractedExpr<'_>, Range<usize>)> {
let text = input.as_document().source();
let doc = input.as_document();

View file

@ -190,30 +190,20 @@ fn audit_unpinned_uses() -> anyhow::Result<()> {
let execution = zizmor().args(cli_args).output()?;
assert_eq!(execution.status.code(), Some(14));
assert_eq!(execution.status.code(), Some(13));
let findings = serde_json::from_slice(&execution.stdout)?;
assert_value_match(&findings, "$[0].determinations.confidence", "High");
assert_value_match(&findings, "$[0].determinations.severity", "High");
assert_value_match(&findings, "$[0].determinations.severity", "Medium");
assert_value_match(
&findings,
"$[0].locations[0].concrete.feature",
"uses: actions/checkout",
);
assert_value_match(
&findings,
"$[1].locations[0].concrete.feature",
"uses: github/codeql-action/upload-sarif",
);
assert_value_match(
&findings,
"$[2].locations[0].concrete.feature",
"uses: docker://ubuntu",
);
assert_value_match(
&findings,
"$[3].locations[0].concrete.feature",
"$[1].locations[0].concrete.feature",
"uses: docker://ghcr.io/pypa/gh-action-pypi-publish",
);

View file

@ -15,7 +15,7 @@ fn gha_hazmat() -> Result<()> {
.offline(false)
.output(OutputMode::Both)
.args(["--no-online-audits"])
.input("woodruffw/gha-hazmat@33cd22cdd7823a5795768388aff977fe992b5aad")
.input("woodruffw/gha-hazmat@83e7e24df76fe8b5c0a1748b6fb24107a0e4fa61")
.run()?
);
Ok(())

View file

@ -1,6 +1,6 @@
---
source: crates/zizmor/tests/integration/e2e.rs
expression: "zizmor().offline(false).output(OutputMode::Both).args([\"--no-online-audits\"]).input(\"woodruffw/gha-hazmat@33cd22cdd7823a5795768388aff977fe992b5aad\").run()?"
expression: "zizmor().offline(false).output(OutputMode::Both).args([\"--no-online-audits\"]).input(\"woodruffw/gha-hazmat@83e7e24df76fe8b5c0a1748b6fb24107a0e4fa61\").run()?"
snapshot_kind: text
---
INFO collect_inputs: zizmor: collected 22 inputs from woodruffw/gha-hazmat
@ -302,21 +302,6 @@ error[excessive-permissions]: overly broad permissions
|
= note: audit confidence → High
warning[excessive-permissions]: overly broad permissions
--> .github/workflows/github-env.yml:24:3
|
24 | / vulnerable:
25 | | runs-on: ubuntu-latest
... |
33 | | env:
34 | | TITLE: ${{ github.event.pull_request.title }}
| | -
| |________________________________________________________|
| this job
| default permissions used due to no permissions: block
|
= note: audit confidence → Medium
error[dangerous-triggers]: use of fundamentally insecure workflow trigger
--> .github/workflows/github-env.yml:19:1
|
@ -328,12 +313,12 @@ error[dangerous-triggers]: use of fundamentally insecure workflow trigger
= note: audit confidence → Medium
error[github-env]: dangerous use of environment file
--> .github/workflows/github-env.yml:30:9
--> .github/workflows/github-env.yml:32:9
|
30 | - run: |
32 | - run: |
| _________^
31 | | message=$(echo "$TITLE" | grep -oP '[{\[][^}\]]+[}\]]' | sed 's/{\|}\|\[\|\]//g')
32 | | echo "message=$message" >> $GITHUB_ENV
33 | | message=$(echo "$TITLE" | grep -oP '[{\[][^}\]]+[}\]]' | sed 's/{\|}\|\[\|\]//g')
34 | | echo "message=$message" >> $GITHUB_ENV
| |________________________________________________^ write to GITHUB_ENV may allow code execution
|
= note: audit confidence → Low
@ -678,25 +663,10 @@ error[unpinned-uses]: unpinned action reference
|
= note: audit confidence → High
warning[excessive-permissions]: overly broad permissions
--> .github/workflows/ref-confusion.yml:20:3
|
20 | / commit:
21 | | runs-on: ubuntu-latest
22 | | steps:
23 | | # NOT OK: `confusable` is both a tag and a branch
24 | | - uses: woodruffw/gha-hazmat/ref-confusion@confusable
| | -
| |____________________________________________________________|
| this job
| default permissions used due to no permissions: block
|
= note: audit confidence → Medium
error[unpinned-uses]: unpinned action reference
--> .github/workflows/ref-confusion.yml:24:9
--> .github/workflows/ref-confusion.yml:26:9
|
24 | - uses: woodruffw/gha-hazmat/ref-confusion@confusable
26 | - uses: woodruffw/gha-hazmat/ref-confusion@confusable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
@ -780,158 +750,71 @@ warning[secrets-inherit]: secrets unconditionally inherited by called workflow
|
= note: audit confidence → High
warning[excessive-permissions]: overly broad permissions
--> .github/workflows/self-hosted.yml:22:3
|
22 | / vulnerable:
23 | | # NOT OK: self-hosted runners are difficult to secure in public repos
... |
27 | | - run: |
28 | | echo "hello from a self-hosted runner"
| | -
| |_________________________________________________|
| this job
| default permissions used due to no permissions: block
|
= note: audit confidence → Medium
warning[excessive-permissions]: overly broad permissions
--> .github/workflows/template-injection.yml:20:1
|
20 | / name: example
21 | | on:
... |
127 | | run: |
128 | | ${{ some.context == 'success' }}
| |___________________________________________- default permissions used due to no permissions: block
|
= note: audit confidence → Medium
warning[excessive-permissions]: overly broad permissions
--> .github/workflows/template-injection.yml:36:3
|
36 | / vulnerable-1:
37 | | runs-on: ubuntu-latest
... |
94 | | run: |
95 | | echo "doing a thing: ${{ github.workspace }}"
| | -
| |_______________________________________________________|
| this job
| default permissions used due to no permissions: block
|
= note: audit confidence → Medium
warning[excessive-permissions]: overly broad permissions
--> .github/workflows/template-injection.yml:97:3
|
97 | / vulnerable-2:
98 | | runs-on: ubuntu-latest
... |
106 | | run: |
107 | | echo "doing a thing: ${{ matrix.unknown-key }}"
| | -
| |_________________________________________________________|
| this job
| default permissions used due to no permissions: block
|
= note: audit confidence → Medium
warning[excessive-permissions]: overly broad permissions
--> .github/workflows/template-injection.yml:110:3
|
110 | / vulnerable-3:
111 | | runs-on: ubuntu-latest
... |
118 | | script: |
119 | | return "doing a thing: ${{ github.event.issue.title }}"
| | -
| |___________________________________________________________________|
| this job
| default permissions used due to no permissions: block
|
= note: audit confidence → Medium
warning[excessive-permissions]: overly broad permissions
--> .github/workflows/template-injection.yml:121:3
|
121 | / not-vulnerable-4:
122 | | runs-on: ubuntu-latest
... |
127 | | run: |
128 | | ${{ some.context == 'success' }}
| | -
| |___________________________________________|
| this job
| default permissions used due to no permissions: block
|
= note: audit confidence → Medium
error[template-injection]: code injection via template expansion
--> .github/workflows/template-injection.yml:48:36
--> .github/workflows/template-injection.yml:50:36
|
47 | run: |
49 | run: |
| ^^^ this run block
48 | echo "issue created: ${{ github.event.issue.title }}"
50 | echo "issue created: ${{ github.event.issue.title }}"
| ^^^^^^^^^^^^^^^^^^^^^^^^ may expand into attacker-controllable code
|
= note: audit confidence → High
= note: this finding has an auto-fix
error[template-injection]: code injection via template expansion
--> .github/workflows/template-injection.yml:53:36
--> .github/workflows/template-injection.yml:55:36
|
52 | run: |
54 | run: |
| ^^^ this run block
53 | echo "doing a thing: ${{ inputs.hackme }}"
55 | echo "doing a thing: ${{ inputs.hackme }}"
| ^^^^^^^^^^^^^ may expand into attacker-controllable code
|
= note: audit confidence → High
= note: this finding has an auto-fix
error[template-injection]: code injection via template expansion
--> .github/workflows/template-injection.yml:63:36
--> .github/workflows/template-injection.yml:65:36
|
62 | run: |
64 | run: |
| ^^^ this run block
63 | echo "doing a thing: ${{ inputs.hackme-call }}"
65 | echo "doing a thing: ${{ inputs.hackme-call }}"
| ^^^^^^^^^^^^^^^^^^ may expand into attacker-controllable code
|
= note: audit confidence → High
= note: this finding has an auto-fix
warning[template-injection]: code injection via template expansion
--> .github/workflows/template-injection.yml:85:36
--> .github/workflows/template-injection.yml:87:36
|
84 | run: |
86 | run: |
| --- this run block
85 | echo "doing a thing: ${{ matrix.dynamic }}"
87 | echo "doing a thing: ${{ matrix.dynamic }}"
| -------------- may expand into attacker-controllable code
|
= note: audit confidence → Medium
= note: this finding has an auto-fix
warning[template-injection]: code injection via template expansion
--> .github/workflows/template-injection.yml:107:36
--> .github/workflows/template-injection.yml:109:36
|
106 | run: |
108 | run: |
| --- this run block
107 | echo "doing a thing: ${{ matrix.unknown-key }}"
109 | echo "doing a thing: ${{ matrix.unknown-key }}"
| ------------------ may expand into attacker-controllable code
|
= note: audit confidence → Medium
= note: this finding has an auto-fix
error[template-injection]: code injection via template expansion
--> .github/workflows/template-injection.yml:119:40
--> .github/workflows/template-injection.yml:120:40
|
115 | uses: actions/github-script@v7
116 | uses: actions/github-script@v7
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action accepts arbitrary code
116 | with:
117 | # NOT OK: attacker-controlled issue title
118 | script: |
117 | with:
118 | # NOT OK: attacker-controlled issue title
119 | script: |
| ^^^^^^ via this input
119 | return "doing a thing: ${{ github.event.issue.title }}"
120 | return "doing a thing: ${{ github.event.issue.title }}"
| ^^^^^^^^^^^^^^^^^^^^^^^^ may expand into attacker-controllable code
|
= note: audit confidence → High
@ -974,42 +857,27 @@ warning[excessive-permissions]: overly broad permissions
16 | / unpinned-0:
17 | | runs-on: ubuntu-latest
... |
37 | | args: hello!
38 | |
| |_-- this job
| |
| default permissions used due to no permissions: block
28 | | entrypoint: /bin/echo
29 | | args: hello!
| | -
| |_______________________|
| this job
| default permissions used due to no permissions: block
|
= note: audit confidence → Medium
error[unpinned-uses]: unpinned action reference
warning[unpinned-uses]: unpinned action reference
--> .github/workflows/unpinned.yml:20:9
|
20 | - uses: actions/checkout
| ^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a ref or hash (required by actions/* policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> .github/workflows/unpinned.yml:25:9
|
25 | - uses: github/codeql-action/upload-sarif
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a ref or hash (required by github/* policy)
|
= note: audit confidence → High
warning[unpinned-uses]: unpinned action reference
--> .github/workflows/unpinned.yml:28:9
|
28 | - uses: docker://ubuntu
20 | - uses: docker://ubuntu
| --------------------- action is not pinned to a tag, branch, or hash ref
|
= note: audit confidence → High
warning[unpinned-uses]: unpinned action reference
--> .github/workflows/unpinned.yml:34:9
--> .github/workflows/unpinned.yml:26:9
|
34 | - uses: docker://ghcr.io/pypa/gh-action-pypi-publish
26 | - uses: docker://ghcr.io/pypa/gh-action-pypi-publish
| -------------------------------------------------- action is not pinned to a tag, branch, or hash ref
|
= note: audit confidence → High
@ -1096,4 +964,4 @@ error[dangerous-triggers]: use of fundamentally insecure workflow trigger
|
= note: audit confidence → Medium
172 findings (77 suppressed): 0 unknown, 6 informational, 0 low, 37 medium, 52 high
159 findings (74 suppressed): 0 unknown, 6 informational, 0 low, 29 medium, 50 high

View file

@ -1,6 +1,7 @@
---
source: crates/zizmor/tests/integration/snapshot.rs
expression: "zizmor().input(input_under_test(\"template-injection/pr-425-backstop/action.yml\")).run()?"
snapshot_kind: text
---
error[template-injection]: code injection via template expansion
--> @@INPUT@@:14:19
@ -42,8 +43,8 @@ error[template-injection]: code injection via template expansion
error[template-injection]: code injection via template expansion
--> @@INPUT@@:31:56
|
29 | uses: azure/powershell
| ^^^^^^^^^^^^^^^^^^^^^^ action accepts arbitrary code
29 | uses: azure/powershell@whatever
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action accepts arbitrary code
30 | with:
31 | inlineScript: Get-AzVM -ResourceGroupName "${{ inputs.expandme }}"
| ^^^^^^^^^^^^ via this input ^^^^^^^^^^^^^^^ may expand into attacker-controllable code
@ -53,8 +54,8 @@ error[template-injection]: code injection via template expansion
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:29:7
|
29 | uses: azure/powershell
| ^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
29 | uses: azure/powershell@whatever
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High

View file

@ -1,29 +1,22 @@
---
source: crates/zizmor/tests/integration/snapshot.rs
expression: "zizmor().config(input_under_test(\"unpinned-uses/configs/composite-2.yml\")).input(input_under_test(\"unpinned-uses/menagerie-of-uses.yml\")).run()?"
snapshot_kind: text
---
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:14:9
--> @@INPUT@@:24:9
|
14 | - uses: actions/checkout
| ^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a ref or hash (required by actions/* policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:28:9
|
28 | - uses: github/codeql-action/init@v3
24 | - uses: github/codeql-action/init@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by github/codeql-action/init policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:30:9
--> @@INPUT@@:26:9
|
30 | - uses: github/codeql-action/upload-sarif@v3
26 | - uses: github/codeql-action/upload-sarif@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by github/codeql-action/upload-sarif policy)
|
= note: audit confidence → High
3 findings: 0 unknown, 0 informational, 0 low, 0 medium, 3 high
2 findings: 0 unknown, 0 informational, 0 low, 0 medium, 2 high

View file

@ -1,6 +1,7 @@
---
source: crates/zizmor/tests/integration/snapshot.rs
expression: "zizmor().config(input_under_test(\"unpinned-uses/configs/composite.yml\")).input(input_under_test(\"unpinned-uses/menagerie-of-uses.yml\")).run()?"
snapshot_kind: text
---
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:12:9
@ -11,17 +12,17 @@ error[unpinned-uses]: unpinned action reference
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:28:9
--> @@INPUT@@:24:9
|
28 | - uses: github/codeql-action/init@v3
24 | - uses: github/codeql-action/init@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:30:9
--> @@INPUT@@:26:9
|
30 | - uses: github/codeql-action/upload-sarif@v3
26 | - uses: github/codeql-action/upload-sarif@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High

View file

@ -1,21 +1,14 @@
---
source: crates/zizmor/tests/integration/snapshot.rs
expression: "zizmor().input(input_under_test(\"unpinned-uses/menagerie-of-uses.yml\")).run()?"
snapshot_kind: text
---
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:14:9
--> @@INPUT@@:22:9
|
14 | - uses: actions/checkout
| ^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a ref or hash (required by actions/* policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:26:9
|
26 | - uses: pypa/gh-action-pypi-publish@release/v1
22 | - uses: pypa/gh-action-pypi-publish@release/v1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
2 findings: 0 unknown, 0 informational, 0 low, 0 medium, 2 high
1 finding: 0 unknown, 0 informational, 0 low, 0 medium, 1 high

View file

@ -1,6 +1,7 @@
---
source: crates/zizmor/tests/integration/snapshot.rs
expression: "zizmor().config(input_under_test(\"unpinned-uses/configs/empty.yml\")).input(input_under_test(\"unpinned-uses/menagerie-of-uses.yml\")).run()?"
snapshot_kind: text
---
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:12:9
@ -13,41 +14,33 @@ error[unpinned-uses]: unpinned action reference
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:14:9
|
14 | - uses: actions/checkout
| ^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
14 | - uses: actions/checkout@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:18:9
--> @@INPUT@@:22:9
|
18 | - uses: actions/checkout@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
22 | - uses: pypa/gh-action-pypi-publish@release/v1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:24:9
|
24 | - uses: github/codeql-action/init@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:26:9
|
26 | - uses: pypa/gh-action-pypi-publish@release/v1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:28:9
|
28 | - uses: github/codeql-action/init@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:30:9
|
30 | - uses: github/codeql-action/upload-sarif@v3
26 | - uses: github/codeql-action/upload-sarif@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
6 findings: 0 unknown, 0 informational, 0 low, 0 medium, 6 high
5 findings: 0 unknown, 0 informational, 0 low, 0 medium, 5 high

View file

@ -1,6 +1,7 @@
---
source: crates/zizmor/tests/integration/snapshot.rs
expression: "zizmor().config(input_under_test(\"unpinned-uses/configs/hash-pin-everything.yml\")).input(input_under_test(\"unpinned-uses/menagerie-of-uses.yml\")).run()?"
snapshot_kind: text
---
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:12:9
@ -13,41 +14,33 @@ error[unpinned-uses]: unpinned action reference
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:14:9
|
14 | - uses: actions/checkout
| ^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
14 | - uses: actions/checkout@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:18:9
--> @@INPUT@@:22:9
|
18 | - uses: actions/checkout@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
22 | - uses: pypa/gh-action-pypi-publish@release/v1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:24:9
|
24 | - uses: github/codeql-action/init@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:26:9
|
26 | - uses: pypa/gh-action-pypi-publish@release/v1
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:28:9
|
28 | - uses: github/codeql-action/init@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:30:9
|
30 | - uses: github/codeql-action/upload-sarif@v3
26 | - uses: github/codeql-action/upload-sarif@v3
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a hash (required by blanket policy)
|
= note: audit confidence → High
6 findings: 0 unknown, 0 informational, 0 low, 0 medium, 6 high
5 findings: 0 unknown, 0 informational, 0 low, 0 medium, 5 high

View file

@ -1,13 +1,6 @@
---
source: crates/zizmor/tests/integration/snapshot.rs
expression: "zizmor().config(input_under_test(\"unpinned-uses/configs/ref-pin-everything.yml\")).input(input_under_test(\"unpinned-uses/menagerie-of-uses.yml\")).run()?"
snapshot_kind: text
---
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:14:9
|
14 | - uses: actions/checkout
| ^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a ref or hash (required by blanket policy)
|
= note: audit confidence → High
1 finding: 0 unknown, 0 informational, 0 low, 0 medium, 1 high
No findings to report. Good job!

View file

@ -1,37 +1,22 @@
---
source: crates/zizmor/tests/integration/snapshot.rs
expression: "zizmor().input(input_under_test(\"unpinned-uses.yml\")).run()?"
snapshot_kind: text
---
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:12:9
|
12 | - uses: actions/checkout
| ^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a ref or hash (required by actions/* policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:22:9
|
22 | - uses: github/codeql-action/upload-sarif
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a ref or hash (required by github/* policy)
|
= note: audit confidence → High
warning[unpinned-uses]: unpinned action reference
--> @@INPUT@@:25:9
--> @@INPUT@@:17:9
|
25 | - uses: docker://ubuntu
17 | - uses: docker://ubuntu
| --------------------- action is not pinned to a tag, branch, or hash ref
|
= note: audit confidence → High
warning[unpinned-uses]: unpinned action reference
--> @@INPUT@@:31:9
--> @@INPUT@@:23:9
|
31 | - uses: docker://ghcr.io/pypa/gh-action-pypi-publish
23 | - uses: docker://ghcr.io/pypa/gh-action-pypi-publish
| -------------------------------------------------- action is not pinned to a tag, branch, or hash ref
|
= note: audit confidence → High
4 findings: 0 unknown, 0 informational, 0 low, 2 medium, 2 high
2 findings: 0 unknown, 0 informational, 0 low, 2 medium, 0 high

View file

@ -1,37 +1,22 @@
---
source: crates/zizmor/tests/integration/snapshot.rs
expression: "zizmor().input(input_under_test(\"unpinned-uses.yml\")).args([\"--pedantic\"]).run()?"
snapshot_kind: text
---
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:12:9
|
12 | - uses: actions/checkout
| ^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a ref or hash (required by actions/* policy)
|
= note: audit confidence → High
error[unpinned-uses]: unpinned action reference
--> @@INPUT@@:22:9
|
22 | - uses: github/codeql-action/upload-sarif
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ action is not pinned to a ref or hash (required by github/* policy)
|
= note: audit confidence → High
warning[unpinned-uses]: unpinned action reference
--> @@INPUT@@:25:9
--> @@INPUT@@:17:9
|
25 | - uses: docker://ubuntu
17 | - uses: docker://ubuntu
| --------------------- action is not pinned to a tag, branch, or hash ref
|
= note: audit confidence → High
warning[unpinned-uses]: unpinned action reference
--> @@INPUT@@:31:9
--> @@INPUT@@:23:9
|
31 | - uses: docker://ghcr.io/pypa/gh-action-pypi-publish
23 | - uses: docker://ghcr.io/pypa/gh-action-pypi-publish
| -------------------------------------------------- action is not pinned to a tag, branch, or hash ref
|
= note: audit confidence → High
4 findings: 0 unknown, 0 informational, 0 low, 2 medium, 2 high
2 findings: 0 unknown, 0 informational, 0 low, 2 medium, 0 high

View file

@ -34,4 +34,4 @@ jobs:
unpinned-uses-ignored:
runs-on: ubuntu-latest
steps:
- uses: github/codeql-action/upload-sarif # zizmor: ignore[unpinned-uses]
- uses: something/random@v1 # zizmor: ignore[unpinned-uses]

View file

@ -26,6 +26,6 @@ runs:
echo "hello ${{ inputs.expandme }}"
- name: case4
uses: azure/powershell
uses: azure/powershell@whatever
with:
inlineScript: Get-AzVM -ResourceGroupName "${{ inputs.expandme }}"

View file

@ -8,19 +8,11 @@ jobs:
name: unpinned-0
runs-on: ubuntu-latest
steps:
# NOT OK: unpinned
- uses: actions/checkout
with:
persist-credentials: false
# PEDANTIC: pinned but unhashed
- uses: actions/checkout@v3
with:
persist-credentials: false
# NOT OK: unpinned
- uses: github/codeql-action/upload-sarif
# NOT OK: unpinned
- uses: docker://ubuntu
with:

View file

@ -11,10 +11,6 @@ jobs:
steps:
- uses: actions/setup-python@v4
- uses: actions/checkout
with:
persist-credentials: false
- uses: actions/checkout@v3
with:
persist-credentials: false

View file

@ -1209,8 +1209,7 @@ actions should be pinned by SHA reference.
By default, this audit applies the following policy:
* Official GitHub actions namespaces can be pinned by branch or tag.
In other words, `actions/checkout@v4` is acceptable, but `actions/checkout`
is not.
In other words, `actions/checkout@v4` is acceptable.
* All other actions must be pinned by SHA reference.
This audit can be configured with a custom set of rules, e.g. to
@ -1256,6 +1255,12 @@ The valid policies are:
* `any`: no pinning is required for any `#!yaml uses:` clauses that match the associated
pattern.
!!! tip
For repository `#!yaml uses` clauses like `#!yaml uses: actions/checkout@v4`
this is equivalent to `ref-pin`, as GitHub Actions does not permit
completely unpinned repository actions.
If a `#!yaml uses:` clauses matches multiple rules, the most specific one is used
regardless of definition order.
@ -1332,7 +1337,7 @@ For Docker actions (like `docker://ubuntu`): add an appropriate
unpinned-uses:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout
- uses: pypa/gh-action-pypi-publish@v1.12.4
with:
persist-credentials: false
@ -1352,7 +1357,7 @@ For Docker actions (like `docker://ubuntu`): add an appropriate
unpinned-uses:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- uses: pypa/gh-action-pypi-publish@76f52bc884231f62b9a034ebfe128415bbaabdfc # v1.12.4
with:
persist-credentials: false

View file

@ -12,6 +12,11 @@ of `zizmor`.
### Enhancements 🌱
* The [cache-poisoning] audit now supports auto-fixes for many findings (#923)
* The [known-vulnerable-actions] audit now supports auto-fixes for many findings
(#1019)
* `zizmor` is now stricter about parsing `uses:` clauses. In particular,
`zizmor` will no longer accept `uses: org/repo` without a trailing
`@ref`, as GitHub Actions itself does not accept this syntax (#1019)
### Bug Fixes 🐛