feat: Add validation for extended Dependabot schedule intervals (#1247)

Co-authored-by: William Woodruff <william@yossarian.net>

Cargo.lock

@@ -840,7 +840,7 @@ dependencies = [
 
 [[package]]
 name = "github-actions-models"
-version = "0.34.0"
+version = "0.35.0"
 dependencies = [
  "indexmap",
  "serde",

Cargo.toml

@@ -20,7 +20,7 @@ rust-version = "1.88.0"
 [workspace.dependencies]
 anyhow = "1.0.100"
 github-actions-expressions = { path = "crates/github-actions-expressions", version = "0.0.10" }
-github-actions-models = { path = "crates/github-actions-models", version = "0.34.0" }
+github-actions-models = { path = "crates/github-actions-models", version = "0.35.0" }
 itertools = "0.14.0"
 pest = "2.8.3"
 pest_derive = "2.8.3"

crates/github-actions-models/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "github-actions-models"
-version = "0.34.0"
+version = "0.35.0"
 description = "Unofficial, high-quality data models for GitHub Actions workflows, actions, and related components"
 repository = "https://github.com/zizmorcore/zizmor/tree/main/crates/github-actions-models"
 keywords = ["github", "ci"]

crates/github-actions-models/src/dependabot/v2.rs

@@ -7,6 +7,8 @@
 use indexmap::{IndexMap, IndexSet};
 use serde::Deserialize;
 
+use crate::common::custom_error;
+
 /// A `dependabot.yml` configuration file.
 #[derive(Deserialize, Debug)]
 #[serde(rename_all = "kebab-case")]
@@ -310,12 +312,48 @@ pub enum RebaseStrategy {
 
 /// Scheduling settings for Dependabot updates.
 #[derive(Deserialize, Debug)]
-#[serde(rename_all = "kebab-case")]
+#[serde(rename_all = "kebab-case", remote = "Self")]
 pub struct Schedule {
     pub interval: Interval,
     pub day: Option<Day>,
     pub time: Option<String>,
     pub timezone: Option<String>,
+    pub cronjob: Option<String>,
+}
+
+impl<'de> Deserialize<'de> for Schedule {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        let schedule = Self::deserialize(deserializer)?;
+
+        if schedule.interval == Interval::Cron && schedule.cronjob.is_none() {
+            return Err(custom_error::<D>(
+                "`schedule.cronjob` must be set when `schedule.interval` is `cron`",
+            ));
+        }
+
+        if schedule.interval != Interval::Cron && schedule.cronjob.is_some() {
+            return Err(custom_error::<D>(
+                "`schedule.cronjob` may only be set when `schedule.interval` is `cron`",
+            ));
+        }
+
+        if schedule.interval != Interval::Weekly && schedule.day.is_some() {
+            return Err(custom_error::<D>(
+                "`schedule.day` is only valid when `schedule.interval` is `weekly`",
+            ));
+        }
+
+        Ok(Self {
+            interval: schedule.interval,
+            day: schedule.day,
+            time: schedule.time,
+            timezone: schedule.timezone,
+            cronjob: schedule.cronjob,
+        })
+    }
 }
 
 /// Schedule intervals.
@@ -325,6 +363,10 @@ pub enum Interval {
     Daily,
     Weekly,
     Monthly,
+    Quarterly,
+    Semiannually,
+    Yearly,
+    Cron,
 }
 
 /// Days of the week.

crates/github-actions-models/tests/sample-dependabot/v2/cron-missing-cronjob.invalid.yml

@@ -0,0 +1,6 @@
+version: 2
+updates:
+  - package-ecosystem: npm
+    directory: "/"
+    schedule:
+      interval: cron

crates/github-actions-models/tests/sample-dependabot/v2/cronjob-on-daily.invalid.yml

@@ -0,0 +1,7 @@
+version: 2
+updates:
+  - package-ecosystem: npm
+    directory: "/"
+    schedule:
+      interval: daily
+      cronjob: "0 3 * * *"

crates/github-actions-models/tests/sample-dependabot/v2/day-on-daily.invalid.yml

@@ -0,0 +1,7 @@
+version: 2
+updates:
+  - package-ecosystem: npm
+    directory: "/"
+    schedule:
+      interval: daily
+      day: monday

crates/github-actions-models/tests/sample-dependabot/v2/weekly-with-day.yml

@@ -0,0 +1,7 @@
+version: 2
+updates:
+  - package-ecosystem: npm
+    directory: "/"
+    schedule:
+      interval: weekly
+      day: friday

crates/github-actions-models/tests/test_dependabot_v2.rs

@@ -1,26 +1,51 @@
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use github_actions_models::dependabot::v2::{
-    Dependabot, Directories, Interval, PackageEcosystem, RebaseStrategy,
+    Day, Dependabot, Directories, Interval, PackageEcosystem, RebaseStrategy,
 };
 use indexmap::IndexSet;
 
+fn sample_dir() -> PathBuf {
+    Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/sample-dependabot/v2")
+}
+
+fn load_dependabot_result(name: &str) -> Result<Dependabot, serde_yaml::Error> {
+    let workflow_path = sample_dir().join(name);
+    let dependabot_contents = std::fs::read_to_string(&workflow_path)
+        .unwrap_or_else(|err| panic!("failed to read {}: {err}", workflow_path.display()));
+    serde_yaml::from_str(&dependabot_contents)
+}
+
 fn load_dependabot(name: &str) -> Dependabot {
-    let workflow_path = Path::new(env!("CARGO_MANIFEST_DIR"))
-        .join("tests/sample-dependabot/v2")
-        .join(name);
-    let dependabot_contents = std::fs::read_to_string(workflow_path).unwrap();
-    serde_yaml::from_str(&dependabot_contents).unwrap()
+    load_dependabot_result(name).unwrap()
 }
 
 #[test]
 fn test_load_all() {
-    let sample_configs = Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/sample-dependabot/v2");
+    for sample_config in std::fs::read_dir(sample_dir()).unwrap() {
+        let sample_path = sample_config.unwrap().path();
 
-    for sample_config in std::fs::read_dir(sample_configs).unwrap() {
-        let sample_workflow = sample_config.unwrap().path();
-        let contents = std::fs::read_to_string(sample_workflow).unwrap();
-        serde_yaml::from_str::<Dependabot>(&contents).unwrap();
+        if sample_path.extension().and_then(|ext| ext.to_str()) != Some("yml") {
+            continue;
+        }
+
+        let sample_name = sample_path
+            .file_name()
+            .and_then(|name| name.to_str())
+            .expect("sample file name not valid UTF-8");
+
+        let result = load_dependabot_result(sample_name);
+
+        let is_invalid = sample_name.contains(".invalid.");
+
+        if is_invalid {
+            assert!(
+                result.is_err(),
+                "expected {sample_name} to fail deserialization"
+            );
+        } else {
+            result.unwrap();
+        }
     }
 }
 
@@ -71,3 +96,39 @@ fn test_contents() {
         IndexSet::from(["*".to_string()])
     );
 }
+
+#[test]
+fn test_schedule_cron_requires_expression() {
+    let err = load_dependabot_result("cron-missing-cronjob.invalid.yml").unwrap_err();
+    assert!(
+        err.to_string()
+            .contains("`schedule.cronjob` must be set when `schedule.interval` is `cron`")
+    );
+}
+
+#[test]
+fn test_schedule_cronjob_rejected_for_non_cron() {
+    let err = load_dependabot_result("cronjob-on-daily.invalid.yml").unwrap_err();
+    assert!(
+        err.to_string()
+            .contains("`schedule.cronjob` may only be set when `schedule.interval` is `cron`")
+    );
+}
+
+#[test]
+fn test_schedule_day_only_for_weekly() {
+    let err = load_dependabot_result("day-on-daily.invalid.yml").unwrap_err();
+    assert!(
+        err.to_string()
+            .contains("`schedule.day` is only valid when `schedule.interval` is `weekly`")
+    );
+}
+
+#[test]
+fn test_schedule_weekly_accepts_day() {
+    let dependabot = load_dependabot("weekly-with-day.yml");
+    assert_eq!(dependabot.updates.len(), 1);
+    let schedule = &dependabot.updates[0].schedule;
+    assert_eq!(schedule.interval, Interval::Weekly);
+    assert_eq!(schedule.day, Some(Day::Friday));
+}

docs/release-notes.md

@@ -9,6 +9,13 @@ of `zizmor`.
 
 ## Next (UNRELEASED)
 
+## 1.15.2
+
+### Bug Fixes 🐛
+
+* Fixed a bug where `zizmor` would fail to parse some Dependabot configuration
+  files due to missing support for some schedule formats (#1247)
+
 ## 1.15.1
 
 ### Bug Fixes 🐛