mason: fail closed workspace memory sharing

Co-authored-by: Alfonso [Magic Context] <288211368+alfonso-magic-context@users.noreply.github.com>

Author: ualtinok

packages/pi-plugin/src/inject-compartments-pi.test.ts

@@ -109,6 +109,41 @@ describe("workspace memory sharing", () => {
 			closeQuietly(db);
 		}
 	});
+
+	it("does not render foreign memories when share_categories is malformed", () => {
+		const db = createTestDb();
+		const dir = mkdtempSync(join(tmpdir(), "mc-pi-share-malformed-"));
+		try {
+			db.exec(`
+				INSERT INTO workspaces (id, name, share_categories, created_at, updated_at)
+				VALUES (1, 'ws', 'not-json', 1, 1);
+				INSERT INTO workspace_members (workspace_id, project_path, display_name, display_path, added_at)
+				VALUES (1, 'git:own', 'Own', '/own', 1), (1, 'git:foreign', 'Foreign', '/foreign', 1);
+			`);
+			insertMemory(db, {
+				projectPath: "git:own",
+				category: "CONSTRAINTS",
+				content: "own malformed Pi memory remains visible",
+			});
+			insertMemory(db, {
+				projectPath: "git:foreign",
+				category: "CONSTRAINTS",
+				content: "foreign malformed Pi memory is hidden",
+			});
+			const state = {
+				sessionId: "pi-share-malformed",
+				projectIdentity: "git:own",
+				projectDirectory: dir,
+			};
+
+			const m0 = renderM0Pi(state, db, "");
+			expect(m0).toContain("own malformed Pi memory remains visible");
+			expect(m0).not.toContain("foreign malformed Pi memory is hidden");
+		} finally {
+			rmSync(dir, { recursive: true, force: true });
+			closeQuietly(db);
+		}
+	});
 });
 
 describe("trimPiMessagesToBoundary", () => {

packages/pi-plugin/src/tools/ctx-memory.test.ts

@@ -130,7 +130,7 @@ describe("createCtxMemoryTool", () => {
 		}
 	});
 
-	it("updates a foreign workspace memory under the target identity", async () => {
+	it("rejects updating a foreign workspace memory even when the category is shared", async () => {
 		const db = createTestDb();
 		try {
 			const primary = createCtxMemoryTool({
@@ -141,54 +141,46 @@ describe("createCtxMemoryTool", () => {
 			});
 			const ctx = fakeContext("ses-memory") as never;
 			const ownIdentity = resolveProjectIdentity((ctx as { cwd: string }).cwd);
-			// Default workspace shares CONSTRAINTS; this test exercises
-			// target-identity routing, so use a shared category (foreign memory
-			// visible) and verify the mutation routes under the target identity.
 			db.exec(`
 				INSERT INTO workspaces (id, name, created_at, updated_at) VALUES (1, 'ws', 1, 1);
 				INSERT INTO workspace_members (workspace_id, project_path, display_name, display_path, added_at)
 				VALUES (1, '${ownIdentity}', 'Own', '${ownIdentity}', 1),
 				       (1, 'git:foreign', 'Foreign', '/foreign', 1);
 			`);
-			insertMemory(db, {
-				projectPath: ownIdentity,
-				category: "CONSTRAINTS",
-				content: "Use the shared formatter.",
-			});
 			const foreign = insertMemory(db, {
 				projectPath: "git:foreign",
 				category: "CONSTRAINTS",
-				content: "Old foreign directive.",
+				content: "Old foreign shared constraint.",
 			});
 
 			const result = await primary.execute(
 				"call-u",
 				{
 					action: "update",
 					ids: [foreign.id],
-					content: "Use the shared formatter.",
+					content: "Updated foreign shared constraint.",
 				},
 				new AbortController().signal,
 				undefined,
 				ctx,
 			);
 
-			expect(result.isError).toBeUndefined();
+			expect(result.isError).toBe(true);
+			expect(result.content[0]?.text).toBe(
+				`Error: Memory with ID ${foreign.id} was not found.`,
+			);
 			expect(getMemoryById(db, foreign.id)?.content).toBe(
-				"Use the shared formatter.",
+				"Old foreign shared constraint.",
 			);
-			expect(
-				getMemoryMutationsForRender(db, ownIdentity, 0, [foreign.id]),
-			).toHaveLength(0);
 			expect(
 				getMemoryMutationsForRender(db, "git:foreign", 0, [foreign.id]),
-			).toHaveLength(1);
+			).toHaveLength(0);
 		} finally {
 			closeQuietly(db);
 		}
 	});
 
-	it("archives a foreign workspace memory under the target identity", async () => {
+	it("rejects archiving a foreign workspace memory even when the category is shared", async () => {
 		const db = createTestDb();
 		try {
 			const primary = createCtxMemoryTool({
@@ -199,8 +191,6 @@ describe("createCtxMemoryTool", () => {
 			});
 			const ctx = fakeContext("ses-memory") as never;
 			const ownIdentity = resolveProjectIdentity((ctx as { cwd: string }).cwd);
-			// Default workspace shares CONSTRAINTS; this test exercises
-			// target-identity routing, so use a shared category (foreign visible).
 			db.exec(`
 				INSERT INTO workspaces (id, name, created_at, updated_at) VALUES (1, 'ws', 1, 1);
 				INSERT INTO workspace_members (workspace_id, project_path, display_name, display_path, added_at)
@@ -210,7 +200,7 @@ describe("createCtxMemoryTool", () => {
 			const foreign = insertMemory(db, {
 				projectPath: "git:foreign",
 				category: "CONSTRAINTS",
-				content: "Foreign issue.",
+				content: "Foreign shared constraint.",
 			});
 
 			const result = await primary.execute(
@@ -221,11 +211,14 @@ describe("createCtxMemoryTool", () => {
 				ctx,
 			);
 
-			expect(result.isError).toBeUndefined();
-			expect(getMemoryById(db, foreign.id)?.status).toBe("archived");
+			expect(result.isError).toBe(true);
+			expect(result.content[0]?.text).toBe(
+				`Error: Memory with ID ${foreign.id} was not found.`,
+			);
+			expect(getMemoryById(db, foreign.id)?.status).toBe("active");
 			expect(
 				getMemoryMutationsForRender(db, "git:foreign", 0, [foreign.id]),
-			).toHaveLength(1);
+			).toHaveLength(0);
 		} finally {
 			closeQuietly(db);
 		}
@@ -271,7 +264,7 @@ describe("createCtxMemoryTool", () => {
 		}
 	});
 
-	it("archives a foreign memory in a SHARED category (P0 parity)", async () => {
+	it("rejects archiving a foreign memory in a SHARED category", async () => {
 		const db = createTestDb();
 		try {
 			const primary = createCtxMemoryTool({
@@ -302,8 +295,11 @@ describe("createCtxMemoryTool", () => {
 				ctx,
 			);
 
-			expect(result.isError).toBeUndefined();
-			expect(getMemoryById(db, foreignShared.id)?.status).toBe("archived");
+			expect(result.isError).toBe(true);
+			expect(result.content[0]?.text).toBe(
+				`Error: Memory with ID ${foreignShared.id} was not found.`,
+			);
+			expect(getMemoryById(db, foreignShared.id)?.status).toBe("active");
 		} finally {
 			closeQuietly(db);
 		}
@@ -424,7 +420,7 @@ describe("createCtxMemoryTool", () => {
 		}
 	});
 
-	it("allows a PRIMARY merge of a foreign SHARED-category memory (P0 parity)", async () => {
+	it("rejects a PRIMARY merge of a foreign SHARED-category memory", async () => {
 		const db = createTestDb();
 		try {
 			const primary = createCtxMemoryTool({
@@ -465,12 +461,12 @@ describe("createCtxMemoryTool", () => {
 				ctx,
 			);
 
-			// New merged content matches neither source, so a FRESH canonical is
-			// inserted and both sources are superseded → archived (parity with
-			// OpenCode's shared-merge test).
-			expect(result.isError).toBeUndefined();
-			expect(getMemoryById(db, own.id)?.status).toBe("archived");
-			expect(getMemoryById(db, foreignShared.id)?.status).toBe("archived");
+			expect(result.isError).toBe(true);
+			expect(result.content[0]?.text).toBe(
+				`Error: Memory with ID ${foreignShared.id} was not found.`,
+			);
+			expect(getMemoryById(db, own.id)?.status).toBe("active");
+			expect(getMemoryById(db, foreignShared.id)?.status).toBe("active");
 		} finally {
 			closeQuietly(db);
 		}
@@ -575,6 +571,58 @@ describe("createCtxMemoryTool", () => {
 		}
 	});
 
+	it("REFUSES a DREAMER merge when workspace share_categories is malformed", async () => {
+		const db = createTestDb();
+		try {
+			const dreamer = createCtxMemoryTool({
+				db,
+				memoryEnabled: true,
+				embeddingEnabled: false,
+				allowDreamerActions: true,
+			});
+			const ctx = fakeContext("ses-dreamer") as never;
+			const ownIdentity = resolveProjectIdentity((ctx as { cwd: string }).cwd);
+			db.exec(`
+				INSERT INTO workspaces (id, name, created_at, updated_at, share_categories) VALUES (1, 'ws', 1, 1, 'not-json');
+				INSERT INTO workspace_members (workspace_id, project_path, display_name, display_path, added_at)
+				VALUES (1, '${ownIdentity}', 'Own', '${ownIdentity}', 1),
+				       (1, 'git:foreign', 'Foreign', '/foreign', 1);
+			`);
+			const own = insertMemory(db, {
+				projectPath: ownIdentity,
+				category: "CONSTRAINTS",
+				content: "Own constraint malformed policy.",
+			});
+			const foreign = insertMemory(db, {
+				projectPath: "git:foreign",
+				category: "CONSTRAINTS",
+				content: "Foreign constraint hidden by malformed policy.",
+			});
+
+			const result = await dreamer.execute(
+				"call-d1-malformed",
+				{
+					action: "merge",
+					ids: [own.id, foreign.id],
+					content: "Merged malformed policy constraint.",
+					category: "CONSTRAINTS",
+				},
+				new AbortController().signal,
+				undefined,
+				ctx,
+			);
+
+			expect(result.isError).toBe(true);
+			expect(result.content[0]?.text).toContain(
+				"not shared with this workspace member",
+			);
+			expect(getMemoryById(db, own.id)?.status).toBe("active");
+			expect(getMemoryById(db, foreign.id)?.status).toBe("active");
+		} finally {
+			closeQuietly(db);
+		}
+	});
+
 	it("ALLOWS a DREAMER merge of a foreign SHARED-category memory INSIDE a workspace (D1 parity)", async () => {
 		const db = createTestDb();
 		try {

packages/pi-plugin/src/tools/ctx-memory.ts

@@ -349,18 +349,18 @@ export function createCtxMemoryTool(
 							expandedWorkspace.canonicalIdentityByStoredPath,
 						) ?? normalizeStoredProjectPath(rawProjectPath))
 					: normalizeStoredProjectPath(rawProjectPath);
-			// The workspace's share-category policy, identical to the render path
-			// (resolveWorkspaceRenderContextPi): null = share all categories.
+			// The workspace's share-category policy matches the render path.
+			// null means there is no workspace filter; a workspaced caller gets
+			// an explicit list where [] shares no foreign categories.
 			const toolShareCategories =
 				workspaceIdentitySet.identities.length > 1
 					? resolveWorkspaceShareCategories(deps.db, projectIdentity)
 					: null;
-			// Tool visibility MUST match render visibility, or the agent could
-			// mutate (update/archive) a foreign workspace memory it can't even
-			// see. Own-project memories: every category is mutable. Foreign member
-			// memories: only when shared — shareCategories===null shares all, an
-			// empty list shares none, otherwise only the listed categories.
-			// Mirrors buildWorkspaceMemorySqlFilter's own/foreign split.
+			// Visibility is the READ contract: own memories are visible in every
+			// category, while foreign workspace memories are visible only in
+			// categories the workspace explicitly shares. Mutations by primary
+			// agents use memoryOwnedByTool below so shared visibility never
+			// grants write access to another project.
 			const memoryVisibleToTool = (memory: Memory): boolean => {
 				if (workspaceIdentitySet.identities.length <= 1) {
 					return storedPathBelongsToIdentity(
@@ -381,11 +381,12 @@ export function createCtxMemoryTool(
 				const isOwn =
 					targetIdentityForStoredPath(memory.projectPath) === projectIdentity;
 				if (isOwn) return true;
-				return (
-					toolShareCategories === null ||
-					toolShareCategories.includes(memory.category)
-				);
+				return toolShareCategories?.includes(memory.category) ?? false;
 			};
+			const memoryOwnedByTool = (memory: Memory): boolean =>
+				workspaceIdentitySet.identities.length > 1
+					? targetIdentityForStoredPath(memory.projectPath) === projectIdentity
+					: storedPathBelongsToIdentity(memory.projectPath, projectIdentity);
 			const snapshot = getProjectEmbeddingSnapshot(projectIdentity);
 			if (
 				snapshot
@@ -463,7 +464,12 @@ export function createCtxMemoryTool(
 				}
 
 				const memory = getMemoryById(deps.db, updateId);
-				if (!memory || !memoryVisibleToTool(memory)) {
+				const updateAllowed = memory
+					? dreamerAllowed
+						? memoryVisibleToTool(memory)
+						: memoryOwnedByTool(memory)
+					: false;
+				if (!memory || !updateAllowed) {
 					return err(`Error: Memory with ID ${updateId} was not found.`);
 				}
 				if (!dreamerAllowed && !isPrimaryMutableMemory(memory)) {
@@ -542,7 +548,7 @@ export function createCtxMemoryTool(
 				// update/archive ownership (parity with OpenCode).
 				if (!dreamerAllowed) {
 					const foreign = sourceMemories.find(
-						(memory) => !memoryVisibleToTool(memory),
+						(memory) => !memoryOwnedByTool(memory),
 					);
 					if (foreign) {
 						return err(`Error: Memory with ID ${foreign.id} was not found.`);
@@ -749,7 +755,12 @@ export function createCtxMemoryTool(
 				// half-archive a batch (all-or-nothing, matching the transaction).
 				for (const memoryId of archiveIds) {
 					const memory = getMemoryById(deps.db, memoryId);
-					if (!memory || !memoryVisibleToTool(memory)) {
+					const archiveAllowed = memory
+						? dreamerAllowed
+							? memoryVisibleToTool(memory)
+							: memoryOwnedByTool(memory)
+						: false;
+					if (!memory || !archiveAllowed) {
 						return err(`Error: Memory with ID ${memoryId} was not found.`);
 					}
 					if (!dreamerAllowed && !isPrimaryMutableMemory(memory)) {

packages/plugin/src/features/magic-context/memory/storage-memory.ts

@@ -599,6 +599,7 @@ function uniqueValues(values: readonly string[]): string[] {
 
 export interface WorkspaceMemorySharingFilter {
     ownIdentities?: readonly string[];
+    /** null/undefined means no workspace filter is active; pass [] to share no foreign categories. */
     shareCategories?: readonly string[] | null;
 }