bugfix: repro, #988 (#989)

Author: woodruffw

Cargo.lock

@@ -42,6 +42,7 @@ indicatif = "0.17.11"
 insta = "1.43.0"
 jsonschema = "0.30.0"
 line-index = "0.1.2"
+memchr = "2.7.5"
 owo-colors = "4.2.1"
 regex = "1.11.1"
 reqwest = { version = "0.12.20", default-features = false }

Cargo.toml

@@ -134,7 +134,7 @@ impl<'src> Literal<'src> {
 // TODO: Move this type to some kind of common crate? It's useful to have
 // a single span type everywhere, instead of the current mash of span helpers
 // and ranges we have throughout zizmor.
-/// Represents a `[start, end)` span for a source expression.
+/// Represents a `[start, end)` byte span for a source expression.
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub struct Span {
     /// The start of the span, inclusive.

crates/github-actions-expressions/src/lib.rs

@@ -47,6 +47,7 @@ indicatif.workspace = true
 itertools.workspace = true
 jsonschema.workspace = true
 line-index.workspace = true
+memchr.workspace = true
 owo-colors.workspace = true
 regex.workspace = true
 reqwest = { workspace = true, features = ["blocking", "json", "rustls-tls"] }

crates/zizmor/Cargo.toml

@@ -129,11 +129,11 @@ pub(crate) enum Fragment<'doc> {
     /// might contain multiple lines, e.g. a multi-line GitHub Actions
     /// expression, since the subfeature's indentation won't necessarily match
     /// the surrounding feature's YAML-level indentation.
-    Regex(#[serde(serialize_with = "Fragment::serialize_regex")] regex::Regex),
+    Regex(#[serde(serialize_with = "Fragment::serialize_regex")] regex::bytes::Regex),
 }
 
 impl<'doc> Fragment<'doc> {
-    fn serialize_regex<S>(regex: &regex::Regex, serializer: S) -> Result<S::Ok, S::Error>
+    fn serialize_regex<S>(regex: &regex::bytes::Regex, serializer: S) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
     {
@@ -172,7 +172,7 @@ impl<'doc> Fragment<'doc> {
             static WHITESPACE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\s+").unwrap());
             let regex = WHITESPACE.replace_all(&escaped, "\\s+");
 
-            Fragment::Regex(Regex::new(&regex).unwrap())
+            Fragment::Regex(regex::bytes::Regex::new(&regex).unwrap())
         }
     }
 }
@@ -211,14 +211,26 @@ impl<'doc> Subfeature<'doc> {
     /// can't be found. The returned span is relative to the feature's
     /// start.
     fn locate_within(&self, feature: &str) -> Option<Span> {
+        // NOTE: Our inputs are always valid UTF-8 but `after` may not
+        // be a valid UTF-8 codepoint index, so everything below operates
+        // on a byte slice.
+        // Why, you might ask, might `after` not be a valid codepoint index?
+        // Because `after` is a fuzzy anchor: we know our subfeature starts
+        // *somewhere* after `after`, but we don't know exactly where.
+        // This happens because we have a rough sense of where the subfeature
+        // is *after* YAML parsing, but we don't know exactly where it is
+        // in the original YAML feature due to significant whitespace.
+        let feature = feature.as_bytes();
         let bias = self.after;
         let focus = &feature[bias..];
 
         match &self.fragment {
-            Fragment::Raw(fragment) => focus.find(fragment).map(|start| {
-                let end = start + fragment.len();
-                Span::from(start..end).adjust(bias)
-            }),
+            Fragment::Raw(fragment) => {
+                memchr::memmem::find(focus, fragment.as_bytes()).map(|start| {
+                    let end = start + fragment.len();
+                    Span::from(start..end).adjust(bias)
+                })
+            }
             Fragment::Regex(regex) => regex
                 .find(focus)
                 .map(|m| Span::from(m.range()).adjust(bias)),

crates/zizmor/src/finding/location.rs

@@ -413,6 +413,13 @@ fn template_injection() -> Result<()> {
             .run()?
     );
 
+    insta::assert_snapshot!(
+        zizmor()
+            .input(input_under_test("template-injection/issue-988-repro.yml"))
+            .args(["--persona=pedantic"])
+            .run()?
+    );
+
     Ok(())
 }
 

crates/zizmor/tests/integration/snapshot.rs

@@ -0,0 +1,29 @@
+---
+source: crates/zizmor/tests/integration/snapshot.rs
+expression: "zizmor().input(input_under_test(\"template-injection/issue-988-repro.yml\")).args([\"--persona=pedantic\"]).run()?"
+snapshot_kind: text
+---
+note[template-injection]: code injection via template expansion
+  --> @@INPUT@@:16:29
+   |
+13 |         run: |
+   |         --- note: this run block
+14 |           for index in {1..2}; do
+15 |             # ドドド
+16 |             event_name="${{ github.event_name }}"
+   |                             ----------------- note: may expand into attacker-controllable code
+   |
+   = note: audit confidence → Unknown
+
+note[template-injection]: code injection via template expansion
+  --> @@INPUT@@:27:57
+   |
+25 |         run: |
+   |         --- note: this run block
+26 |           curl -X POST https://api.example.com -H "Content-type: application/json" \
+27 |             -d "{\"text\":\"ドドド: https://github.com/${{ github.repository }}\"}"
+   |                                                            ----------------- note: may expand into attacker-controllable code
+   |
+   = note: audit confidence → Unknown
+
+2 findings: 2 unknown, 0 informational, 0 low, 0 medium, 0 high

crates/zizmor/tests/integration/snapshots/integration__snapshot__template_injection-15.snap

@@ -0,0 +1,27 @@
+# https://github.com/zizmorcore/zizmor/issues/988
+name: Panic
+on:
+  pull_request: {}
+permissions: {}
+jobs:
+  example1:
+    name: "Example 1"
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - name: "Panic occurred in file 'crates/zizmor/src/finding/location.rs' at line 215"
+        run: |
+          for index in {1..2}; do
+            # ドドド
+            event_name="${{ github.event_name }}"
+            echo "$index: $event_name"
+          done
+  example2:
+    name: "Example 2"
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    steps:
+      - name: "Panic occurred in file 'crates/zizmor/src/finding/location.rs' at line 215"
+        run: |
+          curl -X POST https://api.example.com -H "Content-type: application/json" \
+            -d "{\"text\":\"ドドド: https://github.com/${{ github.repository }}\"}"

crates/zizmor/tests/integration/test-data/template-injection/issue-988-repro.yml

@@ -15,6 +15,11 @@ of `zizmor`.
 * The [bot-conditions] audit now produces findings on triggers other than
   `pull_request_target` (#921)
 
+### Bug Fixes 🐛
+
+* Fixed a bug where `zizmor` would crash when attempting to extract
+  subfeatures from features containing non-ASCII codepoints (#989)
+
 ## 1.10.0
 
 This is a **huge** new release, with multiple new features, enhancements,