fix: graceful shutdown, resumable key backfill, constant-time OTP compare, escaped email templates

[graceuvala-collab]

Warning

Not tested. This PR is an implementation only. No tests were written or run (cargo test, cargo check, cargo fmt and clippy were not run; no Rust toolchain was available). The test cases listed in #271–#274 still need to be added. Reviewers should build and run CI before merging.

Closes #271
Closes #272
Closes #273
Closes #274

#271: Graceful shutdown on SIGTERM (bin/server, crates/ingest)

Audit: the process did not handle SIGTERM. axum::serve(...).await had no shutdown hook, and the ingest supervisor was a detached tokio::spawn running loop { tick; sleep }. So a SIGTERM killed it immediately, mid-request or mid-page.

• shutdown_signal() listens for SIGTERM (tokio::signal::unix) and SIGINT, and falls back to Ctrl-C only on non-unix.
• A tokio_util::sync::CancellationToken drives both axum::serve(..).with_graceful_shutdown(..) and a new Supervisor::run_until_cancelled. The supervisor only checks the token between ticks, so the current tick always finishes its page. Cancellation also cuts the inter-tick sleep short. Supervisor::run keeps its old signature.
• The whole drain (HTTP + ingest) is bounded by SHUTDOWN_DRAIN_TIMEOUT_SECS (default 25, below k8s' 30 s grace period). After that the tasks are aborted and the process exits. A page cut short this way is safe because the cursor advances per record and deposit inserts are deduplicated on (tx_hash, op_index).
• If the server fails on its own, before any signal, that is still returned as an error, not treated as a clean shutdown.
• Log sequence: shutdown signal received → draining … → ingest supervisor stopped … → drained (or drain timeout elapsed; forcing exit) → exiting.
• Documented in a new Deployment section of README.md and in .env.example.

#272: Resumable key-rotation backfill (bin/migrate-keys)

Audit: after_id lived only in memory, so an interrupted run started over from the beginning.

• After each fully completed batch, the cursor is written to migrate-keys.checkpoint (override with MIGRATE_KEYS_CHECKPOINT). It is written as a temp file and renamed, so a crash never leaves a half-written checkpoint. On startup the tool resumes from it, and it is removed on clean completion.
• Security hardening: the checkpoint stores a SHA-256 fingerprint of the (MASTER_KEY, MASTER_KEY_NEXT) pair (domain-separated; no key material). A checkpoint left over from a different rotation is refused. Without this, a stale cursor would silently skip unmigrated rows while the tool still reported "0 remaining". Malformed checkpoints are also refused, with a clear "delete it to restart" message.
• Logs a progress line per batch (batches_completed, total_migrated, total_skipped, after_id) and a final summary.
• Also fixed: --batch-size 0 (or a negative value) produced LIMIT 0, an empty first page, and a false "migration complete". It is now rejected.
• Location, lifecycle and how to force a full re-run are documented in the module doc comment.

#274: Constant-time OTP hash compare (crates/store)

• otp.code_hash != code_hash is replaced by otp_hash_matches, which uses subtle::ConstantTimeEq. OTP semantics (attempts, expiry, consumption, tx binding) are unchanged.
• Why subtle rather than a hand-rolled loop: it is already in Cargo.lock (transitively via hmac, the same primitive behind the verify_slice checks for JWT and webhook signatures), so it adds no new code to the supply chain. It is also well reviewed and guards against the compiler optimizing it into an early-exit comparison, which a manual loop can't guarantee.
• Other comparisons audited: API-key, token-denylist and JWT-denylist lookups are indexed DB equality (out of scope per the issue). JWT and webhook signatures already use verify_slice. tx_hash_bound is a public tx hash, not a secret.
• The constant-time property is deliberately not unit-tested, because timing tests are flaky. This is noted in the doc comment.

#273: HTML-escape template values (crates/email)

Added pub fn html_escape (escapes & < > " '; no new dependency) and a module-level escaping convention.

Template	Value	Source	Escaped?
otp_email	code	server-generated digits	no (never user-controlled)
otp_email	purpose	mapped to a fixed string, never interpolated raw	no (safe)
welcome_email	email	user input at signup (RFC 5322 local parts may contain <"'&)	yes
withdrawal_success_email / withdrawal_failed_email	amount	server-formatted from integer stroops	no (digits and . only)
both withdrawal templates	asset	user-signed XDR	yes
both withdrawal templates	destination	user-signed XDR	yes
withdrawal_success_email	tx_hash	Horizon response	yes (defense in depth)
withdrawal_failed_email	reason	fixed string, or Horizon detail	yes
shell / socials	icon/logo/social URLs, year	compile-time constants / clock	no (safe)

No template currently interpolates a wallet label or username; the convention covers future additions.

Out-of-scope findings (reported, not fixed here)

1. Key rotation looks like a no-op when the scheme doesn't change (high). list_wallets_needing_reseal filters on sealed_scheme <> SCHEME_V1, and SCHEME_V1 is the only scheme. So a MASTER_KEY → MASTER_KEY_NEXT rotation under the same scheme would select zero rows and report success, and the server's scheme-based choice between old and new key can't tell migrated rows from unmigrated ones. Needs a key-version marker (for example a sealed_key_id column) rather than relying on the scheme tag. Worth a dedicated issue.
2. OTP hashes are unkeyed SHA-256 of 6-digit codes. Anyone who can read email_otps can invert every live code with a 10⁶-entry table. Suggest HMAC-SHA256 with a server secret in octo_email::hash_otp. This matters more than the timing fix in Use a constant-time comparison for OTP code-hash verification #274.

[drips-wave]

@graceuvala-collab Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits