Skip to content

Fix updateUsers Cron Tasks Has Unprotected Promises - #387

Merged
pm-McFly merged 10 commits into
mainfrom
fix/cron/update-users-has-unsafe-calls
Jul 30, 2026
Merged

Fix updateUsers Cron Tasks Has Unprotected Promises#387
pm-McFly merged 10 commits into
mainfrom
fix/cron/update-users-has-unsafe-calls

Conversation

@pm-McFly

Copy link
Copy Markdown
Collaborator

What

This PR refactors the updateUsers cron task to improve performance, reliability, and stability. It introduces chunked data processing, migrates from monolithic promise chains to a queued task execution model to prevent connection pool exhaustion.

Why

This change addresses a critical stability issue where malformed user records (e.g., empty uid values) were causing unhandled promise rejections that crashed the entire Node.js process. Additionally, the existing implementation suffered from PostgreSQL integer overflow errors on the timestamp field due to millisecond-precision epochs, and performance issues where simultaneous queries during high-volume syncs exhausted database connection limits.

Closes #386

How

The implementation was refactored into modular, single-responsibility functions under 20 lines each.

  • Error Resilience: Re-introduced toMatrixIdSafe and wrapped processing logic in try/catch blocks to ensure malformed entries are logged and skipped rather than triggering process-wide failures.
  • Database Stability: Implemented a task queue system with executeTasks to process DB write operations in controlled chunks of 50, preventing connection pool saturation.
  • Data Integrity: Corrected the timestamp format by using Math.floor(epoch() / 1000) to conform to the 32-bit INTEGER type constraint in the PostgreSQL schema.
  • Architecture: Applied cleaner functional decomposition, ensuring strict adherence to SRP (Single Responsibility Principle) and reducing complexity.

pm-McFly added 3 commits July 30, 2026 17:59
Signed-off-by: Pierre 'McFly' Marty <pmarty@linagora.com>
Signed-off-by: Pierre 'McFly' Marty <pmarty@linagora.com>
@pm-McFly pm-McFly self-assigned this Jul 30, 2026
@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e8f301e3-8a22-4b06-ac72-34786345e577

📥 Commits

Reviewing files that changed from the base of the PR and between 2d123b3 and aad4b28.

📒 Files selected for processing (3)
  • packages/matrix-identity-server/src/cron/index.ts
  • packages/matrix-identity-server/src/cron/updateUsers.ts
  • packages/matrix-identity-server/src/types.ts
📜 Recent review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Test / Test Affected Packages
🧰 Additional context used
📓 Path-based instructions (6)
packages/matrix-identity-server/src/**/*.ts

📄 CodeRabbit inference engine (packages/matrix-identity-server/src/AGENTS.md)

All Matrix Identity Server endpoints must be mounted at /_matrix/identity/v2/ prefix

Files:

  • packages/matrix-identity-server/src/types.ts
  • packages/matrix-identity-server/src/cron/index.ts
  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{js,ts,jsx,tsx}: Code must follow the philosophy of simplicity over cleverness - junior developers should understand code in 30 seconds, avoid metaprogramming, deep generics, decorator magic, and prefer readable for loops over complex chains like .reduce().flatMap().filter()
Code must follow the philosophy of explicit over implicit - dependencies must be injected not imported globally, errors must be typed not caught-and-rethrown, data flows must be traceable through function signatures
Code comments must explain why, never what - the code itself explains what
Do not use // comments to disable code - delete dead code instead, as Git has history

Files:

  • packages/matrix-identity-server/src/types.ts
  • packages/matrix-identity-server/src/cron/index.ts
  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx}: Code must follow the philosophy of boundaries over conventions - use module facades enforced by lint rules instead of comments, prefer #private fields over naming conventions, prefer TypeScript types over JSDoc comments
Do not introduce new any types in TypeScript - warnings are existing tech debt, new ones are blockers

Enforce TypeScript strict mode across all packages in ToM-Server

**/*.{ts,tsx}: Use PascalCase for types, interfaces, classes, and enums
Return types must be explicit on all non-trivial functions
Every function must return a meaningful value (void is forbidden)
Use ActionResult type for functions that perform actions with no natural data return
Use Result<T, E> type for functions that produce data, making failure first-class
any type is forbidden without exception
as unknown as T double casting is forbidden without exception
Use unknown over any for data from external sources (HTTP, JSON.parse, databases)
Prefer type for unions and intersections, interface for object shapes
Avoid TypeScript enum; use string union types for internal values
Provide type guards and validation helpers for string unions that cross system boundaries
Use Result or ActionResult for expected, domain-meaningful failures (not exceptions)
Use Error.cause when wrapping or rethrowing errors to preserve the original error chain
Type caught errors correctly using instanceof checks (not casting unknown to Error)
Prefer functions and plain objects over classes; use classes only for genuine encapsulation with private mutable state and lifecycle
Do not use static-only classes to group related functions; use named exports instead
Classes must have a single responsibility
@ts-ignore and @ts-expect-error must have a written explanation

Files:

  • packages/matrix-identity-server/src/types.ts
  • packages/matrix-identity-server/src/cron/index.ts
  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use ES modules (type: module) throughout the ToM-Server project

Files:

  • packages/matrix-identity-server/src/types.ts
  • packages/matrix-identity-server/src/cron/index.ts
  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (CODING_STYLE.md)

**/*.{js,ts,tsx,jsx}: Use 2 spaces for indentation (not 4, not tabs)
Opening braces must go on the same line (never on a new line)
Use trailing commas in multi-line structures
Semicolons are required on all statements
Maximum line length is 120 characters (hard limit)
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE only for module-level primitive constants that never change
Boolean variables must use is/has/can prefix (e.g., isLoading, hasPermission, canRetry)
Do not abbreviate variable names beyond accepted list (i, j, e, err, ctx, req, res)
Functions must have a single responsibility (no 'and' in function names)
Use function declarations for named, standalone, exported units; use arrow functions for callbacks and inline helpers
Maximum 5 function arguments (use options object for more parameters)
Keep functions short (25-40 lines maximum, fit on one screen without scrolling)
Recursion must be tail-call only, or use an iterative loop instead
Maximum 2 levels of nesting (no level 3)
Use early returns to reduce nesting and establish preconditions
Avoid else after a return
Throw exceptions only for invariant violations and programming errors
Catch errors at system boundaries (HTTP handlers, job runners, event listeners), not deep in business logic
Never swallow errors silently (no empty catch blocks)
finally is for cleanup only, not for conditional logic
Import from specific files, not barrel exports
Organize imports in order: Node built-ins, external packages, internal absolute paths, internal relative paths (with blank lines between groups)
Comments must explain why, not what
Document function contracts in JSDoc, not implementation mechanics
TODO comments must have an owner name and ticket reference
Prefer async/await over .then() chains
Run independent async operations in parallel using Promise.all
Never fire-and-forget async operations without a .catch() handler
Use === instead of == (never use loose equality)
Do not use mutabl...

Files:

  • packages/matrix-identity-server/src/types.ts
  • packages/matrix-identity-server/src/cron/index.ts
  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/index.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CODING_STYLE.md)

Do not use barrel re-exports of entire directories

Files:

  • packages/matrix-identity-server/src/cron/index.ts
🧠 Learnings (1)
📚 Learning: 2026-03-31T07:26:27.898Z
Learnt from: pm-McFly
Repo: linagora/ToM-server PR: 355
File: packages/matrix-identity-server/src/db/index.ts:413-413
Timestamp: 2026-03-31T07:26:27.898Z
Learning: When reviewing TypeScript code in this repo, follow Biome’s `noDoubleEquals` rule: do not use loose equality (`== null` / `!= null`) as a shorthand. For nullish checks, use explicit strict comparisons instead (e.g., `value === null || value === undefined` or `value !== null && value !== undefined`).

Applied to files:

  • packages/matrix-identity-server/src/types.ts
  • packages/matrix-identity-server/src/cron/index.ts
  • packages/matrix-identity-server/src/cron/updateUsers.ts
🪛 GitHub Check: CodeQL
packages/matrix-identity-server/src/cron/index.ts

[failure] 108-108: Clear-text logging of sensitive information
This logs sensitive data returned by an access to DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to database_password as clear text.
This logs sensitive data returned by an access to LDAP_PASSWORD as clear text.
This logs sensitive data returned by an access to ldap_password as clear text.
This logs sensitive data returned by an access to MATRIX_DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to matrix_database_password as clear text.
This logs sensitive data returned by an access to USERDB_PASSWORD as clear text.
This logs sensitive data returned by an access to userdb_password as clear text.
This logs sensitive data returned by process environment as clear text.
This logs sensitive data returned by an access to sms_api_key as clear text.
This logs sensitive data returned by an access to SMS_API_KEY as clear text.
This logs sensitive data returned by an access to MATRIX_ADMIN_PASSWORD as clear text.
This logs sensitive data returned by an access to DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to database_password as clear text.
This logs sensitive data returned by an access to LDAP_PASSWORD as clear text.
This logs sensitive data returned by an access to ldap_password as clear text.
This logs sensitive data returned by an access to MATRIX_DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to matrix_database_password as clear text.
This logs sensitive data returned by an access to SMTP_PASSWORD as clear text.
This logs sensitive data returned by an access to smtp_password as clear text.
This logs sensitive data returned by an access to USERDB_PASSWORD as clear text.
This logs sensitive data returned by an access to userdb_password as clear text.
This logs sensitive data returned by an access to sms_api_key as clear text.
This logs sensitive data returned by an access to SMS_API_KEY as clear text.
This logs sensitive data returned by an access to MATRIX_ADMIN_PASSWORD as clear text.
This logs sensitive data returned by an access to DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to database_password as clear text.
This logs sensitive data returned by process environment as clear text.
This logs sensitive data returned by an access to LDAP_PASSWORD as clear text.
This logs sensitive data returned by an access to ldap_password as clear text.
This logs sensitive data returned by an access to MATRIX_DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to matrix_database_password as clear text.
This logs sensitive data returned by an access to sms_api_key as clear text.
This logs sensitive data returned by an access to SMS_API_KEY as clear text.
This logs sensitive data returned by an access to MATRIX_ADMIN_PASSWORD as clear text.
This logs sensitive data returned by an access to SMTP_PASSWORD as clear text.
This logs sensitive data returned by an access to smtp_password as clear text.

packages/matrix-identity-server/src/cron/updateUsers.ts

[failure] 164-164: Clear-text logging of sensitive information
This logs sensitive data returned by an access to DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to database_password as clear text.
This logs sensitive data returned by an access to LDAP_PASSWORD as clear text.
This logs sensitive data returned by an access to ldap_password as clear text.
This logs sensitive data returned by an access to MATRIX_DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to matrix_database_password as clear text.
This logs sensitive data returned by an access to USERDB_PASSWORD as clear text.
This logs sensitive data returned by an access to userdb_password as clear text.
This logs sensitive data returned by process environment as clear text.
This logs sensitive data returned by an access to sms_api_key as clear text.
This logs sensitive data returned by an access to SMS_API_KEY as clear text.
This logs sensitive data returned by an access to MATRIX_ADMIN_PASSWORD as clear text.
This logs sensitive data returned by an access to DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to database_password as clear text.
This logs sensitive data returned by an access to LDAP_PASSWORD as clear text.
This logs sensitive data returned by an access to ldap_password as clear text.
This logs sensitive data returned by an access to MATRIX_DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to matrix_database_password as clear text.
This logs sensitive data returned by an access to SMTP_PASSWORD as clear text.
This logs sensitive data returned by an access to smtp_password as clear text.
This logs sensitive data returned by an access to USERDB_PASSWORD as clear text.
This logs sensitive data returned by an access to userdb_password as clear text.
This logs sensitive data returned by an access to sms_api_key as clear text.
This logs sensitive data returned by an access to SMS_API_KEY as clear text.
This logs sensitive data returned by an access to MATRIX_ADMIN_PASSWORD as clear text.
This logs sensitive data returned by an access to DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to database_password as clear text.
This logs sensitive data returned by process environment as clear text.
This logs sensitive data returned by an access to LDAP_PASSWORD as clear text.
This logs sensitive data returned by an access to ldap_password as clear text.
This logs sensitive data returned by an access to MATRIX_DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to matrix_database_password as clear text.
This logs sensitive data returned by an access to sms_api_key as clear text.
This logs sensitive data returned by an access to SMS_API_KEY as clear text.
This logs sensitive data returned by an access to MATRIX_ADMIN_PASSWORD as clear text.
This logs sensitive data returned by an access to SMTP_PASSWORD as clear text.
This logs sensitive data returned by an access to smtp_password as clear text.

🔇 Additional comments (4)
packages/matrix-identity-server/src/cron/updateUsers.ts (2)

5-5: LGTM!


156-162: LGTM!

Also applies to: 182-184

packages/matrix-identity-server/src/types.ts (1)

71-71: LGTM!

packages/matrix-identity-server/src/cron/index.ts (1)

102-105: LGTM!


📝 Walkthrough

Fundamental flaw fixed

  • updateUsers could crash the cron run when encountering malformed user records (e.g., empty/bad uid)—before the old “promise rejection handler” patterns effectively covered the failure path—so the process could die mid-run.
  • It also wrote millisecond-precision epochs into userHistory.timestamp (an INTEGER), which can overflow in PostgreSQL. The cron now stores second precision via Math.floor(epoch() / 1000).

Core changes (systemic data flow + algorithm)

  • Refactored the cron from “build large arrays + positional matching + one big Promise.all” into a staged sync pipeline with explicit task queuing:

    • Derive Matrix local parts safely (extractLocalPart) and convert defensively (toMatrixIdSafe), so bad inputs don’t explode—malformed uid values are detected and warned about, then skipped.
    • When MatrixDB is configured, load Matrix user local parts into a Set (fetchMatrixUids) for O(1) membership checks.
      • If this MatrixDB UID load fails, the cron logs and throws to abort and avoid incorrect activation (“mass activation” scenario).
    • Load current “active” state into an activeMap (getHashes).
    • Preload existing userHistory state per chunk (populateChunkState) into a Set to drive idempotent upsert decisions.
    • For each user, evaluateUser enqueues only the required operations:
      • userHistory upserts with the fixed second-precision timestamp
      • hashes activation updates
      • conditional updateHash / field updates when needed
    • Generate deactivation tasks for previously-active users missing from the current run (getInactiveTasks), by set-diffing against the current Matrix UID set.
    • Execute queued DB writes via executeTasks in bounded chunks:
      • CHUNK_SIZE = 10
      • each chunk runs Promise.all(...), but each task has its own .catch(...) so one failure doesn’t cancel the whole batch
      • the cron returns { success: false, error: ... } if any task failed
  • Contract change: the cron now returns an ActionResult instead of Promise<void>:

    • packages/matrix-identity-server/src/types.ts adds ActionResult
    • packages/.../cron/index.ts updates logging to key off result.success / result.error

Legacy code removed / replaced

  • Removed the prior monolithic orchestration that depended on positional/array-index matching and a single “accumulate everything then Promise.all” execution model.
  • Replaced the old “errors hidden by aggregate promise handling” behavior with explicit task-level failure handling and chunked execution.

Technical debt (explicitly not addressed)

  • No retry/backoff for transient MatrixDB/DB failures; execution either logs+skips per-task failures during queued writes, or aborts early if MatrixDB UID loading fails to prevent incorrect activation.

Walkthrough

Changes

The updateUsers cron now uses a staged, chunked synchronization pipeline with safe Matrix ID handling, second-based timestamps, batched database tasks, explicit failure results, and status-aware cron logging.

User synchronization

Layer / File(s) Summary
Synchronization context and source state
packages/matrix-identity-server/src/cron/updateUsers.ts
Builds synchronization context, safely derives Matrix identifiers, loads Matrix UIDs and hash activation state, and computes second-based timestamps.
History evaluation tasks
packages/matrix-identity-server/src/cron/updateUsers.ts
Preloads chunk history state and evaluates users for history upserts, hash activation, and aggregated hash updates.
Inactive reconciliation and execution
packages/matrix-identity-server/src/cron/updateUsers.ts
Queues inactive-user updates, executes tasks in batches of 10, logs task failures, and returns an ActionResult.
Result reporting
packages/matrix-identity-server/src/types.ts, packages/matrix-identity-server/src/cron/index.ts
Adds the ActionResult union and logs resolved synchronization failures using the returned error.

Suggested labels: javascript

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is specific and matches the main change: hardening updateUsers against promise-related cron crashes.
Description check ✅ Passed It includes What, Why, and How plus the linked issue; the checklist is incomplete but the core description is there.
Linked Issues check ✅ Passed The refactor skips malformed users, avoids unhandled rejections, and fixes the Postgres timestamp overflow, which matches #386.
Out of Scope Changes check ✅ Passed The changes stay centered on updateUsers reliability, batching, and timestamp handling; no unrelated scope jumps are evident.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Fix failing CI checks

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.

@pm-McFly pm-McFly added bug Something does not behave as expected. contribution::easy Issues that are well explained and require little project knowledge. priority::urgent We want to do this as soon as possible. severity::major Some important parts of the project cannot be used. package::identity-server labels Jul 30, 2026
@nx-cloud

nx-cloud Bot commented Jul 30, 2026

Copy link
Copy Markdown

View your CI Pipeline Execution ↗ for commit aad4b28

Command Status Duration Result
nx affected -t test ✅ Succeeded 2m 46s View ↗
nx affected -t build ✅ Succeeded 17s View ↗

💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗


☁️ Nx Cloud last updated this comment at 2026-07-30 19:43:51 UTC

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 7


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 58a8c6fc-959e-4104-9ae7-f5a7d1a46fdf

📥 Commits

Reviewing files that changed from the base of the PR and between 79b983c and f9c75c2.

📒 Files selected for processing (1)
  • packages/matrix-identity-server/src/cron/updateUsers.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: Security / CodeQL
  • GitHub Check: Docs / Update Documentation
  • GitHub Check: Security / njsscan
  • GitHub Check: Build / Build Affected Packages
🧰 Additional context used
📓 Path-based instructions (5)
packages/matrix-identity-server/src/**/*.ts

📄 CodeRabbit inference engine (packages/matrix-identity-server/src/AGENTS.md)

All Matrix Identity Server endpoints must be mounted at /_matrix/identity/v2/ prefix

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{js,ts,jsx,tsx}: Code must follow the philosophy of simplicity over cleverness - junior developers should understand code in 30 seconds, avoid metaprogramming, deep generics, decorator magic, and prefer readable for loops over complex chains like .reduce().flatMap().filter()
Code must follow the philosophy of explicit over implicit - dependencies must be injected not imported globally, errors must be typed not caught-and-rethrown, data flows must be traceable through function signatures
Code comments must explain why, never what - the code itself explains what
Do not use // comments to disable code - delete dead code instead, as Git has history

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx}: Code must follow the philosophy of boundaries over conventions - use module facades enforced by lint rules instead of comments, prefer #private fields over naming conventions, prefer TypeScript types over JSDoc comments
Do not introduce new any types in TypeScript - warnings are existing tech debt, new ones are blockers

Enforce TypeScript strict mode across all packages in ToM-Server

**/*.{ts,tsx}: Use PascalCase for types, interfaces, classes, and enums
Return types must be explicit on all non-trivial functions
Every function must return a meaningful value (void is forbidden)
Use ActionResult type for functions that perform actions with no natural data return
Use Result<T, E> type for functions that produce data, making failure first-class
any type is forbidden without exception
as unknown as T double casting is forbidden without exception
Use unknown over any for data from external sources (HTTP, JSON.parse, databases)
Prefer type for unions and intersections, interface for object shapes
Avoid TypeScript enum; use string union types for internal values
Provide type guards and validation helpers for string unions that cross system boundaries
Use Result or ActionResult for expected, domain-meaningful failures (not exceptions)
Use Error.cause when wrapping or rethrowing errors to preserve the original error chain
Type caught errors correctly using instanceof checks (not casting unknown to Error)
Prefer functions and plain objects over classes; use classes only for genuine encapsulation with private mutable state and lifecycle
Do not use static-only classes to group related functions; use named exports instead
Classes must have a single responsibility
@ts-ignore and @ts-expect-error must have a written explanation

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use ES modules (type: module) throughout the ToM-Server project

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (CODING_STYLE.md)

**/*.{js,ts,tsx,jsx}: Use 2 spaces for indentation (not 4, not tabs)
Opening braces must go on the same line (never on a new line)
Use trailing commas in multi-line structures
Semicolons are required on all statements
Maximum line length is 120 characters (hard limit)
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE only for module-level primitive constants that never change
Boolean variables must use is/has/can prefix (e.g., isLoading, hasPermission, canRetry)
Do not abbreviate variable names beyond accepted list (i, j, e, err, ctx, req, res)
Functions must have a single responsibility (no 'and' in function names)
Use function declarations for named, standalone, exported units; use arrow functions for callbacks and inline helpers
Maximum 5 function arguments (use options object for more parameters)
Keep functions short (25-40 lines maximum, fit on one screen without scrolling)
Recursion must be tail-call only, or use an iterative loop instead
Maximum 2 levels of nesting (no level 3)
Use early returns to reduce nesting and establish preconditions
Avoid else after a return
Throw exceptions only for invariant violations and programming errors
Catch errors at system boundaries (HTTP handlers, job runners, event listeners), not deep in business logic
Never swallow errors silently (no empty catch blocks)
finally is for cleanup only, not for conditional logic
Import from specific files, not barrel exports
Organize imports in order: Node built-ins, external packages, internal absolute paths, internal relative paths (with blank lines between groups)
Comments must explain why, not what
Document function contracts in JSDoc, not implementation mechanics
TODO comments must have an owner name and ticket reference
Prefer async/await over .then() chains
Run independent async operations in parallel using Promise.all
Never fire-and-forget async operations without a .catch() handler
Use === instead of == (never use loose equality)
Do not use mutabl...

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
🧠 Learnings (1)
📚 Learning: 2026-03-31T07:26:27.898Z
Learnt from: pm-McFly
Repo: linagora/ToM-server PR: 355
File: packages/matrix-identity-server/src/db/index.ts:413-413
Timestamp: 2026-03-31T07:26:27.898Z
Learning: When reviewing TypeScript code in this repo, follow Biome’s `noDoubleEquals` rule: do not use loose equality (`== null` / `!= null`) as a shorthand. For nullish checks, use explicit strict comparisons instead (e.g., `value === null || value === undefined` or `value !== null && value !== undefined`).

Applied to files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts

Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts
Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts
Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts
Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts
Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts Outdated
Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts
Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts Outdated
pm-McFly added 5 commits July 30, 2026 19:53
Signed-off-by: Pierre 'McFly' Marty <pmarty@linagora.com>
Signed-off-by: Pierre 'McFly' Marty <pmarty@linagora.com>
Signed-off-by: Pierre 'McFly' Marty <pmarty@linagora.com>
Signed-off-by: Pierre 'McFly' Marty <pmarty@linagora.com>
Signed-off-by: Pierre 'McFly' Marty <pmarty@linagora.com>
Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts Fixed
@pm-McFly

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/matrix-identity-server/src/cron/updateUsers.ts (1)

114-132: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

Stop routing evaluation output through hidden mutation.

evaluateUser returns void while mutating state.tasks and state.toUpdate. Return a typed evaluation/result and merge it in updateUsers so the data flow is explicit.

As per coding guidelines, “data flows must be traceable through function signatures” and “Every function must return a meaningful value (void is forbidden)”.

Source: Coding guidelines

♻️ Duplicate comments (1)
packages/matrix-identity-server/src/cron/updateUsers.ts (1)

52-54: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Do not log raw database initialization errors.

Both catch blocks serialize err; config-backed database errors can include DSNs or password-derived fields. CodeQL already reports this flow at Line 169. Log a fixed event plus a safe error category, unless logger redaction is verified.

  • packages/matrix-identity-server/src/cron/updateUsers.ts#L52-L54: replace raw Matrix DB error logging with sanitized diagnostics.
  • packages/matrix-identity-server/src/cron/updateUsers.ts#L168-L170: apply the same sanitization to aggregate initialization failures.
#!/bin/bash
set -euo pipefail

rg -n -i -C2 '(redact|password|secret|token|serialize)' \
  packages/logger/src packages/matrix-identity-server/src

Source: Linters/SAST tools


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 65259e69-d917-44ba-8151-1084cb0c0bfb

📥 Commits

Reviewing files that changed from the base of the PR and between f9c75c2 and 2d123b3.

📒 Files selected for processing (1)
  • packages/matrix-identity-server/src/cron/updateUsers.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
packages/matrix-identity-server/src/**/*.ts

📄 CodeRabbit inference engine (packages/matrix-identity-server/src/AGENTS.md)

All Matrix Identity Server endpoints must be mounted at /_matrix/identity/v2/ prefix

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{js,ts,jsx,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{js,ts,jsx,tsx}: Code must follow the philosophy of simplicity over cleverness - junior developers should understand code in 30 seconds, avoid metaprogramming, deep generics, decorator magic, and prefer readable for loops over complex chains like .reduce().flatMap().filter()
Code must follow the philosophy of explicit over implicit - dependencies must be injected not imported globally, errors must be typed not caught-and-rethrown, data flows must be traceable through function signatures
Code comments must explain why, never what - the code itself explains what
Do not use // comments to disable code - delete dead code instead, as Git has history

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{ts,tsx}: Code must follow the philosophy of boundaries over conventions - use module facades enforced by lint rules instead of comments, prefer #private fields over naming conventions, prefer TypeScript types over JSDoc comments
Do not introduce new any types in TypeScript - warnings are existing tech debt, new ones are blockers

Enforce TypeScript strict mode across all packages in ToM-Server

**/*.{ts,tsx}: Use PascalCase for types, interfaces, classes, and enums
Return types must be explicit on all non-trivial functions
Every function must return a meaningful value (void is forbidden)
Use ActionResult type for functions that perform actions with no natural data return
Use Result<T, E> type for functions that produce data, making failure first-class
any type is forbidden without exception
as unknown as T double casting is forbidden without exception
Use unknown over any for data from external sources (HTTP, JSON.parse, databases)
Prefer type for unions and intersections, interface for object shapes
Avoid TypeScript enum; use string union types for internal values
Provide type guards and validation helpers for string unions that cross system boundaries
Use Result or ActionResult for expected, domain-meaningful failures (not exceptions)
Use Error.cause when wrapping or rethrowing errors to preserve the original error chain
Type caught errors correctly using instanceof checks (not casting unknown to Error)
Prefer functions and plain objects over classes; use classes only for genuine encapsulation with private mutable state and lifecycle
Do not use static-only classes to group related functions; use named exports instead
Classes must have a single responsibility
@ts-ignore and @ts-expect-error must have a written explanation

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use ES modules (type: module) throughout the ToM-Server project

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
**/*.{js,ts,tsx,jsx}

📄 CodeRabbit inference engine (CODING_STYLE.md)

**/*.{js,ts,tsx,jsx}: Use 2 spaces for indentation (not 4, not tabs)
Opening braces must go on the same line (never on a new line)
Use trailing commas in multi-line structures
Semicolons are required on all statements
Maximum line length is 120 characters (hard limit)
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE only for module-level primitive constants that never change
Boolean variables must use is/has/can prefix (e.g., isLoading, hasPermission, canRetry)
Do not abbreviate variable names beyond accepted list (i, j, e, err, ctx, req, res)
Functions must have a single responsibility (no 'and' in function names)
Use function declarations for named, standalone, exported units; use arrow functions for callbacks and inline helpers
Maximum 5 function arguments (use options object for more parameters)
Keep functions short (25-40 lines maximum, fit on one screen without scrolling)
Recursion must be tail-call only, or use an iterative loop instead
Maximum 2 levels of nesting (no level 3)
Use early returns to reduce nesting and establish preconditions
Avoid else after a return
Throw exceptions only for invariant violations and programming errors
Catch errors at system boundaries (HTTP handlers, job runners, event listeners), not deep in business logic
Never swallow errors silently (no empty catch blocks)
finally is for cleanup only, not for conditional logic
Import from specific files, not barrel exports
Organize imports in order: Node built-ins, external packages, internal absolute paths, internal relative paths (with blank lines between groups)
Comments must explain why, not what
Document function contracts in JSDoc, not implementation mechanics
TODO comments must have an owner name and ticket reference
Prefer async/await over .then() chains
Run independent async operations in parallel using Promise.all
Never fire-and-forget async operations without a .catch() handler
Use === instead of == (never use loose equality)
Do not use mutabl...

Files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
🧠 Learnings (1)
📚 Learning: 2026-03-31T07:26:27.898Z
Learnt from: pm-McFly
Repo: linagora/ToM-server PR: 355
File: packages/matrix-identity-server/src/db/index.ts:413-413
Timestamp: 2026-03-31T07:26:27.898Z
Learning: When reviewing TypeScript code in this repo, follow Biome’s `noDoubleEquals` rule: do not use loose equality (`== null` / `!= null`) as a shorthand. For nullish checks, use explicit strict comparisons instead (e.g., `value === null || value === undefined` or `value !== null && value !== undefined`).

Applied to files:

  • packages/matrix-identity-server/src/cron/updateUsers.ts
🪛 GitHub Check: CodeQL
packages/matrix-identity-server/src/cron/updateUsers.ts

[failure] 169-169: Clear-text logging of sensitive information
This logs sensitive data returned by an access to DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to database_password as clear text.
This logs sensitive data returned by an access to LDAP_PASSWORD as clear text.
This logs sensitive data returned by an access to ldap_password as clear text.
This logs sensitive data returned by an access to MATRIX_DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to matrix_database_password as clear text.
This logs sensitive data returned by an access to USERDB_PASSWORD as clear text.
This logs sensitive data returned by an access to userdb_password as clear text.
This logs sensitive data returned by process environment as clear text.
This logs sensitive data returned by an access to sms_api_key as clear text.
This logs sensitive data returned by an access to SMS_API_KEY as clear text.
This logs sensitive data returned by an access to MATRIX_ADMIN_PASSWORD as clear text.
This logs sensitive data returned by an access to DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to database_password as clear text.
This logs sensitive data returned by an access to LDAP_PASSWORD as clear text.
This logs sensitive data returned by an access to ldap_password as clear text.
This logs sensitive data returned by an access to MATRIX_DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to matrix_database_password as clear text.
This logs sensitive data returned by an access to SMTP_PASSWORD as clear text.
This logs sensitive data returned by an access to smtp_password as clear text.
This logs sensitive data returned by an access to USERDB_PASSWORD as clear text.
This logs sensitive data returned by an access to userdb_password as clear text.
This logs sensitive data returned by an access to sms_api_key as clear text.
This logs sensitive data returned by an access to SMS_API_KEY as clear text.
This logs sensitive data returned by an access to MATRIX_ADMIN_PASSWORD as clear text.
This logs sensitive data returned by an access to DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to database_password as clear text.
This logs sensitive data returned by process environment as clear text.
This logs sensitive data returned by an access to LDAP_PASSWORD as clear text.
This logs sensitive data returned by an access to ldap_password as clear text.
This logs sensitive data returned by an access to MATRIX_DATABASE_PASSWORD as clear text.
This logs sensitive data returned by an access to matrix_database_password as clear text.
This logs sensitive data returned by an access to sms_api_key as clear text.
This logs sensitive data returned by an access to SMS_API_KEY as clear text.
This logs sensitive data returned by an access to MATRIX_ADMIN_PASSWORD as clear text.
This logs sensitive data returned by an access to SMTP_PASSWORD as clear text.
This logs sensitive data returned by an access to smtp_password as clear text.

🔇 Additional comments (3)
packages/matrix-identity-server/src/cron/updateUsers.ts (3)

72-76: LGTM!


103-112: LGTM!

Also applies to: 136-139


163-167: LGTM!

Also applies to: 175-185

Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts
pm-McFly added 2 commits July 30, 2026 21:27
Signed-off-by: Pierre 'McFly' Marty <pmarty@linagora.com>
Signed-off-by: Pierre 'McFly' Marty <pmarty@linagora.com>
Comment thread packages/matrix-identity-server/src/cron/index.ts Dismissed
Comment thread packages/matrix-identity-server/src/cron/updateUsers.ts Dismissed
@pm-McFly

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@pm-McFly
pm-McFly merged commit bc2027c into main Jul 30, 2026
10 of 11 checks passed
@pm-McFly
pm-McFly deleted the fix/cron/update-users-has-unsafe-calls branch July 30, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something does not behave as expected. contribution::easy Issues that are well explained and require little project knowledge. package::identity-server priority::urgent We want to do this as soon as possible. severity::major Some important parts of the project cannot be used.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

updateUsers cron crashes the server on a malformed user entry

2 participants