From 354edc81f9da6328647afd78e1c32344ec60d468 Mon Sep 17 00:00:00 2001 From: Klaas van Schelven Date: Tue, 29 Jul 2025 22:23:37 +0200 Subject: [PATCH] Use django.utils._os.safe_join to construct paths Even though '_os' suggests a private interface, this is likely to be stable (for our purposes, i.e. may get more secure); if it ever isn't our tests will expose it. See #173 --- events/storage.py | 7 ++++--- ingest/filestore.py | 9 +++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/events/storage.py b/events/storage.py index 7253ce3..226f544 100644 --- a/events/storage.py +++ b/events/storage.py @@ -3,6 +3,8 @@ import contextlib import os.path from pathlib import Path +from django.utils._os import safe_join + class EventStorage(object): @@ -44,12 +46,11 @@ class FileEventStorage(EventStorage): # not a problem. # event_id comes from event.id, i.e. it's a UUID object which is generated by Bugsink itself (i.e. trusted). - # The check below exists exclusively such that the security-implications of os.path.join can be understood right - # here in the code without needing to inspect all call-sites: + # In addition, the UUID check ensures path safety independently of call-sites _and_ safe_join is used. if not isinstance(event_id, uuid.UUID): raise ValueError("event_id must be a UUID") - return os.path.join(self.basepath, str(event_id) + ".json") + return safe_join(self.basepath, str(event_id) + ".json") @contextlib.contextmanager def open(self, event_id, mode='r'): diff --git a/ingest/filestore.py b/ingest/filestore.py index 9c5520a..b8737cf 100644 --- a/ingest/filestore.py +++ b/ingest/filestore.py @@ -1,5 +1,6 @@ import uuid -import os + +from django.utils._os import safe_join from bugsink.app_settings import get_settings @@ -10,8 +11,8 @@ def get_filename_for_event_id(event_id): # ensure that event_id is a uuid, and remove dashes if present; also doubles as a security-check (event_id is # user-provided (but at this point already validated to be a valid UUID), but b/c of the below the - # security-implications of os.path.join can be understood right here in the code without needing to inspect all - # call-sites). + # security-implications of path-joining (even though we use safe_join) can be understood right here in the code + # without needing to inspect all call-sites) event_id_normalized = uuid.UUID(event_id).hex - return os.path.join(get_settings().INGEST_STORE_BASE_DIR, event_id_normalized) + return safe_join(get_settings().INGEST_STORE_BASE_DIR, event_id_normalized)