Skip to content

ENG-1951 Tighten table access control#1157

Open
maparent wants to merge 3 commits into
mainfrom
eng-1951-tighten-table-access-control
Open

ENG-1951 Tighten table access control#1157
maparent wants to merge 3 commits into
mainfrom
eng-1951-tighten-table-access-control

Conversation

@maparent

@maparent maparent commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

https://linear.app/discourse-graphs/issue/ENG-1951/tighten-table-access-control

https://www.loom.com/share/d1755b3bbc8a4197a293f431db373a84

Tightened access controls so group membership does not normally allow cross-space edits. Also added tests.


Open in Devin Review

@linear-code

linear-code Bot commented Jun 25, 2026

Copy link
Copy Markdown

ENG-1951

@vercel

vercel Bot commented Jun 25, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
discourse-graph Ready Ready Preview, Comment Jun 25, 2026 6:22pm

Request Review

@supabase

supabase Bot commented Jun 25, 2026

Copy link
Copy Markdown

Updates to Preview Branch (eng-1951-tighten-table-access-control) ↗︎

Deployments Status Updated
Database Thu, 25 Jun 2026 18:21:22 UTC
Services Thu, 25 Jun 2026 18:21:22 UTC
APIs Thu, 25 Jun 2026 18:21:22 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Thu, 25 Jun 2026 18:21:23 UTC
Migrations Thu, 25 Jun 2026 18:21:23 UTC
Seeding Thu, 25 Jun 2026 18:21:23 UTC
Edge Functions Thu, 25 Jun 2026 18:21:27 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@graphite-app

graphite-app Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

PR size/scope check

This PR is over our review-size guideline.

  • Recommended: ~200 lines changed
  • Acceptable limit: up to 400 lines when well-scoped/self-contained
  • Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

  • What single problem this PR solves
  • Why the files/changes are coupled

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines +294 to 301
WHEN target_type = 'Space' THEN public.in_space(target_id, 'editor')
WHEN target_type = 'Content' THEN public.content_in_space(target_id, 'editor')
WHEN target_type = 'Concept' THEN public.concept_in_space(target_id, 'editor')
WHEN target_type = 'Document' THEN public.document_in_space(target_id, 'editor')
WHEN target_type = 'PlatformAccount' THEN public.account_in_shared_space(target_id, 'editor')
ELSE false
END;
$$;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 sync_info policy now requires editor access for SELECT due to generic_entity_access change

The sync_info_policy at packages/database/supabase/schemas/sync.sql:311 uses FOR ALL USING (public.generic_entity_access(sync_target, target_type)). Since generic_entity_access was changed from defaulting to 'reader' checks to explicitly passing 'editor' (packages/database/supabase/schemas/sync.sql:294-298), SELECT on sync_info now requires editor access. Every other table in this PR was given separate policies for SELECT (reader) vs write (editor), but sync_info was not refactored — its FOR ALL policy implicitly inherited the tighter requirement. This means readers can no longer view sync status. The old behavior (readers could INSERT/UPDATE/DELETE sync_info) was clearly wrong, but losing SELECT visibility for readers may be unintentional.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's natural for sync to require editor access.

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.

2 participants