Merge branch 'main' into fix/listchanged-overwrite

Skip06 · web-flow · commit 080a7c3f4fa0 · 2026-02-09T00:34:28.000+05:30
diff --git a/.changeset/fix-task-session-isolation.md b/.changeset/fix-task-session-isolation.md
@@ -0,0 +1,5 @@
+---
+'@modelcontextprotocol/core': patch
+---
+
+Fix InMemoryTaskStore to enforce session isolation. Previously, sessionId was accepted but ignored on all TaskStore methods, allowing any session to enumerate, read, and mutate tasks created by other sessions. The store now persists sessionId at creation time and enforces ownership on all reads and writes.
diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml
@@ -0,0 +1,31 @@
+# Source: https://github.com/anthropics/claude-code-action/blob/main/docs/code-review.md
+name: Claude Code Review
+
+on:
+  pull_request:
+    types: [opened, synchronize, ready_for_review, reopened]
+
+jobs:
+  claude-review:
+    if: github.event.pull_request.head.repo.fork == false && github.actor != 'dependabot[bot]'
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: read
+      issues: read
+      id-token: write
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v6
+        with:
+          fetch-depth: 1
+
+      - name: Run Claude Code Review
+        id: claude-review
+        uses: anthropics/claude-code-action@v1
+        with:
+          anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
+          plugin_marketplaces: "https://github.com/anthropics/claude-code.git"
+          plugins: "code-review@claude-code-plugins"
+          prompt: "/code-review:code-review ${{ github.repository }}/pull/${{ github.event.pull_request.number }}"
diff --git a/.github/workflows/claude.yml b/.github/workflows/claude.yml
@@ -0,0 +1,41 @@
+# Source: https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md
+name: Claude Code
+
+on:
+  issue_comment:
+    types: [created]
+  pull_request_review_comment:
+    types: [created]
+  issues:
+    types: [opened, assigned]
+  pull_request_review:
+    types: [submitted]
+
+jobs:
+  claude:
+    if: |
+      (github.event_name == 'issue_comment' && contains(github.event.comment.body, '@claude')) ||
+      (github.event_name == 'pull_request_review_comment' && contains(github.event.comment.body, '@claude')) ||
+      (github.event_name == 'pull_request_review' && contains(github.event.review.body, '@claude')) ||
+      (github.event_name == 'issues' && (contains(github.event.issue.body, '@claude') || contains(github.event.issue.title, '@claude')))
+    runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      pull-requests: read
+      issues: read
+      id-token: write
+      actions: read
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v6
+        with:
+          fetch-depth: 1
+
+      - name: Run Claude Code
+        id: claude
+        uses: anthropics/claude-code-action@v1
+        with:
+          anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
+          use_commit_signing: true
+          additional_permissions: |
+            actions: read
diff --git a/packages/core/src/experimental/tasks/stores/inMemory.ts b/packages/core/src/experimental/tasks/stores/inMemory.ts
@@ -11,6 +11,7 @@ interface StoredTask {
     task: Task;
     request: Request;
     requestId: RequestId;
+    sessionId?: string;
     result?: Result;
 }
 
@@ -30,7 +31,7 @@ export class InMemoryTaskStore implements TaskStore {
         return crypto.randomUUID().replaceAll('-', '');
     }
 
