fix!: `title` and `value` properties should be required on `attachments.fields` #37233

yash-rajpal · 2025-10-15T15:44:23Z

Proposed changes (including videos or screenshots)

Adds validation on API endpoints for required attachment.fields properties. It shouldn't be possible to send a message with missing title and value properties.

Issue(s)

Steps to test or reproduce

Further comments

CORE-1498

Summary by CodeRabbit

Bug Fixes
- Stricter validation for message attachments: each attachment field now requires both title and value. Requests missing either return a 400 error with a clear message; valid attachments post successfully.
Tests
- Added end-to-end tests verifying errors for missing title or value and success when both are provided.
Chores
- Minor version bump recorded and release notes updated to reflect required attachment field properties.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

dionisio-bot · 2025-10-15T15:44:27Z

Looks like this PR is not ready to merge, because of the following issues:

This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

changeset-bot · 2025-10-15T15:44:28Z

🦋 Changeset detected

Latest commit: ac92f9d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 40 packages

Name	Type
@rocket.chat/meteor	Minor
@rocket.chat/core-typings	Minor
@rocket.chat/rest-typings	Minor
@rocket.chat/uikit-playground	Patch
@rocket.chat/api-client	Patch
@rocket.chat/apps	Patch
@rocket.chat/core-services	Patch
@rocket.chat/cron	Patch
@rocket.chat/ddp-client	Patch
@rocket.chat/fuselage-ui-kit	Major
@rocket.chat/gazzodown	Major
@rocket.chat/http-router	Patch
@rocket.chat/livechat	Patch
@rocket.chat/model-typings	Patch
@rocket.chat/ui-avatar	Major
@rocket.chat/ui-client	Major
@rocket.chat/ui-contexts	Major
@rocket.chat/ui-voip	Major
@rocket.chat/web-ui-registration	Major
@rocket.chat/account-service	Patch
@rocket.chat/authorization-service	Patch
@rocket.chat/ddp-streamer	Patch
@rocket.chat/omnichannel-transcript	Patch
@rocket.chat/presence-service	Patch
@rocket.chat/queue-worker	Patch
@rocket.chat/abac	Patch
@rocket.chat/federation-matrix	Patch
@rocket.chat/license	Patch
@rocket.chat/media-calls	Patch
@rocket.chat/omnichannel-services	Patch
@rocket.chat/pdf-worker	Patch
@rocket.chat/presence	Patch
rocketchat-services	Patch
@rocket.chat/models	Patch
@rocket.chat/network-broker	Patch
@rocket.chat/omni-core-ee	Patch
@rocket.chat/mock-providers	Patch
@rocket.chat/ui-video-conf	Major
@rocket.chat/instance-status	Patch
@rocket.chat/omni-core	Patch

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

coderabbitai · 2025-10-15T15:45:09Z

Walkthrough

Adds server-side validation requiring each attachment.fields object to include both title and value, replacing silent coercion with an error; adds end-to-end tests for missing title/value and a changeset declaring a minor version bump.

Changes

Cohort / File(s)	Summary of changes
Attachment fields validation (server) `apps/meteor/app/lib/server/functions/sendMessage.ts`	Replaced prior coercion of `attachmentField.value` with explicit validation requiring both `attachmentField.title` and `attachmentField.value` to be truthy; throws "Invalid attachment field, title and value is required" when missing.
API tests for validation (E2E) `apps/meteor/tests/end-to-end/api/chat.ts`	Added three tests: 400 when `attachments.fields.title` is missing, 400 when `attachments.fields.value` is missing, and success when both `title` and `value` are present; placed alongside existing attachment validation tests.
Release metadata (changeset) `.changeset/hungry-fans-wait.md`	New changeset declaring a minor version bump for `@rocket.chat/meteor` and documenting that `attachment.fields` must include `title` and `value` for chat APIs.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client as API Client
  participant API as Chat API (Server)
  participant Validator as Attachment Validator
  participant Service as Message Service
  participant DB as Database

  Client->>API: POST /api/v1/chat.postMessage { attachments.fields[...] }
  API->>Validator: Validate each attachment.field (require title & value)
  alt Missing title or value
    Validator-->>API: Validation error
    API-->>Client: 400 { success: false, error: "Invalid attachment field, title and value is required" }
  else All fields valid
    Validator-->>API: Validation passed
    API->>Service: sendMessage(payload)
    Service->>DB: Insert message with attachments
    DB-->>Service: OK
    Service-->>API: Message stored
    API-->>Client: 200 { success: true, message }
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Focus review on:
- apps/meteor/app/lib/server/functions/sendMessage.ts — validation logic and exact error message.
- apps/meteor/tests/end-to-end/api/chat.ts — test placement and determinism.
- .changeset/hungry-fans-wait.md — package/version wording.

Suggested labels

stat: ready to merge, stat: QA assured

Suggested reviewers

rodrigok
MartinSchoeler

Poem

Thump-thump, I hop and check each field,
Titles and values both must be revealed.
Tests stand guard with ears up high,
Errors halt what shouldn't fly.
A carrot cheer — the message lands, sealed! 🥕🐇

Pre-merge checks and finishing touches

✅ 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 describes the main change: requiring both title and value properties on attachment fields.
Linked Issues check	✅ Passed	The PR implements the core requirement from CORE-1498: validates attachment.fields on APIs and requires both title and value properties.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue: validation logic, tests, and changeset entry for attachment field requirements.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/attachement-fields-api

📜 Recent review details

Configuration used: Organization 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.

📥 Commits

Reviewing files that changed from the base of the PR and between bd0a5ef and ac92f9d.

📒 Files selected for processing (2)

