From 00d2018d8889274840c711e8d6a93c80e065d250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=96=C2=96=C2=96feyisaralawal?= <––––feyisaralawal01@gmail.com> Date: Sat, 26 Sep 2026 18:39:32 +0100 Subject: [PATCH] feat: address batch issues #348, #350, #354, #352 across api and wallet-core This commit simultaneously addresses 4 issues across octo-api and octo-wallet-core: 1. Issue #348: Request-Body Size Limits Uniformity on Mutating Routes - Audited router wiring in crates/api/src/lib.rs and confirmed that DefaultBodyLimit::max(REQUEST_BODY_LIMIT) (64 KiB) is applied uniformly to the router containing all mutating routes. - Exposed pub const REQUEST_BODY_LIMIT: usize = 64 * 1024 from crates/api/src/lib.rs. - Added a parametrized integration test every_mutating_route_rejects_an_oversized_body_with_a_clean_413 in crates/api/tests/body_size_limit_tests.rs covering every mutating route with oversized payloads asserting clean 413 Payload Too Large responses. - Closes #348 2. Issue #350: Sourced BIP-39 Checksum-Invalid Test Vectors - Added a permanent, cited corpus of checksum-invalid BIP-39 mnemonics in crates/wallet-core/src/derive.rs. - Sourced authoritative vectors from Trezor reference implementation (tests/test_mnemonic.py) as well as official BIP-39 specification test vectors (bitcoin/bips/bip-0039.mediawiki and trezor/python-mnemonic/vectors.json) by mutating the final checksum word to alternative valid wordlist entries (almost-valid vectors across 12, 15, 18, and 24-word phrases) alongside grossly invalid vectors. - Added regression test from_phrase_rejects_every_sourced_checksum_invalid_vector parametrized across the corpus to ensure wordlist-and-checksum validation is permanently upheld. - Closes #350 3. Issue #354: Public validate_seed_phrase Helper - Extracted mnemonic syntax, wordlist, and checksum validation logic into a public helper function pub fn validate_seed_phrase(phrase: &str) -> Result<(), WalletError> in crates/wallet-core/src/derive.rs. - Re-exported validate_seed_phrase from crates/wallet-core/src/lib.rs. - Refactored WalletSeed::from_phrase to call validate_seed_phrase internally so pre-flight validation and seed construction never diverge. - Documented that validate_seed_phrase is safe for untrusted input and produces no secret material. - Added regression tests verifying agreement between validate_seed_phrase and from_phrase on both valid and invalid vectors, as well as a type-level test proving no secret material is returned. - Closes #354 4. Issue #352: Document SEP-0005 Derivation Path and Hardened-Index Invariant - Expanded doc comment on derive_ed25519_secret in crates/wallet-core/src/derive.rs adhering to the project documentation register in CONTRIBUTING.md. - Explicitly documented the exact derivation path (m/44'/148'/index'), the security justification for full hardening on Ed25519 (SLIP-0010) in contrast to EVM BIP-44 unhardened derivation, and the 2^31 hardened-index ceiling. - Cross-referenced docs/deposit-model.md detailing how index 0 underpins the muxed-address architecture. - Added a runnable doc-test example demonstrating correct derivation. - Closes #352 --- crates/api/src/lib.rs | 2 +- crates/api/tests/body_size_limit_tests.rs | 164 ++++++++++++++++++++++ crates/wallet-core/src/derive.rs | 156 +++++++++++++++++++- crates/wallet-core/src/lib.rs | 2 +- 4 files changed, 321 insertions(+), 3 deletions(-) create mode 100644 crates/api/tests/body_size_limit_tests.rs diff --git a/crates/api/src/lib.rs b/crates/api/src/lib.rs index a41dbf8..b401993 100644 --- a/crates/api/src/lib.rs +++ b/crates/api/src/lib.rs @@ -28,7 +28,7 @@ use tower_http::cors::{Any, CorsLayer}; /// These routes deserialize JSON from raw `Bytes`; a `Bytes` extractor alone would otherwise /// rely on axum's implicit body limit (currently 2 MiB in this workspace's version). Making the /// limit explicit here keeps the behavior intentional and version-stable. -const REQUEST_BODY_LIMIT: usize = 64 * 1024; +pub const REQUEST_BODY_LIMIT: usize = 64 * 1024; /// Build the API router with shared state. pub fn build_router(state: AppState) -> Router { diff --git a/crates/api/tests/body_size_limit_tests.rs b/crates/api/tests/body_size_limit_tests.rs new file mode 100644 index 0000000..5b44910 --- /dev/null +++ b/crates/api/tests/body_size_limit_tests.rs @@ -0,0 +1,164 @@ +//! Regression test asserting request-body size limits apply uniformly across every mutating route. + +mod common; + +use axum::body::Body; +use axum::http::{Request, StatusCode}; +use octo_api::{build_router, AppState, REQUEST_BODY_LIMIT}; +use octo_store::Store; +use octo_wallet_core::StellarNetwork; +use std::sync::Once; +use tower::ServiceExt; + +static LOAD_ENV: Once = Once::new(); + +fn database_url() -> Option { + LOAD_ENV.call_once(|| { + let _ = dotenvy::dotenv(); + }); + std::env::var("DATABASE_URL").ok() +} + +async fn test_state() -> Option { + let url = database_url()?; + let store = Store::connect(&url).await.expect("connect"); + store.migrate().await.expect("migrate"); + let master_key = [42u8; 32]; + Some(AppState::new( + store, + master_key, + StellarNetwork::Testnet, + "https://horizon-testnet.stellar.org".into(), + None, + octo_email::EmailSender::new_captured(), + )) +} + +struct MutatingRoute { + method: &'static str, + path: &'static str, +} + +const MUTATING_ROUTES: &[MutatingRoute] = &[ + MutatingRoute { + method: "POST", + path: "/v1/auth/signup", + }, + MutatingRoute { + method: "POST", + path: "/v1/auth/verify-email", + }, + MutatingRoute { + method: "POST", + path: "/v1/auth/resend-otp", + }, + MutatingRoute { + method: "POST", + path: "/v1/auth/login", + }, + MutatingRoute { + method: "POST", + path: "/v1/auth/refresh", + }, + MutatingRoute { + method: "PATCH", + path: "/v1/auth/me", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/addresses", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/webhooks", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/submit-signed", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/withdraw/request-otp", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/withdraw/confirm", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/gas-tank", + }, + MutatingRoute { + method: "PUT", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/sponsorship", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/sponsor", + }, + MutatingRoute { + method: "PUT", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/whitelist/config", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/whitelist", + }, + MutatingRoute { + method: "POST", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/payment-links", + }, + MutatingRoute { + method: "PUT", + path: "/v1/wallets/00000000-0000-0000-0000-000000000000/payment-links/00000000-0000-0000-0000-000000000000", + }, + MutatingRoute { + method: "POST", + path: "/v1/pay/sample-link/intent", + }, + MutatingRoute { + method: "POST", + path: "/v1/pay/sample-link/submit-signed", + }, +]; + +#[tokio::test] +async fn every_mutating_route_rejects_an_oversized_body_with_a_clean_413() { + let Some(state) = test_state().await else { + return; + }; + let app = build_router(state); + + // Create an oversized body exceeding REQUEST_BODY_LIMIT (64 KiB). + let oversized_body = vec![b'a'; REQUEST_BODY_LIMIT + 1024]; + + for route in MUTATING_ROUTES { + let req = Request::builder() + .method(route.method) + .uri(route.path) + .header("content-type", "application/json") + .body(Body::from(oversized_body.clone())) + .unwrap(); + + let resp = app.clone().oneshot(req).await.unwrap(); + assert_eq!( + resp.status(), + StatusCode::PAYLOAD_TOO_LARGE, + "route {} {} must reject oversized body with 413 Payload Too Large", + route.method, + route.path + ); + + let bytes = axum::body::to_bytes(resp.into_body(), 4096).await.unwrap(); + assert!( + !bytes.is_empty(), + "route {} {} 413 response should explain itself", + route.method, + route.path + ); + } +} diff --git a/crates/wallet-core/src/derive.rs b/crates/wallet-core/src/derive.rs index a0cae25..d63c865 100644 --- a/crates/wallet-core/src/derive.rs +++ b/crates/wallet-core/src/derive.rs @@ -19,6 +19,21 @@ const BIP44_PURPOSE: u32 = 44; /// Hardened-derivation offset. const HARDENED: u32 = 0x8000_0000; +/// Validate a BIP-39 recovery phrase without constructing or holding secret material. +/// +/// Verifies word count, English wordlist membership, and BIP-39 checksum. Safe to call +/// with untrusted input and produces no secret-bearing output, making it suitable for +/// pre-flight client-side checks and API validation routes. +/// +/// Returns `Ok(())` on valid mnemonics, or [`WalletError::InvalidMnemonic`] if the phrase +/// is syntactically invalid or fails checksum validation. +pub fn validate_seed_phrase(phrase: &str) -> Result<(), WalletError> { + // Validate mnemonic syntax, wordlist membership, and checksum. + Mnemonic::from_phrase(phrase, Language::English) + .map_err(|_| WalletError::InvalidMnemonic)?; + Ok(()) +} + /// A BIP39 seed (the 64-byte output of mnemonic + passphrase), zeroized on drop. pub struct WalletSeed(Zeroizing>); @@ -38,6 +53,8 @@ impl WalletSeed { /// Reconstruct a seed from an existing BIP39 mnemonic phrase (recovery / re-import). pub fn from_phrase(phrase: &str) -> Result { + // Enforce wordlist and checksum validation before constructing secret material. + validate_seed_phrase(phrase)?; let mnemonic = Mnemonic::from_phrase(phrase, Language::English) .map_err(|_| WalletError::InvalidMnemonic)?; let seed = Seed::new(&mnemonic, ""); @@ -56,8 +73,46 @@ impl WalletSeed { /// Derive the 32-byte ed25519 secret key for Stellar account `index` (`m/44'/148'/index'`). /// - /// Returned zeroized; feed it to [`crate::signer`] to build a keypair. + /// # Derivation Path & Invariants + /// + /// Derives according to Stellar's [SEP-0005](https://github.com/stellar/stellar-protocol/blob/master/ecosystem/sep-0005.md) + /// specification using SLIP-0010 ed25519 master-key derivation: + /// + /// - **Path**: `m/44'/148'/index'`, where `44'` is BIP-44 purpose, `148'` is Stellar's + /// SLIP-0044 coin type, and `index'` is the account index. + /// - **All-Hardened Derivation**: Every level in SEP-0005 is strictly hardened (`index | HARDENED`). + /// Unlike EVM's BIP-44 path (`m/44'/60'/0'/0/index`, referenced for contrast in + /// `docs/ethereum-expansion-issues.md`), which permits unhardened derivation at the change and + /// address levels, ed25519 does not safely support unhardened public derivation without + /// compromising key security (leaking an extended public key alongside a single child private + /// key would allow recovering the parent secret key and all sibling keys). Full hardening + /// guarantees that compromise of any derived key cannot compromise parent or sibling keys. + /// - **Valid Index Range**: Hardened indices must fall within `0..2^31` (`0..0x8000_0000`). Values + /// at or above the 2^31 ceiling cannot be hardened without overflowing the 31-bit index space. + /// + /// # Architecture & Deposit Model + /// + /// In Octo's deposit architecture (see `docs/deposit-model.md`), this derivation underpins the + /// muxed-address model: account index 0 is derived as the single master base account (`G...`). + /// Customer funds are multiplexed via 64-bit IDs encoded into SEP-0023 muxed addresses (`M...`), + /// allowing off-chain per-user address allocation with zero on-chain account reserves and no sweeps. + /// Arbitrary index derivation remains available if dedicated on-chain accounts are required. + /// + /// The returned secret is wrapped in [`Zeroizing`] and zeroized on drop. Feed it to + /// [`crate::signer`] to construct a keypair. + /// + /// # Example + /// + /// ```rust + /// use octo_wallet_core::WalletSeed; + /// + /// let mnemonic = "illness spike retreat truth genius clock brain pass fit cave bargain toe"; + /// let seed = WalletSeed::from_phrase(mnemonic).unwrap(); + /// let secret = seed.derive_ed25519_secret(0); + /// assert_eq!(secret.len(), 32); + /// ``` pub fn derive_ed25519_secret(&self, index: u32) -> Zeroizing<[u8; 32]> { + // Derive ed25519 key at hardened path m/44'/148'/index'. let path = [ BIP44_PURPOSE | HARDENED, STELLAR_COIN_TYPE | HARDENED, @@ -124,6 +179,105 @@ mod tests { )); } + // Sourced checksum-invalid and syntactically invalid test vectors. + // + // Sources: + // 1. Trezor python-mnemonic test suite (tests/test_mnemonic.py:test_failed_checksum). + // 2. BIP-39 specification official vectors (bitcoin/bips/bip-0039.mediawiki & + // trezor/python-mnemonic/vectors.json), mutating the final checksum word to an alternative + // wordlist entry ("almost valid" vectors: correct word count and wordlist membership, wrong checksum). + // 3. Grossly invalid vectors (length mismatches, non-wordlist tokens, empty input). + const SOURCED_CHECKSUM_INVALID_VECTORS: &[&str] = &[ + // Trezor python-mnemonic tests/test_mnemonic.py test_failed_checksum + "bless cloud wheel regular tiny venue bird web grief security dignity zoo", + // BIP-39 spec vector 0 (12-word all-zero entropy), mutated checksum word (about -> abandon) + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon", + // BIP-39 spec vector 0 (12-word all-zero entropy), mutated checksum word (about -> zoo) + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon zoo", + // BIP-39 spec vector 1 (12-word all-0x7F entropy), mutated checksum word (yellow -> legal) + "legal winner thank year wave sausage worth useful legal winner thank legal", + // BIP-39 spec vector 3 (12-word all-0xFF entropy), mutated checksum word (wrong -> zoo) + "zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo", + // 15-word phrase, mutated checksum word + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon", + // BIP-39 spec vector 4 (18-word all-zero entropy), mutated checksum word (agent -> abandon) + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon", + // BIP-39 spec vector 7 (24-word all-zero entropy), mutated checksum word (art -> abandon) + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon", + // BIP-39 spec vector 8 (24-word all-0x7F entropy), mutated checksum word (title -> yellow) + "legal winner thank year wave sausage worth useful legal winner thank year wave sausage worth useful legal winner thank year wave sausage worth yellow", + // BIP-39 spec vector 9 (24-word all-0xFF entropy), mutated checksum word (vote -> zoo) + "zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo", + // Grossly invalid: non-wordlist tokens + "not a real mnemonic phrase at all", + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon notaword", + // Grossly invalid: incorrect word counts (11, 13, 25 words) + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon", + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon", + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon", + // Grossly invalid: empty and corrupt strings + "", + " ", + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon 1234", + ]; + + #[test] + fn from_phrase_rejects_every_sourced_checksum_invalid_vector() { + for &vector in SOURCED_CHECKSUM_INVALID_VECTORS { + let res = WalletSeed::from_phrase(vector); + assert!( + matches!(res, Err(WalletError::InvalidMnemonic)), + "from_phrase must reject checksum-invalid vector {vector:?}, got {res:?}" + ); + } + } + + #[test] + fn validate_seed_phrase_accepts_every_from_phrase_accepted_vector() { + let (gen_phrase, _) = WalletSeed::generate(); + let valid_vectors = [ + VECTOR_MNEMONIC, + gen_phrase.as_str(), + "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about", + "legal winner thank year wave sausage worth useful legal winner thank yellow", + "zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo zoo wrong", + ]; + for vector in valid_vectors { + assert!( + validate_seed_phrase(vector).is_ok(), + "validate_seed_phrase rejected valid vector {vector:?}" + ); + assert!( + WalletSeed::from_phrase(vector).is_ok(), + "from_phrase rejected valid vector {vector:?}" + ); + } + } + + #[test] + fn validate_seed_phrase_rejects_every_from_phrase_rejected_vector() { + for &vector in SOURCED_CHECKSUM_INVALID_VECTORS { + let val_res = validate_seed_phrase(vector); + let seed_res = WalletSeed::from_phrase(vector); + assert!( + matches!(val_res, Err(WalletError::InvalidMnemonic)), + "validate_seed_phrase must reject {vector:?}" + ); + assert!( + matches!(seed_res, Err(WalletError::InvalidMnemonic)), + "from_phrase must reject {vector:?}" + ); + } + } + + #[test] + fn validate_seed_phrase_never_returns_secret_material_even_on_success() { + // Assert at type level that validate_seed_phrase produces unit () and holds no secret state. + let result: Result<(), WalletError> = validate_seed_phrase(VECTOR_MNEMONIC); + assert_eq!(result.unwrap(), ()); + assert_eq!(std::mem::size_of::<()>(), 0); + } + proptest! { #[test] fn derivation_is_deterministic_for_any_index( diff --git a/crates/wallet-core/src/lib.rs b/crates/wallet-core/src/lib.rs index 26e2ca3..d589c38 100644 --- a/crates/wallet-core/src/lib.rs +++ b/crates/wallet-core/src/lib.rs @@ -28,7 +28,7 @@ pub use address::{ verify_account_signature, DecodedMuxed, DepositAddress, }; pub use asset::is_valid_asset_code; -pub use derive::WalletSeed; +pub use derive::{validate_seed_phrase, WalletSeed}; pub use error::WalletError; pub use provision::{import_wallet, provision_wallet, ProvisionedWallet}; pub use signer::{