fix(security): scope pre-existing cross-user read leaks (MCP handlers + aggregates)

docs/testing/automated_test_catalog.md

@@ -61,8 +61,8 @@ flowchart TD
 - Do not hand-edit suite inventory entries in this file. Update the generator or the repository tree, then regenerate.
 
 ## Repo-wide summary
-- Total automated test files: **502**
-- Backend and repo Vitest files: **468**
+- Total automated test files: **504**
+- Backend and repo Vitest files: **470**
 - Frontend Vitest files: **9**
 - Playwright spec files: **25**
 
@@ -72,10 +72,10 @@ flowchart TD
 | Vitest unit tests | 136 |
 | Vitest service tests | 35 |
 | Source-adjacent tests | 62 |
-| Vitest integration tests | 142 |
+| Vitest integration tests | 143 |
 | Vitest CLI tests | 65 |
 | Vitest contract tests | 14 |
-| Vitest security tests | 3 |
+| Vitest security tests | 4 |
 | Vitest subscription tests | 5 |
 | Vitest agent tests | 1 |
 | Vitest fixture tests | 1 |
@@ -363,7 +363,7 @@ flowchart TD
 **Runner:** `vitest`
 **Command:** `npm run test:integration` or `npx vitest run tests/integration`
 **Requirements:** Database configured; remote-dependent subsets additionally need `RUN_REMOTE_TESTS=1`.
-**Files (142):**
+**Files (143):**
 - `tests/integration/aauth_attribution_stamping.test.ts`
 - `tests/integration/aauth_mcp_capability_parity.test.ts`
 - `tests/integration/aauth_mcp_initialize_admission.test.ts`
@@ -433,6 +433,7 @@ flowchart TD
 - `tests/integration/mcp_entity_variations.test.ts`
 - `tests/integration/mcp_get_entity_type_counts.test.ts`
 - `tests/integration/mcp_graph_variations.test.ts`
+- `tests/integration/mcp_handler_cross_user_scoping.test.ts`
 - `tests/integration/mcp_invalid_bearer_auth.test.ts`
 - `tests/integration/mcp_npm_check_update_capability_delta.test.ts`
 - `tests/integration/mcp_npm_check_update.test.ts`
@@ -605,8 +606,9 @@ flowchart TD
 **Runner:** `vitest`
 **Command:** `npx vitest run tests/security`
 **Requirements:** Use alongside the dedicated security validation scripts when changing auth or route protection.
-**Files (3):**
+**Files (4):**
 - `tests/security/auth_topology_matrix.test.ts`
+- `tests/security/cross_user_read_scoping.test.ts`
 - `tests/security/sandbox_mode_resolver.test.ts`
 - `tests/security/tenant_isolation_matrix.test.ts`
 

src/server.ts

@@ -905,13 +905,15 @@ export class NeotomaServer {
     });
 
     const parsed = schema.parse(args ?? {});
+    const userId = this.getAuthenticatedUserId();
     const { db } = await import("./db.js");
 
     // Get all entity snapshots with observation_count = 0
     const { data: potentiallyStale, error: snapshotError } = await db
       .from("entity_snapshots")
       .select("entity_id, entity_type, observation_count, computed_at")
       .eq("observation_count", 0)
