Skip to content

fix(webhooks): authorize credential references on webhook upsert - #7800

Open
waleedlatif1 wants to merge 4 commits into
stagingfrom
fix/webhook-credential-reference-authz
Open

fix(webhooks): authorize credential references on webhook upsert#7800
waleedlatif1 wants to merge 4 commits into
stagingfrom
fix/webhook-credential-reference-authz

Conversation

@waleedlatif1

Copy link
Copy Markdown
Collaborator

Summary

  • POST /api/webhooks saved client-supplied providerConfig verbatim; subscription handlers and pollers resolve providerConfig.credentialId by id alone and mint tokens as the credential's owner
  • Authorize credentialId for the acting user within the workflow's workspace (authorizeCredentialUseForAuth) before any external subscription is created or the row is saved — 403 otherwise
  • Require credentialId to be a literal id (400), since the row stores the unresolved text rather than the env-resolved value
  • Never accept a client-supplied providerConfig.userId, which the polling token resolver falls back to; re-saves keep the server-set value
  • Deploy path (lib/webhooks/deploy.ts) already validates credentials and is unchanged

Type of Change

  • Bug fix

Testing

  • New route tests: denied credential, env-var credential id, allowed credential, dropped userId, re-save keeps stored credential/userId — the four refusal/preservation tests fail against the previous route
  • lib/webhooks + route suites passing, type-check, bun run lint, check:audits (46), block registry, docs manifest

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)

POST /api/webhooks persisted client-supplied providerConfig verbatim, while
subscription handlers and pollers resolve providerConfig.credentialId by id
alone and mint tokens as the credential's owner. Authorize credentialId for
the acting user within the workflow's workspace before subscribing or saving,
require it to be a literal id, and never accept a client-supplied userId,
which the polling token resolver falls back to.
@vercel

vercel Bot commented Sep 13, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Sep 13, 2026 3:28am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the prior credential lifecycle issues are fixed and no new actionable failures remain.

Summary

  • Authorizes every credential used by subscription creation, cleanup, polling configuration, or persistence.
  • Rejects environment-resolved and non-string credential IDs because webhook rows retain unresolved configuration.
  • Removes client-supplied and legacy stored userId values before subscription setup and persistence.
  • Allows credential rotation without requiring access to an unused previous credential while retaining authorization when subscription cleanup needs it.
  • Adds route coverage for denied, allowed, omitted, cleared, and rotated credential references.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    R[Webhook upsert request] --> S[Authenticate actor and authorize workflow]
    S --> C[Remove client-supplied userId]
    C --> E[Resolve provider configuration]
    E --> L{Credential ID remains literal?}
    L -->|No| B[Return 400]
    L -->|Yes| X[Load existing webhook]
    X --> D[Determine whether subscription recreation is required]
    D --> A[Collect credentials the save will actually use]
    A --> U{Actor may use every credential?}
    U -->|No| F[Return 403]
    U -->|Yes| P[Check webhook capability policy]
    P --> N[Create or retain provider subscription]
    N --> W[Persist sanitized configuration]
    W --> O[Clean up previous subscription when recreated]
    O --> G[Configure polling]
Loading

Reviews (4) · Last reviewed commit: "fix(webhooks): authorize the stored cred..."

Comment thread apps/sim/app/api/webhooks/route.ts Outdated
Comment thread apps/sim/app/api/webhooks/route.ts
Comment thread apps/sim/app/api/webhooks/route.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

Comment thread apps/sim/app/api/webhooks/route.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

Comment thread apps/sim/app/api/webhooks/route.ts
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Sep 13, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

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