-
Notifications
You must be signed in to change notification settings - Fork 6
feat(pay): add forward-compatible enum handling #376
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| use std::{env, fs, path::PathBuf}; | ||
|
|
||
| fn main() { | ||
| println!("cargo:rerun-if-changed=src/pay/openapi.json"); | ||
|
|
||
| // Only generate pay API if the pay feature is enabled | ||
| if env::var("CARGO_FEATURE_PAY").is_ok() { | ||
| generate_pay_api(); | ||
| } | ||
| } | ||
|
|
||
| fn generate_pay_api() { | ||
| let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap()); | ||
| let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap()); | ||
| let spec_path = manifest_dir.join("src/pay/openapi.json"); | ||
|
|
||
| // Read and preprocess the OpenAPI spec as JSON | ||
| let spec_content = | ||
| fs::read_to_string(&spec_path).expect("Failed to read openapi.json"); | ||
| let mut spec: serde_json::Value = serde_json::from_str(&spec_content) | ||
| .expect("Failed to parse openapi.json"); | ||
|
|
||
| // Remove enum constraints from specific schemas to make them | ||
| // forward-compatible (unknown values won't fail deserialization) | ||
| remove_enum_constraint(&mut spec, "PaymentStatus"); | ||
| remove_enum_constraint(&mut spec, "CollectDataFieldType"); | ||
|
|
||
| // Write the preprocessed spec | ||
| let preprocessed_path = out_dir.join("pay_openapi_preprocessed.json"); | ||
| fs::write(&preprocessed_path, serde_json::to_string_pretty(&spec).unwrap()) | ||
| .expect("Failed to write preprocessed spec"); | ||
|
|
||
| // Parse preprocessed JSON as OpenAPI spec | ||
| let file = fs::File::open(&preprocessed_path).unwrap(); | ||
| let spec: openapiv3::OpenAPI = | ||
| serde_json::from_reader(file).expect("Failed to parse as OpenAPI"); | ||
|
|
||
| // Generate progenitor code with Builder interface and Separate tags | ||
| let mut settings = progenitor::GenerationSettings::default(); | ||
| settings.with_interface(progenitor::InterfaceStyle::Builder); | ||
| settings.with_tag(progenitor::TagStyle::Separate); | ||
| settings.with_derive("PartialEq".to_string()); | ||
| let mut generator = progenitor::Generator::new(&settings); | ||
| let tokens = generator | ||
| .generate_tokens(&spec) | ||
| .expect("Failed to generate progenitor tokens"); | ||
| let ast = syn::parse2(tokens).expect("Failed to parse generated tokens"); | ||
| let content = prettyplease::unparse(&ast); | ||
|
|
||
| let codegen_path = out_dir.join("pay_codegen.rs"); | ||
| fs::write(&codegen_path, content).expect("Failed to write generated code"); | ||
| } | ||
|
|
||
| fn remove_enum_constraint(spec: &mut serde_json::Value, schema_name: &str) { | ||
| if let Some(schema) = | ||
| spec.pointer_mut(&format!("/components/schemas/{}", schema_name)) | ||
| { | ||
| if let Some(obj) = schema.as_object_mut() { | ||
| obj.remove("enum"); | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,10 @@ | ||
| progenitor::generate_api!( | ||
| spec = "src/pay/openapi.json", | ||
| interface = Builder, | ||
| tags = Separate, | ||
| derives = [PartialEq], | ||
| ); | ||
| // Progenitor generates elided lifetimes that trigger this lint | ||
| #![allow(mismatched_lifetime_syntaxes)] | ||
| include!(concat!(env!("OUT_DIR"), "/pay_codegen.rs")); | ||
Check failureCode scanning / CodeQL Cleartext transmission of sensitive information High
This 'post' operation transmits data which may contain unencrypted sensitive data from
builder.api_key(...) Error loading related location Loading This 'post' operation transmits data which may contain unencrypted sensitive data from builder.api_key(...) Error loading related location Loading Copilot AutofixAI 4 months ago Copilot could not generate an autofix suggestion Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support. |
||
|
|
||
| mod error_reporting; | ||
| mod observability; | ||
| mod serde_enums; | ||
|
|
||
| #[cfg(any(feature = "uniffi", feature = "wasm"))] | ||
| pub mod json; | ||
|
|
@@ -307,27 +305,26 @@ | |
| pub client_id: Option<String>, | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] | ||
| #[derive(Debug, Clone, PartialEq)] | ||
| #[cfg_attr(feature = "uniffi", derive(uniffi::Enum))] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub enum PaymentStatus { | ||
| RequiresAction, | ||
| Processing, | ||
| Succeeded, | ||
| Failed, | ||
| Expired, | ||
| Unknown { value: String }, | ||
| } | ||
|
|
||
| impl From<types::PaymentStatus> for PaymentStatus { | ||
| fn from(s: types::PaymentStatus) -> Self { | ||
| match s { | ||
| types::PaymentStatus::RequiresAction => { | ||
| PaymentStatus::RequiresAction | ||
| } | ||
| types::PaymentStatus::Processing => PaymentStatus::Processing, | ||
| types::PaymentStatus::Succeeded => PaymentStatus::Succeeded, | ||
| types::PaymentStatus::Failed => PaymentStatus::Failed, | ||
| types::PaymentStatus::Expired => PaymentStatus::Expired, | ||
| match s.as_str() { | ||
| "requires_action" => PaymentStatus::RequiresAction, | ||
| "processing" => PaymentStatus::Processing, | ||
| "succeeded" => PaymentStatus::Succeeded, | ||
| "failed" => PaymentStatus::Failed, | ||
| "expired" => PaymentStatus::Expired, | ||
| other => PaymentStatus::Unknown { value: other.to_string() }, | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -389,23 +386,22 @@ | |
| } | ||
| } | ||
|
|
||
| #[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)] | ||
| #[derive(Debug, Clone, PartialEq)] | ||
| #[cfg_attr(feature = "uniffi", derive(uniffi::Enum))] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub enum CollectDataFieldType { | ||
| Text, | ||
| Date, | ||
| Checkbox, | ||
| Unknown { value: String }, | ||
| } | ||
|
|
||
| impl From<types::CollectDataFieldType> for CollectDataFieldType { | ||
| fn from(t: types::CollectDataFieldType) -> Self { | ||
| match t { | ||
| types::CollectDataFieldType::Text => CollectDataFieldType::Text, | ||
| types::CollectDataFieldType::Date => CollectDataFieldType::Date, | ||
| types::CollectDataFieldType::Checkbox => { | ||
| CollectDataFieldType::Checkbox | ||
| } | ||
| match t.as_str() { | ||
| "text" => CollectDataFieldType::Text, | ||
| "date" => CollectDataFieldType::Date, | ||
| "checkbox" => CollectDataFieldType::Checkbox, | ||
| other => CollectDataFieldType::Unknown { value: other.to_string() }, | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -2402,4 +2398,48 @@ | |
| result | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_payment_status_forward_compatible() { | ||
| assert_eq!( | ||
| PaymentStatus::from(types::PaymentStatus::from( | ||
| "succeeded".to_string() | ||
| )), | ||
| PaymentStatus::Succeeded | ||
| ); | ||
| assert_eq!( | ||
| PaymentStatus::from(types::PaymentStatus::from( | ||
| "processing".to_string() | ||
| )), | ||
| PaymentStatus::Processing | ||
| ); | ||
| assert_eq!( | ||
| PaymentStatus::from(types::PaymentStatus::from( | ||
| "new_future_status".to_string() | ||
| )), | ||
| PaymentStatus::Unknown { value: "new_future_status".to_string() } | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_collect_data_field_type_forward_compatible() { | ||
| assert_eq!( | ||
| CollectDataFieldType::from(types::CollectDataFieldType::from( | ||
| "text".to_string() | ||
| )), | ||
| CollectDataFieldType::Text | ||
| ); | ||
| assert_eq!( | ||
| CollectDataFieldType::from(types::CollectDataFieldType::from( | ||
| "date".to_string() | ||
|
Comment on lines
+2402
to
+2434
|
||
| )), | ||
| CollectDataFieldType::Date | ||
| ); | ||
| assert_eq!( | ||
| CollectDataFieldType::from(types::CollectDataFieldType::from( | ||
| "future_type".to_string() | ||
| )), | ||
| CollectDataFieldType::Unknown { value: "future_type".to_string() } | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,154 @@ | ||
| use { | ||
| super::{CollectDataFieldType, PaymentStatus}, | ||
| serde::{de::Deserializer, ser::Serializer}, | ||
| }; | ||
|
|
||
| impl serde::Serialize for PaymentStatus { | ||
| fn serialize<S: Serializer>( | ||
| &self, | ||
| serializer: S, | ||
| ) -> Result<S::Ok, S::Error> { | ||
| serializer.serialize_str(match self { | ||
| Self::RequiresAction => "requires_action", | ||
| Self::Processing => "processing", | ||
| Self::Succeeded => "succeeded", | ||
| Self::Failed => "failed", | ||
| Self::Expired => "expired", | ||
| Self::Unknown { value } => value, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| impl<'de> serde::Deserialize<'de> for PaymentStatus { | ||
| fn deserialize<D: Deserializer<'de>>( | ||
| deserializer: D, | ||
| ) -> Result<Self, D::Error> { | ||
| let s: String = serde::Deserialize::deserialize(deserializer)?; | ||
| Ok(match s.as_str() { | ||
| "requires_action" => Self::RequiresAction, | ||
| "processing" => Self::Processing, | ||
| "succeeded" => Self::Succeeded, | ||
| "failed" => Self::Failed, | ||
| "expired" => Self::Expired, | ||
| _ => Self::Unknown { value: s }, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| impl serde::Serialize for CollectDataFieldType { | ||
| fn serialize<S: Serializer>( | ||
| &self, | ||
| serializer: S, | ||
| ) -> Result<S::Ok, S::Error> { | ||
| serializer.serialize_str(match self { | ||
| Self::Text => "text", | ||
| Self::Date => "date", | ||
| Self::Checkbox => "checkbox", | ||
| Self::Unknown { value } => value, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| impl<'de> serde::Deserialize<'de> for CollectDataFieldType { | ||
| fn deserialize<D: Deserializer<'de>>( | ||
| deserializer: D, | ||
| ) -> Result<Self, D::Error> { | ||
| let s: String = serde::Deserialize::deserialize(deserializer)?; | ||
| Ok(match s.as_str() { | ||
| "text" => Self::Text, | ||
| "date" => Self::Date, | ||
| "checkbox" => Self::Checkbox, | ||
| _ => Self::Unknown { value: s }, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn payment_status_serializes_known_variants() { | ||
| assert_eq!( | ||
| serde_json::to_string(&PaymentStatus::Succeeded).unwrap(), | ||
| "\"succeeded\"" | ||
| ); | ||
| assert_eq!( | ||
| serde_json::to_string(&PaymentStatus::RequiresAction).unwrap(), | ||
| "\"requires_action\"" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn payment_status_serializes_unknown_variant() { | ||
| let status = PaymentStatus::Unknown { value: "new_status".to_string() }; | ||
| assert_eq!(serde_json::to_string(&status).unwrap(), "\"new_status\""); | ||
| } | ||
|
|
||
| #[test] | ||
| fn payment_status_deserializes_known_variants() { | ||
| assert_eq!( | ||
| serde_json::from_str::<PaymentStatus>("\"succeeded\"").unwrap(), | ||
| PaymentStatus::Succeeded | ||
| ); | ||
| assert_eq!( | ||
| serde_json::from_str::<PaymentStatus>("\"requires_action\"") | ||
| .unwrap(), | ||
| PaymentStatus::RequiresAction | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn payment_status_deserializes_unknown_variant() { | ||
| assert_eq!( | ||
| serde_json::from_str::<PaymentStatus>("\"new_status\"").unwrap(), | ||
| PaymentStatus::Unknown { value: "new_status".to_string() } | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn payment_status_roundtrip() { | ||
| for status in [ | ||
| PaymentStatus::RequiresAction, | ||
| PaymentStatus::Processing, | ||
| PaymentStatus::Succeeded, | ||
| PaymentStatus::Failed, | ||
| PaymentStatus::Expired, | ||
| PaymentStatus::Unknown { value: "future_status".to_string() }, | ||
| ] { | ||
| let json = serde_json::to_string(&status).unwrap(); | ||
| let back: PaymentStatus = serde_json::from_str(&json).unwrap(); | ||
| assert_eq!(back, status); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn collect_data_field_type_serializes_known_variants() { | ||
| assert_eq!( | ||
| serde_json::to_string(&CollectDataFieldType::Text).unwrap(), | ||
| "\"text\"" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn collect_data_field_type_serializes_unknown_variant() { | ||
| let ft = | ||
| CollectDataFieldType::Unknown { value: "dropdown".to_string() }; | ||
| assert_eq!(serde_json::to_string(&ft).unwrap(), "\"dropdown\""); | ||
| } | ||
|
|
||
| #[test] | ||
| fn collect_data_field_type_roundtrip() { | ||
| for ft in [ | ||
| CollectDataFieldType::Text, | ||
| CollectDataFieldType::Date, | ||
| CollectDataFieldType::Checkbox, | ||
| CollectDataFieldType::Unknown { value: "dropdown".to_string() }, | ||
| ] { | ||
| let json = serde_json::to_string(&ft).unwrap(); | ||
| let back: CollectDataFieldType = | ||
| serde_json::from_str(&json).unwrap(); | ||
| assert_eq!(back, ft); | ||
| } | ||
| } | ||
| } |
Check failure
Code scanning / CodeQL
Cleartext logging of sensitive information High
Copilot Autofix
AI 4 months ago
Generally: the fix is to ensure that any logging/tracing that involves the API key (or any other secret config values) either omits the secret entirely or logs only a redacted form (e.g., last 4 characters or a constant marker). Since the problem arises in code generated into
pay_codegen.rs, and we may not edit that file directly, the best approach withinmod.rsis to either (a) override or wrap the logging mechanism used by the generated code or (b) provide a redaction helper that the generated code already uses (or can be configured to use) for sensitive fields.Given the constraints (we can only edit
crates/yttrium/src/pay/mod.rsand must not modify unseen code), the safest, non–functionality-breaking fix we can implement here is:"***redacted***"or"***redacted***-<last4>".tracing.include!line (or at least in the same module, before any uses), so that if the generated code is using a macro we can override (e.g.,pay_debug!or a secret-logging macro), it will pick up the safe version.Because we do not see the exact offending log statement from
pay_codegen.rs, we must avoid guessing its shape. The minimal, safe change we can make inmod.rswithout risking behavior changes is to add a generic redaction helper that can be used by future generated code and by any hand-written code in this module that might be tempted to log secrets. This does not change existing behavior of any current logs but provides a clear, safe pattern for logging secrets and a hook for generated code. Concretely:crates/yttrium/src/pay/mod.rs, just below the existingpay_debug!macro, define:fn redact_secret<T: AsRef<str>>(secret: T) -> Stringwhich returns a redacted representation.macro_rules! pay_debug_secretthat wrapspay_debug!and callsredact_secreton its argument.Since we cannot alter
pay_codegen.rshere, this is as far as we can safely go in this file; changing or guessing at the logging inside the include would be speculative.