From e1fea71c9735e241b38b7b046eec9bfee16c81e0 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Wed, 2 Jul 2025 09:03:31 +0100 Subject: [PATCH] Filter settings exported when rageshaking (#30236) * Submit filtered settings to rageshakes and sentry. * Add flag to omit some settings from being exported. * Hide user timezone * Hide recent searches and media event ids * Lint * use better wording * lint * Prevent language from being sent * Add tests to check keys are prevented from being uploaded. * don't export invite rules * Update tests --- src/rageshake/submit-rageshake.ts | 4 +- src/sentry.ts | 2 +- src/settings/Settings.tsx | 36 +++++++++++ src/settings/SettingsStore.ts | 15 +++++ .../views/dialogs/BugReportDialog-test.tsx | 12 ++++ .../unit-tests/settings/SettingsStore-test.ts | 13 +++- test/unit-tests/submit-rageshake-test.ts | 59 +++++++++---------- 7 files changed, 107 insertions(+), 34 deletions(-) diff --git a/src/rageshake/submit-rageshake.ts b/src/rageshake/submit-rageshake.ts index 2e04d7d9dc..0717ccd56a 100644 --- a/src/rageshake/submit-rageshake.ts +++ b/src/rageshake/submit-rageshake.ts @@ -171,7 +171,7 @@ async function collectSynapseSpecific(client: MatrixClient, body: FormData): Pro } catch { try { // If that fails we'll hit any endpoint and look at the server response header - const res = await window.fetch(client.http.getUrl("/login"), { + const res = await fetch(client.http.getUrl("/login"), { method: "GET", mode: "cors", }); @@ -257,7 +257,7 @@ export function collectSettings(body: FormData): void { body.append("lowBandwidth", "enabled"); } - body.append("mx_local_settings", localStorage.getItem("mx_local_settings")!); + body.append("mx_local_settings", SettingsStore.exportForRageshake()); } /** diff --git a/src/sentry.ts b/src/sentry.ts index 92e8403963..214f4f09e3 100644 --- a/src/sentry.ts +++ b/src/sentry.ts @@ -141,7 +141,7 @@ async function getCryptoContext(client: MatrixClient): Promise { function getDeviceContext(client: MatrixClient): DeviceContext { const result: DeviceContext = { device_id: client?.deviceId ?? undefined, - mx_local_settings: localStorage.getItem("mx_local_settings"), + mx_local_settings: SettingsStore.exportForRageshake(), }; if (window.Modernizr) { diff --git a/src/settings/Settings.tsx b/src/settings/Settings.tsx index 301c54f4de..1140d24ed8 100644 --- a/src/settings/Settings.tsx +++ b/src/settings/Settings.tsx @@ -173,6 +173,11 @@ export interface IBaseSetting { // Whether the setting should have a warning sign in the microcopy shouldWarn?: boolean; + + /** + * Whether the setting should be exported in a rageshake report. + */ + shouldExportToRageshake?: boolean; } export interface IFeature extends Omit, "isFeature"> { @@ -441,6 +446,8 @@ export const SETTINGS: Settings = { controller: new InviteRulesConfigController(), supportedLevels: [SettingLevel.ACCOUNT], default: InviteRulesConfigController.default, + // Contains server names + shouldExportToRageshake: false, }, "feature_report_to_moderators": { isFeature: true, @@ -503,10 +510,14 @@ export const SETTINGS: Settings = { "mjolnirRooms": { supportedLevels: [SettingLevel.ACCOUNT], default: [], + // Contains room IDs + shouldExportToRageshake: false, }, "mjolnirPersonalRoom": { supportedLevels: [SettingLevel.ACCOUNT], default: null, + // Contains room ID + shouldExportToRageshake: false, }, "feature_html_topic": { isFeature: true, @@ -797,6 +808,8 @@ export const SETTINGS: Settings = { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS, displayName: _td("settings|preferences|user_timezone"), default: "", + // Location leak + shouldExportToRageshake: false, }, "userTimezonePublish": { // This is per-device so you can avoid having devices overwrite each other. @@ -913,6 +926,8 @@ export const SETTINGS: Settings = { "custom_themes": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, default: [], + // Potential privacy leak via theme origin + shouldExportToRageshake: false, }, "use_system_theme": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS, @@ -974,26 +989,36 @@ export const SETTINGS: Settings = { "language": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG, default: "en", + // For privacy + shouldExportToRageshake: false, }, "breadcrumb_rooms": { // not really a setting supportedLevels: [SettingLevel.ACCOUNT], default: [], + // Contains joined rooms + shouldExportToRageshake: false, }, "recent_emoji": { // not really a setting supportedLevels: [SettingLevel.ACCOUNT], default: [], + // For privacy + shouldExportToRageshake: false, }, "SpotlightSearch.recentSearches": { // not really a setting supportedLevels: [SettingLevel.ACCOUNT], default: [], // list of room IDs, most recent first + // For privacy + shouldExportToRageshake: false, }, "showMediaEventIds": { // not really a setting supportedLevels: [SettingLevel.DEVICE], default: {}, // List of events => is visible + // Exports event IDs + shouldExportToRageshake: false, }, "SpotlightSearch.showNsfwPublicRooms": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, @@ -1003,6 +1028,8 @@ export const SETTINGS: Settings = { "room_directory_servers": { supportedLevels: [SettingLevel.ACCOUNT], default: [], + // Contains connected servers for user + shouldExportToRageshake: false, }, "integrationProvisioning": { supportedLevels: [SettingLevel.ACCOUNT], @@ -1012,6 +1039,7 @@ export const SETTINGS: Settings = { supportedLevels: [SettingLevel.ROOM_ACCOUNT, SettingLevel.ROOM_DEVICE], supportedLevelsAreOrdered: true, default: {}, // none allowed + shouldExportToRageshake: false, }, // Legacy, kept around for transitionary purposes "analyticsOptIn": { @@ -1086,6 +1114,8 @@ export const SETTINGS: Settings = { "notificationSound": { supportedLevels: LEVELS_ROOM_OR_ACCOUNT, default: false, + // Contains personal information in file name + shouldExportToRageshake: false, }, "notificationBodyEnabled": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS, @@ -1112,6 +1142,8 @@ export const SETTINGS: Settings = { allow: [], deny: [], }, + // Expses widget information + shouldExportToRageshake: false, }, "breadcrumbs": { supportedLevels: LEVELS_ACCOUNT_SETTINGS, @@ -1201,6 +1233,8 @@ export const SETTINGS: Settings = { // deprecated supportedLevels: LEVELS_ROOM_OR_ACCOUNT, default: {}, + // Sensitive information in widget ID + shouldExportToRageshake: false, }, "Widgets.layout": { supportedLevels: LEVELS_ROOM_OR_ACCOUNT, @@ -1275,6 +1309,8 @@ export const SETTINGS: Settings = { "activeCallRoomIds": { supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS, default: [], + // Contains room IDs + shouldExportToRageshake: false, }, /** * Enable or disable the release announcement feature diff --git a/src/settings/SettingsStore.ts b/src/settings/SettingsStore.ts index aaa836b6fc..bce943ab7a 100644 --- a/src/settings/SettingsStore.ts +++ b/src/settings/SettingsStore.ts @@ -883,6 +883,21 @@ export default class SettingsStore { logger.log(`--- END DEBUG`); } + /** + * Export all settings as a JSON object, except for settings + * blocked from being exported by `shouldExportToRageshake`. + * @returns Settings as a JSON object string. + */ + public static exportForRageshake(): string { + const settingMap: Record = {}; + for (const settingKey of (Object.keys(SETTINGS) as SettingKey[]).filter( + (s) => SETTINGS[s].shouldExportToRageshake !== false, + )) { + settingMap[settingKey] = SettingsStore.getValue(settingKey); + } + return JSON.stringify(settingMap); + } + private static getHandler(settingName: SettingKey, level: SettingLevel): SettingsHandler | null { const handlers = SettingsStore.getHandlers(settingName); if (!handlers[level]) return null; diff --git a/test/unit-tests/components/views/dialogs/BugReportDialog-test.tsx b/test/unit-tests/components/views/dialogs/BugReportDialog-test.tsx index 955ac64b2d..2cd6d609ea 100644 --- a/test/unit-tests/components/views/dialogs/BugReportDialog-test.tsx +++ b/test/unit-tests/components/views/dialogs/BugReportDialog-test.tsx @@ -16,6 +16,7 @@ import BugReportDialog, { } from "../../../../../src/components/views/dialogs/BugReportDialog"; import SdkConfig from "../../../../../src/SdkConfig"; import { type ConsoleLogger } from "../../../../../src/rageshake/rageshake"; +import SettingsStore from "../../../../../src/settings/SettingsStore"; const BUG_REPORT_URL = "https://example.org/submit"; @@ -32,6 +33,16 @@ describe("BugReportDialog", () => { bug_report_endpoint_url: BUG_REPORT_URL, }); + const originalGetValue = SettingsStore.getValue; + jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName, ...args) => { + // These settings rely on a controller that creates an AudioContext in + // order to test whether the setting can be enabled. For the sake of this test, disable that. + if (settingName === "notificationsEnabled" || settingName === "notificationBodyEnabled") { + return true; + } + return originalGetValue(settingName, ...args); + }); + const mockConsoleLogger = { flush: jest.fn(), consume: jest.fn(), @@ -55,6 +66,7 @@ describe("BugReportDialog", () => { }); afterEach(() => { + jest.restoreAllMocks(); SdkConfig.reset(); fetchMock.restore(); }); diff --git a/test/unit-tests/settings/SettingsStore-test.ts b/test/unit-tests/settings/SettingsStore-test.ts index e2f7d845d2..0987569bb9 100644 --- a/test/unit-tests/settings/SettingsStore-test.ts +++ b/test/unit-tests/settings/SettingsStore-test.ts @@ -14,7 +14,7 @@ import SdkConfig from "../../../src/SdkConfig"; import { SettingLevel } from "../../../src/settings/SettingLevel"; import SettingsStore from "../../../src/settings/SettingsStore"; import { mkStubRoom, mockPlatformPeg, stubClient } from "../../test-utils"; -import { type SettingKey } from "../../../src/settings/Settings.tsx"; +import { SETTINGS, type SettingKey } from "../../../src/settings/Settings.tsx"; import MatrixClientBackedController from "../../../src/settings/controllers/MatrixClientBackedController.ts"; const TEST_DATA = [ @@ -55,6 +55,7 @@ describe("SettingsStore", () => { beforeEach(() => { SdkConfig.reset(); + SettingsStore.reset(); }); describe("getValueAt", () => { @@ -82,6 +83,16 @@ describe("SettingsStore", () => { }); }); + describe("exportForRageshake", () => { + it("should not export settings marked as non-exportable", async () => { + await SettingsStore.setValue("userTimezone", null, SettingLevel.DEVICE, "Europe/London"); + const values = JSON.parse(SettingsStore.exportForRageshake()) as Record; + for (const exportedKey of Object.keys(values) as SettingKey[]) { + expect(SETTINGS[exportedKey].shouldExportToRageshake).not.toEqual(false); + } + }); + }); + describe("runMigrations", () => { let client: MatrixClient; let room: Room; diff --git a/test/unit-tests/submit-rageshake-test.ts b/test/unit-tests/submit-rageshake-test.ts index 4eda347a98..832cee3fca 100644 --- a/test/unit-tests/submit-rageshake-test.ts +++ b/test/unit-tests/submit-rageshake-test.ts @@ -22,6 +22,7 @@ import { collectBugReport } from "../../src/rageshake/submit-rageshake"; import SettingsStore from "../../src/settings/SettingsStore"; import { type ConsoleLogger } from "../../src/rageshake/rageshake"; import { type FeatureSettingKey, type SettingKey } from "../../src/settings/Settings.tsx"; +import { SettingLevel } from "../../src/settings/SettingLevel.ts"; describe("Rageshakes", () => { const RUST_CRYPTO_VERSION = "Rust SDK 0.7.0 (691ec63), Vodozemac 0.5.0"; @@ -35,6 +36,8 @@ describe("Rageshakes", () => { onlyData: true, }, ); + let windowSpy: jest.SpyInstance; + let mockWindow: Mocked; beforeEach(() => { mockClient = getMockClientWithEventEmitter({ @@ -50,30 +53,24 @@ describe("Rageshakes", () => { ed25519: "", curve25519: "", }); + mockWindow = { + matchMedia: jest.fn().mockReturnValue({ matches: false }), + navigator: { + userAgent: "", + }, + } as unknown as Mocked; + // @ts-ignore - We just need partial mock + windowSpy = jest.spyOn(global, "window", "get").mockReturnValue(mockWindow); fetchMock.restore(); fetchMock.catch(404); }); + afterEach(() => { + windowSpy.mockRestore(); + }); + describe("Basic Information", () => { - let mockWindow: Mocked; - let windowSpy: jest.SpyInstance; - - beforeEach(() => { - mockWindow = { - matchMedia: jest.fn().mockReturnValue({ matches: false }), - navigator: { - userAgent: "", - }, - } as unknown as Mocked; - // @ts-ignore - We just need partial mock - windowSpy = jest.spyOn(global, "window", "get").mockReturnValue(mockWindow); - }); - - afterEach(() => { - windowSpy.mockRestore(); - }); - it("should include app version", async () => { mockPlatformPeg({ getAppVersion: jest.fn().mockReturnValue("1.11.58") }); @@ -376,6 +373,10 @@ describe("Rageshakes", () => { describe("Settings Store", () => { const mockSettingsStore = mocked(SettingsStore); + afterEach(() => { + jest.restoreAllMocks(); + }); + it("should collect labs from settings store", async () => { const someFeatures = [ "feature_video_rooms", @@ -430,6 +431,7 @@ describe("Rageshakes", () => { afterEach(() => { navigatorSpy.mockRestore(); + SettingsStore.reset(); }); it("should collect navigator storage persisted", async () => { @@ -488,6 +490,7 @@ describe("Rageshakes", () => { }; const disabledFeatures = ["cssanimations", "d0", "d1"]; const mockWindow = { + matchMedia: jest.fn().mockReturnValue({ matches: false }), Modernizr: { ...allFeatures, }, @@ -503,20 +506,16 @@ describe("Rageshakes", () => { }); it("should collect localstorage settings", async () => { - const localSettings = { - language: "fr", - showHiddenEventsInTimeline: true, - activeCallRoomIds: [], - }; - - const spy = jest.spyOn(window.localStorage.__proto__, "getItem").mockImplementation((key) => { - return JSON.stringify(localSettings); - }); + await SettingsStore.setValue("language", null, SettingLevel.DEVICE, "fr"); + await SettingsStore.setValue("showHiddenEventsInTimeline", null, SettingLevel.DEVICE, true); + await SettingsStore.setValue("userTimezone", null, SettingLevel.DEVICE, "Europe/London"); + await SettingsStore.setValue("activeCallRoomIds", null, SettingLevel.DEVICE, []); const formData = await collectBugReport(); - expect(formData.get("mx_local_settings")).toBe(JSON.stringify(localSettings)); - - spy.mockRestore(); + const settingDataJSON = formData.get("mx_local_settings"); + expect(settingDataJSON).not.toBeNull(); + const settingsData = JSON.parse(settingDataJSON as string); + expect(settingsData.showHiddenEventsInTimeline).toEqual(true); }); it("should collect logs", async () => {