fix: use membership role as source-of-truth for organization roles by rafaumeu · Pull Request #1988 · CapSoftware/Cap

rafaumeu · 2026-07-08T01:15:29Z

Summary

The getEffectiveOrganizationRole function previously used organizations.ownerId as the primary source for determining the 'owner' role. This caused two problems:

ownerId overrides memberRole: When a user matched ownerId but their membership record had a different role (e.g. member), the function would incorrectly return owner.
memberRole='owner' forced to 'member': When a user's membership had role='owner' but they weren't the ownerId, the function would return member, silently stripping owner privileges.

Changes

apps/web/lib/permissions/roles.ts: getEffectiveOrganizationRole now uses memberRole from organization_members as source-of-truth, with ownerId only as a fallback when no membership record exists. Same fix applied to isOrganizationOwnerTarget.
apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx: isMemberOwner now checks member.role === 'owner' instead of comparing with organization.ownerId.
Updated existing tests that validated the buggy behavior to expect the correct behavior.

Validation

All 945 tests pass
Typecheck passes (pnpm tsc -b)
Lint passes (biome lint)

How to test

Create an organization where user A is the owner
Transfer ownership to user B (user A's membership role becomes 'member')
Before fix: user A still sees owner privileges (incorrect)
After fix: user A correctly sees only member privileges

The reverse scenario (user with membership role='owner' but different ownerId) is also handled correctly.

Greptile Summary

This PR makes organization membership role the source of truth for owner checks. The main changes are:

Updated effective organization role resolution in shared permission helpers.
Updated members-table owner display logic to read member.role.
Adjusted unit tests for membership-role precedence and ownerId fallback.

Confidence Score: 4/5

The changed permission target check needs a fix before merging.

Known non-owner membership roles can still be overridden by stale ownerId.
Member role and removal flows can reject valid changes for a user who is no longer owner.
The members UI can expose controls for legacy owner rows that the server rejects.

apps/web/lib/permissions/roles.ts; apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx

Security Review

A permission helper can still treat a non-owner membership as owner when stale ownerId matches the target user, blocking legitimate member-management actions.

Important Files Changed

Filename	Overview
apps/web/lib/permissions/roles.ts	Updates effective role resolution, but owner-target detection still falls back to stale ownerId even when a non-owner membership role is known.
apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx	Moves UI owner detection to member role, but drops the legacy ownerId fallback used by shared permission helpers.
apps/web/tests/unit/roles-permissions.test.ts	Updates tests for membership-role precedence and ownerId fallback in the main role helper.
apps/web/tests/unit/desktop-organization-branding.test.ts	Updates branding permission expectations for membership owner precedence.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/web/lib/permissions/roles.ts:143
**Stale OwnerId Still Wins**

When `targetRole` is already known to be `"member"` or `"admin"`, this fallback can still classify the target as owner because their id matches stale `ownerId`. In that drift state, `canChangeOrganizationMemberRole` and `canRemoveOrganizationMember` reject changes to a non-owner member, so the member can become stuck even though membership role is now the source of truth.

### Issue 2 of 2
apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx:209
**Legacy Owner Row Loses Protection**

When an organization owner is represented only by `organization.ownerId` or has a stale/missing membership role, this returns false while the shared permission helpers still treat that user as owner through their fallback. The row then shows editable role, Pro-seat, and remove controls that the server rejects, so legacy owner rows become misleading and inconsistent.

_{Reviews (1): Last reviewed commit: "fix: use membership role as source-of-tr..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

Context used:

Context used - CLAUDE.md (source)
Context used - AGENTS.md (source)

…apSoftware#1641) The getEffectiveOrganizationRole function previously used organizations.ownerId as the primary source for determining the 'owner' role, with memberRole as a fallback. This caused two problems: 1. When a user was the organization owner (ownerId match) but their membership record had a different role (e.g. 'member'), the ownerId check would incorrectly override their actual membership role. 2. When a user's membership record had role='owner' but they weren't the ownerId, the function would force their role to 'member', silently stripping their owner privileges. This fix makes memberRole from organization_members the primary source of truth, with ownerId only used as a fallback when no membership record exists. Also updates isMemberOwner in MembersCard and isOrganizationOwnerTarget to use membership role instead of ownerId comparison. Closes CapSoftware#1641

greptile-apps · 2026-07-08T01:17:42Z

+	// Prefer membership role over ownerId (see #1641).
+	if (targetRole === "owner") return true;
+	// Fallback for legacy/missing membership records.
+	return !!targetUserId && !!ownerId && targetUserId === ownerId;


Stale OwnerId Still Wins

When targetRole is already known to be "member" or "admin", this fallback can still classify the target as owner because their id matches stale ownerId. In that drift state, canChangeOrganizationMemberRole and canRemoveOrganizationMember reject changes to a non-owner member, so the member can become stuck even though membership role is now the source of truth.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/web/lib/permissions/roles.ts Line: 143 Comment: **Stale OwnerId Still Wins** When `targetRole` is already known to be `"member"` or `"admin"`, this fallback can still classify the target as owner because their id matches stale `ownerId`. In that drift state, `canChangeOrganizationMemberRole` and `canRemoveOrganizationMember` reject changes to a non-owner member, so the member can become stuck even though membership role is now the source of truth. **Context Used:** AGENTS.md ([source](https://app.greptile.com/cap/github/capsoftware/cap/-/custom-context?memory=27801409-c24c-4476-9c6c-180f1ef0a7f2)) How can I resolve this? If you propose a fix, please make it concise.

Fixed in f7d0dfd — isOrganizationOwnerTarget now only falls back to ownerId when targetRole is null/undefined. When targetRole is a known non-owner role ("member"/"admin"), it returns false immediately. Same logic applied to MembersCard.isMemberOwner — falls back to ownerId only when no membership record exists.

greptile-apps · 2026-07-08T01:17:43Z

 	const isMemberOwner = (id: string) => {
-		return id === activeOrganization?.organization.ownerId;
+		const member = activeOrganization?.members.find((m) => m.userId === id);
+		return member?.role === "owner";


Legacy Owner Row Loses Protection

When an organization owner is represented only by organization.ownerId or has a stale/missing membership role, this returns false while the shared permission helpers still treat that user as owner through their fallback. The row then shows editable role, Pro-seat, and remove controls that the server rejects, so legacy owner rows become misleading and inconsistent.

Context Used: AGENTS.md (source)

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/web/app/(org)/dashboard/settings/organization/components/MembersCard.tsx Line: 209 Comment: **Legacy Owner Row Loses Protection** When an organization owner is represented only by `organization.ownerId` or has a stale/missing membership role, this returns false while the shared permission helpers still treat that user as owner through their fallback. The row then shows editable role, Pro-seat, and remove controls that the server rejects, so legacy owner rows become misleading and inconsistent. **Context Used:** AGENTS.md ([source](https://app.greptile.com/cap/github/capsoftware/cap/-/custom-context?memory=27801409-c24c-4476-9c6c-180f1ef0a7f2)) How can I resolve this? If you propose a fix, please make it concise.

Fixed in f7d0dfd — isMemberOwner now falls back to ownerId comparison only when no membership record is found in activeOrganization.members. When the membership record exists, it always uses `member.role === 'owner'" as the source of truth.

tembo · 2026-07-08T01:19:41Z

+	// Prefer membership role over ownerId (see #1641).
+	if (targetRole === "owner") return true;
+	// Fallback for legacy/missing membership records.
+	return !!targetUserId && !!ownerId && targetUserId === ownerId;


Minor edge case: if targetRole is already known to be "member"/"admin", the current ownerId fallback can still classify the target as owner if ownerId is stale. You can restrict the fallback to only when targetRole is missing.

Suggested change

// Prefer membership role over ownerId (see #1641).

if (targetRole === "owner") return true;

// Fallback for legacy/missing membership records.

return !!targetUserId && !!ownerId && targetUserId === ownerId;

// Prefer membership role over ownerId (see #1641).

if (targetRole === "owner") return true;

if (targetRole) return false;

// Fallback for legacy/missing membership records.

return !!targetUserId && !!ownerId && targetUserId === ownerId;

Applied your suggestion in f7d0dfd. if (targetRole) return false; now short-circuits when targetRole is a known non-owner role before reaching the ownerId fallback.

tembo · 2026-07-08T01:19:46Z

@@ -205,7 +205,8 @@ export const MembersCard = ({ setIsInviteDialogOpen }: MembersCardProps) => {
 	};

 	const isMemberOwner = (id: string) => {


If activeOrganization.members can ever be missing the owner membership row (legacy/migration state), this drops the ownerId fallback and can show non-owner UI for the owner. If you still want a legacy fallback without reintroducing the stale-ownerId override, only fall back when no membership record exists.

Suggested change

const isMemberOwner = (id: string) => {

return member ? member.role === "owner" : id === activeOrganization?.organization.ownerId;

Applied your suggestion in f7d0dfd. isMemberOwner now falls back to ownerId only when no membership record is found: member ? member.role === 'owner' : id === activeOrganization?.organization.ownerId

…embership - isOrganizationOwnerTarget: only use ownerId fallback when targetRole is null/undefined (not when it's a known non-owner role) - MembersCard.isMemberOwner: fall back to ownerId comparison when no membership record exists (legacy/migration state) Co-authored-by: tembo (suggestion)

rafaumeu mentioned this pull request Jul 8, 2026

Organization role source-of-truth mismatch: incorrect role display and owner invite authorization #1641

Open

greptile-apps Bot reviewed Jul 8, 2026

View reviewed changes

tembo Bot reviewed Jul 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: use membership role as source-of-truth for organization roles#1988

fix: use membership role as source-of-truth for organization roles#1988
rafaumeu wants to merge 2 commits into
CapSoftware:mainfrom
rafaumeu:fix/organization-role-source-of-truth-1641

rafaumeu commented Jul 8, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

greptile-apps Bot Jul 8, 2026

Uh oh!

rafaumeu Jul 8, 2026

Uh oh!

greptile-apps Bot Jul 8, 2026

Uh oh!

rafaumeu Jul 8, 2026

Uh oh!

tembo Bot Jul 8, 2026

Uh oh!

rafaumeu Jul 8, 2026

Uh oh!

tembo Bot Jul 8, 2026

Uh oh!

rafaumeu Jul 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		@@ -205,7 +205,8 @@ export const MembersCard = ({ setIsInviteDialogOpen }: MembersCardProps) => {
		};

		const isMemberOwner = (id: string) => {

	const isMemberOwner = (id: string) => {
	return member ? member.role === "owner" : id === activeOrganization?.organization.ownerId;

Uh oh!

Conversation

rafaumeu commented Jul 8, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Validation

How to test

Greptile Summary

Confidence Score: 4/5

Security Review

Important Files Changed

Uh oh!

greptile-apps Bot Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

rafaumeu Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

rafaumeu Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

tembo Bot Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

rafaumeu Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

tembo Bot Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

rafaumeu Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

rafaumeu commented Jul 8, 2026 •

edited by greptile-apps Bot

Loading