diff --git a/server/managers/BackupManager.js b/server/managers/BackupManager.js index c91def08c..e27c2f111 100644 --- a/server/managers/BackupManager.js +++ b/server/managers/BackupManager.js @@ -588,6 +588,31 @@ class BackupManager { }) } + /** + * Build pg_dump/pg_restore connection arguments from DATABASE_URL without + * exposing credentials in argv. execFile error messages and the host process + * list include argv, so the password is passed via PGPASSWORD env instead. + */ + getPostgresConnection() { + let dbUrl + try { + dbUrl = new URL(Database.dbPath) + } catch (error) { + throw new Error('DATABASE_URL must be a valid postgres connection URI to run backups') + } + + const args = ['--host', dbUrl.hostname, '--dbname', decodeURIComponent(dbUrl.pathname.replace(/^\//, ''))] + if (dbUrl.port) args.push('--port', dbUrl.port) + if (dbUrl.username) args.push('--username', decodeURIComponent(dbUrl.username)) + + // Redact both the percent-encoded and decoded password from any error output + const decodedPassword = dbUrl.password ? decodeURIComponent(dbUrl.password) : null + const secrets = dbUrl.password ? [dbUrl.password, decodedPassword] : [] + const env = decodedPassword ? { ...process.env, PGPASSWORD: decodedPassword } : process.env + + return { args, env, secrets } + } + backupPostgresDb(backup) { const dbFilePath = Path.join(global.ConfigPath, `absdatabase.${backup.id}.postgres.dump`) return this.runPostgresCommand('pg_dump', [ @@ -595,9 +620,7 @@ class BackupManager { '--no-owner', '--no-acl', '--file', - dbFilePath, - '--dbname', - Database.dbPath + dbFilePath ]) .then(() => dbFilePath) .catch(async (error) => { @@ -614,17 +637,33 @@ class BackupManager { '--single-transaction', '--no-owner', '--no-acl', - '--dbname', - Database.dbPath, dbFilePath ]) } runPostgresCommand(command, args) { return new Promise((resolve, reject) => { - childProcess.execFile(command, args, { maxBuffer: 10 * 1024 * 1024 }, (error, stdout, stderr) => { + let connection + try { + connection = this.getPostgresConnection() + } catch (error) { + return reject(error) + } + + const redact = (text) => { + if (typeof text !== 'string') return text + return connection.secrets.reduce((redacted, secret) => redacted.split(secret).join('***'), text) + } + + const options = { + maxBuffer: 10 * 1024 * 1024, + env: connection.env + } + childProcess.execFile(command, [...args, ...connection.args], options, (error, stdout, stderr) => { if (error) { - error.stderr = stderr + error.message = redact(error.message) + if (error.cmd) error.cmd = redact(error.cmd) + error.stderr = redact(stderr) return reject(error) } resolve({ stdout, stderr }) diff --git a/test/server/managers/BackupManager.test.js b/test/server/managers/BackupManager.test.js index c17a529a1..b5bbcc311 100644 --- a/test/server/managers/BackupManager.test.js +++ b/test/server/managers/BackupManager.test.js @@ -42,9 +42,9 @@ describe('BackupManager', () => { }) }) - it('should create Postgres dumps with pg_dump and preserve the connection URL', async () => { + it('should create Postgres dumps with pg_dump without exposing credentials in argv', async () => { Database.dialect = 'postgres' - Database.dbPath = 'postgresql://localhost:5432/audiobookshelf' + Database.dbPath = 'postgresql://absuser:secretpass@localhost:5432/audiobookshelf' global.ConfigPath = os.tmpdir() const execFileStub = sinon.stub(childProcess, 'execFile').callsFake((command, args, options, callback) => { @@ -65,14 +65,22 @@ describe('BackupManager', () => { '--no-acl', '--file', dumpPath, + '--host', + 'localhost', '--dbname', - Database.dbPath + 'audiobookshelf', + '--port', + '5432', + '--username', + 'absuser' ]) + expect(execFileStub.firstCall.args[1].join(' ')).to.not.include('secretpass') + expect(execFileStub.firstCall.args[2].env.PGPASSWORD).to.equal('secretpass') }) it('should restore Postgres dumps in one transaction and clean existing objects', async () => { Database.dialect = 'postgres' - Database.dbPath = 'postgresql://localhost:5432/audiobookshelf' + Database.dbPath = 'postgresql://absuser:secretpass@localhost:5432/audiobookshelf' const execFileStub = sinon.stub(childProcess, 'execFile').callsFake((command, args, options, callback) => { callback(null, '', '') @@ -89,10 +97,66 @@ describe('BackupManager', () => { '--single-transaction', '--no-owner', '--no-acl', + '/config/absdatabase-postgres-temp.dump', + '--host', + 'localhost', '--dbname', - Database.dbPath, - '/config/absdatabase-postgres-temp.dump' + 'audiobookshelf', + '--port', + '5432', + '--username', + 'absuser' ]) + expect(execFileStub.firstCall.args[1].join(' ')).to.not.include('secretpass') + expect(execFileStub.firstCall.args[2].env.PGPASSWORD).to.equal('secretpass') + }) + + it('should redact database credentials from failed pg command errors', async () => { + Database.dialect = 'postgres' + Database.dbPath = 'postgresql://absuser:secretpass@localhost:5432/audiobookshelf' + global.ConfigPath = os.tmpdir() + + sinon.stub(childProcess, 'execFile').callsFake((command, args, options, callback) => { + const error = new Error('Command failed: pg_dump --dbname postgresql://absuser:secretpass@localhost/audiobookshelf\npg_dump: error: password authentication failed') + error.cmd = 'pg_dump --dbname postgresql://absuser:secretpass@localhost/audiobookshelf' + callback(error, '', 'connection using password secretpass failed') + }) + const manager = new BackupManager() + const backup = new Backup() + backup.id = '2026-08-02T0130' + + let error + try { + await manager.backupPostgresDb(backup) + } catch (caughtError) { + error = caughtError + } + + expect(error).to.be.an('error') + expect(error.message).to.not.include('secretpass') + expect(error.cmd).to.not.include('secretpass') + expect(error.stderr).to.not.include('secretpass') + expect(error.message).to.include('***') + }) + + it('should reject pg commands when DATABASE_URL is not a valid URI', async () => { + Database.dialect = 'postgres' + Database.dbPath = 'not a connection uri' + global.ConfigPath = os.tmpdir() + + const manager = new BackupManager() + const backup = new Backup() + backup.id = '2026-08-02T0130' + + let error + try { + await manager.backupPostgresDb(backup) + } catch (caughtError) { + error = caughtError + } + + expect(error).to.be.an('error') + expect(error.message).to.include('valid postgres connection URI') }) it('should reject SQLite backup open errors without an uncaught sqlite event', async () => {