Skip to content

Commit

Permalink
feat: improve SARIF comprehension on GitHub (#528)
Browse files Browse the repository at this point in the history
* feat: improve SARIF comprehension on GitHub

* add Finding::to_markdown

* tweak

* record release notes for #528
  • Loading branch information
woodruffw authored Feb 11, 2025
1 parent 2ce9f4b commit 2a65d51
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 7 deletions.
5 changes: 4 additions & 1 deletion docs/release-notes.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ of `zizmor`.

## Next (UNRELEASED)

Nothing yet!
### Improvements 🌱

* SARIF outputs are now slightly more aligned with GitHub Code Scanning
expectations (#528)

## v1.3.1

Expand Down
12 changes: 12 additions & 0 deletions src/finding/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,18 @@ pub(crate) struct Finding<'w> {
pub(crate) ignored: bool,
}

impl Finding<'_> {
/// A basic Markdown representation of the finding's metadata.
pub(crate) fn to_markdown(&self) -> String {
format!(
"`{ident}`: {desc}\n\nDocs: <{url}>",
ident = self.ident,
desc = self.desc,
url = self.url
)
}
}

pub(crate) struct FindingBuilder<'w> {
ident: &'static str,
desc: &'static str,
Expand Down
31 changes: 25 additions & 6 deletions src/sarif.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use std::collections::HashSet;

use serde_sarif::sarif::{
ArtifactContent, ArtifactLocation, Location as SarifLocation, LogicalLocation, Message,
PhysicalLocation, PropertyBag, Region, ReportingDescriptor, Result as SarifResult, ResultKind,
ResultLevel, Run, Sarif, Tool, ToolComponent,
MultiformatMessageString, PhysicalLocation, PropertyBag, Region, ReportingDescriptor,
Result as SarifResult, ResultKind, ResultLevel, Run, Sarif, Tool, ToolComponent,
};

use crate::finding::{Finding, Location, Severity};
Expand Down Expand Up @@ -78,6 +78,12 @@ fn build_rule(finding: &Finding) -> ReportingDescriptor {
ReportingDescriptor::builder()
.id(finding.ident)
.help_uri(finding.url)
.help(
MultiformatMessageString::builder()
.text(finding.desc)
.markdown(finding.to_markdown())
.build(),
)
.build()
}

Expand All @@ -86,12 +92,25 @@ fn build_results(findings: &[Finding]) -> Vec<SarifResult> {
}

fn build_result(finding: &Finding<'_>) -> SarifResult {
// NOTE: Safe unwrap because FindingBuilder::build ensures a primary location.
let primary = finding
.locations
.iter()
.find(|l| l.symbolic.primary)
.unwrap();

SarifResult::builder()
.message(finding.desc)
.rule_id(finding.ident)
.locations(build_locations(
finding.locations.iter().filter(|l| l.symbolic.primary),
))
// NOTE: We use the primary location's annotation for the result's message.
// This is conceptually incorrect since the location's annotation should
// only be on the location itself. However, GitHub's SARIF viewer does not
// render location-level messages, so we use the primary location's message
// to ensure something reasonable is presented.
// This ends up being OK since the only other thing we'd put here
// is the finding's description, which is already in the rule's help message.
// See https://github.com/woodruffw/zizmor/issues/526 for context.
.message(&primary.symbolic.annotation)
.locations(build_locations(std::iter::once(primary)))
.related_locations(build_locations(
finding.locations.iter().filter(|l| !l.symbolic.primary),
))
Expand Down

0 comments on commit 2a65d51

Please sign in to comment.