Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Remove hardcoded SLACK_FILE_HOSTS allowlist that silently dropped files from unrecognized Slack hostnames
  • Add files.info API fallback when event payload contains partial file objects (missing url_private)
  • Use resolved metadata (name, mimetype, size) from files.info throughout download flow
  • Bump max files from 10 to 15

Type of Change

  • Bug fix

Testing

Tested manually

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

vercel bot commented Feb 10, 2026

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Feb 10, 2026 3:19am

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 10, 2026

Greptile Overview

Greptile Summary

This change updates Slack webhook file downloading to (1) handle partial file objects by calling Slack files.info to resolve url_private/metadata and (2) increase the maximum number of processed files from 10 to 15.

The new resolveSlackFileInfo helper is used inside downloadSlackFiles when the event payload lacks url_private, and the resolved name/mimetype/size are then used in the download output.

Key integration point: formatWebhookInput for the slack provider conditionally calls downloadSlackFiles(rawEvent.files, botToken) when includeFiles is enabled.

Blocking issue: removing the Slack hostname allowlist changes the security model — downloadSlackFiles can now send the bot Authorization header to any HTTPS host that resolves to a public IP. This should be re-restricted to Slack-controlled domains before merge.

Confidence Score: 2/5

  • This PR should not be merged as-is due to a security regression in Slack file downloading.
  • While the files.info fallback likely fixes missing metadata in Slack events, removing the Slack file host allowlist means downloadSlackFiles can send the bot token in an Authorization header to any HTTPS host that resolves to a public IP (per validateUrlWithDNS), which is a meaningful security posture change/regression.
  • apps/sim/lib/webhooks/utils.server.ts

Important Files Changed

Filename Overview
apps/sim/lib/webhooks/utils.server.ts Adds Slack files.info fallback and raises max file count, but removing the Slack file host allowlist means the bot Authorization header may be sent to arbitrary public hosts via crafted url_private URLs.

Sequence Diagram

sequenceDiagram
  participant Slack as Slack Event API
  participant Webhook as formatWebhookInput (Sim)
  participant DL as downloadSlackFiles
  participant Info as Slack files.info
  participant Fetch as secureFetchWithPinnedIP

  Slack->>Webhook: event payload (may include partial file objects)
  Webhook->>DL: downloadSlackFiles(rawEvent.files, botToken)
  alt file.url_private missing
    DL->>Info: GET /api/files.info?file={fileId} (Bearer botToken)
    Info-->>DL: file { url_private, name, mimetype, size }
  end
  DL->>Fetch: GET url_private (Bearer botToken)
  Fetch-->>DL: file bytes
  DL-->>Webhook: [{name, mimeType, size, data(base64)}]
  Webhook-->>Slack: 200 OK (processed input)
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 10, 2026

Additional Comments (1)

apps/sim/lib/webhooks/utils.server.ts
Slack token sent off-domain

downloadSlackFiles no longer restricts url_private to Slack-controlled hosts (the SLACK_FILE_HOSTS allowlist was removed), but it still sends Authorization: Bearer ${botToken} to whatever url_private points at. Since validateUrlWithDNS only enforces “https + public/unicast IP” (no Slack-domain constraint), a crafted url_private could cause the bot token to be sent to an arbitrary public host and fetch attacker-controlled content. This needs a host restriction (e.g., Slack domain allowlist / suffix check) before merging.

@waleedlatif1 waleedlatif1 merged commit e5d3049 into staging Feb 10, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/slack branch February 10, 2026 03:29
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