Stop a deleted file locking a scoped staff member out of a group for good

#1701 closed a real hole: group membership decides what a client reaches,
and through File::scopeVisibleToClient it decides what the staff member
holding that client reaches, so `edit_groups` alone was never a boundary.
The predicate it added asks whether everything shared with a group is
already inside the actor's library.

It asked by counting: pluck the group's assignment rows, count how many
of those ids the library query returns, and require the two to match. An
assignment row outlives the thing it points at — nothing clears them when
a file or folder is deleted — while files() and folders() exclude trashed
rows by construction. So one deleted file left a count that could never
balance again, and the group closed permanently: the scoped staff member
could no longer add their own client to it, or remove anybody from it,
with a 403 and nothing to explain it. Every group accumulates dead
assignments over time, so groups would have gone quiet one at a time.

Asked the other way round — is there anything live, shared with this
group, that is outside my library — the dead rows drop out by
construction, because the query starts from File/Folder rather than from
the assignment. That is also the truer question: a deleted file is not
reach, since nobody can reach it.

Three tests. A group stays usable after a file shared with it is deleted,
including removing a member; the same for a deleted folder assignment;
and the half that must not soften — a live file still out of reach is
still refused, deleted siblings or not.

CHANGELOG.md

@@ -201,6 +201,20 @@ a version is cut.
   (found, diagnosed and fixed by [@denkfabrik-li](https://github.com/denkfabrik-li) in
   [#1683](https://github.com/projectsend/projectsend/pull/1683))
 
+- **Group membership now respects a limited role's boundary.** A staff role can be limited to its
+  own assigned clients. Adding somebody to a group, or taking them out, checked only that the person
+  held the "edit groups" permission — not that the group was any of their business. Because joining a
+  group hands the new member everything shared with it, someone with a limited role could put one of
+  their own clients into any group on the installation and, through that client, reach files they
+  had been refused a moment earlier. Approving or denying a membership request was the same write
+  through a second door, and the requests screen listed every pending request by name and email,
+  including clients outside the viewer's roster. All of it is now held to the same boundary the rest
+  of the library uses, and the sidebar count agrees with the screen behind it. No role that ships
+  with ProjectSend combines the two permissions this needed, so reaching it took a custom role.
+  Nothing changes for an administrator or any unrestricted role.
+  (found, diagnosed and fixed by [@denkfabrik-li](https://github.com/denkfabrik-li) in
+  [#1701](https://github.com/projectsend/projectsend/pull/1701))
+
 ## 2.1.0 — 18 August 2026
 
 Updating, mostly. ProjectSend now tells you when there is a new version, ends an update somewhere

app/Modules/Files/Access/StaffLibraryScope.php

@@ -219,6 +219,18 @@ class StaffLibraryScope
      * Whether everything shared with this group is already inside the
      * user's library — files assigned to it, and the folders whose
      * subtrees it can browse.
+     *
+     * Asked as "is anything shared with this group outside my library",
+     * rather than by counting assignment rows against library rows. An
+     * assignment outlives the thing it points at: nothing clears these
+     * rows when a file or folder is deleted, and a deleted one can never
+     * appear in files()/folders(), which exclude trashed rows. Counting
+     * therefore never balanced again, and the group became permanently
+     * unmanageable for a scoped staff member — including for their own
+     * clients, and including removing somebody. Starting from the live
+     * row rather than from the assignment ignores the dead ones by
+     * construction, which is also the right answer: a deleted file is
+     * not reach, because nobody can reach it.
      */
     private function groupReachesNoFurther(User $user, Group $group): bool
     {
@@ -228,19 +240,24 @@ class StaffLibraryScope
 
         $morph = $group->getMorphClass();
 
-        $fileIds = FileAssignment::query()
-            ->where('assignable_type', $morph)->where('assignable_id', $group->id)
-            ->pluck('file_id')->unique();
+        $assignedFiles = FileAssignment::query()->select('file_id')
+            ->where('assignable_type', $morph)->where('assignable_id', $group->id);
 
-        if ($fileIds->isNotEmpty() && $this->files($user)->whereIn('id', $fileIds)->count() !== $fileIds->count()) {
+        $outside = File::query()
+            ->whereIn('id', $assignedFiles)
+            ->whereNotIn('id', $this->files($user)->select('id'))
+            ->exists();
+
+        if ($outside) {
             return false;
         }
 
-        $folderIds = FolderAssignment::query()
-            ->where('assignable_type', $morph)->where('assignable_id', $group->id)
-            ->pluck('folder_id')->unique();
+        $assignedFolders = FolderAssignment::query()->select('folder_id')
+            ->where('assignable_type', $morph)->where('assignable_id', $group->id);
 
-        return $folderIds->isEmpty()
-            || $this->folders($user)->whereIn('id', $folderIds)->count() === $folderIds->count();
+        return ! Folder::query()
+            ->whereIn('id', $assignedFolders)
+            ->whereNotIn('id', $this->folders($user)->select('id'))
+            ->exists();
     }
 }

tests/Feature/Groups/GroupMembershipScopeTest.php

@@ -245,3 +245,80 @@ test('a folder shared with a group counts as reach too', function () {
 
     expect($bare->members()->count())->toBe(0);
 });
+
+// An assignment row outlives the file it points at — nothing clears them
+// on delete — and a trashed file can never appear in files(). Asking
+// "is anything outside my library" from the live row rather than counting
+// assignment rows is what keeps a group usable after somebody deletes a
+// file that was once shared with it.
+test('a group is not locked shut by a file that has since been deleted', function () {
+    $group = Group::query()->create(['name' => 'Newsletter', 'slug' => 'newsletter', 'public' => false]);
+    $group->members()->syncWithoutDetaching([$this->mine->id]);
+
+    // Shared with the group, and reachable by this rep because their own
+    // client is a member — so the group is theirs to manage.
+    $file = uploadNamedFile($this->admin, 'seasonal-offer');
+    shareFileWithGroup($file, $group);
+
+    $second = User::factory()->client()->create(['name' => 'Also Mine']);
+    $this->rep->assignedClients()->attach($second->id);
+
+    $this->actingAs($this->rep)
+        ->post("/groups/{$group->id}/members", ['user_id' => $second->id])
+        ->assertRedirect();
+
+    // The uploader deletes it. The assignment row stays behind.
+    $file->delete();
+
+    $third = User::factory()->client()->create(['name' => 'Mine Too']);
+    $this->rep->assignedClients()->attach($third->id);
+
+    $this->actingAs($this->rep)
+        ->post("/groups/{$group->id}/members", ['user_id' => $third->id])
+        ->assertRedirect();
+
+    expect($group->members()->count())->toBe(3);
+
+    // And taking somebody out again still works, which the count form
+    // also blocked.
+    $this->actingAs($this->rep)
+        ->delete("/groups/{$group->id}/members/{$third->id}")
+        ->assertRedirect();
+
+    expect($group->members()->count())->toBe(2);
+});
+
+test('a deleted folder assignment does not lock a group either', function () {
+    $group = Group::query()->create(['name' => 'Bulletin', 'slug' => 'bulletin', 'public' => false]);
+    $group->members()->syncWithoutDetaching([$this->mine->id]);
+
+    $folder = app(FolderService::class)->create('Seasonal', null);
+    FolderAssignment::query()->create([
+        'folder_id' => $folder->id,
+        'assignable_type' => $group->getMorphClass(),
+        'assignable_id' => $group->id,
+    ]);
+
+    $folder->delete();
+
+    $second = User::factory()->client()->create(['name' => 'Second']);
+    $this->rep->assignedClients()->attach($second->id);
+
+    $this->actingAs($this->rep)
+        ->post("/groups/{$group->id}/members", ['user_id' => $second->id])
+        ->assertRedirect();
+
+    expect($group->members()->count())->toBe(2);
+});
+
+// The half that must not soften: a live file outside the library is still
+// reach, deleted siblings or not.
+test('a deleted file does not excuse a live one that is still out of reach', function () {
+    $dead = uploadNamedFile($this->admin, 'was-shared');
+    shareFileWithGroup($dead, $this->strangerGroup);
+    $dead->delete();
+
+    $this->actingAs($this->rep)
+        ->post("/groups/{$this->strangerGroup->id}/members", ['user_id' => $this->mine->id])
+        ->assertForbidden();
+});