From f68532ecb29d466b14641b9598ba31b773545959 Mon Sep 17 00:00:00 2001 From: Will Schurman Date: Thu, 26 Feb 2026 22:12:20 -0800 Subject: [PATCH] fix: fix some security issues --- ...uthorizationResultBasedKnexEntityLoader.ts | 3 +- .../src/PostgresEntityDatabaseAdapter.ts | 3 +- .../src/SQLOperator.ts | 39 +++++++++++++++---- .../PostgresEntityIntegration-test.ts | 4 +- 4 files changed, 36 insertions(+), 13 deletions(-) diff --git a/packages/entity-database-adapter-knex/src/AuthorizationResultBasedKnexEntityLoader.ts b/packages/entity-database-adapter-knex/src/AuthorizationResultBasedKnexEntityLoader.ts index 4c4176b80..6d00840ba 100644 --- a/packages/entity-database-adapter-knex/src/AuthorizationResultBasedKnexEntityLoader.ts +++ b/packages/entity-database-adapter-knex/src/AuthorizationResultBasedKnexEntityLoader.ts @@ -95,8 +95,7 @@ export type EntityLoaderFieldNameConstructorFn< /** * Specification for a search field that is a manually constructed SQLFragment. Useful for complex search fields that require - * transformations or combinations of multiple fields, such as `COALESCE(NULLIF(display_name, ''), split_part(full_name, '/', 2))` - * to search by display name with fallback to full name. + * transformations to make nullable fields non-null or to make combinations of multiple fields. */ export type EntityLoaderSearchFieldSQLFragmentFnSpecification< TFields extends Record, diff --git a/packages/entity-database-adapter-knex/src/PostgresEntityDatabaseAdapter.ts b/packages/entity-database-adapter-knex/src/PostgresEntityDatabaseAdapter.ts index 454e4a77c..fde6196fc 100644 --- a/packages/entity-database-adapter-knex/src/PostgresEntityDatabaseAdapter.ts +++ b/packages/entity-database-adapter-knex/src/PostgresEntityDatabaseAdapter.ts @@ -139,11 +139,12 @@ export class PostgresEntityDatabaseAdapter< orderBySpecification.nulls, ); } else { + const orderDirection = orderBySpecification.order === 'asc' ? 'ASC' : 'DESC'; const nullsSuffix = orderBySpecification.nulls ? ` NULLS ${orderBySpecification.nulls === NullsOrdering.FIRST ? 'FIRST' : 'LAST'}` : ''; ret = ret.orderByRaw( - `(${orderBySpecification.columnFragment.sql}) ${orderBySpecification.order}${nullsSuffix}`, + `(${orderBySpecification.columnFragment.sql}) ${orderDirection}${nullsSuffix}`, orderBySpecification.columnFragment.getKnexBindings((fieldName) => getDatabaseFieldForEntityField(this.entityConfiguration, fieldName), ), diff --git a/packages/entity-database-adapter-knex/src/SQLOperator.ts b/packages/entity-database-adapter-knex/src/SQLOperator.ts index 4d74fa87e..009e886fe 100644 --- a/packages/entity-database-adapter-knex/src/SQLOperator.ts +++ b/packages/entity-database-adapter-knex/src/SQLOperator.ts @@ -112,6 +112,9 @@ export class SQLFragment> { if (match === '??' && binding.type === 'identifier') { // For identifiers, show them quoted as they would appear return `"${binding.name.replace(/"/g, '""')}"`; + } else if (match === '??' && binding.type === 'entityField') { + // For entity fields, show the entity field name as the identifier for debugging + return `"${binding.fieldName.toString()}"`; } else if (match === '?' && binding.type === 'value') { return SQLFragment.formatDebugValue(binding.value); } else { @@ -309,6 +312,18 @@ type PickSupportedSQLValueKeys = { [K in keyof T]: T[K] extends SupportedSQLValue ? K : never; }[keyof T]; +type PickStringValueKeys = { + [K in keyof T]: T[K] extends string | null | undefined ? K : never; +}[keyof T]; + +type JsonSerializable = + | string + | number + | boolean + | null + | readonly JsonSerializable[] + | { readonly [key: string]: JsonSerializable }; + /** * Common SQL helper functions for building queries */ @@ -385,7 +400,7 @@ export const SQLFragmentHelpers = { * ``` */ like>( - fieldName: keyof TFields, + fieldName: PickStringValueKeys, pattern: string, ): SQLFragment { return sql`${entityField(fieldName)} LIKE ${pattern}`; @@ -395,7 +410,7 @@ export const SQLFragmentHelpers = { * NOT LIKE helper */ notLike>( - fieldName: keyof TFields, + fieldName: PickStringValueKeys, pattern: string, ): SQLFragment { return sql`${entityField(fieldName)} NOT LIKE ${pattern}`; @@ -405,7 +420,7 @@ export const SQLFragmentHelpers = { * ILIKE helper for case-insensitive matching */ ilike>( - fieldName: keyof TFields, + fieldName: PickStringValueKeys, pattern: string, ): SQLFragment { return sql`${entityField(fieldName)} ILIKE ${pattern}`; @@ -415,7 +430,7 @@ export const SQLFragmentHelpers = { * NOT ILIKE helper for case-insensitive non-matching */ notIlike>( - fieldName: keyof TFields, + fieldName: PickStringValueKeys, pattern: string, ): SQLFragment { return sql`${entityField(fieldName)} NOT ILIKE ${pattern}`; @@ -506,9 +521,13 @@ export const SQLFragmentHelpers = { */ jsonContains>( fieldName: keyof TFields, - value: unknown, + value: JsonSerializable, ): SQLFragment { - return sql`${entityField(fieldName)} @> ${JSON.stringify(value)}::jsonb`; + const serialized = JSON.stringify(value); + if (serialized === undefined) { + throw new Error('jsonContains: value is not JSON-serializable'); + } + return sql`${entityField(fieldName)} @> ${serialized}::jsonb`; }, /** @@ -516,9 +535,13 @@ export const SQLFragmentHelpers = { */ jsonContainedBy>( fieldName: keyof TFields, - value: unknown, + value: JsonSerializable, ): SQLFragment { - return sql`${entityField(fieldName)} <@ ${JSON.stringify(value)}::jsonb`; + const serialized = JSON.stringify(value); + if (serialized === undefined) { + throw new Error('jsonContainedBy: value is not JSON-serializable'); + } + return sql`${entityField(fieldName)} <@ ${serialized}::jsonb`; }, /** diff --git a/packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts b/packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts index 291eccf6c..d6c4444e0 100644 --- a/packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts +++ b/packages/entity-database-adapter-knex/src/__integration-tests__/PostgresEntityIntegration-test.ts @@ -3668,9 +3668,9 @@ describe('postgres entity integration', () => { } const paginationSpec: PaginationSpecification = { - strategy: PaginationStrategy.TRIGRAM_SEARCH as const, + strategy: PaginationStrategy.TRIGRAM_SEARCH, term: 'Johnson', - fields: ['label' as const], + fields: ['label'], threshold: 0.2, extraOrderByFields: [ {