Skip to content

Commit c9cb601

Browse files
Stop printing API keys back out, and bound the hosted listing walk
Four things the review found, all in the hosted-engine path this PR added. `RequestBuilder::bearer_auth` marks its value sensitive; `header` handed a plain string does not. So the two schemes with no such helper -- Cognee's `X-API-Key` and Mem0's `Authorization: Token` -- carried a live credential through every `Debug` rendering of the request. The test that pins this prints the leak when reverted: `no sensitive header on {"x-api-key": "cg-secret"}`. Both now go through one helper that sets the flag, and that parses the value up front so a credential holding a newline fails at the call site by name rather than inside `send` where it reads as a transport fault. The parse error carries no value, so the refusal cannot echo the key either. Mem0's hosted listing stopped on an empty page or a null `next` -- both server-controlled. A server that keeps answering a full page and a cursor spun the loop and grew the buffer until the process died. It is now bounded at 500 pages of 200 and fails saying so, which is what the self-hosted arm already did at its own ceiling. The page size became a constant so the message cannot drift from the request. The `X-API-Key` doc line had ended up above `token` instead of `api_key`, leaving one constructor with two contradictory doc lines and the other with none. The README still said "plus self-hosted Mem0" one paragraph after the table started advertising the hosted platform, and showed `CogneeMemory::api` twice with the same shape; the second is now the `Mem0Memory::cloud` constructor that section exists to document, and the auth paragraph names Mem0's two schemes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 514f07e commit c9cb601

4 files changed

Lines changed: 169 additions & 19 deletions

File tree

README.md

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -177,10 +177,10 @@ that skips enforcement is the entire reason the policy layer exists.
177177
## Remote engines
178178

179179
The `tinymemory-remote` crate supports the managed and self-hosted native APIs
180-
of Supermemory and Cognee, plus self-hosted Mem0. Each adapter stores
181-
TinyMemory's key, category, session, and provenance in backend metadata (or a
182-
Cognee raw-data envelope), so exact CRUD and portability survive the seam while
183-
recall remains engine-native. Provider-facing dataset names, container tags,
180+
of Supermemory, Cognee, and Mem0. Each adapter stores TinyMemory's key,
181+
category, session, and provenance in backend metadata (or a Cognee raw-data
182+
envelope), so exact CRUD and portability survive the seam while recall remains
183+
engine-native. Provider-facing dataset names, container tags,
184184
and filenames are bounded stable hashes, so every namespace and key accepted by
185185
the TinyMemory contract remains valid on the remote API.
186186

@@ -196,21 +196,23 @@ Managed APIs have explicit constructors so their authentication cannot be
196196
confused with a self-hosted token:
197197

198198
```rust
199-
use tinymemory_remote::{CogneeMemory, SupermemoryMemory};
199+
use tinymemory_remote::{CogneeMemory, Mem0Memory, SupermemoryMemory};
200200

201201
// Cognee Cloud issues a per-tenant base URL (the API-key dashboard shows it);
202-
// there is no shared endpoint.
202+
// there is no shared endpoint, so its constructor takes one.
203203
let cognee = CogneeMemory::api("https://tenant-<uuid>.aws.cognee.ai", "cognee-api-key")?;
204-
let supermemory = SupermemoryMemory::cloud("sm_...")?;
205204

206-
// Cognee also issues tenant-specific API origins.
207-
let tenant = CogneeMemory::api("https://tenant.example.cognee.ai", "api-key")?;
208-
# Ok::<_, anyhow::Error>((cognee, supermemory, tenant))
205+
// Supermemory and Mem0 both serve one hosted origin, so theirs take only a key.
206+
let supermemory = SupermemoryMemory::cloud("sm_...")?;
207+
let mem0 = Mem0Memory::cloud("m0-...")?;
208+
# Ok::<_, anyhow::Error>((cognee, supermemory, mem0))
209209
```
210210

