diff --git a/deploy/backup.sh b/deploy/backup.sh index 906c3d6..0c77a51 100755 --- a/deploy/backup.sh +++ b/deploy/backup.sh @@ -29,7 +29,13 @@ grep -Ev '^(DB_PASSWORD|DB_ROOT_PASSWORD)=' "$ENV_FILE" > "$COMPOSE_ENV_FILE" COMPOSE="docker compose -p bookstore-${ENVIRONMENT} -f docker-compose.yml -f docker-compose.${ENVIRONMENT}.yml --env-file ${COMPOSE_ENV_FILE}" echo "==> dumping database" -$COMPOSE exec -T db sh -c "exec mariadb-dump -u\"\$MARIADB_USER\" -p\"\$(cat /run/secrets/db_password)\" \"\$MARIADB_DATABASE\"" \ +# --single-transaction: without it, a dump against a live site either +# table-locks for its duration (blocking writes) or, if MARIADB_USER lacks +# LOCK TABLES privilege, produces a non-atomic dump — rows written after the +# dump starts but before it reaches their table can be captured +# inconsistently with rows it already passed. InnoDB (this project's engine +# throughout) supports a consistent snapshot via a single transaction instead. +$COMPOSE exec -T db sh -c "exec mariadb-dump --single-transaction -u\"\$MARIADB_USER\" -p\"\$(cat /run/secrets/db_password)\" \"\$MARIADB_DATABASE\"" \ | gzip > "$BACKUP_DIR/db-${TIMESTAMP}.sql.gz" echo "==> archiving uploads" diff --git a/deploy/deploy.sh b/deploy/deploy.sh index df627e0..9cfa350 100755 --- a/deploy/deploy.sh +++ b/deploy/deploy.sh @@ -60,12 +60,19 @@ DB_ROOT_PASSWORD_VALUE="$(env_get "$ENV_FILE" DB_ROOT_PASSWORD)" : "${DB_ROOT_PASSWORD_VALUE:?set DB_ROOT_PASSWORD in ${ENV_FILE}}" printf '%s' "$DB_PASSWORD_VALUE" > "$SECRETS_DIR/db_password" printf '%s' "$DB_ROOT_PASSWORD_VALUE" > "$SECRETS_DIR/db_root_password" +# Optional, unlike the two above — an environment without a Hardcover token +# just leaves HardcoverAdapter::is_configured() false. Written unconditionally +# (even empty) so the bind mount in docker-compose.yml always has a real file +# to point at, never a directory Docker auto-creates for a missing path (the +# exact bug ENVIRONMENT-unset guards elsewhere in this file exist to prevent). +HARDCOVER_API_TOKEN_VALUE="$(env_get "$ENV_FILE" HARDCOVER_API_TOKEN)" +printf '%s' "$HARDCOVER_API_TOKEN_VALUE" > "$SECRETS_DIR/hardcover_api_token" # 644, not 600: the db/wordpress/cron containers read this as their own # (non-host-matching) container UID, e.g. www-data — chmod 600 made it # unreadable to them. The containing directory (700, above) is what # actually keeps other host users out; these just need to be world-readable # within that already-restricted directory. -chmod 644 "$SECRETS_DIR/db_password" "$SECRETS_DIR/db_root_password" +chmod 644 "$SECRETS_DIR/db_password" "$SECRETS_DIR/db_root_password" "$SECRETS_DIR/hardcover_api_token" echo "==> building and starting ${ENVIRONMENT}" $COMPOSE up -d --build diff --git a/docker-compose.yml b/docker-compose.yml index a50bf7a..27764de 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -22,10 +22,11 @@ # bind mount never touches the file's content, only its path, so it's # immune to this entirely. # -# The supplier/payment keys below are still passed the old ${VAR} way and -# remain exposed to the same class of bug — nothing consumes them yet -# (bookstore-core is still a stub), so when that code is written it should -# read `_FILE` variants the same way. +# The remaining supplier/payment keys below are still passed the old ${VAR} +# way and remain exposed to the same class of bug — nothing consumes them +# yet (bookstore-core is still a stub for those), so when that code is +# written it should read `_FILE` variants the same way HARDCOVER_API_TOKEN +# now does below. x-bookstore-env: &bookstore-env WORDPRESS_DB_HOST: db WORDPRESS_DB_NAME: ${DB_NAME} @@ -47,7 +48,7 @@ x-bookstore-env: &bookstore-env HELCIM_API_TOKEN: ${HELCIM_API_TOKEN:-} HELCIM_ACCOUNT_ID: ${HELCIM_ACCOUNT_ID:-} MAILERLITE_API_KEY: ${MAILERLITE_API_KEY:-} - HARDCOVER_API_TOKEN: ${HARDCOVER_API_TOKEN:-} + HARDCOVER_API_TOKEN_FILE: /run/secrets/hardcover_api_token services: db: @@ -60,8 +61,8 @@ services: MARIADB_ROOT_PASSWORD_FILE: /run/secrets/db_root_password volumes: - db_data:/var/lib/mysql - - ./secrets/${ENVIRONMENT}/db_password:/run/secrets/db_password:ro - - ./secrets/${ENVIRONMENT}/db_root_password:/run/secrets/db_root_password:ro + - ./secrets/${ENVIRONMENT:?ENVIRONMENT must be set}/db_password:/run/secrets/db_password:ro + - ./secrets/${ENVIRONMENT:?ENVIRONMENT must be set}/db_root_password:/run/secrets/db_root_password:ro healthcheck: test: ["CMD-SHELL", "mariadb-admin ping -h 127.0.0.1 -u$$MARIADB_USER -p\"$$(cat /run/secrets/db_password)\" --silent"] interval: 5s @@ -95,7 +96,8 @@ services: - wp_themes:/var/www/html/wp-content/themes - ./wp-content/plugins/bookstore-core:/var/www/html/wp-content/plugins/bookstore-core - ./wp-content/mu-plugins:/var/www/html/wp-content/mu-plugins - - ./secrets/${ENVIRONMENT}/db_password:/run/secrets/db_password:ro + - ./secrets/${ENVIRONMENT:?ENVIRONMENT must be set}/db_password:/run/secrets/db_password:ro + - ./secrets/${ENVIRONMENT:?ENVIRONMENT must be set}/hardcover_api_token:/run/secrets/hardcover_api_token:ro cron: build: @@ -119,7 +121,8 @@ services: - ./wp-content/plugins/bookstore-core:/var/www/html/wp-content/plugins/bookstore-core - ./wp-content/mu-plugins:/var/www/html/wp-content/mu-plugins - ./docker/cron/entrypoint.sh:/usr/local/bin/cron-entrypoint.sh:ro - - ./secrets/${ENVIRONMENT}/db_password:/run/secrets/db_password:ro + - ./secrets/${ENVIRONMENT:?ENVIRONMENT must be set}/db_password:/run/secrets/db_password:ro + - ./secrets/${ENVIRONMENT:?ENVIRONMENT must be set}/hardcover_api_token:/run/secrets/hardcover_api_token:ro volumes: db_data: diff --git a/wp-content/plugins/bookstore-core/includes/Cli/Commands.php b/wp-content/plugins/bookstore-core/includes/Cli/Commands.php index 662b181..b6a34d1 100644 --- a/wp-content/plugins/bookstore-core/includes/Cli/Commands.php +++ b/wp-content/plugins/bookstore-core/includes/Cli/Commands.php @@ -174,10 +174,23 @@ class Commands $book = HardcoverAdapter::find_book($isbn13, $work->title, $work->primary_author); if ($book) { $tags = HardcoverAdapter::tags_for_book($book); - wp_set_object_terms($product_id, $tags['genre'], TagTaxonomies::GENRE, false); - wp_set_object_terms($product_id, $tags['mood'], TagTaxonomies::MOOD, false); - wp_set_object_terms($product_id, $tags['content_warning'], TagTaxonomies::CONTENT_WARNING, false); - wp_set_object_terms($product_id, $tags['tag'], TagTaxonomies::TAG, false); + // wp_set_object_terms() with an empty array CLEARS the + // taxonomy rather than leaving it untouched — verified + // directly. Hardcover legitimately returns no moods/ + // content-warnings for plenty of books, and the schema + // here is still unverified, so an empty category must + // not be treated as "wipe whatever was there before" + // (including anything hand-tagged by an admin). + foreach ([ + TagTaxonomies::GENRE => $tags['genre'], + TagTaxonomies::MOOD => $tags['mood'], + TagTaxonomies::CONTENT_WARNING => $tags['content_warning'], + TagTaxonomies::TAG => $tags['tag'], + ] as $taxonomy => $terms) { + if (!empty($terms)) { + wp_set_object_terms($product_id, $terms, $taxonomy, false); + } + } $matched++; } else { $missed++; diff --git a/wp-content/plugins/bookstore-core/includes/Integration/HardcoverAdapter.php b/wp-content/plugins/bookstore-core/includes/Integration/HardcoverAdapter.php index 8365a7f..e600899 100644 --- a/wp-content/plugins/bookstore-core/includes/Integration/HardcoverAdapter.php +++ b/wp-content/plugins/bookstore-core/includes/Integration/HardcoverAdapter.php @@ -21,7 +21,25 @@ class HardcoverAdapter public static function is_configured(): bool { - return (bool) getenv('HARDCOVER_API_TOKEN'); + return self::token() !== null; + } + + /** + * Read from the file HARDCOVER_API_TOKEN_FILE points at, not a plain + * ${VAR}-style env var — same reasoning as DB_PASSWORD/DB_ROOT_PASSWORD + * (see docker-compose.yml): Compose's ${VAR} interpolation mangles any + * value containing `$` followed by a letter, and a plain env var is also + * visible via `docker inspect`/`/proc//environ` to anything with + * host/container access, unlike a 644 file inside a 700 directory. + */ + private static function token(): ?string + { + $file = getenv('HARDCOVER_API_TOKEN_FILE'); + if (!$file || !is_readable($file)) { + return null; + } + $value = trim((string) file_get_contents($file)); + return $value !== '' ? $value : null; } /** @@ -129,9 +147,9 @@ class HardcoverAdapter private static function query(string $query, array $variables = []): array { - $token = getenv('HARDCOVER_API_TOKEN'); + $token = self::token(); if (!$token) { - throw new \RuntimeException('HARDCOVER_API_TOKEN is not set'); + throw new \RuntimeException('HARDCOVER_API_TOKEN_FILE is not set or empty'); } $response = wp_remote_post(self::ENDPOINT, [ diff --git a/wp-content/plugins/bookstore-core/includes/Product/ProductSync.php b/wp-content/plugins/bookstore-core/includes/Product/ProductSync.php index 9553699..4009ef7 100644 --- a/wp-content/plugins/bookstore-core/includes/Product/ProductSync.php +++ b/wp-content/plugins/bookstore-core/includes/Product/ProductSync.php @@ -165,9 +165,16 @@ class ProductSync $product_id = $product->save(); update_post_meta($product_id, '_bsc_work_id', $work->work_id); - if (!$existing_id) { - Work::set_product_id((int) $work->work_id, $product_id); - } + // Unconditional, not "if new": $existing_id can be a stale pointer to + // a product that was deleted out-of-band (wp-admin, a cleanup script, + // anything that didn't also clear bsc_work.wc_product_id). In that + // case wc_get_product() above returned false, a *new* product was + // just created, and the guard this replaced (`if (!$existing_id)`) + // would have skipped repointing bsc_work at it — leaving the stale ID + // in place forever and creating another duplicate on every future + // run. This UPDATE is idempotent, so making it unconditional costs + // nothing on the normal "already correct" path. + Work::set_product_id((int) $work->work_id, (int) $product_id); self::sync_categories($product_id, $work->subjects ?? null);