Add ML-DSA-44 / sign-subtree support to witness_worker

[lukevalenta]

Summary

Wires the witness_worker to produce ML-DSA-44 subtree/v1 cosignatures and exposes the optional POST /sign-subtree endpoint when the deployed signing key is ML-DSA-44. Algorithm dispatch is driven entirely by the OID embedded in the WITNESS_SIGNING_KEY PKCS#8 PEM secret:

• id-Ed25519 → cosignature/v1 signer; /sign-subtree returns 404.
• id-ml-dsa-44 → subtree/v1 signer; /sign-subtree is wired up and /add-checkpoint returns a subtree/v1 cosignature covering the entire submitted tree (start = 0, end = checkpoint size), per the spec.

Operators choose the algorithm purely by which key they generate; there is no separate config field. The OID is the single source of truth.

Stack

• Add subtree_v1 ML-DSA-44 cosignatures to tlog_cosignature #227 (SubtreeV1CheckpointSigner in tlog_cosignature) — merged. Currently pulled in via a merge commit at the base of this branch; will fold out cleanly when this PR is rebased onto main.
• Add sign-subtree wire format to tlog_witness crate #228 (sign_subtree wire-format parser/serializer in tlog_witness) — open. This PR will rebase onto main once Add sign-subtree wire format to tlog_witness crate #228 lands.

What's in this PR

• New WitnessSigner enum with one variant per algorithm. Both variants box their inner signer because the expanded ML-DSA-44 key is ~64 KiB and even the Ed25519 signer is ~470 bytes — both large enough to merit indirection.
• build_witness_signer parses the PEM as a SecretDocument, dispatches on PrivateKeyInfoRef::algorithm.oid, and returns the appropriate signer. Unknown OIDs produce an operator-readable error mentioning both supported OIDs.
• The /sign-subtree handler uses stateless verification (the spec lists three strategies; this is the simplest): the submitted reference checkpoint must carry one of the witness's own past subtree/v1 cosignatures. The handler doc-comment explicitly notes the trust-window inheritance from /add-checkpoint (compromise of the witness key forges both endpoints in lockstep).
• .dev.vars switched to ML-DSA-44 (deterministic seed [0x42; 32], repo-public, dev-only).
• config.dev.json unchanged — the log key remains Ed25519. The witness verifies log signatures with Ed25519 regardless of its own signing algorithm; only the witness's output changes.
• Pinned the dev log SPKI and the dev witness PEM in witness_worker::dev_config_tests so a rotation breaks closed. Integration tests learn the witness public key from /metadata at runtime; they don't carry their own copy of the witness PEM.
• Both MAX_*_BODY_SIZE per-handler caps now have a comment block noting the Workers platform cap (100 MB) bounds peak memory at the runtime level; our caps tighten that to spec-shaped maxima.

Tests

• 4 new unit tests in witness_worker covering OID dispatch (Ed25519, ML-DSA-44, unsupported P-256, malformed PEM).
• 1 new dev_vars_witness_key_matches_embedded_pem test pinning the .dev.vars value against an embedded PKCS#8 PEM constant.
• 4 new integration test steps exercising /sign-subtree: happy path (cosign a subtree of a previously witness-cosigned checkpoint and verify the response), 403 when the reference checkpoint isn't cosigned by the witness, 400 when end > checkpoint.size, and 400 when start == end (Subtree::new rejection path).
• The existing add-checkpoint integration tests now verify the response with a SubtreeV1NoteVerifier instead of the Ed25519 one. End-to-end run against wrangler dev passes.

Out of scope (potential follow-ups)