.changeset/hungry-fans-wait.md (1 hunks)
apps/meteor/app/lib/server/functions/sendMessage.ts (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

.changeset/hungry-fans-wait.md

🧰 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:

apps/meteor/app/lib/server/functions/sendMessage.ts

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

GitHub Check: 📦 Build Packages
GitHub Check: CodeQL-Build
GitHub Check: CodeQL-Build

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2025-10-15T16:08:01Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.89%. Comparing base (d072acd) to head (0617568).

Additional details and impacted files

@@                Coverage Diff                @@
##           release-8.0.0   #37233      +/-   ##
=================================================
- Coverage          70.24%   69.89%   -0.35%     
=================================================
  Files               2999     2983      -16     
  Lines             102301   102087     -214     
  Branches           18221    18175      -46     
=================================================
- Hits               71861    71357     -504     
- Misses             28576    28889     +313     
+ Partials            1864     1841      -23

Flag	Coverage Δ
e2e	`56.58% <ø> (-1.30%)`	⬇️
unit	`71.38% <ø> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

coderabbitai

Actionable comments posted: 1

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between d072acd and 0617568.

📒 Files selected for processing (3)

.changeset/hungry-fans-wait.md (1 hunks)
apps/meteor/app/lib/server/functions/sendMessage.ts (1 hunks)
apps/meteor/tests/end-to-end/api/chat.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

apps/meteor/tests/end-to-end/api/chat.ts (1)

apps/meteor/tests/data/api-data.ts (3)

request (10-10)

api (46-48)

credentials (39-42)

🔇 Additional comments (2)

.changeset/hungry-fans-wait.md (1)

1-5: LGTM!

The changeset correctly documents the validation changes and uses an appropriate version bump (minor) since this adds validation that could potentially break existing API calls with malformed attachment fields.

apps/meteor/tests/end-to-end/api/chat.ts (1)

557-585: Verify JSON schema for falsy values in attachment.fields

Couldn't locate the ChatPostMessageSchema; please confirm the validation allows 0, false, and empty string for fields.value and title and add or adjust tests accordingly.

apps/meteor/app/lib/server/functions/sendMessage.ts

yash-rajpal mentioned this pull request Oct 15, 2025

fix: Client crash on missing proper attachments.fields #37147

Merged

yash-rajpal marked this pull request as ready for review October 15, 2025 16:28

yash-rajpal requested a review from a team as a code owner October 15, 2025 16:28

dougfabris added this to the 8.0.0 milestone Oct 15, 2025

coderabbitai bot reviewed Oct 15, 2025

View reviewed changes

apps/meteor/app/lib/server/functions/sendMessage.ts Show resolved Hide resolved

ggazzo force-pushed the release-8.0.0 branch from e7e8538 to c78ad2e Compare October 31, 2025 17:40

ggazzo requested review from a team as code owners October 31, 2025 17:40

ggazzo force-pushed the release-8.0.0 branch 10 times, most recently from dd959e2 to 589f2a1 Compare November 6, 2025 18:06

ggazzo force-pushed the release-8.0.0 branch 5 times, most recently from 481c8af to 94daba9 Compare November 18, 2025 18:57

ggazzo force-pushed the release-8.0.0 branch from 94daba9 to 949b49c Compare December 2, 2025 21:26

ggazzo force-pushed the release-8.0.0 branch 17 times, most recently from e06b0cd to ee2a34f Compare December 20, 2025 14:53

ggazzo force-pushed the fix/attachement-fields-api branch from 27fe006 to bd0a5ef Compare December 20, 2025 15:09

ggazzo force-pushed the release-8.0.0 branch from 22cdc8e to 6199d29 Compare December 20, 2025 18:43

Base automatically changed from release-8.0.0 to develop December 20, 2025 18:44

yash-rajpal added 5 commits December 20, 2025 16:37

attachment field params should be required by api endpoint

f067eac

add tests

af53a18

add changeset

1f043d1

remove client fix

f4ba099

update changeset

ac92f9d

ggazzo force-pushed the fix/attachement-fields-api branch from a9a5e60 to ac92f9d Compare December 20, 2025 19:37

ggazzo approved these changes Dec 20, 2025

View reviewed changes

ggazzo merged commit 62708dc into develop Dec 20, 2025
41 of 42 checks passed

ggazzo deleted the fix/attachement-fields-api branch December 20, 2025 20:34

rocketchat-github-ci mentioned this pull request Dec 21, 2025

Release 8.0.0 #37915

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix!: `title` and `value` properties should be required on `attachments.fields` #37233

fix!: `title` and `value` properties should be required on `attachments.fields` #37233

Uh oh!

yash-rajpal commented Oct 15, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

dionisio-bot bot commented Oct 15, 2025 •

edited

Loading

Uh oh!

changeset-bot bot commented Oct 15, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Oct 15, 2025 •

edited

Loading

Uh oh!

codecov bot commented Oct 15, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix!: title and value properties should be required on attachments.fields #37233

fix!: title and value properties should be required on attachments.fields #37233

Uh oh!

Conversation

yash-rajpal commented Oct 15, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes (including videos or screenshots)

Issue(s)

Steps to test or reproduce

Further comments

Summary by CodeRabbit

Uh oh!

dionisio-bot bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

coderabbitai bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested labels

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

codecov bot commented Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fix!: `title` and `value` properties should be required on `attachments.fields` #37233

fix!: `title` and `value` properties should be required on `attachments.fields` #37233

yash-rajpal commented Oct 15, 2025 •

edited by coderabbitai bot

Loading

dionisio-bot bot commented Oct 15, 2025 •

edited

Loading

changeset-bot bot commented Oct 15, 2025 •

edited

Loading

coderabbitai bot commented Oct 15, 2025 •

edited

Loading

codecov bot commented Oct 15, 2025 •

edited

Loading