fix: enforce validation for Incoming WebHook alias field (#38849)#38909
fix: enforce validation for Incoming WebHook alias field (#38849)#38909TechByAnshuu wants to merge 2 commits intoRocketChat:developfrom
Conversation
…at#38849) Problem: The Alias field in Incoming WebHook integrations accepted any input without validation — unlimited length and arbitrary special characters were silently saved and rendered in chat messages, causing UI breakage and inconsistent behavior with other webhook fields. Solution: - Add shared constants (ALIAS_MAX_LENGTH: 50, ALIAS_ALLOWED_PATTERN: Unicode /^[\p{L}\p{N} _-]*$/u) to packages/core-typings as a single source of truth for both client and server - Update packages/core-typings/tsconfig.json to target es2020 to support Unicode property escape compilation - Enforce maxLength and pattern constraints in IntegrationsCreateProps.ts and IntegrationsUpdateProps.ts REST schemas via Ajv — rejects malformed aliases at API level before any DB write - Integrate react-hook-form validation in IncomingWebhookForm.tsx with live character counter (12/50) and inline FieldError - Set HTML maxLength as browser-level hard cap - Add 7 end-to-end test cases covering: over-limit length, invalid characters, boundary (50 chars), valid alias, Unicode alias (Hürriyet_日本語_عربى), omitted alias, and empty string alias. Closes RocketChat#38849 Co-authored-by: roushnisareen <roushnisareen@gmail.com>
|
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 |
|
|
|
WalkthroughThis PR implements validation for the incoming webhook alias field, enforcing a maximum length of 50 characters and restricting characters to alphanumeric, underscore, hyphen, and Unicode characters. Changes span UI form validation, API schemas, shared constants, and end-to-end tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 7 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/core-typings/src/integrations/constants.ts">
<violation number="1" location="packages/core-typings/src/integrations/constants.ts:3">
P2: Alias validation regex excludes Unicode combining marks (\p{M}), which will reject valid aliases for scripts that require combining marks (e.g., Hindi/Devanagari, Thai, fully vocalized Arabic).</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,6 @@ | |||
| export const ALIAS_MAX_LENGTH = 50; | |||
|
|
|||
| export const ALIAS_ALLOWED_PATTERN = /^[\p{L}\p{N} _-]*$/u; | |||
There was a problem hiding this comment.
P2: Alias validation regex excludes Unicode combining marks (\p{M}), which will reject valid aliases for scripts that require combining marks (e.g., Hindi/Devanagari, Thai, fully vocalized Arabic).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/core-typings/src/integrations/constants.ts, line 3:
<comment>Alias validation regex excludes Unicode combining marks (\p{M}), which will reject valid aliases for scripts that require combining marks (e.g., Hindi/Devanagari, Thai, fully vocalized Arabic).</comment>
<file context>
@@ -0,0 +1,6 @@
+export const ALIAS_MAX_LENGTH = 50;
+
+export const ALIAS_ALLOWED_PATTERN = /^[\p{L}\p{N} _-]*$/u;
+
+export const ALIAS_ALLOWED_PATTERN_DESCRIPTION =
</file context>
|
This is my first contribution to Rocket.Chat. I have gone through the Happy to make any changes based on your feedback — whether that is adjusting Looking forward to your review. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/meteor/tests/end-to-end/api/incoming-integrations.ts (1)
177-177: Import and useALIAS_MAX_LENGTHinstead of hardcoded51/50.If
ALIAS_MAX_LENGTHever changes, these tests will silently test the wrong boundary. Importing the constant keeps the tests automatically in sync with the validation rule.♻️ Proposed fix
At the top of the file, add:
+ import { ALIAS_MAX_LENGTH } from '@rocket.chat/core-typings';Then in the test bodies:
- alias: 'a'.repeat(51), + alias: 'a'.repeat(ALIAS_MAX_LENGTH + 1),- alias: 'a'.repeat(50), + alias: 'a'.repeat(ALIAS_MAX_LENGTH),Also applies to: 215-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/meteor/tests/end-to-end/api/incoming-integrations.ts` at line 177, Import the validation constant ALIAS_MAX_LENGTH at the top of the test file and replace hardcoded usages like alias: 'a'.repeat(51) (and the other occurrence at the later test) with dynamic expressions using that constant: use alias: 'a'.repeat(ALIAS_MAX_LENGTH + 1) for the "too long" case and alias: 'a'.repeat(ALIAS_MAX_LENGTH) for the boundary/valid case so tests remain correct if ALIAS_MAX_LENGTH changes; update the two alias properties in the test bodies accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsxapps/meteor/tests/end-to-end/api/incoming-integrations.tspackages/core-typings/src/index.tspackages/core-typings/src/integrations/constants.tspackages/core-typings/tsconfig.jsonpackages/rest-typings/src/v1/integrations/IntegrationsCreateProps.tspackages/rest-typings/src/v1/integrations/IntegrationsUpdateProps.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/core-typings/src/index.tspackages/core-typings/src/integrations/constants.tsapps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsxpackages/rest-typings/src/v1/integrations/IntegrationsUpdateProps.tsapps/meteor/tests/end-to-end/api/incoming-integrations.tspackages/rest-typings/src/v1/integrations/IntegrationsCreateProps.ts
🧠 Learnings (8)
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/core-typings/tsconfig.json
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
packages/core-typings/tsconfig.jsonapps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 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
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
apps/meteor/tests/end-to-end/api/incoming-integrations.ts
🧬 Code graph analysis (4)
apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx (1)
packages/core-typings/src/integrations/constants.ts (3)
ALIAS_MAX_LENGTH(1-1)ALIAS_ALLOWED_PATTERN(3-3)ALIAS_ALLOWED_PATTERN_DESCRIPTION(5-6)
packages/rest-typings/src/v1/integrations/IntegrationsUpdateProps.ts (1)
packages/core-typings/src/integrations/constants.ts (2)
ALIAS_MAX_LENGTH(1-1)ALIAS_ALLOWED_PATTERN(3-3)
apps/meteor/tests/end-to-end/api/incoming-integrations.ts (2)
packages/core-services/src/index.ts (1)
api(55-55)apps/meteor/app/integrations/server/lib/triggerHandler.ts (1)
removeIntegration(100-104)
packages/rest-typings/src/v1/integrations/IntegrationsCreateProps.ts (1)
packages/core-typings/src/integrations/constants.ts (2)
ALIAS_MAX_LENGTH(1-1)ALIAS_ALLOWED_PATTERN(3-3)
🔇 Additional comments (3)
packages/core-typings/src/integrations/constants.ts (1)
1-6: LGTM!Constants are cleanly defined. The
uflag onALIAS_ALLOWED_PATTERNcorrectly enables Unicode property escapes (\p{L},\p{N}), and the*quantifier intentionally permits an empty alias.packages/core-typings/src/index.ts (1)
26-26: LGTM!Re-export is correctly positioned alongside other integration-related exports.
packages/rest-typings/src/v1/integrations/IntegrationsCreateProps.ts (1)
91-93: No action required. The codebase uses Ajv v8.17.1, which defaults tounicodeRegExp: trueand automatically applies theuflag when compiling thepatternkeyword. Unicode property escapes (\p{L},\p{N}) inALIAS_ALLOWED_PATTERN.sourceare processed correctly with no configuration needed.Likely an incorrect or invalid review comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx`:
- Around line 238-241: The validation message ALIAS_ALLOWED_PATTERN_DESCRIPTION
is not localized; update the incoming webhook form to use the translation
function instead of the hardcoded constant: replace the direct use of
ALIAS_ALLOWED_PATTERN_DESCRIPTION in the pattern.message (inside the component
handling validation in IncomingWebhookForm, where pattern: { value:
ALIAS_ALLOWED_PATTERN, message: ALIAS_ALLOWED_PATTERN_DESCRIPTION }) with
t('Alias_allowed_characters') (or t('Alias_allowed_pattern_description')) and
add the matching key/value to the i18n JSON resources so the validator falls
back to the translated string.
- Around line 244-268: The TextInput for the alias field is passing an error
prop (error={errors?.alias?.message}) which duplicates the message already
rendered by the separate <FieldError> block; remove the redundant error prop
from the TextInput inside the Controller render (the JSX using TextInput with
id={aliasField} and {...field}) and keep only
aria-invalid={Boolean(errors?.alias)} plus the existing <FieldError> that
displays errors.alias.message so the alias validation message is shown once.
In `@packages/core-typings/tsconfig.json`:
- Around line 5-9: This tsconfig includes browser libs unnecessarily; remove
"dom" and "dom.iterable" from the "lib" array so only "es2020" remains, ensuring
server-side typings are used (it extends the server tsconfig) and the
ALIAS_ALLOWED_PATTERN regex that relies on Unicode property escapes (\p{L},
\p{N}) will still be supported by es2020; update the "lib" array entries in the
current tsconfig (remove the "dom" and "dom.iterable" strings) and run a quick
typecheck to confirm no browser globals (Window, Document, HTMLElement) are
exposed.
---
Duplicate comments:
In `@packages/rest-typings/src/v1/integrations/IntegrationsUpdateProps.ts`:
- Around line 97-99: The pattern is passing a RegExp.source
(ALIAS_ALLOWED_PATTERN.source) which can misbehave with Ajv's Unicode handling;
replace usage with a dedicated string pattern export (e.g., export const
ALIAS_ALLOWED_PATTERN_STRING = ALIAS_ALLOWED_PATTERN.toString().slice(1, -1) or
explicitly defined pattern string) and use that string in
IntegrationsUpdateProps (replace ALIAS_ALLOWED_PATTERN.source with
ALIAS_ALLOWED_PATTERN_STRING) for both pattern occurrences so Ajv receives a
proper string regex and Unicode behavior is deterministic.
---
Nitpick comments:
In `@apps/meteor/tests/end-to-end/api/incoming-integrations.ts`:
- Line 177: Import the validation constant ALIAS_MAX_LENGTH at the top of the
test file and replace hardcoded usages like alias: 'a'.repeat(51) (and the other
occurrence at the later test) with dynamic expressions using that constant: use
alias: 'a'.repeat(ALIAS_MAX_LENGTH + 1) for the "too long" case and alias:
'a'.repeat(ALIAS_MAX_LENGTH) for the boundary/valid case so tests remain correct
if ALIAS_MAX_LENGTH changes; update the two alias properties in the test bodies
accordingly.
| pattern: { | ||
| value: ALIAS_ALLOWED_PATTERN, | ||
| message: ALIAS_ALLOWED_PATTERN_DESCRIPTION, | ||
| }, |
There was a problem hiding this comment.
ALIAS_ALLOWED_PATTERN_DESCRIPTION is not translated.
Every other validation error message in this form goes through t() (e.g., t('Max_length_is', ...), t('Required_field', ...)). ALIAS_ALLOWED_PATTERN_DESCRIPTION is a hardcoded English string and will not be localized for non-English users. Either add a translation key to the i18n resources and use t('Alias_allowed_pattern_description'), or pass the constant only as a fallback.
♻️ Proposed fix — use translation
pattern: {
value: ALIAS_ALLOWED_PATTERN,
- message: ALIAS_ALLOWED_PATTERN_DESCRIPTION,
+ message: t('Alias_allowed_characters'),
},Then add the key to the relevant i18n JSON:
"Alias_allowed_characters": "Only letters, numbers, spaces, hyphens, and underscores are allowed"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx`
around lines 238 - 241, The validation message ALIAS_ALLOWED_PATTERN_DESCRIPTION
is not localized; update the incoming webhook form to use the translation
function instead of the hardcoded constant: replace the direct use of
ALIAS_ALLOWED_PATTERN_DESCRIPTION in the pattern.message (inside the component
handling validation in IncomingWebhookForm, where pattern: { value:
ALIAS_ALLOWED_PATTERN, message: ALIAS_ALLOWED_PATTERN_DESCRIPTION }) with
t('Alias_allowed_characters') (or t('Alias_allowed_pattern_description')) and
add the matching key/value to the i18n JSON resources so the validator falls
back to the translated string.
| <TextInput | ||
| id={aliasField} | ||
| {...field} | ||
| maxLength={ALIAS_MAX_LENGTH} | ||
| aria-describedby={`${aliasField}-hint ${aliasField}-error`} | ||
| aria-invalid={Boolean(errors?.alias)} | ||
| error={errors?.alias?.message} | ||
| addon={ | ||
| <> | ||
| <Box is='span' fontScale='micro' color='hint' mie='x8'> | ||
| {alias?.length ?? 0}/{ALIAS_MAX_LENGTH} | ||
| </Box> | ||
| <Icon name='edit' size='x20' /> | ||
| </> | ||
| } | ||
| /> | ||
| )} | ||
| /> | ||
| </FieldRow> | ||
| <FieldHint id={`${aliasField}-hint`}>{t('Choose_the_alias_that_will_appear_before_the_username_in_messages')}</FieldHint> | ||
| {errors?.alias && ( | ||
| <FieldError aria-live='assertive' id={`${aliasField}-error`}> | ||
| {errors.alias.message} | ||
| </FieldError> | ||
| )} |
There was a problem hiding this comment.
Redundant error prop causes double error rendering.
The error={errors?.alias?.message} prop on TextInput (line 250) renders the error message inside the input component. The <FieldError> block at lines 264–268 renders the same message again. Every other validated field in this component (channel, username) follows the correct pattern: only aria-invalid on TextInput plus a separate <FieldError>, with no error prop on the input itself.
🐛 Proposed fix — remove the redundant `error` prop
<TextInput
id={aliasField}
{...field}
maxLength={ALIAS_MAX_LENGTH}
aria-describedby={`${aliasField}-hint ${aliasField}-error`}
aria-invalid={Boolean(errors?.alias)}
- error={errors?.alias?.message}
addon={📝 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.
| <TextInput | |
| id={aliasField} | |
| {...field} | |
| maxLength={ALIAS_MAX_LENGTH} | |
| aria-describedby={`${aliasField}-hint ${aliasField}-error`} | |
| aria-invalid={Boolean(errors?.alias)} | |
| error={errors?.alias?.message} | |
| addon={ | |
| <> | |
| <Box is='span' fontScale='micro' color='hint' mie='x8'> | |
| {alias?.length ?? 0}/{ALIAS_MAX_LENGTH} | |
| </Box> | |
| <Icon name='edit' size='x20' /> | |
| </> | |
| } | |
| /> | |
| )} | |
| /> | |
| </FieldRow> | |
| <FieldHint id={`${aliasField}-hint`}>{t('Choose_the_alias_that_will_appear_before_the_username_in_messages')}</FieldHint> | |
| {errors?.alias && ( | |
| <FieldError aria-live='assertive' id={`${aliasField}-error`}> | |
| {errors.alias.message} | |
| </FieldError> | |
| )} | |
| <TextInput | |
| id={aliasField} | |
| {...field} | |
| maxLength={ALIAS_MAX_LENGTH} | |
| aria-describedby={`${aliasField}-hint ${aliasField}-error`} | |
| aria-invalid={Boolean(errors?.alias)} | |
| addon={ | |
| <> | |
| <Box is='span' fontScale='micro' color='hint' mie='x8'> | |
| {alias?.length ?? 0}/{ALIAS_MAX_LENGTH} | |
| </Box> | |
| <Icon name='edit' size='x20' /> | |
| </> | |
| } | |
| /> | |
| )} | |
| </FieldRow> | |
| <FieldHint id={`${aliasField}-hint`}>{t('Choose_the_alias_that_will_appear_before_the_username_in_messages')}</FieldHint> | |
| {errors?.alias && ( | |
| <FieldError aria-live='assertive' id={`${aliasField}-error`}> | |
| {errors.alias.message} | |
| </FieldError> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/client/views/admin/integrations/incoming/IncomingWebhookForm.tsx`
around lines 244 - 268, The TextInput for the alias field is passing an error
prop (error={errors?.alias?.message}) which duplicates the message already
rendered by the separate <FieldError> block; remove the redundant error prop
from the TextInput inside the Controller render (the JSX using TextInput with
id={aliasField} and {...field}) and keep only
aria-invalid={Boolean(errors?.alias)} plus the existing <FieldError> that
displays errors.alias.message so the alias validation message is shown once.
| "lib": [ | ||
| "dom", | ||
| "dom.iterable", | ||
| "es2020" | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether any existing file in packages/core-typings/src references DOM-only types
rg -n "\b(HTMLElement|HTMLInputElement|Window|Document|EventTarget|MouseEvent|KeyboardEvent|CustomEvent)\b" packages/core-typings/src/Repository: RocketChat/Rocket.Chat
Length of output: 269
🏁 Script executed:
cat -n packages/core-typings/tsconfig.jsonRepository: RocketChat/Rocket.Chat
Length of output: 544
🏁 Script executed:
rg -n "ALIAS_ALLOWED_PATTERN" packages/core-typings/src/ -A 2 -B 2Repository: RocketChat/Rocket.Chat
Length of output: 608
Remove dom and dom.iterable from the lib array.
packages/core-typings extends @rocket.chat/tsconfig/server.json, indicating server-side usage. The ALIAS_ALLOWED_PATTERN regex uses Unicode property escapes (\p{L}, \p{N}), which require only es2020—not DOM types. Adding dom and dom.iterable unnecessarily exposes browser-specific globals (Window, Document, HTMLElement, etc.) to server-side consumers, risking accidental browser API usage in server-only code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core-typings/tsconfig.json` around lines 5 - 9, This tsconfig
includes browser libs unnecessarily; remove "dom" and "dom.iterable" from the
"lib" array so only "es2020" remains, ensuring server-side typings are used (it
extends the server tsconfig) and the ALIAS_ALLOWED_PATTERN regex that relies on
Unicode property escapes (\p{L}, \p{N}) will still be supported by es2020;
update the "lib" array entries in the current tsconfig (remove the "dom" and
"dom.iterable" strings) and run a quick typecheck to confirm no browser globals
(Window, Document, HTMLElement) are exposed.
2 participants
Description
Problem
The
Aliasfield in Incoming WebHook integrations had no validation of any kind. Aliases of unlimited length or containing arbitrary special characters were silently accepted and rendered as the message sender name in chat — causing UI overflow, inconsistent behavior with other validated webhook fields, and no rejection even when calling the REST API directly, bypassing the UI entirely.Solution
This PR introduces dual-layer validation — enforced independently at both the client and server — with a Unicode-aware character pattern so the fix works correctly for all Rocket.Chat workspaces globally, not just English ones.
Changes
⚙️ Server
packages/core-typings/src/integrations/constants.ts— centralized single source of truth exporting:ALIAS_MAX_LENGTH→50ALIAS_ALLOWED_PATTERN→/^[\p{L}\p{N} _-]*$/uALIAS_ALLOWED_PATTERN_DESCRIPTION→ human-readable error stringpackages/core-typings/tsconfig.jsonto targetes2020— required for TypeScript to compile Unicode property escapes without errorspackages/core-typings/src/index.tsfor cross-package consumptionmaxLengthandpatternconstraints toaliasin both:
Both endpoints are protected — a developer calling the API directly via curl or Postman is rejected before any database write occursIntegrationsCreateProps.tsIntegrationsUpdateProps.ts🎨 Client
react-hook-formvalidation on thealiasinput inIncomingWebhookForm.tsx12/50shown as input addon — user is guided before hitting SaveFieldErrordisplayed instantly on invalid inputmaxLengthattribute set as browser-level hard cap🧪 Tests
7 end-to-end test cases added to
incoming-integrations.ts:Issue
Closes #38849
Verification Steps
Reproduce the original bug (on
developwithout this PR):Administration → Workspace → Integrations → New Integration → Incoming WebHook@#$$in the Alias field and save — accepted with no errorVerify the fix (on this branch):
Administration → Workspace → Integrations → New Integration → Incoming WebHook51/50, inline error appears, Save is disabled@#$$— inline error appears instantlyMy-Bot_v2— no error, Save enabled, webhook posts correctlyHürriyet_日本語— accepted, no error (Unicode support confirmed)Run tests:
Design Decisions
Why Unicode pattern instead of
a-zA-Z0-9? Rocket.Chat is a global product. An ASCII-only pattern silently rejects valid bot names in Arabic, Japanese, German, and Hindi workspaces with no explanation to the user./^[\p{L}\p{N} _-]*$/uhandles all Unicode scripts correctly with a single pattern and zero special cases.Why dual-layer validation? Client validation is UX. Server validation is security. A developer calling the REST API directly bypasses the React form entirely — Ajv schema enforcement is the only layer that protects the database in that path.
Why 50 characters? The alias renders as the sender name above every webhook message. 50 characters fits within Rocket.Chat's message UI on the smallest commonly supported viewport without overflow or truncation.
Why
es2020in tsconfig? Unicode property escapes require ES2018+ to compile. The previous target caused TypeScript to error on the/uflag.es2020aligns with what the broader Rocket.Chat codebase already targets and introduces no breaking changes.Summary by CodeRabbit