Let a maintenance script find the database the site actually uses
Running `python scripts/export_catalog.py` on a deployed site died with 1045 "Access denied for user 'root'@'localhost'", naming a user that site does not have. Nothing loads .env for a plain script run. wsgi.py calls load_dotenv() for the served app and the flask CLI loads .env for itself, so both reach the real database; a bare `python scripts/...` gets neither. DATABASE_URL then stays unset, FLASK_ENV defaults to development, and config.py falls back to a hardcoded root@localhost URL. The read-only failure above is the harmless version. Six of these seven scripts WRITE - backfill, reclassify, retype, collapse, refile, import - and on any box where those fallback root credentials happen to resolve, they would have silently rewritten the wrong database with no error at all. That is also why this survived: the dev .env points at root, so the fallback matched what the developer expected and nothing ever looked wrong here. All seven now load .env from the repo root, resolved from __file__ rather than the working directory, since these are run by hand during a deployment and rarely from the repo root. tests/test_maintenance_scripts.py pins it for any future script: every script that calls create_app must call load_dotenv, before create_app rather than after it, and with an explicit path rather than a cwd-relative search.
This commit is contained in:
94
tests/test_maintenance_scripts.py
Normal file
94
tests/test_maintenance_scripts.py
Normal file
@@ -0,0 +1,94 @@
|
||||
"""Standalone maintenance scripts must load .env before reading config.
|
||||
|
||||
`wsgi.py` calls `load_dotenv()` for the served app, and the `flask` CLI loads
|
||||
.env for itself, so both reach the site's real database. A plain
|
||||
`python scripts/<name>.py` run gets neither. Without an explicit load,
|
||||
DATABASE_URL is unset, FLASK_ENV defaults to 'development', and config.py falls
|
||||
back to a hardcoded `root@localhost` URL.
|
||||
|
||||
On a real deployment that surfaced as a bare 1045 "Access denied for user
|
||||
'root'@'localhost'" from `export_catalog.py`, naming a user the site does not
|
||||
use. The read-only case is merely confusing. The dangerous case is a box where
|
||||
those root credentials DO resolve: six of these scripts write, and they would
|
||||
have silently rewritten the wrong database.
|
||||
|
||||
This went unnoticed for so long because the dev .env happens to point at root,
|
||||
so the fallback matched what the developer expected.
|
||||
"""
|
||||
|
||||
import ast
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
SCRIPTS_DIR = Path(__file__).resolve().parent.parent / 'scripts'
|
||||
|
||||
|
||||
def _scripts_that_build_the_app():
|
||||
"""Every script that calls create_app, so it resolves a database URL."""
|
||||
found = []
|
||||
for path in sorted(SCRIPTS_DIR.glob('*.py')):
|
||||
if 'create_app' in path.read_text(encoding='utf-8'):
|
||||
found.append(path)
|
||||
return found
|
||||
|
||||
|
||||
def test_there_are_scripts_to_check():
|
||||
"""Guard the guard: a glob that matches nothing passes every test below."""
|
||||
assert _scripts_that_build_the_app(), \
|
||||
'no scripts call create_app - has the layout moved?'
|
||||
|
||||
|
||||
@pytest.mark.parametrize('path', _scripts_that_build_the_app(),
|
||||
ids=lambda p: p.name)
|
||||
def test_script_loads_dotenv_before_building_the_app(path):
|
||||
"""The script calls load_dotenv, and does so before create_app runs.
|
||||
|
||||
Order matters as much as presence: config is read when create_app builds
|
||||
the app, so a load_dotenv underneath it changes nothing.
|
||||
"""
|
||||
source = path.read_text(encoding='utf-8')
|
||||
assert 'load_dotenv' in source, (
|
||||
f'{path.name} builds the app but never loads .env, so a plain '
|
||||
f'python run falls back to the hardcoded root@localhost URL'
|
||||
)
|
||||
|
||||
tree = ast.parse(source)
|
||||
load_lines = [
|
||||
node.lineno for node in ast.walk(tree)
|
||||
if isinstance(node, ast.Call)
|
||||
and getattr(node.func, 'id', None) == 'load_dotenv'
|
||||
]
|
||||
assert load_lines, f'{path.name} imports load_dotenv but never calls it'
|
||||
|
||||
create_lines = [
|
||||
node.lineno for node in ast.walk(tree)
|
||||
if isinstance(node, ast.Call)
|
||||
and getattr(node.func, 'id', None) == 'create_app'
|
||||
]
|
||||
assert create_lines, f'{path.name} references create_app but never calls it'
|
||||
|
||||
assert min(load_lines) < min(create_lines), (
|
||||
f'{path.name} calls load_dotenv at line {min(load_lines)}, after '
|
||||
f'create_app at line {min(create_lines)} - config is already read'
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize('path', _scripts_that_build_the_app(),
|
||||
ids=lambda p: p.name)
|
||||
def test_dotenv_is_located_from_the_script_not_the_cwd(path):
|
||||
"""load_dotenv is given an explicit repo-root path.
|
||||
|
||||
A bare `load_dotenv()` searches upward from the CURRENT DIRECTORY, so the
|
||||
same command works from the repo root and silently finds nothing when run
|
||||
from anywhere else - the failure these scripts are most likely to hit,
|
||||
since they are run by hand during a deployment.
|
||||
"""
|
||||
tree = ast.parse(path.read_text(encoding='utf-8'))
|
||||
for node in ast.walk(tree):
|
||||
if (isinstance(node, ast.Call)
|
||||
and getattr(node.func, 'id', None) == 'load_dotenv'):
|
||||
assert node.args or node.keywords, (
|
||||
f'{path.name} calls load_dotenv() with no path, so it resolves '
|
||||
f'.env relative to the working directory'
|
||||
)
|
||||
Reference in New Issue
Block a user