enforce column-level read permissions across queries and responses - #1866
Conversation
📝 WalkthroughWalkthroughColumn read permissions now constrain filtering, ordering, searching, DAO settings, row reads, CSV exports, and response projections. New utilities provide shared validation and restriction logic, with unit and end-to-end coverage. ChangesReadable-column enforcement
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant TableUseCase
participant CedarPermissionsService
participant DAO
TableUseCase->>CedarPermissionsService: Resolve readableColumns
TableUseCase->>TableUseCase: Build queryableStructure and restricted settings
TableUseCase->>DAO: Execute bounded query or export
DAO-->>TableUseCase: Return rows or export data
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR strengthens column-level read permissions in the backend by ensuring “readable columns” constrain not only the response projection but also the underlying query (filters, search, ordering, and select lists), preventing information leaks via pagination.total and row selection behavior.
Changes:
- Introduces shared helpers to (1) fail closed on empty readable-column sets, (2) reduce table structure to readable columns for parsing, and (3) restrict DAO table settings to readable columns.
- Updates table row retrieval and CSV export use-cases to apply readable-column restrictions before parsing filters/ordering and before executing queries.
- Adds unit and e2e tests to pin the security behavior for public/anonymous access paths and settings restriction logic.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/test/ava-tests/non-saas-tests/non-saas-restrict-query-to-readable-columns.test.ts | New unit tests for the readable-column restriction helpers. |
| backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-public-permissions-e2e.test.ts | New e2e coverage ensuring public readable columns bound query behavior (filters/search) and response projection. |
| backend/src/entities/table/utils/restrict-query-to-readable-columns.util.ts | Adds helper utilities to enforce readable columns at query-construction time and fail closed on empty readable sets. |
| backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts | Applies readable-column constraints to CSV export query parsing/settings and keeps response projection as defense-in-depth. |
| backend/src/entities/table/table-pure-crud-operations/use-cases/pure-read-row-from-table.use.case.ts | Adds fail-closed behavior when no readable columns exist for a row read. |
| backend/src/entities/table/table-pure-crud-operations/use-cases/pure-get-rows-from-table.use.case.ts | Applies readable-column constraints before filter/order parsing and enforces readable-only DAO settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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); | ||
| } |
| ? 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); |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
backend/test/ava-tests/non-saas-tests/non-saas-restrict-query-to-readable-columns.test.ts (1)
14-16: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an arrow helper.
Replace the
structurefunction declaration with aconstarrow function.As per coding guidelines: “Prefer arrow functions over function declarations.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/test/ava-tests/non-saas-tests/non-saas-restrict-query-to-readable-columns.test.ts` around lines 14 - 16, Replace the structure function declaration with a const-bound arrow function while preserving its variadic string parameters, TableStructureDS return type, and existing columnNames.map behavior.Source: Coding guidelines
backend/src/entities/table/utils/restrict-query-to-readable-columns.util.ts (1)
35-66: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse arrow functions for the new helper exports.
Replace the three
export functiondeclarations withexport constarrow functions.As per coding guidelines: “Prefer arrow functions over function declarations.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/table/utils/restrict-query-to-readable-columns.util.ts` around lines 35 - 66, Convert the exported helpers assertSomeColumnReadable, readableTableStructure, and restrictTableSettingsToReadableColumns from function declarations to exported const arrow functions, preserving their parameters, return types, bodies, and behavior.Source: Coding guidelines
backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-public-permissions-e2e.test.ts (1)
320-325: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an arrow helper and annotate
extraQuery.Replace
anonymousCrudRowsRequestwith aconstarrow function. DeclareextraQueryasstring.As per coding guidelines: “Prefer arrow functions over function declarations” and “Always add type annotations to function parameters and return types in TypeScript.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-public-permissions-e2e.test.ts` around lines 320 - 325, Update anonymousCrudRowsRequest to use a const arrow function instead of a function declaration, and add an explicit string type annotation to extraQuery while keeping the existing request.Test return type and other parameter types unchanged. Anchor the change around anonymousCrudRowsRequest so the helper matches the TypeScript style guidelines without altering its behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts`:
- Around line 111-116: Update the binary-search branch in the table export flow
to assign the filtered binary column names to
builtDAOsTableSettings.search_fields, which is the settings object passed to
dao.getTableRowsStream, instead of tableSettings.search_fields. Add a CSV
regression test covering configured text search_fields combined with a
hexadecimal search on a binary column.
---
Nitpick comments:
In `@backend/src/entities/table/utils/restrict-query-to-readable-columns.util.ts`:
- Around line 35-66: Convert the exported helpers assertSomeColumnReadable,
readableTableStructure, and restrictTableSettingsToReadableColumns from function
declarations to exported const arrow functions, preserving their parameters,
return types, bodies, and behavior.
In
`@backend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-public-permissions-e2e.test.ts`:
- Around line 320-325: Update anonymousCrudRowsRequest to use a const arrow
function instead of a function declaration, and add an explicit string type
annotation to extraQuery while keeping the existing request.Test return type and
other parameter types unchanged. Anchor the change around
anonymousCrudRowsRequest so the helper matches the TypeScript style guidelines
without altering its behavior.
In
`@backend/test/ava-tests/non-saas-tests/non-saas-restrict-query-to-readable-columns.test.ts`:
- Around line 14-16: Replace the structure function declaration with a
const-bound arrow function while preserving its variadic string parameters,
TableStructureDS return type, and existing columnNames.map behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1eb552ce-2838-41a4-a7a9-eb086875fa5e
📒 Files selected for processing (6)
backend/src/entities/table/table-pure-crud-operations/use-cases/pure-get-rows-from-table.use.case.tsbackend/src/entities/table/table-pure-crud-operations/use-cases/pure-read-row-from-table.use.case.tsbackend/src/entities/table/use-cases/export-csv-from-table.use.case.tsbackend/src/entities/table/utils/restrict-query-to-readable-columns.util.tsbackend/test/ava-tests/non-saas-tests/non-saas-agents-microservice-public-permissions-e2e.test.tsbackend/test/ava-tests/non-saas-tests/non-saas-restrict-query-to-readable-columns.test.ts
| 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); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Update the DAO settings object for binary searches.
dao.getTableRowsStream receives builtDAOsTableSettings at Line 121. Line 114 updates tableSettings.search_fields after builtDAOsTableSettings was built. The binary-only search-field restriction therefore has no effect.
Set builtDAOsTableSettings.search_fields instead. Add a CSV regression test with configured text search_fields and a binary-column hex search.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/entities/table/use-cases/export-csv-from-table.use.case.ts`
around lines 111 - 116, Update the binary-search branch in the table export flow
to assign the filtered binary column names to
builtDAOsTableSettings.search_fields, which is the settings object passed to
dao.getTableRowsStream, instead of tableSettings.search_fields. Add a CSV
regression test covering configured text search_fields combined with a
hexadecimal search on a binary column.
Summary by CodeRabbit
Bug Fixes
Tests