Skip to content

fix(audio): validate settings and finite duration in shouldSaveDiscardedRecording - #1806

Closed
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1805-discarded-recording-validation
Closed

fix(audio): validate settings and finite duration in shouldSaveDiscardedRecording#1806
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1805-discarded-recording-validation

Conversation

@hsusul

@hsusul hsusul commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #1805

Problem

In src/helpers/discardedRecording.js:

  1. shouldSaveDiscardedRecording checked durationSeconds >= MIN_DISCARDED_DURATION_SECONDS, which allowed Infinity to improperly evaluate to true.
  2. When settings was passed as a non-object primitive, it wasn't validated upfront.

Solution

  • Added if (!settings || typeof settings !== "object") return false; upfront.
  • Required Number.isFinite(durationSeconds) && durationSeconds >= MIN_DISCARDED_DURATION_SECONDS.
  • Added unit tests in test/helpers/discardedRecording.test.js.

Verification

  • node --test test/helpers/discardedRecording.test.js (passes, 5/5 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

Thanks for this one, @hsusul — closing along with #1805 after tracing where both inputs come from.

durationSeconds is pure wall-clock arithmetic: (Date.now() - recordingStartTime) / 1000, or null when there's no start time (which already returns false today). It never comes from media metadata, so the well-known "WebM reports Infinity duration" situation can't feed this helper — the value is always null or finite. And settings is always a Zustand store snapshot, so it's always an object (property access on primitives doesn't throw in JS either, so the non-object path already returned false).

The direction of the guard matters here too: this feature exists to preserve discarded audio (#964/#907 — the min-duration check only filters accidental sub-second Escape taps). If an unknown duration ever did appear, silently declining to save would lose exactly the audio the feature protects, so failing open is the safer default for this one.

Really appreciate the attention to the audio pipeline — thank you!

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.

shouldSaveDiscardedRecording permits Infinity duration and non-object settings

2 participants