211211
Cognee Cloud uses `X-Api-Key`; authenticated self-hosted Cognee uses a bearer
212-
access token. Supermemory uses bearer API keys for both deployment modes. All
213-
constructors redact credentials from `Debug` output and transport errors.
212+
access token. Supermemory uses bearer API keys for both deployment modes. Mem0's
213+
hosted platform uses `Authorization: Token`, and self-hosted Mem0 uses
214+
`X-API-Key`. All constructors redact credentials from `Debug` output, from
215+
transport errors, and from the request's own header rendering.
214216

215217
All three advertise the mandatory Core, Recall, and Portability families. The
216218
live Docker harness and conformance command are documented in

adapters/remote/src/common.rs

Lines changed: 93 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ use std::collections::BTreeMap;
44

55
use anyhow::{bail, Context};
66
use async_trait::async_trait;
7+
use reqwest::header::{HeaderValue, AUTHORIZATION};
78
use reqwest::{Method, RequestBuilder, StatusCode, Url};
89
use serde::{de::DeserializeOwned, Deserialize, Serialize};
910
use sha2::{Digest, Sha256};
@@ -97,6 +98,27 @@ async fn read_capped(response: reqwest::Response, path: &str) -> anyhow::Result<
9798
Ok(body)
9899
}
99100

