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
This commit is contained in:
Klaas van Schelven
2025-07-29 22:23:37 +02:00
parent aab9b38352
commit 354edc81f9
2 changed files with 9 additions and 7 deletions

View File

@@ -3,6 +3,8 @@ import contextlib
import os.path import os.path
from pathlib import Path from pathlib import Path
from django.utils._os import safe_join
class EventStorage(object): class EventStorage(object):
@@ -44,12 +46,11 @@ class FileEventStorage(EventStorage):
# not a problem. # 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). # 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 # In addition, the UUID check ensures path safety independently of call-sites _and_ safe_join is used.
# here in the code without needing to inspect all call-sites:
if not isinstance(event_id, uuid.UUID): if not isinstance(event_id, uuid.UUID):
raise ValueError("event_id must be a 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 @contextlib.contextmanager
def open(self, event_id, mode='r'): def open(self, event_id, mode='r'):

View File

@@ -1,5 +1,6 @@
import uuid import uuid
import os
from django.utils._os import safe_join
from bugsink.app_settings import get_settings 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 # 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 # 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 # security-implications of path-joining (even though we use safe_join) can be understood right here in the code
# call-sites). # without needing to inspect all call-sites)
event_id_normalized = uuid.UUID(event_id).hex 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)