+      .eq("user_id", userId)
       .order("computed_at", { ascending: false });
 
     if (snapshotError) {
@@ -942,7 +944,8 @@ export class NeotomaServer {
       const { data: observations, error: obsError } = await db
         .from("observations")
         .select("id")
-        .eq("entity_id", snapshot.entity_id);
+        .eq("entity_id", snapshot.entity_id)
+        .eq("user_id", userId);
 
       if (obsError) {
         continue;
@@ -972,6 +975,7 @@ export class NeotomaServer {
             .from("observations")
             .select("*")
             .eq("entity_id", stale.entity_id)
+            .eq("user_id", userId)
             .order("observed_at", { ascending: false });
 
           if (!observations || observations.length === 0) continue;
@@ -2730,6 +2734,10 @@ export class NeotomaServer {
     const { schemaRegistry } = await import("./services/schema_registry.js");
 
     const parsed = EntitySnapshotRequestSchema.parse(args ?? {});
+    // Scope reads to the authenticated user so a cross-user entity_id cannot be
+    // fetched by id alone. The HTTP callers of getEntityWithProvenance precheck
+    // ownership; this MCP path historically did not.
+    const userId = this.getAuthenticatedUserId();
     const responseFormat = parsed.format ?? "markdown";
 
     const renderEntitySnapshotResponse = async (payload: {
@@ -2802,7 +2810,7 @@ export class NeotomaServer {
     };
 
     // Get entity first to check if it exists and handle merged entity redirection
-    const entity = await getEntityWithProvenance(parsed.entity_id);
+    const entity = await getEntityWithProvenance(parsed.entity_id, false, userId);
 
     if (!entity) {
       throw new McpError(ErrorCode.InvalidParams, `Entity not found: ${parsed.entity_id}`);
@@ -2841,7 +2849,11 @@ export class NeotomaServer {
         }
 
         // Build observations query applying whichever bounds are set
-        let obsQuery = db.from("observations").select("*").eq("entity_id", entity.entity_id);
+        let obsQuery = db
+          .from("observations")
+          .select("*")
+          .eq("entity_id", entity.entity_id)
+          .eq("user_id", userId);
 
         if (parsed.at) {
           // Event-time upper bound: filter by when the event occurred
@@ -3356,7 +3368,7 @@ export class NeotomaServer {
   ): Promise<{ content: Array<{ type: string; text: string }> }> {
     const parsed = RelationshipSnapshotRequestSchema.parse(args ?? {});
 
-    const userId = "00000000-0000-0000-0000-000000000000"; // Default for v0.1.0 single-user
+    const userId = this.getAuthenticatedUserId();
 
     // Get relationship snapshot
     const relationshipKey = `${parsed.relationship_type}:${parsed.source_entity_id}:${parsed.target_entity_id}`;
@@ -3511,8 +3523,9 @@ export class NeotomaServer {
     args: unknown
   ): Promise<{ content: Array<{ type: string; text: string }> }> {
     const parsed = TimelineEventsRequestSchema.parse(args ?? {});
+    const userId = this.getAuthenticatedUserId();
 
-    let query = db.from("timeline_events").select("*");
+    let query = db.from("timeline_events").select("*").eq("user_id", userId);
 
     if (parsed.event_type) {
       query = query.eq("event_type", parsed.event_type);
@@ -3531,10 +3544,13 @@ export class NeotomaServer {
     }
 
     // Get total count
-    let countQuery = db.from("timeline_events").select("*", {
-      count: "exact",
-      head: true,
-    });
+    let countQuery = db
+      .from("timeline_events")
+      .select("*", {
+        count: "exact",
+        head: true,
+      })
+      .eq("user_id", userId);
 
     if (parsed.event_type) {
       countQuery = countQuery.eq("event_type", parsed.event_type);
@@ -4013,7 +4029,10 @@ export class NeotomaServer {
     const schemaRegistry = new SchemaRegistryService();
 
     try {
-      const entityTypes = await schemaRegistry.listEntityTypes(parsed.keyword);
+      const entityTypes = await schemaRegistry.listEntityTypes(
+        parsed.keyword,
+        this.getAuthenticatedUserId()
+      );
 
       // When no keyword, always return summary to avoid huge payload (all types × full schema).
       // When keyword is provided, respect summary param (default full detail for the few matches).

src/services/dashboard_stats.ts

@@ -93,13 +93,15 @@ export async function getDashboardStats(userId?: string): Promise<DashboardStats
     stats.total_entities = entities.length;
   }
 
-  // Get total events count
-  // Note: timeline_events table doesn't have user_id column
-  // User filtering should be done through source_id -> sources.user_id if needed
-  // For now, we rely on RLS policies to enforce user isolation
-  const { count: eventsCount } = await db
-    .from("timeline_events")
-    .select("*", { count: "exact", head: true });
+  // Get total events count. timeline_events DOES carry a user_id column, so
+  // scope it like every other count in this function when a userId is supplied.
+  // (The prior code counted timeline_events across all users — a cross-user
+  // leak in any multi-user deployment, since there is no runtime RLS.)
+  let eventsQuery = db.from("timeline_events").select("*", { count: "exact", head: true });
+  if (userId) {
+    eventsQuery = eventsQuery.eq("user_id", userId);
+  }
+  const { count: eventsCount } = await eventsQuery;
   stats.total_events = eventsCount || 0;
 
   // Get total observations count

src/services/entity_queries.ts

@@ -704,30 +704,37 @@ export async function queryEntities(
  */
 export async function getEntityWithProvenance(
   entityId: string,
-  includeDeleted: boolean = false
+  includeDeleted: boolean = false,
+  userId?: string
 ): Promise<EntityWithProvenance | null> {
-  // Get entity
-  const { data: entity, error: entityError } = await db
-    .from("entities")
-    .select("*")
-    .eq("id", entityId)
-    .single();
+  // Get entity. When a userId is supplied, scope by it so a cross-user
+  // entity_id cannot be read by id alone (the HTTP callers precheck ownership;
+  // the MCP retrieve_entity_snapshot path historically did not).
+  let entityQuery = db.from("entities").select("*").eq("id", entityId);
+  if (userId) {
+    entityQuery = entityQuery.eq("user_id", userId);
+  }
+  const { data: entity, error: entityError } = await entityQuery.single();
 
   if (entityError || !entity) {
     return null;
   }
 
   // Check if entity is merged - redirect to target
   if (entity.merged_to_entity_id) {
-    return getEntityWithProvenance(entity.merged_to_entity_id, includeDeleted);
+    return getEntityWithProvenance(entity.merged_to_entity_id, includeDeleted, userId);
   }
 
   // Check if entity is deleted (unless explicitly requested)
   if (!includeDeleted) {
-    const { data: observations } = await db
+    let deletedCheckQuery = db
       .from("observations")
       .select("source_priority, observed_at, fields")
-      .eq("entity_id", entityId)
+      .eq("entity_id", entityId);
+    if (userId) {
+      deletedCheckQuery = deletedCheckQuery.eq("user_id", userId);
+    }
+    const { data: observations } = await deletedCheckQuery
       .order("source_priority", { ascending: false })
       .order("observed_at", { ascending: false })
       .limit(1);
@@ -742,11 +749,11 @@ export async function getEntityWithProvenance(
   }
 
   // Get snapshot (treat non-PGRST116 errors as "no snapshot" so entity detail still returns)
-  const { data: snapshot, error: snapshotError } = await db
-    .from("entity_snapshots")
-    .select("*")
-    .eq("entity_id", entityId)
-    .single();
+  let snapshotQuery = db.from("entity_snapshots").select("*").eq("entity_id", entityId);
+  if (userId) {
+    snapshotQuery = snapshotQuery.eq("user_id", userId);
+  }
+  const { data: snapshot, error: snapshotError } = await snapshotQuery.single();
 
   let effectiveSnapshot: EntitySnapshotRow | null = snapshot as EntitySnapshotRow | null;
   if (snapshotError && snapshotError.code !== "PGRST116") {
@@ -758,12 +765,15 @@ export async function getEntityWithProvenance(
 
   // Get raw_fragments for this entity
   // Find all sources that have observations for this entity
-  const { data: observations } = await db
+  let sourceObsQuery = db
     .from("observations")
     .select("source_id, user_id")
     .eq("entity_id", entityId)
-    .not("source_id", "is", null)
-    .limit(100); // Get sample of sources
+    .not("source_id", "is", null);
+  if (userId) {
+    sourceObsQuery = sourceObsQuery.eq("user_id", userId);
+  }
+  const { data: observations } = await sourceObsQuery.limit(100); // Get sample of sources
 
   const rawFragments: Record<string, unknown> = {};
   if (observations && observations.length > 0) {

src/services/relationships.ts

@@ -290,7 +290,8 @@ export class RelationshipsService {
   async getRelationshipsForEntity(
     entityId: string,
     direction: "outgoing" | "incoming" | "both" = "both",
-    includeDeleted: boolean = false
+    includeDeleted: boolean = false,
+    userId?: string
   ): Promise<RelationshipSnapshot[]> {
     let query;
 
@@ -305,6 +306,12 @@ export class RelationshipsService {
         .or(`source_entity_id.eq.${entityId},target_entity_id.eq.${entityId}`);
     }
 
+    // Scope to the caller when a userId is supplied. These methods have no
+    // callers today; the optional param keeps them safe-by-construction for reuse.
+    if (userId) {
+      query = query.eq("user_id", userId);
+    }
+
     // Order by recency, then by relationship_key (the relationship_snapshots
     // PRIMARY KEY) as a stable secondary sort so ties on last_observation_at
     // are deterministic. See docs/architecture/determinism.md.
@@ -320,7 +327,7 @@ export class RelationshipsService {
 
     // Filter deleted relationships unless explicitly requested
     if (!includeDeleted) {
-      return this.filterDeletedRelationships(relationships);
+      return this.filterDeletedRelationships(relationships, userId);
     }
 
     return relationships;
@@ -339,7 +346,8 @@ export class RelationshipsService {
    * handler). Returns the input list unchanged when it is empty.
    */
   async filterDeletedRelationships(
-    relationships: RelationshipSnapshot[]
+    relationships: RelationshipSnapshot[],
+    userId?: string
   ): Promise<RelationshipSnapshot[]> {
     if (relationships.length === 0) {
       return relationships;
@@ -348,10 +356,14 @@ export class RelationshipsService {
     const relationshipKeys = relationships.map((r) => r.relationship_key);
 
     // Check for deletion observations (highest priority with _deleted: true)
-    const { data: deletionObservations } = await db
+    let deletionQuery = db
       .from("relationship_observations")
       .select("relationship_key, source_priority, observed_at, metadata")
-      .in("relationship_key", relationshipKeys)
+      .in("relationship_key", relationshipKeys);
+    if (userId) {
+      deletionQuery = deletionQuery.eq("user_id", userId);
+    }
+    const { data: deletionObservations } = await deletionQuery
       .order("source_priority", { ascending: false })
       .order("observed_at", { ascending: false });
 
@@ -391,15 +403,17 @@ export class RelationshipsService {
    */
   async getRelationshipsByType(
     type: RelationshipType,
-    includeDeleted: boolean = false
+    includeDeleted: boolean = false,
+    userId?: string
   ): Promise<RelationshipSnapshot[]> {
     // Order by recency, then by relationship_key (the relationship_snapshots
     // PRIMARY KEY) as a stable secondary sort so ties on last_observation_at
     // are deterministic. See docs/architecture/determinism.md.
-    const { data, error } = await db
-      .from("relationship_snapshots")
-      .select("*")
-      .eq("relationship_type", type)
+    let query = db.from("relationship_snapshots").select("*").eq("relationship_type", type);
+    if (userId) {
+      query = query.eq("user_id", userId);
+    }
+    const { data, error } = await query
       .order("last_observation_at", { ascending: false })
       .order("relationship_key", { ascending: true });
 
@@ -411,7 +425,7 @@ export class RelationshipsService {
 
     // Filter deleted relationships unless explicitly requested
     if (!includeDeleted) {
-      return this.filterDeletedRelationships(relationships);
+      return this.filterDeletedRelationships(relationships, userId);
     }
 
     return relationships;

src/services/schema_registry.ts

@@ -1903,7 +1903,10 @@ export class SchemaRegistryService {
   /**
    * List all active entity types, optionally filtered by keyword with vector search fallback
    */
-  async listEntityTypes(keyword?: string): Promise<
+  async listEntityTypes(
+    keyword?: string,
+    userId?: string
+  ): Promise<
     Array<{
       entity_type: string;
       schema_version: string;
@@ -1913,11 +1916,16 @@ export class SchemaRegistryService {
       match_type?: "keyword" | "vector";
     }>
   > {
-    // Get all active schemas from database
-    const query = db
+    // Get all active schemas from database. When a userId is supplied, return
+    // only global schemas (user_id IS NULL) plus that user's own schemas, so a
+    // user's private entity-type names/shapes are not disclosed cross-user.
+    let query = db
       .from("schema_registry")
       .select("entity_type, schema_version, schema_definition")
       .eq("active", true);
+    if (userId) {
+      query = query.or(`user_id.is.null,user_id.eq.${userId}`);
+    }
 
     const { data: dbSchemas } = await query;