fix(slack): replace deprecated @slack/events-api with native crypto validation#4313
fix(slack): replace deprecated @slack/events-api with native crypto validation#4313angelcaamal wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request replaces the @slack/events-api dependency with a manual implementation of Slack webhook signature verification using the Node.js crypto module. The changes include a new verifyWebhook function and updated integration and unit tests to support the manual signing process. Feedback was provided to enhance security by implementing replay attack protection via timestamp verification, adding explicit checks for the signing secret, and optimizing the HMAC calculation process.
| const {SLACK_SECRET} = process.env; | ||
| const SLACK_TIMESTAMP = Date.now(); | ||
| process.env.SLACK_SECRET = process.env.SLACK_SECRET || 'test-slack-secret'; | ||
| const SLACK_SECRET = process.env.SLACK_SECRET; |
There was a problem hiding this comment.
Please do not change the values of global states like env variables, since it's not a generally a good habit in practice. Just pull the value directly in your code's const where you can use the ternary operator. However, also is good practice to raise an exception when an environment variable is not set, as it is a good practice to avoid obfuscating errors in real life.
| const SLACK_SECRET = process.env.SLACK_SECRET; | |
| if (process.env.SLACK_SECRET is not undefined){ | |
| const SLACK_SECRET = process.env.SLACK_SECRET; | |
| } | |
| else: | |
| raise("Error, SLACK_SECRET env var is not set.") |
p.d: pardon my node.js syntax, might not be accurate.
| const SLACK_TOKEN = 'slack-token'; | ||
| const KG_API_KEY = 'kg-api-key'; | ||
| process.env.SLACK_SECRET = process.env.SLACK_SECRET || 'slack-token'; | ||
| process.env.KG_API_KEY = process.env.KG_API_KEY || 'test-kg-api-key'; |
There was a problem hiding this comment.
Same as above: do not change var envs, raise exception on being unset.
Description
This PR refactors the Slack function sample to remove the deprecated
@slack/events-apilibrary. The request validation has been rewritten to use manual signature verification with the native Node.jscryptomodule.Additionally, the test suite has been updated to reflect these changes and ensure stability in the CI pipeline.
Fixes Internal: b/414440396
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
npm test(see Testing)npm run lint(see Style)GoogleCloudPlatform/nodejs-docs-samples. Not a fork.