Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
filmaj opened this issue Sep 5, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@filmaj
Copy link

filmaj commented Sep 5, 2024

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 filmaj added the enhancement New feature or request label Sep 5, 2024
@filmaj
Copy link
Author

filmaj commented Sep 5, 2024

This was filed on the deno-slack-sdk here: slackapi/deno-slack-sdk#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: slackapi/deno-slack-sdk#246

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant