From 313bf856ffb58a2c136531ec3953e3f2ae692a0f Mon Sep 17 00:00:00 2001 From: Narasimha-sc <166327228+Narasimha-sc@users.noreply.github.com> Date: Sat, 17 Oct 2026 18:45:45 +0000 Subject: [PATCH] plans: build Windows toasts through the XML DOM only, no escaping --- .../2026-10-17-windows-notification-click.md | 441 ++++++++++-------- 1 file changed, 237 insertions(+), 204 deletions(-) diff --git a/plans/2026-10-17-windows-notification-click.md b/plans/2026-10-17-windows-notification-click.md index eb17423740..dea37ba699 100644 --- a/plans/2026-10-17-windows-notification-click.md +++ b/plans/2026-10-17-windows-notification-click.md @@ -1,7 +1,13 @@ # Windows: clicking a notification opens the chat -Status: draft rev 10 (after review round 7). Toast history model decided 2026-10-17: Android-parity clearing plus a 1-hour expiry. All decisions taken. Implementation is gated by G1. Base: `origin/master` a932c8298. Paths are relative to `apps/multiplatform` unless they start with `src/` (the Haskell core). -Related plans: [sound](2026-10-17-desktop-notification-sound.md), [turn off](2026-10-17-desktop-notifications-turn-off.md). Research notes: `~/ntf-research/windows-findings.md`, `twoslices-findings.md`. +Status: draft rev 13 (after the rev 12 review). Redesigned 2026-10-17 so that no injection is possible by construction and the diff is smaller: +- Toasts are built only through Windows' XML DOM. Our code never builds, escapes or parses XML. +- The two-slices fallback uses only its plain-text AWT backend, constructed directly. +- Remote-host clicks open the clicked chat on its host. + +All decisions are taken. Implementation is gated by G1. Base: `origin/master` a932c8298. Paths are relative to `apps/multiplatform` unless they start with `src/` (the Haskell core). + +Related plans: [sound](2026-10-17-desktop-notification-sound.md), [turn off](2026-10-17-desktop-notifications-turn-off.md). ## Problem @@ -10,270 +16,297 @@ On Windows, clicking a SimpleX notification does nothing: - The window isn't raised. - From Action Center, nothing happens at all. -Related: #4201 (SimpleX is not a stable app in Windows notification settings; its sound switch resets), #3301, closed PR #7182. No issue states the click bug directly; the reports come from users. +Related: #4201 (SimpleX is not a stable app in Windows notification settings; its sound switch resets), #3301, closed PR #7182. ## Cause -- INFERENCE [`twoslices-findings.md:48`]: on Windows, two-slices selects `AWTToaster` (GNTP only if Snarl or Growl listens on port 23053). -- FACT [`AWTToaster.java:103, 173-175`, md5-identical in 0.9.1–0.9.7]: +- On Windows, two-slices selects `AWTToaster`, or `GNTPToaster` if Snarl or Growl listens on port 23053 at the host's own address (FACT [two-slices `ToasterFactory.java:50-76`, the service order, the app's classpath]). +- FACT [two-slices `AWTToaster.java:103, 173-175`, identical in 0.9.1–0.9.7]: - It registers itself as the `TrayIcon` ActionListener, but `actionPerformed` is empty. - `builder.defaultAction()` and `builder.actions()` are never read. - - So the click and the Accept/Reject buttons can never reach the app. Upgrading the library doesn't help. + - So clicks never reach the app, and upgrading the library doesn't help. - FACT [`AWTToaster.java:92-128`]: it adds a second tray icon for toasts and removes it 10 s after the last toast. INFERENCE: after that, an Action Center click has no target. -- FACT [`NtfManager.kt:70-83`]: `openChatAction` never raises the window. It is effectively unused on desktop today, since no toaster in use invokes it, and it hard-codes the local host (`changeActiveUser(null, …)` `:76`, `openChat(rhId = null)` `:81`, TODO at `:74`). +- FACT [`NtfManager.kt:70-83`]: `openChatAction` never raises the window. It also hard-codes the local host: `changeActiveUser(null, …)` at `:76`, `openChat(rhId = null)` at `:81`, and a TODO at `:74`. It is effectively unused on desktop today. +- FACT [two-slices `GNTPToaster.java:77-78`]: the GNTP backend writes `Notification-Title: %s\r\n` and `Notification-Text: %s\r\n` without escaping. Today, with Snarl or Growl listening on the host's own address (port 23053), a CR/LF in peer text injects GNTP headers. §2 removes this on Windows. +- FACT [two-slices `ToasterFactory.java:56-74`]: a preferred toaster class is only a preference. The factory still constructs every service in order, and when the preferred one fails, it falls back to the first that was constructed, which can be GNTP. ## Existing structure to reuse - FACT [`AppLinkScheme.desktop.kt:67-71, 97-106, 168+`]: - On each start, a background thread registers `simplexchat:` and `simplex:` per user under `HKCU\Software\Classes` for any jpackage launcher on Windows, with the command `"" "%1"`. - A scheme held by another program is left alone. `registered` reflects `simplexchat:` (`:180`). - - The app already uses `chat.simplex.app` as its Windows capabilities name (`:26`) and the display name `APP_DISPLAY_NAME = "SimpleX Chat"` (`:43`). + - The app already uses `chat.simplex.app` (`:26`) and `APP_DISPLAY_NAME = "SimpleX Chat"` (`:43`). - FACT [`SingleInstance.kt:49-66, 139-157, 205-214, 227-245`; `Main.kt:28-32`]: - - A second launch writes its link to `simplex.show`, allows the running instance the foreground (`AllowSetForegroundWindow`), and exits. - - The running instance takes the signal on the Swing thread and calls `openDesktopLink(link)` (`AppLinks.kt:19-22`), which stores `appOpenUrl` and calls `showWindow()` (`DesktopApp.kt:259-273`). - - The watcher starts before `initApp` (`Main.kt:32` vs `:41`). On a cold start, `Main.kt:30` stores the link from args. -- FACT [`AppLinks.kt:12-13`, `ChatListView.kt:797-798`]: an accepted link is a case-insensitive `simplexchat:` prefix (or a connection link), at most 8192 bytes. Signal files are re-checked in `takeSignal`. + - A second launch writes its link to `simplex.show`, allows the running instance the foreground, and exits. + - The running instance takes the signal on the Swing thread and calls `openDesktopLink(link)` (`AppLinks.kt:19-22`), which raises the window (`DesktopApp.kt:259-273`). + - On a cold start, `Main.kt:30` stores the link from args. +- FACT [`AppLinks.kt:12-13`, `ChatListView.kt:797-798`]: an accepted link has a case-insensitive `simplexchat:` prefix (or is a connection link) and is at most 8192 bytes. Signal files are re-checked in `takeSignal`. - FACT [`App.kt:240-249`, `ChatListView.kt:775-789, 804-814`]: - - `appOpenUrl` is processed only when authorized, onboarding is complete and chat is running. - - `simplexchat:` links go to `openAppLink` unless a badge redemption is in flight (`isBadgeLinkIssuing()`), in which case the link is dropped. - - `openAppLink` (commonMain) shows "not supported" for unknown paths. -- FACT [`SimpleXAPI.kt:2937`]: `rhId` of every event comes from the desktop's own core. -- FACT [`src/Simplex/Chat/Library/Commands.hs:561, 574, 4456`]: hiding a profile sets `showNtfs = False`, unhiding sets it back, and muting is refused while hidden. -- FACT [`SimpleXAPI.kt:787-789`, `HiddenProfileView.kt:38-39`, `UserProfilesView.kt:371`, `LocalAuthView.kt:73`, `DatabaseView.kt:598`]: these are the clearing calls, the same set Android has: - - `cancelNotificationsForUser` when leaving a hidden active profile, when hiding a non-active profile, and on user deletion; - - `cancelAllNotifications` on self-destruct and database deletion. -- FACT [`AppLock.kt:23-24, 271-284`; `DesktopApp.kt:184-205`; `PrivacySettings.kt:487`]: - - `userAuthorized` / `enteredBackground` are the lock state. - - With lock delay 0, re-locking happens only in the focus handler. - - `clearOverlays` closes the passcode prompt (`App.kt:111-115`), and ChatView stays composed under the lock screen (`App.kt:157-172`), where it marks visible items read (`ChatView.kt:3090-3094`). -- FACT [`desktop/build.gradle.kts:70-78`]: the MSI makes a desktop shortcut only (no `menu`) and sets no AppUserModelID. + - Ordinary links are processed only when the app is authorized, onboarding is complete and chat is running. + - Links are dropped during a badge redemption. + - Unknown `simplexchat:` paths show "not supported". +- FACT [`SimpleXAPI.kt:2937`]: every event's `rhId` comes from the desktop's own core. +- FACT [`SimpleXAPI.kt:787-789`, `HiddenProfileView.kt:38-39`, `UserProfilesView.kt:371`, `LocalAuthView.kt:73`, `DatabaseView.kt:598`]: these are the clearing calls, the same set Android has. + - `cancelNotificationsForUser`: leaving a hidden active profile, hiding a non-active profile, user deletion. + - `cancelAllNotifications`: self-destruct, database deletion. +- FACT [`AppLock.kt:23-24, 271-284`; `DesktopApp.kt:184-205`; `PrivacySettings.kt:487`; `App.kt:111-115, 157-172`; `ChatView.kt:3090-3094`]: + - The lock state is `userAuthorized` / `enteredBackground`. + - A focused window never re-locks. With lock delay 0, re-locking happens only in the focus handler. + - `clearOverlays` closes the passcode prompt. + - ChatView stays composed under the lock screen and marks visible items read. +- FACT [`IncomingCallAlertView.kt:31`]: the in-app call ring always plays on desktop. +- FACT [`desktop/build.gradle.kts:70-78`, `shortcut = true` at `:75`]: the MSI makes a desktop shortcut only. It has no Start-menu group and no AppUserModelID. ## Ideal final result -Clicking a SimpleX toast from the current run, on screen or in Action Center, behaves like opening a `simplexchat:` link: Windows starts SimpleX with the link, and the existing single-instance path raises the running window, which then opens the chat. A toast from an earlier run starts or raises the app. +Clicking a SimpleX toast from the current run, on screen or in Action Center, behaves like opening a `simplexchat:` link. Windows starts SimpleX with the link. The existing single-instance path raises the running window, which then opens the chat on the host it came from: this computer or a connected mobile. -Microsoft documents a "stub" CLSID with protocol-activation toasts for unpackaged apps, in the form where the AUMID and CLSID sit on a Start-menu shortcut. The registry-only form (`HKCU\Software\Classes\AppUserModelId\` with `CustomActivator`, as the Windows App SDK and CommunityToolkit write it, but without their `LocalServer32`) is **INFERENCE**. Hence G1. +Peer-controlled text reaches Windows only as DOM text-node values. No code of ours turns it into markup or protocol text. + +The registry-only AUMID form (`HKCU\Software\Classes\AppUserModelId\` with a stub `CustomActivator`) is INFERENCE; Microsoft documents the stub CLSID on a Start-menu shortcut. Hence G1. ## Options -1. **WinRT toasts with protocol activation** (chosen, D1). The click works from the popup and from Action Center, and SimpleX becomes a stable app in Windows notification settings (#4201, INFERENCE). Cost: about 300–400 lines of Windows-only JNA/COM code, verifiable only on Windows. Each click starts a second JVM to forward the link (about 1–3 s, INFERENCE). -2. **Code-only tray-icon stopgap.** It works only while the balloon is on screen and only for the last notification. Rejected. -3. **A third-party WinRT library.** A new native dependency in trusted code. Rejected. -4. **PowerShell per toast.** Slow, blockable, and message content would reach command lines and logs. Rejected. +1. **WinRT toasts built through the XML DOM, with protocol activation** (chosen). + - Injection-free by construction. The click works from the popup and from Action Center, and #4201 is addressed (INFERENCE). + - About 250 lines of Windows-only JNA/COM code in one new file, plus small edits in three desktop files. In common code: the `rhId` parameter and the `openChatAction` routing. +2. **AWT balloon with an in-app click listener**, reviving the dead `windowsDisplayNotification`. + - A plain-text API in about 25 lines. + - No Action Center clicks and no fix for #4201. Not chosen. +3. **Hand-built XML with an escaping function** (rev 10). It relies on our escaping being right. Replaced by option 1. +4. **A third-party WinRT library, or PowerShell per toast.** Rejected: a new native dependency, or content exposed on the command line. ## Design (option 1) -Narrative: on Windows, each notification becomes a Windows toast whose click opens `simplexchat:/notification/`. When the desktop app receives such a link, it raises the window and, if the app is unlocked, opens the chat the token stands for, on the host and in the profile it came from. +Narrative: on Windows, each notification becomes a Windows toast, built from Windows' own template with peer text inserted as text nodes. Its click opens `simplexchat:/notification/`. When the desktop app receives that link, it raises the window. If the app is unlocked and ready, it opens the chat the token stands for, on that chat's host and in its profile. -Commits, in this order: -- (a) add the `rhId` parameter and pass it through: structure only, no behaviour change; -- (b) `openChatAction` routes by `rhId`; -- (c) the AUMID registration; -- (d) `WindowsToast`, tokens and the link handler; -- (e) docs. +Commits: +- (a) add the `rhId` parameter and pass it through (structure only, no behaviour change); +- (b) `openChatAction` routes by `rhId` and gains the lock guard; +- (c) on Windows, two-slices uses only a directly constructed AWT toaster; +- (d) the AUMID registration; +- (e) `WindowsToast` and tokens; this also removes the sound plan's Windows toggle hiding, if V1 added it; +- (f) the link handler; +- (g) docs. -1. **AUMID registration** in `AppLinkScheme.desktop.kt`, on the same thread and through the same `WindowsShell`/`RegistryValue` abstraction as `registerWindowsSchemes`. It runs only after `simplexchat:` registered successfully, because without the scheme a click can't come back. +1. **AUMID registration**, in `AppLinkScheme.desktop.kt`, on the same thread and through the same `WindowsShell`/`RegistryValue` abstraction as `registerWindowsSchemes`. It runs only after `simplexchat:` registered successfully. - Key: `Software\Classes\AppUserModelId\chat.simplex.app`. - `DisplayName = APP_DISPLAY_NAME`. - - `IconUri` = a PNG written to the data dir when missing (not `tmpDir`, which is wiped at start, `Main.kt:43`). - - `CustomActivator = {fixed GUID}`. No `LocalServer32` (stub). - - The outcome (ready or failed) is published to `WindowsToast`. -2. **`WindowsToast`** (new `platform/WindowsToast.desktop.kt`, Windows only). - - **Thread and commands.** - - One daemon thread takes commands from one **unbounded** `LinkedBlockingDeque`. - - Clear commands and the registration outcome go to the front (`offerFirst`); toast commands go to the back. - - A separate `AtomicInteger` counts toasts waiting, including the held list below. Offering a toast, in order: - 1. Reserve a slot: `incrementAndGet() <= 32`. Otherwise `decrementAndGet()` and drop the toast without touching the token map. - 2. Put its token in the map. - 3. `offerLast`. - - Accepted toasts are processed in order and at most 32 wait, so a waiting toast's token is never evicted by the 256-entry map. - - The count is decremented in a `finally` around routing each toast, whatever the exit: dropped, `DisabledFor*`, shown, or fallback. No exit leaks a slot. - - Clears are never dropped. - - The thread blocks on the deque, so a clear runs as soon as it is queued, even with no toast after it. Callers never block. - - **Registration wait**: one deadline, 10 s after the thread starts. - - Until the outcome arrives, toasts taken from the deque are moved to a local held list (they still count toward the 32), and the thread waits with `pollFirst(deadline - now)`, so clears keep running. - - On "ready", held toasts are shown in order. On "failed" or at the deadline, they go to the fallback, and the thread returns to a blocking `takeFirst`. - - The deadline is not an outcome. Toasts arriving after it, while no outcome has arrived, go straight to the fallback. A late "ready" switches every later toast to WinRT, so a slow first-run registration costs only the toasts before it. "Failed" is final. - - The first outcome wins. The registration code publishes "ready" after writing the key. Its `finally` publishes "failed" only if nothing was published yet, which covers `Unpackaged`, a scheme held by another program, and exceptions. - - **Thread start**: `RoInitialize(RO_INIT_MULTITHREADED)`. Accept `S_OK` and `S_FALSE`; `RPC_E_CHANGED_MODE` or any other error disables the toaster, and every toast goes to the fallback. - - **Once**, with each interface's IID and the vtable slot of each call written in the code next to the call: - - `RoGetActivationFactory("Windows.UI.Notifications.ToastNotificationManager", IToastNotificationManagerStatics {50AC103F-D235-4598-BBEF-98FE4D1A3AD4})` → `CreateToastNotifierWithId(aumid)` (slot 7) → `IToastNotifier`. - - QI `IToastNotificationManagerStatics2` → `get_History` → `IToastNotificationHistory`: `RemoveGroupWithId(group, aumid)` and `ClearWithId(aumid)`. The `…WithId` forms are needed without package identity. - - `RoGetActivationFactory("Windows.UI.Notifications.ToastNotification", IToastNotificationFactory)`. - - `RoGetActivationFactory("Windows.Foundation.PropertyValue", IPropertyValueStatics)`. - - **Before routing** each toast, whether to WinRT or to the fallback (held toasts, a failed registration, a disabled toaster): if it has a token and that token is no longer in the map, drop it. Its user, or everything, was cleared after it was queued. - - **Per WinRT toast**: - 1. Read `IToastNotifier.Setting` (slot 8). + - `IconUri`: only if G1 shows the toast or Settings entry needs it. If so, it is a PNG written to the data dir when missing; otherwise it is left out with its file. + - `CustomActivator = {fixed GUID}`. No `LocalServer32`, so this is a stub. + - When done, it sets `@Volatile windowsToastsReady = true`. Until then, and if registration never succeeds, toasts use the fallback (a known limit). +2. **Fallback: AWT only, constructed directly.** On Windows, `initApp` installs `ToasterFactory.setFactory(object : ToasterFactory() { override fun toaster() = awtToaster })` (`ToasterFactory.java:107`). + - `awtToaster` is a lazily constructed `AWTToaster(ToasterFactory.getSettings())`. No other backend is ever constructed, so GNTP never opens a socket or sends its REGISTER. + - If `AWTToaster` can't be constructed (no system tray), `toaster()` throws. The existing catch logs it, and the fallback toast is not shown. It never reaches another backend. + - Behaviour change: Snarl/Growl users on Windows get the balloon instead of GNTP. +3. **`WindowsToast`** (new `platform/WindowsToast.desktop.kt`, Windows only). + - **Thread.** + - One single-thread executor, started on Windows at `initApp` whatever the registration outcome, so clears always run. + - Toasts and clears are submitted in order (FIFO). Only toast tasks reserve a slot; clear tasks don't. + - An `AtomicInteger` limits waiting toasts to 32. Offering a toast: + 1. Reserve a slot: `incrementAndGet() <= 32`; otherwise decrement and drop. + 2. Put its token in the map. + 3. Submit. + - Each toast task decrements the count in `finally`. Every task catches `Throwable`. + - **Wedge check**: each task sets `@Volatile runningSince` when it starts and clears it in `finally`. + - At offer time, `runningSince != 0 && now - runningSince > 10 s` means the toaster is wedged (for example the notification service is stuck). The new toast then goes to the fallback instead of the queue. + - This is a live condition, not a latch. Once the stuck task returns, later toasts use WinRT again. + - Toast tasks that waited longer than 30 s are dropped when they start, so a wedge doesn't end in a burst of stale toasts and sounds. + - Clears queued behind a wedged task run when it returns (known limit). + - **Thread start**: `RoInitialize(RO_INIT_MULTITHREADED)`. `S_OK` and `S_FALSE` continue. Anything else disables the toaster: toasts go to the fallback, and clears are no-ops. + - **COM helper**: one function calls vtable slot *n* of a COM pointer and checks the HRESULT. + - Every interface pointer and HSTRING is released in the `finally` of the scope that owns it: `Release`, `WindowsDeleteString`. That is the per-toast scope for the document, nodes, toast and boxed `DateTime`, and the process for the factories and the notifier. + - Logging: `WindowsToast` logs only the HRESULT, the slot and the step, never a title or body. + - HSTRINGs are made with `WindowsCreateString` from a `char[]` with an explicit length, so embedded NULs are kept as data. + - Each IID, QI and slot is written next to its call. + - **Once**: activation factories for: + - `ToastNotificationManager`: `IToastNotificationManagerStatics` for `CreateToastNotifierWithId` and `GetTemplateContent`; QI to `…Statics2` for `get_History`; + - `ToastNotification`: `IToastNotificationFactory`; + - `Windows.Foundation.PropertyValue`: `IPropertyValueStatics`. + - **Per toast**: + 1. If it has a token that is no longer in the map, drop it: it was cleared after it was queued. + 2. Read `IToastNotifier.Setting`. - `Enabled`: continue. - - `DisabledForApplication` / `DisabledForUser` / `DisabledByGroupPolicy`: drop the toast, and no sound. The user turned SimpleX off in Windows. - - `DisabledByManifest` or an error: fallback. - 2. `RoActivateInstance("Windows.Data.Xml.Dom.XmlDocument")` → QI `IXmlDocumentIO` → `LoadXml`. - 3. QI `IXmlDocument` {F7F3A506-1E87-42D6-BCFB-B8C809FA5494} → `IToastNotificationFactory::CreateToastNotification` (slot 6). - 4. On the new toast: - - `put_ExpirationTime` (`IToastNotification`, slot 7) = now + 1 h. The value is `IPropertyValueStatics::CreateDateTime` → QI `IReference` {5541D8A7-497C-5AA4-86FC-7713ADBF2A2C}, with `DateTime.UniversalTime` in 100 ns ticks since 1601-01-01 UTC. - - `put_Group` (`IToastNotification2`, slot 8) = `"-"`. - - `put_ExpiresOnReboot(true)` through QI `IToastNotification6` {43EBFE53-89AE-5C1E-A279-3AECFE9B6F54}. That interface exists only on Windows 10 1903+; `E_NOINTERFACE` just skips the step, since the 1 h expiry still applies. - - No Tag. - 5. `IToastNotifier::Show` (slot 6), then run the toast's `onShown`. - - A failure in steps 2–5 sends that toast to the fallback. Only factory or apartment failures disable the toaster for the session. - - **Fallback** = `displayViaTwoSlices(…)`: today's two-slices code, renamed in place, which never routes back to `WindowsToast`. `WindowsToast` passes it `onShown.takeIf { BALLOON_SILENT }`, where `BALLOON_SILENT` is a constant set from V1 (sound plan). The balloon's own sound and the app's sound never both play. - - **XML**, one function, in this order: - 1. Truncate the title to 128 and the body to 512 code points. - 2. Remove characters XML 1.0 forbids (U+0000–0008, 000B, 000C, 000E–001F, U+FFFE/FFFF, lone surrogates). - 3. Escape `& < > " '`. - - All values go through it: title, body, the `launch` attribute, and the image URI (`file.toPath().toUri()`, i.e. `file:///C:/…`). - - Elements: `ToastGeneric`, two ``, optional `appLogoOverride` from the existing `prepareIconPath`, and `activationType="protocol" launch="…"` on ``. - - **Audio** is decided once, at the `displayNotification` call site, and passed in the request as `ToastAudio.Silent` or `ToastAudio.Default`. Every toast from `displayNotification` (messages, contact requests, contact-connected, missed calls) gets `Silent` (``, `&`, `{title}`, NUL and control characters, lone surrogates, bidi overrides, and 100 KB strings: either shown literally (cut), or the constant-text retry is shown. Never an extra element, a changed link, an image or a balloon. +- If G1 fails, the fallback is a Start-menu shortcut carrying `System.AppUserModel.ID` and `ToastActivatorCLSID`. That becomes a new decision for you. ## Threat model -- **XML injection.** The display name, group name and message text are peer-controlled. - - Unescaped, they could add ``, change `launch`, add a remote `` (an IP leak) or `