Skip worktree trust queries for remote collab project clients (#46256)

Kirill Bulatov created

Release Notes:

- Skip worktree trust queries for remote collab project clients

Change summary

crates/collab/src/tests/channel_buffer_tests.rs |   6 
crates/collab/src/tests/editor_tests.rs         | 115 +++++++++++++++++++
crates/collab/src/tests/test_server.rs          |  27 +++
crates/project/src/project.rs                   |  13 +
crates/project/src/worktree_store.rs            |  26 +++
crates/workspace/src/workspace.rs               |  23 ---
crates/worktree/src/worktree.rs                 |   2 
7 files changed, 176 insertions(+), 36 deletions(-)

Detailed changes

crates/collab/src/tests/channel_buffer_tests.rs 🔗

@@ -153,9 +153,9 @@ async fn test_channel_notes_participant_indices(
         .fs()
         .insert_tree("/root", json!({"file.txt": "123"}))
         .await;
-    let (project_a, worktree_id_a) = client_a.build_local_project("/root", cx_a).await;
-    let project_b = client_b.build_empty_local_project(cx_b);
-    let project_c = client_c.build_empty_local_project(cx_c);
+    let (project_a, worktree_id_a) = client_a.build_local_project_with_trust("/root", cx_a).await;
+    let project_b = client_b.build_empty_local_project(false, cx_b);
+    let project_c = client_c.build_empty_local_project(false, cx_c);
 
     let (workspace_a, mut cx_a) = client_a.build_workspace(&project_a, cx_a);
     let (workspace_b, mut cx_b) = client_b.build_workspace(&project_b, cx_b);

crates/collab/src/tests/editor_tests.rs 🔗

@@ -1,5 +1,6 @@
 use crate::{rpc::RECONNECT_TIMEOUT, tests::TestServer};
 use call::ActiveCall;
+use collections::{HashMap, HashSet};
 use editor::{
     DocumentColorsRenderMode, Editor, FETCH_COLORS_DEBOUNCE_TIMEOUT, MultiBufferOffset, RowInfo,
     SelectionEffects,
@@ -26,6 +27,7 @@ use pretty_assertions::assert_eq;
 use project::{
     ProgressToken, ProjectPath, SERVER_PROGRESS_THROTTLE_TIMEOUT,
     lsp_store::lsp_ext_command::{ExpandedMacro, LspExtExpandMacro},
+    trusted_worktrees::{PathTrust, TrustedWorktrees},
 };
 use recent_projects::disconnected_overlay::DisconnectedOverlay;
 use rpc::RECEIVE_TIMEOUT;
@@ -4639,6 +4641,119 @@ fn extract_color_inlays(editor: &Editor, cx: &App) -> Vec<Rgba> {
         .collect()
 }
 
+#[gpui::test]
+async fn test_remote_project_worktree_trust(cx_a: &mut TestAppContext, cx_b: &mut TestAppContext) {
+    let has_restricted_worktrees = |project: &gpui::Entity<project::Project>,
+                                    cx: &mut VisualTestContext| {
+        cx.update(|_, cx| {
+            let worktree_store = project.read(cx).worktree_store();
+            TrustedWorktrees::try_get_global(cx)
+                .unwrap()
+                .read(cx)
+                .has_restricted_worktrees(&worktree_store, cx)
+        })
+    };
+
+    cx_a.update(|cx| {
+        project::trusted_worktrees::init(HashMap::default(), None, None, cx);
+    });
+    cx_b.update(|cx| {
+        project::trusted_worktrees::init(HashMap::default(), None, None, cx);
+    });
+
+    let mut server = TestServer::start(cx_a.executor()).await;
+    let client_a = server.create_client(cx_a, "user_a").await;
+    let client_b = server.create_client(cx_b, "user_b").await;
+    server
+        .create_room(&mut [(&client_a, cx_a), (&client_b, cx_b)])
+        .await;
+
+    client_a
+        .fs()
+        .insert_tree(
+            path!("/a"),
+            json!({
+                "file.txt": "contents",
+            }),
+        )
+        .await;
+
+    let (project_a, worktree_id) = client_a
+        .build_local_project_with_trust(path!("/a"), cx_a)
+        .await;
+    let (workspace_a, cx_a) = client_a.build_workspace(&project_a, cx_a);
+    let active_call_a = cx_a.read(ActiveCall::global);
+    let project_id = active_call_a
+        .update(cx_a, |call, cx| call.share_project(project_a.clone(), cx))
+        .await
+        .unwrap();
+
+    let project_b = client_b.join_remote_project(project_id, cx_b).await;
+    let (workspace_b, cx_b) = client_b.build_workspace(&project_b, cx_b);
+
+    let _editor_a = workspace_a
+        .update_in(cx_a, |workspace, window, cx| {
+            workspace.open_path(
+                (worktree_id, rel_path("src/main.rs")),
+                None,
+                true,
+                window,
+                cx,
+            )
+        })
+        .await
+        .unwrap()
+        .downcast::<Editor>()
+        .unwrap();
+
+    let _editor_b = workspace_b
+        .update_in(cx_b, |workspace, window, cx| {
+            workspace.open_path(
+                (worktree_id, rel_path("src/main.rs")),
+                None,
+                true,
+                window,
+                cx,
+            )
+        })
+        .await
+        .unwrap()
+        .downcast::<Editor>()
+        .unwrap();
+
+    cx_a.run_until_parked();
+    cx_b.run_until_parked();
+
+    assert!(
+        has_restricted_worktrees(&project_a, cx_a),
+        "local client should have restricted worktrees after opening it"
+    );
+    assert!(
+        !has_restricted_worktrees(&project_b, cx_b),
+        "remote client joined a project should have no restricted worktrees"
+    );
+
+    cx_a.update(|_, cx| {
+        if let Some(trusted_worktrees) = TrustedWorktrees::try_get_global(cx) {
+            trusted_worktrees.update(cx, |trusted_worktrees, cx| {
+                trusted_worktrees.trust(
+                    &project_a.read(cx).worktree_store(),
+                    HashSet::from_iter([PathTrust::Worktree(worktree_id)]),
+                    cx,
+                );
+            });
+        }
+    });
+    assert!(
+        !has_restricted_worktrees(&project_a, cx_a),
+        "local client should have no worktrees after trusting those"
+    );
+    assert!(
+        !has_restricted_worktrees(&project_b, cx_b),
+        "remote client should still be trusted"
+    );
+}
+
 fn blame_entry(sha: &str, range: Range<u32>) -> git::blame::BlameEntry {
     git::blame::BlameEntry {
         sha: sha.parse().unwrap(),

crates/collab/src/tests/test_server.rs 🔗

@@ -745,7 +745,24 @@ impl TestClient {
         root_path: impl AsRef<Path>,
         cx: &mut TestAppContext,
     ) -> (Entity<Project>, WorktreeId) {
-        let project = self.build_empty_local_project(cx);
+        let project = self.build_empty_local_project(false, cx);
+        let (worktree, _) = project
+            .update(cx, |p, cx| p.find_or_create_worktree(root_path, true, cx))
+            .await
+            .unwrap();
+        worktree
+            .read_with(cx, |tree, _| tree.as_local().unwrap().scan_complete())
+            .await;
+        cx.run_until_parked();
+        (project, worktree.read_with(cx, |tree, _| tree.id()))
+    }
+
+    pub async fn build_local_project_with_trust(
+        &self,
+        root_path: impl AsRef<Path>,
+        cx: &mut TestAppContext,
+    ) -> (Entity<Project>, WorktreeId) {
+        let project = self.build_empty_local_project(true, cx);
         let (worktree, _) = project
             .update(cx, |p, cx| p.find_or_create_worktree(root_path, true, cx))
             .await
@@ -832,7 +849,11 @@ impl TestClient {
         self.active_workspace(cx)
     }
 
-    pub fn build_empty_local_project(&self, cx: &mut TestAppContext) -> Entity<Project> {
+    pub fn build_empty_local_project(
+        &self,
+        init_worktree_trust: bool,
+        cx: &mut TestAppContext,
+    ) -> Entity<Project> {
         cx.update(|cx| {
             Project::local(
                 self.client().clone(),
@@ -841,7 +862,7 @@ impl TestClient {
                 self.app_state.languages.clone(),
                 self.app_state.fs.clone(),
                 None,
-                false,
+                init_worktree_trust,
                 cx,
             )
         })

crates/project/src/project.rs 🔗

@@ -4854,11 +4854,6 @@ impl Project {
     ) -> Result<()> {
         this.update(&mut cx, |project, cx| {
             let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id);
-            if let Some(trusted_worktrees) = TrustedWorktrees::try_get_global(cx) {
-                trusted_worktrees.update(cx, |trusted_worktrees, cx| {
-                    trusted_worktrees.can_trust(&project.worktree_store, worktree_id, cx)
-                });
-            }
             if let Some(worktree) = project.worktree_for_id(worktree_id, cx) {
                 worktree.update(cx, |worktree, _| {
                     let worktree = worktree.as_remote_mut().unwrap();
@@ -4891,6 +4886,10 @@ impl Project {
         envelope: TypedEnvelope<proto::TrustWorktrees>,
         mut cx: AsyncApp,
     ) -> Result<proto::Ack> {
+        if this.read_with(&cx, |project, _| project.is_via_collab())? {
+            return Ok(proto::Ack {});
+        }
+
         let trusted_worktrees = cx
             .update(|cx| TrustedWorktrees::try_get_global(cx))?
             .context("missing trusted worktrees")?;
@@ -4914,6 +4913,10 @@ impl Project {
         envelope: TypedEnvelope<proto::RestrictWorktrees>,
         mut cx: AsyncApp,
     ) -> Result<proto::Ack> {
+        if this.read_with(&cx, |project, _| project.is_via_collab())? {
+            return Ok(proto::Ack {});
+        }
+
         let trusted_worktrees = cx
             .update(|cx| TrustedWorktrees::try_get_global(cx))?
             .context("missing trusted worktrees")?;

crates/project/src/worktree_store.rs 🔗

@@ -25,7 +25,7 @@ use worktree::{
     WorktreeId,
 };
 
-use crate::ProjectPath;
+use crate::{ProjectPath, trusted_worktrees::TrustedWorktrees};
 
 enum WorktreeStoreState {
     Local {
@@ -469,6 +469,7 @@ impl WorktreeStore {
         cx: &mut Context<Self>,
     ) -> Task<Result<Entity<Worktree>>> {
         let abs_path: Arc<SanitizedPath> = SanitizedPath::new_arc(&abs_path);
+        let is_via_collab = matches!(&self.state, WorktreeStoreState::Remote { upstream_client, .. } if upstream_client.is_via_collab());
         if !self.loading_worktrees.contains_key(&abs_path) {
             let task = match &self.state {
                 WorktreeStoreState::Remote {
@@ -497,7 +498,28 @@ impl WorktreeStore {
             this.update(cx, |this, _| this.loading_worktrees.remove(&abs_path))
                 .ok();
             match result {
-                Ok(worktree) => Ok(worktree),
+                Ok(worktree) => {
+                    if !is_via_collab {
+                        if let Some((trusted_worktrees, worktree_store)) = this
+                            .update(cx, |_, cx| {
+                                TrustedWorktrees::try_get_global(cx).zip(Some(cx.entity()))
+                            })
+                            .ok()
+                            .flatten()
+                        {
+                            trusted_worktrees
+                                .update(cx, |trusted_worktrees, cx| {
+                                    trusted_worktrees.can_trust(
+                                        &worktree_store,
+                                        worktree.read(cx).id(),
+                                        cx,
+                                    );
+                                })
+                                .ok();
+                        }
+                    }
+                    Ok(worktree)
+                }
                 Err(err) => Err((*err).cloned()),
             }
         })

crates/workspace/src/workspace.rs 🔗

@@ -1283,32 +1283,11 @@ impl Workspace {
                     this.collaborator_left(*peer_id, window, cx);
                 }
 
-                project::Event::WorktreeUpdatedEntries(worktree_id, _) => {
-                    if let Some(trusted_worktrees) = TrustedWorktrees::try_get_global(cx) {
-                        trusted_worktrees.update(cx, |trusted_worktrees, cx| {
-                            trusted_worktrees.can_trust(
-                                &this.project().read(cx).worktree_store(),
-                                *worktree_id,
-                                cx,
-                            );
-                        });
-                    }
-                }
-
                 project::Event::WorktreeRemoved(_) => {
                     this.update_worktree_data(window, cx);
                 }
 
-                project::Event::WorktreeAdded(worktree_id) => {
-                    if let Some(trusted_worktrees) = TrustedWorktrees::try_get_global(cx) {
-                        trusted_worktrees.update(cx, |trusted_worktrees, cx| {
-                            trusted_worktrees.can_trust(
-                                &this.project().read(cx).worktree_store(),
-                                *worktree_id,
-                                cx,
-                            );
-                        });
-                    }
+                project::Event::WorktreeUpdatedEntries(..) | project::Event::WorktreeAdded(..) => {
                     this.update_worktree_data(window, cx);
                 }
 

crates/worktree/src/worktree.rs 🔗

@@ -354,7 +354,7 @@ struct UpdateObservationState {
     _maintain_remote_snapshot: Task<Option<()>>,
 }
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub enum Event {
     UpdatedEntries(UpdatedEntriesSet),
     UpdatedGitRepositories(UpdatedGitRepositoriesSet),