Adr implementation plan by suggestied · Pull Request #40 · workorder-systems/db

suggestied · 2026-01-29T10:44:35Z

Summary

Adds a detailed implementation plan for proposed Architectural Decision Records (ADRs). This provides a structured approach to guide the development work required to move these ADRs to an 'Accepted' status.

Changes

Created docs/adr/implementation-plan.md outlining the implementation steps for ADRs 0002, 0005, 0007, 0008, 0009, and 0010.
Updated docs/adr/README.md to include a link to the new implementation plan.

Testing

npm test
Not run (Documentation-only change)

Checklist

Changes are focused and scoped
Docs updated if contract changed
New migrations include RLS policies where required
New write paths use RPC (no direct table writes)

Notes

This PR is purely documentation-focused and does not introduce any code changes. It serves as a blueprint for future development work.

Note

High Risk
High risk because it changes tenant scoping semantics and authentication/token issuance flow (new custom access-token hook + metadata-backed tenant context), and adds new RLS-protected schemas/RPCs for audit retention and plugin installation that affect security and data access patterns.

Overview
Core change: tenant scoping for public views moves from “all memberships via auth.uid()” to a single active tenant context derived primarily from a new JWT tenant_id claim (with session-variable fallback). This adds a Supabase Auth custom_access_token_hook, updates rpc_set_tenant_context to persist current_tenant_id into user metadata (requiring token refresh), and updates many public.v_* views to filter by authz.get_current_tenant_id(); several views now explicitly grant/revoke anon access.

Audit + governance: adds tenant-configurable audit retention (cfg.audit_retention_configs, public.v_audit_retention_configs, public.rpc_set_audit_retention_config) plus a purge helper (util.purge_audit_records), and introduces an admin-only public.v_audit_permission_changes view. Audit coverage is expanded (new departments audit trigger) and indexes added to speed common audit queries.

Integrations/plugins: introduces int.plugins and int.plugin_installations with RLS, audit triggers, discovery/admin views (public.v_plugins, public.v_plugin_installations), and admin RPCs to install/update/uninstall; registration is restricted to service_role. Public API naming is aligned (renames summary/overview views to plural forms), removes a now-redundant migration, tightens local API exposure in supabase/config.toml, and adds docs/checklist updates plus tests (including naming-convention enforcement) while refactoring existing tests away from direct table writes toward RPC/view usage.

^{Written by Cursor Bugbot for commit 7e2f56f. This will update automatically on new commits. Configure here.}

Co-authored-by: suggestied <suggestied@gmail.com>

cursor · 2026-01-29T10:44:37Z

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
_{Learn more about Cursor Agents}

Co-authored-by: suggestied <suggestied@gmail.com>

- Enable custom access token hook in configuration to support JWT claims. - Update SQL functions to read tenant ID from JWT claims and maintain backward compatibility with session variables. - Modify comments to clarify the use of user metadata for tenant context and the necessity of refreshing tokens. - Enhance tenant context setting in tests to ensure persistence across PostgREST requests. Co-authored-by: suggestied <suggestied@gmail.com>

- Wrap custom access token hook in exception handling to ensure graceful failure without impacting authentication. - Update user ID extraction logic for compatibility with multiple event structures. - Modify test tenant context setting to force token refresh by signing out and back in, ensuring the custom access token hook runs correctly. - Improve error handling during user sign-in process in tests. Co-authored-by: suggestied <suggestied@gmail.com>

…n installation - Update asset tests to create separate clients for each tenant, ensuring proper tenant context is set during asset creation. - Enhance integration tests by adding configuration parameters for plugin installation, including a new secret reference and region setting. Co-authored-by: suggestied <suggestied@gmail.com>

…orrect tenant context - Modify the SQL migration to separately retrieve the user ID after calling the rpc_setup function, which now returns the tenant ID. - Add comments for clarity on the changes made to user ID extraction and its implications for tenant context handling.

- Recreate the v_plugin_installations view with explicit permission checks in the WHERE clause to ensure that only users with tenant.admin permissions can see plugin installations. - Add comments for clarity on the permission checks and their integration with RLS policies. - Update the view's comment to reflect the new permission requirements and security measures.

- Grant execute permission on the `authz.get_current_tenant_id()` function to the anon role, allowing anonymous users to query views that utilize this function while ensuring they receive empty results. - Update comments on various views to clarify that anonymous users can query them but will receive no data due to tenant context restrictions. - Adjust permissions on tenant-scoped views to ensure proper access control, including granting select permissions to authenticated users and revoking from anon where necessary. - Implement a custom access token hook to add tenant_id to JWT claims, ensuring stateless tenant context across requests and validating tenant membership before adding claims. Co-authored-by: suggestied <suggestied@gmail.com>

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

cursor · 2026-01-30T15:59:07Z

supabase/migrations/20260121125000_enterprise_features.sql

+begin
+  v_user_id := authz.rpc_setup(p_tenant_id, 'tenant.admin');
+
+  perform util.check_rate_limit('plugin_update', null, 20, 1, v_user_id, p_tenant_id);


