diff --git a/apps/api/src/admin/backup-admin.e2e.db.test.ts b/apps/api/src/admin/backup-admin.e2e.db.test.ts index 0e0fa05..31c3728 100644 --- a/apps/api/src/admin/backup-admin.e2e.db.test.ts +++ b/apps/api/src/admin/backup-admin.e2e.db.test.ts @@ -155,6 +155,9 @@ describe.skipIf(!hasTestDb)('backup admin (e2e, issue #103)', () => { secretsFile = join(mkdtempSync(join(tmpdir(), 'dorfteich-backup-secrets-')), 'secrets.env'); process.env.BACKUPS_DIR = backupsDir; process.env.SECRETS_FILE = secretsFile; + // The in-test WebDAV server must be allowlisted (issue #192) — the + // policy paths themselves are covered by backup-allowlist*.e2e.db.test.ts. + process.env.BACKUP_ALLOWED_TARGETS = '127.0.0.1'; davUrl = await dav.start(); prisma = createTestPrisma(); await prisma.rateLimit.deleteMany({}); diff --git a/apps/api/src/admin/backup-admin.service.ts b/apps/api/src/admin/backup-admin.service.ts index cb4b062..4396bc8 100644 --- a/apps/api/src/admin/backup-admin.service.ts +++ b/apps/api/src/admin/backup-admin.service.ts @@ -65,6 +65,7 @@ export class BackupAdminService { */ async saveSettings(input: BackupSettingsInput, actor: User): Promise { if (input.nextcloud.enabled) { + this.assertTargetAllowed(input.nextcloud.baseUrl); const test = await this.target.testConnection({ baseUrl: input.nextcloud.baseUrl, username: input.nextcloud.username, @@ -100,9 +101,29 @@ export class BackupAdminService { } testConnection(input: BackupConnectionTestInput): Promise { + // Policy first (issue #192): the "test connection" button must not be + // usable as an egress probe towards non-allowlisted hosts. + this.assertTargetAllowed(input.baseUrl); return this.target.testConnection(input); } + /** + * Deploy-level target policy (issue #192, ADR 0026): an empty + * `BACKUP_ALLOWED_TARGETS` disables remote targets outright; a host + * outside the list is rejected with an admin-visible error. + */ + private assertTargetAllowed(baseUrl: string): void { + if (!this.target.remoteAllowed()) { + throw new BadRequestException({ code: 'backup_remote_disabled_by_policy' }); + } + if (!this.target.targetAllowed(baseUrl)) { + throw new BadRequestException({ + code: 'backup_target_not_allowed', + details: { nextcloud: [`host is not in BACKUP_ALLOWED_TARGETS`] }, + }); + } + } + /** Both restore sources for the picker: newest first. */ async sets(): Promise { const local = this.localSets(); diff --git a/apps/api/src/admin/backup-allowlist-empty.e2e.db.test.ts b/apps/api/src/admin/backup-allowlist-empty.e2e.db.test.ts new file mode 100644 index 0000000..4d2bb7b --- /dev/null +++ b/apps/api/src/admin/backup-allowlist-empty.e2e.db.test.ts @@ -0,0 +1,101 @@ +import { INestApplication } from '@nestjs/common'; +import { PrismaClient } from '@prisma/client'; +import request from 'supertest'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { createTestApp, sessionCookieOf } from '../testing/test-app'; +import { createTestPrisma, hasTestDb, uniqueSuffix } from '../testing/test-db'; +import { UsersService } from '../users/users.service'; + +/** + * Backup target allowlist (issue #192, ADR 0026), empty-list half: with + * `BACKUP_ALLOWED_TARGETS` unset (the default) every remote target is + * unavailable by policy — the view says so, and enabling one is rejected + * before any connection attempt. Lives in its own file because the env is + * read once at app boot. + */ +describe.skipIf(!hasTestDb)('backup targets disabled by empty allowlist (e2e, issue #192)', () => { + let app: INestApplication; + let prisma: PrismaClient; + const suffix = uniqueSuffix(); + const password = 'backup allowlist pass 2'; + let adminId: string; + let adminCookie: string; + + const api = () => request(app.getHttpServer()); + + beforeAll(async () => { + delete process.env.BACKUP_ALLOWED_TARGETS; + prisma = createTestPrisma(); + app = await createTestApp(); + const users = app.get(UsersService); + const username = `bae-admin-${suffix}`; + const admin = await users.createUser({ + username, + email: `${username}@example.org`, + displayName: 'Backup Admin Empty', + password, + locale: 'en', + }); + adminId = admin.id; + await users.markEmailVerified(adminId); + await prisma.user.update({ where: { id: adminId }, data: { isSiteAdmin: true } }); + adminCookie = sessionCookieOf( + await api() + .post('/api/v1/auth/login') + .send({ usernameOrEmail: username, password }) + .expect(200), + ); + }); + + afterAll(async () => { + await prisma.auditEntry.deleteMany({ where: { actorId: adminId } }); + await prisma.session.deleteMany({ where: { userId: adminId } }); + await prisma.userIdentity.deleteMany({ where: { userId: adminId } }); + await prisma.user.deleteMany({ where: { id: adminId } }); + await prisma.$disconnect(); + await app.close(); + }); + + it('reports remote targets as unavailable by policy', async () => { + const res = await api() + .get('/api/v1/admin/system/backup/settings') + .set('Cookie', adminCookie) + .expect(200); + expect(res.body.remoteTargets).toEqual({ allowed: false, allowlist: [] }); + }); + + it('rejects enabling any remote destination', async () => { + const res = await api() + .put('/api/v1/admin/system/backup/settings') + .set('Cookie', adminCookie) + .send({ + localRetentionDays: null, + remoteRetentionDays: 30, + nextcloud: { + enabled: true, + baseUrl: 'https://cloud.example.org/dav', + username: 'backupuser', + folder: 'dorfteich-backups', + uploadSchedule: 'daily', + password: 'app-pass', + }, + }) + .expect(400); + expect(res.body.code).toBe('backup_remote_disabled_by_policy'); + }); + + it('rejects the connection test outright', async () => { + const res = await api() + .post('/api/v1/admin/system/backup/nextcloud/test') + .set('Cookie', adminCookie) + .send({ + baseUrl: 'https://cloud.example.org/dav', + username: 'backupuser', + folder: 'dorfteich-backups', + password: 'app-pass', + }) + .expect(400); + expect(res.body.code).toBe('backup_remote_disabled_by_policy'); + }); +}); diff --git a/apps/api/src/admin/backup-allowlist.e2e.db.test.ts b/apps/api/src/admin/backup-allowlist.e2e.db.test.ts new file mode 100644 index 0000000..71eb136 --- /dev/null +++ b/apps/api/src/admin/backup-allowlist.e2e.db.test.ts @@ -0,0 +1,134 @@ +import { INestApplication } from '@nestjs/common'; +import { PrismaClient } from '@prisma/client'; +import request from 'supertest'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +import { createTestApp, sessionCookieOf } from '../testing/test-app'; +import { createTestPrisma, hasTestDb, uniqueSuffix } from '../testing/test-db'; +import { UsersService } from '../users/users.service'; + +// Deploy-level env — must be set BEFORE the app (AppConfig) boots. +process.env.BACKUP_ALLOWED_TARGETS = 'cloud.example.org'; + +/** + * Backup target allowlist (issue #192, ADR 0026), populated-list half: + * hosts outside `BACKUP_ALLOWED_TARGETS` are rejected admin-visibly, hosts + * inside pass the policy. The empty-list half lives in its own file + * (`backup-allowlist-empty.e2e.db.test.ts`) because the env is read once + * at app boot. + */ +describe.skipIf(!hasTestDb)('backup target allowlist (e2e, issue #192)', () => { + let app: INestApplication; + let prisma: PrismaClient; + const suffix = uniqueSuffix(); + const password = 'backup allowlist pass 1'; + let adminId: string; + let adminCookie: string; + + const api = () => request(app.getHttpServer()); + + const settingsInput = (baseUrl: string, enabled = true) => ({ + localRetentionDays: null, + remoteRetentionDays: 30, + nextcloud: { + enabled, + baseUrl, + username: 'backupuser', + folder: 'dorfteich-backups', + uploadSchedule: 'daily', + password: 'app-pass', + }, + }); + + beforeAll(async () => { + prisma = createTestPrisma(); + app = await createTestApp(); + const users = app.get(UsersService); + const username = `bal-admin-${suffix}`; + const admin = await users.createUser({ + username, + email: `${username}@example.org`, + displayName: 'Backup Admin', + password, + locale: 'en', + }); + adminId = admin.id; + await users.markEmailVerified(adminId); + await prisma.user.update({ where: { id: adminId }, data: { isSiteAdmin: true } }); + adminCookie = sessionCookieOf( + await api() + .post('/api/v1/auth/login') + .send({ usernameOrEmail: username, password }) + .expect(200), + ); + }); + + afterAll(async () => { + await prisma.instanceSetting.deleteMany({ where: { key: { startsWith: 'backup.' } } }); + await prisma.auditEntry.deleteMany({ where: { actorId: adminId } }); + await prisma.session.deleteMany({ where: { userId: adminId } }); + await prisma.userIdentity.deleteMany({ where: { userId: adminId } }); + await prisma.user.deleteMany({ where: { id: adminId } }); + await prisma.$disconnect(); + await app.close(); + }); + + it('exposes the policy in the settings view', async () => { + const res = await api() + .get('/api/v1/admin/system/backup/settings') + .set('Cookie', adminCookie) + .expect(200); + expect(res.body.remoteTargets).toEqual({ + allowed: true, + allowlist: ['cloud.example.org'], + }); + }); + + it('rejects enabling a destination outside the allowlist', async () => { + const res = await api() + .put('/api/v1/admin/system/backup/settings') + .set('Cookie', adminCookie) + .send(settingsInput('https://evil.example.net/dav')) + .expect(400); + expect(res.body.code).toBe('backup_target_not_allowed'); + }); + + it('rejects the connection test towards a non-allowlisted host', async () => { + const res = await api() + .post('/api/v1/admin/system/backup/nextcloud/test') + .set('Cookie', adminCookie) + .send({ + baseUrl: 'https://evil.example.net/dav', + username: 'backupuser', + folder: 'dorfteich-backups', + password: 'app-pass', + }) + .expect(400); + expect(res.body.code).toBe('backup_target_not_allowed'); + }); + + it('lets an allowlisted destination through the policy', async () => { + // The host passes the policy; what fails afterwards is the live + // connection test against the (unreachable) example host — proving the + // rejection above was the policy, not the connectivity. + const res = await api() + .put('/api/v1/admin/system/backup/settings') + .set('Cookie', adminCookie) + .send(settingsInput('https://cloud.example.org/dav')) + .expect(400); + expect(res.body.code).toBe('backup_connection_failed'); + + // Saving the same destination disabled skips the connection test and + // persists — an existing in-allowlist configuration stays untouched. + await api() + .put('/api/v1/admin/system/backup/settings') + .set('Cookie', adminCookie) + .send(settingsInput('https://cloud.example.org/dav', false)) + .expect(200); + const view = await api() + .get('/api/v1/admin/system/backup/settings') + .set('Cookie', adminCookie) + .expect(200); + expect(view.body.nextcloud.baseUrl).toBe('https://cloud.example.org/dav'); + }); +}); diff --git a/apps/api/src/backup/backup-target.service.ts b/apps/api/src/backup/backup-target.service.ts index 8d5f3fb..2e0eb5d 100644 --- a/apps/api/src/backup/backup-target.service.ts +++ b/apps/api/src/backup/backup-target.service.ts @@ -1,7 +1,12 @@ import { Injectable } from '@nestjs/common'; -import type { BackupConnectionTestResult, BackupSettingsView } from '@dorfteich/shared'; +import { + isBackupTargetAllowed, + type BackupConnectionTestResult, + type BackupSettingsView, +} from '@dorfteich/shared'; import { webdavCheck, type WebDavTarget } from '@dorfteich/shared/webdav'; +import { AppConfig } from '../config/app-config.service'; import { SecretStoreService } from '../config/secret-store.service'; import { InstanceSettingsService } from '../settings/instance-settings.service'; @@ -19,12 +24,27 @@ export class BackupTargetService { constructor( private readonly settings: InstanceSettingsService, private readonly secretStore: SecretStoreService, + private readonly config: AppConfig, ) {} + /** Deploy-level allowlist (issue #192): empty = remote targets disabled. */ + allowlist(): string[] { + return this.config.env.BACKUP_ALLOWED_TARGETS; + } + + remoteAllowed(): boolean { + return this.allowlist().length > 0; + } + + targetAllowed(target: string): boolean { + return isBackupTargetAllowed(this.allowlist(), target); + } + async settingsView(): Promise { return { localRetentionDays: await this.settings.get('backup.localRetentionDays'), remoteRetentionDays: await this.settings.get('backup.remoteRetentionDays'), + remoteTargets: { allowed: this.remoteAllowed(), allowlist: this.allowlist() }, nextcloud: { enabled: await this.settings.get('backup.nextcloud.enabled'), baseUrl: await this.settings.get('backup.nextcloud.baseUrl'), @@ -45,6 +65,9 @@ export class BackupTargetService { const password = this.storedPassword(); const { enabled, baseUrl, username, folder } = view.nextcloud; if (!enabled || !baseUrl || !username || !password) return null; + // Policy backstop (issue #192): a configured target outside the deploy + // allowlist behaves like no target at all. + if (!this.targetAllowed(baseUrl)) return null; return { baseUrl, username, password, folder }; } diff --git a/apps/backup/src/index.ts b/apps/backup/src/index.ts index 2372209..69af953 100644 --- a/apps/backup/src/index.ts +++ b/apps/backup/src/index.ts @@ -43,8 +43,8 @@ function readSecrets(): Record { */ async function buildRunnerDeps(trigger: 'scheduled' | 'manual'): Promise { const settings = await readBackupDbSettings(env.DATABASE_URL); - const target = resolveRemoteTarget(settings, readSecrets()); - const mirrorConfig = resolveMirrorConfig(env); + const target = resolveRemoteTarget(settings, readSecrets(), env.BACKUP_ALLOWED_TARGETS, log); + const mirrorConfig = resolveMirrorConfig(env, log); return { backupsDir: env.BACKUPS_DIR, retentionDays: settings.localRetentionDays ?? env.BACKUP_RETENTION_DAYS, @@ -162,7 +162,7 @@ async function terminateOtherConnections(): Promise { async function handleRestore(command: Extract): Promise { const settings = await readBackupDbSettings(env.DATABASE_URL); - const target = resolveRemoteTarget(settings, readSecrets()); + const target = resolveRemoteTarget(settings, readSecrets(), env.BACKUP_ALLOWED_TARGETS, log); await orchestrateRestore( { backupsDir: env.BACKUPS_DIR, diff --git a/apps/backup/src/mirror.test.ts b/apps/backup/src/mirror.test.ts index e4f2e70..56153be 100644 --- a/apps/backup/src/mirror.test.ts +++ b/apps/backup/src/mirror.test.ts @@ -27,8 +27,12 @@ function hasRsync(): boolean { } describe('resolveMirrorConfig', () => { + const base = { + BACKUP_MIRROR_SSH_PORT: 22, + BACKUP_ALLOWED_TARGETS: ['h'], + } as unknown as BackupEnv; + it('requires both target and key; port defaults to 22', () => { - const base = { BACKUP_MIRROR_SSH_PORT: 22 } as unknown as BackupEnv; expect(resolveMirrorConfig(base)).toBeNull(); expect( resolveMirrorConfig({ ...base, BACKUP_MIRROR_TARGET: 'u@h:/x/' } as BackupEnv), @@ -41,6 +45,21 @@ describe('resolveMirrorConfig', () => { } as BackupEnv), ).toEqual({ target: 'u@h:/x/', sshKeyFile: '/data/secrets/key', sshPort: 22 }); }); + + it('is null when the target host is outside the deploy allowlist (issue #192)', () => { + const configured = { + ...base, + BACKUP_MIRROR_TARGET: 'u@h:/x/', + BACKUP_MIRROR_SSH_KEY: '/data/secrets/key', + } as BackupEnv; + expect( + resolveMirrorConfig({ ...configured, BACKUP_ALLOWED_TARGETS: ['other.host'] } as BackupEnv), + ).toBeNull(); + // The empty allowlist disables the mirror outright. + expect( + resolveMirrorConfig({ ...configured, BACKUP_ALLOWED_TARGETS: [] } as BackupEnv), + ).toBeNull(); + }); }); describe('buildRsyncArgs', () => { diff --git a/apps/backup/src/mirror.ts b/apps/backup/src/mirror.ts index 7164a81..e0a5bab 100644 --- a/apps/backup/src/mirror.ts +++ b/apps/backup/src/mirror.ts @@ -3,7 +3,7 @@ import { existsSync } from 'node:fs'; import { join } from 'node:path'; import { promisify } from 'node:util'; -import type { BackupEnv, BackupMirrorStatus } from '@dorfteich/shared'; +import { isBackupTargetAllowed, type BackupEnv, type BackupMirrorStatus } from '@dorfteich/shared'; import { sendMirrorFailureMail } from './mail.js'; import type { RemoteLogger } from './remote.js'; @@ -26,9 +26,20 @@ export interface MirrorConfig { sshPort: number; } -/** The mirror configuration, or null when the env does not enable it. */ -export function resolveMirrorConfig(env: BackupEnv): MirrorConfig | null { +/** + * The mirror configuration, or null when the env does not enable it — or + * when the target host is outside the deploy-level allowlist (issue #192, + * ADR 0026): an empty `BACKUP_ALLOWED_TARGETS` disables the mirror too. + */ +export function resolveMirrorConfig(env: BackupEnv, log?: RemoteLogger): MirrorConfig | null { if (!env.BACKUP_MIRROR_TARGET || !env.BACKUP_MIRROR_SSH_KEY) return null; + if (!isBackupTargetAllowed(env.BACKUP_ALLOWED_TARGETS, env.BACKUP_MIRROR_TARGET)) { + log?.warn( + { target: env.BACKUP_MIRROR_TARGET, allowlist: env.BACKUP_ALLOWED_TARGETS }, + 'rsync mirror blocked: host not in BACKUP_ALLOWED_TARGETS (issue #192)', + ); + return null; + } return { target: env.BACKUP_MIRROR_TARGET, sshKeyFile: env.BACKUP_MIRROR_SSH_KEY, diff --git a/apps/backup/src/remote.ts b/apps/backup/src/remote.ts index 27132cf..09581db 100644 --- a/apps/backup/src/remote.ts +++ b/apps/backup/src/remote.ts @@ -5,6 +5,7 @@ import { Readable } from 'node:stream'; import { pipeline } from 'node:stream/promises'; import { + isBackupTargetAllowed, remoteBundleId, remoteBundleName, type BackupEnv, @@ -41,14 +42,26 @@ export interface RemoteLogger { * The effective WebDAV target, or null when the feature is off or not fully * configured. The app password comes from the wizard-written secret store * (never the database); base URL and username from instance settings. + * Enforces the deploy-level target policy (issue #192, ADR 0026) at the + * point of egress: a configured host outside `BACKUP_ALLOWED_TARGETS` + * behaves like no target — `log` (when given) says why. */ export function resolveRemoteTarget( settings: BackupDbSettings, secrets: Record, + allowlist: string[], + log?: RemoteLogger, ): WebDavTarget | null { const { enabled, baseUrl, username, folder } = settings.nextcloud; const password = secrets[NEXTCLOUD_PASSWORD_SECRET_KEY] ?? ''; if (!enabled || !baseUrl || !username || !password) return null; + if (!isBackupTargetAllowed(allowlist, baseUrl)) { + log?.warn( + { baseUrl, allowlist }, + 'remote backup target blocked: host not in BACKUP_ALLOWED_TARGETS (issue #192)', + ); + return null; + } return { baseUrl, username, password, folder }; } diff --git a/apps/backup/src/settings.test.ts b/apps/backup/src/settings.test.ts index b329d1e..aaa0ba3 100644 --- a/apps/backup/src/settings.test.ts +++ b/apps/backup/src/settings.test.ts @@ -46,8 +46,12 @@ describe('resolveRemoteTarget', () => { { key: 'backup.nextcloud.username', value: 'backupuser' }, ]); + const allowlist = ['cloud.example.com']; + it('combines settings with the secret-store app password', () => { - expect(resolveRemoteTarget(settings, { BACKUP_NEXTCLOUD_PASSWORD: 'app-pass' })).toEqual({ + expect( + resolveRemoteTarget(settings, { BACKUP_NEXTCLOUD_PASSWORD: 'app-pass' }, allowlist), + ).toEqual({ baseUrl: 'https://cloud.example.com', username: 'backupuser', password: 'app-pass', @@ -56,11 +60,22 @@ describe('resolveRemoteTarget', () => { }); it('is null when disabled or incompletely configured', () => { - expect(resolveRemoteTarget(settings, {})).toBeNull(); + expect(resolveRemoteTarget(settings, {}, allowlist)).toBeNull(); expect( - resolveRemoteTarget(parseBackupSettings([]), { BACKUP_NEXTCLOUD_PASSWORD: 'app-pass' }), + resolveRemoteTarget( + parseBackupSettings([]), + { BACKUP_NEXTCLOUD_PASSWORD: 'app-pass' }, + allowlist, + ), ).toBeNull(); }); + + it('is null when the configured host is outside the deploy allowlist (issue #192)', () => { + const secrets = { BACKUP_NEXTCLOUD_PASSWORD: 'app-pass' }; + expect(resolveRemoteTarget(settings, secrets, ['other.host'])).toBeNull(); + // The empty allowlist disables the WebDAV target outright. + expect(resolveRemoteTarget(settings, secrets, [])).toBeNull(); + }); }); describe('parseCommand', () => { diff --git a/apps/web/src/pages/AdminBackupSection.tsx b/apps/web/src/pages/AdminBackupSection.tsx index 70d3330..11c41e8 100644 --- a/apps/web/src/pages/AdminBackupSection.tsx +++ b/apps/web/src/pages/AdminBackupSection.tsx @@ -291,15 +291,32 @@ function BackupSettingsForm(): React.JSX.Element { />

{t('backup.settings.localRetentionHint')}

+ {!view.remoteTargets.allowed && ( + // Unavailable by deploy policy (#192) — not merely unconfigured. +

+ {t('backup.settings.remoteDisabledByPolicy')} +

+ )} + {view.remoteTargets.allowed && ( +

+ {t('backup.settings.allowedTargets', { + hosts: view.remoteTargets.allowlist.join(', '), + })} +

+ )} -
+