Mirror of https://github.com/roostorg/coop github.com/roostorg/coop
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

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 <juansmrad@gmail.com>

authored by

Caleb McQuaid
Juan S. Mrad
and committed by
GitHub
9225ac7d 94375bb0

+334 -9
+79
client/src/rechartsScaleWrapper.js
··· 1 + /** 2 + * Wrapper around `recharts-scale/es6/getNiceTickValues` that recovers from 3 + * `[DecimalError] Division by zero` errors thrown by upstream when a chart 4 + * receives a degenerate domain (e.g., all values identical, or non-numeric). 5 + * 6 + * Aliased via `resolve.alias` in `client/vite.config.ts`, so any `recharts` 7 + * import of `recharts-scale/es6/getNiceTickValues` resolves to this module 8 + * instead of the upstream implementation. We use Vite's alias — not craco — 9 + * because this app is built with Vite, not Create React App. 10 + */ 11 + const actual = require('recharts-scale/es6/getNiceTickValues'); 12 + 13 + const DEFAULT_TICK_COUNT = 5; 14 + 15 + /** 16 + * Build a plain linear set of ticks as a last resort when upstream can't 17 + * compute one. Safe for any `tickCount >= 1` (returns a single tick when 18 + * `tickCount === 1`, avoiding its own division-by-zero). 19 + */ 20 + function fallbackTicks(domain, tickCount) { 21 + const count = 22 + Number.isInteger(tickCount) && tickCount > 0 ? tickCount : DEFAULT_TICK_COUNT; 23 + const min = typeof domain[0] === 'number' ? domain[0] : 0; 24 + const max = typeof domain[1] === 'number' && domain[1] > min ? domain[1] : min + 1; 25 + if (count === 1) return [min]; 26 + const step = (max - min) / (count - 1); 27 + const ticks = new Array(count); 28 + for (let i = 0; i < count; i++) { 29 + ticks[i] = min + step * i; 30 + } 31 + return ticks; 32 + } 33 + 34 + function isDivisionByZeroError(err) { 35 + // `recharts-scale` uses `decimal.js` under the hood, which tags the error 36 + // with `name === 'DecimalError'`. Fall back to a message-substring check 37 + // for older versions that don't set `name`. 38 + return ( 39 + (err && err.name === 'DecimalError') || 40 + (err && typeof err.message === 'string' && err.message.includes('Division by zero')) 41 + ); 42 + } 43 + 44 + function safelyCall(fn, fnName, domain, tickCount, allowDecimals) { 45 + try { 46 + return fn(domain, tickCount, allowDecimals); 47 + } catch (err) { 48 + if (!isDivisionByZeroError(err)) throw err; 49 + // eslint-disable-next-line no-console 50 + console.warn( 51 + `[rechartsScaleWrapper] ${fnName} threw DecimalError; using linear fallback.`, 52 + { domain, tickCount }, 53 + ); 54 + return fallbackTicks(domain, tickCount); 55 + } 56 + } 57 + 58 + function getNiceTickValues(domain, tickCount, allowDecimals) { 59 + return safelyCall( 60 + actual.getNiceTickValues, 61 + 'getNiceTickValues', 62 + domain, 63 + tickCount, 64 + allowDecimals, 65 + ); 66 + } 67 + 68 + function getTickValuesFixedDomain(domain, tickCount, allowDecimals) { 69 + return safelyCall( 70 + actual.getTickValuesFixedDomain, 71 + 'getTickValuesFixedDomain', 72 + domain, 73 + tickCount, 74 + allowDecimals, 75 + ); 76 + } 77 + 78 + exports.getNiceTickValues = getNiceTickValues; 79 + exports.getTickValuesFixedDomain = getTickValuesFixedDomain;
+7
client/vite.config.ts
··· 15 15 resolve: { 16 16 alias: { 17 17 '@': path.resolve(__dirname, './src'), 18 + // Redirect `recharts-scale/es6/getNiceTickValues` through our wrapper so 19 + // we can recover from upstream's DecimalError "Division by zero" on 20 + // degenerate chart domains. See `src/rechartsScaleWrapper.js`. 21 + 'recharts-scale/es6/getNiceTickValues': path.resolve( 22 + __dirname, 23 + './src/rechartsScaleWrapper.js', 24 + ), 18 25 }, 19 26 }, 20 27 build: {
+21 -4
server/api.ts
··· 33 33 import typeDefs from './graphql/schema.js'; 34 34 import { authSchemaWrapper } from './graphql/utils/authorization.js'; 35 35 import { type Dependencies } from './iocContainer/index.js'; 36 - import { safeGetEnvInt } from './iocContainer/utils.js'; 36 + import { isEnvTrue, safeGetEnvInt } from './iocContainer/utils.js'; 37 37 import controllers from './routes/index.js'; 38 + import { createBodySchemaValidator } from './utils/bodySchemaValidation.js'; 38 39 import { jsonStringify } from './utils/encoding.js'; 39 40 import { 40 41 ErrorType, ··· 134 135 DATABASE_PASSWORD, 135 136 } = process.env; 136 137 137 - const connectionString = `postgres://${DATABASE_USER}:${DATABASE_PASSWORD}@${DATABASE_HOST}:${DATABASE_PORT}/${DATABASE_NAME}`; 138 + const conObject = { 139 + host: DATABASE_HOST, 140 + port: Number(DATABASE_PORT), 141 + user: DATABASE_USER, 142 + password: DATABASE_PASSWORD, 143 + database: DATABASE_NAME, 144 + // NB: `rejectUnauthorized: false` keeps the connection encrypted but skips 145 + // certificate validation. 146 + ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, 147 + }; 138 148 139 149 app.use( 140 150 session({ 141 151 secret: process.env.SESSION_SECRET!, 142 - store: new sessionStore({ conString: connectionString }), 152 + store: new sessionStore({ conObject }), 143 153 cookie: { 144 154 secure: process.env.NODE_ENV === 'production', 145 155 httpOnly: true, ··· 432 442 Object.entries(controllers).forEach(([_k, controller]) => { 433 443 controller.routes.forEach((it) => { 434 444 const handler = it.handler(deps); 445 + const handlers = Array.isArray(handler) ? handler : [handler]; 446 + // If the route declares a bodySchema, validate the request body against 447 + // it before any handler runs. Routes without a schema (e.g., GETs) skip 448 + // validation entirely. 449 + const middlewares = it.bodySchema 450 + ? [createBodySchemaValidator(it.bodySchema), ...handlers] 451 + : handlers; 435 452 app[it.method]( 436 453 path.join(controller.pathPrefix, it.path), 437 - ...(Array.isArray(handler) ? handler : [handler]), 454 + ...middlewares, 438 455 ); 439 456 }); 440 457 });
+25 -4
server/iocContainer/index.ts
··· 244 244 } from '../utils/typescript-types.js'; 245 245 import { registerGqlDataSources } from './services/gqlDataSources.js'; 246 246 import { registerWorkersAndJobs } from './services/workersAndJobs.js'; 247 - import { register, safeGetEnvVar } from './utils.js'; 247 + import { isEnvTrue, register, safeGetEnvVar } from './utils.js'; 248 248 249 249 // the otel instrumentation currently intercepts require statements. support for 250 250 // esm support is experimental so we should wait until it is stable ··· 456 456 max: 30, 457 457 application_name: 458 458 getEnvVarOrWarn('OTEL_SERVICE_NAME') ?? 'unknown-coop-service', 459 + ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, 459 460 }); 460 461 461 462 const bottle = new Bottle<Dependencies>(); ··· 525 526 maxRetriesPerRequest: null, 526 527 port: parseInt(process.env.REDIS_PORT ?? '6379'), 527 528 host: safeGetEnvVar('REDIS_HOST'), 529 + ...(isEnvTrue('REDIS_TLS') 530 + ? { tls: { servername: safeGetEnvVar('REDIS_HOST') } } 531 + : {}), 528 532 }), 529 533 ); 530 534 ··· 675 679 // keyspace aware and it's very annoying and likely error prone to be 676 680 // switching keyspaces with `USE KEYSPACE` all the time. 677 681 bottle.factory('Scylla', () => { 682 + const contactPoints = safeGetEnvVar('SCYLLA_HOSTS') 683 + .split(',') 684 + .map((it) => it.trim()) 685 + .filter((it) => it.length > 0); 686 + // For TLS hostname verification we need an SNI value that matches the 687 + // server cert. Prefer an explicit `SCYLLA_SSL_SERVERNAME` (e.g., the 688 + // Keyspaces regional endpoint) over inferring one from `SCYLLA_HOSTS`, 689 + // which may contain multiple contact points with different cert names. 690 + const sslServerName = 691 + process.env.SCYLLA_SSL_SERVERNAME ?? contactPoints[0]; 678 692 const scyllaDriver = new ScyllaClient({ 679 - contactPoints: safeGetEnvVar('SCYLLA_HOSTS') 680 - .split(',') 681 - .map((it) => it.trim()), 693 + contactPoints, 682 694 credentials: { 683 695 username: safeGetEnvVar('SCYLLA_USERNAME'), 684 696 password: safeGetEnvVar('SCYLLA_PASSWORD'), 685 697 }, 686 698 localDataCenter: safeGetEnvVar('SCYLLA_LOCAL_DATACENTER'), 687 699 keyspace: 'item_investigation_service', 700 + protocolOptions: { 701 + port: parseInt(process.env.SCYLLA_PORT ?? '9042'), 702 + }, 703 + sslOptions: isEnvTrue('SCYLLA_SSL') 704 + ? { 705 + host: sslServerName, 706 + rejectUnauthorized: true, 707 + } 708 + : undefined, 688 709 pooling: { 689 710 coreConnectionsPerHost: { 690 711 [scyllaTypes.distance.local]: 3,
+11
server/iocContainer/utils.ts
··· 664 664 } 665 665 666 666 /** 667 + * Returns true when the env var is set to a truthy value. Accepts `true`, `1`, 668 + * and `yes` (case-insensitive) so callers don't have to worry about casing or 669 + * common aliases. Any other value (including unset) returns false. 670 + */ 671 + export function isEnvTrue(varName: string): boolean { 672 + const raw = process.env[varName]; 673 + if (raw == null) return false; 674 + return ['true', '1', 'yes'].includes(raw.trim().toLowerCase()); 675 + } 676 + 677 + /** 667 678 * Gets an env var and parses it as a positive integer. Returns `defaultValue` 668 679 * if the variable is unset or invalid, logging an error on misconfiguration. 669 680 */
+2 -1
server/models/sequelizeSetup.ts
··· 8 8 import clsHooked from 'cls-hooked'; 9 9 import pkg, { type Transaction, type TransactionOptions } from 'sequelize'; 10 10 11 + import { isEnvTrue } from '../iocContainer/utils.js'; 11 12 import { safeGet } from '../utils/misc.js'; 12 13 13 14 const { Sequelize } = pkg; ··· 58 59 // Think about how/if we'll do this w/ our kysely connection pools. 59 60 }, 60 61 dialectOptions: { 61 - //ssl: true, 62 + ssl: isEnvTrue('DATABASE_SSL') ? { rejectUnauthorized: false } : undefined, 62 63 query_timeout: 1_000_000, 63 64 idle_in_transaction_session_timeout: 300_000, 64 65 },
+107
server/utils/bodySchemaValidation.test.ts
··· 1 + import type { Request, Response } from 'express'; 2 + 3 + import { createBodySchemaValidator } from './bodySchemaValidation.js'; 4 + import { CoopError } from './errors.js'; 5 + 6 + const schema: Record<string, unknown> = { 7 + $schema: 'http://json-schema.org/draft-04/schema#', 8 + type: 'object', 9 + properties: { 10 + name: { type: 'string' }, 11 + count: { type: 'integer' }, 12 + }, 13 + required: ['name'], 14 + additionalProperties: false, 15 + }; 16 + 17 + function invoke( 18 + middleware: ReturnType<typeof createBodySchemaValidator>, 19 + body: unknown, 20 + ) { 21 + const req: Partial<Request> = { body }; 22 + const res: Partial<Response> = {}; 23 + const next = jest.fn(); 24 + middleware(req as Request, res as Response, next); 25 + return { next }; 26 + } 27 + 28 + function firstNextArg(next: ReturnType<typeof jest.fn>): unknown { 29 + return next.mock.calls[0]?.[0]; 30 + } 31 + 32 + describe('createBodySchemaValidator', () => { 33 + test('passes valid bodies through to next()', () => { 34 + const middleware = createBodySchemaValidator(schema); 35 + const { next } = invoke(middleware, { name: 'ok', count: 3 }); 36 + 37 + expect(next).toHaveBeenCalledTimes(1); 38 + expect(next).toHaveBeenCalledWith(); 39 + }); 40 + 41 + test('allows optional fields to be omitted', () => { 42 + const middleware = createBodySchemaValidator(schema); 43 + const { next } = invoke(middleware, { name: 'ok' }); 44 + 45 + expect(next).toHaveBeenCalledTimes(1); 46 + expect(next).toHaveBeenCalledWith(); 47 + }); 48 + 49 + test('forwards a BadRequestError when a required field is missing', () => { 50 + const middleware = createBodySchemaValidator(schema); 51 + const { next } = invoke(middleware, { count: 3 }); 52 + 53 + expect(next).toHaveBeenCalledTimes(1); 54 + const err = firstNextArg(next); 55 + expect(err).toBeInstanceOf(CoopError); 56 + expect(err).toMatchObject({ 57 + name: 'BadRequestError', 58 + status: 400, 59 + title: 'Request body failed schema validation.', 60 + }); 61 + // Error message should reference the missing field, not crash. 62 + expect((err as CoopError).detail).toContain('name'); 63 + }); 64 + 65 + test('forwards a BadRequestError when a field has the wrong type', () => { 66 + const middleware = createBodySchemaValidator(schema); 67 + const { next } = invoke(middleware, { name: 'ok', count: 'not-a-number' }); 68 + 69 + expect(next).toHaveBeenCalledTimes(1); 70 + const err = firstNextArg(next); 71 + expect(err).toBeInstanceOf(CoopError); 72 + expect(err).toMatchObject({ 73 + name: 'BadRequestError', 74 + status: 400, 75 + pointer: '/count', 76 + }); 77 + }); 78 + 79 + test('rejects unknown additional properties when the schema forbids them', () => { 80 + const middleware = createBodySchemaValidator(schema); 81 + const { next } = invoke(middleware, { name: 'ok', surprise: true }); 82 + 83 + expect(next).toHaveBeenCalledTimes(1); 84 + const err = firstNextArg(next); 85 + expect(err).toBeInstanceOf(CoopError); 86 + expect(err).toMatchObject({ name: 'BadRequestError', status: 400 }); 87 + }); 88 + 89 + test('rejects non-object bodies (e.g., undefined from a request with no body)', () => { 90 + const middleware = createBodySchemaValidator(schema); 91 + const { next } = invoke(middleware, undefined); 92 + 93 + expect(next).toHaveBeenCalledTimes(1); 94 + const err = firstNextArg(next); 95 + expect(err).toBeInstanceOf(CoopError); 96 + expect(err).toMatchObject({ name: 'BadRequestError', status: 400 }); 97 + }); 98 + 99 + test('does not leak Ajv internals (schemaPath / params) in the error detail', () => { 100 + const middleware = createBodySchemaValidator(schema); 101 + const { next } = invoke(middleware, { name: 42 }); 102 + 103 + const err = firstNextArg(next) as CoopError; 104 + expect(err.detail ?? '').not.toContain('schemaPath'); 105 + expect(err.detail ?? '').not.toContain('params'); 106 + }); 107 + });
+82
server/utils/bodySchemaValidation.ts
··· 1 + import type { RequestHandler } from 'express'; 2 + import _Ajv, { type ErrorObject } from 'ajv-draft-04'; 3 + 4 + import { makeBadRequestError } from './errors.js'; 5 + 6 + // `ajv-draft-04` is a CJS module; under our `"module": "NodeNext"` ESM setup 7 + // the real constructor ends up at `_Ajv.default`. This matches the pattern 8 + // used elsewhere in this codebase (see `services/ncmecService/ncmecService.ts` 9 + // and `services/partialItemsService/partialItemsService.ts`). 10 + const Ajv = _Ajv as unknown as typeof _Ajv.default; 11 + 12 + // Module-level singleton: Ajv internally caches compiled schemas by reference, 13 + // and `ajv.compile` is idempotent per schema object. 14 + const ajv = new Ajv({ 15 + // Report every validation error, not just the first one, so the response 16 + // tells the caller about all their mistakes at once. 17 + allErrors: true, 18 + // Fail loudly if a schema uses keywords Ajv doesn't understand — that's 19 + // almost always a bug in the route definition rather than something we 20 + // want to silently ignore at runtime. 21 + strictSchema: true, 22 + }); 23 + 24 + /** 25 + * Build an Express middleware that validates `req.body` against `schema`. 26 + * 27 + * On failure the middleware forwards a `BadRequestError` (a `CoopError`) to the 28 + * standard Express error handler, which serializes it into the project's 29 + * canonical `{ errors: [...] }` shape. We intentionally do NOT echo the raw 30 + * Ajv error objects back to the client: those include `params`, `schemaPath`, 31 + * and sometimes slices of the request body, which can leak implementation 32 + * details or user-supplied data back in the response. 33 + */ 34 + export function createBodySchemaValidator( 35 + // We intentionally take a permissive schema type here. The `Route.bodySchema` 36 + // field is already typed against its specific `ReqBody` at route-definition 37 + // sites; this middleware only forwards the schema to Ajv at runtime, which 38 + // doesn't care about the TS-side body type. 39 + schema: Record<string, unknown>, 40 + ): RequestHandler { 41 + const validate = ajv.compile(schema); 42 + 43 + return (req, _res, next) => { 44 + if (validate(req.body)) { 45 + next(); 46 + return; 47 + } 48 + 49 + const errors = validate.errors ?? []; 50 + next( 51 + makeBadRequestError('Request body failed schema validation.', { 52 + shouldErrorSpan: false, 53 + pointer: errors[0] && toJsonPointer(errors[0]), 54 + detail: formatErrors(errors), 55 + }), 56 + ); 57 + }; 58 + } 59 + 60 + /** 61 + * Ajv's `instancePath` is already a JSON Pointer (e.g. `/items/0/name`), so we 62 + * just normalise an empty string (root) to `undefined` and return it. 63 + */ 64 + function toJsonPointer(err: ErrorObject): string | undefined { 65 + return err.instancePath.length > 0 ? err.instancePath : undefined; 66 + } 67 + 68 + /** 69 + * Build a short human-readable summary of the validation errors. We only 70 + * include Ajv's own `message` (which is derived from the schema, not the 71 + * request body) and the JSON Pointer into the request. No `params`, 72 + * `schemaPath`, or raw input data are exposed. 73 + */ 74 + function formatErrors(errors: readonly ErrorObject[]): string { 75 + if (errors.length === 0) return 'Unknown validation error.'; 76 + return errors 77 + .map((err) => { 78 + const loc = err.instancePath || '/'; 79 + return `${loc}: ${err.message ?? 'invalid value'}`; 80 + }) 81 + .join('; '); 82 + }