• Subtree-cosignature DoS-protection lines (the optional cosig lines on a /sign-subtree request) are accepted on the wire but currently ignored. The handler relies on the witness's own past cosignature alone.
• Stateful and recent-cache verification strategies for /sign-subtree (the spec lists three; we implement only the stateless one).
• Cold-start deduplication of the PKCS#8 parse + ML-DSA-44 key expansion would need OnceLock::get_or_try_init (unstable, rust-lang/rust#109737) or a non-stdlib dep; the current code keeps the redundant-work pattern from Replace OnceCell::get_or_try_init with deadlock-safe non-blocking load_roots #205 with a brief comment noting the situation.
• Caching of signed subtrees, rate-limiting, etc. — runtime concerns out of scope for the wire-format change.

[lukevalenta]

/bonk review

[ask-bonk]

Review of #229 — witness_worker: add ML-DSA-44 / sign-subtree support

Solid, well-scoped change. Algorithm dispatch off the PKCS#8 OID is the right design (no separate config field to drift), and WitnessSigner cleanly factors the per-algorithm divergence so the add-checkpoint path stays algorithm-agnostic. Pre-push checks (cargo build, cargo clippy --workspace --all-targets -- -Dwarnings -Dclippy::pedantic, cargo fmt --all --check, full cargo test) all pass locally; the 4 new dispatch unit tests + the new dev_vars_witness_key_matches_embedded_pem pin all pass.

Findings below; nothing blocks merge — one stale doc comment, one stale doc reference, one observation about a load-time race that's harmless on Workers but worth a comment.

---

1. Stale "Ed25519" doc comment on MetadataResponse::witness_public_key — crates/witness_worker/src/frontend_worker.rs:96

/// DER-encoded `SubjectPublicKeyInfo` for the witness's Ed25519 key.
#[serde_as(as = "Base64As")]
witness_public_key: &'a [u8],

After this PR, the SPKI here is whatever algorithm WITNESS_SIGNING_KEY carries — Ed25519 or ML-DSA-44. Suggest:

/// DER-encoded `SubjectPublicKeyInfo` for the witness's verifying key.
/// The algorithm matches whatever `WITNESS_SIGNING_KEY` was loaded with
/// (Ed25519 or ML-DSA-44, see [`WitnessSigner`](crate::WitnessSigner)).

This is the field surfaced by /metadata, so external clients (the integration tests already do this on line 299) need to know the SPKI may not be Ed25519.

2. Doc comment in dev_config_tests overstates the contract — crates/witness_worker/src/lib.rs:393-401, 427-431

The new module-level doc says:

.dev.vars carries the witness's ML-DSA-44 signing key. The integration tests embed the matching PEM as a constant. We pin both to a fixed PKCS#8 byte string…

and the DEV_WITNESS_SIGNING_KEY_PEM doc says:

MUST match the constant in crates/integration_tests/tests/tlog_witness.rs and the value of WITNESS_SIGNING_KEY in .dev.vars.

But crates/integration_tests/tests/tlog_witness.rs does not embed the witness PEM as a constant — it derives the witness verifier from meta.witness_public_key returned by /metadata (line 299). Only .dev.vars and the DEV_WITNESS_SIGNING_KEY_PEM constant in this same module need to stay in sync. Recommend trimming the doc to reflect what's actually pinned (just .dev.vars ↔ this constant).

This is important because someone reading the comment and rotating the seed will go looking for an integration-test PEM that doesn't exist and may believe the test surface is broken.

3. OnceLock load races allocate twice on cold start — crates/witness_worker/src/lib.rs:223-230

pub(crate) fn load_witness_signer(env: &Env) -> Result<&'static WitnessSigner> {
    if let Some(s) = WITNESS_SIGNER.get() { return Ok(s); }
    let pem = env.secret("WITNESS_SIGNING_KEY")?.to_string();
    let signer = build_witness_signer(&pem)?;        // ← always runs on cold start
    Ok(WITNESS_SIGNER.get_or_init(|| signer))
}

