search

slackapi / deno-slack-hooks

Helper library implementing the contract between the Slack CLI and Slack application SDKs
MIT License
11 stars 3 forks source link

[FEATURE] get-manifest hook should remove protocol from `outgoingDomains` if present #95

Open filmaj opened 2 months ago

filmaj commented 2 months ago

Very annoying sharp edge: if you happen to include the protocol in outgoingDomain entries, you will get a cryptic error message when you go to deploy or run your app:

➜ slak deploy
The following error was returned by the apps.manifest.validate Slack API method

🚫 The provided manifest file does not validate against schema. Consult the additional errors field to locate specific issues (invalid_manifest)
   input must match regex pattern: ^(?![\.\-])([-a-zA-Z0-9\.])+([a-zA-Z0-9])$ (failed_constraint)
   Source: /outgoing_domains/0

In this case, I had outgoingDomains: ['https://google.com'] in my manifest. Dropping https:// fixes the issue.

My suggested solution is for the get-manifest hook to parse these entries using the URL module, and extract only the hostname, dropping protocol. Feels like it belongs squarely in the cleanManifest function of the get-manifest hook.

filmaj commented 2 months ago

This was filed on the deno-slack-sdk here: https://github.com/slackapi/deno-slack-sdk/issues/245 And there's even an open PR for it here, but couldn't get the contributor to get that PR over the finish line: https://github.com/slackapi/deno-slack-sdk/pull/246