-
-
Notifications
You must be signed in to change notification settings - Fork 580
feat: allow HTTP and HTTPS POST as Backup location #4061
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -297,8 +297,13 @@ export class BackupController { | |||||||||||||||||||||
|
|
||||||||||||||||||||||
| this.#logger.info(`Cleaning up ${backupsToDelete.length} old backups for rule ${rule.name}`) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Delete the old backup files | ||||||||||||||||||||||
| // Delete old local backup files. For HTTP push backups, only trim history. | ||||||||||||||||||||||
| const deletionPromises = backupsToDelete.map(async (backup) => { | ||||||||||||||||||||||
| if (this.#isHttpUrl(backup.filePath)) { | ||||||||||||||||||||||
| this.#logger.info(`Removed old push backup entry: ${backup.filePath}`) | ||||||||||||||||||||||
| return backup | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+300
to
+305
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| try { | ||||||||||||||||||||||
| await fs.unlink(backup.filePath) | ||||||||||||||||||||||
| this.#logger.info(`Deleted old backup: ${backup.filePath}`) | ||||||||||||||||||||||
|
|
@@ -376,12 +381,6 @@ export class BackupController { | |||||||||||||||||||||
| */ | ||||||||||||||||||||||
| async #createBackup(logger: Logger, rule: BackupRulesConfig): Promise<PreviousBackupInfo | null> { | ||||||||||||||||||||||
| try { | ||||||||||||||||||||||
| await this.#ensureBackupDirExists() | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Determine backup directory - use rule path if specified, otherwise default | ||||||||||||||||||||||
| const backupDir = rule.backupPath ? rule.backupPath : this.#defaultBackupDir | ||||||||||||||||||||||
| await fs.mkdir(backupDir, { recursive: true }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Generate backup filename | ||||||||||||||||||||||
| const parser = this.#variableValuesController.createVariablesAndExpressionParser(null, null, null) | ||||||||||||||||||||||
| const backupName = parser.parseVariables(rule.backupNamePattern).text | ||||||||||||||||||||||
|
|
@@ -390,6 +389,21 @@ export class BackupController { | |||||||||||||||||||||
| return null | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const backupPath = rule.backupPath ? rule.backupPath : this.#defaultBackupDir | ||||||||||||||||||||||
| if (this.#isHttpUrl(backupPath)) { | ||||||||||||||||||||||
| if (rule.backupType === 'db') { | ||||||||||||||||||||||
| throw new Error('Raw database backups must be saved to a local file path, not HTTP/HTTPS URLs') | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
Comment on lines
+392
to
+396
|
||||||||||||||||||||||
|
|
||||||||||||||||||||||
| return this.#pushExportBackup(logger, backupPath, backupName, rule.backupType) | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| await this.#ensureBackupDirExists() | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Determine backup directory - use rule path if specified, otherwise default | ||||||||||||||||||||||
| const backupDir = backupPath | ||||||||||||||||||||||
| await fs.mkdir(backupDir, { recursive: true }) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| // Create the backup based on type | ||||||||||||||||||||||
| switch (rule.backupType) { | ||||||||||||||||||||||
| case 'db': { | ||||||||||||||||||||||
|
|
@@ -425,6 +439,56 @@ export class BackupController { | |||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| #isHttpUrl(value: string): boolean { | ||||||||||||||||||||||
| try { | ||||||||||||||||||||||
| const parsed = new URL(value) | ||||||||||||||||||||||
| return parsed.protocol === 'http:' || parsed.protocol === 'https:' | ||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||
| return false | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
| } | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| async #pushExportBackup( | ||||||||||||||||||||||
| logger: Logger, | ||||||||||||||||||||||
| backupUrl: string, | ||||||||||||||||||||||
| filename: string, | ||||||||||||||||||||||
| backupType: BackupRulesConfig['backupType'] | ||||||||||||||||||||||
| ): Promise<PreviousBackupInfo> { | ||||||||||||||||||||||
| const data = this.#exportController.generateCustomExport(null) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const format: ExportFormat = backupType === 'export-gz' ? 'json-gz' : backupType === 'export-json' ? 'json' : 'yaml' | ||||||||||||||||||||||
| const exportData = await stringifyExport(logger, data, `${filename}.companionconfig`, format) | ||||||||||||||||||||||
| if (!exportData) throw new Error('Failed to stringify export data') | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| const contentType = | ||||||||||||||||||||||
| backupType === 'export-gz' | ||||||||||||||||||||||
| ? 'application/gzip' | ||||||||||||||||||||||
| : backupType === 'export-yaml' | ||||||||||||||||||||||
| ? 'application/yaml' | ||||||||||||||||||||||
| : 'application/json' | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
| logger.info(`Pushing backup to ${backupUrl}`) | ||||||||||||||||||||||
|
|
||||||||||||||||||||||
|
Comment on lines
+470
to
+471
|
||||||||||||||||||||||
| logger.info(`Pushing backup to ${backupUrl}`) | |
| let redactedUrl = backupUrl | |
| try { | |
| const parsed = new URL(backupUrl) | |
| redactedUrl = `${parsed.protocol}//${parsed.host}${parsed.pathname}` | |
| } catch { | |
| // If parsing fails, fall back to the original string | |
| } | |
| logger.info(`Pushing backup to ${redactedUrl}`) |
Copilot
AI
Apr 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Returning filePath: backupUrl for push backups makes multiple history entries indistinguishable (same filePath each run). That breaks keep-count trimming and makes UI deletion ambiguous because deletion is keyed by filePath. Consider storing a unique per-backup identifier (eg ${backupUrl}#${createdAt} or include the generated filename) and/or extending the delete API to target by both filePath and createdAt.
| return { | |
| filePath: backupUrl, | |
| fileSize: typeof exportData.data === 'string' ? Buffer.byteLength(exportData.data) : exportData.data.length, | |
| createdAt: Date.now(), | |
| const createdAt = Date.now() | |
| return { | |
| filePath: `${backupUrl}#${createdAt}`, | |
| fileSize: typeof exportData.data === 'string' ? Buffer.byteLength(exportData.data) : exportData.data.length, | |
| createdAt, |
Copilot
AI
Apr 1, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipping filesystem deletion for HTTP paths is fine, but the backend removes a history entry by looking up previousBackups via filePath. If multiple push backups share the same URL, the UI can't delete a specific entry (it will remove the first match). Consider updating the delete input to include createdAt (or an explicit id) so push-backup history entries are uniquely addressable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logging the full push-backup URL can leak credentials or tokens if the URL contains userinfo/query parameters. Please redact sensitive URL components (eg strip username/password and querystring) before logging.