fix(merchant): contain payment-goroutine panics

[Amperstrand]

Context

PurchaseSession runs the wallet Receive call in a goroutine with no recover:

go func() {
    amount, err := m.tollwallet.Receive(paymentCashuToken)
    ch <- receiveResult{amount, err}
}()

Any panic in the wallet layer (e.g. a mint serving malformed keysets — see the empty-proofs proofsToSwap[0] class of bugs) therefore kills the entire tollgate-wrt process, taking down every concurrently active customer session, and can be triggered by any unauthenticated portal client that submits a crafted token.

Fix (W-1b: panic containment)

The goroutine now defers a recover that sends an explicit error into the existing buffered channel:

defer func() {
    if r := recover(); r != nil {
        ch <- receiveResult{0, fmt.Errorf("payment processing panicked: %v", r)}
    }
}()

Fast-error contract: the caller's select (which keeps its 30s timeout arm unchanged) receives this result immediately and returns a signed payment-processing-failed notice — instead of process death or a misleading 30-second payment-processing-timeout notice.

Channel safety: a panic can only fire before the normal send, so the deferred send never double-sends; ch is buffered(1), so the deferred send never blocks even if the caller has already taken the timeout arm.

Test

TestPurchaseSessionPanicContainment (src/merchant/purchasesession_panic_test.go) drives the real PurchaseSession with a stub WalletPort whose Receive panics, and asserts:

• a notice event of kind 21023 with code payment-processing-failed comes back,
• its content contains panicked,
• it all completes in < 5 s (deadline enforced inside the test — proves the fast-error path, not the 30s timeout arm),
• the test process survives (panic contained, not propagated).

On unpatched code this test crashes the test binary (panic: wallet exploded ... at merchant.go:475 in PurchaseSession.func1) — i.e. it demonstrably bites.

Note: running tests inside src/merchant requires go mod tidy first — a pre-existing go.mod staleness on main (gonuts pinned at v0.10.0 while the code imports v0.11.1), unchanged by this PR and out of scope here.

[Amperstrand]

⚠️ Branch cleaned up (force-push) — please review the new head.

This branch (and #361, #362, #363) was based on pre-incident history and still carried deploy-backup-20260730/ — the router backup with the merchant private identity key, wallet.db and recovery tokens that were purged from main via the history rewrite (see the incident report at main@HEAD and #364). Merging as-is would have re-committed the leaked key.

The branch has been rebased onto rewritten main (6c974f7) and now carries only the panic-containment change:

• src/merchant/merchant.go — defer recover() in the payment goroutine sends the error into the existing buffered channel
• src/merchant/purchasesession_panic_test.go — unchanged
• CHANGELOG entry

Dropped: the secrets directory, the DEPENDS packaging change (nodogsplash,luci,jq → libc), .pre-commit-config.yaml, packaging/local-build-ipk.sh, the token-recovery binary, and unrelated go.mod churn. If any of those are wanted, please propose them as separate PRs — the DEPENDS change in particular needs its own discussion (it removes the declared nodogsplash/luci/jq deps from the ipk).

Verified locally: build clean, and TestPurchaseSessionPanicContainment passes on the cleaned branch (panic contained, payment-processing-failed notice in <5s). E2E cloud-lab run on this head is in flight; results will follow.

[felixfelix-bot]

Thanks for this — panic containment in the payment goroutine is a solid hardening fix. The recover() + buffered channel pattern is correct: a panic in Receive skips the normal send, so the deferred send is the only one that fires, and the buffer-1 channel guarantees it never blocks.

A few observations:

1. [NIT] No stack trace captured in recovery. src/merchant/merchant.go:477 — the recovered error is fmt.Errorf("payment processing panicked: %v", r), which gives the panic message but no origin. In production, diagnosing why a wallet-layer panic occurred will be difficult without a stack trace. Consider capturing runtime/debug.Stack() (or at minimum runtime.Stack()) into the error message or a structured log field so the incident is traceable post-mortem.

2. [NIT] Comment correctness is fragile against future edits. src/merchant/merchant.go:475-476 — the comment "A panic can only fire before the normal send, so this never double-sends" is true today because Receive is the last statement before the send. If someone later adds logic after ch <- receiveResult{amount, err}, a panic there would cause a double-send (the normal send succeeds, then the deferred recover fires). The buffer-1 channel prevents a goroutine leak, but the consumer would see a stale "panicked" error overwrite a valid result. Consider adding a guard flag or reordering the comment to document this constraint explicitly for future maintainers.

3. [NIT] Test relies on nil-interface panic propagation. src/merchant/purchasesession_panic_test.go:35-37 — panicReceiveWallet embeds tollwallet.WalletPort as a nil interface to make unimplemented methods panic. This is a clever guard, but if PurchaseSession is later refactored to call another WalletPort method before Receive, the test would pass for the wrong reason (nil-deref panic instead of the intended Receive panic). A brief comment noting this assumption in the test would help future contributors avoid false positives.

Test coverage looks good — verifies the <5s timeout, correct event kind (21023), correct notice code (payment-processing-failed), and content contains "panicked". The CHANGELOG entry is well-written.

[felixfelix-bot]

SEC-AUDIT T2 review — APPROVED (cold review) + verification evidence

Cold review verdict (kimi-k3, cross-family): APPROVED — no blocking issues. The defer/recover is correct and race-free (single sender, buffer-1 channel, no double-send, no block). Test genuinely pins the panic-containment contract. Only low/nit suggestions (capture stack trace, signature check on notice).

{"verdict": "APPROVED", "summary": "defer/recover correct and race-free; test pins the contract end-to-end; CHANGELOG accurate", "issues": [{"severity":"low","file":"src/merchant/merchant.go","line":479,"issue":"recover discards stack trace"},{"severity":"low","file":"src/merchant/merchant.go","line":479,"issue":"raw panic value into customer-facing notice"},{"severity":"nit","file":"src/merchant/merchant.go","line":477,"issue":"panic(nil)/Goexit degrade to 30s timeout (safe)"},{"severity":"nit","file":"src/merchant/purchasesession_panic_test.go","line":68,"issue":"no signature verification on notice"},{"severity":"nit","file":"src/merchant/purchasesession_panic_test.go","line":39,"issue":"no post-panic merchant-health check"}], "test_quality": "Strong — end-to-end contract test through real PurchaseSession, discriminates all three failure modes"}

Panic containment test (empty-proofs / malformed-keyset mint does NOT crash daemon):

=== RUN   TestPurchaseSessionPanicContainment
PurchaseSession: calling Receive for mint=https://panic-mint.example.com token_amount=1 mac=AA:BB:CC:DD:EE:FF
PurchaseSession: Receive completed, amount=0, err=payment processing panicked: wallet exploded: simulated keyset corruption
--- PASS: TestPurchaseSessionPanicContainment (0.28s)
ok  github.com/OpenTollGate/tollgate-module-basic-go/src/merchant

The panic is recovered and surfaced as an explicit payment processing panicked: ... error → signed payment-processing-failed notice, NOT process death and NOT the 30s timeout path.

Full test suite on merged tree (main + PR #360): all packages pass except config_manager, which fails with a pre-existing mint-count issue also present on main's own CI (run 33080382404, identical buildinfo_test.go failures). PR #360 touches only merchant.go, purchasesession_panic_test.go, CHANGELOG.md — it does not cause that failure.

ok  github.com/OpenTollGate/tollgate-module-basic-go  13.688s
ok  .../src/cli  1.231s
FAIL .../src/config_manager  (pre-existing on main)
ok  .../src/identity  8.521s
ok  .../src/lightning  1.024s
ok  .../src/merchant  202.966s
ok  .../src/merchant_types  1.109s
ok  .../src/sysexec  1.259s
ok  .../src/tollgate_protocol  1.870s
ok  .../src/tollwallet  1.063s
ok  .../src/upstream_session_manager  1.036s
ok  .../src/utils  1.097s
ok  .../src/valve  2.088s
ok  tollgate-module-basic-go  1.500s

Merge check: PR branch is based on pre-#361 main (merge-base 14e4884) and still carries gonuts-tollgate v0.10.0 in cli/merchant + the felixfelix-bot fork replace in tollwallet. However, PR #360 does NOT touch any go.mod — a squash merge into current main yields main's v0.11.1 everywhere (check-deps-sync.py: ✅ All 124 shared dependencies in sync). No deps regression reintroduced. Merge is clean and safe.

[Amperstrand]

E2E results (follow-up to review): deployed head bbfd84d as ci-pr-360.197.bbfd84d (CI-built ipk) on the OpenWrt x86_64 QEMU lab:

Suite	Result
api/test_quote_persistence	4/4 passed
api/test_lightning_backoff	3/3 passed

The panic-containment change itself carries the strongest evidence in its unit test — re-verified on the cleaned branch: TestPurchaseSessionPanicContainment passes (panic contained, payment-processing-failed notice returned in <5s, test process survives). Combined with the green suites above on the built binary, this is merge-ready from our side.