Skip to content

Commit e1fea71

Browse files
authored
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
1 parent 99f7656 commit e1fea71

7 files changed

Lines changed: 107 additions & 34 deletions

File tree

src/rageshake/submit-rageshake.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ async function collectSynapseSpecific(client: MatrixClient, body: FormData): Pro
171171
} catch {
172172
try {
173173
// If that fails we'll hit any endpoint and look at the server response header
174-
const res = await window.fetch(client.http.getUrl("/login"), {
174+
const res = await fetch(client.http.getUrl("/login"), {
175175
method: "GET",
176176
mode: "cors",
177177
});
@@ -257,7 +257,7 @@ export function collectSettings(body: FormData): void {
257257
body.append("lowBandwidth", "enabled");
258258
}
259259

260-
body.append("mx_local_settings", localStorage.getItem("mx_local_settings")!);
260+
body.append("mx_local_settings", SettingsStore.exportForRageshake());
261261
}
262262

263263
/**

src/sentry.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ async function getCryptoContext(client: MatrixClient): Promise<CryptoContext> {
141141
function getDeviceContext(client: MatrixClient): DeviceContext {
142142
const result: DeviceContext = {
143143
device_id: client?.deviceId ?? undefined,
144-
mx_local_settings: localStorage.getItem("mx_local_settings"),
144+
mx_local_settings: SettingsStore.exportForRageshake(),
145145
};
146146

147147
if (window.Modernizr) {

src/settings/Settings.tsx

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,11 @@ export interface IBaseSetting<T extends SettingValueType = SettingValueType> {
173173

174174
// Whether the setting should have a warning sign in the microcopy
175175
shouldWarn?: boolean;
176+
177+
/**
178+
* Whether the setting should be exported in a rageshake report.
179+
*/
180+
shouldExportToRageshake?: boolean;
176181
}
177182

178183
export interface IFeature extends Omit<IBaseSetting<boolean>, "isFeature"> {
@@ -441,6 +446,8 @@ export const SETTINGS: Settings = {
441446
controller: new InviteRulesConfigController(),
442447
supportedLevels: [SettingLevel.ACCOUNT],
443448
default: InviteRulesConfigController.default,
449+
// Contains server names
450+
shouldExportToRageshake: false,
444451
},
445452
"feature_report_to_moderators": {
446453
isFeature: true,
@@ -503,10 +510,14 @@ export const SETTINGS: Settings = {
503510
"mjolnirRooms": {
504511
supportedLevels: [SettingLevel.ACCOUNT],
505512
default: [],
513+
// Contains room IDs
514+
shouldExportToRageshake: false,
506515
},
507516
"mjolnirPersonalRoom": {
508517
supportedLevels: [SettingLevel.ACCOUNT],
509518
default: null,
519+
// Contains room ID
520+
shouldExportToRageshake: false,
510521
},
511522
"feature_html_topic": {
512523
isFeature: true,
@@ -797,6 +808,8 @@ export const SETTINGS: Settings = {
797808
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
798809
displayName: _td("settings|preferences|user_timezone"),
799810
default: "",
811+
// Location leak
812+
shouldExportToRageshake: false,
800813
},
801814
"userTimezonePublish": {
802815
// This is per-device so you can avoid having devices overwrite each other.
@@ -913,6 +926,8 @@ export const SETTINGS: Settings = {
913926
"custom_themes": {
914927
supportedLevels: LEVELS_ACCOUNT_SETTINGS,
915928
default: [],
929+
// Potential privacy leak via theme origin
930+
shouldExportToRageshake: false,
916931
},
917932
"use_system_theme": {
918933
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
@@ -974,26 +989,36 @@ export const SETTINGS: Settings = {
974989
"language": {
975990
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS_WITH_CONFIG,
976991
default: "en",
992+
// For privacy
993+
shouldExportToRageshake: false,
977994
},
978995
"breadcrumb_rooms": {
979996
// not really a setting
980997
supportedLevels: [SettingLevel.ACCOUNT],
981998
default: [],
999+
// Contains joined rooms
1000+
shouldExportToRageshake: false,
9821001
},
9831002
"recent_emoji": {
9841003
// not really a setting
9851004
supportedLevels: [SettingLevel.ACCOUNT],
9861005
default: [],
1006+
// For privacy
1007+
shouldExportToRageshake: false,
9871008
},
9881009
"SpotlightSearch.recentSearches": {
9891010
// not really a setting
9901011
supportedLevels: [SettingLevel.ACCOUNT],
9911012
default: [], // list of room IDs, most recent first
1013+
// For privacy
1014+
shouldExportToRageshake: false,
9921015
},
9931016
"showMediaEventIds": {
9941017
// not really a setting
9951018
supportedLevels: [SettingLevel.DEVICE],
9961019
default: {}, // List of events => is visible
1020+
// Exports event IDs
1021+
shouldExportToRageshake: false,
9971022
},
9981023
"SpotlightSearch.showNsfwPublicRooms": {
9991024
supportedLevels: LEVELS_ACCOUNT_SETTINGS,
@@ -1003,6 +1028,8 @@ export const SETTINGS: Settings = {
10031028
"room_directory_servers": {
10041029
supportedLevels: [SettingLevel.ACCOUNT],
10051030
default: [],
1031+
// Contains connected servers for user
1032+
shouldExportToRageshake: false,
10061033
},
10071034
"integrationProvisioning": {
10081035
supportedLevels: [SettingLevel.ACCOUNT],
@@ -1012,6 +1039,7 @@ export const SETTINGS: Settings = {
10121039
supportedLevels: [SettingLevel.ROOM_ACCOUNT, SettingLevel.ROOM_DEVICE],
10131040
supportedLevelsAreOrdered: true,
10141041
default: {}, // none allowed
1042+
shouldExportToRageshake: false,
10151043
},
10161044
// Legacy, kept around for transitionary purposes
10171045
"analyticsOptIn": {
@@ -1086,6 +1114,8 @@ export const SETTINGS: Settings = {
10861114
"notificationSound": {
10871115
supportedLevels: LEVELS_ROOM_OR_ACCOUNT,
10881116
default: false,
1117+
// Contains personal information in file name
1118+
shouldExportToRageshake: false,
10891119
},
10901120
"notificationBodyEnabled": {
10911121
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
@@ -1112,6 +1142,8 @@ export const SETTINGS: Settings = {
11121142
allow: [],
11131143
deny: [],
11141144
},
1145+
// Expses widget information
1146+
shouldExportToRageshake: false,
11151147
},
11161148
"breadcrumbs": {
11171149
supportedLevels: LEVELS_ACCOUNT_SETTINGS,
@@ -1201,6 +1233,8 @@ export const SETTINGS: Settings = {
12011233
// deprecated
12021234
supportedLevels: LEVELS_ROOM_OR_ACCOUNT,
12031235
default: {},
1236+
// Sensitive information in widget ID
1237+
shouldExportToRageshake: false,
12041238
},
12051239
"Widgets.layout": {
12061240
supportedLevels: LEVELS_ROOM_OR_ACCOUNT,
@@ -1275,6 +1309,8 @@ export const SETTINGS: Settings = {
12751309
"activeCallRoomIds": {
12761310
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
12771311
default: [],
1312+
// Contains room IDs
1313+
shouldExportToRageshake: false,
12781314
},
12791315
/**
12801316
* Enable or disable the release announcement feature

src/settings/SettingsStore.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -883,6 +883,21 @@ export default class SettingsStore {
883883
logger.log(`--- END DEBUG`);
884884
}
885885

886+
/**
887+
* Export all settings as a JSON object, except for settings
888+
* blocked from being exported by `shouldExportToRageshake`.
889+
* @returns Settings as a JSON object string.
890+
*/
891+
public static exportForRageshake(): string {
892+
const settingMap: Record<string, unknown> = {};
893+
for (const settingKey of (Object.keys(SETTINGS) as SettingKey[]).filter(
894+
(s) => SETTINGS[s].shouldExportToRageshake !== false,
895+
)) {
896+
settingMap[settingKey] = SettingsStore.getValue(settingKey);
897+
}
898+
return JSON.stringify(settingMap);
899+
}
900+
886901
private static getHandler(settingName: SettingKey, level: SettingLevel): SettingsHandler | null {
887902
const handlers = SettingsStore.getHandlers(settingName);
888903
if (!handlers[level]) return null;

test/unit-tests/components/views/dialogs/BugReportDialog-test.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import BugReportDialog, {
1616
} from "../../../../../src/components/views/dialogs/BugReportDialog";
1717
import SdkConfig from "../../../../../src/SdkConfig";
1818
import { type ConsoleLogger } from "../../../../../src/rageshake/rageshake";
19+
import SettingsStore from "../../../../../src/settings/SettingsStore";
1920

2021
const BUG_REPORT_URL = "https://example.org/submit";
2122

@@ -32,6 +33,16 @@ describe("BugReportDialog", () => {
3233
bug_report_endpoint_url: BUG_REPORT_URL,
3334
});
3435

36+
const originalGetValue = SettingsStore.getValue;
37+
jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName, ...args) => {
38+
// These settings rely on a controller that creates an AudioContext in
39+
// order to test whether the setting can be enabled. For the sake of this test, disable that.
40+
if (settingName === "notificationsEnabled" || settingName === "notificationBodyEnabled") {
41+
return true;
42+
}
43+
return originalGetValue(settingName, ...args);
44+
});
45+
3546
const mockConsoleLogger = {
3647
flush: jest.fn(),
3748
consume: jest.fn(),
@@ -55,6 +66,7 @@ describe("BugReportDialog", () => {
5566
});
5667

5768
afterEach(() => {
69+
jest.restoreAllMocks();
5870
SdkConfig.reset();
5971
fetchMock.restore();
6072
});

test/unit-tests/settings/SettingsStore-test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import SdkConfig from "../../../src/SdkConfig";
1414
import { SettingLevel } from "../../../src/settings/SettingLevel";
1515
import SettingsStore from "../../../src/settings/SettingsStore";
1616
import { mkStubRoom, mockPlatformPeg, stubClient } from "../../test-utils";
17-
import { type SettingKey } from "../../../src/settings/Settings.tsx";
17+
import { SETTINGS, type SettingKey } from "../../../src/settings/Settings.tsx";
1818
import MatrixClientBackedController from "../../../src/settings/controllers/MatrixClientBackedController.ts";
1919

2020
const TEST_DATA = [
@@ -55,6 +55,7 @@ describe("SettingsStore", () => {
5555

5656
beforeEach(() => {
5757
SdkConfig.reset();
58+
SettingsStore.reset();
5859
});
5960

6061
describe("getValueAt", () => {
@@ -82,6 +83,16 @@ describe("SettingsStore", () => {
8283
});
8384
});
8485

86+
describe("exportForRageshake", () => {
87+
it("should not export settings marked as non-exportable", async () => {
88+
await SettingsStore.setValue("userTimezone", null, SettingLevel.DEVICE, "Europe/London");
89+
const values = JSON.parse(SettingsStore.exportForRageshake()) as Record<SettingKey, unknown>;
90+
for (const exportedKey of Object.keys(values) as SettingKey[]) {
91+
expect(SETTINGS[exportedKey].shouldExportToRageshake).not.toEqual(false);
92+
}
93+
});
94+
});
95+
8596
describe("runMigrations", () => {
8697
let client: MatrixClient;
8798
let room: Room;

test/unit-tests/submit-rageshake-test.ts

Lines changed: 29 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { collectBugReport } from "../../src/rageshake/submit-rageshake";
2222
import SettingsStore from "../../src/settings/SettingsStore";
2323
import { type ConsoleLogger } from "../../src/rageshake/rageshake";
2424
import { type FeatureSettingKey, type SettingKey } from "../../src/settings/Settings.tsx";
25+
import { SettingLevel } from "../../src/settings/SettingLevel.ts";
2526

2627
describe("Rageshakes", () => {
2728
const RUST_CRYPTO_VERSION = "Rust SDK 0.7.0 (691ec63), Vodozemac 0.5.0";
@@ -35,6 +36,8 @@ describe("Rageshakes", () => {
3536
onlyData: true,
3637
},
3738
);
39+
let windowSpy: jest.SpyInstance;
40+
let mockWindow: Mocked<Window>;
3841

3942
beforeEach(() => {
4043
mockClient = getMockClientWithEventEmitter({
@@ -50,30 +53,24 @@ describe("Rageshakes", () => {
5053
ed25519: "",
5154
curve25519: "",
5255
});
56+
mockWindow = {
57+
matchMedia: jest.fn().mockReturnValue({ matches: false }),
58+
navigator: {
59+
userAgent: "",
60+
},
61+
} as unknown as Mocked<Window>;
62+
// @ts-ignore - We just need partial mock
63+
windowSpy = jest.spyOn(global, "window", "get").mockReturnValue(mockWindow);
5364

5465
fetchMock.restore();
5566
fetchMock.catch(404);
5667
});
5768

58-
describe("Basic Information", () => {
59-
let mockWindow: Mocked<Window>;
60-
let windowSpy: jest.SpyInstance;
61-
62-
beforeEach(() => {
63-
mockWindow = {
64-
matchMedia: jest.fn().mockReturnValue({ matches: false }),
65-
navigator: {
66-
userAgent: "",
67-
},
68-
} as unknown as Mocked<Window>;
69-
// @ts-ignore - We just need partial mock
70-
windowSpy = jest.spyOn(global, "window", "get").mockReturnValue(mockWindow);
71-
});
72-
73-
afterEach(() => {
74-
windowSpy.mockRestore();
75-
});
69+
afterEach(() => {
70+
windowSpy.mockRestore();
71+
});
7672

73+
describe("Basic Information", () => {
7774
it("should include app version", async () => {
7875
mockPlatformPeg({ getAppVersion: jest.fn().mockReturnValue("1.11.58") });
7976

@@ -376,6 +373,10 @@ describe("Rageshakes", () => {
376373
describe("Settings Store", () => {
377374
const mockSettingsStore = mocked(SettingsStore);
378375

376+
afterEach(() => {
377+
jest.restoreAllMocks();
378+
});
379+
379380
it("should collect labs from settings store", async () => {
380381
const someFeatures = [
381382
"feature_video_rooms",
@@ -430,6 +431,7 @@ describe("Rageshakes", () => {
430431

431432
afterEach(() => {
432433
navigatorSpy.mockRestore();
434+
SettingsStore.reset();
433435
});
434436

435437
it("should collect navigator storage persisted", async () => {
@@ -488,6 +490,7 @@ describe("Rageshakes", () => {
488490
};
489491
const disabledFeatures = ["cssanimations", "d0", "d1"];
490492
const mockWindow = {
493+
matchMedia: jest.fn().mockReturnValue({ matches: false }),
491494
Modernizr: {
492495
...allFeatures,
493496
},
@@ -503,20 +506,16 @@ describe("Rageshakes", () => {
503506
});
504507

505508
it("should collect localstorage settings", async () => {
506-
const localSettings = {
507-
language: "fr",
508-
showHiddenEventsInTimeline: true,
509-
activeCallRoomIds: [],
510-
};
511-
512-
const spy = jest.spyOn(window.localStorage.__proto__, "getItem").mockImplementation((key) => {
513-
return JSON.stringify(localSettings);
514-
});
509+
await SettingsStore.setValue("language", null, SettingLevel.DEVICE, "fr");
510+
await SettingsStore.setValue("showHiddenEventsInTimeline", null, SettingLevel.DEVICE, true);
511+
await SettingsStore.setValue("userTimezone", null, SettingLevel.DEVICE, "Europe/London");
512+
await SettingsStore.setValue("activeCallRoomIds", null, SettingLevel.DEVICE, []);
515513

516514
const formData = await collectBugReport();
517-
expect(formData.get("mx_local_settings")).toBe(JSON.stringify(localSettings));
518-
519-
spy.mockRestore();
515+
const settingDataJSON = formData.get("mx_local_settings");
516+
expect(settingDataJSON).not.toBeNull();
517+
const settingsData = JSON.parse(settingDataJSON as string);
518+
expect(settingsData.showHiddenEventsInTimeline).toEqual(true);
520519
});
521520

522521
it("should collect logs", async () => {

0 commit comments

Comments
 (0)