diff --git a/backend/src/backups/backup-storage.spec.ts b/backend/src/backups/backup-storage.spec.ts index 80c63f1..c1177bb 100644 --- a/backend/src/backups/backup-storage.spec.ts +++ b/backend/src/backups/backup-storage.spec.ts @@ -41,7 +41,7 @@ describe('Backup trust boundaries', () => { ).toThrow('verification'); expect(() => validateSettings({ - provider: 'ftp' as never, + provider: 'ftp', enabled: false, intervalMinutes: 60, }), @@ -79,3 +79,9 @@ it('rejects non-string manifest dates before status sorting', () => { 'verification', ); }); +it.each([null, [], 'not settings'])( + 'rejects malformed stored settings at the boundary: %p', + (value) => { + expect(() => validateSettings(value)).toThrow('Invalid backup settings'); + }, +); diff --git a/backend/src/backups/backup-storage.ts b/backend/src/backups/backup-storage.ts index b2fdd25..a458fa9 100644 --- a/backend/src/backups/backup-storage.ts +++ b/backend/src/backups/backup-storage.ts @@ -18,6 +18,29 @@ export interface BackupSettings { hostKey?: string; authentication?: 'privateKey' | 'password'; } +export type LocalBackupSettings = BackupSettings & { + provider: 'local'; + directory: string; +}; +export type S3BackupSettings = BackupSettings & { + provider: 's3'; + bucket: string; + region: string; + endpoint: string; + prefix: string; + forcePathStyle: boolean; +}; +export type SftpBackupSettings = BackupSettings & { + provider: 'sftp'; + host: string; + port: number; + username: string; + remoteDirectory: string; + hostKey: string; + authentication: 'privateKey' | 'password'; +}; +export type ValidatedBackupSettings = + LocalBackupSettings | S3BackupSettings | SftpBackupSettings; export interface BackupSecrets { accessKeyId?: string; secretAccessKey?: string; @@ -77,7 +100,10 @@ export function validateSnapshot(value: unknown, id: string): Snapshot { ); return s; } -export function validateSettings(input: BackupSettings): BackupSettings { +export function validateSettings(value: unknown): ValidatedBackupSettings { + if (!value || typeof value !== 'object' || Array.isArray(value)) + throw new BadRequestException('Invalid backup settings.'); + const input = value as BackupSettings; const provider = input.provider ?? 'local'; if ( !['local', 's3', 'sftp'].includes(provider) || @@ -103,7 +129,11 @@ export function validateSettings(input: BackupSettings): BackupSettings { return value; }; if (provider === 'local') - return { ...shared, directory: text(input.directory, 'directory') }; + return { + ...shared, + provider: 'local', + directory: text(input.directory, 'directory'), + }; if (provider === 's3') { const endpoint = input.endpoint || ''; if (endpoint) { @@ -144,6 +174,7 @@ export function validateSettings(input: BackupSettings): BackupSettings { throw new BadRequestException('Invalid S3 bucket.'); return { ...shared, + provider: 's3', bucket, region: text(input.region || 'us-east-1', 'region'), endpoint, @@ -151,6 +182,9 @@ export function validateSettings(input: BackupSettings): BackupSettings { forcePathStyle: input.forcePathStyle ?? false, }; } + const authentication = input.authentication; + if (authentication !== 'password' && authentication !== 'privateKey') + throw new BadRequestException('Invalid SFTP authentication.'); const host = text(input.host, 'host'); const username = text(input.username, 'username'); const remoteDirectory = text(input.remoteDirectory, 'remote directory'); @@ -175,11 +209,12 @@ export function validateSettings(input: BackupSettings): BackupSettings { ); return { ...shared, + provider: 'sftp', host, username, remoteDirectory: remoteDirectory.replace(/\/+$/, ''), hostKey, port: input.port ?? 22, - authentication: input.authentication, + authentication, }; } diff --git a/backend/src/backups/backups.service.spec.ts b/backend/src/backups/backups.service.spec.ts index 8eb935b..3b8e337 100644 --- a/backend/src/backups/backups.service.spec.ts +++ b/backend/src/backups/backups.service.spec.ts @@ -174,7 +174,10 @@ describe('Remote backup settings', () => { storageProvider: 'local', uploadRoot: join(root, 'uploads'), }); - const backup = new BackupsService(config as never, prisma as never); + const backup = await resolveService(BackupsService, [ + { provide: ConfigService, useValue: config }, + { provide: PrismaService, useValue: prisma }, + ]); try { const input = { provider: 's3' as const, @@ -209,6 +212,11 @@ describe('Remote backup settings', () => { expect(capture).not.toHaveBeenCalled(); await backup.runScheduled(new Date('2026-10-12T01:00:00Z')); expect(capture).toHaveBeenCalledWith('scheduled'); + value = { ...status.settings, credentials: null }; + expect((await backup.status()).reason).toBe('STORAGE_UNAVAILABLE'); + await expect(backup.configure(input)).resolves.toMatchObject({ + configured: true, + }); list.mockRejectedValue(new Error('private error')); expect((await backup.status()).reason).toBe('STORAGE_UNAVAILABLE'); } finally { @@ -229,7 +237,10 @@ it('preserves a truncated marker when the first upload rename fails', async () = storageProvider: 'local', uploadRoot: uploads, }); - const backup = new BackupsService(config as never, {} as PrismaService); + const backup = await resolveService(BackupsService, [ + { provide: ConfigService, useValue: config }, + { provide: PrismaService, useValue: {} }, + ]); // A vanished live upload directory reproduces the first-rename failure without touching a database. const internals = backup as unknown as { verifySnapshot: () => Promise; diff --git a/backend/src/backups/backups.service.ts b/backend/src/backups/backups.service.ts index 60e529b..e230803 100644 --- a/backend/src/backups/backups.service.ts +++ b/backend/src/backups/backups.service.ts @@ -31,6 +31,7 @@ import { BACKUP_UUID as UUID, validateSettings, type BackupSettings, + type ValidatedBackupSettings, type BackupStorage, type BackupSecrets, type Snapshot, @@ -43,9 +44,9 @@ import { import { LocalBackupStorage } from './local-backup-storage'; import { S3BackupStorage } from './s3-backup-storage'; import { SftpBackupStorage } from './sftp-backup-storage'; -interface StoredSettings extends BackupSettings { +type StoredSettings = ValidatedBackupSettings & { credentials?: EncryptedSecrets; -} +}; const execute = promisify(execFile); const KEY = 'backups.local'; @@ -111,7 +112,35 @@ export class BackupsService implements OnModuleInit, OnModuleDestroy { const row = await this.prisma.siteSetting.findUnique({ where: { key: KEY }, }); - return row ? (row.value as unknown as StoredSettings) : null; + if (!row) return null; + const settings = validateSettings(row.value); + if ( + row.value && + typeof row.value === 'object' && + !Array.isArray(row.value) && + 'credentials' in row.value && + row.value.credentials !== undefined + ) { + const encrypted = row.value.credentials; + if ( + !encrypted || + typeof encrypted !== 'object' || + Array.isArray(encrypted) || + typeof encrypted.iv !== 'string' || + typeof encrypted.tag !== 'string' || + typeof encrypted.ciphertext !== 'string' + ) + return settings; // Missing credentials can be repaired by re-entering them. + return { + ...settings, + credentials: { + iv: encrypted.iv, + tag: encrypted.tag, + ciphertext: encrypted.ciphertext, + }, + }; + } + return settings; } private credentials() { @@ -124,8 +153,8 @@ export class BackupsService implements OnModuleInit, OnModuleDestroy { settings: StoredSettings, explicitSecrets?: BackupSecrets, ): Promise { - if (!settings.provider || settings.provider === 'local') - return new LocalBackupStorage(settings.directory!); + if (settings.provider === 'local') + return new LocalBackupStorage(settings.directory); const secrets = explicitSecrets ?? (settings.credentials @@ -158,7 +187,7 @@ export class BackupsService implements OnModuleInit, OnModuleDestroy { try { const value: StoredSettings = validateSettings(input); if (value.provider === 'local') - value.directory = await this.storageDirectory(value.directory!); + value.directory = await this.storageDirectory(value.directory); else { await mkdir(resolve(this.config.get('uploadRoot', { infer: true })), { recursive: true, @@ -236,11 +265,11 @@ export class BackupsService implements OnModuleInit, OnModuleDestroy { storage = await this.storage(settings); for (const snapshot of await storage.list()) { let verified: boolean | null = null; - if (!settings.provider || settings.provider === 'local') { + if (settings.provider === 'local') { verified = false; try { await this.verifySnapshot( - join(settings.directory!, snapshot.id), + join(settings.directory, snapshot.id), snapshot, ); verified = true; @@ -261,13 +290,12 @@ export class BackupsService implements OnModuleInit, OnModuleDestroy { storage?.close?.(); } } - const publicSettings = { ...settings }; - delete publicSettings.credentials; + const publicSettings = settings ? validateSettings(settings) : null; backups.sort((a, b) => Date.parse(b.createdAt) - Date.parse(a.createdAt)); return { configured: !!settings, provider: settings ? (settings.provider ?? 'local') : null, - settings: settings ? (publicSettings as BackupSettings) : null, + settings: publicSettings, canBackup: supported && !!settings && !unavailable && !this.busy, canRestore: supported && @@ -425,6 +453,7 @@ export class BackupsService implements OnModuleInit, OnModuleDestroy { ? validateSettings(remote) : directory ? { + provider: 'local', directory: await this.storageDirectory(directory), enabled: false, intervalMinutes: 60, diff --git a/backend/src/backups/remote-backup-storage.spec.ts b/backend/src/backups/remote-backup-storage.spec.ts index 882bbf3..ae2de99 100644 --- a/backend/src/backups/remote-backup-storage.spec.ts +++ b/backend/src/backups/remote-backup-storage.spec.ts @@ -1,3 +1,4 @@ +import { S3Client } from '@aws-sdk/client-s3'; import { mkdtemp, mkdir, writeFile, readFile, rm } from 'node:fs/promises'; import { join } from 'node:path'; import { tmpdir } from 'node:os'; @@ -5,6 +6,13 @@ import { Readable } from 'node:stream'; import { S3BackupStorage } from './s3-backup-storage'; import { SftpBackupStorage, quoteSftp } from './sftp-backup-storage'; import { LocalBackupStorage } from './local-backup-storage'; +function mockedClient(send: unknown) { + if (typeof send !== 'function') + throw new Error('Expected a mocked SDK send function.'); + const client = new S3Client({ region: 'us-east-1' }); + Object.defineProperty(client, 'send', { value: send }); + return client; +} const id = 'aaaaaaaa-aaaa-4aaa-aaaa-aaaaaaaaaaaa'; const snapshot = { version: 1 as const, @@ -61,7 +69,7 @@ describe('Backup storage publication and downloads', () => { bucket: 'bucket', prefix: 'prefix', }, - { send } as never, + mockedClient(send), ); expect(await storage.list()).toEqual([snapshot]); const calls = send.mock.calls as unknown as [ @@ -83,9 +91,7 @@ describe('Backup storage publication and downloads', () => { ); const storage = new S3BackupStorage( { enabled: false, intervalMinutes: 60, bucket: 'bucket' }, - { - send, - } as never, + mockedClient(send), ); await storage.save(snapshot, join(root, 'source')); expect(keys).toEqual([ @@ -101,7 +107,7 @@ describe('Backup storage publication and downloads', () => { await expect( new S3BackupStorage( { enabled: false, intervalMinutes: 60, bucket: 'bucket' }, - { send } as never, + mockedClient(send), ).save(snapshot, join(root, 'source')), ).rejects.toThrow('S3'); expect(send).toHaveBeenCalledTimes(1); @@ -118,7 +124,7 @@ describe('Backup storage publication and downloads', () => { await expect( new S3BackupStorage( { enabled: false, intervalMinutes: 60, bucket: 'bucket' }, - { send } as never, + mockedClient(send), ).load(id, join(root, 'loaded')), ).rejects.toThrow('verification'); expect(send).toHaveBeenCalledTimes(1); diff --git a/backend/src/backups/s3-backup-storage.ts b/backend/src/backups/s3-backup-storage.ts index 9f3e6e7..0a61950 100644 --- a/backend/src/backups/s3-backup-storage.ts +++ b/backend/src/backups/s3-backup-storage.ts @@ -15,31 +15,45 @@ import { validateSnapshot, type BackupStorage, type BackupSettings, + type S3BackupSettings, + validateSettings, type BackupSecrets, type Snapshot, } from './backup-storage'; export class S3BackupStorage implements BackupStorage { private readonly client: S3Client; + private readonly settings: S3BackupSettings; constructor( - private readonly settings: BackupSettings, + settings: BackupSettings, clientOrSecrets: S3Client | BackupSecrets, ) { - this.client = - 'send' in clientOrSecrets - ? clientOrSecrets - : new S3Client({ - region: settings.region, - endpoint: settings.endpoint || undefined, - forcePathStyle: settings.forcePathStyle, - credentials: { - accessKeyId: clientOrSecrets.accessKeyId!, - secretAccessKey: clientOrSecrets.secretAccessKey!, - }, - requestHandler: { - requestTimeout: 300_000, - connectionTimeout: 15_000, - }, - }); + const validated = validateSettings({ + ...settings, + provider: settings.provider ?? 's3', + }); + if (validated.provider !== 's3') + throw new BadRequestException('S3 backup settings required.'); + this.settings = validated; + if ('send' in clientOrSecrets) { + this.client = clientOrSecrets; + return; + } + const { accessKeyId, secretAccessKey } = clientOrSecrets; + if (!accessKeyId || !secretAccessKey) + throw new BadRequestException('S3 backup credentials required.'); + this.client = new S3Client({ + region: this.settings.region, + endpoint: this.settings.endpoint || undefined, + forcePathStyle: this.settings.forcePathStyle, + credentials: { + accessKeyId: accessKeyId, + secretAccessKey: secretAccessKey, + }, + requestHandler: { + requestTimeout: 300_000, + connectionTimeout: 15_000, + }, + }); } close() { this.client.destroy?.(); diff --git a/backend/src/backups/sftp-backup-storage.ts b/backend/src/backups/sftp-backup-storage.ts index 8796375..6188e46 100644 --- a/backend/src/backups/sftp-backup-storage.ts +++ b/backend/src/backups/sftp-backup-storage.ts @@ -9,6 +9,8 @@ import { validateSnapshot, type BackupStorage, type BackupSettings, + type SftpBackupSettings, + validateSettings, type BackupSecrets, type Snapshot, } from './backup-storage'; @@ -53,18 +55,27 @@ async function executeSftp( }); } export class SftpBackupStorage implements BackupStorage { + private readonly settings: SftpBackupSettings; constructor( - private readonly settings: BackupSettings, + settings: BackupSettings, private readonly secrets: BackupSecrets, private readonly execute = executeSftp, - ) {} + ) { + const validated = validateSettings({ + ...settings, + provider: settings.provider ?? 'sftp', + }); + if (validated.provider !== 'sftp') + throw new BadRequestException('SFTP backup settings required.'); + this.settings = validated; + } private path(id: string, file = '') { - return posix.join(this.settings.remoteDirectory!, id, file); + return posix.join(this.settings.remoteDirectory, id, file); } private async run(script: string) { const folder = await mkdtemp(join(tmpdir(), 'amirlab-sftp-')); try { - const host = this.settings.host!; + const host = this.settings.host; const port = this.settings.port ?? 22; const pin = port === 22 ? host : `[${host}]:${port}`; const known = join(folder, 'known_hosts'); @@ -106,7 +117,10 @@ export class SftpBackupStorage implements BackupStorage { ); } else { const key = join(folder, 'identity'); - await writeFile(key, this.secrets.privateKey!, { mode: 0o600 }); + const privateKey = this.secrets.privateKey; + if (!privateKey) + throw new BadRequestException('SFTP private key required.'); + await writeFile(key, privateKey, { mode: 0o600 }); args.push('-i', key); options.push( 'BatchMode=yes', @@ -130,14 +144,14 @@ export class SftpBackupStorage implements BackupStorage { async list() { const parents: string[] = []; let current = ''; - for (const part of this.settings - .remoteDirectory!.split('/') + for (const part of this.settings.remoteDirectory + .split('/') .filter(Boolean)) { current += `/${part}`; parents.push(`@-mkdir ${quoteSftp(current)}`); } const output = await this.run( - [...parents, `@ls -1 ${quoteSftp(this.settings.remoteDirectory!)}`].join( + [...parents, `@ls -1 ${quoteSftp(this.settings.remoteDirectory)}`].join( '\n', ) + '\n', ); diff --git a/frontend/src/components/account-settings.test.tsx b/frontend/src/components/account-settings.test.tsx index 582f4ab..6fa0b8e 100644 --- a/frontend/src/components/account-settings.test.tsx +++ b/frontend/src/components/account-settings.test.tsx @@ -135,7 +135,8 @@ it("groups matching browser sessions across different IPs and keeps each login r ).toHaveLength(1); const summary = screen.getByText("2 similar sessions"); fireEvent.click(summary); - const details = summary.closest("details")!; + const details = summary.closest("details"); + if (!details) throw new Error("Session group disclosure missing"); vi.mocked(apiRequest).mockResolvedValueOnce({ revoked: true }); fireEvent.click(within(details).getByRole("button", { name: "Sign out" })); await waitFor(() => diff --git a/frontend/src/components/account-settings.tsx b/frontend/src/components/account-settings.tsx index 0a9db64..3be6712 100644 --- a/frontend/src/components/account-settings.tsx +++ b/frontend/src/components/account-settings.tsx @@ -279,7 +279,7 @@ export function AccountSettings() { {similar.length ? (
- {group!.length} similar sessions + {similar.length + 1} similar sessions

Same reported browser and device; these may be different diff --git a/frontend/src/components/auth-provider.test.tsx b/frontend/src/components/auth-provider.test.tsx index 9c26a11..4af1447 100644 --- a/frontend/src/components/auth-provider.test.tsx +++ b/frontend/src/components/auth-provider.test.tsx @@ -13,7 +13,7 @@ vi.mock("@/lib/client-api", async (importOriginal) => { }); const pwa = vi.hoisted(() => ({ - registration: null as ServiceWorkerRegistration | null, + registration: null as object | null, })); vi.mock("./pwa-provider", () => ({ usePwa: () => pwa })); const request = vi.mocked(apiRequest); @@ -133,7 +133,7 @@ describe("AuthProvider", () => { const worker = { pushManager: { getSubscription: async () => subscription }, }; - pwa.registration = worker as unknown as ServiceWorkerRegistration; + pwa.registration = worker; vi.stubGlobal("PushManager", class {}); vi.stubGlobal("Notification", { permission: "granted", @@ -182,7 +182,7 @@ describe("AuthProvider", () => { const worker = { pushManager: { getSubscription: async () => subscription }, }; - pwa.registration = worker as unknown as ServiceWorkerRegistration; + pwa.registration = worker; vi.stubGlobal("PushManager", class {}); vi.stubGlobal("Notification", { permission: "granted" }); Object.defineProperty(navigator, "serviceWorker", { diff --git a/frontend/src/components/profile-editor.test.tsx b/frontend/src/components/profile-editor.test.tsx index e0264cb..4586e3f 100644 --- a/frontend/src/components/profile-editor.test.tsx +++ b/frontend/src/components/profile-editor.test.tsx @@ -201,13 +201,13 @@ describe("ProfileEditor auth reconciliation", () => { ).toBe("Staff Member"), ); fireEvent.click(screen.getByRole("button", { name: "Add section" })); - if (filled) - fireEvent.change( - screen - .getByText("Section title") - .parentElement!.querySelector("input")!, - { target: { value: "My research" } }, - ); + if (filled) { + const titleInput = screen + .getByText("Section title") + .parentElement?.querySelector("input"); + if (!titleInput) throw new Error("Section title input missing"); + fireEvent.change(titleInput, { target: { value: "My research" } }); + } auth.mockReturnValue({ ...state, user: { ...user } }); await act(async () => view.rerender()); expect(