Incorrect variable assignment causes broken rate limiting

High Severity

In rpc_update_plugin_installation and rpc_uninstall_plugin, v_user_id is assigned the return value of authz.rpc_setup(), which returns the tenant_id, not a user_id. This causes the rate limiting to track against tenant_id instead of user_id, allowing individual users to potentially bypass their rate limits or incorrectly blocking legitimate users based on other users' actions. The correct pattern is shown in rpc_install_plugin which uses perform authz.rpc_setup() followed by v_user_id := authz.validate_authenticated().

Additional Locations (1)

supabase/migrations/20260121125000_enterprise_features.sql#L1311-L1314

cursor · 2026-01-30T15:59:07Z

supabase/migrations/20260121125000_enterprise_features.sql

+where tenant_id = authz.get_current_tenant_id();
+
+comment on view public.v_audit_retention_configs is
+  'Tenant-scoped audit retention configuration for the current tenant context. Requires tenant.admin access via RLS.';


Missing grants prevent access to new audit views

Medium Severity

The views v_audit_retention_configs, v_audit_permission_changes, and v_plugins are created without grant select statements to authenticated users. Unlike v_plugin_installations which has explicit grants (line 1153), these views lack permission grants. This means authenticated users cannot query them, despite the views being documented as accessible ("Requires tenant.admin access via RLS", "Read-only view for client discovery") and having tests that expect to query them. The tests in audit_retention.test.ts will fail due to these missing grants.

Additional Locations (2)

supabase/migrations/20260121125000_enterprise_features.sql#L707-L733

supabase/migrations/20260121125000_enterprise_features.sql#L1106-L1121

Add implementation plan for proposed ADRs

e5a4fe0

Co-authored-by: suggestied <suggestied@gmail.com>

cursoragent and others added 22 commits January 29, 2026 11:25

Start ADR implementation with docs and naming test

829d253

Co-authored-by: suggestied <suggestied@gmail.com>

Add migrations for integrations, audit retention, naming

ea720e5

Co-authored-by: suggestied <suggestied@gmail.com>

Align analytics view naming with ADR conventions

a431542

Co-authored-by: suggestied <suggestied@gmail.com>

Implement ADR 0007 deletion policy

5b0da07

Co-authored-by: suggestied <suggestied@gmail.com>

Fold deletion policy trigger into core migration

4334874

Co-authored-by: suggestied <suggestied@gmail.com>

Optimize tenant-scoped public view filters

83569e0

Co-authored-by: suggestied <suggestied@gmail.com>

Consolidate integration and audit retention migrations

ea9a6a3

Co-authored-by: suggestied <suggestied@gmail.com>

Mark implemented ADRs as accepted

b78258c

Co-authored-by: suggestied <suggestied@gmail.com>

Add ADR integration and audit tests

8299a37

Co-authored-by: suggestied <suggestied@gmail.com>

Align ADR tests with public RPCs and views

7af6a9c

Co-authored-by: suggestied <suggestied@gmail.com>

Restrict API schemas to public and refactor tests

f8a0f2b

Co-authored-by: suggestied <suggestied@gmail.com>

Scope tenant role views to tenant context

a7433b6

Co-authored-by: suggestied <suggestied@gmail.com>

Optimize audit query indexes

3242938

Co-authored-by: suggestied <suggestied@gmail.com>

Optimize view security migration for tenant context

7f87129

Co-authored-by: suggestied <suggestied@gmail.com>

Add strict tenant isolation tests

6db7609

Co-authored-by: suggestied <suggestied@gmail.com>

Restrict plugin installation visibility to admins

14bfcfb

Co-authored-by: suggestied <suggestied@gmail.com>

suggestied marked this pull request as ready for review January 30, 2026 15:48

suggestied merged commit 545a93e into main Jan 30, 2026
2 checks passed

suggestied deleted the cursor/adr-implementation-plan-fb70 branch January 30, 2026 15:49

cursor bot reviewed Jan 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adr implementation plan#40

Adr implementation plan#40
suggestied merged 23 commits intomainfrom
cursor/adr-implementation-plan-fb70

suggestied commented Jan 29, 2026 •

edited by cursor bot

Loading

Uh oh!

cursor bot commented Jan 29, 2026

Uh oh!

Uh oh!

cursor bot left a comment

Uh oh!

cursor bot Jan 30, 2026

Uh oh!

cursor bot Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

suggestied commented Jan 29, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Testing

Checklist

Notes

Uh oh!

cursor bot commented Jan 29, 2026

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

This PR is being reviewed by Cursor Bugbot

Uh oh!

cursor bot Jan 30, 2026

Choose a reason for hiding this comment

Incorrect variable assignment causes broken rate limiting

Uh oh!

cursor bot Jan 30, 2026

Choose a reason for hiding this comment

Missing grants prevent access to new audit views

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

suggestied commented Jan 29, 2026 •

edited by cursor bot

Loading