mirror_worker: address lbaquerofierro review on PR 273

- Return the Display form of a malformed reference-checkpoint parse
  error, matching add-checkpoint, so the Rust type name stays out of the
  400 body.
- Reuse tlog_witness::MAX_REQUEST_BODY_SIZE for the sign-subtree body cap
  instead of duplicating the constant the parser already enforces.
- Log the previously-silent rejections (unknown origin 404, subtree
  bounds 400s, consistency-proof 422) with the subtree range and
  checkpoint size, so operators can tell which check failed.
- Map the unexpected verify errors (MismatchedVerifier/AmbiguousKey,
  unreachable over a one-element verifier list) to 500 rather than
  blaming the client with 400.

Author: lukevalenta

crates/mirror_worker/src/frontend_worker.rs

@@ -45,8 +45,8 @@ use signed_note::{NoteError, NoteVerifier, VerifierList};
 use tlog_checkpoint::{CheckpointSigner as _, CheckpointText};
 use tlog_core::{Subtree, verify_subtree_consistency_proof};
 use tlog_witness::{
-    AddCheckpointRequest, CONTENT_TYPE_TLOG_SIZE, SignSubtreeRequest, parse_add_checkpoint_request,
-    parse_sign_subtree_request, serialize_sign_subtree_response,
+    AddCheckpointRequest, CONTENT_TYPE_TLOG_SIZE, MAX_REQUEST_BODY_SIZE, SignSubtreeRequest,
+    parse_add_checkpoint_request, parse_sign_subtree_request, serialize_sign_subtree_response,
 };
 use tower_service::Service as _;
 #[allow(clippy::wildcard_imports)]
@@ -87,7 +87,7 @@ async fn fetch(
             )
             .route(
                 "/sign-subtree",
-                post(sign_subtree).layer(DefaultBodyLimit::max(MAX_SIGN_SUBTREE_BODY_SIZE)),
+                post(sign_subtree).layer(DefaultBodyLimit::max(MAX_REQUEST_BODY_SIZE)),
             )
             .route("/metadata", get(metadata))
             .route("/", get(root))
@@ -386,25 +386,33 @@ async fn sign_subtree(State(env): State<Env>, body: Bytes) -> ApiResult<axum::re
         Ok(t) => t,
         Err(e) => {
             log::warn!("sign-subtree: malformed checkpoint text: {e:?}");
-            return Err(AppError::BadRequest(format!("{e:?}")));
+            return Err(AppError::BadRequest(e.to_string()));
         }
     };
     if subtree_end > cp_text.size() {
+        log::info!(
+            "sign-subtree: subtree end {subtree_end} exceeds checkpoint size {}",
+            cp_text.size()
+        );
         return Err(AppError::BadRequest(format!(
             "subtree end {subtree_end} > checkpoint size {}",
             cp_text.size()
         )));
     }
     let subtree = match Subtree::new(subtree_start, subtree_end) {
         Ok(s) => s,
-        Err(e) => return Err(AppError::BadRequest(format!("invalid subtree: {e:?}"))),
+        Err(e) => {
+            log::info!("sign-subtree: invalid subtree [{subtree_start}, {subtree_end}): {e:?}");
+            return Err(AppError::BadRequest(format!("invalid subtree: {e:?}")));
+        }
     };
 
     // Look up the log by its origin. Subtree DoS-protection cosignatures
     // in the request are ignored: this implementation applies no
     // pre-screening policy, as the spec leaves their use to the operator.
     let origin = cp_text.origin();
     if log_verifiers(origin).is_none() {
+        log::info!("sign-subtree: unknown log origin {origin:?}");
         return Err(AppError::UnknownLogOrigin);
     }
 
@@ -420,9 +428,13 @@ async fn sign_subtree(State(env): State<Env>, body: Bytes) -> ApiResult<axum::re
                 log::info!("sign-subtree: reference checkpoint not cosigned by this mirror: {e:?}");
                 return Err(AppError::ReferenceCheckpointNotCosignedByThisMirror);
             }
+            // `MismatchedVerifier`/`AmbiguousKey` mean the verifier list
+            // we built is malformed, not that the client sent a bad
+            // request; over a one-element list they are unreachable
+            // today. Surface them as 500 rather than blaming the client.
             _ => {
-                log::warn!("sign-subtree: checkpoint verify failed: {e:?}");
-                return Err(AppError::BadRequest(e.to_string()));
+                log::error!("sign-subtree: checkpoint verify failed unexpectedly: {e:?}");
+                return Err(AppError::InternalServerError(e.to_string()));
             }
         }
     }
@@ -438,6 +450,11 @@ async fn sign_subtree(State(env): State<Env>, body: Bytes) -> ApiResult<axum::re
     )
     .is_err()
     {
+        log::info!(
+            "sign-subtree: consistency proof failed for subtree [{subtree_start}, {subtree_end}) \
+             against checkpoint size {}",
+            cp_text.size()
+        );
         return Err(AppError::UnprocessableEntity(
             "subtree consistency proof failed".to_owned(),
         ));
@@ -458,14 +475,6 @@ async fn sign_subtree(State(env): State<Env>, body: Bytes) -> ApiResult<axum::re
 // Helpers
 // ---------------------------------------------------------------------------
 
-/// Maximum `sign-subtree` request body, enforced by the route's
-/// [`DefaultBodyLimit`] layer. A well-formed request is a `subtree` range
-/// line, a base64 hash line, up to 8 subtree cosignature lines (~3.3 KiB
-/// each for ML-DSA-44), up to 63 base64 consistency-proof hash lines, and
-/// a reference checkpoint of up to `signed_note::MAX_NOTE_SIZE` (1 MiB);
-/// 1 MiB plus 64 KiB of headroom covers that.
-const MAX_SIGN_SUBTREE_BODY_SIZE: usize = 1_024 * 1_024 + 64 * 1_024;
-
 /// Maximum `add-checkpoint` request body, enforced by the route's
 /// [`DefaultBodyLimit`] layer. A well-formed request is an `old <N>`
 /// line, up to 63 base64 hash lines, and a checkpoint note of up to