Skip to main content

max / makenotwork

ux: friendly errors for validated extractors, widen hide/strobe guards, posture+clamp fixes Drive UX to A (ultra-fuzz Run 7). UX-S4: a request struct embedding a validating newtype (Slug, PriceCents, ...) failed at extraction with axum's raw plain-text rejection — no friendly error template, no JSON on API routes. New src/extractors.rs adds ValidatedForm / ValidatedJson (axum) and ValidatedHtmlForm (axum_extra repeated-field form) that map any extraction failure to AppError::validation, so it flows through the same path as an explicit validation error (friendly template, {"error":...} on API routes, 422). Migrated the request extractors in api/projects, api/blog, api/subscriptions, api/users/profile, and the project wizard's BasicsForm (the audit's named slug-vs-title case). UX-M1: OAuth authorize POST was registered post_csrf_skip although the handler validates the consent-form _csrf — switch to post_csrf_manual (the accurate posture). /oauth/token stays skip (genuinely pre-auth). UX-M2: clamp the synckit keys list offset like the limit, so a huge offset can't force a deep SQL OFFSET scan. UX-M3: support.rs subject/message length checks use chars().count() to match the "characters" wording (and broadcast.rs), not byte length. UX-M4/M5: widen the custom-page hiding-property allowlist (clip-path inset(100%)/ circle(0), font-size:0, max-width/height:0, clip:rect(0...), off-screen text-indent) and make the strobe guard also drop a fast animation with a high finite iteration-count, not only `infinite`. Regression tests added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-24 23:13 UTC
Commit: 832b7b7654293a3a7847379d4047202ec4c64329
Parent: 69ab513
11 files changed, +239 insertions, -20 deletions
M server/src/custom_pages/css_sanitizer.rs +93 -1
@@ -404,15 +404,45 @@ fn is_hiding_property(prop: &Property) -> bool {
404 404 | "width:0px"
405 405 | "height:0"
406 406 | "height:0px"
407 + // UX-M4: 0-sized box via max-* and 0 font-size hide content too.
408 + | "max-width:0"
409 + | "max-width:0px"
410 + | "max-height:0"
411 + | "max-height:0px"
412 + | "font-size:0"
413 + | "font-size:0px"
414 + // Legacy clip:rect(0,0,0,0) screen-reader-hide trick.
415 + | "clip:rect(0,0,0,0)"
416 + | "clip:rect(0px,0px,0px,0px)"
407 417 ) || (norm.starts_with("transform:") && norm.contains("scale(0)"))
418 + // clip-path clipping the element to nothing (UX-M4).
419 + || (norm.starts_with("clip-path:")
420 + && (norm.contains("inset(100%") || norm.contains("circle(0")))
421 + // Off-screen text via large-negative text-indent (UX-M4).
422 + || is_offscreen_text_indent(&norm)
423 + }
424 +
425 + /// Large-negative `text-indent` — the classic off-screen text-hiding trick
426 + /// (`text-indent:-9999px`). Anything ≤ -1000px (or unitless) counts as hiding.
427 + fn is_offscreen_text_indent(norm: &str) -> bool {
428 + norm.strip_prefix("text-indent:")
429 + .map(|rest| rest.strip_suffix("px").unwrap_or(rest))
430 + .and_then(|n| n.parse::<f32>().ok())
431 + .map(|v| v <= -1000.0)
432 + .unwrap_or(false)
408 433 }
409 434
410 435 /// Drop infinite animations faster than 2s (strobe guard, decision #3). The
411 436 /// reduced-motion override handles accessibility; this caps the worst abuse for
412 437 /// everyone else.
413 438 fn enforce_animation_budget(decls: &mut DeclarationBlock, rejections: &mut Vec<Rejection>) {
439 + /// A finite iteration-count this high at a sub-2s duration strobes just like
440 + /// `infinite` does (UX-M5) — `infinite` was the only case caught before.
441 + const STROBE_MAX_ITERATIONS: f32 = 20.0;
442 +
414 443 let mut has_infinite = false;
415 444 let mut min_duration: Option<f32> = None;
445 + let mut max_iterations: Option<f32> = None;
416 446
417 447 for list in [&decls.declarations, &decls.important_declarations] {
418 448 for prop in list {
@@ -424,13 +454,18 @@ fn enforce_animation_budget(decls: &mut DeclarationBlock, rejections: &mut Vec<R
424 454 }
425 455 if let Some(rest) = raw.strip_prefix("animation-duration:") {
426 456 update_min_duration(rest, &mut min_duration);
457 + } else if let Some(rest) = raw.strip_prefix("animation-iteration-count:") {
458 + update_max_iterations(rest, &mut max_iterations);
427 459 } else if let Some(rest) = raw.strip_prefix("animation:") {
428 460 update_min_duration(rest, &mut min_duration);
461 + update_max_iterations(rest, &mut max_iterations);
429 462 }
430 463 }
431 464 }
432 465
433 - let strobe = has_infinite && min_duration.map(|d| d < 2.0).unwrap_or(false);
466 + let fast = min_duration.map(|d| d < 2.0).unwrap_or(false);
467 + let high_count = max_iterations.map(|n| n >= STROBE_MAX_ITERATIONS).unwrap_or(false);
468 + let strobe = (has_infinite || high_count) && fast;
434 469 if !strobe {
435 470 return;
436 471 }
@@ -465,6 +500,22 @@ fn update_min_duration(value: &str, min: &mut Option<f32>) {
465 500 }
466 501 }
467 502
503 + /// Track the largest finite iteration-count seen, scanning either the explicit
504 + /// `animation-iteration-count` value or the `animation` shorthand (UX-M5). A bare
505 + /// unitless number is the count; durations carry `s`/`ms` and percentages `%`, so
506 + /// they're skipped. `infinite` is handled separately.
507 + fn update_max_iterations(value: &str, max: &mut Option<f32>) {
508 + for token in value.split([' ', ',']) {
509 + let token = token.trim();
510 + if token.is_empty() || token.ends_with('s') || token.ends_with('%') {
511 + continue;
512 + }
513 + if let Ok(n) = token.parse::<f32>() {
514 + *max = Some(max.map_or(n, |m| m.max(n)));
515 + }
516 + }
517 + }
518 +
468 519 /// Parse a CSS time token to seconds. Returns None for non-time tokens.
469 520 fn parse_seconds(token: &str) -> Option<f32> {
470 521 let t = token.trim();
@@ -603,6 +654,24 @@ mod tests {
603 654 }
604 655
605 656 #[test]
657 + fn mnw_widened_hiding_properties_stripped() {
658 + // UX-M4: clip-path, font-size:0, and off-screen text-indent are all hides.
659 + for decl in [
660 + "clip-path: inset(100%)",
661 + "font-size: 0",
662 + "text-indent: -9999px",
663 + "max-height: 0",
664 + "clip: rect(0, 0, 0, 0)",
665 + ] {
666 + let (_out, rej) = san(&format!(".mnw-buy {{ {decl} }}"));
667 + assert!(
668 + rej.iter().any(|r| r.kind == RejectionKind::HidingProperty),
669 + "expected {decl} to be treated as hiding"
670 + );
671 + }
672 + }
673 +
674 + #[test]
606 675 fn reduced_motion_appended() {
607 676 let out = scoped("p { color: red }");
608 677 assert!(out.contains("prefers-reduced-motion"));
@@ -624,6 +693,29 @@ mod tests {
624 693 }
625 694
626 695 #[test]
696 + fn fast_high_finite_count_animation_dropped() {
697 + // UX-M5: a fast animation with a high *finite* iteration-count strobes too,
698 + // not just `infinite`.
699 + let (out, rej) = san(".spin { animation: spin 1s linear 100 }");
700 + assert!(!normalize(&out).contains("animation:spin"));
701 + assert!(rej.iter().any(|r| r.kind == RejectionKind::AnimationBudget));
702 +
703 + // Explicit property form is caught as well.
704 + let (_out2, rej2) = san(
705 + ".spin { animation-name: spin; animation-duration: 0.5s; animation-iteration-count: 50 }",
706 + );
707 + assert!(rej2.iter().any(|r| r.kind == RejectionKind::AnimationBudget));
708 + }
709 +
710 + #[test]
711 + fn fast_low_finite_count_animation_kept() {
712 + // A handful of iterations at a fast duration is fine — not a strobe.
713 + let (out, rej) = san(".spin { animation: spin 1s linear 3 }");
714 + assert!(out.to_lowercase().contains("animation"));
715 + assert!(!rej.iter().any(|r| r.kind == RejectionKind::AnimationBudget));
716 + }
717 +
718 + #[test]
627 719 fn expression_function_recorded() {
628 720 let (_out, rej) = san(".x { width: expression(alert(1)) }");
629 721 assert!(rej.iter().any(|r| r.kind == RejectionKind::BlockedFunction));
A server/src/extractors.rs +120
@@ -0,0 +1,120 @@
1 + //! Body extractors that route deserialization/validation failures through
2 + //! [`AppError::validation`] instead of axum's raw plain-text rejection (UX-S4).
3 + //!
4 + //! Several request structs embed validating newtypes (e.g. `Slug`, `PriceCents`)
5 + //! whose `Deserialize` impls reject bad input. With a bare `Form<T>`/`Json<T>`
6 + //! that rejection surfaces as axum's default plain-text 4xx — no friendly error
7 + //! template, no JSON-on-API-routes treatment, no preserved status semantics. These
8 + //! wrappers map any extraction failure to `AppError::validation`, so the request
9 + //! flows through the same error path as an explicit validation failure: the
10 + //! friendly template for page routes, `{"error": "..."}` for API routes (via
11 + //! `json_error_layer`), and a 422.
12 +
13 + use axum::extract::{FromRequest, Request};
14 + use serde::de::DeserializeOwned;
15 +
16 + use crate::error::AppError;
17 +
18 + /// `axum::Form<T>` whose extraction failures become [`AppError::validation`].
19 + pub struct ValidatedForm<T>(pub T);
20 +
21 + impl<T, S> FromRequest<S> for ValidatedForm<T>
22 + where
23 + T: DeserializeOwned,
24 + S: Send + Sync,
25 + {
26 + type Rejection = AppError;
27 +
28 + async fn from_request(req: Request, state: &S) -> Result<Self, Self::Rejection> {
29 + match axum::Form::<T>::from_request(req, state).await {
30 + Ok(axum::Form(value)) => Ok(Self(value)),
31 + Err(rejection) => Err(rejection_to_validation(rejection.body_text())),
32 + }
33 + }
34 + }
35 +
36 + /// `axum::Json<T>` whose extraction failures become [`AppError::validation`].
37 + pub struct ValidatedJson<T>(pub T);
38 +
39 + impl<T, S> FromRequest<S> for ValidatedJson<T>
40 + where
41 + T: DeserializeOwned,
42 + S: Send + Sync,
43 + {
44 + type Rejection = AppError;
45 +
46 + async fn from_request(req: Request, state: &S) -> Result<Self, Self::Rejection> {
47 + match axum::Json::<T>::from_request(req, state).await {
48 + Ok(axum::Json(value)) => Ok(Self(value)),
49 + Err(rejection) => Err(rejection_to_validation(rejection.body_text())),
50 + }
51 + }
52 + }
53 +
54 + /// `axum_extra::extract::Form<T>` (repeated-field form) whose extraction failures
55 + /// become [`AppError::validation`]. Used by the project wizard, whose `BasicsForm`
56 + /// embeds a validating `Slug` newtype — without this, a bad slug fails extraction
57 + /// with a raw rejection while a bad title (validated in-handler) got the friendly
58 + /// path, the exact split the audit called out (UX-S4).
59 + pub struct ValidatedHtmlForm<T>(pub T);
60 +
61 + impl<T, S> FromRequest<S> for ValidatedHtmlForm<T>
62 + where
63 + T: DeserializeOwned,
64 + S: Send + Sync,
65 + {
66 + type Rejection = AppError;
67 +
68 + async fn from_request(req: Request, state: &S) -> Result<Self, Self::Rejection> {
69 + match axum_extra::extract::Form::<T>::from_request(req, state).await {
70 + Ok(axum_extra::extract::Form(value)) => Ok(Self(value)),
71 + Err(rejection) => Err(rejection_to_validation(rejection.to_string())),
72 + }
73 + }
74 + }
75 +
76 + /// Strip axum's "Failed to deserialize …: " machinery so the user-facing message
77 + /// is the underlying cause (which, for a validating newtype, is its own error
78 + /// string). Falls back to a generic message when nothing useful remains.
79 + fn rejection_to_validation(detail: String) -> AppError {
80 + let msg = detail
81 + .strip_prefix("Failed to deserialize form body: ")
82 + .or_else(|| {
83 + detail.strip_prefix("Failed to deserialize the JSON body into the target type: ")
84 + })
85 + .unwrap_or(&detail)
86 + .trim();
87 + let msg = if msg.is_empty() {
88 + "Please check your input and try again.".to_string()
89 + } else {
90 + msg.to_string()
91 + };
92 + AppError::validation(msg)
93 + }
94 +
95 + #[cfg(test)]
96 + mod tests {
97 + use super::*;
98 +
99 + #[test]
100 + fn strips_form_prefix() {
101 + let e = rejection_to_validation(
102 + "Failed to deserialize form body: slug must be lowercase".to_string(),
103 + );
104 + assert_eq!(e.user_message(), "slug must be lowercase");
105 + }
106 +
107 + #[test]
108 + fn strips_json_prefix() {
109 + let e = rejection_to_validation(
110 + "Failed to deserialize the JSON body into the target type: price too high".to_string(),
111 + );
112 + assert_eq!(e.user_message(), "price too high");
113 + }
114 +
115 + #[test]
116 + fn falls_back_when_empty() {
117 + let e = rejection_to_validation(String::new());
118 + assert_eq!(e.user_message(), "Please check your input and try again.");
119 + }
120 + }
M server/src/lib.rs +1
@@ -12,6 +12,7 @@ pub mod custom_pages;
12 12 pub mod db;
13 13 pub mod email;
14 14 pub mod error;
15 + pub mod extractors;
15 16 pub mod git;
16 17 pub mod git_ssh;
17 18 pub mod license_templates;
M server/src/routes/api/blog.rs +3 -2
@@ -17,6 +17,7 @@ use crate::{
17 17 AppState,
18 18 };
19 19
20 + use crate::extractors::ValidatedJson;
20 21 use super::{verify_blog_post_ownership, verify_project_ownership};
21 22
22 23 // =============================================================================
@@ -126,7 +127,7 @@ pub(super) async fn create_blog_post(
126 127 State(state): State<AppState>,
127 128 AuthUser(user): AuthUser,
128 129 Path(project_id): Path<ProjectId>,
129 - Json(req): Json<CreateBlogPostRequest>,
130 + ValidatedJson(req): ValidatedJson<CreateBlogPostRequest>,
130 131 ) -> Result<impl IntoResponse> {
131 132 user.check_not_suspended()?;
132 133 verify_project_ownership(&state, project_id, user.id).await?;
@@ -184,7 +185,7 @@ pub(super) async fn update_blog_post(
184 185 State(state): State<AppState>,
185 186 AuthUser(user): AuthUser,
186 187 Path(id): Path<BlogPostId>,
187 - Json(req): Json<UpdateBlogPostRequest>,
188 + ValidatedJson(req): ValidatedJson<UpdateBlogPostRequest>,
188 189 ) -> Result<impl IntoResponse> {
189 190 user.check_not_suspended()?;
190 191 let existing = verify_blog_post_ownership(&state, id, user.id).await?;
M server/src/routes/api/projects.rs +3 -2
@@ -18,6 +18,7 @@ use crate::{
18 18 AppState,
19 19 };
20 20
21 + use crate::extractors::{ValidatedForm, ValidatedJson};
21 22 use super::verify_project_ownership;
22 23
23 24 // =============================================================================
@@ -53,7 +54,7 @@ pub(super) async fn create_project(
53 54 State(state): State<AppState>,
54 55 headers: HeaderMap,
55 56 AuthUser(user): AuthUser,
56 - Form(req): Form<CreateProjectRequest>,
57 + ValidatedForm(req): ValidatedForm<CreateProjectRequest>,
57 58 ) -> Result<Response> {
58 59 user.check_not_suspended()?;
59 60
@@ -214,7 +215,7 @@ pub(super) async fn update_project(
214 215 State(state): State<AppState>,
215 216 AuthUser(user): AuthUser,
216 217 Path(id): Path<ProjectId>,
217 - Json(req): Json<UpdateProjectRequest>,
218 + ValidatedJson(req): ValidatedJson<UpdateProjectRequest>,
218 219 ) -> Result<impl IntoResponse> {
219 220 tracing::Span::current().record("project_id", tracing::field::display(&id));
220 221 user.check_not_suspended()?;
M server/src/routes/api/subscriptions.rs +3 -2
@@ -18,6 +18,7 @@ use crate::{
18 18 AppState,
19 19 };
20 20
21 + use crate::extractors::ValidatedJson;
21 22 use super::verify_project_ownership;
22 23
23 24 /// JSON response representing a subscription tier.
@@ -54,7 +55,7 @@ pub(super) async fn create_tier(
54 55 State(state): State<AppState>,
55 56 AuthUser(user): AuthUser,
56 57 Path(project_id): Path<ProjectId>,
57 - Json(req): Json<CreateTierRequest>,
58 + ValidatedJson(req): ValidatedJson<CreateTierRequest>,
58 59 ) -> Result<impl IntoResponse> {
59 60 user.check_not_suspended()?;
60 61 // Verify ownership
@@ -151,7 +152,7 @@ pub(super) async fn update_tier(
151 152 State(state): State<AppState>,
152 153 AuthUser(user): AuthUser,
153 154 Path(tier_id): Path<SubscriptionTierId>,
154 - Json(req): Json<UpdateTierRequest>,
155 + ValidatedJson(req): ValidatedJson<UpdateTierRequest>,
155 156 ) -> Result<impl IntoResponse> {
156 157 user.check_not_suspended()?;
157 158 // Get tier and verify project ownership
M server/src/routes/api/users/profile.rs +7 -6
@@ -4,7 +4,7 @@ use axum::{
4 4 extract::State,
5 5 http::{header::HeaderMap, StatusCode},
6 6 response::{Html, IntoResponse, Response},
7 - Form, Json,
7 + Json,
8 8 };
9 9 use serde::{Deserialize, Serialize};
10 10 use tower_sessions::Session;
@@ -20,6 +20,7 @@ use crate::{
20 20 AppState,
21 21 };
22 22
23 + use crate::extractors::ValidatedForm;
23 24 use super::SuccessMessageResponse;
24 25
25 26 /// JSON response for profile updates.
@@ -44,7 +45,7 @@ pub(in crate::routes::api) async fn update_profile(
44 45 State(state): State<AppState>,
45 46 headers: HeaderMap,
46 47 AuthUser(user): AuthUser,
47 - Form(req): Form<UpdateProfileRequest>,
48 + ValidatedForm(req): ValidatedForm<UpdateProfileRequest>,
48 49 ) -> Result<Response> {
49 50 user.check_not_suspended()?;
50 51 // Validate input
@@ -91,7 +92,7 @@ pub(in crate::routes::api) async fn update_profile_theme(
91 92 State(state): State<AppState>,
92 93 headers: HeaderMap,
93 94 AuthUser(user): AuthUser,
94 - Form(req): Form<UpdateThemeRequest>,
95 + ValidatedForm(req): ValidatedForm<UpdateThemeRequest>,
95 96 ) -> Result<Response> {
96 97 user.check_not_suspended()?;
97 98 let theme_id = crate::theming::normalize_theme_id(req.theme_id.as_deref())
@@ -121,7 +122,7 @@ pub(in crate::routes::api) async fn update_password(
121 122 headers: HeaderMap,
122 123 session: Session,
123 124 AuthUser(user): AuthUser,
124 - Form(req): Form<UpdatePasswordRequest>,
125 + ValidatedForm(req): ValidatedForm<UpdatePasswordRequest>,
125 126 ) -> Result<Response> {
126 127 user.check_not_sandbox()?;
127 128 let is_htmx = is_htmx_request(&headers);
@@ -417,7 +418,7 @@ pub(in crate::routes::api) async fn request_account_deletion(
417 418 State(state): State<AppState>,
418 419 headers: HeaderMap,
419 420 AuthUser(user): AuthUser,
420 - Form(form): Form<RequestDeletionForm>,
421 + ValidatedForm(form): ValidatedForm<RequestDeletionForm>,
421 422 ) -> Result<Response> {
422 423 user.check_not_sandbox()?;
423 424 let is_htmx = is_htmx_request(&headers);
@@ -488,7 +489,7 @@ pub(in crate::routes::api) async fn submit_appeal(
488 489 State(state): State<AppState>,
489 490 headers: HeaderMap,
490 491 AuthUser(user): AuthUser,
491 - Form(form): Form<AppealForm>,
492 + ValidatedForm(form): ValidatedForm<AppealForm>,
492 493 ) -> Result<Response> {
493 494 let is_htmx = is_htmx_request(&headers);
494 495
M server/src/routes/api/users/support.rs +2 -2
@@ -34,14 +34,14 @@ pub(in crate::routes::api) async fn submit_support_ticket(
34 34 let message = form.message.trim();
35 35 let category = form.category.trim();
36 36
37 - if subject.is_empty() || subject.len() > 200 {
37 + if subject.is_empty() || subject.chars().count() > 200 {
38 38 return Ok(Html(FormStatusTemplate {
39 39 success: false,
40 40 message: "Subject must be between 1 and 200 characters.".to_string(),
41 41 }.render_string()).into_response());
42 42 }
43 43
44 - if message.is_empty() || message.len() > 5000 {
44 + if message.is_empty() || message.chars().count() > 5000 {
45 45 return Ok(Html(FormStatusTemplate {
46 46 success: false,
47 47 message: "Message must be between 1 and 5000 characters.".to_string(),
M server/src/routes/oauth.rs +2 -2
@@ -11,7 +11,7 @@ use axum::{
11 11 routing::get,
12 12 Form, Json,
13 13 };
14 - use crate::csrf::{post_csrf_skip, CsrfRouter};
14 + use crate::csrf::{post_csrf_manual, post_csrf_skip, CsrfRouter};
15 15 use rand::RngCore;
16 16 use serde::{Deserialize, Serialize};
17 17 use sha2::{Digest, Sha256};
@@ -1029,7 +1029,7 @@ pub fn oauth_routes() -> CsrfRouter<AppState> {
1029 1029
1030 1030 let authorize_routes = CsrfRouter::new()
1031 1031 .route_get("/oauth/authorize", get(authorize_get))
1032 - .route("/oauth/authorize", post_csrf_skip("pre-auth OAuth authorize endpoint", authorize_post))
1032 + .route("/oauth/authorize", post_csrf_manual("OAuth authorize validates the consent form _csrf in-handler via validate_token_consuming", authorize_post))
1033 1033 .route_layer(GovernorLayer {
1034 1034 config: authorize_rate_limit,
1035 1035 });
M server/src/routes/pages/dashboard/wizards/project.rs +2 -2
@@ -9,7 +9,6 @@ use axum::{
9 9 response::{IntoResponse, Response},
10 10 Form,
11 11 };
12 - use axum_extra::extract::Form as HtmlForm;
13 12 use tower_sessions::Session;
14 13
15 14 use crate::{
@@ -23,6 +22,7 @@ use crate::{
23 22 AppState,
24 23 };
25 24
25 + use crate::extractors::ValidatedHtmlForm;
26 26 use super::build_step_nav;
27 27
28 28 /// Ordered step names for the project wizard.
@@ -105,7 +105,7 @@ pub async fn step_basics_create(
105 105 State(state): State<AppState>,
106 106 session: Session,
107 107 AuthUser(user): AuthUser,
108 - HtmlForm(form): HtmlForm<BasicsForm>,
108 + ValidatedHtmlForm(form): ValidatedHtmlForm<BasicsForm>,
109 109 ) -> Result<Response> {
110 110 user.check_not_suspended()?;
111 111 if !user.can_create_projects {
M server/src/routes/synckit/keys.rs +3 -1
@@ -124,7 +124,9 @@ pub(super) async fn list(
124 124 .ok_or(AppError::Unauthorized)?;
125 125
126 126 let limit = req.limit.unwrap_or(100).clamp(1, 1000) as i64;
127 - let offset = req.offset.unwrap_or(0) as i64;
127 + // Clamp the offset too (UX-M2): an unbounded offset becomes a giant SQL OFFSET
128 + // deep-scan, the same DoS-shaped cost the limit clamp guards against.
129 + let offset = (req.offset.unwrap_or(0) as i64).clamp(0, 1_000_000_000);
128 130
129 131 let rows = synckit_billing::list_active_keys(&state.db, app.id, limit, offset).await?;
130 132