feat: location subspans/subfeatures (#949)

This commit is contained in:
William Woodruff 2025-06-19 16:41:39 -04:00 committed by GitHub
parent 64f9be57c9
commit f03869f52b
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
18 changed files with 484 additions and 260 deletions

View file

@ -39,6 +39,8 @@ jobs:
- name: Test dependencies
run: |
# Don't waste time on man-db updates
sudo apt-get remove --purge man-db
# Needed for tty-tests
sudo apt install -y expect

View file

@ -169,15 +169,18 @@ impl From<std::ops::Range<usize>> for Span {
pub struct SpannedExpr<'src> {
/// The expression's source span.
pub span: Span,
/// The expression's unparsed form.
pub raw: &'src str,
/// The expression itself.
pub inner: Expr<'src>,
}
impl<'a> SpannedExpr<'a> {
/// Creates a new `SpannedExpr` from an expression and its span.
pub(crate) fn new(span: impl Into<Span>, inner: Expr<'a>) -> Self {
pub(crate) fn new(span: impl Into<Span>, raw: &'a str, inner: Expr<'a>) -> Self {
Self {
inner,
raw,
span: span.into(),
}
}
@ -367,7 +370,7 @@ impl<'src> Expr<'src> {
}
/// Parses the given string into an expression.
pub fn parse(expr: &str) -> Result<SpannedExpr> {
pub fn parse(expr: &'src str) -> Result<SpannedExpr<'src>> {
// Top level `expression` is a single `or_expr`.
let or_expr = ExprParser::parse(Rule::expression, expr)?
.next()
@ -390,12 +393,13 @@ impl<'src> Expr<'src> {
match pair.as_rule() {
Rule::or_expr => {
let span = pair.as_span();
let (span, raw) = (pair.as_span(), pair.as_str());
let mut pairs = pair.into_inner();
let lhs = parse_pair(pairs.next().unwrap())?;
pairs.try_fold(lhs, |expr, next| {
Ok(SpannedExpr::new(
span,
raw,
Expr::BinOp {
lhs: expr,
op: BinOp::Or,
@ -406,12 +410,13 @@ impl<'src> Expr<'src> {
})
}
Rule::and_expr => {
let span = pair.as_span();
let (span, raw) = (pair.as_span(), pair.as_str());
let mut pairs = pair.into_inner();
let lhs = parse_pair(pairs.next().unwrap())?;
pairs.try_fold(lhs, |expr, next| {
Ok(SpannedExpr::new(
span,
raw,
Expr::BinOp {
lhs: expr,
op: BinOp::And,
@ -425,7 +430,7 @@ impl<'src> Expr<'src> {
// eq_expr matches both `==` and `!=` and captures
// them in the `eq_op` capture, so we fold with
// two-tuples of (eq_op, comp_expr).
let span = pair.as_span();
let (span, raw) = (pair.as_span(), pair.as_str());
let mut pairs = pair.into_inner();
let lhs = parse_pair(pairs.next().unwrap())?;
@ -442,6 +447,7 @@ impl<'src> Expr<'src> {
Ok(SpannedExpr::new(
span,
raw,
Expr::BinOp {
lhs: expr,
op: eq_op,
@ -453,7 +459,7 @@ impl<'src> Expr<'src> {
}
Rule::comp_expr => {
// Same as eq_expr, but with comparison operators.
let span = pair.as_span();
let (span, raw) = (pair.as_span(), pair.as_str());
let mut pairs = pair.into_inner();
let lhs = parse_pair(pairs.next().unwrap())?;
@ -472,6 +478,7 @@ impl<'src> Expr<'src> {
Ok(SpannedExpr::new(
span,
raw,
Expr::BinOp {
lhs: expr,
op: eq_op,
@ -482,20 +489,21 @@ impl<'src> Expr<'src> {
})
}
Rule::unary_expr => {
let span = pair.as_span();
let (span, raw) = (pair.as_span(), pair.as_str());
let mut pairs = pair.into_inner();
let pair = pairs.next().unwrap();
let inner_pair = pairs.next().unwrap();
match pair.as_rule() {
match inner_pair.as_rule() {
Rule::unary_op => Ok(SpannedExpr::new(
span,
raw,
Expr::UnOp {
op: UnOp::Not,
expr: parse_pair(pairs.next().unwrap())?,
},
)
.into()),
Rule::primary_expr => parse_pair(pair),
Rule::primary_expr => parse_pair(inner_pair),
_ => unreachable!(),
}
}
@ -505,33 +513,43 @@ impl<'src> Expr<'src> {
}
Rule::number => Ok(SpannedExpr::new(
pair.as_span(),
pair.as_str(),
pair.as_str().parse::<f64>().unwrap().into(),
)
.into()),
Rule::string => {
let (span, raw) = (pair.as_span(), pair.as_str());
// string -> string_inner
let span = pair.as_span();
let string_inner = pair.into_inner().next().unwrap().as_str();
// Optimization: if our string literal doesn't have any
// escaped quotes in it, we can save ourselves a clone.
if !string_inner.contains('\'') {
Ok(SpannedExpr::new(span, string_inner.into()).into())
Ok(SpannedExpr::new(span, raw, string_inner.into()).into())
} else {
Ok(SpannedExpr::new(span, string_inner.replace("''", "'").into()).into())
Ok(
SpannedExpr::new(span, raw, string_inner.replace("''", "'").into())
.into(),
)
}
}
Rule::boolean => Ok(SpannedExpr::new(
pair.as_span(),
pair.as_str(),
pair.as_str().parse::<bool>().unwrap().into(),
)
.into()),
Rule::null => {
Ok(SpannedExpr::new(pair.as_span(), Expr::Literal(Literal::Null)).into())
Rule::null => Ok(SpannedExpr::new(
pair.as_span(),
pair.as_str(),
Expr::Literal(Literal::Null),
)
.into()),
Rule::star => {
Ok(SpannedExpr::new(pair.as_span(), pair.as_str(), Expr::Star).into())
}
Rule::star => Ok(SpannedExpr::new(pair.as_span(), Expr::Star).into()),
Rule::function_call => {
let span = pair.as_span();
let (span, raw) = (pair.as_span(), pair.as_str());
let mut pairs = pair.into_inner();
let identifier = pairs.next().unwrap();
@ -541,6 +559,7 @@ impl<'src> Expr<'src> {
Ok(SpannedExpr::new(
span,
raw,
Expr::Call {
func: Function(identifier.as_str()),
args,
@ -549,16 +568,19 @@ impl<'src> Expr<'src> {
.into())
}
Rule::identifier => {
Ok(SpannedExpr::new(pair.as_span(), Expr::ident(pair.as_str())).into())
Ok(
SpannedExpr::new(pair.as_span(), pair.as_str(), Expr::ident(pair.as_str()))
.into(),
)
}
Rule::index => Ok(SpannedExpr::new(
pair.as_span(),
pair.as_str(),
Expr::Index(parse_pair(pair.into_inner().next().unwrap())?),
)
.into()),
Rule::context => {
let span = pair.as_span();
let raw = pair.as_str();
let (span, raw) = (pair.as_span(), pair.as_str());
let pairs = pair.into_inner();
let mut inner: Vec<SpannedExpr> = pairs
@ -571,7 +593,7 @@ impl<'src> Expr<'src> {
if inner.len() == 1 && matches!(inner[0].inner, Expr::Call { .. }) {
Ok(inner.remove(0).into())
} else {
Ok(SpannedExpr::new(span, Expr::context(raw, inner)).into())
Ok(SpannedExpr::new(span, raw, Expr::context(raw, inner)).into())
}
}
r => panic!("unrecognized rule: {r:?}"),
@ -803,50 +825,66 @@ mod tests {
"!true || false || true",
SpannedExpr::new(
0..22,
"!true || false || true",
Expr::BinOp {
lhs: SpannedExpr::new(
0..22,
"!true || false || true",
Expr::BinOp {
lhs: SpannedExpr::new(
0..5,
"!true",
Expr::UnOp {
op: UnOp::Not,
expr: SpannedExpr::new(1..5, true.into()).into(),
expr: SpannedExpr::new(1..5, "true", true.into()).into(),
},
)
.into(),
op: BinOp::Or,
rhs: SpannedExpr::new(9..14, false.into()).into(),
rhs: SpannedExpr::new(9..14, "false", false.into()).into(),
},
)
.into(),
op: BinOp::Or,
rhs: SpannedExpr::new(18..22, true.into()).into(),
rhs: SpannedExpr::new(18..22, "true", true.into()).into(),
},
),
),
(
"'foo '' bar'",
SpannedExpr::new(0..12, Expr::Literal(Literal::String("foo ' bar".into()))),
SpannedExpr::new(
0..12,
"'foo '' bar'",
Expr::Literal(Literal::String("foo ' bar".into())),
),
),
(
"('foo '' bar')",
SpannedExpr::new(1..13, Expr::Literal(Literal::String("foo ' bar".into()))),
SpannedExpr::new(
1..13,
"'foo '' bar'",
Expr::Literal(Literal::String("foo ' bar".into())),
),
),
(
"((('foo '' bar')))",
SpannedExpr::new(3..15, Expr::Literal(Literal::String("foo ' bar".into()))),
SpannedExpr::new(
3..15,
"'foo '' bar'",
Expr::Literal(Literal::String("foo ' bar".into())),
),
),
(
"foo(1, 2, 3)",
SpannedExpr::new(
0..12,
"foo(1, 2, 3)",
Expr::Call {
func: Function("foo"),
args: vec![
SpannedExpr::new(4..5, 1.0.into()),
SpannedExpr::new(7..8, 2.0.into()),
SpannedExpr::new(10..11, 3.0.into()),
SpannedExpr::new(4..5, "1", 1.0.into()),
SpannedExpr::new(7..8, "2", 2.0.into()),
SpannedExpr::new(10..11, "3", 3.0.into()),
],
},
),
@ -855,12 +893,13 @@ mod tests {
"foo.bar.baz",
SpannedExpr::new(
0..11,
"foo.bar.baz",
Expr::context(
"foo.bar.baz",
vec![
SpannedExpr::new(0..3, Expr::ident("foo")),
SpannedExpr::new(4..7, Expr::ident("bar")),
SpannedExpr::new(8..11, Expr::ident("baz")),
SpannedExpr::new(0..3, "foo", Expr::ident("foo")),
SpannedExpr::new(4..7, "bar", Expr::ident("bar")),
SpannedExpr::new(8..11, "baz", Expr::ident("baz")),
],
),
),
@ -869,19 +908,22 @@ mod tests {
"foo.bar.baz[1][2]",
SpannedExpr::new(
0..17,
"foo.bar.baz[1][2]",
Expr::context(
"foo.bar.baz[1][2]",
vec![
SpannedExpr::new(0..3, Expr::ident("foo")),
SpannedExpr::new(4..7, Expr::ident("bar")),
SpannedExpr::new(8..11, Expr::ident("baz")),
SpannedExpr::new(0..3, "foo", Expr::ident("foo")),
SpannedExpr::new(4..7, "bar", Expr::ident("bar")),
SpannedExpr::new(8..11, "baz", Expr::ident("baz")),
SpannedExpr::new(
11..14,
Expr::Index(Box::new(SpannedExpr::new(12..13, 1.0.into()))),
"[1]",
Expr::Index(Box::new(SpannedExpr::new(12..13, "1", 1.0.into()))),
),
SpannedExpr::new(
14..17,
Expr::Index(Box::new(SpannedExpr::new(15..16, 2.0.into()))),
"[2]",
Expr::Index(Box::new(SpannedExpr::new(15..16, "2", 2.0.into()))),
),
],
),
@ -891,15 +933,17 @@ mod tests {
"foo.bar.baz[*]",
SpannedExpr::new(
0..14,
"foo.bar.baz[*]",
Expr::context(
"foo.bar.baz[*]",
[
SpannedExpr::new(0..3, Expr::ident("foo")),
SpannedExpr::new(4..7, Expr::ident("bar")),
SpannedExpr::new(8..11, Expr::ident("baz")),
SpannedExpr::new(0..3, "foo", Expr::ident("foo")),
SpannedExpr::new(4..7, "bar", Expr::ident("bar")),
SpannedExpr::new(8..11, "baz", Expr::ident("baz")),
SpannedExpr::new(
11..14,
Expr::Index(Box::new(SpannedExpr::new(12..13, Expr::Star))),
"[*]",
Expr::Index(Box::new(SpannedExpr::new(12..13, "*", Expr::Star))),
),
],
),
@ -909,12 +953,17 @@ mod tests {
"vegetables.*.ediblePortions",
SpannedExpr::new(
0..27,
"vegetables.*.ediblePortions",
Expr::context(
"vegetables.*.ediblePortions",
vec![
SpannedExpr::new(0..10, Expr::ident("vegetables")),
SpannedExpr::new(11..12, Expr::Star),
SpannedExpr::new(13..27, Expr::ident("ediblePortions")),
SpannedExpr::new(0..10, "vegetables", Expr::ident("vegetables")),
SpannedExpr::new(11..12, "*", Expr::Star),
SpannedExpr::new(
13..27,
"ediblePortions",
Expr::ident("ediblePortions"),
),
],
),
),
@ -925,26 +974,39 @@ mod tests {
"github.ref == 'refs/heads/main' && 'value_for_main_branch' || 'value_for_other_branches'",
SpannedExpr::new(
0..88,
"github.ref == 'refs/heads/main' && 'value_for_main_branch' || 'value_for_other_branches'",
Expr::BinOp {
lhs: Box::new(SpannedExpr::new(
0..59,
"github.ref == 'refs/heads/main' && 'value_for_main_branch' ",
Expr::BinOp {
lhs: Box::new(SpannedExpr::new(
0..32,
"github.ref == 'refs/heads/main' ",
Expr::BinOp {
lhs: Box::new(SpannedExpr::new(
0..10,
"github.ref",
Expr::context(
"github.ref",
vec![
SpannedExpr::new(0..6, Expr::ident("github")),
SpannedExpr::new(7..10, Expr::ident("ref")),
SpannedExpr::new(
0..6,
"github",
Expr::ident("github"),
),
SpannedExpr::new(
7..10,
"ref",
Expr::ident("ref"),
),
],
),
)),
op: BinOp::Eq,
rhs: Box::new(SpannedExpr::new(
14..31,
"'refs/heads/main'",
Expr::Literal(Literal::String(
"refs/heads/main".into(),
)),
@ -954,6 +1016,7 @@ mod tests {
op: BinOp::And,
rhs: Box::new(SpannedExpr::new(
35..58,
"'value_for_main_branch'",
Expr::Literal(Literal::String("value_for_main_branch".into())),
)),
},
@ -961,6 +1024,7 @@ mod tests {
op: BinOp::Or,
rhs: Box::new(SpannedExpr::new(
62..88,
"'value_for_other_branches'",
Expr::Literal(Literal::String("value_for_other_branches".into())),
)),
},
@ -970,17 +1034,19 @@ mod tests {
"(true || false) == true",
SpannedExpr::new(
0..23,
"(true || false) == true",
Expr::BinOp {
lhs: Box::new(SpannedExpr::new(
1..14,
"true || false",
Expr::BinOp {
lhs: Box::new(SpannedExpr::new(1..5, true.into())),
lhs: Box::new(SpannedExpr::new(1..5, "true", true.into())),
op: BinOp::Or,
rhs: Box::new(SpannedExpr::new(9..14, false.into())),
rhs: Box::new(SpannedExpr::new(9..14, "false", false.into())),
},
)),
op: BinOp::Eq,
rhs: Box::new(SpannedExpr::new(19..23, true.into())),
rhs: Box::new(SpannedExpr::new(19..23, "true", true.into())),
},
),
),
@ -988,20 +1054,23 @@ mod tests {
"!(!true || false)",
SpannedExpr::new(
0..17,
"!(!true || false)",
Expr::UnOp {
op: UnOp::Not,
expr: Box::new(SpannedExpr::new(
2..16,
"!true || false",
Expr::BinOp {
lhs: Box::new(SpannedExpr::new(
2..7,
"!true",
Expr::UnOp {
op: UnOp::Not,
expr: Box::new(SpannedExpr::new(3..7, true.into())),
expr: Box::new(SpannedExpr::new(3..7, "true", true.into())),
},
)),
op: BinOp::Or,
rhs: Box::new(SpannedExpr::new(11..16, false.into())),
rhs: Box::new(SpannedExpr::new(11..16, "false", false.into())),
},
)),
},
@ -1011,19 +1080,26 @@ mod tests {
"foobar[format('{0}', 'event')]",
SpannedExpr::new(
0..30,
"foobar[format('{0}', 'event')]",
Expr::context(
"foobar[format('{0}', 'event')]",
[
SpannedExpr::new(0..6, Expr::ident("foobar")),
SpannedExpr::new(0..6, "foobar", Expr::ident("foobar")),
SpannedExpr::new(
6..30,
"[format('{0}', 'event')]",
Expr::Index(Box::new(SpannedExpr::new(
7..29,
"format('{0}', 'event')",
Expr::Call {
func: Function("format"),
args: vec![
SpannedExpr::new(14..19, Expr::from("{0}")),
SpannedExpr::new(21..28, Expr::from("event")),
SpannedExpr::new(14..19, "'{0}'", Expr::from("{0}")),
SpannedExpr::new(
21..28,
"'event'",
Expr::from("event"),
),
],
},
))),
@ -1036,20 +1112,26 @@ mod tests {
"github.actor_id == '49699333'",
SpannedExpr::new(
0..29,
"github.actor_id == '49699333'",
Expr::BinOp {
lhs: SpannedExpr::new(
0..15,
"github.actor_id",
Expr::context(
"github.actor_id",
vec![
SpannedExpr::new(0..6, Expr::ident("github")),
SpannedExpr::new(7..15, Expr::ident("actor_id")),
SpannedExpr::new(0..6, "github", Expr::ident("github")),
SpannedExpr::new(7..15, "actor_id", Expr::ident("actor_id")),
],
),
)
.into(),
op: BinOp::Eq,
rhs: Box::new(SpannedExpr::new(19..29, Expr::from("49699333"))),
rhs: Box::new(SpannedExpr::new(
19..29,
"'49699333'",
Expr::from("49699333"),
)),
},
),
),

View file

@ -12,11 +12,8 @@ impl ExplicitExpr {
///
/// Returns `None` if the input is not a valid explicit expression.
pub fn from_curly(expr: impl Into<String>) -> Option<Self> {
// Invariant preservation: we store the full string, but
// we expect it to be a well-formed expression.
let expr = expr.into();
let trimmed = expr.trim();
if !trimmed.starts_with("${{") || !trimmed.ends_with("}}") {
if !expr.starts_with("${{") || !expr.ends_with("}}") {
return None;
}
@ -101,6 +98,8 @@ mod tests {
"not an expression",
"${{ missing end ",
"missing beginning }}",
" ${{ leading whitespace }}",
"${{ trailing whitespace }} ",
];
for case in cases {
@ -111,9 +110,18 @@ mod tests {
#[test]
fn test_expr() {
let expr = "\" ${{ foo }} \\t \"";
let expr: ExplicitExpr = serde_yaml::from_str(expr).unwrap();
assert_eq!(expr.as_bare(), "foo");
for (case, expected) in &[
("${{ foo }}", "foo"),
("${{ foo.bar }}", "foo.bar"),
("${{ foo['bar'] }}", "foo['bar']"),
("${{foo}}", "foo"),
("${{ foo}}", "foo"),
("${{ foo }}", "foo"),
] {
let case = format!("\"{case}\"");
let expr: ExplicitExpr = serde_yaml::from_str(&case).unwrap();
assert_eq!(expr.as_bare(), *expected);
}
}
#[test]

View file

@ -437,7 +437,7 @@ impl Document {
/// not encapsulated by the feature itself.
///
/// Panics if the feature's span is invalid.
pub fn extract_with_leading_whitespace(&self, feature: &Feature) -> &str {
pub fn extract_with_leading_whitespace<'a>(&'a self, feature: &Feature) -> &'a str {
let mut start_idx = feature.location.byte_span.0;
let pre_slice = &self.source[0..start_idx];
if let Some(last_newline) = pre_slice.rfind('\n') {

View file

@ -1,15 +1,19 @@
use std::{ops::Deref, sync::LazyLock};
use github_actions_expressions::{
BinOp, Expr, UnOp,
BinOp, Expr, SpannedExpr, UnOp,
context::{Context, ContextPattern},
};
use github_actions_models::common::{If, expr::ExplicitExpr};
use github_actions_models::common::If;
use super::{Audit, AuditLoadError, AuditState, audit_meta};
use crate::{
finding::{Confidence, Severity, location::Locatable as _},
finding::{
Confidence, Severity,
location::{Locatable as _, Subfeature},
},
models::workflow::JobExt,
utils::ExtractedExpr,
};
pub(crate) struct BotConditions;
@ -88,13 +92,18 @@ impl Audit for BotConditions {
}
for (expr, parent, if_loc) in conds {
if let Some(confidence) = Self::bot_condition(expr) {
if let Some((subfeature, confidence)) = Self::bot_condition(expr) {
findings.push(
Self::finding()
.severity(Severity::High)
.confidence(confidence)
.add_location(parent)
.add_location(if_loc.primary().annotated("actor context may be spoofable"))
.add_location(
if_loc
.primary()
.subfeature(subfeature)
.annotated("actor context may be spoofable"),
)
.build(job.parent())?,
);
}
@ -105,8 +114,11 @@ impl Audit for BotConditions {
}
impl BotConditions {
fn walk_tree_for_bot_condition(expr: &Expr, dominating: bool) -> (bool, bool) {
match expr {
fn walk_tree_for_bot_condition<'a, 'src>(
expr: &'a SpannedExpr<'src>,
dominating: bool,
) -> (Option<&'a SpannedExpr<'src>>, bool) {
match expr.deref() {
// We can't easily analyze the call's semantics, but we can
// check to see if any of the call's arguments are
// bot conditions. We treat a call as non-dominating always.
@ -118,8 +130,8 @@ impl BotConditions {
| Expr::Context(Context { parts: exprs, .. }) => exprs
.iter()
.map(|arg| Self::walk_tree_for_bot_condition(arg, false))
.reduce(|(bc, _), (bc_next, _)| (bc || bc_next, false))
.unwrap_or((false, dominating)),
.reduce(|(bc, _), (bc_next, _)| (bc.or(bc_next), false))
.unwrap_or((None, dominating)),
Expr::Index(expr) => Self::walk_tree_for_bot_condition(expr, dominating),
Expr::BinOp { lhs, op, rhs } => match op {
// || is dominating.
@ -127,7 +139,7 @@ impl BotConditions {
let (bc_lhs, _) = Self::walk_tree_for_bot_condition(lhs, true);
let (bc_rhs, _) = Self::walk_tree_for_bot_condition(rhs, true);
(bc_lhs || bc_rhs, true)
(bc_lhs.or(bc_rhs), true)
}
// == is trivially dominating.
BinOp::Eq => match (lhs.as_ref().deref(), rhs.as_ref().deref()) {
@ -138,16 +150,16 @@ impl BotConditions {
|| (SPOOFABLE_ACTOR_ID_CONTEXTS.iter().any(|x| x.matches(ctx))
&& BOT_ACTOR_IDS.contains(&lit.as_str().as_ref()))
{
(true, true)
((Some(expr)), true)
} else {
(false, true)
(None, true)
}
}
(lhs, rhs) => {
(_, _) => {
let (bc_lhs, _) = Self::walk_tree_for_bot_condition(lhs, true);
let (bc_rhs, _) = Self::walk_tree_for_bot_condition(rhs, true);
(bc_lhs || bc_rhs, true)
(bc_lhs.or(bc_rhs), true)
}
},
// Every other binop is non-dominating.
@ -155,7 +167,7 @@ impl BotConditions {
let (bc_lhs, _) = Self::walk_tree_for_bot_condition(lhs, false);
let (bc_rhs, _) = Self::walk_tree_for_bot_condition(rhs, false);
(bc_lhs || bc_rhs, false)
(bc_lhs.or(bc_rhs), false)
}
},
Expr::UnOp { op, expr } => match op {
@ -170,18 +182,14 @@ impl BotConditions {
// an explicitly toggled condition, and `None` for no condition.
UnOp::Not => Self::walk_tree_for_bot_condition(expr, false),
},
_ => (false, dominating),
_ => (None, dominating),
}
}
fn bot_condition(expr: &str) -> Option<Confidence> {
// TODO: Remove clones here.
let bare = match ExplicitExpr::from_curly(expr) {
Some(raw_expr) => raw_expr.as_bare().to_string(),
None => expr.to_string(),
};
fn bot_condition<'doc>(expr: &'doc str) -> Option<(Subfeature<'doc>, Confidence)> {
let unparsed = ExtractedExpr::new(expr);
let Ok(expr) = Expr::parse(&bare) else {
let Ok(expr) = Expr::parse(unparsed.as_bare()) else {
tracing::warn!("couldn't parse expression: {expr}");
return None;
};
@ -193,9 +201,11 @@ impl BotConditions {
// always passes if the actor is dependabot[bot].
match Self::walk_tree_for_bot_condition(&expr, true) {
// We have a bot condition and it dominates the expression.
(true, true) => Some(Confidence::High),
(Some(expr), true) => Some((Subfeature::from_spanned_expr(expr, 0), Confidence::High)),
// We have a bot condition but it doesn't dominate the expression.
(true, false) => Some(Confidence::Medium),
(Some(expr), false) => {
Some((Subfeature::from_spanned_expr(expr, 0), Confidence::Medium))
}
// No bot condition.
(..) => None,
}
@ -245,7 +255,7 @@ mod tests {
Confidence::Medium,
),
] {
assert_eq!(BotConditions::bot_condition(cond).unwrap(), *confidence);
assert_eq!(BotConditions::bot_condition(cond).unwrap().1, *confidence);
}
}
}

View file

@ -89,12 +89,12 @@ impl OverprovisionedSecrets {
// is not a literal.
[
SpannedExpr {
span: _,
inner: Expr::Identifier(ident),
..
},
SpannedExpr {
span: _,
inner: Expr::Index(idx),
..
},
] if ident == "secrets" && !idx.is_literal() => results.push(()),
_ => results.extend(ctx.parts.iter().flat_map(Self::secrets_expansions)),

View file

@ -20,10 +20,7 @@ use std::{collections::HashMap, env, ops::Deref, sync::LazyLock};
use fst::Map;
use github_actions_expressions::{Expr, Literal, context::Context};
use github_actions_models::{
common::{
RepositoryUses, Uses,
expr::{ExplicitExpr, LoE},
},
common::{EnvValue, RepositoryUses, Uses, expr::LoE},
workflow::job::Strategy,
};
use itertools::Itertools as _;
@ -39,7 +36,7 @@ use crate::{
workflow::Step,
},
state::AuditState,
utils::{DEFAULT_ENVIRONMENT_VARIABLES, extract_expressions},
utils::{DEFAULT_ENVIRONMENT_VARIABLES, ExtractedExpr, extract_expressions},
yaml_patch::{Op, Patch},
};
@ -104,7 +101,7 @@ impl TemplateInjection {
fn scripts_with_location<'doc>(
step: &impl StepCommon<'doc>,
) -> Vec<(String, SymbolicLocation<'doc>)> {
) -> Vec<(&'doc str, SymbolicLocation<'doc>)> {
match step.body() {
models::StepBodyCommon::Uses {
uses: Uses::Repository(uses),
@ -113,16 +110,24 @@ impl TemplateInjection {
.iter()
.filter_map(|input| {
let input = *input;
with.get(input).map(|script| {
(
script.to_string(),
step.location().with_keys(&["with".into(), input.into()]),
)
})
with.get(input)
.and_then(|script| {
if let EnvValue::String(script) = script {
Some(script.as_str())
} else {
None
}
})
.map(|script| {
(
script,
step.location().with_keys(&["with".into(), input.into()]),
)
})
})
.collect(),
models::StepBodyCommon::Run { run, .. } => {
vec![(run.to_string(), step.location().with_keys(&["run".into()]))]
vec![(run, step.location().with_keys(&["run".into()]))]
}
_ => vec![],
}
@ -213,7 +218,7 @@ impl TemplateInjection {
/// Attempts to produce a `Fix` for a given expression.
fn attempt_fix<'doc>(
&self,
raw: &ExplicitExpr,
raw: &ExtractedExpr<'doc>,
parsed: &Expr,
step: &impl StepCommon<'doc>,
) -> Option<Fix<'doc>> {
@ -268,11 +273,8 @@ impl TemplateInjection {
route: step.route(),
operation: Op::MergeInto {
key: "env".to_string(),
value: serde_yaml::to_value(HashMap::from([(
env_var.as_str(),
raw.as_curly(),
)]))
.unwrap(),
value: serde_yaml::to_value(HashMap::from([(env_var.as_str(), raw.as_raw())]))
.unwrap(),
},
});
}
@ -287,13 +289,13 @@ impl TemplateInjection {
fn injectable_template_expressions<'doc>(
&self,
script: &str,
script: &'doc str,
step: &impl StepCommon<'doc>,
) -> Vec<(String, Option<Fix<'doc>>, Severity, Confidence, Persona)> {
let mut bad_expressions = vec![];
for (expr, _) in extract_expressions(script) {
let Ok(parsed) = Expr::parse(expr.as_bare()) else {
tracing::warn!("couldn't parse expression: {expr}", expr = expr.as_bare());
tracing::warn!("couldn't parse expression: {expr}", expr = expr.as_raw());
continue;
};
@ -301,7 +303,7 @@ impl TemplateInjection {
// since any expression in a code context is a code smell,
// even if unexploitable.
bad_expressions.push((
expr.as_curly().into(),
expr.as_raw().into(),
// Intentionally not providing a fix here,
None,
Severity::Unknown,
@ -469,7 +471,7 @@ impl TemplateInjection {
for (script, script_loc) in Self::scripts_with_location(step) {
for (context, fix, severity, confidence, persona) in
self.injectable_template_expressions(&script, step)
self.injectable_template_expressions(script, step)
{
let mut finding_builder = Self::finding()
.severity(severity)

View file

@ -85,12 +85,12 @@ impl UnsoundContains {
Expr::Call { func, args: exprs } if func == "contains" => match exprs.as_slice() {
[
SpannedExpr {
span: _,
inner: Expr::Literal(Literal::String(s)),
..
},
SpannedExpr {
span: _,
inner: Expr::Context(c),
..
},
] => Box::new(std::iter::once((s.as_ref(), c))),
args => Box::new(args.iter().flat_map(Self::walk_tree_for_unsound_contains)),

View file

@ -2,7 +2,8 @@
use std::{ops::Range, sync::LazyLock};
use crate::{audit::AuditInput, models::AsDocument as _, registry::InputKey};
use crate::{audit::AuditInput, models::AsDocument, registry::InputKey};
use github_actions_expressions::SpannedExpr;
use line_index::{LineCol, TextSize};
use regex::Regex;
use serde::Serialize;
@ -107,6 +108,23 @@ macro_rules! route {
};
}
/// Represents a "subfeature" of a symbolic location, such as a substring
/// within a YAML string.
#[derive(Serialize, Clone, Debug)]
pub(crate) struct Subfeature<'doc> {
pub(crate) after: usize,
pub(crate) fragment: &'doc str,
}
impl<'doc> Subfeature<'doc> {
pub(crate) fn from_spanned_expr(expr: &SpannedExpr<'doc>, bias: usize) -> Subfeature<'doc> {
Self {
after: expr.span.start + bias,
fragment: expr.raw,
}
}
}
/// Represents a symbolic location.
#[derive(Serialize, Clone, Debug)]
pub(crate) struct SymbolicLocation<'doc> {
@ -125,6 +143,9 @@ pub(crate) struct SymbolicLocation<'doc> {
/// A symbolic route (of keys and indices) to the final location.
pub(crate) route: Route<'doc>,
/// An optional subfeature for the symbolic location.
pub(crate) subfeature: Option<Subfeature<'doc>>,
/// The kind of location.
pub(crate) kind: LocationKind,
}
@ -136,10 +157,17 @@ impl<'doc> SymbolicLocation<'doc> {
annotation: self.annotation.clone(),
link: None,
route: self.route.with_keys(keys),
subfeature: None,
kind: self.kind,
}
}
/// Adds a subfeature to the current `SymbolicLocation`.
pub(crate) fn subfeature(mut self, subfeature: Subfeature<'doc>) -> SymbolicLocation<'doc> {
self.subfeature = Some(subfeature);
self
}
/// Adds a human-readable annotation to the current `SymbolicLocation`.
pub(crate) fn annotated(mut self, annotation: impl Into<String>) -> SymbolicLocation<'doc> {
self.annotation = annotation.into();
@ -177,30 +205,66 @@ impl<'doc> SymbolicLocation<'doc> {
self,
document: &'doc yamlpath::Document,
) -> anyhow::Result<Location<'doc>> {
// If we don't have a path into the workflow, all
// we have is the workflow itself.
let feature = if self.route.components.is_empty() {
document.root()
} else {
let mut builder = yamlpath::QueryBuilder::new();
let (extracted, location, feature) = match &self.subfeature {
Some(subfeature) => {
// If we have a subfeature, we have to extract its exact
// parent feature.
let feature = match self.route.to_query() {
Some(query) => document.query_exact(&query)?.ok_or_else(|| {
// This should never fail in practice, unless our
// route is malformed or ends in a key-only feature
// (e.g. `foo:`). The latter shouldn't really happen,
// since there's no meaningful subfeature in that case.
anyhow::anyhow!(
"failed to extract exact feature for symbolic location: {}",
self.annotation
)
})?,
None => document.root(),
};
for component in &self.route.components {
builder = match component {
RouteComponent::Key(key) => builder.key(key),
RouteComponent::Index(idx) => builder.index(*idx),
}
let extracted = document.extract(&feature);
let subfeature_span = {
let bias = feature.location.byte_span.0;
let start = &extracted[subfeature.after..]
.find(subfeature.fragment)
.ok_or_else(|| {
anyhow::anyhow!(
"failed to find subfeature '{}' in feature '{}'",
subfeature.fragment,
extracted
)
})?;
(start + bias)..(start + bias + subfeature.fragment.len())
};
(
extracted,
ConcreteLocation::from_span(subfeature_span, document),
feature,
)
}
None => {
let feature = match self.route.to_query() {
Some(query) => document.query_pretty(&query)?,
None => document.root(),
};
let query = builder.build();
document.query_pretty(&query)?
(
document.extract_with_leading_whitespace(&feature),
ConcreteLocation::from(&feature.location),
feature,
)
}
};
Ok(Location {
symbolic: self,
concrete: Feature {
location: ConcreteLocation::from(&feature.location),
feature: document.extract_with_leading_whitespace(&feature),
location,
feature: extracted,
comments: document
.feature_comments(&feature)
.into_iter()
@ -271,6 +335,20 @@ impl ConcreteLocation {
offset_span,
}
}
pub(crate) fn from_span(span: Range<usize>, doc: &yamlpath::Document) -> Self {
let start = TextSize::new(span.start as u32);
let end = TextSize::new(span.end as u32);
let start_point = doc.line_index().line_col(start);
let end_point = doc.line_index().line_col(end);
Self {
start_point: start_point.into(),
end_point: end_point.into(),
offset_span: span.clone(),
}
}
}
impl From<&yamlpath::Location> for ConcreteLocation {

View file

@ -50,7 +50,7 @@ pub(crate) trait StepCommon<'doc>: Locatable<'doc> + HasInputs {
fn strategy(&self) -> Option<&Strategy>;
/// Returns a [`StepBodyCommon`] for this step.
fn body(&self) -> StepBodyCommon;
fn body(&self) -> StepBodyCommon<'doc>;
/// Returns the document which contains this step.
fn document(&self) -> &'doc yamlpath::Document;

View file

@ -98,6 +98,7 @@ impl Action {
annotation: "this action".to_string(),
link: None,
route: Route::new(),
subfeature: None,
kind: Default::default(),
}
}
@ -198,7 +199,7 @@ impl<'doc> StepCommon<'doc> for CompositeStep<'doc> {
None
}
fn body(&self) -> StepBodyCommon {
fn body(&self) -> StepBodyCommon<'doc> {
match &self.body {
action::StepBody::Uses { uses, with } => StepBodyCommon::Uses { uses, with },
action::StepBody::Run {

View file

@ -324,88 +324,72 @@ pub(crate) enum Usage {
mod tests {
use std::str::FromStr;
use github_actions_expressions::context;
use github_actions_models::workflow::job::Step;
use super::{ActionCoordinate, StepCommon};
use super::ActionCoordinate;
use crate::{
finding::location::{Locatable, SymbolicLocation},
models::{
coordinate::{ControlExpr, ControlFieldType, Toggle, Usage},
inputs::HasInputs,
uses::RepositoryUsesPattern,
workflow::{Job, Workflow},
},
registry::InputKey,
};
// Test-only trait impls.
impl<'doc> Locatable<'doc> for Step {
fn location(&self) -> SymbolicLocation<'doc> {
unreachable!()
}
fn location_with_name(&self) -> crate::finding::location::SymbolicLocation<'doc> {
unreachable!()
}
}
impl HasInputs for Step {
fn get_input(&self, _name: &str) -> Option<crate::models::inputs::Capability> {
unreachable!()
}
}
impl<'doc> StepCommon<'doc> for Step {
fn index(&self) -> usize {
unreachable!()
}
fn env_is_static(&self, _ctx: &context::Context) -> bool {
unreachable!()
}
fn uses(&self) -> Option<&github_actions_models::common::Uses> {
unreachable!()
}
fn strategy(&self) -> Option<&github_actions_models::workflow::job::Strategy> {
unreachable!()
}
fn body(&self) -> super::StepBodyCommon {
match &self.body {
github_actions_models::workflow::job::StepBody::Uses { uses, with } => {
super::StepBodyCommon::Uses { uses, with }
}
github_actions_models::workflow::job::StepBody::Run {
run,
working_directory,
shell,
} => super::StepBodyCommon::Run {
run,
_working_directory: working_directory.as_deref(),
_shell: shell.as_deref(),
},
}
}
fn document(&self) -> &'doc yamlpath::Document {
unreachable!()
}
}
#[test]
fn test_usage() {
let workflow = r#"
name: test_usage
on: push
jobs:
test_usage:
runs-on: ubuntu-latest
steps:
- uses: foo/bar # 0
- uses: foo/bar@v1 # 1
- uses: not/thesame # 2
with:
set-me: true
- uses: not/thesame # 3
- uses: foo/bar # 4
with:
set-me: true
- uses: foo/bar # 5
with:
set-me: false
- uses: foo/bar # 6
with:
disable-cache: true
- uses: foo/bar # 7
with:
disable-cache: false
"#;
let workflow =
Workflow::from_string(workflow.into(), InputKey::local("dummy", None).unwrap())
.unwrap();
let Job::NormalJob(job) = workflow.jobs().next().unwrap() else {
panic!("Expected a normal job");
};
let steps = job.steps().collect::<Vec<_>>();
// Trivial case: no usage is possible, since the coordinate's `uses:`
// does not match the step.
let coord =
ActionCoordinate::NotConfigurable(RepositoryUsesPattern::from_str("foo/bar").unwrap());
let step: Step = serde_yaml::from_str("uses: not/thesame").unwrap();
assert_eq!(coord.usage(&step), None);
let step = &steps[3];
assert_eq!(coord.usage(step), None);
// Trivial cases: coordinate is not configurable and matches the `uses:`.
for step in &["uses: foo/bar", "uses: foo/bar@v1"] {
let step: Step = serde_yaml::from_str(step).unwrap();
assert_eq!(coord.usage(&step), Some(Usage::Always));
for step in &[&steps[0], &steps[1]] {
assert_eq!(coord.usage(*step), Some(Usage::Always));
}
// Coordinate `uses:` matches but is not enabled by default and is
@ -414,32 +398,32 @@ mod tests {
uses_pattern: RepositoryUsesPattern::from_str("foo/bar").unwrap(),
control: ControlExpr::single(Toggle::OptIn, "set-me", ControlFieldType::Boolean, false),
};
let step: Step = serde_yaml::from_str("uses: foo/bar").unwrap();
assert_eq!(coord.usage(&step), None);
let step = &steps[0];
assert_eq!(coord.usage(step), None);
// Coordinate `uses:` matches and is explicitly toggled on.
let step: Step = serde_yaml::from_str("uses: foo/bar\nwith:\n set-me: true").unwrap();
assert_eq!(coord.usage(&step), Some(Usage::DirectOptIn));
let step = &steps[4];
assert_eq!(coord.usage(step), Some(Usage::DirectOptIn));
// Coordinate `uses:` matches but is explicitly toggled off.
let step: Step = serde_yaml::from_str("uses: foo/bar\nwith:\n set-me: false").unwrap();
assert_eq!(coord.usage(&step), None);
let step = &steps[5];
assert_eq!(coord.usage(step), None);
// Coordinate `uses:` matches and is enabled by default.
let coord = ActionCoordinate::Configurable {
uses_pattern: RepositoryUsesPattern::from_str("foo/bar").unwrap(),
control: ControlExpr::single(Toggle::OptIn, "set-me", ControlFieldType::Boolean, true),
};
let step: Step = serde_yaml::from_str("uses: foo/bar").unwrap();
assert_eq!(coord.usage(&step), Some(Usage::DefaultActionBehaviour));
let step = &steps[0];
assert_eq!(coord.usage(step), Some(Usage::DefaultActionBehaviour));
// Coordinate `uses:` matches and is explicitly toggled on.
let step: Step = serde_yaml::from_str("uses: foo/bar\nwith:\n set-me: true").unwrap();
assert_eq!(coord.usage(&step), Some(Usage::DirectOptIn));
let step = &steps[4];
assert_eq!(coord.usage(step), Some(Usage::DirectOptIn));
// Coordinate `uses:` matches but is explicitly toggled off, despite default enablement.
let step: Step = serde_yaml::from_str("uses: foo/bar\nwith:\n set-me: false").unwrap();
assert_eq!(coord.usage(&step), None);
let step = &steps[5];
assert_eq!(coord.usage(step), None);
// Coordinate `uses:` matches and has an opt-out toggle, which does not affect
// the default.
@ -452,17 +436,15 @@ mod tests {
false,
),
};
let step: Step = serde_yaml::from_str("uses: foo/bar").unwrap();
assert_eq!(coord.usage(&step), None);
let step = &steps[0];
assert_eq!(coord.usage(step), None);
// Coordinate `uses:` matches and the opt-out inverts the match, clearing it.
let step: Step =
serde_yaml::from_str("uses: foo/bar\nwith:\n disable-cache: true").unwrap();
assert_eq!(coord.usage(&step), None);
let step = &steps[6];
assert_eq!(coord.usage(step), None);
// Coordinate `uses:` matches and the opt-out inverts the match, clearing it.
let step: Step =
serde_yaml::from_str("uses: foo/bar\nwith:\n disable-cache: false").unwrap();
assert_eq!(coord.usage(&step), Some(Usage::DirectOptIn));
let step = &steps[7];
assert_eq!(coord.usage(step), Some(Usage::DirectOptIn));
}
}

View file

@ -197,6 +197,7 @@ impl Workflow {
annotation: "this workflow".to_string(),
link: None,
route: Route::new(),
subfeature: None,
kind: Default::default(),
}
}
@ -620,7 +621,7 @@ impl<'doc> StepCommon<'doc> for Step<'doc> {
self.job().strategy.as_ref()
}
fn body(&self) -> StepBodyCommon {
fn body(&self) -> StepBodyCommon<'doc> {
match &self.body {
StepBody::Uses { uses, with } => StepBodyCommon::Uses { uses, with },
StepBody::Run {

View file

@ -3,10 +3,7 @@
use anyhow::{Context as _, Error, anyhow};
use camino::Utf8Path;
use github_actions_expressions::context::{Context, ContextPattern};
use github_actions_models::common::{
Env,
expr::{ExplicitExpr, LoE},
};
use github_actions_models::common::{Env, expr::LoE};
use jsonschema::{
BasicOutput::{Invalid, Valid},
Validator,
@ -397,6 +394,61 @@ pub(crate) fn split_patterns(patterns: &str) -> impl Iterator<Item = &str> {
.filter(|line| !line.is_empty() && !line.starts_with('#'))
}
/// Represents an expression that has been extracted from some surrounding
/// text, but has not been parsed yet.
///
/// Depending on the context, this may be a "bare" expression or a "fenced"
/// expression internally.
pub(crate) struct ExtractedExpr<'a> {
inner: &'a str,
fenced: bool,
}
impl<'a> ExtractedExpr<'a> {
/// Creates a new [`ExtractedExpr`] from the given expression,
/// which may be either fenced or bare.
pub(crate) fn new(expr: &'a str) -> Self {
Self::from_fenced(expr).unwrap_or_else(|| Self::from_bare(expr))
}
/// Creates a new [`ExtractedExpr`] from a fenced expression.
fn from_fenced(expr: &'a str) -> Option<Self> {
expr.strip_prefix("${{")
.and_then(|e| e.strip_suffix("}}"))
.map(|_| ExtractedExpr {
inner: expr,
fenced: true,
})
}
/// Creates a new [`ExtractedExpr`] from a bare expression.
fn from_bare(expr: &'a str) -> Self {
ExtractedExpr {
inner: expr,
fenced: false,
}
}
/// Returns the extracted expression as a "bare" expression,
/// i.e. without any fencing.
pub(crate) fn as_bare(&self) -> &'a str {
if self.fenced {
self.inner
.strip_prefix("${{")
.and_then(|e| e.strip_suffix("}}"))
.expect("invariant violated: not a fenced expression")
} else {
self.inner
}
}
// Returns the extracted expression exactly as it was extracted,
// including any fencing.
pub(crate) fn as_raw(&self) -> &str {
self.inner
}
}
/// Parse an expression from the given free-form text, starting
/// at the given offset. The returned span is absolute.
///
@ -405,7 +457,10 @@ pub(crate) fn split_patterns(patterns: &str) -> impl Iterator<Item = &str> {
///
/// 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(text: &str, offset: usize) -> Option<(ExplicitExpr, Range<usize>)> {
fn extract_expression<'a>(
text: &'a str,
offset: usize,
) -> Option<(ExtractedExpr<'a>, Range<usize>)> {
let view = &text[offset..];
let start = view.find("${{")?;
@ -423,14 +478,14 @@ fn extract_expression(text: &str, offset: usize) -> Option<(ExplicitExpr, Range<
end.map(|end| {
(
ExplicitExpr::from_curly(&view[start..=end]).unwrap(),
ExtractedExpr::from_fenced(&view[start..=end]).unwrap(),
start + offset..end + offset + 1,
)
})
}
/// Extract zero or more expressions from the given free-form text.
pub(crate) fn extract_expressions(text: &str) -> Vec<(ExplicitExpr, Range<usize>)> {
pub(crate) fn extract_expressions<'a>(text: &'a str) -> Vec<(ExtractedExpr<'a>, Range<usize>)> {
let mut exprs = vec![];
let mut offset = 0;
@ -453,9 +508,9 @@ pub(crate) fn extract_expressions(text: &str) -> Vec<(ExplicitExpr, Range<usize>
/// 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(
input: &AuditInput,
) -> Vec<(ExplicitExpr, Range<usize>)> {
pub(crate) fn parse_expressions_from_input<'doc>(
input: &'doc AuditInput,
) -> Vec<(ExtractedExpr<'doc>, Range<usize>)> {
let text = input.as_document().source();
let doc = input.as_document();
@ -592,15 +647,15 @@ mod tests {
#[test]
fn test_extract_expression() {
let exprs = &[
("${{ foo }}", "foo", 0..10),
("${{ foo }}${{ bar }}", "foo", 0..10),
("leading ${{ foo }} trailing", "foo", 8..18),
("${{ foo }}", " foo ", 0..10),
("${{ foo }}${{ bar }}", " foo ", 0..10),
("leading ${{ foo }} trailing", " foo ", 8..18),
(
"leading ${{ '${{ quoted! }}' }} trailing",
"'${{ quoted! }}'",
" '${{ quoted! }}' ",
8..31,
),
("${{ 'es''cape' }}", "'es''cape'", 0..17),
("${{ 'es''cape' }}", " 'es''cape' ", 0..17),
];
for (text, expected_expr, expected_span) in exprs {
@ -626,7 +681,7 @@ mod tests {
);
let exprs = extract_expressions(raw)
.into_iter()
.map(|(e, _)| e.as_curly().to_string())
.map(|(e, _)| e.as_raw().to_string())
.collect::<Vec<_>>();
assert_eq!(exprs, *expected)
@ -654,10 +709,11 @@ runs:
"#;
let action = Action::from_string(action.into(), InputKey::local("fake", None)?)?;
let action = action.into();
let exprs = parse_expressions_from_input(&action.into());
let exprs = parse_expressions_from_input(&action);
assert_eq!(exprs.len(), 1);
assert_eq!(exprs[0].0.as_curly().to_string(), "${{ '' }}");
assert_eq!(exprs[0].0.as_raw().to_string(), "${{ '' }}");
let workflow = r#"
# ${{ 'don''t parse me' }}

View file

@ -1384,7 +1384,7 @@ foo:
echo "foo: ${{ foo }}"
"#);
// Now test with not_before set to skip the first occurrence
// Now test with after set to skip the first occurrence
let operations = vec![Patch {
route: route!("foo", "bar"),
operation: Op::RewriteFragment {

View file

@ -150,7 +150,7 @@ error[bot-conditions]: spoofable bot actor check
16 | / hackme:
17 | | runs-on: ubuntu-latest
18 | | if: github.actor == 'dependabot[bot]'
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
19 | | steps:
... |
33 | | run: echo hello
@ -166,7 +166,7 @@ error[bot-conditions]: spoofable bot actor check
| ^^^^^^^^^^^^^^^^^^ this step
21 | run: echo hello
22 | if: ${{ github.actor == 'dependabot[bot]' }}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -177,7 +177,7 @@ error[bot-conditions]: spoofable bot actor check
| ^^^^^^^^^^^^^^^^^^ this step
25 | run: echo hello
26 | if: ${{ github.actor == 'dependabot[bot]' && github.repository == 'example/example' }}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → Medium
@ -188,7 +188,7 @@ error[bot-conditions]: spoofable bot actor check
| ^^^^^^^^^^^^^^^^^^ this step
29 | run: echo hello
30 | if: github.actor == 'renovate[bot]'
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High

View file

@ -17,7 +17,7 @@ error[bot-conditions]: spoofable bot actor check
| ^^^^^^^^^^^^ this job
10 | runs-on: ubuntu-latest
11 | if: github.actor == 'dependabot[bot]'
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -28,7 +28,7 @@ error[bot-conditions]: spoofable bot actor check
| ^^^^^^^^^^^^^^^^^^ this step
14 | run: echo hello
15 | if: ${{ github.actor == 'dependabot[bot]' }}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -39,7 +39,7 @@ error[bot-conditions]: spoofable bot actor check
| ^^^^^^^^^^^^^^^^^^ this step
18 | run: echo hello
19 | if: ${{ github.actor == 'dependabot[bot]' && github.repository == 'example/example' }}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → Medium
@ -50,7 +50,7 @@ error[bot-conditions]: spoofable bot actor check
| ^^^^^^^^^^^^^^^^^^ this step
22 | run: echo hello
23 | if: github.actor == 'renovate[bot]'
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -62,7 +62,7 @@ error[bot-conditions]: spoofable bot actor check
30 | run: echo hello
31 | # ensure we're case insensitive
32 | if: github.ACTOR == 'dependabot[bot]'
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -74,7 +74,7 @@ error[bot-conditions]: spoofable bot actor check
35 | run: echo hello
36 | # ensure we detect unknown bots
37 | if: github.actor == 'mystery[bot]'
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -86,7 +86,7 @@ error[bot-conditions]: spoofable bot actor check
40 | run: echo hello
41 | # ensure we handle index-style contexts
42 | if: github['actor'] == 'dependabot[bot]'
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -98,7 +98,7 @@ error[bot-conditions]: spoofable bot actor check
45 | run: echo hello
46 | # ensure we handle index-style contexts with a different case
47 | if: github['ACTOR'] == 'dependabot[bot]'
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -110,7 +110,7 @@ error[bot-conditions]: spoofable bot actor check
50 | run: echo hello
51 | # ensure we handle actor ID checks
52 | if: github.actor_id == 49699333
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -121,7 +121,7 @@ error[bot-conditions]: spoofable bot actor check
| ^^^^^^^^^^^^^^^^^^^ this step
55 | run: echo hello
56 | if: github['ACTOR_ID'] == 49699333
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High
@ -132,7 +132,7 @@ error[bot-conditions]: spoofable bot actor check
| ^^^^^^^^^^^^^^^^^^^ this step
59 | run: echo hello
60 | if: ${{ github.actor_id == '49699333' }}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ actor context may be spoofable
|
= note: audit confidence → High

View file

@ -29,6 +29,8 @@ of `zizmor`.
when analyzing `inputs` context accesses (#919)
* zizmor now produces more descriptive error messages when it fails to
parse a workflow or action definition (#956)
* The [bot-conditions] audit now returns precise spans for flagged
actor checks, instead of flagging the entire `if:` value (#949)
### Bug Fixes 🐛