fix(connectors): stop granting act-as-artist authority off the flat account_artist_ids roster edge (P0b)#772
Conversation
…ccount_artist_ids roster edge (P0b) Interim cut per chat#1860 decision (2026-07-08): connector authorize, execute, disconnect, and the getComposioTools artist branch now require self, org co-membership, workspace ownership, org membership, or RECOUP_ORG staff - never a bare account_artist_ids roster row. checkAccountArtistAccess is unchanged; read/analytics paths keep roster-based access. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 48 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (5)
✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 10 files
Confidence score: 2/5
- In
lib/composio/checkConnectorAuthority.ts, lookup errors returningnullcan fall through as if no org rows were found, which may incorrectly grant connector authority; merging as-is risks an authorization bypass and unintended access — returnfalseexplicitly on artist-org/account-org lookup errors (and add a regression test for the error path) before merging.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/composio/checkConnectorAuthority.ts">
<violation number="1" location="lib/composio/checkConnectorAuthority.ts:42">
P2: Connector authority can still be granted after an artist-org or account-org lookup error because `null` is treated like “no rows” and the function falls through to later grants. Returning `false` explicitly on those `null` results keeps the fail-closed contract for connector operations.</violation>
</file>
Architecture diagram
sequenceDiagram
participant Client
participant API as API Route (authorize/execute/disconnect)
participant V as validate*Request
participant CCA as checkConnectorAuthority
participant GetOrgs as getAccountOrganizations
participant SelArtOrgs as selectArtistOrganizationIds
participant SelAccOrgs as selectAccountOrganizationIds
participant SelWs as selectAccountWorkspaceId
participant VOA as validateOrganizationAccess
participant DB
Note over Client,V: NEW: Connector authority flow (P0b cut)
Client->>API: POST /api/connectors/authorize (or execute/disconnect) with account_id
API->>V: validate request
V->>CCA: checkConnectorAuthority(callerId, targetAccountId)
CCA->>CCA: target === caller? (self)
alt Self-access
CCA-->>V: true
else Not self
CCA->>GetOrgs: getAccountOrganizations(callerId)
GetOrgs->>DB: query caller orgs
DB-->>GetOrgs: rows
alt Caller is RECOUP_ORG staff
CCA-->>V: true
else Not staff
CCA->>SelArtOrgs: selectArtistOrganizationIds(target)
SelArtOrgs->>DB: query target artist orgs
DB-->>SelArtOrgs: rows or null
alt DB error (null)
CCA-->>V: false (fail closed)
else Has orgs
CCA->>SelAccOrgs: selectAccountOrganizationIds(caller, orgIds)
SelAccOrgs->>DB: query caller orgs in set
DB-->>SelAccOrgs: rows or null
alt Shared org found
CCA-->>V: true
else No shared org
CCA->>SelWs: selectAccountWorkspaceId(caller, target)
SelWs->>DB: check workspace ownership
DB-->>SelWs: record or null
alt Workspace owned
CCA-->>V: true
else Not workspace
CCA->>VOA: validateOrganizationAccess(caller, target)
VOA->>DB: check org membership
DB-->>VOA: boolean
alt Belongs to target org
CCA-->>V: true
else Denied
CCA-->>V: false
end
end
end
else No orgs (target has no orgs)
CCA->>SelWs: selectAccountWorkspaceId(caller, target)
SelWs->>DB: ...
alt Workspace owned
CCA-->>V: true
else Not workspace
CCA->>VOA: validateOrganizationAccess(...)
alt Org member
CCA-->>V: true
else Denied
CCA-->>V: false
end
end
end
end
end
alt Authority granted
V-->>API: validated request payload
API-->>Client: 200 (authorize/execute/disconnect proceeds)
else Authority denied
V-->>API: 403 response
API-->>Client: 403 Access denied
end
Note over Client,V: Also used by tool router (artist tool branch)
participant TR as getComposioTools
Note over TR: NEW: Uses checkConnectorAuthority (was checkAccountArtistAccess)
Client->>TR: getComposioTools(accountId, artistId)
TR->>CCA: checkConnectorAuthority(accountId, artistId)
CCA->>CCA: same checks (self, RECOUP_ORG, org co-membership, workspace, org)
alt Authority granted
CCA-->>TR: true
TR->>TR: fetch artist connectors and tools
TR-->>Client: tool set including artist tools
else Authority denied (roster-only)
CCA-->>TR: false
TR->>TR: skip artist tools (still return customer tools)
TR-->>Client: tool set without artist tools
end
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
|
||
| // 3. Org co-membership with a target artist | ||
| const targetOrgs = await selectArtistOrganizationIds(targetAccountId); | ||
| if (targetOrgs?.length) { |
There was a problem hiding this comment.
P2: Connector authority can still be granted after an artist-org or account-org lookup error because null is treated like “no rows” and the function falls through to later grants. Returning false explicitly on those null results keeps the fail-closed contract for connector operations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/composio/checkConnectorAuthority.ts, line 42:
<comment>Connector authority can still be granted after an artist-org or account-org lookup error because `null` is treated like “no rows” and the function falls through to later grants. Returning `false` explicitly on those `null` results keeps the fail-closed contract for connector operations.</comment>
<file context>
@@ -0,0 +1,59 @@
+
+ // 3. Org co-membership with a target artist
+ const targetOrgs = await selectArtistOrganizationIds(targetAccountId);
+ if (targetOrgs?.length) {
+ const orgIds = targetOrgs.map(o => o.organization_id).filter((id): id is string => Boolean(id));
+ if (orgIds.length) {
</file context>
Preview verification — P0b connector authority cutPreview:
|
| # | Surface | Request | Documented | Actual | Verdict |
|---|---|---|---|---|---|
| 1 | Auth gate | GET /api/connectors no token |
401 | HTTP 401 |
✅ |
| 2 | Auth gate | GET /api/connectors bad Bearer |
401 | HTTP 401 {"message":"Failed to verify authentication token"} |
✅ |
| 3 | Read path (unchanged, checkAccountArtistAccess) |
GET /api/connectors self |
200 | HTTP 200, 8 connectors |
✅ |
| 4 | Read path (unchanged) | GET /api/connectors?account_id=<rostered artist> |
200 | HTTP 200 |
✅ roster read preserved |
| 5 | Authorize (now checkConnectorAuthority) |
POST /api/connectors {connector:"googlesheets", account_id:<rostered artist>} |
200 + redirectUrl | HTTP 200 {success:true, redirectUrl:"…lk_8KVoKqXitpK1"} |
✅ authority granted (staff) |
| 6 | Execute (now checkConnectorAuthority) |
POST /api/connectors/actions {actionSlug:"NOT_A_REAL_ACTION", account_id:<rostered artist>} |
past auth → 404 | HTTP 404 {"error":"Connector action not found: NOT_A_REAL_ACTION"} |
✅ passed authority, failed downstream as expected |
| 7 | Disconnect (now checkConnectorAuthority) |
DELETE /api/connectors {connected_account_id:"ca_bogus…", account_id:<rostered artist>} |
past auth → not-found | HTTP 500 {"error":"Connection not found for this account"} |
✅ passed authority (see note) |
| 8 | Staff bypass kept | POST /api/connectors {account_id:<random UUID, not rostered/not my org>} |
200 (admin bypass) | HTTP 200 {success:true, redirectUrl:"…lk_RiV9GD8Hibnj"} |
✅ RECOUP_ORG bypass intact |
Rostered artist used: Ana Bárbara f584e4f5-6d91-409d-a9ad-8d4047b0ad26 (confirmed in caller roster via GET /api/artists).
The core denial (roster-only → 403): not live-reproducible here, covered by tests
The security fix — a non-staff account that rosters artist A but shares no org gets 403/zero tools — can't be exercised with a staff token (bypass). It is covered by lib/composio/__tests__/checkConnectorAuthority.test.ts → "denies a roster-only manager (bare account_artist_ids row, no org tie)", which also asserts selectAccountArtistId is never called. Full suite: 272 tests green (pnpm exec vitest run lib/composio lib/artists). The getComposioTools artist branch is a chat-agent tool-router path (not a REST surface), so it's likewise covered by unit tests + review rather than a live curl here. Recommend a follow-up live check with a non-staff, roster-only account to close the loop on the denial before/at rollout.
Non-blocking observation (pre-existing, not from this PR)
Row 7: disconnect of a non-existent connection returns HTTP 500, while route.ts JSDoc documents 404 for "connected account does not exist." This is in disconnectConnectorHandler downstream of the authority check (untouched by this PR), so it's pre-existing doc/behavior drift, not a regression here — worth a separate tidy-up.
Verdict
Authority-preserving behavior verified live across all four narrowed surfaces; auth gates and roster read paths intact; staff bypass intact. The roster-only denial is unit-tested but should get one live pass with a non-staff account. No regressions observed from this change.
🤖 Generated with Claude Code
Follow-up: roster-only denial verified live (non-staff account)Closes the gap from the previous comment. Re-ran against the same preview Caller: account When: 2026-07-09 15:40 UTC
ResultThe roster edge no longer grants act-as-artist connector authority: a non-staff account that rosters an artist is denied 403 on authorize, execute, and disconnect for that artist (C/D/E), while its own connectors (B) and roster read/analytics (A) remain fully functional. Combined with the earlier staff-path run (grants + bypass preserved), all four narrowed call sites are now verified live in both directions — grant preserved and denial enforced. No regressions. 🤖 Generated with Claude Code |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
1 participant
Summary
P0b interim cut per the decision callout in recoupable/chat#1860 (sweetman, 2026-07-08): connector paths no longer treat a bare
account_artist_idsroster row as act-as-artist authority. Two accounts that both roster canonical artist A can no longer use each other's OAuth connectors for A.What changed
lib/composio/checkConnectorAuthority.ts— connector-scoped authority check. Grants on: self, RECOUP_ORG staff (admin bypass, kept), org co-membership with the target artist, workspace ownership, or membership in a target organization. It never reads theaccount_artist_idsroster edge. Fails closed on DB errors.checkAccountAccess/checkAccountArtistAccess):lib/composio/toolRouter/getComposioTools.ts(artist tool branch)lib/composio/connectors/validateAuthorizeConnectorRequest.tslib/composio/connectors/validateExecuteConnectorActionRequest.ts(theaccount_idoverride on POST /api/connectors/actions)lib/composio/connectors/validateDisconnectConnectorRequest.tscheckAccountArtistAccessstill honors the roster edge — read/analytics paths (posts, fans, socials, artist update/delete, YouTube revenue MCP tool) keep working. Connector read paths (GET /api/connectors,GET /api/connectors/actionslisting) also keep the flat check per the issue's authorize/execute/disconnect scope.Verification
pnpm exec vitest run lib/composio lib/artists: 51 files, 272 tests, all passing (includes 8 new tests inlib/composio/__tests__/checkConnectorAuthority.test.tscovering self, roster-only denial, org co-membership, RECOUP_ORG bypass, workspace, org target, and both fail-closed paths).pnpm exec tsc --noEmit: 200 pre-existing errors on this branch and onmainbaseline (all in unrelated__tests__files); zero errors introduced by this change.Preview verification pending — tracked in chat#1860.
🤖 Generated with Claude Code
Summary by cubic
Connector actions and artist tool exposure no longer treat a bare
account_artist_idsroster row as act-as-artist authority. Managers who only share a rostered artist can’t use each other’s OAuth connectors for that artist.lib/composio/checkConnectorAuthority.tsto enforce connector authority; fails closed on DB errors.RECOUP_ORGstaff, shared org with the target artist, caller’s workspace, or caller’s organization.validateAuthorizeConnectorRequest,validateExecuteConnectorActionRequest,validateDisconnectConnectorRequest, andgetComposioTools(artist branch).checkAccountArtistAccessfor read/analytics and connector listings; roster-based access still applies there.Written for commit b019813. Summary will update on new commits.