Skip to content

fix(neon-auth): 020 — FORCE RLS + complete write policies (split from #245)#256

Open
chitcommit wants to merge 2 commits into
mainfrom
fix/neon-auth-rls-020-split
Open

fix(neon-auth): 020 — FORCE RLS + complete write policies (split from #245)#256
chitcommit wants to merge 2 commits into
mainfrom
fix/neon-auth-rls-020-split

Conversation

@chitcommit

Copy link
Copy Markdown
Contributor

Summary

Split from PR #245 (hotfix/020-neon-auth-rls), which mixes this Neon RLS security fix with unrelated broker-primitives work. This PR contains only migration 020 — a clean cherry-pick of the RLS hotfix with critical corrections.

Original regression: PR #241 was admin-merged on 2026-06-09 through a failing governance check. Migration 019 was never applied to production (restless-grass-40598426 / ChittyOS-Core). All 9 neon_auth tables have rowsecurity=false in prod today.

Why PR #245's version is still inert (critical finding):

The runtime connection (NEON_DATABASE_URL) connects as neondb_owner, which owns the neon_auth tables. In Postgres, table owners bypass RLS unless FORCE ROW LEVEL SECURITY is set on the table. PR #245 issued only ENABLE ROW LEVEL SECURITY — never FORCE. The 9 policies would be applied but never consulted at runtime. User data would still be cross-visible in production.

Evidence: scripts/e2e/neon-auth-rls.mjs:112-124 explicitly creates a shadow non-owner role (SET LOCAL ROLE) to work around owner-bypass when testing — the test authors knew about the bypass. src/auth/neon-user-store.js:20-26 asserts "This module never bypasses RLS" — that comment is factually incorrect for an owner-role connection and should be corrected in a follow-up.

Scope corrections vs PR #245:

Concern PR #245 This PR
FORCE ROW LEVEL SECURITY Missing Added on all 7 ChittyConnect-owned tables
neon_auth.jwks RLS enable Enabled (no policy = deny-all under FORCE) Not touched — ChittyAuth-owned; no ChittyConnect code path reads it from DB (jwks-verify.js uses remote HTTP JWKS)
neon_auth.project_config RLS enable Enabled (deny-all under FORCE) Not touched — ChittyAuth-owned
INSERT policy on user Missing Added (user_self_insert) — required for POST /me upsert
INSERT policy on organization Missing Added (organization_self_insert) — required for POST /organizations
INSERT policy on member Missing Added (member_self_insert) — required for org-create first-member row
Total policies 9 12

Policy inventory (12 total):

  • user: user_self_select (SELECT), user_self_update (UPDATE), user_self_insert (INSERT)
  • account: account_self_all (ALL)
  • session: session_self_all (ALL)
  • verification: verification_self_all (ALL)
  • organization: organization_member_select (SELECT), organization_self_insert (INSERT)
  • member: member_self_org_select (SELECT), member_self_insert (INSERT)
  • invitation: invitation_org_select (SELECT), invitation_self_email (SELECT)

SQL parse validation

Verified via structural analysis (no local Postgres available):

  • 1 BEGIN / 1 COMMIT
  • 7 ENABLE ROW LEVEL SECURITY (ChittyConnect-owned tables only)
  • 7 FORCE ROW LEVEL SECURITY (same 7 tables)
  • 13 DROP POLICY IF EXISTS / 12 CREATE POLICY (idempotent)
  • 3 INSERT policies (user, organization, member) covering all write paths in neon-user-store.js

Full Neon-branch validation (scripts/e2e/neon-auth-rls.mjs) is blocked on credential provisioning for a branch clone — per task hand-off to chittyconnect-concierge. The e2e test's shadow-role approach still validates FORCE enforcement correctly since SET LOCAL ROLE to a non-owner exercises the policy path.

Post-apply verification

-- Check FORCE is set (the discriminating query PR #245 lacked):
SELECT t.tablename, t.rowsecurity, c.relforcerowsecurity
  FROM pg_tables t
  JOIN pg_class c ON c.relname = t.tablename
  JOIN pg_namespace n ON n.oid = c.relnamespace AND n.nspname = 'neon_auth'
 WHERE t.schemaname = 'neon_auth'
 ORDER BY t.tablename;
-- Expect: 7 rows, all rowsecurity=true AND relforcerowsecurity=true
-- jwks and project_config must NOT appear (not touched by this migration)

SELECT tablename, policyname, cmd
  FROM pg_policies WHERE schemaname = 'neon_auth'
 ORDER BY tablename, policyname;
-- Expect: 12 policies

What stays in #245

The broker-primitives surface (src/api/routes/broker-primitives.js, src/schemas/v1/, migrations/019_policy_resolve_extensions.sql, tests/api/v1/broker-primitives.test.js, package changes) remains untouched on hotfix/020-neon-auth-rls for separate review.

Fixes: prod regression from #241
Splits: #245 (broker-primitives work remains there)
Refs: restless-grass-40598426 (ChittyOS-Core Neon project)

🤖 Generated with Claude Code

Prod regression from PR #241 (admin-merged, never applied to prod):
all 9 neon_auth tables have rowsecurity=false in production on project
restless-grass-40598426 (ChittyOS-Core).

Split from PR #245 (hotfix/020-neon-auth-rls) which mixed this RLS
hotfix with unrelated broker-primitives work. This branch contains
ONLY migration 020 — no broker code, no schema extensions, no deps.

Critical fix over PR #245's version — adds FORCE ROW LEVEL SECURITY:
The runtime connection role (neondb_owner) owns the neon_auth tables.
Postgres exempts table owners from RLS unless FORCE is set. PR #245's
ALTER TABLE ... ENABLE ROW LEVEL SECURITY was inert at runtime — all 9
policies applied to a production owner-role connection were bypassed.
Confirmed by: e2e test (scripts/e2e/neon-auth-rls.mjs:112-124) creates
a shadow non-owner role to work around owner-bypass, implicitly
acknowledging the bypass. Comment at neon-user-store.js:20-26 asserts
"This module never bypasses RLS" — that comment is incorrect given the
owner role; flagged in PR description.

Scope corrections vs PR #245:
- Removed RLS enable on neon_auth.jwks + neon_auth.project_config
  (ChittyAuth-owned; RLS enable with no policy = deny-all under FORCE;
  jwks-verify.js reads from remote HTTP JWKS, not the DB table, so
  no ChittyConnect path reads these; ChittyAuth must own policy DDL)
- Added INSERT policy on neon_auth.user (POST /me upsert path)
- Added INSERT policy on neon_auth.organization (POST /organizations)
- Added INSERT policy on neon_auth.member (org-create adds first member)
Total policies: 12 (was 9 in PR #245 — missing write-path coverage)

Refs: #241 (prod regression), #245 (split source)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 16, 2026 17:11
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@chitcommit, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 33 minutes and 37 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7a5a3c8f-a490-4160-8e1d-e06f966ea636

📥 Commits

Reviewing files that changed from the base of the PR and between 4ccf15b and 96909c9.

📒 Files selected for processing (2)
  • migrations/020_neon_auth_rls_hotfix.sql
  • src/auth/neon-user-store.js
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/neon-auth-rls-020-split

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
chittyconnect 96909c9 Jun 17 2026, 03:41 PM

Copilot AI 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.

Pull request overview

This PR adds a production hotfix migration (020) for the Neon Postgres neon_auth schema to ensure Row Level Security is actually enforced under the runtime owner-role connection by enabling and forcing RLS on the 7 ChittyConnect-owned tables, along with a complete policy set needed for current write paths.

Changes:

  • Adds ALTER TABLE ... ENABLE ROW LEVEL SECURITY and ... FORCE ROW LEVEL SECURITY for the 7 ChittyConnect-owned neon_auth tables.
  • Installs/refreshes 12 idempotent RLS policies, including missing INSERT policies for the current app write paths.
  • Ensures neon_auth."user".chittyDid and its partial unique index exist (to support policy predicates and upsert behavior).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +257 to +264
CREATE POLICY member_self_insert ON neon_auth.member
FOR INSERT
WITH CHECK (
"userId" IN (
SELECT "id" FROM neon_auth."user"
WHERE "chittyDid" = current_setting('request.jwt.claim.sub', true)
)
);
@chitcommit

Copy link
Copy Markdown
Contributor Author

Live Neon validation — migration is necessary but NOT sufficient

Validated migration 020 on a throwaway Neon branch (br-sparkling-cherry-aeh6918f, forked from default) of project restless-grass-40598426.

Structural — PASS

  • Baseline: all 9 neon_auth tables rowsecurity=false (confirms the live regression).
  • After 020: 7 ChittyConnect-owned tables rowsecurity=true AND relforcerowsecurity=true; 12 policies; jwks/project_config untouched (0). ✅

Behavioral — policies are CORRECT, but runtime role defeats them

  • Seeded two users (Alpha, Bravo), each self-inserting under its own request.jwt.claim.sub.
  • Read under a non-BYPASSRLS shadow role with Alpha's claim → returns only Alpha's row (Bravo hidden). Policies isolate correctly. ✅
  • Read under the actual runtime role neondb_owner with Alpha's claim → returns both rows. ❌

Root cause (deeper than owner-bypass):

SELECT rolbypassrls FROM pg_roles WHERE rolname='neondb_owner';  -- => true

neondb_owner (the role behind NEON_DATABASE_URL) has rolbypassrls = true. FORCE ROW LEVEL SECURITY does NOT override a role's BYPASSRLS attribute — only the table-owner exemption. So even with this migration applied, prod still bypasses every policy. The #241 regression is not closed by the migration alone.

Required companion change (blocks effectiveness, not this PR's DDL):
Provision a dedicated non-owner application role without BYPASSRLS, GRANT SELECT/INSERT/UPDATE ON neon_auth.* to it, and repoint NEON_DATABASE_URL to that role. This is a ChittyConnect connection/credential change (chico/concierge). The migration in this PR is correct and should land — but it must not be marked as 'fixing the regression' until the runtime role is switched.

Test branch will be deleted after review. Validation run by ChittyConnect session (no mocks — real Neon branch).

…uard

Per the Neon Auth ownership-split authority, the user-store must connect as
the non-BYPASSRLS role protected_app_rw — not neondb_owner (bypasses RLS even
under FORCE) and not the neon_auth owner role (would grant ChittyConnect
authority over ChittyAuth's jwks/project_config).

- migration 020: add USAGE + least-privilege DML grants to protected_app_rw on
  the 7 ChittyConnect-owned neon_auth tables.
- neon-user-store withRlsBinding: fail closed (rls_not_enforced) if the
  connected role has rolbypassrls — makes the inert-RLS regression impossible to
  reintroduce silently. Enforcement is verified at runtime, not assumed.
- refresh the stale module comment (FORCE + protected_app_rw model).

Validated live on Neon branch br-sparkling-cherry-aeh6918f: as protected_app_rw,
SELECT isolates per-DID, self-INSERT allowed, foreign-INSERT raises RLS WITH
CHECK violation; role rolbypassrls=false.

Still required before merge: NEON_AUTH_DATABASE_URL secret pointing to
protected_app_rw (concierge/operator) — without it the new guard fails closed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chitcommit

Copy link
Copy Markdown
Contributor Author

Coordinated fix applied + end-to-end validated as the canonical role

Per the Neon Auth ownership-split authority, the runtime role is protected_app_rw (non-owner, rolbypassrls=false) — not neondb_owner (bypasses RLS even under FORCE) and not the neon_auth owner role (would hand ChittyConnect authority over ChittyAuth's jwks/project_config).

Pushed in 96909c9:

  • Migration 020: GRANT USAGE + least-privilege SELECT/INSERT/UPDATE/DELETE to protected_app_rw on the 7 owned tables (idempotent). FORCE kept as defense-in-depth.
  • neon-user-store.withRlsBinding: fail-closed guard — refuses (rls_not_enforced) if the connected role has rolbypassrls. Enforcement is now verified at runtime, not assumed from config; the inert-RLS regression cannot recur silently.
  • Stale module comment refreshed.

Live validation (Neon branch br-sparkling-cherry-aeh6918f, real DB, no mocks):

Check Result
7 tables ENABLE + FORCE, 12 policies, jwks/project_config untouched
As protected_app_rw (bypassrls=false), read under DID-A returns only A's row ✅
Self-INSERT (chittyDid = claim sub) allowed ✅
Foreign-INSERT (chittyDid ≠ claim) RLS WITH CHECK violation
Baseline before migration all tables rowsecurity=false (regression confirmed)

REMAINING GATING STEP before merge (ordering matters): NEON_AUTH_DATABASE_URL must be set in ChittyConnect's Worker secrets pointing to protected_app_rw's pooled connection string (currently ABSENT — verified via wrangler secret list; prod falls back to neondb_owner). Because the new guard fails closed, merging+deploying #256 before that secret is set will make the user-store return rls_not_enforced on every call. So: set the secret (broker/operator) → then merge. protected_app_rw may need a password provisioned first (Neon console / API).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

access-reviewed Access control review approved docs-approved Documentation review approved security-approved Security review approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants