Skip to content

fix(triggers): env var resolution in provider configs#4160

Merged
icecrasher321 merged 2 commits intostagingfrom
fix/env-resolution-triggers
Apr 14, 2026
Merged

fix(triggers): env var resolution in provider configs#4160
icecrasher321 merged 2 commits intostagingfrom
fix/env-resolution-triggers

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

@icecrasher321 icecrasher321 commented Apr 14, 2026

Summary

Env var resolution in provider configs

Type of Change

  • Bug fix

Testing

Test in staging environment

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)

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 14, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Apr 14, 2026 9:23pm

Request Review

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 14, 2026

PR Summary

Medium Risk
Touches webhook execution input formatting by resolving provider configs from decrypted env vars, so mis-resolution could break webhook runs or change runtime credentials. Adds tests, but behavior changes occur in a core trigger path.

Overview
Webhook executions now resolve {{ENV_VAR}} references inside providerConfig before provider handlers format input, using decrypted env values scoped by user/workspace.

Adds a small env-resolver API (normalizeWebhookProviderConfig, resolveWebhookProviderConfig, resolveWebhookRecordProviderConfig) plus a wrapper in webhook-execution that rethrows failures with provider/webhook context, with new Vitest coverage for both successful resolution and error messaging.

Reviewed by Cursor Bugbot for commit 72e805a. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 14, 2026

Greptile Summary

This PR fixes missing environment variable resolution for webhook provider configs (e.g. {{SLACK_BOT_TOKEN}}) by introducing three new helpers in env-resolver.ts and wiring them into webhook-execution.ts before handler.formatInput is called. The implementation is clean, follows project conventions, and includes well-structured tests.

Confidence Score: 5/5

Safe to merge; all findings are P2 style/quality suggestions that do not block correct operation.

The fix is targeted and well-tested. Both open comments are P2 — one is a log-message clarity improvement, the other is a subtle null-vs-empty-object normalisation that may affect no current providers. No P0/P1 issues found.

apps/sim/background/webhook-execution.ts (silent fallback log clarity), apps/sim/lib/webhooks/env-resolver.ts (null normalisation)

Important Files Changed

Filename Overview
apps/sim/lib/webhooks/env-resolver.ts Adds normalizeWebhookProviderConfig, resolveWebhookProviderConfig, and resolveWebhookRecordProviderConfig helpers; clean implementation with good TSDoc comments
apps/sim/background/webhook-execution.ts Wires in env-var resolution before formatInput; uses a try/catch silent-fallback pattern that can leave unresolved template literals in the config passed downstream
apps/sim/lib/webhooks/env-resolver.test.ts Well-structured tests using vi.hoisted and static imports per project guidelines; covers the two main happy paths

Sequence Diagram

sequenceDiagram
    participant WE as webhook-execution.ts
    participant ER as env-resolver.ts
    participant EU as getEffectiveDecryptedEnv
    participant RV as resolveEnvVarReferences
    participant H as provider handler

    WE->>ER: resolveWebhookRecordProviderConfig(webhookRecord, userId, workspaceId)
    ER->>ER: normalizeWebhookProviderConfig(providerConfig)
    ER->>EU: getEffectiveDecryptedEnv(userId, workspaceId)
    EU-->>ER: envVars map
    ER->>RV: resolveEnvVarReferences(config, envVars, { deep: true })
    RV-->>ER: resolved config
    ER-->>WE: { ...webhookRecord, providerConfig: resolved }
    Note over WE: On error: falls back to original webhookRecord
    WE->>H: formatInput({ webhook: resolvedWebhookRecord, ... })
Loading

Reviews (1): Last reviewed commit: "fix(triggers): env var resolution in pro..." | Re-trigger Greptile

@icecrasher321
Copy link
Copy Markdown
Collaborator Author

bugbot run

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 72e805a. Configure here.

@icecrasher321 icecrasher321 merged commit 7529a75 into staging Apr 14, 2026
14 checks passed
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