Repository navigation
config: refuse boot on malformed accepted_mints instead of factory-default fallback - #931
Amperstrand wants to merge 2 commits into
Conversation
A well-formed config document whose accepted_mints entries do not unmarshal into MintConfig (the plain string array an operator writes intuitively) failed the whole-document unmarshal and fell into the corrupt-file recovery: the operator's file was backed up, factory defaults were written, and the daemon booted accepting tokens for — and paying profit-share to — the seven built-in public mints. EnsureDefaultConfig now distinguishes the two failure classes. A syntactically valid document with mis-shaped accepted_mints is an operator error: the load is refused with an error naming the offending entry, and the file is left untouched (main.go Fatal-exits; procd keeps the failure visible). Genuinely unparseable documents keep the historical backup-and-defaults recovery (#402 truncated-write class). The reload path already had the right semantics (error surfaced by the CLI, previous config kept) and is unchanged.
|
@felixfelix-bot — independent audit of this PR's four claims, run from a fresh worktree off the true branch head Malformed File byte-identical on disk — REPRODUCED, and genuinely asserted. The repro test writes the operator document, drives Mutation-checked fence — REPRODUCED, at two levels.
Battery green — REPRODUCED for the touched module. After restoring the fix: full No discrepancies found — every claim reproduced. One non-blocking observation: because the discrimination test is white-box, the "revert the whole fix" mutation collapses to a compile failure; the behavioral guarantee rests on the surgical verdict above, which it already covers. Audit disposition: evidence supports landing. |
Tier evidence — config/setup wave vs
|
Review: APPROVEReviewed cold at head The bug is the exact inverse of a helpful-looking default. The guard is precise about what it refuses. It names the offending index and shows the correct shape — which matters because the failure is I disabled the guard and the tests caught all four variants:
Docs updated in the same PR, and the wording is honest about the trade: "a malformed value Relationship to #930, worth noting for merge order. The two PRs guard adjacent cases from the Not hardware-tested. Config-loading logic, verified by unit tests and mutation. A router check Labels: not applied — this account has FULL_REVIEW: ~/reports/reviews/tollgate-module-basic-go-pr931-kimi.md |
Local CI gate — PR #931: config: refuse boot on malformed accepted_mints instead of factory-default fallbackRun of
Verdict: 6 legs green. What this gate does not run (no vacuous green)
|
A/B Test: PR #931 — config: refuse boot on malformed accepted_mintsAutomated bench evidence (2026-10-16). Build verification
What this PR changesCompanion to #930 — extends the refuse-on-malformed behavior to the Behavior review
Spec cross-reference |
Local CI gate — PR #931: config: refuse boot on malformed accepted_mints instead of factory-default fallbackRun of
Verdict: 6 legs green. What this gate does not run (no vacuous green)
|
Fixes #929. cc @felixfelix-bot
Root cause
EnsureDefaultConfighad one recovery for every unmarshal failure: log a WARNING, back the operator's file up, rewrite factory defaults, and boot. A well-formed config document whoseaccepted_mintsis the intuitive-but-wrong plain string array ("accepted_mints": ["http://10.0.0.1:8383"]) fails the whole-documentjson.Unmarshal(string intoMintConfig), so it landed in that path — reproduced on main @ 7fec9e3 exactly as filed: daemon boots happy,merchantregisters the seven built-in public mints, and the operator's file is destructively rewritten on disk. On a commissioned router that silently swaps the money path.Change
EnsureDefaultConfignow distinguishes two failure classes with opposite recoveries:accepted_mints(string entries, non-array value, wrong per-field types) → operator error: the load is refused with an error naming the offending entry (accepted_mints[0] is a string — each entry must be a mint object like {"url": …}), and the file is left untouched.main.goFatal-exits (Failed to create config manager), so procd keeps the failure visible instead of a silently re-minted router. No backup rename, no defaults write.The reload path (
cli/config.go→ReloadConfig) already had the right semantics — error surfaced to the operator, previous config kept — and is unchanged.Scope note: the refusal is deliberately scoped to
accepted_mints(the money path this issue is about), not every field-level type error; generalizing to all fields would change boot behavior for configs that currently limp along on defaults and is left as a maintainer call.Tests
New
src/config_manager/config_mints_shape_test.go:Mutation-checked per the contract-suite doctrine: with the fix reverted the new suite fails; with it, green.
Validation
make go-battery— 16 modules green (gofmt/vet/build/go test -race -tags testenv)node tests/contract/js-schema-lint.mjs,bash tests/contract/build-purity.sh,bash tests/contract/check-ssid-format.sh— all passHonest limits: verified at unit/boot-path level only; no router-runtime (OpenWrt/procd) leg was run for this PR — the changed behavior is pure config-load logic exercised by the new tests, but on-router behavior (procd restart loop presentation) is asserted from code reading, not a bench run.