Repository: woodruffw/zizmor (rust)

Issue:
[BUG]: SARIF presentation on GH is broken (again)
### Pre-submission checks

- [x] I am **not** filing a feature request. These should be filed via the feature request form instead.
- [x] I have looked through the [open issues](https://github.com/woodruffw/zizmor/issues?q=is%3Aissue+is%3Aopen+) for a duplicate report.

### Expected behavior

SARIF should preview correctly on GitHub.

### Actual behavior

Looks like this regressed again, because of some (otherwise good) changes to relative path handling.

In particular, it looks like GitHub's SARIF consumer can handle relative paths like `foo/bar` fine, but they don't currently handle `./foo/bar`.

Consequently, findings get rendered without a correct preview:

<img width="1583" alt="Image" src="https://github.com/user-attachments/assets/0a25510b-5341-4fc8-bdda-2afae5a43c4a" />

### Reproduction steps

See above.

### Logs

```plain text

```

### Additional context

_No response_

Pull request:
bugfix: sarif: add working directory to invocation
Fixes #604.

This performs the fix by partially reverting #572 -- the "presentation" path is still used in formats like the "plain" (cargo-style) output, but the SARIF format now uses a `sarif_path` helper that removes the input prefix (even when that input prefix is something relative that GitHub _should_ be handling correctly).

To test this, I've added a new `test-sarif` workflow that gets triggered by the `test-sarif-presentation` label -- when a PR is labeled as such, the workflow runs and uses a custom category to test any SARIF changes made. 

~~I don't have a good way to test this yet -- my initial thought was to add some kind of `issue_comment` trigger so that I could do something like `/sariftest`, but I don't really like that. Another option would be a `pull_request` with the `labeled` sub-event. Either way, the goal would be to run a separate workflow (or job) that performs the SARIF steps for testing purposes.~~

Patch:
diff --git a/docs/release-notes.md b/docs/release-notes.md
index fc4f4e2..61c7942 100644
--- a/docs/release-notes.md
+++ b/docs/release-notes.md
@@ -15,6 +15,8 @@ of `zizmor`.
   commented-out expressions, resulting in spurious warnings (#570)
 * Fixed a bug where `zizmor` would fail to honor `# zizmor: ignore[rule]`
   comments in unintuitive cases (#612)
+* Fixed a regression in `zizmor`'s SARIF output format that caused suboptimal
+  presentation of findings on GitHub (#621)
 
 ### Upcoming Changes 🚧
 
diff --git a/src/registry.rs b/src/registry.rs
index 20e9839..dfa4378 100644
--- a/src/registry.rs
+++ b/src/registry.rs
@@ -94,6 +94,32 @@ impl InputKey {
         }))
     }
 
+    /// Returns a path for this [`InputKey`] that's suitable for SARIF
+    /// outputs.
+    ///
+    /// This is similar to [`InputKey::presentation_path`] in terms of being
+    /// a relative path (if the input is relative), but it also strips
+    /// the prefix from local paths, if one is present.
+    ///
+    /// For example, if the user runs `zizmor .`, then an input at
+    /// `./.github/workflows/foo.yml` will be returned as `.github/workflows/foo.yml`,
+    /// rather than `./.github/workflows/foo.yml`.
+    ///
+    /// This is needed for GitHub's interpretation of SARIF, which is brittle
+    /// with absolute paths but _also_ doesn't like relative paths that
+    /// start with relative directory markers.
+    pub(crate) fn sarif_path(&self) -> &str {
+        match self {
+            InputKey::Local(local) => local
+                .prefix
+                .as_ref()
+                .and_then(|pfx| local.given_path.strip_prefix(pfx).ok())
+                .unwrap_or_else(|| &local.given_path)
+                .as_str(),
+            InputKey::Remote(remote) => remote.path.as_str(),
+        }
+    }
+
     /// Return a "presentation" path for this [`InputKey`].
     ///
     /// This will always be a relative path for remote keys,
@@ -331,7 +357,7 @@ mod tests {
     }
 
     #[test]
-    fn test_input_key_local_path() {
+    fn test_input_key_local_presentation_path() {
         let local = InputKey::local("/foo/bar/baz.yml", None).unwrap();
         assert_eq!(local.presentation_path(), "/foo/bar/baz.yml");
 
@@ -351,4 +377,26 @@ mod tests {
             "/home/runner/work/repo/repo/.github/workflows/baz.yml"
         );
     }
+
+    #[test]
+    fn test_input_key_local_sarif_path() {
+        let local = InputKey::local("/foo/bar/baz.yml", None).unwrap();
+        assert_eq!(local.sarif_path(), "/foo/bar/baz.yml");
+
+        let local = InputKey::local("/foo/bar/baz.yml", Some("/foo")).unwrap();
+        assert_eq!(local.sarif_path(), "bar/baz.yml");
+
+        let local = InputKey::local("/foo/bar/baz.yml", Some("/foo/bar/")).unwrap();
+        assert_eq!(local.sarif_path(), "baz.yml");
+
+        let local = InputKey::local(
+            "/home/runner/work/repo/repo/.github/workflows/baz.yml",
+            Some("/home/runner/work/repo/repo"),
+        )
+        .unwrap();
+        assert_eq!(local.sarif_path(), ".github/workflows/baz.yml");
+
+        let local = InputKey::local("./.github/workflows/baz.yml", Some(".")).unwrap();
+        assert_eq!(local.sarif_path(), ".github/workflows/baz.yml");
+    }
 }
diff --git a/src/sarif.rs b/src/sarif.rs
index e390c09..455250c 100644
--- a/src/sarif.rs
+++ b/src/sarif.rs
@@ -3,8 +3,8 @@
 use std::collections::HashSet;
 
 use serde_sarif::sarif::{
-    ArtifactContent, ArtifactLocation, Location as SarifLocation, LogicalLocation, Message,
-    MultiformatMessageString, PhysicalLocation, PropertyBag, Region, ReportingDescriptor,
+    ArtifactContent, ArtifactLocation, Invocation, Location as SarifLocation, LogicalLocation,
+    Message, MultiformatMessageString, PhysicalLocation, PropertyBag, Region, ReportingDescriptor,
     Result as SarifResult, ResultKind, ResultLevel, Run, Sarif, Tool, ToolComponent,
 };
 
@@ -61,6 +61,10 @@ fn build_run(findings: &[Finding]) -> Run {
                 .build(),
         )
         .results(build_results(findings))
+        .invocations([Invocation::builder()
+            // We only produce results on successful executions.
+            .execution_successful(true)
+            .build()])
         .build()
 }
 
