diff --git a/app/api/plugin-secrets/route.ts b/app/api/plugin-secrets/route.ts index e4080fcb5..e235b5a0c 100644 --- a/app/api/plugin-secrets/route.ts +++ b/app/api/plugin-secrets/route.ts @@ -12,6 +12,7 @@ import { putUserCredential, type VaultUser, } from '@/lib/plugin-secrets/vault'; +import { storedDataUnavailableResponse } from '@/lib/stored-data-errors'; const PLUGIN_ID_RE = /^[a-z0-9][a-z0-9-]*[a-z0-9]$/; const MAX_SECRET_LENGTH = 4096; @@ -35,7 +36,14 @@ export async function GET(request: NextRequest) { if (pluginId !== undefined && !PLUGIN_ID_RE.test(pluginId)) { return NextResponse.json({ error: 'Invalid plugin id' }, { status: 400 }); } - const credentials = await listUserCredentials(user, pluginId); + let credentials; + try { + credentials = await listUserCredentials(user, pluginId); + } catch (error) { + const unavailable = storedDataUnavailableResponse(error, NO_STORE); + if (unavailable) return unavailable; + throw error; + } // For one plugin, also say which of its admin credentials are set up, and // on which servers: the host page checks the links a plugin shows against // them (the plugin itself only gets yes/no, see proxyAccounts). @@ -93,6 +101,8 @@ export async function POST(request: NextRequest) { }); } catch (error) { if (error instanceof VaultFullError) return NextResponse.json({ error: error.message }, { status: 409 }); + const unavailable = storedDataUnavailableResponse(error, NO_STORE); + if (unavailable) return unavailable; throw error; } return NextResponse.json({ ok: true }, { headers: NO_STORE }); @@ -108,7 +118,13 @@ export async function DELETE(request: NextRequest) { const credentialId = request.nextUrl.searchParams.get('credential') ?? undefined; if (!PLUGIN_ID_RE.test(pluginId)) return bad('Invalid plugin id'); if (credentialId !== undefined && !CREDENTIAL_ID_RE.test(credentialId)) return bad('Invalid credential id'); - await deleteUserCredentials(user, pluginId, credentialId); + try { + await deleteUserCredentials(user, pluginId, credentialId); + } catch (error) { + const unavailable = storedDataUnavailableResponse(error, NO_STORE); + if (unavailable) return unavailable; + throw error; + } return NextResponse.json({ ok: true }, { headers: NO_STORE }); } diff --git a/app/api/settings/route.ts b/app/api/settings/route.ts index 18080c4b5..b1ecf91e6 100644 --- a/app/api/settings/route.ts +++ b/app/api/settings/route.ts @@ -12,6 +12,7 @@ import { hasSessionSecret } from '@/lib/auth/session-secret'; import { MAX_ACCOUNT_SLOTS } from '@/lib/account-utils'; import { mergeStoredTemplates } from '@/lib/template-merge'; import { mergeStoredSubscriptions } from '@/lib/calendar-subscription-sync'; +import { storedDataUnavailableResponse } from '@/lib/stored-data-errors'; /** Largest settings blob a client may store (templates and signatures included). */ const SETTINGS_MAX_BYTES = 2 * 1024 * 1024; @@ -141,6 +142,8 @@ export async function GET(request: NextRequest) { const message = error instanceof Error ? error.message : 'Unknown error'; const code = (error as NodeJS.ErrnoException).code; logger.error('Settings load error', { error: message, code }); + const unavailable = storedDataUnavailableResponse(error); + if (unavailable) return unavailable; const classified = classifyError(error); return NextResponse.json({ error: classified.message }, { status: classified.status }); } @@ -221,6 +224,10 @@ export async function POST(request: NextRequest) { const message = error instanceof Error ? error.message : 'Unknown error'; const code = (error as NodeJS.ErrnoException).code; logger.error('Settings save error', { error: message, code }); + // Stored settings that don't decrypt, or no store: write nothing, so + // the user's real settings survive and the client keeps its copy. + const unavailable = storedDataUnavailableResponse(error); + if (unavailable) return unavailable; const classified = classifyError(error); return NextResponse.json({ error: classified.message }, { status: classified.status }); } @@ -252,6 +259,8 @@ export async function DELETE(request: NextRequest) { const message = error instanceof Error ? error.message : 'Unknown error'; const code = (error as NodeJS.ErrnoException).code; logger.error('Settings delete error', { error: message, code }); + const unavailable = storedDataUnavailableResponse(error); + if (unavailable) return unavailable; const classified = classifyError(error); return NextResponse.json({ error: classified.message }, { status: classified.status }); } diff --git a/lib/__tests__/stored-data-unreadable.test.ts b/lib/__tests__/stored-data-unreadable.test.ts new file mode 100644 index 000000000..db9948f7a --- /dev/null +++ b/lib/__tests__/stored-data-unreadable.test.ts @@ -0,0 +1,162 @@ +// @vitest-environment node +import { mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { NextRequest } from 'next/server'; + +// A settings file or plugin vault that exists but doesn't decrypt (written +// with another session secret, or cut short) used to read as "nothing +// stored". The next save then merged into nothing and overwrote the user's +// real data. Now the read throws, and the routes answer 503 and write nothing. + +let secret = 'a'.repeat(64); +vi.mock('@/lib/auth/session-secret', () => ({ + getSessionSecret: () => secret, + hasSessionSecret: () => true, +})); +vi.mock('@/lib/logger', () => ({ + logger: { warn: () => {}, error: () => {}, info: () => {}, debug: () => {} }, +})); +vi.mock('@/lib/admin/config-manager', () => ({ + configManager: { + ensureLoaded: async () => {}, + get: (key: string, fallback: unknown) => (key === 'settingsSyncEnabled' ? true : fallback), + getPolicy: () => ({ restrictions: {}, features: { pluginsEnabled: true } }), + }, +})); +vi.mock('@/lib/admin/third-party-mode', () => ({ isNoThirdPartyTrafficOn: async () => false, thirdPartyRefusal: () => null })); + +const jar = new Map(); +vi.mock('next/headers', () => ({ + cookies: async () => ({ + get: (name: string) => (jar.has(name) ? { name, value: jar.get(name)! } : undefined), + set: (name: string, value: string) => { jar.set(name, value); }, + delete: (name: string) => { jar.delete(name); }, + getAll: () => [...jar].map(([name, value]) => ({ name, value })), + }), +})); + +const SERVER = 'https://mail.example.org'; +const USER = 'alice@example.org'; +vi.mock('@/lib/stalwart/credentials', () => ({ + getStalwartCredentials: async () => ({ username: USER, accountName: USER, serverUrl: SERVER, authHeader: 'x' }), +})); +vi.mock('@/lib/plugin-secrets/known-plugins', () => ({ + declaredUserCredential: async () => ({ scope: 'user', label: 'Nextcloud', inject: { type: 'basic' } }), +})); + +let dataDir: string; + +beforeEach(async () => { + dataDir = mkdtempSync(path.join(tmpdir(), 'bw-unreadable-')); + vi.stubEnv('SETTINGS_DATA_DIR', path.join(dataDir, 'settings')); + vi.stubEnv('PLUGIN_SECRETS_DIR', path.join(dataDir, 'plugin-secrets')); + vi.stubEnv('SETTINGS_SYNC_ENABLED', 'true'); + secret = 'a'.repeat(64); + jar.clear(); +}); + +afterEach(() => { + vi.unstubAllEnvs(); + rmSync(dataDir, { recursive: true, force: true }); +}); + +/** Sign in after the secret change: the context cookie is sealed with the current secret. */ +async function signIn() { + const { setStalwartAuthContextInStore } = await import('@/lib/stalwart/auth-context'); + setStalwartAuthContextInStore({ + get: (n: string) => (jar.has(n) ? { name: n, value: jar.get(n)! } : undefined), + set: (n: string, v: string) => { jar.set(n, v); }, + delete: (n: string) => { jar.delete(n); }, + } as never, 0, { serverUrl: SERVER, authHeader: 'Bearer t', username: USER }); +} + +function onlyFile(dir: string): string { + const [file] = readdirSync(dir).filter((name) => name.endsWith('.enc')); + return path.join(dir, file); +} + +describe('settings that do not decrypt', () => { + it('reads a missing file as nothing stored and one written with another secret as an error', async () => { + const { loadUserSettings, saveUserSettings } = await import('@/lib/settings-sync'); + const { StoredDataUnreadableError } = await import('@/lib/stored-data-errors'); + expect(await loadUserSettings(USER, SERVER)).toBeNull(); + + await saveUserSettings(USER, SERVER, { theme: 'dark' }); + secret = 'b'.repeat(64); + await expect(loadUserSettings(USER, SERVER)).rejects.toBeInstanceOf(StoredDataUnreadableError); + + writeFileSync(onlyFile(path.join(dataDir, 'settings')), Buffer.alloc(10)); + await expect(loadUserSettings(USER, SERVER)).rejects.toBeInstanceOf(StoredDataUnreadableError); + }); + + it('answers 503 to GET and POST and leaves the stored file as it was', async () => { + const { saveUserSettings } = await import('@/lib/settings-sync'); + await saveUserSettings(USER, SERVER, { theme: 'dark', templates: [{ id: 'a' }] }); + const file = onlyFile(path.join(dataDir, 'settings')); + const before = readFileSync(file); + secret = 'b'.repeat(64); + await signIn(); + + const { GET, POST } = await import('@/app/api/settings/route'); + const got = await GET(new NextRequest('https://webmail.example/api/settings', { + headers: { 'x-settings-username': USER, 'x-settings-server': SERVER }, + })); + expect(got.status).toBe(503); + + const saved = await POST(new NextRequest('https://webmail.example/api/settings', { + method: 'POST', + headers: { 'sec-fetch-site': 'same-origin', 'content-type': 'application/json' }, + body: JSON.stringify({ username: USER, serverUrl: SERVER, settings: { theme: 'light' } }), + })); + expect(saved.status).toBe(503); + expect(readFileSync(file).equals(before)).toBe(true); + }); +}); + +describe('a plugin vault that does not decrypt', () => { + const user = { username: USER, serverUrl: SERVER }; + const input = { origin: 'https://cloud.example.org', inject: { type: 'basic' } as const, label: 'Nextcloud', secret: 'pw' }; + + it('reads a missing vault as empty and one written with another secret as an error', async () => { + const { listUserCredentials, putUserCredential } = await import('@/lib/plugin-secrets/vault'); + const { StoredDataUnreadableError } = await import('@/lib/stored-data-errors'); + expect(await listUserCredentials(user)).toEqual([]); + + await putUserCredential(user, 'nextcloud', 'cloud', input); + secret = 'b'.repeat(64); + await expect(listUserCredentials(user)).rejects.toBeInstanceOf(StoredDataUnreadableError); + // A write merges into what is stored, so it must not land either. + await expect(putUserCredential(user, 'other', 'x', input)).rejects.toBeInstanceOf(StoredDataUnreadableError); + }); + + it('answers 503 on the credential routes and the proxy and leaves the vault as it was', async () => { + const { putUserCredential } = await import('@/lib/plugin-secrets/vault'); + await putUserCredential(user, 'nextcloud', 'cloud', input); + const file = onlyFile(path.join(dataDir, 'plugin-secrets')); + const before = readFileSync(file); + secret = 'b'.repeat(64); + + const { GET, POST, DELETE } = await import('@/app/api/plugin-secrets/route'); + const { POST: proxy } = await import('@/app/api/plugin-proxy/route'); + const same = { 'sec-fetch-site': 'same-origin' }; + + expect((await GET(new NextRequest('http://localhost/api/plugin-secrets'))).status).toBe(503); + const post = await POST(new NextRequest('http://localhost/api/plugin-secrets', { + method: 'POST', + headers: same, + body: JSON.stringify({ pluginId: 'nextcloud', credentialId: 'other', label: 'x', origin: 'https://cloud.example.org', inject: { type: 'basic' }, secret: 's' }), + })); + expect(post.status).toBe(503); + const del = await DELETE(new NextRequest('http://localhost/api/plugin-secrets?plugin=nextcloud', { method: 'DELETE', headers: same })); + expect(del.status).toBe(503); + const proxied = await proxy(new NextRequest( + 'http://localhost/api/plugin-proxy?plugin=nextcloud&credential=cloud&url=https%3A%2F%2Fcloud.example.org%2F', + { method: 'POST', headers: same }, + )); + expect(proxied.status).toBe(503); + + expect(readFileSync(file).equals(before)).toBe(true); + }); +}); diff --git a/lib/plugin-secrets/proxy.ts b/lib/plugin-secrets/proxy.ts index 26de61aa9..f9d3653e4 100644 --- a/lib/plugin-secrets/proxy.ts +++ b/lib/plugin-secrets/proxy.ts @@ -12,6 +12,7 @@ import { DisallowedUrlError, fetchPublicUrl } from '@/lib/security/url-guard'; import { authHeaderFor, credentialOrigin, privateProxyOrigins, type CredentialInject, type CredentialSpec } from './credentials'; import { getUserCredential, setUserCredentialSecret, userKey, type VaultUser } from './vault'; import { declaredUserCredential } from './known-plugins'; +import { isStoredDataUnavailable } from '@/lib/stored-data-errors'; /** Largest request body the proxy forwards (one upload chunk). */ export const MAX_PROXY_REQUEST_BYTES = 100 * 1024 * 1024; @@ -269,7 +270,15 @@ export async function proxyRequest(input: ProxyInput): Promise { const method = input.method.toUpperCase(); if (!PROXY_METHODS.has(method)) throw new ProxyError(`Method ${method} is not allowed`, 405, 'bad_method'); - const credential = await resolveCredential(input.user, input.pluginId, input.credentialId); + let credential: ResolvedCredential | null; + try { + credential = await resolveCredential(input.user, input.pluginId, input.credentialId); + } catch (error) { + if (isStoredDataUnavailable(error)) { + throw Object.assign(new ProxyError('The connected accounts could not be read right now', 503, 'store_unavailable'), { retryAfter: 10 }); + } + throw error; + } if (!credential) throw new ProxyError('No such credential', 404, 'no_credential'); let target: URL; @@ -381,6 +390,12 @@ export async function proxyRequest(input: ProxyInput): Promise { audit(error.status, 0, error.code); throw error; } + // A captured secret that couldn't be stored: the vault doesn't read, + // or the shared store is down. Nothing was written. + if (isStoredDataUnavailable(error)) { + audit(503, 0, 'store unavailable'); + throw Object.assign(new ProxyError('The connected accounts could not be read or saved right now', 503, 'store_unavailable'), { retryAfter: 10 }); + } if (error instanceof DisallowedUrlError) { audit(403, 0, 'blocked address'); throw new ProxyError('This server address is not allowed', 403, 'blocked_address'); diff --git a/lib/plugin-secrets/vault.ts b/lib/plugin-secrets/vault.ts index 5a1eee25c..79459941d 100644 --- a/lib/plugin-secrets/vault.ts +++ b/lib/plugin-secrets/vault.ts @@ -7,6 +7,7 @@ import { createHash } from 'node:crypto'; import { readFile, writeFile, mkdir, rename, unlink } from 'node:fs/promises'; import path from 'node:path'; import { decryptPayload, encryptPayload } from '@/lib/auth/crypto'; +import { StoredDataUnreadableError } from '@/lib/stored-data-errors'; import type { CredentialInject } from './credentials'; export interface VaultUser { @@ -56,18 +57,24 @@ function vaultPath(user: VaultUser): string { return file; } +/** + * The user's credentials; {} when none are stored. A file that exists but + * can't be read or doesn't decrypt (another session secret) throws + * StoredDataUnreadableError, so no write lands on top of it. + */ async function load(user: VaultUser): Promise { let raw: string; try { raw = await readFile(vaultPath(user), 'utf8'); } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') return {}; - throw error; + throw new StoredDataUnreadableError('plugin credentials', { cause: error }); } const payload = decryptPayload(raw, 'plugin-secrets'); + if (!payload) throw new StoredDataUnreadableError('plugin credentials'); // Bound to this user: a file copied over from another account's name // decrypts, but carries the other account's key and is ignored. - if (!payload || payload.u !== userKey(user)) return {}; + if (payload.u !== userKey(user)) return {}; const data = payload.d; return data && typeof data === 'object' ? (data as VaultData) : {}; } diff --git a/lib/settings-sync.ts b/lib/settings-sync.ts index dc819046b..a7b8e0742 100644 --- a/lib/settings-sync.ts +++ b/lib/settings-sync.ts @@ -4,6 +4,7 @@ import { existsSync } from 'node:fs'; import path from 'node:path'; import { logger } from '@/lib/logger'; import { getSessionSecret } from '@/lib/auth/session-secret'; +import { StoredDataUnreadableError } from '@/lib/stored-data-errors'; const ALGORITHM = 'aes-256-gcm'; const IV_LENGTH = 12; @@ -52,14 +53,30 @@ export async function saveUserSettings(username: string, serverUrl: string, sett await rename(tmpPath, targetPath); } +/** + * The user's stored settings, or null when none are stored. A file that + * exists but can't be read or decrypted (another session secret, cut short) + * throws StoredDataUnreadableError: treating it as "nothing stored" would + * let the next save overwrite the user's real settings. + */ export async function loadUserSettings(username: string, serverUrl: string): Promise | null> { const filePath = getSettingsPath(username, serverUrl); + let data: Buffer; try { - const data = await readFile(filePath); - if (data.length < IV_LENGTH + TAG_LENGTH) return null; + data = await readFile(filePath); + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null; + logger.warn('Failed to read user settings', { error: error instanceof Error ? error.message : 'Unknown error' }); + throw new StoredDataUnreadableError('settings', { cause: error }); + } + return decryptSettings(data); +} - const key = getKey(); +function decryptSettings(data: Buffer): Record { + if (data.length < IV_LENGTH + TAG_LENGTH) throw new StoredDataUnreadableError('settings'); + const key = getKey(); + try { const iv = data.subarray(0, IV_LENGTH); const tag = data.subarray(IV_LENGTH, IV_LENGTH + TAG_LENGTH); const encrypted = data.subarray(IV_LENGTH + TAG_LENGTH); @@ -70,9 +87,8 @@ export async function loadUserSettings(username: string, serverUrl: string): Pro const decrypted = Buffer.concat([decipher.update(encrypted), decipher.final()]); return JSON.parse(decrypted.toString('utf8')); } catch (error) { - if ((error as NodeJS.ErrnoException).code === 'ENOENT') return null; - logger.warn('Failed to load user settings', { error: error instanceof Error ? error.message : 'Unknown error' }); - return null; + logger.warn('Failed to decrypt user settings', { error: error instanceof Error ? error.message : 'Unknown error' }); + throw new StoredDataUnreadableError('settings', { cause: error }); } } diff --git a/lib/stored-data-errors.ts b/lib/stored-data-errors.ts new file mode 100644 index 000000000..a09534a1d --- /dev/null +++ b/lib/stored-data-errors.ts @@ -0,0 +1,44 @@ +import { NextResponse } from 'next/server'; +import { StoreUnavailableError } from '@/lib/shared-store/types'; + +/** + * A user's stored document exists but can't be read: it doesn't decrypt + * (another session secret), it is cut short, or the read failed. It is not + * the same as "nothing stored": a route that treated it as empty would merge + * into nothing and overwrite the user's real data on the next save. Routes + * answer 503 and write nothing. + */ +export class StoredDataUnreadableError extends Error { + constructor(what: string, options?: { cause?: unknown }) { + super(`Stored ${what} could not be read`, options); + this.name = 'StoredDataUnreadableError'; + } +} + +/** True for the errors that mean "try again later, nothing was written". */ +export function isStoredDataUnavailable(error: unknown): boolean { + return error instanceof StoredDataUnreadableError || error instanceof StoreUnavailableError; +} + +/** + * The 503 for an unreadable document or an unreachable shared store, or null + * for any other error. The client keeps its own copy and retries. + */ +export function storedDataUnavailableResponse( + error: unknown, + headers?: Record, +): NextResponse | null { + if (error instanceof StoredDataUnreadableError) { + return NextResponse.json( + { error: 'Your stored data could not be read on the server. Nothing was changed.', code: 'stored_data_unreadable' }, + { status: 503, headers: { 'Retry-After': '10', ...headers } }, + ); + } + if (error instanceof StoreUnavailableError) { + return NextResponse.json( + { error: 'Storage is temporarily unavailable. Nothing was changed.', code: 'store_unavailable' }, + { status: 503, headers: { 'Retry-After': '10', ...headers } }, + ); + } + return null; +}