From 886caaecfa27f84589eff9d7bbc2b4dbf89389b3 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Wed, 1 Jul 2026 07:17:14 +0200 Subject: [PATCH] fix(settings): bound manga_limit + the timeout knobs the finding named Co-Authored-By: Claude Opus 4.8 --- backend/Cargo.lock | 2 +- backend/Cargo.toml | 2 +- backend/src/settings.rs | 164 +++++++++++++++++++++++++++++++++++++++- frontend/package.json | 2 +- 4 files changed, 165 insertions(+), 5 deletions(-) diff --git a/backend/Cargo.lock b/backend/Cargo.lock index a634053..8d202e5 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.90.3" +version = "0.90.4" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 32c27a4..30cbe98 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.90.3" +version = "0.90.4" edition = "2021" default-run = "mangalord" diff --git a/backend/src/settings.rs b/backend/src/settings.rs index 417a41c..29c13c4 100644 --- a/backend/src/settings.rs +++ b/backend/src/settings.rs @@ -33,6 +33,28 @@ pub const KEY_CRAWLER: &str = "crawler"; /// `app_settings.key` for the analysis group. pub const KEY_ANALYSIS: &str = "analysis"; +// Upper bounds on numeric settings. These are sanity caps, not tuned limits — +// they keep a fat-fingered (or CSRF-injected) value from spawning thousands of +// workers, demanding gigabyte buffers, or sending out-of-range sampling +// params. Generous enough that no realistic deployment hits them. +const MAX_WORKERS: u64 = 64; +const MAX_CHAPTER_WORKERS: u64 = 64; +const MAX_ANALYSIS_MAX_TOKENS: u32 = 1_000_000; +const MAX_ANALYSIS_SLICES: u64 = 1024; +/// 1 GiB — far above any real page, well below "exhaust the host". +const MAX_ANALYSIS_IMAGE_BYTES: u64 = 1024 * 1024 * 1024; +/// OpenAI-compatible sampling ranges (the analysis endpoint speaks that API). +const MAX_TEMPERATURE: f64 = 2.0; +const FREQUENCY_PENALTY_MIN: f64 = -2.0; +const FREQUENCY_PENALTY_MAX: f64 = 2.0; +/// Max manga-detail fetches per metadata pass. `0` stays special-cased as +/// "unlimited"; this only bounds an explicit positive value. +const MAX_MANGA_LIMIT: u64 = 1_000_000; +/// Upper bound (seconds) shared by every timeout knob — 24h. A timeout +/// longer than a day is almost certainly a fat-fingered value (e.g. ms +/// mistaken for s) and would wedge a worker for far too long. +const MAX_TIMEOUT_SECS: u64 = 86_400; + /// One field-level validation failure, surfaced to the UI per input. #[derive(Debug, Clone, Serialize, PartialEq, Eq)] pub struct FieldError { @@ -151,6 +173,8 @@ impl CrawlerSettings { }; if self.chapter_workers < 1 { errs.push("chapter_workers", "must be at least 1"); + } else if self.chapter_workers > MAX_CHAPTER_WORKERS { + errs.push("chapter_workers", format!("must be at most {MAX_CHAPTER_WORKERS}")); } if let Some(url) = self.start_url.as_deref().map(str::trim).filter(|s| !s.is_empty()) { // SSRF defence: Url::parse alone admits http://169.254.169.254 @@ -164,6 +188,16 @@ impl CrawlerSettings { } if self.job_timeout_secs < 1 { errs.push("job_timeout_secs", "must be at least 1 second"); + } else if self.job_timeout_secs > MAX_TIMEOUT_SECS { + errs.push("job_timeout_secs", format!("must be at most {MAX_TIMEOUT_SECS} seconds")); + } + if self.idle_timeout_secs > MAX_TIMEOUT_SECS { + errs.push("idle_timeout_secs", format!("must be at most {MAX_TIMEOUT_SECS} seconds")); + } + // 0 is intentionally "unlimited"; only an explicit positive value is + // capped. + if self.manga_limit > MAX_MANGA_LIMIT { + errs.push("manga_limit", format!("must be at most {MAX_MANGA_LIMIT}")); } if !errs.is_empty() { @@ -354,6 +388,8 @@ impl AnalysisSettings { if self.workers < 1 { errs.push("workers", "must be at least 1"); + } else if self.workers > MAX_WORKERS { + errs.push("workers", format!("must be at most {MAX_WORKERS}")); } // The endpoint/model are vision-only knobs — the OCR backend never // dials a URL or sends a model id. While vision is dormant @@ -393,18 +429,35 @@ impl AnalysisSettings { } if self.max_tokens < 1 { errs.push("max_tokens", "must be at least 1"); + } else if self.max_tokens > MAX_ANALYSIS_MAX_TOKENS { + errs.push("max_tokens", format!("must be at most {MAX_ANALYSIS_MAX_TOKENS}")); } if self.request_timeout_secs < 1 { errs.push("request_timeout_secs", "must be at least 1 second"); + } else if self.request_timeout_secs > MAX_TIMEOUT_SECS { + errs.push( + "request_timeout_secs", + format!("must be at most {MAX_TIMEOUT_SECS} seconds"), + ); } if self.job_timeout_secs < 1 { errs.push("job_timeout_secs", "must be at least 1 second"); + } else if self.job_timeout_secs > MAX_TIMEOUT_SECS { + errs.push( + "job_timeout_secs", + format!("must be at most {MAX_TIMEOUT_SECS} seconds"), + ); } if self.max_pixels < 1 { errs.push("max_pixels", "must be at least 1"); } if self.max_image_bytes < 1 { errs.push("max_image_bytes", "must be greater than 0"); + } else if self.max_image_bytes > MAX_ANALYSIS_IMAGE_BYTES { + errs.push( + "max_image_bytes", + format!("must be at most {MAX_ANALYSIS_IMAGE_BYTES} (1 GiB)"), + ); } if !(0.0..=0.9).contains(&self.slice_overlap) { errs.push("slice_overlap", "must be between 0.0 and 0.9"); @@ -417,9 +470,17 @@ impl AnalysisSettings { } if self.max_slices < 1 { errs.push("max_slices", "must be at least 1"); + } else if self.max_slices > MAX_ANALYSIS_SLICES { + errs.push("max_slices", format!("must be at most {MAX_ANALYSIS_SLICES}")); } - if self.temperature < 0.0 { - errs.push("temperature", "must be 0 or greater"); + if !(0.0..=MAX_TEMPERATURE).contains(&self.temperature) { + errs.push("temperature", format!("must be between 0 and {MAX_TEMPERATURE}")); + } + if !(FREQUENCY_PENALTY_MIN..=FREQUENCY_PENALTY_MAX).contains(&self.frequency_penalty) { + errs.push( + "frequency_penalty", + format!("must be between {FREQUENCY_PENALTY_MIN} and {FREQUENCY_PENALTY_MAX}"), + ); } let response_format = match ResponseFormat::parse_strict(&self.response_format) { Some(rf) => rf, @@ -706,6 +767,105 @@ mod tests { } } + #[test] + fn analysis_rejects_over_upper_bounds() { + // Sanity caps: an absurdly large worker count / buffer / token budget + // and out-of-range sampling params are all refused so a fat-fingered + // or CSRF-injected value can't exhaust the host or break the upstream + // API contract. + let base = AnalysisConfig::default(); + let dto = AnalysisSettings { + workers: MAX_WORKERS + 1, + max_tokens: MAX_ANALYSIS_MAX_TOKENS + 1, + max_slices: MAX_ANALYSIS_SLICES + 1, + max_image_bytes: MAX_ANALYSIS_IMAGE_BYTES + 1, + temperature: MAX_TEMPERATURE + 0.5, + frequency_penalty: FREQUENCY_PENALTY_MAX + 0.5, + request_timeout_secs: MAX_TIMEOUT_SECS + 1, + job_timeout_secs: MAX_TIMEOUT_SECS + 1, + ..AnalysisSettings::from_config(&base) + }; + let errs = dto.to_config(&base).unwrap_err(); + let fields: Vec<_> = errs.errors.iter().map(|e| e.field.as_str()).collect(); + for f in [ + "workers", + "max_tokens", + "max_slices", + "max_image_bytes", + "temperature", + "frequency_penalty", + "request_timeout_secs", + "job_timeout_secs", + ] { + assert!(fields.contains(&f), "missing upper-bound error for {f}"); + } + } + + #[test] + fn crawler_rejects_over_upper_bounds() { + // manga_limit ceiling + timeout caps. manga_limit=0 stays "unlimited" + // and is asserted valid by the round-trip tests above. + let base = CrawlerConfig::default(); + let dto = CrawlerSettings { + manga_limit: MAX_MANGA_LIMIT + 1, + job_timeout_secs: MAX_TIMEOUT_SECS + 1, + idle_timeout_secs: MAX_TIMEOUT_SECS + 1, + ..CrawlerSettings::from_config(&base) + }; + let errs = dto.to_config(&base).unwrap_err(); + let fields: Vec<_> = errs.errors.iter().map(|e| e.field.as_str()).collect(); + for f in ["manga_limit", "job_timeout_secs", "idle_timeout_secs"] { + assert!(fields.contains(&f), "missing upper-bound error for {f}"); + } + } + + #[test] + fn crawler_allows_unlimited_manga_limit() { + // 0 means "no cap" and must stay valid. + let base = CrawlerConfig::default(); + let dto = CrawlerSettings { + manga_limit: 0, + ..CrawlerSettings::from_config(&base) + }; + assert!(dto.to_config(&base).is_ok()); + } + + #[test] + fn analysis_rejects_negative_frequency_penalty_below_range() { + let base = AnalysisConfig::default(); + let dto = AnalysisSettings { + frequency_penalty: FREQUENCY_PENALTY_MIN - 0.5, + ..AnalysisSettings::from_config(&base) + }; + let errs = dto.to_config(&base).unwrap_err(); + let fields: Vec<_> = errs.errors.iter().map(|e| e.field.as_str()).collect(); + assert!(fields.contains(&"frequency_penalty")); + } + + #[test] + fn analysis_accepts_in_range_sampling_params() { + // A normal config with mid-range sampling values stays valid. + let base = AnalysisConfig::default(); + let dto = AnalysisSettings { + temperature: 0.7, + frequency_penalty: 0.3, + ..AnalysisSettings::from_config(&base) + }; + assert!(dto.to_config(&base).is_ok()); + } + + #[test] + fn crawler_rejects_over_chapter_worker_cap() { + let base = CrawlerConfig::default(); + let dto = CrawlerSettings { + chapter_workers: MAX_CHAPTER_WORKERS + 1, + ..CrawlerSettings::from_config(&base) + }; + let errs = dto.to_config(&base).unwrap_err(); + let fields: Vec<_> = errs.errors.iter().map(|e| e.field.as_str()).collect(); + assert!(fields.contains(&"chapter_workers")); + } + #[test] fn analysis_endpoint_rejects_ip_literal_attacks_when_enabled() { // The vision worker bearer-attaches an env secret to every call; diff --git a/frontend/package.json b/frontend/package.json index 5fa7fce..dd8f9a0 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.90.3", + "version": "0.90.4", "private": true, "type": "module", "scripts": {