malpercio.dev / ezpds
An easy-to-host PDS on the ATProtocol, MacOS. Grandma-approved.
fork atom

fix(relay): address final PR review issues for MM-92

- Fix 1: Replace wrong sqlx error source for corrupt migration version. Use
Protocol(format!(...)) instead of RowNotFound to accurately describe the
i64-to-u32 conversion failure.

- Fix 2: Remove InvalidKeyId from CryptoError variants list in CLAUDE.md (never
implemented).

- Fix 3: Rename three test functions to match their assertions:
unsupported_algorithm_returns_422, empty_algorithm_returns_422,
missing_algorithm_field_returns_422.

- Fix 4: Add test for null algorithm field. Null deserialization returns 400
(Bad Request) from Axum's default JSON rejection, distinct from missing/invalid
enum variants (422).

- Fix 5: Add key_id context to DB insert error log for better debugging when
signing key persistence fails.

- Fix 6: Add comment explaining why Sensitive<T> has pub T field — deliberate
design choice to make raw value access visible in source for code review.

authored by malpercio.dev and committed by

Tangled 76642f61 3e96165c

+26 -7
+5 -1
crates/common/src/config.rs
··· 3 3 use std::path::PathBuf; 4 4 use zeroize::Zeroizing; 5 5 6 - /// A wrapper that suppresses Debug output for sensitive values. 6 + /// A wrapper that suppresses [`Debug`] output for sensitive values, printing `***` instead. 7 + /// 8 + /// `T` is `pub` to allow deliberate access via `.0` at call sites. This is an explicit choice: 9 + /// any read of the raw value is visible in source, making accidental logging harder to miss in 10 + /// code review. 7 11 #[derive(Clone)] 8 12 pub struct Sensitive<T>(pub T); 9 13
+1 -1
crates/crypto/CLAUDE.md
··· 12 12 - **P256Keypair fields**: `key_id` (full `did:key:z...` URI), `public_key` (multibase base58btc compressed point, no did:key: prefix), `private_key_bytes` (`Zeroizing<[u8; 32]>` -- zeroized on drop) 13 13 - **Encryption format**: `base64(nonce(12) || ciphertext(32) || tag(16))` = 80 base64 chars. Fresh 12-byte nonce from OS RNG per call. 14 14 - **did:key format**: P-256 multicodec varint `[0x80, 0x24]` + compressed public key, multibase base58btc encoded 15 - - **CryptoError variants**: `KeyGeneration`, `Encryption`, `Decryption`, `InvalidKeyId` 15 + - **CryptoError variants**: `KeyGeneration`, `Encryption`, `Decryption` 16 16 17 17 ## Dependencies 18 18 - **Uses**: p256 (ECDSA/key generation), aes-gcm (AES-256-GCM), multibase (base58btc encoding), rand_core (OS RNG), base64 (storage encoding), zeroize (secret cleanup)
+1 -1
crates/relay/src/db/mod.rs
··· 101 101 .map(|(v,)| { 102 102 u32::try_from(v).map_err(|_| DbError::Setup { 103 103 step: "parse migration version from schema_migrations", 104 - source: sqlx::Error::RowNotFound, 104 + source: sqlx::Error::Protocol(format!("version {v} does not fit in u32")), 105 105 }) 106 106 }) 107 107 .collect::<Result<_, _>>()?;
+19 -4
crates/relay/src/routes/create_signing_key.rs
··· 114 114 .execute(&state.db) 115 115 .await 116 116 .map_err(|e| { 117 - tracing::error!(error = %e, "failed to insert relay signing key"); 117 + tracing::error!(error = %e, key_id = %keypair.key_id, "failed to insert relay signing key"); 118 118 ApiError::new(ErrorCode::InternalError, "failed to store signing key") 119 119 })?; 120 120 ··· 352 352 // --- Algorithm tests --- 353 353 354 354 #[tokio::test] 355 - async fn unsupported_algorithm_returns_400() { 355 + async fn unsupported_algorithm_returns_422() { 356 356 // MM-92.AC5.1: serde rejects unknown enum variant with 422 Unprocessable Entity 357 357 let response = app(test_state_with_keys().await) 358 358 .oneshot(post_keys( ··· 365 365 } 366 366 367 367 #[tokio::test] 368 - async fn empty_algorithm_returns_400() { 368 + async fn empty_algorithm_returns_422() { 369 369 // MM-92.AC5.2: serde rejects empty string for enum variant with 422 Unprocessable Entity 370 370 let response = app(test_state_with_keys().await) 371 371 .oneshot(post_keys(r#"{"algorithm": ""}"#, Some("test-admin-token"))) ··· 375 375 } 376 376 377 377 #[tokio::test] 378 - async fn missing_algorithm_field_returns_400() { 378 + async fn missing_algorithm_field_returns_422() { 379 379 // MM-92.AC5.3: missing required field returns 422 Unprocessable Entity 380 380 let response = app(test_state_with_keys().await) 381 381 .oneshot(post_keys(r#"{}"#, Some("test-admin-token"))) 382 382 .await 383 383 .unwrap(); 384 384 assert_eq!(response.status(), StatusCode::UNPROCESSABLE_ENTITY); 385 + } 386 + 387 + #[tokio::test] 388 + async fn null_algorithm_returns_400() { 389 + // null is a distinct JSON token that some API clients send for unset fields. 390 + // Unlike missing/invalid enum variants (422), Axum's default JSON rejection treats 391 + // null deserialization as a general Bad Request (400). 392 + let response = app(test_state_with_keys().await) 393 + .oneshot(post_keys( 394 + r#"{"algorithm": null}"#, 395 + Some("test-admin-token"), 396 + )) 397 + .await 398 + .unwrap(); 399 + assert_eq!(response.status(), StatusCode::BAD_REQUEST); 385 400 } 386 401 387 402 // --- Master key test ---