🚑 Revoke file access when a member leaves the team (#12097)

Delete the team-scoped file, project and pin relations on leave-team
and delete-team-member, require a live team membership in the file,
project, search and listing permission queries, and clean
pre-existing orphan rows with a migration. Former members now get
not-found on file actions instead of keeping read and rename access
(GHSA-v9r9-h77c-55m2).

AI-assisted-by: muse-spark-1.3-contributor

.serena/memories/backend/auth-permissions-product-domains.md

@@ -15,6 +15,7 @@
 
 - `app.rpc.permissions` provides predicate/check factories. Failed permission checks intentionally raise `:not-found` / `:object-not-found`, not an authorization-specific error, to avoid leaking object existence.
 - Team role flags are normalized as owner > admin > editor > viewer. Owner/admin imply edit; any membership row implies read.
+- File/project role rows only count with a live team membership: the file/project permission queries require a matching `team_profile_rel` row, and leaving or removing a member deletes their team-scoped `file`/`project`/`team-project-profile-rel` rows (GHSA-v9r9-h77c-55m2).
 - File/project/comment checks are implemented in the owning command namespaces, often via helpers imported from `files`, `teams`, or `projects`; do not bypass those helpers with direct DB lookups unless preserving their not-found semantics.
 - Comment permission includes both logged-in state and the file/team comment policy. Shared viewer paths may pass `share-id`; preserve that path when changing comment queries.
 

backend/src/app/binfile/common.clj

@@ -344,8 +344,13 @@
           fpr.can_edit
      from file_profile_rel as fpr
     inner join file as f on (f.id = fpr.file_id)
+    inner join project as p on (p.id = f.project_id)
     where fpr.file_id = ?
       and fpr.profile_id = ?
+      and exists (select 1
+                    from team_profile_rel as tpr
+                   where tpr.team_id = p.team_id
+                     and tpr.profile_id = fpr.profile_id)
    union all
    select tpr.is_owner,
           tpr.is_admin,
@@ -361,8 +366,13 @@
           ppr.can_edit
      from project_profile_rel as ppr
     inner join file as f on (f.project_id = ppr.project_id)
+    inner join project as p on (p.id = ppr.project_id)
     where f.id = ?
-      and ppr.profile_id = ?")
+      and ppr.profile_id = ?
+      and exists (select 1
+                    from team_profile_rel as tpr
+                   where tpr.team_id = p.team_id
+                     and tpr.profile_id = ppr.profile_id)")
 
 (defn- get-file-permissions*
   [conn profile-id file-id]

backend/src/app/migrations.clj

@@ -499,7 +499,10 @@
     :fn (mg/resource "app/migrations/sql/0152-improve-uuid-defaults-and-drop-extension.sql")}
 
    {:name "0152-rename-version-and-add-indexes-to-server-error-report"
-    :fn (mg/resource "app/migrations/sql/0152-rename-version-and-add-indexes-to-server-error-report.sql")}])
+    :fn (mg/resource "app/migrations/sql/0152-rename-version-and-add-indexes-to-server-error-report.sql")}
+
+   {:name "0153-del-orphan-profile-rels-after-team-leave"
+    :fn (mg/resource "app/migrations/sql/0153-del-orphan-profile-rels-after-team-leave.sql")}])
 
 (defn apply-migrations!
   [pool name migrations]

backend/src/app/migrations/sql/0153-del-orphan-profile-rels-after-team-leave.sql

@@ -0,0 +1,27 @@
+-- Remove team-scoped role rows whose profile is no longer a member of
+-- the team that owns the project/file. Leftover from before
+-- leave-team/delete-team-member cleaned them up (GHSA-v9r9-h77c-55m2).
+-- Rows with a live team membership are kept.
+
+DELETE FROM file_profile_rel AS fpr
+USING file AS f
+INNER JOIN project AS p ON (p.id = f.project_id)
+WHERE fpr.file_id = f.id
+  AND NOT EXISTS (SELECT 1
+                    FROM team_profile_rel AS tpr
+                   WHERE tpr.team_id = p.team_id
+                     AND tpr.profile_id = fpr.profile_id);
+
+DELETE FROM project_profile_rel AS ppr
+USING project AS p
+WHERE ppr.project_id = p.id
+  AND NOT EXISTS (SELECT 1
+                    FROM team_profile_rel AS tpr
+                   WHERE tpr.team_id = p.team_id
+                     AND tpr.profile_id = ppr.profile_id);
+
+DELETE FROM team_project_profile_rel AS tppr
+WHERE NOT EXISTS (SELECT 1
+                    FROM team_profile_rel AS tpr
+                   WHERE tpr.team_id = tppr.team_id
+                     AND tpr.profile_id = tppr.profile_id);

backend/src/app/rpc/commands/projects.clj

@@ -41,8 +41,13 @@
           ppr.is_admin,
           ppr.can_edit
      from project_profile_rel as ppr
+    inner join project as p on (p.id = ppr.project_id)
     where ppr.project_id = ?
-      and ppr.profile_id = ?")
+      and ppr.profile_id = ?
+      and exists (select 1
+                    from team_profile_rel as tpr
+                   where tpr.team_id = p.team_id
+                     and tpr.profile_id = ppr.profile_id)")
 
 (defn- get-permissions
   [conn profile-id project-id]
@@ -143,7 +148,12 @@
     inner join team as t on (t.id = p2.team_id)
     where p2.id in (select project_id
                      from project_profile_rel as ppr
+                    inner join project as p on (p.id = ppr.project_id)
                     where ppr.profile_id = ?
+                      and exists (select 1
+                                    from team_profile_rel as tpr
+                                   where tpr.team_id = p.team_id
+                                     and tpr.profile_id = ppr.profile_id)
                       and (ppr.can_edit = true or
                            ppr.is_owner = true or
                            ppr.is_admin = true))

backend/src/app/rpc/commands/search.clj

@@ -32,6 +32,10 @@
       where ppr.profile_id = ?
         and p.team_id = ?
         and (p.deleted_at is null)
+        and exists (select 1
+                      from team_profile_rel as tpr
+                     where tpr.team_id = p.team_id
+                       and tpr.profile_id = ppr.profile_id)
         and (ppr.is_admin = true or
              ppr.is_owner = true or
              ppr.can_edit = true)

backend/src/app/rpc/commands/teams.clj

@@ -732,6 +732,37 @@
 
 ;; --- Mutation: Leave Team
 
+;; Remove every team-scoped relation of a profile that leaves (or is
+;; removed from) a team. File and project roles hang from the team
+;; membership: keeping them would let the former member keep reading
+;; and editing files through `sql:file-permissions`. Pins are personal
+;; state of the same team, so they go too. Other teams are untouched,
+;; and rejoining grants fresh roles. Runs inside the caller's
+;; transaction.
+(def ^:private sql:delete-team-file-rels
+  "DELETE FROM file_profile_rel AS fpr
+    USING file AS f
+    INNER JOIN project AS p ON (p.id = f.project_id)
+    WHERE fpr.file_id = f.id
+      AND p.team_id = ?
+      AND fpr.profile_id = ?")
+
+(def ^:private sql:delete-team-project-rels
+  "DELETE FROM project_profile_rel AS ppr
+    USING project AS p
+    WHERE ppr.project_id = p.id
+      AND p.team_id = ?
+      AND ppr.profile_id = ?")
+
+(defn- delete-team-rels
+  [conn profile-id team-id]
+  (db/exec! conn [sql:delete-team-file-rels team-id profile-id])
+  (db/exec! conn [sql:delete-team-project-rels team-id profile-id])
+  (db/delete! conn :team-project-profile-rel {:profile-id profile-id
+                                              :team-id team-id})
+  (db/delete! conn :team-profile-rel {:profile-id profile-id
+                                      :team-id team-id}))
+
 (defn leave-team
   [{:keys [::db/conn ::mbus/msgbus]} {:keys [profile-id id reassign-to]}]
   (let [perms   (get-permissions conn profile-id id)
@@ -782,9 +813,7 @@
                 :code :owner-cant-leave-team
                 :hint "releasing owner before leave"))
 
-    (db/delete! conn :team-profile-rel
-                {:profile-id profile-id
-                 :team-id id})
+    (delete-team-rels conn profile-id id)
 
     nil))
 
@@ -966,8 +995,7 @@
       (ex/raise :type :validation
                 :code :cant-remove-owner))
 
-    (db/delete! conn :team-profile-rel {:profile-id member-id
-                                        :team-id team-id})
+    (delete-team-rels conn member-id team-id)
 
     ;; A removed member that owns the organization of this team keeps
     ;; read-only access to it, so instead of kicking them out we degrade

backend/test/backend_tests/helpers.clj

@@ -299,9 +299,7 @@
   ([params] (create-project-role* *system* params))
   ([system {:keys [project-id profile-id role] :or {role :owner}}]
    (dm/with-open [conn (db/open system)]
-     (#'teams/create-project-role conn {:project-id project-id
-                                        :profile-id profile-id
-                                        :role role}))))
+     (#'teams/create-project-role conn profile-id project-id role))))
 
 (defn create-file-role*
   ([params] (create-file-role* *system* params))

backend/test/backend_tests/rpc_team_test.clj

@@ -1471,3 +1471,315 @@
       (t/is (not (th/success? out)))
       (t/is (th/ex-of-type? (:error out) :not-found))
       (t/is (th/ex-of-code? (:error out) :member-does-not-exist)))))
