fix Deref impls, make key locations more flexible

Signed-off-by: William Woodruff <[email protected]>

src/audit/excessive_permissions.rs

@@ -63,7 +63,7 @@ impl<'a> WorkflowAudit<'a> for ExcessivePermissions<'a> {
                 Self::finding()
                     .severity(severity)
                     .confidence(confidence)
-                    .add_location(workflow.key_location("permissions").annotated(note))
+                    .add_location(workflow.key_location(&["permissions"]).annotated(note))
                     .build(workflow)?,
             )
         }
@@ -80,7 +80,7 @@ impl<'a> WorkflowAudit<'a> for ExcessivePermissions<'a> {
                     Self::finding()
                         .severity(severity)
                         .confidence(confidence)
-                        .add_location(job.key_location("permissions").annotated(note))
+                        .add_location(job.key_location(&["permissions"]).annotated(note))
                         .build(workflow)?,
                 )
             }

src/audit/hardcoded_container_credentials.rs

@@ -61,7 +61,7 @@ impl<'a> WorkflowAudit<'a> for HardcodedContainerCredentials<'a> {
                             .severity(Severity::High)
                             .confidence(Confidence::High)
                             .add_location(
-                                job.key_location("container")
+                                job.key_location(&["container", "credentials"])
                                     .annotated("container registry password is hard-coded"),
                             )
                             .build(workflow)?,
@@ -85,9 +85,17 @@ impl<'a> WorkflowAudit<'a> for HardcodedContainerCredentials<'a> {
                             Self::finding()
                                 .severity(Severity::High)
                                 .confidence(Confidence::High)
-                                .add_location(job.key_location("services").annotated(format!(
-                                    "service {service}: container registry password is hard-coded"
-                                )))
+                                .add_location(
+                                    job.key_location(&[
+                                        "services",
+                                        service.as_str(),
+                                        "credentials",
+                                    ])
+                                    .annotated(format!(
+                                        "service {service}: container registry password is \
+                                         hard-coded"
+                                    )),
+                                )
                                 .build(workflow)?,
                         )
                     }

src/audit/pull_request_target.rs

@@ -35,7 +35,7 @@ impl<'a> WorkflowAudit<'a> for PullRequestTarget<'a> {
                 Self::finding()
                     .confidence(Confidence::Medium)
                     .severity(Severity::High)
