diff --git a/docs/speech.md b/docs/speech.md index fdf706c3..e7a1558c 100644 --- a/docs/speech.md +++ b/docs/speech.md @@ -92,6 +92,32 @@ drift apart. Treat audio backups as sensitive clinical data even when encrypted. +### Where object storage settings come from + +`src/utils/objectStorage.js` resolves them for every purpose the same way, so +moving the app to a different MinIO — or to a real S3 — is one set of variables +rather than three schemes. For a purpose (`documents`, `generated-images`, +`audio-backups`) it reads, in order: + +1. that purpose's own variables — `AUDIO_BACKUPS_S3_ENDPOINT`, `..._BUCKET`, + `..._REGION`, `..._ACCESS_KEY[_FILE]`, `..._SECRET_KEY[_FILE]`; +2. the shared ones — `S3_ENDPOINT`, `S3_REGION`, `S3_ACCESS_KEY[_FILE]`, + `S3_SECRET_KEY[_FILE]`; +3. a per-purpose bucket — `S3_BUCKET_AUDIO_BACKUPS`, `S3_BUCKET_GENERATED_IMAGES`. + +So one endpoint plus three bucket names covers everything, while a purpose that +needs its own account overrides all of it. A `_FILE` variant always beats an +inline value, because a mounted secret should not be shadowed by an inherited +environment variable. No bucket means "not configured", which is never an error +— all three are optional. + +Every name previously accepted still works, including `S3_ACCESS_KEY_ID`, +`S3_SECRET_ACCESS_KEY` and the `AWS_*` fallbacks for documents, and +`GENERATED_IMAGES_S3_*`. Path-style addressing keeps each purpose's old default +(off for documents, so Backblaze keeps working) unless `S3_FORCE_PATH_STYLE` +says otherwise; a custom endpoint turns it on where there was no older default, +because that is nearly always MinIO. + ### Switching audio backups to MinIO Storing every recording, rather than only the failures, makes object storage diff --git a/src/routes/documents.js b/src/routes/documents.js index df6d2e15..3f083e8d 100644 --- a/src/routes/documents.js +++ b/src/routes/documents.js @@ -27,30 +27,15 @@ var ALLOWED_TYPES = [ // Lazy-load S3 client var _s3Client = null; +// Settings come from src/utils/objectStorage.js, so documents, generated images +// and audio backups all resolve the same way: the purpose's own variables, then +// the shared S3_* ones. Every name this route has ever accepted still works. function getS3Client() { if (_s3Client) return _s3Client; try { - var { S3Client } = require('@aws-sdk/client-s3'); - var region = process.env.S3_REGION || process.env.AWS_BEDROCK_REGION || 'us-east-1'; - var config = { region: region }; - - // Custom endpoint for S3-compatible providers (Backblaze B2, MinIO, etc.) - if (process.env.S3_ENDPOINT) { - config.endpoint = process.env.S3_ENDPOINT; - config.forcePathStyle = process.env.S3_FORCE_PATH_STYLE === 'true'; // Required for MinIO - } - - // Credentials: use S3-specific keys first, fall back to AWS keys - var accessKey = process.env.S3_ACCESS_KEY_ID || process.env.AWS_ACCESS_KEY_ID; - var secretKey = process.env.S3_SECRET_ACCESS_KEY || process.env.AWS_SECRET_ACCESS_KEY; - if (accessKey && secretKey) { - config.credentials = { - accessKeyId: accessKey, - secretAccessKey: secretKey - }; - } - - _s3Client = new S3Client(config); + var store = require('../utils/objectStorage').storeFor('documents'); + if (!store) return null; + _s3Client = store.client; return _s3Client; } catch (e) { return null; diff --git a/src/utils/audioBackupStore.js b/src/utils/audioBackupStore.js index f08095a6..c1b9a9ed 100644 --- a/src/utils/audioBackupStore.js +++ b/src/utils/audioBackupStore.js @@ -5,12 +5,13 @@ // material: every one is kept for 24 hours, whether its transcription // succeeded or not, and it is always compressed and encrypted at rest. // -// Payload goes to object storage when AUDIO_BACKUPS_S3_* is configured, and -// to Postgres otherwise. Metadata (owner, module, sizes, expiry) always lives -// in Postgres, so listing, ownership and expiry work the same either way. +// Payload goes to object storage when one is configured for the audio-backups +// purpose (see src/utils/objectStorage.js — its own AUDIO_BACKUPS_S3_* or the +// shared S3_* plus a bucket name), and to Postgres otherwise. Metadata (owner, +// module, sizes, expiry) always lives in Postgres, so listing, ownership and +// expiry work the same either way. // ============================================================ -var fs = require('fs'); var zlib = require('zlib'); var db = require('../db/database'); var cryptoUtil = require('./crypto'); @@ -28,37 +29,16 @@ function gunzip(buffer) { }); } -// Credentials come from files, as the rest of the stack does, so they are never -// environment strings in a process listing. -function readSecret(value, file) { - if (file) return fs.readFileSync(file, 'utf8').trim(); - return value || ''; -} +var objectStorage = require('./objectStorage'); var _client = null; function objectStore(env) { - env = env || process.env; - if (!env.AUDIO_BACKUPS_S3_ENDPOINT || !env.AUDIO_BACKUPS_S3_BUCKET) return null; if (_client) return _client; - var { S3Client } = require('@aws-sdk/client-s3'); - _client = { - Bucket: env.AUDIO_BACKUPS_S3_BUCKET, - client: new S3Client({ - endpoint: env.AUDIO_BACKUPS_S3_ENDPOINT, - region: env.AUDIO_BACKUPS_S3_REGION || 'us-east-1', - forcePathStyle: true, - credentials: { - accessKeyId: readSecret(env.AUDIO_BACKUPS_S3_ACCESS_KEY, env.AUDIO_BACKUPS_S3_ACCESS_KEY_FILE), - secretAccessKey: readSecret(env.AUDIO_BACKUPS_S3_SECRET_KEY, env.AUDIO_BACKUPS_S3_SECRET_KEY_FILE) - }, - maxAttempts: 2, - requestHandler: { connectionTimeout: 3000, requestTimeout: 20000 } - }) - }; + _client = objectStorage.storeFor('audio-backups', env); return _client; } -function isObjectStoreConfigured(env) { return !!objectStore(env); } +function isObjectStoreConfigured(env) { return objectStorage.isConfigured('audio-backups', env); } // Keyed by owner so one user's recordings can never be addressed by another, // even if an id leaks. diff --git a/src/utils/objectStorage.js b/src/utils/objectStorage.js new file mode 100644 index 00000000..58f4480b --- /dev/null +++ b/src/utils/objectStorage.js @@ -0,0 +1,115 @@ +// ============================================================ +// OBJECT STORAGE SETTINGS +// One place that answers "which bucket, on which S3, with which credentials", +// so pointing the whole app at a different MinIO — or at a real S3 — is one set +// of variables rather than three schemes that grew separately. +// +// Each purpose (documents, generated images, audio backups) reads, in order: +// +// 1. its own variables AUDIO_BACKUPS_S3_ENDPOINT, ... +// 2. the shared ones S3_ENDPOINT, S3_REGION, S3_ACCESS_KEY, ... +// 3. a per-purpose bucket S3_BUCKET_AUDIO_BACKUPS +// +// So one external endpoint plus three bucket names is enough, while a purpose +// that needs its own account keeps overriding everything. Credentials can be +// given inline or, preferably, as a path to a file — a file keeps them out of +// the process environment, which is where `docker inspect` reads from. +// ============================================================ + +var fs = require('fs'); + +// Historic names, kept working so no deployment has to be edited to upgrade. +var LEGACY = { + documents: { + endpoint: ['S3_ENDPOINT'], + region: ['S3_REGION', 'AWS_BEDROCK_REGION'], + bucket: ['S3_BUCKET'], + accessKey: ['S3_ACCESS_KEY_ID', 'AWS_ACCESS_KEY_ID'], + secretKey: ['S3_SECRET_ACCESS_KEY', 'AWS_SECRET_ACCESS_KEY'], + forcePathStyle: ['S3_FORCE_PATH_STYLE'] + } +}; + +function prefixOf(purpose) { + return String(purpose).toUpperCase().replace(/[^A-Z0-9]+/g, '_') + '_S3_'; +} + +function firstValue(env, names) { + for (var i = 0; i < names.length; i++) { + var value = env[names[i]]; + if (value !== undefined && value !== null && String(value) !== '') return String(value); + } + return ''; +} + +// A *_FILE variant always wins over an inline value: if someone has gone to the +// trouble of mounting a secret, an inherited environment variable must not +// quietly take precedence over it. +function readCredential(env, names) { + var fileNames = names.map(function (name) { return name + '_FILE'; }); + var file = firstValue(env, fileNames); + if (file) { + try { return fs.readFileSync(file, 'utf8').trim(); } + catch (e) { throw new Error('Cannot read credential file ' + file + ': ' + e.message); } + } + return firstValue(env, names); +} + +/** + * Resolve the settings for one purpose. Returns null when no endpoint and no + * bucket can be found, which callers read as "not configured" — never as an + * error, because every one of these is optional. + */ +function settingsFor(purpose, env) { + env = env || process.env; + var own = prefixOf(purpose); + var legacy = LEGACY[purpose] || {}; + + var endpoint = firstValue(env, [own + 'ENDPOINT'].concat(legacy.endpoint || [], ['S3_ENDPOINT'])); + var bucket = firstValue(env, [own + 'BUCKET', 'S3_BUCKET_' + own.replace(/_S3_$/, '')].concat(legacy.bucket || [])); + if (!bucket) return null; + + var region = firstValue(env, [own + 'REGION'].concat(legacy.region || [], ['S3_REGION'])) || 'us-east-1'; + var accessKey = readCredential(env, [own + 'ACCESS_KEY'].concat(legacy.accessKey || [], ['S3_ACCESS_KEY'])); + var secretKey = readCredential(env, [own + 'SECRET_KEY'].concat(legacy.secretKey || [], ['S3_SECRET_KEY'])); + + // MinIO needs path-style addressing; hosted S3 does not care, and Backblaze + // prefers virtual-hosted. An explicit setting always wins. Where a purpose + // already had its own default, that default is kept: documents has always + // been off unless asked for, so a Backblaze endpoint keeps working. + var forced = firstValue(env, [own + 'FORCE_PATH_STYLE'].concat(legacy.forcePathStyle || [], ['S3_FORCE_PATH_STYLE'])); + var forcePathStyle = forced ? forced === 'true' : (legacy.forcePathStyle ? false : !!endpoint); + + return { + purpose: purpose, + endpoint: endpoint || undefined, + region: region, + bucket: bucket, + forcePathStyle: forcePathStyle, + credentials: (accessKey && secretKey) ? { accessKeyId: accessKey, secretAccessKey: secretKey } : undefined + }; +} + +function isConfigured(purpose, env) { return !!settingsFor(purpose, env); } + +// Built per call site; callers cache their own client. +function createClient(settings) { + var { S3Client } = require('@aws-sdk/client-s3'); + var config = { + region: settings.region, + maxAttempts: 2, + requestHandler: { connectionTimeout: 3000, requestTimeout: 20000 } + }; + if (settings.endpoint) { config.endpoint = settings.endpoint; config.forcePathStyle = settings.forcePathStyle; } + if (settings.credentials) config.credentials = settings.credentials; + return new S3Client(config); +} + +// Convenience: settings plus a client, or null when the purpose is unconfigured. +function storeFor(purpose, env) { + var settings = settingsFor(purpose, env); + if (!settings) return null; + return { settings: settings, Bucket: settings.bucket, client: createClient(settings) }; +} + +module.exports = { settingsFor, isConfigured, createClient, storeFor, prefixOf }; diff --git a/test/backend-hardening.test.js b/test/backend-hardening.test.js index 2d084b62..e799c44d 100644 --- a/test/backend-hardening.test.js +++ b/test/backend-hardening.test.js @@ -81,3 +81,48 @@ test('admin routers state their own authentication, not mount order', () => { assert.match(auth, /async function adminMiddleware\(req, res, next\) \{\s*\n\s*if \(!req\.user \|\| req\.user\.role !== 'admin'\)/, 'and the role check stays a role check, so it fails closed on its own'); }); + +// Three S3 schemes grew separately — S3_*, GENERATED_IMAGES_S3_*, and later +// audio backups — which is why pointing the app at a different MinIO meant +// hunting through three files. One resolver answers it for every purpose. +test('object storage resolves the same way for every purpose', () => { + const storage = require('../src/utils/objectStorage'); + + // One endpoint plus a bucket name per purpose is enough for all of them. + const shared = { S3_ENDPOINT: 'https://s3.example.com', S3_ACCESS_KEY: 'AK', S3_SECRET_KEY: 'SK', + S3_BUCKET_AUDIO_BACKUPS: 'audio', S3_BUCKET_GENERATED_IMAGES: 'images', S3_BUCKET: 'docs' }; + for (const [purpose, bucket] of [['audio-backups', 'audio'], ['generated-images', 'images'], ['documents', 'docs']]) { + const resolved = storage.settingsFor(purpose, shared); + assert.equal(resolved.bucket, bucket, purpose + ' finds its bucket'); + assert.equal(resolved.endpoint, 'https://s3.example.com'); + assert.deepEqual(resolved.credentials, { accessKeyId: 'AK', secretAccessKey: 'SK' }); + } + + // A purpose that needs its own account still overrides everything. + const overridden = storage.settingsFor('audio-backups', + Object.assign({}, shared, { AUDIO_BACKUPS_S3_ENDPOINT: 'http://assets:9000', AUDIO_BACKUPS_S3_BUCKET: 'audio-backups' })); + assert.equal(overridden.endpoint, 'http://assets:9000'); + assert.equal(overridden.bucket, 'audio-backups'); + + // Existing deployments keep working untouched, including the old key names. + const legacy = storage.settingsFor('documents', + { S3_BUCKET: 'd', S3_REGION: 'us-west-004', S3_ENDPOINT: 'https://b2', S3_ACCESS_KEY_ID: 'A', S3_SECRET_ACCESS_KEY: 'B' }); + assert.equal(legacy.region, 'us-west-004'); + assert.deepEqual(legacy.credentials, { accessKeyId: 'A', secretAccessKey: 'B' }); + // Documents defaulted path-style off; a Backblaze endpoint must keep working. + assert.equal(legacy.forcePathStyle, false); + assert.equal(storage.settingsFor('audio-backups', { AUDIO_BACKUPS_S3_BUCKET: 'a', AUDIO_BACKUPS_S3_ENDPOINT: 'http://assets:9000' }).forcePathStyle, true, + 'but MinIO needs it, so a custom endpoint turns it on where there is no older default'); + + // No bucket means "not configured" — never an error, since all of this is optional. + assert.equal(storage.settingsFor('audio-backups', {}), null); + assert.equal(storage.isConfigured('audio-backups', {}), false); + + // A mounted secret must not be overridden by an inherited environment value. + const fs = require('node:fs'); const os = require('node:os'); const path = require('node:path'); + const keyFile = path.join(fs.mkdtempSync(path.join(os.tmpdir(), 'objstore-')), 'key'); + fs.writeFileSync(keyFile, 'from-file\n'); + const fileWins = storage.settingsFor('audio-backups', + { AUDIO_BACKUPS_S3_BUCKET: 'a', AUDIO_BACKUPS_S3_ACCESS_KEY: 'inline', AUDIO_BACKUPS_S3_ACCESS_KEY_FILE: keyFile, AUDIO_BACKUPS_S3_SECRET_KEY: 'S' }); + assert.equal(fileWins.credentials.accessKeyId, 'from-file'); +}); diff --git a/test/transcription-memory-policy.test.js b/test/transcription-memory-policy.test.js index d8871404..d5208e9d 100644 --- a/test/transcription-memory-policy.test.js +++ b/test/transcription-memory-policy.test.js @@ -159,7 +159,9 @@ test('every recording is kept for 24 hours, not only the failures', () => { 'a storage failure must not lose the transcription someone is waiting for'); // Object storage when configured, the encrypted database column otherwise. - assert.match(store, /if \(!env\.AUDIO_BACKUPS_S3_ENDPOINT \|\| !env\.AUDIO_BACKUPS_S3_BUCKET\) return null;/); + // Which one, and with what credentials, is resolved centrally now. + assert.match(store, /_client = objectStorage\.storeFor\('audio-backups', env\);/); + assert.match(store, /objectStorage\.isConfigured\('audio-backups', env\)/); assert.match(store, /cryptoUtil\.encryptBuffer\(compressed\)/, 'compressed and encrypted either way'); assert.match(store, /'recordings\/' \+ userId \+ '\/'/, 'keys are scoped to their owner'); assert.match(store, /WHERE id = \$1 AND user_id = \$2 AND expires_at > NOW\(\)/, 'ownership and expiry are in the query');