stash

backend/files/media_urls.py

@@ -28,8 +28,8 @@ def _accessible_files(request):
     # friends-or-self with whatever references it (item, profile picture), a member of the group
     # that owns the item it's attached to, or it's their own staged photo.
     return File.objects.filter(
-        Q(connected_items__owner__in=request.user.friends_or_self()) |
-        Q(connected_items__owner_group__in=request.user.member_of_groups.all()) |
+        Q(connected_items__owner__in=request.user.friends_or_self(), connected_items__is_deleted=False) |
+        Q(connected_items__owner_group__in=request.user.member_of_groups.all(), connected_items__is_deleted=False) |
         Q(profile_picture_users__in=request.user.friends_or_self()) |
         Q(staged_by_workflows__owner__in=request.user.user.all())
     ).distinct()

backend/files/tests.py

@@ -187,6 +187,16 @@ class MediaUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, Toolsh
         self.assertEqual(reply.status_code, 404)
         self.assertTrue('X-Accel-Redirect' not in reply.headers)
 
+    def test_file_url_only_connected_via_deleted_item(self):
+        # test_file2 is only reachable through item1; soft-deleting it doesn't sever the files
+        # M2M row, so this would regress to serving test_file2 as if item1 were still live if
+        # _accessible_files ever stops excluding soft-deleted items again.
+        self.f['item1'].delete()
+        reply = client.get(
+            f"/media/{self.f['hash2'][:2]}/{self.f['hash2'][2:4]}/{self.f['hash2'][4:6]}/{self.f['hash2'][6:]}",
+            self.f['local_user1'])
+        self.assertEqual(reply.status_code, 404)
+
     @override_settings(SERVE_X_ACCEL_REDIRECT=True)
     def test_profile_picture_url(self):
         self.f['local_user1'].profile_picture = self.f['test_file3']

backend/toolshed/api/files.py

@@ -33,7 +33,8 @@ def list_all_files(request, format=None):
     # request.user is a ToolshedUser here; reach group membership via public_identity.
     files = File.objects.select_related().filter(
         Q(connected_items__owner=request.user) |
-        Q(connected_items__owner_group__in=request.user.public_identity.member_of_groups.all())
+        Q(connected_items__owner_group__in=request.user.public_identity.member_of_groups.all()),
+        connected_items__is_deleted=False
     ).distinct()
     return Response(FileSerializer(files, many=True).data)
 

backend/toolshed/tests/test_files.py

@@ -39,6 +39,17 @@ class FileApiTestCase(UserTestMixin, FilesTestMixin, InventoryTestMixin, Toolshe
         self.assertEqual(response.json()[1]['name'],
                          f"/media/{self.f['hash2'][:2]}/{self.f['hash2'][2:4]}/{self.f['hash2'][4:6]}/{self.f['hash2'][6:]}")
 
+    def test_list_all_files_excludes_files_only_connected_via_deleted_item(self):
+        # test_file2 is only reachable through item1; test_file1 is also reachable through item2,
+        # which stays live. Soft-deleting item1 doesn't sever its files M2M rows, so this would
+        # regress to listing test_file2 as if it were still owned if the join-based filter ever
+        # stops excluding soft-deleted items again.
+        self.f['item1'].delete()
+        response = client.get(f"/api/v1/files/", self.f['local_user1'])
+        self.assertEqual(response.status_code, 200)
+        hashes = [f['hash'] for f in response.json()]
+        self.assertEqual(hashes, [self.f['hash1']])
+
     def test_files(self):
         response = client.get(f"/api/v1/item_files/{self.f['item1'].id}/", self.f['local_user1'])
         self.assertEqual(response.status_code, 200)