Skip to content

Isolate wallet secrets, add withdrawal idempotency, validate CORS origins - #1007

Open
victorisiguzoruzoma874 wants to merge 1 commit into
kellymusk:masterfrom
victorisiguzoruzoma874:fix/wallet-secret-isolation-idempotency-and-cors-validation
Open

Isolate wallet secrets, add withdrawal idempotency, validate CORS origins#1007
victorisiguzoruzoma874 wants to merge 1 commit into
kellymusk:masterfrom
victorisiguzoruzoma874:fix/wallet-secret-isolation-idempotency-and-cors-validation

Conversation

@victorisiguzoruzoma874

@victorisiguzoruzoma874 victorisiguzoruzoma874 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes #981
closes #982
closes #983

  • Bug: wallets table secret_key_encrypted column is never exposed via API but is in the DB row #981 — Split the wallet row shape in two. Wallet (Serialize, API-facing) documents that secret_key_encrypted must never be added to it. A new WalletSecretRow (no Serialize derive — the compiler rejects Json(..) on it outright) carries the encrypted seed for the one sanctioned accessor, services::wallets::wallet_secret_by_id, intended for the blockchain module only. Every existing wallet-returning query already named columns explicitly (no SELECT *); this closes the gap for future code that needs the secret.
  • Enhancement: add idempotency key support to POST /withdraw to prevent double-submit withdrawals #982POST /withdraw accepts an optional Idempotency-Key header (new migration: withdrawals.idempotency_key, unique per merchant). A resubmission with the same key returns the original withdrawal instead of creating a second one and debiting the balance twice. A check-then-insert race (two concurrent requests with a fresh key) is closed by catching the UNIQUE_VIOLATION on insert and re-reading instead of erroring. Documented in API.md and openapi.yaml.
  • Bug: CORS_ALLOWED_ORIGINS parsing trims whitespace but does not validate that each value is a valid origin #983CORS_ALLOWED_ORIGINS is now validated at startup — each origin must parse as a URL with an http/https scheme and a host, no path/query, and * is rejected outright (this API sends credentials). A bad value now fails AppConfig::from_env() with a clear message instead of surfacing later as a confusing HeaderValue parse panic in main.rs.

Also fixes the same pre-existing extractor.rs compile break described in earlier PRs on this repo (ApiError gained a field member; two call sites weren't updated), needed for this branch to build.

Test plan

  • cargo check passes clean (only pre-existing warnings).
  • Test code intentionally left untouched per task scope.

…gins

Closes kellymusk#981, kellymusk#982, kellymusk#983.

- kellymusk#981: Split the wallet row shape in two. Wallet (Serialize, API-facing)
  documents that secret_key_encrypted must never be added to it. A new
  WalletSecretRow (no Serialize derive — the compiler rejects Json(..) on
  it outright) carries the encrypted seed for the one sanctioned accessor,
  services::wallets::wallet_secret_by_id, intended for the blockchain
  module only. Every existing wallet-returning query already named columns
  explicitly (no SELECT *); this closes the gap for future code that needs
  the secret.
- kellymusk#982: POST /withdraw accepts an optional Idempotency-Key header (new
  migration: withdrawals.idempotency_key, unique per merchant). A
  resubmission with the same key returns the original withdrawal instead
  of creating a second one and debiting the balance twice. A
  check-then-insert race (two concurrent requests with a fresh key) is
  closed by catching the UNIQUE_VIOLATION on insert and re-reading instead
  of erroring. Documented in API.md and openapi.yaml.
- kellymusk#983: CORS_ALLOWED_ORIGINS is now validated at startup — each origin must
  parse as a URL with an http/https scheme and a host, no path/query, and
  `*` is rejected outright (this API sends credentials). A bad value now
  fails AppConfig::from_env() with a clear message instead of surfacing
  later as a confusing HeaderValue parse panic in main.rs.

Also fixes the same pre-existing extractor.rs compile break described in
earlier PRs on this repo (ApiError gained a `field` member; two call sites
weren't updated), needed for this branch to build.
@drips-wave

drips-wave Bot commented Aug 31, 2026

Copy link
Copy Markdown

@victorisiguzoruzoma874 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

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

Labels

None yet

Projects

None yet

1 participant