fix(mothership): enforce ownership check on workflow resource attachments#4468
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryHigh Risk Overview Tightens workspace scoping for table attachments by requiring a Includes minor test/mock updates for the new DB query path and a small Reviewed by Cursor Bugbot for commit 8515dce. Configure here. |
Greptile SummaryThis PR fixes a BOLA (Broken Object Level Authorization) vulnerability in the Mothership copilot chat where
Confidence Score: 5/5The changes close three cross-workspace read vulnerabilities and add a missing workspace membership gate; the logic is narrow, targeted, and backed by existing authorization helpers already in use on the corrected paths. All three BOLA fixes correctly thread the authenticated userId and verified workspaceId into the existing authorization helpers that were already tested and trusted. The new workspace permission query in resolveBranch is straightforward and the mock in the test file validates the happy path. No new data paths are opened and no existing permission checks are weakened. No files require special attention; the only minor point is that the access-denied response in post.ts uses a 400 status code where a 403 would be more semantically correct. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant post.ts as post.ts (resolveBranch)
participant process-contents.ts as resolveActiveResourceContext
participant DB
Client->>post.ts: POST /api/copilot { resourceAttachments, workspaceId }
post.ts->>DB: resolveWorkflowIdForUser(userId, workflowId)
DB-->>post.ts: resolved workflowId
Note over post.ts: NEW: fetch canonical workspaceId from DB
post.ts->>DB: getWorkflowById(resolvedWorkflowId)
DB-->>post.ts: workflow.workspaceId
alt No workflowId — workspace-only path
Note over post.ts: NEW: workspace membership check
post.ts->>DB: SELECT permissionType FROM permissions WHERE userId=? AND entityId=?
DB-->>post.ts: permissionRow
alt permissionRow is null
post.ts-->>Client: 400 Workspace not found or access denied
end
end
post.ts->>process-contents.ts: resolveActiveResourceContext(type, id, workspaceId, userId)
alt type = workflow
Note over process-contents.ts: NEW: passes real userId + workspaceId
process-contents.ts->>DB: processWorkflowFromDb(id, userId, workspaceId)
DB-->>process-contents.ts: authorizeWorkflowByWorkspacePermission check runs
else type = knowledgebase
Note over process-contents.ts: NEW: passes real userId
process-contents.ts->>DB: checkKnowledgeBaseAccess(id, userId)
DB-->>process-contents.ts: hasAccess result
else type = table
process-contents.ts->>DB: getTableById(tableId)
DB-->>process-contents.ts: table record
Note over process-contents.ts: NEW: table.workspaceId !== workspaceId → reject
end
process-contents.ts-->>Client: resource content or null
Reviews (4): Last reviewed commit: "fix(mothership): always derive workspace..." | Re-trigger Greptile |
|
Both findings are correct — fixed in the two follow-up commits. Table (no ownership check): Knowledge base (userId not forwarded): Also audited the remaining two resource types for completeness — both were already clean: |
|
@greptile |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
@cursor review |
|
@greptile |
|
@greptile |
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 8515dce. Configure here.
|
Thank you for the clarification, detailed investigation, and for confirming
and patching the issue so quickly. I appreciate the transparency regarding
the actual impact and CVSS assessment.
I also appreciate the confirmation that the issue has been classified as
High severity (~7.5).
I wanted to ask about the reward process for High severity findings. I
noticed from the publicly shared bounty criteria that High severity reports
may qualify for a reward range depending on impact and validation.
Could you please share:
-
Whether this report is eligible for a bounty reward
-
The estimated reward range being considered
-
The payment/reward process and timeline after triage closure
…On Wed, 6 May 2026 at 12:06 PM, Waleed ***@***.***> wrote:
Summary
- Fixed BOLA vulnerability in Mothership chat where resourceAttachments
of type workflow bypassed authorization
- resolveActiveResourceContext was passing undefined for userId and
workspaceId when calling processWorkflowFromDb, skipping the
authorizeWorkflowByWorkspacePermission check entirely
- Any authenticated user could inject a victim's workflow ID into
resourceAttachments and read its full block config (system prompts,
code, hardcoded secrets)
- Fix: pass the authenticated userId and request workspaceId through
so the existing ownership check runs normally
Credit: @Maxx191 <https://github.com/Maxx191>
Type of Change
- Bug fix
Testing
Tested manually — existing legitimate resource attachment flow works
unchanged. Cross-workspace workflow IDs are now rejected.
Checklist
- Code follows project style guidelines
- Self-reviewed my changes
- Tests added/updated and passing
- No new warnings introduced
- I confirm that I have read and agree to the terms outlined in the Contributor
License Agreement (CLA)
<http://./CONTRIBUTING.md#contributor-license-agreement-cla>
------------------------------
You can view, comment on, or merge this pull request online at:
#4468
Commit Summary
- 74bc60c
<74bc60c>
fix(mothership): enforce ownership check on workflow resource attachments
- 58d734c
<58d734c>
fix(mothership): fix table and knowledgebase BOLA in resource attachment
resolution
- 7413b77
<7413b77>
fix(mothership): apply workspace scope to table in processContextsServer
- bb1e8b6
<bb1e8b6>
fix(mothership): verify workspace membership before resolving workspace
branch
- 2c1b3bb
<2c1b3bb>
fix(data-drains): use const for timeoutId in sleepUntilAborted
- cb1f9eb
<cb1f9eb>
fix(test): mock db.select and drizzle and for workspace permissions check
- 8515dce
<8515dce>
fix(mothership): always derive workspace from workflow record in workflow
branch
File Changes
(4 files <https://github.com/simstudioai/sim/pull/4468/files>)
- *M* apps/sim/lib/copilot/chat/post.test.ts
<https://github.com/simstudioai/sim/pull/4468/files#diff-0540c905a8ad71a1496dcdd48b1a21b0d7c4cd9f0760c49f1733e53ba17be369>
(8)
- *M* apps/sim/lib/copilot/chat/post.ts
<https://github.com/simstudioai/sim/pull/4468/files#diff-60de90a5f54e09f0885aef9b2ced6b38c99d9cee23fd9ca290313eca4923b729>
(34)
- *M* apps/sim/lib/copilot/chat/process-contents.ts
<https://github.com/simstudioai/sim/pull/4468/files#diff-5bcd42156d3a5495626a4daf1824c2f08197a0af8c9059219a423042c94c2388>
(20)
- *M* apps/sim/lib/data-drains/destinations/webhook.ts
<https://github.com/simstudioai/sim/pull/4468/files#diff-ac0da9d05056c2bb8a114e8c26b3f578b278068104721cf813403377a40bf885>
(3)
Patch Links:
- https://github.com/simstudioai/sim/pull/4468.patch
- https://github.com/simstudioai/sim/pull/4468.diff
—
Reply to this email directly, view it on GitHub
<#4468>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A3MYCKAV3BFMVWXGNZ7LQQ34ZLMOVAVCNFSM6AAAAACYSQBCZCVHI2DSMVQWIX3LMV43ASLTON2WKOZUGM4DQOBQHE4DMMI>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
…ents (#4468) * fix(mothership): enforce ownership check on workflow resource attachments * fix(mothership): fix table and knowledgebase BOLA in resource attachment resolution * fix(mothership): apply workspace scope to table in processContextsServer * fix(mothership): verify workspace membership before resolving workspace branch * fix(data-drains): use const for timeoutId in sleepUntilAborted * fix(test): mock db.select and drizzle and for workspace permissions check * fix(mothership): always derive workspace from workflow record in workflow branch
2 participants
Summary
resourceAttachmentsof typeworkflowbypassed authorizationresolveActiveResourceContextwas passingundefinedforuserIdandworkspaceIdwhen callingprocessWorkflowFromDb, skipping theauthorizeWorkflowByWorkspacePermissioncheck entirelyresourceAttachmentsand read its full block config (system prompts, code, hardcoded secrets)userIdand requestworkspaceIdthrough so the existing ownership check runs normallyCredit: @Maxx191
Type of Change
Testing
Tested manually — existing legitimate resource attachment flow works unchanged. Cross-workspace workflow IDs are now rejected.
Checklist