An easy-to-host PDS on the ATProtocol, iPhone and MacOS. Maintain control of your keys and data, always.
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

fix(relay): register V010/V011 migrations; fix retry test precondition; add share assertions

Blocking fixes from second review pass:
- Register V010 (recovery_share) and V011 (pending_share_{1,2,3}) in migration runner;
missing registration meant all route tests hit 500 (column not found) in CI
- Fix retry test broken precondition: pre-store all three shares alongside pending_did so
the retry branch doesn't 500 on NULL share columns; assert returned shares match
pre-stored values, proving idempotent reuse rather than fresh generation
- Add shamir_share_1 / shamir_share_3 assertions to happy-path test (presence, 52-char,
BASE32 alphabet); add recovery_share DB assertion (NOT NULL, 52-char, BASE32 alphabet)

Minor fixes:
- Fix doc comment: shamirShare1/3 → shamir_share_1/3 (actual wire field names)
- Add comment in generate_recovery_shares noting base32 String outputs are not zeroized
- Add console.error in ShamirBackupScreen clipboard catch for debug visibility

authored by

Malpercio and committed by
Tangled
e6b0518c e94bccd7

+77 -14
+2 -1
apps/identity-wallet/src/lib/components/onboarding/ShamirBackupScreen.svelte
··· 41 41 setTimeout(() => { 42 42 copied = false; 43 43 }, 2000); 44 - } catch { 44 + } catch (e) { 45 45 // Clipboard denied or bridge error — show failure so the user knows to use another method. 46 + console.error('clipboard write failed:', e); 46 47 copyFailed = true; 47 48 setTimeout(() => { 48 49 copyFailed = false;
+8
crates/relay/src/db/mod.rs
··· 64 64 version: 9, 65 65 sql: include_str!("migrations/V009__sessions_v2.sql"), 66 66 }, 67 + Migration { 68 + version: 10, 69 + sql: include_str!("migrations/V010__recovery_shares.sql"), 70 + }, 71 + Migration { 72 + version: 11, 73 + sql: include_str!("migrations/V011__pending_shares.sql"), 74 + }, 67 75 ]; 68 76 69 77 /// Open a WAL-mode SQLite connection pool with a maximum of 1 connection.
+67 -13
crates/relay/src/routes/create_did.rs
··· 34 34 // DELETE devices WHERE account_id = ? 35 35 // DELETE pending_accounts WHERE id = ? 36 36 // 13. Return { "did", "did_document", "status": "active", "session_token", 37 - // "shamirShare1": <base32>, "shamirShare3": <base32> } 37 + // "shamir_share_1": <base32>, "shamir_share_3": <base32> } 38 38 // 39 39 // Note: handles are NOT inserted here. Handle creation is the caller's responsibility 40 40 // via POST /v1/handles, which validates format and optionally creates DNS records. 41 41 // 42 42 // Outputs (success): 200 { "did": "...", "did_document": {...}, "status": "active", 43 - // "session_token": "...", "shamirShare1": "...", "shamirShare3": "..." } 43 + // "session_token": "...", "shamir_share_1": "...", "shamir_share_3": "..." } 44 44 // Outputs (error): 400 INVALID_CLAIM, 401 UNAUTHORIZED, 409 DID_ALREADY_EXISTS, 45 45 // 502 PLC_DIRECTORY_ERROR, 500 INTERNAL_ERROR 46 46 ··· 347 347 ApiError::new(ErrorCode::InternalError, "failed to split recovery secret") 348 348 })?; 349 349 350 + // The raw 32-byte secret is cleared on drop via Zeroizing. The base32 String 351 + // outputs are plain heap allocations; String does not implement Zeroize. 350 352 Ok(( 351 353 BASE32_NOPAD.encode(s1.data.as_ref()), 352 354 BASE32_NOPAD.encode(s2.data.as_ref()), ··· 737 739 "response should include a non-empty session_token" 738 740 ); 739 741 742 + // shamir_share_1 and shamir_share_3 must be present, 52-char BASE32 (A-Z, 2-7). 743 + // Missing fields or wrong alphabet here means a rename or swap would go undetected. 744 + let share1 = body["shamir_share_1"] 745 + .as_str() 746 + .expect("shamir_share_1 missing from response"); 747 + let share3 = body["shamir_share_3"] 748 + .as_str() 749 + .expect("shamir_share_3 missing from response"); 750 + assert_eq!(share1.len(), 52, "shamir_share_1 should be 52 chars"); 751 + assert_eq!(share3.len(), 52, "shamir_share_3 should be 52 chars"); 752 + assert!( 753 + share1.chars().all(|c| matches!(c, 'A'..='Z' | '2'..='7')), 754 + "shamir_share_1 should be valid BASE32 (A-Z, 2-7), got: {share1}" 755 + ); 756 + assert!( 757 + share3.chars().all(|c| matches!(c, 'A'..='Z' | '2'..='7')), 758 + "shamir_share_3 should be valid BASE32 (A-Z, 2-7), got: {share3}" 759 + ); 760 + 740 761 let did = body["did"].as_str().unwrap(); 741 762 let doc = &body["did_document"]; 742 763 ··· 768 789 "serviceEndpoint should match config.public_url" 769 790 ); 770 791 771 - // accounts row with correct did, email; password_hash IS NULL 772 - let row: Option<(String, Option<String>)> = 773 - sqlx::query_as("SELECT email, password_hash FROM accounts WHERE did = ?") 792 + // accounts row with correct did, email; password_hash IS NULL; recovery_share persisted. 793 + let row: Option<(String, Option<String>, Option<String>)> = 794 + sqlx::query_as("SELECT email, password_hash, recovery_share FROM accounts WHERE did = ?") 774 795 .bind(did) 775 796 .fetch_optional(&db) 776 797 .await 777 798 .unwrap(); 778 - let (email, password_hash) = row.expect("accounts row should exist"); 799 + let (email, password_hash, recovery_share) = row.expect("accounts row should exist"); 779 800 assert!(email.contains("alice"), "email should match test account"); 780 801 assert!( 781 802 password_hash.is_none(), 782 803 "password_hash should be NULL for device-provisioned account" 783 804 ); 805 + let rs = recovery_share 806 + .as_deref() 807 + .expect("recovery_share should not be NULL — Share 2 must be stored for relay custody"); 808 + assert_eq!(rs.len(), 52, "recovery_share should be 52 chars"); 809 + assert!( 810 + rs.chars().all(|c| matches!(c, 'A'..='Z' | '2'..='7')), 811 + "recovery_share should be valid BASE32 (A-Z, 2-7), got: {rs}" 812 + ); 784 813 785 814 // did_documents row exists with non-empty document 786 815 let doc_row: Option<(String,)> = ··· 870 899 ) 871 900 .expect("verify should succeed"); 872 901 873 - // Pre-set pending_did to simulate a retry scenario. 874 - sqlx::query("UPDATE pending_accounts SET pending_did = ? WHERE id = ?") 875 - .bind(&verified.did) 876 - .bind(&setup.account_id) 877 - .execute(&db) 878 - .await 879 - .expect("pre-store pending_did"); 902 + // Pre-set pending_did and all three shares to simulate a retry scenario. 903 + // The retry branch in pre_store_did_and_shares requires all share columns to be 904 + // non-NULL; leaving them NULL would cause a 500 instead of 200. 905 + let pre_share_1 = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; 906 + let pre_share_2 = "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; 907 + let pre_share_3 = "CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC"; 908 + sqlx::query( 909 + "UPDATE pending_accounts \ 910 + SET pending_did = ?, pending_share_1 = ?, pending_share_2 = ?, pending_share_3 = ? \ 911 + WHERE id = ?", 912 + ) 913 + .bind(&verified.did) 914 + .bind(pre_share_1) 915 + .bind(pre_share_2) 916 + .bind(pre_share_3) 917 + .bind(&setup.account_id) 918 + .execute(&db) 919 + .await 920 + .expect("pre-store pending_did and shares"); 880 921 881 922 let app = crate::app::app(state); 882 923 let response = app ··· 897 938 body["did"].as_str(), 898 939 Some(verified.did.as_str()), 899 940 "did should match pre-computed DID" 941 + ); 942 + // Shares returned must be the pre-stored ones, not freshly generated ones. 943 + // This is the core invariant of idempotent share storage: retrying the ceremony 944 + // must return the same shares so Share 2 in accounts.recovery_share stays consistent. 945 + assert_eq!( 946 + body["shamir_share_1"].as_str(), 947 + Some(pre_share_1), 948 + "retry should return pre-stored share 1, not a new one" 949 + ); 950 + assert_eq!( 951 + body["shamir_share_3"].as_str(), 952 + Some(pre_share_3), 953 + "retry should return pre-stored share 3, not a new one" 900 954 ); 901 955 // wiremock verifies expect(0) on mock_server drop 902 956 }