Restrict organization management email updates

[slashdevcorpse]

Summary

• Keep organization management_email changes behind the dedicated email-change path.
• Sync Stripe before persisting the management email and use the service-role client only for the approved final write.
• Block authenticated direct table updates to orgs.management_email with a database trigger.
• Route organization set --email through private/set_org_email and document the super-admin requirement.
• Add regressions for direct table writes and the public organization update path.

Motivation

The management email is used for billing-related communication and customer sync. Org admin update paths could change that field without the dedicated Stripe synchronization and rollback behavior.

Business Impact

This reduces billing-contact takeover risk, keeps Capgo's Stripe customer email aligned with the organization record, and preserves the existing public update endpoint while tightening the sensitive field boundary.

/claim #1667

Test Plan

• GitHub CI passed on head 18cf36e.
• bunx eslint "supabase/functions/_backend/public/organization/put.ts" "supabase/functions/_backend/private/set_org_email.ts" "tests/organization-api.test.ts" "tests/organization-put-stripe-sync.unit.test.ts" "cli/src/organization/set.ts" "cli/src/index.ts"
• bunx vitest run tests/organization-put-stripe-sync.unit.test.ts
• bun run typecheck
• bun run lint in cli/
• bun run build in cli/
• bun run test:mcp in cli/
• bun run test:bundle in cli/
• git diff --check
• Supabase-backed focused organization API regression was attempted locally, but Docker Desktop/Supabase was unavailable.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds authorization control for management_email updates. A new helper function restricts such updates to users with org.update_billing permission. The PUT endpoint conditionally invokes this check, and an integration test verifies that unauthorized users receive a 403 error.

Changes

Management Email Authorization

Layer / File(s)	Summary
Authorization Helper supabase/functions/_backend/public/organization/put.ts	New ensureManagementEmailAccess function checks org.update_billing permission and throws 403 not_authorized if access is denied.
Endpoint Integration supabase/functions/_backend/public/organization/put.ts	PUT endpoint conditionally calls the authorization helper when management_email is present in the request body.
Authorization Test tests/organization-api.test.ts	Integration test confirms org admin cannot update management_email without org.update_billing permission, expects 403 not_authorized, and verifies the database value remains unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• Cap-go/capgo#1845: Both PRs modify the same PUT /organization endpoint in supabase/functions/_backend/public/organization/put.ts with different authorization and validation logic.

Suggested labels

codex

Poem

🐰 A bunny hops down authorization lane,
Guarding management_email with RBAC's chain,
Org admins now get a friendly 403,
Only billing hearts may pass, you'll see! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main change—restricting management email updates to prevent unauthorized changes by regular org admins.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description includes all required template sections with substantial detail, though Screenshots section can be skipped for backend changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codspeed-hq]

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks¹

---

_{Comparing slashdevcorpse:codex/capgo-management-email-authz (18cf36e) with main (38e5856)²}

Footnotes

1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

2. No successful run was found on main (3d184f8) during the generation of this report, so 38e5856 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[Copilot]

Pull request overview

Restricts updates to an organization’s management_email via the public [PUT] /organization endpoint by requiring billing-level permission (org.update_billing), closing a privilege boundary where an org admin could previously change the management email through the broader settings update path.

Changes:

• Add an extra authorization gate (org.update_billing) when management_email is present in the update payload.
• Add a regression test asserting org admins cannot update management_email via [PUT] /organization.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
supabase/functions/_backend/public/organization/put.ts	Adds a dedicated permission check for management_email updates requiring org.update_billing.
tests/organization-api.test.ts	Adds a regression test ensuring org admins receive a 403 and the management email remains unchanged.

[ShravanthReddy]

I think this still leaves one side-effect bypass in place. The dedicated management-email path in supabase/functions/_backend/private/set_org_email.ts checks super-admin rights, calls updateCustomerEmail(...), then updates orgs.management_email and rolls Stripe back if the DB update fails. This PR now lets billing-level users send management_email through PUT /organization, but this handler only adds the field to updateFields; it never calls the Stripe customer-email sync path.\n\nThat means a super admin can still change the billing/management email through the broader organization update endpoint while the Stripe customer email remains stale. If this public endpoint is intended to remain a valid management-email update path, I would route it through the same updateCustomerEmail / rollback flow as set_org_email. If the dedicated endpoint is the only intended billing-email path, reject management_email here and force callers through that route. A regression with an org that has customer_id would help lock down whichever behavior is intended.

[slashdevcorpse]

CI note: the repository Run tests workflow reached setup, lint, typecheck, Supabase start/reset, DB tests, and many backend tests successfully, then the Run all backend and CLI tests step ended with The operation was canceled.

I tried to rerun the cancelled job, but GitHub returned 403 for this fork PR. Could a maintainer rerun the cancelled Run tests job when convenient?

[SpeedyArt]

I think this still leaves the permission boundary open outside this handler.

The new gate only runs for PUT /organization, but orgs is still directly updateable by admin-level users through the Supabase table policy: Allow update for auth (admin+) checks only admin rights for the whole row (supabase/schemas/prod.sql, and the latest policy migration). There is also at least one caller that still writes the protected column directly: cli/src/organization/set.ts does .from('orgs').update({ name, management_email: email }).

So an org admin with a normal authenticated Supabase client can bypass the new endpoint check and change orgs.management_email directly. This is separate from the Stripe-sync path already mentioned above: even if this endpoint’s update flow is fixed or disabled, the table policy still allows the column write.

I’d move this boundary into the database as well, either by splitting/replacing the orgs update policy or by adding a trigger/RPC-only rule that rejects management_email changes unless the caller has the billing/super-admin permission. The CLI/web direct writes for this field should then route through the dedicated private/set_org_email flow. A regression that attempts a direct .from('orgs').update({ management_email }) as an admin user would catch this.

[slashdevcorpse]

Update: I corrected the regression test fixture so it uses an org admin caller without creator/super-admin authority.

Latest head b60021d is now green: backend tests, Playwright, SonarCloud, CodSpeed, Socket, dead-code, and benchmarks all pass.

[slashdevcorpse]

Update:

• Addressed the Stripe sync bypass: PUT /organization still accepts management_email, but now requires the same super-admin boundary as the dedicated email path and calls updateCustomerEmail(...) with rollback if the DB update fails or a later name-sync rollback is needed.
• Closed the direct table/CLI bypass: added a migration trigger that blocks non-super-admin direct updates to orgs.management_email, and changed organization set --email to invoke private/set_org_email instead of writing the column directly.
• Added regression coverage for admin-level API updates and direct Supabase table updates leaving management_email unchanged.
• Verification: local targeted ESLint, root typecheck, CLI lint/build/MCP smoke/bundle tests passed. GitHub CI is green on d61100f with 25/25 checks passing.

[zinc-builds]

🔍 Security Review — Hermes Agent

Verdict: ✅ Approved — Solid defense-in-depth approach to restricting management email updates.

What the PR fixes

Previously, any org admin could update management_email directly on the orgs table or via the organization API. This bypassed the Stripe customer email sync and allowed privilege escalation — a non-super_admin could change the billing contact email. The fix adds enforcement at two layers.

✅ Good patterns

1. DB trigger as safety net: The prevent_org_management_email_non_super_admin_update trigger catches direct table writes (UPDATE orgs SET management_email = ...), even from authenticated clients bypassing the API. This is critical — API-level checks alone don't protect against direct PostgREST access.
2. API-level enforcement: ensureManagementEmailAccess() checks super_admin via check_min_rights RPC before allowing the update through the organization endpoint.
3. Stripe sync with rollback: The email update now syncs to Stripe (updateCustomerEmail) before the DB update, with rollbackStripeCustomerEmail on failure. This prevents the Stripe customer email from drifting from the org email.
4. Service role bypass: The trigger correctly allows service_role to update without restriction — needed for backend operations.
5. Clean test coverage: Tests both API-level rejection (403) and direct DB-level rejection (trigger error message assertion), with proper cleanup in finally blocks.

⚠️ Suggestions (non-blocking)

1. Race condition on Stripe sync: updateCustomerEmail is called before updateOrg. If Stripe succeeds but the DB update fails, the rollback restores the Stripe email — good. But what about the reverse? If the DB update succeeds but another concurrent request reads the org between the Stripe sync and DB commit, it sees the old email. Minor timing issue, unlikely in practice.
2. Trigger grants: The trigger function only grants EXECUTE to service_role. This is correct for a security trigger, but differs from other security definer functions that grant to authenticated too. Since this is invoked by the trigger (not called directly), service_role-only is correct — just noting the departure from convention.
3. CLI changes: The CLI now calls private/set_org_email instead of directly updating the orgs table. This is good (uses the API instead of direct DB access), but the CLI function updateOrganizationManagementEmail doesn't handle the case where the user isn't a super_admin gracefully — it just throws. Consider adding a specific error message like "Only super admins can update the management email."

Strong work — the Stripe rollback pattern is particularly well thought out.

[SpeedyArt]

Thanks, this closes the admin-level direct-write bypass I was worried about.

One residual invariant looks worth tightening: the new trigger still permits a super_admin user-context PostgREST update to orgs.management_email directly, because it returns NEW once the check_min_rights('super_admin', ...) check passes. That means an authorized super-admin can still bypass the private/set_org_email / PUT /organization Stripe sync and rollback path, leaving the Capgo org email and Stripe customer email out of sync.

If the intended invariant is that every management_email change syncs the billing customer email, I would make this column service-role/RPC-only for all user-context table writes and have the private endpoint perform the actual service-role update after it syncs Stripe. A regression as a super_admin doing a direct .from('orgs').update({ management_email }) would catch whether the table path is still bypassing the sync boundary.

