fix(import): stop hardcoding the Docker image's paths

Import uploads were always written to /usr/src/app/temp, presets were
read from /usr/src/app/import_presets and the caches lived in /var/tmp,
so WYGIWYH only worked from the Docker image's layout. Running it as a
systemd service with a strict sandbox, or on an immutable distro like
NixOS, needs these folders to follow the app or be configurable.

- Presets are read from IMPORT_PRESETS_ROOT, the import_presets folder
  next to the app.
- Uploads wait in IMPORT_TEMP_ROOT, the temp folder next to the app by
  default, and movable with the IMPORT_TEMP_ROOT env var. The web views,
  the API and the import service all use it.
- The file-based caches live under CACHE_ROOT, /var/tmp by default, and
  movable with the CACHE_ROOT env var.

In the Docker image the app sits in /usr/src/app, so every default
resolves to the same folder as before.

The import service checked that a file was inside the temp folder with a
plain string prefix, which also accepted sibling folders such as
/usr/src/app/temp-other. It now compares paths.

CI no longer needs to create /usr/src/app before running the tests.

Fixes #472

.github/workflows/tests.yml

@@ -60,12 +60,6 @@ jobs:
       - name: Install dependencies
         run: uv sync --frozen
 
-      # Import code reads/writes the Docker image's paths directly
-      - name: Create container paths used by the app
-        run: |
-          sudo mkdir -p /usr/src/app/temp /usr/src/app/import_presets
-          sudo chown -R "$(id -u):$(id -g)" /usr/src/app
-
       - name: Check for missing migrations
         working-directory: app
         run: uv run python manage.py makemigrations --check --dry-run

.gitignore

@@ -168,3 +168,4 @@ postgres_data/
 
 # Private local uploads
 app/attachments/
+app/temp/

README.md

@@ -149,6 +149,8 @@ To create the first user, open the container's console using Unraid's UI, by cli
 | DJANGO_VITE_DEV_MODE          | true\|false | false                             | Enables Vite dev server mode for frontend development. When true, assets are served from Vite's dev server instead of the build manifest. For development only!                                                                          |
 | DJANGO_VITE_DEV_SERVER_PORT   | int         | 5173                              | The port where Vite's dev server is running. Only used when DJANGO_VITE_DEV_MODE is true. For development only!                                                                                                                          |
 | DJANGO_VITE_DEV_SERVER_HOST   | string      | localhost                         | The host where Vite's dev server is running. Only used when DJANGO_VITE_DEV_MODE is true. For development only!                                                                                                                          |
+| IMPORT_TEMP_ROOT              | string      | temp, in the app folder           | Where uploaded import files wait to be processed. Must be writable and shared between the web and task processes.                                                                                                                        |
+| CACHE_ROOT                    | string      | /var/tmp                          | The writable folder the file-based caches are stored in.                                                                                                                                                                                 |
 
 ## OIDC Configuration
 

app/WYGIWYH/settings.py

@@ -313,6 +313,9 @@ LOCALE_PATHS = [BASE_DIR / "locale"]
 STATIC_URL = "static/"
 STATIC_ROOT = BASE_DIR / "static_files"
 ATTACHMENT_MEDIA_ROOT = BASE_DIR / "attachments"
