From 67f237733e46ec9fb2459aedbbe55d17bfd82714 Mon Sep 17 00:00:00 2001 From: jubnl Date: Thu, 20 Aug 2026 08:19:02 +0200 Subject: [PATCH] =?UTF-8?q?feat(server):=20api/admin/storage/test=20?= =?UTF-8?q?=E2=80=94=20per-target=20ephemeral=20probes,=20mirror=20targets?= =?UTF-8?q?=20probed=20individually?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- server/src/nest/audit/audit.service.ts | 1 + .../nest/common/validate-managed-routes.ts | 1 + .../nest/storage/storage-admin.controller.ts | 28 +++++- .../src/nest/storage/storage-admin.service.ts | 52 +++++++++- server/src/nest/storage/storage-probe.ts | 47 ++++++++++ server/src/nest/storage/storage-secrets.ts | 16 +++- .../storage/storage-admin.controller.test.ts | 26 ++++- .../storage/storage-admin.service.test.ts | 94 +++++++++++++++++++ .../unit/nest/storage/storage-probe.test.ts | 68 ++++++++++++++ .../unit/nest/storage/storage-secrets.test.ts | 13 +++ 10 files changed, 341 insertions(+), 5 deletions(-) create mode 100644 server/src/nest/storage/storage-probe.ts create mode 100644 server/tests/unit/nest/storage/storage-probe.test.ts diff --git a/server/src/nest/audit/audit.service.ts b/server/src/nest/audit/audit.service.ts index dca7da2c0..9eb7052fe 100644 --- a/server/src/nest/audit/audit.service.ts +++ b/server/src/nest/audit/audit.service.ts @@ -19,6 +19,7 @@ const ACTION_LABELS: Record = { 'admin.plugin_retrust': "re-trusted a plugin's author signing key", 'admin.invite_create': 'created invite', 'admin.storage_update': 'updated storage configuration', + 'admin.storage_test': 'tested a storage backend', 'immich.private_ip_configured': 'configured Immich with private IP', }; diff --git a/server/src/nest/common/validate-managed-routes.ts b/server/src/nest/common/validate-managed-routes.ts index 0b5403d8b..88ee148ac 100644 --- a/server/src/nest/common/validate-managed-routes.ts +++ b/server/src/nest/common/validate-managed-routes.ts @@ -80,6 +80,7 @@ export const MANAGED_ROUTE_ALLOW_LIST: string[] = [ 'PluginsController.reload', 'PluginsController.upload', 'StorageAdminController.get', + 'StorageAdminController.test', 'StorageAdminController.update', ]; diff --git a/server/src/nest/storage/storage-admin.controller.ts b/server/src/nest/storage/storage-admin.controller.ts index 9371306bf..bd26b8ab7 100644 --- a/server/src/nest/storage/storage-admin.controller.ts +++ b/server/src/nest/storage/storage-admin.controller.ts @@ -1,4 +1,4 @@ -import { Body, Controller, Get, HttpException, Put, Req, UseGuards } from '@nestjs/common'; +import { Body, Controller, Get, HttpCode, HttpException, Post, Put, Req, UseGuards } from '@nestjs/common'; import type { Request } from 'express'; import { redactStorageSecrets } from './storage-secrets'; import type { User } from '../../types'; @@ -9,7 +9,7 @@ import { AuditService } from '../audit/audit.service'; import { getClientIp } from '../audit/client-ip'; import { ManagedForbidden } from '../common/managed'; import { StorageAdminService } from './storage-admin.service'; -import { StorageConfigDto } from './storage-admin.dto'; +import { StorageConfigDto, StorageTestRequestDto } from './storage-admin.dto'; /** * /api/admin/storage — the admin surface over the storage registry (spec: @@ -48,4 +48,28 @@ export class StorageAdminController { // The defaults-tab contract: never echo the request — answer the fresh effective world. return this.service.state(); } + + @Post('test') + @HttpCode(200) // a probe answers 200 with per-target results, not a 201 resource + async test(@CurrentUser() user: User, @Body() body: StorageTestRequestDto, @Req() req: Request) { + let result; + try { + result = await this.service.testBackend(body.backend); + } catch (err) { + throw new HttpException({ error: err instanceof Error ? err.message : String(err) }, 400); + } + // Audited because the probe writes and deletes an object; names only, never options. + this.audit.writeAudit({ + userId: user.id, + action: 'admin.storage_test', + ip: getClientIp(req), + details: { + backend: body.backend.name, + type: body.backend.type, + ok: result.ok, + targets: result.targets.map(({ name, ok }) => ({ name, ok })), + }, + }); + return result; + } } diff --git a/server/src/nest/storage/storage-admin.service.ts b/server/src/nest/storage/storage-admin.service.ts index ab368c2d7..dff077200 100644 --- a/server/src/nest/storage/storage-admin.service.ts +++ b/server/src/nest/storage/storage-admin.service.ts @@ -1,6 +1,6 @@ import fs from 'node:fs'; import { Injectable } from '@nestjs/common'; -import type { StorageAdminState, StorageConfig } from '@trek/shared'; +import type { StorageAdminState, StorageBackend, StorageConfig, StorageTestResponse } from '@trek/shared'; import { DatabaseService } from '../database/database.service'; import { RuntimeEnvService } from '../app-config/runtime-env.service'; import { @@ -11,12 +11,14 @@ import { import { StorageService } from './storage.service'; import { SEED_CONFIG_PATH } from './storage-paths'; import { + decryptBackendSecrets, encryptionGateError, encryptStorageSecrets, listPlaintextSecrets, maskBackendOptions, unmaskStorageConfig, } from './storage-secrets'; +import { ephemeralDriverFor, probeDriver, type ProbeTargetResult } from './storage-probe'; import { StorageBackendError } from './storage.types'; /** @@ -76,6 +78,54 @@ export class StorageAdminService { this.registry.reload(); } + /** + * Probe an (unsaved or stored) candidate with ephemeral drivers. Mirrors are + * expanded and probed per target — the probe bypasses MirrorDriver, which + * hides replica failures by design. Registry state is never touched. + */ + async testBackend(candidate: StorageBackend): Promise { + const { backends } = unmaskStorageConfig( + { backends: [candidate], categories: {} }, + this.storedBackendsRow(), + ); + const backend = backends[0]!; + const targets = this.probeTargetsFor(backend).map(decryptBackendSecrets) as Array< + Extract + >; + const results: ProbeTargetResult[] = []; + for (const target of targets) { + results.push(await this.probeTarget(target)); + } + return { ok: results.every((r) => r.ok), targets: results }; + } + + /** Expand a mirror candidate into its concrete targets via the live snapshot. */ + private probeTargetsFor(backend: StorageBackend): StorageBackend[] { + if (backend.type !== 'mirror') return [backend]; + const byName = new Map(this.registry.snapshot().backends.map((b) => [b.name, b])); + const names = [backend.options.primary, ...backend.options.replicas]; + return names.map((name) => { + const resolved = byName.get(name); + if (!resolved) { + throw new StorageBackendError(`mirror '${backend.name}' references unknown backend '${name}'`); + } + if (resolved.type === 'mirror') { + throw new StorageBackendError(`mirror '${backend.name}' nests mirror '${name}' — nesting is rejected`); + } + return { name: resolved.name, type: resolved.type, options: resolved.options } as StorageBackend; + }); + } + + private async probeTarget(target: Extract): Promise { + let driver; + try { + driver = ephemeralDriverFor(target); + } catch (err) { + return { name: target.name, ok: false, error: err instanceof Error ? err.message : String(err) }; + } + return probeDriver(target.name, driver); + } + /** The raw stored backends row — the unmask source (tolerates absent/garbage rows). */ private storedBackendsRow(): unknown { const row = this.db.get<{ value: string }>('SELECT value FROM app_settings WHERE key = ?', BACKENDS_KEY); diff --git a/server/src/nest/storage/storage-probe.ts b/server/src/nest/storage/storage-probe.ts new file mode 100644 index 000000000..3e024c932 --- /dev/null +++ b/server/src/nest/storage/storage-probe.ts @@ -0,0 +1,47 @@ +import { randomUUID } from 'node:crypto'; +import { Readable } from 'node:stream'; +import type { StorageBackend } from '@trek/shared'; +import { LocalDriver } from './drivers/local.driver'; +import { S3Driver } from './drivers/s3.driver'; +import { StorageBackendError, type StorageDriver } from './storage.types'; + +/** + * Test-connection probes (spec: POST /api/admin/storage/test). Drivers are + * EPHEMERAL — constructed per probe, never registered, never touching + * registry state. A mirror is probed per target (primary and each replica + * individually) because MirrorDriver hides replica failures by design. + * + * Residue: a local probe may leave an empty `trek-probe/` directory in the + * target root. It is inert — not a category prefix, invisible to lists + * (objects only), and outside every retention pattern. + */ + +export interface ProbeTargetResult { + name: string; + ok: boolean; + error?: string; +} + +/** put → stat → delete of a unique probe key; any throw becomes the target's error. */ +export async function probeDriver(name: string, driver: StorageDriver): Promise { + const key = `trek-probe/${randomUUID()}`; + try { + await driver.put(key, Readable.from('trek storage probe')); + const stat = await driver.stat(key); + if (!stat) throw new StorageBackendError('probe object vanished between put and stat'); + await driver.delete(key); + return { name, ok: true }; + } catch (err) { + return { name, ok: false, error: err instanceof Error ? err.message : String(err) }; + } +} + +/** A non-mirror backend definition (secrets already decrypted) → throwaway driver. */ +export function ephemeralDriverFor(backend: Extract): StorageDriver { + if (backend.type === 'local') { + const driver = new LocalDriver({ id: `probe-${backend.name}`, root: backend.options.root }); + driver.init({ ensurePrefixes: [], cleanSpool: false }); + return driver; + } + return new S3Driver({ id: `probe-${backend.name}`, ...backend.options }); +} diff --git a/server/src/nest/storage/storage-secrets.ts b/server/src/nest/storage/storage-secrets.ts index ed356e887..68a99d4f6 100644 --- a/server/src/nest/storage/storage-secrets.ts +++ b/server/src/nest/storage/storage-secrets.ts @@ -5,7 +5,7 @@ import { type StorageBackendTypeId, type StorageConfig, } from '@trek/shared'; -import { is_encrypted_api_key, maybe_encrypt_api_key } from '../common/crypto/apiKeyCrypto'; +import { decrypt_api_key, is_encrypted_api_key, maybe_encrypt_api_key } from '../common/crypto/apiKeyCrypto'; import { StorageBackendError } from './storage.types'; /** @@ -131,3 +131,17 @@ export function redactStorageSecrets(config: StorageConfig): StorageConfig { backends: config.backends.map((backend) => mapSecrets(backend, () => '***')), }; } + +/** Decrypt secret fields for ephemeral use (test probes). Never persisted. */ +export function decryptBackendSecrets(backend: StorageBackend): StorageBackend { + return mapSecrets(backend, (value, field) => { + if (typeof value !== 'string' || value === '') return value; + const plain = decrypt_api_key(value); + if (plain === null) { + throw new StorageBackendError( + `${backend.type} backend '${backend.name}': could not decrypt '${field}' — was ENCRYPTION_KEY changed or the row edited by hand?`, + ); + } + return plain; + }); +} diff --git a/server/tests/unit/nest/storage/storage-admin.controller.test.ts b/server/tests/unit/nest/storage/storage-admin.controller.test.ts index 49dd058f5..e3e80a883 100644 --- a/server/tests/unit/nest/storage/storage-admin.controller.test.ts +++ b/server/tests/unit/nest/storage/storage-admin.controller.test.ts @@ -5,7 +5,7 @@ import type { User } from '../../../../src/types'; import { StorageAdminController } from '../../../../src/nest/storage/storage-admin.controller'; import type { StorageAdminService } from '../../../../src/nest/storage/storage-admin.service'; import type { AuditService } from '../../../../src/nest/audit/audit.service'; -import type { StorageConfigDto } from '../../../../src/nest/storage/storage-admin.dto'; +import type { StorageConfigDto, StorageTestRequestDto } from '../../../../src/nest/storage/storage-admin.dto'; import { StorageModule } from '../../../../src/nest/storage/storage.module'; import { StorageBackendError } from '../../../../src/nest/storage/storage.types'; import { expectRegisteredController } from '../../../helpers/module-providers'; @@ -87,4 +87,28 @@ describe('StorageAdminController', () => { it('STORCTL-004 the controller is registered in StorageModule', () => { expectRegisteredController(StorageModule, StorageAdminController); }); + + it('STORCTL-005 POST /test probes, audits names only, answers 200-shaped result', async () => { + const testResult = { ok: true, targets: [{ name: 'cand', ok: true }] }; + const { controller, writeAudit } = makeController({ testBackend: vi.fn(async () => testResult) }); + const result = await controller.test(user, { backend: CONFIG.backends[0]! } as StorageTestRequestDto, req); + expect(result).toBe(testResult); + expect(writeAudit).toHaveBeenCalledWith( + expect.objectContaining({ + action: 'admin.storage_test', + details: { backend: 'off-box', type: 's3', ok: true, targets: [{ name: 'cand', ok: true }] }, + }), + ); + }); + + it('STORCTL-006 POST /test maps thrown refusals to 400 verbatim', async () => { + const { controller } = makeController({ + testBackend: vi.fn(async () => { + throw new StorageBackendError("mirror 'm' references unknown backend 'nope'"); + }), + }); + await expect( + controller.test(user, { backend: CONFIG.backends[0]! } as StorageTestRequestDto, req), + ).rejects.toMatchObject({ status: 400 }); + }); }); diff --git a/server/tests/unit/nest/storage/storage-admin.service.test.ts b/server/tests/unit/nest/storage/storage-admin.service.test.ts index 670a69560..f5a9aff59 100644 --- a/server/tests/unit/nest/storage/storage-admin.service.test.ts +++ b/server/tests/unit/nest/storage/storage-admin.service.test.ts @@ -215,3 +215,97 @@ describe('StorageAdminService.applyConfig', () => { expect(reloadSpy).toHaveBeenCalledTimes(1); }); }); + +describe('StorageAdminService.testBackend', () => { + it('STORADM-020 probes a healthy local candidate: ok with one green target', async () => { + const { service } = makeService(); + const result = await service.testBackend({ name: 'cand', type: 'local', options: { root: makeTmpDir() } }); + expect(result).toEqual({ ok: true, targets: [{ name: 'cand', ok: true }] }); + }); + + it('STORADM-021 an unreachable s3 candidate fails fast with a per-target error (no registry impact)', async () => { + const { service, registry } = makeService(); + const result = await service.testBackend({ + name: 'cand', + type: 's3', + options: { + endpoint: 'http://127.0.0.1:1', + bucket: 'trek', + accessKeyId: 'ak', + secretAccessKey: 'sk', + region: 'us-east-1', + keyPrefix: '', + retries: 0, + timeoutMs: 200, + }, + }); + expect(result.ok).toBe(false); + expect(result.targets[0]!.ok).toBe(false); + expect(result.targets[0]!.error).toBeTruthy(); + expect(registry.resolve('backups').backendName).toBe('backups-local'); // state untouched + }); + + it('STORADM-022 a mirror is probed per target, replica failures reported individually', async () => { + const goodRoot = makeTmpDir(); + // Not yet a file: registry.build() eagerly driver.init()s EVERY local + // backend (used or not) during applyConfig's preview, so a root that's + // already a file at that point would make applyConfig itself throw + // (unrelated to testBackend). Register it as a fresh, empty dir, then + // corrupt it afterward — testBackend's ephemeralDriverFor re-runs + // init() fresh at probe time and hits the same EEXIST there instead. + const badRoot = path.join(makeTmpDir(), 'a-file'); + const { service, uploadsRoot } = makeService(); + service.applyConfig(configWith(uploadsRoot, { + backends: [ + { name: 'good-local', type: 'local', options: { root: goodRoot } }, + { name: 'bad-local', type: 'local', options: { root: badRoot } }, + ], + categories: {}, + })); + fs.rmSync(badRoot, { recursive: true, force: true }); + fs.writeFileSync(badRoot, 'not a dir'); + const result = await service.testBackend({ + name: 'cand-mirror', + type: 'mirror', + options: { primary: 'good-local', replicas: ['bad-local'] }, + }); + expect(result.ok).toBe(false); + expect(result.targets).toEqual([ + { name: 'good-local', ok: true }, + expect.objectContaining({ name: 'bad-local', ok: false }), + ]); + }); + + it('STORADM-023 a mirror referencing an unknown or mirror-typed backend throws (→ 400)', async () => { + const { service } = makeService(); + await expect( + service.testBackend({ name: 'm', type: 'mirror', options: { primary: 'nope', replicas: [] } }), + ).rejects.toThrow("mirror 'm' references unknown backend 'nope'"); + }); + + it('STORADM-024 unmasks a stored backend by name before probing (mask echo works on /test)', async () => { + const { service, uploadsRoot } = makeService(); + const root = makeTmpDir(); + service.applyConfig(configWith(uploadsRoot, { + backends: [{ name: 'nas', type: 'local', options: { root } }], + categories: {}, + })); + // local has no secrets — the unmask contract is pinned via an s3 mask with no counterpart: + await expect( + service.testBackend({ + name: 'ghost', + type: 's3', + options: { + endpoint: 'http://127.0.0.1:1', + bucket: 'trek', + accessKeyId: 'ak', + secretAccessKey: MASKED_SETTING_VALUE, + region: 'us-east-1', + keyPrefix: '', + retries: 0, + timeoutMs: 200, + }, + }), + ).rejects.toThrow("re-enter the secret 'secretAccessKey' for 'ghost'"); + }); +}); diff --git a/server/tests/unit/nest/storage/storage-probe.test.ts b/server/tests/unit/nest/storage/storage-probe.test.ts new file mode 100644 index 000000000..f0cfd242a --- /dev/null +++ b/server/tests/unit/nest/storage/storage-probe.test.ts @@ -0,0 +1,68 @@ +import { describe, it, expect, vi } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { LocalDriver } from '../../../../src/nest/storage/drivers/local.driver'; +import { S3Driver } from '../../../../src/nest/storage/drivers/s3.driver'; +import { ephemeralDriverFor, probeDriver } from '../../../../src/nest/storage/storage-probe'; +import type { StorageDriver } from '../../../../src/nest/storage/storage.types'; + +function makeTmpDir(): string { + return fs.mkdtempSync(path.join(os.tmpdir(), 'trek-probe-')); +} + +describe('probeDriver', () => { + it('PROBE-001 round-trips put/stat/delete against a healthy local driver and leaves no object', async () => { + const root = makeTmpDir(); + const driver = new LocalDriver({ id: 'probe-t', root }); + driver.init({ ensurePrefixes: [], cleanSpool: false }); + const result = await probeDriver('t', driver); + expect(result).toEqual({ name: 't', ok: true }); + // The probe object itself is gone (an empty trek-probe/ dir may remain — inert). + const leftovers: string[] = []; + for await (const obj of driver.list('')) leftovers.push(obj.key); + expect(leftovers).toEqual([]); + }); + + it('PROBE-002 captures a failing driver as ok:false with the error message', async () => { + const driver = { + id: 'boom', + put: vi.fn().mockRejectedValue(new Error('disk on fire')), + stat: vi.fn(), + delete: vi.fn(), + getStream: vi.fn(), + list: vi.fn(), + } as unknown as StorageDriver; + const result = await probeDriver('boom', driver); + expect(result.ok).toBe(false); + expect(result.error).toContain('disk on fire'); + }); +}); + +describe('ephemeralDriverFor', () => { + it('PROBE-003 builds an initialized LocalDriver for local and an S3Driver for s3 — no registry involved', () => { + const local = ephemeralDriverFor({ name: 'l', type: 'local', options: { root: makeTmpDir() } }); + expect(local).toBeInstanceOf(LocalDriver); + const s3 = ephemeralDriverFor({ + name: 's', + type: 's3', + options: { + endpoint: 'http://127.0.0.1:1', + bucket: 'trek', + accessKeyId: 'ak', + secretAccessKey: 'sk', + region: 'us-east-1', + keyPrefix: '', + retries: 0, + timeoutMs: 200, + }, + }); + expect(s3).toBeInstanceOf(S3Driver); + }); + + it('PROBE-004 an uncreatable local root surfaces as a throw (the service turns it into a target error)', () => { + const file = path.join(makeTmpDir(), 'a-file'); + fs.writeFileSync(file, 'not a dir'); + expect(() => ephemeralDriverFor({ name: 'l', type: 'local', options: { root: file } })).toThrow(); + }); +}); diff --git a/server/tests/unit/nest/storage/storage-secrets.test.ts b/server/tests/unit/nest/storage/storage-secrets.test.ts index d978e1661..eeef28c46 100644 --- a/server/tests/unit/nest/storage/storage-secrets.test.ts +++ b/server/tests/unit/nest/storage/storage-secrets.test.ts @@ -9,6 +9,7 @@ import { } from '../../../../src/nest/common/crypto/apiKeyCrypto'; import { assertNoMaskSentinels, + decryptBackendSecrets, encryptionGateError, encryptStorageSecrets, listPlaintextSecrets, @@ -109,6 +110,18 @@ describe('assertNoMaskSentinels', () => { }); }); +describe('decryptBackendSecrets', () => { + it('decrypts enc:v1: secrets, passes plaintext through, throws on tampered ciphertext', () => { + const encrypted = s3Config(encrypt_api_key('sk-secret')).backends[0]!; + expect((decryptBackendSecrets(encrypted).options as Record).secretAccessKey).toBe('sk-secret'); + const plain = s3Config('sk-plain').backends[0]!; + expect((decryptBackendSecrets(plain).options as Record).secretAccessKey).toBe('sk-plain'); + expect(() => decryptBackendSecrets(s3Config('enc:v1:AAAA').backends[0]!)).toThrow( + "could not decrypt 'secretAccessKey'", + ); + }); +}); + describe('maskBackendOptions / redactStorageSecrets', () => { it('masks exactly the secret fields on GET-shaped options', () => { const masked = maskBackendOptions('s3', { ...S3_OPTIONS });