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: address PR review feedback for get_did

- Add DID format validation to prevent path traversal into PLC URL
- Trim trailing slash from plc_directory_url before constructing path
- Log truncated raw document string when DB JSON is malformed
- Log truncated PLC response body on non-success status
- Add tracing::debug at PLC fallback and 404 branch
- Fix Processes/Returns header comments; add caching note
- Add 6 new tests: invalid DID, malformed DB JSON, network failure,
non-404 4xx from PLC, invalid JSON body from PLC, trailing-slash URL

authored by

Malpercio and committed by
Tangled
e43bd511 c705fbd9

+167 -10
+4 -3
crates/relay/src/db/dids.rs
··· 1 1 // pattern: Imperative Shell 2 2 // 3 - // DID document lookup. Returns a parsed JSON document from the did_documents table. 4 - // No business logic — callers decide what to do with the result. 3 + // DID document lookup queries against the did_documents table. 4 + // Returns plain data structs; no business logic — callers decide what to do with the result. 5 5 6 6 use common::{ApiError, ErrorCode}; 7 7 ··· 26 26 None => Ok(None), 27 27 Some((doc_str,)) => { 28 28 let doc = serde_json::from_str(&doc_str).map_err(|e| { 29 - tracing::error!(did = %did, error = %e, "malformed DID document in DB"); 29 + let preview = &doc_str[..doc_str.len().min(500)]; 30 + tracing::error!(did = %did, error = %e, raw = %preview, "malformed DID document in DB"); 30 31 ApiError::new(ErrorCode::InternalError, "malformed DID document") 31 32 })?; 32 33 Ok(Some(doc))
+163 -7
crates/relay/src/routes/get_did.rs
··· 1 1 // pattern: Imperative Shell 2 2 // 3 3 // Gathers: DID from path parameter, document from local cache or PLC directory proxy 4 - // Processes: none (lookup priority is local did_documents → PLC directory) 5 - // Returns: DID document JSON with 200, or 404 if not found anywhere 4 + // Processes: DID format validation (rejects strings that don't start with "did:") 5 + // Returns: DID document JSON with 200; 400 on invalid DID format; 404 if not found 6 + // in local cache or PLC directory; 502 on PLC infrastructure errors 6 7 7 8 use axum::{ 8 9 extract::{Path, State}, ··· 18 19 Path(did): Path<String>, 19 20 State(state): State<AppState>, 20 21 ) -> Result<Json<Value>, ApiError> { 21 - // 1. Check local cache. 22 + // Reject strings that don't look like DIDs to prevent path traversal into the 23 + // upstream PLC URL (e.g. "../../other-path" appended to plc_directory_url). 24 + if !did.starts_with("did:") { 25 + return Err(ApiError::new(ErrorCode::InvalidClaim, "invalid DID format")); 26 + } 27 + 28 + // 1. Check local cache. DB errors propagate as 500 — we do NOT fall through to 29 + // PLC on infrastructure failures; that would mask broken rows. 22 30 if let Some(doc) = get_did_document(&state.db, &did).await? { 23 31 return Ok(Json(doc)); 24 32 } 25 33 26 - // 2. Proxy to PLC directory. 27 - let plc_url = format!("{}/{}", state.config.plc_directory_url, did); 34 + // 2. Proxy to PLC directory. Responses are not cached locally — the caller 35 + // receives the live document but it is not written to did_documents here. 36 + tracing::debug!(did = %did, "DID not in local cache; proxying to plc.directory"); 37 + let plc_url = format!( 38 + "{}/{}", 39 + state.config.plc_directory_url.trim_end_matches('/'), 40 + did 41 + ); 28 42 let response = state 29 43 .http_client 30 44 .get(&plc_url) ··· 36 50 })?; 37 51 38 52 if response.status() == reqwest::StatusCode::NOT_FOUND { 53 + tracing::debug!(did = %did, "DID not found in plc.directory"); 39 54 return Err(ApiError::new(ErrorCode::NotFound, "DID not found")); 40 55 } 41 56 42 57 if !response.status().is_success() { 43 58 let status = response.status(); 44 - tracing::error!(did = %did, status = %status, "plc.directory returned error"); 59 + let body_preview = response.text().await.unwrap_or_default(); 60 + let truncated = &body_preview[..body_preview.len().min(500)]; 61 + tracing::error!(did = %did, status = %status, response_body = %truncated, "plc.directory returned error"); 45 62 return Err(ApiError::new( 46 63 ErrorCode::PlcDirectoryError, 47 64 "plc.directory returned error", ··· 77 94 .unwrap() 78 95 } 79 96 97 + // ── Input validation ────────────────────────────────────────────────────── 98 + 99 + #[tokio::test] 100 + async fn invalid_did_format_returns_400() { 101 + let state = test_state_with_plc_url("https://plc.directory".to_string()).await; 102 + 103 + let response = app(state) 104 + .oneshot(get_did_request("notadid")) 105 + .await 106 + .unwrap(); 107 + 108 + assert_eq!(response.status(), StatusCode::BAD_REQUEST); 109 + let body = body_json(response).await; 110 + assert_eq!(body["error"]["code"], "INVALID_CLAIM"); 111 + } 112 + 80 113 // ── Local cache hit ─────────────────────────────────────────────────────── 81 114 82 115 #[tokio::test] ··· 103 136 assert_eq!(body["alsoKnownAs"][0], "at://alice.test"); 104 137 } 105 138 139 + #[tokio::test] 140 + async fn malformed_json_in_db_returns_500() { 141 + let state = test_state_with_plc_url("https://plc.directory".to_string()).await; 142 + let did = "did:plc:malformedjsontest12345678901"; 143 + 144 + // Bypass seed_did_document to insert invalid JSON directly. 145 + sqlx::query( 146 + "INSERT INTO did_documents (did, document, created_at, updated_at) \ 147 + VALUES (?, 'not valid json', datetime('now'), datetime('now'))", 148 + ) 149 + .bind(did) 150 + .execute(&state.db) 151 + .await 152 + .unwrap(); 153 + 154 + let response = app(state) 155 + .oneshot(get_did_request(did)) 156 + .await 157 + .unwrap(); 158 + 159 + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); 160 + let body = body_json(response).await; 161 + assert_eq!(body["error"]["code"], "INTERNAL_ERROR"); 162 + } 163 + 106 164 // ── PLC directory proxy ─────────────────────────────────────────────────── 107 165 108 166 #[tokio::test] ··· 163 221 } 164 222 165 223 #[tokio::test] 166 - async fn plc_directory_error_returns_502() { 224 + async fn plc_directory_5xx_returns_502() { 167 225 let mock_server = MockServer::start().await; 168 226 let did = "did:plc:errordid12345678901234567"; 169 227 ··· 187 245 assert_eq!(body["error"]["code"], "PLC_DIRECTORY_ERROR"); 188 246 } 189 247 248 + #[tokio::test] 249 + async fn plc_directory_4xx_non_404_returns_502() { 250 + let mock_server = MockServer::start().await; 251 + let did = "did:plc:ratelimitedtest1234567890123"; 252 + 253 + Mock::given(method("GET")) 254 + .and(path_regex(r"^/did:plc:.+")) 255 + .respond_with(ResponseTemplate::new(429)) 256 + .expect(1) 257 + .named("plc.directory 429") 258 + .mount(&mock_server) 259 + .await; 260 + 261 + let state = test_state_with_plc_url(mock_server.uri()).await; 262 + 263 + let response = app(state) 264 + .oneshot(get_did_request(did)) 265 + .await 266 + .unwrap(); 267 + 268 + assert_eq!(response.status(), StatusCode::BAD_GATEWAY); 269 + let body = body_json(response).await; 270 + assert_eq!(body["error"]["code"], "PLC_DIRECTORY_ERROR"); 271 + } 272 + 273 + #[tokio::test] 274 + async fn plc_returns_non_json_body_returns_502() { 275 + let mock_server = MockServer::start().await; 276 + let did = "did:plc:htmlresponsedid12345678901234"; 277 + 278 + Mock::given(method("GET")) 279 + .and(path_regex(r"^/did:plc:.+")) 280 + .respond_with( 281 + ResponseTemplate::new(200) 282 + .set_body_string("<html><body>Not JSON</body></html>") 283 + .append_header("content-type", "text/html"), 284 + ) 285 + .expect(1) 286 + .named("plc.directory html response") 287 + .mount(&mock_server) 288 + .await; 289 + 290 + let state = test_state_with_plc_url(mock_server.uri()).await; 291 + 292 + let response = app(state) 293 + .oneshot(get_did_request(did)) 294 + .await 295 + .unwrap(); 296 + 297 + assert_eq!(response.status(), StatusCode::BAD_GATEWAY); 298 + let body = body_json(response).await; 299 + assert_eq!(body["error"]["code"], "PLC_DIRECTORY_ERROR"); 300 + } 301 + 302 + #[tokio::test] 303 + async fn plc_network_failure_returns_502() { 304 + // Connect to a port that refuses connections. Port 1 (tcpmux) is not in use 305 + // on standard developer machines and returns ECONNREFUSED immediately. 306 + let state = test_state_with_plc_url("http://127.0.0.1:1".to_string()).await; 307 + 308 + let response = app(state) 309 + .oneshot(get_did_request("did:plc:networkfailuretest1234567890")) 310 + .await 311 + .unwrap(); 312 + 313 + assert_eq!(response.status(), StatusCode::BAD_GATEWAY); 314 + let body = body_json(response).await; 315 + assert_eq!(body["error"]["code"], "PLC_DIRECTORY_ERROR"); 316 + } 317 + 190 318 // ── Local cache priority ────────────────────────────────────────────────── 191 319 192 320 #[tokio::test] ··· 221 349 assert_eq!(response.status(), StatusCode::OK); 222 350 let body = body_json(response).await; 223 351 assert_eq!(body["alsoKnownAs"][0], "at://local.test"); 352 + } 353 + 354 + // ── URL construction ────────────────────────────────────────────────────── 355 + 356 + #[tokio::test] 357 + async fn plc_url_with_trailing_slash_produces_correct_path() { 358 + let mock_server = MockServer::start().await; 359 + let did = "did:plc:trailingslashtest1234567890"; 360 + let plc_doc = json!({"id": did, "@context": [], "alsoKnownAs": [], "verificationMethod": [], "service": []}); 361 + 362 + Mock::given(method("GET")) 363 + .and(path_regex(r"^/did:plc:.+")) 364 + .respond_with(ResponseTemplate::new(200).set_body_json(&plc_doc)) 365 + .expect(1) 366 + .named("plc.directory trailing slash") 367 + .mount(&mock_server) 368 + .await; 369 + 370 + // URI with trailing slash — should not produce a double-slash path. 371 + let plc_url_with_slash = format!("{}/", mock_server.uri()); 372 + let state = test_state_with_plc_url(plc_url_with_slash).await; 373 + 374 + let response = app(state) 375 + .oneshot(get_did_request(did)) 376 + .await 377 + .unwrap(); 378 + 379 + assert_eq!(response.status(), StatusCode::OK); 224 380 } 225 381 }