fix(core,server): close child-ID cross-dataset leak on Dataset handle

Address CodeRabbit round-2 review on MASTRA-4438:

- Dataset.getItem / getItemHistory / getExperiment / listExperimentResults
  now verify the returned child row's parent datasetId matches this.id
  before returning. Prevents a caller with a valid handle to dataset A from
  reading item/experiment rows belonging to dataset B via a caller-supplied
  child id.
- Dataset.updateExperimentResult / deleteExperiment throw NOT_FOUND on
  ownership mismatch, matching the existing deleteItem contract.
- Dataset.listItems now forwards this.#scope via ListDatasetItemsInput.filters
  instead of an extra preflight roundtrip.
- Server handler tests: assert HTTP 404 status on scope-mismatch (not just
  HTTPException) and add projectId-mismatch cases for update + delete
  routes.

Co-Authored-By: Mastra Code (anthropic/claude-opus-4-7) <noreply@mastra.ai>

Author: NikAiyer

packages/core/src/datasets/__tests__/dataset.test.ts

@@ -567,4 +567,81 @@ describe('Dataset', () => {
     expect('items' in (result as any)).toBe(true);
     expect((result as any).items.length).toBe(2);
   });
+
+  // ---------------------------------------------------------------------------
+  // Ownership gates: reject child records that belong to a different dataset
+  // even when the caller holds a valid handle to the current dataset.
+  // ---------------------------------------------------------------------------
+
+  it('getItem returns null when the item belongs to a different dataset', async () => {
+    const other = await datasetsStorage.createDataset({ name: 'Other DS' });
+    const otherItem = await datasetsStorage.addItem({ datasetId: other.id, input: { x: 1 } });
+    const leaked = await ds.getItem({ itemId: otherItem.id });
+    expect(leaked).toBeNull();
+  });
+
+  it('getItemHistory filters out rows that belong to a different dataset', async () => {
+    const other = await datasetsStorage.createDataset({ name: 'Other DS' });
+    const otherItem = await datasetsStorage.addItem({ datasetId: other.id, input: { x: 1 } });
+    const rows = await ds.getItemHistory({ itemId: otherItem.id });
+    expect(rows).toEqual([]);
+  });
+
+  it('getExperiment returns null when the experiment belongs to a different dataset', async () => {
+    const other = await datasetsStorage.createDataset({ name: 'Other DS' });
+    const otherExp = await experimentsStorage.createExperiment({
+      datasetId: other.id,
+      datasetVersion: 0,
+      targetType: 'agent',
+      targetId: 'inline',
+      totalItems: 0,
+    });
+    const leaked = await ds.getExperiment({ experimentId: otherExp.id });
+    expect(leaked).toBeNull();
+  });
+
+  it('deleteExperiment throws NOT_FOUND when the experiment belongs to a different dataset', async () => {
+    const other = await datasetsStorage.createDataset({ name: 'Other DS' });
+    const otherExp = await experimentsStorage.createExperiment({
+      datasetId: other.id,
+      datasetVersion: 0,
+      targetType: 'agent',
+      targetId: 'inline',
+      totalItems: 0,
+    });
+    await expect(ds.deleteExperiment({ experimentId: otherExp.id })).rejects.toThrow(MastraError);
+    // Original experiment should still exist under its true dataset
+    const stillThere = await experimentsStorage.getExperimentById({ id: otherExp.id });
+    expect(stillThere?.id).toBe(otherExp.id);
+  });
+
+  it('listExperimentResults throws NOT_FOUND when experiment belongs to a different dataset', async () => {
+    const other = await datasetsStorage.createDataset({ name: 'Other DS' });
+    const otherExp = await experimentsStorage.createExperiment({
+      datasetId: other.id,
+      datasetVersion: 0,
+      targetType: 'agent',
+      targetId: 'inline',
+      totalItems: 0,
+    });
+    await expect(ds.listExperimentResults({ experimentId: otherExp.id })).rejects.toThrow(MastraError);
+  });
+
+  it('updateExperimentResult throws when experiment belongs to a different dataset', async () => {
+    const other = await datasetsStorage.createDataset({ name: 'Other DS' });
+    const otherExp = await experimentsStorage.createExperiment({
+      datasetId: other.id,
+      datasetVersion: 0,
+      targetType: 'agent',
+      targetId: 'inline',
+      totalItems: 0,
+    });
+    await expect(
+      ds.updateExperimentResult({ id: 'any-result-id', experimentId: otherExp.id, status: 'reviewed' }),
+    ).rejects.toThrow(MastraError);
+  });
+
+  it('updateExperimentResult rejects when called without experimentId', async () => {
+    await expect(ds.updateExperimentResult({ id: 'any-result-id', status: 'reviewed' })).rejects.toThrow(MastraError);
+  });
 });

packages/core/src/datasets/dataset.ts

@@ -123,6 +123,20 @@ export class Dataset {
     }
   }
 