+
+(t/deftest leaver-loses-file-access
+  ;; GHSA-v9r9-h77c-55m2: leaving the team must revoke the file and
+  ;; project roles granted in that team.
+  (let [owner  (th/create-profile* 1 {:is-active true})
+        editor (th/create-profile* 2 {:is-active true})
+        team   (th/create-team* 1 {:profile-id (:id owner)})]
+
+    (th/create-team-role* {:team-id (:id team)
+                           :profile-id (:id editor)
+                           :role :editor})
+
+    (let [project (th/create-project* 1 {:profile-id (:id editor)
+                                         :team-id (:id team)})
+          file    (th/create-file* 1 {:profile-id (:id editor)
+                                      :project-id (:id project)})]
+
+      (t/testing "editor can rename and read summary before leaving"
+        (let [out (th/command! {::th/type :rename-file
+                                ::rpc/profile-id (:id editor)
+                                :id (:id file)
+                                :name "renamed"})]
+          (t/is (th/success? out)))
+        (let [out (th/command! {::th/type :get-file-summary
+                                ::rpc/profile-id (:id editor)
+                                :id (:id file)})]
+          (t/is (th/success? out))))
+
+      (t/testing "leave team"
+        (let [out (th/command! {::th/type :leave-team
+                                ::rpc/profile-id (:id editor)
+                                :id (:id team)})]
+          (t/is (th/success? out))))
+
+      (t/testing "renaming after leaving fails with not-found"
+        (let [out   (th/command! {::th/type :rename-file
+                                  ::rpc/profile-id (:id editor)
+                                  :id (:id file)
+                                  :name "renamed-again"})
+              error (:error out)]
+          (t/is (th/ex-info? error))
+          (t/is (th/ex-of-type? error :not-found))))
+
+      (t/testing "file summary after leaving fails with not-found"
+        (let [out   (th/command! {::th/type :get-file-summary
+                                  ::rpc/profile-id (:id editor)
+                                  :id (:id file)})
+              error (:error out)]
+          (t/is (th/ex-info? error))
+          (t/is (th/ex-of-type? error :not-found))))
+
+      (t/testing "get-file after leaving fails with not-found"
+        (let [out   (th/command! {::th/type :get-file
+                                  ::rpc/profile-id (:id editor)
+                                  :id (:id file)
+                                  :components-v2 true})
+              error (:error out)]
+          (t/is (th/ex-info? error))
+          (t/is (th/ex-of-type? error :not-found)))))))
+
+(t/deftest removed-member-loses-file-access
+  ;; Same as above through the expulsion path.
+  (let [owner  (th/create-profile* 1 {:is-active true})
+        editor (th/create-profile* 2 {:is-active true})
+        team   (th/create-team* 1 {:profile-id (:id owner)})]
+
+    (th/create-team-role* {:team-id (:id team)
+                           :profile-id (:id editor)
+                           :role :editor})
+
+    (let [project (th/create-project* 1 {:profile-id (:id editor)
+                                         :team-id (:id team)})
+          file    (th/create-file* 1 {:profile-id (:id editor)
+                                      :project-id (:id project)})]
+
+      (t/testing "remove member"
+        (let [out (th/command! {::th/type :delete-team-member
+                                ::rpc/profile-id (:id owner)
+                                :team-id (:id team)
+                                :member-id (:id editor)})]
+          (t/is (th/success? out))))
+
+      (t/testing "renaming after removal fails with not-found"
+        (let [out   (th/command! {::th/type :rename-file
+                                  ::rpc/profile-id (:id editor)
+                                  :id (:id file)
+                                  :name "renamed-again"})
+              error (:error out)]
+          (t/is (th/ex-info? error))
+          (t/is (th/ex-of-type? error :not-found))))
+
+      (t/testing "file summary after removal fails with not-found"
+        (let [out   (th/command! {::th/type :get-file-summary
+                                  ::rpc/profile-id (:id editor)
+                                  :id (:id file)})
+              error (:error out)]
+          (t/is (th/ex-info? error))
+          (t/is (th/ex-of-type? error :not-found)))))))
+
+(t/deftest orphan-file-role-without-team-membership-grants-nothing
+  ;; Defense in depth: a file role row without a live team membership
+  ;; (leftover from before the leave-team cleanup) grants no access.
+  (let [owner    (th/create-profile* 1 {:is-active true})
+        stranger (th/create-profile* 2 {:is-active true})
+        team     (th/create-team* 1 {:profile-id (:id owner)})
+        project  (th/create-project* 1 {:profile-id (:id owner)
+                                        :team-id (:id team)})
+        file     (th/create-file* 1 {:profile-id (:id owner)
+                                     :project-id (:id project)})]
+
+    ;; Plant an orphan file role for a profile that was never a member.
+    (th/create-file-role* {:file-id (:id file)
+                           :profile-id (:id stranger)
+                           :role :editor})
+
+    (t/testing "orphan file role cannot rename"
+      (let [out   (th/command! {::th/type :rename-file
+                                ::rpc/profile-id (:id stranger)
+                                :id (:id file)
+                                :name "renamed"})
+            error (:error out)]
+        (t/is (th/ex-info? error))
+        (t/is (th/ex-of-type? error :not-found))))
+
+    (t/testing "orphan file role cannot read summary"
+      (let [out   (th/command! {::th/type :get-file-summary
+                                ::rpc/profile-id (:id stranger)
+                                :id (:id file)})
+            error (:error out)]
+        (t/is (th/ex-info? error))
+        (t/is (th/ex-of-type? error :not-found))))))
+
+(t/deftest leaver-loses-project-access
+  ;; Same revocation at project level (covers sql:project-permissions).
+  (let [owner  (th/create-profile* 1 {:is-active true})
+        editor (th/create-profile* 2 {:is-active true})
+        team   (th/create-team* 1 {:profile-id (:id owner)})]
+
+    (th/create-team-role* {:team-id (:id team)
+                           :profile-id (:id editor)
+                           :role :editor})
+
+    (let [project (th/create-project* 1 {:profile-id (:id editor)
+                                         :team-id (:id team)})]
+
+      (t/testing "leave team"
+        (let [out (th/command! {::th/type :leave-team
+                                ::rpc/profile-id (:id editor)
+                                :id (:id team)})]
+          (t/is (th/success? out))))
+
+      (t/testing "get-project after leaving fails with not-found"
+        (let [out   (th/command! {::th/type :get-project
+                                  ::rpc/profile-id (:id editor)
+                                  :id (:id project)})
+              error (:error out)]
+          (t/is (th/ex-info? error))
+          (t/is (th/ex-of-type? error :not-found))))
+
+      (t/testing "rename-project after leaving fails with not-found"
+        (let [out   (th/command! {::th/type :rename-project
+                                  ::rpc/profile-id (:id editor)
+                                  :id (:id project)
+                                  :name "renamed"})
+              error (:error out)]
+          (t/is (th/ex-info? error))
+          (t/is (th/ex-of-type? error :not-found)))))))
+
+(t/deftest leaver-vanishes-from-search
+  ;; The old team's files are no longer searchable by the leaver.
+  (let [owner  (th/create-profile* 1 {:is-active true})
+        editor (th/create-profile* 2 {:is-active true})
+        team   (th/create-team* 1 {:profile-id (:id owner)})]
+
+    (th/create-team-role* {:team-id (:id team)
+                           :profile-id (:id editor)
+                           :role :editor})
+
+    (let [project (th/create-project* 1 {:profile-id (:id editor)
+                                         :team-id (:id team)})
+          file    (th/create-file* 1 {:profile-id (:id editor)
+                                      :project-id (:id project)
+                                      :name "searchable-file"})]
+
+      (t/testing "member finds the file before leaving"
+        (let [out (th/command! {::th/type :search-files
+                                ::rpc/profile-id (:id editor)
+                                :team-id (:id team)
+                                :search-term "searchable"})]
+          (t/is (th/success? out))
+          (t/is (= (:id file) (-> out :result first :id)))))
+
+      (t/testing "leave team"
+        (let [out (th/command! {::th/type :leave-team
+                                ::rpc/profile-id (:id editor)
+                                :id (:id team)})]
+          (t/is (th/success? out))))
+
+      (t/testing "search on the old team fails with not-found"
+        (let [out   (th/command! {::th/type :search-files
+                                  ::rpc/profile-id (:id editor)
+                                  :team-id (:id team)
+                                  :search-term "searchable"})
+              error (:error out)]
+          (t/is (th/ex-info? error))
+          (t/is (th/ex-of-type? error :not-found)))))))
+
+(t/deftest pins-are-cleaned-on-leave
+  ;; The project pin (team-project-profile-rel) goes with the leaver.
+  (let [owner  (th/create-profile* 1 {:is-active true})
+        editor (th/create-profile* 2 {:is-active true})
+        team   (th/create-team* 1 {:profile-id (:id owner)})]
+
+    (th/create-team-role* {:team-id (:id team)
+                           :profile-id (:id editor)
+                           :role :editor})
+
+    (let [project (th/create-project* 1 {:profile-id (:id editor)
+                                         :team-id (:id team)})]
+
+      (t/testing "pin the project"
+        (let [out (th/command! {::th/type :update-project-pin
+                                ::rpc/profile-id (:id editor)
+                                :id (:id project)
+                                :team-id (:id team)
+                                :is-pinned true})]
+          (t/is (th/success? out)))
+        (t/is (= 1 (count (th/db-query :team-project-profile-rel
+                                       {:team-id (:id team)
+                                        :profile-id (:id editor)})))))
+
+      (t/testing "leave team removes the pin row"
+        (let [out (th/command! {::th/type :leave-team
+                                ::rpc/profile-id (:id editor)
+                                :id (:id team)})]
+          (t/is (th/success? out)))
+        (t/is (= 0 (count (th/db-query :team-project-profile-rel
+                                       {:team-id (:id team)
+                                        :profile-id (:id editor)}))))))))
+
+(t/deftest leaving-one-team-keeps-other-team-access
+  ;; Deletes are scoped by team: leaving A keeps B's files usable.
+  (let [owner   (th/create-profile* 1 {:is-active true})
+        editor  (th/create-profile* 2 {:is-active true})
+        team-a  (th/create-team* 1 {:profile-id (:id owner)})
+        team-b  (th/create-team* 2 {:profile-id (:id owner)})]
+
+    (th/create-team-role* {:team-id (:id team-a)
+                           :profile-id (:id editor)
+                           :role :editor})
+    (th/create-team-role* {:team-id (:id team-b)
+                           :profile-id (:id editor)
+                           :role :editor})
+
+    (let [project-a (th/create-project* 1 {:profile-id (:id editor)
+                                           :team-id (:id team-a)})
+          file-a    (th/create-file* 1 {:profile-id (:id editor)
+                                        :project-id (:id project-a)})
+          project-b (th/create-project* 2 {:profile-id (:id editor)
+                                           :team-id (:id team-b)})
+          file-b    (th/create-file* 2 {:profile-id (:id editor)
+                                        :project-id (:id project-b)})]
+
+      (t/testing "leave team A"
+        (let [out (th/command! {::th/type :leave-team
+                                ::rpc/profile-id (:id editor)
+                                :id (:id team-a)})]
+          (t/is (th/success? out))))
+
+      (t/testing "team A file is gone"
+        (let [out   (th/command! {::th/type :rename-file
+                                  ::rpc/profile-id (:id editor)
+                                  :id (:id file-a)
+                                  :name "renamed"})
+              error (:error out)]
+          (t/is (th/ex-info? error))
+          (t/is (th/ex-of-type? error :not-found))))
+
+      (t/testing "team B file still works"
+        (let [out (th/command! {::th/type :rename-file
+                                ::rpc/profile-id (:id editor)
+                                :id (:id file-b)
+                                :name "renamed"})]
+          (t/is (th/success? out)))))))
+
+(t/deftest orphan-project-role-grants-nothing-and-lists-nothing
+  ;; Mirror of the file orphan test at project level, including the
+  ;; get-all-projects listing (covers the F1 guard).
+  (let [owner    (th/create-profile* 1 {:is-active true})
+        stranger (th/create-profile* 2 {:is-active true})
+        team     (th/create-team* 1 {:profile-id (:id owner)})
+        project  (th/create-project* 1 {:profile-id (:id owner)
+                                        :team-id (:id team)})]
+
+    ;; Plant an orphan project role for a profile that was never a member.
+    (th/create-project-role* {:project-id (:id project)
+                              :profile-id (:id stranger)
+                              :role :editor})
+
+    (t/testing "orphan project role cannot read the project"
+      (let [out   (th/command! {::th/type :get-project
+                                ::rpc/profile-id (:id stranger)
+                                :id (:id project)})
+            error (:error out)]
+        (t/is (th/ex-info? error))
+        (t/is (th/ex-of-type? error :not-found))))
+
+    (t/testing "orphan project is not listed"
+      (let [out (th/command! {::th/type :get-all-projects
+                              ::rpc/profile-id (:id stranger)})]
+        (t/is (th/success? out))
+        (t/is (nil? (some #(= (:id project) (:id %)) (:result out))))))))