-
Notifications
You must be signed in to change notification settings - Fork 13k
fix(integrations): Incoming webhook 'Emoji' field has no validation #37452
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
base: develop
Are you sure you want to change the base?
fix(integrations): Incoming webhook 'Emoji' field has no validation #37452
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: cc0a775 The changes in this PR will be included in the next version bump. This PR includes changesets to release 41 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughAdds client-side validation for Incoming Webhook Avatar URL and Emoji fields; updates form error UI/ARIA. Includes three changeset release notes and minor formatting/type helper edits in related webhook editor files. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Form as IncomingWebhookForm
participant Validators as Validator functions
participant UI as Field UI
User->>Form: Populate Avatar_URL
Form->>Validators: validateAvatarUrl(value)
alt valid http/https URL
Validators-->>Form: no error
Form->>UI: ensure no error shown
else invalid URL
Validators-->>Form: error message
Form->>UI: show FieldError, set aria-invalid
end
User->>Form: Populate Emoji
Form->>Validators: validateEmoji(value)
alt valid (:name:) or single pictograph or empty
Validators-->>Form: no error
Form->>UI: ensure no error shown
else invalid format
Validators-->>Form: error message
Form->>UI: show FieldError, set aria-invalid
end
User->>Form: Submit
alt all validations pass
Form-->>User: proceed to save
else validation fails
Form-->>User: prevent save, display errors
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.changeset/funny-islands-run.md(1 hunks).changeset/light-singers-join.md(1 hunks).changeset/mighty-queens-hug.md(1 hunks)apps/meteor/client/views/admin/integrations/incoming/EditIncomingWebhook.tsx(3 hunks)apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx(2 hunks)apps/meteor/client/views/admin/integrations/outgoing/EditOutgoingWebhook.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-18T17:32:33.969Z
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
Applied to files:
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx
🪛 ast-grep (0.39.7)
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx
[warning] 318-318: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx
[error] 319-319: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
| '@rocket.chat/meteor': major | ||
| --- | ||
|
|
||
| fix(integrations): Add validation for Incoming Webhook Avatar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changelog entry should reflect the primary fix (emoji validation) and mention both validations added.
The changeset message only mentions "Avatar" validation, but according to the PR objectives and the linked issue #37451, the primary fix is for emoji field validation. The AI summary context also indicates that both validateAvatarUrl and validateEmoji validators are being added. The changelog entry should be more accurate and complete.
Consider updating the changelog message to clarify the primary objective and mention both validations:
- fix(integrations): Add validation for Incoming Webhook Avatar
+ fix(integrations): Add validation for Incoming Webhook Emoji and Avatar URLAlternatively, if emoji validation is the primary fix:
- fix(integrations): Add validation for Incoming Webhook Avatar
+ fix(integrations): Add validation for Incoming Webhook Emoji field format📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fix(integrations): Add validation for Incoming Webhook Avatar | |
| fix(integrations): Add validation for Incoming Webhook Emoji and Avatar URL |
🤖 Prompt for AI Agents
In .changeset/funny-islands-run.md around line 5, the changelog line only
mentions "Avatar" validation but the PR primarily fixes emoji field validation
and adds both validateAvatarUrl and validateEmoji; update the changelog entry to
clearly state the primary fix (emoji validation) and also mention that avatar
URL validation was added so both validations are reflected in the message.
| const validateEmoji = (value: string) => { | ||
| if (!value) return true; | ||
|
|
||
| if (/^:.+:$/.test(value)) { | ||
| return true; | ||
| } | ||
|
|
||
| return t('Invalid_emoji_format_Must_be_in_colon_format', { example: ':ghost:' }); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Allow actual emoji glyphs as well
The new validator only accepts :name: patterns, but the issue description and help text say the field should also allow a single real emoji (e.g. 🙂). With the current regex that input is rejected, so users still get blocked. Please extend the validator (and error copy) so both formats pass—for example by accepting ^:.+:$ or a single Extended_Pictographic code point:
- if (/^:.+:$/.test(value)) {
- return true;
- }
-
- return t('Invalid_emoji_format_Must_be_in_colon_format', { example: ':ghost:' });
+ if (/^:.+:$/.test(value)) {
+ return true;
+ }
+
+ if ([...value.trim()].length === 1 && /\p{Extended_Pictographic}/u.test(value)) {
+ return true;
+ }
+
+ return t('Invalid_emoji_format_Must_be_in_colon_format', { example: ':ghost:' });Please also update the error message so it mentions both accepted formats after adjusting the translation.
🤖 Prompt for AI Agents
In apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx
around lines 48 to 56, the validator currently only accepts colon-wrapped
shortcode patterns and rejects actual emoji glyphs; update the validation to
accept either a :name: pattern OR a single Extended_Pictographic emoji (e.g. use
a regex like /^:.+:$|^\p{Extended_Pictographic}$/u or equivalent unicode-aware
check) while keeping empty values valid, and change the error
message/translation key used to mention both accepted formats (colon shortcodes
and single emoji glyph) so the UI reflects the new validation.
apps/meteor/client/views/admin/integrations/outgoing/EditOutgoingWebhook.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx (1)
48-60: Fix indentation inconsistency.The validation logic correctly handles both colon-wrapped emoji (
:ghost:) and actual emoji glyphs as required. However, lines 52-53 have inconsistent indentation.Apply this diff to fix the indentation:
const validateEmoji = (value: string) => { if (!value) return true; if (/^:.+:$/.test(value)) { - return true; - } + return true; + } if ([...value.trim()].length === 1 && /\p{Extended_Pictographic}/u.test(value)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.changeset/mighty-queens-hug.md(1 hunks)apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx(2 hunks)apps/meteor/client/views/admin/integrations/outgoing/EditOutgoingWebhook.tsx(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/meteor/client/views/admin/integrations/outgoing/EditOutgoingWebhook.tsx
- .changeset/mighty-queens-hug.md
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
📚 Learning: 2025-09-18T17:32:33.969Z
Learnt from: aleksandernsilva
Repo: RocketChat/Rocket.Chat PR: 36974
File: apps/meteor/client/components/Omnichannel/OutboundMessage/components/OutboundMessageWizard/forms/MessageForm/MessageForm.tsx:124-129
Timestamp: 2025-09-18T17:32:33.969Z
Learning: The ARIA mismatch issue in MessageForm's template field (where FieldError id used templateId instead of messageFormId) was addressed in PR #36972 through refactoring the template field into a separate TemplateField component, which uses consistent templateFieldId for both aria-describedby and FieldError id.
Applied to files:
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx
🪛 ast-grep (0.39.7)
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx
[warning] 322-322: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🪛 Biome (2.1.2)
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx
[error] 323-323: Avoid passing content using the dangerouslySetInnerHTML prop.
Setting content using code can expose users to cross-site scripting (XSS) attacks
(lint/security/noDangerouslySetInnerHtml)
🔇 Additional comments (3)
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx (3)
35-46: LGTM! Clean URL validation.The validation logic correctly handles HTTP/HTTPS URLs, allows empty values, and returns translated error messages for invalid input.
267-293: LGTM! Proper validation and ARIA implementation.The avatar field validation follows the same pattern as the channel and username fields in this file. The ARIA attributes are correctly configured with matching error ids, and the FieldError is properly positioned.
295-325: LGTM! Consistent validation implementation.The emoji field validation correctly mirrors the avatar field pattern with proper ARIA attributes and error handling. The static analysis warnings about
dangerouslySetInnerHTMLon line 323 are false positives—the content is sanitized usingDOMPurify.sanitize, which is the correct approach and consistent with other usages in this file (lines 142, 210).
[x] I have signed the CLA - https://cla-assistant.io/RocketChat/Rocket.Chat
[x] Lint and unit tests pass locally with my changes
[x] I have tested my fix locally and it is effective
[ ] I have added tests that prove my fix is effective or that my feature works (if applicable)
[ ] I have added necessary documentation (if applicable)
[ ] Any dependent changes have been merged and published in downstream modules
This PR fixes a bug in the Incoming Webhook form where the "Emoji" field was not being validated.
Previously, the form would accept any invalid string (e.g., "myemoji" or "ibjkn"), even though the help text implies a colon-wrapped format (e.g., 👻). This would save the integration but result in a broken avatar icon for all messages posted by the webhook.
This fix adds client-side validation using the react-hook-form validate prop. The logic checks if the input is either a single, valid emoji character OR if it follows the :emoji_name: format.
If the validation fails, it now triggers the validateEmoji error to show the error notification under the emoji filed (e.g., "Invalid emoji format"), preventing the form from being saved in a broken state.
Issue(s)
Closes #37451
Steps to test or reproduce
Navigate to Administration > Integrations.
Click the "New" button in the top-right corner.
Select the "Incoming" tab.
Fill in the required fields (e.g., Name: Test Emoji, Post to Channel: #general, Post as: kalyan12).
Scroll down to the "Emoji" field.
Type an invalid string that is not an emoji (e.g., ibjkn).
Click the "Save" button.
Before this fix: The form would save successfully.
After this fix: A red pop-up error toast appears (e.g., "Invalid emoji format"), and the form does not save.
Further comments
This bug appears to be part of a larger pattern of missing validation on the integration forms, similar to the URLs field bug (#37411) and the Avatar URL bug (#37423).
This fix follows the same pattern as the others:
The validateEmoji logic is added to the parent component (EditIncomingWebhookPage.tsx).
The handleInvalid function in the parent is used to trigger the error toast.
The rules prop is added to the in the child component (IncomingWebhookForm.tsx).
Summary by CodeRabbit
Bug Fixes
Chores