101+
/// Wraps a credential in a header value that will not be printed back out.
102+
///
103+
/// `RequestBuilder::bearer_auth` marks its `Authorization` value sensitive on
104+
/// the caller's behalf; `RequestBuilder::header` handed a plain string does
105+
/// not. So the two schemes that have no such helper -- `X-API-Key` and
106+
/// `Authorization: Token` -- would otherwise carry a live API key through
107+
/// every `Debug` rendering of the request and through any middleware that
108+
/// formats headers. The flag is set here instead.
109+
///
110+
/// Parsing up front is the second half of the same fix: a credential holding a
111+
/// newline or another byte no header may carry becomes an error at the call
112+
/// site, naming the credential, rather than a deferred failure inside `send`
113+
/// that reads as a transport fault. The parse error carries no value, so the
114+
/// credential does not reach the message either.
115+
fn credential_header(value: &str) -> anyhow::Result<HeaderValue> {
116+
let mut header =
117+
HeaderValue::from_str(value).context("credential is not a valid HTTP header value")?;
118+
header.set_sensitive(true);
119+
Ok(header)
120+
}
121+
100122
impl HttpClient {
101123
/// Builds a client that optionally authenticates with a bearer token.
102124
pub(crate) fn bearer(endpoint: &str, credential: Option<&str>) -> anyhow::Result<Self> {
@@ -106,7 +128,6 @@ impl HttpClient {
106128
)
107129
}
108130

109-
/// Builds a client that optionally authenticates with `X-API-Key`.
110131
/// A client authenticating with `Authorization: Token <key>`.
111132
pub(crate) fn token(endpoint: &str, credential: Option<&str>) -> anyhow::Result<Self> {
112133
Self::new(
@@ -115,6 +136,7 @@ impl HttpClient {
115136
)
116137
}
117138

139+
/// Builds a client that optionally authenticates with `X-API-Key`.
118140
pub(crate) fn api_key(endpoint: &str, credential: Option<&str>) -> anyhow::Result<Self> {
119141
Self::new(
120142
endpoint,
@@ -151,8 +173,10 @@ impl HttpClient {
151173
Ok(match &self.auth {
152174
Auth::None => request,
153175
Auth::Bearer(token) => request.bearer_auth(token),
154-
Auth::ApiKey(key) => request.header("X-API-Key", key),
155-
Auth::Token(key) => request.header("Authorization", format!("Token {key}")),
176+
Auth::ApiKey(key) => request.header("X-API-Key", credential_header(key)?),
177+
Auth::Token(key) => {
178+
request.header(AUTHORIZATION, credential_header(&format!("Token {key}"))?)
179+
}
156180
})
157181
}
158182

@@ -622,6 +646,72 @@ fn classify_transport(is_timeout: bool, is_connect: bool, chain: &str) -> &'stat
622646
}
623647
}
624648

649+
#[cfg(test)]
650+
mod credential_header_tests {
651+
#![allow(clippy::expect_used, clippy::panic)]
652+
653+
use super::{credential_header, Auth, HttpClient};
654+
655+
/// The point of the helper. `reqwest` only redacts a header value whose
656+
/// sensitive flag is set, and `RequestBuilder::header` handed a plain
657+
/// string leaves it clear -- which is how an API key ends up rendered in
658+
/// full by anything that formats the request.
659+
#[test]
660+
fn a_credential_header_is_marked_sensitive() {
661+
let header = credential_header("Token m0-secret").expect("a plain key is a valid header");
662+
assert!(header.is_sensitive());
663+
}
664+
665+
/// The value still has to be the credential; marking it sensitive must not
666+
/// change what goes on the wire.
667+
#[test]
668+
fn marking_it_sensitive_does_not_change_the_value() {
669+
let header = credential_header("Token m0-secret").expect("valid");
670+
assert_eq!(header.as_bytes(), b"Token m0-secret");
671+
}
672+
673+
/// A credential carrying a newline cannot be a header. Rejecting it here
674+
/// names the credential; letting it through defers the failure into `send`,
675+
/// where it reads as a transport fault.
676+
#[test]
677+
fn a_credential_that_cannot_be_a_header_is_refused_by_name() {
678+
let error = credential_header("key\r\nX-Injected: 1").expect_err("must not be accepted");
679+
assert!(format!("{error}").contains("credential"), "got: {error}");
680+
}
681+
682+
/// And the refusal must not print the credential it refused.
683+
#[test]
684+
fn the_refusal_does_not_echo_the_credential() {
685+
let error =
686+
credential_header("supersecret\nX-Injected: 1").expect_err("must not be accepted");
687+
let rendered = format!("{error:?}");
688+
assert!(!rendered.contains("supersecret"), "leaked: {rendered}");
689+
}
690+
691+
/// Both credential-bearing schemes go through the helper, so both reach
692+
/// the wire redacted. `Auth::Bearer` is covered by `reqwest`'s own
693+
/// `bearer_auth`, which sets the flag itself.
694+
#[test]
695+
fn both_manual_schemes_send_a_sensitive_authorization_value() {
696+
for auth in [
697+
Auth::ApiKey("cg-secret".into()),
698+
Auth::Token("m0-secret".into()),
699+
] {
700+
let client = HttpClient::new("https://example.test", auth).expect("valid endpoint");
701+
let request = client
702+
.request(reqwest::Method::GET, "v1/thing")
703+
.expect("a plain key builds")
704+
.build()
705+
.expect("request builds");
706+
let sensitive = request
707+
.headers()
708+
.values()
709+
.any(reqwest::header::HeaderValue::is_sensitive);
710+
assert!(sensitive, "no sensitive header on {:?}", request.headers());
711+
}
712+
}
713+
}
714+
625715
#[cfg(test)]
626716
mod transport_tests {
627717
use super::classify_transport;

adapters/remote/src/failure_test.rs

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,41 @@ async fn an_unreachable_backend_is_reported_rather_than_hanging() {
180180
}
181181
}
182182

183+
#[tokio::test]
184+
async fn a_cursor_that_never_clears_is_refused_rather_than_walked_for_ever() {
185+
// Mem0's hosted arm pages until the server says stop: an empty page or a
186+
// null `next`. Both are things the *server* controls, so a server that
187+
// keeps answering a page and a cursor -- a bug, a proxy replaying one
188+
// response, a filter that never narrows -- would spin the request loop and
189+
// grow the buffer until the process died. The self-hosted arm already
190+
// refuses past its ceiling; this pins the hosted one doing the same.
191+
let app = Router::new().fallback(any(|| async {
192+
axum::Json(serde_json::json!({
193+
"count": 1,
194+
"next": "https://api.mem0.ai/v3/memories/?page=2",
195+
"previous": null,
196+
"results": [{"id": "m-1", "memory": "x", "metadata": {}}]
197+
}))
198+
}));
199+
let endpoint = serve(app).await;
200+
let memory = Mem0Memory::api(&endpoint, "m0-test-key").expect("client");
201+
202+
// Bounded so a genuinely unbounded loop fails the test rather than hanging
203+
// the suite: the ceiling is 500 requests against a local socket, which
204+
// finishes far inside this.
205+
let outcome =
206+
tokio::time::timeout(std::time::Duration::from_secs(60), memory.get("ns", "k")).await;
207+
208+
let Ok(result) = outcome else {
209+
panic!("the hosted listing never terminated against a cursor that never clears");
210+
};
211+
let error = result.expect_err("a cursor that never clears cannot be answered correctly");
212+
assert!(
213+
format!("{error:#}").contains("pages"),
214+
"the refusal must name the page ceiling it hit, got: {error:#}"
215+
);
216+
}
217+
183218
#[tokio::test]
184219
async fn a_paginated_export_terminates_instead_of_looping() {
185220
// The partial-page leg of §E6. A backend that keeps answering with a page

adapters/remote/src/mem0.rs

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,19 @@ enum Flavour {
4747
/// from anything else in the same Mem0 project.
4848
const CLOUD_AGENT_ID: &str = "tinymemory";
4949

50+
/// Records requested per hosted-platform listing page.
51+
const CLOUD_PAGE_SIZE: u32 = 200;
52+
53+
/// The most pages one hosted-platform listing will walk.
54+
///
55+
/// The walk already stops on an empty page and on a null `next`, which covers
56+
/// a well-behaved server. It does not cover a server that keeps answering a
57+
/// full page and a non-null cursor: that spins the loop and grows the buffer
58+
/// until the process dies. 500 pages is 100_000 records -- far past any real
59+
/// account this adapter writes, and small enough that the failure arrives as a
60+
/// message rather than an OOM.
61+
const CLOUD_MAX_PAGES: u32 = 500;
62+
5063
/// A Mem0 service — self-hosted or hosted — exposed through TinyMemory's
5164
/// storage contract.
5265
#[derive(Debug)]
@@ -253,9 +266,12 @@ impl Mem0Dialect {
253266
// `{count, next, previous, results}`. That is the paging this
254267
// adapter's self-hosted arm documents as unverified — here it is
255268
// verified against Mem0's API reference, so this arm has no
256-
// ceiling to refuse at. Paging stops on an empty page as well as
257-
// a null `next`, so a server that omits the cursor cannot spin
258-
// the loop.
269+
// ceiling to refuse at for a *correct* server. Paging stops on an
270+
// empty page as well as a null `next`, so a server that omits the
271+
// cursor cannot spin the loop -- but one that keeps answering a
272+
// full page and a non-null `next` still can, so the walk is
273+
// bounded below and fails loudly at the bound rather than
274+
// collecting for ever.
259275
Flavour::Cloud => {
260276
let mut all = Vec::new();
261277
let mut page = 1_u32;
@@ -264,7 +280,7 @@ impl Mem0Dialect {
264280
.client
265281
.json(
266282
Method::POST,
267-
&format!("v3/memories/?page={page}&page_size=200"),
283+
&format!("v3/memories/?page={page}&page_size={CLOUD_PAGE_SIZE}"),
268284
Some(&json!({"filters": {"agent_id": CLOUD_AGENT_ID}})),
269285
)
270286
.await?;
@@ -279,6 +295,13 @@ impl Mem0Dialect {
279295
if exhausted {
280296
break;
281297
}
298+
anyhow::ensure!(
299+
page < CLOUD_MAX_PAGES,
300+
"mem0's hosted platform still reported more memories after \
301+
{CLOUD_MAX_PAGES} pages of {CLOUD_PAGE_SIZE}. A cursor that never \
302+
clears is a server fault, not a large account, and continuing \
303+
would neither terminate nor answer correctly."
304+
);
282305
page = page.saturating_add(1);
283306
}
284307
Ok(all)

0 commit comments

Comments
 (0)