From de6f9d6042be33cf8d7eff1994a9a42b1cd40277 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Thu, 18 Dec 2025 17:25:07 -0500 Subject: [PATCH] ci: add plain presentation test (#1454) --- .github/workflows/test-output.yml | 24 +++++++++++ crates/zizmor/src/main.rs | 49 +++++++++++++++++++++++ crates/zizmor/src/output/plain.rs | 37 ++++++++++++----- crates/zizmor/tests/integration/common.rs | 26 ++++++++---- crates/zizmor/tests/integration/e2e.rs | 13 ++++-- docs/release-notes.md | 8 ++++ docs/snippets/help.txt | 2 + 7 files changed, 138 insertions(+), 21 deletions(-) diff --git a/.github/workflows/test-output.yml b/.github/workflows/test-output.yml index d065830a..ec9fdec6 100644 --- a/.github/workflows/test-output.yml +++ b/.github/workflows/test-output.yml @@ -67,3 +67,27 @@ jobs: --no-exit-codes \ --format github \ crates/zizmor/tests/integration/test-data/several-vulnerabilities.yml + + test-plain-presentation: + name: Test plain text presentation + runs-on: ubuntu-latest + if: contains(github.event.pull_request.labels.*.name, 'test-plain-presentation') + permissions: {} + + steps: + - name: Checkout repository + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + with: + persist-credentials: false + + - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2.8.2 + + - name: Run zizmor + run: | + # Normally we'd want a workflow to fail if the audit fails, + # but we're only testing presentation here. + cargo run \ + -- \ + --no-exit-codes \ + --format plain \ + crates/zizmor/tests/integration/test-data/several-vulnerabilities.yml diff --git a/crates/zizmor/src/main.rs b/crates/zizmor/src/main.rs index bcf411f6..620ad0f5 100644 --- a/crates/zizmor/src/main.rs +++ b/crates/zizmor/src/main.rs @@ -118,6 +118,15 @@ struct App { #[arg(long, value_enum, default_value_t)] format: OutputFormat, + /// Whether to render OSC 8 links in the output. + /// + /// This affects links under audit IDs, as well as any links + /// produced by audit rules. + /// + /// Only affects `--format=plain` (the default). + #[arg(long, value_enum, default_value_t, env = "ZIZMOR_RENDER_LINKS")] + render_links: CliRenderLinks, + /// Whether to render audit URLs in the output, separately from any URLs /// embedded in OSC 8 links. /// @@ -325,6 +334,44 @@ pub(crate) enum OutputFormat { Github, } +#[derive(Debug, Default, Copy, Clone, ValueEnum)] +pub(crate) enum CliRenderLinks { + /// Render OSC 8 links in output if support is detected. + #[default] + Auto, + /// Always render OSC 8 links in output. + Always, + /// Never render OSC 8 links in output. + Never, +} + +#[derive(Debug, Copy, Clone)] +pub(crate) enum RenderLinks { + Always, + Never, +} + +impl From for RenderLinks { + fn from(value: CliRenderLinks) -> Self { + match value { + CliRenderLinks::Auto => { + // We render links if stdout is a terminal. This is assumed + // to preclude CI environments and log files. + // + // TODO: Switch this to the support-hyperlinks crate? + // See: https://github.com/zkat/supports-hyperlinks/pull/8 + if stdout().is_terminal() { + RenderLinks::Always + } else { + RenderLinks::Never + } + } + CliRenderLinks::Always => RenderLinks::Always, + CliRenderLinks::Never => RenderLinks::Never, + } + } +} + #[derive(Debug, Default, Copy, Clone, ValueEnum)] pub(crate) enum CliShowAuditUrls { /// Render audit URLs in output automatically based on output format and runtime context. @@ -641,6 +688,7 @@ async fn run(app: &mut App) -> Result { ColorMode::Never } else if std::env::var("FORCE_COLOR").is_ok() || std::env::var("CLICOLOR_FORCE").is_ok() + || utils::is_ci() { ColorMode::Always } else { @@ -816,6 +864,7 @@ async fn run(app: &mut App) -> Result { ®istry, &results, &app.show_audit_urls.into(), + &app.render_links.into(), app.naches, ), OutputFormat::Json | OutputFormat::JsonV1 => { diff --git a/crates/zizmor/src/output/plain.rs b/crates/zizmor/src/output/plain.rs index d9227e0c..e10749c6 100644 --- a/crates/zizmor/src/output/plain.rs +++ b/crates/zizmor/src/output/plain.rs @@ -7,7 +7,7 @@ use anstream::{eprintln, print, println}; use owo_colors::OwoColorize; use crate::{ - ShowAuditUrls, + RenderLinks, ShowAuditUrls, finding::{ Finding, Severity, location::{Location, LocationKind}, @@ -44,6 +44,7 @@ impl From<&Severity> for Level<'_> { pub(crate) fn finding_snippets<'doc>( registry: &'doc InputRegistry, finding: &'doc Finding<'doc>, + render_links_mode: &RenderLinks, ) -> Vec>> { // Our finding might span multiple workflows, so we need to group locations // by their enclosing workflow to generate each snippet correctly. @@ -68,15 +69,20 @@ pub(crate) fn finding_snippets<'doc>( for (input_key, locations) in locations_by_workflow { let input = registry.get_input(input_key); + let path = match render_links_mode { + RenderLinks::Always => input.link().unwrap_or(input_key.presentation_path()), + RenderLinks::Never => input_key.presentation_path(), + }; + snippets.push( Snippet::source(input.as_document().source()) .fold(true) .line_start(1) - .path(input.link().unwrap_or(input_key.presentation_path())) + .path(path) .annotations(locations.iter().map(|loc| { - let annotation = match loc.symbolic.link { - Some(ref link) => link, - None => &loc.symbolic.annotation, + let annotation = match (loc.symbolic.link.as_deref(), render_links_mode) { + (Some(link), RenderLinks::Always) => link, + _ => &loc.symbolic.annotation, }; AnnotationKind::from(loc.symbolic.kind) @@ -96,10 +102,11 @@ pub(crate) fn render_findings( registry: &InputRegistry, findings: &FindingRegistry, show_urls_mode: &ShowAuditUrls, + render_links_mode: &RenderLinks, naches_mode: bool, ) { for finding in findings.findings() { - render_finding(registry, finding, show_urls_mode); + render_finding(registry, finding, show_urls_mode, render_links_mode); println!(); } @@ -192,11 +199,19 @@ pub(crate) fn render_findings( } } -fn render_finding(registry: &InputRegistry, finding: &Finding, show_urls_mode: &ShowAuditUrls) { - let title = Level::from(&finding.determinations.severity) +fn render_finding( + registry: &InputRegistry, + finding: &Finding, + show_urls_mode: &ShowAuditUrls, + render_links_mode: &RenderLinks, +) { + let mut title = Level::from(&finding.determinations.severity) .primary_title(finding.desc) - .id(finding.ident) - .id_url(finding.url); + .id(finding.ident); + + if matches!(render_links_mode, RenderLinks::Always) { + title = title.id_url(finding.url); + } let confidence = format!( "audit confidence → {:?}", @@ -204,7 +219,7 @@ fn render_finding(registry: &InputRegistry, finding: &Finding, show_urls_mode: & ); let mut group = Group::with_title(title) - .elements(finding_snippets(registry, finding)) + .elements(finding_snippets(registry, finding, render_links_mode)) .element(Level::NOTE.message(confidence)); if let Some(tip) = &finding.tip { diff --git a/crates/zizmor/tests/integration/common.rs b/crates/zizmor/tests/integration/common.rs index 6ae9f233..52cfcb1b 100644 --- a/crates/zizmor/tests/integration/common.rs +++ b/crates/zizmor/tests/integration/common.rs @@ -42,6 +42,7 @@ pub struct Zizmor { stdin: Option, unbuffer: bool, offline: bool, + gh_token: bool, inputs: Vec, config: Option, no_config: bool, @@ -53,13 +54,19 @@ pub struct Zizmor { impl Zizmor { /// Create a new zizmor runner. pub fn new() -> Self { - let cmd = Command::new(cargo::cargo_bin!()); + let mut cmd = Command::new(cargo::cargo_bin!()); + + // Our child `zizmor` process starts with a clean environment, to + // ensure we explicitly test interactions with things like `CI` + // and `GH_TOKEN`. + cmd.env_clear(); Self { cmd, stdin: None, unbuffer: false, offline: true, + gh_token: true, inputs: vec![], config: None, no_config: false, @@ -84,11 +91,6 @@ impl Zizmor { self } - pub fn unsetenv(mut self, key: &str) -> Self { - self.cmd.env_remove(key); - self - } - pub fn input(mut self, input: impl Into) -> Self { self.inputs.push(input.into()); self @@ -114,6 +116,11 @@ impl Zizmor { self } + pub fn gh_token(mut self, flag: bool) -> Self { + self.gh_token = flag; + self + } + pub fn output(mut self, output: OutputMode) -> Self { self.output = output; self @@ -147,7 +154,12 @@ impl Zizmor { } else { // If we're running in online mode, we pre-assert the // presence of GH_TOKEN to make configuration failures more obvious. - std::env::var("GH_TOKEN").context("online tests require GH_TOKEN to be set")?; + let token = + std::env::var("GH_TOKEN").context("online tests require GH_TOKEN to be set")?; + + if self.gh_token { + self.cmd.env("GH_TOKEN", token); + } } if self.no_config && self.config.is_some() { diff --git a/crates/zizmor/tests/integration/e2e.rs b/crates/zizmor/tests/integration/e2e.rs index d64ed9bb..fc56c027 100644 --- a/crates/zizmor/tests/integration/e2e.rs +++ b/crates/zizmor/tests/integration/e2e.rs @@ -83,13 +83,21 @@ fn menagerie() -> Result<()> { #[test] fn color_control_basic() -> Result<()> { - // No terminal, so no color by default. + // No terminal and not CI, so no color by default. let no_color_default_output = zizmor() .output(OutputMode::Both) .input(input_under_test("e2e-menagerie")) .run()?; assert!(!no_color_default_output.contains("\x1b[")); + // No terminal but CI, so color by default. + let color_default_ci_output = zizmor() + .setenv("CI", "true") + .output(OutputMode::Both) + .input(input_under_test("e2e-menagerie")) + .run()?; + assert!(color_default_ci_output.contains("\x1b[")); + // Force color via --color=always. let forced_color_via_arg_output = zizmor() .output(OutputMode::Both) @@ -600,7 +608,6 @@ fn test_cant_retrieve_offline() -> Result<()> { zizmor() .expects_failure(true) .offline(true) - .unsetenv("GH_TOKEN") .args(["pypa/sampleproject"]) .run()?, @r" @@ -626,7 +633,7 @@ fn test_cant_retrieve_no_gh_token() -> Result<()> { zizmor() .expects_failure(true) .offline(false) - .unsetenv("GH_TOKEN") + .gh_token(false) .args(["pypa/sampleproject"]) .run()?, @r" diff --git a/docs/release-notes.md b/docs/release-notes.md index 6b0beee7..6cc6859c 100644 --- a/docs/release-notes.md +++ b/docs/release-notes.md @@ -37,6 +37,10 @@ of `zizmor`. * zizmor now produces a more useful error message when input collection yields no inputs (#1439) +* The `--render-links` flag now allows users to control `zizmor`'s OSC 8 terminal + link rendering behavior. This is particularly useful in environments that + advertise themselves as terminals but fail to correctly render or ignore + OSC 8 links (#1454) ### Performance Improvements 🚄 @@ -54,6 +58,10 @@ of `zizmor`. * Fixed a bug where the `opentofu` ecosystem was not recognized in Dependabot configuration files (#1452) +* `--color=always` no longer implies `--render-links=always`, as some + environments (like GitHub Actions) support ANSI color codes but fail + to handle OSC escapes gracefully (#1454) + ## 1.18.0 ### Enhancements 🌱 diff --git a/docs/snippets/help.txt b/docs/snippets/help.txt index 0c3cd8fc..49ac502d 100644 --- a/docs/snippets/help.txt +++ b/docs/snippets/help.txt @@ -28,6 +28,8 @@ Options: Don't show progress bars, even if the terminal supports them --format The output format to emit. By default, cargo-style diagnostics will be emitted [default: plain] [possible values: plain, json, json-v1, sarif, github] + --render-links + Whether to render OSC 8 links in the output [env: ZIZMOR_RENDER_LINKS=] [default: auto] [possible values: auto, always, never] --show-audit-urls Whether to render audit URLs in the output, separately from any URLs embedded in OSC 8 links [env: ZIZMOR_SHOW_AUDIT_URLS=] [default: auto] [possible values: auto, always, never] --color