If two requests hit a freshly-spun isolate concurrently they'll both run build_witness_signer, which on the ML-DSA-44 path allocates an ~64 KiB expanded signing key (per the doc comment on WitnessSigner) plus the full PEM parse, and the loser drops it. Cloudflare Workers isolates run JS-thread requests cooperatively, so contention here is rare but not impossible across separate fetch events that yield. OnceLock::get_or_try_init would let you do the parse inside the lock and avoid the double-allocation entirely, at the cost of a tiny bit more closure boilerplate. Not a correctness issue — just worth either fixing or adding a one-line comment that the redundant work is intentional.

4. sign-subtree body limit is enforced after buffering — crates/witness_worker/src/frontend_worker.rs:309-318

let body = req.bytes().await?;
if body.len() > MAX_SIGN_SUBTREE_BODY_SIZE { return Response::error(...); }

Same shape as add-checkpoint, so this matches the existing pattern in the file and the Workers runtime imposes its own limit upstream. Fine as defense-in-depth, not a finding — just flagging that the cap doesn't bound peak memory.

5. Subtree validity check ordering looks correct (sanity-checked) — crates/witness_worker/src/frontend_worker.rs:351-371

The handler's comment claims parse_sign_subtree_request enforces no leading zeros but not start < end, and that Subtree::new covers the rest. Verified:

• tlog_witness/src/sign_subtree.rs:323 parse_subtree_range_line only parses decimals; no start < end check.
• tlog_tiles/src/tlog.rs:833 Subtree::new returns ConditionNotMet("lo < hi") for start >= end and a separate alignment check.
• The subtree_end > cp_text.size() early-out is needed because Subtree::new doesn't take size; otherwise a start = 0, end = 1_000_000 request against a size-5 tree would only fail later inside verify_subtree_consistency_proof. Good defensive ordering.

The integration test (13) exercises the end > size path explicitly. There's no test for start >= end (would land in the Subtree::new branch → 400 with "invalid subtree"); the existing wire-format tests in tlog_witness cover the parser side. Adding one more integration step for start = end would round out the boundary coverage but is not required.

6. Stateless verification scope is documented and bounded — informational

The handler intentionally checks only the witness's own past subtree/v1 cosignature on the reference checkpoint, not any of the optional subtree-cosignatures-from-other-witnesses lines, and the PR description and code comment both call this out as the simplest of the three strategies the spec lists. The witness also doesn't re-verify the original log signature on the reference checkpoint. That's safe because the witness only ever produces a self-cosignature inside add-checkpoint, which itself requires a valid log signature, so a checkpoint carrying this witness's subtree/v1 line is guaranteed to have been log-signed at cosign time. Worth keeping in mind that /sign-subtree therefore inherits the trust window of /add-checkpoint — if the witness's signing key is ever compromised, both endpoints become forgeable in lockstep, which matches the spec's threat model.

---

Tests and process

• 4 new unit tests in witness_worker::tests (Ed25519 dispatch, ML-DSA-44 dispatch, P-256 rejected, malformed PEM rejected) + 1 new dev_config_tests::dev_vars_witness_key_matches_embedded_pem. All 12 tests in the crate pass (cargo test -p witness_worker).
• 3 new integration steps (tlog_witness::tlog_witness_end_to_end) for /sign-subtree happy-path, 403, and 400. Per AGENTS.md these require a running wrangler dev and aren't in the default cargo test set; the PR description states the end-to-end run passes.
• Pre-push checks: cargo build, cargo clippy --workspace --all-targets -- -Dwarnings -Dclippy::pedantic, cargo fmt --all --check, cargo test — all clean. (cargo machete is not installed in this environment.)
• One pre-existing rustdoc warning remains on the [WitnessState] intra-doc link in lib.rs:47; verified it predates this PR (also present at b2560ea), so out of scope here.

LGTM with the doc fixes in (1) and (2). (3) is a quality-of-implementation nit.

[ask-bonk]

Review posted to PR #229. Summary of what I posted:

Findings (none blocking):

