diff --git a/.env.githubci b/.env.githubci index 8630ae61c..eab8fe804 100644 --- a/.env.githubci +++ b/.env.githubci @@ -66,7 +66,7 @@ REDIS_PORT=6379 OTEL_SERVICE_NAME=COOP_TEST_SERVICE -NODE_ENV=CI +NODE_ENV=test ITEM_QUEUE_TRAFFIC_PERCENTAGE='0' UI_URL=http://localhost:3000 diff --git a/CHANGELOG.md b/CHANGELOG.md index bc13d7c8b..3966e937e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,7 +18,10 @@ For more information about each release including git tags and artifacts, see [R ### Changed -- Scylla is now optional via `ITEM_INVESTIGATION_AND_STRIKES_ENABLED` ([#918](https://github.com/roostorg/coop/pull/918) by [@sunilatlas](https://github.com/sunilatlas)) +- Invalid environment variable values now prevent startup instead of falling back to defaults ([#1235](https://github.com/roostorg/coop/pull/1235) by [@ThisIsMissEm](https://github.com/ThisIsMissEm)) +- Boolean environment variables accept only `1`, `0`, `true` and `false` ([#1235](https://github.com/roostorg/coop/pull/1235) by [@ThisIsMissEm](https://github.com/ThisIsMissEm)) +- `DATABASE_READ_ONLY_HOST` is now optional, falling back to `DATABASE_HOST` ([#1235](https://github.com/roostorg/coop/pull/1235) by [@ThisIsMissEm](https://github.com/ThisIsMissEm)) +- Scylla is now optional via `SCYLLA_ENABLED` ([#918](https://github.com/roostorg/coop/pull/918) by [@sunilatlas](https://github.com/sunilatlas)) - Settings "Other" tab renamed to "Partial Items" and its settings relocated ([#965](https://github.com/roostorg/coop/pull/965) by [@golden-fox07](https://github.com/golden-fox07)) - Queue deletion is refused while routing rules still reference the queue ([#808](https://github.com/roostorg/coop/pull/808) by [@reitblatt](https://github.com/reitblatt)) - Long text fields in the review console collapse behind a "Read more" control ([#903](https://github.com/roostorg/coop/pull/903) by [@taobojlen](https://github.com/taobojlen)) @@ -26,12 +29,15 @@ For more information about each release including git tags and artifacts, see [R ### Removed +- `postgresql` as a `WAREHOUSE_ADAPTER` / `ANALYTICS_ADAPTER` value, until it is implemented ([#1235](https://github.com/roostorg/coop/pull/1235) by [@ThisIsMissEm](https://github.com/ThisIsMissEm)) - Unused `GRAPHQL_OPAQUE_SCALAR_SECRET` and `LAUNCHDARKLY_SECRET` environment variables ([#1246](https://github.com/roostorg/coop/pull/1246) by [@ThisIsMissEm](https://github.com/ThisIsMissEm)) - Google Cloud Translation API, the `ENGLISH_TRANSLATION` derived field, and `GOOGLE_TRANSLATE_API_KEY` ([#1045](https://github.com/roostorg/coop/pull/1045) by [@julietshen](https://github.com/julietshen)) - `IMAGE_SIMILARITY_SCORE` and `IMAGE_EXACT_MATCH` signal types ([#1043](https://github.com/roostorg/coop/pull/1043) by [@julietshen](https://github.com/julietshen), closes [#686](https://github.com/roostorg/coop/issues/686)) ### Fixed +- Doubled slashes in generated links when `UI_URL` ends with a slash ([#1235](https://github.com/roostorg/coop/pull/1235) by [@ThisIsMissEm](https://github.com/ThisIsMissEm)) +- Scylla TLS retargeting every connection to the certificate hostname instead of the cluster node ([#1235](https://github.com/roostorg/coop/pull/1235) by [@ThisIsMissEm](https://github.com/ThisIsMissEm)) - Rule history dropping other rules' versions when filtered by start date ([#1056](https://github.com/roostorg/coop/pull/1056) by [@juanmrad](https://github.com/juanmrad)) - `RetryFailedNcmecDecisionsJob` ignoring `NCMEC_ENV` and retrying test decisions ([#928](https://github.com/roostorg/coop/pull/928) by [@taobojlen](https://github.com/taobojlen)) - Queue creation failing with "name already exists" on the default reviewer selection ([#1069](https://github.com/roostorg/coop/pull/1069) by [@jess-upscrolled](https://github.com/jess-upscrolled), closes [#1074](https://github.com/roostorg/coop/issues/1074)) diff --git a/docs/development/architecture.md b/docs/development/architecture.md index 7eaa3d987..d8cc6dc63 100644 --- a/docs/development/architecture.md +++ b/docs/development/architecture.md @@ -364,7 +364,7 @@ Session configuration: - Store: PostgreSQL-backed - Cookie: Secure flag in production, 30-day expiry -- Session secret: process.env.SESSION_SECRET +- Session secret: SESSION_SECRET environment variable Files: `/server/api.ts` diff --git a/server/.env.example b/server/.env.example index 15ab39bfd..8f3c5d932 100644 --- a/server/.env.example +++ b/server/.env.example @@ -76,17 +76,23 @@ CLICKHOUSE_PROTOCOL=http HMA_SERVICE_URL=http://localhost:9876 # Scylla Cluster Details -# Set to "false" to run Coop without a Scylla cluster. This disables the two -# Scylla-backed features — Item Investigation (item/user history views) and -# User Strikes (repeat-offender strike counts) — which then no-op: reads return -# empty (strike counts read as 0) and writes are dropped. When "false" (or -# "0"/"no"), the SCYLLA_* connection settings below are not required. -# Defaults to enabled. -ITEM_INVESTIGATION_AND_STRIKES_ENABLED=true +# Set to "false" (or "0") to run Coop without a Scylla cluster. This disables +# the two Scylla-backed features — Item Investigation (item/user history views) +# and User Strikes (repeat-offender strike counts) — which then no-op: reads +# return empty (strike counts read as 0) and writes are dropped. The SCYLLA_* +# connection settings below are then not required; otherwise they are, and the +# process refuses to start without them. Defaults to enabled. +SCYLLA_ENABLED=true SCYLLA_USERNAME=cassandra SCYLLA_PASSWORD=cassandra SCYLLA_HOSTS='127.0.0.1:9042' SCYLLA_LOCAL_DATACENTER='datacenter1' +SCYLLA_PORT=9042 +# TLS. SCYLLA_SSL_SERVERNAME sets the SNI value for certificate verification, +# for when the contact points don't match the server certificate (e.g. an AWS +# Keyspaces regional endpoint); it defaults to the first contact point. +SCYLLA_SSL=false +SCYLLA_SSL_SERVERNAME= # Local dev NODE_ENV=development @@ -99,6 +105,8 @@ REDIS_HOST=localhost REDIS_PORT=6379 REDIS_USER= REDIS_PASSWORD= +# Single-node connections only; cluster connections are always TLS. +REDIS_TLS=false # Signal API Keys/URLs OPEN_AI_API_KEY= diff --git a/server/.eslintrc.cjs b/server/.eslintrc.cjs index 15ec558b3..1bd28d1ba 100644 --- a/server/.eslintrc.cjs +++ b/server/.eslintrc.cjs @@ -677,20 +677,14 @@ module.exports = { { files: ['test/**/*.ts', 'e2e/**/*.ts', './**/*.{spec,test}.ts'], rules: { - // Match prior test-only mutation policy: allow `this`, class internals, - // and `process.env.*`; production code is not in this override. + // Match prior test-only mutation policy: allow `this` and class + // internals; production code is not in this override. 'functional/immutable-data': [ 'error', { ignoreImmediateMutation: true, ignoreClasses: true, - ignoreAccessorPattern: [ - 'this', - 'this.*', - 'this.*.*', - // Tests toggle env vars and clean up with `delete process.env.*`. - 'process.env.*', - ], + ignoreAccessorPattern: ['this', 'this.*', 'this.*.*'], }, ], 'no-console': 'off', diff --git a/server/api.ts b/server/api.ts index a832ab875..e67d801fe 100644 --- a/server/api.ts +++ b/server/api.ts @@ -17,6 +17,10 @@ import { ATTR_EXCEPTION_STACKTRACE, ATTR_EXCEPTION_TYPE, } from '@opentelemetry/semantic-conventions'; +import appConfig from '#config/app'; +import graphqlConfig from '#config/graphql'; +import securityConfig from '#config/security'; +import sessionConfig from '#config/session'; import connectPgSimple from 'connect-pg-simple'; import cors from 'cors'; import express, { type ErrorRequestHandler, type Request } from 'express'; @@ -34,7 +38,6 @@ import { buildPassportContext } from './graphql/utils/passportContext.js'; import { resolveSamlUser } from './graphql/utils/resolveSamlUser.js'; import { safeDepthLimit } from './graphql/utils/safeDepthLimit.js'; import { type Dependencies } from './iocContainer/index.js'; -import { safeGetEnvInt } from './iocContainer/utils.js'; import controllers from './routes/index.js'; import { createBodySchemaValidator } from './utils/bodySchemaValidation.js'; import { jsonStringify } from './utils/encoding.js'; @@ -86,8 +89,6 @@ async function getCPUUsage() { return 1 - (endIdle - startIdle) / (endTotal - startTotal); } -// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -const env = process.env.NODE_ENV || 'development'; const sessionStore = connectPgSimple(session); export default async function makeApiServer(deps: Dependencies) { @@ -97,25 +98,7 @@ export default async function makeApiServer(deps: Dependencies) { app.use(cors()); - app.use( - helmet( - env === 'production' - ? {} - : { - contentSecurityPolicy: { - directives: { - defaultSrc: ["'self'"], - scriptSrc: ["'self'", "'unsafe-inline'", "'unsafe-eval'"], - styleSrc: ["'self'", "'unsafe-inline'"], - imgSrc: ["'self'", 'data:', 'blob:', 'https:', 'http:'], - connectSrc: ["'self'", 'ws:', 'wss:', 'https:', 'http:'], - fontSrc: ["'self'", 'data:', 'https:'], - frameSrc: ["'self'"], - }, - }, - }, - ), - ); + app.use(helmet(securityConfig.helmet)); app.use(express.json({ limit: '50mb' })); app.get('/ready', async (_req, res) => { @@ -132,15 +115,9 @@ export default async function makeApiServer(deps: Dependencies) { const sessionStoreInstance = new sessionStore({ pool: KyselyPgPool }); app.use( session({ - secret: process.env.SESSION_SECRET!, + secret: sessionConfig.secret, store: sessionStoreInstance, - cookie: { - secure: process.env.NODE_ENV === 'production', - httpOnly: true, - sameSite: 'lax', - // 30 Days in milliseconds - maxAge: 30 * 24 * 60 * 60 * 1000, - }, + cookie: sessionConfig.cookie, resave: false, saveUninitialized: false, proxy: true, @@ -194,11 +171,8 @@ export default async function makeApiServer(deps: Dependencies) { done(null, { entryPoint: samlSettings.sso_url as string, idpCert: samlSettings.cert as string, - // I could use UI_URL here but technically the API could be hosted - // on a different domain in the future so hopefully this is more - // robust, not that it will likely matter. - callbackUrl: `${deps.ConfigService.uiUrl}/api/v1/saml/login/${orgId}/callback`, - issuer: deps.ConfigService.uiUrl, + callbackUrl: deps.ConfigService.samlCallbackUrl(orgId), + issuer: deps.ConfigService.samlIssuer, }); }, }, @@ -220,7 +194,7 @@ export default async function makeApiServer(deps: Dependencies) { failureFlash: true, }), (_req, res) => { - res.redirect(`${deps.ConfigService.uiUrl}/dashboard`); + res.redirect(deps.ConfigService.dashboardUrl); }, ); @@ -267,12 +241,12 @@ export default async function makeApiServer(deps: Dependencies) { }, }), plugins: [ - ...(process.env.NODE_ENV === 'production' + ...(appConfig.inProduction ? [ApolloServerPluginLandingPageDisabled()] : []), ], - validationRules: [safeDepthLimit(safeGetEnvInt('GRAPHQL_MAX_DEPTH', 10))], - introspection: process.env.NODE_ENV !== 'production', + validationRules: [safeDepthLimit(graphqlConfig.maxDepth)], + introspection: !appConfig.inProduction, formatError(formattedError, error) { // unwrapResolverError removes the GraphQLError wrapper added by graphql-js // when a non-GraphQL error is thrown from a resolver. diff --git a/server/bin/get-invite-token.ts b/server/bin/get-invite-token.ts index 95f1bb391..e4f0fe4ae 100644 --- a/server/bin/get-invite-token.ts +++ b/server/bin/get-invite-token.ts @@ -6,6 +6,7 @@ * Usage: * npm run get-invite -- --email "user@example.com" */ +import appConfig from '#config/app'; import yargs from 'yargs'; import { hideBin } from 'yargs/helpers'; @@ -42,8 +43,7 @@ async function getInviteToken() { } const invite = result[0]; - const uiUrl = process.env.UI_URL ?? 'http://localhost:3000'; - const signupUrl = `${uiUrl}/signup/${invite.token}`; + const signupUrl = `${appConfig.uiUrl}/signup/${invite.token}`; console.log('\n✅ Invite Token Found!\n'); console.log('═'.repeat(60)); diff --git a/server/bin/www.ts b/server/bin/www.ts index 9612a4727..ba2d667f4 100755 --- a/server/bin/www.ts +++ b/server/bin/www.ts @@ -1,6 +1,7 @@ #!/usr/bin/env node import http from 'http'; import { promisify } from 'util'; +import env from '#start/env'; import _ from 'lodash'; import getBottle from '../iocContainer/index.js'; @@ -35,7 +36,7 @@ try { process.exit(1); } -const port = parsePort(process.env.PORT) ?? 8080; +const port = env.get('PORT', 8080); app.set('port', port); const server = http @@ -145,10 +146,3 @@ process.on('unhandledRejection', (reason) => { process.once('SIGTERM', shutdownOnce); process.once('SIGINT', shutdownOnce); - -function parsePort(val: string | undefined): number | undefined { - if (!val) return undefined; - - const parsed = parseInt(val, 10); - return isNaN(parsed) ? undefined : parsed; -} diff --git a/server/config/app.ts b/server/config/app.ts new file mode 100644 index 000000000..ac6b4c8a7 --- /dev/null +++ b/server/config/app.ts @@ -0,0 +1,40 @@ +import env from '#start/env'; + +/** + * Derived on access rather than at import. + * + * `env.get` reads the validated values `Env.create` produced, and `env.set` + * updates them — so a getter lets a test say `env.set('NODE_ENV', 'production')` + * and have the rest of the application agree. A constant would instead hold + * whatever the environment was when this module was first imported. + * + * `NODE_ENV === 'prod'` is silently non-production everywhere it appears; + * `inProduction` is not, which is why the comparison lives here and not at each + * call site. + */ +export default { + get env() { + return env.get('NODE_ENV', 'development'); + }, + get inProduction() { + return this.env === 'production'; + }, + get inDev() { + return this.env === 'development'; + }, + get inTest() { + return this.env === 'test'; + }, + + // Public origin of the frontend. Used to build the links and redirects the + // application hands out, so it must be the origin a browser reaches, not an + // internal one. + get uiUrl() { + return env.get('UI_URL'); + }, + + /** Identifies this process in traces and as the Postgres `application_name`. */ + get serviceName() { + return env.get('OTEL_SERVICE_NAME', 'coop-service'); + }, +}; diff --git a/server/config/dataWarehouse.ts b/server/config/dataWarehouse.ts new file mode 100644 index 000000000..a578562cd --- /dev/null +++ b/server/config/dataWarehouse.ts @@ -0,0 +1,112 @@ +import env from '#start/env'; + +import { type ClickhouseInsertRetrySettings } from '../plugins/analytics/adapters/clickhouseRetry.js'; +import { type ClickhouseMemorySettings } from '../plugins/warehouse/utils/clickhouseSettings.js'; +import { type DataWarehouseConfig } from '../storage/dataWarehouse/DataWarehouseFactory.js'; + +/** + * `max_bytes_before_external_*` above this spills to disk instead of failing + * the query. Zero disables the threshold entirely. + */ +const DEFAULT_MAX_BYTES_BEFORE_EXTERNAL = 1_500_000_000; + +const memory: ClickhouseMemorySettings = { + max_bytes_before_external_group_by: String( + env.get( + 'CLICKHOUSE_MAX_BYTES_BEFORE_EXTERNAL_GROUP_BY', + DEFAULT_MAX_BYTES_BEFORE_EXTERNAL, + ), + ), + max_bytes_before_external_sort: String( + env.get( + 'CLICKHOUSE_MAX_BYTES_BEFORE_EXTERNAL_SORT', + DEFAULT_MAX_BYTES_BEFORE_EXTERNAL, + ), + ), + max_threads: env.get('CLICKHOUSE_MAX_THREADS', 2), + max_block_size: String(env.get('CLICKHOUSE_MAX_BLOCK_SIZE', 32768)), +}; + +/** + * ClickHouse over HTTP can reset in-flight connections (remote restart, an + * idle-socket reaper in between), which is transient and worth a retry. + */ +const insertRetry: ClickhouseInsertRetrySettings = { + maxRetries: env.get('CLICKHOUSE_INSERT_MAX_RETRIES', 2), + initialTimeMsBetweenRetries: env.get( + 'CLICKHOUSE_INSERT_RETRY_INITIAL_MS', + 100, + ), + maxTimeMsBetweenRetries: env.get('CLICKHOUSE_INSERT_RETRY_MAX_MS', 1000), +}; + +/** + * Which warehouse backs analytics and reporting. + * + * `DATA_WAREHOUSE_PROVIDER` is the superseded spelling of `WAREHOUSE_ADAPTER` + * and is still honoured. `ANALYTICS_ADAPTER` defaults to whatever the warehouse + * is, so a deployment only sets it to split the two. + */ +const provider = env.get( + 'WAREHOUSE_ADAPTER', + env.get('DATA_WAREHOUSE_PROVIDER', 'clickhouse'), +); + +const analyticsProvider = env.get('ANALYTICS_ADAPTER', provider); + +/** + * Every warehouse Coop can talk to, keyed by the value `WAREHOUSE_ADAPTER` + * takes, with `connection` naming the one in use — the shape `@adonisjs/lucid` + * and `@adonisjs/redis` both use for "several possible backends, one selected". + * + * `satisfies Record` is what makes this exhaustive: the map + * must cover exactly the values the schema permits, so widening the enum is a + * type error here until the connection exists. A `switch` could only have caught + * that at runtime. + * + * `postgresql` is absent deliberately. The factory still carries its branches, + * but they are unimplemented — a no-op warehouse and a dialect that throws — so + * it is not offered as an adapter value until that work lands. + */ +const connections = { + noop: { provider: 'noop' }, + + clickhouse: { + provider: 'clickhouse', + connection: { + host: env.get('CLICKHOUSE_HOST', 'localhost'), + port: env.get('CLICKHOUSE_PORT', 8123), + username: env.get('CLICKHOUSE_USERNAME', 'default'), + password: env.get('CLICKHOUSE_PASSWORD')?.release() ?? '', + database: env.get('CLICKHOUSE_DATABASE', 'default'), + protocol: env.get('CLICKHOUSE_PROTOCOL', 'http'), + }, + pool: { max: env.get('CLICKHOUSE_POOL_SIZE', 10) }, + memory, + insertRetry, + }, +} satisfies Record; + +export default { + connections, + + /** Queries and transactions: the `DataWarehouse` and `DataWarehouseDialect` services. */ + warehouse: { + connection: provider, + + /** + * Bounds the window Rule Insights scans, so a memory-constrained instance + * reads far less than a full year. The client filters further and defaults + * to a one-week view. + */ + ruleInsightsLookbackDays: env.get( + 'CLICKHOUSE_RULE_INSIGHTS_LOOKBACK_DAYS', + 90, + ), + }, + + /** Bulk writes, CDC and logging: the `DataWarehouseAnalytics` service. */ + analytics: { + connection: analyticsProvider, + }, +}; diff --git a/server/config/database.ts b/server/config/database.ts new file mode 100644 index 000000000..619b42ce5 --- /dev/null +++ b/server/config/database.ts @@ -0,0 +1,154 @@ +import appConfig from '#config/app'; +import env from '#start/env'; +import { LOG_LEVELS, type LogLevel } from 'kysely'; +import type { ClientConfig, PoolConfig } from 'pg'; + +/** + * Postgres connection and pool settings. + * + * Defaults live here rather than at the call site, so the value a variable + * takes when unset is visible next to the variable itself. + * + * `statementTimeoutMs` and `keepAliveInitialDelayMs` are deliberately left as + * `number | undefined`: the pg options they map to must be *absent* rather than + * zero when unconfigured, so Postgres and pg apply their own defaults. + */ +const config = { + host: env.get('DATABASE_HOST'), + readOnlyHost: env.get('DATABASE_READ_ONLY_HOST'), + port: env.get('DATABASE_PORT', 5432), + name: env.get('DATABASE_NAME', 'development'), + user: env.get('DATABASE_USER', 'postgres'), + password: env.get('DATABASE_PASSWORD'), + ssl: env.get('DATABASE_SSL', false), + + /** Logs every executed query, with SQL, bound params and duration. */ + printLogs: env.get('DATABASE_PRINT_LOGS', false), + + pool: { + max: env.get('DATABASE_POOL_MAX', 30), + readMax: env.get('DATABASE_READ_POOL_MAX', 150), + + /** pg's own default is 10s, which churns connections during quiet periods. */ + idleTimeoutMs: env.get('DATABASE_POOL_IDLE_TIMEOUT_MS', 300_000), + + /** pg's own default is 0 (wait forever). Fail fast if the db is unreachable. */ + connectionTimeoutMs: env.get('DATABASE_POOL_CONNECTION_TIMEOUT_MS', 15_000), + + /** Client-side bound on long-running queries. */ + queryTimeoutMs: env.get('DATABASE_QUERY_TIMEOUT_MS', 1_000_000), + + /** + * Server-side bound, defence in depth alongside `queryTimeoutMs`. Unset + * leaves Postgres' own default, which is no limit. + */ + statementTimeoutMs: env.get('DATABASE_STATEMENT_TIMEOUT_MS'), + + /** Kills sessions sitting idle inside an open transaction, holding locks. */ + idleInTransactionTimeoutMs: env.get( + 'DATABASE_IDLE_IN_TRANSACTION_TIMEOUT_MS', + 300_000, + ), + + /** Recycles each client after N seconds to dodge stale connections. */ + maxLifetimeSeconds: env.get('DATABASE_POOL_MAX_LIFETIME_SECONDS', 0), + + /** + * TCP keepalive surfaces NAT and load-balancer connection drops as pool + * errors rather than hung queries. On unless explicitly disabled. + */ + keepAlive: env.get('DATABASE_KEEPALIVE', true), + + /** Unset leaves pg's own initial delay. */ + keepAliveInitialDelayMs: env.get('DATABASE_KEEPALIVE_INITIAL_DELAY_MS'), + }, +}; + +/** + * Where to connect and as whom, with no pool tuning: what a single `pg.Client` + * should be built from. + * + * `connections.primary` is not a substitute. `PoolConfig extends ClientConfig`, + * so it type-checks, but pg forwards `statement_timeout`, `query_timeout` and + * `idle_in_transaction_session_timeout` to a plain client as well — and the + * last of those would terminate a connection that is deliberately held open + * inside a transaction, which is exactly what the test harness does. + */ +const connectionParams: ClientConfig = { + user: config.user, + database: config.name, + password: config.password.release(), + port: config.port, + host: config.host, + ssl: config.ssl ? { rejectUnauthorized: false } : undefined, +}; + +/** + * Pool tuning shared by both pools, in pg's own option names. + * + * `statement_timeout` and `keepAliveInitialDelayMillis` are omitted rather than + * zeroed when unconfigured, so Postgres and pg apply their own defaults. + */ +const poolTuning: PoolConfig = { + idleTimeoutMillis: config.pool.idleTimeoutMs, + connectionTimeoutMillis: config.pool.connectionTimeoutMs, + query_timeout: config.pool.queryTimeoutMs, + ...(config.pool.statementTimeoutMs !== undefined && { + statement_timeout: config.pool.statementTimeoutMs, + }), + idle_in_transaction_session_timeout: config.pool.idleInTransactionTimeoutMs, + maxLifetimeSeconds: config.pool.maxLifetimeSeconds, + // Connection drops surface as pool errors, handled by `createPgPool`. + keepAlive: config.pool.keepAlive, + ...(config.pool.keepAliveInitialDelayMs !== undefined && { + keepAliveInitialDelayMillis: config.pool.keepAliveInitialDelayMs, + }), +}; + +/** Everything needed to build the primary pool. */ +const primary: PoolConfig = { + ...connectionParams, + max: config.pool.max, + application_name: appConfig.serviceName, + ...poolTuning, +}; + +/** + * The primary pool's settings, redirected at the read replica. + * + * Falls back to the primary host when no replica is configured, so a + * single-database deployment works without repeating the host. It keeps its + * own pool size either way, since read and write traffic are sized + * differently. + */ +const readReplica: PoolConfig = { + ...primary, + max: config.pool.readMax, + host: config.readOnlyHost ?? config.host, +}; + +/** + * Kysely's own default is `['error']`; `DATABASE_PRINT_LOGS` opts in to + * `LOG_LEVELS`, every level Kysely defines, which also logs each executed + * query with its SQL, bound params and duration. + * + * Tests log nothing by default. Kysely logs a query error even when the caller + * catches and handles it, so a suite that deliberately exercises a failure + * path - a unique violation surfacing as a friendly "name already exists", for + * instance - fills the output with errors that are not failures, and are easily + * mistaken for them. `DATABASE_PRINT_LOGS` still overrides this when debugging. + */ +function resolveLogLevels(): ReadonlyArray { + if (config.printLogs) { + return LOG_LEVELS; + } + + return appConfig.inTest ? [] : ['error']; +} + +export default { + ...config, + connectionParams, + connections: { primary, readReplica }, + logLevels: resolveLogLevels(), +}; diff --git a/server/config/debug.ts b/server/config/debug.ts new file mode 100644 index 000000000..be5178d7d --- /dev/null +++ b/server/config/debug.ts @@ -0,0 +1,25 @@ +import env from '#start/env'; + +/** + * Switches that trade safety for local diagnosability. Each one is off by + * default and should stay off anywhere shared. + */ +export default { + /** + * Include implementation detail — stack traces, internal messages — in error + * responses, rather than the sanitised message. + */ + get exposeUnsafeErrorDetails() { + return env.get('EXPOSE_SENSITIVE_IMPLEMENTATION_DETAILS_IN_ERRORS', false); + }, + + /** + * Permit loopback addresses in user-supplied URLs. Off by default because + * user-supplied URLs reach outbound requests (e.g. webhook callbacks), so + * allowing them widens SSRF reach to anything the server can see on + * localhost. + */ + get allowUserInputLocalhostUris() { + return env.get('ALLOW_USER_INPUT_LOCALHOST_URIS', false); + }, +}; diff --git a/server/config/email.ts b/server/config/email.ts new file mode 100644 index 000000000..51a32c41c --- /dev/null +++ b/server/config/email.ts @@ -0,0 +1,35 @@ +import env from '#start/env'; + +/** + * Outbound email. + * + * `transport` is only half a selector today: `makeSendEmail` falls through from + * an explicitly-injected SES client, to `console`, to SendGrid if an API key + * happens to be set, to SES. Making the choice explicit — and replacing the + * `console` transport with SMTP against a local mail catcher — is left to the + * change that adds an SMTP transport, so this module only moves the reads. + */ +export default { + /** Derived on access so `env.set('EMAIL_TRANSPORT', …)` is respected. */ + get transport() { + return env.get('EMAIL_TRANSPORT'); + }, + + /** Only consulted when no SES client is injected and `transport` is unset. */ + get sendgridApiKey() { + return env.get('SENDGRID_API_KEY'); + }, + + /** The addresses Coop sends as. */ + addresses: { + get noReply() { + return env.get('NOREPLY_EMAIL', 'noreply@example.com'); + }, + get support() { + return env.get('SUPPORT_EMAIL', 'support@example.com'); + }, + get team() { + return env.get('TEAM_EMAIL', 'team@example.com'); + }, + }, +}; diff --git a/server/config/featureFlags.ts b/server/config/featureFlags.ts new file mode 100644 index 000000000..c47d6476f --- /dev/null +++ b/server/config/featureFlags.ts @@ -0,0 +1,10 @@ +import env from '#start/env'; + +export default { + /** + * The fraction of item submissions routed through the async processing queue + * (BullMQ) rather than handled inline after returning 202. `1` sends all + * traffic through the queue, `0` none of it. + */ + itemQueueTrafficPercentage: env.get('ITEM_QUEUE_TRAFFIC_PERCENTAGE'), +}; diff --git a/server/config/graphql.ts b/server/config/graphql.ts new file mode 100644 index 000000000..7fdb467b0 --- /dev/null +++ b/server/config/graphql.ts @@ -0,0 +1,9 @@ +import env from '#start/env'; + +export default { + /** + * Rejects queries nested deeper than this, so a hostile or accidental deeply + * recursive query can't be turned into a denial of service. + */ + maxDepth: env.get('GRAPHQL_MAX_DEPTH', 10), +}; diff --git a/server/config/integrations.ts b/server/config/integrations.ts new file mode 100644 index 000000000..362b65d8b --- /dev/null +++ b/server/config/integrations.ts @@ -0,0 +1,20 @@ +import env from '#start/env'; + +/** + * Third-party services Coop talks out to, and where the adopter's integrations + * manifest lives. + */ +export default { + /** + * Where to find the adopter's integrations config file. Relative paths are + * resolved against the working directory; when unset, the loader falls back to + * `integrations.config.json` in the working directory and then in `server/`, + * since `npm run start` runs from the repo root. + */ + configPath: env.get('INTEGRATIONS_CONFIG_PATH'), + + googlePlacesApiKey: env.get('GOOGLE_PLACES_API_KEY'), + + /** Hasher-Matcher-Actioner, for perceptual hash matching. */ + hmaServiceUrl: env.get('HMA_SERVICE_URL', 'http://localhost:9876/'), +}; diff --git a/server/config/ncmec.ts b/server/config/ncmec.ts new file mode 100644 index 000000000..de131c49b --- /dev/null +++ b/server/config/ncmec.ts @@ -0,0 +1,32 @@ +import appConfig from '#config/app'; +import env from '#start/env'; + +/** + * NCMEC CyberTipline reporting. + * + * `isTest` selects the endpoint submissions are routed to. Anything other than + * `NCMEC_ENV=production` — including being unset — sends to + * https://exttest.cybertip.org, the NCMEC sandbox, where reports are discarded. + * Operators are responsible for matching this to whether the credentials + * configured in Settings → NCMEC are production or test credentials issued by + * NCMEC. + * + * Derived in one place because getting it wrong in either direction is + * serious: live reports sent to the sandbox are silently discarded, and test + * reports sent to production are filed as real CyberTipline reports. + */ +export default { + /** Derived on access so `env.set('NCMEC_ENV', …)` is respected. */ + get isTest() { + return env.get('NCMEC_ENV') !== 'production'; + }, + + /** + * Opt-in debug logs and XML/JSON dumps for submissions. Also gated on not + * being in production, since the dumps contain reportable content and must + * not be written in a shared environment. Never includes credentials. + */ + get debug() { + return env.get('NCMEC_DEBUG', false) && !appConfig.inProduction; + }, +}; diff --git a/server/config/redis.ts b/server/config/redis.ts new file mode 100644 index 000000000..19eba383c --- /dev/null +++ b/server/config/redis.ts @@ -0,0 +1,84 @@ +import env from '#start/env'; +import type { + ClusterNode, + ClusterOptions, + DNSLookupFunction, + RedisOptions, +} from 'ioredis'; + +/** + * Redis connections, shaped after `@adonisjs/redis`' `defineConfig`: a map of + * named connections, each either plain ioredis options or a cluster config. + * The two are told apart by the presence of `clusters`, so consumers branch on + * the shape they are handed rather than re-reading `REDIS_USE_CLUSTER`. + */ +type ClusterConnection = { + clusters: ClusterNode[]; + clusterOptions: ClusterOptions; +}; + +export type RedisConnection = RedisOptions | ClusterConnection; + +const host = env.get('REDIS_HOST'); +const port = env.get('REDIS_PORT', 6379); +const password = env.get('REDIS_PASSWORD'); +const user = env.get('REDIS_USER'); + +/** + * AUTH-enabled Redis (e.g. ElastiCache with an auth token) rejects every + * command with NOAUTH unless credentials are sent, which leaves ioredis stuck + * before "ready" and parks commands in the offline queue forever. Local dev + * Redis has no password, so only pass credentials when REDIS_PASSWORD is set; + * REDIS_USER may be set-but-empty, which means the default user. + */ +const auth: RedisOptions = password + ? { + ...(user ? { username: user } : {}), + password: password.release(), + } + : {}; + +/** + * See + * https://github.com/luin/ioredis/blob/c275e9a337a4aee1565e96fe631d28a29ecb4efa/README.md#special-note-aws-elasticache-clusters-with-tls + */ +const dnsLookup: DNSLookupFunction = (address, callback) => + callback(null, address); + +function connection(extra: RedisOptions = {}): RedisConnection { + const options: RedisOptions = { + // Required by BullMQ: its workers use blocking Redis commands that would + // otherwise be misinterpreted as timed-out requests. + maxRetriesPerRequest: null, + ...auth, + ...extra, + }; + + return env.get('REDIS_USE_CLUSTER') + ? { + clusters: [{ host, port }], + clusterOptions: { + dnsLookup, + // Cluster connections are always TLS, regardless of REDIS_TLS. + redisOptions: { tls: {}, ...options }, + }, + } + : { + host, + port, + ...options, + ...(env.get('REDIS_TLS', false) && { tls: { servername: host } }), + }; +} + +export default { + connections: { + main: connection(), + /** + * With `enableOfflineQueue: false`, a `queue.addBulk` while Redis is + * unreachable rejects immediately instead of resolving against the + * in-process buffer, failing the enqueue early with "couldn't enqueue". + */ + enqueueNoBuffer: connection({ enableOfflineQueue: false }), + }, +}; diff --git a/server/config/scylla.ts b/server/config/scylla.ts new file mode 100644 index 000000000..3411bd2eb --- /dev/null +++ b/server/config/scylla.ts @@ -0,0 +1,92 @@ +import env from '#start/env'; +import { types as scyllaTypes, type ClientOptions } from 'cassandra-driver'; + +const enabled = env.get('SCYLLA_ENABLED', true); + +const contactPoints = env.get('SCYLLA_HOSTS'); +const username = env.get('SCYLLA_USERNAME'); +const password = env.get('SCYLLA_PASSWORD'); + +const firstContactPoint = contactPoints?.[0]; + +/** + * A contact point may carry an explicit `:port`, which is not part of the + * hostname a certificate is issued for. Only a single colon is a port + * separator: a bare IPv6 address has several, and no port to strip. + */ +const firstHostname = + firstContactPoint !== undefined && firstContactPoint.split(':').length === 2 + ? firstContactPoint.split(':')[0] + : firstContactPoint; + +// For TLS hostname verification we need an SNI value that matches the server +// cert. Prefer an explicit `SCYLLA_SSL_SERVERNAME` (e.g. the Keyspaces regional +// endpoint) over inferring one from `SCYLLA_HOSTS`, which may contain multiple +// contact points with different cert names. +const sslServerName = env.get('SCYLLA_SSL_SERVERNAME') ?? firstHostname; + +/** + * Scylla settings. + * + * `SCYLLA_ENABLED=false` swaps in a no-op client that drops writes and returns + * empty reads, disabling Item Investigation and User Strikes. The connection + * settings are then not needed, which is why they are optional here; when it is + * on, `start/env.ts` requires them. + * + * N.B. Currently all our services that use Scylla as a backing datastore use the + * same keyspace. If we ever need to add one (e.g. because we have tables that + * need a new replication strategy, or we support multiple datacenters) we will + * want one client per keyspace, since the driver is keyspace aware and switching + * with `USE KEYSPACE` all the time is annoying and likely error prone. + */ +export default { + enabled, + + /** + * `null` when disabled, so a consumer cannot accidentally build a client + * against settings that were never required. + */ + connection: !enabled + ? null + : ({ + contactPoints: contactPoints ? [...contactPoints] : undefined, + localDataCenter: env.get('SCYLLA_LOCAL_DATACENTER'), + keyspace: 'item_investigation_service', + // Omitted rather than half-filled when unset, so the driver falls back to + // connecting without authentication. + ...(username !== undefined && + password !== undefined && { + credentials: { username, password: password.release() }, + }), + protocolOptions: { + port: env.get('SCYLLA_PORT', 9042), + }, + // `servername` rather than `host`: the driver passes these straight to + // `tls.connect(port, address, sslOptions)`, where an options `host` + // overrides the positional address and so retargets the connection + // itself. `servername` sets the SNI value and the certificate identity + // check without moving where the driver connects. + sslOptions: env.get('SCYLLA_SSL', false) + ? { + servername: sslServerName, + rejectUnauthorized: true, + } + : undefined, + pooling: { + coreConnectionsPerHost: { + [scyllaTypes.distance.local]: 3, + [scyllaTypes.distance.remote]: 1, + }, + }, + queryOptions: { + // Quorum consistency requires a simple majority of nodes in a replica + // group to respond to read/write requests. Local Quorum is the same + // except it only expects nodes in the local datacenter to respond. For + // our current Scylla infrastructure quorum and local quorum will have + // identical behavior, but if we ever add another datacenter to the + // cluster using Quorum and requiring responses from multiple DCs would + // degrade performance significantly. + consistency: scyllaTypes.consistencies.localQuorum, + }, + } satisfies ClientOptions), +}; diff --git a/server/config/security.ts b/server/config/security.ts new file mode 100644 index 000000000..ec4aec1c8 --- /dev/null +++ b/server/config/security.ts @@ -0,0 +1,23 @@ +import app from '#config/app'; + +export default { + // Production keeps helmet's own defaults, which include a strict + // Content-Security-Policy. Development relaxes it for Vite's dev server, + // which needs inline and eval'd scripts plus a websocket for HMR — so these + // directives must never be the production branch. + helmet: app.inProduction + ? {} + : { + contentSecurityPolicy: { + directives: { + defaultSrc: ["'self'"], + scriptSrc: ["'self'", "'unsafe-inline'", "'unsafe-eval'"], + styleSrc: ["'self'", "'unsafe-inline'"], + imgSrc: ["'self'", 'data:', 'blob:', 'https:', 'http:'], + connectSrc: ["'self'", 'ws:', 'wss:', 'https:', 'http:'], + fontSrc: ["'self'", 'data:', 'https:'], + frameSrc: ["'self'"], + }, + }, + }, +}; diff --git a/server/config/session.ts b/server/config/session.ts new file mode 100644 index 000000000..64d4f70c4 --- /dev/null +++ b/server/config/session.ts @@ -0,0 +1,19 @@ +import appConfig from '#config/app'; +import env from '#start/env'; +import type { CookieOptions } from 'express-session'; + +/** + * The express-session cookie. + */ +export default { + secret: env.get('SESSION_SECRET'), + + cookie: { + /** HTTPS-only outside development, where there is no TLS terminator. */ + secure: appConfig.inProduction, + httpOnly: true, + sameSite: 'lax', + /** 30 days, in milliseconds. */ + maxAge: 30 * 24 * 60 * 60 * 1000, + } satisfies CookieOptions, +}; diff --git a/server/decs.d.ts b/server/decs.d.ts index 0e1e402ec..3c329a900 100644 --- a/server/decs.d.ts +++ b/server/decs.d.ts @@ -227,40 +227,3 @@ declare module '@stdlib/stats-binomial-test' { export = binomialTest; } - -namespace NodeJS { - interface ProcessEnv { - DATABASE_HOST?: string; - DATABASE_READ_ONLY_HOST?: string; - DATABASE_PORT?: string; - DATABASE_NAME?: string; - DATABASE_USER?: string; - DATABASE_PASSWORD?: string; - DATABASE_SSL?: string; - DATABASE_POOL_MAX?: string; - DATABASE_READ_POOL_MAX?: string; - DATABASE_POOL_IDLE_TIMEOUT_MS?: string; - DATABASE_POOL_CONNECTION_TIMEOUT_MS?: string; - DATABASE_QUERY_TIMEOUT_MS?: string; - DATABASE_IDLE_IN_TRANSACTION_TIMEOUT_MS?: string; - DATABASE_PRINT_LOGS?: string; - SESSION_SECRET?: string; - WAREHOUSE_ADAPTER?: string; - ANALYTICS_ADAPTER?: string; - DATA_WAREHOUSE_PROVIDER?: string; - NCMEC_ENV?: string; - NODE_ENV?: string; - EXPOSE_SENSITIVE_IMPLEMENTATION_DETAILS_IN_ERRORS?: string; - ALLOW_USER_INPUT_LOCALHOST_URIS?: string; - REDIS_USE_CLUSTER?: string; - REDIS_HOST?: string; - REDIS_PORT?: string; - REDIS_USER?: string; - REDIS_PASSWORD?: string; - GROQ_SECRET_KEY?: string; - SENDGRID_API_KEY?: string; - GOOGLE_PLACES_API_KEY?: string; - OPEN_AI_API_KEY?: string; - SLACK_APP_BEARER_TOKEN?: string; - } -} diff --git a/server/graphql/datasources/OrgApi.ts b/server/graphql/datasources/OrgApi.ts index 255318a7e..3988978fe 100644 --- a/server/graphql/datasources/OrgApi.ts +++ b/server/graphql/datasources/OrgApi.ts @@ -1,5 +1,4 @@ import crypto from 'node:crypto'; -import { URL } from 'node:url'; import { inject, type Dependencies } from '../../iocContainer/index.js'; import { CoopEmailAddress } from '../../services/sendEmailService/index.js'; @@ -56,14 +55,14 @@ class OrgAPI { orgId, }); - const url = new URL(`${this.config.uiUrl}/signup/${token}`); + const url = this.config.signupUrl(token); const msg = { to: email, from: CoopEmailAddress.NoReply, subject: "You've been invited to join your team on Coop!", html: `Hi, and welcome to Coop! Your admin has invited you to join the ${org.name} Coop team.

