Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Budget and missing-table failures bypass the stable error taxonomy, and the live SQL generation boundary needs correction.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 4
Open (5)
What changed in this PR
Adds an atomic Go executor for applying complete row-level security definitions to existing tables.
Changes:
- Adds bounded, transactional RLS replacement and convergence verification.
- Adds statement qualification, scratch introspection, reports, and outcome codes.
- Updates tests, capability metadata, safety contracts, and documentation.
| File | Description |
|---|---|
SAFETY.md |
Extends the trusted-core model for RLS execution. |
README.md |
Documents Go API support. |
.agents/checks/review.md |
Updates review guidance for RLS execution. |
pkg/statement/row_security_execution.go |
Generates qualified RLS statements. |
pkg/statement/row_security_execution_test.go |
Tests RLS statement qualification. |
pkg/statement/desired_rls.go |
Expands the declaration type’s execution scope. |
pkg/schemadiff/row_security_roundtrip.go |
Adds transactional desired-state inspection. |
pkg/schemadiff/desired.go |
Refactors scratch inspection around caller transactions. |
pkg/executor/row_security.go |
Implements atomic RLS execution. |
pkg/executor/row_security_test.go |
Tests input validation. |
pkg/executor/row_security_integration_test.go |
Covers execution, rollback, locking, and convergence. |
pkg/executor/code.go |
Adds an uncertain RLS commit outcome code. |
pkg/executor/code_test.go |
Tests outcome-code classification. |
pkg/capabilities/capabilities.yaml |
Records Go API RLS support. |
pkg/capabilities/capabilities_test.go |
Updates capability validation expectations. |
docs/tcb-model.md |
Documents the RLS declaration proof boundary. |
docs/limitations.md |
Narrows remaining RLS limitations. |
docs/invariants.md |
Defines atomic RLS invariants. |
docs/execution-model.md |
Documents the new outcome code. |
docs/declarative-row-security.md |
Updates the RLS workflow and roadmap. |
docs/capabilities.md |
Regenerates the support matrix. |
docs/atomic-row-security.md |
Documents the new API and execution contract. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| operation: "Complete table-local RLS definition (Go API only)" | ||
| tier: "t1" | ||
| status_mark: "✅" | ||
| engine_path: "native_as_is" |
Comment on lines
+50
to
+52
| // INV: RS-3 — the whole attempt has one deadline, not a fresh budget per policy. | ||
| ctx, cancel := context.WithTimeout(ctx, b.StatementTimeout) | ||
| defer cancel() |
Comment on lines
+68
to
+69
| if _, err := tx.Exec(ctx, "LOCK TABLE ONLY "+target+" IN ACCESS EXCLUSIVE MODE"); err != nil { | ||
| return RowSecurityReport{}, fmt.Errorf("lock row security target %s: %w", target, err) |
Comment on lines
+40
to
+44
| case node.GetAlterTableStmt() != nil: | ||
| node.GetAlterTableStmt().Relation.Schemaname = schema | ||
| case node.GetCreatePolicyStmt() != nil: | ||
| node.GetCreatePolicyStmt().Table.Schemaname = schema | ||
| case node.GetCommentStmt() != nil: |
Comment on lines
+84
to
+86
| // INV: RS-4 — helpers are explicitly qualified; no target-schema function | ||
| // may shadow a built-in while replaying policy expressions. | ||
| if _, err := tx.Exec(ctx, dbconn.LocalSearchPath("pg_catalog")); err != nil { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
pg-sprite can compare row-level security (RLS) definitions, but cannot yet apply them declaratively. Policy replacement needs one transaction so applications never see an intermediate access rule.
What
Add
executor.ExecuteRowSecurityfor complete RLS declarations on existing supported tables. This is a Go API; it adds no CLI flags or fingerprint convention.How
The executor supports ENABLE/DISABLE, FORCE/NO FORCE, policies, and policy comments. A converged retry does no live DDL. Scratch inspection uses the same connection.
Risk
This changes access rules and briefly blocks readers and writers with an exclusive table lock. Both lock waits and the whole attempt are bounded. Missing tables, unsupported table shapes, and mixed table/policy changes are refused. An uncertain commit response requires catalog inspection before retrying.
Grants, role membership, helper bodies, and authentication remain outside this operation. A changed definition replaces the complete policy set, including unchanged policies.
Testing
No manual testing. Regression coverage includes real DDL fault injection, deadline rollback, lock contention, concurrent policy changes, non-owner default deny, quoted names, and single-connection execution.
Bigger picture
Follows #119 and replaces the approach explored in closed #122. Orchestrators retain their existing replan and consent workflow. CLI execution and Supabase application-level validation are follow-ups.
Generated with Codex (GPT-6)