security: enforce Organization ownership and fail-closed tenant sessions - #36
Conversation
|
Warning Review limit reached
Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughAdds a bounded Organization security slice: transaction-local tenant context, Organization and ChangesOrganization RLS isolation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb219fe231
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![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
🧹 Nitpick comments (1)
engine/persistence/schema_security_manifest.yaml (1)
1-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFile is strict JSON but carries a
.yamlextension.The content is pure JSON and every consumer parses it with
json.loads(tests/unit/test_schema_security_manifest.pyline 12 andtests/integration/test_organization_isolation.pyline 886). This works today, but a future YAML-style edit (a#comment, anchor, or unquoted key) in a file named.yamlwill silently breakjson.loadsat runtime. Either rename to.json(and update the two reader paths) or switch the readers to a YAML loader.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@engine/persistence/schema_security_manifest.yaml` around lines 1 - 90, Resolve the format-extension mismatch for schema_security_manifest by either renaming the strict JSON manifest to a .json file and updating both readers in test_schema_security_manifest and test_organization_isolation, or converting those readers to a YAML loader while preserving current parsing behavior. Apply the same choice consistently across the manifest and both consumer paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 12-14: Update every README evidence/status statement that
hardcodes PostgreSQL 17, including the referenced sections, to derive or defer
the version claim to compose.yaml and the recorded evidence report. Keep the
existing validation and NOT_ACTIVE status descriptions unchanged while ensuring
future harness version changes require updating only the source-of-truth files.
---
Nitpick comments:
In `@engine/persistence/schema_security_manifest.yaml`:
- Around line 1-90: Resolve the format-extension mismatch for
schema_security_manifest by either renaming the strict JSON manifest to a .json
file and updating both readers in test_schema_security_manifest and
test_organization_isolation, or converting those readers to a YAML loader while
preserving current parsing behavior. Apply the same choice consistently across
the manifest and both consumer paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 50db834e-4d1c-4993-bb63-46f619abb766
📒 Files selected for processing (13)
AGENTS.mdREADME.mddocs/decisions/0020-stage-organization-rls-before-actor-context.mddocs/decisions/README.mdengine/persistence/__init__.pyengine/persistence/schema_security_manifest.yamlengine/persistence/tenant_context.pymigrations/versions/20260720_0002_organization_rls.pytests/integration/conftest.pytests/integration/test_migrations.pytests/integration/test_organization_isolation.pytests/unit/test_schema_security_manifest.pytests/unit/test_tenant_context.py
|
CodeRabbit manifest-format nit assessed and intentionally not renamed: |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Closes #8
Outcome
Implements the first complete PostgreSQL Organization-isolation evidence slice:
organizationsecurity root and exactly one representative tenant-ownedorganization_recordtable;USINGandWITH CHECKpredicates;organization_transaction(engine, organization_id)boundary that begins, binds, verifies, and owns the transaction before yielding a connection;ADR-0020 records why this issue proves only the Organization GUC slice.
DB-009remainsNOT_ACTIVEuntil the closed ActorContext protocol and its owning negative test exist.Two-Organization matrix
425014250142501Hard-oracle output for the slice is
wrong-Organization effect = 0andmissing-context fallback = 0. Missing-context SELECT returns zero tenant rows; every write is rejected before row filtering, includingINSERT ... SELECT ... WHERE false,UPDATE ... WHERE false, andDELETE ... WHERE false.The pool-reuse case runs 12 alternating Organization/context-free cycles on one backend PID without retries. Each context-free cycle proves both zero visible rows and a
42501zero-candidate write rejection. Normal completion and raised-exception rollback leave the GUC absent/empty; rollback also leaves zero durable rows.Database evidence
NOSUPERUSER,NOBYPASSRLS,NOINHERIT;organization,organization_record, and the invoker-security guard function;SELECT,INSERT,UPDATE,DELETEon the representative table and onlyEXECUTEon the trigger function; it has no access to the Organization root;ENABLE ROW LEVEL SECURITY,FORCE ROW LEVEL SECURITY, policy role/command/USING/WITH CHECK, statement trigger timing/events, function body/owner/security/search path, nullability, exact PK/FKs, and manifest/table equality;20260720_0002cleanly downgrades/reapplies.Mutation evidence
Each mutation was applied only to the disposable test database, observed red, restored, and followed by a clean-volume green suite:
NO FORCE ROW LEVEL SECURITY-> owner behavior and catalog tests fail;WITH CHECK (true)-> wrong-Organization insert test fails;Verification
make db-resetfrom an empty disposable database volumemake checkScope check
No User, Membership, Source, Revision, Fragment, Package, EffectiveScope, provider, index, cache, HTTP authorization, or generalized authorization framework is added.
Summary by CodeRabbit
New Features
Documentation
Tests