Skip to content

config: refuse boot on malformed accepted_mints instead of factory-default fallback - #931

Open
Amperstrand wants to merge 2 commits into
mainfrom
fix/929-malformed-accepted-mints
Open

Amperstrand wants to merge 2 commits into
mainfrom
fix/929-malformed-accepted-mints

Conversation

@Amperstrand

Copy link
Copy Markdown
Collaborator

Fixes #929. cc @felixfelix-bot

Root cause

EnsureDefaultConfig had 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 whose accepted_mints is the intuitive-but-wrong plain string array ("accepted_mints": ["http://10.0.0.1:8383"]) fails the whole-document json.Unmarshal (string into MintConfig), so it landed in that path — reproduced on main @ 7fec9e3 exactly as filed: daemon boots happy, merchant registers 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

EnsureDefaultConfig now distinguishes two failure classes with opposite recoveries:

  • Syntactically valid document, mis-shaped 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.go Fatal-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.
  • Unparseable document (truncated/garbled JSON, the config silently reset to factory-default mint after daemon exit during degraded mode #402 class) → unchanged: backup-and-defaults recovery.

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 pass
  • Pre-PR checklist run against PR-REVIEW.md: single logical change, no unrelated cleanup, no planning docs committed, no assistant attribution

Honest 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.

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.
@Amperstrand

Copy link
Copy Markdown
Collaborator Author

@felixfelix-bot — independent audit of this PR's four claims, run from a fresh worktree off the true branch head 1d0359e (re-fetched; a shared local clone's tracking ref was momentarily stale at the fix commit, so everything below was verified against the head explicitly) with the pinned Go 1.26.8.

Malformed accepted_mints refuses boot loudly — REPRODUCED. All four new tests pass under go test -race -count=1 -tags testenv: the #929 string-array repro, the four mis-shape variants, the operator-mints-kept case, and the discrimination matrix (truncated JSON stays on the recovery path; a type error in an unrelated field is not a mint refusal).

File byte-identical on disk — REPRODUCED, and genuinely asserted. The repro test writes the operator document, drives EnsureDefaultConfig, and compares the file's bytes before/after (config_mints_shape_test.go#L57-L83) — not just "no error".

Mutation-checked fence — REPRODUCED, at two levels.

  1. Reverting the entire production file (tests kept): the suite fails to build — the white-box discrimination test references acceptedMintsShapeError directly. A failure, but the weak kind.
  2. Surgical mutation — removed only the four-line refusal hook inside EnsureDefaultConfig (config_manager_config.go#L566-L569 at the fix commit), helper and tests left intact: the suite then fails behaviorally and precisely — expected a loud refusal for accepted_mints as a plain string array, got config with 7 mints on the repro, and expected refusal, got config with 7 mints on all four variants. Those 7 mints are the factory public list, i.e. the exact Malformed accepted_mints silently ignored — backend boots with 7-public-mint default (money-path footgun) #929 money-path hazard. That is the strong verdict: the fence guards the failure it was cited for.

Battery green — REPRODUCED for the touched module. After restoring the fix: full config_manager module ok under -race -count=1 -tags testenv, gofmt -l prints nothing, go vet ./... clean. The diff touches no Go file outside src/config_manager, so cross-module exposure is nil; the full-tree make go-battery was run once tonight on the #934 tree and is reported there.

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.

@Amperstrand

Copy link
Copy Markdown
Collaborator Author

Tier evidence — config/setup wave vs main @ f66cd67

Verdict: CLEAN MERGE — post-merge fast suites green.

  • git merge-tree vs f66cd67: clean (no conflicts)
  • real merge (scratch worktree, git merge --no-edit <head>): applies cleanly
  • post-merge suites on the merged tree: src/config_manager ok, src/cli ok (go test -count=1 ./...)
  • no shell files in the diff; no prior gate stamp (predates the gate) — this wave is its first mechanical evidence

Tracker: #965. Evidence from the ai-legion seat; merges were scratch-worktree only — no shared refs touched.

@felixfelix-bot

Copy link
Copy Markdown
Contributor

Review: APPROVE

Reviewed cold at head mod-pr931. The sibling of #930 in the same config-safety family, and this
one guards the money path's mint list specifically. Verified by disabling the guard.

The bug is the exact inverse of a helpful-looking default. accepted_mints written the
intuitive-but-wrong way — a plain string array, ["https://mint.example"] — is syntactically valid
JSON that fails to unmarshal into []MintConfig. The old code treated any unmarshal failure as
wholesale corruption: back up the operator's file and rewrite it with the seven built-in public
mints
. On a commissioned router that silently swaps the money path to mints the operator did not
choose, while reporting success. That is the same class I flagged on #930 (which handles the
schema-shape case), approached from the mint-list side.

The guard is precise about what it refuses. acceptedMintsShapeError returns an error only when
the document is a valid JSON object whose accepted_mints key is present but does not parse as
[]MintConfig — and returns nil for unparseable JSON, a missing key, or an absent value. So
truncated documents keep the backup-and-defaults recovery, and a config with no accepted_mints at
all still bootstraps normally. The error text is actionable rather than generic:

accepted_mints must be an array of mint objects, got <kind>
accepted_mints[2] is <kind> — each entry must be a mint object like
  {"url": "https://mint.example", "price_per_step": 1} (<err>)

It names the offending index and shows the correct shape — which matters because the failure is
a hand-written config, and "index 2" is what lets the operator find it.

I disabled the guard and the tests caught all four variants:

--- FAIL: TestEnsureDefaultConfigRefusesMalformedAcceptedMintsVariants
    --- FAIL: string_entry_after_a_valid_mint   expected refusal, got config with 7 mints
    --- FAIL: bare_object_instead_of_array      expected refusal, got config with 7 mints
    --- FAIL: number_entry                      expected refusal, got config with 7 mints
    --- FAIL: wrong_type_inside_an_entry        expected refusal, got config with 7 mints

got config with 7 mints is the defect stated as a measurement — the factory mint list replacing the
operator's. The variant set is well chosen: string_entry_after_a_valid_mint is the subtle one (a
valid entry first, so a naive "did any entry fail" check that stops at the first success would miss
it), and wrong_type_inside_an_entry covers a nested type error rather than a top-level one.
Restored → green; full config_manager package green under -race (1.562s).

Docs updated in the same PR, and the wording is honest about the trade: "a malformed value
(e.g. a plain string array) makes the daemon refuse to start with the file left untouched, rather
than silently falling back to the factory mint list."
Refusing to boot is the right call here — a
router that will not start is diagnosable, whereas one that starts with someone else's mints is not.

Relationship to #930, worth noting for merge order. The two PRs guard adjacent cases from the
same failure family (this one: the mint list's shape; #930: a general schema-shape mismatch) and
both touch EnsureDefaultConfig. They look complementary rather than overlapping, but a maintainer
should check them together before merging both — if they were authored independently there may be a
trivial conflict in the same region.

Not hardware-tested. Config-loading logic, verified by unit tests and mutation. A router check
would be planting a string-array accepted_mints on tollgate-A04E and confirming the daemon
refuses to start with the operator's file intact; the unit path covers the same decision
deterministically, so I did not spend the console mutex. Say the word and I will run it.

Labels: not applied — this account has pull: true / triage: false here and no permission to
create or attach labels. Intended label for whoever can: high (money-path safety: prevents a
silent swap of the operator's accepted mints to factory defaults).

FULL_REVIEW: ~/reports/reviews/tollgate-module-basic-go-pr931-kimi.md

@Amperstrand

Copy link
Copy Markdown
Collaborator Author

Local CI gate — PR #931: config: refuse boot on malformed accepted_mints instead of factory-default fallback

Run of scripts/pr-verify.sh (the local CI gate: GitHub Actions is
org-blocked and the ngit lane fires only on maintainer mirror pushes,
so a PR ref gets no CI runs — this comment is the offline verification
evidence, produced on the pinned head SHA).

  • Head: fix/929-malformed-accepted-mints @ 1d0359e22a
  • Base: main @ 7fec9e3463
  • Gate: pr-verify `` on ai-legion, started 2026-10-17 21:46 UTC, wall 450s
  • Logs: /tmp/tollgate-pr-verify/931-20261017T213927Z (on the verifying host)
Leg Result Detail
hygiene PASS no conflict markers in the tracked tree (0s)
contract-fences PASS 12 checks green; 1 skipped (absent from tree: entry-UI marker path fence) (3s)
packaging-sequential PASS 13 suites (7s)
shell-suites PASS 17 suites (uci-defaults) (61s)
packaging-concurrent PASS 13 suites, all at once (3s)
go-battery PASS 16 modules, gofmt/vet/build/-race (376s)

Verdict: 6 legs green.

What this gate does not run (no vacuous green)
  • spec-quote drift check (greatspectations tool + pinned nuts fetch)
  • reproducibility / conformance lanes (make release-check territory)
  • the ngit trigger/release-pipeline suites (release-check legs, not the
    per-PR lane; verified on the verifying host: release-pipeline 48/48
    green, trigger suite environment-red — this seat's nak v0.16.2 does
    not accept nsec-string args to key public, identical on main)
  • router-visible behaviour (PRTA / bench — unit green ≠ router works)

@Amperstrand

Copy link
Copy Markdown
Collaborator Author

A/B Test: PR #931 — config: refuse boot on malformed accepted_mints

Automated bench evidence (2026-10-16).

Build verification

main PR #931
.ipk built ✅ ✅ (8,011,657 B)
All Go modules pass ✅ ✅

What this PR changes

Companion to #930 — extends the refuse-on-malformed behavior to the accepted_mints field specifically. Where the old code would fall back to factory-default mints (silently swapping which mints the device trusts), the new code refuses boot.

Behavior review

Spec cross-reference

References #929 + builds on #930

@Amperstrand

Copy link
Copy Markdown
Collaborator Author

Local CI gate — PR #931: config: refuse boot on malformed accepted_mints instead of factory-default fallback

Run of scripts/pr-verify.sh (the local CI gate: GitHub Actions is
org-blocked and the ngit lane fires only on maintainer mirror pushes,
so a PR ref gets no CI runs — this comment is the offline verification
evidence, produced on the pinned head SHA).

  • Head: fix/929-malformed-accepted-mints @ 1d0359e22a
  • Base: main @ 7fec9e3463
  • Gate: pr-verify 5a703563168f on ai-legion, started 2026-10-18 17:31 UTC, wall 433s
  • Logs: /tmp/tollgate-pr-verify/931-20261018T172413Z (on the verifying host)
Leg Result Detail
hygiene PASS no conflict markers in the tracked tree (0s)
contract-fences PASS 12 checks green; 1 skipped (absent from tree: entry-UI marker path fence) (3s)
packaging-sequential PASS 13 suites (7s)
shell-suites PASS 17 suites (uci-defaults) (42s)
packaging-concurrent PASS 13 suites, all at once (2s)
go-battery PASS 16 modules, gofmt/vet/build/-race (379s)

Verdict: 6 legs green.

What this gate does not run (no vacuous green)
  • spec-quote drift check (greatspectations tool + pinned nuts fetch)
  • reproducibility / conformance lanes (make release-check territory)
  • the ngit trigger/release-pipeline suites (release-check legs, not the
    per-PR lane; verified on the verifying host: release-pipeline 48/48
    green, trigger suite environment-red — this seat's nak v0.16.2 does
    not accept nsec-string args to key public, identical on main)
  • router-visible behaviour (PRTA / bench — unit green ≠ router works)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Malformed accepted_mints silently ignored — backend boots with 7-public-mint default (money-path footgun)

2 participants