+  /**
+   * Ownership gate: verifies a child record's `datasetId` matches `this.id`.
+   * Prevents a valid scoped handle from reading/mutating child records
+   * (items, experiments, results) that live under a different dataset — even
+   * one in the same tenant. Returns `null` when the record is missing or
+   * belongs to a different dataset, so callers can either return null or
+   * translate to NOT_FOUND depending on their contract.
+   */
+  #ownsChild<T extends { datasetId?: string | null }>(record: T | null | undefined): T | null {
+    if (!record) return null;
+    if (record.datasetId !== this.id) return null;
+    return record;
+  }
+
   // ---------------------------------------------------------------------------
   // Dataset metadata
   // ---------------------------------------------------------------------------
@@ -209,7 +223,8 @@ export class Dataset {
   async getItem(args: { itemId: string; version?: number }): Promise<DatasetItem | null> {
     await this.#assertScope();
     const store = await this.#getDatasetsStore();
-    return store.getItemById({ id: args.itemId, datasetVersion: args.version });
+    const item = await store.getItemById({ id: args.itemId, datasetVersion: args.version });
+    return this.#ownsChild(item);
   }
 
   /**
@@ -224,15 +239,17 @@ export class Dataset {
     | DatasetItem[]
     | { items: DatasetItem[]; pagination: { total: number; page: number; perPage: number | false; hasMore: boolean } }
   > {
-    await this.#assertScope();
     const store = await this.#getDatasetsStore();
     if (args?.version) {
+      // getItemsByVersion is keyed by datasetId — gate via scoped parent existence
+      await this.#assertScope();
       return store.getItemsByVersion({ datasetId: this.id, version: args.version });
     }
     return store.listItems({
       datasetId: this.id,
       search: args?.search,
       pagination: { page: args?.page ?? 0, perPage: args?.perPage ?? 20 },
+      filters: this.#scope,
     });
   }
 
@@ -287,7 +304,11 @@ export class Dataset {
   async getItemHistory(args: { itemId: string }): Promise<DatasetItemRow[]> {
     await this.#assertScope();
     const store = await this.#getDatasetsStore();
-    return store.getItemHistory(args.itemId);
+    const rows = await store.getItemHistory(args.itemId);
+    // Ownership gate: SCD-2 history is keyed only by item id — filter out any
+    // rows that don't belong to this dataset so a known cross-dataset item id
+    // cannot leak history through a valid scoped handle.
+    return rows.filter(row => row.datasetId === this.id);
   }
 
   // ---------------------------------------------------------------------------
@@ -391,41 +412,74 @@ export class Dataset {
     });
   }
 
+  /**
+   * Verify the experiment belongs to this dataset (and, by extension, to the
+   * handle's tenancy scope which was enforced when the handle was minted).
+   * Throws NOT_FOUND on missing or cross-dataset experiments so cross-tenant
+   * mutation via a valid scoped handle + a known foreign experimentId is
+   * rejected.
+   */
+  async #assertExperimentOwnership(experimentId: string): Promise<void> {
+    const experimentsStore = await this.#getExperimentsStore();
+    const experiment = await experimentsStore.getExperimentById({ id: experimentId });
+    if (!experiment || experiment.datasetId !== this.id) {
+      throw new MastraError({
+        id: 'EXPERIMENT_NOT_FOUND',
+        text: `Experiment not found: ${experimentId}`,
+        domain: 'STORAGE',
+        category: 'USER',
+      });
+    }
+  }
+
   /**
    * Get a specific experiment (run) by ID.
    */
   async getExperiment(args: { experimentId: string }) {
     await this.#assertScope();
     const experimentsStore = await this.#getExperimentsStore();
-    return experimentsStore.getExperimentById({ id: args.experimentId });
+    const experiment = await experimentsStore.getExperimentById({ id: args.experimentId });
+    if (!experiment || experiment.datasetId !== this.id) return null;
+    return experiment;
   }
 
   /**
    * List results for a specific experiment.
    */
   async listExperimentResults(args: { experimentId: string; page?: number; perPage?: number }) {
-    await this.#assertScope();
+    await this.#assertExperimentOwnership(args.experimentId);
     const experimentsStore = await this.#getExperimentsStore();
     return experimentsStore.listExperimentResults({
       experimentId: args.experimentId,
       pagination: { page: args?.page ?? 0, perPage: args?.perPage ?? 20 },
     });
   }
 
-  /**
-   * Delete an experiment (run) by ID.
-   */
   /**
    * Update an experiment result's status or tags.
    */
   async updateExperimentResult(input: UpdateExperimentResultInput) {
-    await this.#assertScope();
+    // The result's parent experiment must belong to this dataset. If the
+    // caller supplied `experimentId`, verify ownership on that; otherwise we
+    // cannot bind the update to this dataset and must reject.
+    if (!input.experimentId) {
+      throw new MastraError({
+        id: 'EXPERIMENT_RESULT_MISSING_EXPERIMENT_ID',
+        text: 'updateExperimentResult requires experimentId when called via a Dataset handle',
+        domain: 'STORAGE',
+        category: 'USER',
+      });
+    }
+    await this.#assertExperimentOwnership(input.experimentId);
     const experimentsStore = await this.#getExperimentsStore();
     return experimentsStore.updateExperimentResult(input);
   }
 
