Skip to content

Commit 229a0fd

Browse files
rtibblesbotclaude
andcommitted
fix(security): replace str(e) in viewsets/contentnode.py, channel.py, user.py error lists with static messages (pattern 3)
Fixes Pattern 3 across all remaining viewsets. Also chains ValidationError with from e in validate_completion_criteria, and normalises the _handle_relationship_changes error value to a list for consistency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f8310f8 commit 229a0fd

3 files changed

Lines changed: 25 additions & 16 deletions

File tree

contentcuration/contentcuration/viewsets/channel.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,7 @@ def publish_from_changes(self, changes):
561561
)
562562
except Exception as e:
563563
log_sync_exception(e, user=self.request.user, change=publish)
564-
publish["errors"] = [str(e)]
564+
publish["errors"] = ["Internal server error"]
565565
errors.append(publish)
566566
return errors
567567

@@ -651,7 +651,7 @@ def publish_next_from_changes(self, changes):
651651
)
652652
except Exception as e:
653653
log_sync_exception(e, user=self.request.user, change=publish)
654-
publish["errors"] = [str(e)]
654+
publish["errors"] = ["Internal server error"]
655655
errors.append(publish)
656656
return errors
657657

@@ -709,7 +709,7 @@ def sync_from_changes(self, changes):
709709
)
710710
except Exception as e:
711711
log_sync_exception(e, user=self.request.user, change=sync)
712-
sync["errors"] = [str(e)]
712+
sync["errors"] = ["Internal server error"]
713713
errors.append(sync)
714714
return errors
715715

@@ -760,7 +760,7 @@ def deploy_from_changes(self, changes):
760760
self.deploy(self.request.user, deploy["key"])
761761
except Exception as e:
762762
log_sync_exception(e, user=self.request.user, change=deploy)
763-
deploy["errors"] = [str(e)]
763+
deploy["errors"] = ["Internal server error"]
764764
errors.append(deploy)
765765
return errors
766766

@@ -814,7 +814,7 @@ def add_to_community_library_from_changes(self, changes):
814814
)
815815
except Exception as e:
816816
log_sync_exception(e, user=self.request.user, change=change)
817-
change["errors"] = [str(e)]
817+
change["errors"] = ["Internal server error"]
818818
errors.append(change)
819819
return errors
820820

contentcuration/contentcuration/viewsets/contentnode.py

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import json
2+
import logging
23
from functools import partial
34
from functools import reduce
45

@@ -81,6 +82,8 @@
8182
from contentcuration.viewsets.sync.utils import generate_update_event
8283
from contentcuration.viewsets.sync.utils import log_sync_exception
8384

85+
logger = logging.getLogger(__name__)
86+
8487
channel_query = Channel.objects.filter(main_tree__tree_id=OuterRef("tree_id"))
8588

8689

@@ -477,7 +480,7 @@ def _check_completion_criteria(self, kind, complete, validated_data):
477480
completion_criteria, kind
478481
)
479482
except DjangoValidationError as e:
480-
raise ValidationError(e)
483+
raise ValidationError("Invalid completion criteria") from e
481484

482485
def _ensure_complete(self, instance):
483486
"""
@@ -713,9 +716,10 @@ def _handle_relationship_changes(self, changes):
713716
# In Django 2.2 add ignore_conflicts to make this fool proof
714717
try:
715718
self._execute_changes(change_type, data)
716-
except IntegrityError as e:
719+
except IntegrityError:
720+
logger.exception("_handle_relationship_changes IntegrityError")
717721
for change in valid_changes:
718-
change.update({"errors": str(e)})
722+
change.update({"errors": ["Internal server error"]})
719723
errors.append(change)
720724

721725
return errors or None
@@ -1067,8 +1071,7 @@ def move(self, pk, target=None, position=None):
10671071
try:
10681072
contentnode = self.get_edit_queryset().get(pk=pk)
10691073
except ContentNode.DoesNotExist:
1070-
error = ValidationError("Specified node does not exist")
1071-
return str(error)
1074+
return "Specified node does not exist"
10721075

10731076
try:
10741077
target, position = self.validate_targeting_args(target, position)
@@ -1079,8 +1082,9 @@ def move(self, pk, target=None, position=None):
10791082
)
10801083

10811084
return None
1082-
except ValidationError as e:
1083-
return str(e)
1085+
except ValidationError:
1086+
logger.exception("move validation error for pk=%s", pk)
1087+
return "Validation error"
10841088

10851089
def copy_from_changes(self, changes):
10861090
errors = []
@@ -1091,7 +1095,7 @@ def copy_from_changes(self, changes):
10911095
self.copy(copy["key"], **copy)
10921096
except Exception as e:
10931097
log_sync_exception(e, user=self.request.user, change=copy)
1094-
copy["errors"] = [str(e)]
1098+
copy["errors"] = ["Internal server error"]
10951099
errors.append(copy)
10961100
failed_copy_node = self.get_queryset().filter(pk=copy["key"]).first()
10971101
if failed_copy_node is not None:
@@ -1196,6 +1200,6 @@ def update_descendants_from_changes(self, changes):
11961200
errors += change_errors
11971201
except Exception as e:
11981202
log_sync_exception(e, user=self.request.user, change=change)
1199-
change["errors"] = [str(e)]
1203+
change["errors"] = ["Internal server error"]
12001204
errors.append(change)
12011205
return errors

contentcuration/contentcuration/viewsets/user.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
from functools import reduce
23

34
from django.db import IntegrityError
@@ -45,6 +46,9 @@
4546
from contentcuration.viewsets.sync.constants import VIEWER_M2M
4647

4748

49+
logger = logging.getLogger(__name__)
50+
51+
4852
class IsAdminUser(BasePermission):
4953
"""
5054
Our custom permission to check admin authorization.
@@ -284,9 +288,10 @@ def _handle_relationship_changes(self, changes):
284288
# In Django 2.2 add ignore_conflicts to make this fool proof
285289
try:
286290
self._execute_changes(table, change_type, data)
287-
except IntegrityError as e:
291+
except IntegrityError:
292+
logger.exception("_handle_relationship_changes IntegrityError")
288293
for change in valid_changes:
289-
change.update({"errors": str(e)})
294+
change.update({"errors": ["Internal server error"]})
290295
errors.append(change)
291296

292297
for change in invalid_changes:

0 commit comments

Comments
 (0)