fix occasional issue in backup/restore, revert recent change

Author: bwp91

CHANGELOG.md

@@ -18,6 +18,7 @@ All notable changes to `homebridge-config-ui-x` will be documented in this file.
 
 - improvements to various accessory tiles and modals
 - improvements to plugin config validation
+- fix occasional issue in backup/restore, revert recent change
 
 ### Homebridge Dependencies
 

src/modules/backup/backup.service.ts

@@ -193,7 +193,7 @@ export class BackupService {
     if (this.configService.ui.scheduledBackupPath) {
       // If using a custom backup path, check it exists
       if (!await pathExists(this.configService.instanceBackupPath)) {
-        throw new Error(`Custom instance backup path does not exist: ${this.configService.instanceBackupPath}`)
+        throw new Error('Custom instance backup path does not exist')
       }
 
       try {
@@ -308,47 +308,27 @@ export class BackupService {
    * Downloads a scheduled backup .tar.gz
    */
   async getScheduledBackup(backupId: string): Promise<StreamableFile> {
-    // Validate backupId to ensure it matches the expected pattern
-    if (!/^[0-9A-Z]{12}\.\d{9,15}$/i.test(backupId)) {
-      throw new NotFoundException()
-    }
+    const backupPath = resolve(this.configService.instanceBackupPath, `homebridge-backup-${backupId}.tar.gz`)
 
-    let backupPath = resolve(this.configService.instanceBackupPath, `homebridge-backup-${backupId}.tar.gz`)
-    try {
-      backupPath = await realpath(backupPath)
-    } catch (e) {
+    // Check the file exists
+    if (!await pathExists(backupPath)) {
       throw new NotFoundException()
     }
 
-    // Ensure the backupPath is within the instanceBackupPath
-    if (!backupPath.startsWith(this.configService.instanceBackupPath)) {
-      throw new BadRequestException(`Invalid backup path: ${backupPath} does not start with ${this.configService.instanceBackupPath}`)
-    }
-
     return new StreamableFile(createReadStream(backupPath))
   }
 
   /**
    * Removes a scheduled backup .tar.gz
    */
   async deleteScheduledBackup(backupId: string): Promise<void> {
-    // Validate backupId to ensure it matches the expected pattern
-    if (!/^[0-9A-Z]{12}\.\d{9,15}$/i.test(backupId)) {
-      throw new NotFoundException()
-    }
+    const backupPath = resolve(this.configService.instanceBackupPath, `homebridge-backup-${backupId}.tar.gz`)
 
-    let backupPath = resolve(this.configService.instanceBackupPath, `homebridge-backup-${backupId}.tar.gz`)
-    try {
-      backupPath = await realpath(backupPath)
-    } catch (e) {
+    // Check the file exists
+    if (!await pathExists(backupPath)) {
       throw new NotFoundException()
     }
 
-    // Ensure the backupPath is within the instanceBackupPath
-    if (!backupPath.startsWith(this.configService.instanceBackupPath)) {
-      throw new BadRequestException(`Invalid backup path: ${backupPath} does not start with ${this.configService.instanceBackupPath}`)
-    }
-
     try {
       await remove(backupPath)
       this.logger.warn(`Scheduled backup ${backupId} deleted by request.`)
@@ -362,23 +342,13 @@ export class BackupService {
    * Restore a scheduled backup .tar.gz
    */
   async restoreScheduledBackup(backupId: string): Promise<void> {
-    // Validate backupId to ensure it matches the expected pattern
-    if (!/^[0-9A-Z]{12}\.\d{9,15}$/i.test(backupId)) {
-      throw new NotFoundException()
-    }
+    const backupPath = resolve(this.configService.instanceBackupPath, `homebridge-backup-${backupId}.tar.gz`)
 
-    let backupPath = resolve(this.configService.instanceBackupPath, `homebridge-backup-${backupId}.tar.gz`)
-    try {
-      backupPath = await realpath(backupPath)
-    } catch (e) {
+    // Check the file exists
+    if (!await pathExists(backupPath)) {
       throw new NotFoundException()
     }
 
-    // Ensure the backupPath is within the instanceBackupPath
-    if (!backupPath.startsWith(this.configService.instanceBackupPath)) {
-      throw new BadRequestException(`Invalid backup path: ${backupPath} does not start with ${this.configService.instanceBackupPath}`)
-    }
-
     // Clear restore directory
     this.restoreDirectory = undefined
 
@@ -393,18 +363,18 @@ export class BackupService {
     this.restoreDirectory = restoreDir
   }
 
-  // Remove temp files (called when download finished)
-  async cleanup(backupDir: string) {
-    await remove(resolve(backupDir))
-    this.logger.log(`Backup complete, removing ${backupDir}.`)
-  }
-
   /**
    * Create and download backup archive of the current homebridge instance
    */
   async downloadBackup(reply: FastifyReply): Promise<StreamableFile> {
     const { backupDir, backupPath, backupFileName } = await this.createBackup()
 
+    // Remove temp files (called when download finished)
+    async function cleanup() {
+      await remove(resolve(backupDir))
+      this.logger.log(`Backup complete, removing ${backupDir}.`)
+    }
+
     // Set download headers
     reply.raw.setHeader('Content-type', 'application/octet-stream')
     reply.raw.setHeader('Content-disposition', `attachment; filename=${backupFileName}`)
@@ -415,10 +385,7 @@ export class BackupService {
       reply.raw.setHeader('access-control-allow-origin', 'http://localhost:4200')
     }
 
-    // Remove temp files (called when download finished)
-    const cleanup = this.cleanup.bind(this, backupDir)
-
-    return new StreamableFile(createReadStream(backupPath).on('close', cleanup))
+    return new StreamableFile(createReadStream(backupPath).on('close', cleanup.bind(this)))
   }
 
   /**

src/modules/plugins/plugins.service.ts

@@ -929,18 +929,11 @@ export class PluginsService {
 
     const schemaPath = resolve(plugin.installPath, pluginName, 'config.schema.json')
 
-    if (!schemaPath.startsWith(plugin.installPath)) {
-      throw new BadRequestException('Invalid plugin path')
-    }
-
     let configSchema = await readJson(schemaPath)
 
     // check to see if this plugin implements dynamic schemas
     if (configSchema.dynamicSchemaVersion) {
       const dynamicSchemaPath = resolve(this.configService.storagePath, `.${pluginName}-v${configSchema.dynamicSchemaVersion}.schema.json`)
-      if (!dynamicSchemaPath.startsWith(this.configService.storagePath)) {
-        throw new BadRequestException('Invalid dynamic schema path')
-      }
       this.logger.log(`[${pluginName}] dynamic schema path: ${dynamicSchemaPath}.`)
       if (existsSync(dynamicSchemaPath)) {
         try {

src/modules/server/server.service.ts

@@ -54,12 +54,8 @@ export class ServerService {
   }
 
   private async deleteSingleDeviceAccessories(id: string, cachedAccessoriesDir: string) {
-    const cachedAccessories = resolve(cachedAccessoriesDir, `cachedAccessories.${id}`)
-    const cachedAccessoriesBackup = resolve(cachedAccessoriesDir, `.cachedAccessories.${id}.bak`)
-
-    if (!cachedAccessories.startsWith(cachedAccessoriesDir) || !cachedAccessoriesBackup.startsWith(cachedAccessoriesDir)) {
-      throw new BadRequestException('Invalid device ID.')
-    }
+    const cachedAccessories = join(cachedAccessoriesDir, `cachedAccessories.${id}`)
+    const cachedAccessoriesBackup = join(cachedAccessoriesDir, `.cachedAccessories.${id}.bak`)
 
     if (await pathExists(cachedAccessories)) {
       await unlink(cachedAccessories)
@@ -73,14 +69,9 @@ export class ServerService {
   }
 
   private async deleteSingleDevicePairing(id: string, resetPairingInfo: boolean) {
-    const persistPath = resolve(this.configService.storagePath, 'persist')
-    const accessoryInfo = resolve(persistPath, `AccessoryInfo.${id}.json`)
-    const identifierCache = resolve(persistPath, `IdentifierCache.${id}.json`)
-
-    // Validate paths
-    if (!accessoryInfo.startsWith(persistPath) || !identifierCache.startsWith(persistPath)) {
-      throw new BadRequestException('Invalid device ID.')
-    }
+    const persistPath = join(this.configService.storagePath, 'persist')
+    const accessoryInfo = join(persistPath, `AccessoryInfo.${id}.json`)
+    const identifierCache = join(persistPath, `IdentifierCache.${id}.json`)
 
     // Only available for child bridges
     if (resetPairingInfo) {
@@ -116,17 +107,11 @@ export class ServerService {
       }
     }
 
-    if (!accessoryInfo.startsWith(persistPath) || !identifierCache.startsWith(persistPath)) {
-      throw new BadRequestException('Invalid device ID.')
-    }
     if (await pathExists(accessoryInfo)) {
       await unlink(accessoryInfo)
       this.logger.warn(`Bridge ${id} reset: removed ${accessoryInfo}.`)
     }
 
-    if (!identifierCache.startsWith(persistPath)) {
-      throw new BadRequestException('Invalid identifierCache path.')
-    }
     if (await pathExists(identifierCache)) {
       await unlink(identifierCache)
       this.logger.warn(`Bridge ${id} reset: removed ${identifierCache}.`)
@@ -217,15 +202,10 @@ export class ServerService {
    */
   public async getDevicePairingById(deviceId: string, configFile = null) {
     const persistPath = join(this.configService.storagePath, 'persist')
-    const devicePath = resolve(persistPath, `AccessoryInfo.${deviceId}.json`)
-
-    if (!devicePath.startsWith(persistPath)) {
-      throw new BadRequestException('Invalid device ID')
-    }
 
     let device: any
     try {
-      device = await readJson(devicePath)
+      device = await readJson(join(persistPath, `AccessoryInfo.${deviceId}.json`))
     } catch (e) {
       throw new NotFoundException()
     }
@@ -390,11 +370,6 @@ export class ServerService {
 
     const cachedAccessoriesPath = resolve(this.configService.storagePath, 'accessories', cacheFile)
 
-    if (!cachedAccessoriesPath.startsWith(resolve(this.configService.storagePath, 'accessories'))) {
-      this.logger.error(`Invalid cache file path: ${cacheFile}`)
-      throw new BadRequestException('Invalid cache file path')
-    }
-
     this.logger.warn(`Shutting down Homebridge before removing cached accessory ${uuid}...`)
 
     // Wait for homebridge to stop.