1. Stale doc comment — MetadataResponse::witness_public_key doc still says "Ed25519 key" when it's now algorithm-agnostic.
2. Overstated doc contract in dev_config_tests — references an integration-test PEM constant that doesn't exist (the integration tests derive the verifier from /metadata instead).
3. OnceLock cold-start race — both racing requests run build_witness_signer (allocating ~64 KiB ML-DSA expanded key) before one wins via get_or_init; harmless in practice but worth a comment or get_or_try_init.
4. Body-size cap is post-buffering (matches existing add-checkpoint pattern) — informational only.
5. Verified subtree validity ordering: parse_subtree_range_line doesn't enforce start < end, but Subtree::new and the explicit end > size guard cover it. Suggested adding a start == end integration boundary test.
6. Documented the stateless-verification scope and confirmed the trust model is sound (witness self-cosignature implies log-signature at cosign time).

Verification performed:

• cargo build, cargo clippy --workspace --all-targets -- -Dwarnings -Dclippy::pedantic, cargo fmt --all --check, cargo test — all clean.
• All 12 witness_worker unit tests pass including the 4 new dispatch tests and the new .dev.vars pin.
• Verified the PR description's claim that Subtree::new provides the start < end + alignment checks.
• Confirmed the rustdoc broken-link warning on WitnessState is pre-existing (present at b2560ea), not introduced by this PR.

github run

[lukevalenta]

/bonk review

[ask-bonk]

Review of #229 — witness_worker: add ML-DSA-44 / sign-subtree support

Re-reviewed the current head (818dbdb, force-pushed since the prior bot review at 2026-05-03T00:36:28Z) end-to-end.

Quick verdict: LGTM. The doc nits raised in the previous round are all resolved; the OnceLock race is now explicitly documented. One small gap in integration coverage (carried over from the prior review) and one minor observation below — neither blocks merge.

---

Pre-push gates

All clean against 818dbdb:

• cargo build — clean.
• cargo clippy --workspace --all-targets -- -Dwarnings -Dclippy::pedantic — clean.
• cargo fmt --all --check — clean.
• cargo test — every crate green; the 4 new dispatch tests in witness_worker::tests (Ed25519 → CosignatureV1, ML-DSA-44 → SubtreeV1, P-256 → unsupported, malformed PEM) and the new dev_config_tests::dev_vars_witness_key_matches_embedded_pem pin all pass.
• cargo machete not installed in this environment — advisory only per AGENTS.md.

---

Resolutions of prior-round findings (confirmed in current 818dbdb)

The previous bot review flagged three items; verified each has been addressed:

1. Stale "Ed25519" doc on MetadataResponse::witness_public_key (crates/witness_worker/src/frontend_worker.rs:96-101) — now reads DER-encoded SubjectPublicKeyInfo for the witness's verifying key. The signature algorithm matches whatever WITNESS_SIGNING_KEY was loaded with — Ed25519 (cosignature/v1) or ML-DSA-44 (subtree/v1); see [WitnessSigner]. ✅
2. dev_config_tests doc overstating an integration-test PEM constant (crates/witness_worker/src/lib.rs:393-411 + 437-447) — the module doc and the DEV_WITNESS_SIGNING_KEY_PEM doc now correctly say "The integration tests learn the witness's public key at runtime from /metadata, so they don't need their own copy" and pin only .dev.vars ↔ this constant. ✅
3. OnceLock cold-start race in load_witness_signer (crates/witness_worker/src/lib.rs:227-229) — explicit comment now reads Concurrent cold-start requests will each parse the PEM and expand the ML-DSA-44 key, dropping the loser's result. Deduplication would need OnceLock::get_or_try_init (unstable, rust-lang/rust#109737). ✅

---

New / carried-over findings (non-blocking)

1. Carried over: start == end boundary not exercised at the integration layer — crates/integration_tests/tests/tlog_witness.rs

