refactor(core): refine artwork handling, notifications, and security#48
Merged
flexiondotorg merged 6 commits intomainfrom Mar 25, 2026
Merged
refactor(core): refine artwork handling, notifications, and security#48flexiondotorg merged 6 commits intomainfrom
flexiondotorg merged 6 commits intomainfrom
Conversation
…ersistent cache - Replace https.get() with net.fetch() for consistent error handling - Implement atomic writes using .tmp files with rename-on-success pattern - Replace in-memory lastArtworkUrl/lastArtworkPath with URL-derived filesystem cache - Extract UUID from Apple CDN artwork URLs for human-readable, deterministic cache keys - Use unique .tmp suffixes per invocation to prevent concurrent download race conditions - Add cache hit and download success logging - Fix robustness issues in notifications integration: add missing .catch() handlers, implement will-quit cleanup, ensure non-blocking artwork downloads Signed-off-by: Martin Wimpress <code@wimpress.io>
- Add Content-Security-Policy meta tags to splash.html and about.html with policy: default-src 'none'; style-src 'unsafe-inline'; script-src 'unsafe-inline'; img-src 'self' - Align about.html image reference to use relative path (sidra-logo.png) instead of file:// absolute path, matching splash.html - Remove icon query parameter and file:// handling from about.html and src/tray.ts CSP prevents inline script and style injection while restricting image loads to local files, reducing attack surface for window content. Image alignment ensures both auxiliary windows follow consistent asset patterns. Signed-off-by: Martin Wimpress <code@wimpress.io>
- Explain module-level readFileSync usage in i18n.ts - Document playbackTimeDidChange unit (microseconds) in PlayerEvents Signed-off-by: Martin Wimpress <code@wimpress.io>
Add release/latest*.yml to artefact and release upload globs in builder workflow. This enables electron-updater to locate manifest files required for reporting available updates. Manifests are generated by electron-builder but were excluded from uploads. Signed-off-by: Martin Wimpress <code@wimpress.io>
…dled rejection
- Remove duplicate error logging from try/catch and error event handler
- Swallow re-thrown error from checkForUpdates() with .catch(() => {})
- Filter "No published versions" to [info] level instead of [error]
Prevents unhandled promise rejection warnings when GitHub releases are
not yet published. The autoUpdater still checks for updates; missing
manifests log as info-level and do not disrupt the app.
Signed-off-by: Martin Wimpress <code@wimpress.io>
Contributor
There was a problem hiding this comment.
1 issue found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/integrations/notifications/index.ts">
<violation number="1" location="src/integrations/notifications/index.ts:24">
P2: Handle downloadArtwork rejections even when the timeout wins; otherwise a late rejection becomes an unhandled promise rejection.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Add .catch() to downloadArtwork promise inside Promise.race to prevent unhandled rejection if the timeout wins the race. Maintains existing error handling pattern with warn-level logging. Signed-off-by: Martin Wimpress <code@wimpress.io>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch refines the Sidra codebase across five areas identified in code review:
https.getwithnet.fetch, atomic.tmpfile writes with rename-on-success, UUID-based persistent cache keys extracted from Apple CDN URLs, unique tmp suffixes to prevent concurrent download race conditions, cache hit and download success logging.catch()handler,will-quitcleanup, non-blocking artwork download viaPromise.racewith 500ms fallbackContent-Security-Policymeta tags added to splash and about windows; about window image reference aligned to use relative path (removingfile://construction andfile:CSP directive)i18n.ts, JSDoc microsecond annotation onPlayerEvents.playbackTimeDidChangelatest*.ymlelectron-updater manifests now included in release asset uploadsinfolevel; unhandled promise rejection fixedChanges
Testing