From 61137494f7caccdf2e32902e3f883727a838e2f1 Mon Sep 17 00:00:00 2001 From: Ghost Scripter Date: Mon, 7 Sep 2026 13:57:50 +0530 Subject: [PATCH] Ask CortexDB for every scope instead of its first fifty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CortexDialect::scopes` called `v1/scopes/list` with no parameters. The endpoint caps the listing at fifty and documents neither the cap nor the `limit` that lifts it: its OpenAPI entry lists no parameters at all, and the response carries no cursor, no `has_more` and no total, so there is nothing in it that tells a caller it was cut short. That listing is what `entries` and `namespace_summaries` enumerate, and so what `export_page` walks. A deployment holding more namespaces than the cap exported a subset of itself and reported success — a live instance measured 93. Per-namespace reads address a namespace directly and never notice, which is why this stayed invisible. Ask for a limit far above any namespace count, and refuse a response that fills it rather than returning it: with no cursor, a full page and a truncated one are the same response, and the same reasoning as MAX_PAGES applies — a loud error beats a short answer the caller cannot detect. The test double now models the cap, so every Cortex conformance test exercises the real shape rather than a listing that always returns everything. --- .../tinymemory-remote/src/conformance_test.rs | 101 +++++++++++++++++- crates/tinymemory-remote/src/cortex.rs | 54 ++++++++-- 2 files changed, 144 insertions(+), 11 deletions(-) diff --git a/crates/tinymemory-remote/src/conformance_test.rs b/crates/tinymemory-remote/src/conformance_test.rs index ebedace..0d8bb8f 100644 --- a/crates/tinymemory-remote/src/conformance_test.rs +++ b/crates/tinymemory-remote/src/conformance_test.rs @@ -818,7 +818,19 @@ async fn cortex_recall(State(store): State, Json(body): Json Json(json!({ "layers": { "events": hits } })) } -async fn cortex_scopes(State(store): State) -> Json { +/// The real engine caps this listing at fifty unless `limit` says otherwise, +/// and documents neither the cap nor the parameter — the response carries no +/// cursor and no total, so a caller that does not ask cannot tell it was cut +/// short. The double reproduces the cap, because a double that returns +/// everything cannot catch the adapter forgetting to ask. +async fn cortex_scopes( + State(store): State, + Query(params): Query>, +) -> Json { + let limit = params + .get("limit") + .and_then(|l| l.parse::().ok()) + .unwrap_or(50); let log = store.lock().expect("cortex log"); let mut paths: Vec = log .events @@ -828,6 +840,7 @@ async fn cortex_scopes(State(store): State) -> Json { .collect(); paths.sort(); paths.dedup(); + paths.truncate(limit); Json(json!({ "items": paths.into_iter().map(|p| json!({ "path": p })).collect::>() })) @@ -1207,3 +1220,89 @@ async fn a_replay_returns_the_event_its_own_key_created() { "key-b replayed with another key\'s event" ); } + +/// A namespace count above the engine's undocumented default must not be +/// silently truncated. +/// +/// `scopes` feeds `entries`, which feeds `namespace_summaries`, which feeds +/// `export_page` and so `opencompany memory migrate`. Before this was fixed the +/// adapter sent no `limit`, so a company with more than fifty namespaces +/// migrated a subset of itself and the migration reported success. Per-namespace +/// reads never notice, which is why it stayed invisible. +#[tokio::test] +async fn every_namespace_is_listed_past_the_engines_undocumented_default() { + let store: CortexStore = Arc::new(Mutex::new(CortexLog::default())); + let app = Router::new() + .route("/v1/experience", post(cortex_experience)) + .route("/v1/events", get(cortex_events)) + .route("/v1/forget", post(cortex_forget)) + .route("/v1/recall", post(cortex_recall)) + .route("/v1/scopes/list", get(cortex_scopes)) + .route( + "/v1/admin/health", + get(|| async { Json(json!({ "status": "healthy" })) }), + ) + .with_state(store); + let endpoint = serve(app).await; + + let memory = CortexMemory::api(&endpoint, "test-key").expect("client"); + // Comfortably past fifty, and past it by enough that an off-by-one in the + // cap would not pass by luck. + const NAMESPACES: usize = 64; + for n in 0..NAMESPACES { + memory + .store( + &format!("oc/acme-{n:032x}"), + "k", + "v", + MemoryCategory::Core, + None, + ) + .await + .expect("store"); + } + + let summaries = memory.namespace_summaries().await.expect("summaries"); + assert_eq!( + summaries.len(), + NAMESPACES, + "a migration reading this would have left {} namespaces behind without saying so", + NAMESPACES.saturating_sub(summaries.len()) + ); +} + +/// A listing that exactly fills the limit is refused, not returned. +/// +/// The engine sends no cursor, no `has_more` and no total, so a response +/// holding as many entries as were asked for is indistinguishable from one that +/// was cut short. Returning it would hand a caller a subset labelled as the +/// whole, which is the failure this guard exists to prevent — better a loud +/// error than a migration that quietly leaves namespaces behind. +#[tokio::test] +async fn a_scope_listing_that_fills_the_limit_is_refused_rather_than_trusted() { + // The engine's ceiling as the adapter asks for it. Kept as a literal on + // purpose: this test should fail loudly if `SCOPE_LIST_LIMIT` moves without + // someone reconsidering the guard. + const ASKED_FOR: usize = 10_000; + let app = Router::new().route( + "/v1/scopes/list", + get(|| async { + let items: Vec = (0..ASKED_FOR) + .map(|n| json!({ "path": format!("tenant:acme-{n}") })) + .collect(); + Json(json!({ "items": items })) + }), + ); + let endpoint = serve(app).await; + + let memory = CortexMemory::api(&endpoint, "test-key").expect("client"); + let error = memory + .namespace_summaries() + .await + .expect_err("a full page must not pass for a complete listing"); + let rendered = format!("{error:#}"); + assert!( + rendered.contains("truncated"), + "the error must say why the listing cannot be trusted, got: {rendered}" + ); +} diff --git a/crates/tinymemory-remote/src/cortex.rs b/crates/tinymemory-remote/src/cortex.rs index cd0218b..fd09a00 100644 --- a/crates/tinymemory-remote/src/cortex.rs +++ b/crates/tinymemory-remote/src/cortex.rs @@ -159,6 +159,19 @@ const RECALL_QUERY_CAP: usize = 256; /// failure this whole adapter exists to avoid. const MAX_PAGES: usize = 500; +/// How many scopes one `v1/scopes/list` call asks for. +/// +/// The endpoint defaults to **50** and says so nowhere: its OpenAPI entry +/// documents no parameters at all, and the response carries only `items` — no +/// cursor, no `has_more`, no total. So a bare call silently returns the first +/// fifty of however many exist, which was measured on a live engine holding 93. +/// +/// `limit` is honoured even though it is undocumented, and there is nothing to +/// page with, so the only defence is to ask for far more than a namespace count +/// should ever reach and treat a full response as untrustworthy — see +/// [`CortexDialect::scopes`]. +const SCOPE_LIST_LIMIT: usize = 10_000; + /// CortexDB, adapted to TinyMemory's keyed contract. #[derive(Debug)] pub struct CortexMemory { @@ -606,27 +619,48 @@ impl CortexDialect { } /// Every scope this deployment holds that this adapter wrote. + /// + /// Asks for [`SCOPE_LIST_LIMIT`] explicitly. Without it the engine returns + /// its undocumented default of fifty, and the callers that matter here — + /// `entries`, `namespace_summaries`, and through them `export_page` and + /// `opencompany memory migrate` — would enumerate a *subset* of a company's + /// namespaces while reporting success. Per-namespace reads never notice, + /// because they address a namespace directly, which is why a truncation + /// here stays invisible until a migration quietly leaves records behind. + /// + /// A response that fills the limit is refused rather than returned. There + /// is no cursor and no total to check against, so a full page is + /// indistinguishable from a truncated one, and the same reasoning as + /// [`MAX_PAGES`] applies: a silently short listing is worse than an error, + /// because the caller cannot tell it happened. async fn scopes(&self) -> anyhow::Result> { let listing: Value = self .client .json( Method::GET, - "v1/scopes/list", + &format!("v1/scopes/list?limit={SCOPE_LIST_LIMIT}"), None, Attempts::RetryTransient, ) .await?; - Ok(listing + let items = listing .get("items") .and_then(Value::as_array) - .map(|items| { - items - .iter() - .filter_map(|s| s.get("path").and_then(Value::as_str)) - .filter_map(Self::namespace_of) - .collect() - }) - .unwrap_or_default()) + .cloned() + .unwrap_or_default(); + if items.len() >= SCOPE_LIST_LIMIT { + anyhow::bail!( + "scope listing returned {SCOPE_LIST_LIMIT} entries, the limit it was asked \ + for; the engine offers no cursor, so a complete listing cannot be \ + distinguished from a truncated one and enumerating namespaces would \ + silently skip whatever came after" + ); + } + Ok(items + .iter() + .filter_map(|s| s.get("path").and_then(Value::as_str)) + .filter_map(Self::namespace_of) + .collect()) } }