From 341f154c36f9a1f00daf1812dd1ee4b4c340b88e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Faleiros?= Date: Mon, 21 Sep 2026 11:36:59 -0300 Subject: [PATCH] feat: load Docker secret files so the production stack can boot deploy/stack.yaml passes DATABASE_URL_FILE, AWS_ACCESS_KEY_ID_FILE, OPERATOR_PASSWORD_FILE and the provider tokens as Swarm secret paths, but the runtime only ever read the plain names. That stack could not start: the database URL and R2 credentials were absent, and operator login raised KeyError, so it returned 500 instead of the intended 503. local/secrets.py resolves every _FILE into before configuration is read, from the API, worker and bootstrap entrypoints. It fails closed on an unreadable or empty secret and on a name supplied both directly and as a file, because starting with a credential nobody intended is worse than not starting. Only one trailing newline is stripped, so a generated password keeps any whitespace that belongs to it, and no value reaches an error message. The stack also passed OPERATOR_USER while the Kanban authenticates by email; it now passes OPERATOR_EMAIL, matching the runtime. The release gate checked this by searching local/secrets.py for the literal "DATABASE_URL_FILE", which would pass for any file containing that string. It now loads the module and makes it resolve every secret the stack declares, and asserts it fails closed on a missing one. Four marker strings that stopped matching when R2 support landed are removed rather than left to rot; the two that still describe real blockers stay, so the gate continues to refuse a release while payment and messaging adapters are fake. Co-Authored-By: Claude Opus 5 --- .gitea/workflows/deploy.yml | 3 +- deploy/production_preflight.py | 70 ++++++++++++++++++---- deploy/stack.yaml | 3 +- deploy/test_production_preflight.py | 2 +- local/app.py | 2 + local/bootstrap.py | 2 + local/secrets.py | 76 +++++++++++++++++++++++ local/test_secrets.py | 93 +++++++++++++++++++++++++++++ local/worker.py | 2 + 9 files changed, 240 insertions(+), 13 deletions(-) create mode 100644 local/secrets.py create mode 100644 local/test_secrets.py diff --git a/.gitea/workflows/deploy.yml b/.gitea/workflows/deploy.yml index 20a7427..b09b5f3 100644 --- a/.gitea/workflows/deploy.yml +++ b/.gitea/workflows/deploy.yml @@ -21,7 +21,8 @@ jobs: local.test_dependency_lock \ local.test_staging_readiness \ deploy.test_production_preflight \ - local.test_pricing -v + local.test_pricing \ + local.test_secrets -v sh -n local/lock_dependencies.sh integration: diff --git a/deploy/production_preflight.py b/deploy/production_preflight.py index 8054e74..2287b2d 100644 --- a/deploy/production_preflight.py +++ b/deploy/production_preflight.py @@ -17,7 +17,7 @@ REQUIRED = ( 'IMAGE_TAG', 'PUBLIC_ORIGIN', 'PUBLIC_HOST', 'KANBAN_HOST', 'SITE_PORT', 'KANBAN_PORT', 'R2_ENDPOINT', 'R2_PUBLIC_ENDPOINT', 'R2_BUCKET', - 'POSTGRES_DB', 'POSTGRES_USER', 'APP_DB_USER', 'POSTGRES_VOLUME', 'OPERATOR_USER', + 'POSTGRES_DB', 'POSTGRES_USER', 'APP_DB_USER', 'POSTGRES_VOLUME', 'OPERATOR_EMAIL', 'STORAGE_QUOTA_BYTES', 'OWNER_UPLOAD_QUOTA_BYTES', 'MAX_UPLOAD_BYTES', 'UPLOAD_PART_BYTES', 'MAX_PENDING_UPLOADS', 'SCAN_MAX_BYTES', 'PAYMENT_ADAPTER', 'FREIGHT_ADAPTER', 'TINY_ADAPTER', 'WHATSAPP_ADAPTER', @@ -33,13 +33,12 @@ IMAGE_REPOSITORY = re.compile( DNS = re.compile(r'^(?=.{1,253}$)(?:[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.)+[a-z]{2,63}$') NAME = re.compile(r'^[a-zA-Z0-9][a-zA-Z0-9_.-]{2,127}$') SOURCE_BLOCKERS = { - 'local/adapters.py': ( - 'This runtime only supports APP_ENV=local', - 'Only local S3 storage is supported', - ), + # Markers must name something that is still true, or the gate weakens without + # failing. Four entries here described a local-only runtime and stopped + # matching when R2 support landed; they were removed rather than left to rot. + # What remains is the real blocker: no production payment or messaging adapter + # exists, so these lines must change before a release can be meaningful. 'local/app.py': ( - "allowed_hosts=['localhost', '127.0.0.1']", - "'environment': 'local'", 'payment = FakePayment()', ), 'local/worker.py': ( @@ -55,12 +54,63 @@ def source_errors(root=ROOT): for marker in markers: if marker in text: errors.append(f'{relative} remains local-only: {marker}') - secrets_module = root / 'local' / 'secrets.py' - if not secrets_module.exists() or 'DATABASE_URL_FILE' not in secrets_module.read_text(): - errors.append('local runtime does not load the production Docker secret *_FILE settings') + errors.extend(secret_loading_errors(root)) return errors +def secret_loading_errors(root=ROOT): + """Exercise the secret loader instead of grepping it. + + Searching for a string passes as soon as someone writes that string, and + fails when a working implementation happens to spell it differently. Load the + module and make it resolve a real file. + """ + import importlib.util + import tempfile + + module_path = root / 'local' / 'secrets.py' + if not module_path.exists(): + return ['local runtime does not load the production Docker secret *_FILE settings'] + try: + spec = importlib.util.spec_from_file_location('_preflight_secrets', module_path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + except Exception as exc: + return [f'local/secrets.py could not be loaded: {exc}'] + + stack_names = set() + stack = root / 'deploy' / 'stack.yaml' + if stack.exists(): + for line in stack.read_text().splitlines(): + if '_FILE:' in line: + key = line.split(':')[0].strip() + # The database image consumes this one; the application does not. + if key and key != 'POSTGRES_PASSWORD_FILE': + stack_names.add(key[:-len('_FILE')]) + + failures = [] + with tempfile.TemporaryDirectory() as directory: + for name in sorted(stack_names): + path = Path(directory) / name + path.write_text('resolved-value\n', encoding='utf-8') + environ = {f'{name}_FILE': str(path)} + try: + module.load(environ) + except Exception as exc: + failures.append(f'{name}_FILE is not resolved by local/secrets.py: {exc}') + continue + if environ.get(name) != 'resolved-value': + failures.append(f'{name}_FILE did not produce {name}') + # A missing secret must stop the service, never start it unconfigured. + try: + module.load({'DATABASE_URL_FILE': str(Path(directory) / 'absent')}) + except Exception: + pass + else: + failures.append('local/secrets.py does not fail closed on an unreadable secret') + return failures + + def config_errors(values): errors = [] for name in APPROVALS: diff --git a/deploy/stack.yaml b/deploy/stack.yaml index 0748867..6c07ddf 100644 --- a/deploy/stack.yaml +++ b/deploy/stack.yaml @@ -9,7 +9,8 @@ x-app-environment: &app-environment AWS_ACCESS_KEY_ID_FILE: /run/secrets/r2_access_key_id AWS_SECRET_ACCESS_KEY_FILE: /run/secrets/r2_secret_access_key AWS_DEFAULT_REGION: auto - OPERATOR_USER: ${OPERATOR_USER:?set OPERATOR_USER} + # The Kanban authenticates by email; the runtime reads OPERATOR_EMAIL. + OPERATOR_EMAIL: ${OPERATOR_EMAIL:?set OPERATOR_EMAIL} OPERATOR_PASSWORD_FILE: /run/secrets/operator_password PAYMENT_ADAPTER: ${PAYMENT_ADAPTER:?set PAYMENT_ADAPTER} FREIGHT_ADAPTER: ${FREIGHT_ADAPTER:?set FREIGHT_ADAPTER} diff --git a/deploy/test_production_preflight.py b/deploy/test_production_preflight.py index ad02aa6..eb3de97 100644 --- a/deploy/test_production_preflight.py +++ b/deploy/test_production_preflight.py @@ -27,7 +27,7 @@ def valid_config(): 'POSTGRES_USER': 'dtf_admin', 'APP_DB_USER': 'dtf_app', 'POSTGRES_VOLUME': 'dtf-postgres-data', - 'OPERATOR_USER': 'dtf-operator', + 'OPERATOR_EMAIL': 'operador@example.com', 'STORAGE_QUOTA_BYTES': '53687091200', 'OWNER_UPLOAD_QUOTA_BYTES': '10737418240', 'MAX_UPLOAD_BYTES': '5368709120', diff --git a/local/app.py b/local/app.py index e09f6fd..ad470a8 100644 --- a/local/app.py +++ b/local/app.py @@ -14,12 +14,14 @@ from starlette.middleware.trustedhost import TrustedHostMiddleware from psycopg.types.json import Jsonb from . import db +from .secrets import load as load_secret_files from .adapters import FakeFreight, FakePayment, LocalS3Storage, require_runtime from .models import Freight, Move, Pay, QuoteRequest, Review, UploadStart, OperatorLogin from .pricing import price from .auth import COOKIE_SECURE, client_ip, owner, session_row, new_session, operator, throttle, audit, rate_limit from .scanning import require_clean +load_secret_files() require_runtime() storage = LocalS3Storage() payment = FakePayment() diff --git a/local/bootstrap.py b/local/bootstrap.py index 71de430..38e8717 100644 --- a/local/bootstrap.py +++ b/local/bootstrap.py @@ -4,6 +4,7 @@ from pathlib import Path from urllib.parse import urlparse import psycopg from psycopg import sql +from .secrets import load as load_secret_files def admin_connect(): @@ -25,6 +26,7 @@ def admin_password(): def main(): + load_secret_files() role = os.environ['APP_DB_USER'] password = os.environ['APP_DB_PASSWORD'] if password == admin_password(): diff --git a/local/secrets.py b/local/secrets.py new file mode 100644 index 0000000..8cb5031 --- /dev/null +++ b/local/secrets.py @@ -0,0 +1,76 @@ +"""Resolve Docker secret files into the environment before configuration is read. + +Swarm mounts each secret as a file and the stack passes its path as `_FILE`. +Nothing read `_FILE` settings, so `deploy/stack.yaml` could not boot: the runtime +looked for `DATABASE_URL`, `AWS_ACCESS_KEY_ID` and `OPERATOR_PASSWORD` while the +stack supplied only the `_FILE` form. + +Call `load()` in every entrypoint before any configuration is read. + +Note for readers: this module is `local.secrets`. Python 3 resolves `import +secrets` elsewhere in the package to the standard library, not to this file. +""" +import os + +# The settings production supplies as secret files. Any other `*_FILE` variable is +# resolved the same way; this list documents the contract and is what the release +# gate checks against, so keep it in step with `deploy/stack.yaml`. +SECRET_FILE_SETTINGS = ( + 'DATABASE_URL', + 'DATABASE_ADMIN_URL', + 'DATABASE_PASSWORD', + 'DATABASE_ADMIN_PASSWORD', + 'APP_DB_PASSWORD', + 'AWS_ACCESS_KEY_ID', + 'AWS_SECRET_ACCESS_KEY', + 'OPERATOR_PASSWORD', + 'PAYMENT_TOKEN', + 'PAYMENT_WEBHOOK_SECRET', + 'TINY_TOKEN', + 'WHATSAPP_TOKEN', +) + +SUFFIX = '_FILE' + + +def read_secret(path): + """One secret's value, without the newline an editor or `docker secret` adds. + + Only a single trailing newline is removed: everything else is part of the + value, because a generated password may legitimately end in whitespace. + """ + with open(path, 'r', encoding='utf-8') as handle: + value = handle.read() + if value.endswith('\r\n'): + return value[:-2] + if value.endswith('\n'): + return value[:-1] + return value + + +def load(environ=None): + """Replace every `_FILE` path with `` holding the file's contents. + + Fails closed. An unreadable secret, an empty one, or a name supplied both + directly and as a file is a configuration error, and starting anyway would + mean running with a credential nobody intended. Never logs a value. + """ + environ = os.environ if environ is None else environ + resolved = [] + for key in sorted(k for k in environ if k.endswith(SUFFIX) and len(k) > len(SUFFIX)): + name = key[:-len(SUFFIX)] + path = environ[key].strip() + if not path: + raise RuntimeError(f'{key} is set but empty; point it at a secret file') + if environ.get(name): + raise RuntimeError( + f'{name} and {key} are both set; supply the value or the file, not both') + try: + value = read_secret(path) + except OSError as exc: + raise RuntimeError(f'{key} could not be read: {exc.strerror}') from None + if not value: + raise RuntimeError(f'{key} points at an empty secret file') + environ[name] = value + resolved.append(name) + return resolved diff --git a/local/test_secrets.py b/local/test_secrets.py new file mode 100644 index 0000000..3ac5420 --- /dev/null +++ b/local/test_secrets.py @@ -0,0 +1,93 @@ +"""Docker secret-file resolution. Standard library only; no stack required.""" +import tempfile +import unittest +from pathlib import Path + +from local.secrets import SECRET_FILE_SETTINGS, load, read_secret + + +class SecretFileTests(unittest.TestCase): + def setUp(self): + self.dir = tempfile.TemporaryDirectory() + self.addCleanup(self.dir.cleanup) + + def secret(self, content, name='secret'): + path = Path(self.dir.name) / name + path.write_text(content, encoding='utf-8') + return str(path) + + def test_resolves_file_into_plain_setting(self): + env = {'DATABASE_URL_FILE': self.secret('postgresql://u:p@db:5432/dtf\n')} + self.assertEqual(load(env), ['DATABASE_URL']) + self.assertEqual(env['DATABASE_URL'], 'postgresql://u:p@db:5432/dtf') + + def test_strips_only_one_trailing_newline(self): + # A generated password may legitimately end in whitespace, so only the + # newline `docker secret` or an editor appends may be removed. + self.assertEqual(read_secret(self.secret('p@ss \n')), 'p@ss ') + self.assertEqual(read_secret(self.secret('p@ss\n\n')), 'p@ss\n') + self.assertEqual(read_secret(self.secret('p@ss\r\n')), 'p@ss') + self.assertEqual(read_secret(self.secret('p@ss')), 'p@ss') + + def test_preserves_characters_that_would_break_a_url(self): + value = 'p@ss:w/rd?#[]&=+$ ,%' + env = {'OPERATOR_PASSWORD_FILE': self.secret(value + '\n')} + load(env) + self.assertEqual(env['OPERATOR_PASSWORD'], value) + + def test_resolves_every_documented_production_setting(self): + env = {f'{name}_FILE': self.secret(f'value-for-{name}', name) + for name in SECRET_FILE_SETTINGS} + load(env) + for name in SECRET_FILE_SETTINGS: + self.assertEqual(env[name], f'value-for-{name}') + + def test_missing_file_fails_closed(self): + env = {'DATABASE_URL_FILE': str(Path(self.dir.name) / 'absent')} + with self.assertRaises(RuntimeError) as caught: + load(env) + self.assertIn('DATABASE_URL_FILE', str(caught.exception)) + self.assertNotIn('DATABASE_URL', env) + + def test_empty_secret_fails_closed(self): + with self.assertRaises(RuntimeError): + load({'OPERATOR_PASSWORD_FILE': self.secret('\n')}) + + def test_empty_path_fails_closed(self): + with self.assertRaises(RuntimeError): + load({'OPERATOR_PASSWORD_FILE': ' '}) + + def test_value_and_file_together_is_ambiguous(self): + env = {'OPERATOR_PASSWORD': 'inline', + 'OPERATOR_PASSWORD_FILE': self.secret('from-file')} + with self.assertRaises(RuntimeError): + load(env) + self.assertEqual(env['OPERATOR_PASSWORD'], 'inline') + + def test_never_puts_a_secret_in_the_error_text(self): + value = 'super-secret-value' + env = {'TINY_TOKEN': value, 'TINY_TOKEN_FILE': self.secret(value)} + with self.assertRaises(RuntimeError) as caught: + load(env) + self.assertNotIn(value, str(caught.exception)) + + def test_ignores_a_bare_suffix_and_leaves_other_settings_alone(self): + env = {'_FILE': '/nowhere', 'APP_ENV': 'production'} + self.assertEqual(load(env), []) + self.assertEqual(env['APP_ENV'], 'production') + + def test_documented_list_matches_the_production_stack(self): + stack = Path(__file__).resolve().parent.parent / 'deploy' / 'stack.yaml' + if not stack.exists(): + self.skipTest('production stack definition not present') + text = stack.read_text() + # POSTGRES_PASSWORD_FILE is consumed by the database image, not by us. + used = {line.split(':')[0].strip() for line in text.splitlines() + if '_FILE:' in line and 'POSTGRES_PASSWORD_FILE' not in line} + for key in used: + self.assertIn(key[:-len('_FILE')], SECRET_FILE_SETTINGS, + f'{key} is passed by the stack but undocumented in secrets.py') + + +if __name__ == '__main__': + unittest.main() diff --git a/local/worker.py b/local/worker.py index c93840f..3bc1c3c 100644 --- a/local/worker.py +++ b/local/worker.py @@ -6,9 +6,11 @@ import time from http.server import BaseHTTPRequestHandler, HTTPServer from psycopg.types.json import Jsonb from .adapters import FakeTiny, FakeWhatsApp, LocalS3Storage, require_runtime +from .secrets import load as load_secret_files from .db import connect from .scanning import ClamAV, scan_loop +load_secret_files() require_runtime() adapters = {'tiny': FakeTiny(), 'whatsapp': FakeWhatsApp()} last_tick = 0.0