Skip to content

fix(security): enforce workspace scope on workflow middleware and validate shopify shop domain#4535

Merged
waleedlatif1 merged 4 commits intostagingfrom
waleedlatif1/fix-workspace-api-key-scope
May 9, 2026
Merged

fix(security): enforce workspace scope on workflow middleware and validate shopify shop domain#4535
waleedlatif1 merged 4 commits intostagingfrom
waleedlatif1/fix-workspace-api-key-scope

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • validateWorkflowAccess now rejects workspace-scoped API keys whose workspaceId doesn't match the workflow's workspace, closing a boundary leak across /api/workflows/[id]/{log,paused,status} and /api/resume/[workflowId]/[executionId]/[contextId]
  • shopify authorize route validates the resolved shop domain against shopifyShopDomainSchema before initiating OAuth
  • adds middleware tests covering workspace-key cross-workspace rejection, matching-workspace acceptance, personal key, and session auth paths

Type of Change

  • Bug fix

Testing

  • bun run vitest run app/api/workflows/middleware.test.ts — 5/5 passing
  • Manually verified existing personal-key and session flows still pass

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)

…idate shopify shop domain

- validateWorkflowAccess now rejects workspace-scoped API keys whose
  workspaceId doesn't match the workflow's workspace, closing a boundary
  leak across /api/workflows/[id]/{log,paused,status} and
  /api/resume/[workflowId]/[executionId]/[contextId]
- shopify authorize route now validates the resolved shop domain against
  shopifyShopDomainSchema before proceeding
- adds middleware tests covering workspace/personal/session auth paths
@vercel
Copy link
Copy Markdown

vercel Bot commented May 9, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 9, 2026 7:05pm

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 9, 2026

PR Summary

Medium Risk
Tightens authorization checks in workflow middleware and adds stricter Shopify domain validation, which could block previously-accepted (but unsafe) requests if callers rely on looser inputs. Changes are localized but affect security-sensitive request gating and OAuth initiation.

Overview
Closes a cross-workspace access gap in validateWorkflowAccess (the requireDeployment=false path) by rejecting workspace-scoped API keys whose workspaceId doesn’t match the target workflow’s workspace before running permission checks.

Hardens the Shopify OAuth authorize endpoint by validating the normalized shop domain against shopifyShopDomainSchema (and tightening that schema’s regex), returning 400 for invalid shop domains instead of constructing an OAuth URL.

Adds Vitest coverage for the new workflow middleware behavior across workspace-key mismatch/match, personal API keys, and session auth flows.

Reviewed by Cursor Bugbot for commit 00a5e08. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 9, 2026

Greptile Summary

This PR closes a workspace-scope boundary gap in validateWorkflowAccess and adds Shopify shop domain validation before OAuth initiation.

  • Middleware fix: workspace-scoped API keys are now rejected with a 403 when auth.workspaceId doesn't match workflow.workspaceId in the requireDeployment=false branch; the requireDeployment=true branch was already enforced through authenticateApiKeyFromHeader's workspaceId parameter.
  • Shopify validation: shopifyShopDomainSchema.safeParse is applied to cleanShop after normalisation (lowercase, protocol strip), giving an early 400 for malformed domains before the OAuth redirect is constructed.
  • Regex tightening: the domain regex now enforces lowercase, a 3–63 character subdomain length, and a non-hyphen terminal character, addressing the trailing-hyphen gap noted in the previous review.

Confidence Score: 5/5

Targeted security hardening with no behavioural changes to existing authorised callers; safe to merge.

Both changes are narrowly scoped: the middleware check is fail-closed (undefined workspaceId on a workspace key is rejected), the Shopify validation fires after full normalisation so no valid domain is incorrectly blocked, and the tests cover all four auth branches introduced by the fix.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/api/workflows/middleware.ts Adds a workspace-scope boundary check for workspace-scoped API keys in the requireDeployment=false branch; the requireDeployment=true branch was already safe via authenticateApiKeyFromHeader's workspaceId filter.
apps/sim/app/api/workflows/middleware.test.ts New test file covering the four key auth paths: cross-workspace workspace-key rejection, matching-workspace acceptance, personal-key passthrough, and session auth; all 5 cases are well-targeted.
apps/sim/app/api/auth/shopify/authorize/route.ts Domain validation added at the correct point — after cleanShop has been lowercased and normalised — before the OAuth redirect is built.
apps/sim/lib/api/contracts/oauth-connections.ts Regex tightened to lowercase-only, minimum 3-character subdomain, and maximum 63-character subdomain (RFC 1123); trailing hyphen now correctly rejected because the last character class is [a-z0-9].

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Request arrives] --> B[getWorkflowById]
    B --> C{workflow found?}
    C -- No --> D[404 Not Found]
    C -- Yes --> E[checkHybridAuth]
    E --> F{auth success?}
    F -- No --> G[401 Unauthorized]
    F -- Yes --> H{workspace API key?}
    H -- No --> J[authorizeWorkflowByWorkspacePermission]
    H -- Yes --> I{Key scope matches workflow?}
    I -- No --> K[403 Key not authorized for workspace]
    I -- Yes --> J
    J --> L{access granted?}
    L -- No --> M[403 Access Denied]
    L -- Yes --> N[Return workflow and auth]
Loading

Reviews (3): Last reviewed commit: "fix(security): widen shopify subdomain r..." | Re-trigger Greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

Thanks @greptile-apps — tightened the regex in ec2c02b to require the subdomain label to end in an alphanumeric (no trailing hyphen), matching RFC 1123.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 9, 2026

The updated regex looks correct:

/^[a-zA-Z0-9](?:[a-zA-Z0-9-]*[a-zA-Z0-9])?\.myshopify\.com$/
  • Anchors to alphanumeric on both ends of the subdomain label
  • The (?:...)? group makes it optional, so single-char subdomains like a.myshopify.com still pass
  • Trailing hyphens (store-.myshopify.com) are correctly rejected

Solid fix — this now properly enforces RFC 1123 label rules. LGTM.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Comment thread apps/sim/lib/api/contracts/oauth-connections.ts Outdated
Shopify and RFC 1123 allow labels up to 63 chars; the previous
{1,58} quantifier capped the subdomain at 60 chars and rejected
valid 61–63 char shops with a 400.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 00a5e08. Configure here.

@waleedlatif1 waleedlatif1 merged commit 4cc00b2 into staging May 9, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the waleedlatif1/fix-workspace-api-key-scope branch May 9, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant