diff --git a/src/server.ts b/src/server.ts index 794fb986..f8aa3226 100644 --- a/src/server.ts +++ b/src/server.ts @@ -201,7 +201,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/telemetry.ts b/src/telemetry/telemetry.ts index bdba51a5..6a9db5c1 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -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; @@ -79,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; } @@ -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); @@ -213,14 +213,18 @@ 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 { try { await client.sendEvents( events.map((event) => ({ ...event, - properties: { ...this.getCommonProperties(), ...event.properties }, + properties: { + ...redact(this.getCommonProperties(), this.session.keychain.allSecrets), + ...redact(event.properties, this.session.keychain.allSecrets), + }, })) ); return { success: true }; diff --git a/src/telemetry/types.ts b/src/telemetry/types.ts index c1eced5a..0c32799a 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[]; }; @@ -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/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 0dd75931..8b4a0c9c 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,91 @@ 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"); + + // Simulates sensitive data across random properties + const sensitiveStaticProps = { + mcp_server_version: "secret-server-version", + mcp_server_name: "secret-server-name", + platform: "linux-secret-password", + arch: "x64-secret-key", + os_type: "linux-secret-token", + os_version: "secret-password-version", + }; + + 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", () => { + // 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(""); + }); + }); + }); });