From 758793476fc2046065a72cc08fb758bc9bfbd520 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Sat, 1 Aug 2026 11:21:09 +0200 Subject: [PATCH 1/4] fix(engine): chunk the registration to the registry per-entry content-CID cap The registry caps contentCids at 1000 per register entry and refuses a larger array fail-closed. PublishRequest built a single entry carrying the version root plus every leaf, so at the production framing any file past ~1 GiB was refused - and a register 400 classified as Halt::Unclassified, which is charged nothing and retried every tick, so the op held the strict-FIFO queue head forever while every pass re-uploaded and re-registered. The registration now splits at the cap into several entries under one ipnsName, the head riding the first so the name and its pointer land ahead of any content-only entry; the server collapses them to one name row and a bare re-register leaves the stored head untouched. Register-first still holds - every chunk lands before the record PUT. A register 400 is now classified permanent, so a registration no retry can satisfy dead-letters instead of looping at the queue head. The retire chunker's cap constant moves to net::REGISTRY_BATCH_MAX, shared by both chunkers rather than copied. Closes #920 --- blueprint/api.md | 6 +- crates/contract/tests/contract.rs | 44 +++++++ crates/engine/src/net/mod.rs | 5 + crates/engine/src/net/publish.rs | 37 ++++-- crates/engine/src/net/retire.rs | 11 +- crates/engine/src/sync/drain.rs | 14 ++- crates/engine/src/sync/rebase.rs | 4 +- crates/engine/tests/write_plane.rs | 178 +++++++++++++++++++++++++++++ 8 files changed, 276 insertions(+), 23 deletions(-) diff --git a/blueprint/api.md b/blueprint/api.md index b18573764..b06311169 100644 --- a/blueprint/api.md +++ b/blueprint/api.md @@ -57,7 +57,11 @@ decay) inverted into structure. caps `contentCids` at 1000 per entry), enforced fail-closed with `400` before per-item validation and published as `maxItems` in the OpenAPI document. A bulk caller — a name wave, or an abandoned version whose leaves all need retiring — - chunks to the cap; retire is idempotent, so a replayed chunk is a no-op. + chunks to the cap; retire is idempotent, so a replayed chunk is a no-op. A + version with more leaves than the per-entry cap registers as several entries + under one `ipnsName`, the head riding the first; the server collapses them to + one name row, and a bare re-register carrying no `headCid` leaves the stored + head untouched. - **Register-first, fail-closed**: registration precedes the first publish of a name, and publish is blocked on it. A live-but-uninventoried name is structurally impossible; the worst failure is a registered-never-published diff --git a/crates/contract/tests/contract.rs b/crates/contract/tests/contract.rs index fcdeee840..6d1836abd 100644 --- a/crates/contract/tests/contract.rs +++ b/crates/contract/tests/contract.rs @@ -823,6 +823,50 @@ async fn an_oversize_retire_batch_is_refused_fail_closed() { .expect("a batch at the cap is accepted"); } +/// A register entry's `contentCids` is bounded fail-closed the same way +/// (blueprint/api.md "Batch bounds"): an oversize array is refused, never +/// truncated. Register-first blocks the record PUT on this call, so a version +/// past the cap could never publish without the engine's chunking (#920). +#[tokio::test] +async fn an_oversize_register_entry_is_refused_fail_closed() { + let base = require_stack!("an_oversize_register_entry_is_refused_fail_closed"); + let client = fresh_account(&base).await; + + let name = "k51contractRegisterBound".to_owned(); + let cids: Vec = (0..1001).map(|i| format!("bafyContractEntry{i}")).collect(); + let over_cap = NameRegistration { + ipns_name: name.clone(), + head_cid: Some("bafyContractEntryHead".to_owned()), + content_cids: cids.clone(), + }; + let error = client + .register(std::slice::from_ref(&over_cap)) + .await + .expect_err("an oversize register entry must be refused"); + assert!( + matches!(error, ApiError::Status { status: 400, .. }), + "the per-entry contentCids bound is fail-closed: a 400, got {error:?}" + ); + + // The engine's chunks are exactly this shape: the cap-sized entry carrying + // the head, then a content-only entry for the remainder under one name. + client + .register(&[ + NameRegistration { + ipns_name: name.clone(), + head_cid: Some("bafyContractEntryHead".to_owned()), + content_cids: cids[..1000].to_vec(), + }, + NameRegistration { + ipns_name: name, + head_cid: None, + content_cids: cids[1000..].to_vec(), + }, + ]) + .await + .expect("chunked entries at the cap are accepted"); +} + // --- mailbox (blueprint/api.md, Mailbox; #827) ------------------------------ /// An account addressable as a mailbox recipient: the client plus its identity diff --git a/crates/engine/src/net/mod.rs b/crates/engine/src/net/mod.rs index bea72bdfe..690bd44c9 100644 --- a/crates/engine/src/net/mod.rs +++ b/crates/engine/src/net/mod.rs @@ -18,6 +18,11 @@ mod fanout; mod focus; mod pointer_fetch; +/// The registry's batch cap: the server refuses a larger array — and a larger +/// per-entry `contentCids` array — fail-closed with a `400` (blueprint/api.md +/// "Batch bounds"). Every bulk caller on this plane chunks to it. +pub(crate) const REGISTRY_BATCH_MAX: usize = 1000; + pub mod author; pub mod eol; pub mod liveness; diff --git a/crates/engine/src/net/publish.rs b/crates/engine/src/net/publish.rs index a0b773952..7b511d081 100644 --- a/crates/engine/src/net/publish.rs +++ b/crates/engine/src/net/publish.rs @@ -16,6 +16,7 @@ use core::time::Duration; use cipherbox_core::ipns::{IpnsName, IpnsRecord}; use cipherbox_core::suite::ed25519::Ed25519Signer; +use super::REGISTRY_BATCH_MAX; use super::eol; use super::fanout::{fanout_get_verify, fanout_put}; use crate::api::{ApiClient, ApiError, NameRegistration}; @@ -58,15 +59,25 @@ impl PublishRequest<'_> { format!("{IPFS_PREFIX}{}", self.head_cid).into_bytes() } - /// The single-item registration batch for this publish (ordinary writes - /// register one name; name waves and sweeps batch — that is the caller's - /// concern, blueprint/engine.md). - fn registration(&self) -> NameRegistration { - NameRegistration { + /// The registration entries for this publish, split at the registry's + /// per-entry `contentCids` cap ([`REGISTRY_BATCH_MAX`]): a version with more + /// leaves than the cap registers as several entries under the same name, + /// which the server collapses to one name row. The head rides the first + /// entry, so the name and its pointer land ahead of any content row. + fn registrations(&self) -> Vec { + let mut chunks = self.content_cids.chunks(REGISTRY_BATCH_MAX); + let head_entry = NameRegistration { ipns_name: self.name.as_str().to_owned(), head_cid: Some(self.head_cid.clone()), - content_cids: self.content_cids.clone(), - } + content_cids: chunks.next().unwrap_or_default().to_vec(), + }; + core::iter::once(head_entry) + .chain(chunks.map(|chunk| NameRegistration { + ipns_name: self.name.as_str().to_owned(), + head_cid: None, + content_cids: chunk.to_vec(), + })) + .collect() } } @@ -117,7 +128,9 @@ pub struct PublishReceipt { #[derive(Debug, Clone, PartialEq, Eq)] pub enum PublishError { /// Register-first failed: the API rejected (or could not reach) the - /// registration, so no record was PUT — the fail-closed ordering law. + /// registration, so no record was PUT — the fail-closed ordering law. A + /// chunked registration may have landed earlier chunks, leaving rows the + /// caller must retire. Register(ApiError), /// No endpoint acknowledged the record PUT (the whole endpoint set is /// unreachable). Nothing durable happened; the caller retries later. @@ -158,10 +171,10 @@ where } // Register-first, fail-closed: the record never reaches the transport unless - // the registration succeeds (#24 D6 / #34 D2). - api.register(std::slice::from_ref(&request.registration())) - .await - .map_err(PublishError::Register)?; + // every registration chunk succeeds (#24 D6 / #34 D2). + for batch in request.registrations().chunks(REGISTRY_BATCH_MAX) { + api.register(batch).await.map_err(PublishError::Register)?; + } // CAS expected sequence: floor + 1 (first publish → 1, the "no floor" 0 // sentinel reserved). Revival raises the floor read to the recovered diff --git a/crates/engine/src/net/retire.rs b/crates/engine/src/net/retire.rs index 90286e790..5f87b36a5 100644 --- a/crates/engine/src/net/retire.rs +++ b/crates/engine/src/net/retire.rs @@ -6,26 +6,23 @@ //! scope-root name lingers serving the tombstone until the migration window //! closes ([`root_retire_ready`], stubbed — see below). +use super::REGISTRY_BATCH_MAX; use crate::api::{ApiClient, ApiError}; use crate::seams::{CredentialStore, Http}; -/// The server's retire batch cap, which refuses a larger array fail-closed -/// (blueprint/api.md "Batch bounds"). -const RETIRE_BATCH_MAX: usize = 1000; - /// Batch-retire registry rows for `targets` (`ipnsName`s or CIDs). Idempotent /// server-side (blueprint/api.md), so a replayed batch — a resumed name wave, or /// a chunk a failed pass already sent — is a no-op, never an error. This is the /// interior-name path: it retires the moment the caller says a name is dead. /// -/// Chunked to [`RETIRE_BATCH_MAX`]; a failing chunk leaves the earlier ones +/// Chunked to [`REGISTRY_BATCH_MAX`]; a failing chunk leaves the earlier ones /// retired and returns `Err`. pub async fn retire(api: &ApiClient, targets: &[String]) -> Result<(), ApiError> where H: Http, C: CredentialStore, { - for chunk in targets.chunks(RETIRE_BATCH_MAX) { + for chunk in targets.chunks(REGISTRY_BATCH_MAX) { api.retire(chunk).await?; } Ok(()) @@ -86,7 +83,7 @@ mod tests { #[test] fn an_oversize_batch_splits_into_chunks_the_server_accepts() { let (http, client) = client(); - let targets: Vec = (0..RETIRE_BATCH_MAX + 1) + let targets: Vec = (0..REGISTRY_BATCH_MAX + 1) .map(|i| format!("cid{i}")) .collect(); for _ in 0..2 { diff --git a/crates/engine/src/sync/drain.rs b/crates/engine/src/sync/drain.rs index 687880ea8..22992df68 100644 --- a/crates/engine/src/sync/drain.rs +++ b/crates/engine/src/sync/drain.rs @@ -42,7 +42,7 @@ use crate::net::author::{ AuthoredHead, ENVELOPE_V, EnvelopeAuthoring, NewNodeBody, author_child_envelope, author_scope_root_envelope, new_child, }; -use crate::net::publish::{PublishOutcome, PublishReceipt}; +use crate::net::publish::{PublishError, PublishOutcome, PublishReceipt}; use crate::net::record_publish::{ HeadBinding, RecordPublishError, RecordPublishRequest, preflight, publish_record, }; @@ -1888,12 +1888,24 @@ fn seam(_: crate::seams::SeamError) -> Halt { fn classify_publish(error: RecordPublishError, refused_bytes: u64) -> Halt { match error { RecordPublishError::Upload(error) => classify_upload(error, refused_bytes), + RecordPublishError::Publish(PublishError::Register(error)) => classify_register(error), RecordPublishError::HeadCidMismatch { .. } | RecordPublishError::Publish(_) => { Halt::Unclassified } } } +/// Classify a register-first refusal. A `400` is the registry's fail-closed +/// verdict on the batch this op builds — a malformed or over-cap entry, which +/// no retry changes. The queue is strict FIFO, so leaving it unclassified would +/// park the op at the head forever, re-registering every tick (#920). +fn classify_register(error: ApiError) -> Halt { + match error { + ApiError::Status { status: 400, .. } => Halt::Permanent(DeadLetterReason::PayloadRefused), + _ => Halt::Unclassified, + } +} + /// Classify a content-upload failure for the valve. The same server verdicts a /// head-block upload can carry, since content blocks and head blocks go through /// one endpoint. diff --git a/crates/engine/src/sync/rebase.rs b/crates/engine/src/sync/rebase.rs index b5a345742..ea907cba5 100644 --- a/crates/engine/src/sync/rebase.rs +++ b/crates/engine/src/sync/rebase.rs @@ -89,8 +89,8 @@ pub enum DeadLetterReason { /// header format, or a newer intent grammar — is retained instead /// ([`RecordClass::Retained`]). Undecodable, - /// The network refused the op's own bytes for a reason no retry changes — - /// an over-cap payload, not a full account. + /// The network refused the op's own bytes or its registration for a reason + /// no retry changes — an over-cap payload, not a full account. PayloadRefused, /// The op's drain attempt budget ran out. A budget spent before the record /// PUT retires what the op uploaded; once a PUT is acked the publish may diff --git a/crates/engine/tests/write_plane.rs b/crates/engine/tests/write_plane.rs index 65b22ebc6..51b761fb9 100644 --- a/crates/engine/tests/write_plane.rs +++ b/crates/engine/tests/write_plane.rs @@ -84,6 +84,34 @@ fn upload_413(code: Option<&str>) -> SeamResult { }) } +/// The registry's batch bounds (blueprint/api.md): at most this many entries +/// per register batch, and this many `contentCids` per entry. +const REGISTER_BATCH_CAP: usize = 1000; + +/// The registry's fail-closed answer to a batch past its bounds: a `400`, +/// never a truncated or partial registration (blueprint/api.md "Batch bounds"). +fn register_reply(body: Option<&[u8]>) -> SeamResult { + let entries: Vec = + serde_json::from_slice(body.expect("a register call carries a body")) + .expect("a register body is a JSON array"); + let over_cap = entries.len() > REGISTER_BATCH_CAP + || entries.iter().any(|entry| { + entry["contentCids"] + .as_array() + .is_some_and(|cids| cids.len() > REGISTER_BATCH_CAP) + }); + Ok(HttpResponse { + status: if over_cap { 400 } else { 200 }, + headers: Vec::new(), + body: if over_cap { + br#"{"statusCode":400,"message":"contentCids must contain no more than 1000 elements"}"# + .to_vec() + } else { + Vec::new() + }, + }) +} + /// A 413 from an intermediary that never reached the API: an HTML body, so no /// error envelope parses out of it at all. fn proxy_413() -> SeamResult { @@ -100,6 +128,8 @@ struct Blocks { on_upload: Arc>>, /// What `GET /account/quota` reports, as `(usedBytes, limitBytes)`. quota: Arc>>, + /// A status every `POST /registry/register` answers with instead of acking. + register_refusal: Arc>>, } impl Blocks { @@ -150,6 +180,11 @@ impl Blocks { *self.quota.lock().expect("lock") = Some((used_bytes, limit_bytes)); } + /// Answer every registration with `status` instead of acking. + fn refuse_register(&self, status: u16) { + *self.register_refusal.lock().expect("lock") = Some(status); + } + /// Answer one engine HTTP call: a content upload lands its bytes here and /// echoes their address, a registry call acks, and a gateway GET serves the /// block back. Enqueued as many times as the pass needs, so no test depends @@ -194,6 +229,17 @@ impl Blocks { ) .into_bytes()); } + if url.ends_with("/registry/register") { + if let Some(status) = *self.register_refusal.lock().expect("lock") { + return Ok(HttpResponse { + status, + headers: Vec::new(), + body: format!("{{\"statusCode\":{status},\"message\":\"refused\"}}") + .into_bytes(), + }); + } + return register_reply(request.body.as_deref()); + } if url.contains("/registry/") { return ok(Vec::new()); } @@ -443,6 +489,23 @@ fn registered_content_cids(device: &FakeDevice, name: &IpnsName) -> Vec .unwrap_or_default() } +/// Every registration entry the device sent for `name`, in wire order across +/// however many batches it took — the shape a chunked registration is asserted +/// on (#920). +fn registration_entries(device: &FakeDevice, name: &IpnsName) -> Vec { + device + .http + .requests() + .iter() + .filter(|request| request.url.ends_with("/registry/register")) + .filter_map(|request| { + serde_json::from_slice::>(request.body.as_deref()?).ok() + }) + .flatten() + .filter(|entry| entry["ipnsName"] == name.as_str()) + .collect() +} + /// The node a head block about to be uploaded was sealed for. fn head_of(block: &[u8]) -> Option<[u8; 16]> { decode_envelope(block).ok().map(|envelope| envelope.id) @@ -876,6 +939,121 @@ fn a_published_version_registers_its_whole_block_set() { ); } +/// A version with more blocks than the registry's per-entry `contentCids` cap +/// splits across several entries under one name, so the registration the +/// register-first ordering blocks on is accepted and the version publishes +/// (#920). Unchunked, the batch is refused fail-closed and nothing is PUT. +#[test] +fn a_version_past_the_registration_cap_registers_in_chunks_and_publishes() { + let world = FakeWorld::new(); + let blocks = Blocks::default(); + seed_account(&world, &blocks); + // 1001 leaves at the CI framing, plus the root: one past the cap. + let leaves = REGISTER_BATCH_CAP + 1; + let plaintext: Vec = (0..leaves * 16).map(|byte| byte as u8).collect(); + + let alice = world.device(b"alice"); + let (mut engine_a, _events_a, mut tasks) = boot(&world, &blocks, &alice, 42); + // One HTTP reply per block, plus the metadata plane's own calls. + serve_http(&alice, &blocks, 4 * leaves); + write_file( + &mut engine_a, + WriteTarget::NewFile { + parent: ROOT, + name: "big.bin".into(), + }, + &plaintext, + ) + .unwrap(); + tick(&world, &engine_a, &mut tasks); + + let node = child_id(&engine_a, ROOT, "big.bin"); + let entries = registration_entries(&alice, &write_name(node)); + let sizes: Vec = entries + .iter() + .map(|entry| entry["contentCids"].as_array().expect("contentCids").len()) + .collect(); + assert_eq!( + sizes, + vec![REGISTER_BATCH_CAP, 2], + "the registration splits at the per-entry cap" + ); + let heads: Vec<&str> = entries + .iter() + .filter_map(|entry| entry["headCid"].as_str()) + .collect(); + assert_eq!( + heads.len(), + 1, + "the head rides one entry; the rest carry content only" + ); + assert!( + entries[0]["headCid"].is_string(), + "the head rides the first entry, so the name and its pointer land first" + ); + + let registered: Vec = entries + .iter() + .flat_map(|entry| { + entry["contentCids"] + .as_array() + .expect("contentCids") + .iter() + .map(|cid| cid.as_str().expect("a CID string").to_owned()) + .collect::>() + }) + .collect(); + assert_eq!( + registered.len(), + leaves + 1, + "every block the version links still rides the registration exactly once" + ); + assert!( + registered.iter().all(|cid| blocks.get(cid).is_some()), + "every registered CID names a block the provider holds" + ); + assert!( + block_on(engine_a.snapshot(ROOT)) + .unwrap() + .dead_letters + .is_empty(), + "a chunked registration is accepted, so the op publishes" + ); +} + +/// A registration the registry refuses is refused on every retry, and the queue +/// is strict FIFO — so the op dead-letters instead of holding the head and +/// re-registering every tick (#920). +#[test] +fn a_refused_registration_dead_letters_instead_of_holding_the_queue_head() { + let world = FakeWorld::new(); + let blocks = Blocks::default(); + seed_account(&world, &blocks); + + let alice = world.device(b"alice"); + let (mut engine, _events, mut tasks) = boot(&world, &blocks, &alice, 42); + blocks.refuse_register(400); + let op_id = write_file( + &mut engine, + WriteTarget::NewFile { + parent: ROOT, + name: "photo.bin".into(), + }, + &(0..200u8).collect::>(), + ) + .unwrap(); + + let (dead_letters, passes) = tick_until_dead_lettered(&world, &engine, &mut tasks); + assert_eq!(passes, 1, "a refused registration is permanent on sight"); + assert_eq!( + dead_letters, + vec![DeadLetter { + op_id, + reason: DeadLetterReason::PayloadRefused + }] + ); +} + /// The `pushChunk` total is cross-checked against the `beginWrite` declaration: /// a backing file truncated mid-read fails the commit rather than publishing a /// short version as a success (#830). From 90a9432862cb417d39f7648dead87ba916ea5110 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Sat, 1 Aug 2026 11:45:34 +0200 Subject: [PATCH 2/4] fix: classify a register refusal on the registry's own discriminator The review gates found that treating any 400 as the registry's verdict breaks the positive-evidence rule classify_upload enforces for a 413: a proxy or a version-skewed deploy answering 400 would dead-letter every queued write on sight, retiring its rows and releasing its staged blocks. The registry's batch gate now stamps code REGISTRY_BATCH_REFUSED on its own refusals, published in the OpenAPI document, and the valve branches on that code. A 400 the gate did not stamp is charged like any other pre-PUT refusal and abandons only once the attempt budget runs out. The chunker also moves out of PublishRequest into net::register, the sibling of net::retire, so both of the registry's bounds are enforced on one bounded path no future caller of the raw client can bypass. --- apps/api/openapi.json | 4 +- apps/api/src/registry/dto/registry.dto.ts | 2 +- apps/api/src/registry/registry-error-codes.ts | 17 ++ apps/api/src/registry/registry.controller.ts | 11 +- .../registry.http.integration.test.ts | 35 +++ apps/api/src/registry/registry.pipes.ts | 12 +- blueprint/api.md | 4 +- crates/contract/tests/contract.rs | 35 ++- crates/engine/src/api/error.rs | 5 + crates/engine/src/api/mod.rs | 2 +- crates/engine/src/net/mod.rs | 7 +- crates/engine/src/net/publish.rs | 39 ++-- crates/engine/src/net/register.rs | 200 ++++++++++++++++ crates/engine/src/sync/drain.rs | 35 +-- crates/engine/tests/write_plane.rs | 218 ++++++++++-------- 15 files changed, 471 insertions(+), 155 deletions(-) create mode 100644 apps/api/src/registry/registry-error-codes.ts create mode 100644 crates/engine/src/net/register.rs diff --git a/apps/api/openapi.json b/apps/api/openapi.json index 7d3723283..8ec6d599b 100644 --- a/apps/api/openapi.json +++ b/apps/api/openapi.json @@ -452,7 +452,7 @@ } }, "400": { - "description": "Malformed batch (invalid entry, name, or CID)" + "description": "Malformed or over-cap batch; the body carries code REGISTRY_BATCH_REFUSED" }, "401": { "description": "Missing or invalid access token" @@ -506,7 +506,7 @@ } }, "400": { - "description": "Malformed batch" + "description": "Malformed or over-cap batch; the body carries code REGISTRY_BATCH_REFUSED" }, "401": { "description": "Missing or invalid access token" diff --git a/apps/api/src/registry/dto/registry.dto.ts b/apps/api/src/registry/dto/registry.dto.ts index 2a2dd14e0..b2ce7ccf9 100644 --- a/apps/api/src/registry/dto/registry.dto.ts +++ b/apps/api/src/registry/dto/registry.dto.ts @@ -19,7 +19,7 @@ const CID_OR_NAME = /^[A-Za-z0-9]{1,256}$/; /** Batch bounds: bulk name waves and sweeps are large but not unbounded. */ export const MAX_BATCH = 1000; -const MAX_CONTENT_CIDS = 1000; +export const MAX_CONTENT_CIDS = 1000; export class RegisterEntryDto { @ApiProperty({ diff --git a/apps/api/src/registry/registry-error-codes.ts b/apps/api/src/registry/registry-error-codes.ts new file mode 100644 index 000000000..2c69a820f --- /dev/null +++ b/apps/api/src/registry/registry-error-codes.ts @@ -0,0 +1,17 @@ +/** + * The registry's batch routes answer 400 for a refusal the caller can never + * retry past — an over-cap batch, an over-cap `contentCids`, a malformed entry. + * The body carries a stable `code` so a client classifies on it instead of + * parsing `message`, and so a 400 from anything that is NOT this gate (a proxy, + * a body-size cap) stays unattributable (#920, mirroring the 413 codes #842). + */ +export const REGISTRY_BATCH_REFUSED = 'REGISTRY_BATCH_REFUSED'; + +/** + * The 400 body the register/retire pipes emit. Nest stops synthesizing + * `statusCode`/`error` as soon as an exception is given an object, so the full + * envelope is built here once rather than re-typed at each throw site. + */ +export function batchRefusedBody(message: unknown) { + return { statusCode: 400, message, error: 'Bad Request', code: REGISTRY_BATCH_REFUSED }; +} diff --git a/apps/api/src/registry/registry.controller.ts b/apps/api/src/registry/registry.controller.ts index ff3983bc2..6ae798283 100644 --- a/apps/api/src/registry/registry.controller.ts +++ b/apps/api/src/registry/registry.controller.ts @@ -19,6 +19,7 @@ import { RETIRE_TARGET_MAX_LENGTH, RetireResponseDto, } from './dto/registry.dto'; +import { REGISTRY_BATCH_REFUSED } from './registry-error-codes'; import { registerBodyPipes, retireBodyPipes } from './registry.pipes'; import { RegistryService } from './services/registry.service'; @@ -51,7 +52,10 @@ export class RegistryController { }, }) @ApiCreatedResponse({ type: RegisterResponseDto }) - @ApiResponse({ status: 400, description: 'Malformed batch (invalid entry, name, or CID)' }) + @ApiResponse({ + status: 400, + description: `Malformed or over-cap batch; the body carries code ${REGISTRY_BATCH_REFUSED}`, + }) @ApiResponse({ status: 401, description: 'Missing or invalid access token' }) @ApiResponse({ status: 429, description: 'Registry rate limit exceeded' }) @ApiResponse({ status: 503, description: 'Token serialization contended; retry shortly' }) @@ -76,7 +80,10 @@ export class RegistryController { }, }) @ApiCreatedResponse({ type: RetireResponseDto }) - @ApiResponse({ status: 400, description: 'Malformed batch' }) + @ApiResponse({ + status: 400, + description: `Malformed or over-cap batch; the body carries code ${REGISTRY_BATCH_REFUSED}`, + }) @ApiResponse({ status: 401, description: 'Missing or invalid access token' }) @ApiResponse({ status: 429, description: 'Registry rate limit exceeded' }) @ApiResponse({ status: 503, description: 'Token serialization contended; retry shortly' }) diff --git a/apps/api/src/registry/registry.http.integration.test.ts b/apps/api/src/registry/registry.http.integration.test.ts index b9f06c088..81280e2f4 100644 --- a/apps/api/src/registry/registry.http.integration.test.ts +++ b/apps/api/src/registry/registry.http.integration.test.ts @@ -12,9 +12,11 @@ import { } from '../testing/http-integration-app'; import { createIntegrationDatabase, IntegrationDatabase } from '../testing/integration-db'; import { AccountController } from './account.controller'; +import { MAX_CONTENT_CIDS } from './dto/registry.dto'; import { NameInventory } from './entities/name-inventory.entity'; import { PinnedCid } from './entities/pinned-cid.entity'; import { PinStore } from './pin-store'; +import { REGISTRY_BATCH_REFUSED } from './registry-error-codes'; import { RegistryController } from './registry.controller'; import { AccountService } from './services/account.service'; import { RegistryService } from './services/registry.service'; @@ -147,6 +149,39 @@ describe('registry HTTP surface (real Postgres)', () => { expect(await namesFor(acct.id)).toHaveLength(0); expect((await pinsFor(acct.id)).some((r) => r.cid === 'bafyX')).toBe(false); }); + + it('refuses an over-cap contentCids and stamps the batch-refused code', async () => { + const acct = await account(); + const contentCids = Array.from({ length: MAX_CONTENT_CIDS + 1 }, (_, i) => `bafyOverCap${i}`); + const response = await request(http()) + .post('/registry/register') + .set('Authorization', `Bearer ${acct.token}`) + .send([{ ipnsName: 'k51overcap', contentCids }]) + .expect(400); + // The engine's failure valve dead-letters on this code, never on the + // status alone — a 400 from anything but this gate must not carry it. + expect(response.body.code).toBe(REGISTRY_BATCH_REFUSED); + expect(await namesFor(acct.id)).toHaveLength(0); + }); + + it('splits an over-cap version across entries under one name, keeping the head', async () => { + const acct = await account(); + // The shape the engine's chunker sends: the head rides the first entry, + // the remainder follows as content-only entries under the same name. + await request(http()) + .post('/registry/register') + .set('Authorization', `Bearer ${acct.token}`) + .send([ + { ipnsName: 'k51chunked', headCid: 'bafyChunkedHead', contentCids: ['bafyChunkA'] }, + { ipnsName: 'k51chunked', contentCids: ['bafyChunkB'] }, + ]) + .expect(201); + const names = await namesFor(acct.id); + expect(names).toHaveLength(1); + expect(names[0].headCid).toBe('bafyChunkedHead'); + const cids = (await pinsFor(acct.id)).map((r) => r.cid).sort(); + expect(cids).toEqual(['bafyChunkA', 'bafyChunkB', 'bafyChunkedHead']); + }); }); describe('retire — union liveness, refcounted unpin', () => { diff --git a/apps/api/src/registry/registry.pipes.ts b/apps/api/src/registry/registry.pipes.ts index 3c6ee3e74..1fcfd6448 100644 --- a/apps/api/src/registry/registry.pipes.ts +++ b/apps/api/src/registry/registry.pipes.ts @@ -5,6 +5,10 @@ import { RETIRE_ARRAY_OPTIONS, RETIRE_TARGET_MAX_LENGTH, } from './dto/registry.dto'; +import { batchRefusedBody } from './registry-error-codes'; + +/** Every batch-gate refusal carries the same stable `code` (see its home). */ +const refuse = (message: unknown) => new BadRequestException(batchRefusedBody(message)); /** Reject an oversize batch up front, before per-item validation runs. */ class BatchSizePipe implements PipeTransform { @@ -15,7 +19,7 @@ class BatchSizePipe implements PipeTransform { transform(value: unknown): unknown { if (Array.isArray(value) && value.length > this.max) { - throw new BadRequestException(`Batch exceeds ${this.max} ${this.noun}`); + throw refuse(`Batch exceeds ${this.max} ${this.noun}`); } return value; } @@ -28,7 +32,7 @@ class TargetLengthPipe implements PipeTransform { transform(value: string[]): string[] { for (const target of value) { if (target.length > this.max) { - throw new BadRequestException(`target exceeds ${this.max} characters`); + throw refuse(`target exceeds ${this.max} characters`); } } return value; @@ -38,12 +42,12 @@ class TargetLengthPipe implements PipeTransform { /** Size guard first, then the register DTO validation. */ export const registerBodyPipes = [ new BatchSizePipe(MAX_BATCH, 'entries'), - new ParseArrayPipe(REGISTER_ARRAY_OPTIONS), + new ParseArrayPipe({ ...REGISTER_ARRAY_OPTIONS, exceptionFactory: refuse }), ]; /** Size guard first, then array parse, then the per-target length cap. */ export const retireBodyPipes = [ new BatchSizePipe(MAX_BATCH, 'targets'), - new ParseArrayPipe(RETIRE_ARRAY_OPTIONS), + new ParseArrayPipe({ ...RETIRE_ARRAY_OPTIONS, exceptionFactory: refuse }), new TargetLengthPipe(RETIRE_TARGET_MAX_LENGTH), ]; diff --git a/blueprint/api.md b/blueprint/api.md index b06311169..ec7db5eb9 100644 --- a/blueprint/api.md +++ b/blueprint/api.md @@ -61,7 +61,9 @@ decay) inverted into structure. version with more leaves than the per-entry cap registers as several entries under one `ipnsName`, the head riding the first; the server collapses them to one name row, and a bare re-register carrying no `headCid` leaves the stored - head untouched. + head untouched. The refusal carries `code: REGISTRY_BATCH_REFUSED`, so a + client classifies on the gate's own discriminator rather than on a bare `400` + an intermediary could have answered. - **Register-first, fail-closed**: registration precedes the first publish of a name, and publish is blocked on it. A live-but-uninventoried name is structurally impossible; the worst failure is a registered-never-published diff --git a/crates/contract/tests/contract.rs b/crates/contract/tests/contract.rs index 6d1836abd..168e209f2 100644 --- a/crates/contract/tests/contract.rs +++ b/crates/contract/tests/contract.rs @@ -20,8 +20,10 @@ use cipherbox_contract::{ use cipherbox_core::content::{CONTENT_CID_CODEC, compute_cid, encode_content_cid_str}; use cipherbox_engine::api::{ ApiClient, ApiError, ChallengeSigner, IdentityChallengeSigner, NameRegistration, + REGISTRY_BATCH_REFUSED, }; use cipherbox_engine::content::{ContentProfile, DAG_ROOT_CODEC, assemble}; +use cipherbox_engine::net::REGISTRY_BATCH_MAX; use cipherbox_engine::seams::{CredentialStore, Http, HttpMethod, HttpRequest}; type Client = ApiClient; @@ -825,18 +827,22 @@ async fn an_oversize_retire_batch_is_refused_fail_closed() { /// A register entry's `contentCids` is bounded fail-closed the same way /// (blueprint/api.md "Batch bounds"): an oversize array is refused, never -/// truncated. Register-first blocks the record PUT on this call, so a version -/// past the cap could never publish without the engine's chunking (#920). +/// truncated, and the refusal carries the `code` the engine's failure valve +/// dead-letters on. Register-first blocks the record PUT on this call, so a +/// version past the cap could never publish without the chunking (#920). #[tokio::test] async fn an_oversize_register_entry_is_refused_fail_closed() { let base = require_stack!("an_oversize_register_entry_is_refused_fail_closed"); let client = fresh_account(&base).await; let name = "k51contractRegisterBound".to_owned(); - let cids: Vec = (0..1001).map(|i| format!("bafyContractEntry{i}")).collect(); + let head = "bafyContractEntryHead".to_owned(); + let cids: Vec = (0..REGISTRY_BATCH_MAX + 1) + .map(|i| format!("bafyContractEntry{i}")) + .collect(); let over_cap = NameRegistration { ipns_name: name.clone(), - head_cid: Some("bafyContractEntryHead".to_owned()), + head_cid: Some(head.clone()), content_cids: cids.clone(), }; let error = client @@ -844,23 +850,30 @@ async fn an_oversize_register_entry_is_refused_fail_closed() { .await .expect_err("an oversize register entry must be refused"); assert!( - matches!(error, ApiError::Status { status: 400, .. }), - "the per-entry contentCids bound is fail-closed: a 400, got {error:?}" + matches!( + &error, + ApiError::Status { + status: 400, + code: Some(code), + .. + } if code == REGISTRY_BATCH_REFUSED + ), + "the per-entry contentCids bound is fail-closed and stamped: {error:?}" ); - // The engine's chunks are exactly this shape: the cap-sized entry carrying - // the head, then a content-only entry for the remainder under one name. + // The shape the engine's chunker sends: the cap-sized entry carrying the + // head, then a content-only entry for the remainder under the same name. client .register(&[ NameRegistration { ipns_name: name.clone(), - head_cid: Some("bafyContractEntryHead".to_owned()), - content_cids: cids[..1000].to_vec(), + head_cid: Some(head), + content_cids: cids[..REGISTRY_BATCH_MAX].to_vec(), }, NameRegistration { ipns_name: name, head_cid: None, - content_cids: cids[1000..].to_vec(), + content_cids: cids[REGISTRY_BATCH_MAX..].to_vec(), }, ]) .await diff --git a/crates/engine/src/api/error.rs b/crates/engine/src/api/error.rs index 05169c31f..c11368354 100644 --- a/crates/engine/src/api/error.rs +++ b/crates/engine/src/api/error.rs @@ -12,6 +12,11 @@ pub const QUOTA_EXCEEDED: &str = "QUOTA_EXCEEDED"; /// the request: the same bytes are refused on every retry. pub const UPLOAD_TOO_LARGE: &str = "UPLOAD_TOO_LARGE"; +/// The 400 `code` the registry's batch gate stamps: an over-cap or malformed +/// batch, refused identically on every retry (blueprint/api.md "Batch bounds"). +/// A 400 without it never reached that gate. +pub const REGISTRY_BATCH_REFUSED: &str = "REGISTRY_BATCH_REFUSED"; + /// A failure of an API call. /// /// Diagnostic strings carried here are the API's own error messages, never key diff --git a/crates/engine/src/api/mod.rs b/crates/engine/src/api/mod.rs index 927f3e230..9eaaa808a 100644 --- a/crates/engine/src/api/mod.rs +++ b/crates/engine/src/api/mod.rs @@ -13,7 +13,7 @@ mod signer; mod types; pub use client::ApiClient; -pub use error::{ApiError, QUOTA_EXCEEDED, UPLOAD_TOO_LARGE}; +pub use error::{ApiError, QUOTA_EXCEEDED, REGISTRY_BATCH_REFUSED, UPLOAD_TOO_LARGE}; pub use signer::{ChallengeSigner, IdentityChallengeSigner}; pub use types::{ LoginOutcome, MailboxItem, NameRegistration, Quota, SiweNonce, TestLoginOutcome, UploadResult, diff --git a/crates/engine/src/net/mod.rs b/crates/engine/src/net/mod.rs index 690bd44c9..5bef216e6 100644 --- a/crates/engine/src/net/mod.rs +++ b/crates/engine/src/net/mod.rs @@ -20,14 +20,16 @@ mod pointer_fetch; /// The registry's batch cap: the server refuses a larger array — and a larger /// per-entry `contentCids` array — fail-closed with a `400` (blueprint/api.md -/// "Batch bounds"). Every bulk caller on this plane chunks to it. -pub(crate) const REGISTRY_BATCH_MAX: usize = 1000; +/// "Batch bounds"). [`register`] and [`retire`] are the callers that chunk to +/// it, so nothing on this plane sends the raw client an unbounded batch. +pub const REGISTRY_BATCH_MAX: usize = 1000; pub mod author; pub mod eol; pub mod liveness; pub mod publish; pub mod record_publish; +pub mod register; pub mod resolve; pub mod retire; pub mod revival; @@ -46,6 +48,7 @@ pub use liveness::{ pub use pointer_fetch::RecordPointerFetch; pub use publish::{PublishError, PublishOutcome, PublishReceipt, PublishRequest, publish}; pub use record_publish::{PreflightError, RecordPublishError}; +pub use register::register; pub use resolve::{AdoptOutcome, Adopter, OwnScopeMaterial, ResolveOutcome, Resolved, resolve}; pub(crate) use resolve::{ GatedResolve, HeldMaterial, refresh_base_from_outcome, resolve_and_hold, resolve_gated, diff --git a/crates/engine/src/net/publish.rs b/crates/engine/src/net/publish.rs index 7b511d081..d116fb5c4 100644 --- a/crates/engine/src/net/publish.rs +++ b/crates/engine/src/net/publish.rs @@ -16,9 +16,9 @@ use core::time::Duration; use cipherbox_core::ipns::{IpnsName, IpnsRecord}; use cipherbox_core::suite::ed25519::Ed25519Signer; -use super::REGISTRY_BATCH_MAX; use super::eol; use super::fanout::{fanout_get_verify, fanout_put}; +use super::register::register; use crate::api::{ApiClient, ApiError, NameRegistration}; use crate::profile::SyncTimingProfile; use crate::seams::{ @@ -59,25 +59,16 @@ impl PublishRequest<'_> { format!("{IPFS_PREFIX}{}", self.head_cid).into_bytes() } - /// The registration entries for this publish, split at the registry's - /// per-entry `contentCids` cap ([`REGISTRY_BATCH_MAX`]): a version with more - /// leaves than the cap registers as several entries under the same name, - /// which the server collapses to one name row. The head rides the first - /// entry, so the name and its pointer land ahead of any content row. - fn registrations(&self) -> Vec { - let mut chunks = self.content_cids.chunks(REGISTRY_BATCH_MAX); - let head_entry = NameRegistration { + /// The single-item registration batch for this publish (ordinary writes + /// register one name; name waves and sweeps batch — that is the caller's + /// concern, blueprint/engine.md). [`register`] carries the registry's batch + /// bounds, so a version past the per-entry cap splits there. + fn registration(&self) -> NameRegistration { + NameRegistration { ipns_name: self.name.as_str().to_owned(), head_cid: Some(self.head_cid.clone()), - content_cids: chunks.next().unwrap_or_default().to_vec(), - }; - core::iter::once(head_entry) - .chain(chunks.map(|chunk| NameRegistration { - ipns_name: self.name.as_str().to_owned(), - head_cid: None, - content_cids: chunk.to_vec(), - })) - .collect() + content_cids: self.content_cids.clone(), + } } } @@ -128,9 +119,7 @@ pub struct PublishReceipt { #[derive(Debug, Clone, PartialEq, Eq)] pub enum PublishError { /// Register-first failed: the API rejected (or could not reach) the - /// registration, so no record was PUT — the fail-closed ordering law. A - /// chunked registration may have landed earlier chunks, leaving rows the - /// caller must retire. + /// registration, so no record was PUT — the fail-closed ordering law. Register(ApiError), /// No endpoint acknowledged the record PUT (the whole endpoint set is /// unreachable). Nothing durable happened; the caller retries later. @@ -171,10 +160,10 @@ where } // Register-first, fail-closed: the record never reaches the transport unless - // every registration chunk succeeds (#24 D6 / #34 D2). - for batch in request.registrations().chunks(REGISTRY_BATCH_MAX) { - api.register(batch).await.map_err(PublishError::Register)?; - } + // the registration succeeds (#24 D6 / #34 D2). + register(api, std::slice::from_ref(&request.registration())) + .await + .map_err(PublishError::Register)?; // CAS expected sequence: floor + 1 (first publish → 1, the "no floor" 0 // sentinel reserved). Revival raises the floor read to the recovered diff --git a/crates/engine/src/net/register.rs b/crates/engine/src/net/register.rs new file mode 100644 index 000000000..e9603383c --- /dev/null +++ b/crates/engine/src/net/register.rs @@ -0,0 +1,200 @@ +//! Registration: the bounded path to `POST /registry/register` +//! (blueprint/api.md "Batch bounds", "Register-first, fail-closed"). + +use super::REGISTRY_BATCH_MAX; +use crate::api::{ApiClient, ApiError, NameRegistration}; +use crate::seams::{CredentialStore, Http}; + +/// Batch-register `entries`. Idempotent server-side (blueprint/api.md), so a +/// replayed batch — a resumed name wave, or a chunk a failed pass already sent +/// — is a no-op, never an error. +/// +/// Both of the registry's bounds are enforced here so no caller carries them: +/// an entry past the per-entry `contentCids` cap splits into several entries +/// under the same `ipnsName` (the head rides the first), and the batch itself +/// chunks to [`REGISTRY_BATCH_MAX`] entries. A failing chunk leaves the earlier +/// ones registered and returns `Err`. +pub async fn register( + api: &ApiClient, + entries: &[NameRegistration], +) -> Result<(), ApiError> +where + H: Http, + C: CredentialStore, +{ + let bounded: Vec = entries.iter().flat_map(split_entry).collect(); + for chunk in bounded.chunks(REGISTRY_BATCH_MAX) { + api.register(chunk).await?; + } + Ok(()) +} + +/// One entry as the per-entry cap admits it. The head rides the first piece; +/// the rest carry content only, which leaves the name row's stored head +/// untouched (blueprint/api.md "Batch bounds"). +fn split_entry(entry: &NameRegistration) -> Vec { + let mut chunks = entry.content_cids.chunks(REGISTRY_BATCH_MAX); + let head = NameRegistration { + ipns_name: entry.ipns_name.clone(), + head_cid: entry.head_cid.clone(), + content_cids: chunks.next().unwrap_or_default().to_vec(), + }; + core::iter::once(head) + .chain(chunks.map(|chunk| NameRegistration { + ipns_name: entry.ipns_name.clone(), + head_cid: None, + content_cids: chunk.to_vec(), + })) + .collect() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::seams::{HttpMethod, HttpResponse}; + use crate::testkit::block_on; + use crate::testkit::fakes::{InMemoryCredentialStore, ScriptedHttp}; + + fn client() -> ( + ScriptedHttp, + ApiClient, + ) { + let http = ScriptedHttp::default(); + let client = ApiClient::new( + http.clone(), + InMemoryCredentialStore::default(), + "http://api.test", + ); + (http, client) + } + + fn ack(http: &ScriptedHttp, calls: usize) { + for _ in 0..calls { + http.enqueue_response(HttpResponse { + status: 200, + headers: Vec::new(), + body: Vec::new(), + }); + } + } + + /// The batch each request carried, in wire order. + fn sent(http: &ScriptedHttp) -> Vec> { + http.requests() + .iter() + .map(|request| { + let body = request.body.as_deref().expect("a register call has a body"); + serde_json::from_slice(body).expect("a register body is a JSON array") + }) + .collect() + } + + /// The `contentCids` of one wire entry. + fn cids(entry: &serde_json::Value) -> Vec { + entry["contentCids"] + .as_array() + .expect("contentCids") + .iter() + .map(|cid| cid.as_str().expect("a CID string").to_owned()) + .collect() + } + + /// `entries` as the one batch they should go out as. + fn wire(entries: &[NameRegistration]) -> Vec> { + let batch = serde_json::to_value(entries).expect("entries serialize"); + vec![batch.as_array().expect("a batch is an array").clone()] + } + + fn entry(name: &str, head: Option<&str>, cids: usize) -> NameRegistration { + NameRegistration { + ipns_name: name.to_owned(), + head_cid: head.map(str::to_owned), + content_cids: (0..cids).map(|i| format!("cid{i}")).collect(), + } + } + + #[test] + fn an_empty_batch_is_a_no_op_with_no_request() { + let (http, client) = client(); + block_on(register(&client, &[])).expect("empty register"); + assert!(http.requests().is_empty(), "no entries means no API call"); + } + + #[test] + fn an_in_bounds_entry_goes_out_untouched_in_one_batch() { + let (http, client) = client(); + ack(&http, 1); + let entries = vec![entry("k51name", Some("bafyHead"), 3)]; + block_on(register(&client, &entries)).expect("register"); + + let requests = http.requests(); + assert_eq!(requests.len(), 1); + assert_eq!(requests[0].method, HttpMethod::Post); + assert!(requests[0].url.ends_with("/registry/register")); + assert_eq!(sent(&http), wire(&entries)); + } + + #[test] + fn an_entry_past_the_per_entry_cap_splits_under_one_name() { + let (http, client) = client(); + ack(&http, 1); + let over_cap = entry("k51name", Some("bafyHead"), REGISTRY_BATCH_MAX + 2); + block_on(register(&client, core::slice::from_ref(&over_cap))).expect("register"); + + let batches = sent(&http); + assert_eq!(batches.len(), 1, "the split entries still ride one batch"); + let sizes: Vec = batches[0].iter().map(|entry| cids(entry).len()).collect(); + assert_eq!(sizes, vec![REGISTRY_BATCH_MAX, 2], "split at the cap"); + assert!( + batches[0] + .iter() + .all(|entry| entry["ipnsName"] == over_cap.ipns_name), + "every piece registers under the one name" + ); + let heads: Vec> = batches[0] + .iter() + .map(|entry| entry["headCid"].as_str()) + .collect(); + assert_eq!( + heads, + vec![Some("bafyHead"), None], + "the head rides the first piece; the rest leave the stored head alone" + ); + let sent_cids: Vec = batches[0].iter().flat_map(cids).collect(); + assert_eq!( + sent_cids, over_cap.content_cids, + "every CID reaches the registry once, in order" + ); + } + + #[test] + fn an_entry_with_no_content_still_registers_its_name_and_head() { + let (http, client) = client(); + ack(&http, 1); + let bare = vec![entry("k51name", Some("bafyHead"), 0)]; + block_on(register(&client, &bare)).expect("register"); + assert_eq!(sent(&http), wire(&bare)); + } + + #[test] + fn an_oversize_batch_splits_into_chunks_the_server_accepts() { + let (http, client) = client(); + ack(&http, 2); + let entries: Vec = (0..REGISTRY_BATCH_MAX + 1) + .map(|i| entry(&format!("k51name{i}"), None, 1)) + .collect(); + block_on(register(&client, &entries)).expect("register"); + + let batches = sent(&http); + assert_eq!( + batches.iter().map(Vec::len).collect::>(), + vec![REGISTRY_BATCH_MAX, 1], + "the batch splits at the server's cap" + ); + assert_eq!( + vec![batches.into_iter().flatten().collect::>()], + wire(&entries), + "every entry still reaches the registry once" + ); + } +} diff --git a/crates/engine/src/sync/drain.rs b/crates/engine/src/sync/drain.rs index 22992df68..149856b11 100644 --- a/crates/engine/src/sync/drain.rs +++ b/crates/engine/src/sync/drain.rs @@ -32,7 +32,7 @@ use cipherbox_core::suite::x25519::X25519Secret; use futures_channel::mpsc; use zeroize::Zeroizing; -use crate::api::{ApiClient, ApiError, QUOTA_EXCEEDED, UPLOAD_TOO_LARGE}; +use crate::api::{ApiClient, ApiError, QUOTA_EXCEEDED, REGISTRY_BATCH_REFUSED, UPLOAD_TOO_LARGE}; use crate::content::{Gateway, SealedContent, pre_flight_quota_check}; use crate::entropy::Entropy; use crate::facade::{BlockProgress, Event, NodeId, OpPhase}; @@ -175,9 +175,9 @@ enum Halt { /// against the attempt budget, because a retry re-signs at the same /// sequence and a jammed name would otherwise retry forever. Attempt, - /// An upload refusal this pass cannot attribute. Charged like - /// [`Halt::Attempt`], but raised strictly **before** the record PUT — so - /// exhausting the budget may retire what the op uploaded, which an acked + /// An upload or registration refusal this pass cannot attribute. Charged + /// like [`Halt::Attempt`], but raised strictly **before** the record PUT — + /// so exhausting the budget may retire what the op uploaded, which an acked /// PUT's may not. UploadAttempt, /// Classified-permanent: the same bytes are refused on every retry. @@ -1880,8 +1880,9 @@ fn seam(_: crate::seams::SeamError) -> Halt { Halt::Unclassified } -/// Classify a publish failure for the valve. Only the head-block upload carries -/// a server verdict this pass can act on; everything else is availability. +/// Classify a publish failure for the valve. Only the head-block upload and the +/// register-first call carry a server verdict this pass can act on; everything +/// else is availability. /// /// `refused_bytes` is what the upload asked for, so a block entered here records /// the figure its resume probe must find room for. @@ -1895,14 +1896,20 @@ fn classify_publish(error: RecordPublishError, refused_bytes: u64) -> Halt { } } -/// Classify a register-first refusal. A `400` is the registry's fail-closed -/// verdict on the batch this op builds — a malformed or over-cap entry, which -/// no retry changes. The queue is strict FIFO, so leaving it unclassified would -/// park the op at the head forever, re-registering every tick (#920). +/// Classify a register-first refusal on the discriminator the registry stamps, +/// never the status alone (the [`classify_upload`] discipline): the batch this +/// op builds is refused identically on every retry, and the queue is strict +/// FIFO, so an unclassified refusal parks the op at the head forever (#920). fn classify_register(error: ApiError) -> Halt { - match error { - ApiError::Status { status: 400, .. } => Halt::Permanent(DeadLetterReason::PayloadRefused), - _ => Halt::Unclassified, + let ApiError::Status { + status: 400, code, .. + } = error + else { + return Halt::Unclassified; + }; + match code.as_deref() { + Some(REGISTRY_BATCH_REFUSED) => Halt::Permanent(DeadLetterReason::PayloadRefused), + _ => Halt::UploadAttempt, } } @@ -1946,7 +1953,7 @@ fn upload_failure(halt: Halt) -> Option<&'static str> { // Both charge the attempt budget; which one it is decides only what // exhausting that budget retires, not what the host is told. Halt::Attempt | Halt::UploadAttempt => { - Some("the upload was refused without a classification") + Some("the network refused it without a classification") } Halt::Permanent(DeadLetterReason::PayloadRefused) => { Some("the network refused the payload") diff --git a/crates/engine/tests/write_plane.rs b/crates/engine/tests/write_plane.rs index 51b761fb9..8c28534e6 100644 --- a/crates/engine/tests/write_plane.rs +++ b/crates/engine/tests/write_plane.rs @@ -20,10 +20,11 @@ use cipherbox_core::suite::ecdsa::EcdsaSigner; use cipherbox_core::suite::ed25519::Ed25519Signer; use zeroize::Zeroizing; +use cipherbox_engine::api::REGISTRY_BATCH_REFUSED; use cipherbox_engine::content::{DAG_ROOT_CODEC, GatewaySource, SealedChunk, decode_root}; use cipherbox_engine::facade::PendingClass; use cipherbox_engine::net::author::{AuthoredHead, EnvelopeAuthoring, author_child_envelope}; -use cipherbox_engine::net::{ChildAdopter, ResolveOutcome, resolve}; +use cipherbox_engine::net::{ChildAdopter, REGISTRY_BATCH_MAX, ResolveOutcome, resolve}; use cipherbox_engine::seams::{ BoxedTask, HttpRequest, HttpResponse, OpId, RecordTransport, SeamError, SeamResult, StagingStore, UnixMillis, @@ -84,28 +85,36 @@ fn upload_413(code: Option<&str>) -> SeamResult { }) } -/// The registry's batch bounds (blueprint/api.md): at most this many entries -/// per register batch, and this many `contentCids` per entry. -const REGISTER_BATCH_CAP: usize = 1000; +/// The registry's own 400 for a batch past its bounds: the `code` the batch +/// gate stamps, which is what the valve classifies on. +fn registry_batch_refused() -> Vec { + format!(r#"{{"statusCode":400,"message":"over cap","code":"{REGISTRY_BATCH_REFUSED}"}}"#) + .into_bytes() +} + +/// A 400 answered for a registry it never reached, so it stamps no `code` — +/// [`proxy_413`]'s counterpart on the register path. +fn proxy_400() -> Vec { + b"400 Bad Request".to_vec() +} -/// The registry's fail-closed answer to a batch past its bounds: a `400`, -/// never a truncated or partial registration (blueprint/api.md "Batch bounds"). +/// Ack a registration, refusing one past the registry's bounds fail-closed — +/// never truncated or partially applied (blueprint/api.md "Batch bounds"). fn register_reply(body: Option<&[u8]>) -> SeamResult { let entries: Vec = serde_json::from_slice(body.expect("a register call carries a body")) .expect("a register body is a JSON array"); - let over_cap = entries.len() > REGISTER_BATCH_CAP + let over_cap = entries.len() > REGISTRY_BATCH_MAX || entries.iter().any(|entry| { entry["contentCids"] .as_array() - .is_some_and(|cids| cids.len() > REGISTER_BATCH_CAP) + .is_some_and(|cids| cids.len() > REGISTRY_BATCH_MAX) }); Ok(HttpResponse { status: if over_cap { 400 } else { 200 }, headers: Vec::new(), body: if over_cap { - br#"{"statusCode":400,"message":"contentCids must contain no more than 1000 elements"}"# - .to_vec() + registry_batch_refused() } else { Vec::new() }, @@ -128,8 +137,9 @@ struct Blocks { on_upload: Arc>>, /// What `GET /account/quota` reports, as `(usedBytes, limitBytes)`. quota: Arc>>, - /// A status every `POST /registry/register` answers with instead of acking. - register_refusal: Arc>>, + /// The 400 body every `POST /registry/register` answers with instead of + /// acking. + register_refusal: Arc>>>, } impl Blocks { @@ -180,9 +190,9 @@ impl Blocks { *self.quota.lock().expect("lock") = Some((used_bytes, limit_bytes)); } - /// Answer every registration with `status` instead of acking. - fn refuse_register(&self, status: u16) { - *self.register_refusal.lock().expect("lock") = Some(status); + /// Answer every registration with a 400 carrying `body` instead of acking. + fn refuse_register(&self, body: Vec) { + *self.register_refusal.lock().expect("lock") = Some(body); } /// Answer one engine HTTP call: a content upload lands its bytes here and @@ -230,12 +240,11 @@ impl Blocks { .into_bytes()); } if url.ends_with("/registry/register") { - if let Some(status) = *self.register_refusal.lock().expect("lock") { + if let Some(body) = self.register_refusal.lock().expect("lock").clone() { return Ok(HttpResponse { - status, + status: 400, headers: Vec::new(), - body: format!("{{\"statusCode\":{status},\"message\":\"refused\"}}") - .into_bytes(), + body, }); } return register_reply(request.body.as_deref()); @@ -463,35 +472,8 @@ fn uploaded_node_ids(device: &FakeDevice) -> Vec<[u8; 16]> { .collect() } -/// The `contentCids` the device's last registration for `name` carried — what a -/// sub-EOL renewal will re-pin (#797). -fn registered_content_cids(device: &FakeDevice, name: &IpnsName) -> Vec { - device - .http - .requests() - .iter() - .filter(|request| request.url.ends_with("/registry/register")) - .filter_map(|request| { - serde_json::from_slice::(request.body.as_deref()?).ok() - }) - .filter_map(|body| body.as_array()?.first().cloned()) - .filter(|entry| entry["ipnsName"] == name.as_str()) - .filter_map(|entry| { - Some( - entry["contentCids"] - .as_array()? - .iter() - .filter_map(|cid| cid.as_str().map(str::to_owned)) - .collect::>(), - ) - }) - .next_back() - .unwrap_or_default() -} - /// Every registration entry the device sent for `name`, in wire order across -/// however many batches it took — the shape a chunked registration is asserted -/// on (#920). +/// however many batches it took (#920). fn registration_entries(device: &FakeDevice, name: &IpnsName) -> Vec { device .http @@ -506,6 +488,31 @@ fn registration_entries(device: &FakeDevice, name: &IpnsName) -> Vec Vec { + entry["contentCids"] + .as_array() + .expect("an entry carries contentCids") + .iter() + .map(|cid| cid.as_str().expect("a CID string").to_owned()) + .collect() +} + +/// The `contentCids` the device's last registration for `name` carried — what a +/// sub-EOL renewal will re-pin (#797). One registration is the entry carrying +/// the head plus every content-only entry the chunker split off after it. +fn registered_content_cids(device: &FakeDevice, name: &IpnsName) -> Vec { + let entries = registration_entries(device, name); + let head = entries + .iter() + .rposition(|entry| entry["headCid"].is_string()) + .unwrap_or(0); + entries[head..] + .iter() + .flat_map(entry_content_cids) + .collect() +} + /// The node a head block about to be uploaded was sealed for. fn head_of(block: &[u8]) -> Option<[u8; 16]> { decode_envelope(block).ok().map(|envelope| envelope.id) @@ -940,24 +947,25 @@ fn a_published_version_registers_its_whole_block_set() { } /// A version with more blocks than the registry's per-entry `contentCids` cap -/// splits across several entries under one name, so the registration the -/// register-first ordering blocks on is accepted and the version publishes -/// (#920). Unchunked, the batch is refused fail-closed and nothing is PUT. +/// splits across several entries under one name, so the registration that +/// register-first blocks the record PUT on is accepted and the version +/// publishes (#920). #[test] fn a_version_past_the_registration_cap_registers_in_chunks_and_publishes() { let world = FakeWorld::new(); let blocks = Blocks::default(); seed_account(&world, &blocks); // 1001 leaves at the CI framing, plus the root: one past the cap. - let leaves = REGISTER_BATCH_CAP + 1; + let leaves = REGISTRY_BATCH_MAX + 1; let plaintext: Vec = (0..leaves * 16).map(|byte| byte as u8).collect(); let alice = world.device(b"alice"); - let (mut engine_a, _events_a, mut tasks) = boot(&world, &blocks, &alice, 42); - // One HTTP reply per block, plus the metadata plane's own calls. - serve_http(&alice, &blocks, 4 * leaves); + let (mut engine, _events, mut tasks) = boot(&world, &blocks, &alice, 42); + // An upload apiece for the leaves and the root, plus the metadata plane's + // own calls, on top of what `boot` already scripted. + serve_http(&alice, &blocks, 2 * leaves); write_file( - &mut engine_a, + &mut engine, WriteTarget::NewFile { parent: ROOT, name: "big.bin".into(), @@ -965,55 +973,38 @@ fn a_version_past_the_registration_cap_registers_in_chunks_and_publishes() { &plaintext, ) .unwrap(); - tick(&world, &engine_a, &mut tasks); + tick(&world, &engine, &mut tasks); - let node = child_id(&engine_a, ROOT, "big.bin"); + let node = child_id(&engine, ROOT, "big.bin"); let entries = registration_entries(&alice, &write_name(node)); let sizes: Vec = entries .iter() - .map(|entry| entry["contentCids"].as_array().expect("contentCids").len()) + .map(|e| entry_content_cids(e).len()) .collect(); assert_eq!( sizes, - vec![REGISTER_BATCH_CAP, 2], + vec![REGISTRY_BATCH_MAX, 2], "the registration splits at the per-entry cap" ); - let heads: Vec<&str> = entries + let with_head: Vec = entries .iter() - .filter_map(|entry| entry["headCid"].as_str()) + .enumerate() + .filter(|(_, entry)| entry["headCid"].is_string()) + .map(|(index, _)| index) .collect(); assert_eq!( - heads.len(), - 1, - "the head rides one entry; the rest carry content only" - ); - assert!( - entries[0]["headCid"].is_string(), - "the head rides the first entry, so the name and its pointer land first" + with_head, + vec![0], + "the head rides the first entry alone, so the name and its pointer land first" ); - let registered: Vec = entries - .iter() - .flat_map(|entry| { - entry["contentCids"] - .as_array() - .expect("contentCids") - .iter() - .map(|cid| cid.as_str().expect("a CID string").to_owned()) - .collect::>() - }) - .collect(); - assert_eq!( - registered.len(), - leaves + 1, - "every block the version links still rides the registration exactly once" - ); + let registered = registered_content_cids(&alice, &write_name(node)); assert!( registered.iter().all(|cid| blocks.get(cid).is_some()), "every registered CID names a block the provider holds" ); assert!( - block_on(engine_a.snapshot(ROOT)) + block_on(engine.snapshot(ROOT)) .unwrap() .dead_letters .is_empty(), @@ -1021,18 +1012,18 @@ fn a_version_past_the_registration_cap_registers_in_chunks_and_publishes() { ); } -/// A registration the registry refuses is refused on every retry, and the queue -/// is strict FIFO — so the op dead-letters instead of holding the head and -/// re-registering every tick (#920). +/// A registration the registry itself refuses is refused on every retry, and +/// the queue is strict FIFO — so the op dead-letters instead of holding the head +/// and re-registering every tick (#920). #[test] -fn a_refused_registration_dead_letters_instead_of_holding_the_queue_head() { +fn a_registration_the_registry_refuses_dead_letters_instead_of_holding_the_queue_head() { let world = FakeWorld::new(); let blocks = Blocks::default(); seed_account(&world, &blocks); - let alice = world.device(b"alice"); let (mut engine, _events, mut tasks) = boot(&world, &blocks, &alice, 42); - blocks.refuse_register(400); + blocks.refuse_register(registry_batch_refused()); + let op_id = write_file( &mut engine, WriteTarget::NewFile { @@ -1044,7 +1035,10 @@ fn a_refused_registration_dead_letters_instead_of_holding_the_queue_head() { .unwrap(); let (dead_letters, passes) = tick_until_dead_lettered(&world, &engine, &mut tasks); - assert_eq!(passes, 1, "a refused registration is permanent on sight"); + assert_eq!( + passes, 1, + "the registry's own verdict is permanent on sight" + ); assert_eq!( dead_letters, vec![DeadLetter { @@ -1052,6 +1046,46 @@ fn a_refused_registration_dead_letters_instead_of_holding_the_queue_head() { reason: DeadLetterReason::PayloadRefused }] ); + assert!( + !retire_targets(&alice).is_empty(), + "the abandonment retires what the refused registration's chunks charged" + ); +} + +/// A `400` the registry did not stamp is evidence of nothing (#848): the op is +/// charged like any other pre-PUT refusal and survives until its budget runs +/// out, rather than being abandoned on an intermediary's say-so. +#[test] +fn a_registration_400_from_an_intermediary_is_charged_not_permanent() { + let world = FakeWorld::new(); + let blocks = Blocks::default(); + seed_account(&world, &blocks); + let alice = world.device(b"alice"); + let (mut engine, _events, mut tasks) = boot(&world, &blocks, &alice, 42); + blocks.refuse_register(proxy_400()); + + let op_id = write_file( + &mut engine, + WriteTarget::NewFile { + parent: ROOT, + name: "photo.bin".into(), + }, + &(0..200u8).collect::>(), + ) + .unwrap(); + + let (dead_letters, passes) = tick_until_dead_lettered(&world, &engine, &mut tasks); + assert!( + passes > 1, + "an unattributable refusal is a charged attempt, not a verdict" + ); + assert_eq!( + dead_letters, + vec![DeadLetter { + op_id, + reason: DeadLetterReason::AttemptsExhausted + }] + ); } /// The `pushChunk` total is cross-checked against the `beginWrite` declaration: From fcaf4fb537bea493a033e5cda37471336e267e64 Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Sat, 1 Aug 2026 11:58:58 +0200 Subject: [PATCH 3/4] fix(api): keep the registry's batch refusal from echoing the caller's entry The crypto/privacy gate found that routing ParseArrayPipe's exceptionFactory straight into the error body replaced Nest's flattened constraint strings with the raw ValidationError objects - which carry `target` and `value`, so an over-cap register 400 echoed the caller's ipnsName and every contentCid back, twice, into a body that request logging and intermediaries capture. The factory now flattens to constraint strings, restoring the uniform envelope the rest of the surface emits. An explicit `headCid: null` is also refused rather than clearing the stored head: continuation entries of a chunked registration omit the field, and the invariant blueprint/api.md now states has no way to express the opposite. --- apps/api/src/registry/dto/registry.dto.ts | 7 +++++-- .../registry.http.integration.test.ts | 19 ++++++++++++++++++ apps/api/src/registry/registry.pipes.ts | 20 ++++++++++++++++++- 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/apps/api/src/registry/dto/registry.dto.ts b/apps/api/src/registry/dto/registry.dto.ts index b2ce7ccf9..55720e36d 100644 --- a/apps/api/src/registry/dto/registry.dto.ts +++ b/apps/api/src/registry/dto/registry.dto.ts @@ -3,10 +3,10 @@ import { ArrayMaxSize, IsArray, IsBoolean, - IsOptional, IsString, Matches, MaxLength, + ValidateIf, } from 'class-validator'; /** @@ -36,7 +36,10 @@ export class RegisterEntryDto { required: false, description: 'Current head (metadata) CID this name publishes; omit to register the name only.', }) - @IsOptional() + // Omitted, never null: a chunked registration's continuation entries leave + // the field out so the stored head survives, and an explicit null would clear + // it instead. Refused rather than silently ignored (blueprint/api.md). + @ValidateIf((entry: RegisterEntryDto) => entry.headCid !== undefined) @IsString() @MaxLength(256) @Matches(CID_OR_NAME, { message: 'headCid must be a bare CID token' }) diff --git a/apps/api/src/registry/registry.http.integration.test.ts b/apps/api/src/registry/registry.http.integration.test.ts index 81280e2f4..496e2d483 100644 --- a/apps/api/src/registry/registry.http.integration.test.ts +++ b/apps/api/src/registry/registry.http.integration.test.ts @@ -161,9 +161,28 @@ describe('registry HTTP surface (real Postgres)', () => { // The engine's failure valve dead-letters on this code, never on the // status alone — a 400 from anything but this gate must not carry it. expect(response.body.code).toBe(REGISTRY_BATCH_REFUSED); + // Constraint strings only: an error body that echoed the rejected entry + // would put the caller's name and CIDs everywhere it is logged. + expect(response.body.message).toEqual(expect.arrayContaining([expect.any(String)])); + expect(JSON.stringify(response.body)).not.toContain('k51overcap'); expect(await namesFor(acct.id)).toHaveLength(0); }); + it('refuses an explicit null headCid rather than clearing the stored head', async () => { + const acct = await account(); + await request(http()) + .post('/registry/register') + .set('Authorization', `Bearer ${acct.token}`) + .send([{ ipnsName: 'k51nullhead', headCid: 'bafyKeepMe', contentCids: [] }]) + .expect(201); + await request(http()) + .post('/registry/register') + .set('Authorization', `Bearer ${acct.token}`) + .send([{ ipnsName: 'k51nullhead', headCid: null, contentCids: [] }]) + .expect(400); + expect((await namesFor(acct.id))[0].headCid).toBe('bafyKeepMe'); + }); + it('splits an over-cap version across entries under one name, keeping the head', async () => { const acct = await account(); // The shape the engine's chunker sends: the head rides the first entry, diff --git a/apps/api/src/registry/registry.pipes.ts b/apps/api/src/registry/registry.pipes.ts index 1fcfd6448..d89b564a6 100644 --- a/apps/api/src/registry/registry.pipes.ts +++ b/apps/api/src/registry/registry.pipes.ts @@ -1,4 +1,5 @@ import { BadRequestException, ParseArrayPipe, PipeTransform } from '@nestjs/common'; +import { ValidationError } from 'class-validator'; import { MAX_BATCH, REGISTER_ARRAY_OPTIONS, @@ -7,8 +8,25 @@ import { } from './dto/registry.dto'; import { batchRefusedBody } from './registry-error-codes'; +/** The constraint strings alone: a validation error also carries the rejected + * entry, and echoing a caller's whole batch back into an error body puts its + * names and CIDs everywhere the response is logged. */ +function constraintMessages(errors: ValidationError[]): string[] { + return errors.flatMap((error) => [ + ...Object.values(error.constraints ?? {}), + ...constraintMessages(error.children ?? []), + ]); +} + /** Every batch-gate refusal carries the same stable `code` (see its home). */ -const refuse = (message: unknown) => new BadRequestException(batchRefusedBody(message)); +const refuse = (error: unknown) => + new BadRequestException( + batchRefusedBody( + Array.isArray(error) && error.every((item) => item instanceof ValidationError) + ? constraintMessages(error) + : error + ) + ); /** Reject an oversize batch up front, before per-item validation runs. */ class BatchSizePipe implements PipeTransform { From 2d27cbd49c1286ff457bc2e92fee792639db425a Mon Sep 17 00:00:00 2001 From: Michael Yankelev Date: Sun, 2 Aug 2026 20:12:12 +0200 Subject: [PATCH 4/4] fix(api): document the registry batch-refusal body in the OpenAPI contract The 400 responses promised a stable `code` in prose but carried no schema, so a client had no documented shape to classify on. Both register and retire now answer `BatchRefusedDto`, and every refusal normalizes its message to a string list so the documented shape is the only one emitted. Cover the code on the DTO-validation and target-length paths, and assert a refused batch leaves no pin rows and echoes no submitted content CID. --- apps/api/openapi.json | 50 ++++++++++++++++++- apps/api/src/registry/registry-error-codes.ts | 35 +++++++++---- apps/api/src/registry/registry.controller.ts | 4 +- .../registry.http.integration.test.ts | 12 ++++- apps/api/src/registry/registry.pipes.ts | 20 ++++---- 5 files changed, 97 insertions(+), 24 deletions(-) diff --git a/apps/api/openapi.json b/apps/api/openapi.json index 8ec6d599b..6c372b7f9 100644 --- a/apps/api/openapi.json +++ b/apps/api/openapi.json @@ -452,7 +452,14 @@ } }, "400": { - "description": "Malformed or over-cap batch; the body carries code REGISTRY_BATCH_REFUSED" + "description": "Malformed or over-cap batch; the body carries code REGISTRY_BATCH_REFUSED", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BatchRefusedDto" + } + } + } }, "401": { "description": "Missing or invalid access token" @@ -506,7 +513,14 @@ } }, "400": { - "description": "Malformed or over-cap batch; the body carries code REGISTRY_BATCH_REFUSED" + "description": "Malformed or over-cap batch; the body carries code REGISTRY_BATCH_REFUSED", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/BatchRefusedDto" + } + } + } }, "401": { "description": "Missing or invalid access token" @@ -1114,6 +1128,38 @@ "cids" ] }, + "BatchRefusedDto": { + "type": "object", + "properties": { + "statusCode": { + "type": "number", + "example": 400 + }, + "message": { + "description": "Constraint strings only — never the rejected entry", + "type": "array", + "items": { + "type": "string" + } + }, + "error": { + "type": "string", + "example": "Bad Request" + }, + "code": { + "type": "string", + "enum": [ + "REGISTRY_BATCH_REFUSED" + ] + } + }, + "required": [ + "statusCode", + "message", + "error", + "code" + ] + }, "RetireResponseDto": { "type": "object", "properties": { diff --git a/apps/api/src/registry/registry-error-codes.ts b/apps/api/src/registry/registry-error-codes.ts index 2c69a820f..644f07435 100644 --- a/apps/api/src/registry/registry-error-codes.ts +++ b/apps/api/src/registry/registry-error-codes.ts @@ -1,17 +1,32 @@ +import { ApiProperty } from '@nestjs/swagger'; + /** * The registry's batch routes answer 400 for a refusal the caller can never - * retry past — an over-cap batch, an over-cap `contentCids`, a malformed entry. - * The body carries a stable `code` so a client classifies on it instead of - * parsing `message`, and so a 400 from anything that is NOT this gate (a proxy, - * a body-size cap) stays unattributable (#920, mirroring the 413 codes #842). + * retry past. Clients classify on this stable `code`, so a 400 from anything + * that is NOT this gate stays unattributable (blueprint/api.md). */ export const REGISTRY_BATCH_REFUSED = 'REGISTRY_BATCH_REFUSED'; -/** - * The 400 body the register/retire pipes emit. Nest stops synthesizing - * `statusCode`/`error` as soon as an exception is given an object, so the full - * envelope is built here once rather than re-typed at each throw site. - */ -export function batchRefusedBody(message: unknown) { +/** The documented 400 body; `batchRefusedBody` returns exactly this shape. */ +export class BatchRefusedDto { + @ApiProperty({ example: 400 }) + statusCode!: number; + + @ApiProperty({ + type: [String], + description: 'Constraint strings only — never the rejected entry', + }) + message!: string[]; + + @ApiProperty({ example: 'Bad Request' }) + error!: string; + + @ApiProperty({ enum: [REGISTRY_BATCH_REFUSED] }) + code!: string; +} + +/** Nest stops synthesizing `statusCode`/`error` once an exception carries an + * object, so the whole envelope is built here rather than at each throw site. */ +export function batchRefusedBody(message: string[]): BatchRefusedDto { return { statusCode: 400, message, error: 'Bad Request', code: REGISTRY_BATCH_REFUSED }; } diff --git a/apps/api/src/registry/registry.controller.ts b/apps/api/src/registry/registry.controller.ts index 6ae798283..71bd62d39 100644 --- a/apps/api/src/registry/registry.controller.ts +++ b/apps/api/src/registry/registry.controller.ts @@ -19,7 +19,7 @@ import { RETIRE_TARGET_MAX_LENGTH, RetireResponseDto, } from './dto/registry.dto'; -import { REGISTRY_BATCH_REFUSED } from './registry-error-codes'; +import { BatchRefusedDto, REGISTRY_BATCH_REFUSED } from './registry-error-codes'; import { registerBodyPipes, retireBodyPipes } from './registry.pipes'; import { RegistryService } from './services/registry.service'; @@ -54,6 +54,7 @@ export class RegistryController { @ApiCreatedResponse({ type: RegisterResponseDto }) @ApiResponse({ status: 400, + type: BatchRefusedDto, description: `Malformed or over-cap batch; the body carries code ${REGISTRY_BATCH_REFUSED}`, }) @ApiResponse({ status: 401, description: 'Missing or invalid access token' }) @@ -82,6 +83,7 @@ export class RegistryController { @ApiCreatedResponse({ type: RetireResponseDto }) @ApiResponse({ status: 400, + type: BatchRefusedDto, description: `Malformed or over-cap batch; the body carries code ${REGISTRY_BATCH_REFUSED}`, }) @ApiResponse({ status: 401, description: 'Missing or invalid access token' }) diff --git a/apps/api/src/registry/registry.http.integration.test.ts b/apps/api/src/registry/registry.http.integration.test.ts index 496e2d483..30788bc1e 100644 --- a/apps/api/src/registry/registry.http.integration.test.ts +++ b/apps/api/src/registry/registry.http.integration.test.ts @@ -165,7 +165,9 @@ describe('registry HTTP surface (real Postgres)', () => { // would put the caller's name and CIDs everywhere it is logged. expect(response.body.message).toEqual(expect.arrayContaining([expect.any(String)])); expect(JSON.stringify(response.body)).not.toContain('k51overcap'); + expect(JSON.stringify(response.body)).not.toContain(contentCids[0]); expect(await namesFor(acct.id)).toHaveLength(0); + expect(await pinsFor(acct.id)).toHaveLength(0); }); it('refuses an explicit null headCid rather than clearing the stored head', async () => { @@ -175,11 +177,15 @@ describe('registry HTTP surface (real Postgres)', () => { .set('Authorization', `Bearer ${acct.token}`) .send([{ ipnsName: 'k51nullhead', headCid: 'bafyKeepMe', contentCids: [] }]) .expect(201); - await request(http()) + const refused = await request(http()) .post('/registry/register') .set('Authorization', `Bearer ${acct.token}`) .send([{ ipnsName: 'k51nullhead', headCid: null, contentCids: [] }]) .expect(400); + // DTO validation refuses inside ParseArrayPipe, which hands its + // exceptionFactory flattened strings — the code must survive that path. + expect(refused.body.code).toBe(REGISTRY_BATCH_REFUSED); + expect(refused.body.message).toEqual(expect.arrayContaining([expect.any(String)])); expect((await namesFor(acct.id))[0].headCid).toBe('bafyKeepMe'); }); @@ -241,11 +247,13 @@ describe('registry HTTP surface (real Postgres)', () => { it('rejects an over-length target at the pipe (256-char cap)', async () => { const acct = await account(); - await request(http()) + const refused = await request(http()) .post('/registry/retire') .set('Authorization', `Bearer ${acct.token}`) .send(['a'.repeat(257)]) .expect(400); + expect(refused.body.code).toBe(REGISTRY_BATCH_REFUSED); + expect(refused.body.message).toEqual(expect.arrayContaining([expect.any(String)])); }); }); diff --git a/apps/api/src/registry/registry.pipes.ts b/apps/api/src/registry/registry.pipes.ts index d89b564a6..68b864714 100644 --- a/apps/api/src/registry/registry.pipes.ts +++ b/apps/api/src/registry/registry.pipes.ts @@ -18,15 +18,17 @@ function constraintMessages(errors: ValidationError[]): string[] { ]); } -/** Every batch-gate refusal carries the same stable `code` (see its home). */ -const refuse = (error: unknown) => - new BadRequestException( - batchRefusedBody( - Array.isArray(error) && error.every((item) => item instanceof ValidationError) - ? constraintMessages(error) - : error - ) - ); +/** `ParseArrayPipe` hands its `exceptionFactory` already-flattened strings; the + * size and length guards hand a single string. */ +function messagesOf(error: unknown): string[] { + if (!Array.isArray(error)) return [String(error)]; + return error.every((item) => item instanceof ValidationError) + ? constraintMessages(error) + : error.map(String); +} + +/** Every batch-gate refusal answers the one documented body (see its home). */ +const refuse = (error: unknown) => new BadRequestException(batchRefusedBody(messagesOf(error))); /** Reject an oversize batch up front, before per-item validation runs. */ class BatchSizePipe implements PipeTransform {