feat(store): role delete keeps the row and refuses while a live policy uses it

[AmanGIT07]

Summary

DeleteOrganizationRole and DeleteRole now set deleted_at on the role row instead of removing it. The check that refuses the delete of a role still in use moves from the policies.role_id foreign key into the delete query. Callers get the same responses as before.

Changes

• RoleRepository.Delete runs one transaction with two statements. The first locks the live role row with SELECT ... FOR UPDATE; no row means not found. The second sets deleted_at through softDelete (feat(store): add a helper that marks rows as deleted #1959), only when no live policy references the role; no row changed means ErrRoleInUse.
• The lock makes the delete wait for a policy insert for that role that has not committed yet, the same way the old DELETE waited on the foreign key. The check then sees that policy.
• role.Service.Delete keeps its flow: read the role, delete it, remove the role's permission tuples from SpiceDB. Only its comments change. The organization delete cascade uses the same path.
• The handlers already map ErrRoleInUse to FailedPrecondition and ErrNotExist to NotFound.

Technical Details

Role reads skip deleted rows (#1938) and role names are unique among live rows only (#1961), so a deleted name can be created again and lands on a new row.

The policies.role_id foreign key stays. It never fires on the soft delete because the role row stays. A policy insert that waits for a role delete to commit still succeeds today, because the row it points at exists. The check that the role is live on the policy side comes with the policy soft delete.

roles.org_id has no foreign key, so the kept rows do not block the organization delete, which is still a hard delete.

Test Plan

• go test ./internal/store/postgres/ passes. New TestDelete cases: the row stays, a second delete reports not found, a live policy makes the delete report in use and the role stays readable, deleted policies do not block, and the delete waits for an uncommitted policy insert and then reports in use.
• go test ./core/role/ and go test ./core/deleter/ pass.
• TestOrganizationRoleDeleteInUse (e2e) gains: get returns not found, a second delete returns not found, the same name is created again with a new id, and the organization delete still works. These steps also pass against main; the repository test is what checks that the row is kept.
• Each new repository case fails when the code it covers is reverted: main's hard delete fails three of them, removing the lock fails the waiting case, removing the policy check fails the in-use and waiting cases.
• golangci-lint run reports no issues.
• End to end against a local server. The same checks ran against main on the same database, with only the binary swapped.

Check	main	this branch
Delete an organization role while a policy uses it	failed precondition	failed precondition
Get the role after the refused delete	ok	ok
Role permission tuples after the refused delete	3	3
Delete the policy, then the role	ok	ok
Get the deleted role	not found	not found
List includes the deleted role	no	no
Update the deleted role	not found	not found
Delete it again	not found	not found
Database row of the deleted role	gone	kept, deleted_at set
Role permission tuples after the delete	0	0
Create a policy with the deleted role	not found	not found
Create the same name again	ok, new id	ok, new id
Delete the organization that holds a live and a deleted role	ok, 0 role rows left	ok, 2 role rows left, both deleted_at set
Delete a platform role (AdminService) while a policy uses it	failed precondition	failed precondition
Delete the platform role once the policy is gone	ok	ok
Platform role list includes the deleted role	no	no
Delete it again	not found	not found
Database row of the deleted platform role	gone	kept, deleted_at set
Create the same platform name again	ok, new id	ok, new id
Delete while a policy insert for the role is open and uncommitted: the delete waits	yes	yes
Result once that insert commits	failed precondition	failed precondition
Get the role afterwards	ok	ok

The last three rows hold the policy insert open in a psql transaction and watch pg_stat_activity for the waiting session.

SQL Safety

• Values flow through goqu.Ex{}; the NOT EXISTS subquery is built from goqu expressions with no caller input.
• ToSQL() callers capture and forward params (query, params, err := stmt.ToSQL(); db.…Context(ctx, …, query, params...)). Never query, _, err := ….
• No ? placeholders inside single-quoted SQL literals in goqu.L.
• No new //nolint:forbidigo or // #nosec G20x annotations.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
frontier	Ready	Preview	Oct 1, 2026 6:34am UTC

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Role deletion now marks the role as deleted rather than removing its record. Roles referenced by active policies cannot be deleted; remove those policy references first. References from already-deleted policies do not prevent deletion.
  • Deleting an already-deleted role returns “Not Found.” After deletion, a role with the same name can be recreated with a new ID.

Walkthrough

Role deletion now locks the live role row and soft-deletes it only when no live policy references it. Tests cover repository outcomes, concurrent policy insertion, repeated API operations, and role recreation.

Changes

Role deletion

Layer / File(s)	Summary
Role deletion contract and transaction core/role/service.go, internal/store/postgres/role_repository.go	Service comments describe the deletion behavior. The repository locks the live role row and soft-deletes it only when no live policy references it.
Deletion behavior tests core/role/service_test.go, internal/store/postgres/role_repository_test.go, test/e2e/regression/api_test.go	Repository tests cover soft deletion, missing roles, policy references, and a concurrent policy insert. End-to-end tests check repeated operations, role recreation, and organization deletion.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Suggested reviewers: whoabhisheksah

Merge Risk: 🔵 Low · up to bed3b

Concurrent policy creation and role deletion can leave a policy referencing a deleted role. The risk is narrow but remains actionable; make live-role validation transactional with policy insertion.

Security Architecture Review

Security architecture risk: 🟡 Moderate · up to bed3b

A policy request that started before deletion can still attach to the deleted role. Successful permission cleanup prevents that binding from granting access, but interrupted or failed cleanup can leave grants effective while ordinary deletion retries report the role as missing.

Retained concerns

• Medium · security · inferred: Soft deletion allows a policy request that validated the role before deletion to persist and write authorization bindings afterward. If permission cleanup fails or is interrupted after the database commit, surviving role permissions can make those new bindings effective while ordinary role-delete retries return NotFound. Hard deletion rejected the delayed insert through the foreign key. Successful cleanup makes the binding ineffective; the standalone cleanup-failure problem predates this PR, but its delayed-grant exposure is worsened.

Security review details

Security Blast Radius

• inferred — Conditional exposure follows surviving permissions of the deleted role and the resource targets of concurrent authorized policy requests. Bearers may be users, service users, PATs, or group members. A shared role could affect multiple authorized targets; the inspected path does not establish arbitrary tenant access or a new identity bypass.

Security Findings and Attack Paths

• inferred — The concerning sequence requires a policy request to pass live-role validation, role deletion to commit before the policy insert, and permission cleanup to leave relevant tuples behind. The delayed insert and assignment can then create an effective grant to a hidden role. Without that cleanup failure or interruption, the outcome is a dangling but ineffective binding, not a demonstrated authorization bypass.

Trust Boundaries and Controls

• observed — Organization-role deletion checks role ownership and organization role-management permission; administrative role deletion requires a superuser. Policy creation checks authority on its resource target. Permission checks use the logged-in principal and include PAT scope enforcement before authorization evaluation.

Resilience and Maintainability Implications

• observed — Database deletion followed by fallible external cleanup predates this PR. Ordinary role deletion cannot resume cleanup once its initial read returns NotFound. Policy deletion remains a separate path that removes binding relationships before deleting the policy record, providing a distinct remediation route rather than making role deletion retryable.

Hardening Proposals

• proposed — Lock and revalidate the live role within the policy-write transaction using a lock that coordinates with role deletion. Independently retain durable cleanup work so interrupted permission revocation can resume even after normal reads hide the role.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
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.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository: raystack/frontier/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: bfb29883-47d8-45d9-be06-5d00de963b26

📥 Commits

Reviewing files that changed from the base of the PR and between 5b74374 and bed3b8d.

📒 Files selected for processing (5)

• core/role/service.go
• core/role/service_test.go
• internal/store/postgres/role_repository.go
• internal/store/postgres/role_repository_test.go
• test/e2e/regression/api_test.go

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

[coveralls]

Coverage Report for CI Build 36825456600

Coverage increased (+0.03%) to 53.003%

Details

• Coverage increased (+0.03%) from the base build.
• Patch coverage: 6 uncovered changes across 1 file (35 of 41 lines covered, 85.37%).
• No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
internal/store/postgres/role_repository.go	41	35	85.37%

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	41377
Covered Lines:	21931
Line Coverage:	53.0%
Coverage Strength:	17.15 hits per line

---

💛 - Coveralls

[rohilsurana]

Looks good. Checked the lock and delete order, the error mapping, and that every other read of roles skips deleted rows. The policy-side live-role check is planned as the follow-up.