[slashdevcorpse]

Pushed 77170ba to tighten the remaining table-write invariant.

Changes:

• Direct user-context updates to orgs.management_email are now blocked for both admins and super admins.
• private/set_org_email and PUT /organization still perform permission checks and Stripe sync first, then persist the final email through the service-role client.
• Added a regression for direct super-admin table updates and a unit test that verifies the public organization update path writes through the service-role client after Stripe email sync.

Validation:

• bunx vitest run tests/organization-put-stripe-sync.unit.test.ts
• bunx eslint supabase/functions/_backend/private/set_org_email.ts supabase/functions/_backend/public/organization/put.ts tests/organization-put-stripe-sync.unit.test.ts tests/organization-api.test.ts
• git diff --check

I could not run the Supabase-backed tests/organization-api.test.ts slice locally because Docker Desktop is not running in this environment; the branch CI should cover that database-backed path.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[KCDaemon]

Rechecked latest head (18cf36e). The management-email mutation path is now confined to the controlled endpoints: direct user-context table writes are blocked, the CLI routes email changes through private/set_org_email, and public org updates require super-admin rights plus Stripe email sync/rollback before the service-role write.

The added tests cover org-admin rejection, direct super-admin table-write rejection, and the service-role write path after Stripe sync. GitHub CI is green; I do not see a remaining blocker.

[austin33133-maker]

Nice tightening of the management-email path. Two correctness concerns and a smaller defense-in-depth note before this lands.

1. Concurrent management_email updates can desync Stripe and the DB

In put.ts the update flow for management_email looks like:

1. updateCustomerEmail(c, currentOrg.customer_id, body.management_email) (Stripe first)
2. updateOrg(writeSupabase, ...updateFields) (DB second)

There's an expectedCurrentName optimistic-concurrency guard for the name path, but no equivalent expectedCurrentManagementEmail for the email path. If two super-admins call PUT at the same time with different new emails on the same org:

• Both pass ensureManagementEmailAccess.
• Both run Stripe updateCustomerEmail -- order is non-deterministic, last write wins on Stripe.
• Both run updateOrg with the same orgId -- last write wins on DB.

There's no guarantee the Stripe last-write matches the DB last-write. End state: orgs.management_email says A, Stripe customer.email says B.

Suggestion: extend buildExpectedCurrentFields / expectedCurrentName to include management_email when shouldSyncStripeEmail is true, and pass expectedCurrentManagementEmail: currentOrg.management_email so the DB write fails fast (and the existing Stripe rollback kicks in) if another caller raced ahead.

A regression test that fires two concurrent PUTs with different emails for the same org and asserts the final Stripe email and orgs.management_email agree would catch this.

2. Post-update Stripe sync failure: rollback ordering can leave Stripe with the new email

Around line 117-120 in the post-update Stripe-name-sync error handler, the rollback runs:

ts try { await updateOrg(writeSupabase, body.orgId, rollbackFields, { expectedCurrentName, expectedCurrentFields }) if (shouldSyncStripeEmail && currentOrg) { await rollbackStripeCustomerEmail(c, currentOrg, stripeError) } } catch (rollbackError) { throw simpleError('cannot_update_org', 'Cannot update org', { ... }) }

If updateOrg throws (e.g. expectedCurrentName mismatch from a racing update), the rollbackStripeCustomerEmail line is never reached. Stripe is left with the new email, DB is also still on the new email (since the bulk update fields succeeded earlier), and the caller sees cannot_update_org. The outer name-sync rollback intent fails open instead of restoring Stripe.

Suggestion: invert order, or wrap each in its own try so a DB-rollback failure doesn't prevent Stripe rollback. Something like:

ts const dbRollbackError = await tryUpdateOrg(...).catch(e => e) const stripeRollbackError = shouldSyncStripeEmail ? await rollbackStripeCustomerEmail(c, currentOrg, stripeError).catch(e => e) : null if (dbRollbackError || stripeRollbackError) { throw simpleError('cannot_update_org', 'Cannot update org', { error, dbRollbackError, stripeRollbackError, }) }

3. Minor: trigger doesn't cover SECURITY DEFINER bypass

The migration's prevent_org_management_email_direct_update trigger checks auth.role() = 'service_role'. Any existing PL/pgSQL SECURITY DEFINER function that does UPDATE orgs SET management_email = ... and is owned by a privileged role (postgres/service_role) will bypass this gate. Worth a quick audit:

sql SELECT n.nspname, p.proname FROM pg_proc p JOIN pg_namespace n ON p.pronamespace = n.oid WHERE prosecdef = true AND prosrc ILIKE '%update%orgs%management_email%';

If any hits, route them through the email-sync helper or add an explicit caller-allowlist column to the trigger.

Happy to send a PR with the concurrent-update regression test and the rollback restructuring if (1) and (2) get acked.