fix(ops): la sauvegarde des contrats archivait le mauvais repertoire

OBS-006. Trois racines etaient baties sur os.getcwd() : le magasin de
documents, les journaux et les sauvegardes. La vague G a corrige la
premiere, parce qu'elle bloquait aussi OPS-011, et a laisse les deux autres.
C'est la lecon deja consignee deux fois : un motif fautif corrige dans une
seule couche reste dans les autres.

Le plus serieux n'est pas le motif, c'est l'ecart qu'il a ouvert.
backup.py gardait sa propre constante DOCUMENTS_DIR sur os.getcwd(), donc
il ignorait DOCUMENTS_ROOT -- la variable que la vague G a introduite et que
docs/deployment.md dit maintenant de regler pour sortir les televersements
des repertoires de version. Des qu'un exploitant suit cette consigne, le
script archive un repertoire ou l'application n'a jamais rien ecrit. Et
comme il repond a un repertoire absent par une ligne d'information et un
code de sortie 0, une tache planifiee qui surveille le code de sortie voit
vert indefiniment.

Autrement dit : plus l'exploitant suivait correctement la documentation de
deploiement, plus surement ses sauvegardes de contrats etaient vides.

Les trois racines viennent desormais d'app/storage.py, resolues a l'appel et
non a l'import, et la sauvegarde imprime la source qu'elle a utilisee. Le
message d'absence nomme le chemin ou elle a cherche : "No documents
directory found" se lisait comme "il n'y a pas de documents" plutot que
comme "je regarde au mauvais endroit".

Le test qui porte est celui qui ouvre l'archive : un zip vide est un fichier
de taille non nulle, donc verifier qu'un fichier a ete produit ne prouvait
rien. Verifie par mutation.

Co-Authored-By: Claude Opus 5 <[email protected]>

app/.env.example

@@ -95,8 +95,37 @@ DISCORD_REDIRECT_URI=https://your-domain/auth/discord/callback
 # application. Set it to a path OUTSIDE the deployment directory if you move
 # to a release-directory layout, or a deployment will take the documents with
 # it (OPS-011, app/storage.py).
+#
+# IMPORTANT: the backup script reads this same variable. Before wave J it
+# did not, and archived `./documents` regardless — so setting this here and
+# nowhere else produced empty contract backups that still exited 0.
 DOCUMENTS_ROOT=
 
+# Where the log files go. Empty means `logs/` beside the application. Both
+# defaults are anchored on the application, not on the directory the process
+# was started from, which is what they used to be (OBS-006).
+LOG_DIR=
+
+# Where the backup script writes its archives. Empty means `backups/` beside
+# the application.
+BACKUP_DIR=
+
+# =============================================================================
+# Optional — rate limiting
+# =============================================================================
+
+# Where the rate limiter keeps its counters. Empty means `memory://`, which
+# is correct for a single Waitress process and is what this deployment runs.
+#
+# Set it to a shared backend (redis://…) BEFORE running more than one worker:
+# in-memory counters are per-process, so N workers let through N times every
+# configured limit, with nothing to show for it in the logs.
+#
+# Note that shared storage does not by itself make the limits sound: they are
+# keyed on the client IP, which is forgeable until TRUSTED_PROXY is set
+# correctly (SEC-WEB-002 / OPS-002 — see above).
+RATELIMIT_STORAGE_URI=
+
 # Tables are created at startup when missing. Set to false once Alembic owns
 # the schema (DB-002/DB-004): create_all() never ALTERs, so a column added to
 # a model is silently absent from an existing database.

app/logging_config.py

@@ -14,6 +14,8 @@ import os
 import re
 from logging.handlers import RotatingFileHandler
 
+from app.storage import logs_root
+
 #: Value used when a record is emitted outside a request — startup, the
 #: Discord bot thread, the scheduler. Short and obviously not an id, so a
 #: grep for one never matches it by accident.
@@ -117,7 +119,12 @@ def configure_logging(app):
     Args:
         app: The Flask application instance to configure logging for.
     """
-    log_dir = os.path.join(os.getcwd(), 'logs')
+    # Anchored on the project, not on the working directory (OBS-006). The
+    # old form put the logs wherever the process happened to be started
+    # from, so a service restarted by hand from another directory quietly
+    # began writing somewhere else — and the file you go looking at when
+    # something is wrong is the one that must not move.
+    log_dir = logs_root()
     os.makedirs(log_dir, exist_ok=True)
 
     # Remove default Flask handlers to avoid duplicate logging

app/storage.py

@@ -1,4 +1,13 @@
-"""Where uploaded documents live, and how the database refers to them.
+"""Where the application's files live, and how the database refers to them.
+
+Three roots: the project itself, the document store, and the log directory.
+They are here together because they are the same defect three times over
+(OBS-006) — `os.path.join(os.getcwd(), …)`, evaluated at import or upload
+time, so every one of them moved with whatever directory the process was
+started from. The document store was fixed first, in wave G, because it was
+the one that also blocked OPS-011; the other two were left behind, which is
+the repeated lesson of this project: a faulty pattern corrected in one layer
+stays in the others.
 
 Contracts were stored at `os.path.join(os.getcwd(), 'documents', …)`,
 evaluated at upload time, and the resulting absolute path was written into
@@ -26,21 +35,60 @@ import os
 #: inside whichever one is current.
 DOCUMENTS_ROOT_ENV = 'DOCUMENTS_ROOT'
 
+#: Environment override for the log directory. Same reasoning: a release
+#: directory that carries its own logs loses them at the next switch.
+LOG_DIR_ENV = 'LOG_DIR'
+
+#: Environment override for the backup directory.
+BACKUP_DIR_ENV = 'BACKUP_DIR'
+
 #: Sub-directory holding uploaded contracts, under the document root.
 CONTRACTS_DIR = 'contrats signés'
 
 
+def project_root():
+    """Absolute path of the project, derived from this file's location.
+
+    The anchor every other root falls back to. Not `os.getcwd()`: the
+    process is started by Waitress under Pterodactyl, by pytest, by a task
+    scheduler and by a person in a shell, and only one of those four is
+    reliably in the project directory.
+    """
+    package_dir = os.path.dirname(os.path.abspath(__file__))
+    return os.path.dirname(package_dir)
+
+
 def documents_root():
     """Absolute path of the document store.
 
     Falls back to `documents/` beside this package — the project root
     wherever it is installed, rather than wherever the process was launched.
     """
-    configured = os.getenv(DOCUMENTS_ROOT_ENV)
+    return _rooted(DOCUMENTS_ROOT_ENV, 'documents')
+
+
+def logs_root():
+    """Absolute path of the log directory.
+
+    Was `os.path.join(os.getcwd(), 'logs')`. Starting the server from
+    another directory sent the logs somewhere new without a word, which is
+    the worst possible failure mode for the one file you go and read when
+    something else has gone wrong.
+    """
+    return _rooted(LOG_DIR_ENV, 'logs')
+
+
+def backups_root():
+    """Absolute path of the backup directory."""
+    return _rooted(BACKUP_DIR_ENV, 'backups')
+
+
+def _rooted(env_name, default_name):
+    """The configured path, or `default_name` under the project root."""
+    configured = os.getenv(env_name)
     if configured:
         return os.path.abspath(configured)
-    package_dir = os.path.dirname(os.path.abspath(__file__))
+    return os.path.join(project_root(), default_name)
-    return os.path.join(os.path.dirname(package_dir), 'documents')
 
 
 def discard_documents(stored_paths):

app/supporting_scripts/backup.py

@@ -37,13 +37,33 @@ import sys
 from datetime import datetime, timedelta
 from urllib.parse import unquote, urlparse
 
+# Run as `python app/supporting_scripts/backup.py`, sys.path[0] is this
+# script's directory, so the application package is not importable. It has
+# to be — see DOCUMENTS_DIR below.
+sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__)))))
+
+from app.storage import backups_root, documents_root  # noqa: E402 — needs the path above
+
 # Configuration
-BACKUP_DIR = os.getenv('BACKUP_DIR', os.path.join(os.getcwd(), 'backups'))
+BACKUP_DIR = backups_root()
 BACKUP_RETENTION_DAYS = int(os.getenv('BACKUP_RETENTION_DAYS', 30))
-DOCUMENTS_DIR = os.path.join(os.getcwd(), 'documents')
 PG_DUMP = os.getenv('PG_DUMP', 'pg_dump')
 PG_RESTORE = os.getenv('PG_RESTORE', 'pg_restore')
 
+# There is deliberately no DOCUMENTS_DIR constant any more. It held
+# `os.path.join(os.getcwd(), 'documents')`, which had stopped being true:
+# wave G introduced DOCUMENTS_ROOT so a release-directory deployment could
+# keep uploads outside the releases, and docs/deployment.md now tells the
+# operator to set it — at which point this script archived a directory the
+# application had never written to. It does not fail on a missing directory
+# either; it prints "No documents directory found", skips, and exits 0.
+#
+# So the more correctly an operator followed the deployment documentation,
+# the more certainly their contract backups were empty (OBS-006).
+#
+# backup_documents() now asks app.storage, at call time, the same question
+# the upload path asks. One source of truth, and one that a test can move.
+
 
 class BackupError(Exception):
     """Raised when a backup step fails in a way that must stop the run."""
@@ -235,20 +255,31 @@ def verify_backup(backup_path):
 def backup_documents():
     """Archive the uploaded contract documents directory.
 
