diff --git a/config.json b/config.json index feac16efd..746827789 100644 --- a/config.json +++ b/config.json @@ -1,7 +1,14 @@ { "productName": "Sable", "defaultHomeserver": 0, - "homeserverList": ["matrix.org", "mozilla.org", "unredacted.org", "sable.moe", "kendama.moe"], + "homeserverList": [ + "matrix.org", + "mozilla.org", + "unredacted.org", + "sable.moe", + "kendama.moe", + "hopium.club" + ], "allowCustomHomeservers": true, "elementCallUrl": null, diff --git a/src-tauri/src/network/media_protocol/crypto.rs b/src-tauri/src/network/media_protocol/crypto.rs index f32f2de1a..2563a1cf8 100644 --- a/src-tauri/src/network/media_protocol/crypto.rs +++ b/src-tauri/src/network/media_protocol/crypto.rs @@ -44,21 +44,21 @@ impl EncryptionStore { version: &str, content_type: String, ) -> Result<(), String> { - let key_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + let key_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD_INDIFFERENT .decode(key) .map_err(|error| format!("invalid key base64url: {error}"))?; let key = key_bytes .try_into() .map_err(|bytes: Vec| format!("key must be 32 bytes, got {}", bytes.len()))?; - let iv_bytes = base64::engine::general_purpose::STANDARD_NO_PAD + let iv_bytes = base64::engine::general_purpose::STANDARD_NO_PAD_INDIFFERENT .decode(iv) .map_err(|error| format!("invalid iv base64: {error}"))?; let iv = iv_bytes .try_into() .map_err(|bytes: Vec| format!("iv must be 16 bytes, got {}", bytes.len()))?; - let expected_sha256 = base64::engine::general_purpose::STANDARD_NO_PAD + let expected_sha256 = base64::engine::general_purpose::STANDARD_NO_PAD_INDIFFERENT .decode(sha256) .map_err(|error| format!("invalid sha256 base64: {error}"))?; @@ -168,7 +168,30 @@ pub(super) fn normalize_key(url: &str) -> String { #[cfg(test)] mod tests { - use super::normalize_key; + use super::{normalize_key, EncryptionStore}; + + #[test] + fn register_accepts_padded_base64() { + let store = EncryptionStore::default(); + let url = "https://matrix.example.org/_matrix/client/v1/media/download/matrix.org/abc123"; + for (key, iv, sha256) in [ + ( + "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8", + "G+qzsN3Y9FgAAAAAAAAAAA", + "ICEiIyQlJicoKSorLC0uLzAxMjM0NTY3ODk6Ozw9Pj8", + ), + ( + "AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=", + "G+qzsN3Y9FgAAAAAAAAAAA==", + "ICEiIyQlJicoKSorLC0uLzAxMjM0NTY3ODk6Ozw9Pj8=", + ), + ] { + assert_eq!( + store.register(url, key, iv, sha256, "v2", String::new()), + Ok(()) + ); + } + } #[test] fn normalize_key_strips_sable_media_prefix() { diff --git a/src/app/features/settings/notifications/NotificationTransportRuntime.ts b/src/app/features/settings/notifications/NotificationTransportRuntime.ts index fcc4b1bf3..f9468d1c1 100644 --- a/src/app/features/settings/notifications/NotificationTransportRuntime.ts +++ b/src/app/features/settings/notifications/NotificationTransportRuntime.ts @@ -1,4 +1,11 @@ import type { MatrixClient } from '$types/matrix-sdk'; +import { createDebugLogger } from '$utils/debugLogger'; + +const runtimeLog = createDebugLogger('notification-transport'); + +const logCleanupFailure = (stage: string) => (error: unknown) => { + runtimeLog.warn('notification', `Notification transport ${stage} failed`, { error }); +}; export type NotificationTransportProvider = 'unifiedpush' | 'native' | 'web'; @@ -66,7 +73,7 @@ export class NotificationTransportRuntime { this.#activeProvider = nextActiveProvider; if (previousCleanup) { - await Promise.resolve(previousCleanup()).catch(() => undefined); + await Promise.resolve(previousCleanup()).catch(logCleanupFailure('teardown')); } if (generation !== this.#activeGeneration) return; @@ -74,7 +81,7 @@ export class NotificationTransportRuntime { const listener = await listenerFactory(getContext); if (generation !== this.#activeGeneration || nextActiveProvider !== this.#activeProvider) { - await Promise.resolve(listener.unregister()).catch(() => undefined); + await Promise.resolve(listener.unregister()).catch(logCleanupFailure('stale unregister')); return; } @@ -88,6 +95,6 @@ export class NotificationTransportRuntime { this.#activeProvider = null; if (!cleanup) return; - await Promise.resolve(cleanup()).catch(() => undefined); + await Promise.resolve(cleanup()).catch(logCleanupFailure('dispose')); } } diff --git a/src/app/features/settings/notifications/UnifiedPushMessageListener.test.ts b/src/app/features/settings/notifications/UnifiedPushMessageListener.test.ts index 4dc1ce02b..e590fb662 100644 --- a/src/app/features/settings/notifications/UnifiedPushMessageListener.test.ts +++ b/src/app/features/settings/notifications/UnifiedPushMessageListener.test.ts @@ -1,4 +1,20 @@ import { describe, expect, it, vi } from 'vitest'; + +type LogFn = (category: string, message: string, data?: unknown) => void; + +const { warn } = vi.hoisted(() => ({ + warn: vi.fn(), +})); + +vi.mock('$utils/debugLogger', () => ({ + createDebugLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn, + error: vi.fn(), + }), +})); + import { createUnifiedPushMessageListener, parseUnifiedPushMessage, @@ -117,4 +133,33 @@ describe('parseUnifiedPushMessage', () => { expect(parseUnifiedPushMessage({ message: JSON.stringify(payload) })).toBeNull(); } }); + + it('reports a dropped push instead of discarding it silently', () => { + warn.mockClear(); + + expect( + parseUnifiedPushMessage({ + message: JSON.stringify({ + user_id: '@a:server', + notification: { user_id: '@b:server', room_id: '!r:server' }, + }), + }) + ).toBeNull(); + + expect(warn).toHaveBeenCalledWith('notification', expect.stringContaining('Dropped push'), { + recipientCount: 2, + }); + }); + + it('reports an unparsable push', () => { + warn.mockClear(); + + expect(parseUnifiedPushMessage({ message: 'not json' })).toBeNull(); + + expect(warn).toHaveBeenCalledWith( + 'notification', + expect.stringContaining('Dropped push'), + undefined + ); + }); }); diff --git a/src/app/features/settings/notifications/UnifiedPushMessageListener.ts b/src/app/features/settings/notifications/UnifiedPushMessageListener.ts index b01cd5118..fdccebf4e 100644 --- a/src/app/features/settings/notifications/UnifiedPushMessageListener.ts +++ b/src/app/features/settings/notifications/UnifiedPushMessageListener.ts @@ -1,3 +1,12 @@ +import { createDebugLogger } from '$utils/debugLogger'; + +const listenerLog = createDebugLogger('unifiedpush-listener'); + +const dropPush = (reason: string, data?: unknown): null => { + listenerLog.warn('notification', `Dropped push: ${reason}`, data); + return null; +}; + export type UnifiedPushMessageHandler = (data: Record) => Promise; export type UnifiedPushMessageErrorHandler = (error: unknown) => void; @@ -12,25 +21,25 @@ export function createUnifiedPushMessageListener( export function parseUnifiedPushMessage(raw: unknown): Record | null { const message = (raw as { message?: unknown })?.message; - if (typeof message !== 'string') return null; + if (typeof message !== 'string') return dropPush('no message string'); let payload: unknown; try { payload = JSON.parse(message); } catch { - return null; + return dropPush('unparsable message'); } - if (!isRecord(payload)) return null; + if (!isRecord(payload)) return dropPush('message is not an object'); let notification = payload.notification === undefined ? payload : payload.notification; if (typeof notification === 'string') { try { notification = JSON.parse(notification); } catch { - return null; + return dropPush('unparsable notification'); } } - if (!isRecord(notification)) return null; + if (!isRecord(notification)) return dropPush('notification is not an object'); const recipients = new Set(); const addRecipient = (value: unknown) => { @@ -47,7 +56,9 @@ export function parseUnifiedPushMessage(raw: unknown): Record | } } } - if (recipients.size > 1) return null; + if (recipients.size > 1) { + return dropPush('several recipients', { recipientCount: recipients.size }); + } const [userId] = recipients; return userId ? { ...notification, user_id: userId } : notification; } diff --git a/src/app/features/settings/notifications/UnifiedPushTransport.test.ts b/src/app/features/settings/notifications/UnifiedPushTransport.test.ts index 906bcec76..7dd6d46b5 100644 --- a/src/app/features/settings/notifications/UnifiedPushTransport.test.ts +++ b/src/app/features/settings/notifications/UnifiedPushTransport.test.ts @@ -1,4 +1,20 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; + +type LogFn = (category: string, message: string, data?: unknown) => void; + +const { logError } = vi.hoisted(() => ({ + logError: vi.fn(), +})); + +vi.mock('$utils/debugLogger', () => ({ + createDebugLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: logError, + }), +})); + import { classifyUnifiedPushFailure, ensureUnifiedPushDistributorSelection, @@ -164,6 +180,23 @@ describe('registerUnifiedPushTransport', () => { }); }); + it('reports a registration failure so it reaches telemetry', async () => { + unifiedPushApi.isPermissionGranted.mockResolvedValue(true); + localStorage.removeItem('unifiedpush_distributor'); + unifiedPushApi.listDistributors.mockResolvedValue([]); + unifiedPushApi.registerForPushNotifications.mockRejectedValue(new Error('gateway refused')); + + await expect(registerUnifiedPushTransport(undefined, 'https://ntfy.sh')).resolves.toMatchObject( + { status: 'hard-failure' } + ); + + expect(logError).toHaveBeenCalledWith( + 'notification', + expect.stringContaining('UnifiedPush registration failed'), + expect.objectContaining({ hasEmbeddedGateway: true }) + ); + }); + it('treats a blank-only endpoint as a hard failure', async () => { unifiedPushApi.isPermissionGranted.mockResolvedValue(true); localStorage.setItem('unifiedpush_distributor', 'org.example.up'); diff --git a/src/app/features/settings/notifications/UnifiedPushTransport.ts b/src/app/features/settings/notifications/UnifiedPushTransport.ts index 7f6ef5af4..d09b177af 100644 --- a/src/app/features/settings/notifications/UnifiedPushTransport.ts +++ b/src/app/features/settings/notifications/UnifiedPushTransport.ts @@ -1,6 +1,9 @@ +import { createDebugLogger } from '$utils/debugLogger'; import type { PushAccount } from './pushAccount'; import { getUnifiedPushTransportApi } from './UnifiedPushTransportApiClient'; +const transportLog = createDebugLogger('unifiedpush-transport'); + export type UnifiedPushPermissionState = 'granted' | 'denied' | 'default'; export type UnifiedPushRegistrationStatus = @@ -196,6 +199,11 @@ export async function switchUnifiedPushDistributorSelection( try { return await register(); } catch (error) { + transportLog.error('notification', 'UnifiedPush distributor switch failed, reverting', { + nextDistributor, + previousDistributor, + error, + }); await saveUnifiedPushDistributor(previousDistributor); throw error; } @@ -228,6 +236,9 @@ export async function registerUnifiedPushTransport( // With a gateway configured the app is its own distributor, so an empty list is // no longer a dead end. if (!distributor && !embeddedGatewayUrl?.trim()) { + transportLog.error('notification', 'UnifiedPush registration has no usable distributor', { + installedCount: distributors.length, + }); return { status: 'missing-distributor', permissionState: 'granted', @@ -247,6 +258,10 @@ export async function registerUnifiedPushTransport( ); const endpoint = registration?.deviceToken; if (!endpoint || !endpoint.trim()) { + transportLog.error('notification', 'UnifiedPush registration returned no endpoint', { + distributor: registration?.distributor ?? selectedDistributor, + hasEmbeddedGateway: !!embeddedGatewayUrl?.trim(), + }); return { status: 'hard-failure', permissionState: 'granted', @@ -265,6 +280,11 @@ export async function registerUnifiedPushTransport( }; } catch (error) { const failureStatus = classifyUnifiedPushFailure(error); + transportLog.error('notification', `UnifiedPush registration failed (${failureStatus})`, { + distributor: selectedDistributor, + hasEmbeddedGateway: !!embeddedGatewayUrl?.trim(), + error, + }); return { status: failureStatus, permissionState, diff --git a/src/app/utils/matrix.test.ts b/src/app/utils/matrix.test.ts index b1a00ccee..d5ef6293d 100644 --- a/src/app/utils/matrix.test.ts +++ b/src/app/utils/matrix.test.ts @@ -27,8 +27,39 @@ const { rewriteAuthenticatedMediaUrl, toggleReaction, optimisticallyRedactEvent, + normalizeEncInfo, } = await import('./matrix'); +describe('normalizeEncInfo', () => { + const padded = { + v: 'v2', + key: { + alg: 'A256CTR', + key_ops: ['encrypt', 'decrypt'], + kty: 'oct', + k: 'AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8=', + ext: true, + }, + iv: 'G+qzsN3Y9FgAAAAAAAAAAA==', + hashes: { sha256: 'ICEiIyQlJicoKSorLC0uLzAxMjM0NTY3ODk6Ozw9Pj8=' }, + }; + + it('strips padding from iv, key and hashes', () => { + expect(normalizeEncInfo(padded)).toEqual({ + ...padded, + key: { ...padded.key, k: 'AAECAwQFBgcICQoLDA0ODxAREhMUFRYXGBkaGxwdHh8' }, + iv: 'G+qzsN3Y9FgAAAAAAAAAAA', + hashes: { sha256: 'ICEiIyQlJicoKSorLC0uLzAxMjM0NTY3ODk6Ozw9Pj8' }, + }); + }); + + it('leaves unpadded input untouched and never mutates the event content', () => { + const unpadded = normalizeEncInfo(padded); + expect(normalizeEncInfo(unpadded)).toEqual(unpadded); + expect(padded.iv).toBe('G+qzsN3Y9FgAAAAAAAAAAA=='); + }); +}); + describe('rewriteAuthenticatedMediaUrl', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/src/app/utils/matrix.ts b/src/app/utils/matrix.ts index 4b445c148..964986749 100644 --- a/src/app/utils/matrix.ts +++ b/src/app/utils/matrix.ts @@ -149,12 +149,23 @@ export const encryptFile = async ( }; }; +const stripBase64Padding = (value: string): string => value.replace(/=+$/, ''); + +export const normalizeEncInfo = (encInfo: EncryptedAttachmentInfo): EncryptedAttachmentInfo => ({ + ...encInfo, + iv: stripBase64Padding(encInfo.iv), + key: { ...encInfo.key, k: stripBase64Padding(encInfo.key.k) }, + hashes: Object.fromEntries( + Object.entries(encInfo.hashes).map(([name, hash]) => [name, stripBase64Padding(hash)]) + ), +}); + export const decryptFile = async ( dataBuffer: ArrayBuffer, type: string, encInfo: EncryptedAttachmentInfo ): Promise => { - const dataArray = await decryptAttachment(dataBuffer, encInfo); + const dataArray = await decryptAttachment(dataBuffer, normalizeEncInfo(encInfo)); const blob = new Blob([dataArray], { type }); return blob; };