diff --git a/docs/PROJECT-MAP.md b/docs/PROJECT-MAP.md index 1180d821..e28efa28 100644 --- a/docs/PROJECT-MAP.md +++ b/docs/PROJECT-MAP.md @@ -91,6 +91,6 @@ Manifest-less directories under `plugins/` are core frontend surface and always ## Size -- test functions defined: **1852** (parametrised cases collect higher) +- test functions defined: **1854** (parametrised cases collect higher) - documented API paths: **376** (`docs/openapi.json`, regenerate with `scripts/gen_openapi.py`) diff --git a/scripts/backup/backup.sh b/scripts/backup/backup.sh index 95fee553..6fa6843e 100755 --- a/scripts/backup/backup.sh +++ b/scripts/backup/backup.sh @@ -47,6 +47,12 @@ mkdir -p "${SETDIR}" status_write backup database "${SETNAME}" "Dumping database" # 1. Database: whole-DB logical dump, consistent and non-blocking on InnoDB. +# Dumped WITHOUT --databases on purpose. That flag bakes CREATE DATABASE and USE +# into the file, which pins the dump to the schema name it was taken +# from: restoring onto a site whose MYSQL_DATABASE differs loaded the rows into +# the dump's schema, left the live one untouched, and reported success. +# --no-tablespaces because dumping tablespace metadata needs the PROCESS +# privilege, which the app user does not have on MySQL 8. log "dumping database ${MYSQL_DATABASE} from ${MYSQL_HOST}" mysqldump \ --host="${MYSQL_HOST}" \ @@ -55,7 +61,8 @@ mysqldump \ --single-transaction \ --routines \ --triggers \ - --databases "${MYSQL_DATABASE}" \ + --no-tablespaces \ + "${MYSQL_DATABASE}" \ | gzip -c > "${SETDIR}/db.sql.gz" log "database dump written ($(du -h "${SETDIR}/db.sql.gz" | cut -f1))" @@ -73,7 +80,15 @@ DATAREV="$(mysql \ --skip-column-names --batch \ -e "SELECT version_num FROM alembic_version LIMIT 1;" \ "${MYSQL_DATABASE}" 2>/dev/null | tr -d '\r')" -[ -n "${DATAREV}" ] || log "WARN could not read alembic_version - datarev will be empty" +if [ -z "${DATAREV}" ]; then + # Recorded loudly rather than shrugged off: a set with no data revision + # cannot be gated on restore, and the gate treats an empty stamp as "no + # evidence" and refuses it. Better a visible warning now than a set that + # looks restorable and is not. + log "WARN could not read alembic_version - this set will NOT be restorable" + history_append backup warning "${SETNAME}" \ + "Could not read alembic_version; set has no data revision and cannot be restored" +fi # The same question for every PLUGIN chain. Each plugin owns an # alembic_version_ table, and those move independently of the core one: diff --git a/scripts/backup/restore.sh b/scripts/backup/restore.sh index de6140f0..795fa6f3 100755 --- a/scripts/backup/restore.sh +++ b/scripts/backup/restore.sh @@ -21,8 +21,22 @@ set -euo pipefail BACKUPS_ROOT="${SHOPDB_BACKUPS_ROOT:-/backups}" SETNAME="${1:-}" +# Overridable so the gate can be exercised outside a container against throwaway +# migration trees. In the image these are exactly the mounted paths, so the +# shipped behaviour is unchanged; test-restore-datarev.sh drives THIS script +# rather than a copy of its logic, which is the only way the test can notice +# this script drifting. +BACKUP_LIB_DIR="${BACKUP_LIB_DIR:-/opt/backup}" +APP_MIGRATIONS_DIR="${APP_MIGRATIONS_DIR:-/app/migrations}" +APP_PLUGINS_DIR="${APP_PLUGINS_DIR:-/app/plugins}" +APP_INSTANCE_DIR="${APP_INSTANCE_DIR:-/app/instance}" + +# When set, stop after the compatibility gate and report the verdict as the exit +# code, touching nothing. This is what makes the gate testable. +RESTORE_CHECK_ONLY="${RESTORE_CHECK_ONLY:-}" + # shellcheck source=scripts/backup/status.sh -. /opt/backup/status.sh +. "${BACKUP_LIB_DIR}/status.sh" log() { echo "[restore] $*"; } @@ -91,11 +105,12 @@ if [ -z "${SET_DATAREV}" ]; then die "cannot determine the set's data revision - refusing an unverified restore" fi + # Does the running code know this revision at all? If a migration file with that # revision id exists in the core chain, the code is at-or-past it (the head is # the tip of the same chain), so a post-load upgrade can carry the data forward. # If it is absent, the dump is NEWER than the code - refuse. -if ! grep -rqE "^revision = ['\"]${SET_DATAREV}['\"]" /app/migrations/versions 2>/dev/null; then +if ! grep -rqE "^revision = ['\"]${SET_DATAREV}['\"]" "${APP_MIGRATIONS_DIR}/versions" 2>/dev/null; then log "set datarev : ${SET_DATAREV}" die "the backup's data is at a revision this code does not have - deploy code at or past '${SET_DATAREV}' before restoring" fi @@ -128,7 +143,7 @@ if [ -z "${SET_DATAPLUGINS}" ]; then else while read -r plugin rev; do [ -n "${plugin}" ] || continue - chain="/app/plugins/${plugin}/migrations/versions" + chain="${APP_PLUGINS_DIR}/${plugin}/migrations/versions" if [ ! -d "${chain}" ]; then die "the backup carries data for plugin '${plugin}', which this build does not have - deploy a build that includes it before restoring" fi @@ -142,6 +157,31 @@ ${SET_DATAPLUGINS} PLUGINREVS fi +# Last gate: which schema does this dump target? A dump taken with --databases +# carries CREATE DATABASE and USE , pinning it to the schema it came from. +# Loaded into a site whose MYSQL_DATABASE differs, the rows land in the dump's +# schema, the live database is untouched, and the restore reports success. New +# sets are dumped without it and load into whatever schema is named here; older +# sets are still bound, so refuse rather than restore beside the live data. +# Checked HERE, with the other gates, so it costs nothing and stops before the +# safety copy rather than after it. +DUMP_SCHEMA="$(gunzip -c "${DBDUMP}" \ + | grep -m1 -oE '^USE `[^`]+`' \ + | sed -E 's/^USE `(.*)`/\1/' || true)" +if [ -n "${DUMP_SCHEMA}" ]; then + if [ "${DUMP_SCHEMA}" != "${MYSQL_DATABASE}" ]; then + die "this set is bound to schema '${DUMP_SCHEMA}' but this site's database is '${MYSQL_DATABASE}' - restoring it would load the data beside the live database and report success" + fi + log "legacy schema-bound dump for '${DUMP_SCHEMA}' - matches this site" +fi + +if [ -n "${RESTORE_CHECK_ONLY}" ]; then + log "check-only: every gate passed, stopping before anything destructive" + RESTORE_LOADED=false + trap - EXIT + exit 0 +fi + # Safety set FIRST. Everything below this point is destructive and none of it # is transactional: the dump carries DROP TABLE per table, and the instance tree # is replaced wholesale. Without a copy taken here, a restore of the wrong set - @@ -162,7 +202,7 @@ mysqldump \ --routines \ --triggers \ --no-tablespaces \ - --databases "${MYSQL_DATABASE}" \ + "${MYSQL_DATABASE}" \ | gzip -c > "${SAFETY_DIR}/db.sql.gz" \ || die "pre-restore dump failed - refusing to touch the live database" @@ -182,7 +222,8 @@ restoring set '${SETNAME}' at ${SAFETY_STAMP}. To go back to this state, load the dump the same way a restore does: - gunzip -c db.sql.gz | mysql --host=... --user=... --password=... + gunzip -c db.sql.gz | mysql --host=... --user=... --password=... \ + --database=${MYSQL_DATABASE} tar -xzf instance.tar.gz -C /app This is a local copy on the backups volume, taken for reversibility of one @@ -223,11 +264,14 @@ if command -v restic >/dev/null 2>&1 && [ -n "${RESTIC_REPOSITORY:-}" ]; then fi status_write restore database "${SETNAME}" "Loading database dump" -log "loading database dump" + + +log "loading database dump into ${MYSQL_DATABASE}" gunzip -c "${DBDUMP}" | mysql \ --host="${MYSQL_HOST}" \ --user="${MYSQL_USER}" \ --password="${MYSQL_PASSWORD}" \ + --database="${MYSQL_DATABASE}" \ || die "database load failed" # Hand off to the api container for the schema upgrade. The dump may sit at an diff --git a/scripts/backup/test-restore-datarev.sh b/scripts/backup/test-restore-datarev.sh index 39681759..7a52c030 100755 --- a/scripts/backup/test-restore-datarev.sh +++ b/scripts/backup/test-restore-datarev.sh @@ -79,43 +79,62 @@ build_dump() { } | gzip -c > "${out}" } -# Mirror of the restore.sh predicate: does the code chain know this revision? -# Kept identical to restore.sh so this test tracks the shipped decision. -code_knows_revision() { - versions_dir="$1"; datarev="$2" - grep -rqE "^revision = ['\"]${datarev}['\"]" "${versions_dir}" 2>/dev/null +# Build a throwaway plugin chain, shaped like plugins//migrations/versions. +build_plugin_chain() { + root="$1"; name="$2"; rev="$3" + versions="${root}/${name}/migrations/versions" + mkdir -p "${versions}" + printf "revision = '%s'\ndown_revision = None\n" \ + "${rev}" > "${versions}/${rev}.py" } -# Mirror of the restore.sh datarev extraction: manifest field first, else parse -# the dump. -set_datarev() { - setdir="$1"; dbdump="$2" - value="$(${PY} -c \ - 'import json,sys;print(json.load(open(sys.argv[1])).get("datarev",""))' \ - "${setdir}/manifest.json" 2>/dev/null || echo "")" - if [ -z "${value}" ]; then - value="$(gunzip -c "${dbdump}" \ - | grep -A2 -i 'INSERT INTO `alembic_version`' \ - | grep -oE "'[0-9a-zA-Z_]+'" | tr -d "'" | head -1)" - fi - printf '%s' "${value}" +# Drive the REAL restore.sh gate. This is the point of the rewrite: the test +# used to carry its own copy of the predicate, so it stayed green no matter how +# far restore.sh drifted from it. RESTORE_CHECK_ONLY stops the script right +# after the gate, so the verdict here IS the shipped verdict. +run_gate() { + backups_root="$1"; setname="$2"; chain="$3"; plugins="$4" + RESTORE_CHECK_ONLY=1 \ + SHOPDB_BACKUPS_ROOT="${backups_root}" \ + BACKUP_LIB_DIR="${SCRIPT_DIR}" \ + APP_MIGRATIONS_DIR="${chain}" \ + APP_PLUGINS_DIR="${plugins}" \ + bash "${RESTORE_SH}" "${setname}" >"${WORKDIR}/gate.log" 2>&1 } CHAIN="${WORKDIR}/migrations" build_chain "${CHAIN}" -VERSIONS="${CHAIN}/versions" +PLUGINS="${WORKDIR}/plugins" +build_plugin_chain "${PLUGINS}" 'endpoints' 'endpoints0002known' +ROOT="${WORKDIR}/backups" +mkdir -p "${ROOT}" -# --- 1. manifest.py records datarev distinct from the code head ------------- -SET1="${WORKDIR}/set1"; mkdir -p "${SET1}" -${PY} "${MANIFEST_PY}" --out "${SET1}/manifest.json" \ - --migrations "${CHAIN}" --plugins "${WORKDIR}/noplugins" \ - --datarev '0002_mid' --takenat '2024-01-01T00:00:00Z' >/dev/null 2>&1 +# Lay out one set the way a real backup does, under the backups root. +make_set() { + setname="$1"; datarev="$2"; dumprev="$3"; dataplugins="$4" + setdir="${ROOT}/${setname}" + mkdir -p "${setdir}" + if [ "${datarev}" = "__legacy__" ]; then + printf '{"takenat":"2023-06-01T00:00:00Z","schemarev":{"core":["0003_head"]}}' \ + > "${setdir}/manifest.json" + else + ${PY} "${MANIFEST_PY}" --out "${setdir}/manifest.json" \ + --migrations "${CHAIN}" --plugins "${PLUGINS}" \ + --datarev "${datarev}" --dataplugins "${dataplugins}" \ + --takenat '2024-01-01T00:00:00Z' >/dev/null 2>&1 + fi + build_dump "${setdir}/db.sql.gz" "${dumprev}" + printf '%s' "${setdir}" +} + +# --- 1. manifest.py records datarev separately from the code head ---------- +make_set 'set-behind' '0002_mid' '0002_mid' '{}' >/dev/null recorded="$(${PY} -c \ 'import json,sys;print(json.load(open(sys.argv[1])).get("datarev",""))' \ - "${SET1}/manifest.json" 2>/dev/null)" + "${ROOT}/set-behind/manifest.json" 2>/dev/null)" head_stamp="$(${PY} -c \ 'import json,sys;print(json.load(open(sys.argv[1])).get("schemarev",{}).get("core",[]))' \ - "${SET1}/manifest.json" 2>/dev/null)" + "${ROOT}/set-behind/manifest.json" 2>/dev/null)" if [ "${recorded}" = '0002_mid' ]; then pass "manifest.py records datarev (${recorded}) separate from code head ${head_stamp}" else @@ -123,38 +142,26 @@ else fi # --- 2. datarev the code KNOWS (behind head) is allowed -------------------- -DBDUMP1="${SET1}/db.sql.gz"; build_dump "${DBDUMP1}" '0002_mid' -rev="$(set_datarev "${SET1}" "${DBDUMP1}")" -if [ "${rev}" = '0002_mid' ] && code_knows_revision "${VERSIONS}" "${rev}"; then - pass "data behind code (${rev}) is known and allowed to restore" +if run_gate "${ROOT}" 'set-behind' "${CHAIN}" "${PLUGINS}"; then + pass "data behind code (0002_mid) is known and allowed to restore" else - fail "data behind code (${rev}) was not accepted" + fail "data behind code was refused: $(tail -1 "${WORKDIR}/gate.log")" fi # --- 3. datarev the code does NOT know (newer than build) is refused ------- -SET2="${WORKDIR}/set2"; mkdir -p "${SET2}" -${PY} "${MANIFEST_PY}" --out "${SET2}/manifest.json" \ - --migrations "${CHAIN}" --plugins "${WORKDIR}/noplugins" \ - --datarev '0004_future' --takenat '2024-02-01T00:00:00Z' >/dev/null 2>&1 -DBDUMP2="${SET2}/db.sql.gz"; build_dump "${DBDUMP2}" '0004_future' -rev="$(set_datarev "${SET2}" "${DBDUMP2}")" -if [ "${rev}" = '0004_future' ] && ! code_knows_revision "${VERSIONS}" "${rev}"; then - pass "data ahead of code (${rev}) is unknown and correctly refused" +make_set 'set-ahead' '0004_future' '0004_future' '{}' >/dev/null +if run_gate "${ROOT}" 'set-ahead' "${CHAIN}" "${PLUGINS}"; then + fail "data ahead of code (0004_future) was NOT refused" else - fail "data ahead of code (${rev}) was not refused" + pass "data ahead of code (0004_future) is unknown and correctly refused" fi # --- 4. legacy manifest (no datarev) falls back to the dump ---------------- -SET3="${WORKDIR}/set3"; mkdir -p "${SET3}" -# A pre-datarev manifest: schemarev present, datarev absent/empty. -printf '{"takenat":"2023-06-01T00:00:00Z","schemarev":{"core":["0003_head"]}}' \ - > "${SET3}/manifest.json" -DBDUMP3="${SET3}/db.sql.gz"; build_dump "${DBDUMP3}" '0001_baseline' -rev="$(set_datarev "${SET3}" "${DBDUMP3}")" -if [ "${rev}" = '0001_baseline' ] && code_knows_revision "${VERSIONS}" "${rev}"; then - pass "legacy set with no datarev falls back to dump revision (${rev})" +make_set 'set-legacy' '__legacy__' '0001_baseline' '{}' >/dev/null +if run_gate "${ROOT}" 'set-legacy' "${CHAIN}" "${PLUGINS}"; then + pass "legacy set with no datarev falls back to the dump revision" else - fail "legacy set fallback failed (got '${rev}')" + fail "legacy set fallback failed: $(tail -1 "${WORKDIR}/gate.log")" fi # --- 5. restore.sh hands off to the api instead of self-completing --------- @@ -166,6 +173,58 @@ else fail "restore.sh does not hand off the upgrade to the api" fi +# --- 6. a PLUGIN chain the code does not know is refused ------------------- +# The core head is identical in both of these; only the plugin revision differs. +# That is the whole failure: a core-only gate cannot tell them apart. +make_set 'set-plugin-known' '0002_mid' '0002_mid' '{"endpoints":"endpoints0002known"}' >/dev/null +if run_gate "${ROOT}" 'set-plugin-known' "${CHAIN}" "${PLUGINS}"; then + pass "a set whose plugin revision the code knows is allowed" +else + fail "known plugin revision was refused: $(tail -1 "${WORKDIR}/gate.log")" +fi + +make_set 'set-plugin-future' '0002_mid' '0002_mid' '{"endpoints":"endpoints0099future"}' >/dev/null +if run_gate "${ROOT}" 'set-plugin-future' "${CHAIN}" "${PLUGINS}"; then + fail "plugin data ahead of code was NOT refused (core-only gate regression)" +else + pass "plugin data ahead of code is refused even though the core head matches" +fi + +# --- 7. a plugin the build does not have at all is refused ----------------- +make_set 'set-plugin-missing' '0002_mid' '0002_mid' '{"nosuchplugin":"whatever0001"}' >/dev/null +if run_gate "${ROOT}" 'set-plugin-missing' "${CHAIN}" "${PLUGINS}"; then + fail "a set carrying data for an absent plugin was NOT refused" +else + pass "a set carrying data for a plugin this build lacks is refused" +fi + +# --- 8. a dump bound to another schema is refused -------------------------- +# --databases bakes 'USE ' in, which silently loaded the rows beside the +# live database and reported success. +setdir="${ROOT}/set-otherschema" +mkdir -p "${setdir}" +${PY} "${MANIFEST_PY}" --out "${setdir}/manifest.json" \ + --migrations "${CHAIN}" --plugins "${PLUGINS}" \ + --datarev '0002_mid' --dataplugins '{}' \ + --takenat '2024-01-01T00:00:00Z' >/dev/null 2>&1 +{ + echo 'CREATE DATABASE `someothersite`;' + echo 'USE `someothersite`;' + echo 'INSERT INTO `alembic_version` VALUES' + echo "('0002_mid');" +} | gzip -c > "${setdir}/db.sql.gz" +# Now a gate, so check-only reaches it without needing a database. +out="$(RESTORE_CHECK_ONLY=1 SHOPDB_BACKUPS_ROOT="${ROOT}" \ + BACKUP_LIB_DIR="${SCRIPT_DIR}" \ + APP_MIGRATIONS_DIR="${CHAIN}" APP_PLUGINS_DIR="${PLUGINS}" \ + MYSQL_DATABASE='shopdb_flask' \ + bash "${RESTORE_SH}" 'set-otherschema' 2>&1)" || true +if printf '%s' "${out}" | grep -q "bound to schema 'someothersite'"; then + pass "a dump bound to another site's schema is refused" +else + fail "a schema-bound dump was not caught (got: $(printf '%s' "${out}" | tail -1))" +fi + echo "==================================================" echo "[test] datarev gate: ${passes} passed, ${fails} failed" echo "==================================================" diff --git a/shopdb/core/api/shopdbbackups.py b/shopdb/core/api/shopdbbackups.py index 06e20584..55494b9e 100644 --- a/shopdb/core/api/shopdbbackups.py +++ b/shopdb/core/api/shopdbbackups.py @@ -462,6 +462,15 @@ def _is_compatible(manifest, current): """ if not manifest: return False + + # A manifest that CARRIES the key but left it empty is not a legacy set: it + # is a set whose data revision could not be read when it was taken. Falling + # through to the code-to-code check would badge it restorable on no + # evidence, using exactly the comparison the datarev work replaced. Only a + # manifest with no datarev key at all predates the stamp. + if 'datarev' in manifest and not manifest.get('datarev'): + return False + datarev = manifest.get('datarev') if datarev: known = _known_core_revisions() diff --git a/tests/test_core/test_shopdbbackups_finalize.py b/tests/test_core/test_shopdbbackups_finalize.py index 44b98beb..84b124d6 100644 --- a/tests/test_core/test_shopdbbackups_finalize.py +++ b/tests/test_core/test_shopdbbackups_finalize.py @@ -209,3 +209,34 @@ class TestPluginChainCompatibility: with app.app_context(): assert shopdbbackups._is_compatible(manifest, {}) is False + + +class TestDatarevEvidence: + """An unreadable data revision must not be badged restorable.""" + + def _real_core_rev(self, app): + with app.app_context(): + known = shopdbbackups._known_core_revisions() + return sorted(known)[0] + + def test_an_empty_datarev_is_no_evidence_not_a_legacy_set(self, app): + """backup.sh discards the mysql error, so datarev can be ''. + + Falling back to the code-to-code head check there would badge the set + compatible using the very comparison datarev was introduced to replace. + """ + core = self._real_core_rev(app) + manifest = {'datarev': '', 'schemarev': {'core': [core]}} + + with app.app_context(): + assert shopdbbackups._is_compatible( + manifest, {'core': [core]}) is False + + def test_a_manifest_with_no_datarev_key_still_falls_back(self, app): + """A genuine legacy set predates the field entirely and keeps working.""" + core = self._real_core_rev(app) + manifest = {'schemarev': {'core': [core]}} + + with app.app_context(): + assert shopdbbackups._is_compatible( + manifest, {'core': [core]}) is True