fix(backup): widen mode + fail fast when backup dir isn't writable (#1)

mkdir was 0755 — owner-only write. The dir often gets created once at
deploy time as root or whoever ran the first artisan command, then
www-data tries to write to it at runtime and bash redirects fail with
"Permission denied" mid-pipeline (after mysqldump has already started
streaming, leaving a 0-byte .enc behind).

- mkdir(0775) so group writes too; ensure-group-write is the typical
  pattern for shared deploy/runtime users.
- Best-effort chmod 0775 on existing dirs to repair narrow modes when
  we own the path.
- is_writable() pre-flight before kicking off any pipeline. Throws a
  RuntimeException with the exact `chown` + `chmod` command to run,
  including the discovered owner and the current process user — so the
  fix is one paste away instead of grepping man pages.

This pairs with docker-laravel's start-container change that pre-creates
storage/backups/ owned by www-data on every boot. Either layer alone is
enough; both together means the failure mode disappears whether the
deployment uses our image or not.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

README.md

@@ -0,0 +1,5 @@
+[![Blax Software OSS](https://raw.githubusercontent.com/blax-software/laravel-workkit/master/art/oss-initiative-banner.svg)](https://github.com/blax-software)
+
+# Laravel Workkit
+
+A Laravel collection of helpers and utilities to reduce redundant code over multiple projects.

config/workkit.php

@@ -16,5 +16,8 @@ return [
     'backup' => [
         'path' => env('WORKKIT_BACKUP_PATH'),  // null → storage_path('backups')
         'retention_days' => (int) env('WORKKIT_BACKUP_RETENTION_DAYS', 30),
+        // xz compression level. Lower = faster + larger output. 3 is a
+        // good default for SQL dumps (~10× ratio at ~3× the speed of -9).
+        'xz_level' => (int) env('WORKKIT_BACKUP_XZ_LEVEL', 3),
     ],
 ];

src/Commands/Database/BackupCommand.php

@@ -9,20 +9,22 @@ use Illuminate\Console\Command;
 use RuntimeException;
 
 /**
- * Dump the configured MySQL database to a compressed + APP_KEY-encrypted
+ * Stream a MySQL dump through xz and openssl into a single
- * file under storage/backups. Output filename is
+ * APP_KEY-encrypted file under storage/backups. The whole pipeline
- *   db_<connection>_<timestamp>.sql.xz.enc
+ * is one shell pipe (mysqldump | xz | openssl); PHP holds zero bytes
+ * of database content in memory regardless of dump size.
  *
- * The DB password is passed to mysqldump via the MYSQL_PWD env var, not
+ * Output filename:
- * on the CLI — process listings (`ps auxf`) don't expose it that way.
+ *   storage/backups/db_<connection>_<timestamp>.sql.xz.enc
  */
 class BackupCommand extends Command
 {
     protected $signature = 'workkit:db:backup
         {--connection= : DB connection to back up (defaults to config(database.default))}
-        {--out= : Custom output path (overrides storage/backups default)}';
+        {--out= : Custom output path (overrides storage/backups default)}
+        {--xz-level= : xz compression level 0–9 (default: 3 — fast, ~10× ratio for SQL)}';
 
-    protected $description = 'Create a compressed + encrypted backup of the configured MySQL database.';
+    protected $description = 'Create a streamed, compressed + APP_KEY-encrypted backup of the configured MySQL database.';
 
     public function handle(): int
     {
@@ -38,74 +40,38 @@ class BackupCommand extends Command
             return self::FAILURE;
         }
 
-        try {
-            BackupService::requireBinary('mysqldump');
-        } catch (RuntimeException $e) {
-            $this->error($e->getMessage());
-            return self::FAILURE;
-        }
-
         $stamp = date('Y-m-d_H-i-s');
         $base = BackupService::backupDirectory();
-        $sqlPath = $this->option('out')
+        $outPath = $this->option('out')
-            ?: "{$base}/db_{$connection}_{$stamp}.sql";
+            ?: "{$base}/db_{$connection}_{$stamp}.sql.xz.enc";
 
-        $this->info("Dumping `{$cfg['database']}` from {$cfg['host']} → {$sqlPath}");
+        $xzLevel = (int) ($this->option('xz-level') ?? config('workkit.backup.xz_level', 3));
 
+        $this->info(sprintf(
+            'Streaming dump → xz -%d → openssl → %s',
+            $xzLevel,
+            $outPath,
+        ));
+
+        $startedAt = microtime(true);
         try {
-            $this->dump($cfg, $sqlPath);
+            BackupService::dumpCompressEncrypt($cfg, $outPath, $xzLevel);
-
-            $this->info('Compressing with xz…');
-            $xzPath = BackupService::compressFile($sqlPath);
-            @unlink($sqlPath);
-
-            $this->info('Encrypting with APP_KEY…');
-            $encPath = BackupService::encryptFile($xzPath);
-            @unlink($xzPath);
         } catch (RuntimeException $e) {
-            // Clean up any half-written intermediate files so a failed
-            // backup doesn't leave SQL with secrets sitting unencrypted
-            // on disk.
-            foreach ([$sqlPath, $sqlPath . '.xz'] as $leftover) {
-                if (is_file($leftover)) {
-                    @unlink($leftover);
-                }
-            }
             $this->error($e->getMessage());
             return self::FAILURE;
         }
 
-        $size = filesize($encPath);
+        $elapsed = microtime(true) - $startedAt;
-        $this->info(sprintf('Backup complete: %s (%s)', $encPath, self::humanBytes((int) $size)));
+        $size = filesize($outPath);
+        $this->info(sprintf(
+            'Backup complete in %.1fs: %s (%s)',
+            $elapsed,
+            $outPath,
+            self::humanBytes((int) $size),
+        ));
         return self::SUCCESS;
     }
 
-    /**
-     * Run mysqldump with credentials passed via env vars (MYSQL_PWD) so
-     * the password never appears in the process listing or shell history.
-     */
-    private function dump(array $cfg, string $sqlPath): void
-    {
-        $args = [
-            '--single-transaction',  // consistent dump on InnoDB without FLUSH TABLES locking
-            '--quick',               // stream rows instead of buffering
-            '--skip-lock-tables',
-            '--user=' . escapeshellarg((string) ($cfg['username'] ?? 'root')),
-            '--host=' . escapeshellarg((string) ($cfg['host'] ?? 'localhost')),
-            '--port=' . escapeshellarg((string) ($cfg['port'] ?? 3306)),
-            escapeshellarg((string) $cfg['database']),
-        ];
-
-        $cmd = 'mysqldump ' . implode(' ', $args) . ' > ' . escapeshellarg($sqlPath);
-        BackupService::run($cmd, [
-            'MYSQL_PWD' => (string) ($cfg['password'] ?? ''),
-        ]);
-
-        if (! file_exists($sqlPath) || filesize($sqlPath) === 0) {
-            throw new RuntimeException("mysqldump produced no output at {$sqlPath}");
-        }
-    }
-
     private static function humanBytes(int $bytes): string
     {
         $units = ['B', 'KB', 'MB', 'GB', 'TB'];

src/Commands/Database/RestoreCommand.php

@@ -6,18 +6,17 @@ namespace Blax\Workkit\Commands\Database;
 
 use Blax\Workkit\Services\BackupService;
 use Illuminate\Console\Command;
-use Illuminate\Contracts\Encryption\DecryptException;
 use RuntimeException;
 
 /**
- * Restore a backup produced by workkit:db:backup. Without --file, picks
+ * Restore a backup produced by workkit:db:backup. Streams the file
- * the newest backup in storage/backups by mtime. Detects .enc and .xz
+ * through openssl decrypt → xz decompress → mysql in a single pipe,
- * suffixes to decide which decode steps to run, so this also works
+ * matching the backup pipeline exactly. PHP allocates nothing for the
- * with un-encrypted or un-compressed dumps if the operator restored
+ * payload, so even multi-GB backups restore without bumping memory_limit.
- * a hand-prepared file.
  *
- * Refuses to run in production unless --force is passed: a restore is
+ * Without --file, picks the newest backup in storage/backups by mtime.
- * a destructive operation that overwrites the live database.
+ * Refuses to run unless --force is passed: a restore overwrites whatever
+ * is currently in the target database.
  */
 class RestoreCommand extends Command
 {
@@ -26,7 +25,7 @@ class RestoreCommand extends Command
         {--file= : Specific backup filename inside the backups directory (default: newest by mtime)}
         {--force : Skip the confirmation prompt}';
 
-    protected $description = 'Restore a (compressed + encrypted) database backup. Defaults to the newest file in storage/backups.';
+    protected $description = 'Restore a streaming, APP_KEY-encrypted database backup. Defaults to the newest file in storage/backups.';
 
     public function handle(): int
     {
@@ -42,13 +41,6 @@ class RestoreCommand extends Command
             return self::FAILURE;
         }
 
-        try {
-            BackupService::requireBinary('mysql');
-        } catch (RuntimeException $e) {
-            $this->error($e->getMessage());
-            return self::FAILURE;
-        }
-
         $file = $this->resolveFile();
         if (! $file) {
             $this->error('No backup found.');
@@ -72,28 +64,25 @@ class RestoreCommand extends Command
             return self::SUCCESS;
         }
 
-        $tmpSql = null;
+        $startedAt = microtime(true);
         try {
-            $tmpSql = $this->prepare($file);
+            BackupService::decryptDecompressImport($file, $cfg);
-            $this->info("Restoring → {$cfg['database']}");
-            $this->importInto($cfg, $tmpSql);
-            $this->info('Restore complete.');
-            return self::SUCCESS;
-        } catch (DecryptException $e) {
-            // Almost always means the file was encrypted with a different
-            // APP_KEY than the current one. Spell that out so the operator
-            // doesn't have to recognise the cryptographer's stack trace.
-            $this->error('Decryption failed — likely an APP_KEY mismatch between backup and current environment.');
-            $this->line("Underlying error: {$e->getMessage()}");
-            return self::FAILURE;
         } catch (RuntimeException $e) {
-            $this->error($e->getMessage());
+            $msg = $e->getMessage();
+            // openssl's password mismatch error has a recognisable shape;
+            // translate it into something an operator can act on.
+            if (str_contains($msg, 'bad decrypt') || str_contains($msg, 'bad magic')) {
+                $this->error('Decryption failed — likely an APP_KEY mismatch between this host and the host that produced the backup.');
+                $this->line($msg);
+            } else {
+                $this->error($msg);
+            }
             return self::FAILURE;
-        } finally {
-            if ($tmpSql && is_file($tmpSql)) {
-                @unlink($tmpSql);
-            }
         }
+
+        $elapsed = microtime(true) - $startedAt;
+        $this->info(sprintf('Restore complete in %.1fs.', $elapsed));
+        return self::SUCCESS;
     }
 
     /**
@@ -122,64 +111,4 @@ class RestoreCommand extends Command
         usort($files, fn($a, $b) => filemtime($b) <=> filemtime($a));
         return $files[0];
     }
-
-    /**
-     * Walk the file through decrypt + decompress as needed and write the
-     * resulting SQL to a temp path. Returns the path of the .sql file.
-     */
-    private function prepare(string $file): string
-    {
-        $current = $file;
-        $intermediate = [];
-
-        if (str_ends_with($current, '.enc')) {
-            $this->info('Decrypting…');
-            $current = BackupService::decryptFile($current, sys_get_temp_dir() . '/workkit_restore_' . uniqid() . '.dec');
-            $intermediate[] = $current;
-        }
-
-        if (str_ends_with($current, '.xz')) {
-            $this->info('Decompressing…');
-            $next = BackupService::decompressFile($current, sys_get_temp_dir() . '/workkit_restore_' . uniqid() . '.sql');
-            // Drop the .xz intermediate now that we have the .sql.
-            foreach ($intermediate as $f) {
-                if (is_file($f)) {
-                    @unlink($f);
-                }
-            }
-            $intermediate = [];
-            $current = $next;
-            $intermediate[] = $current;
-        }
-
-        // Sanity check: a real SQL dump always has at least one
-        // CREATE/INSERT/USE statement near the top. Catches the case
-        // where APP_KEY didn't match (Crypt::decryptString throws on
-        // bad key, but if someone hand-renamed a non-encrypted file to
-        // .enc this still helps).
-        $head = (string) @file_get_contents($current, false, null, 0, 8192);
-        if (! preg_match('/(INSERT\s+INTO|CREATE\s+TABLE|USE\s+`)/i', $head)) {
-            throw new RuntimeException('Decoded file does not look like a SQL dump (no CREATE/INSERT/USE found in header).');
-        }
-
-        return $current;
-    }
-
-    /**
-     * Pipe the .sql file into the mysql CLI, password via MYSQL_PWD.
-     */
-    private function importInto(array $cfg, string $sqlPath): void
-    {
-        $args = [
-            '--user=' . escapeshellarg((string) ($cfg['username'] ?? 'root')),
-            '--host=' . escapeshellarg((string) ($cfg['host'] ?? 'localhost')),
-            '--port=' . escapeshellarg((string) ($cfg['port'] ?? 3306)),
-            escapeshellarg((string) $cfg['database']),
-        ];
-
-        $cmd = 'mysql ' . implode(' ', $args) . ' < ' . escapeshellarg($sqlPath);
-        BackupService::run($cmd, [
-            'MYSQL_PWD' => (string) ($cfg['password'] ?? ''),
-        ]);
-    }
 }

src/Services/BackupService.php

@@ -4,106 +4,210 @@ declare(strict_types=1);
 
 namespace Blax\Workkit\Services;
 
-use Illuminate\Support\Facades\Crypt;
 use RuntimeException;
 
 /**
- * Compression + encryption primitives shared by the backup/restore
+ * Streaming backup pipeline. Dumps, compresses and encrypts in a single
- * commands. Encryption uses Laravel's Crypt facade, so the AES key is
+ * shell pipe so PHP holds zero bytes of the database content in memory —
- * derived from APP_KEY — same key that decrypts the rest of the app's
+ * regardless of dump size. The original implementation ran each stage
- * encrypted columns/cookies. That means a backup is restorable only
+ * separately and ran into "Allowed memory size exhausted" on
- * by a deployment that knows the host's APP_KEY.
+ * mid-three-digit-MB compressed dumps because Laravel's `Crypt::encryptString`
+ * reads the whole file, base64-encodes (+33%) and JSON-envelopes it.
  *
- * Compression goes through the system `xz` binary because it gives by
+ * Encryption: AES-256-CBC with PBKDF2 (600 000 iterations, random salt)
- * far the best ratio for repetitive SQL dump text and is cheap to
+ * via the system `openssl` binary. The passphrase is derived from
- * stream. Hosts without `xz` installed get a clear failure rather
+ * APP_KEY (the `base64:` prefix is stripped, the remainder is used
- * than silently falling back to a worse codec.
+ * verbatim — PBKDF2 stretches it into the key). A backup is restorable
+ * only by a deployment that knows the same APP_KEY.
+ *
+ * The output file format is the standard `openssl enc -salt` format,
+ * which means it's also restorable with vanilla openssl on any host:
+ *   openssl enc -d -aes-256-cbc -pbkdf2 -iter 600000 -pass env:K \
+ *     -in backup.sql.xz.enc | xz -d | mysql ...
+ *
+ * Required system binaries: `mysqldump`, `mysql`, `xz`, `openssl`,
+ * `bash` (for `set -o pipefail`). All are standard on every reasonable
+ * Linux server.
  */
 class BackupService
 {
+    public const CIPHER = 'aes-256-cbc';
+    public const PBKDF2_ITER = 600000;
+
     /**
-     * Compress $in with `xz` and return the path of the .xz output.
+     * mysqldump → xz → openssl enc → $outPath. One pipeline, zero PHP
-     * Defaults to writing alongside the input.
+     * memory pressure. Pipefail propagates a failure in any stage out
+     * to the caller as a non-zero exit code.
+     *
+     * $xzLevel defaults to 3 — empirically a good balance for SQL
+     * (about 10× compression with a fraction of xz -9's time cost).
      */
-    public static function compressFile(string $in, ?string $out = null): string
+    public static function dumpCompressEncrypt(array $cfg, string $outPath, int $xzLevel = 3): void
     {
-        $out ??= $in . '.xz';
+        self::requireBinary('mysqldump');
         self::requireBinary('xz');
-        // -9 for max ratio, -T0 for parallel encoding on whatever cores
+        self::requireBinary('openssl');
-        // the host has. -c emits to stdout so we don't clobber $in in
+        self::requireBinary('bash');
-        // place — the caller decides when to delete it.
+
-        self::run(sprintf('xz -z -9 -T0 -c %s > %s', escapeshellarg($in), escapeshellarg($out)));
+        $level = max(0, min(9, $xzLevel));
-        if (! file_exists($out)) {
+
-            throw new RuntimeException("xz compression failed; output not found at {$out}");
+        $mysqldump = 'mysqldump '
+            . '--single-transaction --quick --skip-lock-tables '
+            . '--user=' . escapeshellarg((string) ($cfg['username'] ?? 'root')) . ' '
+            . '--host=' . escapeshellarg((string) ($cfg['host'] ?? 'localhost')) . ' '
+            . '--port=' . escapeshellarg((string) ($cfg['port'] ?? 3306)) . ' '
+            . escapeshellarg((string) $cfg['database']);
+
+        $xz      = "xz -{$level} -T0";
+        $openssl = 'openssl enc -' . self::CIPHER . ' -pbkdf2 -iter ' . self::PBKDF2_ITER . ' -salt -pass env:WK_KEY';
+
+        // bash -c with pipefail: any stage failing trips the whole
+        // pipeline. Without pipefail, a mysqldump crash with xz still
+        // running would leave the output file 0-byte and the exit
+        // code 0 — silent corruption.
+        $pipeline = "{$mysqldump} | {$xz} | {$openssl} > " . escapeshellarg($outPath);
+        $cmd = '/bin/bash -c ' . escapeshellarg('set -o pipefail; ' . $pipeline);
+
+        try {
+            self::run($cmd, [
+                'MYSQL_PWD' => (string) ($cfg['password'] ?? ''),
+                'WK_KEY'    => self::passphrase(),
+            ]);
+        } catch (RuntimeException $e) {
+            // Don't leave a half-written encrypted file lying around —
+            // it's neither valid plaintext (which we'd never want)
+            // nor a complete backup, just confusing partial state.
+            if (is_file($outPath)) {
+                @unlink($outPath);
+            }
+            throw $e;
+        }
+
+        if (! file_exists($outPath) || filesize($outPath) === 0) {
+            throw new RuntimeException("Backup pipeline produced no output at {$outPath}");
         }
-        return $out;
     }
 
     /**
-     * Decompress an .xz file. If $out is null, strips the `.xz` suffix
+     * openssl dec → xz -d → mysql. Streams through the same pipeline
-     * (or generates a uniquely-named sibling if there is none).
+     * in reverse. Does no PHP-side decoding so the file size is bounded
+     * only by disk I/O.
      */
-    public static function decompressFile(string $in, ?string $out = null): string
+    public static function decryptDecompressImport(string $inPath, array $cfg): void
     {
+        self::requireBinary('mysql');
         self::requireBinary('xz');
-        if ($out === null) {
+        self::requireBinary('openssl');
-            $out = str_ends_with($in, '.xz')
+        self::requireBinary('bash');
-                ? substr($in, 0, -3)
+
-                : $in . '.decompressed';
+        // Sanity check on the file's magic bytes. `openssl enc -salt`
+        // output always starts with the literal "Salted__" header.
+        $head = (string) @file_get_contents($inPath, false, null, 0, 8);
+        if ($head !== 'Salted__') {
+            throw new RuntimeException(
+                "File at {$inPath} doesn't look like a streaming openssl backup. "
+                . 'Legacy backups (made with the pre-streaming Crypt envelope) need '
+                . 'a separate restore path; see workkit:db:restore-legacy or restore '
+                . 'manually with `Crypt::decryptString` after raising memory_limit.'
+            );
         }
-        self::run(sprintf('xz -d -T0 -c %s > %s', escapeshellarg($in), escapeshellarg($out)));
+
-        if (! file_exists($out)) {
+        $openssl = 'openssl enc -d -' . self::CIPHER
-            throw new RuntimeException("xz decompression failed; output not found at {$out}");
+            . ' -pbkdf2 -iter ' . self::PBKDF2_ITER
-        }
+            . ' -pass env:WK_KEY '
-        return $out;
+            . '-in ' . escapeshellarg($inPath);
+
+        $mysql = 'mysql '
+            . '--user=' . escapeshellarg((string) ($cfg['username'] ?? 'root')) . ' '
+            . '--host=' . escapeshellarg((string) ($cfg['host'] ?? 'localhost')) . ' '
+            . '--port=' . escapeshellarg((string) ($cfg['port'] ?? 3306)) . ' '
+            . escapeshellarg((string) $cfg['database']);
+
+        $pipeline = "{$openssl} | xz -d -T0 | {$mysql}";
+        $cmd = '/bin/bash -c ' . escapeshellarg('set -o pipefail; ' . $pipeline);
+
+        self::run($cmd, [
+            'MYSQL_PWD' => (string) ($cfg['password'] ?? ''),
+            'WK_KEY'    => self::passphrase(),
+        ]);
     }
 
     /**
-     * Encrypt $in with Crypt:: (APP_KEY-derived) and return the path
+     * Derive the openssl passphrase from APP_KEY. Laravel stores APP_KEY
-     * of the .enc output.
+     * as "base64:<random-bytes>"; we strip the prefix and feed the rest
+     * straight to openssl, which runs PBKDF2 over it to get the AES key.
+     * Same APP_KEY always derives the same key — restore is deterministic.
      */
-    public static function encryptFile(string $in, ?string $out = null): string
+    public static function passphrase(): string
     {
-        $out ??= $in . '.enc';
+        $key = (string) config('app.key');
-        $payload = Crypt::encryptString(file_get_contents($in));
+        if ($key === '') {
-        file_put_contents($out, $payload);
+            throw new RuntimeException('APP_KEY is empty. Run `php artisan key:generate` before using the backup commands.');
-        return $out;
+        }
+        if (str_starts_with($key, 'base64:')) {
+            $key = substr($key, 7);
+        }
+        return $key;
     }
 
     /**
-     * Decrypt $in (Crypt:: payload) and write to $out. If $out is null,
+     * Path of the host's backup directory, created if missing. Defaults
-     * strips the `.enc` suffix.
+     * to storage/backups; overridable via config('workkit.backup.path').
-     */
+     *
-    public static function decryptFile(string $in, ?string $out = null): string
+     * Group-writable on purpose — the dir is often created once at deploy
-    {
+     * time (as root or whoever ran the first artisan command) and then
-        if ($out === null) {
+     * written by www-data at runtime. If we can't make it writable for
-            $out = str_ends_with($in, '.enc')
+     * the current user, we throw with the exact `chown`/`chmod` to run,
-                ? substr($in, 0, -4)
+     * because the alternative is the bash redirect failing mid-pipeline
-                : $in . '.decrypted';
+     * with `Permission denied` after mysqldump has already started.
-        }
-        $payload = file_get_contents($in);
-        file_put_contents($out, Crypt::decryptString($payload));
-        return $out;
-    }
-
-    /**
-     * Path of the host's backup directory, created if missing.
-     * Defaults to storage/backups; the host can override via the
-     * `workkit.backup.path` config (published by the package).
      */
     public static function backupDirectory(): string
     {
         $path = config('workkit.backup.path') ?: storage_path('backups');
+
         if (! is_dir($path)) {
-            mkdir($path, 0755, true);
+            // Suppress because a tight parent dir or umask can race here;
+            // the is_dir() check below is the real gate.
+            @mkdir($path, 0775, true);
+            if (! is_dir($path)) {
+                throw new RuntimeException("Failed to create backup directory: {$path}");
             }
+        }
+
+        // Best-effort widen — only succeeds when we own the dir, which is
+        // exactly when the perms were already too narrow to begin with.
+        @chmod($path, 0775);
+
+        if (! is_writable($path)) {
+            $owner = '?';
+            $current = '?';
+            if (function_exists('posix_geteuid') && function_exists('posix_getpwuid')) {
+                $statOwner = @fileowner($path);
+                if ($statOwner !== false) {
+                    $owner = posix_getpwuid($statOwner)['name'] ?? (string) $statOwner;
+                }
+                $current = posix_getpwuid(posix_geteuid())['name'] ?? (string) posix_geteuid();
+            }
+            throw new RuntimeException(sprintf(
+                "Backup directory not writable: %s\n"
+                . "  owner=%s, current user=%s\n"
+                . "  Fix as root:  chown -R %s %s && chmod 0775 %s",
+                $path,
+                $owner,
+                $current,
+                $current,
+                $path,
+                $path
+            ));
+        }
+
         return rtrim($path, '/');
     }
 
     /**
-     * Bail loudly if a required system binary isn't on $PATH. We do
+     * Bail loudly if a required system binary isn't on PATH. Done early
-     * this early in each command so users get one clear message
+     * in each command so users get one clear message instead of a
-     * instead of a cryptic exec failure halfway through.
+     * cryptic exec failure halfway through.
      */
     public static function requireBinary(string $bin): void
     {
@@ -114,7 +218,10 @@ class BackupService
     }
 
     /**
-     * Run a shell command and throw on non-zero exit.
+     * Run a shell command and throw on non-zero exit, capturing stderr
+     * for the error message. Env vars are passed via proc_open's env
+     * arg — they're scoped to the child process and not visible in the
+     * host's `ps` listing.
      */
     public static function run(string $command, array $env = []): void
     {
@@ -131,15 +238,14 @@ class BackupService
         }
 
         fclose($pipes[0]);
-        // We don't need stdout — most of these commands write to files.
-        $stderr = stream_get_contents($pipes[2]);
         fclose($pipes[1]);
+        $stderr = stream_get_contents($pipes[2]);
         fclose($pipes[2]);
         $exit = proc_close($proc);
 
         if ($exit !== 0) {
             throw new RuntimeException(sprintf(
-                "Command failed (exit %d): %s\nstderr:\n%s",
+                "Command failed (exit %d):\n  %s\nstderr:\n%s",
                 $exit,
                 self::redactCommand($command),
                 trim((string) $stderr) ?: '(empty)'
@@ -149,8 +255,8 @@ class BackupService
 
     /**
      * Hide credential-looking flags in error messages so we don't dump
-     * passwords to logs. Crude but enough for the legacy CLI flag form;
+     * passwords to logs. The streaming pipeline doesn't put creds on
-     * the commands themselves prefer MYSQL_PWD env vars.
+     * the CLI (everything goes via env vars), but defence in depth.
      */
     private static function redactCommand(string $command): string
     {