fix(backup): keep GUI backup paths under BACKUP_ROOT and limit the schedule
backup:configure could point automatic backups at any writable directory (e.g. a statically served one) and set a cron of every second; an invalid expression deleted the running job before failing, silently ending backups. - A path entered in the UI is resolved under BACKUP_ROOT (default ./backups) and refused if it leaves it; env values stay unrestricted. - The schedule must be a five-field cron that runs at most hourly. - reload() validates before replacing the running job. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
1 parent
bd592adf66
commit
ebeb499a18
6 files changed
+148
-3
No files matched your search
@@ -33,6 +33,9 @@ DOCUMENTS_STORAGE_PATH="./uploads"
|
|||||||
# ihre eigenen Backup-Dateien und die Lauf-Erkennung zwischen Replicas greift
|
# ihre eigenen Backup-Dateien und die Lauf-Erkennung zwischen Replicas greift
|
||||||
# nicht.
|
# nicht.
|
||||||
BACKUP_STORAGE_PATH="./backups"
|
BACKUP_STORAGE_PATH="./backups"
|
||||||
|
# Directory a backup path set in the admin UI must stay under (the env
|
||||||
|
# variable above is not restricted). Default "./backups".
|
||||||
|
# BACKUP_ROOT="./backups"
|
||||||
# Cron-Ausdruck (Standard: taeglich um 03:00 Uhr).
|
# Cron-Ausdruck (Standard: taeglich um 03:00 Uhr).
|
||||||
BACKUP_CRON_SCHEDULE="0 3 * * *"
|
BACKUP_CRON_SCHEDULE="0 3 * * *"
|
||||||
# Wie viele der neuesten Backup-Dateien behalten werden, aeltere werden nach
|
# Wie viele der neuesten Backup-Dateien behalten werden, aeltere werden nach
|
||||||
|
|||||||
@@ -0,0 +1,50 @@
|
|||||||
|
import { BadRequestException } from '@nestjs/common';
|
||||||
|
import * as path from 'node:path';
|
||||||
|
import {
|
||||||
|
assertBackupCron,
|
||||||
|
assertGuiBackupPath,
|
||||||
|
resolveGuiBackupPath,
|
||||||
|
} from './backup-schedule-rules';
|
||||||
|
|
||||||
|
describe('backup-schedule-rules', () => {
|
||||||
|
const originalRoot = process.env.BACKUP_ROOT;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
process.env.BACKUP_ROOT = path.resolve('/data/backups');
|
||||||
|
});
|
||||||
|
|
||||||
|
afterAll(() => {
|
||||||
|
process.env.BACKUP_ROOT = originalRoot;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps a GUI path under the backup root', () => {
|
||||||
|
expect(resolveGuiBackupPath('nightly')).toBe(
|
||||||
|
path.resolve('/data/backups/nightly'),
|
||||||
|
);
|
||||||
|
expect(() => assertGuiBackupPath('nightly')).not.toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses a GUI path that leaves the backup root', () => {
|
||||||
|
expect(() => assertGuiBackupPath('../../app/public')).toThrow(
|
||||||
|
BadRequestException,
|
||||||
|
);
|
||||||
|
expect(() => assertGuiBackupPath('/etc')).toThrow(BadRequestException);
|
||||||
|
expect(() => assertGuiBackupPath('/data/backups-evil')).toThrow(
|
||||||
|
BadRequestException,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('accepts a daily schedule', () => {
|
||||||
|
expect(() => assertBackupCron('0 3 * * *')).not.toThrow();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses a schedule that runs more than hourly or has seconds', () => {
|
||||||
|
expect(() => assertBackupCron('* * * * *')).toThrow(BadRequestException);
|
||||||
|
expect(() => assertBackupCron('*/10 * * * *')).toThrow(BadRequestException);
|
||||||
|
expect(() => assertBackupCron('* * * * * *')).toThrow(BadRequestException);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses an invalid expression', () => {
|
||||||
|
expect(() => assertBackupCron('every day')).toThrow(BadRequestException);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -0,0 +1,55 @@
|
|||||||
|
import { BadRequestException } from '@nestjs/common';
|
||||||
|
import { CronTime, validateCronExpression } from 'cron';
|
||||||
|
import * as path from 'node:path';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Limits on what the admin UI may configure for automatic backups. The env
|
||||||
|
* variables (set by the operator) stay unrestricted; a GUI value is data a
|
||||||
|
* logged-in user typed, so it may neither write a full dump anywhere on the
|
||||||
|
* server's file system nor run an export every second.
|
||||||
|
*/
|
||||||
|
const MIN_INTERVAL_MS = 60 * 60 * 1000;
|
||||||
|
const INTERVAL_SAMPLES = 48;
|
||||||
|
|
||||||
|
/** Root every GUI-configured backup directory must stay under. */
|
||||||
|
export function backupRoot(): string {
|
||||||
|
return path.resolve(process.env.BACKUP_ROOT ?? './backups');
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The GUI path resolved under backupRoot(), or null if it would leave it. */
|
||||||
|
export function resolveGuiBackupPath(value: string): string | null {
|
||||||
|
const root = backupRoot();
|
||||||
|
const resolved = path.resolve(root, value);
|
||||||
|
if (resolved !== root && !resolved.startsWith(root + path.sep)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
return resolved;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function assertGuiBackupPath(value: string): void {
|
||||||
|
if (resolveGuiBackupPath(value) === null) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
`Backup-Pfad muss innerhalb von ${backupRoot()} liegen`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** A standard five-field cron expression that fires at most once an hour. */
|
||||||
|
export function assertBackupCron(expression: string): void {
|
||||||
|
const fields = expression.trim().split(/\s+/);
|
||||||
|
if (fields.length !== 5 || !validateCronExpression(expression).valid) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
'Zeitplan muss ein Cron-Ausdruck mit fünf Feldern sein, z. B. "0 3 * * *"',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
const runs = new CronTime(expression)
|
||||||
|
.sendAt(INTERVAL_SAMPLES)
|
||||||
|
.map((run) => run.toMillis());
|
||||||
|
for (let i = 1; i < runs.length; i++) {
|
||||||
|
if (runs[i] - runs[i - 1] < MIN_INTERVAL_MS) {
|
||||||
|
throw new BadRequestException(
|
||||||
|
'Zeitplan darf höchstens einmal pro Stunde sichern',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -5,6 +5,10 @@ import * as fs from 'node:fs/promises';
|
|||||||
import * as path from 'node:path';
|
import * as path from 'node:path';
|
||||||
import { BackupService } from './backup.service';
|
import { BackupService } from './backup.service';
|
||||||
import { BackupSettingsService } from './backup-settings.service';
|
import { BackupSettingsService } from './backup-settings.service';
|
||||||
|
import {
|
||||||
|
assertBackupCron,
|
||||||
|
resolveGuiBackupPath,
|
||||||
|
} from './backup-schedule-rules';
|
||||||
|
|
||||||
const SCHEDULED_JOB_NAME = 'automatic-backup';
|
const SCHEDULED_JOB_NAME = 'automatic-backup';
|
||||||
const DEFAULT_CRON_SCHEDULE = '0 3 * * *';
|
const DEFAULT_CRON_SCHEDULE = '0 3 * * *';
|
||||||
@@ -68,7 +72,13 @@ export class BackupSchedulerService implements OnModuleInit {
|
|||||||
const gui = await this.backupSettingsService.get();
|
const gui = await this.backupSettingsService.get();
|
||||||
|
|
||||||
return {
|
return {
|
||||||
path: hasEnvPath ? (envPath as string) : (gui.storagePath ?? null),
|
// A GUI path counts only inside BACKUP_ROOT (a value stored before
|
||||||
|
// that rule existed and pointing elsewhere disables the schedule).
|
||||||
|
path: hasEnvPath
|
||||||
|
? (envPath as string)
|
||||||
|
: gui.storagePath
|
||||||
|
? resolveGuiBackupPath(gui.storagePath)
|
||||||
|
: null,
|
||||||
cronSchedule: hasEnvCron
|
cronSchedule: hasEnvCron
|
||||||
? (envCron as string)
|
? (envCron as string)
|
||||||
: (gui.cronSchedule ?? DEFAULT_CRON_SCHEDULE),
|
: (gui.cronSchedule ?? DEFAULT_CRON_SCHEDULE),
|
||||||
@@ -88,11 +98,22 @@ export class BackupSchedulerService implements OnModuleInit {
|
|||||||
// admin's schedule/path/retention change takes effect immediately without
|
// admin's schedule/path/retention change takes effect immediately without
|
||||||
// a pod restart.
|
// a pod restart.
|
||||||
async reload(): Promise<void> {
|
async reload(): Promise<void> {
|
||||||
|
const settings = await this.effectiveSettings();
|
||||||
|
if (!settings.envOverrides.cronSchedule) {
|
||||||
|
try {
|
||||||
|
assertBackupCron(settings.cronSchedule);
|
||||||
|
} catch {
|
||||||
|
// Keep whatever is running instead of silently ending backups.
|
||||||
|
this.logger.error(
|
||||||
|
`Ungültiger Backup-Zeitplan "${settings.cronSchedule}" - bisheriger Zeitplan bleibt aktiv`,
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
if (this.schedulerRegistry.doesExist('cron', SCHEDULED_JOB_NAME)) {
|
if (this.schedulerRegistry.doesExist('cron', SCHEDULED_JOB_NAME)) {
|
||||||
this.schedulerRegistry.deleteCronJob(SCHEDULED_JOB_NAME);
|
this.schedulerRegistry.deleteCronJob(SCHEDULED_JOB_NAME);
|
||||||
}
|
}
|
||||||
|
|
||||||
const settings = await this.effectiveSettings();
|
|
||||||
if (!settings.path) {
|
if (!settings.path) {
|
||||||
this.logger.log(
|
this.logger.log(
|
||||||
'Kein Backup-Pfad konfiguriert (weder Env noch GUI) - automatische Backups deaktiviert',
|
'Kein Backup-Pfad konfiguriert (weder Env noch GUI) - automatische Backups deaktiviert',
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import { PrismaService } from '../prisma/prisma.service';
|
|||||||
import { AuditLogService } from '../audit/audit-log.service';
|
import { AuditLogService } from '../audit/audit-log.service';
|
||||||
import { BackupSettings } from '../../generated/prisma';
|
import { BackupSettings } from '../../generated/prisma';
|
||||||
import { UpdateBackupSettingsDto } from './dto/update-backup-settings.dto';
|
import { UpdateBackupSettingsDto } from './dto/update-backup-settings.dto';
|
||||||
|
import { assertBackupCron, assertGuiBackupPath } from './backup-schedule-rules';
|
||||||
|
|
||||||
const SINGLETON_ID = 'singleton';
|
const SINGLETON_ID = 'singleton';
|
||||||
|
|
||||||
@@ -31,6 +32,12 @@ export class BackupSettingsService {
|
|||||||
dto: UpdateBackupSettingsDto,
|
dto: UpdateBackupSettingsDto,
|
||||||
actingUserId: string,
|
actingUserId: string,
|
||||||
): Promise<BackupSettings> {
|
): Promise<BackupSettings> {
|
||||||
|
if (dto.storagePath) {
|
||||||
|
assertGuiBackupPath(dto.storagePath);
|
||||||
|
}
|
||||||
|
if (dto.cronSchedule) {
|
||||||
|
assertBackupCron(dto.cronSchedule);
|
||||||
|
}
|
||||||
const before = await this.get();
|
const before = await this.get();
|
||||||
const settings = await this.prisma.backupSettings.update({
|
const settings = await this.prisma.backupSettings.update({
|
||||||
where: { id: SINGLETON_ID },
|
where: { id: SINGLETON_ID },
|
||||||
|
|||||||
@@ -225,8 +225,13 @@ function ScheduleSettingsForm({
|
|||||||
value={storagePath}
|
value={storagePath}
|
||||||
disabled={envOverrides.storagePath}
|
disabled={envOverrides.storagePath}
|
||||||
onChange={(event) => setStoragePath(event.target.value)}
|
onChange={(event) => setStoragePath(event.target.value)}
|
||||||
placeholder="/data/backups"
|
placeholder="nightly"
|
||||||
/>
|
/>
|
||||||
|
{!envOverrides.storagePath ? (
|
||||||
|
<p className="text-xs text-muted-foreground">
|
||||||
|
Unterordner im Backup-Verzeichnis des Servers (BACKUP_ROOT), z. B. „nightly“.
|
||||||
|
</p>
|
||||||
|
) : null}
|
||||||
{envOverrides.storagePath ? (
|
{envOverrides.storagePath ? (
|
||||||
<p className="text-xs text-muted-foreground">
|
<p className="text-xs text-muted-foreground">
|
||||||
Durch BACKUP_STORAGE_PATH vorgegeben.
|
Durch BACKUP_STORAGE_PATH vorgegeben.
|
||||||
@@ -241,7 +246,11 @@ function ScheduleSettingsForm({
|
|||||||
disabled={envOverrides.cronSchedule}
|
disabled={envOverrides.cronSchedule}
|
||||||
onChange={(event) => setCronSchedule(event.target.value)}
|
onChange={(event) => setCronSchedule(event.target.value)}
|
||||||
placeholder="0 3 * * *"
|
placeholder="0 3 * * *"
|
||||||
|
aria-describedby="backup-cron-hint"
|
||||||
/>
|
/>
|
||||||
|
<p id="backup-cron-hint" className="text-xs text-muted-foreground">
|
||||||
|
Fünf Felder, höchstens einmal pro Stunde. „0 3 * * *“ = täglich um 3 Uhr.
|
||||||
|
</p>
|
||||||
{envOverrides.cronSchedule ? (
|
{envOverrides.cronSchedule ? (
|
||||||
<p className="text-xs text-muted-foreground">
|
<p className="text-xs text-muted-foreground">
|
||||||
Durch BACKUP_CRON_SCHEDULE vorgegeben.
|
Durch BACKUP_CRON_SCHEDULE vorgegeben.
|
||||||
|
|||||||
Reference in new issue
Block a user