mirror of
https://github.com/advplyr/audiobookshelf.git
synced 2026-08-03 07:01:56 +00:00
keep postgres backup credentials out of command argv
Pass connection parts as individual pg_dump/pg_restore args and the password via PGPASSWORD env so credentials never appear in argv, execFile error messages, notifications, or the host process list. Redact the password from any error output as a fallback, and reject non-URI DATABASE_URL values with a clear error.
This commit is contained in:
parent
182ae807bd
commit
67de627a3f
2 changed files with 116 additions and 13 deletions
|
|
@ -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 })
|
||||
|
|
|
|||
|
|
@ -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 () => {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue