Build a scoped staff member's library query once per request, not once per row

#1698 moved the library boundary into FileCommentPolicy, where it
belongs, and said plainly what that cost: the moderation screen went
from 65 queries to 465 for a client-scoped moderator with five assigned
clients. Measured here, those numbers are exactly right.

The cost is not in asking. It is that StaffLibraryScope::files() rebuilds
its query every time, and building one runs four immediate lookups per
assigned client — the client's group ids, the same ids again inside
Folder::sharedFolderIds(), that method's own assignment lookup, and the
shared-folder get() in Folder::scopeVisibleToClient(). None of them
depend on the query being built. Gate resolves a fresh policy for every
check, so a listing paid for all of it once per row.

The built query is now memoised per user and handed back as a clone,
since every caller adds to it, and the scope is registered as `scoped`
rather than transient so the memo survives a request. Scoped rather than
a singleton on purpose: a long-lived queue worker keeps singletons
between jobs, and a library query built from one job's data has no
business answering the next one's question.

That is 465 queries down to 60 on the same page — below the 65 it cost
before #1698, because the memo also helps the callers that were already
asking repeatedly. FileVersions::sharedAudience(), which runs the same
helper twice per candidate while resolving notification recipients, gets
it for free.

So the answer to the question #1698 left open is neither of the two it
offered. can_delete stays a real question asked of the policy; nothing
restates the boundary; and the page is faster than it was before the
fix. Three tests: one user's query never answers another's, one caller's
constraints never follow the next, and the moderation screen does not
ask once per row.

CHANGELOG.md

