Skip to content

fix(updater): handle null and non-object prefs in appUpdatesEnabled - #1828

Closed
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1827-update-check-policy-null-safety
Closed

fix(updater): handle null and non-object prefs in appUpdatesEnabled#1828
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1827-update-check-policy-null-safety

Conversation

@hsusul

@hsusul hsusul commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixes #1827

Problem

In src/helpers/updateCheckPolicy.js:
appUpdatesEnabled({ notificationsEnabled, notifyUpdates } = {}) threw TypeError: Cannot destructure property 'notificationsEnabled' of 'null' as it is null when passed null.

Solution

  • Defaulted parameters safely to handle null and non-object values before destructuring.
  • Added unit tests in test/helpers/updateCheckPolicy.test.js.

Verification

  • node --test test/helpers/updateCheckPolicy.test.js (passes, 3/3 tests)
  • npm run typecheck (passes, 0 errors)
  • npm run lint (passes, 0 errors)
  • npm run i18n:check (passes)
  • npm run build:renderer (passes)
  • git diff --check (clean)

@Chadpiha Chadpiha self-assigned this Aug 25, 2026
@Chadpiha

Copy link
Copy Markdown
Collaborator

Thank you @hsusul — tidy PR and a thorough verification list, as always.

Closing this along with #1827: both call sites (updater.js) pass windowManager?.notificationPrefs, which is initialized in the constructor to a complete object and only ever key-mutated behind an existing !prefs || typeof prefs !== "object" guard at the IPC boundary (sync-notification-preferences) — so null can't reach the helper. The undefined case (missing windowManager via ?.) is already covered by the = {} default and pinned by an existing test.

There's also a behavioral reason we'd keep the throw: this helper enforces the #1605 contract ("only an explicit false disables update checks"). If pref state were ever corrupted to null, silently returning true would quietly re-enable update checks — for that invariant we'd rather crash loudly than fail open.

Appreciate you poking at the updater gating — thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

appUpdatesEnabled throws TypeError when called with null

2 participants