- Click on this link to get started! The link expires in 24 hours, so please make sure to sign up soon. + Click on this link to get started! The link expires in 24 hours, so please make sure to sign up soon.

Best,
Coop Support Team`, diff --git a/server/graphql/datasources/RuleApi.ts b/server/graphql/datasources/RuleApi.ts index a80e88b56..6f85ef631 100644 --- a/server/graphql/datasources/RuleApi.ts +++ b/server/graphql/datasources/RuleApi.ts @@ -2,12 +2,12 @@ import { type Exception } from '@opentelemetry/api'; import { makeEnumLike } from '@roostorg/coop-types'; +import warehouseConfig from '#config/dataWarehouse'; import { type Kysely } from 'kysely'; import { type JsonObject } from 'type-fest'; import { uid } from 'uid'; import { inject, type Dependencies } from '../../iocContainer/index.js'; -import { safeGetEnvInt } from '../../iocContainer/utils.js'; import { type ActionCountsInput } from '../../services/actionStatisticsService/index.js'; import { type AggregationClause } from '../../services/aggregationsService/index.js'; import { type ConditionSetWithResultAsLogged } from '../../services/analyticsLoggers/index.js'; @@ -676,11 +676,9 @@ class RuleAPI { // all five at once, and bound the window (env-tunable, default 90 days) so // a memory-constrained instance scans far less than a full year. The client // filters further client-side and defaults to a one-week view. - const lookbackDays = safeGetEnvInt( - 'CLICKHOUSE_RULE_INSIGHTS_LOOKBACK_DAYS', - 90, + const startAt = new Date( + Date.now() - warehouseConfig.warehouse.ruleInsightsLookbackDays * DAY_MS, ); - const startAt = new Date(Date.now() - lookbackDays * DAY_MS); const runSafely = async ( fn: () => Promise, diff --git a/server/graphql/datasources/orgValidation.ts b/server/graphql/datasources/orgValidation.ts index 2b3d6140a..0911b15c8 100644 --- a/server/graphql/datasources/orgValidation.ts +++ b/server/graphql/datasources/orgValidation.ts @@ -1,7 +1,7 @@ import { createRequire } from 'node:module'; import type { IsEmailOptions } from 'validator/lib/isEmail.js'; -import { validateUrl } from '../../utils/url.js'; +import { validateUrl } from '../../utils/urlValidation.js'; // `validator` is CJS with UMD-style types whose `default` doesn't resolve to // a callable under `module: NodeNext`; `createRequire` gives us `module.exports` diff --git a/server/graphql/modules/apiKey.ts b/server/graphql/modules/apiKey.ts index 23d1759b8..286302d7b 100644 --- a/server/graphql/modules/apiKey.ts +++ b/server/graphql/modules/apiKey.ts @@ -82,7 +82,7 @@ const Query: GQLQueryResolvers = { const apiKeyRecord = await context.services.ApiKeyService.getActiveApiKeyForOrg(user.orgId); if (!apiKeyRecord) { - return process.env.NODE_ENV !== 'production' ? '' : ''; + return ''; } // Return a message indicating the key exists but is hidden for security return 'API key exists (hidden for security)'; diff --git a/server/graphql/modules/org.ts b/server/graphql/modules/org.ts index 73af75047..038bcf38d 100644 --- a/server/graphql/modules/org.ts +++ b/server/graphql/modules/org.ts @@ -1,5 +1,6 @@ /* eslint-disable max-lines */ +import appConfig from '#config/app'; import { GraphQLError } from 'graphql'; import { type JsonObject, type JsonValue } from 'type-fest'; @@ -10,7 +11,7 @@ import { import { filterDecisionsToFailedSubmissions } from '../../services/ncmecService/index.js'; import { UserPermission } from '../../services/userManagementService/index.js'; import { __throw } from '../../utils/misc.js'; -import { isValidUrl } from '../../utils/url.js'; +import { isValidUrl } from '../../utils/urlValidation.js'; import { type GQLIntegrationConfig, type GQLMatchingBanksResolvers, @@ -423,9 +424,9 @@ const Org: GQLOrgResolvers = { // API Keys are required in prod, but no reason to throw outside prod (like // on engineers' local machines) if (!apiKey) { - return process.env.NODE_ENV !== 'production' - ? '' - : __throw(new GraphQLError('API Key not found')); + return appConfig.inProduction + ? __throw(new GraphQLError('API Key not found')) + : ''; } return apiKey.key; diff --git a/server/iocContainer/index.ts b/server/iocContainer/index.ts index 4ce8a390a..bacef3fc2 100644 --- a/server/iocContainer/index.ts +++ b/server/iocContainer/index.ts @@ -1,12 +1,12 @@ /* eslint-disable max-lines */ -import { createRequire } from 'module'; import Bottle from '@ethanresnick/bottlejs'; import opentelemetry from '@opentelemetry/api'; import { type ItemIdentifier } from '@roostorg/coop-types'; -import { - types as scyllaTypes, - type Host as ScyllaHost, -} from 'cassandra-driver'; +import databaseConfig from '#config/database'; +import warehouseConfig from '#config/dataWarehouse'; +import ncmecConfig from '#config/ncmec'; +import redisConfig, { type RedisConnection } from '#config/redis'; +import scyllaConfig from '#config/scylla'; import IORedis, { type Cluster } from 'ioredis'; import { Kysely, PostgresDialect } from 'kysely'; import _ from 'lodash'; @@ -58,10 +58,9 @@ import { import makeRuleEvaluator, { type RuleEvaluator, } from '../rule_engine/RuleEvaluator.js'; -import { Scylla } from '../scylla/index.js'; -import NoOpScylla, { - itemInvestigationAndStrikesEnabled, -} from '../scylla/noOpScylla.js'; +import type { Scylla } from '../scylla/index.js'; +import NoOpScylla from '../scylla/noOpScylla.js'; +import ScyllaDatabase from '../scylla/scyllaDatabase.js'; import { makeActionStatisticsService, type ActionStatisticsService, @@ -101,6 +100,7 @@ import { type ApiKeyService, } from '../services/apiKeyService/index.js'; import { type CombinedPg } from '../services/combinedDbTypes.js'; +import { ConfigService } from '../services/configService/index.js'; import { makeDerivedFieldsService, type DerivedFieldsService, @@ -239,7 +239,7 @@ import { } from '../utils/correlationIds.js'; import { getUsableCoreCount } from '../utils/cpu-helpers.js'; import { jsonStringify, type JsonOf } from '../utils/encoding.js'; -import { logErrorJson, logJson } from '../utils/logging.js'; +import { logJson } from '../utils/logging.js'; import { __throw, assertUnreachable } from '../utils/misc.js'; import SafeTracer from '../utils/SafeTracer.js'; import { @@ -251,17 +251,8 @@ import { import { createPgPool } from './createPgPool.js'; import { registerGqlDataSources } from './services/gqlDataSources.js'; import { registerWorkersAndJobs } from './services/workersAndJobs.js'; -import { - isEnvTrue, - register, - safeGetEnvNonNegativeInt, - safeGetEnvVar, -} from './utils.js'; - -// the otel instrumentation currently intercepts require statements. support for -// esm support is experimental so we should wait until it is stable -const require = createRequire(import.meta.url); -const { Client: ScyllaClient } = require('cassandra-driver'); +import { register } from './utils.js'; + export type { DataSources } from './services/gqlDataSources.js'; export type ItemSubmissionMessageKey = { @@ -327,10 +318,7 @@ export interface Dependencies { // that each dependent service can type its arg more specifically with the set // of tables it is responsible for / allowed to query. // eslint-disable-next-line @typescript-eslint/no-explicit-any - Scylla: Scylla & { - connect: () => Promise; - close: () => Promise; - }; + Scylla: Scylla; // Data Warehouse abstraction DataWarehouse: IDataWarehouse; @@ -441,7 +429,7 @@ export interface Dependencies { Tracer: SafeTracer; Meter: CoopMeter; KeyValueStore: StringNumberKeyValueStore; - ConfigService: { uiUrl: string }; + ConfigService: ConfigService; } // Takes a class and returns a type that just contains its public methods and @@ -450,17 +438,6 @@ export interface Dependencies { // treatment that TS gives to classes with private fields; see https://stackoverflow.com/questions/55281162/can-i-force-the-typescript-compiler-to-use-nominal-typing) export type PublicInterface = { [K in keyof T]: T[K] }; -export function getPgConnectionParams(): pg.ClientConfig { - return { - user: process.env.DATABASE_USER ?? 'postgres', - database: process.env.DATABASE_NAME ?? 'development', - password: safeGetEnvVar('DATABASE_PASSWORD'), - port: parseInt(process.env.DATABASE_PORT ?? '5432'), - host: safeGetEnvVar('DATABASE_HOST'), - ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, - }; -} - /** * A function for creating our service container, configured for production. * Services can be rebound in other contexts (namely, tests) as needed. @@ -472,87 +449,6 @@ export default async function getBottle( manualReviewContentResolver?: ManualReviewContentResolver; } = {}, ) { - // Pool / client tuning shared by both Kysely pools. Defaults preserve our - // pre-Kysely behavior; env var names are generic. - const getPgPoolTuning = () => { - const statementTimeoutMs = - process.env.DATABASE_STATEMENT_TIMEOUT_MS?.trim(); - const keepAliveInitialDelayMs = - process.env.DATABASE_KEEPALIVE_INITIAL_DELAY_MS?.trim(); - return { - // pg's default is 10s, which churns connections during quiet periods. - idleTimeoutMillis: safeGetEnvNonNegativeInt( - 'DATABASE_POOL_IDLE_TIMEOUT_MS', - 300000, - ), - // pg's default is 0 (wait forever); fail fast if the db is unreachable. - connectionTimeoutMillis: safeGetEnvNonNegativeInt( - 'DATABASE_POOL_CONNECTION_TIMEOUT_MS', - 15000, - ), - // Client-side bound on long-running queries. - query_timeout: safeGetEnvNonNegativeInt( - 'DATABASE_QUERY_TIMEOUT_MS', - 1000000, - ), - // Optional server-side bound; defense in depth alongside `query_timeout`. - // Unset => Postgres' own default (no limit). - ...(statementTimeoutMs && { - statement_timeout: safeGetEnvNonNegativeInt( - 'DATABASE_STATEMENT_TIMEOUT_MS', - 0, - ), - }), - // Kill sessions sitting idle inside an open transaction (holding locks). - idle_in_transaction_session_timeout: safeGetEnvNonNegativeInt( - 'DATABASE_IDLE_IN_TRANSACTION_TIMEOUT_MS', - 300000, - ), - // Recycle each client after N seconds to dodge stale connections. - // 0 = never expire (default). - maxLifetimeSeconds: safeGetEnvNonNegativeInt( - 'DATABASE_POOL_MAX_LIFETIME_SECONDS', - 0, - ), - // TCP keepalive surfaces NAT/LB connection drops as pool errors - // (handled by `createPgPool`) rather than as hung queries. Defaults on; - // set DATABASE_KEEPALIVE=false to disable. - keepAlive: - process.env.DATABASE_KEEPALIVE?.trim().toLowerCase() !== 'false', - ...(keepAliveInitialDelayMs && { - keepAliveInitialDelayMillis: safeGetEnvNonNegativeInt( - 'DATABASE_KEEPALIVE_INITIAL_DELAY_MS', - 0, - ), - }), - }; - }; - - // NB: this is a function because safeGetEnvVar can throw, so we only want to - // try to look up the env vars (and throw if they're missing) _if someone - // actually tries to fetch a service from bottle that needs these env vars_. - // Not every worker/job needs every service or is given every var in its env. - // - // NB: while we can reasonably provide default values for some of the env vars - // below, we wouldn't want to provide default values for all of them, as then - // that would defeat the ability of safeGetEnvVar to alert us in prod if a - // worker that needs these vars is run without them. - const getPgMasterConnectionInfo = () => ({ - ...getPgConnectionParams(), - max: parseInt(process.env.DATABASE_POOL_MAX ?? '30'), - application_name: - getEnvVarOrWarn('OTEL_SERVICE_NAME') ?? 'unknown-coop-service', - ...getPgPoolTuning(), - }); - - // Kysely's default is `['error']`; opt-in to also logging every executed - // query (SQL, bound params, duration). - const kyselyLogLevels: ReadonlyArray<'query' | 'error'> = isEnvTrue( - 'DATABASE_PRINT_LOGS', - ) - ? ['query', 'error'] - : ['error']; - const bottle = new Bottle(); // Pg services. @@ -567,7 +463,7 @@ export default async function getBottle( // - KyselyPgReadReplica gives us the same type safety, but sends queries to our // replicas, for when we only need reads and we're ok w/ eventual consistency. bottle.factory('KyselyPgPool', () => - createPgPool(getPgMasterConnectionInfo()), + createPgPool(databaseConfig.connections.primary), ); bottle.factory( @@ -579,7 +475,7 @@ export default async function getBottle( pool: container.KyselyPgPool, cursor: Cursor, }), - log: kyselyLogLevels, + log: databaseConfig.logLevels, }), ); @@ -589,77 +485,21 @@ export default async function getBottle( new Kysely({ dialect: new PostgresDialect({ controlClient: pg.Client, - pool: createPgPool({ - ...getPgMasterConnectionInfo(), - max: parseInt(process.env.DATABASE_READ_POOL_MAX ?? '150'), - host: safeGetEnvVar('DATABASE_READ_ONLY_HOST'), - }), + pool: createPgPool(databaseConfig.connections.readReplica), cursor: Cursor, }), - log: kyselyLogLevels, + log: databaseConfig.logLevels, }), ); - // AUTH-enabled Redis (e.g. ElastiCache with an auth token) rejects every - // command with NOAUTH unless credentials are sent, which leaves ioredis stuck - // before "ready" and parks commands in the offline queue forever. Local dev - // Redis has no password, so only pass credentials when REDIS_PASSWORD is set; - // REDIS_USER may be set-but-empty, which means the default user. Shared by the - // cluster and single-node paths so both authenticate identically. - const redisAuthOptions = (): { username?: string; password?: string } => - process.env.REDIS_PASSWORD - ? { - ...(process.env.REDIS_USER - ? { username: process.env.REDIS_USER } - : {}), - password: process.env.REDIS_PASSWORD, - } - : {}; - - const makeRedis = ( - extraOptions: { enableOfflineQueue?: boolean } = {}, - ): IORedis.Redis | Cluster => - safeGetEnvVar('REDIS_USE_CLUSTER') === 'true' - ? new IORedis.Cluster( - [ - { - host: safeGetEnvVar('REDIS_HOST'), - port: parseInt(process.env.REDIS_PORT ?? '6379'), - }, - ], - { - // See - // https://github.com/luin/ioredis/blob/c275e9a337a4aee1565e96fe631d28a29ecb4efa/README.md#special-note-aws-elasticache-clusters-with-tls - dnsLookup: (address, callback) => callback(null, address), - redisOptions: { - tls: {}, - // Required by BullMQ: its workers use blocking Redis commands - // that would otherwise be misinterpreted as timed-out requests. - maxRetriesPerRequest: null, - ...redisAuthOptions(), - ...extraOptions, - }, - }, - ) - : new IORedis.default({ - // Required by BullMQ: its workers use blocking Redis commands - // that would otherwise be misinterpreted as timed-out requests. - maxRetriesPerRequest: null, - port: parseInt(process.env.REDIS_PORT ?? '6379'), - host: safeGetEnvVar('REDIS_HOST'), - ...redisAuthOptions(), - ...(isEnvTrue('REDIS_TLS') - ? { tls: { servername: safeGetEnvVar('REDIS_HOST') } } - : {}), - ...extraOptions, - }); + const makeRedis = (connection: RedisConnection): IORedis.Redis | Cluster => + 'clusters' in connection + ? new IORedis.Cluster(connection.clusters, connection.clusterOptions) + : new IORedis.default(connection); - bottle.factory('IORedis', () => makeRedis()); - // With `enableOfflineQueue: false`, a `queue.addBulk` while Redis is - // unreachable rejects immediately instead of resolving against the - // in-process buffer. fails enqueue early with "couldn't enqueue". + bottle.factory('IORedis', () => makeRedis(redisConfig.connections.main)); bottle.factory('IORedisEnqueueNoBuffer', () => - makeRedis({ enableOfflineQueue: false }), + makeRedis(redisConfig.connections.enqueueNoBuffer), ); // Data Warehouse abstraction layer @@ -670,25 +510,33 @@ export default async function getBottle( // - 'DataWarehouse' - Core queries and transactions // - 'DataWarehouseDialect' - Type-safe Kysely queries // - 'DataWarehouseAnalytics' - Bulk writes, CDC, logging + // The config names which connection each of these uses; turning a name into a + // connection is wiring, so it belongs here rather than in the config. + function getWarehouseConfig() { + return warehouseConfig.connections[warehouseConfig.warehouse.connection]; + } + + function getAnalyticsConfig() { + return warehouseConfig.connections[warehouseConfig.analytics.connection]; + } + bottle.factory('DataWarehouse', () => { - const config = DataWarehouseFactory.createConfigFromEnv(); - const dataWarehouse = DataWarehouseFactory.createDataWarehouse(config); + const dataWarehouse = + DataWarehouseFactory.createDataWarehouse(getWarehouseConfig()); dataWarehouse.start(); return dataWarehouse; }); - bottle.factory('DataWarehouseDialect', () => { - const config = DataWarehouseFactory.createConfigFromEnv(); - return DataWarehouseFactory.createKyselyDialect(config); - }); + bottle.factory('DataWarehouseDialect', () => + DataWarehouseFactory.createKyselyDialect(getWarehouseConfig()), + ); - bottle.factory('DataWarehouseAnalytics', (container) => { - const config = DataWarehouseFactory.createConfigFromEnv(); - return DataWarehouseFactory.createAnalyticsAdapter( - config, + bottle.factory('DataWarehouseAnalytics', (container) => + DataWarehouseFactory.createAnalyticsAdapter( + getAnalyticsConfig(), container.DataWarehouseDialect, - ); - }); + ), + ); bottle.factory('ActionStatisticsAdapter', (container) => { return new ClickhouseActionStatisticsAdapter( @@ -786,9 +634,7 @@ export default async function getBottle( executionContext, ); }, - itemInvestigationAndStrikesEnabled( - process.env.ITEM_INVESTIGATION_AND_STRIKES_ENABLED, - ), + scyllaConfig.enabled, ), ); @@ -802,124 +648,16 @@ export default async function getBottle( bottle.factory('Scylla', () => { // Scylla backs the item-investigation and user-strike features. Operators // who don't need those (and don't want to run a Scylla cluster) can set - // `ITEM_INVESTIGATION_AND_STRIKES_ENABLED=false` to swap in a no-op that - // drops writes and returns empty reads, so no `SCYLLA_*` connection env - // vars are required. Defaults to enabled to preserve existing behaviour. - if ( - !itemInvestigationAndStrikesEnabled( - process.env.ITEM_INVESTIGATION_AND_STRIKES_ENABLED, - ) - ) { + // `SCYLLA_ENABLED=false` to swap in a no-op that drops writes and returns + // empty reads, so no `SCYLLA_*` connection env vars are required. Defaults + // to enabled to preserve existing behaviour. + if (scyllaConfig.connection === null) { // eslint-disable-next-line no-restricted-syntax - logJson( - 'scylla.disabled ITEM_INVESTIGATION_AND_STRIKES_ENABLED=false; using no-op Scylla', - ); + logJson('scylla.disabled SCYLLA_ENABLED=false; using no-op Scylla'); return new NoOpScylla(); } - const contactPoints = safeGetEnvVar('SCYLLA_HOSTS') - .split(',') - .map((it) => it.trim()) - .filter((it) => it.length > 0); - // For TLS hostname verification we need an SNI value that matches the - // server cert. Prefer an explicit `SCYLLA_SSL_SERVERNAME` (e.g., the - // Keyspaces regional endpoint) over inferring one from `SCYLLA_HOSTS`, - // which may contain multiple contact points with different cert names. - const sslServerName = process.env.SCYLLA_SSL_SERVERNAME ?? contactPoints[0]; - const scyllaDriver = new ScyllaClient({ - contactPoints, - credentials: { - username: safeGetEnvVar('SCYLLA_USERNAME'), - password: safeGetEnvVar('SCYLLA_PASSWORD'), - }, - localDataCenter: safeGetEnvVar('SCYLLA_LOCAL_DATACENTER'), - keyspace: 'item_investigation_service', - protocolOptions: { - port: parseInt(process.env.SCYLLA_PORT ?? '9042'), - }, - sslOptions: isEnvTrue('SCYLLA_SSL') - ? { - host: sslServerName, - rejectUnauthorized: true, - } - : undefined, - pooling: { - coreConnectionsPerHost: { - [scyllaTypes.distance.local]: 3, - [scyllaTypes.distance.remote]: 1, - }, - }, - queryOptions: { - // Quorum consistency requires a simple majority of nodes in a - // replica group to respond to read/write requests. Local Quorum is - // the same except it only expects nodes in the local datacenter to - // respond. For our current Scylla infrastructure quorum and local - // quorum will have identical behavior, but if we ever add another - // datacenter to the cluster using Quorum and requiring responses - // from multiple DCs would degrade performance significantly - consistency: scyllaTypes.consistencies.localQuorum, - }, - }); - - // Surface cluster state changes so reconnect storms are visible in logs. - scyllaDriver.on('hostUp', (host: ScyllaHost) => { - // eslint-disable-next-line no-restricted-syntax - logJson(`scylla.hostUp address=${host.address}`); - }); - scyllaDriver.on('hostDown', (host: ScyllaHost) => { - // eslint-disable-next-line no-restricted-syntax - logJson(`scylla.hostDown address=${host.address}`); - }); - // Forward driver-internal warnings/errors (auth, TLS, connection drops, - // etc.); skip the very chatty `info`/`verbose` levels. - scyllaDriver.on( - 'log', - ( - level: 'verbose' | 'info' | 'warning' | 'error', - source: string, - message: string, - furtherInfo?: unknown, - ) => { - if (level !== 'warning' && level !== 'error') { - return; - } - const wrapped = new Error(`scylla.${level}: [${source}] ${message}`); - if (furtherInfo instanceof Error) { - wrapped.stack = furtherInfo.stack ?? wrapped.stack; - } - // eslint-disable-next-line no-restricted-syntax - logErrorJson({ - message: `scylla.driver.${level}`, - error: wrapped, - }); - }, - ); - - // cassandra-driver leaks ~4 HostMap listeners per failed `Client._connect()` - // retry and never recreates the HostMap, so the default cap of 10 trips - // after ~3 failures. Raise it so transient blips don't spam the warning, - // but keep it bounded so a true runaway is still noticeable. - const controlConnection = ( - scyllaDriver as unknown as { - controlConnection?: { - hosts?: { setMaxListeners?: (n: number) => void }; - }; - } - ).controlConnection; - controlConnection?.hosts?.setMaxListeners?.(15); - - class ClosableScylla< - DB extends Record>, - > extends Scylla { - /** Eagerly connect; idempotent once `connected` is true. */ - async connect() { - return scyllaDriver.connect(); - } - async close() { - return scyllaDriver.shutdown(); - } - } - return new ClosableScylla(scyllaDriver); + return new ScyllaDatabase(scyllaConfig.connection); }); bottle.factory('ItemInvestigationService', (container) => { @@ -1301,16 +1039,9 @@ export default async function getBottle( getItemTypeEventuallyConsistent: container.getItemTypeEventuallyConsistent, }); - // Submissions go to the NCMEC test endpoint - // (exttest.cybertip.org) unless the deployment is explicitly - // configured for production via NCMEC_ENV=production. Operators - // are responsible for matching this to whether the credentials - // configured in Settings → NCMEC are production or test - // credentials issued by NCMEC. - const isTest = process.env.NCMEC_ENV !== 'production'; await container.NcmecService.submitReport( reportParams, - isTest, + ncmecConfig.isTest, ); const actionAndPolicy = await container.NcmecService.getNCMECActionsToRunAndPolicies( @@ -1325,7 +1056,7 @@ export default async function getBottle( actionAndPolicy != null && actionAndPolicy.actionsToRunIds != null && isNonEmptyArray(decisionActions) && - !isTest + !ncmecConfig.isTest ) { await publishActions({ decisionActions, @@ -1666,7 +1397,9 @@ export default async function getBottle( 'SigningKeyPairStorageService', (container) => new PostgresSigningKeyPairStorage(container.KyselyPg), ); - bottle.value('ConfigService', { uiUrl: safeGetEnvVar('UI_URL') }); + // A factory rather than a value so the instance is built on first use. It is + // still a singleton per container, as `bottle.factory` memoises. + bottle.factory('ConfigService', () => new ConfigService()); bottle.value('S3StoreObjectFactory', s3StoreObjectFactory); bottle.factory('sendEmail', makeSendEmail); register(bottle, 'KeyValueStore', makeKeyValueStore); @@ -1867,35 +1600,3 @@ function serviceHasBeenAccessed( const propDesc = Object.getOwnPropertyDescriptor(container, serviceName); return typeof propDesc?.get !== 'function'; } - -/** - * Gets an env var, or logs a warning if the variable is not defined. This is - * useful for cases where an env var should be provided, but the app can recover - * on the off-chance that the variable was improperly omitted, and we'd rather - * have the fallback behavior than create an outage. However, we still want to - * log a warning so that we can see in DD that we need to set this variable. - * - * TODO: create a DD metric that counts these warnings, and set up a monitor to - * alert if there are any. - */ -function getEnvVarOrWarn(varName: string) { - const value = process.env[varName]; - - if (value == null) { - // NB: using this format for the logged JSON is taking on some tech debt - // (esp if/once we create a DD monitor/metric that uses `title` to find - // these errors), because we probably want to reformat these logged errors - // later in a way that makes them more consistent amongst each other and - // possibly also more consistent with CoopError errors. For now, though, - // figuring out that end state isn't worth the brainpower. - // eslint-disable-next-line no-console - console.warn( - jsonStringify({ - title: 'MissingEnvVar', - message: `Missing env var ${varName}`, - }), - ); - } - - return value; -} diff --git a/server/iocContainer/utils.ts b/server/iocContainer/utils.ts index db5baf205..f1776e452 100644 --- a/server/iocContainer/utils.ts +++ b/server/iocContainer/utils.ts @@ -2,8 +2,6 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import type Bottle from '@ethanresnick/bottlejs'; -import { jsonStringify } from '../utils/encoding.js'; -import { __throw } from '../utils/misc.js'; import { type Dependencies as Deps } from './index.js'; const DEPENDENCIES = Symbol(); @@ -650,64 +648,3 @@ function isConstructable(fn: any): fn is new (...args: any[]) => any { return false; } } - -/** - * Gets an env var, or throws if the variable is undefined. This is critical to - * make the app hard crash early (so we'll get alerts) if some expected config - * var is missing. - */ -export function safeGetEnvVar(varName: string): string { - return ( - process.env[varName] ?? __throw(new Error(`Missing env var ${varName}`)) - ); -} - -/** - * Returns true when the env var is set to a truthy value. Accepts `true`, `1`, - * and `yes` (case-insensitive) so callers don't have to worry about casing or - * common aliases. Any other value (including unset) returns false. - */ -export function isEnvTrue(varName: string): boolean { - const raw = process.env[varName]; - if (raw == null) return false; - return ['true', '1', 'yes'].includes(raw.trim().toLowerCase()); -} - -/** - * Gets an env var and parses it as a positive integer. Returns `defaultValue` - * if the variable is unset or invalid, logging an error on misconfiguration. - */ -export function safeGetEnvInt(varName: string, defaultValue: number): number { - const raw = process.env[varName]; - if (raw === undefined) return defaultValue; - const parsed = parseInt(raw, 10); - if (!Number.isInteger(parsed) || parsed <= 0) { - // eslint-disable-next-line no-console - console.error( - `Invalid env var ${varName}: expected a positive integer, got ${jsonStringify(raw)}. Using default value ${defaultValue}.`, - ); - return defaultValue; - } - return parsed; -} - -/** - * Like `safeGetEnvInt` but allows `0`. Use when zero is a meaningful value - * (e.g. disabling retries, no timeout). - */ -export function safeGetEnvNonNegativeInt( - varName: string, - defaultValue: number, -): number { - const raw = process.env[varName]; - if (raw === undefined) return defaultValue; - const parsed = parseInt(raw, 10); - if (!Number.isInteger(parsed) || parsed < 0) { - // eslint-disable-next-line no-console - console.error( - `Invalid env var ${varName}: expected a non-negative integer, got ${jsonStringify(raw)}. Using default value ${defaultValue}.`, - ); - return defaultValue; - } - return parsed; -} diff --git a/server/jest.config.cjs b/server/jest.config.cjs index 89b766db9..70ad01275 100644 --- a/server/jest.config.cjs +++ b/server/jest.config.cjs @@ -59,7 +59,10 @@ module.exports = { moduleNameMapper: {}, // An array of regexp pattern strings, matched against all module paths before considered 'visible' to the module loader - // modulePathIgnorePatterns: [], + // `build/` holds the compiled output, including a copy of `package.json` so + // the `#`-prefixed subpath imports resolve when running it. Without this, + // jest's haste map sees two packages named `server` and warns on every run. + modulePathIgnorePatterns: ['/build/'], // Activates notifications for test results // notify: false, diff --git a/server/lib/env/index.ts b/server/lib/env/index.ts new file mode 100644 index 000000000..84b88b650 --- /dev/null +++ b/server/lib/env/index.ts @@ -0,0 +1,44 @@ +import { Env as BaseEnv } from '@adonisjs/env'; + +import { hostList, integer } from './validators.js'; + +/** + * `@adonisjs/env`'s `Env` with our own validators folded into `schema`, so a + * schema literal reads uniformly as `Env.schema.*` regardless of whether a + * given validator ships with Adonis. + * + * Subclassing is what makes that possible. `Env.schema` is declared as a type + * alias over a const rather than an interface, so it cannot be extended by + * declaration merging, and assigning onto the imported object would leave the + * types behind while making validator availability depend on import order. + * Overriding the inherited static sidesteps both. + * + * Note `BaseEnv.create` constructs a `BaseEnv` rather than `new this(...)`, so + * it returns the base class. That is fine here: the subclass exists for the + * schema namespace, not for instance behaviour. + */ +export class Env< + EnvValues extends Record, +> extends BaseEnv { + static override schema = { + ...BaseEnv.schema, + integer, + hostList, + }; +} + +export { hostList, integer } from './validators.js'; + +/** + * Re-exported so `start/env.ts` imports everything it needs from here, keeping + * `@adonisjs/env` behind this module. + */ +export { errors } from '@adonisjs/env'; + +/** + * Re-exported so `config/*` modules can name secret-typed values without + * importing from `@adonisjs/env`'s own dependency tree. A `Secret` redacts + * itself in logs, `JSON.stringify` and string coercion; call `.release()` to + * read the underlying value. + */ +export type { Secret } from '@poppinss/utils'; diff --git a/server/lib/env/validators.test.ts b/server/lib/env/validators.test.ts new file mode 100644 index 000000000..a631aeead --- /dev/null +++ b/server/lib/env/validators.test.ts @@ -0,0 +1,179 @@ +import { hostList, integer } from './validators.js'; + +describe('integer env validator', () => { + describe('integer', () => { + const validate = integer(); + + test('accepts any integer, including zero and negatives', () => { + expect(validate('SOME_VAR', '10')).toBe(10); + expect(validate('SOME_VAR', '0')).toBe(0); + expect(validate('SOME_VAR', '-3')).toBe(-3); + }); + + // `Env.schema.number` accepts this, because `Number('1.5')` is not NaN. + test('rejects floats', () => { + expect(() => validate('SOME_VAR', '1.5')).toThrow(); + }); + + // The retired helpers used `parseInt`, which read this as 12. + test('rejects a numeric prefix followed by junk', () => { + expect(() => validate('SOME_VAR', '12abc')).toThrow(); + }); + + test('rejects a missing or empty value', () => { + expect(() => validate('SOME_VAR', undefined)).toThrow(); + expect(() => validate('SOME_VAR', '')).toThrow(); + }); + + test('names the variable and the offending value', () => { + expect(() => validate('SOME_VAR', '1.5')).toThrow(/SOME_VAR/); + expect(() => validate('SOME_VAR', '1.5')).toThrow(/1\.5/); + }); + }); + + describe('integer.positive', () => { + const validate = integer.positive(); + + test('accepts an integer greater than zero', () => { + expect(validate('SOME_VAR', '7')).toBe(7); + }); + + test('rejects zero and negatives', () => { + expect(() => validate('SOME_VAR', '0')).toThrow(); + expect(() => validate('SOME_VAR', '-5')).toThrow(); + }); + }); + + describe('integer.nonNegative', () => { + const validate = integer.nonNegative(); + + test('accepts zero, since zero is meaningful for retries and timeouts', () => { + expect(validate('SOME_VAR', '0')).toBe(0); + }); + + test('rejects negatives and floats', () => { + expect(() => validate('SOME_VAR', '-1')).toThrow(); + expect(() => validate('SOME_VAR', '0.5')).toThrow(); + }); + }); + + describe('.optional()', () => { + test('returns undefined when unset or empty', () => { + expect( + integer.positive.optional()('SOME_VAR', undefined), + ).toBeUndefined(); + expect(integer.positive.optional()('SOME_VAR', '')).toBeUndefined(); + }); + + test('still validates a value that is present', () => { + expect(integer.positive.optional()('SOME_VAR', '3')).toBe(3); + expect(() => integer.positive.optional()('SOME_VAR', '0')).toThrow(); + }); + + // An empty value is absent, so the consumer's default applies rather than + // the feature being silently disabled by a zero. + test('treats an empty value as absent rather than zero', () => { + expect(integer.nonNegative.optional()('SOME_VAR', '')).toBeUndefined(); + expect(integer.nonNegative.optional()('SOME_VAR', '0')).toBe(0); + }); + }); +}); + +describe('hostList env validator', () => { + const validate = hostList(); + + // The three shipped env files between them use all of these forms. + test('accepts a bare hostname, an IP, and a host with a port', () => { + expect(validate('SCYLLA_HOSTS', 'scylla')).toEqual(['scylla']); + expect(validate('SCYLLA_HOSTS', '127.0.0.1')).toEqual(['127.0.0.1']); + expect(validate('SCYLLA_HOSTS', '127.0.0.1:9042')).toEqual([ + '127.0.0.1:9042', + ]); + }); + + test('splits a comma-separated list and trims each entry', () => { + expect(validate('SCYLLA_HOSTS', 'db1, db2 ,db3:9043')).toEqual([ + 'db1', + 'db2', + 'db3:9043', + ]); + }); + + test('ignores empty entries from stray commas', () => { + expect(validate('SCYLLA_HOSTS', 'db1,,db2,')).toEqual(['db1', 'db2']); + }); + + // This is what the old post-validation block existed to catch. + test('rejects a value that contains no hosts at all', () => { + expect(() => validate('SCYLLA_HOSTS', ',')).toThrow(/SCYLLA_HOSTS/); + expect(() => validate('SCYLLA_HOSTS', ' , , ')).toThrow(); + }); + + test('rejects an unset or empty value', () => { + expect(() => validate('SCYLLA_HOSTS', undefined)).toThrow(); + expect(() => validate('SCYLLA_HOSTS', '')).toThrow(); + }); + + test('rejects an invalid host', () => { + expect(() => validate('SCYLLA_HOSTS', 'not a host')).toThrow(); + expect(() => validate('SCYLLA_HOSTS', 'db1,not a host')).toThrow(); + }); + + test('rejects an out-of-range or non-numeric port', () => { + expect(() => validate('SCYLLA_HOSTS', 'db1:0')).toThrow(); + expect(() => validate('SCYLLA_HOSTS', 'db1:70000')).toThrow(); + expect(() => validate('SCYLLA_HOSTS', 'db1:abc')).toThrow(); + }); + + describe('.optional()', () => { + test('returns undefined when unset, since Scylla itself is optional', () => { + expect(hostList.optional()('SCYLLA_HOSTS', undefined)).toBeUndefined(); + expect(hostList.optional()('SCYLLA_HOSTS', '')).toBeUndefined(); + }); + + test('still validates a value that is present', () => { + expect(hostList.optional()('SCYLLA_HOSTS', 'db1')).toEqual(['db1']); + expect(() => hostList.optional()('SCYLLA_HOSTS', ',')).toThrow(); + }); + }); + + describe('.optionalWhen()', () => { + test('is required when the condition is false', () => { + expect(() => + hostList.optionalWhen(false)('SCYLLA_HOSTS', undefined), + ).toThrow(); + expect(hostList.optionalWhen(false)('SCYLLA_HOSTS', 'db1')).toEqual([ + 'db1', + ]); + }); + + test('is optional when the condition is true', () => { + expect( + hostList.optionalWhen(true)('SCYLLA_HOSTS', undefined), + ).toBeUndefined(); + }); + + test('still validates a value that is present, even when optional', () => { + expect(() => hostList.optionalWhen(true)('SCYLLA_HOSTS', ',')).toThrow(); + }); + + test('evaluates a function condition at validation time, not schema build time', () => { + let disabled = false; + const validate = hostList.optionalWhen(() => disabled); + + expect(() => validate('SCYLLA_HOSTS', undefined)).toThrow(); + + // The condition is re-read on each call, which is what lets it depend on + // a sibling variable that `Env.create` only puts in `process.env` after + // the schema object has been built. + disabled = true; + expect(validate('SCYLLA_HOSTS', undefined)).toBeUndefined(); + }); + + test('passes the key and value through to a function condition', () => { + const condition = jest.fn(() => true); + hostList.optionalWhen(condition)('SCYLLA_HOSTS', 'db1'); + expect(condition).toHaveBeenCalledWith('SCYLLA_HOSTS', 'db1'); + }); + }); +}); diff --git a/server/lib/env/validators.ts b/server/lib/env/validators.ts new file mode 100644 index 000000000..71f1ee168 --- /dev/null +++ b/server/lib/env/validators.ts @@ -0,0 +1,193 @@ +/** + * Integer validators, folded into `Env.schema` by `./index.js`. + * + * An Adonis schema entry is just a `(key, value) => T` function, so these sit + * alongside the built-in `Env.schema.*` validators without any special support. + * + * They exist because `Env.schema.number` casts with `Number()` and rejects only + * `NaN` — it accepts `1.5`, `-5` and `0` quite happily. Pool sizes, timeouts and + * counts need an integer within range, and a value outside it stops the process + * starting rather than being substituted with a default. + */ + +import { Env as BaseEnv } from '@adonisjs/env'; + +type Validator = (key: string, value?: string) => T; + +/** + * Matches `@poppinss/validator-lite`'s own `optionalWhen` condition: truthy + * means the variable is optional. + * + * Prefer the function form. A plain boolean is evaluated when the schema object + * literal is built, which happens before `Env.create` copies `.env` file values + * into `process.env` — so a condition reading a sibling variable would see it + * only when it came from the real environment, not from an env file. + */ +type Condition = boolean | ((key: string, value?: string) => boolean); + +function isOptional( + condition: Condition, + key: string, + value?: string, +): boolean { + return typeof condition === 'function' ? condition(key, value) : condition; +} + +type IntegerValidator = (() => Validator) & { + optional: () => Validator; +}; + +/** `integer()` plus its range-constrained variants. */ +type IntegerValidators = IntegerValidator & { + positive: IntegerValidator; + nonNegative: IntegerValidator; +}; + +/** + * Schema functions signal failure by throwing a plain `Error`. `EnvValidator` + * catches each one, collects its `message`, and reports every invalid variable + * together in a single `E_INVALID_ENV_VARIABLES` whose `help` lists them all. + * + * Constructing that error here instead would collapse the detail, since its own + * `message` is the generic "Validation failed for one or more environment + * variables" — that, rather than the specifics, is what would be listed. + * + * The wording mirrors `@poppinss/validator-lite` so built-in and custom + * failures read identically in that list. + */ +function invalid(key: string, value: string, expectation: string): never { + throw new Error( + `"${key}" env variable must be ${expectation} (Current value: "${value}")`, + ); +} + +function castToInteger( + key: string, + value: string, + minimum: number, + expectation: string, +): number { + const casted = Number(value); + if (!Number.isInteger(casted) || casted < minimum) { + invalid(key, value, expectation); + } + return casted; +} + +/** + * An empty string is treated as absent rather than as a value, matching how + * `@poppinss/validator-lite` handles every other schema type: `FOO=` means + * unset, so a consumer's default applies instead of `FOO` being read as `0`. + */ +function makeIntegerValidator( + minimum: number, + expectation: string, +): IntegerValidator { + const required = (): Validator => (key, value) => { + if (!value) { + invalid(key, '', expectation); + } + return castToInteger(key, value, minimum, expectation); + }; + + const optional = (): Validator => (key, value) => + value ? castToInteger(key, value, minimum, expectation) : undefined; + + return Object.assign(required, { optional }); +} + +/** + * `integer()` accepts any integer; the sub-validators constrain the range. + * + * - `integer.positive()` — 1 or greater. Ports, pool sizes, limits. + * - `integer.nonNegative()` — 0 or greater, where zero is meaningful: a + * disabled timeout, no retries. + * + * Each also has an `.optional()` form, so a variable the application supplies a + * default for reads as `Env.schema.integer.positive.optional()`. + */ +export const integer: IntegerValidators = Object.assign( + makeIntegerValidator(Number.NEGATIVE_INFINITY, 'an integer'), + { + positive: makeIntegerValidator(1, 'an integer greater than zero'), + nonNegative: makeIntegerValidator(0, 'an integer of zero or greater'), + }, +); + +type HostListValidator = (() => Validator) & { + optional: () => Validator; + optionalWhen: ( + condition: Condition, + ) => Validator; +}; + +const HOST_LIST_EXPECTATION = + 'a comma-separated list of hosts, each optionally suffixed with ":port"'; + +/** Reuses Adonis' own host check rather than reimplementing FQDN/IP matching. */ +const validateHost = BaseEnv.schema.string({ format: 'host' }); + +function parseHostList(key: string, value: string): readonly string[] { + const entries = value + .split(',') + .map((entry) => entry.trim()) + .filter((entry) => entry.length > 0); + + if (entries.length === 0) { + invalid(key, value, HOST_LIST_EXPECTATION); + } + + for (const entry of entries) { + // Only treat a single colon as a port separator: a bare IPv6 address + // contains several, and has no port to split off. + const separators = entry.split(':').length - 1; + const [host, port] = + separators === 1 ? entry.split(':') : [entry, undefined]; + + validateHost(key, host); + + if (port !== undefined) { + const casted = Number(port); + if (!Number.isInteger(casted) || casted < 1 || casted > 65535) { + invalid(key, entry, 'a host with a port between 1 and 65535'); + } + } + } + + return entries; +} + +/** + * A comma-separated list of hosts, such as Scylla's contact points. Entries may + * carry an explicit `:port`; those that don't fall back to the driver's own + * port setting. + * + * Returning the parsed list means consumers receive `string[]` rather than + * re-splitting the raw value, and an empty or malformed list is reported + * alongside every other invalid variable instead of throwing separately after + * validation has already finished. + */ +export const hostList: HostListValidator = Object.assign( + (): Validator => (key, value) => { + if (!value) { + invalid(key, '', HOST_LIST_EXPECTATION); + } + return parseHostList(key, value); + }, + { + optional: (): Validator => (key, value) => + value ? parseHostList(key, value) : undefined, + + /** + * Required unless `condition` says otherwise, mirroring the built-in + * `Env.schema.string.optionalWhen`. Used for a variable that only matters + * when the feature it configures is switched on. + */ + optionalWhen: + (condition: Condition): Validator => + (key, value) => + isOptional(condition, key, value) + ? hostList.optional()(key, value) + : hostList()(key, value), + }, +); diff --git a/server/package-lock.json b/server/package-lock.json index 2ec9fb01a..7a8173db7 100644 --- a/server/package-lock.json +++ b/server/package-lock.json @@ -9,6 +9,7 @@ "version": "1.0.0", "license": "Apache-2.0", "dependencies": { + "@adonisjs/env": "^7.1.0", "@apollo/server": "^5.5.0", "@as-integrations/express5": "^1.1.2", "@aws-sdk/client-s3": "^3.1017.0", @@ -23,6 +24,7 @@ "@node-saml/passport-saml": "^5.1.0", "@opentelemetry/api": "^1.8.0", "@opentelemetry/semantic-conventions": "^1.22.0", + "@poppinss/utils": "^7.1.0", "@roostorg/coop-integration-example": "^2.0.0", "@roostorg/coop-types": "^2.4.0", "@sendgrid/mail": "^8.1.6", @@ -121,6 +123,20 @@ "typescript-eslint": "^8.57.2" } }, + "node_modules/@adonisjs/env": { + "version": "7.1.0", + "resolved": "https://registry.npmjs.org/@adonisjs/env/-/env-7.1.0.tgz", + "integrity": "sha512-DXKCDIDoHzjxx9O7HvFYYBGQENmF5ANTUXBGE65ygOcJTtXyrLBISUkjrIQGVAKUmcLPF9WInkXQfxMnfO2VJw==", + "license": "MIT", + "dependencies": { + "@poppinss/utils": "^7.0.0", + "@poppinss/validator-lite": "^2.1.2", + "split-lines": "^3.0.0" + }, + "engines": { + "node": ">=24.0.0" + } + }, "node_modules/@apollo/cache-control-types": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/@apollo/cache-control-types/-/cache-control-types-1.0.3.tgz", @@ -3539,6 +3555,58 @@ "node": ">=20" } }, + "node_modules/@poppinss/exception": { + "version": "1.2.3", + "resolved": "https://registry.npmjs.org/@poppinss/exception/-/exception-1.2.3.tgz", + "integrity": "sha512-dCED+QRChTVatE9ibtoaxc+WkdzOSjYTKi/+uacHWIsfodVfpsueo3+DKpgU5Px8qXjgmXkSvhXvSCz3fnP9lw==", + "license": "MIT" + }, + "node_modules/@poppinss/object-builder": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@poppinss/object-builder/-/object-builder-1.1.0.tgz", + "integrity": "sha512-FOrOq52l7u8goR5yncX14+k+Ewi5djnrt1JwXeS/FvnwAPOiveFhiczCDuvXdssAwamtrV2hp5Rw9v+n2T7hQg==", + "license": "MIT", + "engines": { + "node": ">=20.6.0" + } + }, + "node_modules/@poppinss/string": { + "version": "1.7.2", + "resolved": "https://registry.npmjs.org/@poppinss/string/-/string-1.7.2.tgz", + "integrity": "sha512-A182GLDfi36iDCbhDrHB0xzrPM1fO3GHnhCDIdadf8C6eycgct4m7zusbLwEh6GPaj2Pz5BVos7XK16w7tZ7wQ==", + "license": "MIT", + "dependencies": { + "@types/pluralize": "^0.0.33", + "case-anything": "^3.1.2", + "pluralize": "^8.0.0", + "slugify": "^1.6.9" + } + }, + "node_modules/@poppinss/types": { + "version": "1.2.1", + "resolved": "https://registry.npmjs.org/@poppinss/types/-/types-1.2.1.tgz", + "integrity": "sha512-qUYnzl0m9HJTWsXtr8Xo7CwDx6wcjrvo14bOVbIMIlKJCzKrm3LX55dRTDr1/x4PpSvKVgmxvC6Ly2YiqXKOvQ==", + "license": "MIT" + }, + "node_modules/@poppinss/utils": { + "version": "7.1.0", + "resolved": "https://registry.npmjs.org/@poppinss/utils/-/utils-7.1.0.tgz", + "integrity": "sha512-/0+NFusRdWFACF75gm/PZSw11HSfPbyg7FT9p0RPsDHSqwJq2ZMCcFA60HwOjSWx2WBaGhdvwf78EI1hEOiXhQ==", + "license": "MIT", + "dependencies": { + "@poppinss/exception": "^1.2.3", + "@poppinss/object-builder": "^1.1.0", + "@poppinss/string": "^1.7.2", + "@poppinss/types": "^1.2.1", + "flattie": "^1.1.1" + } + }, + "node_modules/@poppinss/validator-lite": { + "version": "2.1.2", + "resolved": "https://registry.npmjs.org/@poppinss/validator-lite/-/validator-lite-2.1.2.tgz", + "integrity": "sha512-UhSG1ouT6r67VbEFHK/8ax3EMZYHioew9PqGmEZjV41G15aPZi6cyhXtBVvF9xqkHMflA5V680k7bQzV0kfD5w==", + "license": "MIT" + }, "node_modules/@protobufjs/aspromise": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/@protobufjs/aspromise/-/aspromise-1.1.2.tgz", @@ -11515,6 +11583,12 @@ "@types/pg": "*" } }, + "node_modules/@types/pluralize": { + "version": "0.0.33", + "resolved": "https://registry.npmjs.org/@types/pluralize/-/pluralize-0.0.33.tgz", + "integrity": "sha512-JOqsl+ZoCpP4e8TDke9W79FDcSgPAR0l6pixx2JHkhnRjvShyYiAYw2LVsnA7K08Y6DeOnaU6ujmENO4os/cYg==", + "license": "MIT" + }, "node_modules/@types/qs": { "version": "6.14.0", "resolved": "https://registry.npmjs.org/@types/qs/-/qs-6.14.0.tgz", @@ -13512,6 +13586,18 @@ ], "license": "CC-BY-4.0" }, + "node_modules/case-anything": { + "version": "3.1.2", + "resolved": "https://registry.npmjs.org/case-anything/-/case-anything-3.1.2.tgz", + "integrity": "sha512-wljhAjDDIv/hM2FzgJnYQg90AWmZMNtESCjTeLH680qTzdo0nErlCxOmgzgX4ZsZAtIvqHyD87ES8QyriXB+BQ==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/mesqueeb" + } + }, "node_modules/cassandra-driver": { "version": "4.9.0", "resolved": "https://registry.npmjs.org/cassandra-driver/-/cassandra-driver-4.9.0.tgz", @@ -15400,6 +15486,15 @@ "dev": true, "license": "ISC" }, + "node_modules/flattie": { + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/flattie/-/flattie-1.1.1.tgz", + "integrity": "sha512-9UbaD6XdAL97+k/n+N7JwX46K/M6Zc6KcFYskrYL8wbBV/Uyk0CTAMY0VT+qiK5PM7AIc9aTWYtq65U7T+aCNQ==", + "license": "MIT", + "engines": { + "node": ">=8" + } + }, "node_modules/follow-redirects": { "version": "1.16.0", "resolved": "https://registry.npmjs.org/follow-redirects/-/follow-redirects-1.16.0.tgz", @@ -18875,6 +18970,15 @@ "node": ">=20" } }, + "node_modules/pluralize": { + "version": "8.0.0", + "resolved": "https://registry.npmjs.org/pluralize/-/pluralize-8.0.0.tgz", + "integrity": "sha512-Nc3IT5yHzflTfbjgqWcCPpo7DaKy4FnpB0l/zCAW0Tc7jxAiuqSxHasntB3D7887LSrA93kDJ9IXovxJYxyLCA==", + "license": "MIT", + "engines": { + "node": ">=4" + } + }, "node_modules/possible-typed-array-names": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/possible-typed-array-names/-/possible-typed-array-names-1.1.0.tgz", @@ -19780,6 +19884,15 @@ "node": ">=8" } }, + "node_modules/slugify": { + "version": "1.6.9", + "resolved": "https://registry.npmjs.org/slugify/-/slugify-1.6.9.tgz", + "integrity": "sha512-vZ7rfeehZui7wQs438JXBckYLkIIdfHOXsaVEUMyS5fHo1483l1bMdo0EDSWYclY0yZKFOipDy4KHuKs6ssvdg==", + "license": "MIT", + "engines": { + "node": ">=8.0.0" + } + }, "node_modules/smol-toml": { "version": "1.8.0", "resolved": "https://registry.npmjs.org/smol-toml/-/smol-toml-1.8.0.tgz", @@ -19851,6 +19964,18 @@ "node": "*" } }, + "node_modules/split-lines": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/split-lines/-/split-lines-3.0.0.tgz", + "integrity": "sha512-d0TpRBL/VfKDXsk8JxPF7zgF5pCUDdBMSlEL36xBgVeaX448t+yGXcJaikUyzkoKOJ0l6KpMfygzJU9naIuivw==", + "license": "MIT", + "engines": { + "node": ">=12" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/split-on-first": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/split-on-first/-/split-on-first-1.1.0.tgz", diff --git a/server/package.json b/server/package.json index c8aae2025..7b10656e9 100644 --- a/server/package.json +++ b/server/package.json @@ -5,7 +5,10 @@ "type": "module", "scripts": { "build": "tsc", + "postbuild": "mkdir -p build && cp package.json build/package.json", + "prestart": "npm run postbuild", "start": "tsc-watch --onSuccess \"node --trace-warnings --env-file-if-exists=.env ./build/bin/www.js\"", + "prestart:trace": "npm run postbuild", "start:trace": "tsc-watch --onSuccess \"node --trace-warnings --env-file-if-exists=.env --require ../nodejs-instrumentation/build/autoinstrumentation.js ./build/bin/www.js\"", "test": "npm run test:local", "test:local": "NODE_OPTIONS=\"--no-warnings --loader ts-node/esm\" node --env-file-if-exists=.env node_modules/.bin/jest --watch --detectOpenHandles", @@ -26,7 +29,13 @@ }, "author": "Roostorg", "license": "Apache-2.0", + "imports": { + "#config/*": "./config/*.js", + "#lib/env": "./lib/env/index.js", + "#start/*": "./start/*.js" + }, "dependencies": { + "@adonisjs/env": "^7.1.0", "@apollo/server": "^5.5.0", "@as-integrations/express5": "^1.1.2", "@aws-sdk/client-s3": "^3.1017.0", @@ -41,6 +50,7 @@ "@node-saml/passport-saml": "^5.1.0", "@opentelemetry/api": "^1.8.0", "@opentelemetry/semantic-conventions": "^1.22.0", + "@poppinss/utils": "^7.1.0", "@roostorg/coop-integration-example": "^2.0.0", "@roostorg/coop-types": "^2.4.0", "@sendgrid/mail": "^8.1.6", diff --git a/server/plugins/analytics/adapters/ClickhouseAnalyticsAdapter.ts b/server/plugins/analytics/adapters/ClickhouseAnalyticsAdapter.ts index 7cbd8f2f2..b68ca4e0c 100644 --- a/server/plugins/analytics/adapters/ClickhouseAnalyticsAdapter.ts +++ b/server/plugins/analytics/adapters/ClickhouseAnalyticsAdapter.ts @@ -3,7 +3,7 @@ import { createClient, type ClickHouseClient } from '@clickhouse/client'; import { jsonStringify, tryJsonParse } from '../../../utils/encoding.js'; import { logErrorJson } from '../../../utils/logging.js'; import type SafeTracer from '../../../utils/SafeTracer.js'; -import { getClickhouseMemorySettings } from '../../warehouse/utils/clickhouseSettings.js'; +import { type ClickhouseMemorySettings } from '../../warehouse/utils/clickhouseSettings.js'; import { formatClickhouseQuery } from '../../warehouse/utils/clickhouseSql.js'; import type { IAnalyticsAdapter } from '../IAnalyticsAdapter.js'; import { @@ -14,6 +14,7 @@ import { import { isTransientNetworkError, withClickhouseInsertRetries, + type ClickhouseInsertRetrySettings, } from './clickhouseRetry.js'; export interface ClickhouseAnalyticsConnection { @@ -27,6 +28,10 @@ export interface ClickhouseAnalyticsConnection { export interface ClickhouseAnalyticsAdapterOptions { connection: ClickhouseAnalyticsConnection; + /** Per-query memory limits sent with every statement. */ + memory: ClickhouseMemorySettings; + /** How hard to retry a failed insert. */ + retry: ClickhouseInsertRetrySettings; tracer?: SafeTracer; defaultBatchSize?: number; } @@ -104,7 +109,7 @@ export class ClickhouseAnalyticsAdapter implements IAnalyticsAdapter { ...(password ? { password } : {}), database: options.connection.database, clickhouse_settings: { - ...getClickhouseMemorySettings(), + ...options.memory, }, }); @@ -118,6 +123,7 @@ export class ClickhouseAnalyticsAdapter implements IAnalyticsAdapter { }) => { await this.client.insert({ table, values, format: 'JSONEachRow' }); }, + options.retry, ); } diff --git a/server/plugins/analytics/adapters/clickhouseRetry.ts b/server/plugins/analytics/adapters/clickhouseRetry.ts index 0eba7a823..6f3d64159 100644 --- a/server/plugins/analytics/adapters/clickhouseRetry.ts +++ b/server/plugins/analytics/adapters/clickhouseRetry.ts @@ -1,9 +1,16 @@ -import { - safeGetEnvInt, - safeGetEnvNonNegativeInt, -} from '../../../iocContainer/utils.js'; import { withRetries } from '../../../utils/misc.js'; +/** + * How hard to retry a failed insert. Supplied by the caller rather than read + * from the environment, so adapters under `plugins/` stay independent of how a + * given deployment sources its configuration. + */ +export interface ClickhouseInsertRetrySettings { + maxRetries: number; + initialTimeMsBetweenRetries: number; + maxTimeMsBetweenRetries: number; +} + // Network errors we'll retry on. ClickHouse over HTTP can RST in-flight // connections (remote restart, idle-socket reaper between us and CH, etc.); // these are transient and worth one or two retries before giving up. @@ -27,20 +34,10 @@ export function isTransientNetworkError(err: unknown): boolean { export function withClickhouseInsertRetries( fn: (this: void, ...args: Args) => Promise, + retry: ClickhouseInsertRetrySettings, ): (...args: Args) => Promise { return withRetries( - { - maxRetries: safeGetEnvNonNegativeInt('CLICKHOUSE_INSERT_MAX_RETRIES', 2), - initialTimeMsBetweenRetries: safeGetEnvInt( - 'CLICKHOUSE_INSERT_RETRY_INITIAL_MS', - 100, - ), - maxTimeMsBetweenRetries: safeGetEnvInt( - 'CLICKHOUSE_INSERT_RETRY_MAX_MS', - 1000, - ), - isRetryableError: isTransientNetworkError, - }, + { ...retry, isRetryableError: isTransientNetworkError }, fn, ); } diff --git a/server/plugins/warehouse/utils/clickhouseSettings.ts b/server/plugins/warehouse/utils/clickhouseSettings.ts index e27792a71..68adb27aa 100644 --- a/server/plugins/warehouse/utils/clickhouseSettings.ts +++ b/server/plugins/warehouse/utils/clickhouseSettings.ts @@ -1,29 +1,14 @@ -import { safeGetEnvInt } from '../../../iocContainer/utils.js'; - -const DEFAULT_MAX_BYTES_BEFORE_EXTERNAL = 1_500_000_000; - +/** + * Per-query memory limits sent to ClickHouse with each statement. + * + * Values come from `config/dataWarehouse.ts` and are passed in by whoever + * constructs the adapter. Adapters under `plugins/` deliberately read no + * environment themselves, so a community-published adapter never has to know + * how this deployment sources its configuration. + */ export interface ClickhouseMemorySettings { max_bytes_before_external_group_by: string; max_bytes_before_external_sort: string; max_threads: number; max_block_size: string; } - -export function getClickhouseMemorySettings(): ClickhouseMemorySettings { - return { - max_bytes_before_external_group_by: String( - safeGetEnvInt( - 'CLICKHOUSE_MAX_BYTES_BEFORE_EXTERNAL_GROUP_BY', - DEFAULT_MAX_BYTES_BEFORE_EXTERNAL, - ), - ), - max_bytes_before_external_sort: String( - safeGetEnvInt( - 'CLICKHOUSE_MAX_BYTES_BEFORE_EXTERNAL_SORT', - DEFAULT_MAX_BYTES_BEFORE_EXTERNAL, - ), - ), - max_threads: safeGetEnvInt('CLICKHOUSE_MAX_THREADS', 2), - max_block_size: String(safeGetEnvInt('CLICKHOUSE_MAX_BLOCK_SIZE', 32768)), - }; -} diff --git a/server/routes/items/submitItems.ts b/server/routes/items/submitItems.ts index 33ac96dd2..ac2a7c6c3 100644 --- a/server/routes/items/submitItems.ts +++ b/server/routes/items/submitItems.ts @@ -1,11 +1,11 @@ import { type ItemIdentifier } from '@roostorg/coop-types'; +import featureFlags from '#config/featureFlags'; import { v1 as uuidv1 } from 'uuid'; import { type Dependencies, type ItemSubmissionMessageValue, } from '../../iocContainer/index.js'; -import { safeGetEnvVar } from '../../iocContainer/utils.js'; import { getFieldValueForRole, itemSubmissionToItemSubmissionWithTypeIdentifier, @@ -259,10 +259,7 @@ Dependencies): RequestHandlerWithBodies { // Send a configurable percentage of traffic to the async processing queue // (BullMQ), otherwise handle inline (in this process, immediately after // returning 202 to the user). Set to 1 to route all traffic through the queue. - const trafficPercentage = Number( - safeGetEnvVar('ITEM_QUEUE_TRAFFIC_PERCENTAGE'), - ); - if (Math.random() < trafficPercentage) { + if (Math.random() < featureFlags.itemQueueTrafficPercentage) { // toItemSubmission should always set a `submissionTime` property with a // valid Date, but due to legacy data the type returned, ItemSubmission, an // optional `submissionTime` property. this variable is used to convince diff --git a/server/scylla/noOpScylla.test.ts b/server/scylla/noOpScylla.test.ts index bbf1c4049..f423b7f48 100644 --- a/server/scylla/noOpScylla.test.ts +++ b/server/scylla/noOpScylla.test.ts @@ -1,40 +1,15 @@ -import NoOpScylla, { - itemInvestigationAndStrikesEnabled, -} from './noOpScylla.js'; +import NoOpScylla from './noOpScylla.js'; import Scylla from './scylla.js'; /** - * Tests for the Scylla-disabled path used when - * `ITEM_INVESTIGATION_AND_STRIKES_ENABLED=false`. + * Tests for the Scylla-disabled path used when `SCYLLA_ENABLED=false`: the + * behavioural contract of {@link NoOpScylla} (drops writes, empty reads, + * connect/close resolve). * - * Two things are covered: - * 1. The behavioural contract of {@link NoOpScylla} (drops writes, empty reads, - * connect/close resolve). - * 2. The exact flag-parsing predicate (`itemInvestigationAndStrikesEnabled`) - * used by the `Scylla` DI factory in `iocContainer` to decide - * enabled-vs-disabled. Imported directly (not mirrored) so the - * default-enabled (upstream-preserving) behaviour is guarded by a test. + * The flag itself is now parsed by the env schema (`Env.schema.boolean`), so + * there is no bespoke predicate left to test here. */ -describe('ITEM_INVESTIGATION_AND_STRIKES_ENABLED gate predicate', () => { - test('defaults to enabled when unset (preserves upstream behaviour)', () => { - expect(itemInvestigationAndStrikesEnabled(undefined)).toBe(true); - expect(itemInvestigationAndStrikesEnabled('')).toBe(true); - }); - - test('is disabled only for explicit falsey values', () => { - for (const v of ['false', 'FALSE', ' false ', '0', 'no', 'No']) { - expect(itemInvestigationAndStrikesEnabled(v)).toBe(false); - } - }); - - test('stays enabled for truthy / unrelated values', () => { - for (const v of ['true', 'TRUE', '1', 'yes', 'anything']) { - expect(itemInvestigationAndStrikesEnabled(v)).toBe(true); - } - }); -}); - describe('NoOpScylla', () => { // A minimal DB shape for the generic parameter. type TestDB = { widgets: { id: number; name: string } }; diff --git a/server/scylla/noOpScylla.ts b/server/scylla/noOpScylla.ts index 99ab71fa7..1bdb548e3 100644 --- a/server/scylla/noOpScylla.ts +++ b/server/scylla/noOpScylla.ts @@ -1,30 +1,9 @@ import { type CqlSelectOptions, type DBDefinition } from './cqlUtils.js'; import Scylla from './scylla.js'; -/** - * Parses the `ITEM_INVESTIGATION_AND_STRIKES_ENABLED` feature flag from its raw - * string value (i.e. `process.env.ITEM_INVESTIGATION_AND_STRIKES_ENABLED`). - * - * Shared by the `Scylla` DI factory in `iocContainer` (to decide whether to - * return a real Scylla or a {@link NoOpScylla}) and by the unit tests. Defaults - * to enabled when unset/empty so existing deployments are unaffected; only the - * explicit falsey values `false`/`0`/`no` (case/whitespace-insensitive) disable - * the Scylla-backed features. - * - * Kept as a pure function of its argument (it does not read `process.env` - * itself) so callers own where the value comes from and tests stay independent - * of the ambient environment. - */ -export function itemInvestigationAndStrikesEnabled( - raw: string | undefined, -): boolean { - return !['false', '0', 'no'].includes((raw ?? 'true').trim().toLowerCase()); -} - /** * A no-op implementation of {@link Scylla} used when the Scylla-backed features - * (item investigation and user strikes) are disabled via - * `ITEM_INVESTIGATION_AND_STRIKES_ENABLED=false`. + * (item investigation and user strikes) are disabled via `SCYLLA_ENABLED=false`. * * Scylla has no managed offering on some deployment platforms, and some * operators do not need the features that depend on it. Rather than gate the diff --git a/server/scylla/scylla.ts b/server/scylla/scylla.ts index d0ddc0780..a549564ea 100644 --- a/server/scylla/scylla.ts +++ b/server/scylla/scylla.ts @@ -10,10 +10,17 @@ import { const StreamToAsyncIterator = _S2A.default; -export default class Scylla { - constructor(private client: ScyllaClient) { - this.client = client; - } +export default abstract class Scylla { + constructor(protected client: ScyllaClient) {} + + /** + * Open the connection. Implementations make this idempotent, since callers + * (e.g. the item-processing worker) connect eagerly to fail fast. + */ + abstract connect(): Promise; + + /** Close the connection and release whatever it holds. */ + abstract close(): Promise; async insert( opts: { diff --git a/server/scylla/scyllaDatabase.ts b/server/scylla/scyllaDatabase.ts new file mode 100644 index 000000000..16723a574 --- /dev/null +++ b/server/scylla/scyllaDatabase.ts @@ -0,0 +1,78 @@ +import { createRequire } from 'node:module'; +import type * as CassandraDriver from 'cassandra-driver'; +import { type ClientOptions, type Host as ScyllaHost } from 'cassandra-driver'; + +import { logErrorJson, logJson } from '../utils/logging.js'; +import { type DBDefinition } from './cqlUtils.js'; +import Scylla from './scylla.js'; + +// The otel instrumentation currently intercepts require statements. Support for +// esm is experimental, so we should wait until it is stable; until then the +// driver has to be loaded this way or Scylla calls go untraced. +const require = createRequire(import.meta.url); +const { Client } = require('cassandra-driver') as typeof CassandraDriver; + +/** + * Forwards driver-internal warnings and errors (auth, TLS, connection drops, + * etc.); skips the very chatty `info`/`verbose` levels. + */ +const scyllaLogger = ( + level: 'verbose' | 'info' | 'warning' | 'error', + source: string, + message: string, + furtherInfo?: unknown, +) => { + if (level !== 'warning' && level !== 'error') { + return; + } + const wrapped = new Error(`scylla.${level}: [${source}] ${message}`); + if (furtherInfo instanceof Error) { + wrapped.stack = furtherInfo.stack ?? wrapped.stack; + } + // eslint-disable-next-line no-restricted-syntax + logErrorJson({ + message: `scylla.driver.${level}`, + error: wrapped, + }); +}; + +/** + * A {@link Scylla} backed by a real cluster: owns the driver, the lifecycle + * methods the IoC container needs to open and close it, and the operational + * wiring (log forwarding, cluster-state visibility) that every connection + * wants but that the query layer has no opinion about. + * + * `NoOpScylla` is the counterpart used when `SCYLLA_ENABLED=false`. + */ +export default class ScyllaDatabase extends Scylla { + constructor(options: ClientOptions) { + super(new Client(options)); + + // Surface cluster state changes so reconnect storms are visible in logs. + this.client.on('hostUp', (host: ScyllaHost) => { + // eslint-disable-next-line no-restricted-syntax + logJson(`scylla.hostUp address=${host.address}`); + }); + this.client.on('hostDown', (host: ScyllaHost) => { + // eslint-disable-next-line no-restricted-syntax + logJson(`scylla.hostDown address=${host.address}`); + }); + + this.client.on('log', scyllaLogger); + + // cassandra-driver leaks ~4 HostMap listeners per failed `Client._connect()` + // retry and never recreates the HostMap, so the default cap of 10 trips + // after ~3 failures. Raise it so transient blips don't spam the warning, but + // keep it bounded so a true runaway is still noticeable. + this.client.hosts.setMaxListeners(15); + } + + /** Eagerly connect; idempotent once `connected` is true. */ + async connect() { + return this.client.connect(); + } + + async close() { + return this.client.shutdown(); + } +} diff --git a/server/services/configService/configService.ts b/server/services/configService/configService.ts new file mode 100644 index 000000000..b8ef32054 --- /dev/null +++ b/server/services/configService/configService.ts @@ -0,0 +1,63 @@ +import appConfig from '#config/app'; + +/** + * Owns the externally-facing URLs the application hands out — SAML callbacks, + * post-login redirects, password resets — so their paths are defined once + * rather than interpolated at each call site. + * + * The origin defaults to `config/app`; the constructor parameter exists so a + * test can pin it without going through the environment. + */ +export default class ConfigService { + readonly #uiUrl: string; + + constructor(uiUrl: string = appConfig.uiUrl) { + // `UI_URL` may or may not carry a trailing slash; normalising once here + // avoids every consumer producing `//dashboard` when it does. + this.#uiUrl = uiUrl.replace(/\/+$/, ''); + } + + /** Public origin of the frontend, without a trailing slash. */ + get uiUrl(): string { + return this.#uiUrl; + } + + /** + * SAML issuer. Deliberately the UI origin rather than the API's own, since + * that is the identifier registered with each org's identity provider. + */ + get samlIssuer(): string { + return this.#uiUrl; + } + + /** Where a user lands after a successful login. */ + get dashboardUrl(): string { + return `${this.#uiUrl}/dashboard`; + } + + /** + * Where an org's identity provider posts its SAML assertion back to. Note + * this is on the UI origin: the API could be hosted on a different domain, + * and the frontend proxies the callback through to it. + */ + samlCallbackUrl(orgId: string): string { + return `${this.#uiUrl}/api/v1/saml/login/${orgId}/callback`; + } + + /** + * Link emailed to a user so they can set a new password. The token is + * hex-encoded (`randomBytes(32).toString('hex')`) and therefore already + * path-safe; a token format using other characters would need encoding here. + */ + resetPasswordUrl(token: string): string { + return `${this.#uiUrl}/reset_password/${token}`; + } + + /** + * Link emailed to invite someone into an org. Like the reset token, the + * invite token is hex-encoded and therefore already path-safe. + */ + signupUrl(token: string): string { + return `${this.#uiUrl}/signup/${token}`; + } +} diff --git a/server/services/configService/index.ts b/server/services/configService/index.ts new file mode 100644 index 000000000..cc870e866 --- /dev/null +++ b/server/services/configService/index.ts @@ -0,0 +1 @@ +export { default as ConfigService } from './configService.js'; diff --git a/server/services/hmaService/index.ts b/server/services/hmaService/index.ts index be81a152d..1b53d2e13 100644 --- a/server/services/hmaService/index.ts +++ b/server/services/hmaService/index.ts @@ -1,4 +1,5 @@ /* eslint-disable max-lines */ +import integrationsConfig from '#config/integrations'; import { type JsonValue } from 'type-fest'; import { FormData } from 'undici'; @@ -185,8 +186,7 @@ export class HmaService { private readonly fetchHTTP: Dependencies['fetchHTTP'], kyselyPg: Dependencies['KyselyPg'], ) { - this.hmaServiceUrl = - process.env.HMA_SERVICE_URL ?? 'http://localhost:9876/'; + this.hmaServiceUrl = integrationsConfig.hmaServiceUrl; this.hashBankService = new HashBankService(kyselyPg); } diff --git a/server/services/integrationRegistry/loadIntegrationsConfig.ts b/server/services/integrationRegistry/loadIntegrationsConfig.ts index c7661bfd1..f27174af6 100644 --- a/server/services/integrationRegistry/loadIntegrationsConfig.ts +++ b/server/services/integrationRegistry/loadIntegrationsConfig.ts @@ -6,11 +6,12 @@ import { existsSync, readFileSync } from 'fs'; import path from 'path'; import type { CoopIntegrationsConfig } from '@roostorg/coop-types'; +import integrationsConfig from '#config/integrations'; import { jsonParse, type JsonOf } from '../../utils/encoding.js'; function getConfigPath(): string { - const envPath = process.env.INTEGRATIONS_CONFIG_PATH; + const envPath = integrationsConfig.configPath; if (envPath != null && envPath !== '') { return path.isAbsolute(envPath) ? envPath diff --git a/server/services/itemProcessingService/fieldTypeHandlers.ts b/server/services/itemProcessingService/fieldTypeHandlers.ts index f919d6db9..f63340346 100644 --- a/server/services/itemProcessingService/fieldTypeHandlers.ts +++ b/server/services/itemProcessingService/fieldTypeHandlers.ts @@ -17,7 +17,7 @@ import _ from 'lodash'; import { match } from 'ts-pattern'; import { doesThrow } from '../../utils/misc.js'; -import { isValidUrl, makeUrlString } from '../../utils/url.js'; +import { isValidUrl, makeUrlString } from '../../utils/urlValidation.js'; const { isPlainObject } = _; diff --git a/server/services/ncmecService/ncmecDebug.ts b/server/services/ncmecService/ncmecDebug.ts index 70a26fcb5..11ae01cca 100644 --- a/server/services/ncmecService/ncmecDebug.ts +++ b/server/services/ncmecService/ncmecDebug.ts @@ -1,20 +1,16 @@ +import ncmecConfig from '#config/ncmec'; + import { jsonStringify } from '../../utils/encoding.js'; -// Opt-in debug logs + XML/JSON dumps for NCMEC submissions. Gated on -// `NCMEC_DEBUG=1` and `NODE_ENV !== 'production'` so we cannot leak +// Opt-in debug logs + XML/JSON dumps for NCMEC submissions. `ncmecConfig.debug` +// requires `NCMEC_DEBUG` *and* a non-production environment, so we cannot leak // reportable content in shared environments. Never log credentials. -export function ncmecDebugEnabled(): boolean { - return ( - process.env.NCMEC_DEBUG === '1' && process.env.NODE_ENV !== 'production' - ); -} - export function ncmecDebugLog( event: string, fields: Record, ): void { - if (!ncmecDebugEnabled()) { + if (!ncmecConfig.debug) { return; } // eslint-disable-next-line no-console @@ -25,7 +21,7 @@ export async function ncmecDebugDump( filename: string, contents: string, ): Promise { - if (!ncmecDebugEnabled()) { + if (!ncmecConfig.debug) { return; } try { diff --git a/server/services/ncmecService/ncmecReporting.test.ts b/server/services/ncmecService/ncmecReporting.test.ts index 5f19a320f..fc4b12b60 100644 --- a/server/services/ncmecService/ncmecReporting.test.ts +++ b/server/services/ncmecService/ncmecReporting.test.ts @@ -1,3 +1,5 @@ +import env from '#start/env'; + import { buildInternetDetailsFromOrgSetting, clampIncidentDateTimeToPast, @@ -456,17 +458,17 @@ describe('NCMEC reporting', () => { // (this file was over the 500-line max-lines limit after expansion). describe('summarizeCyberTipFailure', () => { - const previousDebug = process.env.NCMEC_DEBUG; - const previousNodeEnv = process.env.NODE_ENV; + const previousDebug = env.get('NCMEC_DEBUG'); + const previousNodeEnv = env.get('NODE_ENV'); afterEach(() => { - process.env.NCMEC_DEBUG = previousDebug; - process.env.NODE_ENV = previousNodeEnv; + env.set('NCMEC_DEBUG', previousDebug); + env.set('NODE_ENV', previousNodeEnv); }); it('includes NCMEC responseCode and description in production', () => { - process.env.NCMEC_DEBUG = undefined; - process.env.NODE_ENV = 'production'; + env.set('NCMEC_DEBUG', undefined); + env.set('NODE_ENV', 'production'); const body = { reportResponse: { responseCode: { _text: '4100' }, @@ -481,8 +483,8 @@ describe('NCMEC reporting', () => { }); it('does not leak unknown body fields in production', () => { - process.env.NCMEC_DEBUG = undefined; - process.env.NODE_ENV = 'production'; + env.set('NCMEC_DEBUG', undefined); + env.set('NODE_ENV', 'production'); const body = { secret: 'reportable-content-or-pii', unrelated: { nested: 'data' }, @@ -494,8 +496,8 @@ describe('NCMEC reporting', () => { }); it('appends the truncated body when NCMEC_DEBUG is enabled in dev', () => { - process.env.NCMEC_DEBUG = '1'; - process.env.NODE_ENV = 'test'; + env.set('NCMEC_DEBUG', true); + env.set('NODE_ENV', 'test'); const body = { reportResponse: { responseCode: { _text: '4000' }, @@ -510,8 +512,8 @@ describe('NCMEC reporting', () => { }); it('still keeps body off in production even with NCMEC_DEBUG=1', () => { - process.env.NCMEC_DEBUG = '1'; - process.env.NODE_ENV = 'production'; + env.set('NCMEC_DEBUG', true); + env.set('NODE_ENV', 'production'); const body = { reportResponse: { responseCode: { _text: '0' } } }; const message = summarizeCyberTipFailure('/submit', 502, body); expect(message).not.toContain('body='); diff --git a/server/services/ncmecService/ncmecReporting.ts b/server/services/ncmecService/ncmecReporting.ts index c5e388b72..6a2b51186 100644 --- a/server/services/ncmecService/ncmecReporting.ts +++ b/server/services/ncmecService/ncmecReporting.ts @@ -1,6 +1,7 @@ /* eslint-disable max-lines */ import type { Exception } from '@opentelemetry/api'; import { makeEnumLike, type ItemIdentifier } from '@roostorg/coop-types'; +import ncmecEnvConfig from '#config/ncmec'; import _Ajv from 'ajv'; import { sql, type Kysely } from 'kysely'; import _ from 'lodash'; @@ -29,11 +30,7 @@ import { type FormDataLikeWithStreams, } from '../networkingService/index.js'; import { type NcmecReportingServicePg } from './dbTypes.js'; -import { - ncmecDebugDump, - ncmecDebugEnabled, - ncmecDebugLog, -} from './ncmecDebug.js'; +import { ncmecDebugDump, ncmecDebugLog } from './ncmecDebug.js'; import { summarizeNcmecErrorForReviewer } from './ncmecReviewerErrors.js'; export const NCMECEvent = makeEnumLike([ @@ -485,7 +482,7 @@ export function summarizeCyberTipFailure( if (responseDescription != null) { parts.push(`responseDescription=${responseDescription}`); } - if (ncmecDebugEnabled()) { + if (ncmecEnvConfig.debug) { let snippet: string; try { const serialized = jsonStringify(body ?? null); @@ -621,7 +618,7 @@ export function toOriginalFileHashes(opts: { const trimmedAlgorithm = algorithm.trim(); if (trimmedHash === '' || trimmedAlgorithm === '') return; const hashType = trimmedAlgorithm.toUpperCase(); - const key = `${hashType}${trimmedHash}`; + const key = `${hashType}\u0000${trimmedHash}`; if (seen.has(key)) return; seen.add(key); result.push({ _text: trimmedHash, _attributes: { hashType } }); diff --git a/server/services/ncmecService/retryNcmecSubmission.ts b/server/services/ncmecService/retryNcmecSubmission.ts index 83e6ef127..0e68ae795 100644 --- a/server/services/ncmecService/retryNcmecSubmission.ts +++ b/server/services/ncmecService/retryNcmecSubmission.ts @@ -1,3 +1,5 @@ +import ncmecConfig from '#config/ncmec'; + import { type Dependencies } from '../../iocContainer/index.js'; import { type ManualReviewToolService } from '../manualReviewToolService/manualReviewToolService.js'; import { @@ -104,11 +106,13 @@ export async function retryNcmecSubmission( return { kind: 'permanent_error', error }; } - const isTest = process.env.NCMEC_ENV !== 'production'; // submitReport owns `ncmec_reports_errors` writes via `jobId` so retries // always update retry_count/last_error. Don't double-write here. try { - const result = await deps.ncmecReporting.submitReport(reportParams, isTest); + const result = await deps.ncmecReporting.submitReport( + reportParams, + ncmecConfig.isTest, + ); if (result === 'SUCCESS') { return { kind: 'success' }; } diff --git a/server/services/notificationsService/notificationFormatter.ts b/server/services/notificationsService/notificationFormatter.ts index c70fb01f3..d4905d44e 100644 --- a/server/services/notificationsService/notificationFormatter.ts +++ b/server/services/notificationsService/notificationFormatter.ts @@ -1,3 +1,4 @@ +import emailConfig from '#config/email'; import { type ReadonlyDeep } from 'type-fest'; import { @@ -33,7 +34,7 @@ export function formatNotification( secondToLastPeriodPassRate, } = data; - const supportEmail = process.env.SUPPORT_EMAIL ?? 'support@example.com'; + const supportEmail = emailConfig.addresses.support; const rateDetails = lastPeriodPassRate && secondToLastPeriodPassRate ? `In the past hour, the rule's pass rate went up${ diff --git a/server/services/placesApiService/placesApiService.ts b/server/services/placesApiService/placesApiService.ts index 97194e9c4..407a711d5 100644 --- a/server/services/placesApiService/placesApiService.ts +++ b/server/services/placesApiService/placesApiService.ts @@ -1,4 +1,5 @@ import { Client } from '@googlemaps/google-maps-services-js'; +import integrationsConfig from '#config/integrations'; import { inject } from '../../iocContainer/index.js'; @@ -9,7 +10,7 @@ class PlacesApiService { const requestParams = { params: { place_id: placeId, - key: String(process.env.GOOGLE_PLACES_API_KEY), + key: String(integrationsConfig.googlePlacesApiKey?.release()), }, }; diff --git a/server/services/sendEmailService/sendEmailService.test.ts b/server/services/sendEmailService/sendEmailService.test.ts index 9dedd173e..906352adb 100644 --- a/server/services/sendEmailService/sendEmailService.test.ts +++ b/server/services/sendEmailService/sendEmailService.test.ts @@ -1,5 +1,6 @@ import { SendEmailCommand, type SESClient } from '@aws-sdk/client-ses'; import sgMail from '@sendgrid/mail'; +import env from '#start/env'; import makeSendEmail, { CoopEmailAddress, @@ -179,8 +180,8 @@ describe('sendEmailService', () => { describe('console backend', () => { it('prints the email and reports successful delivery', async () => { - const previousNodeEnv = process.env.NODE_ENV; - process.env.NODE_ENV = 'development'; + const previousNodeEnv = env.get('NODE_ENV'); + env.set('NODE_ENV', 'development'); const consoleSpy = jest .spyOn(console, 'log') .mockImplementation(() => {}); @@ -200,20 +201,16 @@ describe('sendEmailService', () => { msg, ); } finally { - if (previousNodeEnv === undefined) { - delete process.env.NODE_ENV; - } else { - process.env.NODE_ENV = previousNodeEnv; - } + env.set('NODE_ENV', previousNodeEnv); consoleSpy.mockRestore(); } }); it('is selected explicitly through EMAIL_TRANSPORT', async () => { - const previousTransport = process.env.EMAIL_TRANSPORT; - const previousNodeEnv = process.env.NODE_ENV; - process.env.EMAIL_TRANSPORT = 'console'; - process.env.NODE_ENV = 'development'; + const previousTransport = env.get('EMAIL_TRANSPORT'); + const previousNodeEnv = env.get('NODE_ENV'); + env.set('EMAIL_TRANSPORT', 'console'); + env.set('NODE_ENV', 'development'); const consoleSpy = jest .spyOn(console, 'log') .mockImplementation(() => {}); @@ -230,25 +227,17 @@ describe('sendEmailService', () => { ).resolves.toBe(true); expect(consoleSpy).toHaveBeenCalledTimes(1); } finally { - if (previousTransport === undefined) { - delete process.env.EMAIL_TRANSPORT; - } else { - process.env.EMAIL_TRANSPORT = previousTransport; - } - if (previousNodeEnv === undefined) { - delete process.env.NODE_ENV; - } else { - process.env.NODE_ENV = previousNodeEnv; - } + env.set('EMAIL_TRANSPORT', previousTransport); + env.set('NODE_ENV', previousNodeEnv); consoleSpy.mockRestore(); } }); it('rejects console transport outside development', () => { - const previousTransport = process.env.EMAIL_TRANSPORT; - const previousNodeEnv = process.env.NODE_ENV; - process.env.EMAIL_TRANSPORT = 'console'; - process.env.NODE_ENV = 'production'; + const previousTransport = env.get('EMAIL_TRANSPORT'); + const previousNodeEnv = env.get('NODE_ENV'); + env.set('EMAIL_TRANSPORT', 'console'); + env.set('NODE_ENV', 'production'); try { for (const makeTransport of [ @@ -260,16 +249,8 @@ describe('sendEmailService', () => { ); } } finally { - if (previousTransport === undefined) { - delete process.env.EMAIL_TRANSPORT; - } else { - process.env.EMAIL_TRANSPORT = previousTransport; - } - if (previousNodeEnv === undefined) { - delete process.env.NODE_ENV; - } else { - process.env.NODE_ENV = previousNodeEnv; - } + env.set('EMAIL_TRANSPORT', previousTransport); + env.set('NODE_ENV', previousNodeEnv); } }); }); diff --git a/server/services/sendEmailService/sendEmailService.ts b/server/services/sendEmailService/sendEmailService.ts index 0384904c9..9a08872da 100644 --- a/server/services/sendEmailService/sendEmailService.ts +++ b/server/services/sendEmailService/sendEmailService.ts @@ -1,10 +1,12 @@ import { SendEmailCommand, SESClient } from '@aws-sdk/client-ses'; import sgMail from '@sendgrid/mail'; +import appConfig from '#config/app'; +import emailConfig from '#config/email'; export const CoopEmailAddress = { - NoReply: process.env.NOREPLY_EMAIL ?? 'noreply@example.com', - Support: process.env.SUPPORT_EMAIL ?? 'support@example.com', - Team: process.env.TEAM_EMAIL ?? 'team@example.com', + NoReply: emailConfig.addresses.noReply, + Support: emailConfig.addresses.support, + Team: emailConfig.addresses.team, } as const; export type CoopEmailAddress = @@ -80,7 +82,7 @@ export function makeSendEmailViaSendGrid(apiKey: string) { } export function makeSendEmailViaConsole() { - if (process.env.NODE_ENV !== 'development') { + if (!appConfig.inDev) { throw new Error( 'EMAIL_TRANSPORT=console is only available when NODE_ENV=development', ); @@ -100,13 +102,13 @@ const makeSendEmail = (clientOrContainer?: SESClient | unknown) => { return makeSendEmailViaSES(clientOrContainer); } - if (process.env.EMAIL_TRANSPORT === 'console') { + if (emailConfig.transport === 'console') { return makeSendEmailViaConsole(); } - const sendGridApiKey = process.env.SENDGRID_API_KEY; + const sendGridApiKey = emailConfig.sendgridApiKey; if (sendGridApiKey) { - return makeSendEmailViaSendGrid(sendGridApiKey); + return makeSendEmailViaSendGrid(sendGridApiKey.release()); } return makeSendEmailViaSES(new SESClient({})); diff --git a/server/services/userManagementService/userManagementService.test.ts b/server/services/userManagementService/userManagementService.test.ts index b51c1ea09..8739fa926 100644 --- a/server/services/userManagementService/userManagementService.test.ts +++ b/server/services/userManagementService/userManagementService.test.ts @@ -1,6 +1,7 @@ import { type Kysely } from 'kysely'; import { makeTestWithFixture } from '../../test/utils.js'; +import { ConfigService } from '../configService/index.js'; import { MIN_PASSWORD_LENGTH } from './constants.js'; import type { UserManagementPg } from './index.js'; import UserManagementService from './userManagementService.js'; @@ -16,9 +17,7 @@ const mockDb = { const mockSendEmail = jest.fn(); -const mockConfigService = { - uiUrl: 'http://localhost:3000', -}; +const mockConfigService = new ConfigService('http://localhost:3000'); describe('UserManagementService', () => { const testWithFixtures = makeTestWithFixture(() => { diff --git a/server/services/userManagementService/userManagementService.ts b/server/services/userManagementService/userManagementService.ts index bf86bdbd0..c4618b97d 100644 --- a/server/services/userManagementService/userManagementService.ts +++ b/server/services/userManagementService/userManagementService.ts @@ -373,12 +373,12 @@ class UserManagementService { const { userId, orgId } = existingUser; const token = await this.#createPasswordResetToken({ userId, orgId }); - const url = new URL(`${this.configService.uiUrl}/reset_password/` + token); + const url = this.configService.resetPasswordUrl(token); const msg = { to: email, from: CoopEmailAddress.NoReply, subject: '[Coop] Reset your password', - html: `You recently indicated that you forgot your Coop password. Click on this link to create a new password. The link expires in 1 hour, so please make sure to sign up soon. + html: `You recently indicated that you forgot your Coop password. Click on this link to create a new password. The link expires in 1 hour, so please make sure to sign up soon.

Best,
Coop Support Team`, @@ -424,12 +424,12 @@ class UserManagementService { }); // Send email using the standard flow (will be no-op if SendGrid not configured) - const url = new URL(`${this.configService.uiUrl}/reset_password/` + token); + const url = this.configService.resetPasswordUrl(token); const msg = { to: email, from: CoopEmailAddress.NoReply, subject: '[Coop] Reset your password', - html: `Your organization administrator has initiated a password reset for your account. Click on this link to create a new password. The link expires in 1 hour, so please make sure to reset your password soon. + html: `Your organization administrator has initiated a password reset for your account. Click on this link to create a new password. The link expires in 1 hour, so please make sure to reset your password soon.

Best,
Coop Support Team`, diff --git a/server/start/env.ts b/server/start/env.ts new file mode 100644 index 000000000..a0d9d4f19 --- /dev/null +++ b/server/start/env.ts @@ -0,0 +1,159 @@ +import { Env } from '#lib/env'; + +/** + * Whether the Scylla-backed features are switched off, which makes the + * connection settings unnecessary. + * + * A function rather than a boolean because a boolean would be evaluated while + * the schema object below is being built — before `Env.create` copies `.env` + * file values into `process.env`. It would then be correct only for a + * deployment that sets `SCYLLA_ENABLED` as a real environment variable, and + * silently wrong for one that sets it in an env file. + */ +const scyllaDisabled = () => + process.env.SCYLLA_ENABLED === 'false' || process.env.SCYLLA_ENABLED === '0'; + +// `optional()` here means the application supplies a default, not that the +// value is unimportant. See the corresponding `config/*.ts` module for what +// that default is. Only variables the app genuinely cannot start without are +// required, so that existing deployments relying on a built-in default keep +// booting. An optional enum still rejects an invalid value when one is set. +const env = await Env.create(new URL('./', import.meta.url), { + NODE_ENV: Env.schema.enum.optional([ + 'development', + 'production', + 'test', + ] as const), + // For a future logger: + // LOG_LEVEL: Env.schema.string(), + PORT: Env.schema.integer.positive.optional(), + // `tld: false` so `http://localhost:3000` is accepted — the URL format + // requires a TLD by default, which rejects every local and container-internal + // hostname. + UI_URL: Env.schema.string({ format: 'url', tld: false }), + + OTEL_SERVICE_NAME: Env.schema.string.optional(), + + // Emails: + NOREPLY_EMAIL: Env.schema.string.optional({ format: 'email' }), + SUPPORT_EMAIL: Env.schema.string.optional({ format: 'email' }), + TEAM_EMAIL: Env.schema.string.optional({ format: 'email' }), + EMAIL_TRANSPORT: Env.schema.enum.optional(['console'] as const), + + // Postgresql configuration: + DATABASE_HOST: Env.schema.string({ format: 'host' }), + // Falls back to DATABASE_HOST, so a deployment without a separate replica + // need not repeat the primary host. + DATABASE_READ_ONLY_HOST: Env.schema.string.optional({ format: 'host' }), + DATABASE_PORT: Env.schema.integer.positive.optional(), + DATABASE_NAME: Env.schema.string.optional(), + DATABASE_USER: Env.schema.string.optional(), + DATABASE_PASSWORD: Env.schema.secret(), + DATABASE_SSL: Env.schema.boolean.optional(), + DATABASE_POOL_MAX: Env.schema.integer.positive.optional(), + DATABASE_READ_POOL_MAX: Env.schema.integer.positive.optional(), + // Zero disables these, so they allow it. + DATABASE_POOL_MAX_LIFETIME_SECONDS: Env.schema.integer.nonNegative.optional(), + DATABASE_POOL_IDLE_TIMEOUT_MS: Env.schema.integer.nonNegative.optional(), + DATABASE_POOL_CONNECTION_TIMEOUT_MS: + Env.schema.integer.nonNegative.optional(), + DATABASE_QUERY_TIMEOUT_MS: Env.schema.integer.nonNegative.optional(), + DATABASE_IDLE_IN_TRANSACTION_TIMEOUT_MS: + Env.schema.integer.nonNegative.optional(), + DATABASE_STATEMENT_TIMEOUT_MS: Env.schema.integer.nonNegative.optional(), + DATABASE_KEEPALIVE: Env.schema.boolean.optional(), + DATABASE_KEEPALIVE_INITIAL_DELAY_MS: + Env.schema.integer.nonNegative.optional(), + DATABASE_PRINT_LOGS: Env.schema.boolean.optional(), + + // Redis: + REDIS_USE_CLUSTER: Env.schema.boolean(), + REDIS_HOST: Env.schema.string({ format: 'host' }), + REDIS_PORT: Env.schema.integer.positive.optional(), + REDIS_USER: Env.schema.string.optional(), + REDIS_PASSWORD: Env.schema.secret.optional(), + // Single-node connections only: the cluster path is always TLS. + REDIS_TLS: Env.schema.boolean.optional(), + + // Secrets: + SESSION_SECRET: Env.schema.string(), + GRAPHQL_MAX_DEPTH: Env.schema.integer.positive.optional(), + + // Default to `clickhouse`; ANALYTICS_ADAPTER falls back to WAREHOUSE_ADAPTER. + WAREHOUSE_ADAPTER: Env.schema.enum.optional(['noop', 'clickhouse'] as const), + ANALYTICS_ADAPTER: Env.schema.enum.optional(['noop', 'clickhouse'] as const), + // Legacy: use WAREHOUSE_ADAPTER and ANALYTICS_ADAPTER instead: + DATA_WAREHOUSE_PROVIDER: Env.schema.enum.optional([ + 'noop', + 'clickhouse', + ] as const), + // Clickhouse settings, only used when WAREHOUSE_ADAPTER or ANALYTICS_ADAPTER is clickhouse: + CLICKHOUSE_HOST: Env.schema.string.optional({ format: 'host' }), + CLICKHOUSE_PORT: Env.schema.integer.positive.optional(), + CLICKHOUSE_USERNAME: Env.schema.string.optional(), + CLICKHOUSE_PASSWORD: Env.schema.secret.optional(), + CLICKHOUSE_DATABASE: Env.schema.string.optional(), + CLICKHOUSE_PROTOCOL: Env.schema.enum.optional(['https', 'http'] as const), + CLICKHOUSE_POOL_SIZE: Env.schema.integer.positive.optional(), + // Zero means no retries and no delay respectively, so these allow it. + CLICKHOUSE_INSERT_MAX_RETRIES: Env.schema.integer.nonNegative.optional(), + CLICKHOUSE_INSERT_RETRY_INITIAL_MS: Env.schema.integer.nonNegative.optional(), + CLICKHOUSE_INSERT_RETRY_MAX_MS: Env.schema.integer.nonNegative.optional(), + // Clickhouse Memory settings. Zero disables the external-memory thresholds. + CLICKHOUSE_MAX_BYTES_BEFORE_EXTERNAL_GROUP_BY: + Env.schema.integer.nonNegative.optional(), + CLICKHOUSE_MAX_BYTES_BEFORE_EXTERNAL_SORT: + Env.schema.integer.nonNegative.optional(), + CLICKHOUSE_MAX_THREADS: Env.schema.integer.positive.optional(), + CLICKHOUSE_MAX_BLOCK_SIZE: Env.schema.integer.positive.optional(), + // Other Clickhouse: + CLICKHOUSE_RULE_INSIGHTS_LOOKBACK_DAYS: + Env.schema.integer.positive.optional(), + + // Scylla: + // Turning this off swaps in a no-op that drops writes and returns empty + // reads, disabling Item Investigation and User Strikes. The connection + // settings below are then not needed, which is what `optionalWhen` expresses. + SCYLLA_ENABLED: Env.schema.boolean.optional(), + // Contact points, e.g. "db1,db2:9043". Parsed into a list here so consumers + // receive `string[]` rather than re-splitting the raw value. + SCYLLA_HOSTS: Env.schema.hostList.optionalWhen(scyllaDisabled), + SCYLLA_USERNAME: Env.schema.string.optionalWhen(scyllaDisabled), + SCYLLA_PASSWORD: Env.schema.secret.optionalWhen(scyllaDisabled), + SCYLLA_LOCAL_DATACENTER: Env.schema.string.optionalWhen(scyllaDisabled), + SCYLLA_PORT: Env.schema.integer.positive.optional(), + SCYLLA_SSL: Env.schema.boolean.optional(), + // An explicit SNI value for TLS hostname verification, for when the contact + // points don't match the server certificate. + SCYLLA_SSL_SERVERNAME: Env.schema.string.optional({ format: 'host' }), + + // Selects the NCMEC CyberTipline endpoint that "Submit to NCMEC" decisions are + // routed to. Anything other than the literal string `production` (including + // being unset) sends submissions to https://exttest.cybertip.org (the NCMEC + // sandbox, reports are discarded). Set to `production` only when the + // CyberTipline credentials configured in Settings → NCMEC are production + // credentials issued by NCMEC and your integration has been approved for live + // reporting. + NCMEC_ENV: Env.schema.enum.optional(['production', 'test'] as const), + NCMEC_DEBUG: Env.schema.boolean.optional(), + + // Debugging: + EXPOSE_SENSITIVE_IMPLEMENTATION_DETAILS_IN_ERRORS: + Env.schema.boolean.optional(), + ALLOW_USER_INPUT_LOCALHOST_URIS: Env.schema.boolean.optional(), + LOG_REQUEST_BODY: Env.schema.boolean.optional(), + + // Integrations: + GROQ_SECRET_KEY: Env.schema.secret.optional(), + SENDGRID_API_KEY: Env.schema.secret.optional(), + GOOGLE_PLACES_API_KEY: Env.schema.secret.optional(), + OPEN_AI_API_KEY: Env.schema.secret.optional(), + SLACK_APP_BEARER_TOKEN: Env.schema.secret.optional(), + // Defaults to http://localhost:9876/ in `hmaService`. + HMA_SERVICE_URL: Env.schema.string.optional({ format: 'url', tld: false }), + + // Others: + ITEM_QUEUE_TRAFFIC_PERCENTAGE: Env.schema.number(), +}); + +export default env; diff --git a/server/storage/dataWarehouse/ClickhouseAdapter.ts b/server/storage/dataWarehouse/ClickhouseAdapter.ts index ee28bf526..1073cab7e 100644 --- a/server/storage/dataWarehouse/ClickhouseAdapter.ts +++ b/server/storage/dataWarehouse/ClickhouseAdapter.ts @@ -14,7 +14,7 @@ import { type TransactionSettings, } from 'kysely'; -import { getClickhouseMemorySettings } from '../../plugins/warehouse/utils/clickhouseSettings.js'; +import { type ClickhouseMemorySettings } from '../../plugins/warehouse/utils/clickhouseSettings.js'; import { formatClickhouseQuery } from '../../plugins/warehouse/utils/clickhouseSql.js'; import type { DataWarehousePoolSettings, @@ -118,6 +118,7 @@ export class ClickhouseKyselyAdapter implements IDataWarehouseDialect { constructor( connectionSettings: ClickhouseConnectionSettings, + memorySettings: ClickhouseMemorySettings, _poolSettings?: DataWarehousePoolSettings, ) { const protocol = connectionSettings.protocol ?? 'http'; @@ -134,7 +135,7 @@ export class ClickhouseKyselyAdapter implements IDataWarehouseDialect { database: connectionSettings.database, clickhouse_settings: { allow_experimental_object_type: 1, - ...getClickhouseMemorySettings(), + ...memorySettings, }, }); diff --git a/server/storage/dataWarehouse/DataWarehouseFactory.ts b/server/storage/dataWarehouse/DataWarehouseFactory.ts index 5f85fd1ba..ff99bba17 100644 --- a/server/storage/dataWarehouse/DataWarehouseFactory.ts +++ b/server/storage/dataWarehouse/DataWarehouseFactory.ts @@ -5,6 +5,7 @@ /* eslint-disable max-classes-per-file */ import { type Kysely } from 'kysely'; +import { type ClickhouseInsertRetrySettings } from '../../plugins/analytics/adapters/clickhouseRetry.js'; import { ClickhouseAnalyticsAdapter as ClickhouseAnalyticsPlugin, NoOpAnalyticsAdapter, @@ -15,6 +16,7 @@ import { NoOpWarehouseAdapter, type IWarehouseAdapter, } from '../../plugins/warehouse/index.js'; +import { type ClickhouseMemorySettings } from '../../plugins/warehouse/utils/clickhouseSettings.js'; import { assertUnreachable } from '../../utils/misc.js'; import type SafeTracer from '../../utils/SafeTracer.js'; import { @@ -43,8 +45,6 @@ import { PostgresAnalyticsAdapter } from './PostgresAnalyticsAdapter.js'; */ export type DataWarehouseProvider = 'clickhouse' | 'postgresql' | 'noop'; -export type AnalyticsProvider = 'clickhouse' | 'postgresql' | 'noop'; - // Re-export the interface provider type for external use export type { IDataWarehouseProvider }; @@ -53,7 +53,10 @@ export type DataWarehouseConfig = provider: 'clickhouse'; connection: ClickhouseConnectionSettings; pool?: DataWarehousePoolSettings; - analyticsProvider?: AnalyticsProvider; + /** Per-query memory limits, passed to every adapter this config builds. */ + memory: ClickhouseMemorySettings; + /** Retry policy for inserts, used by the analytics adapter. */ + insertRetry: ClickhouseInsertRetrySettings; } | { provider: 'postgresql'; @@ -65,11 +68,9 @@ export type DataWarehouseConfig = database: string; }; pool?: DataWarehousePoolSettings; - analyticsProvider?: AnalyticsProvider; } | { provider: 'noop'; - analyticsProvider?: AnalyticsProvider; }; class NoOpKyselyDialect implements IDataWarehouseDialect { @@ -240,7 +241,11 @@ export class DataWarehouseFactory { ): IDataWarehouseDialect { switch (config.provider) { case 'clickhouse': - return new ClickhouseKyselyAdapter(config.connection, config.pool); + return new ClickhouseKyselyAdapter( + config.connection, + config.memory, + config.pool, + ); case 'postgresql': throw new Error('PostgreSQL Kysely dialect not yet implemented'); case 'noop': @@ -263,21 +268,16 @@ export class DataWarehouseFactory { config: DataWarehouseConfig, dialect?: IDataWarehouseDialect, ): IDataWarehouseAnalytics { - const analyticsProvider = config.analyticsProvider ?? config.provider; - - switch (analyticsProvider) { + switch (config.provider) { case 'noop': return new AnalyticsAdapterBridge('noop', new NoOpAnalyticsAdapter()); case 'clickhouse': - if (config.provider !== 'clickhouse') { - throw new Error( - 'Clickhouse analytics provider requires the clickhouse warehouse configuration.', - ); - } return new AnalyticsAdapterBridge( 'clickhouse', new ClickhouseAnalyticsPlugin({ connection: config.connection, + memory: config.memory, + retry: config.insertRetry, }), ); case 'postgresql': { @@ -289,68 +289,10 @@ export class DataWarehouseFactory { } default: return assertUnreachable( - analyticsProvider, - `Unknown analytics provider: ${analyticsProvider as string}`, - ); - } - } - - /** - * Create configuration from environment variables - */ - - static createConfigFromEnv(): DataWarehouseConfig { - const provider = (process.env.WAREHOUSE_ADAPTER ?? - process.env.DATA_WAREHOUSE_PROVIDER ?? - 'clickhouse') as DataWarehouseProvider; - const analyticsProvider = (process.env.ANALYTICS_ADAPTER ?? - provider) as AnalyticsProvider; - - switch (provider) { - case 'noop': - return { - provider: 'noop', - analyticsProvider, - }; - case 'clickhouse': - return { - provider: 'clickhouse', - analyticsProvider, - connection: { - host: process.env.CLICKHOUSE_HOST ?? 'localhost', - port: process.env.CLICKHOUSE_PORT - ? parseInt(process.env.CLICKHOUSE_PORT) - : 8123, - username: process.env.CLICKHOUSE_USERNAME ?? 'default', - password: process.env.CLICKHOUSE_PASSWORD ?? '', - database: process.env.CLICKHOUSE_DATABASE ?? 'default', - protocol: (process.env.CLICKHOUSE_PROTOCOL ?? 'http') as - 'http' | 'https', - }, - pool: { - max: process.env.CLICKHOUSE_POOL_SIZE - ? parseInt(process.env.CLICKHOUSE_POOL_SIZE) - : 10, - }, - }; - case 'postgresql': - return { - provider: 'postgresql', - analyticsProvider, - connection: { - host: process.env.POSTGRES_HOST ?? 'localhost', - port: process.env.POSTGRES_PORT - ? parseInt(process.env.POSTGRES_PORT) - : undefined, - username: process.env.POSTGRES_USERNAME ?? 'postgres', - password: process.env.POSTGRES_PASSWORD ?? '', - database: process.env.POSTGRES_DATABASE ?? 'postgres', - }, - }; - default: - return assertUnreachable( - provider, - `Unknown data warehouse provider: ${provider as string}`, + config, + `Unknown analytics provider: ${ + (config as DataWarehouseConfig).provider + }`, ); } } diff --git a/server/test/harness/transactionalPgPool.integ.test.ts b/server/test/harness/transactionalPgPool.integ.test.ts index b7be617ff..4815d4d54 100644 --- a/server/test/harness/transactionalPgPool.integ.test.ts +++ b/server/test/harness/transactionalPgPool.integ.test.ts @@ -4,14 +4,14 @@ * Proves that `createTransactionalTestDb` lets us wrap a whole test in a single * Postgres transaction that is rolled back at the end. */ +import databaseConfig from '#config/database'; import { Kysely, PostgresDialect, sql, type PostgresPool } from 'kysely'; import pg from 'pg'; -import { getPgConnectionParams } from '../../iocContainer/index.js'; import { makeKyselyTransactionWithRetry } from '../../utils/kyselyTransactionWithRetry.js'; import { createTransactionalTestDb } from './transactionalPgPool.js'; -const pgConfig = getPgConnectionParams(); +const pgConfig = databaseConfig.connectionParams; describe('createTransactionalTestDb', () => { it('exposes the pool metadata Kysely needs for query cancellation', async () => { diff --git a/server/test/integ/ncmec-report-submission.integ.test.ts b/server/test/integ/ncmec-report-submission.integ.test.ts index 02bbc358b..b800f376d 100644 --- a/server/test/integ/ncmec-report-submission.integ.test.ts +++ b/server/test/integ/ncmec-report-submission.integ.test.ts @@ -10,6 +10,7 @@ * Requires: `npm run up && npm run db:update` */ import { ScalarTypes } from '@roostorg/coop-types'; +import ncmecConfig from '#config/ncmec'; import { uid } from 'uid'; import { jsonStringify } from '../../utils/encoding.js'; @@ -412,7 +413,7 @@ describe('NCMEC report and submission (integration)', () => { expect(reportRow.report_id).toBe(ncmecReportId); expect(reportRow.reviewer_id).toBe(reviewerId); - expect(reportRow.is_test).toBe(process.env.NCMEC_ENV !== 'production'); + expect(reportRow.is_test).toBe(ncmecConfig.isTest); const reportedMediaIds = ( reportRow.reported_media as Array<{ id: string; typeId: string }> diff --git a/server/test/setupMockedServer.ts b/server/test/setupMockedServer.ts index b75f75a82..acae3a405 100644 --- a/server/test/setupMockedServer.ts +++ b/server/test/setupMockedServer.ts @@ -3,13 +3,11 @@ // relying on here). import otel from '@opentelemetry/api'; +import databaseConfig from '#config/database'; import type pg from 'pg'; import * as superTest from 'supertest'; -import getBottle, { - getPgConnectionParams, - type Dependencies, -} from '../iocContainer/index.js'; +import getBottle, { type Dependencies } from '../iocContainer/index.js'; import makeServer from '../server.js'; import { type IDataWarehouse } from '../storage/dataWarehouse/IDataWarehouse.js'; import type { IDataWarehouseAnalytics } from '../storage/dataWarehouse/IDataWarehouseAnalytics.js'; @@ -40,7 +38,7 @@ export function disableConsoleLogging() { * `makeTransactionalTestWithFixture`. */ export async function makeMockedServer() { - const tdb = createTransactionalTestDb(getPgConnectionParams()); + const tdb = createTransactionalTestDb(databaseConfig.connectionParams); await tdb.begin(); const deps = await getBottleContainerWithIOMocks({ kyselyPool: tdb.pool }); diff --git a/server/utils/errors.ts b/server/utils/errors.ts index 9185fcd02..13f1c6073 100644 --- a/server/utils/errors.ts +++ b/server/utils/errors.ts @@ -5,6 +5,8 @@ // folder (even though the logic in them really ought to be in a // transport-agnostic service in the services folder), so, for now, this file // has to import just those files from the graphql folder. +import debugConfig from '#config/debug'; + import { type IntegrationErrorType } from '../graphql/datasources/IntegrationApi.js'; import { type OrgErrorType } from '../graphql/datasources/OrgApi.js'; import { @@ -365,8 +367,7 @@ export const makeBadRequestError = (title: string, data: ErrorInstanceData) => name: 'BadRequestError', }); -const exposeUnsafeErrorDetails = - process.env.EXPOSE_SENSITIVE_IMPLEMENTATION_DETAILS_IN_ERRORS === 'true'; +const exposeUnsafeErrorDetails = debugConfig.exposeUnsafeErrorDetails; export const sanitizeError = exposeUnsafeErrorDetails ? // In local dev, when exposeUnsafeErrorDetails is true, sanitizeError diff --git a/server/utils/url.test.ts b/server/utils/url.test.ts index 81e09d5ee..2be666bc4 100644 --- a/server/utils/url.test.ts +++ b/server/utils/url.test.ts @@ -1,46 +1,44 @@ -import { validateUrl } from './url.js'; +import { LOOPBACK_HOSTNAMES, validateUrl } from './url.js'; -describe('URL Tests', () => { - describe('Loopback gating (default)', () => { - beforeEach(() => { - // This absolutely is unsafe mutation of a global that'll be visible - // across test suites. However, this env var should only be relied upon - // by this module, so it should be ok. - process.env.ALLOW_USER_INPUT_LOCALHOST_URIS = 'false'; - }); +const blockingLoopback = { + allowedSchemes: ['http', 'https'], + blockedHostnames: [...LOOPBACK_HOSTNAMES], +}; - afterEach(() => { - delete process.env.ALLOW_USER_INPUT_LOCALHOST_URIS; - }); +const allowingLoopback = { + allowedSchemes: ['http', 'https'], + blockedHostnames: [], +}; +describe('URL Tests', () => { + describe('Loopback blocked', () => { test('Deny localhost domains', () => { - expect(() => validateUrl('https://localhost:3000')).toThrow(); - expect(() => validateUrl('https://127.0.0.1')).toThrow(); + expect(() => + validateUrl('https://localhost:3000', blockingLoopback), + ).toThrow(); + expect(() => + validateUrl('https://127.0.0.1', blockingLoopback), + ).toThrow(); }); test('Allow arbitrary external URLs', () => { - expect(() => validateUrl('https://example.com')).not.toThrow(); expect(() => - validateUrl('https://api.example.org/webhook'), + validateUrl('https://example.com', blockingLoopback), + ).not.toThrow(); + expect(() => + validateUrl('https://api.example.org/webhook', blockingLoopback), ).not.toThrow(); }); }); - describe('Loopback gating (development override)', () => { - beforeEach(() => { - // This absolutely is unsafe mutation of a global that'll be visible - // across test suites. However, this env var should only be relied upon - // by this module, so it should be ok. - process.env.ALLOW_USER_INPUT_LOCALHOST_URIS = 'true'; - }); - - afterEach(() => { - delete process.env.ALLOW_USER_INPUT_LOCALHOST_URIS; - }); - + describe('Loopback allowed', () => { test('Allow localhost domains', () => { - expect(() => validateUrl('https://localhost:3000')).not.toThrow(); - expect(() => validateUrl('https://127.0.0.1')).not.toThrow(); + expect(() => + validateUrl('https://localhost:3000', allowingLoopback), + ).not.toThrow(); + expect(() => + validateUrl('https://127.0.0.1', allowingLoopback), + ).not.toThrow(); }); }); diff --git a/server/utils/url.ts b/server/utils/url.ts index 555aa65c2..ec13e8efb 100644 --- a/server/utils/url.ts +++ b/server/utils/url.ts @@ -1,51 +1,45 @@ -import { type UrlString } from '@roostorg/coop-types'; - -import { instantiateOpaqueType } from './typescript-types.js'; +/** + * URL checks with no notion of how this deployment is configured — callers + * supply the rules. `./urlValidation.js` is the configured entry point that + * applies Coop's own rules; use that unless you specifically want to say what + * counts as valid. + */ -type UrlValidationOptions = { +export type UrlValidationOptions = { allowedSchemes: string[]; blockedHostnames: string[]; }; -function defaultBlockedHostnames(): string[] { - const { ALLOW_USER_INPUT_LOCALHOST_URIS } = process.env; +/** + * Loopback addresses, blocked by default to reduce SSRF risk from user-supplied + * URLs (e.g. webhook callbacks). Deployments wanting stricter blocking against + * their own public hostname pass `blockedHostnames` themselves. + */ +export const LOOPBACK_HOSTNAMES: readonly string[] = Object.freeze([ + 'localhost', + '127.0.0.1', +]); - // Block loopback addresses to reduce SSRF risk from user-supplied URLs (e.g. - // webhook callbacks). Operators who want stricter blocking against their own - // public hostname should pass `blockedHostnames` via `UrlValidationOptions`. - return ALLOW_USER_INPUT_LOCALHOST_URIS === 'true' - ? [] - : ['localhost', '127.0.0.1']; -} +export function validateUrl(value: string, opts: UrlValidationOptions) { + if (!URL.canParse(value)) { + throw new Error('Invalid URL'); + } -export function validateUrl( - value: string, - // If you update these opts make sure to update validateUrlOrNull's opts as - // well - opts: UrlValidationOptions = { - allowedSchemes: ['http', 'https'], - blockedHostnames: defaultBlockedHostnames(), - }, -) { - try { - const { allowedSchemes, blockedHostnames } = opts; - const { hostname, protocol } = new URL(value); // might throw. - const containsValidScheme = allowedSchemes.includes(protocol.slice(0, -1)); - if (!containsValidScheme) { - throw new Error('URL contains invalid scheme'); - } + const { allowedSchemes, blockedHostnames } = opts; + const { hostname, protocol } = new URL(value); + const containsValidScheme = allowedSchemes.includes(protocol.slice(0, -1)); + if (!containsValidScheme) { + throw new Error('URL contains invalid scheme'); + } - const containsBlockedHostname = blockedHostnames.includes(hostname); + const containsBlockedHostname = blockedHostnames.includes(hostname); - if (containsBlockedHostname) { - throw new Error('URL contains blocked hostname'); - } - } catch (_) { - throw new Error('Invalid URL'); + if (containsBlockedHostname) { + throw new Error('URL contains blocked hostname'); } } -export function isValidUrl(url: string, opts?: UrlValidationOptions) { +export function isValidUrl(url: string, opts: UrlValidationOptions) { try { validateUrl(url, opts); return true; @@ -53,26 +47,3 @@ export function isValidUrl(url: string, opts?: UrlValidationOptions) { return false; } } - -/** - * Returns a {@link UrlString} if the input string is a valid URL; else - * undefined. Does not accept urls that are invalid according to the default - * {@link UrlValidationOptions} used by {@link validateUrl}. - */ -export function makeUrlString(it: string) { - return isValidUrl(it) ? instantiateOpaqueType(it) : undefined; -} - -export function validateUrlOrNull( - value?: string, - // Keep this default in sync with `validateUrl`'s default. - opts: UrlValidationOptions = { - allowedSchemes: ['http', 'https'], - blockedHostnames: defaultBlockedHostnames(), - }, -) { - if (value == null) { - return; - } - validateUrl(value, opts); -} diff --git a/server/utils/urlValidation.ts b/server/utils/urlValidation.ts new file mode 100644 index 000000000..b9a65516b --- /dev/null +++ b/server/utils/urlValidation.ts @@ -0,0 +1,48 @@ +import { type UrlString } from '@roostorg/coop-types'; +import debugConfig from '#config/debug'; + +import { instantiateOpaqueType } from './typescript-types.js'; +import { + isValidUrl as isValidUrlAgainst, + LOOPBACK_HOSTNAMES, + validateUrl as validateUrlAgainst, + type UrlValidationOptions, +} from './url.js'; + +/** + * The rules this deployment applies to user-supplied URLs. + * + * Read on each call rather than captured, so `env.set` in a test is respected — + * see the note in `config/app.ts`. + */ +function deploymentOptions(): UrlValidationOptions { + return { + allowedSchemes: ['http', 'https'], + blockedHostnames: debugConfig.allowUserInputLocalhostUris + ? [] + : [...LOOPBACK_HOSTNAMES], + }; +} + +/** Throws `Invalid URL` unless `value` passes this deployment's rules. */ +export function validateUrl( + value: string, + opts: UrlValidationOptions = deploymentOptions(), +) { + validateUrlAgainst(value, opts); +} + +export function isValidUrl( + value: string, + opts: UrlValidationOptions = deploymentOptions(), +) { + return isValidUrlAgainst(value, opts); +} + +/** + * Returns a {@link UrlString} if the input passes this deployment's rules, else + * undefined. + */ +export function makeUrlString(it: string) { + return isValidUrl(it) ? instantiateOpaqueType(it) : undefined; +} diff --git a/server/workers_jobs/RetryFailedNcmecDecisionsJob.test.ts b/server/workers_jobs/RetryFailedNcmecDecisionsJob.test.ts index 48a3de45a..57f513087 100644 --- a/server/workers_jobs/RetryFailedNcmecDecisionsJob.test.ts +++ b/server/workers_jobs/RetryFailedNcmecDecisionsJob.test.ts @@ -1,3 +1,4 @@ +import env from '#start/env'; import { v1 as uuidv1 } from 'uuid'; import { @@ -143,21 +144,17 @@ describe('RetryFailedNcmecDecisionsJob', () => { /** Snapshot of NCMEC_ENV across the suite so each test can mutate it * freely and we restore the original value in afterEach. */ - let originalNcmecEnv: string | undefined; + let originalNcmecEnv: 'production' | 'test' | undefined; beforeEach(() => { - originalNcmecEnv = process.env.NCMEC_ENV; + originalNcmecEnv = env.get('NCMEC_ENV'); }); afterEach(() => { - if (originalNcmecEnv === undefined) { - delete process.env.NCMEC_ENV; - } else { - process.env.NCMEC_ENV = originalNcmecEnv; - } + env.set('NCMEC_ENV', originalNcmecEnv); }); it('passes isTest=true to submitReport when NCMEC_ENV is unset', async () => { - delete process.env.NCMEC_ENV; + env.set('NCMEC_ENV', undefined); const deps = makeDeps({ decisions: [makeNcmecDecisionRow(ORG_ID)], }); @@ -179,7 +176,7 @@ describe('RetryFailedNcmecDecisionsJob', () => { }); it('passes isTest=true to submitReport when NCMEC_ENV is "test"', async () => { - process.env.NCMEC_ENV = 'test'; + env.set('NCMEC_ENV', 'test'); const deps = makeDeps({ decisions: [makeNcmecDecisionRow(ORG_ID)], }); @@ -201,7 +198,7 @@ describe('RetryFailedNcmecDecisionsJob', () => { }); it('passes isTest=false to submitReport when NCMEC_ENV=production', async () => { - process.env.NCMEC_ENV = 'production'; + env.set('NCMEC_ENV', 'production'); const deps = makeDeps({ decisions: [makeNcmecDecisionRow(ORG_ID)], }); @@ -223,7 +220,7 @@ describe('RetryFailedNcmecDecisionsJob', () => { }); it('does not publish actions when NCMEC_ENV is unset (isTest=true)', async () => { - delete process.env.NCMEC_ENV; + env.set('NCMEC_ENV', undefined); const deps = makeDeps({ decisions: [makeNcmecDecisionRow(ORG_ID)], // Simulate an org that has actions configured to run on NCMEC report @@ -251,7 +248,7 @@ describe('RetryFailedNcmecDecisionsJob', () => { }); it('publishes actions when NCMEC_ENV=production (isTest=false)', async () => { - process.env.NCMEC_ENV = 'production'; + env.set('NCMEC_ENV', 'production'); const deps = makeDeps({ decisions: [makeNcmecDecisionRow(ORG_ID)], getNCMECActionsToRunAndPolicies: jest.fn(async () => ({ diff --git a/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts b/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts index 0cdb7d232..bd9f2ae62 100644 --- a/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts +++ b/server/workers_jobs/RetryFailedNcmecDecisionsJob.ts @@ -1,3 +1,4 @@ +import ncmecConfig from '#config/ncmec'; import _ from 'lodash'; import { v1 as uuidv1 } from 'uuid'; @@ -118,10 +119,9 @@ export default inject( getItemTypeEventuallyConsistent, }); submitReportInvoked = true; - const isTest = process.env.NCMEC_ENV !== 'production'; const reportResult = await ncmecService.submitReport( reportParams, - isTest, + ncmecConfig.isTest, ); if ( reportResult === 'UNSUPPORTED_ORG' || @@ -135,7 +135,7 @@ export default inject( if ( actionAndPolicy != null && actionAndPolicy.actionsToRunIds != null && - !isTest + !ncmecConfig.isTest ) { const actions = await moderationConfigService.getActions({ orgId,