From 53551b0ecf4a61b51bcb9ef0feabdf464511d8d4 Mon Sep 17 00:00:00 2001
From: filou <philipperolet@gmail.com>
Date: Thu, 21 Nov 2024 15:39:18 +0100
Subject: [PATCH] [JIT Datasources] Dust apps: restrict access to
 "conversations" space

Description
---
Fixes https://github.com/dust-tt/tasks/issues/1658

As discussed in shipping JIT datasources, the "conversations" space is
readable by all and as such should not be accessed by API, including
by running a dust app with a datasource block.

This is to prevent an attacker enumerating data source[view]s ids
passing them as config of a datasource block in a dust app

This is particularly important with public dust apps that can be executed
on another workspace with no space permissions checks, but the gating
is also legitimate for private apps.

Only case in which this is allowed is for our packaged apps, via a
system key, in particular "assistant-retrieval-v2" that needs access
to the conversation space

This solution relies on the assumptions that:
- system keys are a good way to distinguish internal calls to our
packaged apps, from calls from API users;
- our packaged apps internal calls cannot be made to pass arbitrary
datasource names.

Risks
---
None, since the conversations space is not used yet so
forbiddenAccessToConv is always false

Deploy
---
front
---
 front/pages/api/registry/[type]/lookup.ts | 54 +++++++++++++++--------
 1 file changed, 36 insertions(+), 18 deletions(-)

diff --git a/front/pages/api/registry/[type]/lookup.ts b/front/pages/api/registry/[type]/lookup.ts
index 8a1ea7f73cd86..d7ea06bc09030 100644
--- a/front/pages/api/registry/[type]/lookup.ts
+++ b/front/pages/api/registry/[type]/lookup.ts
@@ -180,28 +180,37 @@ async function handleDataSourceView(
     auth,
     dataSourceViewId
   );
-  if (!dataSourceView) {
+
+  // This check is meant to block access to "conversations" space through a
+  // datasource block in a dust app, which could lead to data leaks, see related PR
+  // Only case in which this is allowed is for our packaged apps, via a system
+  // key, in particular "assistant-retrieval-v2" that needs access to the
+  // conversation space
+  const forbiddenAccessToConversations =
+    dataSourceView?.space?.kind === "conversations" && !auth.isSystemKey();
+
+  if (!dataSourceView || forbiddenAccessToConversations) {
     return new Err(new Error("Data source view not found."));
   }
 
-  if (dataSourceView.canRead(auth)) {
-    const { dataSource } = dataSourceView;
-
-    return new Ok({
-      project_id: parseInt(dataSource.dustAPIProjectId),
-      data_source_id: dataSource.dustAPIDataSourceId,
-      view_filter: {
-        tags: null,
-        parents: {
-          in: dataSourceView.parentsIn,
-          not: null,
-        },
-        timestamp: null,
-      },
-    });
+  if (!dataSourceView.canRead(auth)) {
+    return new Err(new Error("No access to data source view."));
   }
 
-  return new Err(new Error("No access to data source view."));
+  const { dataSource } = dataSourceView;
+
+  return new Ok({
+    project_id: parseInt(dataSource.dustAPIProjectId),
+    data_source_id: dataSource.dustAPIDataSourceId,
+    view_filter: {
+      tags: null,
+      parents: {
+        in: dataSourceView.parentsIn,
+        not: null,
+      },
+      timestamp: null,
+    },
+  });
 }
 
 async function handleDataSource(
@@ -227,7 +236,16 @@ async function handleDataSource(
     // TODO(DATASOURCE_SID): Clean-up
     { origin: "registry_lookup" }
   );
-  if (!dataSource) {
+
+  // This check is meant to block access to "conversations" space through a
+  // datasource block in a dust app, which could lead to data leaks, see related PR
+  // Only case in which this is allowed is for our packaged apps, via a system
+  // key, in particular "assistant-retrieval-v2" that needs access to the
+  // conversation space
+  const forbiddenAccessToConversations =
+    dataSource?.space?.kind === "conversations" && !auth.isSystemKey();
+
+  if (!dataSource || forbiddenAccessToConversations) {
     return new Err(new Error("Data source not found."));
   }