From d531f13583c7bda0524ea700a82c1a618c148c89 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Mon, 27 Oct 2025 14:37:31 +0000 Subject: [PATCH 1/9] Make `add_params_to_url` template function deterministic --- crates/templates/src/functions.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/templates/src/functions.rs b/crates/templates/src/functions.rs index 631e4742e..c8943ebc0 100644 --- a/crates/templates/src/functions.rs +++ b/crates/templates/src/functions.rs @@ -10,7 +10,7 @@ //! Additional functions, tests and filters used in templates use std::{ - collections::HashMap, + collections::{BTreeMap, HashMap}, fmt::Formatter, str::FromStr, sync::{Arc, atomic::AtomicUsize}, @@ -182,7 +182,8 @@ fn function_add_params_to_url( .unwrap_or_default(); // Merge the exising and the additional parameters together - let params: HashMap<&String, &Value> = params.iter().chain(existing.iter()).collect(); + // Use a BTreeMap for determinism (because it orders keys) + let params: BTreeMap<&String, &Value> = params.iter().chain(existing.iter()).collect(); // Transform them back to urlencoded let params = serde_urlencoded::to_string(params).map_err(|e| { From d3cabf4a4b32122c701672ecb1cd6176757ab733 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Mon, 27 Oct 2025 14:40:28 +0000 Subject: [PATCH 2/9] cli: templates check: add option to --stabilise date and RNG --- crates/cli/src/commands/templates.rs | 25 +++- crates/templates/src/context.rs | 148 ++++++++++++------------ crates/templates/src/context/captcha.rs | 5 +- crates/templates/src/lib.rs | 8 +- crates/templates/src/macros.rs | 19 +-- 5 files changed, 113 insertions(+), 92 deletions(-) diff --git a/crates/cli/src/commands/templates.rs b/crates/cli/src/commands/templates.rs index 8f1b0dd4e..75acce21d 100644 --- a/crates/cli/src/commands/templates.rs +++ b/crates/cli/src/commands/templates.rs @@ -8,6 +8,7 @@ use std::{fmt::Write, process::ExitCode}; use anyhow::{Context as _, bail}; use camino::Utf8PathBuf; +use chrono::DateTime; use clap::Parser; use figment::Figment; use mas_config::{ @@ -34,6 +35,12 @@ enum Subcommand { /// The directory must either not exist or be empty. #[arg(long = "out-dir")] out_dir: Option, + + /// Attempt to remove 'unstable' template input data such as asset + /// hashes, in order to make renders more reproducible between + /// versions. + #[arg(long = "stabilise")] + stabilise: bool, }, } @@ -41,7 +48,7 @@ impl Options { pub async fn run(self, figment: &Figment) -> anyhow::Result { use Subcommand as SC; match self.subcommand { - SC::Check { out_dir } => { + SC::Check { out_dir, stabilise } => { let _span = info_span!("cli.templates.check").entered(); let template_config = TemplatesConfig::extract_or_default(figment) @@ -59,9 +66,17 @@ impl Options { let captcha_config = CaptchaConfig::extract_or_default(figment) .map_err(anyhow::Error::from_boxed)?; - let clock = SystemClock::default(); - // XXX: we should disallow SeedableRng::from_entropy - let mut rng = rand_chacha::ChaChaRng::from_entropy(); + let now = if stabilise { + DateTime::from_timestamp_secs(0).unwrap() + } else { + SystemClock::default().now() + }; + let rng = if stabilise { + rand_chacha::ChaChaRng::from_seed([42; 32]) + } else { + // XXX: we should disallow SeedableRng::from_entropy + rand_chacha::ChaChaRng::from_entropy() + }; let url_builder = mas_router::UrlBuilder::new("https://example.com/".parse()?, None, None); let site_config = site_config_from_config( @@ -79,7 +94,7 @@ impl Options { true, ) .await?; - let all_renders = templates.check_render(clock.now(), &mut rng)?; + let all_renders = templates.check_render(now, &rng)?; if let Some(out_dir) = out_dir { // Save renders to disk. diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index 597683a03..c32691c4c 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -106,9 +106,9 @@ pub trait TemplateContext: Serialize { /// /// This is then used to check for template validity in unit tests and in /// the CLI (`cargo run -- templates check`) - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -144,9 +144,9 @@ pub(crate) fn sample_list(samples: Vec) -> BTreeMap( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -181,9 +181,9 @@ impl std::ops::Deref for WithLanguage { } impl TemplateContext for WithLanguage { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -192,7 +192,9 @@ impl TemplateContext for WithLanguage { locales .iter() .flat_map(|locale| { - T::sample(now, rng, locales) + // Make samples deterministic between locales + let mut rng = rng.clone(); + T::sample(now, &mut rng, locales) .into_iter() .map(|(sample_id, sample)| { ( @@ -218,9 +220,9 @@ pub struct WithCsrf { } impl TemplateContext for WithCsrf { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -251,9 +253,9 @@ pub struct WithSession { } impl TemplateContext for WithSession { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -289,9 +291,9 @@ pub struct WithOptionalSession { } impl TemplateContext for WithOptionalSession { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -340,9 +342,9 @@ impl Serialize for EmptyContext { } impl TemplateContext for EmptyContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -368,9 +370,9 @@ impl IndexContext { } impl TemplateContext for IndexContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -414,9 +416,9 @@ impl AppContext { } impl TemplateContext for AppContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -447,9 +449,9 @@ impl ApiDocContext { } impl TemplateContext for ApiDocContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -539,9 +541,9 @@ pub struct LoginContext { } impl TemplateContext for LoginContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -647,9 +649,9 @@ pub struct RegisterContext { } impl TemplateContext for RegisterContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -690,9 +692,9 @@ pub struct PasswordRegisterContext { } impl TemplateContext for PasswordRegisterContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -732,9 +734,9 @@ pub struct ConsentContext { } impl TemplateContext for ConsentContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -790,9 +792,9 @@ pub struct PolicyViolationContext { } impl TemplateContext for PolicyViolationContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -865,9 +867,9 @@ pub struct CompatSsoContext { } impl TemplateContext for CompatSsoContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -927,9 +929,9 @@ impl EmailRecoveryContext { } impl TemplateContext for EmailRecoveryContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -992,9 +994,9 @@ impl EmailVerificationContext { } impl TemplateContext for EmailVerificationContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1067,9 +1069,9 @@ impl RegisterStepsVerifyEmailContext { } impl TemplateContext for RegisterStepsVerifyEmailContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1107,9 +1109,9 @@ impl RegisterStepsEmailInUseContext { } impl TemplateContext for RegisterStepsEmailInUseContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1162,9 +1164,9 @@ impl RegisterStepsDisplayNameContext { } impl TemplateContext for RegisterStepsDisplayNameContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1217,9 +1219,9 @@ impl RegisterStepsRegistrationTokenContext { } impl TemplateContext for RegisterStepsRegistrationTokenContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1268,9 +1270,9 @@ impl RecoveryStartContext { } impl TemplateContext for RecoveryStartContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1310,9 +1312,9 @@ impl RecoveryProgressContext { } impl TemplateContext for RecoveryProgressContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1356,9 +1358,9 @@ impl RecoveryExpiredContext { } impl TemplateContext for RecoveryExpiredContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1420,9 +1422,9 @@ impl RecoveryFinishContext { } impl TemplateContext for RecoveryFinishContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1469,9 +1471,9 @@ impl UpstreamExistingLinkContext { } impl TemplateContext for UpstreamExistingLinkContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1507,9 +1509,9 @@ impl UpstreamSuggestLink { } impl TemplateContext for UpstreamSuggestLink { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1636,9 +1638,9 @@ impl UpstreamRegister { } impl TemplateContext for UpstreamRegister { - fn sample( + fn sample( now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1722,9 +1724,9 @@ impl DeviceLinkContext { } impl TemplateContext for DeviceLinkContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1756,9 +1758,9 @@ impl DeviceConsentContext { } impl TemplateContext for DeviceConsentContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1801,9 +1803,9 @@ impl AccountInactiveContext { } impl TemplateContext for AccountInactiveContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1837,9 +1839,9 @@ impl DeviceNameContext { } impl TemplateContext for DeviceNameContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -1863,9 +1865,9 @@ pub struct FormPostContext { } impl TemplateContext for FormPostContext { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where @@ -1945,9 +1947,9 @@ impl std::fmt::Display for ErrorContext { } impl TemplateContext for ErrorContext { - fn sample( + fn sample( _now: chrono::DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where @@ -2039,9 +2041,9 @@ impl NotFoundContext { } impl TemplateContext for NotFoundContext { - fn sample( + fn sample( _now: DateTime, - _rng: &mut impl Rng, + _rng: &mut R, _locales: &[DataLocale], ) -> BTreeMap where diff --git a/crates/templates/src/context/captcha.rs b/crates/templates/src/context/captcha.rs index 3daafb745..32c2d1068 100644 --- a/crates/templates/src/context/captcha.rs +++ b/crates/templates/src/context/captcha.rs @@ -11,6 +11,7 @@ use minijinja::{ Value, value::{Enumerator, Object}, }; +use rand::Rng; use serde::Serialize; use crate::{TemplateContext, context::SampleIdentifier}; @@ -58,9 +59,9 @@ impl WithCaptcha { } impl TemplateContext for WithCaptcha { - fn sample( + fn sample( now: chrono::DateTime, - rng: &mut impl rand::prelude::Rng, + rng: &mut R, locales: &[DataLocale], ) -> BTreeMap where diff --git a/crates/templates/src/lib.rs b/crates/templates/src/lib.rs index 603dcfdf6..a57f718f0 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -471,10 +471,10 @@ impl Templates { /// # Errors /// /// Returns an error if any of the templates fails to render - pub fn check_render( + pub fn check_render( &self, now: chrono::DateTime, - rng: &mut impl Rng, + rng: &R, ) -> anyhow::Result> { check::all(self, now, rng) } @@ -489,7 +489,7 @@ mod tests { #[allow(clippy::disallowed_methods)] let now = chrono::Utc::now(); #[allow(clippy::disallowed_methods)] - let mut rng = rand::thread_rng(); + let rng = rand::thread_rng(); let path = Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("../../templates/"); let url_builder = UrlBuilder::new("https://example.com/".parse().unwrap(), None, None); @@ -517,6 +517,6 @@ mod tests { ) .await .unwrap(); - templates.check_render(now, &mut rng).unwrap(); + templates.check_render(now, &rng).unwrap(); } } diff --git a/crates/templates/src/macros.rs b/crates/templates/src/macros.rs index 95b57f0d9..c6d5e9302 100644 --- a/crates/templates/src/macros.rs +++ b/crates/templates/src/macros.rs @@ -78,15 +78,18 @@ macro_rules! register_templates { /// # Errors /// /// Returns an error if any template fails to render with any of the sample. - pub(crate) fn all(templates: &Templates, now: chrono::DateTime, rng: &mut impl rand::Rng) -> anyhow::Result<::std::collections::BTreeMap<(&'static str, SampleIdentifier), String>> { + pub(crate) fn all(templates: &Templates, now: chrono::DateTime, rng: &R) -> anyhow::Result<::std::collections::BTreeMap<(&'static str, SampleIdentifier), String>> { let mut out = ::std::collections::BTreeMap::new(); // TODO shouldn't the Rng be independent for each render? $( - out.extend( - $name $(::< $( $generic_default ),* >)? (templates, now, rng)? - .into_iter() - .map(|(sample_identifier, rendered)| (($template, sample_identifier), rendered)) - ); + { + let mut rng = rng.clone(); + out.extend( + $name $(::< _ $( , $generic_default ),* >)? (templates, now, &mut rng)? + .into_iter() + .map(|(sample_identifier, rendered)| (($template, sample_identifier), rendered)) + ); + } )* Ok(out) @@ -101,8 +104,8 @@ macro_rules! register_templates { /// /// Returns an error if the template fails to render with any of the sample. pub(crate) fn $name - $(< $( $lt $( : $clt $(+ $dlt )* + TemplateContext )? ),+ >)? - (templates: &Templates, now: chrono::DateTime, rng: &mut impl rand::Rng) + < __R: Rng + Clone $( , $( $lt $( : $clt $(+ $dlt )* + TemplateContext )? ),+ )? > + (templates: &Templates, now: chrono::DateTime, rng: &mut __R) -> anyhow::Result> { let locales = templates.translator().available_locales(); let samples: BTreeMap = TemplateContext::sample(now, rng, &locales); From eba9c5e1e1a8bf0723c1afc43ad9ee6b41a4f602 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Mon, 27 Oct 2025 14:46:04 +0000 Subject: [PATCH 3/9] document new options on `templates check` --- docs/reference/cli/templates.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/reference/cli/templates.md b/docs/reference/cli/templates.md index ccced6bed..c6423b32b 100644 --- a/docs/reference/cli/templates.md +++ b/docs/reference/cli/templates.md @@ -17,3 +17,7 @@ INFO mas_core::templates::check: Rendering template name="register.html" context INFO mas_core::templates::check: Rendering template name="index.html" context={"csrf_token":"fake_csrf_token","current_session":{"active":true,"created_at":"2021-09-24T13:26:52.962135085Z","id":1,"last_authd_at":"2021-09-24T13:26:52.962135316Z","user_id":2,"username":"john"},"discovery_url":"https://example.com/.well-known/openid-configuration"} ... ``` + +Options: +- `--out-dir `: Render templates and emit them to the specified directory, which must either not exist or be empty. +- `--stabilise`: Remove sources of nondeterminism from template inputs, so that renders are reproducible. Only useful with `--out-dir`. From 4a06f72b0a8efae4dab4f5398302ca7e0ae2009b Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Mon, 27 Oct 2025 14:51:03 +0000 Subject: [PATCH 4/9] template test: check for determinism --- Cargo.lock | 1 + crates/templates/Cargo.toml | 3 +++ crates/templates/src/lib.rs | 12 +++++++++--- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f9bcd749..8d3c87b80 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3727,6 +3727,7 @@ dependencies = [ "minijinja-contrib", "oauth2-types", "rand 0.8.5", + "rand_chacha 0.3.1", "serde", "serde_json", "serde_urlencoded", diff --git a/crates/templates/Cargo.toml b/crates/templates/Cargo.toml index ffe2dbf74..11bc3380b 100644 --- a/crates/templates/Cargo.toml +++ b/crates/templates/Cargo.toml @@ -42,3 +42,6 @@ mas-i18n.workspace = true mas-iana.workspace = true mas-router.workspace = true mas-spa.workspace = true + +[dev-dependencies] +rand_chacha.workspace = true diff --git a/crates/templates/src/lib.rs b/crates/templates/src/lib.rs index a57f718f0..3c3ea7391 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -482,14 +482,15 @@ impl Templates { #[cfg(test)] mod tests { + use rand::SeedableRng; + use super::*; #[tokio::test] async fn check_builtin_templates() { #[allow(clippy::disallowed_methods)] let now = chrono::Utc::now(); - #[allow(clippy::disallowed_methods)] - let rng = rand::thread_rng(); + let rng = rand_chacha::ChaCha8Rng::from_seed([42; 32]); let path = Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("../../templates/"); let url_builder = UrlBuilder::new("https://example.com/".parse().unwrap(), None, None); @@ -517,6 +518,11 @@ mod tests { ) .await .unwrap(); - templates.check_render(now, &rng).unwrap(); + + // Check the renders are deterministic, when given the same rng + let render1 = templates.check_render(now, &rng).unwrap(); + let render2 = templates.check_render(now, &rng).unwrap(); + + assert_eq!(render1, render2); } } From 09dd5e6d839da98f7fc1e27aa2c64a6eed0dd0e6 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Wed, 29 Oct 2025 17:01:50 +0000 Subject: [PATCH 5/9] Stub out the vite manifest when stabilising template renders --- crates/cli/src/commands/server.rs | 2 + crates/cli/src/commands/templates.rs | 4 +- crates/cli/src/commands/worker.rs | 2 + crates/cli/src/util.rs | 3 +- crates/handlers/src/test_utils.rs | 2 +- crates/spa/src/vite.rs | 42 +++++++++++++++++ crates/templates/src/functions.rs | 4 +- crates/templates/src/lib.rs | 67 +++++++++++++++++----------- 8 files changed, 95 insertions(+), 31 deletions(-) diff --git a/crates/cli/src/commands/server.rs b/crates/cli/src/commands/server.rs index 52465f077..cb723431b 100644 --- a/crates/cli/src/commands/server.rs +++ b/crates/cli/src/commands/server.rs @@ -166,6 +166,8 @@ impl Options { &url_builder, // Don't use strict mode in production yet false, + // Don't stabilise in production + false, ) .await?; shutdown.register_reloadable(&templates); diff --git a/crates/cli/src/commands/templates.rs b/crates/cli/src/commands/templates.rs index 75acce21d..39eb5e412 100644 --- a/crates/cli/src/commands/templates.rs +++ b/crates/cli/src/commands/templates.rs @@ -90,8 +90,10 @@ impl Options { let templates = templates_from_config( &template_config, &site_config, - &url_builder, // Use strict mode in template checks + &url_builder, + // Use strict mode in template checks true, + stabilise, ) .await?; let all_renders = templates.check_render(now, &rng)?; diff --git a/crates/cli/src/commands/worker.rs b/crates/cli/src/commands/worker.rs index a1eb0fcce..6dfcd27f1 100644 --- a/crates/cli/src/commands/worker.rs +++ b/crates/cli/src/commands/worker.rs @@ -58,6 +58,8 @@ impl Options { &url_builder, // Don't use strict mode on task workers for now false, + // Don't stabilise in production + false, ) .await?; diff --git a/crates/cli/src/util.rs b/crates/cli/src/util.rs index 4925d9866..cd69a8acf 100644 --- a/crates/cli/src/util.rs +++ b/crates/cli/src/util.rs @@ -233,11 +233,12 @@ pub async fn templates_from_config( site_config: &SiteConfig, url_builder: &UrlBuilder, strict: bool, + stabilise: bool, ) -> Result { Templates::load( config.path.clone(), url_builder.clone(), - config.assets_manifest.clone(), + (!stabilise).then(|| config.assets_manifest.clone()), config.translations_path.clone(), site_config.templates_branding(), site_config.templates_features(), diff --git a/crates/handlers/src/test_utils.rs b/crates/handlers/src/test_utils.rs index f1859f352..1f54b7022 100644 --- a/crates/handlers/src/test_utils.rs +++ b/crates/handlers/src/test_utils.rs @@ -172,7 +172,7 @@ impl TestState { let templates = Templates::load( workspace_root.join("templates"), url_builder.clone(), - workspace_root.join("frontend/dist/manifest.json"), + Some(workspace_root.join("frontend/dist/manifest.json")), workspace_root.join("translations"), site_config.templates_branding(), site_config.templates_features(), diff --git a/crates/spa/src/vite.rs b/crates/spa/src/vite.rs index b488bea6b..e40233595 100644 --- a/crates/spa/src/vite.rs +++ b/crates/spa/src/vite.rs @@ -47,6 +47,48 @@ pub struct Manifest { inner: HashMap, } +impl Manifest { + /// Produce a sample manifest for use in reproducible sample renders. + #[must_use] + #[allow(clippy::missing_panics_doc)] + pub fn sample() -> Self { + let mut inner = HashMap::new(); + + for name in &[ + "src/shared.css", + "src/templates.css", + "src/main.tsx", + "src/swagger.ts", + ] { + inner.insert( + name.parse().unwrap(), + ManifestEntry { + name: None, + names: None, + src: None, + // Construct a fake but slightly plausible dummy asset name. + file: name + .replace('/', "__") + .replace('.', "-XXXXX.") + .replace(".tsx", ".js") + .replace(".ts", ".js") + .parse() + .unwrap(), + css: None, + assets: None, + is_entry: None, + is_dynamic_entry: None, + imports: None, + dynamic_imports: None, + integrity: None, + }, + ); + } + + Manifest { inner } + } +} + #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] enum FileType { Script, diff --git a/crates/templates/src/functions.rs b/crates/templates/src/functions.rs index c8943ebc0..82b79133a 100644 --- a/crates/templates/src/functions.rs +++ b/crates/templates/src/functions.rs @@ -434,10 +434,10 @@ impl Object for IncludeAsset { let path: &Utf8Path = path.into(); - let (main, imported) = self.vite_manifest.find_assets(path).map_err(|_e| { + let (main, imported) = self.vite_manifest.find_assets(path).map_err(|e| { Error::new( ErrorKind::InvalidOperation, - "Invalid assets manifest while calling function `include_asset`", + format!("Invalid assets manifest while calling function `include_asset` with path = {path:?}: {e}"), ) })?; diff --git a/crates/templates/src/lib.rs b/crates/templates/src/lib.rs index 3c3ea7391..ec23bfdb4 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -72,7 +72,7 @@ pub struct Templates { url_builder: UrlBuilder, branding: SiteBranding, features: SiteFeatures, - vite_manifest_path: Utf8PathBuf, + vite_manifest_path: Option, translations_path: Utf8PathBuf, path: Utf8PathBuf, /// Whether template rendering is in strict mode (for testing, @@ -143,6 +143,11 @@ fn is_hidden(entry: &DirEntry) -> bool { impl Templates { /// Load the templates from the given config /// + /// # Parameters + /// + /// - `vite_manifest_path`: None if we are rendering resources for + /// reproducibility, in which case a dummy Vite manifest will be used. + /// /// # Errors /// /// Returns an error if the templates could not be loaded from disk. @@ -154,7 +159,7 @@ impl Templates { pub async fn load( path: Utf8PathBuf, url_builder: UrlBuilder, - vite_manifest_path: Utf8PathBuf, + vite_manifest_path: Option, translations_path: Utf8PathBuf, branding: SiteBranding, features: SiteFeatures, @@ -163,7 +168,7 @@ impl Templates { let (translator, environment) = Self::load_( &path, url_builder.clone(), - &vite_manifest_path, + vite_manifest_path.as_deref(), &translations_path, branding.clone(), features, @@ -186,7 +191,7 @@ impl Templates { async fn load_( path: &Utf8Path, url_builder: UrlBuilder, - vite_manifest_path: &Utf8Path, + vite_manifest_path: Option<&Utf8Path>, translations_path: &Utf8Path, branding: SiteBranding, features: SiteFeatures, @@ -196,13 +201,18 @@ impl Templates { let span = tracing::Span::current(); // Read the assets manifest from disk - let vite_manifest = tokio::fs::read(vite_manifest_path) - .await - .map_err(TemplateLoadingError::ViteManifestIO)?; + let vite_manifest = if let Some(vite_manifest_path) = vite_manifest_path { + let raw_vite_manifest = tokio::fs::read(vite_manifest_path) + .await + .map_err(TemplateLoadingError::ViteManifestIO)?; + + serde_json::from_slice::(&raw_vite_manifest) + .map_err(TemplateLoadingError::ViteManifest)? + } else { + ViteManifest::sample() + }; // Parse it - let vite_manifest: ViteManifest = - serde_json::from_slice(&vite_manifest).map_err(TemplateLoadingError::ViteManifest)?; let translations_path = translations_path.to_owned(); let translator = @@ -291,7 +301,7 @@ impl Templates { let (translator, environment) = Self::load_( &self.path, self.url_builder.clone(), - &self.vite_manifest_path, + self.vite_manifest_path.as_deref(), &self.translations_path, self.branding.clone(), self.features, @@ -506,23 +516,28 @@ mod tests { Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("../../frontend/dist/manifest.json"); let translations_path = Utf8Path::new(env!("CARGO_MANIFEST_DIR")).join("../../translations"); - let templates = Templates::load( - path, - url_builder, - vite_manifest_path, - translations_path, - branding, - features, - // Use strict mode in tests - true, - ) - .await - .unwrap(); - // Check the renders are deterministic, when given the same rng - let render1 = templates.check_render(now, &rng).unwrap(); - let render2 = templates.check_render(now, &rng).unwrap(); + for use_real_vite_manifest in [true, false] { + let templates = Templates::load( + path.clone(), + url_builder.clone(), + // Check both renders against the real vite manifest and the 'dummy' vite manifest + // used for reproducible renders. + use_real_vite_manifest.then_some(vite_manifest_path.clone()), + translations_path.clone(), + branding.clone(), + features, + // Use strict mode in tests + true, + ) + .await + .unwrap(); + + // Check the renders are deterministic, when given the same rng + let render1 = templates.check_render(now, &rng).unwrap(); + let render2 = templates.check_render(now, &rng).unwrap(); - assert_eq!(render1, render2); + assert_eq!(render1, render2); + } } } From 4793f534e6cee5db2131ab2960ab77fc2c34182e Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Thu, 30 Oct 2025 12:09:23 +0000 Subject: [PATCH 6/9] Add rest of documentation on `templates check` --- docs/reference/cli/templates.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/reference/cli/templates.md b/docs/reference/cli/templates.md index c6423b32b..9877f3af9 100644 --- a/docs/reference/cli/templates.md +++ b/docs/reference/cli/templates.md @@ -21,3 +21,13 @@ INFO mas_core::templates::check: Rendering template name="index.html" context={" Options: - `--out-dir `: Render templates and emit them to the specified directory, which must either not exist or be empty. - `--stabilise`: Remove sources of nondeterminism from template inputs, so that renders are reproducible. Only useful with `--out-dir`. + +What is checked: +- the Jinja templates are syntactically valid +- the templates can render with a few sample values, with the branding from the MAS configuration + - undefined variables (`{{ undefined_variable }}`) will raise errors +- all translation keys exist + +What is not checked: +- the validity of the generated HTML (you can forget closing tags, or otherwise create invalid HTML output) +- all translation keys exist *in your intended language(s)* (so some translation keys may fall back to English) From 37e596937451796c749a8b2b6f2686d19cdd4456 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Thu, 6 Nov 2025 15:33:31 +0000 Subject: [PATCH 7/9] Use less zero-y timestamp --- crates/cli/src/commands/templates.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cli/src/commands/templates.rs b/crates/cli/src/commands/templates.rs index 39eb5e412..1199e1f9a 100644 --- a/crates/cli/src/commands/templates.rs +++ b/crates/cli/src/commands/templates.rs @@ -67,7 +67,7 @@ impl Options { .map_err(anyhow::Error::from_boxed)?; let now = if stabilise { - DateTime::from_timestamp_secs(0).unwrap() + DateTime::from_timestamp_secs(1_446_823_992).unwrap() } else { SystemClock::default().now() }; From 743cca5805f8b263d0730a1fac2f36112a3755e7 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Thu, 6 Nov 2025 15:36:50 +0000 Subject: [PATCH 8/9] Don't require proliferation of Rng: Clone in sample method --- crates/templates/Cargo.toml | 1 + crates/templates/src/context.rs | 81 +++++++++++++------------ crates/templates/src/context/captcha.rs | 2 +- 3 files changed, 43 insertions(+), 41 deletions(-) diff --git a/crates/templates/Cargo.toml b/crates/templates/Cargo.toml index 11bc3380b..46ff80c74 100644 --- a/crates/templates/Cargo.toml +++ b/crates/templates/Cargo.toml @@ -25,6 +25,7 @@ http.workspace = true minijinja-contrib.workspace = true minijinja.workspace = true rand.workspace = true +rand_chacha.workspace = true serde_json.workspace = true serde_urlencoded.workspace = true serde.workspace = true diff --git a/crates/templates/src/context.rs b/crates/templates/src/context.rs index c32691c4c..4ed09c3e1 100644 --- a/crates/templates/src/context.rs +++ b/crates/templates/src/context.rs @@ -31,9 +31,10 @@ use mas_iana::jose::JsonWebSignatureAlg; use mas_router::{Account, GraphQL, PostAuthAction, UrlBuilder}; use oauth2_types::scope::{OPENID, Scope}; use rand::{ - Rng, + Rng, SeedableRng, distributions::{Alphanumeric, DistString}, }; +use rand_chacha::ChaCha8Rng; use serde::{Deserialize, Serialize, ser::SerializeStruct}; use ulid::Ulid; use url::Url; @@ -106,7 +107,7 @@ pub trait TemplateContext: Serialize { /// /// This is then used to check for template validity in unit tests and in /// the CLI (`cargo run -- templates check`) - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, locales: &[DataLocale], @@ -144,7 +145,7 @@ pub(crate) fn sample_list(samples: Vec) -> BTreeMap( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -181,7 +182,7 @@ impl std::ops::Deref for WithLanguage { } impl TemplateContext for WithLanguage { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, locales: &[DataLocale], @@ -189,12 +190,12 @@ impl TemplateContext for WithLanguage { where Self: Sized, { + // Create a forked RNG so we make samples deterministic between locales + let rng = ChaCha8Rng::from_rng(rng).unwrap(); locales .iter() .flat_map(|locale| { - // Make samples deterministic between locales - let mut rng = rng.clone(); - T::sample(now, &mut rng, locales) + T::sample(now, &mut rng.clone(), locales) .into_iter() .map(|(sample_id, sample)| { ( @@ -220,7 +221,7 @@ pub struct WithCsrf { } impl TemplateContext for WithCsrf { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, locales: &[DataLocale], @@ -253,7 +254,7 @@ pub struct WithSession { } impl TemplateContext for WithSession { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, locales: &[DataLocale], @@ -291,7 +292,7 @@ pub struct WithOptionalSession { } impl TemplateContext for WithOptionalSession { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, locales: &[DataLocale], @@ -342,7 +343,7 @@ impl Serialize for EmptyContext { } impl TemplateContext for EmptyContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -370,7 +371,7 @@ impl IndexContext { } impl TemplateContext for IndexContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -416,7 +417,7 @@ impl AppContext { } impl TemplateContext for AppContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -449,7 +450,7 @@ impl ApiDocContext { } impl TemplateContext for ApiDocContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -541,7 +542,7 @@ pub struct LoginContext { } impl TemplateContext for LoginContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -649,7 +650,7 @@ pub struct RegisterContext { } impl TemplateContext for RegisterContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -692,7 +693,7 @@ pub struct PasswordRegisterContext { } impl TemplateContext for PasswordRegisterContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -734,7 +735,7 @@ pub struct ConsentContext { } impl TemplateContext for ConsentContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -792,7 +793,7 @@ pub struct PolicyViolationContext { } impl TemplateContext for PolicyViolationContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -867,7 +868,7 @@ pub struct CompatSsoContext { } impl TemplateContext for CompatSsoContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -929,7 +930,7 @@ impl EmailRecoveryContext { } impl TemplateContext for EmailRecoveryContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -994,7 +995,7 @@ impl EmailVerificationContext { } impl TemplateContext for EmailVerificationContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1069,7 +1070,7 @@ impl RegisterStepsVerifyEmailContext { } impl TemplateContext for RegisterStepsVerifyEmailContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1109,7 +1110,7 @@ impl RegisterStepsEmailInUseContext { } impl TemplateContext for RegisterStepsEmailInUseContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -1164,7 +1165,7 @@ impl RegisterStepsDisplayNameContext { } impl TemplateContext for RegisterStepsDisplayNameContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -1219,7 +1220,7 @@ impl RegisterStepsRegistrationTokenContext { } impl TemplateContext for RegisterStepsRegistrationTokenContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -1270,7 +1271,7 @@ impl RecoveryStartContext { } impl TemplateContext for RecoveryStartContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -1312,7 +1313,7 @@ impl RecoveryProgressContext { } impl TemplateContext for RecoveryProgressContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1358,7 +1359,7 @@ impl RecoveryExpiredContext { } impl TemplateContext for RecoveryExpiredContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1422,7 +1423,7 @@ impl RecoveryFinishContext { } impl TemplateContext for RecoveryFinishContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1471,7 +1472,7 @@ impl UpstreamExistingLinkContext { } impl TemplateContext for UpstreamExistingLinkContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1509,7 +1510,7 @@ impl UpstreamSuggestLink { } impl TemplateContext for UpstreamSuggestLink { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1638,7 +1639,7 @@ impl UpstreamRegister { } impl TemplateContext for UpstreamRegister { - fn sample( + fn sample( now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -1724,7 +1725,7 @@ impl DeviceLinkContext { } impl TemplateContext for DeviceLinkContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -1758,7 +1759,7 @@ impl DeviceConsentContext { } impl TemplateContext for DeviceConsentContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1803,7 +1804,7 @@ impl AccountInactiveContext { } impl TemplateContext for AccountInactiveContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1839,7 +1840,7 @@ impl DeviceNameContext { } impl TemplateContext for DeviceNameContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, _locales: &[DataLocale], @@ -1865,7 +1866,7 @@ pub struct FormPostContext { } impl TemplateContext for FormPostContext { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, locales: &[DataLocale], @@ -1947,7 +1948,7 @@ impl std::fmt::Display for ErrorContext { } impl TemplateContext for ErrorContext { - fn sample( + fn sample( _now: chrono::DateTime, _rng: &mut R, _locales: &[DataLocale], @@ -2041,7 +2042,7 @@ impl NotFoundContext { } impl TemplateContext for NotFoundContext { - fn sample( + fn sample( _now: DateTime, _rng: &mut R, _locales: &[DataLocale], diff --git a/crates/templates/src/context/captcha.rs b/crates/templates/src/context/captcha.rs index 32c2d1068..f9d8723b4 100644 --- a/crates/templates/src/context/captcha.rs +++ b/crates/templates/src/context/captcha.rs @@ -59,7 +59,7 @@ impl WithCaptcha { } impl TemplateContext for WithCaptcha { - fn sample( + fn sample( now: chrono::DateTime, rng: &mut R, locales: &[DataLocale], From f6783340280c9f895607bcd4bb7a4e6ecb55e647 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Thu, 6 Nov 2025 21:32:24 +0000 Subject: [PATCH 9/9] Replace dummy manifest with fake include_asset function --- crates/spa/src/vite.rs | 42 ----------------------------- crates/templates/src/functions.rs | 44 +++++++++++++++++++++++++------ crates/templates/src/lib.rs | 8 +++--- 3 files changed, 41 insertions(+), 53 deletions(-) diff --git a/crates/spa/src/vite.rs b/crates/spa/src/vite.rs index e40233595..b488bea6b 100644 --- a/crates/spa/src/vite.rs +++ b/crates/spa/src/vite.rs @@ -47,48 +47,6 @@ pub struct Manifest { inner: HashMap, } -impl Manifest { - /// Produce a sample manifest for use in reproducible sample renders. - #[must_use] - #[allow(clippy::missing_panics_doc)] - pub fn sample() -> Self { - let mut inner = HashMap::new(); - - for name in &[ - "src/shared.css", - "src/templates.css", - "src/main.tsx", - "src/swagger.ts", - ] { - inner.insert( - name.parse().unwrap(), - ManifestEntry { - name: None, - names: None, - src: None, - // Construct a fake but slightly plausible dummy asset name. - file: name - .replace('/', "__") - .replace('.', "-XXXXX.") - .replace(".tsx", ".js") - .replace(".ts", ".js") - .parse() - .unwrap(), - css: None, - assets: None, - is_entry: None, - is_dynamic_entry: None, - imports: None, - dynamic_imports: None, - integrity: None, - }, - ); - } - - Manifest { inner } - } -} - #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)] enum FileType { Script, diff --git a/crates/templates/src/functions.rs b/crates/templates/src/functions.rs index 82b79133a..9d764e032 100644 --- a/crates/templates/src/functions.rs +++ b/crates/templates/src/functions.rs @@ -30,7 +30,7 @@ use url::Url; pub fn register( env: &mut minijinja::Environment, url_builder: UrlBuilder, - vite_manifest: ViteManifest, + vite_manifest: Option, translator: Arc, ) { env.set_unknown_method_callback(minijinja_contrib::pycompat::unknown_method_callback); @@ -43,13 +43,17 @@ pub fn register( env.add_filter("parse_user_agent", filter_parse_user_agent); env.add_function("add_params_to_url", function_add_params_to_url); env.add_function("counter", || Ok(Value::from_object(Counter::default()))); - env.add_global( - "include_asset", - Value::from_object(IncludeAsset { - url_builder: url_builder.clone(), - vite_manifest, - }), - ); + if let Some(vite_manifest) = vite_manifest { + env.add_global( + "include_asset", + Value::from_object(IncludeAsset { + url_builder: url_builder.clone(), + vite_manifest, + }), + ); + } else { + env.add_global("include_asset", Value::from_object(FakeIncludeAsset {})); + } env.add_global( "translator", Value::from_object(TranslatorFunc { translator }), @@ -456,6 +460,30 @@ impl Object for IncludeAsset { } } +struct FakeIncludeAsset {} + +impl std::fmt::Debug for FakeIncludeAsset { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("FakeIncludeAsset").finish() + } +} + +impl std::fmt::Display for FakeIncludeAsset { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("fake_include_asset") + } +} + +impl Object for FakeIncludeAsset { + fn call(self: &Arc, _state: &State, args: &[Value]) -> Result { + let (path,): (&str,) = from_args(args)?; + + Ok(Value::from_safe_string(format!( + "" + ))) + } +} + #[derive(Debug, Default)] struct Counter { count: AtomicUsize, diff --git a/crates/templates/src/lib.rs b/crates/templates/src/lib.rs index ec23bfdb4..32a41e8b2 100644 --- a/crates/templates/src/lib.rs +++ b/crates/templates/src/lib.rs @@ -206,10 +206,12 @@ impl Templates { .await .map_err(TemplateLoadingError::ViteManifestIO)?; - serde_json::from_slice::(&raw_vite_manifest) - .map_err(TemplateLoadingError::ViteManifest)? + Some( + serde_json::from_slice::(&raw_vite_manifest) + .map_err(TemplateLoadingError::ViteManifest)?, + ) } else { - ViteManifest::sample() + None }; // Parse it