From bf1beb5c99b96642c0f42094ff025c72e001a98c Mon Sep 17 00:00:00 2001 From: Hamza Younas Date: Fri, 10 Apr 2026 19:56:29 +0500 Subject: [PATCH] Append leader hostname to backup filename (#1257) Backup tar files currently use a filename that only encodes a timestamp and the leader's IP, e.g. caprover-backup-2026_04_10-19_30_00-1744312200000-ip-1_2_3_4.tar When you run several CapRover instances behind a NAT or with similar IPs, that filename is not enough to tell the backups apart after downloading them. This appends the swarm leader's hostname to the existing filename: caprover-backup-...-ip-1_2_3_4-host-captain-prod.tar The hostname is sanitized to a portable charset ([A-Za-z0-9._-]) so it is safe in filesystems and HTTP Content-Disposition headers, and the segment is omitted entirely when the leader has no hostname, so existing single-node setups see no change beyond an additional suffix when one is available. The sanitization step is exposed as a small static helper (`BackupManager.sanitizeHostnameForFilename`) so it can be unit tested in isolation. Top-level cleanup hooks in the existing test file are now gated on `process.env.CI`, matching how the integration tests in the same file are gated, so the new pure helper tests can run on a developer workstation. Closes #1257 --- src/user/system/BackupManager.ts | 29 +++++++++++++++++++++++++-- tests/backup.test.ts | 34 +++++++++++++++++++++++++++----- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/user/system/BackupManager.ts b/src/user/system/BackupManager.ts index fab064d..359da8d 100644 --- a/src/user/system/BackupManager.ts +++ b/src/user/system/BackupManager.ts @@ -59,6 +59,19 @@ export default class BackupManager { return !!this.longOperationInProgress } + /** + * Sanitizes a hostname for safe use inside the backup download + * filename. Returns only characters that are portable across + * filesystems and safe for HTTP Content-Disposition headers + * ([A-Za-z0-9._-]). Returns an empty string if there is nothing + * usable left after sanitization, in which case the caller should + * omit the hostname segment entirely. + */ + static sanitizeHostnameForFilename(hostname: string | undefined): string { + if (!hostname) return '' + return hostname.replace(/[^A-Za-z0-9._-]/g, '_') + } + startRestorationIfNeededPhase1(captainIpAddress: string) { // if (/captain/restore/restore-instructions.json does exist): // - Connect all extra nodes via SSH and get their NodeID @@ -707,18 +720,30 @@ export default class BackupManager { .then(function (tarFilePath) { const namespace = CaptainConstants.rootNameSpace let mainIP = '' + let hostname = '' nodeInfo.forEach((n) => { - if (n.isLeader) + if (n.isLeader) { mainIP = (n.ip || '').split('.').join('_') + hostname = + BackupManager.sanitizeHostnameForFilename( + n.hostname + ) + } }) + // Append the leader's hostname to the filename so users + // running multiple CapRover instances can tell their + // backups apart after downloading them. See + // https://github.com/caprover/caprover/issues/1257 + const hostSegment = hostname ? `-host-${hostname}` : '' + const now = moment() const newName = `${ CaptainConstants.captainDownloadsDirectory }/${namespace}/caprover-backup-${`${now.format( 'YYYY_MM_DD-HH_mm_ss' - )}-${now.valueOf()}`}${`-ip-${mainIP}.tar`}` + )}-${now.valueOf()}`}${`-ip-${mainIP}${hostSegment}.tar`}` fs.moveSync(tarFilePath, newName) setTimeout( diff --git a/tests/backup.test.ts b/tests/backup.test.ts index 114ecc9..fcc5f10 100644 --- a/tests/backup.test.ts +++ b/tests/backup.test.ts @@ -21,12 +21,36 @@ function cleanup() { }) } -beforeEach(() => { - return cleanup() -}) +if (process.env.CI) { + beforeEach(() => { + return cleanup() + }) -afterEach(() => { - return cleanup() + afterEach(() => { + return cleanup() + }) +} + +describe('BackupManager.sanitizeHostnameForFilename', () => { + test('returns the hostname unchanged when it only contains safe chars', () => { + expect( + BackupManager.sanitizeHostnameForFilename('captain-prod.example.com') + ).toBe('captain-prod.example.com') + }) + + test('replaces filesystem-unsafe characters with underscores', () => { + expect( + BackupManager.sanitizeHostnameForFilename('host/with bad?chars') + ).toBe('host_with_bad_chars') + }) + + test('returns an empty string for an empty hostname', () => { + expect(BackupManager.sanitizeHostnameForFilename('')).toBe('') + }) + + test('returns an empty string for an undefined hostname', () => { + expect(BackupManager.sanitizeHostnameForFilename(undefined)).toBe('') + }) }) if (process.env.CI) {