feat: precomputed scopes #1728
feat: precomputed scopes #1728
Conversation
WalkthroughThis PR introduces a scope-based RLS system with UUID-scoped tracking, precomputed mode support, and event-driven scope materialization. It adds Changes
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 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 |
adityathebe
force-pushed
the
feat/precomputed-scopes
branch
from
a4519c6 to
43b8b6d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/rls_test.go`:
- Around line 69-112: Add parallel RLS tests for components, canaries,
playbooks, and views mirroring the existing config_items/config_changes cases:
use rls.Payload with Scopes, multiple Scopes (OR), WildcardScopes, and an
unknown scope and call payload.SetPostgresSessionRLS(tx) before counting rows
via tx.Model(&models.Component{}), tx.Model(&models.Canary{}),
tx.Model(&models.Playbook{}), tx.Model(&models.View{}) (and
tx.Table("component_changes") style where inheritance applies) to assert counts
(e.g., awsComponents/totalComponents/awsOrGcpComponents/etc.); ensure you create
appropriate expected count variables (aws*, total*, awsOrGcp*, unknown->0) and
mirror the exact test structure and assertions used in the existing tests for
config_items and config_changes.
🧹 Nitpick comments (3)
tests/rls_test.go (2)
35-61: Consider usingSucceed()matcher for error assertions.The test setup correctly initializes scopes, runs RLS migrations, and prepares the transaction context. However, using
To(BeNil())for error assertions is less idiomatic thanTo(Succeed()).♻️ Suggested improvement for error assertions
- Expect(DefaultContext.DB().Model(&models.ConfigItem{}).Count(&totalConfigs).Error).To(BeNil()) + Expect(DefaultContext.DB().Model(&models.ConfigItem{}).Count(&totalConfigs).Error).To(Succeed())Apply similar changes throughout the file for consistency.
63-67: Minor: ConsiderRollback()instead ofCommit()to preserve test isolation.Using
Commit()inAfterAllpersists the__scopeupdates made during test setup. If the test database is reused, this could affect other test suites. Rolling back would ensure clean state.♻️ Suggested change
AfterAll(func() { if tx != nil { - Expect(tx.Commit().Error).To(BeNil()) + Expect(tx.Rollback().Error).To(BeNil()) } })Note: This only matters if the test database persists across test runs. If the database is reset between runs,
Commit()is fine.rls/payload.go (1)
60-92: Redundant final sort ofpartsslice.The
slices.Sort(parts)on line 90 is unnecessary sincepartsis appended in a deterministic order (Scopes first, then WildcardScopes). The individual components are already sorted, so the final sort doesn't change the outcome.♻️ Suggested simplification
if len(parts) == 0 { t.fingerprint = "empty" return } - slices.Sort(parts) t.fingerprint = hash.Sha256Hex(strings.Join(parts, " | ")) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
AGENTS.mdMakefilebench/bench_test.gobench/utils_test.gorls/payload.gorls/payload_test.goschema/checks.hclschema/components.hclschema/config.hclschema/playbooks.hclschema/views.hcltests/rls_test.goviews/035_rls_utils.sqlviews/035_view_rls.sqlviews/043_scope_permission_events.sqlviews/9998_rls_enable.sql
🚧 Files skipped from review as they are similar to previous changes (6)
- schema/config.hcl
- Makefile
- AGENTS.md
- schema/components.hcl
- schema/playbooks.hcl
- views/035_rls_utils.sql
🧰 Additional context used
📓 Path-based instructions (2)
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Always usegithub.com/onsi/gomegapackage for assertions in test files
When using gomega with native go tests, use thegomega.NewWithT(t)approach for assertions
Files:
bench/utils_test.gorls/payload_test.gobench/bench_test.gotests/rls_test.go
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Only add comments if really necessary; do not add comments that simply explain the code
Comments about functions are considered good practice in Go even if they are self-explanatory
Files:
bench/utils_test.gorls/payload_test.gorls/payload.gobench/bench_test.gotests/rls_test.go
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : Tables with direct RLS policies must use the `match_scope()` function to evaluate JWT claims against row attributes (tags, agents, names, id)
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to rls/payload.go : Build RLS Payload with scopes in `rls/payload.go` and use `SetPostgresSessionRLS()` to serialize the Payload to JSON
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : RLS policies must read JWT claims from PostgreSQL session variables via `request.jwt.claims` using `current_setting('request.jwt.claims')::jsonb`
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Use PostgreSQL Row-Level Security (RLS) to enforce multi-tenant access control with JWT claims passed via PostgREST
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/9998_rls_enable.sql : Add RLS enable logic to `views/9998_rls_enable.sql` including enable RLS on the table and create the policy
Applied to files:
views/9998_rls_enable.sqlviews/043_scope_permission_events.sqlviews/035_view_rls.sqlschema/views.hcl
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : Tables with direct RLS policies must use the `match_scope()` function to evaluate JWT claims against row attributes (tags, agents, names, id)
Applied to files:
views/9998_rls_enable.sqlviews/043_scope_permission_events.sqlviews/035_view_rls.sqltests/rls_test.goschema/views.hcl
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/9999_rls_disable.sql : Add RLS disable logic to `views/9999_rls_disable.sql` including disable RLS on the table and drop the policy
Applied to files:
views/9998_rls_enable.sqlviews/043_scope_permission_events.sqlviews/035_view_rls.sqlschema/views.hcl
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : Child tables must inherit access control from their parent using `EXISTS` clauses in RLS policies
Applied to files:
views/9998_rls_enable.sqlviews/035_view_rls.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : RLS policies must read JWT claims from PostgreSQL session variables via `request.jwt.claims` using `current_setting('request.jwt.claims')::jsonb`
Applied to files:
views/9998_rls_enable.sqlviews/035_view_rls.sqltests/rls_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to rls/payload.go : Build RLS Payload with scopes in `rls/payload.go` and use `SetPostgresSessionRLS()` to serialize the Payload to JSON
Applied to files:
views/9998_rls_enable.sqlrls/payload_test.gorls/payload.gobench/bench_test.gotests/rls_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to tests/rls_test.go : Add comprehensive RLS test cases to `tests/rls_test.go` covering access granted scenarios, access denied scenarios, and edge cases
Applied to files:
views/9998_rls_enable.sqlrls/payload.gobench/bench_test.gotests/rls_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Use PostgreSQL Row-Level Security (RLS) to enforce multi-tenant access control with JWT claims passed via PostgREST
Applied to files:
views/9998_rls_enable.sqlviews/035_view_rls.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to **/*_test.go : Always use `github.com/onsi/gomega` package for assertions in test files
Applied to files:
rls/payload_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to **/*_test.go : When using gomega with native go tests, use the `gomega.NewWithT(t)` approach for assertions
Applied to files:
rls/payload_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to tests/**/*_test.go : Use resources from the dummy dataset in tests/fixtures/dummy/all.go in tests before creating new test resources
Applied to files:
rls/payload_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Always use ginkgo to run tests, never run `go test` directly
Applied to files:
rls/payload_test.go
🧬 Code graph analysis (2)
rls/payload_test.go (1)
rls/payload.go (6)
WildcardResourceScope(15-15)WildcardResourceScopePlaybook(21-21)WildcardResourceScopeConfig(18-18)WildcardResourceScopeView(22-22)Payload(26-39)WildcardResourceScopeCanary(20-20)
bench/bench_test.go (1)
rls/payload.go (1)
Payload(26-39)
🔇 Additional comments (16)
views/035_view_rls.sql (1)
11-27: LGTM! Clean refactor to centralized scope utilities.The function correctly:
- Short-circuits with wildcard access check for efficiency
- Denies access for NULL/empty grants
- Uses array overlap with
rls_scope_access()for scope matchingThe change from
STABLEtoVOLATILEis appropriate sincerls_scope_access()reads session variables that can change within a transaction.schema/views.hcl (1)
29-32: LGTM! Consistent with the PR's scope architecture.The
__scopecolumn and GIN index follow the same pattern applied to other resource tables (config_items, components, canaries, playbooks). The GIN index is the correct choice for efficient array overlap (&&) queries used in RLS policies.Also applies to: 68-71
views/9998_rls_enable.sql (2)
57-65: Consistent and secure RLS policy pattern across resources.The unified pattern
rls_has_wildcard('<resource>') OR (COALESCE(table.__scope, '{}'::uuid[]) && rls_scope_access())is correctly applied. TheCOALESCEwith empty array ensures rows withNULL__scopeare hidden by default (secure-by-default behavior).Also applies to: 133-141, 146-154, 159-167, 221-229
70-81: Child table RLS inheritance correctly maintained.The
EXISTSpattern for child tables (config_changes, config_analysis, checks, view_panels, etc.) properly delegates access control to their parent tables, ensuring the new scope-based RLS automatically propagates.Also applies to: 205-216, 234-244
schema/checks.hcl (1)
25-28: LGTM! Consistent scope column and index for canaries.The
__scopecolumn and GIN index follow the established pattern. This enables thecanaries_authRLS policy to efficiently evaluate scope overlap queries.Also applies to: 91-94
rls/payload.go (3)
15-23: LGTM! Well-defined wildcard scope constants.The
WildcardResourceScopetype and constants provide clear, type-safe identifiers for resource types that support wildcard access. This aligns with the RLS payload design documented in the learnings.
26-39: Clean payload structure with UUID-based scopes.The consolidated
Payloadstruct withScopes []uuid.UUIDandWildcardScopes []WildcardResourceScopesimplifies the previous per-resource scope model. The comments clearly explain the wildcard scope behavior and why they're not materialized into table rows.
102-135: LGTM! Secure RLS session setup.The use of
pq.QuoteLiteralfor the RLS JSON payload properly prevents SQL injection. The local vs global scope distinction is handled correctly.rls/payload_test.go (3)
10-47: LGTM! Well-structured test cases following guidelines.Tests correctly use
gomega.NewWithT(t)for assertions as per coding guidelines. Coverage includes disabled state, deterministic fingerprint computation, and empty scope handling.
49-72: Good test for deterministic sorting.This test verifies that different ordering of
ScopesandWildcardScopesproduces the same fingerprint, which is essential for caching correctness.
74-87: LGTM! Cache behavior test is correct.The test properly verifies that the fingerprint is cached after
EvalFingerprint()and doesn't change when the underlyingScopesslice is mutated. This confirms the expected caching behavior.bench/utils_test.go (2)
32-33: Consider test isolation forbenchScopeIDs.Using a package-level variable for
benchScopeIDsis acceptable for benchmark setup but couplessetupConfigsForSizeto the global state. This is fine for benchmarks but worth noting.
124-141: LGTM! Scope materialization logic is correct.The SQL update correctly:
- Handles
NULL__scopecolumns withCOALESCE- Uses JSONB containment (
@>) for tag matching- Prevents duplicate scope UUIDs with the
NOT (... @> ARRAY[?])checkbench/bench_test.go (1)
138-161: LGTM! Benchmark updated to use UUID-based scopes.The benchmark correctly uses the new
Payload.Scopesfield withbenchScopeIDspopulated during setup. The modulo cycling through scope IDs provides varied test coverage.views/043_scope_permission_events.sql (2)
35-64: No changes needed for DELETE operations.Hard deletes are not performed on the
scopesorpermissionstables in this codebase. The application uses soft-delete exclusively via thedeleted_atcolumn, which is already handled by the trigger's UPDATE logic. When a row is soft-deleted (deleted_at transitions from NULL to NOT NULL), the trigger emits a 'remove' action as intended.Likely an incorrect or invalid review comment.
1-34: LGTM! Scope materialization trigger is well-designed.The trigger correctly emits events for INSERT and UPDATE operations, with appropriate action determination for soft deletes. The upsert pattern with conflict resolution on
(name, properties)ensures idempotent event creation. The unique index onevent_queue(name, properties)is properly configured in the schema.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
adityathebe
force-pushed
the
feat/precomputed-scopes
branch
from
b568073 to
e677f4f
Compare
BenchstatBase: |
adityathebe
force-pushed
the
feat/precomputed-scopes
branch
from
e677f4f to
f5312b3
Compare
|
adityathebe
force-pushed
the
feat/precomputed-scopes
branch
from
1a3b5d2 to
c7bc2c5
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
bench/utils_test.go (1)
160-164: Misleading error message in the non-RLS branch.The error message says "failed to enable rls" but this branch runs migrations without RLS enabled.
Suggested fix
} else { if err := duty.Migrate(duty.RunMigrations(api.NewConfig(connUrl))); err != nil { - b.Fatalf("failed to enable rls: %v", err) + b.Fatalf("failed to run migrations: %v", err) } }rls/payload.go (1)
86-102: Potential SQL injection risk in role parameter.The
roleparameter is inserted directly into the SQL query usingfmt.Sprintfwithout sanitization. While the current callers use hardcoded"postgrest_api", the public methodsSetPostgresSessionRLSWithRoleandSetGlobalPostgresSessionRLSWithRoleaccept arbitrary role strings.If a caller passes an untrusted role value, this could lead to SQL injection.
🔒 Recommended fix: Sanitize the role parameter
+import "github.com/lib/pq" + func (t Payload) setPostgresSessionRLS(db *gorm.DB, local bool, role string) error { if role == "" { return fmt.Errorf("role is required") } + + // Validate role is a valid PostgreSQL identifier + // PostgreSQL role names must be valid identifiers (alphanumeric + underscore) + for _, r := range role { + if !((r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || (r >= '0' && r <= '9') || r == '_') { + return fmt.Errorf("invalid role name: %s", role) + } + } + rlsJSON, err := json.Marshal(t) if err != nil { return fmt.Errorf("failed to marshall to json: %w", err) } // ... rest of function - if err := db.Exec(fmt.Sprintf("SET %s ROLE %s", scope, role)).Error; err != nil { + if err := db.Exec(fmt.Sprintf("SET %s ROLE %s", scope, pq.QuoteIdentifier(role))).Error; err != nil { return fmt.Errorf("failed to set role: %w", err) }Alternatively, use
pq.QuoteIdentifier(role)for the role name to ensure proper escaping.
🤖 Fix all issues with AI agents
In `@api/config.go`:
- Line 15: The default DB role name DBRoleBypass currently uses a hyphen
("rls-bypasser") which breaks PostgreSQL identifier syntax; change the default
to use an underscore (e.g., "rls_bypasser") in api/config.go and update any
places that rely on DBRoleBypass accordingly (search for DBRoleBypass, db.go,
and migrate/migrate.go) so SQL identifiers no longer require quoting; if you
prefer to keep the hyphen, instead ensure all SQL generation code in db.go and
migrate/migrate.go always double-quote the identifier where it's interpolated.
In `@db.go`:
- Around line 262-266: The ALTER ROLE SQL interpolates role names without
quoting (e.g. config.Postgrest.DBRoleBypass) which breaks on hyphens; update the
Raw(...) calls that build ALTER ROLE for DBRole, DBRoleBypass and AnonDBRole to
quote the identifier with double quotes and escape any embedded double quotes in
the role value before formatting (or use a proper identifier-quoting helper), so
the SQL becomes ALTER ROLE "role-name" ... instead of unquoted text; locate the
ctx.DB().Raw(fmt.Sprintf(...)) calls that construct ALTER ROLE and apply the
quoting/escaping to each role variable.
In `@migrate/migrate.go`:
- Around line 274-284: The SQL uses unquoted role identifiers which break on
names with hyphens (config.Postgrest.DBRoleBypass); update createRole (the
function that issues CREATE ROLE and GRANT statements) to quote/escape the role
identifier in every SQL string it builds, and also change the pool.Exec call
that runs fmt.Sprintf("ALTER ROLE %s BYPASSRLS", config.Postgrest.DBRoleBypass)
to quote/escape the role name before insertion so the ALTER ROLE ... BYPASSRLS
statement uses a properly quoted identifier; ensure all SQL templates in
createRole and the ALTER ROLE invocation consistently wrap the role name as an
SQL identifier.
🧹 Nitpick comments (1)
tests/rls_test.go (1)
24-26: ThesetRLShelper swallows the error type check.Using
BeNil()is correct, but the function namesetRLSdoesn't indicate it will fail the test on error. Consider renaming or documenting this side effect.♻️ Optional: Make the assertion clearer
-func setRLS(tx *gorm.DB, payload rls.Payload) { - Expect(payload.SetPostgresSessionRLS(tx)).To(BeNil()) -} +func setRLS(tx *gorm.DB, payload rls.Payload) { + Expect(payload.SetPostgresSessionRLS(tx)).To(Succeed()) +}Using
Succeed()is more idiomatic for error checks in Gomega.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.github/workflows/benchmark.ymlAGENTS.mdMakefileapi/config.gobench/bench_test.gobench/utils_test.godb.gomigrate/migrate.gorls/payload.gorls/payload_test.goschema/checks.hclschema/components.hclschema/config.hclschema/playbooks.hclschema/views.hcltests/rls_test.gotypes/resource_selector.goviews/035_rls_utils.sqlviews/035_view_rls.sqlviews/043_scope_permission_events.sqlviews/9998_rls_enable.sql
💤 Files with no reviewable changes (2)
- types/resource_selector.go
- .github/workflows/benchmark.yml
🚧 Files skipped from review as they are similar to previous changes (5)
- schema/checks.hcl
- AGENTS.md
- schema/components.hcl
- schema/playbooks.hcl
- views/043_scope_permission_events.sql
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Only add comments if really necessary; do not add comments that simply explain the code
Comments about functions are considered good practice in Go even if they are self-explanatory
Files:
db.gomigrate/migrate.gorls/payload_test.gobench/utils_test.gotests/rls_test.gobench/bench_test.gorls/payload.goapi/config.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Always usegithub.com/onsi/gomegapackage for assertions in test files
When using gomega with native go tests, use thegomega.NewWithT(t)approach for assertions
Files:
rls/payload_test.gobench/utils_test.gotests/rls_test.gobench/bench_test.go
🧠 Learnings (15)
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to rls/payload.go : Build RLS Payload with scopes in `rls/payload.go` and use `SetPostgresSessionRLS()` to serialize the Payload to JSON
Applied to files:
migrate/migrate.gorls/payload_test.goviews/9998_rls_enable.sqltests/rls_test.gobench/bench_test.gorls/payload.goviews/035_rls_utils.sqlviews/035_view_rls.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to tests/rls_test.go : Add comprehensive RLS test cases to `tests/rls_test.go` covering access granted scenarios, access denied scenarios, and edge cases
Applied to files:
rls/payload_test.gotests/rls_test.gobench/bench_test.gorls/payload.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to **/*_test.go : Always use `github.com/onsi/gomega` package for assertions in test files
Applied to files:
rls/payload_test.goMakefile
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to **/*_test.go : When using gomega with native go tests, use the `gomega.NewWithT(t)` approach for assertions
Applied to files:
rls/payload_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Always use ginkgo to run tests, never run `go test` directly
Applied to files:
rls/payload_test.goMakefile
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to tests/**/*_test.go : Use resources from the dummy dataset in tests/fixtures/dummy/all.go in tests before creating new test resources
Applied to files:
rls/payload_test.goMakefiletests/rls_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : Tables with direct RLS policies must use the `match_scope()` function to evaluate JWT claims against row attributes (tags, agents, names, id)
Applied to files:
schema/views.hclviews/9998_rls_enable.sqltests/rls_test.gorls/payload.goviews/035_rls_utils.sqlviews/035_view_rls.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/9998_rls_enable.sql : Add RLS enable logic to `views/9998_rls_enable.sql` including enable RLS on the table and create the policy
Applied to files:
schema/views.hclviews/9998_rls_enable.sqltests/rls_test.goviews/035_rls_utils.sqlviews/035_view_rls.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to **/zz_generated.deepcopy.go : Never manually edit `zz_generated.deepcopy.go` files; use `make generate` to generate them
Applied to files:
Makefile
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Use ginkgo with `--label-filter='!ignore_local'` flag to run tests in a package
Applied to files:
Makefile
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Use `ginkgo -focus "TestName" -r` to run a specific test
Applied to files:
Makefile
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/9999_rls_disable.sql : Add RLS disable logic to `views/9999_rls_disable.sql` including disable RLS on the table and drop the policy
Applied to files:
views/9998_rls_enable.sqltests/rls_test.goviews/035_rls_utils.sqlviews/035_view_rls.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : Child tables must inherit access control from their parent using `EXISTS` clauses in RLS policies
Applied to files:
views/9998_rls_enable.sqltests/rls_test.goviews/035_view_rls.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : RLS policies must read JWT claims from PostgreSQL session variables via `request.jwt.claims` using `current_setting('request.jwt.claims')::jsonb`
Applied to files:
views/9998_rls_enable.sqltests/rls_test.goviews/035_rls_utils.sqlviews/035_view_rls.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Use PostgreSQL Row-Level Security (RLS) to enforce multi-tenant access control with JWT claims passed via PostgREST
Applied to files:
views/9998_rls_enable.sqlrls/payload.goviews/035_rls_utils.sqlviews/035_view_rls.sql
🧬 Code graph analysis (3)
rls/payload_test.go (1)
rls/payload.go (1)
Payload(16-24)
tests/rls_test.go (6)
rls/payload.go (1)
Payload(16-24)tests/setup/common.go (1)
DefaultContext(53-53)tests/fixtures/dummy/all.go (2)
Logistics(468-477)LogisticsAPICanary(635-641)models/playbooks.go (1)
Playbook(93-107)tests/fixtures/dummy/playbooks.go (2)
EchoConfig(9-34)RestartPod(36-61)tests/fixtures/dummy/views.go (2)
PodView(60-128)ViewDev(130-161)
bench/bench_test.go (1)
rls/payload.go (1)
Payload(16-24)
🪛 GitHub Actions: Test
migrate/migrate.go
[error] 79-79: failed to grant postgrest roles: ERROR: syntax error at or near '-' (SQLSTATE 42601)
🔇 Additional comments (20)
Makefile (1)
15-18: LGTM!The pre-test cleanup step is a sensible addition to ensure a clean state before running tests. Removing previously downloaded git directories prevents their unit tests from being inadvertently executed. Based on learnings, this correctly integrates with the ginkgo-based test workflow.
schema/views.hcl (1)
29-32: LGTM!The
__scopecolumn and corresponding GIN index are well-structured for the RLS scope-based access pattern. The GIN index will efficiently support the array overlap (&&) queries used inrls_scope_access()checks.Also applies to: 68-71
schema/config.hcl (1)
311-314: LGTM!The
__scopecolumn and GIN index additions toconfig_itemsare consistent with the broader RLS scope pattern introduced in this PR. This will enable efficient scope-based filtering for config items.Also applies to: 415-418
api/config.go (1)
126-146: LGTM on the struct definition and documentation.The field comments clearly document the purpose of each role. The struct organization is clean.
views/035_view_rls.sql (1)
18-23: LGTM!The updated
check_view_grantsfunction correctly usesrls_scope_access()to check scope overlap. TheEXISTSpattern withjsonb_array_elements_textis an efficient approach for checking array membership. TheVOLATILEmarking is appropriate since it depends on session state.bench/bench_test.go (1)
164-186: LGTM!The benchmark correctly migrates to the UUID-based
Scopespayload structure. The cycling throughbenchScopeIDswith modulo ensures varied scope testing, and error messages are appropriately updated.bench/utils_test.go (1)
124-141: LGTM with a minor observation.The scope materialization logic correctly associates UUIDs with tags and updates the
__scopecolumn. The idempotency check (NOT (__scope @> ARRAY[?])) is technically unnecessary sinceuuid.New()generates unique UUIDs each time, but it's a harmless safeguard that doesn't impact benchmark correctness.rls/payload_test.go (1)
10-84: LGTM!Tests correctly migrate to UUID-based
Scopesand maintain comprehensive coverage of fingerprint functionality. The use ofgomega.NewWithT(t)follows the coding guidelines. Test cases appropriately cover edge cases including empty scopes, deterministic ordering, and caching behavior.views/035_rls_utils.sql (1)
1-17: Verify upstream payload construction for scope validation.The function correctly extracts scope UUIDs from JWT claims using
current_setting('request.jwt.claims')::jsonb. TheSTABLEmarking is appropriate since session settings don't change within a transaction, andSECURITY INVOKERensures proper permission context.However, the
::uuidcast on line 13 will raise a runtime error ifscopescontains non-UUID values. Verify that the upstream payload construction inrls/payload.gousingSetPostgresSessionRLS()validates and guarantees only valid UUIDs are serialized to the JWT claimsscopesfield.views/9998_rls_enable.sql (4)
54-61: Clean and consistent scope-based policy pattern.The simplified RLS policy using
__scope && rls_scope_access()is a good approach. This relies on the GIN index on__scopefor performance.
143-170: The playbook_runs policy logic is comprehensive.The policy correctly enforces:
- User must have access to the parent playbook
- If run has a config_id, user must have access to that config
- If run has a check_id, user must have access to that check
The
IS NULL OR EXISTSpattern correctly handles optional foreign keys.
207-234: Security invoker settings ensure views inherit RLS from underlying tables.The
security_invoker = truesetting on all these views ensures RLS policies are evaluated for the current user rather than the view owner. This is the correct approach for multi-tenant access control.
102-114: Verify ifconfig_component_relationshipsshould check access to both sides of the relationship.Currently, the policy only checks if the user has access to the
config_id. This means a user can see a relationship if they have access to the config, even if they lack access to the component. If both sides should be protected equally, add a check forcomponent_id:EXISTS (SELECT 1 FROM config_items WHERE config_items.id = config_component_relationships.config_id) AND EXISTS (SELECT 1 FROM components WHERE components.id = config_component_relationships.component_id)Check if other similar relationship tables in the codebase verify access to both sides to determine the intended access control pattern.
tests/rls_test.go (4)
46-55: SQL injection risk in test helpers mitigated by context.The
resetScopesandassignScopefunctions usefmt.Sprintffor table names. While this is generally risky, it's acceptable in test code where table names are hardcoded strings. Thewhereclause arguments are properly parameterized.
64-112: Good test coverage for config_items scope-based RLS.The test correctly:
- Sets up scope IDs before tests
- Resets and assigns scopes to specific rows
- Tests both roles (
postgrest_anon,postgrest_api)- Covers no scopes (0 results), single scope, and combined scopes (OR)
Based on learnings, this aligns with the expected comprehensive RLS test cases.
215-271: Tests verify RLS inheritance for checks via canaries.This correctly tests that checks inherit access control from their parent canary via the EXISTS clause in the RLS policy.
273-341: Tests verify RLS inheritance for view_panels via views.This correctly tests that view_panels inherit access control from their parent view via the EXISTS clause in the RLS policy.
rls/payload.go (3)
15-24: Clean simplified Payload structure.The transition from complex per-resource-type scopes to a single
[]uuid.UUIDaligns with the precomputed scopes approach mentioned in the PR objectives. This should improve query performance by eliminating the complexmatch_scope()function evaluations.
47-57: Fingerprint computation is deterministic and correct.Sorting the scope UUIDs before hashing ensures consistent fingerprints regardless of input order. Converting UUIDs to strings before sorting and joining is the correct approach.
68-84: Good API design with default and custom role variants.The separation of
SetPostgresSessionRLS(uses defaultpostgrest_apirole) andSetPostgresSessionRLSWithRole(custom role) provides both convenience and flexibility. The local vs global variants handle transaction-scoped vs session-scoped settings appropriately.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
adityathebe
force-pushed
the
feat/precomputed-scopes
branch
from
c7bc2c5 to
442560f
Compare
adityathebe
force-pushed
the
feat/precomputed-scopes
branch
from
442560f to
5dc9494
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@views/9998_rls_enable.sql`:
- Around line 119-123: The policies components_auth, canaries_auth,
playbooks_auth, and views_auth currently block rows where __scope is NULL;
update each policy's USING clause to treat NULL __scope as non-restrictive by
either coalescing __scope to an empty array before the overlap check (e.g., use
coalesce(<table>.__scope, ARRAY[]::text[]) && rls_scope_access()) or by adding
an OR <table>.__scope IS NULL condition so rows with NULL __scope are allowed;
apply this change to the components_auth, canaries_auth, playbooks_auth and
views_auth policy definitions that call rls_scope_access().
🧹 Nitpick comments (5)
views/035_rls_utils.sql (1)
8-15: Consider defensive handling for malformed JWT claims.The function will throw a runtime error if:
- The
scopesfield exists but is not a JSON array- Any element in the
scopesarray is not a valid UUIDWhile JWT claims are typically controlled server-side, a malformed payload could cause query failures. Consider wrapping the extraction in an exception handler:
♻️ Proposed defensive implementation
CREATE OR REPLACE FUNCTION rls_scope_access() RETURNS UUID[] AS $$ DECLARE jwt_claims TEXT; BEGIN jwt_claims := current_setting('request.jwt.claims', TRUE); IF jwt_claims IS NULL OR jwt_claims = '' THEN RETURN '{}'::uuid[]; END IF; - RETURN COALESCE( - ARRAY(SELECT jsonb_array_elements_text(jwt_claims::jsonb -> 'scopes')::uuid), - '{}'::uuid[] - ); + BEGIN + RETURN COALESCE( + ARRAY(SELECT jsonb_array_elements_text(jwt_claims::jsonb -> 'scopes')::uuid), + '{}'::uuid[] + ); + EXCEPTION WHEN OTHERS THEN + -- Return empty array on malformed claims (invalid UUID, non-array scopes, etc.) + RETURN '{}'::uuid[]; + END; END; $$ LANGUAGE plpgsql STABLE SECURITY INVOKER;migrate/migrate.go (1)
236-261: LGTM - Proper identifier quoting fixes SQL injection risk and syntax errors.Role identifiers are now properly quoted using
pq.QuoteIdentifier, addressing the previous pipeline failure with hyphenated role names.For consistency, consider using
pq.QuoteIdentifierfor the user parameter as well on line 249:♻️ Optional consistency improvement
- if _, err := db.Exec(fmt.Sprintf(`GRANT %s TO "%s"`, pq.QuoteIdentifier(roleName), user)); err != nil { + if _, err := db.Exec(fmt.Sprintf(`GRANT %s TO %s`, pq.QuoteIdentifier(roleName), pq.QuoteIdentifier(user))); err != nil {rls/payload.go (1)
68-84: Consider parameterizing the default role.The convenience methods hardcode
"postgrest_api"as the role. While this works, it creates a coupling that could diverge fromapi.DefaultConfig.Postgrest.DBRole.If the default role name ever changes in configuration, these methods would need updating. Consider whether a package-level constant or accessing the config would be more maintainable:
♻️ Optional: Extract default role as constant
+const DefaultDBRole = "postgrest_api" + // Injects the payload as local parameter func (t Payload) SetPostgresSessionRLS(db *gorm.DB) error { - return t.setPostgresSessionRLS(db, true, "postgrest_api") + return t.setPostgresSessionRLS(db, true, DefaultDBRole) } // Injects the payload as sessions parameter func (t Payload) SetGlobalPostgresSessionRLS(db *gorm.DB) error { - return t.setPostgresSessionRLS(db, false, "postgrest_api") + return t.setPostgresSessionRLS(db, false, DefaultDBRole) }tests/rls_test.go (2)
46-55: Consider using parameterized queries for table names.While this is test code with hardcoded table names, using
fmt.Sprintfto interpolate table names into SQL queries is a pattern that could be problematic if replicated elsewhere. Since table names cannot be parameterized in prepared statements, this is acceptable here, but worth noting for consistency.
67-68: Unused variables:totalConfigsandtotalComponents.These variables are declared and populated but never used in the test assertions. If they're intentionally kept for potential future wildcard scope tests, consider adding a comment. Otherwise, they can be removed to reduce clutter.
Also applies to: 117-118
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.github/workflows/benchmark.ymlAGENTS.mdMakefileapi/config.gobench/bench_test.gobench/utils_test.godb.gomigrate/migrate.gorls/payload.gorls/payload_test.goschema/checks.hclschema/components.hclschema/config.hclschema/playbooks.hclschema/views.hcltests/rls_test.gotypes/resource_selector.goview/db.goviews/035_rls_utils.sqlviews/035_view_rls.sqlviews/043_scope_permission_events.sqlviews/9998_rls_enable.sql
💤 Files with no reviewable changes (2)
- types/resource_selector.go
- .github/workflows/benchmark.yml
🚧 Files skipped from review as they are similar to previous changes (7)
- schema/checks.hcl
- Makefile
- db.go
- views/035_view_rls.sql
- schema/config.hcl
- views/043_scope_permission_events.sql
- AGENTS.md
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Only add comments if really necessary; do not add comments that simply explain the code
Comments about functions are considered good practice in Go even if they are self-explanatory
Files:
view/db.gobench/bench_test.gomigrate/migrate.gobench/utils_test.gorls/payload.gotests/rls_test.goapi/config.gorls/payload_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Always usegithub.com/onsi/gomegapackage for assertions in test files
When using gomega with native go tests, use thegomega.NewWithT(t)approach for assertions
Files:
bench/bench_test.gobench/utils_test.gotests/rls_test.gorls/payload_test.go
🧠 Learnings (13)
📓 Common learnings
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : Tables with direct RLS policies must use the `match_scope()` function to evaluate JWT claims against row attributes (tags, agents, names, id)
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to rls/payload.go : Build RLS Payload with scopes in `rls/payload.go` and use `SetPostgresSessionRLS()` to serialize the Payload to JSON
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : RLS policies must read JWT claims from PostgreSQL session variables via `request.jwt.claims` using `current_setting('request.jwt.claims')::jsonb`
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/9998_rls_enable.sql : Add RLS enable logic to `views/9998_rls_enable.sql` including enable RLS on the table and create the policy
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : Child tables must inherit access control from their parent using `EXISTS` clauses in RLS policies
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Use PostgreSQL Row-Level Security (RLS) to enforce multi-tenant access control with JWT claims passed via PostgREST
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/9998_rls_enable.sql : Add RLS enable logic to `views/9998_rls_enable.sql` including enable RLS on the table and create the policy
Applied to files:
views/9998_rls_enable.sqlview/db.gotests/rls_test.goviews/035_rls_utils.sqlschema/views.hcl
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/9999_rls_disable.sql : Add RLS disable logic to `views/9999_rls_disable.sql` including disable RLS on the table and drop the policy
Applied to files:
views/9998_rls_enable.sqlview/db.gotests/rls_test.goviews/035_rls_utils.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : Tables with direct RLS policies must use the `match_scope()` function to evaluate JWT claims against row attributes (tags, agents, names, id)
Applied to files:
views/9998_rls_enable.sqlview/db.gomigrate/migrate.gotests/rls_test.goviews/035_rls_utils.sqlschema/views.hcl
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : Child tables must inherit access control from their parent using `EXISTS` clauses in RLS policies
Applied to files:
views/9998_rls_enable.sqlview/db.gotests/rls_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to views/**/*.sql : RLS policies must read JWT claims from PostgreSQL session variables via `request.jwt.claims` using `current_setting('request.jwt.claims')::jsonb`
Applied to files:
views/9998_rls_enable.sqlview/db.gomigrate/migrate.gotests/rls_test.goviews/035_rls_utils.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to rls/payload.go : Build RLS Payload with scopes in `rls/payload.go` and use `SetPostgresSessionRLS()` to serialize the Payload to JSON
Applied to files:
views/9998_rls_enable.sqlbench/bench_test.gomigrate/migrate.gorls/payload.gotests/rls_test.gorls/payload_test.goviews/035_rls_utils.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Use PostgreSQL Row-Level Security (RLS) to enforce multi-tenant access control with JWT claims passed via PostgREST
Applied to files:
views/9998_rls_enable.sqlview/db.gorls/payload.goviews/035_rls_utils.sql
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to tests/rls_test.go : Add comprehensive RLS test cases to `tests/rls_test.go` covering access granted scenarios, access denied scenarios, and edge cases
Applied to files:
view/db.gobench/bench_test.gorls/payload.gotests/rls_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to tests/**/*_test.go : Use resources from the dummy dataset in tests/fixtures/dummy/all.go in tests before creating new test resources
Applied to files:
tests/rls_test.gorls/payload_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to **/*_test.go : Always use `github.com/onsi/gomega` package for assertions in test files
Applied to files:
rls/payload_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Applies to **/*_test.go : When using gomega with native go tests, use the `gomega.NewWithT(t)` approach for assertions
Applied to files:
rls/payload_test.go
📚 Learning: 2025-12-11T13:04:55.228Z
Learnt from: CR
Repo: flanksource/duty PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-11T13:04:55.228Z
Learning: Always use ginkgo to run tests, never run `go test` directly
Applied to files:
rls/payload_test.go
🧬 Code graph analysis (2)
bench/bench_test.go (1)
rls/payload.go (1)
Payload(16-24)
rls/payload_test.go (1)
rls/payload.go (1)
Payload(16-24)
🔇 Additional comments (25)
schema/components.hcl (1)
200-203: LGTM - Schema changes for scope-based RLS.The
__scopecolumn withuuid[]type and GIN index are correctly configured to support the&&(overlap) operator used in RLS policy checks. The nullable column allows backward-compatible migration.Also applies to: 371-374
schema/playbooks.hcl (1)
34-37: LGTM - Consistent with scope-based RLS pattern.The
__scopecolumn and GIN index follow the same pattern as other tables, enabling efficient scope overlap checks in RLS policies.Also applies to: 73-76
schema/views.hcl (1)
29-32: LGTM - Consistent scope column and index.The
__scopecolumn and GIN index are properly configured for the views table, matching the pattern used across other resource tables.Also applies to: 68-71
views/9998_rls_enable.sql (5)
146-170: LGTM - Playbook runs policy correctly inherits access from related resources.The policy properly:
- Requires access to the parent playbook
- Conditionally requires access to related config/check if present
- Documents the intentional omission of component_id check
The
NULL OR EXISTSpattern correctly handles optional foreign keys.
66-75: LGTM - Child table policies correctly inherit from parents.The
EXISTSclause pattern properly delegates access control to the parent tables (config_items, canaries, views), following the established RLS inheritance pattern.Also applies to: 80-89, 105-114, 175-184, 198-205
94-100: LGTM - Relationship visibility requires access to both endpoints.Correctly enforces that users can only see config relationships when they have access to both the source and target config items, preventing information leakage.
54-61: The referenced bypass functions do not exist in the current implementation.The review references
rls_has_wildcard()andis_rls_disabled()functions in an EXPLAIN output, but these functions are not present in the codebase. The current RLS implementation uses a strictly scope-based approach viarls_scope_access(), which returns an empty array when JWT claims are missing—effectively blocking all access without explicit scope assignments.If admin bypass is required, you should either:
- Verify this stricter security model is intentional
- Implement an admin bypass mechanism (e.g., checking for wildcard scopes or special roles in the JWT claims)
- Handle admin access at the application or PostgREST configuration level
57-61: No action needed. The RLS policy correctly denies access to rows with NULL__scopevalues; this is the intended behavior as confirmed by the test suite and documentation. Rows without assigned scopes should be inaccessible to all users.api/config.go (2)
13-19: LGTM - Role name fix and proper defaults.The
DBRoleBypassdefault correctly uses underscore (rls_bypasser) instead of hyphen, addressing the previous SQL syntax error issue. The default configuration is well-structured.
126-147: Good documentation of role fields.The added comments clarify the purpose of each PostgREST role, improving maintainability.
migrate/migrate.go (1)
275-286: LGTM - Bypass role creation with appropriate error handling.The bypass role is created with equivalent privileges and
BYPASSRLSis granted. The error logging (rather than returning) on line 284 is reasonable sinceBYPASSRLSmay require superuser privileges that aren't always available, and the role can still function without it in some scenarios.rls/payload.go (3)
16-24: Good transition to UUID-based scopes.Using
uuid.UUIDinstead of strings provides stronger type safety and aligns with the database's UUID representation for scope identifiers.
41-58: LGTM - Deterministic fingerprint calculation.The implementation correctly converts UUIDs to strings, sorts them, and computes a hash. This ensures fingerprints are consistent regardless of input order.
86-112: LGTM - Proper role validation and secure SQL generation.Good defensive check for empty role. The SQL generation correctly uses
pq.QuoteIdentifierfor the role andpq.QuoteLiteralfor the JSON payload, preventing SQL injection.rls/payload_test.go (1)
10-83: LGTM - Comprehensive test coverage for UUID-based scopes.The tests properly cover key behaviors:
- Disabled state fingerprint
- Deterministic fingerprint computation
- Empty scopes handling
- Order-independent fingerprint (sorting)
- Fingerprint caching after initial computation
Tests correctly use
gomega.NewWithT(t)per coding guidelines.view/db.go (1)
152-158: LGTM - RLS policy correctly delegates grant checks to scope-based framework.The policy properly uses
check_view_grants(__grants)which internally validates that the row's grant UUIDs match the user's accessible scope viarls_scope_access(). The__grantscolumn is part of the view table structure and the function correctly handles null/empty grant cases.bench/bench_test.go (2)
11-11: LGTM!Import added correctly to support the new UUID-based scope model.
169-172: LGTM!The payload construction correctly uses the new
Scopesfield with UUID values, and the error message is appropriately updated to reference "scope" instead of "tag".tests/rls_test.go (5)
19-26: LGTM!The
scopeCasestruct andsetRLShelper provide a clean abstraction for the table-driven tests. Using a function forpayloadallows lazy evaluation, which is appropriate since scope IDs are generated inBeforeAll.
28-44: LGTM!Well-designed helper functions.
payloadWithScopescorrectly filters out nil pointers before constructing the payload, preventing potential issues with optional scope parameters.
64-112: LGTM!The config_items test suite is well-structured:
- Properly sets up scope assignments based on tag filtering
- Tests no scopes (expects 0), single scope, and combined scopes (OR behavior)
- Runs for both
postgrest_anonandpostgrest_apiroles- Uses
resetScopesinBeforeAllto ensure clean state
215-271: Good coverage of RLS inheritance for checks.The test correctly validates that checks inherit RLS access from their parent canaries via the scope assigned to
canariestable, not directly tochecks. This aligns with the learning that "Child tables must inherit access control from their parent usingEXISTSclauses in RLS policies."
273-341: Good coverage of RLS inheritance for view_panels.Similar to checks, the test validates that view_panels inherit access from their parent views. The combined scopes test ensures OR behavior works correctly across multiple views.
bench/utils_test.go (2)
32-33: LGTM!Package-level variable declaration for
benchScopeIDsis appropriate here since it's populated duringsetupConfigsForSizeand consumed byBenchmarkMaininbench_test.go.
124-141: LGTM!The scope materialization logic is well-implemented:
- Creates a unique scope UUID for each sample tag
- Uses
tags @> ?::jsonbfor proper JSON containment matching- Includes idempotency check to avoid duplicate scope assignments
- Properly handles null
__scopecolumns withCOALESCE- Error handling includes descriptive wrapped messages
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@views/9998_rls_enable_precomputed.sql`:
- Around line 54-234: The policies use precomputed __scope (uuid[]), but __scope
is never populated and no handler exists for
scope.materialize/permission.materialize events, so either implement a
materialization handler that listens for scope.materialize and
permission.materialize events and populates table.__scope using the same logic
as match_scope()/match_scope function (ensuring compatibility with
rls_scope_access() and the rls.precomputed_scope flag), or revert the policies
to call match_scope() directly (remove direct "__scope && rls_scope_access()"
uses in policies like components_auth, canaries_auth, playbooks_auth,
views_auth, config_items_auth and others) and disable the precomputed mode;
update the config flag handling code where rls.precomputed_scope is read to
reflect non-functional status if you choose the revert path.
| -- Policy config items | ||
| DROP POLICY IF EXISTS config_items_auth ON config_items; | ||
|
|
||
| CREATE POLICY config_items_auth ON config_items | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE (config_items.__scope && rls_scope_access()) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy config_changes | ||
| DROP POLICY IF EXISTS config_changes_auth ON config_changes; | ||
|
|
||
| CREATE POLICY config_changes_auth ON config_changes | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE EXISTS ( | ||
| -- just leverage the RLS on config_items | ||
| SELECT 1 | ||
| FROM config_items | ||
| WHERE config_items.id = config_changes.config_id | ||
| ) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy config_analysis | ||
| DROP POLICY IF EXISTS config_analysis_auth ON config_analysis; | ||
|
|
||
| CREATE POLICY config_analysis_auth ON config_analysis | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE EXISTS ( | ||
| -- just leverage the RLS on config_items | ||
| SELECT 1 | ||
| FROM config_items | ||
| WHERE config_items.id = config_analysis.config_id | ||
| ) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy config_relationships | ||
| DROP POLICY IF EXISTS config_relationships_auth ON config_relationships; | ||
|
|
||
| CREATE POLICY config_relationships_auth ON config_relationships | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE ( | ||
| -- just leverage the RLS on config_items - user must have access to both items | ||
| EXISTS (SELECT 1 FROM config_items WHERE config_items.id = config_relationships.config_id) | ||
| AND EXISTS (SELECT 1 FROM config_items WHERE config_items.id = config_relationships.related_id) | ||
| ) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy config_component_relationships | ||
| DROP POLICY IF EXISTS config_component_relationships_auth ON config_component_relationships; | ||
|
|
||
| CREATE POLICY config_component_relationships_auth ON config_component_relationships | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE EXISTS ( | ||
| -- just leverage the RLS on config_items | ||
| SELECT 1 | ||
| FROM config_items | ||
| WHERE config_items.id = config_component_relationships.config_id | ||
| ) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy components | ||
| DROP POLICY IF EXISTS components_auth ON components; | ||
|
|
||
| CREATE POLICY components_auth ON components | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE (components.__scope && rls_scope_access()) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy canaries | ||
| DROP POLICY IF EXISTS canaries_auth ON canaries; | ||
|
|
||
| CREATE POLICY canaries_auth ON canaries | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE (canaries.__scope && rls_scope_access()) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy playbooks | ||
| DROP POLICY IF EXISTS playbooks_auth ON playbooks; | ||
|
|
||
| CREATE POLICY playbooks_auth ON playbooks | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE (playbooks.__scope && rls_scope_access()) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy playbook_runs | ||
| DROP POLICY IF EXISTS playbook_runs_auth ON playbook_runs; | ||
|
|
||
| CREATE POLICY playbook_runs_auth ON playbook_runs | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE ( | ||
| -- User must have access to the playbook | ||
| EXISTS ( | ||
| SELECT 1 | ||
| FROM playbooks | ||
| WHERE playbooks.id = playbook_runs.playbook_id | ||
| ) | ||
| AND | ||
| -- AND if run has a config_id, user must have access to that config | ||
| (playbook_runs.config_id IS NULL OR EXISTS ( | ||
| SELECT 1 | ||
| FROM config_items | ||
| WHERE config_items.id = playbook_runs.config_id | ||
| )) | ||
| AND | ||
| -- AND if run has a check_id, user must have access to that check (via its canary) | ||
| (playbook_runs.check_id IS NULL OR EXISTS ( | ||
| SELECT 1 | ||
| FROM checks | ||
| WHERE checks.id = playbook_runs.check_id | ||
| )) | ||
| -- Note: component_id check omitted (phasing out topology soon) | ||
| ) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy checks | ||
| DROP POLICY IF EXISTS checks_auth ON checks; | ||
|
|
||
| CREATE POLICY checks_auth ON checks | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE EXISTS ( | ||
| -- just leverage the RLS on canaries | ||
| SELECT 1 | ||
| FROM canaries | ||
| WHERE canaries.id = checks.canary_id | ||
| ) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy views | ||
| DROP POLICY IF EXISTS views_auth ON views; | ||
|
|
||
| CREATE POLICY views_auth ON views | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE (views.__scope && rls_scope_access()) | ||
| END | ||
| ); | ||
|
|
||
| -- Policy view_panels (inherits from parent views table) | ||
| DROP POLICY IF EXISTS view_panels_auth ON view_panels; | ||
|
|
||
| CREATE POLICY view_panels_auth ON view_panels | ||
| FOR ALL TO postgrest_api, postgrest_anon | ||
| USING ( | ||
| CASE WHEN (SELECT is_rls_disabled()) THEN TRUE | ||
| ELSE EXISTS ( | ||
| SELECT 1 FROM views | ||
| WHERE views.id = view_panels.view_id | ||
| ) | ||
| END | ||
| ); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate RLS helper functions and JWT claim usage
echo "=== Searching for rls_scope_access function ==="
rg -n "rls_scope_access" -g '*.sql' -B2 -A10 | head -100
echo -e "\n=== Searching for match_scope function ==="
rg -n "match_scope" -g '*.sql' -B2 -A10 | head -100
echo -e "\n=== Searching for JWT claims usage ==="
rg -n "request\.jwt\.claims" -g '*.sql' -B1 -A1
echo -e "\n=== Searching for __scope references ==="
rg -n "__scope" -g '*.sql' | head -50Repository: flanksource/duty
Length of output: 12325
🏁 Script executed:
#!/bin/bash
# Find where __scope column is populated/materialized
echo "=== Checking scope_permission_events.sql ==="
cat -n views/043_scope_permission_events.sql
echo -e "\n=== Searching for __scope column creation ==="
rg -n "ALTER TABLE.*ADD.*__scope" -g '*.sql'
rg -n "CREATE TABLE.*__scope" -g '*.sql'
echo -e "\n=== Searching for UPDATE/INSERT with __scope assignment ==="
rg -n "UPDATE.*__scope\|INSERT.*__scope" -g '*.sql' -B2 -A2 | head -80
echo -e "\n=== Searching for triggers related to __scope ==="
rg -n "CREATE.*TRIGGER.*__scope\|__scope.*TRIGGER" -g '*.sql' -B2 -A10 | head -100Repository: flanksource/duty
Length of output: 2570
🏁 Script executed:
#!/bin/bash
# Search for __scope computation in Go code
echo "=== Searching for __scope in Go files ==="
rg -n "__scope" -g '*.go' -B3 -A3 | head -150
echo -e "\n=== Searching for scope.materialize event handler ==="
rg -n "scope\.materialize\|materialize.*scope" -g '*.go' -i -B2 -A5 | head -100
echo -e "\n=== Checking rls/payload.go per learnings ==="
cat -n rls/payload.go 2>/dev/null | head -150Repository: flanksource/duty
Length of output: 5486
🏁 Script executed:
#!/bin/bash
# Search for event processor that handles materialization
echo "=== Searching for event processor/handler ==="
rg -n "scope.materialize\|permission.materialize" -g '*.go' -i -B2 -A10
echo -e "\n=== Searching for materialization logic in Go ==="
rg -n "materialize|materialization" -g '*.go' -i | head -40
echo -e "\n=== Searching for scope computation logic ==="
rg -n "func.*Scope.*UUID|func.*Compute.*Scope|scope.*permission" -g '*.go' -i -B1 -A5 | head -100Repository: flanksource/duty
Length of output: 2619
🏁 Script executed:
#!/bin/bash
# Search more broadly for __scope updates
echo "=== Searching for actual __scope updates (not tests) ==="
rg -n "UPDATE.*__scope\|SET __scope" -g '*.go' --type-list | grep -v test | head -50
echo -e "\n=== Searching for scope permission logic ==="
rg -n "scope.*permission\|permission.*scope" -g '*.go' -i | grep -v test | head -50
echo -e "\n=== Check for scope models/database operations ==="
find . -type f -name "*.go" | xargs grep -l "type.*Scope" | grep -v test | head -20
echo -e "\n=== Look for migration files that might define __scope ==="
find . -type f \( -name "*.sql" -o -name "*.go" \) -path "*/migrations/*" | head -20
</parameter>Repository: flanksource/duty
Length of output: 843
🏁 Script executed:
#!/bin/bash
# Inspect scope model
echo "=== Checking models/scope.go ==="
cat -n models/scope.go
echo -e "\n=== Checking for scope permission relationship ==="
rg -n "Scope\|Permission" models/scope.goRepository: flanksource/duty
Length of output: 1514
🏁 Script executed:
#!/bin/bash
# Search for table definitions with __scope column
echo "=== Searching for __scope column in table definitions ==="
rg -n "ALTER TABLE.*ADD.*__scope|__scope.*uuid" -g '*.sql'
echo -e "\n=== Searching for scope assignment/computation ==="
rg -n "SELECT.*scope\|scope.*permission" -g '*.sql' -i | head -50
echo -e "\n=== Check what file defines config_items and check for __scope ==="
rg -n "CREATE TABLE config_items" -g '*.sql' -A 30 | head -80
echo -e "\n=== Search for any precomputed or materialization comments ==="
rg -n "precomputed|materialization" -g '*.sql' -B2 -A2Repository: flanksource/duty
Length of output: 1572
🏁 Script executed:
#!/bin/bash
# Check if __scope column actually exists in table definitions
echo "=== Searching for any schema definition mentioning __scope ==="
find . -type f -name "*.sql" -exec grep -l "__scope" {} \;
echo -e "\n=== Check all views files for table schema definitions ==="
ls -la views/*.sql | head -20
echo -e "\n=== Look for migration or schema files ==="
find . -type f \( -name "schema.sql" -o -name "*migration*" -o -name "*.sql" \) -path "*/migrations/*" -o -path "*/schema/*" 2>/dev/null | head -20
echo -e "\n=== Search views directory for table create statements ==="
grep -r "CREATE TABLE" views/ --include="*.sql" | grep -E "config_items|components|canaries" | head -10Repository: flanksource/duty
Length of output: 2545
🏁 Script executed:
#!/bin/bash
# Check HCL schema files for __scope column
echo "=== Searching for __scope in HCL schema files ==="
rg -n "__scope" schema/ 2>/dev/null | head -50
echo -e "\n=== Check if schema files define config_items, components, etc ==="
rg -n "config_items|components|canaries" schema/ --include="*.hcl" -B2 -A10 | head -100
echo -e "\n=== Search entire repo for where __scope is added/defined ==="
rg -n "ADD COLUMN.*__scope\|__scope.*uuid\[\]" -g '*.{sql,hcl,go}' --no-heading | head -50Repository: flanksource/duty
Length of output: 1482
🏁 Script executed:
#!/bin/bash
# Check the schema definition of __scope
echo "=== Check __scope column type in schema/config.hcl ==="
sed -n '311,320p' schema/config.hcl
echo -e "\n=== Check __scope in schema/views.hcl ==="
sed -n '29,40p' schema/views.hcl
echo -e "\n=== Check if there's any documentation about precomputed vs non-precomputed ==="
rg -n "precomputed" -g '*.md' -B2 -A2
echo -e "\n=== Search for scope event handler in other directories ==="
find . -type f -name "*.go" -exec grep -l "scope.materialize\|permission.materialize" {} \; 2>/dev/null
echo -e "\n=== Look for any processor or worker that handles events ==="
find . -type f -name "*.go" | xargs grep -l "event_queue\|EventQueue" 2>/dev/null | head -20Repository: flanksource/duty
Length of output: 1848
🏁 Script executed:
#!/bin/bash
# Check the event handler to confirm __scope is being materialized
echo "=== Inspect postq/event.go for scope materialization handling ==="
cat -n postq/event.go | head -100
echo -e "\n=== Check async_consumer.go for event processing ==="
grep -n "scope.materialize\|permission.materialize" postq/async_consumer.go -B5 -A10 2>/dev/null | head -80
echo -e "\n=== Check if there's scope materialization in models ==="
find . -type f -name "*.go" -path "*/models/*" -exec grep -l "Scope\|Permission" {} \; 2>/dev/nullRepository: flanksource/duty
Length of output: 3220
🏁 Script executed:
#!/bin/bash
# Check async_consumer for all event handlers
echo "=== Full async_consumer.go ==="
wc -l postq/async_consumer.go
cat -n postq/async_consumer.go | head -150
echo -e "\n=== Check models/permission.go for scope materialization logic ==="
cat -n models/permission.go | head -100Repository: flanksource/duty
Length of output: 7325
🏁 Script executed:
#!/bin/bash
# Search for where scope.materialize events are actually handled
echo "=== Search for event handler registration/dispatch ==="
find . -type f -name "*.go" -exec grep -l "scope.materialize\|permission.materialize" {} \; 2>/dev/null
echo -e "\n=== Check if there's an events module ==="
ls -la events/ postq/ 2>/dev/null | head -20
echo -e "\n=== Search for any updates to config_items/components with __scope ==="
find . -type f -name "*.go" -exec grep -n "config_items\|components\|canaries" {} \; 2>/dev/null | grep -i "scope\|update" | head -30
echo -e "\n=== Check if precomputed scope is actually implemented ==="
rg -n "rls\.precomputed\|precomputed.*scope" -g '*.go' -B2 -A5
echo -e "\n=== Look in main app init or startup for event handlers ==="
rg -n "scope.materialize\|permission.materialize\|WatchEvents" -g '*.go' -B2 -A2 | head -50Repository: flanksource/duty
Length of output: 1477
🏁 Script executed:
#!/bin/bash
# Check AGENTS.md more thoroughly
echo "=== Full AGENTS.md content ==="
cat -n AGENTS.md
echo -e "\n=== Check if precomputed scope is documented as incomplete/WIP ==="
rg -n "TODO\|FIXME\|WIP\|incomplete\|work.*progress" AGENTS.md -i
echo -e "\n=== Search for any configuration that enables/disables precomputed ==="
rg -n "precomputed.*scope\|rls.*precomputed" -g '*.go' -i -B2 -A2Repository: flanksource/duty
Length of output: 4375
🏁 Script executed:
#!/bin/bash
# Check if precomputed mode is actually being used
echo "=== Check if views/9998_rls_enable_precomputed.sql is even loaded ==="
rg -n "9998_rls_enable_precomputed" -g '*.sql' -g '*.go' -g '*.md'
echo -e "\n=== Check migrations to see if precomputed is enabled ==="
find . -name "*.sql" | xargs grep -l "rls_enable_precomputed" 2>/dev/null
echo -e "\n=== Check if there's any comment about unimplemented precomputed ==="
rg -n "TODO\|FIXME\|precomputed" views/9998_rls_enable_precomputed.sql -i -B1 -A1
echo -e "\n=== Verify the default RLS file is being used ==="
ls -la views/9998_rls_enable*.sql
</parameter>Repository: flanksource/duty
Length of output: 867
🏁 Script executed:
#!/bin/bash
# Check views/views.go to understand when precomputed is used
echo "=== Check views/views.go logic ==="
cat -n views/views.goRepository: flanksource/duty
Length of output: 1049
Precomputed RLS policies rely on incomplete __scope materialization—direct __scope && rls_scope_access() cannot substitute match_scope() without population logic.
The flagged policies (lines 57–63, 131–137, 142–148, 153–159, 213–219) correctly use rls_scope_access() to read JWT claims via current_setting('request.jwt.claims', TRUE). However, the precomputed approach depends entirely on __scope being populated with scope UUIDs computed by match_scope()-equivalent logic.
Verification found:
__scopecolumn (uuid[] type) is defined in schema but never populated in SQL or Go- Event system emits
scope.materializeandpermission.materializeevents (views/043_scope_permission_events.sql) but no handler exists to process them - If
rls.precomputed_scopeproperty is enabled (currently OFF by default), all direct-table RLS policies will fail silently—NULL arrays fail the&&intersection, blocking all access
Either implement the missing __scope population handler in the application, or revert to match_scope() approach documented in AGENTS.md. If precomputed mode remains WIP, mark it explicitly as non-functional until materialization is complete.
🤖 Prompt for AI Agents
In `@views/9998_rls_enable_precomputed.sql` around lines 54 - 234, The policies
use precomputed __scope (uuid[]), but __scope is never populated and no handler
exists for scope.materialize/permission.materialize events, so either implement
a materialization handler that listens for scope.materialize and
permission.materialize events and populates table.__scope using the same logic
as match_scope()/match_scope function (ensuring compatibility with
rls_scope_access() and the rls.precomputed_scope flag), or revert the policies
to call match_scope() directly (remove direct "__scope && rls_scope_access()"
uses in policies like components_auth, canaries_auth, playbooks_auth,
views_auth, config_items_auth and others) and disable the precomputed mode;
update the config flag handling code where rls.precomputed_scope is read to
reflect non-functional status if you choose the revert path.
related: flanksource/mission-control#2395
Summary by CodeRabbit
Release Notes
New Features
Database Schema
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.