From 9225ac7d8eb792a8f087fc83e045af476fae0f0a Mon Sep 17 00:00:00 2001 From: Caleb McQuaid Date: Tue, 28 Apr 2026 15:30:45 -0400 Subject: [PATCH] Add ssl tls support, recharts patch, body schema validation (#326) * feat: add SSL/TLS support for managed AWS services * fix: add recharts-scale wrapper to prevent division-by-zero errors * feat: add request body schema validation middleware * updates to align code review --------- Co-authored-by: Juan S. Mrad --- client/src/rechartsScaleWrapper.js | 79 ++++++++++++++++ client/vite.config.ts | 7 ++ server/api.ts | 25 ++++- server/iocContainer/index.ts | 29 +++++- server/iocContainer/utils.ts | 11 +++ server/models/sequelizeSetup.ts | 3 +- server/utils/bodySchemaValidation.test.ts | 107 ++++++++++++++++++++++ server/utils/bodySchemaValidation.ts | 82 +++++++++++++++++ 8 files changed, 334 insertions(+), 9 deletions(-) create mode 100644 client/src/rechartsScaleWrapper.js create mode 100644 server/utils/bodySchemaValidation.test.ts create mode 100644 server/utils/bodySchemaValidation.ts diff --git a/client/src/rechartsScaleWrapper.js b/client/src/rechartsScaleWrapper.js new file mode 100644 index 0000000..18de58f --- /dev/null +++ b/client/src/rechartsScaleWrapper.js @@ -0,0 +1,79 @@ +/** + * Wrapper around `recharts-scale/es6/getNiceTickValues` that recovers from + * `[DecimalError] Division by zero` errors thrown by upstream when a chart + * receives a degenerate domain (e.g., all values identical, or non-numeric). + * + * Aliased via `resolve.alias` in `client/vite.config.ts`, so any `recharts` + * import of `recharts-scale/es6/getNiceTickValues` resolves to this module + * instead of the upstream implementation. We use Vite's alias — not craco — + * because this app is built with Vite, not Create React App. + */ +const actual = require('recharts-scale/es6/getNiceTickValues'); + +const DEFAULT_TICK_COUNT = 5; + +/** + * Build a plain linear set of ticks as a last resort when upstream can't + * compute one. Safe for any `tickCount >= 1` (returns a single tick when + * `tickCount === 1`, avoiding its own division-by-zero). + */ +function fallbackTicks(domain, tickCount) { + const count = + Number.isInteger(tickCount) && tickCount > 0 ? tickCount : DEFAULT_TICK_COUNT; + const min = typeof domain[0] === 'number' ? domain[0] : 0; + const max = typeof domain[1] === 'number' && domain[1] > min ? domain[1] : min + 1; + if (count === 1) return [min]; + const step = (max - min) / (count - 1); + const ticks = new Array(count); + for (let i = 0; i < count; i++) { + ticks[i] = min + step * i; + } + return ticks; +} + +function isDivisionByZeroError(err) { + // `recharts-scale` uses `decimal.js` under the hood, which tags the error + // with `name === 'DecimalError'`. Fall back to a message-substring check + // for older versions that don't set `name`. + return ( + (err && err.name === 'DecimalError') || + (err && typeof err.message === 'string' && err.message.includes('Division by zero')) + ); +} + +function safelyCall(fn, fnName, domain, tickCount, allowDecimals) { + try { + return fn(domain, tickCount, allowDecimals); + } catch (err) { + if (!isDivisionByZeroError(err)) throw err; + // eslint-disable-next-line no-console + console.warn( + `[rechartsScaleWrapper] ${fnName} threw DecimalError; using linear fallback.`, + { domain, tickCount }, + ); + return fallbackTicks(domain, tickCount); + } +} + +function getNiceTickValues(domain, tickCount, allowDecimals) { + return safelyCall( + actual.getNiceTickValues, + 'getNiceTickValues', + domain, + tickCount, + allowDecimals, + ); +} + +function getTickValuesFixedDomain(domain, tickCount, allowDecimals) { + return safelyCall( + actual.getTickValuesFixedDomain, + 'getTickValuesFixedDomain', + domain, + tickCount, + allowDecimals, + ); +} + +exports.getNiceTickValues = getNiceTickValues; +exports.getTickValuesFixedDomain = getTickValuesFixedDomain; diff --git a/client/vite.config.ts b/client/vite.config.ts index 9595a17..c3a1ee8 100644 --- a/client/vite.config.ts +++ b/client/vite.config.ts @@ -15,6 +15,13 @@ export default defineConfig({ resolve: { alias: { '@': path.resolve(__dirname, './src'), + // Redirect `recharts-scale/es6/getNiceTickValues` through our wrapper so + // we can recover from upstream's DecimalError "Division by zero" on + // degenerate chart domains. See `src/rechartsScaleWrapper.js`. + 'recharts-scale/es6/getNiceTickValues': path.resolve( + __dirname, + './src/rechartsScaleWrapper.js', + ), }, }, build: { diff --git a/server/api.ts b/server/api.ts index 2758884..bd77c6e 100644 --- a/server/api.ts +++ b/server/api.ts @@ -33,8 +33,9 @@ import resolvers, { type Context } from './graphql/resolvers.js'; import typeDefs from './graphql/schema.js'; import { authSchemaWrapper } from './graphql/utils/authorization.js'; import { type Dependencies } from './iocContainer/index.js'; -import { safeGetEnvInt } from './iocContainer/utils.js'; +import { isEnvTrue, safeGetEnvInt } from './iocContainer/utils.js'; import controllers from './routes/index.js'; +import { createBodySchemaValidator } from './utils/bodySchemaValidation.js'; import { jsonStringify } from './utils/encoding.js'; import { ErrorType, @@ -134,12 +135,21 @@ export default async function makeApiServer(deps: Dependencies) { DATABASE_PASSWORD, } = process.env; - const connectionString = `postgres://${DATABASE_USER}:${DATABASE_PASSWORD}@${DATABASE_HOST}:${DATABASE_PORT}/${DATABASE_NAME}`; + const conObject = { + host: DATABASE_HOST, + port: Number(DATABASE_PORT), + user: DATABASE_USER, + password: DATABASE_PASSWORD, + database: DATABASE_NAME, + // NB: `rejectUnauthorized: false` keeps the connection encrypted but skips + // certificate validation. + ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, + }; app.use( session({ secret: process.env.SESSION_SECRET!, - store: new sessionStore({ conString: connectionString }), + store: new sessionStore({ conObject }), cookie: { secure: process.env.NODE_ENV === 'production', httpOnly: true, @@ -432,9 +442,16 @@ export default async function makeApiServer(deps: Dependencies) { Object.entries(controllers).forEach(([_k, controller]) => { controller.routes.forEach((it) => { const handler = it.handler(deps); + const handlers = Array.isArray(handler) ? handler : [handler]; + // If the route declares a bodySchema, validate the request body against + // it before any handler runs. Routes without a schema (e.g., GETs) skip + // validation entirely. + const middlewares = it.bodySchema + ? [createBodySchemaValidator(it.bodySchema), ...handlers] + : handlers; app[it.method]( path.join(controller.pathPrefix, it.path), - ...(Array.isArray(handler) ? handler : [handler]), + ...middlewares, ); }); }); diff --git a/server/iocContainer/index.ts b/server/iocContainer/index.ts index 0035885..a8f1e04 100644 --- a/server/iocContainer/index.ts +++ b/server/iocContainer/index.ts @@ -244,7 +244,7 @@ import { } from '../utils/typescript-types.js'; import { registerGqlDataSources } from './services/gqlDataSources.js'; import { registerWorkersAndJobs } from './services/workersAndJobs.js'; -import { register, safeGetEnvVar } from './utils.js'; +import { isEnvTrue, register, 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 @@ -456,6 +456,7 @@ export default async function getBottle() { max: 30, application_name: getEnvVarOrWarn('OTEL_SERVICE_NAME') ?? 'unknown-coop-service', + ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, }); const bottle = new Bottle(); @@ -525,6 +526,9 @@ export default async function getBottle() { maxRetriesPerRequest: null, port: parseInt(process.env.REDIS_PORT ?? '6379'), host: safeGetEnvVar('REDIS_HOST'), + ...(isEnvTrue('REDIS_TLS') + ? { tls: { servername: safeGetEnvVar('REDIS_HOST') } } + : {}), }), ); @@ -675,16 +679,33 @@ export default async function getBottle() { // keyspace aware and it's very annoying and likely error prone to be // switching keyspaces with `USE KEYSPACE` all the time. bottle.factory('Scylla', () => { + 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: safeGetEnvVar('SCYLLA_HOSTS') - .split(',') - .map((it) => it.trim()), + 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, diff --git a/server/iocContainer/utils.ts b/server/iocContainer/utils.ts index c667c78..f41e045 100644 --- a/server/iocContainer/utils.ts +++ b/server/iocContainer/utils.ts @@ -663,6 +663,17 @@ export function safeGetEnvVar(varName: string): string { ); } +/** + * 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. diff --git a/server/models/sequelizeSetup.ts b/server/models/sequelizeSetup.ts index 36ab69a..8cdf25a 100644 --- a/server/models/sequelizeSetup.ts +++ b/server/models/sequelizeSetup.ts @@ -8,6 +8,7 @@ import clsHooked from 'cls-hooked'; import pkg, { type Transaction, type TransactionOptions } from 'sequelize'; +import { isEnvTrue } from '../iocContainer/utils.js'; import { safeGet } from '../utils/misc.js'; const { Sequelize } = pkg; @@ -58,7 +59,7 @@ export const makeSequelize = () => // Think about how/if we'll do this w/ our kysely connection pools. }, dialectOptions: { - //ssl: true, + ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, query_timeout: 1_000_000, idle_in_transaction_session_timeout: 300_000, }, diff --git a/server/utils/bodySchemaValidation.test.ts b/server/utils/bodySchemaValidation.test.ts new file mode 100644 index 0000000..dcf7341 --- /dev/null +++ b/server/utils/bodySchemaValidation.test.ts @@ -0,0 +1,107 @@ +import type { Request, Response } from 'express'; + +import { createBodySchemaValidator } from './bodySchemaValidation.js'; +import { CoopError } from './errors.js'; + +const schema: Record = { + $schema: 'http://json-schema.org/draft-04/schema#', + type: 'object', + properties: { + name: { type: 'string' }, + count: { type: 'integer' }, + }, + required: ['name'], + additionalProperties: false, +}; + +function invoke( + middleware: ReturnType, + body: unknown, +) { + const req: Partial = { body }; + const res: Partial = {}; + const next = jest.fn(); + middleware(req as Request, res as Response, next); + return { next }; +} + +function firstNextArg(next: ReturnType): unknown { + return next.mock.calls[0]?.[0]; +} + +describe('createBodySchemaValidator', () => { + test('passes valid bodies through to next()', () => { + const middleware = createBodySchemaValidator(schema); + const { next } = invoke(middleware, { name: 'ok', count: 3 }); + + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(); + }); + + test('allows optional fields to be omitted', () => { + const middleware = createBodySchemaValidator(schema); + const { next } = invoke(middleware, { name: 'ok' }); + + expect(next).toHaveBeenCalledTimes(1); + expect(next).toHaveBeenCalledWith(); + }); + + test('forwards a BadRequestError when a required field is missing', () => { + const middleware = createBodySchemaValidator(schema); + const { next } = invoke(middleware, { count: 3 }); + + expect(next).toHaveBeenCalledTimes(1); + const err = firstNextArg(next); + expect(err).toBeInstanceOf(CoopError); + expect(err).toMatchObject({ + name: 'BadRequestError', + status: 400, + title: 'Request body failed schema validation.', + }); + // Error message should reference the missing field, not crash. + expect((err as CoopError).detail).toContain('name'); + }); + + test('forwards a BadRequestError when a field has the wrong type', () => { + const middleware = createBodySchemaValidator(schema); + const { next } = invoke(middleware, { name: 'ok', count: 'not-a-number' }); + + expect(next).toHaveBeenCalledTimes(1); + const err = firstNextArg(next); + expect(err).toBeInstanceOf(CoopError); + expect(err).toMatchObject({ + name: 'BadRequestError', + status: 400, + pointer: '/count', + }); + }); + + test('rejects unknown additional properties when the schema forbids them', () => { + const middleware = createBodySchemaValidator(schema); + const { next } = invoke(middleware, { name: 'ok', surprise: true }); + + expect(next).toHaveBeenCalledTimes(1); + const err = firstNextArg(next); + expect(err).toBeInstanceOf(CoopError); + expect(err).toMatchObject({ name: 'BadRequestError', status: 400 }); + }); + + test('rejects non-object bodies (e.g., undefined from a request with no body)', () => { + const middleware = createBodySchemaValidator(schema); + const { next } = invoke(middleware, undefined); + + expect(next).toHaveBeenCalledTimes(1); + const err = firstNextArg(next); + expect(err).toBeInstanceOf(CoopError); + expect(err).toMatchObject({ name: 'BadRequestError', status: 400 }); + }); + + test('does not leak Ajv internals (schemaPath / params) in the error detail', () => { + const middleware = createBodySchemaValidator(schema); + const { next } = invoke(middleware, { name: 42 }); + + const err = firstNextArg(next) as CoopError; + expect(err.detail ?? '').not.toContain('schemaPath'); + expect(err.detail ?? '').not.toContain('params'); + }); +}); diff --git a/server/utils/bodySchemaValidation.ts b/server/utils/bodySchemaValidation.ts new file mode 100644 index 0000000..ad7bd13 --- /dev/null +++ b/server/utils/bodySchemaValidation.ts @@ -0,0 +1,82 @@ +import type { RequestHandler } from 'express'; +import _Ajv, { type ErrorObject } from 'ajv-draft-04'; + +import { makeBadRequestError } from './errors.js'; + +// `ajv-draft-04` is a CJS module; under our `"module": "NodeNext"` ESM setup +// the real constructor ends up at `_Ajv.default`. This matches the pattern +// used elsewhere in this codebase (see `services/ncmecService/ncmecService.ts` +// and `services/partialItemsService/partialItemsService.ts`). +const Ajv = _Ajv as unknown as typeof _Ajv.default; + +// Module-level singleton: Ajv internally caches compiled schemas by reference, +// and `ajv.compile` is idempotent per schema object. +const ajv = new Ajv({ + // Report every validation error, not just the first one, so the response + // tells the caller about all their mistakes at once. + allErrors: true, + // Fail loudly if a schema uses keywords Ajv doesn't understand — that's + // almost always a bug in the route definition rather than something we + // want to silently ignore at runtime. + strictSchema: true, +}); + +/** + * Build an Express middleware that validates `req.body` against `schema`. + * + * On failure the middleware forwards a `BadRequestError` (a `CoopError`) to the + * standard Express error handler, which serializes it into the project's + * canonical `{ errors: [...] }` shape. We intentionally do NOT echo the raw + * Ajv error objects back to the client: those include `params`, `schemaPath`, + * and sometimes slices of the request body, which can leak implementation + * details or user-supplied data back in the response. + */ +export function createBodySchemaValidator( + // We intentionally take a permissive schema type here. The `Route.bodySchema` + // field is already typed against its specific `ReqBody` at route-definition + // sites; this middleware only forwards the schema to Ajv at runtime, which + // doesn't care about the TS-side body type. + schema: Record, +): RequestHandler { + const validate = ajv.compile(schema); + + return (req, _res, next) => { + if (validate(req.body)) { + next(); + return; + } + + const errors = validate.errors ?? []; + next( + makeBadRequestError('Request body failed schema validation.', { + shouldErrorSpan: false, + pointer: errors[0] && toJsonPointer(errors[0]), + detail: formatErrors(errors), + }), + ); + }; +} + +/** + * Ajv's `instancePath` is already a JSON Pointer (e.g. `/items/0/name`), so we + * just normalise an empty string (root) to `undefined` and return it. + */ +function toJsonPointer(err: ErrorObject): string | undefined { + return err.instancePath.length > 0 ? err.instancePath : undefined; +} + +/** + * Build a short human-readable summary of the validation errors. We only + * include Ajv's own `message` (which is derived from the schema, not the + * request body) and the JSON Pointer into the request. No `params`, + * `schemaPath`, or raw input data are exposed. + */ +function formatErrors(errors: readonly ErrorObject[]): string { + if (errors.length === 0) return 'Unknown validation error.'; + return errors + .map((err) => { + const loc = err.instancePath || '/'; + return `${loc}: ${err.message ?? 'invalid value'}`; + }) + .join('; '); +} -- 2.51.2