From a98f20ead788e9d4bf21d77e70d379aefa64b0db Mon Sep 17 00:00:00 2001 From: Nate Scherer <376408+natescherer@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:12:23 -0400 Subject: [PATCH] feat(repository): manage private vulnerability reporting `repository.security.private_vulnerability_reporting` is applied through its own endpoint, PUT to enable and DELETE to disable, since GitHub keeps it out of `security_and_analysis`. A setting GitHub does not report still plans a change, as elsewhere. Verified against a real repository: plan, sync and re-plan converge in both directions, and export includes it. --- README.md | 1 + docs/configuration.md | 1 + docs/roadmap.md | 2 +- schema/v1/settings.json | 7 ++ src/config/merge/mod.rs | 2 + src/resources/repository/mod.rs | 84 +++++++++++++++++++-- src/resources/repository/model.rs | 12 ++- src/resources/repository/tests.rs | 118 ++++++++++++++++++++++++++++++ tests/live.rs | 21 ++++++ tests/sync.rs | 62 ++++++++++++++++ 10 files changed, 302 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index d62f439..8934a82 100644 --- a/README.md +++ b/README.md @@ -169,6 +169,7 @@ repository: security: secret_scanning: true secret_scanning_push_protection: true + private_vulnerability_reporting: true topics: - rust diff --git a/docs/configuration.md b/docs/configuration.md index eec801c..2353b23 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -287,6 +287,7 @@ Security and analysis features. |---|---|---|---| | `advanced_security` | boolean | no | Dependency graph advanced security (private repositories). | | `dependabot_security_updates` | boolean | no | Automatic Dependabot security fixes. | +| `private_vulnerability_reporting` | boolean | no | Private vulnerability reporting, which lets anyone report a security vulnerability to the maintainers privately. | | `secret_scanning` | boolean | no | Secret scanning. | | `secret_scanning_push_protection` | boolean | no | Secret scanning push protection. | | `secret_scanning_validity_checks` | boolean | no | Secret scanning validity checks. | diff --git a/docs/roadmap.md b/docs/roadmap.md index 26def93..801b4b2 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -14,7 +14,7 @@ eventually be manageable here.* | Resource | Status | Notes | |---|---|---| | Repository metadata | ✅ | description, homepage, features, merge strategies, commit title/message, default branch | -| Repository security | ✅ | secret scanning, push protection, Dependabot updates, advanced security | +| Repository security | ✅ | secret scanning, push protection, Dependabot updates, advanced security, private vulnerability reporting | | Topics | ✅ | | | Labels | ✅ | including renames, which preserve issue assignments | | Autolinks | ✅ | changes are delete-and-recreate; GitHub has no update endpoint | diff --git a/schema/v1/settings.json b/schema/v1/settings.json index facf4c0..e550b9a 100644 --- a/schema/v1/settings.json +++ b/schema/v1/settings.json @@ -1074,6 +1074,13 @@ "null" ] }, + "private_vulnerability_reporting": { + "description": "Private vulnerability reporting, which lets anyone report a security\nvulnerability to the maintainers privately.", + "type": [ + "boolean", + "null" + ] + }, "secret_scanning": { "description": "Secret scanning.", "type": [ diff --git a/src/config/merge/mod.rs b/src/config/merge/mod.rs index d5d5350..595fc00 100644 --- a/src/config/merge/mod.rs +++ b/src/config/merge/mod.rs @@ -322,6 +322,7 @@ fn merge_security( secret_scanning_push_protection, dependabot_security_updates, secret_scanning_validity_checks, + private_vulnerability_reporting, } = c; macro_rules! field { @@ -343,6 +344,7 @@ fn merge_security( secret_scanning_push_protection: field!(secret_scanning_push_protection), dependabot_security_updates: field!(dependabot_security_updates), secret_scanning_validity_checks: field!(secret_scanning_validity_checks), + private_vulnerability_reporting: field!(private_vulnerability_reporting), }) } diff --git a/src/resources/repository/mod.rs b/src/resources/repository/mod.rs index 9475755..8f8bce6 100644 --- a/src/resources/repository/mod.rs +++ b/src/resources/repository/mod.rs @@ -1,9 +1,10 @@ //! Repository metadata. //! //! A singleton resource: rather than a collection of items, it produces at most -//! two changes — one `PATCH /repos/{owner}/{repo}` covering the plain fields, and -//! one per security feature, which GitHub exposes through a differently shaped -//! sub-object. +//! three changes — one `PATCH /repos/{owner}/{repo}` covering the plain fields, +//! one for the security features GitHub exposes through a differently shaped +//! sub-object, and one for private vulnerability reporting, which has an +//! endpoint of its own. use async_trait::async_trait; use serde::{Deserialize, Serialize}; @@ -103,6 +104,21 @@ pub struct Current { /// Security and analysis features. #[serde(default)] pub security_and_analysis: Option>, + /// Whether private vulnerability reporting is enabled. + /// + /// Not part of the repository object: [`Repository::current`] reads it from + /// `GET /repos/{owner}/{repo}/private-vulnerability-reporting` and fills it + /// in. `None` when GitHub did not report it, which is a different claim + /// from reporting it disabled. + #[serde(skip)] + pub private_vulnerability_reporting: Option, +} + +/// The private vulnerability reporting endpoint's answer. +#[derive(Debug, Clone, Default, Deserialize)] +struct PrivateVulnerabilityReportingState { + #[serde(default)] + enabled: Option, } impl Current { @@ -161,6 +177,12 @@ pub enum Payload { Settings(Value), /// A `PATCH` targeting `security_and_analysis`. Security(Value), + /// A `PUT` (enable) or `DELETE` (disable) of + /// `private-vulnerability-reporting`. + PrivateVulnerabilityReporting { + /// Whether to enable it. + enabled: bool, + }, } #[async_trait] @@ -189,7 +211,15 @@ impl Resource for Repository { client: &dyn GitHubClient, target: &Target, ) -> GitHubResult { - let current: Current = client.send(Request::get(target.endpoint(""))).await?; + let mut current: Current = client.send(Request::get(target.endpoint(""))).await?; + // `send_optional` because the endpoint may not exist at all, as on an + // older GitHub Enterprise Server; `None` then means "not reported". + let reporting: Option = client + .send_optional(Request::get( + target.endpoint("private-vulnerability-reporting"), + )) + .await?; + current.private_vulnerability_reporting = reporting.and_then(|state| state.enabled); Ok(current.normalized()) } @@ -426,6 +456,35 @@ impl Resource for Repository { )), ); } + + // A declared setting GitHub did not report still produces a change: + // claiming it is up to date would be a convergence nobody observed, + // and applying it surfaces GitHub's own answer. + if let Some(enabled) = security.private_vulnerability_reporting + && current.private_vulnerability_reporting != Some(enabled) + { + changes.push( + Change::new( + ResourceId::Repository, + Op::Update, + "private_vulnerability_reporting", + ) + .summary(if enabled { + "enable private vulnerability reporting" + } else { + "disable private vulnerability reporting" + }) + .fields(vec![FieldDiff::changed( + "private_vulnerability_reporting", + current + .private_vulnerability_reporting + .map(|value| value.to_string()) + .unwrap_or_else(|| "(not reported)".into()), + enabled.to_string(), + )]) + .payload(Payload::PrivateVulnerabilityReporting { enabled }), + ); + } } changes @@ -443,6 +502,17 @@ impl Resource for Repository { let body = match payload { Payload::Settings(body) | Payload::Security(body) => body, + // The endpoint takes no body: enabling is a `PUT`, disabling a + // `DELETE`. + Payload::PrivateVulnerabilityReporting { enabled } => { + let endpoint = target.endpoint("private-vulnerability-reporting"); + let request = if enabled { + Request::put(endpoint, json!({})) + } else { + Request::delete(endpoint) + }; + return client.execute(request).await; + } }; client @@ -510,8 +580,9 @@ impl Resource for Repository { /// Export the security block, omitting it when the API reported nothing. fn export_security(current: &Current) -> Option { - let features = current.security_and_analysis.as_ref()?; - if features.is_empty() { + let empty = Map::new(); + let features = current.security_and_analysis.as_ref().unwrap_or(&empty); + if features.is_empty() && current.private_vulnerability_reporting.is_none() { return None; } Some(SecuritySettings { @@ -530,6 +601,7 @@ fn export_security(current: &Current) -> Option { secret_scanning_validity_checks: features .contains_key("secret_scanning_validity_checks") .then(|| current.security_enabled("secret_scanning_validity_checks")), + private_vulnerability_reporting: current.private_vulnerability_reporting, }) } diff --git a/src/resources/repository/model.rs b/src/resources/repository/model.rs index f66fee3..06e0002 100644 --- a/src/resources/repository/model.rs +++ b/src/resources/repository/model.rs @@ -237,10 +237,19 @@ pub struct SecuritySettings { /// Secret scanning validity checks. #[serde(default, skip_serializing_if = "Option::is_none")] pub secret_scanning_validity_checks: Option, + + /// Private vulnerability reporting, which lets anyone report a security + /// vulnerability to the maintainers privately. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub private_vulnerability_reporting: Option, } impl SecuritySettings { - /// Every declared feature, as `(api_key, enabled)` pairs. + /// Every declared `security_and_analysis` feature, as `(api_key, enabled)` + /// pairs. + /// + /// `private_vulnerability_reporting` is not one of them: GitHub manages it + /// through its own endpoint, so the resource diffs it separately. pub fn declared(&self) -> Vec<(&'static str, bool)> { [ ("advanced_security", self.advanced_security), @@ -483,6 +492,7 @@ mod tests { let security = SecuritySettings { secret_scanning: Some(true), advanced_security: Some(false), + private_vulnerability_reporting: Some(true), ..Default::default() }; assert_eq!( diff --git a/src/resources/repository/tests.rs b/src/resources/repository/tests.rs index 85f276b..b52fc73 100644 --- a/src/resources/repository/tests.rs +++ b/src/resources/repository/tests.rs @@ -28,9 +28,28 @@ fn plan(desired_source: &str, current_value: Value) -> Vec { fn body(change: &Change) -> Value { match change.decode::().unwrap() { Payload::Settings(body) | Payload::Security(body) => body, + Payload::PrivateVulnerabilityReporting { enabled } => json!(enabled), } } +/// A current state with private vulnerability reporting as GitHub reported it. +/// +/// Set after deserialising, as the real `current()` does: the repository +/// object never carries the field. +fn current_with_reporting(value: Value, reporting: Option) -> Current { + let mut current = current(value); + current.private_vulnerability_reporting = reporting; + current +} + +fn plan_with_reporting(desired_source: &str, reporting: Option) -> Vec { + Repository.diff( + &desired(desired_source), + ¤t_with_reporting(json!({}), reporting), + &PruneOpts::default(), + ) +} + #[test] fn an_empty_section_produces_no_change() { assert!(plan("{}", json!({"description": "hello"})).is_empty()); @@ -301,3 +320,102 @@ fn web_commit_signoff_is_required_when_declared() { json!(true) ); } + +#[test] +fn private_vulnerability_reporting_travels_in_its_own_request() { + // It has its own endpoint, so it can share a body with neither the + // repository PATCH nor `security_and_analysis`. + let changes = Repository.diff( + &desired( + "description: new\nsecurity:\n secret_scanning: true\n private_vulnerability_reporting: true", + ), + ¤t_with_reporting( + json!({ + "description": "old", + "security_and_analysis": {"secret_scanning": {"status": "disabled"}} + }), + Some(false), + ), + &PruneOpts::default(), + ); + assert_eq!(changes.len(), 3); + assert_eq!( + changes[2].decode::().unwrap(), + Payload::PrivateVulnerabilityReporting { enabled: true } + ); + assert!( + body(&changes[1])["security_and_analysis"] + .get("private_vulnerability_reporting") + .is_none(), + "private vulnerability reporting leaked into security_and_analysis" + ); +} + +#[test] +fn enabling_private_vulnerability_reporting_is_planned() { + let changes = plan_with_reporting( + "security:\n private_vulnerability_reporting: true", + Some(false), + ); + assert_eq!(changes.len(), 1); + assert_eq!(changes[0].op, Op::Update); + assert_eq!(changes[0].summary, "enable private vulnerability reporting"); + assert_eq!( + changes[0].decode::().unwrap(), + Payload::PrivateVulnerabilityReporting { enabled: true } + ); +} + +#[test] +fn disabling_private_vulnerability_reporting_is_planned() { + let changes = plan_with_reporting( + "security:\n private_vulnerability_reporting: false", + Some(true), + ); + assert_eq!(changes.len(), 1); + assert_eq!( + changes[0].summary, + "disable private vulnerability reporting" + ); + assert_eq!( + changes[0].decode::().unwrap(), + Payload::PrivateVulnerabilityReporting { enabled: false } + ); +} + +#[test] +fn private_vulnerability_reporting_already_in_place_produces_no_change() { + assert!( + plan_with_reporting( + "security:\n private_vulnerability_reporting: true", + Some(true) + ) + .is_empty() + ); +} + +#[test] +fn private_vulnerability_reporting_is_left_alone_when_omitted() { + assert!(plan_with_reporting("security:\n secret_scanning: false", Some(true)).is_empty()); +} + +#[test] +fn unreported_private_vulnerability_reporting_is_still_planned() { + // Saying "up to date" about a setting GitHub never reported would claim a + // convergence nobody observed; applying it lets GitHub answer instead. + let changes = plan_with_reporting("security:\n private_vulnerability_reporting: false", None); + assert_eq!(changes.len(), 1); + assert_eq!( + changes[0].fields[0].before.as_deref(), + Some("(not reported)") + ); +} + +#[test] +fn private_vulnerability_reporting_is_exported_only_when_reported() { + assert_eq!(export_security(¤t(json!({}))), None); + + let exported = export_security(¤t_with_reporting(json!({}), Some(true))) + .expect("a reported setting is exported"); + assert_eq!(exported.private_vulnerability_reporting, Some(true)); +} diff --git a/tests/live.rs b/tests/live.rs index d26a99c..3171434 100644 --- a/tests/live.rs +++ b/tests/live.rs @@ -191,6 +191,27 @@ fn live_repository_security_travels_in_its_own_request() { .expect_up_to_date(); } +#[test] +#[ignore = "live: requires GH_SETTINGS_TEST_REPO"] +fn live_private_vulnerability_reporting_round_trips() { + let live = live_or_skip!(); + + // Its endpoint takes no body: enabling is a PUT and disabling a DELETE. + // Both directions, in that order, so the sandbox ends as it began. + for enabled in [true, false] { + live.config(&only(&format!( + "repository: + security: + private_vulnerability_reporting: {enabled} +" + ))); + live.run(&["sync", "--yes", "--only", "repository"]) + .expect_status(0); + live.run(&["plan", "--only", "repository"]) + .expect_up_to_date(); + } +} + #[test] #[ignore = "live: requires GH_SETTINGS_TEST_REPO"] fn live_export_round_trips_to_an_empty_plan() { diff --git a/tests/sync.rs b/tests/sync.rs index 13fe3dd..d10ee61 100644 --- a/tests/sync.rs +++ b/tests/sync.rs @@ -1806,3 +1806,65 @@ fn actions_endpoints_that_do_not_apply_are_not_read_as_failures() { output.stderr ); } + +#[test] +fn private_vulnerability_reporting_is_enabled_through_its_own_endpoint() { + let runner = Sandbox::new() + .config("version: 1\nrepository:\n security:\n private_vulnerability_reporting: true\n") + .repository(&default_repository()) + .get( + "repos/o/r/private-vulnerability-reporting", + r#"{"enabled": false}"#, + ) + .accept("PUT", "repos/o/r/private-vulnerability-reporting") + .build(); + + let output = runner.run(&["sync", "-R", "o/r", "--yes"]); + output.expect_status(0); + + let writes = output.writes(); + assert_eq!(writes.len(), 1, "{writes:?}"); + assert!( + writes[0].starts_with("PUT repos/o/r/private-vulnerability-reporting"), + "{writes:?}" + ); +} + +#[test] +fn private_vulnerability_reporting_is_disabled_with_a_delete() { + let runner = Sandbox::new() + .config( + "version: 1\nrepository:\n security:\n private_vulnerability_reporting: false\n", + ) + .repository(&default_repository()) + .get( + "repos/o/r/private-vulnerability-reporting", + r#"{"enabled": true}"#, + ) + .accept("DELETE", "repos/o/r/private-vulnerability-reporting") + .build(); + + let output = runner.run(&["sync", "-R", "o/r", "--yes"]); + output.expect_status(0); + + let writes = output.writes(); + assert_eq!(writes.len(), 1, "{writes:?}"); + assert!( + writes[0].starts_with("DELETE repos/o/r/private-vulnerability-reporting"), + "{writes:?}" + ); +} + +#[test] +fn private_vulnerability_reporting_already_enabled_is_up_to_date() { + let runner = Sandbox::new() + .config("version: 1\nrepository:\n security:\n private_vulnerability_reporting: true\n") + .repository(&default_repository()) + .get( + "repos/o/r/private-vulnerability-reporting", + r#"{"enabled": true}"#, + ) + .build(); + + runner.run(&["plan", "-R", "o/r"]).expect_status(0); +}