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

fix(config): derive BACKEND_CONSUMED from .env.example; scan src/ for undocumented env reads (0.87.25)

Browse Source 
0.87.12 added a regression test for backend env wiring, but its
BACKEND_CONSUMED list was hand-maintained and drifted: ~22 vars read
by backend/src/ never made it into compose's backend.environment.
An operator setting ANALYSIS_TEMPERATURE=0.3 in .env got a silent
no-op — exactly the bug class the test was supposed to catch.

Make .env.example the single source of truth:

  * docker_compose_wires_every_documented_backend_env_var now reads
    .env.example at test time, filters NOT_BACKEND_CONSUMED_IN_ENV_EXAMPLE
    (compose-side composed vars, frontend-only vars, sidecar vars),
    and requires every remaining key to be wired into compose with a
    matching ${KEY...} interpolation OR allowlisted via COMPOSE_RHS_EXEMPT.
  * every_backend_src_env_read_is_documented_or_allowlisted scans
    backend/src/ for env::var / env_bool / env_i64 / ... reads and
    requires each key to be in .env.example OR in INTERNAL_NOT_CONFIGURABLE
    with a per-entry rationale. Catches the silent-no-op bug from the
    other side.

Wire the previously-missing env vars into docker-compose.yml's
backend.environment (CRAWLER_DAEMON, CRAWLER_DAILY_AT, CRAWLER_TZ,
CRAWLER_IDLE_TIMEOUT_S, CRAWLER_CHAPTER_WORKERS, CRAWLER_JOB_RETENTION_DAYS,
CRAWL_METRICS_RETENTION_DAYS, CRAWLER_RATE_MS, CRAWLER_CDN_RATE_MS,
CRAWLER_USER_AGENT, CRAWLER_PHPSESSID, CRAWLER_COOKIE_DOMAIN,
CRAWLER_TOR_CONTROL_COOKIE_PATH, all ANALYSIS_* tuning knobs).
Document each new var in .env.example with the same default value
shown in compose's `${KEY:-default}` form. Allowlist internal/system
vars (HOME = chromium cache dir fallback, CRAWLER_BROWSER_*,
CRAWLER_SKIP_*, the multi-paragraph ANALYSIS_*_PROMPT seeds) with
per-entry rationale.

