From 2b1b304c6ec41514bf10f951596f93648524ff75 Mon Sep 17 00:00:00 2001 From: Girish Ramakrishnan Date: Tue, 9 Apr 2024 13:53:15 +0200 Subject: [PATCH] backup/import/restore: fix crash with root path calcuation rootPath was calculated before the arguments were validated --- src/apps.js | 4 ++-- src/backups.js | 5 ++--- src/provision.js | 4 ++-- src/storage/filesystem.js | 25 ++++++++++++++----------- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/apps.js b/src/apps.js index c6bd83a9f..122ec9bb0 100644 --- a/src/apps.js +++ b/src/apps.js @@ -2242,9 +2242,9 @@ async function importApp(app, data, auditSource) { backupConfig.encryption = null; } - await backups.setupStorage(backupConfig, `/mnt/appimport-${app.id}`); // this validates mountOptions + await backups.setupStorage(backupConfig, `/mnt/appimport-${app.id}`); // this validates mountOptions . this is not cleaned up, it's fine backupConfig.rootPath = backups.getRootPath(backupConfig, `/mnt/appimport-${app.id}`); - error = await backups.testStorage(backupConfig); // this validates provider and it's api options. requires rootPath + error = await backups.testStorage(Object.assign({ mountPath: `/mnt/appimport-${app.id}` }, backupConfig)); // this validates provider and it's api options. requires mountPath if (error) throw error; restoreConfig = { remotePath, backupFormat, backupConfig }; diff --git a/src/backups.js b/src/backups.js index a64049cc6..9b54357ad 100644 --- a/src/backups.js +++ b/src/backups.js @@ -461,7 +461,7 @@ async function setupStorage(storageConfig, hostPath) { const newMount = { name: path.basename(hostPath), - hostPath: hostPath, + hostPath, mountType: storageConfig.provider, mountOptions: storageConfig.mountOptions }; @@ -496,8 +496,7 @@ async function setStorage(storageConfig) { debug('setStorage: validating new storage configuration'); const testMountObject = await setupStorage(storageConfig, '/mnt/backup-storage-validation'); // this validates mountOptions - const rootPath = getRootPath(storageConfig, '/mnt/backup-storage-validation'); - const testStorageError = await testStorage(Object.assign({ rootPath }, storageConfig)); // this validates provider and it's api options. requires rootPath + const testStorageError = await testStorage(Object.assign({ mountPath: '/mnt/backup-storage-validation' }, storageConfig)); // this validates provider and it's api options. requires mountPath if (testMountObject) await mounts.removeMount(testMountObject); if (testStorageError) throw testStorageError; diff --git a/src/provision.js b/src/provision.js index 4e21e5816..eab252823 100644 --- a/src/provision.js +++ b/src/provision.js @@ -169,6 +169,7 @@ async function restoreTask(backupConfig, remotePath, ipv4Config, options, auditS try { setProgress('restore', 'Downloading box backup'); + backupConfig.rootPath = backups.getRootPath(backupConfig, paths.MANAGED_BACKUP_MOUNT_DIR); await backuptask.restore(backupConfig, remotePath, (progress) => setProgress('restore', progress.message)); setProgress('restore', 'Downloading mail backup'); @@ -231,8 +232,7 @@ async function restore(backupConfig, remotePath, version, ipv4Config, options, a } await backups.setupStorage(backupConfig, paths.MANAGED_BACKUP_MOUNT_DIR); // this validates mountOptions - backupConfig.rootPath = backups.getRootPath(backupConfig, paths.MANAGED_BACKUP_MOUNT_DIR); - error = await backups.testStorage(backupConfig); // this validates provider and it's api options. requires rootPath + error = await backups.testStorage(Object.assign({ mountPath: paths.MANAGED_BACKUP_MOUNT_DIR }), backupConfig); // this validates provider and it's api options. requires mountPath if (error) throw error; error = await network.testIPv4Config(ipv4Config); diff --git a/src/storage/filesystem.js b/src/storage/filesystem.js index f45c4e41c..5d36f47a0 100644 --- a/src/storage/filesystem.js +++ b/src/storage/filesystem.js @@ -238,15 +238,20 @@ async function testConfig(apiConfig) { if ('noHardlinks' in apiConfig && typeof apiConfig.noHardlinks !== 'boolean') throw new BoxError(BoxError.BAD_FIELD, 'noHardlinks must be boolean'); if ('chown' in apiConfig && typeof apiConfig.chown !== 'boolean') throw new BoxError(BoxError.BAD_FIELD, 'chown must be boolean'); + let rootPath; // for managed mounts, this uses 'mountPath', which could be some temporary mount location if (apiConfig.provider === PROVIDER_FILESYSTEM) { if (!apiConfig.backupFolder || typeof apiConfig.backupFolder !== 'string') throw new BoxError(BoxError.BAD_FIELD, 'backupFolder must be non-empty string'); const error = validateBackupTarget(apiConfig.backupFolder); if (error) throw error; + rootPath = apiConfig.backupFolder; } else { // xfs/cifs/ext4/nfs/mountpoint/sshfs if (apiConfig.provider === PROVIDER_MOUNTPOINT) { if (!apiConfig.mountPoint || typeof apiConfig.mountPoint !== 'string') throw new BoxError(BoxError.BAD_FIELD, 'mountPoint must be non-empty string'); const error = validateBackupTarget(apiConfig.mountPoint); if (error) throw error; + + const [mountError] = await safe(shell.exec('testStorageConfig', `mountpoint -q -- ${apiConfig.mountPoint}`, { timeout: 5000 })); + if (mountError) throw new BoxError(BoxError.BAD_FIELD, `${apiConfig.mountPoint} is not mounted: ${mountError.message}`); } if (typeof apiConfig.prefix !== 'string') throw new BoxError(BoxError.BAD_FIELD, 'prefix must be a string'); @@ -254,27 +259,25 @@ async function testConfig(apiConfig) { if (path.isAbsolute(apiConfig.prefix)) throw new BoxError(BoxError.BAD_FIELD, 'prefix must be a relative path'); if (path.normalize(apiConfig.prefix) !== apiConfig.prefix) throw new BoxError(BoxError.BAD_FIELD, 'prefix must contain a normalized relative path'); } - } - if (apiConfig.provider === PROVIDER_MOUNTPOINT) { - const [error] = await safe(shell.exec('testStorageConfig', `mountpoint -q -- ${apiConfig.mountPoint}`, { timeout: 5000 })); - if (error) throw new BoxError(BoxError.BAD_FIELD, `${apiConfig.mountPoint} is not mounted: ${error.message}`); + if (apiConfig.provider === PROVIDER_MOUNTPOINT) { + rootPath = path.join(apiConfig.mountPoint, apiConfig.prefix); + } else { + rootPath = path.join(apiConfig.mountPath, apiConfig.prefix); + } } - const rootPath = apiConfig.rootPath; - const field = apiConfig.provider === PROVIDER_FILESYSTEM ? 'backupFolder' : 'mountPoint'; - if (!safe.fs.mkdirSync(path.join(rootPath, 'snapshot'), { recursive: true }) && safe.error.code !== 'EEXIST') { - if (safe.error && safe.error.code === 'EACCES') throw new BoxError(BoxError.BAD_FIELD, `Access denied. Create the directory and run "chown yellowtent:yellowtent ${rootPath}" on the server`, { field }); - throw new BoxError(BoxError.BAD_FIELD, safe.error.message, { field }); + if (safe.error && safe.error.code === 'EACCES') throw new BoxError(BoxError.BAD_FIELD, `Access denied. Create ${rootPath}/snapshot and run "chown yellowtent:yellowtent ${rootPath}" on the server`); + throw new BoxError(BoxError.BAD_FIELD, safe.error.message); } if (!safe.fs.writeFileSync(path.join(rootPath, 'cloudron-testfile'), 'testcontent')) { - throw new BoxError(BoxError.BAD_FIELD, `Unable to create test file as 'yellowtent' user in ${rootPath}: ${safe.error.message}. Check dir/mount permissions`, { field }); + throw new BoxError(BoxError.BAD_FIELD, `Unable to create test file as 'yellowtent' user in ${rootPath}: ${safe.error.message}. Check dir/mount permissions`); } if (!safe.fs.unlinkSync(path.join(rootPath, 'cloudron-testfile'))) { - throw new BoxError(BoxError.BAD_FIELD, `Unable to remove test file as 'yellowtent' user in ${rootPath}: ${safe.error.message}. Check dir/mount permissions`, { field }); + throw new BoxError(BoxError.BAD_FIELD, `Unable to remove test file as 'yellowtent' user in ${rootPath}: ${safe.error.message}. Check dir/mount permissions`); } }