Subtree::new rejects lo >= hi with ConditionNotMet("lo < hi"), which the handler maps to 400 "Bad request: invalid subtree" at frontend_worker.rs:380. The wire-format parser at tlog_witness/src/sign_subtree.rs:343 does not enforce start < end, so this 400 is the only line of defense for that case. The existing integration test cases cover (0, 4) and (0, 1) (happy paths), end > size (case 13), and missing-cosignature (case 12), but no case for start == end (e.g. (2, 2)) or start > end. A single additional integration step would cement that the Subtree::new branch is reachable from the wire and produces the expected status. Not required — the unit test in tlog_tiles covers Subtree::new's rejection — but rounds out the boundary table.

2. Observation: dispatch_check_and_update 500 path leaks the upstream status code to the client — crates/witness_worker/src/frontend_worker.rs:515-518

status => Ok(Some(Response::error(
    format!("Internal error: DO returned {status}"),
    500,
)?)),

The DO's own response status is interpolated into the client-facing error body. In a misconfiguration where the DO starts returning, say, 401 from a binding misconfig, this surfaces an internal implementation detail to anyone hitting /add-checkpoint. Pre-existing in this PR's vicinity (only the path is reachable; not introduced by this commit), and the leak is mild (just an HTTP status integer), but worth either logging-and-eliding or consciously documenting. Not a finding for this PR — flagging because the surrounding code is touched.

3. Observation: verify_subtree_consistency_proof independently re-checks end > n — informational

The handler's explicit subtree_end > cp_text.size() early-out at frontend_worker.rs:368-376 is redundant with tlog_tiles::verify_subtree_consistency_proof's own end > n check (tlog_tiles/src/tlog.rs:719-721 returns InvalidProof). The handler's earlier check produces a more useful 400 with a helpful message rather than letting the consistency proof verification fail with 422; the comment at frontend_worker.rs:357-360 explains this defensive ordering. The two layers cannot disagree — both compare the same subtree_end against the same cp_text.size(). No issue, just confirming the redundancy is intentional.

4. Observation: signed-note verify's fail-fast on InvalidSignature is inherited but not introduced — informational

signed_note::Note::verify (crates/signed_note/src/lib.rs:639-654) returns Err(InvalidSignature) as soon as any single signature line claims a known (name, id) but fails verification — even if a later line with the same (name, id) would have verified. In /sign-subtree's stateless verification this means an attacker who can append a single forged signature line carrying the witness's (name, id) before the witness's real cosignature (in line order, since dedup is via seen.insert after verify) flips the response to 403. This is purely a signed_note property and is the same shape as add-checkpoint's log-key verification, but worth noting in the witness threat model: /sign-subtree's stateless check is line-order-sensitive when a forged-name garbage line is present. Out of scope to fix here.

5. Observation: sign-subtree Ed25519-deployment 404 returns before body buffering — informational

let signer = load_witness_signer(&env)?;
let WitnessSigner::SubtreeV1 { ... } = signer else {
    return Response::error("Not Found", 404);
};
let body = req.bytes().await?;

The 404 path on Ed25519 deployments is reached without the handler buffering the request body — peak handler memory for a wrong-algorithm deployment is bounded by the Workers runtime's upstream caps rather than MAX_SIGN_SUBTREE_BODY_SIZE. This is the right ordering (avoids allocation when we already know we're going to 404). Just calling out the asymmetry with /add-checkpoint (which always buffers).

---

Code I specifically re-checked against the spec

