Fix: bulk-edit rotate/split/owner validators crash on wrong-typed input

BulkEditSerializer's hand-parsed parameter validators only caught the
exception types their happy-path callers happened to raise, not what
untrusted input can actually produce:

- _validate_parameters_rotate: float(None) raises TypeError, only
  ValueError was caught.
- _validate_parameters_split: parameters["pages"].split(",") assumed a
  string; a null value raised AttributeError.
- _validate_owner: User.objects.get(pk=owner) raises DoesNotExist for a
  nonexistent id with no try/except at all (the `if ownerUser is None`
  check below it was dead code, since .get() never returns None).

All three surfaced as an uncaught 500 instead of a 400.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Trenton Holmes
2026-09-13 14:58:27 -07:00
co-authored by Claude Sonnet 5
parent 05b7697c35
commit f1b21628a9
2 changed files with 80 additions and 4 deletions
+74
View File
@@ -1165,6 +1165,33 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
self.assertIn(b"set_permissions not specified", response.content)
m.assert_not_called()
@mock.patch("documents.serialisers.bulk_edit.set_permissions")
def test_set_permissions_rejects_nonexistent_owner(self, m) -> None:
"""
BulkEditSerializer._validate_owner called User.objects.get(pk=owner)
with no try/except, so a syntactically valid but nonexistent user
id raised an uncaught User.DoesNotExist instead of a clean 400.
"""
self.setup_mock(m, "set_permissions")
response = self.client.post(
"/api/documents/bulk_edit/",
json.dumps(
{
"documents": [self.doc2.id],
"method": "set_permissions",
"parameters": {
"set_permissions": {"view": {"users": [self.user.id]}},
"owner": 999999,
},
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
m.assert_not_called()
@mock.patch("documents.serialisers.bulk_edit.set_permissions")
def test_set_permissions_merge(self, m) -> None:
self.setup_mock(m, "set_permissions")
@@ -1438,6 +1465,53 @@ class TestBulkEditAPI(DirectoriesMixin, APITestCase):
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
m.assert_not_called()
@mock.patch("documents.serialisers.bulk_edit.rotate")
def test_bulk_edit_rotate_rejects_null_degrees(self, m) -> None:
"""
BulkEditSerializer._validate_parameters_rotate's
`float(parameters["degrees"])` raised an uncaught TypeError for
None (only ValueError was caught), reachable via the legacy
generic /api/documents/bulk_edit/ method="rotate" path (the
dedicated /api/documents/rotate/ endpoint isn't affected, its
`degrees` field is a typed IntegerField).
"""
self.setup_mock(m, "rotate")
response = self.client.post(
"/api/documents/bulk_edit/",
json.dumps(
{
"documents": [self.doc2.id],
"method": "rotate",
"parameters": {"degrees": None},
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
m.assert_not_called()
@mock.patch("documents.serialisers.bulk_edit.split")
def test_bulk_edit_split_rejects_null_pages(self, m) -> None:
"""
BulkEditSerializer._validate_parameters_split called
parameters["pages"].split(",") with no type check, so a null
value raised an uncaught AttributeError instead of a clean 400.
"""
self.setup_mock(m, "split")
response = self.client.post(
"/api/documents/bulk_edit/",
json.dumps(
{
"documents": [self.doc2.id],
"method": "split",
"parameters": {"pages": None},
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
m.assert_not_called()
@mock.patch("documents.views.bulk_edit.rotate")
def test_rotate_insufficient_permissions(self, m) -> None:
self.doc1.owner = User.objects.get(username="temp_admin")