From 77f64ce08a42a39b44d977c8a0167789a3989080 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Tue, 29 Sep 2026 10:55:02 +0200 Subject: [PATCH] fix(fonts): deny a Google Fonts fetch unless it is opted into A scenario declaring fonts: [{ source: "google", ... }] made validate and still reach fonts.googleapis.com and write the result into the font cache. Neither command asked. The declared consumer of a scenario is a model generating JSON, so the file is untrusted input, and it is the file that chooses the target of the request: validating someone else's scenario should not be a network operation, and on a CI runner it turns a lint step into an outbound fetch. RemoteFontPolicy mirrors RemoteIncludePolicy rather than inventing a second authorization vocabulary: Deny by default, one flag to opt in. --allow-remote-fonts is global, so it works on any subcommand. The refusal sits immediately before the first network call, not at the top of the resolver. So it fires exactly when a request would leave, and a family already in the cache needs no flag at all -- an offline render that has its fonts keeps working, which is what makes this safe to default to deny. Only the weights actually missing from the cache are named. The policy is a process-level atomic rather than a parameter because resolve_font_entry is reached through load_custom_fonts from eight call sites, none of which carry a policy today. Not in scope, and worth its own issue: --allow-remote-include is named in the include refusal but does not exist as a CLI flag. The loader hardcodes RemoteIncludePolicy::Deny, so the message points at a flag nobody can pass. --- crates/rustmotion-core/src/engine/animator.rs | 9 +- .../rustmotion-core/src/engine/paint_pass.rs | 17 +-- .../src/engine/renderer/google_fonts.rs | 132 ++++++++++++++++++ .../src/engine/renderer/mod.rs | 1 + crates/rustmotion-core/src/error.rs | 12 ++ crates/rustmotion/CLAUDE.md | 25 ++++ crates/rustmotion/src/cli/commands/info.rs | 3 +- crates/rustmotion/src/cli/commands/migrate.rs | 3 +- crates/rustmotion/src/cli/commands/render.rs | 3 +- crates/rustmotion/src/cli/mod.rs | 10 ++ crates/rustmotion/src/encode/video_audio.rs | 3 +- 11 files changed, 199 insertions(+), 19 deletions(-) diff --git a/crates/rustmotion-core/src/engine/animator.rs b/crates/rustmotion-core/src/engine/animator.rs index 61385953..ad765d84 100644 --- a/crates/rustmotion-core/src/engine/animator.rs +++ b/crates/rustmotion-core/src/engine/animator.rs @@ -1,7 +1,7 @@ use crate::schema::{ - Animation, AnimationEffect, AnimationPreset, CharAnimPreset, EasingType, GlowConfig, Keyframe, - KeyframeValue, MotionPathConfig, OrbitConfig, PresetConfig, SpringConfig, TextAnimDirection, - TextAnimGranularity, WiggleConfig, + Animation, AnimationEffect, AnimationPreset, BurstConfig, CharAnimPreset, EasingType, + GlowConfig, Keyframe, KeyframeValue, MotionPathConfig, OrbitConfig, PresetConfig, SpringConfig, + TextAnimDirection, TextAnimGranularity, WiggleConfig, }; pub const DEFAULT_CHAR_BLUR_SIGMA: f32 = 14.0; @@ -338,7 +338,7 @@ pub fn shatter_progress(cfg: &crate::schema::ShatterConfig, time: f64) -> Option } } -pub fn burst_progress(cfg: &crate::schema::BurstConfig, time: f64) -> Option { +pub fn burst_progress(cfg: &BurstConfig, time: f64) -> Option { if cfg.duration <= 0.0 { return None; } @@ -3270,7 +3270,6 @@ mod repeat_cycle_tests { #[cfg(test)] mod burst_progress_tests { use super::*; - use crate::schema::BurstConfig; fn cfg() -> BurstConfig { BurstConfig { diff --git a/crates/rustmotion-core/src/engine/paint_pass.rs b/crates/rustmotion-core/src/engine/paint_pass.rs index 74c4496b..921cb493 100644 --- a/crates/rustmotion-core/src/engine/paint_pass.rs +++ b/crates/rustmotion-core/src/engine/paint_pass.rs @@ -14,8 +14,10 @@ use crate::css::style::{ use crate::css::units::{ parse_origin_component, Length, LengthContext, LengthPercentage, ParsedLength, }; +use crate::engine::animator::{burst_progress, burst_stroke_span}; use crate::engine::box_tree::{BoxKind, BoxNode, NodeId}; use crate::engine::layout_pass::{BoxLayout, LayoutResult}; +use crate::schema::{AnimationEffect as SchemaAnimationEffect, BurstConfig}; #[derive(Debug, Clone, Copy)] pub struct PaintFrame { @@ -902,12 +904,12 @@ fn paint_shattered_node( const MAX_BURST_COUNT: u32 = 64; -fn active_burst(css: &CssStyle, time: f64) -> Option<(&crate::schema::BurstConfig, f32)> { +fn active_burst(css: &CssStyle, time: f64) -> Option<(&BurstConfig, f32)> { let cfg = css.animation.iter().find_map(|e| match e { - crate::schema::AnimationEffect::Burst(c) => Some(c), + SchemaAnimationEffect::Burst(c) => Some(c), _ => None, })?; - crate::engine::animator::burst_progress(cfg, time).map(|progress| (cfg, progress)) + burst_progress(cfg, time).map(|progress| (cfg, progress)) } fn burst_track_start(half_width: f32, half_height: f32, dir: (f32, f32)) -> f32 { @@ -929,12 +931,7 @@ fn burst_track_start(half_width: f32, half_height: f32, dir: (f32, f32)) -> f32 } } -fn paint_burst( - canvas: &Canvas, - box_layout: &BoxLayout, - cfg: &crate::schema::BurstConfig, - progress: f32, -) { +fn paint_burst(canvas: &Canvas, box_layout: &BoxLayout, cfg: &BurstConfig, progress: f32) { let count = cfg.count.clamp(1, MAX_BURST_COUNT); let length = cfg.length.max(0.0); let width = cfg.width.max(0.0); @@ -962,7 +959,7 @@ fn paint_burst( let dir = (angle.cos(), angle.sin()); let stroke_length = length * (1.0 + (shatter_hash(cfg.seed, i, 12) - 0.5) * jitter); let phase = shatter_hash(cfg.seed, i, 13) * jitter * 0.5; - let (tail, head) = crate::engine::animator::burst_stroke_span(progress, phase); + let (tail, head) = burst_stroke_span(progress, phase); if (head - tail) * stroke_length < 0.5 { continue; } diff --git a/crates/rustmotion-core/src/engine/renderer/google_fonts.rs b/crates/rustmotion-core/src/engine/renderer/google_fonts.rs index 6925d4ae..89d2fd2c 100644 --- a/crates/rustmotion-core/src/engine/renderer/google_fonts.rs +++ b/crates/rustmotion-core/src/engine/renderer/google_fonts.rs @@ -16,6 +16,31 @@ pub fn font_cache_dir() -> PathBuf { base.join("rustmotion").join("fonts") } +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub enum RemoteFontPolicy { + #[default] + Deny, + Allow, +} + +static REMOTE_FONT_POLICY: std::sync::atomic::AtomicBool = + std::sync::atomic::AtomicBool::new(false); + +pub fn set_remote_font_policy(policy: RemoteFontPolicy) { + REMOTE_FONT_POLICY.store( + policy == RemoteFontPolicy::Allow, + std::sync::atomic::Ordering::Release, + ); +} + +pub fn remote_font_policy() -> RemoteFontPolicy { + if REMOTE_FONT_POLICY.load(std::sync::atomic::Ordering::Acquire) { + RemoteFontPolicy::Allow + } else { + RemoteFontPolicy::Deny + } +} + pub fn resolve_google_font( family: &str, weights: &[u16], @@ -41,6 +66,18 @@ pub fn resolve_google_font( } let url = build_css2_url(family, &missing_weights); + if remote_font_policy() != RemoteFontPolicy::Allow { + return Err(RustmotionError::RemoteFontDenied { + family: family.to_string(), + weights: missing_weights + .iter() + .map(u16::to_string) + .collect::>() + .join(", "), + url: url.clone(), + cache_hint: cache_dir.display().to_string(), + }); + } let css = fetch_css2(&url, family)?; let ttf_urls = parse_ttf_urls(&css); @@ -291,3 +328,98 @@ mod tests { assert_eq!(family_slug("Inter"), "inter"); } } + +#[cfg(test)] +mod remote_font_policy_tests { + use super::*; + + struct Restore(RemoteFontPolicy); + impl Drop for Restore { + fn drop(&mut self) { + set_remote_font_policy(self.0); + } + } + + fn a_cache_dir(name: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "rm_font_policy_{}_{}_{name}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos() + )); + std::fs::create_dir_all(&dir).expect("cache dir"); + dir + } + + #[test] + fn the_default_is_deny() { + assert_eq!( + RemoteFontPolicy::default(), + RemoteFontPolicy::Deny, + "a scenario is untrusted input and it chooses the target of the request" + ); + } + + #[test] + fn a_family_that_is_not_cached_is_refused_by_name_without_touching_the_network() { + let _restore = Restore(remote_font_policy()); + set_remote_font_policy(RemoteFontPolicy::Deny); + let dir = a_cache_dir("denied"); + + let err = resolve_google_font("Zilla Slab Highlight", &[700], &dir) + .expect_err("a missing weight would have to be fetched"); + let message = err.to_string(); + assert!( + message.contains("Zilla Slab Highlight") && message.contains("700"), + "the refusal has to name the family and the weights: {message}" + ); + assert!( + message.contains("fonts.googleapis.com"), + "and the target it declined to reach: {message}" + ); + assert!( + message.contains("--allow-remote-fonts"), + "and the one flag that opts in: {message}" + ); + std::fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn a_family_already_in_the_cache_needs_no_flag() { + let _restore = Restore(remote_font_policy()); + set_remote_font_policy(RemoteFontPolicy::Deny); + let dir = a_cache_dir("cached"); + let slug = family_slug("Zilla Slab Highlight"); + std::fs::write(dir.join(format!("{slug}-700.ttf")), b"not a real face").expect("seed"); + + let paths = resolve_google_font("Zilla Slab Highlight", &[700], &dir) + .expect("nothing has to be fetched, so nothing is denied"); + assert_eq!( + paths.len(), + 1, + "denying the network must not break an offline render that already has its fonts" + ); + std::fs::remove_dir_all(&dir).ok(); + } + + #[test] + fn only_the_weights_that_are_missing_are_named_in_the_refusal() { + let _restore = Restore(remote_font_policy()); + set_remote_font_policy(RemoteFontPolicy::Deny); + let dir = a_cache_dir("partial"); + let slug = family_slug("Inter"); + std::fs::write(dir.join(format!("{slug}-400.ttf")), b"not a real face").expect("seed"); + + let message = resolve_google_font("Inter", &[400, 700], &dir) + .expect_err("700 is missing") + .to_string(); + assert!( + message.contains("700") && !message.contains("400, 700"), + "400 is already on disk and needs no request, so it has no business in the \ + refusal: {message}" + ); + std::fs::remove_dir_all(&dir).ok(); + } +} diff --git a/crates/rustmotion-core/src/engine/renderer/mod.rs b/crates/rustmotion-core/src/engine/renderer/mod.rs index 5a8e20b8..f6d036b8 100644 --- a/crates/rustmotion-core/src/engine/renderer/mod.rs +++ b/crates/rustmotion-core/src/engine/renderer/mod.rs @@ -3,6 +3,7 @@ pub mod audio_analysis; mod colors; mod fonts; pub mod google_fonts; +pub use google_fonts::{remote_font_policy, set_remote_font_policy, RemoteFontPolicy}; mod shapes; mod text; mod yuv; diff --git a/crates/rustmotion-core/src/error.rs b/crates/rustmotion-core/src/error.rs index 6e52aedb..99851823 100644 --- a/crates/rustmotion-core/src/error.rs +++ b/crates/rustmotion-core/src/error.rs @@ -96,6 +96,18 @@ pub enum RustmotionError { cache_hint: String, }, + #[error( + "Google Fonts: '{family}' (weights {weights}) is not in the font cache, and fetching \ + it would reach {url} — a scenario chooses that target, so it is denied by default.\n\ + Pass --allow-remote-fonts to opt in, or place a TTF for each weight at:\n {cache_hint}" + )] + RemoteFontDenied { + family: String, + weights: String, + url: String, + cache_hint: String, + }, + #[error( "Google Fonts: no font URLs found in CSS response for '{family}' — \ the family name may be misspelled or unavailable" diff --git a/crates/rustmotion/CLAUDE.md b/crates/rustmotion/CLAUDE.md index 0a270f17..d2274b4e 100644 --- a/crates/rustmotion/CLAUDE.md +++ b/crates/rustmotion/CLAUDE.md @@ -25,6 +25,31 @@ CLI : - `--strict-attrs` — promeut en erreurs les attributs inconnus (détection schéma + did-you-mean, activée par défaut en warnings) - `--lenient` — warnings au lieu d'errors +## Google Fonts: the network is denied by default + +A scenario declaring `fonts: [{ "family": "Inter", "source": "google", "weights": [400, 700] }]` +downloads the face from `fonts.googleapis.com` into the cache +(`~/.cache/rustmotion/fonts`). The declared consumer of a scenario is a model +generating JSON, so the file is **untrusted input** — and it is the file that +chooses the target of the request. Validating someone else's scenario should not +be a network operation, and on a CI runner it turns a lint step into an outbound +fetch. + +`--allow-remote-fonts` (global, on any subcommand) is the only way to consent to +it. Without it, a weight **missing from the cache** is refused by naming the +family, the missing weights, the URL that was not called, and the directory to +drop a TTF into by hand: + +``` +Google Fonts: 'Zilla Slab Highlight' (weights 700) is not in the font cache, and +fetching it would reach https://fonts.googleapis.com/css2?family=... — a scenario +chooses that target, so it is denied by default. +``` + +A face **already in the cache** asks for nothing: the refusal fires at the exact +moment a request would leave. An offline render that has its fonts keeps working +without the flag. + ## Encodage - ffmpeg est auto-détecté et utilisé par défaut (10-bit H.264, meilleure qualité sur les gradients sombres) diff --git a/crates/rustmotion/src/cli/commands/info.rs b/crates/rustmotion/src/cli/commands/info.rs index 71f17992..b540f57e 100644 --- a/crates/rustmotion/src/cli/commands/info.rs +++ b/crates/rustmotion/src/cli/commands/info.rs @@ -2,6 +2,7 @@ use rustmotion::components::intrinsic::{GradientTextIntrinsic, TextIntrinsic}; use rustmotion::components::{ChildComponent, Component}; use rustmotion::core::engine::box_tree::{AvailableSpace, IntrinsicMeasure}; use rustmotion::encode::build_frame_tasks; +use rustmotion::encode::video::slide_scene_spans; use rustmotion::engine::animator::spring_rest_time; use rustmotion::engine::render::deserialize_children; use rustmotion::error::Result; @@ -52,7 +53,7 @@ pub fn cmd_info(input: &PathBuf) -> Result<()> { view.scenes.len() ); let spans = match view.view_type { - schema::ViewType::Slide => rustmotion::encode::video::slide_scene_spans(view, fps), + schema::ViewType::Slide => slide_scene_spans(view, fps), schema::ViewType::World => Vec::new(), }; for (si, scene) in view.scenes.iter().enumerate() { diff --git a/crates/rustmotion/src/cli/commands/migrate.rs b/crates/rustmotion/src/cli/commands/migrate.rs index e3ce4885..4d00172d 100644 --- a/crates/rustmotion/src/cli/commands/migrate.rs +++ b/crates/rustmotion/src/cli/commands/migrate.rs @@ -1,3 +1,4 @@ +use rustmotion::encode::video::quantised_spans; use rustmotion::error::{Result, RustmotionError}; use serde_json::Value; use std::path::{Path, PathBuf}; @@ -35,7 +36,7 @@ fn migrate_scenes_array(scenes: &mut [Value], fps: u32, label: &str) -> Result<( .collect::>()?; let incoming: Vec = scenes.iter().map(incoming_transition_seconds).collect(); - let spans = rustmotion::encode::video::quantised_spans(&durations, &incoming, fps); + let spans = quantised_spans(&durations, &incoming, fps); let n = scenes.len(); let at_frames: Vec = spans.iter().map(|s| s.start).collect(); diff --git a/crates/rustmotion/src/cli/commands/render.rs b/crates/rustmotion/src/cli/commands/render.rs index 7c553f77..fa927eb3 100644 --- a/crates/rustmotion/src/cli/commands/render.rs +++ b/crates/rustmotion/src/cli/commands/render.rs @@ -2,6 +2,7 @@ use crate::cli::tui; use crate::cli::OutputFormat; use rustmotion::encode; use rustmotion::engine; +use rustmotion::engine::renderer::invalidate_font_caches; use rustmotion::error::{Result, RustmotionError}; use rustmotion::schema::ResolvedScenario; use std::path::{Path, PathBuf}; @@ -12,7 +13,7 @@ fn clear_all_media_caches() { engine::clear_asset_cache(); engine::gif_cache().clear(); engine::video_frame_cache().clear(); - rustmotion_core::engine::renderer::invalidate_font_caches(); + invalidate_font_caches(); } #[allow(clippy::too_many_arguments)] diff --git a/crates/rustmotion/src/cli/mod.rs b/crates/rustmotion/src/cli/mod.rs index 4f1d9271..408ca2c7 100644 --- a/crates/rustmotion/src/cli/mod.rs +++ b/crates/rustmotion/src/cli/mod.rs @@ -4,6 +4,7 @@ mod skills; mod tui; use clap::{CommandFactory, Parser, Subcommand}; +use rustmotion::engine::renderer::{set_remote_font_policy, RemoteFontPolicy}; use rustmotion::error::{Result, RustmotionError}; use rustmotion::schema::ResolvedScenario; use std::collections::HashMap; @@ -27,6 +28,11 @@ struct Cli { /// Number of parallel rendering threads (defaults to all cores) #[arg(long, global = true)] threads: Option, + + /// Allow fetching a Google font that is not already in the font cache. A scenario + /// chooses the target of that request, so it is denied by default. + #[arg(long, global = true)] + allow_remote_fonts: bool, } #[derive(Subcommand)] @@ -670,6 +676,10 @@ pub fn run() -> Result<()> { .ok(); } + if cli.allow_remote_fonts { + set_remote_font_policy(RemoteFontPolicy::Allow); + } + match cli.command { Commands::Render { file, diff --git a/crates/rustmotion/src/encode/video_audio.rs b/crates/rustmotion/src/encode/video_audio.rs index 7fda38e0..b08ff071 100644 --- a/crates/rustmotion/src/encode/video_audio.rs +++ b/crates/rustmotion/src/encode/video_audio.rs @@ -3,6 +3,7 @@ use std::hash::{Hash, Hasher}; use std::path::PathBuf; use crate::components::{ChildComponent, Component}; +use crate::encode::video::slide_scene_spans; use crate::schema::{AudioTrack, ResolvedScenario, ViewType}; pub struct TimelineOffsets { @@ -29,7 +30,7 @@ pub fn timeline_offsets(scenario: &ResolvedScenario) -> TimelineOffsets { match view.view_type { ViewType::Slide => { - let spans = crate::encode::video::slide_scene_spans(view, scenario.video.fps); + let spans = slide_scene_spans(view, scenario.video.fps); let scene_offsets: Vec = spans .iter() .map(|span| cursor + span.start as f64 / fps)