• add_checkpoint algorithm dispatch (frontend_worker.rs:280-285): signer.as_checkpoint_signer().sign(now, &cp_text) — both variants implement CheckpointSigner::sign. The subtree/v1 impl in tlog_cosignature/src/subtree_v1.rs:320-341 builds Subtree::new(0, checkpoint.size()) and divides millis-by-1000 to get seconds. now_millis() is well above zero, so the start = 0 → timestamp != 0 invariant per the spec is satisfied. ✅
• sign_subtree timestamp = 0 (frontend_worker.rs:440): The spec requires timestamp = 0 for non-zero-start cosignatures and the handler uses 0 unconditionally; the subtree/v1 signer asserts !(start != 0 && ts != 0) (subtree_v1.rs:292-295), so ts = 0 is always safe. ✅
• subtree/v1 cosigner-name length cap: KeyName::new enforces ≤ 255 bytes; the cosigned_message body uses length_prefixed with opaque<1..2^8-1> — both layers agree. ✅
• /sign-subtree 404 vs 405: We register the route unconditionally in the router (frontend_worker.rs:72-74) and return 404 from the handler when the algorithm doesn't support it. The spec marks the endpoint OPTIONAL, so 404 ("unknown URL") is conformant; 405 would be wrong because we're claiming the URL doesn't exist for this deployment, not that the method is unsupported. ✅
• build_witness_signer error message (lib.rs:277-280): Surfaces the offending OID and both supported OIDs by name and dotted form — operator-friendly when a P-256 or RSA key is pasted by mistake. The unit test pins the "unsupported" wording. ✅

---

Process

