Skip to main content

max / makenotwork

ux: drop dead price validator, control-char parity on labels, bound discover price filters - Delete the dead (and its tests): the live path is the newtype, so the free function was a second, unreferenced source of truth. - Reject control characters in activation and SSH-key labels, matching display_name/bio (a label with embedded newlines/escapes corrupts logs and admin views). - Sanitize discover min/max price filters: drop negatives and inverted (min > max) ranges instead of issuing an empty-by-construction query. ultra-fuzz Run 5 UX cold spots (deep). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-06-24 02:47 UTC
Commit: 3de6ec0303ceb114fe74d426912c54fd6e2b58b3
Parent: ed25e44
3 files changed, +59 insertions, -44 deletions
M server/src/routes/pages/public/discover.rs +33 -2
@@ -68,6 +68,36 @@ struct DiscoverData {
68 68 showing_end: u32,
69 69 }
70 70
71 + /// Clamp the discover price filters to a sane range. A negative bound is
72 + /// meaningless (prices are non-negative cents) and is dropped; an inverted range
73 + /// (min > max) can only match nothing, so both bounds are dropped rather than
74 + /// issuing an empty-by-construction query.
75 + fn sanitize_price_range(min: Option<i32>, max: Option<i32>) -> (Option<i32>, Option<i32>) {
76 + let min = min.filter(|&v| v >= 0);
77 + let max = max.filter(|&v| v >= 0);
78 + if let (Some(lo), Some(hi)) = (min, max)
79 + && lo > hi
80 + {
81 + return (None, None);
82 + }
83 + (min, max)
84 + }
85 +
86 + #[cfg(test)]
87 + mod price_range_tests {
88 + use super::sanitize_price_range;
89 +
90 + #[test]
91 + fn drops_negatives_and_inverted_ranges() {
92 + assert_eq!(sanitize_price_range(Some(100), Some(500)), (Some(100), Some(500)));
93 + assert_eq!(sanitize_price_range(Some(-1), Some(500)), (None, Some(500)));
94 + assert_eq!(sanitize_price_range(Some(100), Some(-5)), (Some(100), None));
95 + // inverted range can only match nothing -> drop both
96 + assert_eq!(sanitize_price_range(Some(500), Some(100)), (None, None));
97 + assert_eq!(sanitize_price_range(None, None), (None, None));
98 + }
99 + }
100 +
71 101 /// Fetch items or projects with pagination; shared by both handlers.
72 102 async fn fetch_discover_data(pool: &PgPool, query: &DiscoverQuery) -> Result<DiscoverData> {
73 103 // Clamp the upper bound too (UX MINOR, Run #23): an unbounded page yields a
@@ -123,12 +153,13 @@ async fn fetch_discover_data(pool: &PgPool, query: &DiscoverQuery) -> Result<Dis
123 153 .filter(|s| !s.is_empty())
124 154 .and_then(|s| s.parse().ok());
125 155
156 + let (min_price, max_price) = sanitize_price_range(query.min_price, query.max_price);
126 157 let filters = DiscoverFilters {
127 158 search: search_filter,
128 159 item_type: item_type_filter,
129 160 tag: tag_filter,
130 - min_price: query.min_price,
131 - max_price: query.max_price,
161 + min_price,
162 + max_price,
132 163 sort_by: sort_filter,
133 164 ai_tier: ai_tier_filter,
134 165 };
M server/src/validation/items.rs +3 -42
@@ -1,6 +1,5 @@
1 1 //! Validators for items, chapters, tags, blog posts, collections, and related content.
2 2
3 - use crate::constants;
4 3 use crate::error::AppError;
5 4 use super::limits;
6 5
@@ -242,17 +241,9 @@ pub fn validate_section_body(body: &str) -> Result<(), AppError> {
242 241 Ok(())
243 242 }
244 243
245 - /// Validate price in cents (must be non-negative)
246 - pub fn validate_price_cents(price: i32) -> Result<(), AppError> {
247 - if price < 0 {
248 - return Err(AppError::validation("Price cannot be negative".to_string()));
249 - }
250 - // Cap at $10,000
251 - if price > constants::MAX_PRICE_CENTS {
252 - return Err(AppError::validation("Price cannot exceed $10,000".to_string()));
253 - }
254 - Ok(())
255 - }
244 + // Price validation lives on the `PriceCents` newtype (`db::validated_types`),
245 + // which every live write path constructs — the former free-standing
246 + // `validate_price_cents` here was a dead second source of truth and was removed.
256 247
257 248 #[cfg(test)]
258 249 mod tests {
@@ -364,15 +355,6 @@ mod tests {
364 355 }
365 356
366 357 #[test]
367 - fn test_validate_price_cents() {
368 - assert!(validate_price_cents(0).is_ok());
369 - assert!(validate_price_cents(999).is_ok());
370 - assert!(validate_price_cents(1_000_000).is_ok()); // $10,000
371 - assert!(validate_price_cents(-1).is_err()); // negative
372 - assert!(validate_price_cents(1_000_001).is_err()); // over cap
373 - }
374 -
375 - #[test]
376 358 fn test_validate_section_title() {
377 359 assert!(validate_section_title("Features").is_ok());
378 360 assert!(validate_section_title(" Features ").is_ok()); // trimmed
@@ -443,18 +425,6 @@ mod tests {
443 425 }
444 426
445 427 #[test]
446 - fn test_validate_price_cents_exact_max() {
447 - assert!(validate_price_cents(constants::MAX_PRICE_CENTS).is_ok());
448 - assert!(validate_price_cents(constants::MAX_PRICE_CENTS + 1).is_err());
449 - }
450 -
451 - #[test]
452 - fn test_validate_price_cents_i32_extremes() {
453 - assert!(validate_price_cents(i32::MIN).is_err());
454 - assert!(validate_price_cents(i32::MAX).is_err());
455 - }
456 -
457 - #[test]
458 428 fn test_validate_section_title_only_whitespace_padded() {
459 429 // Leading/trailing whitespace with content in the middle should pass
460 430 assert!(validate_section_title(" Features ").is_ok());
@@ -506,14 +476,5 @@ mod tests {
506 476 }
507 477 }
508 478
509 - #[test]
510 - fn prop_price_cents_valid_range(price in 0..=1_000_000i32) {
511 - proptest::prop_assert!(validate_price_cents(price).is_ok(), "Valid price rejected: {}", price);
512 - }
513 -
514 - #[test]
515 - fn prop_price_cents_negative_always_rejected(price in i32::MIN..0i32) {
516 - proptest::prop_assert!(validate_price_cents(price).is_err(), "Negative price accepted: {}", price);
517 - }
518 479 }
519 480 }
M server/src/validation/users.rs +23
@@ -130,6 +130,13 @@ pub fn validate_activation_label(label: &str) -> Result<(), AppError> {
130 130 limits::ACTIVATION_LABEL_MAX
131 131 )));
132 132 }
133 + // Reject control characters (parity with display_name/bio) — a label with
134 + // embedded newlines/escapes would corrupt logs and admin views.
135 + if label.chars().any(|c| c.is_control()) {
136 + return Err(AppError::validation(
137 + "Label cannot contain control characters".to_string(),
138 + ));
139 + }
133 140 Ok(())
134 141 }
135 142
@@ -211,6 +218,12 @@ pub fn validate_ssh_key_label(label: &str) -> std::result::Result<(), AppError>
211 218 limits::SSH_KEY_LABEL_MAX
212 219 )));
213 220 }
221 + // Reject control characters (parity with display_name/bio).
222 + if label.chars().any(|c| c.is_control()) {
223 + return Err(AppError::validation(
224 + "SSH key label cannot contain control characters".to_string(),
225 + ));
226 + }
214 227 Ok(())
215 228 }
216 229
@@ -237,6 +250,16 @@ mod tests {
237 250 }
238 251
239 252 #[test]
253 + fn test_activation_and_ssh_labels_reject_control_chars() {
254 + assert!(validate_activation_label("Living Room").is_ok());
255 + assert!(validate_activation_label("Living\nRoom").is_err());
256 + assert!(validate_activation_label("tab\there").is_err());
257 + assert!(validate_ssh_key_label("laptop").is_ok());
258 + assert!(validate_ssh_key_label("laptop\0").is_err());
259 + assert!(validate_ssh_key_label("esc\u{1b}ape").is_err());
260 + }
261 +
262 + #[test]
240 263 fn test_validate_bio() {
241 264 assert!(validate_bio("I make music").is_ok());
242 265 assert!(validate_bio("").is_ok());