From a028139ad504d047764ee4bc72343671c7128692 Mon Sep 17 00:00:00 2001 From: Artem Niehrieiev Date: Wed, 5 Aug 2026 14:09:04 +0000 Subject: [PATCH] feat: enforce column-level read permissions across queries and responses --- .../pure-get-rows-from-table.use.case.ts | 38 ++++-- .../pure-read-row-from-table.use.case.ts | 4 + .../export-csv-from-table.use.case.ts | 39 ++++-- ...restrict-query-to-readable-columns.util.ts | 66 ++++++++++ ...icroservice-public-permissions-e2e.test.ts | 115 ++++++++++++++++++ ...restrict-query-to-readable-columns.test.ts | 63 ++++++++++ 6 files changed, 303 insertions(+), 22 deletions(-) create mode 100644 backend/src/entities/table/utils/restrict-query-to-readable-columns.util.ts create mode 100644 backend/test/ava-tests/non-saas-tests/non-saas-restrict-query-to-readable-columns.test.ts diff --git a/backend/src/entities/table/table-pure-crud-operations/use-cases/pure-get-rows-from-table.use.case.ts b/backend/src/entities/table/table-pure-crud-operations/use-cases/pure-get-rows-from-table.use.case.ts index 6380b431b..48ab67d42 100644 --- a/backend/src/entities/table/table-pure-crud-operations/use-cases/pure-get-rows-from-table.use.case.ts +++ b/backend/src/entities/table/table-pure-crud-operations/use-cases/pure-get-rows-from-table.use.case.ts @@ -25,6 +25,11 @@ import { findFilteringFieldsUtil, parseFilteringFieldsFromBodyData } from '../.. import { findOrderingFieldUtil } from '../../utils/find-ordering-field.util.js'; import { isHexString } from '../../utils/is-hex-string.js'; import { processRowsUtil } from '../../utils/process-found-rows-util.js'; +import { + assertSomeColumnReadable, + readableTableStructure, + restrictTableSettingsToReadableColumns, +} from '../../utils/restrict-query-to-readable-columns.util.js'; import { getUserEmailForAgent, validateConnection } from '../../utils/validate-connection.util.js'; import { PureFoundRowsResponseDs } from '../application/data-structures/pure-found-rows-response.ds.js'; import { PureGetRowsDs } from '../application/data-structures/pure-get-rows.ds.js'; @@ -66,12 +71,28 @@ export class PureGetRowsFromTableUseCase const tableStructure = await dao.getTableStructure(tableName, userEmail); + // Column-level read permissions must bound the whole query, not just the response (plan 13 + // P0-3): resolved here, BEFORE the filters/ordering are parsed, so a withheld column can be + // neither filtered, searched, ordered by nor selected. Anonymous callers (no userId — the + // public branch of QueryTableGuard) are evaluated against the connection's public policy. + const allColumnNames = tableStructure.map((column) => column.column_name); + const readableColumns = userId + ? await this.cedarPermissions.getReadableColumns(userId, connectionId, tableName, allColumnNames) + : await this.cedarPermissions.getReadableColumnsForPublic(connectionId, tableName, allColumnNames); + assertSomeColumnReadable(readableColumns); + const queryableStructure = readableTableStructure(tableStructure, readableColumns); + const filteringFields: Array = isObjectEmpty(filters) - ? findFilteringFieldsUtil(query, tableStructure) - : parseFilteringFieldsFromBodyData(filters ?? {}, tableStructure); - const orderingField = findOrderingFieldUtil(query, tableStructure, tableSettings ?? ({} as TableSettingsEntity)); + ? findFilteringFieldsUtil(query, queryableStructure) + : parseFilteringFieldsFromBodyData(filters ?? {}, queryableStructure); + const orderingField = findOrderingFieldUtil( + query, + queryableStructure, + tableSettings ?? ({} as TableSettingsEntity), + ); const builtTableSettings = buildDAOsTableSettingsDs(buildCommonTableSettingsInput(tableSettings), null); + restrictTableSettingsToReadableColumns(builtTableSettings, readableColumns, allColumnNames); if (orderingField) { builtTableSettings.ordering_field = orderingField.field; builtTableSettings.ordering = orderingField.value; @@ -79,12 +100,13 @@ export class PureGetRowsFromTableUseCase if ( isHexString(searchingFieldValue) && - (tableStructure.some((field) => isBinary(field.data_type)) || + (queryableStructure.some((field) => isBinary(field.data_type)) || connection.type === ConnectionTypesEnum.mongodb || connection.type === ConnectionTypesEnum.agent_mongodb) ) { searchingFieldValue = hexToBinary(searchingFieldValue) as unknown as string; - builtTableSettings.search_fields = tableStructure + // Readable columns only — a binary search must not reach a withheld column either. + builtTableSettings.search_fields = queryableStructure .filter((field) => isBinary(field.data_type)) .map((field) => field.column_name); if (connection.type === ConnectionTypesEnum.mongodb || connection.type === ConnectionTypesEnum.agent_mongodb) { @@ -111,10 +133,8 @@ export class PureGetRowsFromTableUseCase rows = processRowsUtil(rows, tableWidgets, tableCustomFields); - const allColumnNames = tableStructure.map((column) => column.column_name); - const readableColumns = userId - ? await this.cedarPermissions.getReadableColumns(userId, connectionId, tableName, allColumnNames) - : await this.cedarPermissions.getReadableColumnsForPublic(connectionId, tableName, allColumnNames); + // Defense in depth on top of the query-level restriction above: a widget/custom field or a DAO + // that ignores excluded_fields must not put a withheld column into the response. if (!isAllColumnsReadable(readableColumns, allColumnNames)) { rows.data = filterRowsByReadableColumns(rows.data, readableColumns); } diff --git a/backend/src/entities/table/table-pure-crud-operations/use-cases/pure-read-row-from-table.use.case.ts b/backend/src/entities/table/table-pure-crud-operations/use-cases/pure-read-row-from-table.use.case.ts index 6a9f5d1c7..9f53ce25d 100644 --- a/backend/src/entities/table/table-pure-crud-operations/use-cases/pure-read-row-from-table.use.case.ts +++ b/backend/src/entities/table/table-pure-crud-operations/use-cases/pure-read-row-from-table.use.case.ts @@ -19,6 +19,7 @@ import { isAllColumnsReadable, } from '../../utils/filter-columns-by-read-permission.util.js'; import { removePasswordsFromRowsUtil } from '../../utils/remove-password-from-row.util.js'; +import { assertSomeColumnReadable } from '../../utils/restrict-query-to-readable-columns.util.js'; import { getUserEmailForAgent, validateConnection } from '../../utils/validate-connection.util.js'; import { PureCrudRowResponseDs } from '../application/data-structures/pure-crud-row-response.ds.js'; import { PureReadRowDs } from '../application/data-structures/pure-read-row.ds.js'; @@ -88,6 +89,9 @@ export class PureReadRowFromTableUseCase const readableColumns = userId ? await this.cedarPermissions.getReadableColumns(userId, connectionId, tableName, allColumnNames) : await this.cedarPermissions.getReadableColumnsForPublic(connectionId, tableName, allColumnNames); + // Fail closed (plan 13 P0-3): no readable column ⇒ 403, not a 200 carrying an empty object — + // the 200-vs-400 outcome of the primary-key lookup is itself a row-existence signal. + assertSomeColumnReadable(readableColumns); if (!isAllColumnsReadable(readableColumns, allColumnNames)) { rowData = filterRowByReadableColumns(rowData, readableColumns); } diff --git a/backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts b/backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts index 8f75d360a..0e0c809d2 100644 --- a/backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts +++ b/backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts @@ -23,6 +23,11 @@ import { filterRowByReadableColumns, isAllColumnsReadable } from '../utils/filte import { findFilteringFieldsUtil, parseFilteringFieldsFromBodyData } from '../utils/find-filtering-fields.util.js'; import { findOrderingFieldUtil } from '../utils/find-ordering-field.util.js'; import { isHexString } from '../utils/is-hex-string.js'; +import { + assertSomeColumnReadable, + readableTableStructure, + restrictTableSettingsToReadableColumns, +} from '../utils/restrict-query-to-readable-columns.util.js'; import { getUserEmailForAgent, validateConnection } from '../utils/validate-connection.util.js'; import { IExportCSVFromTable } from './table-use-cases.interface.js'; @@ -72,16 +77,31 @@ export class ExportCSVFromTableUseCase ); } + // Column-level read permissions bound the query, not just the exported rows (plan 13 + // P0-3): a filter/search/ordering on a withheld column would otherwise reveal its + // values through which rows come back. This route requires a userId (`@UserId()`, no + // anonymous branch), so only the authenticated evaluation is needed here. + const allColumnNames = tableStructure.map((column) => column.column_name); + const readableColumns = await this.cedarPermissions.getReadableColumns( + userId, + connectionId, + tableName, + allColumnNames, + ); + assertSomeColumnReadable(readableColumns); + const queryableStructure = readableTableStructure(tableStructure, readableColumns); + const filteringFields: Array = isObjectEmpty(filters) - ? findFilteringFieldsUtil(query, tableStructure) - : parseFilteringFieldsFromBodyData(filters ?? {}, tableStructure); + ? findFilteringFieldsUtil(query, queryableStructure) + : parseFilteringFieldsFromBodyData(filters ?? {}, queryableStructure); - const orderingField = findOrderingFieldUtil(query, tableStructure, tableSettings); + const orderingField = findOrderingFieldUtil(query, queryableStructure, tableSettings); const builtDAOsTableSettings = buildDAOsTableSettingsDs( buildCommonTableSettingsInput(tableSettings), personalTableSettings, ); + restrictTableSettingsToReadableColumns(builtDAOsTableSettings, readableColumns, allColumnNames); if (orderingField) { builtDAOsTableSettings.ordering_field = orderingField.field; @@ -90,7 +110,8 @@ export class ExportCSVFromTableUseCase if (isHexString(searchingFieldValue)) { searchingFieldValue = hexToBinary(searchingFieldValue) as any; - tableSettings.search_fields = tableStructure + // Readable columns only — a binary search must not reach a withheld column either. + tableSettings.search_fields = queryableStructure .filter((field) => isBinary(field.data_type)) .map((field) => field.column_name); } @@ -106,15 +127,7 @@ export class ExportCSVFromTableUseCase operationResult = OperationResultStatusEnum.successfully; - // Column-level read permission (the ColumnRead half of table:read): drop columns the - // user may not read from the exported rows. - const allColumnNames = tableStructure.map((column) => column.column_name); - const readableColumns = await this.cedarPermissions.getReadableColumns( - userId, - connectionId, - tableName, - allColumnNames, - ); + // Response-side projection, on top of the query-level restriction above (defense in depth). const restrictColumns = !isAllColumnsReadable(readableColumns, allColumnNames); //todo: rework as streams when node oracle driver will support it correctly diff --git a/backend/src/entities/table/utils/restrict-query-to-readable-columns.util.ts b/backend/src/entities/table/utils/restrict-query-to-readable-columns.util.ts new file mode 100644 index 000000000..0df55a996 --- /dev/null +++ b/backend/src/entities/table/utils/restrict-query-to-readable-columns.util.ts @@ -0,0 +1,66 @@ +import { ForbiddenException } from '@nestjs/common'; +import { TableSettingsDS } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/data-structures/table-settings.ds.js'; +import { TableStructureDS } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/data-structures/table-structure.ds.js'; +import { Messages } from '../../../exceptions/text/messages.js'; + +// Column-level read permissions must bound the QUERY, not just the response (plan 13 P0-3). +// +// Stripping non-readable columns from the returned rows leaves the withheld values reachable +// indirectly: a filter or search on a hidden column still runs in SQL, and `pagination.total` +// (matched-row count) then answers "does this column start with X?" one character at a time — +// enough to extract a password hash from a table the caller may legitimately query. Ordering by a +// hidden column leaks the same way through the row order across pages. +// +// These helpers make the readable set the INPUT to filtering, search, ordering and the select list: +// +// 1. `readableTableStructure` — parse filters and the ordering field against this instead of the +// full structure, so a filter naming a withheld column is ignored exactly like one naming a +// column that does not exist (no existence oracle, and dropping a filter only ever widens the +// result set, never narrows it to something the caller could not already see). +// 2. `restrictTableSettingsToReadableColumns` — adds every withheld column to `excluded_fields` +// (which `findAvailableFields` honours in every DAO, so it bounds the `select()` list and the +// default search-field set) and intersects an explicit `search_fields` list with the readable +// set. +// 3. `assertSomeColumnReadable` — fails CLOSED on an empty readable set. Without it, excluding +// every column makes the MySQL/MSSQL DAOs fall back to `select('*')`, and the caller still +// gets a real `pagination.total` to run the oracle against. +// +// The post-query projection (`filterRowsByReadableColumns`) stays in place on top of this as +// defense in depth. `universal-backend` enforces the same rule for the generated-site runtime +// (`resolvePublicReadableColumns`, plan 13 Step 0) — keep the two in step. + +// No readable column at all ⇒ 403, for authenticated and public callers alike. A caller who may +// query a table but read none of its columns can still mine it through filters, so an empty +// projection is not a safe answer. +export function assertSomeColumnReadable(readableColumns: Set): void { + if (readableColumns.size === 0) { + throw new ForbiddenException(Messages.DONT_HAVE_PERMISSIONS); + } +} + +// The table structure reduced to the readable columns — the parsing input for filters/ordering. +export function readableTableStructure( + tableStructure: Array, + readableColumns: Set, +): Array { + return tableStructure.filter((column) => readableColumns.has(column.column_name)); +} + +// Bounds the DAO's own column handling (select list, default and explicit search fields) to the +// readable set. Mutates the settings object the caller is about to hand to the DAO. +export function restrictTableSettingsToReadableColumns( + settings: TableSettingsDS, + readableColumns: Set, + allColumnNames: Array, +): void { + const withheld = allColumnNames.filter((columnName) => !readableColumns.has(columnName)); + if (withheld.length === 0) { + return; + } + settings.excluded_fields = [...new Set([...(settings.excluded_fields ?? []), ...withheld])]; + // An explicit search-field list may name withheld columns; intersect it. Emptying it is safe — + // the DAOs then fall back to the available (now readable-only) fields. + if (settings.search_fields && settings.search_fields.length > 0) { + settings.search_fields = settings.search_fields.filter((field) => readableColumns.has(field)); + } +} diff --git a/backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-public-permissions-e2e.test.ts b/backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-public-permissions-e2e.test.ts index 7627ea3a3..42d7d9cc8 100644 --- a/backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-public-permissions-e2e.test.ts +++ b/backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-public-permissions-e2e.test.ts @@ -301,3 +301,118 @@ test.serial(`${currentTest} refuses a policy that is not a JSON object (400)`, a const stringPolicy = await siteRuntimePolicyRequest(connectionId, { userId, policy: 'not-an-object' }); t.is(stringPolicy.status, 400); }); + +// --- plan 13 P0-3: a public column whitelist must bound the QUERY, not just the response --- +// +// `readableColumns` used to be applied only as a projection over rows the query had already +// returned, so an anonymous caller could still filter or search on a WITHHELD column and read its +// values back out of which rows came back (`pagination.total` as a per-character oracle over a +// password hash). These tests pin the fix on the core's anonymous `/table/crud` branch: the +// readable set now feeds filtering, search and the select list. (universal-backend enforces the +// same rule for the generated-site runtime — plan 13 Step 0.) +// +// The seeded table has 42 rows; `testTableColumnName` holds the value 'Vasia' in exactly 3 of them +// and `testTableSecondColumnName` holds a unique email per row. Assertions deliberately use a +// filter/search that matches a NON-EMPTY subset when applied, because the MySQL DAO's +// `getRowsCount` treats a filtered count of 0 as falsy and falls back to the whole-table count — +// so "total === 0" is not a usable signal here, while "total === 3 vs 42" is. + +function anonymousCrudRowsRequest( + connectionId: string, + tableName: string, + body: Record = {}, + extraQuery = '', +): request.Test { + return request(app.getHttpServer()) + .post(`/table/crud/rows/${connectionId}?tableName=${tableName}&page=1&perPage=10${extraQuery}`) + .send(body) + .set('Content-Type', 'application/json') + .set('Accept', 'application/json'); +} + +currentTest = 'POST /table/crud/rows/:connectionId (anonymous, public Cedar policy)'; + +test.serial(`${currentTest} returns only the publicly readable columns`, async (t) => { + const { userId, connectionId, testTableName, testTableColumnName, testTableSecondColumnName } = + await createConnectionAndTable(); + await grantRequest(connectionId, { + userId, + tables: [{ tableName: testTableName, readableColumns: [testTableColumnName] }], + }); + + const response = await anonymousCrudRowsRequest(connectionId, testTableName); + t.is(response.status, 200); + const { rows } = JSON.parse(response.text); + t.is(rows.length, 10); + t.deepEqual(Object.keys(rows[0]), [testTableColumnName]); + t.false(Object.keys(rows[0]).includes(testTableSecondColumnName)); + t.false(Object.keys(rows[0]).includes('id')); +}); + +test.serial(`${currentTest} ignores a filter on a withheld column (kills the pagination oracle)`, async (t) => { + const { userId, connectionId, testTableName, testTableColumnName, testTableSecondColumnName } = + await createConnectionAndTable(); + // Grant the EMAIL column only — the name column (3 rows of 'Vasia') is withheld. + await grantRequest(connectionId, { + userId, + tables: [{ tableName: testTableName, readableColumns: [testTableSecondColumnName] }], + }); + + // Body-filter form: before the fix this ran in SQL and answered "3 rows match 'Vasia'" about a + // column the caller may not read. Now it is ignored exactly like a filter on a column that does + // not exist, so the result set stays the unfiltered one. + const bodyFiltered = await anonymousCrudRowsRequest(connectionId, testTableName, { + filters: { [testTableColumnName]: { eq: 'Vasia' } }, + }); + t.is(bodyFiltered.status, 200); + const bodyFilteredRO = JSON.parse(bodyFiltered.text); + t.is(bodyFilteredRO.pagination.total, 42); + t.is(bodyFilteredRO.rows.length, 10); + t.deepEqual(Object.keys(bodyFilteredRO.rows[0]), [testTableSecondColumnName]); + + // Query-string filter form is bounded the same way. + const queryFiltered = await anonymousCrudRowsRequest( + connectionId, + testTableName, + {}, + `&f_${testTableColumnName}__eq=Vasia`, + ); + t.is(queryFiltered.status, 200); + t.is(JSON.parse(queryFiltered.text).pagination.total, 42); +}); + +test.serial(`${currentTest} never searches a withheld column`, async (t) => { + const { userId, connectionId, testTableName, testTableColumnName, testTableSecondColumnName } = + await createConnectionAndTable(); + await grantRequest(connectionId, { + userId, + tables: [{ tableName: testTableName, readableColumns: [testTableSecondColumnName] }], + }); + + // 'Vasia' exists only in the withheld column: the search must match nothing rather than + // returning those 3 rows (before the fix, search ILIKEd every column of the table). + const response = await anonymousCrudRowsRequest(connectionId, testTableName, {}, '&search=Vasia'); + t.is(response.status, 200); + const body = JSON.parse(response.text); + t.is(body.rows.length, 0); + t.not(body.pagination.total, 3); +}); + +test.serial(`${currentTest} still applies a filter and search on a readable column`, async (t) => { + const { userId, connectionId, testTableName, testTableColumnName } = await createConnectionAndTable(); + await grantRequest(connectionId, { + userId, + tables: [{ tableName: testTableName, readableColumns: [testTableColumnName] }], + }); + + const filtered = await anonymousCrudRowsRequest(connectionId, testTableName, { + filters: { [testTableColumnName]: { eq: 'Vasia' } }, + }); + t.is(filtered.status, 200); + t.is(JSON.parse(filtered.text).pagination.total, 3); + t.is(JSON.parse(filtered.text).rows.length, 3); + + const searched = await anonymousCrudRowsRequest(connectionId, testTableName, {}, '&search=Vasia'); + t.is(searched.status, 200); + t.is(JSON.parse(searched.text).rows.length, 3); +}); diff --git a/backend/test/ava-tests/non-saas-tests/non-saas-restrict-query-to-readable-columns.test.ts b/backend/test/ava-tests/non-saas-tests/non-saas-restrict-query-to-readable-columns.test.ts new file mode 100644 index 000000000..6006fec58 --- /dev/null +++ b/backend/test/ava-tests/non-saas-tests/non-saas-restrict-query-to-readable-columns.test.ts @@ -0,0 +1,63 @@ +import { ForbiddenException } from '@nestjs/common'; +import { TableSettingsDS } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/data-structures/table-settings.ds.js'; +import { TableStructureDS } from '@rocketadmin/shared-code/dist/src/data-access-layer/shared/data-structures/table-structure.ds.js'; +import test from 'ava'; +import { + assertSomeColumnReadable, + readableTableStructure, + restrictTableSettingsToReadableColumns, +} from '../../../src/entities/table/utils/restrict-query-to-readable-columns.util.js'; + +// plan 13 P0-3: these helpers are what makes a column-level read permission bound the QUERY +// (filters, search, ordering, select) instead of only the response projection. + +function structure(...columnNames: Array): Array { + return columnNames.map((column_name) => ({ column_name }) as TableStructureDS); +} + +test('assertSomeColumnReadable fails closed on an empty readable set', (t) => { + // An empty set must never be read as "everything": excluding every column makes the MySQL/MSSQL + // DAOs fall back to select('*'), and the caller would still get a real pagination.total to mine. + t.throws(() => assertSomeColumnReadable(new Set()), { instanceOf: ForbiddenException }); + t.notThrows(() => assertSomeColumnReadable(new Set(['id']))); +}); + +test('readableTableStructure drops withheld columns so filters/ordering cannot name them', (t) => { + const reduced = readableTableStructure(structure('id', 'email', 'password'), new Set(['id', 'email'])); + t.deepEqual( + reduced.map((column) => column.column_name), + ['id', 'email'], + ); +}); + +test('restrictTableSettingsToReadableColumns excludes withheld columns and intersects search fields', (t) => { + const settings = { + excluded_fields: ['internal_note'], + search_fields: ['email', 'password'], + } as TableSettingsDS; + + restrictTableSettingsToReadableColumns(settings, new Set(['id', 'email']), ['id', 'email', 'password']); + + // The withheld column joins the DAO's excluded set (bounds select() and the default search set)… + t.deepEqual([...settings.excluded_fields].sort(), ['internal_note', 'password']); + // …and an explicit search-field list can no longer name it. + t.deepEqual(settings.search_fields, ['email']); +}); + +test('restrictTableSettingsToReadableColumns leaves settings untouched when everything is readable', (t) => { + const settings = { excluded_fields: [], search_fields: ['email'] } as TableSettingsDS; + + restrictTableSettingsToReadableColumns(settings, new Set(['id', 'email']), ['id', 'email']); + + t.deepEqual(settings.excluded_fields, []); + t.deepEqual(settings.search_fields, ['email']); +}); + +test('restrictTableSettingsToReadableColumns tolerates absent settings arrays', (t) => { + const settings = {} as TableSettingsDS; + + restrictTableSettingsToReadableColumns(settings, new Set(['id']), ['id', 'password']); + + t.deepEqual(settings.excluded_fields, ['password']); + t.is(settings.search_fields, undefined); +});