feat: detect shell: cmd as obfuscation (#1361)

crates/zizmor/src/audit/obfuscation.rs

@@ -11,7 +11,7 @@ use crate::{
         location::{Feature, Location, Routable},
     },
     models::{StepCommon, action::CompositeStep, workflow::Step},
-    utils::parse_fenced_expressions_from_input,
+    utils::{self, parse_fenced_expressions_from_input},
 };
 use subfeature::Subfeature;
 
@@ -190,30 +190,58 @@ impl Obfuscation {
     ) -> Result<Vec<Finding<'doc>>, AuditError> {
         let mut findings = vec![];
 
-        if let Some(Uses::Repository(uses)) = step.uses() {
-            let obfuscated_annotations = self.obfuscated_repo_uses(uses);
-            if !obfuscated_annotations.is_empty() {
-                let mut finding_builder = Self::finding()
-                    .confidence(Confidence::High)
-                    .severity(Severity::Low);
+        match step.body() {
+            crate::models::StepBodyCommon::Uses {
+                uses: Uses::Repository(uses),
+                ..
+            } => {
+                let obfuscated_annotations = self.obfuscated_repo_uses(uses);
+                if !obfuscated_annotations.is_empty() {
+                    let mut finding_builder = Self::finding()
+                        .confidence(Confidence::High)
+                        .severity(Severity::Low);
 
-                // Add all annotations as locations
-                for annotation in &obfuscated_annotations {
-                    finding_builder = finding_builder.add_location(
-                        step.location()
-                            .primary()
-                            .with_keys(["uses".into()])
-                            .annotated(*annotation),
+                    // Add all annotations as locations
+                    for annotation in &obfuscated_annotations {
+                        finding_builder = finding_builder.add_location(
+                            step.location()
+                                .primary()
+                                .with_keys(["uses".into()])
+                                .annotated(*annotation),
+                        );
+                    }
+
+                    // Try to create a fix for the obfuscated uses path
+                    if let Some(fix) = self.create_uses_fix(uses, step) {
+                        finding_builder = finding_builder.fix(fix);
+                    }
+
+                    findings.push(finding_builder.build(step).map_err(Self::err)?);
+                }
+            }
+            crate::models::StepBodyCommon::Run { .. } => {
+                if let Some("cmd" | "cmd.exe") = step.shell().map(utils::normalize_shell) {
+                    // `shell: cmd` is basically impossible to analyze: it has no formal
+                    // grammar and has several line continuation mechanisms that stymie
+                    // naive matching. It also hasn't been the default shell on Windows
+                    // runners since 2019.
+                    findings.push(
+                        Self::finding()
+                            .confidence(Confidence::High)
+                            .severity(Severity::Low)
+                            .add_location(
+                                step.location()
+                                    .primary()
+                                    .with_keys(["shell".into()])
+                                    .annotated("Windows CMD shell limits analysis"),
+                            )
+                            .tip("use 'shell: pwsh' or 'shell: bash' for improved analysis")
+                            .build(step)
+                            .map_err(Self::err)?,
                     );
                 }
-
-                // Try to create a fix for the obfuscated uses path
-                if let Some(fix) = self.create_uses_fix(uses, step) {
-                    finding_builder = finding_builder.fix(fix);
-                }
-
-                findings.push(finding_builder.build(step).map_err(Self::err)?);
             }
+            _ => {}
         }
 
         Ok(findings)

crates/zizmor/src/finding.rs

@@ -126,6 +126,8 @@ pub(crate) struct Finding<'doc> {
     /// and carries metadata about how an output layer might choose to
     /// present it.
     pub(crate) locations: Vec<Location<'doc>>,
+    /// A tip or recommendation associated with this finding.
+    pub(crate) tip: Option<String>,
     /// Whether this finding is ignored, either via inline comments or
     /// through a user's configuration.
     pub(crate) ignored: bool,
@@ -176,6 +178,7 @@ pub(crate) struct FindingBuilder<'doc> {
     persona: Persona,
     raw_locations: Vec<Location<'doc>>,
     locations: Vec<SymbolicLocation<'doc>>,
+    tip: Option<String>,
     fixes: Vec<Fix<'doc>>,
 }
 
@@ -190,6 +193,7 @@ impl<'doc> FindingBuilder<'doc> {
             persona: Default::default(),
             raw_locations: vec![],
             locations: vec![],
+            tip: None,
             fixes: vec![],
         }
     }
@@ -219,6 +223,11 @@ impl<'doc> FindingBuilder<'doc> {
         self
     }
 
+    pub(crate) fn tip(mut self, tip: impl Into<String>) -> Self {
+        self.tip = Some(tip.into());
+        self
+    }
+
     pub(crate) fn fix(mut self, fix: Fix<'doc>) -> Self {
         self.fixes.push(fix);
         self
@@ -256,6 +265,7 @@ impl<'doc> FindingBuilder<'doc> {
                 persona: self.persona,
             },
             locations,
+            tip: self.tip,
             ignored: should_ignore,
             fixes: self.fixes,
         })

crates/zizmor/src/output/plain.rs

@@ -205,6 +205,10 @@ fn render_finding(registry: &InputRegistry, finding: &Finding) {
         .elements(finding_snippets(registry, finding))
         .element(Level::NOTE.message(confidence));
 
+    if let Some(tip) = &finding.tip {
+        group = group.element(Level::HELP.with_name("tip").message(tip));
+    }
+
     if !finding.fixes.is_empty() {
         group = group.element(Level::NOTE.message("this finding has an auto-fix"));
     }

crates/zizmor/tests/integration/snapshots/integration__snapshot__github_env.snap

@@ -29,4 +29,13 @@ error[github-env]: dangerous use of environment file
    |
    = note: audit confidence → Low
 
-3 findings: 0 informational, 0 low, 0 medium, 3 high
+help[obfuscation]: obfuscated usage of GitHub Actions features
+  --> @@INPUT@@:22:7
+   |
+22 |       shell: cmd
+   |       ^^^^^^^^^^ Windows CMD shell limits analysis
+   |
+   = note: audit confidence → High
+   = tip: use 'shell: pwsh' or 'shell: bash' for improved analysis
+
+4 findings: 0 informational, 1 low, 0 medium, 3 high