@@ -145,8 +149,7 @@ fn build_locations<'a>(locations: impl Iterator<Item = &'a Location<'a>>) -> Vec
                     PhysicalLocation::builder()
                         .artifact_location(
                             ArtifactLocation::builder()
-                                .uri_base_id("%SRCROOT%")
-                                .uri(location.symbolic.key.presentation_path())
+                                .uri(location.symbolic.key.sarif_path())
                                 .build(),
                         )
                         .region(


Test patch:
diff --git a/.github/workflows/test-sarif.yml b/.github/workflows/test-sarif.yml
new file mode 100644
index 0000000..73486d3
--- /dev/null
+++ b/.github/workflows/test-sarif.yml
@@ -0,0 +1,48 @@
+name: Test SARIF Presentation
+
+on:
+  pull_request:
+    types:
+      - opened
+      - synchronize
+      - reopened
+      - labeled
+
+permissions: {}
+
+jobs:
+  test-sarif-presentation:
+    runs-on: ubuntu-latest
+    if: contains(github.event.pull_request.labels.*.name, 'test-sarif-presentation')
+    permissions:
+      pull-requests: write # for 'Leave comment' step
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4
+        with:
+          persist-credentials: false
+
+      - uses: Swatinem/rust-cache@f0deed1e0edfc6a9be95417288c0e1099b1eeec3 # v2
+
+      - name: Run zizmor
+        run: |
+          cargo run -- --format sarif . > results.sarif
+
+      - name: Upload SARIF file
+        uses: github/codeql-action/upload-sarif@6bb031afdd8eb862ea3fc1848194185e076637e5 # v3.28.11
+        with:
+          sarif_file: results.sarif
+          category: zizmor-test-sarif-presentation
+
+      - name: Leave comment
+        uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7
+        with:
+          script: |
+            let url = `https://github.com/woodruffw/zizmor/security/code-scanning?query=pr%3A${context.issue.number}+is%3Aopen+sort%3Acreated-desc`
+
+            github.rest.issues.createComment({
+              issue_number: context.issue.number,
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              body: `:robot: Presentation results: <${url}>`
+            })