+  /**
+   * Delete an experiment (run) by ID.
+   */
   async deleteExperiment(args: { experimentId: string }) {
-    await this.#assertScope();
+    await this.#assertExperimentOwnership(args.experimentId);
     const experimentsStore = await this.#getExperimentsStore();
     return experimentsStore.deleteExperiment({ id: args.experimentId });
   }

packages/server/src/server/handlers/datasets.test.ts

@@ -112,13 +112,16 @@ describe('Datasets Handlers', () => {
         projectId: 'proj_1',
       });
 
-      await expect(
-        GET_DATASET_ROUTE.handler({
-          ...createTestServerContext({ mastra }),
-          datasetId: created.id,
-          organizationId: 'org_b',
-        } as any),
-      ).rejects.toThrow(HTTPException);
+      const err = await GET_DATASET_ROUTE.handler({
+        ...createTestServerContext({ mastra }),
+        datasetId: created.id,
+        organizationId: 'org_b',
+      } as any).then(
+        () => null,
+        (e: unknown) => e,
+      );
+      expect(err).toBeInstanceOf(HTTPException);
+      expect((err as HTTPException).status).toBe(404);
     });
 
     it('returns 404 when projectId does not match', async () => {
@@ -128,14 +131,17 @@ describe('Datasets Handlers', () => {
         projectId: 'proj_1',
       });
 
-      await expect(
-        GET_DATASET_ROUTE.handler({
-          ...createTestServerContext({ mastra }),
-          datasetId: created.id,
-          organizationId: 'org_a',
-          projectId: 'proj_2',
-        } as any),
-      ).rejects.toThrow(HTTPException);
+      const err = await GET_DATASET_ROUTE.handler({
+        ...createTestServerContext({ mastra }),
+        datasetId: created.id,
+        organizationId: 'org_a',
+        projectId: 'proj_2',
+      } as any).then(
+        () => null,
+        (e: unknown) => e,
+      );
+      expect(err).toBeInstanceOf(HTTPException);
+      expect((err as HTTPException).status).toBe(404);
     });
   });
 
@@ -158,27 +164,55 @@ describe('Datasets Handlers', () => {
       expect(result.name).toBe('After');
     });
 
-    it('rejects update when organizationId does not match', async () => {
+    it('rejects update with 404 when organizationId does not match', async () => {
       const created = await mastra.datasets.create({
         name: 'Before',
         organizationId: 'org_a',
         projectId: 'proj_1',
       });
 
-      await expect(
-        UPDATE_DATASET_ROUTE.handler({
-          ...createTestServerContext({ mastra }),
-          datasetId: created.id,
-          organizationId: 'org_b',
-          name: 'After',
-        } as any),
-      ).rejects.toThrow(HTTPException);
+      const err = await UPDATE_DATASET_ROUTE.handler({
+        ...createTestServerContext({ mastra }),
+        datasetId: created.id,
+        organizationId: 'org_b',
+        name: 'After',
+      } as any).then(
+        () => null,
+        (e: unknown) => e,
+      );
+      expect(err).toBeInstanceOf(HTTPException);
+      expect((err as HTTPException).status).toBe(404);
 
       // dataset unchanged
       const untouched = await mastra.datasets.get({ id: created.id });
       const details = await untouched.getDetails();
       expect(details.name).toBe('Before');
     });
+
+    it('rejects update with 404 when projectId does not match', async () => {
+      const created = await mastra.datasets.create({
+        name: 'Before',
+        organizationId: 'org_a',
+        projectId: 'proj_1',
+      });
+
+      const err = await UPDATE_DATASET_ROUTE.handler({
+        ...createTestServerContext({ mastra }),
+        datasetId: created.id,
+        organizationId: 'org_a',
+        projectId: 'proj_2',
+        name: 'After',
+      } as any).then(
+        () => null,
+        (e: unknown) => e,
+      );
+      expect(err).toBeInstanceOf(HTTPException);
+      expect((err as HTTPException).status).toBe(404);
+
+      const untouched = await mastra.datasets.get({ id: created.id });
+      const details = await untouched.getDetails();
+      expect(details.name).toBe('Before');
+    });
   });
 
   describe('DELETE_DATASET_ROUTE tenancy', () => {
@@ -226,6 +260,28 @@ describe('Datasets Handlers', () => {
       expect(details.id).toBe(created.id);
       expect(details.organizationId).toBe('org_a');
     });
+
+    it('silently no-ops delete when projectId does not match and dataset remains', async () => {
+      const created = await mastra.datasets.create({
+        name: 'Guarded',
+        organizationId: 'org_a',
+        projectId: 'proj_1',
+      });
+
+      const result = (await DELETE_DATASET_ROUTE.handler({
+        ...createTestServerContext({ mastra }),
+        datasetId: created.id,
+        organizationId: 'org_a',
+        projectId: 'proj_2',
+      } as any)) as any;
+
+      expect(result.success).toBe(true);
+
+      const survivor = await mastra.datasets.get({ id: created.id });
+      const details = await survivor.getDetails();
+      expect(details.id).toBe(created.id);
+      expect(details.projectId).toBe('proj_1');
+    });
   });
 
   describe('item tool mocks', () => {