• 12 unit tests pass in witness_worker (4 new dispatch tests + 1 new .dev.vars pin + 7 pre-existing).
• 13 unit tests pass in tlog_cosignature (subtree/v1 added in #227).
• Integration tests (tlog_witness::tlog_witness_end_to_end) require a running wrangler dev; per AGENTS.md they are excluded from default cargo test and the PR description states the end-to-end run passes.
• The pre-existing rustdoc broken-link warning on [WitnessState] (lib.rs:47) is unchanged by this PR (verified in the prior review at b2560ea).

LGTM for merge. Item 1 is the only carry-over and is a coverage nit; everything else is informational.

[ask-bonk]

Review posted to PR #229.

Summary of what I posted:

Re-reviewed the current head 818dbdb (force-pushed since the prior bot review). Confirmed all three of the prior round's findings are now fixed in-tree:

1. The MetadataResponse::witness_public_key doc is now algorithm-agnostic.
2. The dev_config_tests doc now correctly says integration tests learn the witness key from /metadata.
3. The OnceLock cold-start race has an explicit comment citing rust-lang/rust#109737.

Carried-over / new findings (none blocking):

1. Coverage gap — no integration test for start == end on /sign-subtree; would round out the boundary table.
2. Status leak (informational, pre-existing) — dispatch_check_and_update's 500 path interpolates the upstream DO status into the client-facing error body.
3. Redundant guard (intentional) — confirmed subtree_end > size is checked twice (handler + verify_subtree_consistency_proof); the handler's check exists to convert a 422 into a more helpful 400.
4. signed_note::verify fail-fast on InvalidSignature — /sign-subtree stateless check is line-order-sensitive when a forged garbage signature line claiming the witness's (name, id) precedes the real one. Inherited from signed_note, out of scope here.
5. sign-subtree 404 returns before body buffering — confirmed correct ordering on Ed25519 deployments.

Spec re-checks: algorithm dispatch in add_checkpoint, timestamp = 0 invariant in sign_subtree, KeyName length cap matching the opaque<1..2^8-1> wire format, 404 vs 405 choice, and operator-readable error message for unsupported OIDs all match the spec.

Pre-push gates: cargo build, cargo clippy --workspace --all-targets -- -Dwarnings -Dclippy::pedantic, cargo fmt --all --check, cargo test — all clean.

LGTM for merge.

github run

[lukevalenta]

/bonk review again

[ask-bonk]

Review of #229 — witness_worker: add ML-DSA-44 / sign-subtree support

Re-reviewed the current head 6cef65f (one commit beyond the previously-reviewed 818dbdb; the only commit unique to this PR vs the merge base 444f858 is 6cef65f itself plus the merge commit pulling in tlog_cosignature::subtree_v1).

Quick verdict: LGTM. The single carry-over finding from the prior round is now resolved in-tree. Nothing blocks merge.

---

Pre-push gates (all green against 6cef65f)

• cargo build — clean.
• cargo clippy --workspace --all-targets -- -Dwarnings -Dclippy::pedantic — clean.
• cargo fmt --all --check — clean.
• cargo test — every crate green:
  • witness_worker: 12 unit tests pass (4 dispatch + 1 .dev.vars pin + 7 prior).
  • signed_note: 15 unit tests pass (test_key_name_validation covering empty / whitespace / + / 255-byte boundary / multi-byte-char rejection).
  • tlog_cosignature::subtree_v1: 13 unit tests pass.
• cargo machete not installed in this environment — advisory only per AGENTS.md.

Resolution of the carry-over finding

The prior round flagged that start == end on /sign-subtree (the Subtree::new(lo >= hi) rejection branch) wasn't exercised at the integration layer. Resolved at crates/integration_tests/tests/tlog_witness.rs:630-652 — step (14) submits start = 2, end = 2 against the previously-cosigned checkpoint and asserts a 400. The boundary table ((0,4) happy-path, (0,1) happy-path, end > size, no-witness-cosig 403, start == end) now reaches the Subtree::new rejection branch from the wire. ✅

The PR description's "4 new integration test steps" (steps 11–14) matches what's in-tree.

Re-confirmation of prior-round resolutions

• MetadataResponse::witness_public_key doc is algorithm-agnostic at frontend_worker.rs:96-101. ✅
• dev_config_tests module doc and DEV_WITNESS_SIGNING_KEY_PEM doc at lib.rs:393-411 and 437-447 correctly say the integration tests learn the witness public key from /metadata and only .dev.vars ↔ DEV_WITNESS_SIGNING_KEY_PEM are pinned. ✅
• OnceLock cold-start race comment at lib.rs:227-229 cites rust-lang/rust#109737. ✅

Spec re-checks (all match)

• Algorithm dispatch in add_checkpoint (frontend_worker.rs:280-285): signer.as_checkpoint_signer().sign(now, &cp_text) — both WitnessSigner variants implement CheckpointSigner::sign and produce the right cosignature shape. The subtree/v1 impl (tlog_cosignature/src/subtree_v1.rs) constructs Subtree::new(0, checkpoint.size()) so the "whole tree" invariant from the spec holds.
• sign_subtree timestamp = 0 invariant (frontend_worker.rs:450): handler unconditionally passes 0; the subtree/v1 signer asserts !(start != 0 && ts != 0), so 0 is always safe.
• KeyName::MAX_LEN = 255 matches the opaque<1..2^8-1> cap on cosigner_name / log_origin in the subtree/v1 cosigned_message body.
• sign-subtree 404 vs 405: handler returns 404 when the deployed key isn't ML-DSA-44; conformant for an OPTIONAL endpoint.
• Operator-readable error in build_witness_signer (lib.rs:277-280) names the offending OID and both supported OIDs by name and dotted form. The unsupported_oid_is_rejected_with_helpful_error test pins the "unsupported" wording.
• Body caps: MAX_ADD_CHECKPOINT_BODY_SIZE = 1 MiB + 16 KiB, MAX_SIGN_SUBTREE_BODY_SIZE = 1 MiB + 64 KiB. The doc-comment block at frontend_worker.rs:461-468 correctly notes the Workers platform cap (100 MB) bounds peak memory at the runtime level; ours tighten that to spec-shaped maxima.

New / carried-over informational observations (none blocking)

1. dispatch_check_and_update 500 path interpolates the upstream DO status into the client-facing error body — frontend_worker.rs:534-537

status => Ok(Some(Response::error(
    format!("Internal error: DO returned {status}"),
    500,
)?)),

Pre-existing in this PR's vicinity (touched only via the surrounding refactor to share helpers, not introduced here). Mild leak — just an HTTP status integer — but flagging because the surrounding code is in this PR. Not for this PR; would be a one-liner in a follow-up to log-and-elide.

2. Redundant subtree_end > size guard (intentional)

The handler's explicit early-out at frontend_worker.rs:378-386 is redundant with tlog_tiles::verify_subtree_consistency_proof's own end > n check. The handler's check exists to convert what would be a 422 ("consistency proof failed") into a more useful 400 with an explicit message — confirmed by the comment at frontend_worker.rs:367-370. The two layers cannot disagree (same subtree_end against the same cp_text.size()); intentional defense-in-depth.

3. signed_note::Note::verify fail-fast on InvalidSignature (inherited)

signed_note::Note::verify returns Err(InvalidSignature) on the first signature line that claims a known (name, id) but fails verification, even if a later line with the same (name, id) would have verified. In /sign-subtree's stateless check this means an attacker who can prepend a forged (name, id)-matching garbage signature line ahead of the witness's real cosignature flips the response to 403. Inherited signed_note property; same shape as add-checkpoint's log-key verification. Out of scope here.

4. sign-subtree 404 returns before body buffering on Ed25519 deployments — frontend_worker.rs:335-345

let signer = load_witness_signer(&env)?;
let WitnessSigner::SubtreeV1 { ... } = signer else {
    return Response::error("Not Found", 404);
};
let body = req.bytes().await?;

Right ordering — avoids buffering a body we're about to discard with 404. Asymmetric with /add-checkpoint's always-buffer pattern; intentional and correct.

---

Process notes

• The integration tests exercising the four /sign-subtree steps (11–14) require a running wrangler dev; per AGENTS.md they are excluded from default cargo test and the PR description states the end-to-end run passes.
• integration_tests/Cargo.toml and witness_worker/Cargo.toml add ml-dsa / pkcs8 (and p256 to [dev-dependencies]); each is actually used (tlog_witness.rs:55,299-300 for ml-dsa/pkcs8; lib.rs:362 for the p256 unsupported-OID test). No speculative additions.
• The pre-existing rustdoc broken-link warning on [WitnessState] at lib.rs:47 predates this PR and is unchanged.

LGTM for merge.

github run

[ask-bonk]

Review posted to PR #229.

Summary of what I posted:

Re-reviewed the current head 6cef65f (one commit beyond the previously-reviewed 818dbdb). Confirmed the single carry-over finding from the prior round is now resolved.

Resolution of carry-over finding:

• start == end on /sign-subtree is now exercised at the integration layer as step (14) at crates/integration_tests/tests/tlog_witness.rs:630-652.

Re-confirmed prior-round resolutions (still fixed in 6cef65f):

1. MetadataResponse::witness_public_key doc is algorithm-agnostic.
2. dev_config_tests doc correctly says integration tests learn the witness key from /metadata.
3. OnceLock cold-start race comment cites rust-lang/rust#109737.

Spec re-checks (all match): algorithm dispatch, timestamp = 0 invariant, KeyName::MAX_LEN = 255 matching opaque<1..2^8-1>, 404-vs-405 choice, operator-readable unsupported-OID error, body caps with platform-cap context.

Carried/new informational observations (none blocking):

1. dispatch_check_and_update 500 path leaks upstream DO status (pre-existing).
2. Redundant subtree_end > size guard is intentional (converts 422→400).
3. signed_note::verify fail-fast on InvalidSignature is inherited.
4. /sign-subtree 404 returns before body buffering on Ed25519 deployments — correct ordering.

Pre-push gates: cargo build, cargo clippy --workspace --all-targets -- -Dwarnings -Dclippy::pedantic, cargo fmt --all --check, cargo test — all clean. cargo machete not installed (advisory).

Dependency additions verified non-speculative: ml-dsa / pkcs8 in integration_tests/Cargo.toml are used at tlog_witness.rs:55,299-300; p256 in witness_worker's [dev-dependencies] is used at lib.rs:362.

LGTM for merge.

github run