fix(settings): bound manga_limit + the timeout knobs the finding named
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
2
backend/Cargo.lock
generated
2
backend/Cargo.lock
generated
@@ -1558,7 +1558,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
|
||||
|
||||
[[package]]
|
||||
name = "mangalord"
|
||||
version = "0.90.3"
|
||||
version = "0.90.4"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"argon2",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
[package]
|
||||
name = "mangalord"
|
||||
version = "0.90.3"
|
||||
version = "0.90.4"
|
||||
edition = "2021"
|
||||
default-run = "mangalord"
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user