@@ -171,6 +171,18 @@ a version is cut.
   (found, diagnosed and fixed by [@denkfabrik-li](https://github.com/denkfabrik-li) in
   [#1687](https://github.com/projectsend/projectsend/pull/1687))
 
+- **Comment moderation now stops at the same boundary everything else does.** A staff role can be
+  limited to its own assigned clients, and everything in the library respects that — listings,
+  downloads, file details, and the moderation queue itself. Deleting or approving a single comment
+  did not. Someone with a client-limited role who also held the comment moderation permission could
+  remove any comment on the installation by its id, including conversations belonging to clients
+  they were not assigned to, on files they could not open. No role that ships with ProjectSend
+  combines those two things, so this needed a custom role to reach; if you have built one, it is
+  worth updating for. The boundary now lives in the rule itself rather than being restated by each
+  screen, which is how the gap opened in the first place.
+  (found, diagnosed and fixed by [@denkfabrik-li](https://github.com/denkfabrik-li) in
+  [#1698](https://github.com/projectsend/projectsend/pull/1698))
+
 ## 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

@@ -26,10 +26,37 @@ use Illuminate\Database\Eloquent\Builder;
  */
 class StaffLibraryScope
 {
+    /**
+     * Built queries, by user id. Building one is not free: it walks the
+     * assigned clients and File::scopeVisibleToClient runs four immediate
+     * lookups for each of them, none of which depend on the query being
+     * built. Callers ask over and over — the policies ask once per row on
+     * a listing, and Gate resolves a fresh policy for every check — so the
+     * same handful of lookups were being repeated per row.
+     *
+     * A clone goes back rather than the query itself, since every caller
+     * adds to it. Registered with the container as `scoped`, so the memo
+     * lasts a request and is dropped between queue jobs.
+     *
+     * @var array<int, Builder<File>>
+     */
+    private array $files = [];
+
+    /** @var array<int, Builder<Folder>> */
+    private array $folders = [];
+
     /**
      * @return Builder<File>
      */
     public function files(User $user): Builder
+    {
+        return clone ($this->files[$user->id] ??= $this->buildFiles($user));
+    }
+
+    /**
+     * @return Builder<File>
+     */
+    private function buildFiles(User $user): Builder
     {
         $query = File::query();
 
@@ -53,6 +80,14 @@ class StaffLibraryScope
      * @return Builder<Folder>
      */
     public function folders(User $user): Builder
+    {
+        return clone ($this->folders[$user->id] ??= $this->buildFolders($user));
+    }
+
+    /**
+     * @return Builder<Folder>
+     */
+    private function buildFolders(User $user): Builder
     {
         $query = Folder::query();
 

app/Modules/Files/FilesServiceProvider.php

@@ -4,6 +4,7 @@ declare(strict_types=1);
 
 namespace App\Modules\Files;
 
+use App\Modules\Files\Access\StaffLibraryScope;
 use App\Modules\Files\Models\File;
 use App\Modules\Files\Models\Folder;
 use App\Modules\Files\Notifications\FileShareDigestNotification;
@@ -20,6 +21,17 @@ use Illuminate\Support\ServiceProvider;
 
 class FilesServiceProvider extends ServiceProvider
 {
+    public function register(): void
+    {
+        // Scoped rather than transient: the library query it builds costs
+        // several lookups per assigned client, and the policies ask for it
+        // once per row on a listing — Gate resolves a fresh policy for
+        // every check, so without this the instance memo would never be
+        // reached twice. Scoped rather than a singleton so a long-lived
+        // queue worker starts each job with an empty memo.
+        $this->app->scoped(StaffLibraryScope::class);
+    }
+
     public function boot(): void
     {
         Gate::policy(File::class, FilePolicy::class);

tests/Feature/Files/StaffLibraryScopeReuseTest.php

@@ -0,0 +1,108 @@
+<?php
+
+declare(strict_types=1);
+
+use App\Models\User;
+use App\Modules\Comments\Models\FileComment;
+use App\Modules\Files\Access\StaffLibraryScope;
+use App\Modules\Files\Models\File;
+use App\Modules\Identity\Models\Role;
+use App\Modules\Identity\Models\RolePermission;
+use App\Modules\Platform\Settings\Setting;
+use App\Modules\Platform\Settings\Settings;
+use Illuminate\Support\Facades\DB;
+
+/**
+ * The library query is memoised per user, because building one costs
+ * several lookups per assigned client and the policies ask for it once
+ * per row. These pin the two things that makes fragile — one user's
+ * query reaching another, and a caller's own constraints leaking into
+ * everybody else's copy — plus the count that made it worth doing.
+ */
+beforeEach(function () {
+    $this->admin = User::factory()->create();
+
+    $settings = app(Settings::class);
+    $settings->set(Setting::CommentsScope, 'all');
+    $settings->set(Setting::CommentsAuthors, 'everyone');
+});
+
+function scopedModeratorWithLibrary(int $clients, int $filesEach): User
+{
+    $role = Role::query()->create(['name' => 'Scoped moderator '.Role::query()->count(), 'client_scoped' => true]);
+    RolePermission::query()->insert([
+        ['role_id' => $role->id, 'permission' => 'moderate_comments'],
+        ['role_id' => $role->id, 'permission' => 'upload'],
+    ]);
+
+    $manager = User::factory()->create(['role_id' => $role->id]);
+
+    for ($i = 0; $i < $clients; $i++) {
+        $client = User::factory()->client()->create();
+        $manager->assignedClients()->attach($client->id);
+
+        for ($f = 0; $f < $filesEach; $f++) {
+            $file = File::factory()->public()->create(['uploaded_by' => $manager->id]);
+            shareFileWith($file, $client);
+            FileComment::factory()->for($file)->fromGuest()->pending()->create();
+        }
+    }
+
+    return $manager;
+}
+
+test('what one scoped user may see is never handed to another', function () {
+    $scope = app(StaffLibraryScope::class);
+
+    $mine = User::factory()->client()->create();
+    $theirs = User::factory()->client()->create();
+
+    $me = scopedModeratorWithLibrary(0, 0);
+    $me->assignedClients()->attach($mine->id);
+    $them = scopedModeratorWithLibrary(0, 0);
+    $them->assignedClients()->attach($theirs->id);
+
+    $myFile = File::factory()->public()->create(['uploaded_by' => $this->admin->id]);
+    shareFileWith($myFile, $mine);
+
+    expect($scope->files($me)->whereKey($myFile->id)->exists())->toBeTrue()
+        ->and($scope->files($them)->whereKey($myFile->id)->exists())->toBeFalse()
+        // Asked again, in the other order: a memo that answered from the
+        // wrong user's query would only show up on the second call.
+        ->and($scope->files($them)->whereKey($myFile->id)->exists())->toBeFalse()
+        ->and($scope->files($me)->whereKey($myFile->id)->exists())->toBeTrue();
+});
+
+test('what one caller adds to the query does not follow the next one', function () {
+    $scope = app(StaffLibraryScope::class);
+
+    $client = User::factory()->client()->create();
+    $manager = scopedModeratorWithLibrary(0, 0);
+    $manager->assignedClients()->attach($client->id);
+
+    $first = File::factory()->public()->create(['uploaded_by' => $this->admin->id]);
+    $second = File::factory()->public()->create(['uploaded_by' => $this->admin->id]);
+    shareFileWith($first, $client);
+    shareFileWith($second, $client);
+
+    // Narrow one copy to a single file, then ask again from scratch.
+    expect($scope->files($manager)->whereKey($first->id)->count())->toBe(1)
+        ->and($scope->files($manager)->count())->toBe(2);
+});
+
+test('the moderation screen does not ask the library once per row', function () {
+    $manager = scopedModeratorWithLibrary(clients: 5, filesEach: 5);
+
+    $queries = 0;
+    DB::listen(function () use (&$queries): void {
+        $queries++;
+    });
+
+    $this->actingAs($manager)->get('/comments')->assertOk();
+
+    // Twenty-five rows, five assigned clients. Built once per request the
+    // page costs about sixty queries; built per row it was 465. The bound
+    // is deliberately loose — it is here to catch the shape coming back,
+    // not to pin an exact number.
+    expect($queries)->toBeLessThan(150);
+});