fabi/Mangalord
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(config): ANALYSIS_VISION_MODEL reader + compose RHS-interpolation test (0.87.17)

Browse Source 
Two ship-blockers from the rereview:

1. Env var name mismatch: 0.87.12 renamed every operator surface to
   ANALYSIS_VISION_MODEL but the reader still called env::var on
   ANALYSIS_MODEL — every documented deploy ran analysis with an
   empty model. Rename the reader.

2. Compose regression test was substring-only and missed exactly the
   ANALYSIS_VISION_MODEL vs ANALYSIS_MODEL RHS mismatch from 0.87.12.
   Tightened to require each backend-consumed var either be in a
   small hardcoded allowlist (BIND_ADDRESS, STORAGE_DIR) or have its
   RHS interpolate `${KEY...}` with the same name. Mutation-confirmed.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-23 20:05:17 +02:00
parent 54cbbfc440
commit e3d16e49b7
4 changed files with 129 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
backend/Cargo.lock generated
View File

@@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4"
[[package]]
name = "mangalord"
version = "0.87.16"
version = "0.87.17"
dependencies = [
"anyhow",
"argon2",

2
backend/Cargo.toml
View File

@@ -1,6 +1,6 @@
[package]
name = "mangalord"
version = "0.87.16"
version = "0.87.17"
edition = "2021"
default-run = "mangalord"

153
backend/src/config.rs
View File

@@ -131,7 +131,7 @@ pub struct AnalysisConfig {
/// land a `failed` row. `None` (the default) disables the gate, matching
/// the prior behavior for always-on endpoints. Env-only, like `api_key`.
pub vision_health_url: Option<String>,
/// Model id to request (`ANALYSIS_MODEL`).
/// Model id to request (`ANALYSIS_VISION_MODEL`).
pub model: String,
/// Optional bearer token (`ANALYSIS_API_KEY`); local servers usually
/// don't need one.
@@ -232,7 +232,13 @@ impl AnalysisConfig {
vision_health_url: std::env::var("ANALYSIS_VISION_HEALTH_URL")
.ok()
.filter(|s| !s.is_empty()),
model: std::env::var("ANALYSIS_MODEL").unwrap_or(d.model),
// Renamed from `ANALYSIS_MODEL` in 0.87.17 to match the
// `ANALYSIS_VISION_*` naming the docs, compose, and 0.87.12's
// regression test had already standardised on. Pre-0.87.17
// deploys reading the old `ANALYSIS_MODEL` will fall through
// to the default — explicitly noted so an operator hitting
// an empty `model` after upgrade can grep this file.
model: std::env::var("ANALYSIS_VISION_MODEL").unwrap_or(d.model),
api_key: std::env::var("ANALYSIS_API_KEY")
.ok()
.filter(|s| !s.is_empty()),
@@ -718,7 +724,7 @@ mod tests {
"ANALYSIS_ENABLED",
"ANALYSIS_WORKERS",
"ANALYSIS_VISION_URL",
"ANALYSIS_MODEL",
"ANALYSIS_VISION_MODEL",
"ANALYSIS_API_KEY",
"ANALYSIS_MAX_TOKENS",
"ANALYSIS_MAX_PIXELS",
@@ -765,7 +771,7 @@ mod tests {
std::env::set_var("ANALYSIS_ENABLED", "true");
std::env::set_var("ANALYSIS_WORKERS", "4");
std::env::set_var("ANALYSIS_VISION_URL", "http://vis/v1/chat");
std::env::set_var("ANALYSIS_MODEL", "qwen2-vl");
std::env::set_var("ANALYSIS_VISION_MODEL", "qwen2-vl");
std::env::set_var("ANALYSIS_MAX_PIXELS", "768000");
std::env::set_var("ANALYSIS_MAX_SLICES", "8");
std::env::set_var("ANALYSIS_SLICE_OVERLAP", "0.2");
@@ -774,7 +780,7 @@ mod tests {
"ANALYSIS_ENABLED",
"ANALYSIS_WORKERS",
"ANALYSIS_VISION_URL",
"ANALYSIS_MODEL",
"ANALYSIS_VISION_MODEL",
"ANALYSIS_MAX_PIXELS",
"ANALYSIS_MAX_SLICES",
"ANALYSIS_SLICE_OVERLAP",
@@ -823,19 +829,24 @@ mod tests {
assert!(!cfg.auth.private_mode);
}
/// Regression test for the 0.87.1 compose env-wire-through gap. The
/// docker-compose backend service used to omit `environment:` entries
/// for `ADMIN_*`, `PRIVATE_MODE`, `ALLOW_SELF_REGISTER`, `CRAWLER_*`,
/// and `ANALYSIS_*`. Operators populating `.env` got an
/// anonymous-serving site, no admin bootstrap, and missing analysis
/// configuration because Docker Compose's `.env` interpolation only
/// expands inside this file — it does NOT pass vars into the
/// container's process env unless `environment:` (or `env_file:`)
/// names them.
/// Regression test for the 0.87.1 compose env-wire-through gap, with
/// the 0.87.17 strengthening from the rereview:
///
/// This test pins the wiring: every env var the backend reader
/// consumes must have a matching `KEY:` entry in the backend
/// service's `environment:` block.
/// The 0.87.12 version only substring-matched the env-var KEY in the
/// compose YAML — blind to whether the RHS actually substituted from
/// `.env`. That made the test pass for `KEY: hardcoded` lines and
/// for `KEY: ${WRONG_NAME:-default}` lines, which is exactly how the
/// `ANALYSIS_VISION_MODEL` vs `ANALYSIS_MODEL` mismatch slipped
/// through the prior commit. Now also asserts that each operator-
/// facing var's RHS contains a `${KEY` interpolation of the SAME
/// name (allowing the `${KEY:-default}` and `${KEY-default}` forms),
/// with an explicit allow-list for vars that are intentionally
/// hardcoded under `BACKEND_HARDCODED_OK`.
///
/// Every var listed in `BACKEND_CONSUMED` must:
/// * have a `KEY:` line under the backend service `environment:`, AND
/// * either be in `BACKEND_HARDCODED_OK` (intentional literal), OR
/// have its RHS substitute from `${KEY` so `.env` actually wins.
#[test]
fn docker_compose_wires_every_documented_backend_env_var() {
// `.env.example` lines that ARE consumed by the backend. Lines
@@ -883,25 +894,47 @@ mod tests {
"ANALYSIS_VISION_HEALTH_URL",
];
// Keys whose compose RHS is intentionally a literal (not `${KEY..}`).
// Each entry should be justified:
// - BIND_ADDRESS / STORAGE_DIR: container-internal binding/mount,
// overriding from the host would either un-net the container or
// break the volume — keep them pinned.
const BACKEND_HARDCODED_OK: &[&str] = &["BIND_ADDRESS", "STORAGE_DIR"];
// Cargo run dir for `cargo test` is `backend/`; compose file lives one up.
let compose = std::fs::read_to_string("../docker-compose.yml")
.expect("read ../docker-compose.yml");
// Extract the backend service block: everything from ` backend:`
// up to (but not including) the next top-level service key.
let backend_block = compose
.split("\n backend:\n")
.nth(1)
.expect("backend service block")
.split("\n frontend:\n")
.next()
.expect("end of backend block (next service)");
// up to the next top-level service key (` <name>:` at 2-space
// indent, no further indent). Line-based scan instead of the
// 0.87.12 `\n frontend:\n` literal so adding/reordering services
// doesn't silently break the slicer.
let backend_block = extract_compose_service_block(&compose, "backend")
.expect("backend service block in docker-compose.yml");
let mut missing: Vec<&str> = Vec::new();
let mut not_interpolated: Vec<(String, String)> = Vec::new();
for key in BACKEND_CONSUMED {
// Match ` KEY:` indent (6 spaces — under `environment:`).
// The env entries sit at exactly 6 spaces under `environment:`.
let needle = format!(" {key}:");
if !backend_block.contains(&needle) {
let Some(line_start) = backend_block.find(&needle) else {
missing.push(*key);
continue;
};
if BACKEND_HARDCODED_OK.contains(key) {
continue;
}
// RHS: everything from the colon to the end of line.
let from_colon = &backend_block[line_start + needle.len()..];
let rhs_end = from_colon.find('\n').unwrap_or(from_colon.len());
let rhs = from_colon[..rhs_end].trim();
// Expand-from-env requires `${KEY...}` where KEY matches this
// var's name. Accept both `${KEY:-default}` and `${KEY-default}`
// (and the bare `${KEY}` form for completeness).
let expected_prefixes =
[format!("${{{key}:-"), format!("${{{key}-"), format!("${{{key}}}")];
if !expected_prefixes.iter().any(|p| rhs.starts_with(p.as_str())) {
not_interpolated.push(((*key).to_string(), rhs.to_string()));
}
}
assert!(
@@ -910,6 +943,72 @@ mod tests {
Add `KEY: ${{KEY:-default}}` lines under backend.environment so the var \
reaches the container."
);
assert!(
not_interpolated.is_empty(),
"docker-compose.yml backend service has env entries whose RHS does NOT \
substitute from the matching `${{KEY...}}` placeholder, so an operator \
setting `KEY=…` in `.env` will be silently ignored. Either fix the RHS \
to `${{KEY:-default}}` or add the var to `BACKEND_HARDCODED_OK` with a \
rationale. Offenders: {not_interpolated:?}"
);
}
/// Extract a single service's block from a docker-compose YAML.
/// Reads from the line `^ <name>:$` (2-space top-level service
/// indent) up to the next top-level service header (`^ [A-Za-z…]:$`)
/// or end of file. Returns `None` if the service isn't found.
///
/// Line-based scan instead of `splitn("\n next:\n")` so a service
/// added between the target and the literal anchor doesn't silently
/// truncate the block. Tolerates blank lines and comments inside the
/// block (only the precise 2-space service-header shape ends it).
fn extract_compose_service_block<'a>(compose: &'a str, name: &str) -> Option<&'a str> {
let header = format!("\n {name}:\n");
let start = compose.find(&header).map(|i| i + 1)?; // include the header line
let body = &compose[start + header.len() - 1..]; // after the trailing \n
// Find the next 2-space top-level service-key line. The "next"
// candidate must be: a newline, then exactly two spaces, then a
// word char (services have alphanumeric names), then anything up
// to `:` followed by EOL. We scan line-by-line.
let mut offset: usize = 0;
for line in body.split('\n') {
// Detect a top-level service header at 2-space indent. Skip
// the first line iteration since `start` already points at
// our own header.
if offset > 0
&& line.starts_with(" ")
&& !line.starts_with(" ")
&& line.trim_end().ends_with(':')
&& line.trim_start().chars().next().is_some_and(|c| c.is_ascii_alphabetic())
{
return Some(&body[..offset.saturating_sub(1)]);
}
offset += line.len() + 1; // +1 for the newline
}
Some(body)
}
/// Sanity-check the slicer against a hand-built compose so a future
/// edit to the helper doesn't silently misframe the wiring test.
#[test]
fn extract_compose_service_block_finds_named_block() {
let yaml = "\
services:
postgres:
image: pg
backend:
environment:
KEY: ${KEY:-default}
expose:
- \"8080\"
frontend:
image: f
";
let backend = extract_compose_service_block(yaml, "backend").expect("found");
assert!(backend.contains("KEY: ${KEY:-default}"));
// Must NOT bleed into the next service.
assert!(!backend.contains("frontend"));
assert!(!backend.contains("image: f"));
}
}