+IMPORT_PRESETS_ROOT = BASE_DIR / "import_presets"
+# Web and task workers must see the same folder: uploads wait here to be processed
+IMPORT_TEMP_ROOT = Path(os.getenv("IMPORT_TEMP_ROOT") or BASE_DIR / "temp").resolve()
 
 STATICFILES_DIRS = [
     ROOT_DIR / "frontend" / "build",
@@ -324,15 +327,17 @@ STATICFILES_FINDERS = [
     "django.contrib.staticfiles.finders.AppDirectoriesFinder",
 ]
 
+CACHE_ROOT = Path(os.getenv("CACHE_ROOT") or "/var/tmp").resolve()
+
 CACHES = {
     "default": {
         "BACKEND": "django.core.cache.backends.filebased.FileBasedCache",
-        "LOCATION": "/var/tmp/django_cache",
+        "LOCATION": CACHE_ROOT / "django_cache",
     },
     # Kept apart from "default": cachalot fills it and FileBasedCache culls random entries past MAX_ENTRIES
     "update_check": {
         "BACKEND": "django.core.cache.backends.filebased.FileBasedCache",
-        "LOCATION": "/var/tmp/django_cache_update_check",
+        "LOCATION": CACHE_ROOT / "django_cache_update_check",
     },
 }
 

app/apps/api/views/imports.py

@@ -1,3 +1,4 @@
+from django.conf import settings
 from django.core.files.storage import FileSystemStorage
 from drf_spectacular.types import OpenApiTypes
 from drf_spectacular.utils import OpenApiParameter, extend_schema, extend_schema_view, inline_serializer
@@ -130,7 +131,7 @@ class ImportViewSet(viewsets.ViewSet):
         uploaded_file = serializer.validated_data["file"]
 
         # Save file to temp location
-        fs = FileSystemStorage(location="/usr/src/app/temp")
+        fs = FileSystemStorage(location=settings.IMPORT_TEMP_ROOT)
         filename = fs.save(uploaded_file.name, uploaded_file)
         file_path = fs.path(filename)
 

app/apps/import_app/services/presets.py

@@ -1,17 +1,17 @@
 import json
 from pathlib import Path
 
+from django.conf import settings
+
 from apps.import_app.models import ImportProfile
 
 
 class PresetService:
-    PRESET_PATH = "/usr/src/app/import_presets"
-
     @classmethod
     def get_all_presets(cls):
         presets = []
 
-        for folder in Path(cls.PRESET_PATH).iterdir():
+        for folder in settings.IMPORT_PRESETS_ROOT.iterdir():
             if folder.is_dir():
                 manifest_path = folder / "manifest.json"
                 config_path = folder / "config.yml"

app/apps/import_app/services/v1.py

@@ -7,12 +7,14 @@ import zipfile
 from django.db import transaction
 from datetime import datetime, date
 from decimal import Decimal, InvalidOperation
+from pathlib import Path
 from typing import Dict, Any, Literal, Union
 
 import openpyxl
 import xlrd
 import yaml
 from cachalot.api import cachalot_disabled
+from django.conf import settings
 from django.core.exceptions import FieldDoesNotExist
 from django.utils import timezone
 from openpyxl.utils.exceptions import InvalidFileException
@@ -39,8 +41,6 @@ logger = logging.getLogger(__name__)
 
 
 class ImportService:
-    TEMP_DIR = "/usr/src/app/temp"
-
     def __init__(self, import_run: ImportRun):
         self.import_run: ImportRun = import_run
         self.profile: ImportProfile = import_run.profile
@@ -54,7 +54,7 @@ class ImportService:
         self.mapping: Dict[str, version_1.ColumnMapping] = self.config.mapping
 
         # Ensure temp directory exists
-        os.makedirs(self.TEMP_DIR, exist_ok=True)
+        os.makedirs(settings.IMPORT_TEMP_ROOT, exist_ok=True)
 
     def _load_config(self) -> version_1.ImportProfileSchema:
         yaml_data = yaml.safe_load(self.profile.yaml_config)
@@ -1132,8 +1132,10 @@ class ImportService:
         Returns the absolute path.
         """
         abs_path = os.path.abspath(file_path)
-        if not abs_path.startswith(self.TEMP_DIR):
-            raise ValueError(f"Invalid file path. File must be in {self.TEMP_DIR}")
+        if not Path(abs_path).is_relative_to(settings.IMPORT_TEMP_ROOT):
+            raise ValueError(
+                f"Invalid file path. File must be in {settings.IMPORT_TEMP_ROOT}"
+            )
         return abs_path
 
     def process_file(self, file_path: str):

app/apps/import_app/tests/test_paths.py

@@ -0,0 +1,151 @@
+"""Tests for the folders import code reads from and writes to."""
+
+import json
+import os
+import runpy
+import shutil
+import tempfile
+from pathlib import Path
+from unittest.mock import patch
+
+from django.conf import settings
+from django.core.files.uploadedfile import SimpleUploadedFile
+from django.test import SimpleTestCase, override_settings
+from django.urls import reverse
+from rest_framework import status
+from rest_framework.test import APIClient
+
+from apps.import_app.models import ImportRun
+from apps.import_app.services import PresetService
+from apps.import_app.services.v1 import ImportService
+from apps.import_app.tests.test_view_permissions import (
+    HTMX,
+    ImportObjectPermissionTestCase,
+)
+
+
+def load_settings(**env):
+    """Evaluates the settings module from scratch under the given env vars."""
+    with patch.dict(os.environ, env):
+        return runpy.run_path(str(Path(settings.BASE_DIR) / "WYGIWYH" / "settings.py"))
+
+
+class PathSettingsTests(SimpleTestCase):
+    def test_defaults_match_the_docker_image_layout(self):
+        loaded = load_settings(IMPORT_TEMP_ROOT="", CACHE_ROOT="")
+
+        base_dir = loaded["BASE_DIR"]
+        self.assertEqual(loaded["IMPORT_TEMP_ROOT"], base_dir / "temp")
+        self.assertEqual(loaded["IMPORT_PRESETS_ROOT"], base_dir / "import_presets")
+        self.assertEqual(
+            loaded["CACHES"]["default"]["LOCATION"],
+            Path("/var/tmp/django_cache").resolve(),
+        )
+        self.assertEqual(
+            loaded["CACHES"]["update_check"]["LOCATION"],
+            Path("/var/tmp/django_cache_update_check").resolve(),
+        )
+
+    def test_env_vars_move_temp_and_cache_folders(self):
+        with tempfile.TemporaryDirectory() as root:
+            root = Path(root).resolve()
+            loaded = load_settings(
+                IMPORT_TEMP_ROOT=str(root / "uploads"), CACHE_ROOT=str(root / "cache")
+            )
+
+        self.assertEqual(loaded["IMPORT_TEMP_ROOT"], root / "uploads")
+        self.assertEqual(
+            loaded["CACHES"]["default"]["LOCATION"], root / "cache" / "django_cache"
+        )
+        self.assertEqual(
+            loaded["CACHES"]["update_check"]["LOCATION"],
+            root / "cache" / "django_cache_update_check",
+        )
+
+
+class PresetServiceTests(SimpleTestCase):
+    def test_lists_the_presets_shipped_with_the_app(self):
+        folder_names = {
+            preset["folder_name"] for preset in PresetService.get_all_presets()
+        }
+
+        self.assertIn("qif_standard", folder_names)
+
+    def test_reads_presets_from_configured_folder(self):
+        with tempfile.TemporaryDirectory() as root:
+            preset = Path(root) / "my_bank"
+            preset.mkdir()
+            (preset / "manifest.json").write_text(json.dumps({"name": "My Bank"}))
+            (preset / "config.yml").write_text("settings: {}")
+
+            with override_settings(IMPORT_PRESETS_ROOT=Path(root)):
+                presets = PresetService.get_all_presets()
+
+        self.assertEqual([p["name"] for p in presets], ["My Bank"])
+
+
+class ImportTempRootTests(ImportObjectPermissionTestCase):
+    def setUp(self):
+        super().setUp()
+        self.temp_root = Path(tempfile.mkdtemp()).resolve()
+        self.addCleanup(shutil.rmtree, self.temp_root, ignore_errors=True)
+        temp_root_override = override_settings(IMPORT_TEMP_ROOT=self.temp_root)
+        temp_root_override.enable()
+        self.addCleanup(temp_root_override.disable)
+
+    @patch("apps.import_app.views.process_import.defer")
+    def test_web_upload_is_stored_in_temp_root(self, mock_defer):
+        self.login(self.owner)
+        response = self.client.post(
+            reverse("import_run_add", kwargs={"profile_id": self.private_profile.id}),
+            {"file": SimpleUploadedFile("web.csv", b"date,amount\n")},
+            **HTMX,
+        )
+
+        self.assertEqual(response.status_code, 204)
+        self.assertEqual(
+            mock_defer.call_args.kwargs["file_path"], str(self.temp_root / "web.csv")
+        )
+        self.assertTrue((self.temp_root / "web.csv").is_file())
+
+    @patch("apps.api.views.imports.process_import.defer")
+    def test_api_upload_is_stored_in_temp_root(self, mock_defer):
+        client = APIClient()
+        client.force_authenticate(user=self.owner)
+        response = client.post(
+            "/api/import/import/",
+            {
+                "profile_id": self.private_profile.id,
+                "file": SimpleUploadedFile("api.csv", b"date,amount\n"),
+            },
+            format="multipart",
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_202_ACCEPTED)
+        self.assertEqual(
+            mock_defer.call_args.kwargs["file_path"], str(self.temp_root / "api.csv")
+        )
+        self.assertTrue((self.temp_root / "api.csv").is_file())
+
+    def _service(self):
+        run = ImportRun.objects.create(
+            profile=self.private_profile, file_name="file.csv", owner=self.owner
+        )
+        return ImportService(run)
+
+    def test_service_accepts_files_inside_temp_root(self):
+        path = str(self.temp_root / "file.csv")
+
+        self.assertEqual(self._service()._validate_file_path(path), path)
+
+    def test_service_rejects_files_outside_temp_root(self):
+        service = self._service()
+
+        for path in (
+            "/etc/passwd",
+            self.temp_root / ".." / "file.csv",
+            # Shares the temp root as a string prefix, but is a different folder
+            f"{self.temp_root}-other/file.csv",
+        ):
+            with self.subTest(path=path), self.assertRaises(ValueError):
+                service._validate_file_path(str(path))

app/apps/import_app/tests/test_qif_import.py

@@ -1,7 +1,7 @@
 from decimal import Decimal
 import os
 import shutil
-from django.test import TestCase
+from django.test import TestCase, override_settings
 from django.contrib.auth import get_user_model
 from apps.accounts.models import Account, AccountGroup
 from apps.currencies.models import Currency
@@ -15,10 +15,11 @@ from apps.transactions.models import (
 
 class QIFImportTests(TestCase):
     def setUp(self):
-        # Patch TEMP_DIR for testing
-        self.original_temp_dir = ImportService.TEMP_DIR
+        # Point the import temp folder somewhere disposable for testing
         self.test_dir = os.path.abspath("temp_test_import")
-        ImportService.TEMP_DIR = self.test_dir
+        temp_root_override = override_settings(IMPORT_TEMP_ROOT=self.test_dir)
+        temp_root_override.enable()
+        self.addCleanup(temp_root_override.disable)
         os.makedirs(self.test_dir, exist_ok=True)
 
         # Create user and set context
@@ -41,7 +42,6 @@ class QIFImportTests(TestCase):
 
     def tearDown(self):
         delete_current_user()
-        ImportService.TEMP_DIR = self.original_temp_dir
         if os.path.exists(self.test_dir):
             shutil.rmtree(self.test_dir)
 

app/apps/import_app/views.py

@@ -1,5 +1,6 @@
 import shutil
 
+from django.conf import settings
 from django.contrib import messages
 from django.contrib.auth.decorators import login_required
 from django.core.exceptions import PermissionDenied
@@ -204,7 +205,7 @@ def import_run_add(request, profile_id):
 
         if form.is_valid():
             uploaded_file = request.FILES["file"]
-            fs = FileSystemStorage(location="/usr/src/app/temp")
+            fs = FileSystemStorage(location=settings.IMPORT_TEMP_ROOT)
             filename = fs.save(uploaded_file.name, uploaded_file)
             file_path = fs.path(filename)