gsadmin/sencho
1
0
Fork 0
mirror of https://github.com/Studio-Saelix/sencho.git synced 2026-07-29 05:09:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: reject atomic restore when a checksummed backup file is missing (#1466)

Browse Source 
Restore verification iterated the backup directory listing, so a file
recorded in the .checksums manifest but absent from the backup slot was
never checked. The orphan removal then deleted the live file and the copy
restored nothing, reporting success while leaving the stack unrecoverable.

Walk the manifest instead of the directory listing: a recorded file that
the slot no longer holds now aborts the restore before any file is touched,
alongside the existing corrupt-content check. Also fail backup creation when
a managed file exists but cannot be read (non-ENOENT), instead of silently
omitting it and producing an incomplete backup with the same failure mode.
This commit is contained in:
Anso
2026-06-26 10:16:32 -04:00
committed by GitHub
parent 1c82e3e1d4
commit eaf0642d88
2 changed files with 112 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/src/__tests__/filesystem-backup.test.ts
+74
View File
@@ -508,5 +508,79 @@ describe('FileSystemService backup location', () => {
expect(await fsPromises.readFile(path.join(stackDir, 'compose.yaml'), 'utf-8')).toBe('name: original\n');
});
it('aborts the restore when a manifest-recorded file is missing from the backup, leaving the live file unchanged', async () => {
const stackName = 'missingmember';
const stackDir = path.join(composeDir, stackName);
await fsPromises.mkdir(stackDir, { recursive: true });
await fsPromises.writeFile(path.join(stackDir, 'compose.yaml'), 'name: backed-up\n', 'utf-8');
const service = FileSystemService.getInstance();
await service.backupStackFiles(stackName); // manifest records compose.yaml
// The backed-up compose.yaml is lost after the manifest was written, but the
// manifest still names it. An items-only scan would not notice; the orphan
// removal would then delete the live compose.yaml and the copy would restore
// nothing, leaving the stack unrecoverable. The integrity check must catch
// the missing member first.
const backupDir = path.join(dataDir, 'backups', '1', stackName);
await fsPromises.rm(path.join(backupDir, 'compose.yaml'));
// The live file is the post-deploy state a rollback would revert.
await fsPromises.writeFile(path.join(stackDir, 'compose.yaml'), 'name: current\n', 'utf-8');
await expect(service.restoreStackFiles(stackName)).rejects.toThrow(/missing/i);
// The live compose.yaml must survive: not deleted as an orphan, not truncated.
expect(await fsPromises.readFile(path.join(stackDir, 'compose.yaml'), 'utf-8')).toBe('name: current\n');
});
it('aborts on a missing recorded file even when its checksum entry is malformed', async () => {
const stackName = 'missingnonstring';
const stackDir = path.join(composeDir, stackName);
await fsPromises.mkdir(stackDir, { recursive: true });
await fsPromises.writeFile(path.join(stackDir, 'compose.yaml'), 'name: backed-up\n', 'utf-8');
const service = FileSystemService.getInstance();
await service.backupStackFiles(stackName);
// The manifest still names compose.yaml but with a non-string value, and the
// backed-up copy is gone. The missing-file check must fire before the
// value-type skip, otherwise the orphan removal would delete the live
// compose.yaml and restore nothing.
const backupDir = path.join(dataDir, 'backups', '1', stackName);
await fsPromises.writeFile(path.join(backupDir, '.checksums'), JSON.stringify({ 'compose.yaml': 123 }), 'utf-8');
await fsPromises.rm(path.join(backupDir, 'compose.yaml'));
await fsPromises.writeFile(path.join(stackDir, 'compose.yaml'), 'name: current\n', 'utf-8');
await expect(service.restoreStackFiles(stackName)).rejects.toThrow(/missing/i);
expect(await fsPromises.readFile(path.join(stackDir, 'compose.yaml'), 'utf-8')).toBe('name: current\n');
});
it('fails the backup when a managed file exists but cannot be read', async () => {
const stackName = 'readfail';
const stackDir = path.join(composeDir, stackName);
await fsPromises.mkdir(stackDir, { recursive: true });
const composeSrc = path.join(stackDir, 'compose.yaml');
await fsPromises.writeFile(composeSrc, 'name: present\n', 'utf-8');
// The compose file exists but the read fails (e.g. EACCES on a chowned bind
// mount). Silently skipping it would yield a "successful" backup that omits a
// live managed file, so a later rollback would delete it as an orphan.
const realReadFile = fsPromises.readFile.bind(fsPromises);
const readSpy = vi.spyOn(fsPromises, 'readFile').mockImplementation((file, options) => {
if (path.normalize(String(file)) === path.normalize(composeSrc)) {
return Promise.reject(Object.assign(new Error('permission denied'), { code: 'EACCES' }));
}
return realReadFile(file as Parameters<typeof realReadFile>[0], options);
});
try {
await expect(FileSystemService.getInstance().backupStackFiles(stackName))
.rejects.toThrow(/Could not read compose.yaml for backup/);
} finally {
readSpy.mockRestore();
}
});
});
});
backend/src/services/FileSystemService.ts
+38 -12
View File
@@ -1001,10 +1001,15 @@ export class FileSystemService {
buf = await fsPromises.readFile(src);
} catch (e: unknown) {
const code = (e as NodeJS.ErrnoException)?.code;
if (code !== 'ENOENT') {
console.warn(`[FileSystemService] Could not read ${file} for backup:`, (e as Error).message);
}
return;
// ENOENT means the file genuinely is not present: most stacks use one
// compose variant and have no .env, so an absent managed file is expected
// and simply skipped. Any other code (EACCES on a chowned bind mount,
// EBUSY on a held file) means the file exists but could not be read.
// Skipping it would produce a backup that silently omits a live managed
// file, so a later rollback would delete that file as an orphan and
// restore nothing. Fail the backup so the caller aborts before mutating.
if (code === 'ENOENT') return;
throw new Error(`Could not read ${file} for backup: ${(e as Error).message}`, { cause: e });
}
const dest = path.join(backupDir, file);
@@ -1110,16 +1115,37 @@ export class FileSystemService {
}
}
if (checksums) {
for (const item of items) {
if (BACKUP_MARKER_FILES.has(item)) continue;
const expected = checksums[item];
// A missing or non-string entry (a tampered or malformed-but-parseable
// manifest) is treated as no recorded checksum: skip rather than fail, so a
// manifest problem never blocks a rollback whose data files are intact.
// Iterate the recorded manifest, not the directory listing. A file the
// manifest records but the slot no longer holds (lost or deleted after the
// manifest was written) passes an items-only scan unchecked; the orphan
// removal below would then delete the live file and the copy would restore
// nothing, leaving the stack unrecoverable. Walking the manifest catches
// both a corrupt copy and a missing one before anything is mutated.
for (const [file, expected] of Object.entries(checksums)) {
// Every key the manifest records names a file the backup captured. If the
// slot no longer holds it, the backup is incomplete: the orphan removal
// below would delete the live file (for a managed name) and the copy would
// restore nothing, so abort before anything is mutated. This precedes the
// value-type skip so a missing file is caught even when its recorded
// checksum is malformed.
if (!backedUp.has(file)) {
throw new Error(`Rollback aborted: the backup is missing ${file} recorded in its integrity manifest; the stack files were not changed.`);
}
// A non-string entry (a tampered or malformed-but-parseable manifest) is
// treated as no recorded checksum: the file is present, so copy it back
// unverified rather than fail, so a manifest problem never blocks a
// rollback whose data files are intact.
if (typeof expected !== 'string') continue;
const actual = sha256HexBuffer(await fsPromises.readFile(path.join(backupDir, item)));
// Canonical js/path-injection barrier inline with the read sink: the file
// name comes from the manifest, so resolve it against the backup dir and
// confirm containment before re-hashing.
const member = path.resolve(backupDir, file);
if (!member.startsWith(backupDir + path.sep)) {
throw Object.assign(new Error('Path escapes backup directory'), { code: 'INVALID_PATH' });
}
const actual = sha256HexBuffer(await fsPromises.readFile(member));
if (actual !== expected) {
throw new Error(`Rollback aborted: the backup of ${item} is corrupt (integrity check failed); the stack files were not changed.`);
throw new Error(`Rollback aborted: the backup of ${file} is corrupt (integrity check failed); the stack files were not changed.`);
}
}
}