From 3881e1bff6bfb399e91f836f662b9f1fab93bd23 Mon Sep 17 00:00:00 2001 From: akushonkamen Date: Mon, 12 Oct 2026 15:30:07 +0800 Subject: [PATCH] fix(memory): implement the memory/episode/raw-turn scope assertions assertMemoryInScope, assertEpisodeInScope and assertRawTurnInScope discarded both parameters (void memory; void namespace), so the get/delete/archive/retrieval call sites that pass a namespace had no boundary at all (#593). The three assertions now reject a request whose namespace explicitly carries a userId or projectId that contradicts the row's owner (MemoryServiceError forbidden), mirroring the existing user-memory and L3 World Model checks in deleteMemory. A request without a namespace keeps the historical whole-local-store semantics, so single-namespace local installs see no change, and sparse namespaces (no userId/projectId) are not newly rejected. Single-memory and skill-detail GETs stay reachable across namespaces on purpose: the REST contract requires cross-key shared resource reads and the evolution pipeline merges policies across user ids, while the skills list endpoint is already namespace-unfiltered. The misleading assertMemoryInScope call is therefore removed from those two shared read paths; enforcement lives on the mutation paths (archive, delete, redact, panel task delete) and the session/turn/feedback/trial/import pipelines. Negative tests cover cross-namespace archive/delete, cross-project delete, no-namespace whole-store behavior, and cross-namespace episode delete plus raw-turn redaction. --- Memory/src/service/memory-service.ts | 32 +++-- Memory/src/service/read-model/episode.ts | 6 +- Memory/src/service/read-model/skill.ts | 4 +- .../service/memory-scope-assertions.test.ts | 118 ++++++++++++++++++ 4 files changed, 149 insertions(+), 11 deletions(-) create mode 100644 Memory/tests/service/memory-scope-assertions.test.ts diff --git a/Memory/src/service/memory-service.ts b/Memory/src/service/memory-service.ts index 0374a0f5d..86680b448 100644 --- a/Memory/src/service/memory-service.ts +++ b/Memory/src/service/memory-service.ts @@ -141,7 +141,8 @@ import { namespaceForMemory, namespaceForRawTurn, namespaceForSession, - normalizeNamespace + normalizeNamespace, + projectIdFromMemory } from "./namespace/namespace-scope.js"; import { EpisodeReadModel, @@ -542,7 +543,6 @@ export class MemoryService { requireEpisode: this.requireEpisode.bind(this), assertSessionInScope: this.assertSessionInScope.bind(this), assertEpisodeInScope: this.assertEpisodeInScope.bind(this), - assertMemoryInScope: this.assertMemoryInScope.bind(this), namespaceForSession, readableMemoryIdKind, invalidArgument: (message) => new MemoryServiceError("invalid_argument", message), @@ -2844,19 +2844,35 @@ export class MemoryService { } } + // Scope boundary for generic (non-L3) rows. A request that carries no namespace + // keeps the historical whole-local-store semantics; a namespace that explicitly + // carries an identity dimension (userId / projectId) which contradicts the row's + // owner is rejected. Mirrors the user-memory and L3 checks in deleteMemory. private assertMemoryInScope(memory: MemoryRow, namespace?: RuntimeNamespace): void { - void memory; - void namespace; + if (!namespace) return; + if (namespace.userId && namespace.userId !== memory.userId) { + throw new MemoryServiceError("forbidden", `memory ${memory.id} belongs to a different namespace user`); + } + if (namespace.projectId && namespace.projectId !== (projectIdFromMemory(memory) ?? null)) { + throw new MemoryServiceError("forbidden", `memory ${memory.id} belongs to a different namespace project`); + } } private assertEpisodeInScope(episode: EpisodeRecord, namespace?: RuntimeNamespace): void { - void episode; - void namespace; + if (!namespace) return; + if (namespace.userId && namespace.userId !== episode.userId) { + throw new MemoryServiceError("forbidden", `episode ${episode.id} belongs to a different namespace user`); + } + if (namespace.projectId && namespace.projectId !== (episode.projectId ?? null)) { + throw new MemoryServiceError("forbidden", `episode ${episode.id} belongs to a different namespace project`); + } } private assertRawTurnInScope(rawTurn: RawTurnRecord, namespace?: RuntimeNamespace): void { - void rawTurn; - void namespace; + if (!namespace) return; + if (namespace.userId && namespace.userId !== rawTurn.userId) { + throw new MemoryServiceError("forbidden", `raw turn ${rawTurn.id} belongs to a different namespace user`); + } } diff --git a/Memory/src/service/read-model/episode.ts b/Memory/src/service/read-model/episode.ts index c7ae681da..4c132526a 100644 --- a/Memory/src/service/read-model/episode.ts +++ b/Memory/src/service/read-model/episode.ts @@ -127,7 +127,6 @@ export interface EpisodeReadModelDependencies { requireEpisode(id: string): EpisodeRecord; assertSessionInScope(session: SessionRecord, namespace?: RuntimeNamespace): void; assertEpisodeInScope(episode: EpisodeRecord, namespace?: RuntimeNamespace): void; - assertMemoryInScope(memory: MemoryRow, namespace?: RuntimeNamespace): void; namespaceForSession(session: SessionRecord): RuntimeNamespace; readableMemoryIdKind(id: string): "trace" | "policy" | "world" | "skill" | "episode" | "raw" | "unknown"; invalidArgument(message: string): Error; @@ -228,7 +227,10 @@ export class EpisodeReadModel { const memory = this.deps.repos.memories.get(id); if (!memory) throw this.deps.notFound(`memory not found: ${id}`); - this.deps.assertMemoryInScope(memory, request.namespace); + // Shared-resource read: single-memory GETs intentionally stay reachable across + // namespaces of the same local installation (see the "shared resource access" + // REST contract and cross-user evolution merges). Namespace boundaries are + // enforced on the mutation paths instead (archive/delete/redact). const detail = this.deps.detailFromMemory(memory, this.deps.repos.processing.get(memory.id)); const refs = this.refsForMemory(memory); const item = this.deps.memoryDetailWithLayerPayload(detail, memory); diff --git a/Memory/src/service/read-model/skill.ts b/Memory/src/service/read-model/skill.ts index f73d7ce3e..b02b7e056 100644 --- a/Memory/src/service/read-model/skill.ts +++ b/Memory/src/service/read-model/skill.ts @@ -245,7 +245,9 @@ export class SkillReadModel { if (!memory || memory.memoryLayer !== "Skill") { throw this.deps.createError("not_found", `skill not found: ${skillId}`); } - this.deps.assertMemoryInScope(memory, request.namespace); + // Shared-resource read: skill details sit beside the namespace-unfiltered + // skills list (GET /api/v1/skills), so cross-namespace reads stay allowed. + // Boundaries are enforced on mutations (invoke/archive/delete). const detail = this.deps.detailFromMemory(memory); const skill = this.deps.skillMetaFromMemory(memory); const usageStats = this.skillUsageStats(skillId); diff --git a/Memory/tests/service/memory-scope-assertions.test.ts b/Memory/tests/service/memory-scope-assertions.test.ts new file mode 100644 index 000000000..20faacc13 --- /dev/null +++ b/Memory/tests/service/memory-scope-assertions.test.ts @@ -0,0 +1,118 @@ +import { afterEach, describe, expect, it } from "vitest"; +import { createMemoryServiceFixture, createBatchReflectionLlm } from "../fixtures/memory-service-fixture.js"; +import { MemoryServiceError } from "../../src/utils/error.js"; +import type { RuntimeNamespace } from "../../src/types.js"; + +const fixture = createMemoryServiceFixture(); +const { createTestService } = fixture; + +const nsAlice: RuntimeNamespace = { + source: "codex", + profileId: "profile-alice", + userId: "scope-user-alice" +}; + +const nsBob: RuntimeNamespace = { + source: "codex", + profileId: "profile-bob", + userId: "scope-user-bob" +}; + +function caughtError(fn: () => unknown): unknown { + try { + fn(); + } catch (error) { + return error; + } + return undefined; +} + +function expectOutOfScope(fn: () => unknown): void { + const error = caughtError(fn); + expect(error).toBeInstanceOf(MemoryServiceError); + expect((error as MemoryServiceError).code).toBe("forbidden"); +} + +afterEach(() => { + fixture.cleanup(); +}); + +describe("memory/episode/raw-turn scope assertions", () => { + it("rejects cross-namespace archive and delete of generic memories", () => { + const { service } = createTestService({ llm: createBatchReflectionLlm([]) }); + + const archived = service.addMemory({ + namespace: nsAlice, + adapterId: "codex-memory", + requestId: "scope-archive-1", + layer: "L1", + content: "alice memory for archive scope check" + }); + expect(archived.id).toBeTruthy(); + expectOutOfScope(() => service.archiveMemory(archived.id, { namespace: nsBob })); + expect(service.archiveMemory(archived.id, { namespace: nsAlice }).status).toBe("archived"); + + const deleted = service.addMemory({ + namespace: nsAlice, + adapterId: "codex-memory", + requestId: "scope-delete-1", + layer: "L1", + content: "alice memory for delete scope check" + }); + expectOutOfScope(() => service.deleteMemory(deleted.id, { namespace: nsBob })); + expect(service.deleteMemory(deleted.id, { namespace: nsAlice }).status).toBe("deleted"); + }); + + it("rejects memory delete when namespace carries a different projectId", () => { + const { service } = createTestService({ llm: createBatchReflectionLlm([]) }); + const nsProjectA: RuntimeNamespace = { ...nsAlice, projectId: "proj-alpha" }; + + const memory = service.addMemory({ + namespace: nsProjectA, + adapterId: "codex-memory", + requestId: "scope-project-1", + layer: "L1", + content: "alice project memory" + }); + + expectOutOfScope(() => service.deleteMemory(memory.id, { namespace: { ...nsProjectA, projectId: "proj-beta" } })); + expect(service.deleteMemory(memory.id, { namespace: nsProjectA }).status).toBe("deleted"); + }); + + it("keeps whole-store semantics when the request carries no namespace", () => { + const { service } = createTestService({ llm: createBatchReflectionLlm([]) }); + + const memory = service.addMemory({ + namespace: nsAlice, + adapterId: "codex-memory", + requestId: "scope-no-namespace-1", + layer: "L1", + content: "alice memory deleted without namespace" + }); + + expect(service.deleteMemory(memory.id, {}).status).toBe("deleted"); + }); + + it("rejects cross-namespace episode delete and raw-turn redaction", () => { + const { service } = createTestService({ llm: createBatchReflectionLlm([]) }); + + const session = service.openSession({ + namespace: nsAlice, + sessionId: "codex-memory-agent:scope-session" + }); + const turn = service.completeTurn("turn-scope-1", { + sessionId: session.sessionId, + namespace: nsAlice, + query: "remember the scope probe", + answer: "the scope probe is stored" + }); + expect(turn.episodeId).toBeTruthy(); + expect(turn.rawTurnId).toBeTruthy(); + + expectOutOfScope(() => service.redactRawTurn(turn.rawTurnId, { namespace: nsBob })); + expect(service.redactRawTurn(turn.rawTurnId, { namespace: nsAlice }).ok).toBe(true); + + expectOutOfScope(() => service.deletePanelTask(turn.episodeId, { namespace: nsBob })); + expect(service.deletePanelTask(turn.episodeId, { namespace: nsAlice }).ok).toBe(true); + }); +});