Mutation-tested both sides: adding `env::var("MANGALORD_FAKE_NEW_KNOB")`
to main.rs fails the scanner; removing the new compose wiring fails
the documented-var test with all 23 missing keys named.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-24 19:28:00 +02:00
parent 579e6ade0f
commit ae708a72d0
6 changed files with 381 additions and 74 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.24"
version = "0.87.25"
dependencies = [
"anyhow",
"argon2",

2
backend/Cargo.toml
View File

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

341
backend/src/config.rs
View File

@@ -829,79 +829,129 @@ mod tests {
assert!(!cfg.auth.private_mode);
}
/// Vars that live in `.env.example` for operator context but that
/// are NOT consumed by the backend service (so don't need to be in
/// backend.environment). Each entry needs a rationale — if a future
/// var is added to `.env.example` without being wired into compose,
/// the test fails until either the wiring is added or the var is
/// listed here with a justification.
///
/// - POSTGRES_DB/USER/PASSWORD — composed into DATABASE_URL on the
/// compose RHS; the backend container never sees them by name.
/// - TOR_CONTROL_PASSWORD — wired through to BOTH tor and backend
/// under different names (PASSWORD / CRAWLER_TOR_CONTROL_PASSWORD).
/// - BACKEND_URL / BACKEND_PROXY_TIMEOUT_MS — consumed by the
/// SvelteKit frontend container, not the backend.
/// - VISION_MANAGER_DATABASE_URL — consumed by the vision-manager
/// sidecar, not the backend.
const NOT_BACKEND_CONSUMED_IN_ENV_EXAMPLE: &[&str] = &[
"POSTGRES_DB",
"POSTGRES_USER",
"POSTGRES_PASSWORD",
"TOR_CONTROL_PASSWORD",
"BACKEND_URL",
"BACKEND_PROXY_TIMEOUT_MS",
"VISION_MANAGER_DATABASE_URL",
];
/// Keys whose compose RHS is intentionally NOT a `${KEY...}`
/// interpolation. Each entry needs a justification:
///
/// - BIND_ADDRESS / STORAGE_DIR — container-internal binding /
/// mount target; overriding from the host would un-net the
/// container or break the volume.
/// - DATABASE_URL — composed in compose from POSTGRES_USER /
/// PASSWORD / DB; the backend container never sees the raw
/// `${DATABASE_URL...}` placeholder.
/// - CRAWLER_TOR_CONTROL_PASSWORD — wired through TOR_CONTROL_PASSWORD
/// (single source of truth for the tor service's own PASSWORD).
const COMPOSE_RHS_EXEMPT: &[&str] = &[
"BIND_ADDRESS",
"STORAGE_DIR",
"DATABASE_URL",
"CRAWLER_TOR_CONTROL_PASSWORD",
];
/// Vars read by backend src code that are intentionally NOT
/// operator-facing — kept out of `.env.example` because they are
/// either system vars, dev-only debug knobs, or model-tuning hooks
/// only meaningful when forking the codebase. Each entry needs a
/// rationale (paired here as `(name, why)` for grep-ability):
const INTERNAL_NOT_CONFIGURABLE: &[(&str, &str)] = &[
("HOME", "system var, fallback for $HOME/.cache/mangalord/chromium when CRAWLER_CHROMIUM_DIR is unset (see crawler::browser::default_chromium_dir)"),
("CRAWLER_TOR_CONTROL_PASSWORD", "internal name; compose maps TOR_CONTROL_PASSWORD (.env) into this var so the operator sets one secret in one place"),
("ANALYSIS_SYSTEM_PROMPT", "multi-paragraph default; env-override impractical, fork to change"),
("ANALYSIS_OCR_PROMPT", "multi-paragraph default; env-override impractical, fork to change"),
("ANALYSIS_GROUNDING_PROMPT", "multi-paragraph default; env-override impractical, fork to change"),
("CRAWLER_BROWSER_MODE", "local-dev only: chooses between bundled fetcher and system chromium"),
("CRAWLER_BROWSER_ARGS", "local-dev only: extra chromium launch args"),
("CRAWLER_CHROMIUM_DIR", "local-dev only: override for chromium download cache dir"),
("CRAWLER_KEEP_BROWSER_OPEN", "local-dev only: keep headed chromium alive between runs"),
("CRAWLER_FORCE_REFETCH_CHAPTERS", "debug-only: re-fetches even already-downloaded chapters"),
("CRAWLER_SKIP_CHAPTER_CONTENT", "debug-only: skips page downloads, keeps metadata"),
("CRAWLER_SKIP_CHAPTERS", "debug-only: skips chapter pass entirely"),
];
/// Parse the env-var keys listed at column-0 of `.env.example`.
/// Skips comment-only lines and blank lines.
fn parse_env_example_keys(env_example: &str) -> std::collections::HashSet<String> {
let mut keys = std::collections::HashSet::new();
for line in env_example.lines() {
let trimmed = line.trim_start();
if trimmed.is_empty() || trimmed.starts_with('#') {
continue;
}
// Only column-0 lines count (avoid grabbing keys mentioned in
// mid-line prose inside a comment continuation).
if line.starts_with(|c: char| c.is_ascii_uppercase() || c == '_') {
if let Some((k, _)) = line.split_once('=') {
let k = k.trim();
if !k.is_empty()
&& k.chars().all(|c| c.is_ascii_uppercase() || c.is_ascii_digit() || c == '_')
{
keys.insert(k.to_string());
}
}
}
}
keys
}
/// Regression test for the 0.87.1 compose env-wire-through gap, with
/// the 0.87.17 strengthening from the rereview:
/// the 0.87.17 RHS-interpolation strengthening and the 0.87.25
/// list-derivation strengthening:
///
/// 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`.
/// The 0.87.12 version substring-matched the env-var KEY in compose
/// — blind to whether the RHS substituted from `.env`. The 0.87.17
/// version added an RHS-interpolation check but still hardcoded the
/// `BACKEND_CONSUMED` list, which drifted (~22 code-read vars not
/// in compose). The 0.87.25 version derives the list from
/// `.env.example` so the operator-facing surface is the single
/// source of truth: anything documented there must reach the
/// container, and a new line in the example forces a wiring update.
///
/// Every var listed in `BACKEND_CONSUMED` must:
/// Every key in `.env.example` that isn't in
/// `NOT_BACKEND_CONSUMED_IN_ENV_EXAMPLE` must:
/// * have a `KEY:` line under the backend service `environment:`, AND
/// * either be in `BACKEND_HARDCODED_OK` (intentional literal), OR
/// * either be in `COMPOSE_RHS_EXEMPT` (intentional non-interp), 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
// that aren't (POSTGRES_*, TOR_CONTROL_PASSWORD, BACKEND_URL,
// BACKEND_PROXY_TIMEOUT_MS, VISION_MANAGER_*, the `VISION_*`
// sidecar-only vars) are deliberately excluded — keeping this
// list explicit so we get a compile-broken signal rather than
// a silently-passing test if a new var is added.
const BACKEND_CONSUMED: &[&str] = &[
"BIND_ADDRESS",
"STORAGE_DIR",
"RUST_LOG",
"COOKIE_SECURE",
"COOKIE_DOMAIN",
"SESSION_TTL_DAYS",
"AUTH_RATE_PER_SEC",
"AUTH_RATE_BURST",
"CORS_ALLOWED_ORIGINS",
"ADMIN_ALLOWED_ORIGINS",
"ADMIN_USERNAME",
"ADMIN_PASSWORD",
"PRIVATE_MODE",
"ALLOW_SELF_REGISTER",
"MAX_REQUEST_BYTES",
"MAX_FILE_BYTES",
"CRAWLER_START_URL",
"CRAWLER_CDN_HOST",
"CRAWLER_LIMIT",
"CRAWLER_JOB_TIMEOUT_SECS",
"CRAWLER_METADATA_MAX_CONSECUTIVE_FAILURES",
"CRAWLER_BROWSER_RESTART_THRESHOLD",
"CRAWLER_DOWNLOAD_ALLOWLIST",
"CRAWLER_ALLOW_ANY_HOST",
"CRAWLER_MAX_IMAGE_BYTES",
"CRAWLER_CHROMIUM_BINARY",
"CRAWLER_PROXY",
"CRAWLER_TOR_CONTROL_URL",
"CRAWLER_TOR_RECIRCUIT_MAX_ATTEMPTS",
"ANALYSIS_ENABLED",
"ANALYSIS_VISION_URL",
"ANALYSIS_VISION_MODEL",
"ANALYSIS_WORKERS",
"ANALYSIS_JOB_TIMEOUT_SECS",
"ANALYSIS_API_KEY",
"ANALYSIS_VISION_HEALTH_URL",
];
// Cargo run dir for `cargo test` is `backend/`; .env.example and
// compose file live one up.
let env_example = std::fs::read_to_string("../.env.example")
.expect("read ../.env.example");
let example_keys = parse_env_example_keys(&env_example);
let backend_consumed: Vec<&str> = example_keys
.iter()
.map(String::as_str)
.filter(|k| !NOT_BACKEND_CONSUMED_IN_ENV_EXAMPLE.contains(k))
.collect();
assert!(
!backend_consumed.is_empty(),
"parse_env_example_keys returned no keys — check .env.example formatting"
);
// 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:`
@@ -912,16 +962,16 @@ mod tests {
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 missing: Vec<String> = Vec::new();
let mut not_interpolated: Vec<(String, String)> = Vec::new();
for key in BACKEND_CONSUMED {
for key in &backend_consumed {
// The env entries sit at exactly 6 spaces under `environment:`.
let needle = format!(" {key}:");
let Some(line_start) = backend_block.find(&needle) else {
missing.push(*key);
missing.push((*key).to_string());
continue;
};
if BACKEND_HARDCODED_OK.contains(key) {
if COMPOSE_RHS_EXEMPT.contains(key) {
continue;
}
// RHS: everything from the colon to the end of line.
@@ -937,22 +987,171 @@ mod tests {
not_interpolated.push(((*key).to_string(), rhs.to_string()));
}
}
missing.sort();
not_interpolated.sort();
assert!(
missing.is_empty(),
"docker-compose.yml backend service is missing env wiring for: {missing:?}. \
Add `KEY: ${{KEY:-default}}` lines under backend.environment so the var \
reaches the container."
reaches the container. (Source of truth is .env.example.)"
);
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 \
to `${{KEY:-default}}` or add the var to `COMPOSE_RHS_EXEMPT` with a \
rationale. Offenders: {not_interpolated:?}"
);
}
/// New in 0.87.25: every env::var / env_xxx call in backend/src/
/// must read a key that is EITHER documented in `.env.example`
/// (operator-tunable) OR listed in `INTERNAL_NOT_CONFIGURABLE` with
/// a rationale.
///
/// This catches the silent-no-op class of bug from the other side:
/// a new code path that reads e.g. `ANALYSIS_TEMPERATURE` would
/// previously compile and run fine, but an operator setting it in
/// `.env` would see no effect because `.env.example` and compose
/// never learned about it. Forcing the rationale (operator-facing
/// or internal-only?) at test time keeps the surface honest.
#[test]
fn every_backend_src_env_read_is_documented_or_allowlisted() {
let env_example = std::fs::read_to_string("../.env.example")
.expect("read ../.env.example");
let example_keys = parse_env_example_keys(&env_example);
let internal_keys: std::collections::HashSet<&str> = INTERNAL_NOT_CONFIGURABLE
.iter()
.map(|(k, _)| *k)
.collect();
// Walk backend/src/ for *.rs files.
let mut files: Vec<std::path::PathBuf> = Vec::new();
walk_rust_files(std::path::Path::new("src"), &mut files)
.expect("walk backend/src/");
let mut undocumented: std::collections::BTreeMap<String, Vec<String>> =
std::collections::BTreeMap::new();
for file in &files {
let src = std::fs::read_to_string(file).expect("read source");
for key in extract_env_reads(&src) {
if example_keys.contains(&key) || internal_keys.contains(key.as_str()) {
continue;
}
undocumented
.entry(key)
.or_default()
.push(file.display().to_string());
}
}
assert!(
undocumented.is_empty(),
"backend/src/ reads env vars that are neither in .env.example \
(operator-tunable) nor in INTERNAL_NOT_CONFIGURABLE (internal-only \
with rationale). For each entry below, either:\n\
(a) add it to .env.example AND wire it into docker-compose.yml's \
backend.environment as `KEY: ${{KEY:-default}}` so operators \
can override it, OR\n\
(b) add it to INTERNAL_NOT_CONFIGURABLE in this file with a \
one-line rationale (system var, dev-only knob, etc.).\n\
Offenders: {undocumented:#?}"
);
}
/// Recursively collect `*.rs` paths under `dir`. Test-only helper
/// for the env-read coverage scan above. Skips `target/` and any
/// hidden directory just in case.
fn walk_rust_files(dir: &std::path::Path, out: &mut Vec<std::path::PathBuf>) -> std::io::Result<()> {
for entry in std::fs::read_dir(dir)? {
let entry = entry?;
let path = entry.path();
let name = entry.file_name();
let name_str = name.to_string_lossy();
if name_str.starts_with('.') || name_str == "target" {
continue;
}
if path.is_dir() {
walk_rust_files(&path, out)?;
} else if path.extension().and_then(|s| s.to_str()) == Some("rs") {
out.push(path);
}
}
Ok(())
}
/// Find every env-read key in a Rust source string. Matches the
/// project's env helpers (`env_bool`, `env_i64`, …, `env_prompt`)
/// and the std calls (`std::env::var`, `std::env::var_os`). Returns
/// the literal keys passed as the first arg.
///
/// Treats `KEY` as `[A-Z0-9_]+` to avoid false positives like
/// `env::var(some_string)` (not literal). Skips any call with a
/// non-literal first arg.
fn extract_env_reads(src: &str) -> Vec<String> {
const PREFIXES: &[&str] = &[
"env::var(\"",
"env::var_os(\"",
"env_var(\"",
"env_var_os(\"",
"env_bool(\"",
"env_i64(\"",
"env_f64(\"",
"env_usize(\"",
"env_u64(\"",
"env_u32(\"",
"env_prompt(\"",
];
let mut out = Vec::new();
for prefix in PREFIXES {
for (i, _) in src.match_indices(prefix) {
let rest = &src[i + prefix.len()..];
let Some(end) = rest.find('"') else { continue };
let key = &rest[..end];
if !key.is_empty()
&& key
.chars()
.all(|c| c.is_ascii_uppercase() || c.is_ascii_digit() || c == '_')
{
out.push(key.to_string());
}
}
}
out
}
/// Sanity test for the env-read extractor: hand-coded snippet
/// must yield the expected keys.
///
/// The snippet is built at runtime from format! parts so this
/// source file doesn't literally contain any prefix-then-key
/// pattern that the other coverage scan would otherwise pick up
/// and require allowlisting (the scan is text-based and oblivious
/// to comments).
#[test]
fn extract_env_reads_finds_helper_and_std_calls() {
let src = format!(
"let a = std::env::{v}(\"FOO_A\").unwrap();\n\
let b = std::env::{vo}(\"FOO_B\");\n\
let c = {eb}(\"FOO_C\", false);\n\
let d = {ei}(\"FOO_D\", 0);\n\
let e = {ep}(\"FOO_E\", String::new());\n\
// Non-literal call must be skipped:\n\
let _ = std::env::{v}(some_name).unwrap();\n\
// Mixed-case / lowercase keys are skipped (env vars are upper):\n\
let _ = std::env::{v}(\"Lowercase\");\n",
v = "var",
vo = "var_os",
eb = "env_bool",
ei = "env_i64",
ep = "env_prompt",
);
let mut keys = extract_env_reads(&src);
keys.sort();
keys.dedup();
assert_eq!(keys, vec!["FOO_A", "FOO_B", "FOO_C", "FOO_D", "FOO_E"]);
}
/// 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…]:$`)