slackapi / deno-slack-sdk

SDK for building Run on Slack apps using Deno
https://api.slack.com/automation
MIT License
158 stars 27 forks source link

[BUG] files.slack.com not on the outgoing domains allow list #347

Closed mcsescott closed 1 month ago

mcsescott commented 1 month ago

Writing a NextGen app, and I'm trying to download a file from Slack. I get the following error when running the custom function:

error: Uncaught (in promise) PermissionDenied: Detected missing network permissions; add the domain to your manifest's `outgoingDomains`. Original message: Requires net access to "files.slack.com", run again with the --allow-net flag

I thought all the slack.com URLs were allowed?

I am using the latest CLI and SDK versions.

filmaj commented 1 month ago

Do you see this when running locally and/or remotely post deploy? I have a feeling this is an oversight..

filmaj commented 1 month ago

Indeed, locally running functions don't pre-approve files.slack.com - IIUC individual domains including subdomains need to be explicitly approved when running deno. Working on a fix here: https://github.com/slackapi/deno-slack-runtime/pull/66

I believe this will also be an issue once deployed to Slack hosting; we are also working on a fix on that end.

mcsescott commented 1 month ago

Yes, I just verified both slack run and slack deploy get the error.

In addition, I also noticed I need to provide the domain of my workspace... XXXX.enterprise.slack.com I would think that should be allowed, but I can understand why it isn't.

filmaj commented 1 month ago

It seems initially we didn't want to pre-approve files.slack.com in order to ensure admins can gate access to file uploading features via domain approval, but given that file uploading requires its own specific scope (files:write), I think that's an acceptable path to ensure admins have control over apps with this feature.

Separate topic: @mcsescott what are you building such that you needed to access the X.enterprise.slack.com URL? I wonder if this falls into the same category.

mcsescott commented 1 month ago

👋 Hi @filmaj You know I'm always trying to push the boundaries....

Use case: Because the APIs aren't available to read a Canvas, the workaround we were given by the Canvas PM was to download the file and parse the contents.

I actually have it working now, but have added the domains to my manifest.

filmaj commented 1 month ago

And we very much appreciate you boldly going where (clearly) no coded workflows have gone before. 👨‍🚀 ⭐

Just wanted to verify that the use case was one that would be more widely needed. I think we should consider adding some kind of blanket pre-approval for a bunch of Slack domains, including *.enterprise.slack.com. For example, if you decide to deploy your app to a different workspace with a different workspace URL, womp womp, you need to edit the manifest.

I will bring this up internally and see what we can do.

filmaj commented 1 month ago

Hey @mcsescott , FYI if you update your deno-slack-hooks to 1.3.1 (should be referenced in your slack.json) local-running apps now automatically approve files.slack.com. We also just deployed changes to the run-on-slack environment to do the same.

mcsescott commented 1 month ago

Thank you sir!