-                    .add_location(workflow.key_location("on").annotated(
+                    .add_location(workflow.key_location(&["on"]).annotated(
                         "triggers include pull_request_target, which is almost always used \
                          insecurely",
                     ))

src/finding/locate.rs

@@ -3,7 +3,7 @@
 
 use anyhow::Result;
 
-use super::{ConcreteLocation, Feature, JobOrKey, StepOrKey, WorkflowLocation};
+use super::{ConcreteLocation, Feature, JobOrKeys, StepOrKeys, WorkflowLocation};
 use crate::models::Workflow;
 
 pub(crate) struct Locator {}
@@ -21,18 +21,28 @@ impl Locator {
         let mut builder = yamlpath::QueryBuilder::new();
 
         builder = match &location.job_or_key {
-            Some(JobOrKey::Job(job)) => {
+            Some(JobOrKeys::Job(job)) => {
                 builder = builder.key("jobs").key(job.id);
 
-                match &job.step_or_key {
-                    Some(StepOrKey::Step(step)) => builder.key("steps").index(step.index),
-                    Some(StepOrKey::Key(key)) => builder.key(*key),
+                match &job.step_or_keys {
+                    Some(StepOrKeys::Step(step)) => builder.key("steps").index(step.index),
+                    Some(StepOrKeys::Keys(keys)) => {
+                        for key in keys {
+                            builder = builder.key(*key);
+                        }
+
+                        builder
+                    }
                     None => builder,
                 }
             }
-            Some(JobOrKey::Key(key)) => {
+            Some(JobOrKeys::Keys(keys)) => {
                 // Non-job top-level key.
-                builder.key(*key)
+                for key in keys {
+                    builder = builder.key(*key);
+                }
+
+                builder
             }
             None => panic!("API misuse: workflow location must specify a top-level key or job"),
         };

src/finding/mod.rs

@@ -46,8 +46,8 @@ impl<'w> From<&Step<'w>> for StepLocation<'w> {
 
 /// Represents a job-level key or step location.
 #[derive(Serialize, Clone, Debug)]
-pub(crate) enum StepOrKey<'w> {
-    Key(&'w str),
+pub(crate) enum StepOrKeys<'w> {
+    Keys(Vec<&'w str>),
     Step(StepLocation<'w>),
 }
 
@@ -59,19 +59,19 @@ pub(crate) struct JobLocation<'w> {
     /// The job's name, if present.
     pub(crate) name: Option<&'w str>,
 
-    /// The step or non-step key within this workflow.
-    pub(crate) step_or_key: Option<StepOrKey<'w>>,
+    /// The step or non-step keys within this workflow.
+    pub(crate) step_or_keys: Option<StepOrKeys<'w>>,
 }
 
 impl<'w> JobLocation<'w> {
-    /// Creates a new `JobLocation` with the given non-step `key`.
+    /// Creates a new `JobLocation` with the given non-step `keys`.
     ///
     /// Clears any `step` in the process.
-    pub(crate) fn with_key(&self, key: &'w str) -> JobLocation<'w> {
+    pub(crate) fn with_keys(&self, keys: &[&'w str]) -> JobLocation<'w> {
         JobLocation {
             id: self.id,
             name: self.name,
-            step_or_key: Some(StepOrKey::Key(key)),
+            step_or_keys: Some(StepOrKeys::Keys(keys.into())),
         }
     }
 
@@ -82,15 +82,15 @@ impl<'w> JobLocation<'w> {
         JobLocation {
             id: self.id,
             name: self.name,
-            step_or_key: Some(StepOrKey::Step(step.into())),
+            step_or_keys: Some(StepOrKeys::Step(step.into())),
         }
     }
 }
 
 /// Represents a workflow-level key or job location.
 #[derive(Serialize, Clone, Debug)]
-pub(crate) enum JobOrKey<'w> {
-    Key(&'w str),
+pub(crate) enum JobOrKeys<'w> {
+    Keys(Vec<&'w str>),
     Job(JobLocation<'w>),
 }
 
@@ -104,16 +104,16 @@ pub(crate) struct WorkflowLocation<'w> {
     pub(crate) annotation: Option<String>,
 
     /// The job or non-job key within this workflow.
-    pub(crate) job_or_key: Option<JobOrKey<'w>>,
+    pub(crate) job_or_key: Option<JobOrKeys<'w>>,
 }
 
 impl<'w> WorkflowLocation<'w> {
     /// Creates a new `WorkflowLocation` with the given `key`. Any inner
     /// job location is cleared.
-    pub(crate) fn with_key(&self, key: &'w str) -> WorkflowLocation<'w> {
+    pub(crate) fn with_keys(&self, keys: &[&'w str]) -> WorkflowLocation<'w> {
         WorkflowLocation {
             name: self.name,
-            job_or_key: Some(JobOrKey::Key(key)),
+            job_or_key: Some(JobOrKeys::Keys(keys.into())),
             annotation: self.annotation.clone(),
         }
     }
@@ -122,10 +122,10 @@ impl<'w> WorkflowLocation<'w> {
     pub(crate) fn with_job(&self, job: &Job<'w>) -> WorkflowLocation<'w> {
         WorkflowLocation {
             name: self.name,
-            job_or_key: Some(JobOrKey::Job(JobLocation {
+            job_or_key: Some(JobOrKeys::Job(JobLocation {
                 id: job.id,
                 name: job.inner.name(),
-                step_or_key: None,
+                step_or_keys: None,
             })),
             annotation: self.annotation.clone(),
         }
@@ -137,9 +137,9 @@ impl<'w> WorkflowLocation<'w> {
     /// since steps belong to jobs.
     pub(crate) fn with_step(&self, step: &Step<'w>) -> WorkflowLocation<'w> {
         match &self.job_or_key {
-            Some(JobOrKey::Job(job)) => WorkflowLocation {
+            Some(JobOrKeys::Job(job)) => WorkflowLocation {
                 name: self.name,
-                job_or_key: Some(JobOrKey::Job(job.with_step(step))),
+                job_or_key: Some(JobOrKeys::Job(job.with_step(step))),
                 annotation: self.annotation.clone(),
             },
             _ => panic!("API misuse: can't set step without parent job"),

src/models.rs

@@ -3,7 +3,7 @@ use std::{collections::hash_map, iter::Enumerate, ops::Deref, path::Path};
 use anyhow::{Context, Result};
 use github_actions_models::workflow;
 
-use crate::finding::{JobOrKey, WorkflowLocation};
+use crate::finding::{JobOrKeys, WorkflowLocation};
 
 pub(crate) struct Workflow {
     pub(crate) filename: String,
@@ -50,8 +50,8 @@ impl Workflow {
         }
     }
 
-    pub(crate) fn key_location(&self, key: &'static str) -> WorkflowLocation {
-        self.location().with_key(key)
+    pub(crate) fn key_location(&self, keys: &[&'static str]) -> WorkflowLocation {
+        self.location().with_keys(keys)
     }
 
     pub(crate) fn jobs(&self) -> Jobs<'_> {
@@ -66,10 +66,10 @@ pub(crate) struct Job<'w> {
 }
 
 impl<'w> Deref for Job<'w> {
-    type Target = workflow::Job;
+    type Target = &'w workflow::Job;
 
     fn deref(&self) -> &Self::Target {
-        self.inner
+        &self.inner
     }
 }
 
@@ -82,14 +82,14 @@ impl<'w> Job<'w> {
         self.parent.with_job(self)
     }
 
-    pub(crate) fn key_location(&self, key: &'w str) -> WorkflowLocation<'w> {
+    pub(crate) fn key_location(&self, keys: &[&'w str]) -> WorkflowLocation<'w> {
         let mut location = self.parent.with_job(self);
-        let Some(JobOrKey::Job(job)) = location.job_or_key else {
+        let Some(JobOrKeys::Job(job)) = location.job_or_key else {
             panic!("unreachable")
         };
-        let job = job.with_key(key);
+        let job = job.with_keys(keys);
 
-        location.job_or_key = Some(JobOrKey::Job(job));
+        location.job_or_key = Some(JobOrKeys::Job(job));
 
         location
     }
@@ -134,10 +134,10 @@ pub(crate) struct Step<'w> {
 }
 
 impl<'w> Deref for Step<'w> {
-    type Target = workflow::job::Step;
+    type Target = &'w workflow::job::Step;
 
-    fn deref(&self) -> &'w Self::Target {
-        self.inner
+    fn deref(&self) -> &Self::Target {
+        &self.inner
     }
 }