Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix:
crypto.randomUUID()inRoomHistoryManagerbreaks message history on non-secure origins (plain HTTP, non-localhost) since 8.1 #41730New 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?
Uh oh!
There was an error while loading. Please reload this page.
fix:
crypto.randomUUID()inRoomHistoryManagerbreaks message history on non-secure origins (plain HTTP, non-localhost) since 8.1 #41730Changes from all commits
7228dc8bbf483de621c12faaafb074a8d45ff67890File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
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.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 6099
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 2678
Store this metadata outside
.changeset/, or keep the Changesets entry.ddp-migrate-batch5-totp-caller.mdis in.changeset/, but the file no longer has frontmatter; the parser will not treat it as a Changesets release and will ignore it. If this file is meant to record a release, restore it as a separate valid.changeset/*.mdwith package version frontmatter.🤖 Prompt for AI Agents
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 878
🌐 Web query:
Devpilot .changeset metadata format version app rules match replace YAML💡 Result:
Based on available documentation for Devpilot (both the CLI tool for local development and the deployment platform), there is no standard.changeset metadata format version or specific YAML-based match-and-replace rules system as you described. The term "changeset" does not appear as a defined configuration schema in the Devpilot ecosystem. Instead, the relevant configuration and versioning mechanisms are as follows: 1. Local Development Configuration (.devpilot.yaml): The CLI tool uses a.devpilot.yaml file to manage local services (e.g., cmd, port, health checks, and file patterns). This file does not implement a match-replace rule engine for metadata versioning [1][2][3]. 2. Deployment Hooks and Metadata: The deployment platform (docs.devpilot.io) uses "Deployment Hooks" which contain metadata (e.g., shell commands, paths to sync, and status) [4][5]. These hooks are configured through the platform interface rather than a raw.changeset YAML file [4]. 3. Versioning: Devpilot tracks "Script Versions" by saving the body of scripts in a history tab, where each version is a simple integer counter rather than a managed metadata version field in a changeset file [6]. If you are looking for automated rule-based transformations in YAML (e.g., for application migration or refactoring), you may be thinking of tools like the Migration Toolkit for Applications (MTA), which specifically utilizes YAML-based rules defined by metadata, conditions, and actions [7].
Citations:
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 10705
🌐 Web query:
Rocket.Chat changeset ddp-migrate-batch5-TOTP caller version app rules match replace💡 Result:
The term ddp-migrate-batch5-TOTP refers to a specific batch migration changeset in the Rocket.Chat codebase related to the ongoing transition from DDP (Meteor/Realtime) methods to REST API endpoints [1][2][3]. In the Rocket.Chat architectural evolution, batches of client-side DDP method callers (such as Meteor.call, useMethod, or SDK-based calls) are being migrated to use REST equivalents (e.g., /v1/... endpoints) [1][2][4]. This is part of a larger plan to clean up the legacy DDP method infrastructure ahead of the 9.0.0 release [1][2][3]. Regarding the specific context of TOTP and the migration process: 1. Migration Strategy: When a DDP method is migrated, the server-side DDP method registration is often left intact for compatibility with external SDKs or mobile clients, but it is marked with a deprecation logger (e.g., methodDeprecationLogger.method(...)) that points developers toward the new REST route [1][3][4]. 2. TOTP and 2FA Handling: TOTP (Time-based One-Time Password) and two-factor authentication (2FA) mechanisms have undergone significant updates, including API-level changes to support phishing-resistant MFA [5][6] and fixes for TOTP modal validation [7]. 3. Rules and Replacements: The "rules" for these migrations involve auditing the codebase to find DDP call-sites and replacing them with REST-equivalent hooks (such as useEndpoint) or direct API calls [2][3]. If a migration involves a 2FA-protected method, the replacement must ensure that the same security constraints (such as twoFactorRequired checks) are enforced on the new REST endpoint [4][8]. In short, "ddp-migrate-batch5-TOTP" represents an internal maintenance changeset where specific DDP methods—likely those involving TOTP or 2FA processes—were audited and their client-side callers refactored to use the REST API [1][2][3].
Citations:
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 26772
🌐 Web query:
Devpilot rules match replace regex replacement backreference \3 \1 YAML devpilot💡 Result:
Based on available documentation, there is no evidence of a built-in "match-replace" regex functionality with backreferences (\1, \3) within the Devpilot platform or its configuration files (such as.devpilot.yaml) [1][2][3][4][5]. Devpilot is a platform for managing server-side deployments, scripts, and automation [4][5]. While it uses YAML for service configuration and supports various scripting languages (bash, python, node, php), these rely on standard system-level regex engines (e.g., in bash/sed/awk or programming language libraries) rather than a custom Devpilot regex replacement engine [4][6]. Confusion regarding this query may arise from other tools or systems that utilize "match-replace" regex rules with backreferences, such as: 1. ProxySQL: Uses regex capture groups and backreferences (\1, \2, etc.) in its query rules [7]. 2. Devo: Uses custom backreference syntax (e.g., \m1, \d1) for regex in relay rules [8]. 3. General regex usage: Tools like Cloudflare Ruleset Engine also support regex replacement features [9]. If you are attempting to perform text replacement within a Devpilot-managed script, you should use standard Unix/Linux commands such as
sedorperlwithin your script body [4][6]. For example: sed -E 's/(pattern1)(pattern2)(pattern3)/\3 \1/' input.txt References to "devpilot" in your query may also refer to specific plugins or experimental tools [10][11][12][13], but these do not implement a global regex replace feature with backreferences as described [11][13].Citations:
Make the Devpilot rules YAML valid and follow the intended document shape.
# Hyperlinksis a comment. It does not start a second YAML document, soversionandappare duplicated. The rule strings use invalid YAML escapes. Use the required Devpilot document shape and quote or escape the regex/replacement strings.🤖 Prompt for AI Agents
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.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 1436
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 509
Use the link text/URL captures for the inline link.
For
See [Docs](https://example.test), the inline rule putsSeeinto the<a>text, then emits a second literal[Docs]link. Use the text and URL captures directly:Proposed inline-link fix
The reference rule also uses
\3ashref; that only works if Devpilot substitutes the reference key before applying this rule.📝 Committable suggestion
🤖 Prompt for AI Agents
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.
P0: This changeset file was replaced with an unrelated DevPilot config (Hyperlinks/
rules:regex replacement directives) instead of a valid changeset. Every other file in.changeset/uses the---\n'@rocket.chat/meteor': patch\n---\n<description>frontmatter format that@changesets/cliexpects; this new content will be treated as an invalid/empty changeset, so the release note for thecrypto.randomUUID()fix is dropped (and versioning/changelog generation can break). The content is also unrelated to the PR (it's a TOTP DDP-migration filename carrying a DevPilot config). Please restore proper changeset frontmatter describing this fix, or revert this file if it was not meant to change.Prompt for AI agents
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.
P2: The new tests provide no coverage of the actual fix this PR makes. They only exercise a speculative
getHistory()/addHistory()in-memory store and never test thecrypto.randomUUID()fallback for non-secure origins that the PR exists to repair, so they cannot detect a regression of the reported bug. Several cases ('when it is provided', 'in the correct order', 'with the correct messages') are identical duplicate assertions. Consider replacing them with a test that drives the real history-loading path on an insecure origin and asserts room IDs are still generated.Prompt for AI agents
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.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Align the tests with the implemented API.
RoomHistoryManager.tsdefault-exportsRoomHistoryManager, but line 1 uses a named import.addHistoryrequiresroomIdandhistory, andgetHistoryrequiresroomId.Use one room ID per test and pass it to each call. The current suite cannot compile against the supplied implementation or validate room-specific history behavior.
🤖 Prompt for AI Agents
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.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 50379
Generate a room ID before calling
next()for secure requests.req.secureskips ID generation, so the callback receives no room ID andreq.roomIdbecomesundefined. Generate or resolve the ID before callingnext(), and use the result instead of transporting it throughNextFunction.🤖 Prompt for AI Agents
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.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: RocketChat/Rocket.Chat
Length of output: 2270
Remove the trailing Markdown example/fence from the module.
At line 44, the
```typescriptand example code makeRoomHistoryManager.tsinvalid TypeScript. Move the example to Markdown documentation or remove it.🧰 Tools
🪛 Biome (2.5.6)
[error] 44-44: Expected a semicolon or an implicit semicolon after a statement, but found none
(parse)
🤖 Prompt for AI Agents
Source: Linters/SAST tools
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.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
Move Express application wiring into
apps/meteor/.This code creates and configures an Express application. Keep
RoomHistoryManager.tsfocused on room-history behavior, and register middleware in the Meteor application layer.As per coding guidelines, “The main Rocket.Chat Meteor application resides in
apps/meteor/; place its application code there rather than in other monorepo areas.”🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 48-48: Express application should use Helmet
Context: express()
Note: [CWE-693] Protection Mechanism Failure (Express app without Helmet security headers).
(missing-helmet-typescript)
🤖 Prompt for AI Agents
Source: Coding guidelines
Uh oh!
There was an error while loading. Please reload this page.