-    async createTask(taskParams: CreateTaskOptions, requestId: RequestId, request: Request, _sessionId?: string): Promise<Task> {
+    async createTask(taskParams: CreateTaskOptions, requestId: RequestId, request: Request, sessionId?: string): Promise<Task> {
         // Generate a unique task ID
         const taskId = this.generateTaskId();
 
@@ -55,7 +56,8 @@ export class InMemoryTaskStore implements TaskStore {
         this.tasks.set(taskId, {
             task,
             request,
-            requestId
+            requestId,
+            sessionId
         });
 
         // Schedule cleanup if ttl is specified
@@ -72,13 +74,30 @@ export class InMemoryTaskStore implements TaskStore {
         return task;
     }
 
-    async getTask(taskId: string, _sessionId?: string): Promise<Task | null> {
+    /**
+     * Retrieves a stored task, enforcing session ownership when a sessionId is provided.
+     * Returns undefined if the task does not exist or belongs to a different session.
+     */
+    private getStoredTask(taskId: string, sessionId?: string): StoredTask | undefined {
         const stored = this.tasks.get(taskId);
+        if (!stored) {
+            return undefined;
+        }
+        // Enforce session isolation: if a sessionId is provided and the task
+        // was created with a sessionId, they must match.
+        if (sessionId !== undefined && stored.sessionId !== undefined && stored.sessionId !== sessionId) {
+            return undefined;
+        }
+        return stored;
+    }
+
+    async getTask(taskId: string, sessionId?: string): Promise<Task | null> {
+        const stored = this.getStoredTask(taskId, sessionId);
         return stored ? { ...stored.task } : null;
     }
 
-    async storeTaskResult(taskId: string, status: 'completed' | 'failed', result: Result, _sessionId?: string): Promise<void> {
-        const stored = this.tasks.get(taskId);
+    async storeTaskResult(taskId: string, status: 'completed' | 'failed', result: Result, sessionId?: string): Promise<void> {
+        const stored = this.getStoredTask(taskId, sessionId);
         if (!stored) {
             throw new Error(`Task with ID ${taskId} not found`);
         }
@@ -110,8 +129,8 @@ export class InMemoryTaskStore implements TaskStore {
         }
     }
 
-    async getTaskResult(taskId: string, _sessionId?: string): Promise<Result> {
-        const stored = this.tasks.get(taskId);
+    async getTaskResult(taskId: string, sessionId?: string): Promise<Result> {
+        const stored = this.getStoredTask(taskId, sessionId);
         if (!stored) {
             throw new Error(`Task with ID ${taskId} not found`);
         }
@@ -123,8 +142,8 @@ export class InMemoryTaskStore implements TaskStore {
         return stored.result;
     }
 
-    async updateTaskStatus(taskId: string, status: Task['status'], statusMessage?: string, _sessionId?: string): Promise<void> {
-        const stored = this.tasks.get(taskId);
+    async updateTaskStatus(taskId: string, status: Task['status'], statusMessage?: string, sessionId?: string): Promise<void> {
+        const stored = this.getStoredTask(taskId, sessionId);
         if (!stored) {
             throw new Error(`Task with ID ${taskId} not found`);
         }
@@ -159,13 +178,22 @@ export class InMemoryTaskStore implements TaskStore {
         }
     }
 
-    async listTasks(cursor?: string, _sessionId?: string): Promise<{ tasks: Task[]; nextCursor?: string }> {
+    async listTasks(cursor?: string, sessionId?: string): Promise<{ tasks: Task[]; nextCursor?: string }> {
         const PAGE_SIZE = 10;
-        const allTaskIds = [...this.tasks.keys()];
+
+        // Filter tasks by session ownership before pagination
+        const filteredTaskIds = [...this.tasks.entries()]
+            .filter(([, stored]) => {
+                if (sessionId === undefined || stored.sessionId === undefined) {
+                    return true;
+                }
+                return stored.sessionId === sessionId;
+            })
+            .map(([taskId]) => taskId);
 
         let startIndex = 0;
         if (cursor) {
-            const cursorIndex = allTaskIds.indexOf(cursor);
+            const cursorIndex = filteredTaskIds.indexOf(cursor);
             if (cursorIndex === -1) {
                 // Invalid cursor - throw error
                 throw new Error(`Invalid cursor: ${cursor}`);
@@ -174,13 +202,13 @@ export class InMemoryTaskStore implements TaskStore {
             }
         }
 
-        const pageTaskIds = allTaskIds.slice(startIndex, startIndex + PAGE_SIZE);
+        const pageTaskIds = filteredTaskIds.slice(startIndex, startIndex + PAGE_SIZE);
         const tasks = pageTaskIds.map(taskId => {
             const stored = this.tasks.get(taskId)!;
             return { ...stored.task };
         });
 
-        const nextCursor = startIndex + PAGE_SIZE < allTaskIds.length ? pageTaskIds.at(-1) : undefined;
+        const nextCursor = startIndex + PAGE_SIZE < filteredTaskIds.length ? pageTaskIds.at(-1) : undefined;
 
         return { tasks, nextCursor };
     }
diff --git a/packages/core/test/experimental/inMemory.test.ts b/packages/core/test/experimental/inMemory.test.ts
@@ -647,6 +647,105 @@ describe('InMemoryTaskStore', () => {
         });
     });
 
+    describe('session isolation', () => {
+        const baseRequest: Request = { method: 'tools/call', params: { name: 'demo' } };
+
+        it('should not allow session-b to list tasks created by session-a', async () => {
+            await store.createTask({}, 1, baseRequest, 'session-a');
+            await store.createTask({}, 2, baseRequest, 'session-a');
+
+            const result = await store.listTasks(undefined, 'session-b');
+            expect(result.tasks).toHaveLength(0);
+        });
+
+        it('should not allow session-b to read a task created by session-a', async () => {
+            const task = await store.createTask({}, 1, baseRequest, 'session-a');
+
+            const result = await store.getTask(task.taskId, 'session-b');
+            expect(result).toBeNull();
+        });
+
+        it('should not allow session-b to update a task created by session-a', async () => {
+            const task = await store.createTask({}, 1, baseRequest, 'session-a');
+
+            await expect(store.updateTaskStatus(task.taskId, 'cancelled', undefined, 'session-b')).rejects.toThrow('not found');
+        });
+
+        it('should not allow session-b to store a result on session-a task', async () => {
+            const task = await store.createTask({}, 1, baseRequest, 'session-a');
+
+            await expect(store.storeTaskResult(task.taskId, 'completed', { content: [] }, 'session-b')).rejects.toThrow('not found');
+        });
+
+        it('should not allow session-b to get the result of session-a task', async () => {
+            const task = await store.createTask({}, 1, baseRequest, 'session-a');
+            await store.storeTaskResult(task.taskId, 'completed', { content: [{ type: 'text', text: 'secret' }] }, 'session-a');
+
+            await expect(store.getTaskResult(task.taskId, 'session-b')).rejects.toThrow('not found');
+        });
+
+        it('should allow the owning session to access its own tasks', async () => {
+            const task = await store.createTask({}, 1, baseRequest, 'session-a');
+
+            const retrieved = await store.getTask(task.taskId, 'session-a');
+            expect(retrieved).toBeDefined();
+            expect(retrieved?.taskId).toBe(task.taskId);
+        });
+
+        it('should list only tasks belonging to the requesting session', async () => {
+            await store.createTask({}, 1, baseRequest, 'session-a');
+            await store.createTask({}, 2, baseRequest, 'session-b');
+            await store.createTask({}, 3, baseRequest, 'session-a');
+
+            const resultA = await store.listTasks(undefined, 'session-a');
+            expect(resultA.tasks).toHaveLength(2);
+
+            const resultB = await store.listTasks(undefined, 'session-b');
+            expect(resultB.tasks).toHaveLength(1);
+        });
+
+        it('should allow access when no sessionId is provided (backward compatibility)', async () => {
+            const task = await store.createTask({}, 1, baseRequest, 'session-a');
+
+            // No sessionId on read = no filtering
+            const retrieved = await store.getTask(task.taskId);
+            expect(retrieved).toBeDefined();
+        });
+
+        it('should allow access when task was created without sessionId', async () => {
+            const task = await store.createTask({}, 1, baseRequest);
+
+            // Any sessionId on read should still see the task
+            const retrieved = await store.getTask(task.taskId, 'session-b');
+            expect(retrieved).toBeDefined();
+        });
+
+        it('should paginate correctly within a session', async () => {
+            // Create 15 tasks for session-a, 5 for session-b
+            for (let i = 1; i <= 15; i++) {
+                await store.createTask({}, i, baseRequest, 'session-a');
+            }
+            for (let i = 16; i <= 20; i++) {
+                await store.createTask({}, i, baseRequest, 'session-b');
+            }
+
+            // First page for session-a should have 10
+            const page1 = await store.listTasks(undefined, 'session-a');
+            expect(page1.tasks).toHaveLength(10);
+            expect(page1.nextCursor).toBeDefined();
+
+            // Second page for session-a should have 5
+            const page2 = await store.listTasks(page1.nextCursor, 'session-a');
+            expect(page2.tasks).toHaveLength(5);
+            expect(page2.nextCursor).toBeUndefined();
+
+            // session-b should only see its 5
+            const resultB = await store.listTasks(undefined, 'session-b');
+            expect(resultB.tasks).toHaveLength(5);
+            expect(resultB.nextCursor).toBeUndefined();
+        });
+    });
+
     describe('cleanup', () => {
         it('should clear all timers and tasks', async () => {
             await store.createTask({ ttl: 1000 }, 1, {

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'@modelcontextprotocol/core': patch
 +---
++
 +Fix InMemoryTaskStore to enforce session isolation. Previously, sessionId was accepted but ignored on all TaskStore methods, allowing any session to enumerate, read, and mutate tasks created by other sessions. The store now persists sessionId at creation time and enforces ownership on all reads and writes.