Skip to content

[MM-69470] Reject Zoom webhooks when the Zoom webhook secret is unset - #483

Open
nang2049 wants to merge 1 commit into
masterfrom
MM-69470
Open

[MM-69470] Reject Zoom webhooks when the Zoom webhook secret is unset#483
nang2049 wants to merge 1 commit into
masterfrom
MM-69470

Conversation

@nang2049

@nang2049 nang2049 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

verifyZoomWebhookSignature returned nil when ZoomWebhookSecret was empty, silently skipping HMAC-SHA256 verification. With the secret unset, the only remaining check on /plugins/zoom/webhook was the secret query parameter, so an unsigned request carrying the correct WebhookSecret was accepted and processed and a forged meeting.ended event could overwrite the bot's meeting post.

This now fails closes as an unset ZoomWebhookSecret returns an error and the request is rejected with 401.

Ticket Link

https://mattermost.atlassian.net/browse/MM-69470

QA Test Steps

  1. Configure the Zoom plugin with both Webhook Secret and Zoom Webhook Secret set, and validate the webhook URL from the Zoom App Marketplace. Validation succeeds.
  2. Run /zoom start in a channel, join the meeting, then end it. The bot post updates to show the meeting has ended.
  3. Clear Zoom Webhook Secret in the System Console and save. Repeat step 2. The bot post no longer updates, and the server log shows Could not verify webhook signature: zoom webhook secret not set. This is the intended fail-closed behaviour.
  4. Restore Zoom Webhook Secret. Step 2 works again.

Change Impact: 🟡 Medium

Reasoning: The change modifies webhook authentication behavior and rejects unsigned requests when ZoomWebhookSecret is unset. The scope is isolated, and tests cover the affected paths.

Regression Risk: Medium. Existing configurations must define ZoomWebhookSecret. Misconfigured integrations will receive HTTP 401 responses.

** QA Recommendation:** Perform focused manual QA with configured and missing secrets. Skipping manual QA has moderate risk because the change affects webhook authentication.

Generated by CodeRabbitAI

@nang2049
nang2049 requested a review from a team as a code owner August 6, 2026 09:44
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3e202977-e732-4328-961a-04eae762f6d6

📥 Commits

Reviewing files that changed from the base of the PR and between 096c126 and 9e7d891.

📒 Files selected for processing (4)
  • plugin.json
  • server/plugin_test.go
  • server/webhook.go
  • server/webhook_test.go

📝 Walkthrough

Walkthrough

Zoom webhook verification now rejects requests when the secret is empty. Webhook tests generate valid signatures with a configured secret and verify that empty-secret requests return 401 Unauthorized. The setting description documents the requirement.

Changes

Zoom webhook security

Layer / File(s) Summary
Require a configured webhook secret
plugin.json, server/webhook.go, server/webhook_test.go
The setting description marks the Zoom webhook Secret Token as required. Verification rejects empty secrets. Tests cover unsigned and signed requests with an empty secret.
Sign webhook test requests
server/plugin_test.go
Integration tests generate timestamped Zoom signatures and configure the matching webhook secret for stopped and started webhook requests.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: avasconcelos114

Poem

I’m a rabbit with a signed little note,
A secret now guards every webhook boat.
Empty tokens turn requests away,
Valid hashes guide tests through the day.
Hop, hop—secure signals stay!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: rejecting Zoom webhooks when the webhook secret is unset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch MM-69470

Comment @coderabbitai help to get the list of available commands.

@avasconcelos114 avasconcelos114 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

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.

2 participants