From 0779d0db13f5952d436a99006b779ec015941571 Mon Sep 17 00:00:00 2001 From: CleverWild Date: Tue, 30 Jun 2026 19:55:28 +0200 Subject: [PATCH] feat(macros): enhance Integrable derive with validation and versioning improvements --- server/Cargo.lock | 61 +++++++++- server/Cargo.toml | 1 - server/crates/arbiter-crypto/src/integrity.rs | 45 +++++++- server/crates/arbiter-macros/Cargo.toml | 1 + server/crates/arbiter-macros/src/hashable.rs | 32 ++---- .../crates/arbiter-macros/src/integrable.rs | 106 +++++++++++++++--- server/crates/arbiter-macros/src/utils.rs | 10 ++ .../crates/arbiter-macros/tests/integrable.rs | 44 ++++++-- .../arbiter-macros/tests/integrable_errors.rs | 5 + .../tests/ui/integrable/duplicate_attr.rs | 8 ++ .../tests/ui/integrable/duplicate_attr.stderr | 5 + .../tests/ui/integrable/empty_kind.rs | 7 ++ .../tests/ui/integrable/empty_kind.stderr | 5 + .../tests/ui/integrable/enum_not_supported.rs | 8 ++ .../ui/integrable/enum_not_supported.stderr | 5 + .../tests/ui/integrable/invalid_kind.rs | 7 ++ .../tests/ui/integrable/invalid_kind.stderr | 5 + .../tests/ui/integrable/missing_attr.rs | 6 + .../tests/ui/integrable/missing_attr.stderr | 5 + 19 files changed, 314 insertions(+), 52 deletions(-) create mode 100644 server/crates/arbiter-macros/tests/integrable_errors.rs create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/duplicate_attr.rs create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/duplicate_attr.stderr create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/empty_kind.rs create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/empty_kind.stderr create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/enum_not_supported.rs create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/enum_not_supported.stderr create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/invalid_kind.rs create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/invalid_kind.stderr create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/missing_attr.rs create mode 100644 server/crates/arbiter-macros/tests/ui/integrable/missing_attr.stderr diff --git a/server/Cargo.lock b/server/Cargo.lock index e36c264..64de77b 100644 --- a/server/Cargo.lock +++ b/server/Cargo.lock @@ -719,6 +719,7 @@ dependencies = [ "proc-macro2", "quote", "syn 2.0.117", + "trybuild", ] [[package]] @@ -1965,6 +1966,12 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "dissimilar" +version = "1.0.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aeda16ab4059c5fd2a83f2b9c9e9c981327b18aa8e3b313f7e6563799d4f093e" + [[package]] name = "downcast-rs" version = "2.0.2" @@ -3200,7 +3207,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "36c791ecdf977c99f45f23280405d7723727470f6689a5e6dbf513ac547ae10d" dependencies = [ "serde", - "toml", + "toml 0.9.12+spec-1.1.0", ] [[package]] @@ -4972,6 +4979,12 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "55937e1799185b12863d447f42597ed69d9928686b8d88a1df17376a097d8369" +[[package]] +name = "target-triple" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "591ef38edfb78ca4771ee32cf494cb8771944bee237a9b91fc9c1424ac4b777b" + [[package]] name = "tempfile" version = "3.27.0" @@ -4985,6 +4998,15 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "termcolor" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" +dependencies = [ + "winapi-util", +] + [[package]] name = "terminal_size" version = "0.4.4" @@ -5207,6 +5229,21 @@ dependencies = [ "winnow 0.7.15", ] +[[package]] +name = "toml" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "81f3d15e84cbcd896376e6730314d59fb5a87f31e4b038454184435cd57defee" +dependencies = [ + "indexmap 2.14.0", + "serde_core", + "serde_spanned", + "toml_datetime 1.1.1+spec-1.1.0", + "toml_parser", + "toml_writer", + "winnow 1.0.2", +] + [[package]] name = "toml_datetime" version = "0.7.5+spec-1.1.0" @@ -5246,6 +5283,12 @@ dependencies = [ "winnow 1.0.2", ] +[[package]] +name = "toml_writer" +version = "1.1.1+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "756daf9b1013ebe47a8776667b466417e2d4c5679d441c26230efd9ef78692db" + [[package]] name = "tonic" version = "0.14.5" @@ -5433,6 +5476,22 @@ version = "0.2.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e421abadd41a4225275504ea4d6566923418b7f05506fbc9c0fe86ba7396114b" +[[package]] +name = "trybuild" +version = "1.0.117" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0710d4dfbeae4f9c390baa784c49858a7468fa433f3fe5d0ec5ebef651cf59f9" +dependencies = [ + "dissimilar", + "glob", + "serde", + "serde_derive", + "serde_json", + "target-triple", + "termcolor", + "toml 1.1.2+spec-1.1.0", +] + [[package]] name = "typenum" version = "1.20.0" diff --git a/server/Cargo.toml b/server/Cargo.toml index cd588a8..0f93ba1 100644 --- a/server/Cargo.toml +++ b/server/Cargo.toml @@ -129,7 +129,6 @@ rc_buffer = "warn" rc_mutex = "warn" redundant_test_prefix = "warn" redundant_type_annotations = "warn" -ref_patterns = "warn" renamed_function_params = "warn" rest_pat_in_fully_bound_structs = "warn" return_and_then = "warn" diff --git a/server/crates/arbiter-crypto/src/integrity.rs b/server/crates/arbiter-crypto/src/integrity.rs index 818147a..96efecf 100644 --- a/server/crates/arbiter-crypto/src/integrity.rs +++ b/server/crates/arbiter-crypto/src/integrity.rs @@ -1,6 +1,49 @@ use crate::hashing::Hashable; +/// Marks a struct as a participant in the database integrity system. +/// +/// Implementors are protected by an HMAC-SHA256 MAC stored in the +/// `integrity_envelope` table. The MAC is computed over: +/// +/// ```text +/// HMAC-SHA256(key, len(KIND) || KIND || len(entity_id) || entity_id || VERSION || SHA256(Hashable)) +/// ``` +/// +/// Both `KIND` and `VERSION` act as domain separators — they prevent a valid +/// MAC for one entity type or schema version from being accepted for another. +/// +/// # Deriving +/// +/// Use `#[derive(Integrable)]` with the `#[integrable(kind = "...")]` attribute. +/// `VERSION` is computed automatically as an FNV-1a hash of the struct's field +/// names and types, so it changes whenever the schema changes without any manual +/// bookkeeping. +/// +/// ```rust,ignore +/// #[derive(Hashable, Integrable)] +/// #[integrable(kind = "operator_credentials")] +/// pub struct OperatorCredentials { +/// pub pubkey: PublicKey, +/// } +/// ``` +/// +/// # Upgrading schema +/// +/// When fields are added, removed, or reordered, `VERSION` changes automatically. +/// Existing MAC records in the database will return [`PayloadVersionMismatch`] on +/// verification — this is the signal to re-sign all rows for this `KIND` as part +/// of a migration. +/// +/// [`PayloadVersionMismatch`]: crate::integrity::Integrable pub trait Integrable: Hashable { + /// Stable name of this entity type as stored in `integrity_envelope.entity_kind`. + /// + /// Must be a valid schema name: starts with a letter, contains only `[a-zA-Z0-9_]`, + /// and must be globally unique across all `Integrable` types in the system. const KIND: &'static str; - const VERSION: i32 = 1; + + /// FNV-1a hash of the struct's field names and types at the time the derive + /// macro ran. Changes automatically when the schema changes, invalidating + /// existing MACs and signalling that a migration is required. + const VERSION: i32; } diff --git a/server/crates/arbiter-macros/Cargo.toml b/server/crates/arbiter-macros/Cargo.toml index 15a5070..d1787fb 100644 --- a/server/crates/arbiter-macros/Cargo.toml +++ b/server/crates/arbiter-macros/Cargo.toml @@ -14,6 +14,7 @@ syn = { version = "2.0", features = ["derive", "fold", "full", "visit-mut"] } [dev-dependencies] arbiter-crypto = { path = "../arbiter-crypto" } +trybuild = { version = "1.0", features = ["diff"] } [lints] workspace = true diff --git a/server/crates/arbiter-macros/src/hashable.rs b/server/crates/arbiter-macros/src/hashable.rs index bfaec3e..f8cf8b8 100644 --- a/server/crates/arbiter-macros/src/hashable.rs +++ b/server/crates/arbiter-macros/src/hashable.rs @@ -53,32 +53,16 @@ struct FieldAccess { fn collect_field_accesses(struct_data: &DataStruct) -> Vec { match &struct_data.fields { - Fields::Named(fields) => { - // Keep deterministic alphabetical order for named fields. - // Do not remove this sort, because it keeps hash output stable regardless of source order. - let mut named_fields = fields - .named - .iter() - .map(|field| { - let name = field - .ident - .as_ref() - .expect("Fields::Named(fields) must have names") - .clone(); - (name.to_string(), name) - }) - .collect::>(); - - named_fields.sort_by(|a, b| a.0.cmp(&b.0)); - - named_fields - .into_iter() - .map(|(_, name)| FieldAccess { + Fields::Named(fields) => crate::utils::sorted_named_fields(fields) + .into_iter() + .map(|field| { + let name = field.ident.as_ref().unwrap(); + FieldAccess { access: quote! { #name }, span: name.span(), - }) - .collect() - } + } + }) + .collect(), Fields::Unnamed(fields) => fields .unnamed .iter() diff --git a/server/crates/arbiter-macros/src/integrable.rs b/server/crates/arbiter-macros/src/integrable.rs index 4f979ed..ddc2b92 100644 --- a/server/crates/arbiter-macros/src/integrable.rs +++ b/server/crates/arbiter-macros/src/integrable.rs @@ -1,56 +1,128 @@ use crate::utils::INTEGRABLE_TRAIT_PATH; use proc_macro2::TokenStream; use quote::quote; -use syn::{DeriveInput, LitInt, LitStr}; +use syn::{DeriveInput, LitStr, spanned::Spanned as _}; struct IntegrableAttr { kind: String, - version: i32, } impl IntegrableAttr { - fn from_attrs(attrs: &[syn::Attribute], span: proc_macro2::Span) -> Result { + fn from_attrs( + attrs: &[syn::Attribute], + ident_span: proc_macro2::Span, + ) -> Result { let mut kind: Option = None; - let mut version: i32 = 1; + let mut found = false; for attr in attrs { if !attr.path().is_ident("integrable") { continue; } + if found { + return Err(syn::Error::new(attr.span(), "duplicate #[integrable] attribute")); + } + found = true; attr.parse_nested_meta(|meta| { if meta.path.is_ident("kind") { let lit: LitStr = meta.value()?.parse()?; - kind = Some(lit.value()); - } else if meta.path.is_ident("version") { - let lit: LitInt = meta.value()?.parse()?; - version = lit.base10_parse()?; + let v = lit.value(); + if v.is_empty() { + return Err(syn::Error::new(lit.span(), "kind must not be empty")); + } + if !is_valid_kind(&v) { + return Err(syn::Error::new( + lit.span(), + "kind must be a valid schema name: start with a letter, contain only [a-zA-Z0-9_]", + )); + } + kind = Some(v); } else { - return Err(meta.error("unknown key; expected `kind` or `version`")); + return Err(meta.error("unknown key; expected `kind`")); } Ok(()) })?; } let kind = kind.ok_or_else(|| { - syn::Error::new(span, "#[integrable(kind = \"...\")] is required") + syn::Error::new(ident_span, "#[integrable(kind = \"...\")] is required") })?; - Ok(Self { kind, version }) + Ok(Self { kind }) } } -pub(crate) fn derive(input: &DeriveInput) -> TokenStream { - let integrable_trait = INTEGRABLE_TRAIT_PATH.to_path(); - let ident = &input.ident; - let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl(); +fn is_valid_kind(s: &str) -> bool { + let mut chars = s.chars(); + matches!(chars.next(), Some(c) if c.is_ascii_alphabetic()) + && chars.all(|c| c.is_ascii_alphanumeric() || c == '_') +} - let attr = match IntegrableAttr::from_attrs(&input.attrs, proc_macro2::Span::call_site()) { +fn fnv1a(data: &[u8], mut hash: u32) -> u32 { + const FNV_PRIME: u32 = 16_777_619; + for &b in data { + hash ^= u32::from(b); + hash = hash.wrapping_mul(FNV_PRIME); + } + hash +} + +// Hashes field names and types using the same alphabetical sort order as Hashable, +// so that source-level field reordering never changes VERSION. +fn compute_version(fields: &syn::Fields) -> i32 { + const FNV_OFFSET: u32 = 2_166_136_261; + let mut hash = FNV_OFFSET; + + match fields { + syn::Fields::Named(named) => { + for field in crate::utils::sorted_named_fields(named) { + let name = field.ident.as_ref().unwrap().to_string(); + let ty = &field.ty; + hash = fnv1a(name.as_bytes(), hash); + hash = fnv1a(quote!(#ty).to_string().as_bytes(), hash); + } + } + syn::Fields::Unnamed(unnamed) => { + for (i, field) in unnamed.unnamed.iter().enumerate() { + let ty = &field.ty; + hash = fnv1a(i.to_string().as_bytes(), hash); + hash = fnv1a(quote!(#ty).to_string().as_bytes(), hash); + } + } + syn::Fields::Unit => {} + } + + // Clear sign bit to guarantee a positive i32; substitute 0 → 1. + let v = (hash >> 1).cast_signed(); + if v == 0 { 1 } else { v } +} + +pub(crate) fn derive(input: &DeriveInput) -> TokenStream { + let syn::Data::Struct(ref data) = input.data else { + return syn::Error::new( + input.ident.span(), + "#[derive(Integrable)] is only supported on structs", + ) + .to_compile_error(); + }; + + let integrable_trait = INTEGRABLE_TRAIT_PATH.to_path(); + let hashable_trait = crate::utils::HASHABLE_TRAIT_PATH.to_path(); + let ident = &input.ident; + + let mut generics = input.generics.clone(); + for type_param in generics.type_params_mut() { + type_param.bounds.push(syn::parse_quote!(#hashable_trait)); + } + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + + let attr = match IntegrableAttr::from_attrs(&input.attrs, input.ident.span()) { Ok(a) => a, Err(e) => return e.to_compile_error(), }; let kind = attr.kind; - let version = attr.version; + let version = compute_version(&data.fields); quote! { #[automatically_derived] diff --git a/server/crates/arbiter-macros/src/utils.rs b/server/crates/arbiter-macros/src/utils.rs index cecd5ae..283c101 100644 --- a/server/crates/arbiter-macros/src/utils.rs +++ b/server/crates/arbiter-macros/src/utils.rs @@ -23,3 +23,13 @@ macro_rules! ensure_path { ensure_path!(::arbiter_crypto::hashing::Hashable as HASHABLE_TRAIT_PATH); ensure_path!(::arbiter_crypto::hashing::Digest as HMAC_DIGEST_PATH); ensure_path!(::arbiter_crypto::integrity::Integrable as INTEGRABLE_TRAIT_PATH); + +/// Returns named struct fields sorted alphabetically by name. +/// Both `Hashable` and `Integrable` derive macros must iterate fields in the +/// same deterministic order so that source-level reordering never changes +/// either the runtime hash or the compile-time VERSION. +pub(crate) fn sorted_named_fields(fields: &syn::FieldsNamed) -> Vec<&syn::Field> { + let mut v: Vec<&syn::Field> = fields.named.iter().collect(); + v.sort_by_key(|f| f.ident.as_ref().unwrap().to_string()); + v +} diff --git a/server/crates/arbiter-macros/tests/integrable.rs b/server/crates/arbiter-macros/tests/integrable.rs index 086764b..8205969 100644 --- a/server/crates/arbiter-macros/tests/integrable.rs +++ b/server/crates/arbiter-macros/tests/integrable.rs @@ -6,20 +6,48 @@ struct TestEntity { value: i32, } +#[derive(arbiter_macros::Hashable, arbiter_macros::Integrable)] +#[integrable(kind = "other_entity")] +struct OtherEntity { + label: String, + count: u64, +} + #[test] -fn default_version_is_one() { - assert_eq!(::VERSION, 1, "default version must be 1"); +fn kind_is_set_correctly() { assert_eq!(::KIND, "test_entity"); + assert_eq!(::KIND, "other_entity"); +} + +#[test] +fn version_is_positive() { + const { + assert!(::VERSION > 0); + assert!(::VERSION > 0); + } +} + +#[test] +fn different_field_layouts_produce_different_versions() { + assert_ne!( + ::VERSION, + ::VERSION, + ); } #[derive(arbiter_macros::Hashable, arbiter_macros::Integrable)] -#[integrable(kind = "versioned_entity", version = 3)] -struct VersionedEntity { - data: String, +#[integrable(kind = "generic_entity")] +struct GenericEntity { + inner: T, } #[test] -fn explicit_version_attribute() { - assert_eq!(::VERSION, 3); - assert_eq!(::KIND, "versioned_entity"); +fn generic_struct_derives_integrable() { + assert_eq!( + as Integrable>::KIND, + "generic_entity" + ); + const { + assert!( as Integrable>::VERSION > 0); + } } diff --git a/server/crates/arbiter-macros/tests/integrable_errors.rs b/server/crates/arbiter-macros/tests/integrable_errors.rs new file mode 100644 index 0000000..2a33f39 --- /dev/null +++ b/server/crates/arbiter-macros/tests/integrable_errors.rs @@ -0,0 +1,5 @@ +#[test] +fn integrable_compile_fail() { + let t = trybuild::TestCases::new(); + t.compile_fail("tests/ui/integrable/*.rs"); +} diff --git a/server/crates/arbiter-macros/tests/ui/integrable/duplicate_attr.rs b/server/crates/arbiter-macros/tests/ui/integrable/duplicate_attr.rs new file mode 100644 index 0000000..4a46634 --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/duplicate_attr.rs @@ -0,0 +1,8 @@ +#[derive(arbiter_macros::Hashable, arbiter_macros::Integrable)] +#[integrable(kind = "entity_a")] +#[integrable(kind = "entity_b")] +struct DuplicateAttr { + value: i32, +} + +fn main() {} diff --git a/server/crates/arbiter-macros/tests/ui/integrable/duplicate_attr.stderr b/server/crates/arbiter-macros/tests/ui/integrable/duplicate_attr.stderr new file mode 100644 index 0000000..57b4cdc --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/duplicate_attr.stderr @@ -0,0 +1,5 @@ +error: duplicate #[integrable] attribute + --> tests/ui/integrable/duplicate_attr.rs:3:1 + | +3 | #[integrable(kind = "entity_b")] + | ^ diff --git a/server/crates/arbiter-macros/tests/ui/integrable/empty_kind.rs b/server/crates/arbiter-macros/tests/ui/integrable/empty_kind.rs new file mode 100644 index 0000000..03c74b1 --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/empty_kind.rs @@ -0,0 +1,7 @@ +#[derive(arbiter_macros::Hashable, arbiter_macros::Integrable)] +#[integrable(kind = "")] +struct EmptyKind { + value: i32, +} + +fn main() {} diff --git a/server/crates/arbiter-macros/tests/ui/integrable/empty_kind.stderr b/server/crates/arbiter-macros/tests/ui/integrable/empty_kind.stderr new file mode 100644 index 0000000..9c76314 --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/empty_kind.stderr @@ -0,0 +1,5 @@ +error: kind must not be empty + --> tests/ui/integrable/empty_kind.rs:2:21 + | +2 | #[integrable(kind = "")] + | ^^ diff --git a/server/crates/arbiter-macros/tests/ui/integrable/enum_not_supported.rs b/server/crates/arbiter-macros/tests/ui/integrable/enum_not_supported.rs new file mode 100644 index 0000000..f867a91 --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/enum_not_supported.rs @@ -0,0 +1,8 @@ +#[derive(arbiter_macros::Integrable)] +#[integrable(kind = "my_enum")] +enum MyEnum { + A, + B, +} + +fn main() {} diff --git a/server/crates/arbiter-macros/tests/ui/integrable/enum_not_supported.stderr b/server/crates/arbiter-macros/tests/ui/integrable/enum_not_supported.stderr new file mode 100644 index 0000000..aa71dbe --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/enum_not_supported.stderr @@ -0,0 +1,5 @@ +error: #[derive(Integrable)] is only supported on structs + --> tests/ui/integrable/enum_not_supported.rs:3:6 + | +3 | enum MyEnum { + | ^^^^^^ diff --git a/server/crates/arbiter-macros/tests/ui/integrable/invalid_kind.rs b/server/crates/arbiter-macros/tests/ui/integrable/invalid_kind.rs new file mode 100644 index 0000000..e185270 --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/invalid_kind.rs @@ -0,0 +1,7 @@ +#[derive(arbiter_macros::Hashable, arbiter_macros::Integrable)] +#[integrable(kind = "bad kind!")] +struct InvalidKind { + value: i32, +} + +fn main() {} diff --git a/server/crates/arbiter-macros/tests/ui/integrable/invalid_kind.stderr b/server/crates/arbiter-macros/tests/ui/integrable/invalid_kind.stderr new file mode 100644 index 0000000..1aea443 --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/invalid_kind.stderr @@ -0,0 +1,5 @@ +error: kind must be a valid schema name: start with a letter, contain only [a-zA-Z0-9_] + --> tests/ui/integrable/invalid_kind.rs:2:21 + | +2 | #[integrable(kind = "bad kind!")] + | ^^^^^^^^^^^ diff --git a/server/crates/arbiter-macros/tests/ui/integrable/missing_attr.rs b/server/crates/arbiter-macros/tests/ui/integrable/missing_attr.rs new file mode 100644 index 0000000..1617816 --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/missing_attr.rs @@ -0,0 +1,6 @@ +#[derive(arbiter_macros::Hashable, arbiter_macros::Integrable)] +struct MissingAttr { + value: i32, +} + +fn main() {} diff --git a/server/crates/arbiter-macros/tests/ui/integrable/missing_attr.stderr b/server/crates/arbiter-macros/tests/ui/integrable/missing_attr.stderr new file mode 100644 index 0000000..a20b6f4 --- /dev/null +++ b/server/crates/arbiter-macros/tests/ui/integrable/missing_attr.stderr @@ -0,0 +1,5 @@ +error: #[integrable(kind = "...")] is required + --> tests/ui/integrable/missing_attr.rs:2:8 + | +2 | struct MissingAttr { + | ^^^^^^^^^^^