chore: use redact by default (#624)

Author: blva

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 || [];
         }

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<EventResult> {
         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 };

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.

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<string, unknown> };
+                expectDefined(sentEvent);
+
+                const eventProps = sentEvent.properties;
+                expect(eventProps.mcp_server_version).toBe("<password>");
+                expect(eventProps.mcp_server_name).toBe("<password>");
+                expect(eventProps.platform).toBe("linux-<password>");
+                expect(eventProps.arch).toBe("x64-<password>");
+                expect(eventProps.os_type).toBe("linux-<password>");
+                expect(eventProps.os_version).toBe("<password>-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<string, unknown> };
+                expectDefined(sentEvent);
+
+                const eventProps = sentEvent.properties;
+
+                expect(eventProps.device_id).toBe("<password>");
+                expect(eventProps.session_id).toBe("<password>");
+            });
+
+            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<string, unknown> };
+                expectDefined(sentEvent);
+
+                const eventProps = sentEvent.properties;
+                expect(eventProps.device_id).toBe("<password>");
+                expect(eventProps.session_id).toBe("<password>");
+                expect(eventProps.component).toBe("<password>");
+            });
+        });
+    });
 });