mirror of
https://github.com/supermemoryai/supermemory.git
synced 2026-10-01 02:01:40 +00:00
fix(tools): bound and harden the shared /v4/profile request
`supermemoryProfileSearch` in `shared/memory-client.ts` is the only Supermemory HTTP call in this package with neither a request timeout nor redirect handling. The identical `/v4/profile` call in `openai/middleware.ts` sets both, and `/v4/conversations` (`conversations-client.ts`) and `/v4/memories` (`shared/forget-memory.ts`) each set a 30s budget. Two consequences: - **Unbounded request.** A timeout only applied when the caller supplied a signal. `withSupermemory` passes one (5s), but `buildMemoriesText` is called with no signal by the Mastra processor and the VoltAgent middleware, and by the exported `buildMemoriesText` / `addSystemPrompt` helpers. `fetch` has no default deadline, so a stalled connection blocks the agent turn indefinitely — the failure both integrations' surrounding try/catch is written to absorb, but which never surfaces as an error. - **Redirects followed.** The request carries `Authorization: Bearer <apiKey>`; a 3xx from a misconfigured or attacker-influenced `baseUrl` was followed silently rather than refused. Apply a 30s `PROFILE_REQUEST_TIMEOUT_MS` unconditionally and set `redirect: "error"`. A caller signal is composed with the timeout via `AbortSignal.any` rather than replacing it, so a caller-side budget can only shorten the request, never leave it unbounded — the wrapper is kept separate so the composition is stated once rather than re-derived at the call site. `src/shared/memory-client.test.ts` existed but was absent from the `test:unit` file list CI runs, so its assertions never ran on a pull request; add it alongside the new coverage. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L7GkmUn6skD6dCzKtcHDbe
This commit is contained in:
parent
2415a5c796
commit
109674b9f8
3 changed files with 100 additions and 4 deletions
|
|
@ -8,7 +8,7 @@
|
|||
"dev": "tsdown --watch --ignore-watch .turbo",
|
||||
"check-types": "tsc --noEmit",
|
||||
"test": "vitest --testTimeout 100000",
|
||||
"test:unit": "vitest run --testTimeout 100000 src/tools-shared.test.ts src/tool-operations.test.ts src/claude-memory.test.ts test/with-supermemory/unit.test.ts test/with-supermemory/conversation-conversion.test.ts test/openai-middleware.unit.test.ts test/mastra/unit.test.ts test/voltagent.unit.test.ts",
|
||||
"test:unit": "vitest run --testTimeout 100000 src/tools-shared.test.ts src/tool-operations.test.ts src/claude-memory.test.ts src/shared/memory-client.test.ts test/with-supermemory/unit.test.ts test/with-supermemory/conversation-conversion.test.ts test/openai-middleware.unit.test.ts test/mastra/unit.test.ts test/voltagent.unit.test.ts",
|
||||
"test:watch": "vitest --watch --testTimeout 100000"
|
||||
},
|
||||
"dependencies": {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { afterEach, describe, expect, it, vi } from "vitest"
|
||||
import { buildMemoriesText } from "./memory-client"
|
||||
import { buildMemoriesText, supermemoryProfileSearch } from "./memory-client"
|
||||
import { createLogger } from "./logger"
|
||||
|
||||
const API_KEY = "sm_test_key"
|
||||
|
|
@ -76,3 +76,72 @@ describe("buildMemoriesText", () => {
|
|||
expect(memories.match(/User is allergic to peanuts/g)).toHaveLength(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe("supermemoryProfileSearch request hardening", () => {
|
||||
/** Captures the `fetch` init so the request options can be asserted. */
|
||||
function captureRequestInit() {
|
||||
const fetchMock = vi.fn().mockResolvedValue({
|
||||
ok: true,
|
||||
json: async () => ({ profile: { static: [], dynamic: [] } }),
|
||||
})
|
||||
vi.stubGlobal("fetch", fetchMock)
|
||||
return () => fetchMock.mock.calls[0]?.[1] as RequestInit | undefined
|
||||
}
|
||||
|
||||
// The request carries the API key in an Authorization header. Following a
|
||||
// redirect would replay it against a host the caller never configured.
|
||||
it("refuses to follow redirects", async () => {
|
||||
const getInit = captureRequestInit()
|
||||
|
||||
await supermemoryProfileSearch(CONTAINER_TAG, "", BASE_URL, API_KEY)
|
||||
|
||||
expect(getInit()?.redirect).toBe("error")
|
||||
})
|
||||
|
||||
// Mastra, VoltAgent and the exported helpers call this with no signal, so
|
||||
// without an unconditional timeout a hung socket blocks the turn forever.
|
||||
it("bounds the request even when the caller passes no signal", async () => {
|
||||
const getInit = captureRequestInit()
|
||||
|
||||
await supermemoryProfileSearch(CONTAINER_TAG, "", BASE_URL, API_KEY)
|
||||
|
||||
const signal = getInit()?.signal
|
||||
expect(signal).toBeInstanceOf(AbortSignal)
|
||||
expect(signal?.aborted).toBe(false)
|
||||
})
|
||||
|
||||
// The caller signal is composed with the timeout rather than replacing it,
|
||||
// so a caller-side budget still shortens the request.
|
||||
it("still aborts when the caller's signal fires", async () => {
|
||||
const getInit = captureRequestInit()
|
||||
const controller = new AbortController()
|
||||
|
||||
await supermemoryProfileSearch(
|
||||
CONTAINER_TAG,
|
||||
"",
|
||||
BASE_URL,
|
||||
API_KEY,
|
||||
controller.signal,
|
||||
)
|
||||
|
||||
const signal = getInit()?.signal
|
||||
expect(signal?.aborted).toBe(false)
|
||||
controller.abort(new Error("caller budget exhausted"))
|
||||
expect(signal?.aborted).toBe(true)
|
||||
expect((signal?.reason as Error).message).toBe("caller budget exhausted")
|
||||
})
|
||||
|
||||
it("passes an already-aborted caller signal straight through", async () => {
|
||||
const getInit = captureRequestInit()
|
||||
|
||||
await supermemoryProfileSearch(
|
||||
CONTAINER_TAG,
|
||||
"",
|
||||
BASE_URL,
|
||||
API_KEY,
|
||||
AbortSignal.abort(new Error("already cancelled")),
|
||||
)
|
||||
|
||||
expect(getInit()?.signal?.aborted).toBe(true)
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -15,6 +15,28 @@ import {
|
|||
defaultPromptTemplate,
|
||||
} from "./prompt-builder"
|
||||
|
||||
/**
|
||||
* Upper bound for a single `/v4/profile` request, matching the budget the other
|
||||
* Supermemory calls in this package already use (`/v4/conversations` and
|
||||
* `/v4/memories`). Without it a stalled connection has no deadline at all: the
|
||||
* Mastra and VoltAgent integrations, and the exported `buildMemoriesText` /
|
||||
* `addSystemPrompt` helpers, call this function with no signal of their own, so
|
||||
* a hung socket would block the agent turn forever.
|
||||
*/
|
||||
export const PROFILE_REQUEST_TIMEOUT_MS = 30_000
|
||||
|
||||
/**
|
||||
* Bound a request by the package timeout, keeping any caller signal live.
|
||||
*
|
||||
* The caller signal is composed with the timeout rather than replacing it, so
|
||||
* a caller-supplied deadline can only ever shorten the request, never remove
|
||||
* its upper bound.
|
||||
*/
|
||||
const withRequestTimeout = (signal?: AbortSignal): AbortSignal => {
|
||||
const timeout = AbortSignal.timeout(PROFILE_REQUEST_TIMEOUT_MS)
|
||||
return signal ? AbortSignal.any([signal, timeout]) : timeout
|
||||
}
|
||||
|
||||
/**
|
||||
* Fetches profile and search results from the Supermemory API.
|
||||
*
|
||||
|
|
@ -22,7 +44,9 @@ import {
|
|||
* @param queryText - Optional query text for semantic search
|
||||
* @param baseUrl - The API base URL
|
||||
* @param apiKey - The API key for authentication
|
||||
* @param signal - Optional AbortSignal to cancel the request (e.g. retrieval timeout)
|
||||
* @param signal - Optional AbortSignal to cancel the request early (e.g. a
|
||||
* caller-side retrieval budget). It is composed with the package timeout, so
|
||||
* it can shorten the request but never leaves it unbounded.
|
||||
* @returns The profile structure with static, dynamic, and search results
|
||||
*/
|
||||
export const supermemoryProfileSearch = async (
|
||||
|
|
@ -51,7 +75,10 @@ export const supermemoryProfileSearch = async (
|
|||
Authorization: `Bearer ${apiKey}`,
|
||||
},
|
||||
body: payload,
|
||||
...(signal ? { signal } : {}),
|
||||
// The request carries the API key in an Authorization header, so a
|
||||
// redirect is refused rather than followed to an unverified host.
|
||||
redirect: "error",
|
||||
signal: withRequestTimeout(signal),
|
||||
})
|
||||
|
||||
if (!response.ok) {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue