From 87a2a1ed9ffafae6faae7f1c060962b04d12b215 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Wed, 8 Apr 2026 20:57:15 -0300 Subject: [PATCH 01/12] feat: add redaction to apps logs --- .../server/apps/storage/AppRealLogStorage.ts | 28 +++++++++++++++++++ apps/meteor/package.json | 2 ++ yarn.lock | 16 +++++++++++ 3 files changed, 46 insertions(+) diff --git a/apps/meteor/ee/server/apps/storage/AppRealLogStorage.ts b/apps/meteor/ee/server/apps/storage/AppRealLogStorage.ts index 62f61699f2e47..65702ad2226c2 100644 --- a/apps/meteor/ee/server/apps/storage/AppRealLogStorage.ts +++ b/apps/meteor/ee/server/apps/storage/AppRealLogStorage.ts @@ -3,10 +3,34 @@ import type { IAppLogStorageFindOptions } from '@rocket.chat/apps-engine/server/ import { AppLogStorage } from '@rocket.chat/apps-engine/server/storage'; import { InstanceStatus } from '@rocket.chat/instance-status'; import type { AppLogs } from '@rocket.chat/models'; +import fastRedact, { type redactFnNoSerialize } from 'fast-redact'; export class AppRealLogStorage extends AppLogStorage { + private readonly redact: redactFnNoSerialize; + constructor(private db: typeof AppLogs) { super('mongodb'); + + this.redact = fastRedact({ + paths: [ + 'headers.cookie', + 'headers["x-auth-token"]', + 'headers.authorization', + 'headers.access_token', + 'query["x-auth-token"]', + 'query.authorization', + 'query.access_token', + 'query.customFields.*', + 'params["x-auth-token"]', + 'params.authorization', + 'params.access_token', + 'params.customFields.*', + 'user.customFields.*', + 'user.emails[*].address', + ], + serialize: false, + strict: false, + }); } async find( @@ -41,6 +65,10 @@ export class AppRealLogStorage extends AppLogStorage { async storeEntries(logEntry: ILoggerStorageEntry): Promise { logEntry.instanceId = InstanceStatus.id(); + logEntry.entries.forEach((entry) => { + entry.args.forEach((arg) => void (typeof arg === 'object' && arg !== null ? this.redact(arg) : arg)); + }); + const id = (await this.db.insertOne(logEntry)).insertedId; return this.db.findOneById(id); diff --git a/apps/meteor/package.json b/apps/meteor/package.json index ede2b121c42fa..96a7d47676eec 100644 --- a/apps/meteor/package.json +++ b/apps/meteor/package.json @@ -204,6 +204,7 @@ "expiry-map": "^2.0.0", "express": "^4.21.2", "express-rate-limit": "^5.5.1", + "fast-redact": "^3.5.0", "fastq": "^1.17.1", "fflate": "^0.8.2", "file-type": "^16.5.4", @@ -353,6 +354,7 @@ "@types/ejson": "^2.2.2", "@types/express": "^4.17.25", "@types/express-rate-limit": "^5.1.3", + "@types/fast-redact": "^3", "@types/google-libphonenumber": "^7.4.30", "@types/gravatar": "^1.8.6", "@types/he": "^1.2.3", diff --git a/yarn.lock b/yarn.lock index ba967915f200f..605b1a56dbb6e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9912,6 +9912,7 @@ __metadata: "@types/ejson": "npm:^2.2.2" "@types/express": "npm:^4.17.25" "@types/express-rate-limit": "npm:^5.1.3" + "@types/fast-redact": "npm:^3" "@types/google-libphonenumber": "npm:^7.4.30" "@types/gravatar": "npm:^1.8.6" "@types/he": "npm:^1.2.3" @@ -10018,6 +10019,7 @@ __metadata: express: "npm:^4.21.2" express-rate-limit: "npm:^5.5.1" fast-glob: "npm:^3.3.3" + fast-redact: "npm:^3.5.0" fastq: "npm:^1.17.1" fflate: "npm:^0.8.2" file-type: "npm:^16.5.4" @@ -14167,6 +14169,13 @@ __metadata: languageName: node linkType: hard +"@types/fast-redact@npm:^3": + version: 3.0.4 + resolution: "@types/fast-redact@npm:3.0.4" + checksum: 10/d130c7c2d322e761c65ec8ecd821bf56e132672cd9de1703a53a65e051d2f43af6062040d3f0afdd48b364e1e4b29b858a748517ba9a6a7f1dab12ce7000103a + languageName: node + linkType: hard + "@types/fibers@npm:^3.1.4": version: 3.1.4 resolution: "@types/fibers@npm:3.1.4" @@ -22305,6 +22314,13 @@ __metadata: languageName: node linkType: hard +"fast-redact@npm:^3.5.0": + version: 3.5.0 + resolution: "fast-redact@npm:3.5.0" + checksum: 10/24b27e2023bd5a62f908d97a753b1adb8d89206b260f97727728e00b693197dea2fc2aa3711147a385d0ec6e713569fd533df37a4ef947e08cb65af3019c7ad5 + languageName: node + linkType: hard + "fast-safe-stringify@npm:^2.1.1": version: 2.1.1 resolution: "fast-safe-stringify@npm:2.1.1" From 33f9de1f8a387bfd31b76901139c588de4227797 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 9 Apr 2026 15:34:37 -0300 Subject: [PATCH 02/12] feat: add new lib file for the app redactor --- apps/meteor/ee/server/apps/lib/redactor.ts | 36 ++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 apps/meteor/ee/server/apps/lib/redactor.ts diff --git a/apps/meteor/ee/server/apps/lib/redactor.ts b/apps/meteor/ee/server/apps/lib/redactor.ts new file mode 100644 index 0000000000000..a64e883278859 --- /dev/null +++ b/apps/meteor/ee/server/apps/lib/redactor.ts @@ -0,0 +1,36 @@ +import fastRedact from 'fast-redact'; + +export const redactionFields = { + 'cookie': 'cookie', + 'x-auth-token': '["x-auth-token"]', + 'authorization': 'authorization', + 'access_token': 'access_token', + 'customFields': 'customFields.*', + 'emails': 'emails[*].address', + 'email': 'email', + 'password': 'password', + 'pass': 'pass', +}; + +const redactor = fastRedact({ + paths: Object.values(redactionFields), + serialize: false, + strict: false, +}); + +export function redact(value: unknown): void { + if (!value || typeof value !== 'object') return; + + if (Array.isArray(value)) { + return value.forEach(redact); + } + + redactor(value); + + Object.entries(value).forEach(([key, val]) => { + // Don't recurse into properties that have already been redacted + if (!(key in redactionFields)) { + redact(val); + } + }); +} From 0a96d974409a87a03e5437cf6cf8a22f0e7bd15a Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Thu, 9 Apr 2026 16:38:38 -0300 Subject: [PATCH 03/12] refactor: use redactor function from lib --- .../server/apps/storage/AppRealLogStorage.ts | 28 ++----------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/apps/meteor/ee/server/apps/storage/AppRealLogStorage.ts b/apps/meteor/ee/server/apps/storage/AppRealLogStorage.ts index 65702ad2226c2..e86bafe7afe88 100644 --- a/apps/meteor/ee/server/apps/storage/AppRealLogStorage.ts +++ b/apps/meteor/ee/server/apps/storage/AppRealLogStorage.ts @@ -3,34 +3,12 @@ import type { IAppLogStorageFindOptions } from '@rocket.chat/apps-engine/server/ import { AppLogStorage } from '@rocket.chat/apps-engine/server/storage'; import { InstanceStatus } from '@rocket.chat/instance-status'; import type { AppLogs } from '@rocket.chat/models'; -import fastRedact, { type redactFnNoSerialize } from 'fast-redact'; -export class AppRealLogStorage extends AppLogStorage { - private readonly redact: redactFnNoSerialize; +import { redact } from '../lib/redactor'; +export class AppRealLogStorage extends AppLogStorage { constructor(private db: typeof AppLogs) { super('mongodb'); - - this.redact = fastRedact({ - paths: [ - 'headers.cookie', - 'headers["x-auth-token"]', - 'headers.authorization', - 'headers.access_token', - 'query["x-auth-token"]', - 'query.authorization', - 'query.access_token', - 'query.customFields.*', - 'params["x-auth-token"]', - 'params.authorization', - 'params.access_token', - 'params.customFields.*', - 'user.customFields.*', - 'user.emails[*].address', - ], - serialize: false, - strict: false, - }); } async find( @@ -66,7 +44,7 @@ export class AppRealLogStorage extends AppLogStorage { logEntry.instanceId = InstanceStatus.id(); logEntry.entries.forEach((entry) => { - entry.args.forEach((arg) => void (typeof arg === 'object' && arg !== null ? this.redact(arg) : arg)); + entry.args.forEach(redact); }); const id = (await this.db.insertOne(logEntry)).insertedId; From 0140681d922c786e83a0ba8b4596edb24a43e5bd Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 10 Apr 2026 15:22:42 -0300 Subject: [PATCH 04/12] refactor: change redaction to explicit paths instead of recursive refactor: improve paths on redactor --- apps/meteor/ee/server/apps/lib/redactor.ts | 66 ++++++++++++---------- 1 file changed, 36 insertions(+), 30 deletions(-) diff --git a/apps/meteor/ee/server/apps/lib/redactor.ts b/apps/meteor/ee/server/apps/lib/redactor.ts index a64e883278859..c1249ac7f4203 100644 --- a/apps/meteor/ee/server/apps/lib/redactor.ts +++ b/apps/meteor/ee/server/apps/lib/redactor.ts @@ -1,36 +1,42 @@ import fastRedact from 'fast-redact'; -export const redactionFields = { - 'cookie': 'cookie', - 'x-auth-token': '["x-auth-token"]', - 'authorization': 'authorization', - 'access_token': 'access_token', - 'customFields': 'customFields.*', - 'emails': 'emails[*].address', - 'email': 'email', - 'password': 'password', - 'pass': 'pass', -}; +const requestFields = [ + 'headers.Cookie', + 'headers.cookie', + 'headers["x-auth-token"]', + 'headers["X-Auth-Token"]', + 'headers.auth', + 'headers.Auth', + 'headers.authorization', + 'headers.Authorization', + 'headers.access_token', + 'content.password', + 'content.pass', + 'data.password', + 'data.pass', +]; -const redactor = fastRedact({ - paths: Object.values(redactionFields), - serialize: false, - strict: false, -}); - -export function redact(value: unknown): void { - if (!value || typeof value !== 'object') return; +const entityFields = ['password', 'pass', 'customFields.*', '_unmappedProperties_']; - if (Array.isArray(value)) { - return value.forEach(redact); - } +const roomFields = ['customFields.*', '_unmappedProperties_', ...entityFields.map((field) => `creator.${field}`)]; - redactor(value); +export const redactionFieldPaths = [ + // Incoming requests to the Apps API endpoints + ...requestFields, + ...entityFields.map((field) => `user.${field}`), + 'query.access_token', + 'query.query', // The deprecated `query` search param + // Outgoing requests from the Apps to the outter webs + ...requestFields.map((field) => `request.${field}`), + `request.query`, // `query` here is a string, so we have to redact it all + // Slashcommands + ...roomFields.map((field) => `params[0].room.${field}`), + ...entityFields.map((field) => `params[0].sender.${field}`), +]; - Object.entries(value).forEach(([key, val]) => { - // Don't recurse into properties that have already been redacted - if (!(key in redactionFields)) { - redact(val); - } - }); -} +export const redact = fastRedact({ + paths: redactionFieldPaths, + censor: '[Redacted]', + serialize: false, + strict: false, +}); From 68b66b4fdee54109140e3a3b799a91c4cbefc9c3 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 10 Apr 2026 15:23:34 -0300 Subject: [PATCH 05/12] refactor: change apps logger to use the redaction paths --- apps/meteor/ee/server/apps/orchestrator.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/meteor/ee/server/apps/orchestrator.js b/apps/meteor/ee/server/apps/orchestrator.js index 7188695ac3f43..7209aeb0556e8 100644 --- a/apps/meteor/ee/server/apps/orchestrator.js +++ b/apps/meteor/ee/server/apps/orchestrator.js @@ -5,11 +5,11 @@ import * as path from 'path'; import { registerOrchestrator } from '@rocket.chat/apps'; import { EssentialAppDisabledException } from '@rocket.chat/apps-engine/definition/exceptions'; import { AppManager } from '@rocket.chat/apps-engine/server/AppManager'; -import { Logger } from '@rocket.chat/logger'; import { AppLogs, Apps as AppsModel, AppsPersistence, Statistics } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; import { AppServerNotifier, AppsRestApi, AppUIKitInteractionApi } from './communication'; +import { redactionFieldPaths } from './lib/redactor'; import { MarketplaceAPIClient } from './marketplace/MarketplaceAPIClient'; import { isTesting } from './marketplace/isTesting'; import { AppRealLogStorage, AppRealStorage, ConfigurableAppSourceStorage } from './storage'; @@ -28,6 +28,7 @@ import { } from '../../../app/apps/server/converters'; import { AppThreadsConverter } from '../../../app/apps/server/converters/threads'; import { settings } from '../../../app/settings/server'; +import { SystemLogger } from '../../../server/lib/logger/system'; import { canEnableApp } from '../../app/license/server/canEnableApp'; const DISABLED_PRIVATE_APP_INSTALLATION = ['yes', 'true'].includes(String(process.env.DISABLE_PRIVATE_APP_INSTALLATION).toLowerCase()); @@ -44,7 +45,7 @@ export class AppServerOrchestrator { return; } - this._rocketchatLogger = new Logger('Rocket.Chat Apps'); + this._rocketchatLogger = SystemLogger.logger.child({ name: 'Rocket.Chat Apps' }, { redact: redactionFieldPaths }); this._model = AppsModel; this._logModel = AppLogs; From 1abcf9e35d060f902fac46a009ad1bd06eed57e4 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 10 Apr 2026 17:57:01 -0300 Subject: [PATCH 06/12] refactor: adapt debug logs in persistence bridge --- .../app/apps/server/bridges/persistence.ts | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/apps/meteor/app/apps/server/bridges/persistence.ts b/apps/meteor/app/apps/server/bridges/persistence.ts index 857f6a561ed6f..e1dedd270ac6e 100644 --- a/apps/meteor/app/apps/server/bridges/persistence.ts +++ b/apps/meteor/app/apps/server/bridges/persistence.ts @@ -15,7 +15,7 @@ export class AppPersistenceBridge extends PersistenceBridge { } protected async create(data: object, appId: string): Promise { - this.orch.debugLog(`The App ${appId} is storing a new object in their persistence.`, data); + this.orch.debugLog(`The App ${appId} is storing a new object in their persistence.`); if (typeof data !== 'object') { throw new Error('Attempted to store an invalid data type, it must be an object.'); @@ -28,11 +28,10 @@ export class AppPersistenceBridge extends PersistenceBridge { } protected async createWithAssociations(data: object, associations: Array, appId: string): Promise { - this.orch.debugLog( - `The App ${appId} is storing a new object in their persistence that is associated with some models.`, - data, + this.orch.debugLog({ + msg: `The App ${appId} is storing a new object in their persistence that is associated with some models.`, associations, - ); + }); if (typeof data !== 'object') { throw new Error('Attempted to store an invalid data type, it must be an object.'); @@ -53,7 +52,7 @@ export class AppPersistenceBridge extends PersistenceBridge { } protected async readByAssociations(associations: Array, appId: string): Promise> { - this.orch.debugLog(`The App ${appId} is searching for records that are associated with the following:`, associations); + this.orch.debugLog({ msg: `The App ${appId} is searching for records that are associated with the following:`, associations }); const records = await this.orch .getPersistenceModel() @@ -84,7 +83,7 @@ export class AppPersistenceBridge extends PersistenceBridge { associations: Array, appId: string, ): Promise | undefined> { - this.orch.debugLog(`The App ${appId} is removing records with the following associations:`, associations); + this.orch.debugLog({ msg: `The App ${appId} is removing records with the following associations:`, associations }); const query = { appId, @@ -105,7 +104,7 @@ export class AppPersistenceBridge extends PersistenceBridge { } protected async update(id: string, data: object, _upsert: boolean, appId: string): Promise { - this.orch.debugLog(`The App ${appId} is updating the record "${id}" to:`, data); + this.orch.debugLog(`The App ${appId} is updating the record "${id}"`); if (typeof data !== 'object') { throw new Error('Attempted to store an invalid data type, it must be an object.'); @@ -120,7 +119,7 @@ export class AppPersistenceBridge extends PersistenceBridge { upsert = true, appId: string, ): Promise { - this.orch.debugLog(`The App ${appId} is updating the record with association to data as follows:`, associations, data); + this.orch.debugLog({ msg: `The App ${appId} is updating the record with association to data as follows:`, associations }); if (typeof data !== 'object') { throw new Error('Attempted to store an invalid data type, it must be an object.'); From c7e0cf67dd3b4e96fced534dd608f4db4611e930 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 10 Apr 2026 17:55:43 -0300 Subject: [PATCH 07/12] feat: add censorUrl to tools package --- .../app/api/server/middlewares/logger.ts | 3 +- apps/meteor/app/apps/server/bridges/http.ts | 3 +- packages/server-fetch/src/index.ts | 9 ++-- packages/tools/src/censorUrl.ts | 45 +++++++++++++++++++ packages/tools/src/index.ts | 1 + 5 files changed, 55 insertions(+), 6 deletions(-) create mode 100644 packages/tools/src/censorUrl.ts diff --git a/apps/meteor/app/api/server/middlewares/logger.ts b/apps/meteor/app/api/server/middlewares/logger.ts index 6c56de6cfceb0..17759687390df 100644 --- a/apps/meteor/app/api/server/middlewares/logger.ts +++ b/apps/meteor/app/api/server/middlewares/logger.ts @@ -1,4 +1,5 @@ import type { Logger } from '@rocket.chat/logger'; +import { censorUrl } from '@rocket.chat/tools'; import type { MiddlewareHandler } from 'hono'; import { getRestPayload } from '../../../../server/lib/logger/logPayloads'; @@ -11,7 +12,7 @@ export const loggerMiddleware = const log = logger.logger.child( { method: c.req.method, - url: c.req.url, + url: censorUrl(c.req.url), userId: c.req.header('x-user-id'), userAgent: c.req.header('user-agent'), length: c.req.header('content-length'), diff --git a/apps/meteor/app/apps/server/bridges/http.ts b/apps/meteor/app/apps/server/bridges/http.ts index aab0d56d301f0..9352acb1030e4 100644 --- a/apps/meteor/app/apps/server/bridges/http.ts +++ b/apps/meteor/app/apps/server/bridges/http.ts @@ -3,6 +3,7 @@ import type { IHttpResponse } from '@rocket.chat/apps-engine/definition/accessor import type { IHttpBridgeRequestInfo } from '@rocket.chat/apps-engine/server/bridges'; import { HttpBridge } from '@rocket.chat/apps-engine/server/bridges/HttpBridge'; import { serverFetch as fetch, type ExtendedFetchOptions } from '@rocket.chat/server-fetch'; +import { censorUrl } from '@rocket.chat/tools'; import { settings } from '../../../settings/server'; @@ -72,7 +73,7 @@ export class AppHttpBridge extends HttpBridge { // end comptability with old HTTP.call API - this.orch.debugLog(`The App ${info.appId} is requesting from the outter webs:`, info); + this.orch.debugLog({ msg: `The App ${info.appId} is requesting from the outter webs:`, ...info, url: censorUrl(info.url) }); const shouldIgnoreSsrf = request.ssrfValidation !== true; const fetchOptions: ExtendedFetchOptions = { diff --git a/packages/server-fetch/src/index.ts b/packages/server-fetch/src/index.ts index d147761ef1d34..2355d2a1b4754 100644 --- a/packages/server-fetch/src/index.ts +++ b/packages/server-fetch/src/index.ts @@ -2,6 +2,7 @@ import http from 'http'; import https from 'https'; import { Logger } from '@rocket.chat/logger'; +import { censorUrl } from '@rocket.chat/tools'; import { AbortController } from 'abort-controller'; import { HttpProxyAgent } from 'http-proxy-agent'; import { HttpsProxyAgent } from 'https-proxy-agent'; @@ -21,7 +22,7 @@ function getFetchAgent( allowSelfSignedCerts?: boolean, originalHostname?: string, ): http.Agent | https.Agent | null | HttpsProxyAgent | HttpProxyAgent { - const isHttps = /^https/.test(url); + const isHttps = url.startsWith('https'); const proxy = getProxyForUrl(url); if (proxy) { @@ -67,7 +68,7 @@ async function getFetchAgentWithValidation( if (!ignoreSsrfValidation) { const ssrfResult = await checkForSsrfWithIp(url, allowList); if (!ssrfResult.allowed) { - logger.error({ msg: 'SSRF validation failed for URL', url }); + logger.error({ msg: 'SSRF validation failed for URL', url: censorUrl(url) }); throw new Error('error-ssrf-validation-failed'); } @@ -82,7 +83,7 @@ async function getFetchAgentWithValidation( } } } else { - logger.debug({ msg: 'Request not using SSRF validation', url: pinnedUrl }); + logger.debug({ msg: 'Request not using SSRF validation', url: censorUrl(pinnedUrl) }); } return { agent: getFetchAgent(pinnedUrl, allowSelfSignedCerts, originalHostname), pinnedUrl, originalHostname, resolvedIp }; @@ -108,7 +109,7 @@ function followRedirect(response: fetch.Response, redirectCount = 0) { throw new Error('error-too-many-redirects'); } - logger.debug({ msg: 'Following redirect', redirectCount, location, status: response.status }); + logger.debug({ msg: 'Following redirect', redirectCount, location: censorUrl(location), status: response.status }); return location; } diff --git a/packages/tools/src/censorUrl.ts b/packages/tools/src/censorUrl.ts new file mode 100644 index 0000000000000..0059e359f7732 --- /dev/null +++ b/packages/tools/src/censorUrl.ts @@ -0,0 +1,45 @@ +/** + * Redacts sensitive information from a URL string. + * + * This function parses a URL and replaces potentially sensitive data with placeholder text. + * It redacts the following URL components: + * - Username in the authentication section + * - Password in the authentication section + * - Query parameters named: 'query', 'access_token' + * + * Note: We use `*Redacted*` instead of `[Redacted]` for legibility, as `[` and `]` would be encoded by toString() + * + * @param url - The URL string to be censored + * @returns The URL string with sensitive information redacted, or the original URL if parsing fails + * + * @example + * ```ts + * censorUrl('https://user:password@example.com/path?query=secret&access_token=token'); + * // Returns: 'https://*Redacted*:*Redacted*@example.com/path?query=*Redacted*&access_token=*Redacted*' + * ``` + */ +export function censorUrl(url: string | URL): string { + try { + const parsedUrl = new URL(url); + + if (parsedUrl.username) { + parsedUrl.username = '*Redacted*'; + } + + if (parsedUrl.password) { + parsedUrl.password = '*Redacted*'; + } + + if (parsedUrl.searchParams.has('query')) { + parsedUrl.searchParams.set('query', '*Redacted*'); + } + + if (parsedUrl.searchParams.has('access_token')) { + parsedUrl.searchParams.set('access_token', '*Redacted*'); + } + + return parsedUrl.toString(); + } catch { + return url.toString(); + } +} diff --git a/packages/tools/src/index.ts b/packages/tools/src/index.ts index ca7406644c65f..c8df164de8508 100644 --- a/packages/tools/src/index.ts +++ b/packages/tools/src/index.ts @@ -1,3 +1,4 @@ +export * from './censorUrl'; export * from './convertSubObjectsIntoPaths'; export * from './convertPathsIntoSubObjects'; export * from './getObjectKeys'; From a0e0ec59ca8acdd40c48cc0ac8ca8e071a0d607c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 23:03:20 +0000 Subject: [PATCH 08/12] test(tools): add comprehensive unit tests for censorUrl Agent-Logs-Url: https://github.com/RocketChat/Rocket.Chat/sessions/39bac93f-3599-46eb-802a-702b5a9102d5 Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com> --- packages/tools/src/censorUrl.spec.ts | 39 ++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 packages/tools/src/censorUrl.spec.ts diff --git a/packages/tools/src/censorUrl.spec.ts b/packages/tools/src/censorUrl.spec.ts new file mode 100644 index 0000000000000..0a59cb3eaa361 --- /dev/null +++ b/packages/tools/src/censorUrl.spec.ts @@ -0,0 +1,39 @@ +import { censorUrl } from './censorUrl'; + +describe('censorUrl', () => { + it('returns the original value when URL parsing fails', () => { + const input = 'not-a-url'; + + expect(censorUrl(input)).toBe(input); + }); + + it('does not change URLs without sensitive parts', () => { + expect(censorUrl('https://example.com/path?foo=bar')).toBe('https://example.com/path?foo=bar'); + }); + + it('redacts username and password from auth section', () => { + expect(censorUrl('https://user:password@example.com/path')).toBe('https://*Redacted*:*Redacted*@example.com/path'); + }); + + it('redacts only username when password is not present', () => { + expect(censorUrl('https://user@example.com/path')).toBe('https://*Redacted*@example.com/path'); + }); + + it('redacts query and access_token search params', () => { + expect(censorUrl('https://example.com/path?query=secret&access_token=token&foo=bar')).toBe( + 'https://example.com/path?query=*Redacted*&access_token=*Redacted*&foo=bar', + ); + }); + + it('redacts access_token even when query is absent', () => { + expect(censorUrl('https://example.com/path?access_token=token&foo=bar')).toBe( + 'https://example.com/path?access_token=*Redacted*&foo=bar', + ); + }); + + it('accepts URL objects as input', () => { + expect(censorUrl(new URL('https://user:password@example.com/path?query=secret'))).toBe( + 'https://*Redacted*:*Redacted*@example.com/path?query=*Redacted*', + ); + }); +}); From 5e091946937de8e65aa6970e9d18902502d1d903 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 23:54:14 +0000 Subject: [PATCH 09/12] test(tools): cover relative URL and URL object immutability Agent-Logs-Url: https://github.com/RocketChat/Rocket.Chat/sessions/225a35e7-c779-467b-9093-192a9bddc17b Co-authored-by: d-gubert <1810309+d-gubert@users.noreply.github.com> --- packages/tools/src/censorUrl.spec.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/tools/src/censorUrl.spec.ts b/packages/tools/src/censorUrl.spec.ts index 0a59cb3eaa361..0fe51046a58a9 100644 --- a/packages/tools/src/censorUrl.spec.ts +++ b/packages/tools/src/censorUrl.spec.ts @@ -7,6 +7,12 @@ describe('censorUrl', () => { expect(censorUrl(input)).toBe(input); }); + it('returns relative URLs unchanged when no base is provided', () => { + const input = '/path/to/resource?query=secret&access_token=token'; + + expect(censorUrl(input)).toBe(input); + }); + it('does not change URLs without sensitive parts', () => { expect(censorUrl('https://example.com/path?foo=bar')).toBe('https://example.com/path?foo=bar'); }); @@ -36,4 +42,12 @@ describe('censorUrl', () => { 'https://*Redacted*:*Redacted*@example.com/path?query=*Redacted*', ); }); + + it('does not modify the original URL object', () => { + const input = new URL('https://user:password@example.com/path?query=secret&access_token=token'); + const originalValue = input.toString(); + + expect(censorUrl(input)).toBe('https://*Redacted*:*Redacted*@example.com/path?query=*Redacted*&access_token=*Redacted*'); + expect(input.toString()).toBe(originalValue); + }); }); From 40a8c30141913aff9640d61cfadad9e792c7e9b8 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Sun, 12 Apr 2026 21:06:55 -0300 Subject: [PATCH 10/12] add changeset --- .changeset/grumpy-ligers-drum.md | 5 +++++ .changeset/smart-chicken-repair.md | 5 +++++ .changeset/tender-spies-give.md | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 .changeset/grumpy-ligers-drum.md create mode 100644 .changeset/smart-chicken-repair.md create mode 100644 .changeset/tender-spies-give.md diff --git a/.changeset/grumpy-ligers-drum.md b/.changeset/grumpy-ligers-drum.md new file mode 100644 index 0000000000000..43d7caa24e949 --- /dev/null +++ b/.changeset/grumpy-ligers-drum.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/server-fetch': minor +--- + +Introduces redaction of potentially sensitive data when logging request URLs diff --git a/.changeset/smart-chicken-repair.md b/.changeset/smart-chicken-repair.md new file mode 100644 index 0000000000000..9c9b522563e13 --- /dev/null +++ b/.changeset/smart-chicken-repair.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/meteor': minor +--- + +Introduces redaction of potentially sensitive data in logs related to apps-engine diff --git a/.changeset/tender-spies-give.md b/.changeset/tender-spies-give.md new file mode 100644 index 0000000000000..e7f729f448cab --- /dev/null +++ b/.changeset/tender-spies-give.md @@ -0,0 +1,5 @@ +--- +'@rocket.chat/tools': minor +--- + +Adds new function for censoring URL components in logs From 0b48b18fac6123d537a595eafd34d4605f7f0892 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 17 Apr 2026 17:36:51 -0300 Subject: [PATCH 11/12] refactor: group data in debug log call Co-authored-by: Diego Sampaio --- apps/meteor/app/apps/server/bridges/http.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/meteor/app/apps/server/bridges/http.ts b/apps/meteor/app/apps/server/bridges/http.ts index 9352acb1030e4..cd297009c4afc 100644 --- a/apps/meteor/app/apps/server/bridges/http.ts +++ b/apps/meteor/app/apps/server/bridges/http.ts @@ -73,7 +73,7 @@ export class AppHttpBridge extends HttpBridge { // end comptability with old HTTP.call API - this.orch.debugLog({ msg: `The App ${info.appId} is requesting from the outter webs:`, ...info, url: censorUrl(info.url) }); + this.orch.debugLog({ msg: `The App ${info.appId} is requesting from the outter webs:`, info: { ...info, url: censorUrl(info.url) } }); const shouldIgnoreSsrf = request.ssrfValidation !== true; const fetchOptions: ExtendedFetchOptions = { From 9f0c0e5f54dd825db35d2404289e1468ce4d1191 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Fri, 17 Apr 2026 17:34:59 -0300 Subject: [PATCH 12/12] refactor: add options to logger package --- apps/meteor/ee/server/apps/orchestrator.js | 4 ++-- packages/logger/src/getPino.ts | 12 +++++++++--- packages/logger/src/index.ts | 10 +++++----- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/apps/meteor/ee/server/apps/orchestrator.js b/apps/meteor/ee/server/apps/orchestrator.js index 7209aeb0556e8..919f695bedc6d 100644 --- a/apps/meteor/ee/server/apps/orchestrator.js +++ b/apps/meteor/ee/server/apps/orchestrator.js @@ -5,6 +5,7 @@ import * as path from 'path'; import { registerOrchestrator } from '@rocket.chat/apps'; import { EssentialAppDisabledException } from '@rocket.chat/apps-engine/definition/exceptions'; import { AppManager } from '@rocket.chat/apps-engine/server/AppManager'; +import { Logger } from '@rocket.chat/logger'; import { AppLogs, Apps as AppsModel, AppsPersistence, Statistics } from '@rocket.chat/models'; import { Meteor } from 'meteor/meteor'; @@ -28,7 +29,6 @@ import { } from '../../../app/apps/server/converters'; import { AppThreadsConverter } from '../../../app/apps/server/converters/threads'; import { settings } from '../../../app/settings/server'; -import { SystemLogger } from '../../../server/lib/logger/system'; import { canEnableApp } from '../../app/license/server/canEnableApp'; const DISABLED_PRIVATE_APP_INSTALLATION = ['yes', 'true'].includes(String(process.env.DISABLE_PRIVATE_APP_INSTALLATION).toLowerCase()); @@ -45,7 +45,7 @@ export class AppServerOrchestrator { return; } - this._rocketchatLogger = SystemLogger.logger.child({ name: 'Rocket.Chat Apps' }, { redact: redactionFieldPaths }); + this._rocketchatLogger = new Logger('Rocket.Chat Apps', { redact: redactionFieldPaths }); this._model = AppsModel; this._logModel = AppLogs; diff --git a/packages/logger/src/getPino.ts b/packages/logger/src/getPino.ts index c9727b084673b..14cbacb53af29 100644 --- a/packages/logger/src/getPino.ts +++ b/packages/logger/src/getPino.ts @@ -1,4 +1,4 @@ -import { pino } from 'pino'; +import { pino, type ChildLoggerOptions } from 'pino'; const infoLevel = process.env.LESS_INFO_LOGS ? 20 : 35; @@ -25,6 +25,12 @@ const mainPino = pino({ export type MainLogger = typeof mainPino; -export function getPino(name: string, level = 'warn'): MainLogger { - return mainPino.child({ name }, { level }) as MainLogger; +export type LoggerOptions = Pick, 'level' | 'redact'>; + +const defaultOptions: LoggerOptions = { + level: 'warn', +}; + +export function getPino(name: string, options: LoggerOptions = {}): MainLogger { + return mainPino.child({ name }, { ...defaultOptions, ...options }); } diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index b731cb0982459..1e7b33489c6fa 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -1,4 +1,4 @@ -import { getPino, type MainLogger } from './getPino'; +import { getPino, type LoggerOptions, type MainLogger } from './getPino'; import type { LogLevelSetting } from './logLevel'; import { logLevel } from './logLevel'; @@ -27,16 +27,16 @@ logLevel.once('changed', (level: LogLevelSetting) => { export class Logger { readonly logger: MainLogger; - constructor(loggerLabel: string) { - this.logger = getPino(loggerLabel, defaultLevel); + constructor(loggerLabel: string, options: LoggerOptions = {}) { + this.logger = getPino(loggerLabel, { level: defaultLevel, ...options }); logLevel.on('changed', (level: LogLevelSetting) => { this.logger.level = getLevel(level); }); } - section(name: string): MainLogger { - const child = this.logger.child({ section: name }) as MainLogger; + section(name: string, options: LoggerOptions = {}): MainLogger { + const child = this.logger.child({ section: name }, { ...options }); logLevel.on('changed', (level: LogLevelSetting) => { child.level = getLevel(level);