From e64c6c7bc30f2d37ab5a8a1f7fdf8c5724301629 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 03:08:30 +0100 Subject: [PATCH 01/10] fix: keep data on a mistyped offline policy and use the shortest lock time for an unreadable one Forced OfflineEnabled values that make no sense read as forced off, which wipes kept mail. They now stop new mail being kept and delete nothing. An unreadable AppLockTimeoutMinutes now means the shortest time, so the existing out-of-range test changes from no value to 1: no value let the user's own choice win. - recommended AutoUpdate, AppLockRequired, TelemetryAllowed and CrashReportsAllowed are gone: nothing read them, or one acted as forced - auth lists and certificate rules can't name a whole top-level domain - ALL_WIRED is gone, as every key has a consumer --- lib/app-lock/__tests__/controller.test.ts | 3 + lib/app-lock/policy.ts | 8 +- lib/desktop/__tests__/policy-client.test.tsx | 7 ++ lib/desktop/__tests__/policy-schema.test.ts | 29 +++++-- lib/desktop/policy-schema.ts | 87 +++++++++++++++++-- lib/desktop/policy.ts | 5 +- lib/device-policy.ts | 6 +- .../__tests__/effective-scope-wiring.test.ts | 31 +++++++ lib/offline/offline-mode.ts | 3 +- 9 files changed, 161 insertions(+), 18 deletions(-) diff --git a/lib/app-lock/__tests__/controller.test.ts b/lib/app-lock/__tests__/controller.test.ts index ad3bd3121..1c2d41e91 100644 --- a/lib/app-lock/__tests__/controller.test.ts +++ b/lib/app-lock/__tests__/controller.test.ts @@ -770,6 +770,9 @@ describe('policy', () => { it('a forced timeout wins over the user\'s choice, a recommended one only over no choice', () => { withPolicy({ value: false, locked: false }, { value: 3, locked: true }); expect(effectiveTimeout(60)).toEqual({ minutes: 3, locked: true }); + // A value nobody could read: the user's choice was never what the admin meant. + (window.bulwarkDesktop as unknown as { policy: { get(): unknown } }).policy.get = () => ({ AppLockTimeoutMinutes: { value: null, locked: true, source: 'forced-machine', invalid: 'out_of_range' } }); + expect(effectiveTimeout(60)).toEqual({ minutes: 1, locked: true }); withPolicy({ value: false, locked: false }, { value: 3, locked: false }); expect(effectiveTimeout(60)).toEqual({ minutes: 60, locked: false }); expect(effectiveTimeout(undefined)).toEqual({ minutes: 3, locked: false }); diff --git a/lib/app-lock/policy.ts b/lib/app-lock/policy.ts index 718e2b27a..a12207dfa 100644 --- a/lib/app-lock/policy.ts +++ b/lib/app-lock/policy.ts @@ -5,8 +5,12 @@ */ import { getDesktopBridge } from '@/lib/desktop/bridge'; +import { POLICY_SCHEMA } from '@/lib/desktop/policy-schema'; import { DEFAULT_TIMEOUT_MINUTES } from './state'; +const timeoutDefinition = POLICY_SCHEMA.AppLockTimeoutMinutes; +const SHORTEST_MINUTES = timeoutDefinition.type === 'integer' ? timeoutDefinition.min : 1; + interface PolicyEntry { value: T; locked: boolean; @@ -51,7 +55,9 @@ export interface EffectiveTimeout { export function effectiveTimeout(choice: number | undefined): EffectiveTimeout { const entry = read().AppLockTimeoutMinutes; const value = typeof entry?.value === 'number' && entry.value > 0 ? entry.value : null; - if (entry?.locked && value !== null) return { minutes: value, locked: true }; + // Forced to something nobody can read, by a shell that hands that on as nothing: the + // tightest there is, since the user's choice was never meant to apply. + if (entry?.locked) return { minutes: value ?? SHORTEST_MINUTES, locked: true }; return { minutes: choice ?? value ?? DEFAULT_TIMEOUT_MINUTES, locked: false }; } diff --git a/lib/desktop/__tests__/policy-client.test.tsx b/lib/desktop/__tests__/policy-client.test.tsx index 37670f0ac..dcc1ce17d 100644 --- a/lib/desktop/__tests__/policy-client.test.tsx +++ b/lib/desktop/__tests__/policy-client.test.tsx @@ -116,6 +116,13 @@ describe('in the app', () => { expect(devicesForAccount('some-account').noOffline).toBe(false); }, 60_000); + it('a forced value that makes no sense keeps offline off without forbidding it, which would wipe', async () => { + setBridgePolicy(snapshotOf({ OfflineEnabled: 'yes' })); + expect(desktopOfflinePolicy()).toEqual({ forced: false, fallback: null, invalid: true }); + const { devicesForAccount } = await import('@/lib/device-policy'); + expect(devicesForAccount('some-account').noOffline).toBe(false); + }, 60_000); + it('the hook settles on a bridge that copies what it returns, as contextBridge does', async () => { setBridgePolicy(snapshotOf({ PluginsMode: 'off' })); window.bulwarkDesktop!.policy!.get = () => JSON.parse(JSON.stringify(current)); diff --git a/lib/desktop/__tests__/policy-schema.test.ts b/lib/desktop/__tests__/policy-schema.test.ts index 3411ad9ea..3ad319aed 100644 --- a/lib/desktop/__tests__/policy-schema.test.ts +++ b/lib/desktop/__tests__/policy-schema.test.ts @@ -5,6 +5,7 @@ import { DEFAULT_POLICY_SNAPSHOT, EMPTY_POLICY_LAYERS, POLICY_KEYS, + policyProblems, POLICY_CONSUMERS, POLICY_MAX_LIST_ITEMS, POLICY_SCHEMA, @@ -133,9 +134,9 @@ describe('hostile values fail closed', () => { expect(entry.raw!.length).toBeLessThanOrEqual(201); }); - it('an out of range timeout falls to no value, not to a huge one', () => { + it('an out of range timeout falls to the shortest, not to a huge one or the user\'s own', () => { const entry = resolvePolicies(layers({ forcedMachine: { AppLockTimeoutMinutes: 999999 } })).AppLockTimeoutMinutes; - expect(entry).toMatchObject({ value: null, invalid: 'out_of_range' }); + expect(entry).toMatchObject({ value: 1, invalid: 'out_of_range' }); }); }); @@ -322,6 +323,26 @@ describe('KeystoreAllowedOrigins', () => { }); }); +describe('what a recommendation can do', () => { + it.each(['AppLockRequired', 'TelemetryAllowed', 'CrashReportsAllowed', 'AutoUpdate'] as const)('%s: nothing, and the templates do not offer one', (key) => { + expect(POLICY_SCHEMA[key].forcedOnly).toBe(true); + expect(resolvePolicies({ ...EMPTY_POLICY_LAYERS, recommendedMachine: { [key]: 0 } })[key]).toMatchObject({ source: 'default', locked: false }); + expect(resolvePolicies({ ...EMPTY_POLICY_LAYERS, forcedMachine: { [key]: 0 } })[key]).toMatchObject({ value: false, locked: true }); + }); +}); + +describe('values that fail open', () => { + it('a lock timeout nobody can read is the shortest there is', () => { + expect(resolvePolicies({ ...EMPTY_POLICY_LAYERS, forcedMachine: { AppLockTimeoutMinutes: '99999' } }).AppLockTimeoutMinutes).toMatchObject({ value: 1, locked: true, invalid: 'out_of_range' }); + }); + + it('a wipe code too long to use is named as invalid', () => { + const snapshot = resolvePolicies({ ...EMPTY_POLICY_LAYERS, forcedMachine: { WipeOfflineData: 'w'.repeat(200) } }); + expect(snapshot.WipeOfflineData).toMatchObject({ value: null, invalid: 'too_long' }); + expect(policyProblems(snapshot).map((problem) => problem.key)).toEqual(['WipeOfflineData']); + }); +}); + describe('every key has someone acting on it', () => { it('names a consumer or who owes one', () => { for (const key of POLICY_KEYS) { @@ -338,9 +359,7 @@ describe('every key has someone acting on it', () => { } }); - // The coordinator flips this when step 7 is done: then no key may be waiting. - const ALL_WIRED = false; - it.runIf(ALL_WIRED)('nobody is waiting any more', () => { + it('nobody is waiting any more', () => { expect(POLICY_KEYS.filter((key) => 'pendingConsumer' in POLICY_CONSUMERS[key])).toEqual([]); }); }); diff --git a/lib/desktop/policy-schema.ts b/lib/desktop/policy-schema.ts index 749a8361d..781726d39 100644 --- a/lib/desktop/policy-schema.ts +++ b/lib/desktop/policy-schema.ts @@ -86,9 +86,10 @@ interface PolicyBase { title: PolicyText; description: PolicyText; /** - * Only a forced value counts. Anything that decides what's allowed: it has no user - * choice to sit under, so a "recommended" one would act as forced without being - * shown as such, and only a forced value may loosen a forced one. + * Only a forced value counts. Anything that decides what's allowed, or a yes or no with + * nothing for a recommendation to set: it has no user choice to sit under, so a + * "recommended" one would act as forced without being shown as such, or do nothing, and + * only a forced value may loosen a forced one. */ forcedOnly?: boolean; } @@ -208,6 +209,7 @@ export const POLICY_SCHEMA: Record = { }, AutoUpdate: { category: 'updates', + forcedOnly: true, type: 'boolean', default: true, onInvalid: false, @@ -264,6 +266,7 @@ export const POLICY_SCHEMA: Record = { }, AppLockRequired: { category: 'security', + forcedOnly: true, type: 'boolean', default: false, onInvalid: true, @@ -290,6 +293,7 @@ export const POLICY_SCHEMA: Record = { }, TelemetryAllowed: { category: 'privacy', + forcedOnly: true, type: 'boolean', default: true, onInvalid: false, @@ -302,6 +306,7 @@ export const POLICY_SCHEMA: Record = { }, CrashReportsAllowed: { category: 'privacy', + forcedOnly: true, type: 'boolean', default: true, onInvalid: false, @@ -395,8 +400,8 @@ export const POLICY_SCHEMA: Record = { label: { en: 'Wipe code', de: 'Löschcode' }, title: { en: 'Delete mail kept on the device', de: 'Auf dem Gerät behaltene E-Mails löschen' }, description: { - en: 'Set this to a new value, such as today\'s date, to delete everything the app keeps on the device for every server and account: mail, keys, and sign-ins, so people sign in again. Each value deletes once: the app remembers the ones it has acted on, so it stays safe to leave a value in place. Use a new value to delete again.', - de: 'Setzen Sie dies auf einen neuen Wert, etwa das heutige Datum, um alles zu löschen, was die App für alle Server und Konten auf dem Gerät behält: E-Mails, Schlüssel und Anmeldungen, Benutzer melden sich also neu an. Jeder Wert löscht einmal: Die App merkt sich, welche sie schon ausgeführt hat, ein Wert kann also stehen bleiben. Verwenden Sie einen neuen Wert, um erneut zu löschen.', + en: 'Set this to a new value, such as today\'s date, to delete everything the app keeps on the device for every server and account: mail, keys, and sign-ins, so people sign in again. Each value deletes once: the app remembers the ones it has acted on, so it stays safe to leave a value in place. Use a new value to delete again. That memory is in the user\'s own profile, so someone who edits it can skip a wipe they know is coming: this cleans up a device, it doesn\'t take data from the person using it.', + de: 'Setzen Sie dies auf einen neuen Wert, etwa das heutige Datum, um alles zu löschen, was die App für alle Server und Konten auf dem Gerät behält: E-Mails, Schlüssel und Anmeldungen, Benutzer melden sich also neu an. Jeder Wert löscht einmal: Die App merkt sich, welche sie schon ausgeführt hat, ein Wert kann also stehen bleiben. Verwenden Sie einen neuen Wert, um erneut zu löschen. Diese Liste liegt im Profil des Benutzers. Wer sie bearbeitet, kann also eine Löschung umgehen, von der er weiß: Dies räumt ein Gerät auf, es nimmt der Person am Gerät keine Daten weg.', }, }, }; @@ -560,6 +565,19 @@ function parseShortcut(raw: unknown): Parsed { const HOST_LIST = /^[A-Za-z0-9*.,:_[\]-]+$/; +const COMMON_TOP_LEVEL = /^(?:[a-z]{2}|com|net|org|edu|gov|mil|int|info|biz|name|pro|mobi|app|dev|cloud|online|site|xyz|top|shop|store|tech|club|live|blog|page|arpa)$/; + +/** + * Chromium reads a plain name in these lists as "ends with", so `com` means every .com + * address, and `.com` is `*.com`. `intranet` or `sso` stay fine: that's how a server with + * no domain is named. + */ +function coversTopLevelDomain(host: string): boolean { + const name = host.replace(/:\d+$/, '').replace(/\.+$/, '').toLowerCase(); + if (name.startsWith('.')) return !/^\.+[^.]+\.[^.]/.test(name); + return COMMON_TOP_LEVEL.test(name); +} + /** What Chromium takes for its auth lists: names and wildcards, commas between them. */ function parseHostList(raw: unknown): Parsed { if (typeof raw !== 'string') return fail('not_text'); @@ -572,6 +590,7 @@ function parseHostList(raw: unknown): Parsed { // A wildcard must sit in front of a real domain of two labels or more: `*` or `*.com` // would hand Windows credentials to any site that asks. if (hosts.some((host) => host.includes('*') && !/^\*\.[A-Za-z0-9_-]+(?:\.[A-Za-z0-9_-]+)+(?::\d+)?$/.test(host))) return fail('invalid_host_list'); + if (hosts.some(coversTopLevelDomain)) return fail('invalid_host_list'); return { ok: true, value: hosts.join(',') }; } @@ -616,12 +635,15 @@ export function parseCertificateRule(raw: unknown): CertificateRule | null { const match = /^(?:https:\/\/)?(\[\*\.\]|\*\.)?([^/*[\]]+(?::\d+)?)\/?$/i.exec(pattern); if (!match) return null; let host: string; + let hostname: string; try { - host = new URL(`https://${match[2]}`).host; + ({ host, hostname } = new URL(`https://${match[2]}`)); } catch { return null; } if (!host) return null; + // A wildcard over a bare name is the whole of a top-level domain. + if (match[1] && !hostname.includes('.')) return null; if (filter !== undefined && (typeof filter !== 'object' || filter === null || Array.isArray(filter))) return null; const { ISSUER, SUBJECT } = (filter ?? {}) as { ISSUER?: unknown; SUBJECT?: unknown }; const issuer = certificateName(ISSUER); @@ -737,6 +759,8 @@ function failClosedValue(key: PolicyKey): PolicyValues[PolicyKey] { if (definition.type === 'list') return [] as PolicyValues[PolicyKey]; // A shortcut nobody can read doesn't get the built-in one instead. if (definition.type === 'string' && definition.format === 'shortcut') return 'off' as PolicyValues[PolicyKey]; + // A limit nobody can read is the tightest one, not the user's own. + if (definition.type === 'integer') return definition.min as PolicyValues[PolicyKey]; return definition.default as PolicyValues[PolicyKey]; } @@ -802,6 +826,53 @@ export function effectivePolicyValue(entry: { value: T; locked: boolean }, ch export const DEFAULT_POLICY_SNAPSHOT: PolicySnapshot = resolvePolicies(EMPTY_POLICY_LAYERS); +export interface PolicyProblem { + key: PolicyKey; + reason: string; + raw?: string; +} + +/** Forced values the app can't use, and lists with entries it had to leave out. */ +export function policyProblems(snapshot: PolicySnapshot): PolicyProblem[] { + const problems: PolicyProblem[] = []; + for (const key of POLICY_KEYS) { + const { invalid, dropped, raw } = snapshot[key]; + if (invalid) problems.push({ key, reason: invalid, ...(raw ? { raw } : {}) }); + else if (dropped) problems.push({ key, reason: 'entries_left_out' }); + } + return problems; +} + +/** What the webmail reads, in `lib/` and `components/`. A page gets nothing else. */ +const PAGE_POLICY_KEYS: readonly PolicyKey[] = [ + 'AllowStandalone', + 'AllowedJmapServers', + 'InstanceUrl', + 'PluginsMode', + 'PluginAllowlist', + 'OfflineEnabled', + 'AppLockRequired', + 'AppLockTimeoutMinutes', + 'CrashReportsAllowed', +]; + +/** + * The policies as a page gets them. A page can be any server someone chose, and the auth + * lists and certificate rules name an organisation's internal hosts, so those stay at their + * defaults. Nor does it get a bad value's text, or the address of a server: only whether + * one is forced. + */ +export function pagePolicySnapshot(snapshot: PolicySnapshot): PolicySnapshot { + const page = { ...DEFAULT_POLICY_SNAPSHOT } as Record; + for (const key of PAGE_POLICY_KEYS) { + const entry: PolicyEntry = { ...snapshot[key] }; + delete entry.raw; + if (key === 'InstanceUrl') (entry as { value: unknown }).value = null; + page[key] = entry; + } + return page as PolicySnapshot; +} + /** * Who acts on each key, so none can sit idle: the desktop repo's test and the webmail's * read the named file for the key. `pendingConsumer` names who still has to wire one. @@ -819,8 +890,8 @@ export const POLICY_CONSUMERS: Record = { WipeOfflineData: { repo: 'shell', file: 'src/main/features/policies.ts' }, AuthServerAllowlist: { repo: 'shell', file: 'src/main/features/policies.ts' }, AuthNegotiateDelegateAllowlist: { repo: 'shell', file: 'src/main/features/policies.ts' }, - AutoUpdate: { pendingConsumer: 'feat/packaging: src/main/features/updater.ts' }, - UpdateChannel: { pendingConsumer: 'feat/packaging: src/main/features/updater.ts' }, + AutoUpdate: { repo: 'shell', file: 'src/main/update-rules.ts' }, + UpdateChannel: { repo: 'shell', file: 'src/main/update-rules.ts' }, AppLockRequired: { repo: 'webmail', file: 'lib/app-lock/policy.ts' }, AppLockTimeoutMinutes: { repo: 'webmail', file: 'lib/app-lock/policy.ts' }, TelemetryAllowed: { repo: 'shell', file: 'src/main/features/privacy.ts' }, diff --git a/lib/desktop/policy.ts b/lib/desktop/policy.ts index 5e9ec2f2f..d541d2229 100644 --- a/lib/desktop/policy.ts +++ b/lib/desktop/policy.ts @@ -102,9 +102,12 @@ export function customServerAllowedByDesktopPolicy(snapshot: PolicySnapshot = ge * Whether accounts may keep mail on this device: `forced` when the policy says so * either way, `fallback` for the default where the user hasn't chosen. Forced off * feeds `noOffline` (lib/device-policy.ts), which also clears what's there already. + * `invalid`: forced to something that makes no sense. Nothing new is kept, and a typo + * deletes nothing. */ -export function desktopOfflinePolicy(snapshot: PolicySnapshot = getDesktopPolicy()): { forced: boolean | null; fallback: boolean | null } { +export function desktopOfflinePolicy(snapshot: PolicySnapshot = getDesktopPolicy()): { forced: boolean | null; fallback: boolean | null; invalid?: true } { const { OfflineEnabled } = snapshot; + if (OfflineEnabled.locked && OfflineEnabled.invalid) return { forced: false, fallback: null, invalid: true }; if (OfflineEnabled.locked) return { forced: OfflineEnabled.value, fallback: null }; return { forced: null, fallback: OfflineEnabled.source === 'default' ? null : OfflineEnabled.value }; } diff --git a/lib/device-policy.ts b/lib/device-policy.ts index f5cf9dc3e..93486080a 100644 --- a/lib/device-policy.ts +++ b/lib/device-policy.ts @@ -83,8 +83,10 @@ export function devicesForAccount(subject: DeviceSubject): DevicePolicy { const accountSubject = subjectForAccount(accountId); const policy = accountSubject === store.subject ? store.policy : store.resolveFor(accountSubject); const devices = devicesOf(policy); - // The desktop app's OS policy says the same thing as the organisation's switch. - return desktopOfflinePolicy().forced === false ? { ...devices, noOffline: true } : devices; + // The desktop app's OS policy says the same thing as the organisation's switch. Not a + // value that makes no sense: noOffline wipes, and offlineAllowed asks about that one itself. + const offline = desktopOfflinePolicy(); + return offline.forced === false && !offline.invalid ? { ...devices, noOffline: true } : devices; } /** What applies to anyone, with no account's groups: for data of an account nobody here is signed in to. */ diff --git a/lib/offline/__tests__/effective-scope-wiring.test.ts b/lib/offline/__tests__/effective-scope-wiring.test.ts index 4bf221c12..d9590bc76 100644 --- a/lib/offline/__tests__/effective-scope-wiring.test.ts +++ b/lib/offline/__tests__/effective-scope-wiring.test.ts @@ -1,6 +1,7 @@ import 'fake-indexeddb/auto'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { DEFAULT_ORGANISATION_POLICY } from '@/lib/admin/organisation-policy'; +import { EMPTY_POLICY_LAYERS, resolvePolicies } from '@/lib/desktop/policy-schema'; import { getOfflineHooks, setOfflineHooks } from '@/lib/jmap/offline-hooks'; import { setStoreBackend } from '@/lib/store/client/api'; import { FakeJmap } from '@/lib/store/__tests__/helpers/fake-jmap'; @@ -14,6 +15,8 @@ import { disableOffline, enableOffline, enforcePolicy, + offlineActive, + offlineAllowed, refreshQueueState, refreshUsage, registerOfflineHooks, @@ -62,6 +65,34 @@ afterEach(async () => { setStoreBackend(null); }); +describe('OfflineEnabled from the desktop app\'s OS policy', () => { + function forced(value: unknown): void { + const snapshot = resolvePolicies({ ...EMPTY_POLICY_LAYERS, forcedMachine: { OfflineEnabled: value } }); + (window as unknown as { bulwarkDesktop: unknown }).bulwarkDesktop = { version: 1, platform: 'win32', mode: 'standalone', policy: { get: () => snapshot, onChange: () => () => {} } }; + } + + it('a value that makes no sense stops keeping mail and deletes nothing', async () => { + await enableOffline('acc'); + expect(useOfflineStore.getState().enabled).toEqual(['acc']); + + forced('yes'); + expect(offlineAllowed('acc')).toBe(false); + enforcePolicy(); + await vi.waitFor(() => expect(offlineActive('acc')).toBe(false)); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(useOfflineStore.getState().enabled).toEqual(['acc']); + expect(notices.map((notice) => notice.kind)).not.toContain('forbidden'); + }); + + it('forced off does delete', async () => { + await enableOffline('acc'); + forced(0); + enforcePolicy(); + await vi.waitFor(() => expect(useOfflineStore.getState().enabled).toEqual([])); + expect(notices.map((notice) => notice.kind)).toContain('forbidden'); + }); +}); + describe('at page load, before the subject is known', () => { const forbiddenExceptGroup = { ...usePolicyStore.getState().rawPolicy, diff --git a/lib/offline/offline-mode.ts b/lib/offline/offline-mode.ts index 2de80e0f6..0fc20a875 100644 --- a/lib/offline/offline-mode.ts +++ b/lib/offline/offline-mode.ts @@ -86,6 +86,7 @@ import { type OutboxRow, type UserChoice, } from './state'; +import { desktopOfflinePolicy } from '@/lib/desktop/policy'; import { effectiveScope, offlineDefaultOn, olderThanCap, platformDefault, userChoiceFor, type EffectiveScope } from './scope'; import { isStandaloneDisplayMode } from '@/lib/standalone-display'; import { readPersistence, requestPersistence, safariMayDelete } from './storage-health'; @@ -163,7 +164,7 @@ export function offlineSupported(): boolean { /** The organisation allows this account to keep mail on the device. */ export function offlineAllowed(account: string | null | undefined): boolean { - return offlineSupported() && !devicesForAccount(account ?? null).noOffline; + return offlineSupported() && !devicesForAccount(account ?? null).noOffline && !desktopOfflinePolicy().invalid; } /** A dropped account's data isn't judged by the groups of whoever is signed in now. */ From 6d3d7707a3f6727189838b0833a230e846553b12 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 06:00:28 +0100 Subject: [PATCH 02/10] fix: let a page see the forced server it runs on, and only that --- lib/desktop/policy-schema.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/desktop/policy-schema.ts b/lib/desktop/policy-schema.ts index 781726d39..a6fc4c5c4 100644 --- a/lib/desktop/policy-schema.ts +++ b/lib/desktop/policy-schema.ts @@ -859,15 +859,15 @@ const PAGE_POLICY_KEYS: readonly PolicyKey[] = [ /** * The policies as a page gets them. A page can be any server someone chose, and the auth * lists and certificate rules name an organisation's internal hosts, so those stay at their - * defaults. Nor does it get a bad value's text, or the address of a server: only whether - * one is forced. + * defaults. Nor does it get a bad value's text, or a recommended server's address: a forced + * one is the only server such a page can be on. */ export function pagePolicySnapshot(snapshot: PolicySnapshot): PolicySnapshot { const page = { ...DEFAULT_POLICY_SNAPSHOT } as Record; for (const key of PAGE_POLICY_KEYS) { const entry: PolicyEntry = { ...snapshot[key] }; delete entry.raw; - if (key === 'InstanceUrl') (entry as { value: unknown }).value = null; + if (key === 'InstanceUrl' && !entry.locked) (entry as { value: unknown }).value = null; page[key] = entry; } return page as PolicySnapshot; From cbe7d5da49ced76b97124c2a8dc40e604cbe36e5 Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 06:09:09 +0100 Subject: [PATCH 03/10] fix: drop the recommended forms of policies that do nothing as a recommendation from the templates --- desktop-policies/Bulwark.admx | 24 ------------------------ desktop-policies/de-DE/Bulwark.adml | 2 +- desktop-policies/en-US/Bulwark.adml | 2 +- 3 files changed, 2 insertions(+), 26 deletions(-) diff --git a/desktop-policies/Bulwark.admx b/desktop-policies/Bulwark.admx index d9020a931..c45e3459a 100644 --- a/desktop-policies/Bulwark.admx +++ b/desktop-policies/Bulwark.admx @@ -81,12 +81,6 @@ - - - - - - @@ -123,12 +117,6 @@ - - - - - - @@ -149,24 +137,12 @@ - - - - - - - - - - - - diff --git a/desktop-policies/de-DE/Bulwark.adml b/desktop-policies/de-DE/Bulwark.adml index 2dc61f9cc..7b8e93636 100644 --- a/desktop-policies/de-DE/Bulwark.adml +++ b/desktop-policies/de-DE/Bulwark.adml @@ -61,7 +61,7 @@ Anmeldung über den Browser anbieten Wenn aktiviert oder nicht konfiguriert, bietet die Anmeldeseite „Mit Ihrem Browser anmelden“ an, damit Benutzer dort ihren Passwort-Manager, Passkeys und Ihr Single Sign-on verwenden können. Wenn deaktiviert, fehlt die Schaltfläche und die App lehnt den Vorgang ab. Das Formular in der App bleibt, sofern der Server nicht nur OAuth erlaubt. Auf dem Gerät behaltene E-Mails löschen - Setzen Sie dies auf einen neuen Wert, etwa das heutige Datum, um alles zu löschen, was die App für alle Server und Konten auf dem Gerät behält: E-Mails, Schlüssel und Anmeldungen, Benutzer melden sich also neu an. Jeder Wert löscht einmal: Die App merkt sich, welche sie schon ausgeführt hat, ein Wert kann also stehen bleiben. Verwenden Sie einen neuen Wert, um erneut zu löschen. + Setzen Sie dies auf einen neuen Wert, etwa das heutige Datum, um alles zu löschen, was die App für alle Server und Konten auf dem Gerät behält: E-Mails, Schlüssel und Anmeldungen, Benutzer melden sich also neu an. Jeder Wert löscht einmal: Die App merkt sich, welche sie schon ausgeführt hat, ein Wert kann also stehen bleiben. Verwenden Sie einen neuen Wert, um erneut zu löschen. Diese Liste liegt im Profil des Benutzers. Wer sie bearbeitet, kann also eine Löschung umgehen, von der er weiß: Dies räumt ein Gerät auf, es nimmt der Person am Gerät keine Daten weg. diff --git a/desktop-policies/en-US/Bulwark.adml b/desktop-policies/en-US/Bulwark.adml index 1ef436c69..167772e1b 100644 --- a/desktop-policies/en-US/Bulwark.adml +++ b/desktop-policies/en-US/Bulwark.adml @@ -61,7 +61,7 @@ Offer sign-in through the browser When enabled or not configured, the login page offers "Sign in with your browser", so people can use their password manager, passkeys and your single sign-on there. When disabled, the button is gone and the app refuses the flow. The form in the app stays, unless the server only allows OAuth. Delete mail kept on the device - Set this to a new value, such as today's date, to delete everything the app keeps on the device for every server and account: mail, keys, and sign-ins, so people sign in again. Each value deletes once: the app remembers the ones it has acted on, so it stays safe to leave a value in place. Use a new value to delete again. + Set this to a new value, such as today's date, to delete everything the app keeps on the device for every server and account: mail, keys, and sign-ins, so people sign in again. Each value deletes once: the app remembers the ones it has acted on, so it stays safe to leave a value in place. Use a new value to delete again. That memory is in the user's own profile, so someone who edits it can skip a wipe they know is coming: this cleans up a device, it doesn't take data from the person using it. From 141748e1d668c24163a02af76bf740f3abde7a0a Mon Sep 17 00:00:00 2001 From: Linus Rath <139418639+rathlinus@users.noreply.github.com> Date: Wed, 4 Nov 2026 06:29:23 +0100 Subject: [PATCH 04/10] fix: show first-start notices one at a time above the message list, each with a close button The dictionary test now reads the notice queue instead of the toasts: the question is no longer a toast. --- .../desktop/__tests__/desktop-host.test.tsx | 31 ++++++- components/desktop/desktop-host.tsx | 22 +++-- components/mail/mail-app.tsx | 6 +- .../offline/__tests__/offline-host.test.tsx | 16 +++- components/offline/offline-host.tsx | 4 +- .../ui/__tests__/first-start-notice.test.tsx | 61 ++++++++++++++ components/ui/first-start-notice.tsx | 45 +++++++++++ components/ui/welcome-banner.tsx | 19 +++++ lib/first-start-notices.ts | 81 +++++++++++++++++++ 9 files changed, 273 insertions(+), 12 deletions(-) create mode 100644 components/ui/__tests__/first-start-notice.test.tsx create mode 100644 components/ui/first-start-notice.tsx create mode 100644 lib/first-start-notices.ts diff --git a/components/desktop/__tests__/desktop-host.test.tsx b/components/desktop/__tests__/desktop-host.test.tsx index 61f8a3b8d..76b83aaa3 100644 --- a/components/desktop/__tests__/desktop-host.test.tsx +++ b/components/desktop/__tests__/desktop-host.test.tsx @@ -6,6 +6,7 @@ import { readDesktopPrefs, resetDesktopPrefsCache, setDesktopPref } from '@/lib/ import { resetDesktopLanguages } from '@/lib/desktop/languages'; import { registerComposeFlush, resetComposeFlushRegistry } from '@/lib/compose-flush-registry'; import { useToastStore } from '@/stores/toast-store'; +import { closeFirstStartNotice, firstStartNoticesWaiting, resetFirstStartNotices } from '@/lib/first-start-notices'; import { useAuthStore } from '@/stores/auth-store'; import { useLocaleStore } from '@/stores/locale-store'; import { useOfflineStore } from '@/lib/offline/state'; @@ -116,6 +117,13 @@ describe('DesktopHost', () => { expect(push).toHaveBeenLastCalledWith('/contacts'); }); + it('Help > Settings opens settings as a page of this window', () => { + const shell = fakeBridge(); + render(); + shell.run('go.settings'); + expect(push).toHaveBeenLastCalledWith('/settings'); + }); + it('a link to a folder goes through the router, encoded', () => { const shell = fakeBridge(); render(); @@ -233,8 +241,10 @@ describe('DesktopHost', () => { }); describe('a language with no checker in the OS', () => { + beforeEach(() => resetFirstStartNotices()); afterEach(() => resetComposeFlushRegistry()); - const toasts = () => useToastStore.getState().toasts; + // A first-start notice above the message list, not a toast over the composer. + const toasts = () => firstStartNoticesWaiting(); it('the composer asks once, and only a yes opens the downloads', async () => { useToastStore.setState({ toasts: [] }); @@ -246,6 +256,7 @@ describe('DesktopHost', () => { expect(toasts()).toHaveLength(0); const stop = registerComposeFlush(async () => {}); await waitFor(() => expect(toasts()).toHaveLength(1)); + expect(useToastStore.getState().toasts).toHaveLength(0); expect(shell.allowDownload).not.toHaveBeenCalled(); act(() => toasts()[0].action!.onClick()); expect(shell.allowDownload).toHaveBeenCalledTimes(1); @@ -253,6 +264,24 @@ describe('DesktopHost', () => { stop(); }); + it("closing it says no, and it doesn't come back", async () => { + const shell = fakeBridge(); + render(); + await act(async () => {}); + const stop = registerComposeFlush(async () => {}); + shell.missing('nl-NL'); + await waitFor(() => expect(toasts()).toHaveLength(1)); + // Only asked: nothing decided until the user closes it. + expect(readDesktopPrefs().dictionaryDownload).toBe('ask'); + act(() => closeFirstStartNotice(toasts()[0].id)); + expect(readDesktopPrefs().dictionaryDownload).toBe('no'); + shell.missing('fr-FR'); + await act(async () => {}); + expect(toasts()).toHaveLength(0); + expect(shell.allowDownload).not.toHaveBeenCalled(); + stop(); + }); + it('never a second time', async () => { useToastStore.setState({ toasts: [] }); setDesktopPref('dictionaryDownload', 'no'); diff --git a/components/desktop/desktop-host.tsx b/components/desktop/desktop-host.tsx index 909ca657b..57d427302 100644 --- a/components/desktop/desktop-host.tsx +++ b/components/desktop/desktop-host.tsx @@ -25,7 +25,7 @@ import { useEmailStore } from "@/stores/email-store"; import { useLocaleStore } from "@/stores/locale-store"; import { useSettingsStore } from "@/stores/settings-store"; import { useThemeStore } from "@/stores/theme-store"; -import { toast } from "@/stores/toast-store"; +import { showFirstStartNotice } from "@/lib/first-start-notices"; /** * The webmail's side of what every desktop window needs: runs menu commands nothing @@ -71,6 +71,8 @@ export function DesktopHost() { "go.calendar": () => routerRef.current.push("/calendar"), "go.contacts": () => routerRef.current.push("/contacts"), "go.files": () => routerRef.current.push("/files"), + // Help > Settings: a page of this window, as the rail's gear opens it. + "go.settings": () => routerRef.current.push("/settings"), "help.shortcuts": () => openKeyboardShortcuts(), }; return menu.onCommand((id) => { @@ -127,9 +129,10 @@ export function DesktopHost() { noteComposerLanguage("settings", writingLanguage && writingLanguage !== "auto" ? writingLanguage : null); }, [writingLanguage]); - // A language with no checker in the OS needs a dictionary from Google: the composer - // asks once, and never in no-third-party mode. Until a yes, the shell has nowhere to - // fetch from at all. + // A language with no checker in the OS needs a dictionary from Google: once a composer + // is open, a first-start notice asks, never in no-third-party mode, and not again once + // closed. Until a yes, the shell has nowhere to fetch from at all. Windows never asks: + // it checks spelling itself (the shell's context-menu.ts). useEffect(() => { const spellcheck = getDesktopBridge()?.spellcheck; if (!spellcheck) return; @@ -139,8 +142,6 @@ export function DesktopHost() { const [language] = waiting; if (!language || !composerIsOpen() || !dictionaryDownloadAllowed()) return; if (readDesktopPrefs().dictionaryDownload !== "ask") return; - // Asked once, whatever comes of it. - setDesktopPref("dictionaryDownload", "no"); const translate = tRef.current; let name = language; try { @@ -148,8 +149,9 @@ export function DesktopHost() { } catch { // The tag itself will do. } - toast.info(translate("desktopUi.spellcheck.offer", { language: name }), { - duration: 0, + showFirstStartNotice({ + id: "dictionary-download", + text: translate("desktopUi.spellcheck.offer", { language: name }), action: { label: translate("desktopUi.spellcheck.download"), onClick: () => { @@ -157,6 +159,10 @@ export function DesktopHost() { if (dictionaryDownloadAllowed()) spellcheck.allowDownload(); }, }, + // Closed without a yes: asked once, not again. + onClose: () => { + if (readDesktopPrefs().dictionaryDownload === "ask") setDesktopPref("dictionaryDownload", "no"); + }, }); }; const heard = (language: string) => { diff --git a/components/mail/mail-app.tsx b/components/mail/mail-app.tsx index 11d4246c0..25454c2a6 100644 --- a/components/mail/mail-app.tsx +++ b/components/mail/mail-app.tsx @@ -104,7 +104,8 @@ import type { Attachment, SendEmailResult } from "@/lib/jmap/types"; import type { IJMAPClient } from "@/lib/jmap/client-interface"; import { peekListAttachments, requestListAttachments, type ListAttachmentSource, type LoadListAttachments } from "@/lib/list-attachments"; import { useSearchHistoryStore } from "@/stores/search-history-store"; -import { WelcomeBanner } from "@/components/ui/welcome-banner"; +import { useWelcomeBannerDue, WelcomeBanner } from "@/components/ui/welcome-banner"; +import { FirstStartNoticeBar } from "@/components/ui/first-start-notice"; import { NavigationRail } from "@/components/layout/navigation-rail"; import { AppMain } from "@/components/layout/app-landmarks"; import { MvTopBar } from "@/components/layout/mv-top-bar"; @@ -253,6 +254,8 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { // Static Lite build: the route params are empty, read the link from the URL. const linkSegments = useLiteLinkSegments('mail', routeSegments); const t = useTranslations(); + // First-start notices wait until the welcome tips are closed: one thing at a time. + const welcomeDue = useWelcomeBannerDue(); // The page skeleton the gates below draw: the same one the server sent. const skeletonLabels = useSkeletonLabels(); const tNotifications = useTranslations('notifications'); @@ -5718,6 +5721,7 @@ export function MailApp({ linkSegments: routeSegments }: MailAppProps = {}) { unless an enabled plugin registered tabs via api.tabs.set. */} {!isScheduledView && } +