fix(settings): don't require vision endpoint/model when OCR is active
Enabling the analysis worker ran the vision validators: the SSRF gate rejected the default `localhost` endpoint and a model id was required. With the OCR backend active those fields are never used, and the OCR-only settings UI no longer exposes them — so an operator could not turn the worker on (a 422 with no fixable field). Gate both checks on `base.backend == Vision`. The basic malformed-URL sanity check stays unconditional. Existing vision-validation tests now use a Vision base; a new test pins that enabling on the OCR backend with the default localhost endpoint and an empty model validates cleanly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -25,7 +25,7 @@ use serde::{Deserialize, Serialize};
|
||||
use crate::analysis::prompt::{
|
||||
GROUNDING_PROMPT_DEFAULT, OCR_PROMPT_DEFAULT, SYSTEM_PROMPT_DEFAULT,
|
||||
};
|
||||
use crate::config::{AnalysisConfig, CrawlerConfig, ResponseFormat};
|
||||
use crate::config::{AnalysisBackend, AnalysisConfig, CrawlerConfig, ResponseFormat};
|
||||
use crate::crawler::safety::DownloadAllowlist;
|
||||
|
||||
/// `app_settings.key` for the crawler group.
|
||||
@@ -355,6 +355,15 @@ impl AnalysisSettings {
|
||||
if self.workers < 1 {
|
||||
errs.push("workers", "must be at least 1");
|
||||
}
|
||||
// The endpoint/model are vision-only knobs — the OCR backend never
|
||||
// dials a URL or sends a model id. While vision is dormant
|
||||
// (`base.backend == Ocr`), skip the live-worker SSRF gate and the
|
||||
// model-required check so an OCR operator can enable the worker
|
||||
// without a vision endpoint/model (the OCR settings UI doesn't even
|
||||
// expose them). The basic malformed-URL sanity check below stays
|
||||
// unconditional. Re-enabling vision restores the full gate via the
|
||||
// `base.backend == Vision` predicate.
|
||||
let vision_active = base.backend == AnalysisBackend::Vision;
|
||||
let trimmed_endpoint = self.endpoint.trim();
|
||||
if trimmed_endpoint.is_empty() {
|
||||
errs.push("endpoint", "must be a valid absolute URL");
|
||||
@@ -362,7 +371,7 @@ impl AnalysisSettings {
|
||||
// Always reject obviously-malformed URLs (matches prior behaviour).
|
||||
let _ = e;
|
||||
errs.push("endpoint", "must be a valid absolute URL");
|
||||
} else if self.enabled {
|
||||
} else if self.enabled && vision_active {
|
||||
// SSRF + API-key exfiltration defence — only enforced when the
|
||||
// worker is actually live. Worker attaches an env-managed bearer
|
||||
// token to every call; without this check, an admin (or one
|
||||
@@ -379,7 +388,7 @@ impl AnalysisSettings {
|
||||
errs.push("endpoint", url_safety_message(&e));
|
||||
}
|
||||
}
|
||||
if self.enabled && self.model.trim().is_empty() {
|
||||
if self.enabled && vision_active && self.model.trim().is_empty() {
|
||||
errs.push("model", "required when analysis is enabled");
|
||||
}
|
||||
if self.max_tokens < 1 {
|
||||
@@ -701,7 +710,8 @@ mod tests {
|
||||
// a hostile/CSRF-able admin must NOT be able to point endpoint at
|
||||
// cloud metadata, loopback services, or RFC1918 hosts — when the
|
||||
// worker is enabled (toggling enabled=true later re-runs this gate).
|
||||
let base = AnalysisConfig::default();
|
||||
let mut base = AnalysisConfig::default();
|
||||
base.backend = AnalysisBackend::Vision;
|
||||
for url in [
|
||||
"http://169.254.169.254/v1/chat/completions",
|
||||
"http://127.0.0.1:5432/",
|
||||
@@ -725,7 +735,8 @@ mod tests {
|
||||
// The documented default — docker DNS name resolving to a private IP
|
||||
// at runtime — must still validate, because the bearer recipient
|
||||
// identity is the operator-chosen hostname, not the underlying IP.
|
||||
let base = AnalysisConfig::default();
|
||||
let mut base = AnalysisConfig::default();
|
||||
base.backend = AnalysisBackend::Vision;
|
||||
for url in [
|
||||
"http://mangalord-vision:8000/v1/chat/completions",
|
||||
"https://api.openai.com/v1/chat/completions",
|
||||
@@ -756,7 +767,10 @@ mod tests {
|
||||
|
||||
#[test]
|
||||
fn analysis_requires_model_only_when_enabled() {
|
||||
let base = AnalysisConfig::default();
|
||||
// Vision base: the model id is required only when the vision worker
|
||||
// is actually live (see the OCR carve-out below).
|
||||
let mut base = AnalysisConfig::default();
|
||||
base.backend = AnalysisBackend::Vision;
|
||||
let disabled = AnalysisSettings {
|
||||
enabled: false,
|
||||
model: "".to_string(),
|
||||
@@ -772,6 +786,28 @@ mod tests {
|
||||
assert!(errs.errors.iter().any(|e| e.field == "model"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn analysis_ocr_backend_enable_skips_vision_endpoint_and_model_validation() {
|
||||
// With the OCR backend active the worker never dials the vision
|
||||
// endpoint nor sends a model id, so enabling analysis must NOT
|
||||
// validate those vision-only fields. The default base carries the
|
||||
// dev-localhost endpoint (which the SSRF gate would otherwise reject)
|
||||
// and an empty model — under OCR, enabling is still valid. Without
|
||||
// this carve-out an OCR operator can't turn the worker on, since the
|
||||
// OCR settings UI doesn't expose endpoint/model to fix.
|
||||
let base = AnalysisConfig::default(); // backend = Ocr
|
||||
let dto = AnalysisSettings {
|
||||
enabled: true,
|
||||
endpoint: "http://localhost:8000/v1/chat/completions".to_string(),
|
||||
model: "".to_string(),
|
||||
..AnalysisSettings::from_config(&base)
|
||||
};
|
||||
assert!(
|
||||
dto.to_config(&base).is_ok(),
|
||||
"OCR-enabled config must not fail on vision endpoint/model"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dtos_serialize_to_json_and_back() {
|
||||
let c = CrawlerSettings::default();
|
||||
|
||||
Reference in New Issue
Block a user