ci: add plain presentation test (#1454)

.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

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<CliRenderLinks> 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<ExitCode, Error> {
                 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<ExitCode, Error> {
             &registry,
             &results,
             &app.show_audit_urls.into(),
+            &app.render_links.into(),
             app.naches,
         ),
         OutputFormat::Json | OutputFormat::JsonV1 => {

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<Snippet<'doc, Annotation<'doc>>> {
     // 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 {

crates/zizmor/tests/integration/common.rs

@@ -42,6 +42,7 @@ pub struct Zizmor {
     stdin: Option<String>,
     unbuffer: bool,
     offline: bool,
+    gh_token: bool,
     inputs: Vec<String>,
     config: Option<String>,
     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<String>) -> 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() {

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"

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 🌱

docs/snippets/help.txt

@@ -28,6 +28,8 @@ Options:
           Don't show progress bars, even if the terminal supports them
       --format <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 <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 <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 <MODE>