diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e0437ce..d7e53515 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -75,6 +75,18 @@ ADR-007 and ADR-002. ### Fixed +- **A maintenance script could not find the database the site actually uses.** + `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` does it for the served + app and the `flask` CLI does it for itself, so both reach the real database, + while a bare `python scripts/...` gets neither and falls back to a hardcoded + `root@localhost` URL. The read-only failure is the harmless version - six of + the seven scripts write, and on a box where those fallback credentials happen + to resolve they would have silently rewritten the wrong database. All seven + now load `.env` from the repo root, resolved from the script's own location + rather than the working directory. + - **A search result opened the list it sits in, not the record it found.** A NOTICE linked to `/notifications`, so clicking the row search had already identified sent the reader to the list page to find it again by hand. The id diff --git a/scripts/backfill_vendor_from_model.py b/scripts/backfill_vendor_from_model.py index d80f923f..7b51d4aa 100644 --- a/scripts/backfill_vendor_from_model.py +++ b/scripts/backfill_vendor_from_model.py @@ -33,9 +33,22 @@ Dry run by default; nothing is written without --commit. import argparse import os import sys +from pathlib import Path sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from dotenv import load_dotenv + +# Load .env before anything reads config. wsgi.py does this for the served app +# and the `flask` CLI does it for itself, but a plain `python scripts/...` run +# gets neither: DATABASE_URL stays unset, FLASK_ENV defaults to development, +# and config falls back to a hardcoded root@localhost URL. That fails on a real +# deployment with a bare 1045 Access denied for 'root', naming a user the site +# does not use - and on a box where those root credentials happen to work, it +# is worse, because the script silently reads or WRITES the wrong database. +load_dotenv(Path(__file__).resolve().parent.parent / '.env') + + # Each asset table that carries both a model link and its own vendor, with the # column that identifies a row in reports and logs. TARGETS = [ diff --git a/scripts/collapse_duplicate_backup_revisions.py b/scripts/collapse_duplicate_backup_revisions.py index d76fc82f..fe792980 100644 --- a/scripts/collapse_duplicate_backup_revisions.py +++ b/scripts/collapse_duplicate_backup_revisions.py @@ -38,11 +38,24 @@ database backup before --commit anyway; this deletes rows. import argparse import os import sys +from pathlib import Path from collections import defaultdict # Run from anywhere: put the repo root (parent of scripts/) on the path. sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from dotenv import load_dotenv + +# Load .env before anything reads config. wsgi.py does this for the served app +# and the `flask` CLI does it for itself, but a plain `python scripts/...` run +# gets neither: DATABASE_URL stays unset, FLASK_ENV defaults to development, +# and config falls back to a hardcoded root@localhost URL. That fails on a real +# deployment with a bare 1045 Access denied for 'root', naming a user the site +# does not use - and on a box where those root credentials happen to work, it +# is worse, because the script silently reads or WRITES the wrong database. +load_dotenv(Path(__file__).resolve().parent.parent / '.env') + + from shopdb import create_app from shopdb.api import db from shopdb.core.models import Asset diff --git a/scripts/export_catalog.py b/scripts/export_catalog.py index cf0d8b77..f3ba662d 100644 --- a/scripts/export_catalog.py +++ b/scripts/export_catalog.py @@ -25,10 +25,23 @@ import argparse import json import os import sys +from pathlib import Path from collections import OrderedDict sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from dotenv import load_dotenv + +# Load .env before anything reads config. wsgi.py does this for the served app +# and the `flask` CLI does it for itself, but a plain `python scripts/...` run +# gets neither: DATABASE_URL stays unset, FLASK_ENV defaults to development, +# and config falls back to a hardcoded root@localhost URL. That fails on a real +# deployment with a bare 1045 Access denied for 'root', naming a user the site +# does not use - and on a box where those root credentials happen to work, it +# is worse, because the script silently reads or WRITES the wrong database. +load_dotenv(Path(__file__).resolve().parent.parent / '.env') + + DEFAULT_OUT = os.path.join('shopdb', 'data', 'catalog.json') diff --git a/scripts/import_ntlars_backups.py b/scripts/import_ntlars_backups.py index 95b69aad..212e9c9c 100644 --- a/scripts/import_ntlars_backups.py +++ b/scripts/import_ntlars_backups.py @@ -21,10 +21,23 @@ Dry run by default. Nothing is written without --commit. import argparse import os import sys +from pathlib import Path from datetime import datetime sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from dotenv import load_dotenv + +# Load .env before anything reads config. wsgi.py does this for the served app +# and the `flask` CLI does it for itself, but a plain `python scripts/...` run +# gets neither: DATABASE_URL stays unset, FLASK_ENV defaults to development, +# and config falls back to a hardcoded root@localhost URL. That fails on a real +# deployment with a bare 1045 Access denied for 'root', naming a user the site +# does not use - and on a box where those root credentials happen to work, it +# is worse, because the script silently reads or WRITES the wrong database. +load_dotenv(Path(__file__).resolve().parent.parent / '.env') + + from shopdb import create_app # noqa: E402 from shopdb.api import db # noqa: E402 diff --git a/scripts/reclassify_servers_to_network.py b/scripts/reclassify_servers_to_network.py index ba296799..2bf50e5b 100644 --- a/scripts/reclassify_servers_to_network.py +++ b/scripts/reclassify_servers_to_network.py @@ -19,10 +19,23 @@ Usage (on the target instance, in the app dir): import argparse import os import sys +from pathlib import Path # Run from anywhere: put the repo root (parent of scripts/) on the path. sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from dotenv import load_dotenv + +# Load .env before anything reads config. wsgi.py does this for the served app +# and the `flask` CLI does it for itself, but a plain `python scripts/...` run +# gets neither: DATABASE_URL stays unset, FLASK_ENV defaults to development, +# and config falls back to a hardcoded root@localhost URL. That fails on a real +# deployment with a bare 1045 Access denied for 'root', naming a user the site +# does not use - and on a box where those root credentials happen to work, it +# is worse, because the script silently reads or WRITES the wrong database. +load_dotenv(Path(__file__).resolve().parent.parent / '.env') + + from shopdb import create_app from shopdb.api import db from shopdb.core.models import Asset, AssetType diff --git a/scripts/refile_partmarker_backups.py b/scripts/refile_partmarker_backups.py index a5f7f9f2..b29a4474 100644 --- a/scripts/refile_partmarker_backups.py +++ b/scripts/refile_partmarker_backups.py @@ -35,11 +35,24 @@ Dry run by default. Take a database backup before --commit. import argparse import os import sys +from pathlib import Path from collections import defaultdict # Run from anywhere: put the repo root (parent of scripts/) on the path. sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from dotenv import load_dotenv + +# Load .env before anything reads config. wsgi.py does this for the served app +# and the `flask` CLI does it for itself, but a plain `python scripts/...` run +# gets neither: DATABASE_URL stays unset, FLASK_ENV defaults to development, +# and config falls back to a hardcoded root@localhost URL. That fails on a real +# deployment with a bare 1045 Access denied for 'root', naming a user the site +# does not use - and on a box where those root credentials happen to work, it +# is worse, because the script silently reads or WRITES the wrong database. +load_dotenv(Path(__file__).resolve().parent.parent / '.env') + + from shopdb import create_app from shopdb.api import db, Asset, AssetRelationship, RelationshipType from plugins.backups.models import BackupRevision diff --git a/scripts/retype_models.py b/scripts/retype_models.py index 1b94843d..01f5d1af 100644 --- a/scripts/retype_models.py +++ b/scripts/retype_models.py @@ -33,9 +33,22 @@ import csv import os import re import sys +from pathlib import Path sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) +from dotenv import load_dotenv + +# Load .env before anything reads config. wsgi.py does this for the served app +# and the `flask` CLI does it for itself, but a plain `python scripts/...` run +# gets neither: DATABASE_URL stays unset, FLASK_ENV defaults to development, +# and config falls back to a hardcoded root@localhost URL. That fails on a real +# deployment with a bare 1045 Access denied for 'root', naming a user the site +# does not use - and on a box where those root credentials happen to work, it +# is worse, because the script silently reads or WRITES the wrong database. +load_dotenv(Path(__file__).resolve().parent.parent / '.env') + + # Each asset class, its own type vocabulary, and the modeltypes.category that a # newly created model type should carry. CLASSES = { diff --git a/tests/test_maintenance_scripts.py b/tests/test_maintenance_scripts.py new file mode 100644 index 00000000..ea829823 --- /dev/null +++ b/tests/test_maintenance_scripts.py @@ -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/.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' + )