diff --git a/backend/Cargo.lock b/backend/Cargo.lock index 53538e4..7891cd5 100644 --- a/backend/Cargo.lock +++ b/backend/Cargo.lock @@ -1517,7 +1517,7 @@ checksum = "c41e0c4fef86961ac6d6f8a82609f55f31b05e4fce149ac5710e439df7619ba4" [[package]] name = "mangalord" -version = "0.87.16" +version = "0.87.17" dependencies = [ "anyhow", "argon2", diff --git a/backend/Cargo.toml b/backend/Cargo.toml index 5852227..4a9cc54 100644 --- a/backend/Cargo.toml +++ b/backend/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mangalord" -version = "0.87.16" +version = "0.87.17" edition = "2021" default-run = "mangalord" diff --git a/backend/src/config.rs b/backend/src/config.rs index 9815ff6..90d04b1 100644 --- a/backend/src/config.rs +++ b/backend/src/config.rs @@ -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, - /// 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 (` :` 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 `^ :$` (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")); } } diff --git a/frontend/package.json b/frontend/package.json index de1c74d..110db96 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,6 +1,6 @@ { "name": "mangalord-frontend", - "version": "0.87.16", + "version": "0.87.17", "private": true, "type": "module", "scripts": {