Skip to content

feat: read RLS config from typed rls_settings table with api_modules fallback#1086

Merged
pyramation merged 3 commits intomainfrom
feat/phase2-server-rls-settings
May 9, 2026
Merged

feat: read RLS config from typed rls_settings table with api_modules fallback#1086
pyramation merged 3 commits intomainfrom
feat/phase2-server-rls-settings

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

@pyramation pyramation commented May 9, 2026

Summary

Updates the GraphQL server to prefer the new typed services_public.rls_settings table (introduced in constructive-db PR #1075) over the legacy services_public.api_modules JSONB blob when resolving RLS module configuration. This is the server-side component of Phase 2 of the unified settings architecture (constructive-planning#812).

Three files changed:

  • api.tsqueryRlsModule now takes (pool, databaseId, apiId) and tries rls_settings by database_id first; falls back to api_modules by api_id. toApiStructure now accepts an already-resolved RlsModule instead of a raw RlsModuleRow.
  • upload.tsqueryRlsModuleByDatabaseId and queryRlsModuleByDbname each try the typed table first via new queryRlsSettingsByDatabaseId / queryRlsSettingsByDbname helpers. queryRlsModuleByApiId is unchanged (no database_id available at that call site; the upload resolver tries database_id and dbname paths first anyway). The RLS_SETTINGS_BY_DBNAME_SQL query joins through services_public.apis (on a.dbname = $1) rather than metaschema_public.database, since apis.dbname holds the actual PostgreSQL database name used by the server.
  • upload.test.ts — Existing tests updated with additional mockResolvedValueOnce({ rows: [] }) calls for the new typed-table queries that now precede the legacy api_modules queries.

All new typed-table queries are wrapped in try/catch returning undefined on failure, so the server gracefully degrades to api_modules for databases that haven't been reprovisioned yet or where the rls_settings table doesn't exist.

Review & Testing Checklist for Human

  • Check toRlsModuleFromSettings truthy-object behavior: If an rls_settings row exists but all FK joins resolve to NULL (e.g. stale/orphaned row), toRlsModuleFromSettings returns { authenticate: null, ... } — a truthy object that prevents fallback to api_modules. Consider whether the converter should check that at least authenticate and authenticate_schema are non-null before returning a result.
  • Verify SQL column aliases match RlsModuleData interface: The 8-way LEFT JOIN queries select authenticate_schema, role_schema, authenticate, authenticate_strict, current_role, current_role_id, current_user_agent, current_ip_address. These must exactly match the fields on RlsModuleData (lines 133-142 in api.ts, mirrored in upload.ts). A mismatch produces an RlsModule with undefined fields instead of falling back.
  • Silent catch blocks swallow all errors: queryRlsSettings / queryRlsSettingsByDatabaseId / queryRlsSettingsByDbname catch all exceptions and return undefined with no logging. This is intentional for backwards compat but means a broken query (typo, permission issue) is invisible. Consider whether a log.debug should be added inside the catch.
  • End-to-end test: Deploy a database with the trigger rewiring from constructive-db PR fix(presigned-url): match files→buckets by table name prefix instead of schema name #1075 so rls_settings is populated, then verify the server actually reads from the typed table (not just silently falling back). One way: temporarily add a log line in queryRlsSettings on success, provision a DB, and confirm the log fires.

Notes

  • The same 22-line SQL query (8 LEFT JOINs) appears 3 times across both files. A future cleanup could extract it into a shared constant or query builder, but keeping it inline matches the existing patterns in these files.
  • queryRlsModuleByApiId in upload.ts intentionally has no typed-table path — in resolveUploadRlsModule, the database_id and dbname paths (which do use typed tables) are tried first; apiId is the last resort.
  • RLS_SETTINGS_BY_DBNAME_SQL joins through services_public.apis (not metaschema_public.database) because apis.dbname is the PostgreSQL database name the server uses at runtime, whereas metaschema_public.database.name is a display name.
  • Companion DB-side PR: constructive-db#1075 (trigger rewiring to populate rls_settings, pubkey_settings, webauthn_settings).

Link to Devin session: https://app.devin.ai/sessions/94a2728a9c414500bead29cbbc829c15
Requested by: @pyramation

…fallback

Update the server to prefer services_public.rls_settings (joined with
metaschema_public.schema and metaschema_public.function to resolve names)
over the legacy api_modules JSONB path.

api.ts:
- Add RLS_SETTINGS_SQL query that joins rls_settings with schema/function
  tables to resolve UUID FKs back to names
- queryRlsModule now tries rls_settings by database_id first, falls back
  to api_modules by api_id

upload.ts:
- Add RLS_SETTINGS_BY_DATABASE_ID_SQL and RLS_SETTINGS_BY_DBNAME_SQL
- queryRlsModuleByDatabaseId and queryRlsModuleByDbname try typed tables
  first with api_modules fallback
- queryRlsModuleByApiId unchanged (no database_id available at that call
  site, falls through to api_modules)

All fallbacks are wrapped in try/catch so the server gracefully degrades
to api_modules if rls_settings doesn't exist yet (e.g. pre-migration DBs).

Refs: constructive-planning#812
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@blacksmith-sh

This comment has been minimized.

pyramation added 2 commits May 9, 2026 18:17
metaschema_public.database.name is not the same as the PG database name
(apis.dbname). Join through services_public.apis instead to correctly
resolve dbname to database_id for the rls_settings lookup.
The new typed table queries (queryRlsSettingsByDatabaseId,
queryRlsSettingsByDbname) make an additional pool.query call before the
legacy api_modules fallback. Add mockResolvedValueOnce({ rows: [] }) for
these calls so the legacy mock data is consumed by the correct query.
@pyramation pyramation merged commit 63fe328 into main May 9, 2026
54 checks passed
@pyramation pyramation deleted the feat/phase2-server-rls-settings branch May 9, 2026 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant