From 30dfc7dc3e3efd1e4878c3270fa6f0474cd90c9b Mon Sep 17 00:00:00 2001 From: Ricardo Mancinas Date: Thu, 30 Jul 2026 16:21:00 -0700 Subject: [PATCH] fix(ops): run backups as an admin login, and stop recording failed dumps as good MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Operaciones panel (backup, restore, sync, re-import) shelled out to mysqldump as the application user, parsed straight out of DATABASE_URL. `--single-transaction` issues FLUSH TABLES, which needs the global RELOAD privilege, and the app user is granted only ALL ON jorgecuadros.* plus USAGE ON *.*. BACKUP failed outright; SYNC and REIMPORT failed with it, since both take a safety backup first. An admin credential is now supplied out of band via OPS_DB_ADMIN_USER / OPS_DB_ADMIN_PASSWORD, mirroring what deploy/scripts/pre-migrate-backup.mjs already does, rather than permanently elevating the user the API serves requests as. Host, port and database still come from DATABASE_URL, so the override can only change who logs in, never which server. Unset, it falls back to the DATABASE_URL credentials and warns — local development is unaffected. Two defects in the dumps themselves, both shared with the deploy backup before it was rewritten: - No --set-gtid-purged=OFF. The production server is the replication source with GTID on, so every dump embedded SET @@GLOBAL.GTID_PURGED and was unrestorable onto the server it came from — the one thing the restore screen is for. - The pipeline's exit status was gzip's, and gzip succeeded. A mysqldump that died on its first statement left a small, perfectly valid archive that the job recorded as SUCCESS and the restore screen listed as an ordinary restore point. Dumps now run under `set -o pipefail`, assert a CREATE TABLE count, and delete their own output on failure. Verified with a stubbed mysqldump: a failing dump exits 1, surfaces the real error, removes the partial file, and — critically — stops SYNC/REIMPORT before the ETL touches anything. Restores gained pipefail too: a corrupt archive made gunzip fail while mysql, fed a truncated stream, could still exit 0. Co-Authored-By: Claude Opus 5 --- .env.example | 8 ++ .gitea/workflows/deploy-galactus.yml | 2 + .gitea/workflows/deploy.yml | 2 + apps/api/src/ops/ops.service.ts | 96 ++++++++++++++++++-- deploy/galactus/jorgecuadros-app.compose.yml | 8 ++ deploy/jorgecuadros-app.stack.yml | 8 ++ docs/DEPLOY_AND_MIGRATIONS.md | 37 ++++++++ 7 files changed, 154 insertions(+), 7 deletions(-) diff --git a/.env.example b/.env.example index d5c3b24..ef96662 100644 --- a/.env.example +++ b/.env.example @@ -4,6 +4,14 @@ SESSION_SECRET=change-me-to-a-random-string WEB_ORIGIN=http://localhost:3000 NEXT_PUBLIC_API_ORIGIN=http://localhost:3001 +# Login the "Operaciones" screen runs mysqldump/mysql as. Optional locally: when +# unset it falls back to the DATABASE_URL credentials, which a dev MySQL usually +# grants enough for. Required in any deployment, where the application user has +# only ALL ON jorgecuadros.* and mysqldump --single-transaction needs the global +# RELOAD privilege. Host/port/database always come from DATABASE_URL. +OPS_DB_ADMIN_USER= +OPS_DB_ADMIN_PASSWORD= + # Company info — printed in the header of every report (PDF + browser # print). Leave blank to use the placeholders. COMPANY_LOGO_PATH is # optional; when unset the API falls back to apps/api/assets/company_logo.png. diff --git a/.gitea/workflows/deploy-galactus.yml b/.gitea/workflows/deploy-galactus.yml index 972058a..eedaaa9 100644 --- a/.gitea/workflows/deploy-galactus.yml +++ b/.gitea/workflows/deploy-galactus.yml @@ -257,6 +257,8 @@ jobs: "DATABASE_URL": "${{ secrets.DATABASE_URL_GALACTUS }}", "SESSION_SECRET": "${{ secrets.SESSION_SECRET_GALACTUS }}", "SESSION_COOKIE_SECURE": "false", + "OPS_DB_ADMIN_USER": "root", + "OPS_DB_ADMIN_PASSWORD": "${{ secrets.MYSQL_ROOT_PASSWORD }}", "MINIO_ROOT_USER": "${{ secrets.MINIO_ROOT_USER }}", "MINIO_ROOT_PASSWORD": "${{ secrets.MINIO_ROOT_PASSWORD }}" } diff --git a/.gitea/workflows/deploy.yml b/.gitea/workflows/deploy.yml index c52514c..6280030 100644 --- a/.gitea/workflows/deploy.yml +++ b/.gitea/workflows/deploy.yml @@ -264,6 +264,8 @@ jobs: "S3_ENDPOINT": "${{ secrets.APP_S3_ENDPOINT }}", "DATABASE_URL": "${{ secrets.DATABASE_URL }}", "SESSION_SECRET": "${{ secrets.SESSION_SECRET }}", + "OPS_DB_ADMIN_USER": "root", + "OPS_DB_ADMIN_PASSWORD": "${{ secrets.MYSQL_ROOT_PASSWORD }}", "MINIO_ROOT_USER": "${{ secrets.MINIO_ROOT_USER }}", "MINIO_ROOT_PASSWORD": "${{ secrets.MINIO_ROOT_PASSWORD }}" } diff --git a/apps/api/src/ops/ops.service.ts b/apps/api/src/ops/ops.service.ts index e4fe3c8..532a86d 100644 --- a/apps/api/src/ops/ops.service.ts +++ b/apps/api/src/ops/ops.service.ts @@ -31,6 +31,14 @@ export const INGEST_FILES = [ ] as const; export type IngestName = (typeof INGEST_FILES)[number]; +/** + * Prefix for every command containing a pipe. Without it the exit status of + * `mysqldump | gzip` is gzip's, so a dump that failed immediately still looks + * like a successful job. Both Alpine's busybox ash (the API image) and macOS + * `sh` (dev) support it; POSIX does not require it, so `sh -c` is the contract. + */ +const PIPEFAIL = "set -o pipefail; "; + interface MysqlConn { host: string; port: string; @@ -175,7 +183,7 @@ export class OpsService implements OnModuleInit { ); } - const conn = this.parseDbUrl(); + const conn = this.opsConn(); const { cmd, resolvedParams } = await this.buildCommand(kind, params, conn); const job = await this.prisma.opsJob.create({ @@ -207,6 +215,37 @@ export class OpsService implements OnModuleInit { }; } + /** + * The credentials mysqldump/mysql run as — deliberately NOT the application + * user. `--single-transaction` issues FLUSH TABLES, which needs the global + * RELOAD privilege, and the app user is granted only `ALL ON jorgecuadros.*` + * plus `USAGE ON *.*`; `--skip-lock-tables` does not avoid it. A restore of a + * dump taken before --set-gtid-purged=OFF likewise needs SUPER to replay its + * SET @@GLOBAL.GTID_PURGED. So an admin credential is supplied out of band + * rather than elevating the runtime user for the sake of one admin screen — + * the same choice deploy/scripts/pre-migrate-backup.mjs makes. + * + * Host, port and database always come from DATABASE_URL: the ops user is a + * different login on the SAME server, never a way to point at another one. + * + * With the vars unset this falls back to the DATABASE_URL credentials, which + * is what local development wants — a dev MySQL grants the app user far more. + */ + private opsConn(): MysqlConn { + const conn = this.parseDbUrl(); + const user = process.env.OPS_DB_ADMIN_USER; + const password = process.env.OPS_DB_ADMIN_PASSWORD; + if (!user || !password) { + this.logger.warn( + "OPS_DB_ADMIN_USER/OPS_DB_ADMIN_PASSWORD no configuradas; " + + `usando el usuario de la aplicación (${conn.user}) para mysqldump. ` + + "En producción esto falla por falta del privilegio RELOAD.", + ); + return conn; + } + return { ...conn, user, password }; + } + /** mysql/mysqldump connection flags. The password goes through MYSQL_PWD in * the child env, never on the command line (which would leak via `ps`). */ private connFlags(c: MysqlConn): string { @@ -217,6 +256,37 @@ export class OpsService implements OnModuleInit { return new Date().toISOString().replace(/[:.]/g, "-").replace("T", "_").slice(0, 19); } + /** + * One hardened mysqldump, shared by BACKUP and by the safety backups SYNC and + * REIMPORT take first. Kept byte-for-byte in spirit with the dump in + * deploy/scripts/pre-migrate-backup.mjs — the two write into the same volume + * and both are listed as restore points by this same screen. + * + * --set-gtid-purged=OFF: the production server is the replication SOURCE with + * GTID on, so without it every dump embeds SET @@GLOBAL.GTID_PURGED and is + * unrestorable onto the very server it came from. + * + * The table-count assertion is not belt-and-braces: `gzip -t` passes on the + * ~372-byte output of a mysqldump that died on its first statement, so a + * failed dump would otherwise be recorded as a successful backup. (`set -o + * pipefail` is set by the caller for the same reason — without it the exit + * status of the pipeline is gzip's, and gzip succeeded.) + * + * A failed attempt deletes its own output, so a truncated file never appears + * in the restore list looking like an ordinary restore point. + */ + private dumpCommand(flags: string, db: string, out: string): string { + return ( + `( mysqldump ${flags} --single-transaction --routines --triggers ` + + `--no-tablespaces --set-gtid-purged=OFF ${db} | gzip -c > ${out} && ` + + `gzip -t ${out} && ` + + `TABLAS=$(gunzip -c ${out} | grep -c 'CREATE TABLE') && ` + + `echo "tablas capturadas: $TABLAS" && ` + + `[ "$TABLAS" -ge 1 ] ) || ` + + `{ rm -f ${out}; echo 'respaldo incompleto eliminado'; exit 1; }` + ); + } + private async buildCommand( kind: OpsJobKind, params: Record, @@ -229,7 +299,7 @@ export class OpsService implements OnModuleInit { const file = `backup-${this.migrationEnv}-${this.timestamp()}.sql.gz`; const out = shq(path.join(this.backupDir, file)); return { - cmd: `mysqldump ${flags} --single-transaction --routines --triggers --no-tablespaces ${db} | gzip -c > ${out}`, + cmd: `${PIPEFAIL}${this.dumpCommand(flags, db, out)}`, resolvedParams: { file }, }; } @@ -241,7 +311,10 @@ export class OpsService implements OnModuleInit { throw new NotFoundException(`Respaldo no encontrado: ${name}`); }); return { - cmd: `gunzip -c ${shq(full)} | mysql ${flags} ${db}`, + // pipefail matters here too: a corrupt archive makes gunzip fail while + // mysql, fed a truncated stream, can still exit 0 — a restore that + // reported success having replayed only part of the dump. + cmd: `${PIPEFAIL}gunzip -c ${shq(full)} | mysql ${flags} ${db}`, resolvedParams: { file: name }, }; } @@ -252,8 +325,8 @@ export class OpsService implements OnModuleInit { const py = await this.pythonBin(); const runAll = shq(path.join(this.migrationDir, "run_all.py")); const cmd = - `echo '== Respaldo de seguridad previo ==' && ` + - `mysqldump ${flags} --single-transaction --routines --triggers --no-tablespaces ${db} | gzip -c > ${out} && ` + + `${PIPEFAIL}echo '== Respaldo de seguridad previo ==' && ` + + `${this.dumpCommand(flags, db, out)} && ` + `echo '== Sincronización aditiva desde carpeta de ingesta ==' && ` + `${shq(py)} ${runAll} --env ${shq(this.migrationEnv)} --sync`; return { cmd, resolvedParams: { safetyBackup: file } }; @@ -266,8 +339,8 @@ export class OpsService implements OnModuleInit { const py = await this.pythonBin(); const runAll = shq(path.join(this.migrationDir, "run_all.py")); const cmd = - `echo '== Respaldo de seguridad previo ==' && ` + - `mysqldump ${flags} --single-transaction --routines --triggers --no-tablespaces ${db} | gzip -c > ${out} && ` + + `${PIPEFAIL}echo '== Respaldo de seguridad previo ==' && ` + + `${this.dumpCommand(flags, db, out)} && ` + `echo '== Reimportación desde carpeta de ingesta ==' && ` + `${shq(py)} ${runAll} --env ${shq(this.migrationEnv)} --stage`; return { cmd, resolvedParams: { safetyBackup: file } }; @@ -287,6 +360,15 @@ export class OpsService implements OnModuleInit { } } + /** + * `password` is the ops credential from opsConn(), exported as MYSQL_PWD so it + * never reaches argv (which `ps` exposes to every process on the host). + * + * It does not leak into the Python ETL that SYNC and REIMPORT go on to run: + * migration/dbenv.py connects with pymysql using the credentials inside + * DATABASE_URL and never consults MYSQL_PWD. The ETL keeps running as the + * application user, which is what it should be doing. + */ private run(jobId: string, cmd: string, password: string): void { const child = spawn("sh", ["-c", cmd], { cwd: this.migrationDir, diff --git a/deploy/galactus/jorgecuadros-app.compose.yml b/deploy/galactus/jorgecuadros-app.compose.yml index 2018669..7cd152e 100644 --- a/deploy/galactus/jorgecuadros-app.compose.yml +++ b/deploy/galactus/jorgecuadros-app.compose.yml @@ -61,6 +61,14 @@ services: INGEST_DIR: /data/ingest BACKUP_DIR: /data/backups MIGRATION_ENV: prod + # Credentials the "Operaciones" screen runs mysqldump/mysql as. NOT the + # application user: --single-transaction needs the global RELOAD privilege + # and the app user has only ALL ON jorgecuadros.*, so every backup, sync + # and re-import fails without this. Host/port/database still come from + # DATABASE_URL — this only changes who logs in. See opsConn() in + # apps/api/src/ops/ops.service.ts. + OPS_DB_ADMIN_USER: ${OPS_DB_ADMIN_USER:-root} + OPS_DB_ADMIN_PASSWORD: ${OPS_DB_ADMIN_PASSWORD:?OPS_DB_ADMIN_PASSWORD must be set} S3_ENDPOINT: ${S3_ENDPOINT:?S3_ENDPOINT must be set} S3_BUCKET: ${S3_BUCKET:-jorgecuadros-documents} MINIO_ROOT_USER: ${MINIO_ROOT_USER:?MINIO_ROOT_USER must be set} diff --git a/deploy/jorgecuadros-app.stack.yml b/deploy/jorgecuadros-app.stack.yml index 1b62a1f..336ce47 100644 --- a/deploy/jorgecuadros-app.stack.yml +++ b/deploy/jorgecuadros-app.stack.yml @@ -41,6 +41,14 @@ services: INGEST_DIR: /data/ingest BACKUP_DIR: /data/backups MIGRATION_ENV: prod + # Credentials the "Operaciones" screen runs mysqldump/mysql as. NOT the + # application user: --single-transaction needs the global RELOAD privilege + # and the app user has only ALL ON jorgecuadros.*, so every backup, sync + # and re-import fails without this. Host/port/database still come from + # DATABASE_URL — this only changes who logs in. See opsConn() in + # apps/api/src/ops/ops.service.ts. + OPS_DB_ADMIN_USER: ${OPS_DB_ADMIN_USER:-root} + OPS_DB_ADMIN_PASSWORD: ${OPS_DB_ADMIN_PASSWORD:?OPS_DB_ADMIN_PASSWORD must be set} # Object storage — internal endpoint the API (server-side) uses to reach # the minio stack. Not browser-facing (downloads proxy through the API). S3_ENDPOINT: ${S3_ENDPOINT:?S3_ENDPOINT must be set} diff --git a/docs/DEPLOY_AND_MIGRATIONS.md b/docs/DEPLOY_AND_MIGRATIONS.md index 260b452..0e008b2 100644 --- a/docs/DEPLOY_AND_MIGRATIONS.md +++ b/docs/DEPLOY_AND_MIGRATIONS.md @@ -253,6 +253,43 @@ had only ever been exercised with the API running on a developer machine, where the Oracle client is installed, which is why this went unnoticed until the first containerised deploy. +## The Operaciones panel needs its own database login + +The panel's four jobs all shell out to `mysqldump`/`mysql`, and they cannot do +so as the application user. `mysqldump --single-transaction` issues +`FLUSH TABLES`, which requires the **global** `RELOAD` privilege; the MySQL +image grants the app user only `ALL PRIVILEGES ON jorgecuadros.*` plus +`USAGE ON *.*`. `--skip-lock-tables` does not avoid it. BACKUP therefore failed +outright, and SYNC and RE-IMPORT with it, because both take a safety backup +first. + +The API is given an admin login out of band rather than permanently elevating +the user it serves requests as: + +``` +OPS_DB_ADMIN_USER=root +OPS_DB_ADMIN_PASSWORD= +``` + +Both deploy workflows pass these into the app stack from the existing +`MYSQL_ROOT_PASSWORD` secret. Host, port and database still come from +`DATABASE_URL` — the override changes *who logs in*, never *which server*. With +the pair unset the service falls back to the `DATABASE_URL` credentials and logs +a warning, which is what local development wants. + +Two more things the panel's dumps now do, for the same reasons the pre-migrate +backup does them (see `deploy/scripts/pre-migrate-backup.mjs`): + +- **`--set-gtid-purged=OFF`.** galactus is the replication *source* with GTID + on, so without this every dump embeds `SET @@GLOBAL.GTID_PURGED` and cannot be + restored onto the server it came from — which is precisely what the restore + screen exists to do. +- **`set -o pipefail` and a `CREATE TABLE` count.** `mysqldump | gzip` reports + gzip's exit status, and a `mysqldump` that dies on its first statement still + produces a ~372-byte perfectly valid archive that passes `gzip -t`. Without + both checks a failed backup was recorded as a successful one and listed as an + ordinary restore point. A dump that fails now deletes its own output. + ## Known caveats in the deploy path - The pre-migrate backup step sets `NODE_TLS_REJECT_UNAUTHORIZED=0` because