+    The directory is resolved through `app.storage.documents_root()` — the
+    same function the upload path uses — so that setting DOCUMENTS_ROOT
+    moves both together. Resolved here rather than at import, so that what
+    is backed up depends on the environment the run has, not on the one the
+    module happened to be imported with.
+
     Returns:
         str: Path to the created archive, or None if there is nothing to
         archive. Signed contracts live only on disk, so losing this
         directory loses the documents themselves.
     """
-    if not os.path.exists(DOCUMENTS_DIR):
+    documents_dir = documents_root()
-        print('[INFO] No documents directory found. Skipping document backup.')
+
+    if not os.path.exists(documents_dir):
+        # Says where it looked. The previous message named no path, so an
+        # operator who had moved the documents read it as "there are no
+        # documents" rather than "I am looking in the wrong place".
+        print(f'[INFO] No documents directory at {documents_dir}. Skipping document backup.')
         return None
 
     timestamp = datetime.now().strftime('%Y%m%d_%H%M%S')
     archive_basename = os.path.join(BACKUP_DIR, f'documents_backup_{timestamp}')
 
     try:
-        shutil.make_archive(archive_basename, 'zip', DOCUMENTS_DIR)
+        shutil.make_archive(archive_basename, 'zip', documents_dir)
     except Exception as exc:  # noqa: BLE001 — a failed document archive must not lose the dump
         # This runs after the database dump has already succeeded. Letting
         # anything through here would abort the script with a traceback and
@@ -314,6 +345,9 @@ def main(argv=None):
     print('=== Team Tryouts Backup ===')
     print(f'Started at: {datetime.now().strftime("%Y-%m-%d %H:%M:%S")}')
     print(f'Backup directory: {BACKUP_DIR}')
+    # Printed because it is the value that was wrong for months without
+    # anyone being able to see it from the output.
+    print(f'Document source: {documents_root()}')
     print(f'Retention period: {BACKUP_RETENTION_DAYS} days')
     print()
 

docs/deployment.md

@@ -270,7 +270,7 @@ Three prerequisites. One is done, two are not:
 | # | Prerequisite | State |
 |---|---|---|
 | 1 | The Pterodactyl startup command must run the app from `current/`, and the server must be restarted on switch | **Panel change.** Cannot be made or verified from the repository |
-| 2 | `documents/`, `logs/` and `.env` must sit beside the releases, not inside one. `DOCUMENTS_ROOT` points the document store at a fixed path (`app/storage.py`) | Mechanism ready, **not yet configured on the node** |
+| 2 | `documents/`, `logs/` and `.env` must sit beside the releases, not inside one. `DOCUMENTS_ROOT` and `LOG_DIR` point the document store and the logs at fixed paths (`app/storage.py`) | Mechanism ready, **not yet configured on the node** |
 | 3 | Contract paths must be relative to that root, or the first switch strands every contract ever uploaded | **Done.** New rows store a relative path; rows written earlier keep their absolute one and still resolve, so no data migration is needed |
 
 Prerequisite 3 was a defect on its own, not just a blocker: paths were built
@@ -278,6 +278,20 @@ from `os.getcwd()`, so starting the server from a different directory would
 have sent new contracts to a new tree and made the existing ones unreadable
 — with the database still claiming they were there.
 
+**The same defect was in two more places, and one of them mattered more.**
+`logs/` and `backups/` were built from `os.getcwd()` too (OBS-006), and the
+backup script kept its own copy of the document path — so it archived
+`./documents` no matter what `DOCUMENTS_ROOT` said. Following prerequisite 2
+was therefore enough, on its own, to make every contract backup empty; the
+script prints `No documents directory…` and still exits 0, so a scheduled
+task watching the exit code would have seen green indefinitely. All three
+roots now come from `app/storage.py`, and the backup run prints the document
+source it used.
+
+**After setting `DOCUMENTS_ROOT` on the node, run the backup once by hand**
+and check the `Document source:` line and the size of the resulting
+`documents_backup_*.zip`.
+
 ## Security Checklist
 
 - [ ] `.env` is not committed to repository

tests/test_filesystem_roots.py

@@ -0,0 +1,144 @@
+"""Where the application puts its files, and who agrees about it.
+
+OBS-006. Three roots were built as `os.path.join(os.getcwd(), …)`: the
+document store, the log directory and the backup directory. Each therefore
+moved with whatever directory the process was started from — and the
+processes that start this application are Waitress under Pterodactyl, a task
+scheduler, pytest, and a person in a shell. Only one of the four is reliably
+in the project directory.
+
+The document store was fixed in wave G. The other two were not, and the gap
+that opened between them is the reason this file exists: `backup.py` kept
+archiving `./documents` while the application wrote to `DOCUMENTS_ROOT`, and
+the script's answer to a missing directory is to print a line and exit 0.
+Following the deployment documentation was what broke it.
+"""
+
+import os
+
+import pytest
+
+from app.storage import backups_root, documents_root, logs_root, project_root
+
+
+class TestTheProjectAnchor:
+    def test_it_is_the_directory_holding_the_app_package(self):
+        assert os.path.isdir(os.path.join(project_root(), 'app'))
+
+    def test_it_does_not_follow_the_working_directory(self, tmp_path, monkeypatch):
+        """The whole point. Every root below inherits this."""
+        before = project_root()
+        monkeypatch.chdir(tmp_path)
+
+        assert project_root() == before
+
+
+@pytest.mark.parametrize(
+    ('root', 'env_name', 'default_name'),
+    [
+        (documents_root, 'DOCUMENTS_ROOT', 'documents'),
+        (logs_root, 'LOG_DIR', 'logs'),
+        (backups_root, 'BACKUP_DIR', 'backups'),
+    ],
+)
+class TestEachRoot:
+    def test_it_defaults_beside_the_project(self, root, env_name, default_name, monkeypatch):
+        monkeypatch.delenv(env_name, raising=False)
+
+        assert root() == os.path.join(project_root(), default_name)
+
+    def test_it_ignores_the_working_directory(
+        self, root, env_name, default_name, tmp_path, monkeypatch
+    ):
+        monkeypatch.delenv(env_name, raising=False)
+        monkeypatch.chdir(tmp_path)
+
+        assert root() == os.path.join(project_root(), default_name)
+        assert not root().startswith(str(tmp_path))
+
+    def test_the_environment_wins(self, root, env_name, default_name, tmp_path, monkeypatch):
+        monkeypatch.setenv(env_name, str(tmp_path / 'elsewhere'))
+
+        assert root() == os.path.abspath(str(tmp_path / 'elsewhere'))
+
+
+class TestTheBackupScriptAgreesWithTheApplication:
+    """The regression this file was written for.
+
+    `backup.py` held its own `os.getcwd()`-based constant, so the two
+    answers to "where are the contracts" could differ — and did, for anyone
+    who set DOCUMENTS_ROOT because docs/deployment.md said to.
+    """
+
+    def test_it_archives_the_directory_the_application_writes_to(self, tmp_path, monkeypatch):
+        from app.supporting_scripts import backup
+
+        store = tmp_path / 'somewhere-else' / 'documents'
+        (store / 'contrats signés').mkdir(parents=True)
+        (store / 'contrats signés' / 'a.pdf').write_bytes(b'%PDF-1.4 test')
+        monkeypatch.setenv('DOCUMENTS_ROOT', str(store))
+        monkeypatch.setattr(backup, 'BACKUP_DIR', str(tmp_path / 'backups'))
+        (tmp_path / 'backups').mkdir()
+
+        archive = backup.backup_documents()
+
+        assert archive is not None, (
+            'the script skipped, which is what it did on production while reporting success'
+        )
+        assert os.path.getsize(archive) > 0
+
+    def test_the_archive_actually_contains_the_contract(self, tmp_path, monkeypatch):
+        """An empty zip is still a file of non-zero size. This is the
+        assertion that would have caught the original defect."""
+        import zipfile
+
+        from app.supporting_scripts import backup
+
+        store = tmp_path / 'documents'
+        (store / 'contrats signés').mkdir(parents=True)
+        (store / 'contrats signés' / 'contrat.pdf').write_bytes(b'%PDF-1.4 test')
+        monkeypatch.setenv('DOCUMENTS_ROOT', str(store))
+        monkeypatch.setattr(backup, 'BACKUP_DIR', str(tmp_path / 'backups'))
+        (tmp_path / 'backups').mkdir()
+
+        archive = backup.backup_documents()
+
+        with zipfile.ZipFile(archive) as zf:
+            assert any(name.endswith('contrat.pdf') for name in zf.namelist())
+
+    def test_a_missing_store_names_the_path_it_looked_in(self, tmp_path, monkeypatch, capsys):
+        """ "No documents directory found" read as "there are no documents"
+        rather than "I am looking in the wrong place"."""
+        from app.supporting_scripts import backup
+
+        missing = tmp_path / 'not-here'
+        monkeypatch.setenv('DOCUMENTS_ROOT', str(missing))
+
+        assert backup.backup_documents() is None
+        assert str(missing) in capsys.readouterr().out
+
+
+class TestLogsFollowTheSameRule:
+    def test_configure_logging_writes_where_logs_root_says(self, tmp_path, monkeypatch):
+        from app.app import create_app
+
+        target = tmp_path / 'var' / 'log'
+        monkeypatch.setenv('LOG_DIR', str(target))
+
+        create_app(
+            {
+                'SECRET_KEY': 'test-secret-not-used-anywhere-real',
+                'SQLALCHEMY_DATABASE_URI': f'sqlite:///{tmp_path / "t.sqlite"}',
+                'TESTING': True,
+                'WTF_CSRF_ENABLED': False,
+                'FORCE_HTTPS': False,
+                'SESSION_COOKIE_SECURE': False,
+                'ENABLE_DISCORD_BOT': False,
+                'AUTO_CREATE_TABLES': True,
+                'CORS_ALLOWED_ORIGINS': '',
+                'RATELIMIT_ENABLED': False,
+            }
+        )
+
+        assert target.is_dir(), 'the log directory is created where LOG_DIR points'
+        assert (target / 'app.log').exists()