From ea1c1d7dbdf6c902ffd5612b5facfa114370090b Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 8 Oct 2025 13:37:51 +0100 Subject: [PATCH 1/9] fix: set readOnly as telemetry bool --- src/server.ts | 2 +- src/telemetry/types.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/server.ts b/src/server.ts index 458bcd28..70c65f18 100644 --- a/src/server.ts +++ b/src/server.ts @@ -190,7 +190,7 @@ export class Server { if (command === "start") { event.properties.startup_time_ms = commandDuration; - event.properties.read_only_mode = this.userConfig.readOnly || false; + event.properties.read_only_mode = this.userConfig.readOnly ? "true" : "false"; event.properties.disabled_tools = this.userConfig.disabledTools || []; event.properties.confirmation_required_tools = this.userConfig.confirmationRequiredTools || []; } diff --git a/src/telemetry/types.ts b/src/telemetry/types.ts index c1eced5a..11d75f06 100644 --- a/src/telemetry/types.ts +++ b/src/telemetry/types.ts @@ -43,7 +43,7 @@ export type ServerEventProperties = { reason?: string; startup_time_ms?: number; runtime_duration_ms?: number; - read_only_mode?: boolean; + read_only_mode?: TelemetryBoolSet; disabled_tools?: string[]; confirmation_required_tools?: string[]; }; From afe863eb09f0f246953ddd3e358cf8bcb121eb4e Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 8 Oct 2025 14:02:08 +0100 Subject: [PATCH 2/9] apply redact in telemetry properties --- src/telemetry/telemetry.ts | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index bdba51a5..c33be6b5 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -1,5 +1,5 @@ import type { Session } from "../common/session.js"; -import type { BaseEvent, CommonProperties } from "./types.js"; +import type { BaseEvent, CommonProperties, TelemetryEvent } from "./types.js"; import type { UserConfig } from "../common/config.js"; import { LogId } from "../common/logger.js"; import type { ApiClient } from "../common/atlas/apiClient.js"; @@ -8,6 +8,7 @@ import { EventCache } from "./eventCache.js"; import { detectContainerEnv } from "../helpers/container.js"; import type { DeviceId } from "../helpers/deviceId.js"; import { EventEmitter } from "events"; +import { redact } from "mongodb-redact"; type EventResult = { success: boolean; @@ -52,8 +53,8 @@ export class Telemetry { } = {} ): Telemetry { const mergedProperties = { - ...MACHINE_METADATA, - ...commonProperties, + ...redact(MACHINE_METADATA, session.keychain.allSecrets), + ...redact(commonProperties, session.keychain.allSecrets), }; const instance = new Telemetry(session, userConfig, mergedProperties, { eventCache, @@ -123,7 +124,6 @@ export class Telemetry { this.events.emit("events-skipped"); return; } - // Don't wait for events to be sent - we should not block regular server // operations on telemetry void this.emit(events); @@ -135,11 +135,11 @@ export class Telemetry { */ public getCommonProperties(): CommonProperties { return { - ...this.commonProperties, + ...redact(this.commonProperties, this.session.keychain.allSecrets), transport: this.userConfig.transport, - mcp_client_version: this.session.mcpClient?.version, - mcp_client_name: this.session.mcpClient?.name, - session_id: this.session.sessionId, + mcp_client_version: redact(this.session.mcpClient?.version, this.session.keychain.allSecrets), + mcp_client_name: redact(this.session.mcpClient?.name, this.session.keychain.allSecrets), + session_id: redact(this.session.sessionId, this.session.keychain.allSecrets), config_atlas_auth: this.session.apiClient.hasCredentials() ? "true" : "false", config_connection_string: this.userConfig.connectionString ? "true" : "false", }; @@ -214,13 +214,17 @@ export class Telemetry { /** * Attempts to send events through the provided API client + * Events are redacted before being sent to ensure no sensitive data is transmitted */ private async sendEvents(client: ApiClient, events: BaseEvent[]): Promise { try { await client.sendEvents( events.map((event) => ({ ...event, - properties: { ...this.getCommonProperties(), ...event.properties }, + properties: { + ...this.getCommonProperties(), + ...redact(event.properties, this.session.keychain.allSecrets), + }, })) ); return { success: true }; From c52d91f4d677111967313836084f9512a33d2a07 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 8 Oct 2025 14:21:15 +0100 Subject: [PATCH 3/9] fixes and add test --- src/telemetry/telemetry.ts | 4 ++-- src/telemetry/types.ts | 2 +- tests/integration/telemetry.test.ts | 28 ++++++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index c33be6b5..080049f3 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -1,5 +1,5 @@ import type { Session } from "../common/session.js"; -import type { BaseEvent, CommonProperties, TelemetryEvent } from "./types.js"; +import type { BaseEvent, CommonProperties } from "./types.js"; import type { UserConfig } from "../common/config.js"; import { LogId } from "../common/logger.js"; import type { ApiClient } from "../common/atlas/apiClient.js"; @@ -80,7 +80,7 @@ export class Telemetry { const [deviceIdValue, containerEnv] = await this.setupPromise; this.commonProperties.device_id = deviceIdValue; - this.commonProperties.is_container_env = containerEnv; + this.commonProperties.is_container_env = containerEnv ? "true" : "false"; this.isBufferingEvents = false; } diff --git a/src/telemetry/types.ts b/src/telemetry/types.ts index 11d75f06..0c32799a 100644 --- a/src/telemetry/types.ts +++ b/src/telemetry/types.ts @@ -97,7 +97,7 @@ export type CommonProperties = { /** * A boolean indicating whether the server is running in a container environment. */ - is_container_env?: boolean; + is_container_env?: TelemetryBoolSet; /** * The version of the MCP client as reported by the client on session establishment. diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index c05e4100..b0d7cdc6 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -36,4 +36,32 @@ describe("Telemetry", () => { expect(telemetry.getCommonProperties().device_id).toBe(actualDeviceId); expect(telemetry["isBufferingEvents"]).toBe(false); }); + + it("should redact sensitive data", async () => { + const logger = new CompositeLogger(); + const deviceId = DeviceId.create(logger); + + // configure keychain with a secret that would show up in random properties + const keychain = new Keychain(); + keychain.register(process.platform, "url"); + + const telemetry = Telemetry.create( + new Session({ + apiBaseUrl: "", + logger, + exportsManager: ExportsManager.init(config, logger), + connectionManager: new MCPConnectionManager(config, driverOptions, logger, deviceId), + keychain: keychain, + }), + config, + deviceId + ); + + await telemetry.setupPromise; + + // expect the platform to be redacted + const commonProperties = telemetry.getCommonProperties(); + expect(commonProperties.platform).toBe(""); + expect(telemetry["isBufferingEvents"]).toBe(false); + }); }); From 6df4a93f77463c374e7b206a92cd257ce9e7335e Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 8 Oct 2025 15:18:07 +0100 Subject: [PATCH 4/9] redact on emission --- src/telemetry/telemetry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 080049f3..34c6abfa 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -222,7 +222,7 @@ export class Telemetry { events.map((event) => ({ ...event, properties: { - ...this.getCommonProperties(), + ...redact(this.getCommonProperties()), ...redact(event.properties, this.session.keychain.allSecrets), }, })) From 183f9c5fe0b97f9f8528791737912cc427f1dcef Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 8 Oct 2025 15:20:04 +0100 Subject: [PATCH 5/9] fix get common properties --- src/telemetry/telemetry.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 34c6abfa..68603e45 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -135,11 +135,11 @@ export class Telemetry { */ public getCommonProperties(): CommonProperties { return { - ...redact(this.commonProperties, this.session.keychain.allSecrets), + ...this.commonProperties, transport: this.userConfig.transport, - mcp_client_version: redact(this.session.mcpClient?.version, this.session.keychain.allSecrets), - mcp_client_name: redact(this.session.mcpClient?.name, this.session.keychain.allSecrets), - session_id: redact(this.session.sessionId, this.session.keychain.allSecrets), + mcp_client_version: this.session.mcpClient?.version, + mcp_client_name: this.session.mcpClient?.name, + session_id: this.session.sessionId, config_atlas_auth: this.session.apiClient.hasCredentials() ? "true" : "false", config_connection_string: this.userConfig.connectionString ? "true" : "false", }; From d84c79361847d0b13c62e33de81c0f38d26a01f7 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 8 Oct 2025 17:44:28 +0100 Subject: [PATCH 6/9] update and add unit tests --- src/telemetry/telemetry.ts | 6 +- tests/integration/telemetry.test.ts | 28 --------- tests/unit/telemetry.test.ts | 88 +++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 31 deletions(-) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 68603e45..a364dcb3 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -53,8 +53,8 @@ export class Telemetry { } = {} ): Telemetry { const mergedProperties = { - ...redact(MACHINE_METADATA, session.keychain.allSecrets), - ...redact(commonProperties, session.keychain.allSecrets), + ...MACHINE_METADATA, + ...commonProperties, }; const instance = new Telemetry(session, userConfig, mergedProperties, { eventCache, @@ -222,7 +222,7 @@ export class Telemetry { events.map((event) => ({ ...event, properties: { - ...redact(this.getCommonProperties()), + ...redact(this.getCommonProperties(), this.session.keychain.allSecrets), ...redact(event.properties, this.session.keychain.allSecrets), }, })) diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index b0d7cdc6..c05e4100 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -36,32 +36,4 @@ describe("Telemetry", () => { expect(telemetry.getCommonProperties().device_id).toBe(actualDeviceId); expect(telemetry["isBufferingEvents"]).toBe(false); }); - - it("should redact sensitive data", async () => { - const logger = new CompositeLogger(); - const deviceId = DeviceId.create(logger); - - // configure keychain with a secret that would show up in random properties - const keychain = new Keychain(); - keychain.register(process.platform, "url"); - - const telemetry = Telemetry.create( - new Session({ - apiBaseUrl: "", - logger, - exportsManager: ExportsManager.init(config, logger), - connectionManager: new MCPConnectionManager(config, driverOptions, logger, deviceId), - keychain: keychain, - }), - config, - deviceId - ); - - await telemetry.setupPromise; - - // expect the platform to be redacted - const commonProperties = telemetry.getCommonProperties(); - expect(commonProperties.platform).toBe(""); - expect(telemetry["isBufferingEvents"]).toBe(false); - }); }); diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 0dd75931..9ac3defe 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -9,6 +9,7 @@ import { NullLogger } from "../../tests/utils/index.js"; import type { MockedFunction } from "vitest"; import type { DeviceId } from "../../src/helpers/deviceId.js"; import { expectDefined } from "../integration/helpers.js"; +import { Keychain } from "../../src/common/keychain.js"; // Mock the ApiClient to avoid real API calls vi.mock("../../src/common/atlas/apiClient.js"); @@ -140,6 +141,7 @@ describe("Telemetry", () => { close: vi.fn().mockResolvedValue(undefined), setAgentRunner: vi.fn().mockResolvedValue(undefined), logger: new NullLogger(), + keychain: new Keychain(), } as unknown as Session; telemetry = Telemetry.create(session, config, mockDeviceId, { @@ -345,4 +347,90 @@ describe("Telemetry", () => { verifyMockCalls(); }); }); + + describe("when secrets are registered", () => { + describe("comprehensive redaction coverage", () => { + it("should redact sensitive data from CommonStaticProperties", async () => { + session.keychain.register("secret-server-version", "password"); + session.keychain.register("secret-server-name", "password"); + session.keychain.register("secret-password", "password"); + session.keychain.register("secret-key", "password"); + session.keychain.register("secret-token", "password"); + session.keychain.register("secret-password-version", "password"); + + const sensitiveStaticProps = { + mcp_server_version: "secret-server-version", + mcp_server_name: "secret-server-name", + platform: "linux-secret-password", // OS info might contain sensitive paths + arch: "x64-secret-key", // Architecture info might contain sensitive details + os_type: "linux-secret-token", // OS type might contain sensitive info + os_version: "secret-password-version", // OS version might contain sensitive details + }; + + telemetry = Telemetry.create(session, config, mockDeviceId, { + eventCache: mockEventCache as unknown as EventCache, + commonProperties: sensitiveStaticProps, + }); + + await telemetry.setupPromise; + + telemetry.emitEvents([createTestEvent()]); + + const calls = mockApiClient.sendEvents.mock.calls; + expect(calls).toHaveLength(1); + + // get event properties + const sentEvent = calls[0]?.[0][0] as { properties: Record }; + expectDefined(sentEvent); + + const eventProps = sentEvent.properties; + expect(eventProps.mcp_server_version).toBe(""); + expect(eventProps.mcp_server_name).toBe(""); + expect(eventProps.platform).toBe("linux-"); + expect(eventProps.arch).toBe("x64-"); + expect(eventProps.os_type).toBe("linux-"); + expect(eventProps.os_version).toBe("-version"); + }); + + it("should redact sensitive data from CommonProperties", async () => { + // register the common properties as sensitive data + session.keychain.register("test-device-id", "password"); + session.keychain.register(session.sessionId, "password"); + + telemetry.emitEvents([createTestEvent()]); + + const calls = mockApiClient.sendEvents.mock.calls; + expect(calls).toHaveLength(1); + + // get event properties + const sentEvent = calls[0]?.[0][0] as { properties: Record }; + expectDefined(sentEvent); + + const eventProps = sentEvent.properties; + + expect(eventProps.device_id).toBe(""); + expect(eventProps.session_id).toBe(""); + }); + + it("should redact sensitive data that is added to events", () => { + session.keychain.register("test-device-id", "password"); + session.keychain.register(session.sessionId, "password"); + session.keychain.register("test-component", "password"); + + telemetry.emitEvents([createTestEvent()]); + + const calls = mockApiClient.sendEvents.mock.calls; + expect(calls).toHaveLength(1); + + // get event properties + const sentEvent = calls[0]?.[0][0] as { properties: Record }; + expectDefined(sentEvent); + + const eventProps = sentEvent.properties; + expect(eventProps.device_id).toBe(""); + expect(eventProps.session_id).toBe(""); + expect(eventProps.component).toBe(""); + }); + }); + }); }); From bfdf4b686219caf496384cc9643647a1a1aca123 Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 8 Oct 2025 17:45:48 +0100 Subject: [PATCH 7/9] update comments --- tests/unit/telemetry.test.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 9ac3defe..9ecbac17 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -358,13 +358,14 @@ describe("Telemetry", () => { session.keychain.register("secret-token", "password"); session.keychain.register("secret-password-version", "password"); + // Simulates sensitive data across random properties const sensitiveStaticProps = { mcp_server_version: "secret-server-version", mcp_server_name: "secret-server-name", - platform: "linux-secret-password", // OS info might contain sensitive paths - arch: "x64-secret-key", // Architecture info might contain sensitive details - os_type: "linux-secret-token", // OS type might contain sensitive info - os_version: "secret-password-version", // OS version might contain sensitive details + platform: "linux-secret-password", + arch: "x64-secret-key", + os_type: "linux-secret-token", + os_version: "secret-password-version", }; telemetry = Telemetry.create(session, config, mockDeviceId, { From 92d5d83cc2065c88a0a129709caa95395a629d3f Mon Sep 17 00:00:00 2001 From: Bianca Lisle Date: Wed, 8 Oct 2025 17:48:11 +0100 Subject: [PATCH 8/9] lint --- tests/unit/telemetry.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 9ecbac17..8b4a0c9c 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -393,7 +393,7 @@ describe("Telemetry", () => { expect(eventProps.os_version).toBe("-version"); }); - it("should redact sensitive data from CommonProperties", async () => { + it("should redact sensitive data from CommonProperties", () => { // register the common properties as sensitive data session.keychain.register("test-device-id", "password"); session.keychain.register(session.sessionId, "password"); From a9101fbc185efb37d22c73cf05f4d7377bd5356f Mon Sep 17 00:00:00 2001 From: Bianca Lisle <40155621+blva@users.noreply.github.com> Date: Wed, 8 Oct 2025 18:06:08 +0100 Subject: [PATCH 9/9] Update src/telemetry/telemetry.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/telemetry/telemetry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index a364dcb3..6a9db5c1 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -213,7 +213,7 @@ export class Telemetry { } /** - * Attempts to send events through the provided API client + * Attempts to send events through the provided API client. * Events are redacted before being sent to ensure no sensitive data is transmitted */ private async sendEvents(client: ApiClient, events: BaseEvent[]): Promise {