From e643cf695051c00d804b12c649959d67b39791bf Mon Sep 17 00:00:00 2001 From: cyrus Date: Wed, 8 Jul 2026 18:45:22 +0530 Subject: [PATCH] fix(expr): normalize bare scope-key refs before compiling as jq (B17) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A workflow builder can emit a hybrid condition field like `item.labels | any(.name == "in progress") | not` — the simple-path shorthand's bare scope key (no leading dot) piped into a real jq program. That string fails is_simple_dotted_path (it contains a `|`) so it falls to run_jq, where a bare `item` at the head parses as an undefined jq function, the compile fails, and the expression silently resolves to Null — which a condition node's truthiness check then routes to `false` for every item. normalize_bare_scope_ref() rewrites a program that starts with a bare identifier which is a top-level key of the expression scope (item/items/run/nodes) into a leading-dot jq field access, as long as the identifier isn't being called as a jq function (next char is not `(`). Leading-dot programs and unrecognized identifiers pass through unchanged. --- src/expr.rs | 142 ++++++++++++++++++++++++++++ src/nodes/control_flow/condition.rs | 36 ++++++- 2 files changed, 177 insertions(+), 1 deletion(-) diff --git a/src/expr.rs b/src/expr.rs index 9874819..2c9978c 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -37,6 +37,8 @@ //! [jq]: https://jqlang.org/ //! [`jaq`]: https://crates.io/crates/jaq +use std::borrow::Cow; + use jaq_core::load::{Arena, File, Loader}; use jaq_core::{Compiler, Ctx, Vars, data, unwrap_valr}; use jaq_json::Val; @@ -298,10 +300,76 @@ fn is_ident(seg: &str) -> bool { /// and would leak host secrets into node output; it is stripped before compile. const EXPR_DENIED_BUILTINS: &[&str] = &["env"]; +/// Repairs a hybrid expression that mixes the simple-path shorthand's bare +/// scope key (e.g. `item`, with no leading dot) with a real jq pipe, such as +/// `"item.labels | any(.name == \"in progress\") | not"`. That whole string +/// fails [`is_simple_dotted_path`] (it contains a `|`), so [`evaluate`] routes +/// it to [`run_jq`] as a jq program — but a *bare* `item` at the start of a jq +/// program parses as a call to an undefined jq function named `item`, so the +/// program fails to compile and [`run_jq`] silently returns [`Value::Null`] +/// (which, fed into a `condition` node's truthiness check, routes every item +/// to `false`). +/// +/// If `program` starts with a bare identifier +/// (`[A-Za-z_][A-Za-z0-9_-]*`) that is a **top-level key of `scope`** (e.g. +/// `item`/`items`/`run`/`nodes`, per `nodes::expr_scope_for`), and the +/// character immediately following that identifier is `.`, `|`, `[`, ` `, or +/// end-of-string, this prepends a `.` so the identifier is read as a jq field +/// access on the scope root (`.item...`) instead of a function call. A +/// following `(` is deliberately excluded from that boundary set: it marks a +/// jq function *call* (`any(...)`, `map(...)`) rather than a field reference, +/// so `any(...)`/`map(...)` are left untouched even if `scope` happened to +/// have a same-named top-level key. +/// +/// Programs that do not start with a scope-key identifier (leading-dot jq +/// like `.item...`, or an identifier scope doesn't recognize) are returned +/// unchanged — this only changes expressions that would otherwise +/// unconditionally fail to compile and resolve to `Null`. Plain dotted paths +/// with no jq (`"item.labels"`, no `|`/`[`/space) never reach `run_jq` at all: +/// [`evaluate`] takes the [`is_simple_dotted_path`] fast path for those. +fn normalize_bare_scope_ref<'a>(program: &'a str, scope: &Value) -> Cow<'a, str> { + let mut chars = program.char_indices(); + let Some((_, first)) = chars.next() else { + return Cow::Borrowed(program); + }; + if !(first == '_' || first.is_ascii_alphabetic()) { + return Cow::Borrowed(program); + } + + // Walk the rest of the leading identifier (ASCII letters/digits/`_`/`-`), + // tracking the byte offset just past its last matching char. + let mut end = first.len_utf8(); + for (i, c) in chars { + if c == '_' || c == '-' || c.is_ascii_alphanumeric() { + end = i + c.len_utf8(); + } else { + break; + } + } + + let ident = &program[..end]; + if scope.get(ident).is_none() { + return Cow::Borrowed(program); + } + + let boundary_ok = match program[end..].chars().next() { + None => true, + Some(c) => matches!(c, '.' | '|' | '[' | ' '), + }; + if boundary_ok { + Cow::Owned(format!(".{program}")) + } else { + Cow::Borrowed(program) + } +} + /// Compiles `program` as a jq filter and runs it once against `scope` (the jq /// input `.`), returning the first output value. Any compile/run failure, /// non-JSON output, or empty output yields [`Value::Null`]; this never panics. fn run_jq(program: &str, scope: &Value) -> Value { + let program = normalize_bare_scope_ref(program, scope); + let program = program.as_ref(); + // Convert the scope into a jq value; bail to Null if it cannot be represented. let Ok(input) = serde_json::from_value::(scope.clone()) else { return Value::Null; @@ -609,6 +677,80 @@ mod tests { assert_eq!(evaluate(&json!("=(((("), &scope), Value::Null); } + // --- bare-scope-key normalization (hybrid shorthand + jq pipe) --------- + + #[test] + fn bare_scope_key_with_jq_pipe_is_normalized() { + // `item.labels | any(...) | not` is a hybrid: no leading dot (like the + // simple-path shorthand), but it has a jq pipe, so it fails + // `is_simple_dotted_path` and falls to `run_jq`, where a bare `item` + // would otherwise parse as an undefined jq function and fail to + // compile. It must be normalized to `.item...` and evaluate for real. + let program = r#"=item.labels | any(.name == "in progress") | not"#; + + let with_label = json!({ + "item": { "labels": [{ "name": "in progress" }] }, + }); + assert_eq!( + evaluate(&json!(program), &with_label), + json!(false), + "an item carrying the 'in progress' label must route/evaluate to false" + ); + + let without_label = json!({ + "item": { "labels": [{ "name": "done" }] }, + }); + assert_eq!( + evaluate(&json!(program), &without_label), + json!(true), + "an item without the 'in progress' label must route/evaluate to true" + ); + } + + #[test] + fn bare_scope_key_normalization_does_not_break_leading_dot() { + // An expression that already starts with `.` is untouched by + // normalization (its first char is `.`, not an identifier char), so + // no double-dot (`..item`) is ever introduced. + let scope = json!({ "item": { "labels": [{ "name": "urgent" }] } }); + assert_eq!( + evaluate(&json!(r#"=.item.labels | any(.name == "urgent")"#), &scope), + json!(true) + ); + } + + #[test] + fn bare_scope_key_normalization_does_not_clobber_jq_builtins() { + // `any(` is followed by `(`, which is excluded from the normalization + // boundary set, so a jq builtin call is never mistaken for a bare + // scope-key field reference. This expression already has its leading + // dot; it must keep evaluating exactly as jq intends. + let scope = json!({ "item": { "xs": [1, 2, 3] } }); + assert_eq!( + evaluate(&json!("=.item.xs | any(. > 2)"), &scope), + json!(true) + ); + } + + #[test] + fn bare_scope_key_with_pipe_only() { + // `items | length` (no leading dot): `items` is a top-level scope key + // and the boundary char is a space, so it normalizes to + // `.items | length` and counts the array. + let scope = json!({ "items": [1, 2, 3, 4] }); + assert_eq!(evaluate(&json!("=items | length"), &scope), json!(4)); + } + + #[test] + fn bare_ident_not_in_scope_stays_as_is() { + // `foobar` is not a top-level key of scope, so normalization leaves + // the program untouched; it still fails to compile as jq (bare + // `foobar` is an undefined function) and yields Null, same as before + // this fix. + let scope = json!({ "item": {} }); + assert_eq!(evaluate(&json!("=foobar.x | length"), &scope), Value::Null); + } + // --- resolve (recursive config data-binding) -------------------------- #[test] diff --git a/src/nodes/control_flow/condition.rs b/src/nodes/control_flow/condition.rs index 417c232..04d7007 100644 --- a/src/nodes/control_flow/condition.rs +++ b/src/nodes/control_flow/condition.rs @@ -222,7 +222,10 @@ mod tests { vec![Item::new(json!({ "assignee": "" }))], ) .await; - assert_eq!(empty_port, "false", "an empty assignee value must route false"); + assert_eq!( + empty_port, "false", + "an empty assignee value must route false" + ); let (missing_port, _) = route( json!({ "field": "=item.assignee" }), @@ -234,4 +237,35 @@ mod tests { "a missing assignee resolves to null via the expression and routes false" ); } + + #[tokio::test] + async fn jq_expression_in_field_evaluates_correctly() { + // A hybrid `field` value: the simple-path shorthand's bare scope key + // (`item`, no leading dot) piped into real jq (`any(...)`). Before the + // bare-scope-key normalization fix, a bare `item` at the head of a jq + // program parsed as an undefined jq function, the program failed to + // compile, `run_jq` returned `Null`, and the condition routed every + // item to `false` regardless of its labels. + let field = json!({ "field": r#"=item.labels | any(.name == "urgent")"# }); + + let (with_label_port, _) = route( + field.clone(), + vec![Item::new(json!({ "labels": [{ "name": "urgent" }] }))], + ) + .await; + assert_eq!( + with_label_port, "true", + "an item carrying the 'urgent' label must route true" + ); + + let (without_label_port, _) = route( + field, + vec![Item::new(json!({ "labels": [{ "name": "normal" }] }))], + ) + .await; + assert_eq!( + without_label_port, "false", + "an item without the 'urgent' label must route false" + ); + } }