fix(security): harden application against identified vulnerabilities#59
Merged
flexiondotorg merged 18 commits intomainfrom Mar 27, 2026
Merged
fix(security): harden application against identified vulnerabilities#59flexiondotorg merged 18 commits intomainfrom
flexiondotorg merged 18 commits intomainfrom
Conversation
fix(dmg): position app icons over light gold background for improved label contrast Move Sidra app icon and Applications link icons from y: 220 to y: 100, positioning labels over the lighter portion of the DMG background to improve readability of black label text. Signed-off-by: Martin Wimpress <code@wimpress.io>
- Add guard condition to skip login if Discord is disabled at startup - Export enable() and disable() functions for immediate connect/disconnect - Add disconnectClient() to properly cleanup timers and destroy client - Attempt lazy reconnect when re-enabled via next player event - Update tray toggle to call enable/disable functions directly The previous implementation would attempt connection regardless of the disabled setting and had no way to immediately disconnect when toggled off at runtime. Now the integration respects the disabled state and provides explicit control via exported functions. Signed-off-by: Martin Wimpress <code@wimpress.io>
- Polish opening paragraph for better flow - Consolidate Features section language - Replace Development recipe tables with guidance to use `just` - Rewrite "Why Sidra?" as compelling narrative with personal context - Enhance Install section formatting and prose - Position Sidra as "a proper desktop citizen" in introduction Signed-off-by: Martin Wimpress <code@wimpress.io>
- Remove self-discoverable sections (runtime deps, project structure, dev environment) - Compress development commands to self-discovery note (just --list) - Move Widevine VMP signing setup to README Development section - Add startPage and lastPageUrl to Configuration table - Add two Widevine build pipeline bullets to Architecture notes - Fix Sec-CH-UA-Platform claim (removed) - Fix playbackTimeDidChange timing assertion - Fix dbus-next semver range Signed-off-by: Martin Wimpress <code@wimpress.io>
- Change getDiscordEnabled() default from true to false - Update AGENTS.md Configuration table to reflect new default Discord Rich Presence should require explicit opt-in rather than assuming users have Discord installed. Signed-off-by: Martin Wimpress <code@wimpress.io>
…ancy - Correct 21 discrepancies between specification and codebase - Package versions, dependencies, file structure, IPC flow - Platform identifiers, defaults, feature descriptions - Remove pre-implementation research sections (Prior Art, Sources) - Remove Design Rationale (one-time technology justification) - Remove duplicated content from AGENTS.md (Volume Sync code, Theming CSS subsections) - Fix stale references (Architecture diagram, Technology Stack table) Signed-off-by: Martin Wimpress <code@wimpress.io>
- Full codebase SAST scan using Semgrep, Gitleaks, OSV-Scanner - Manual code review covering OWASP Top 10 and CWE-Top-25 categories - Three warnings identified (W1: unsigned update signatures on Windows, W2: missing protocol validation on update URL, W3: main window missing sandbox flag) - Four observations for defence-in-depth improvements - One medium-severity dev dependency (brace-expansion ReDoS) - Remediation roadmap with priority levels and effort estimates Signed-off-by: Martin Wimpress <code@wimpress.io>
Add protocol validation to reject non-HTTP(S) URLs before calling shell.openExternal(), preventing exploitation via malformed GitHub API responses (CWE-350). Signed-off-by: Martin Wimpress <code@wimpress.io>
Replace wildcard '*' with specific origin 'https://music.apple.com' to mitigate unauthorized cross-origin messaging (CWE-942). Signed-off-by: Martin Wimpress <code@wimpress.io>
Use execFileSync() with argument array instead of execSync() with shell interpolation to eliminate command injection risk (CWE-78) when processing appOutDir during the Electron build hook. This prevents untrusted path content from being interpreted as shell commands. - Replace execSync(shell string) with execFileSync(command, args[]) - Update import to destructure execFileSync instead of execSync Signed-off-by: Martin Wimpress <code@wimpress.io>
Replace prefix-based validation
(`startsWith('https://music.apple.com/')`) with explicit URL parsing
that verifies both hostname and protocol. Prefix matching allows
subdomain spoofing (e.g., attacker.music.apple.com) and open redirect
attacks where hostile schemes bypass the check.
Also add exception handling for malformed URI inputs.
Signed-off-by: Martin Wimpress <code@wimpress.io>
Add override for brace-expansion >=5.0.5 to patch GHSA-f886-m6hf-6m8v (ReDoS) in transitive dependencies without downgrading electron-builder. Signed-off-by: Martin Wimpress <code@wimpress.io>
Adds sandbox: true to webPreferences to enable OS-level process isolation in the renderer process. This limits the impact scope of potential renderer compromises. Tested with Widevine CDM (DRM-protected audio) to confirm compatibility. Signed-off-by: Martin Wimpress <code@wimpress.io>
Signed-off-by: Martin Wimpress <code@wimpress.io>
- Add VALID_LEVELS set to constrain accepted values - Check envLevel exists before type assertion - Prevent invalid log level configurations Signed-off-by: Martin Wimpress <code@wimpress.io>
Signed-off-by: Martin Wimpress <code@wimpress.io>
Contributor
There was a problem hiding this comment.
2 issues found across 14 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/discord-presence/index.ts">
<violation number="1" location="src/integrations/discord-presence/index.ts:85">
P2: `disable()` skips cleanup when the client is already disconnected, so pending reconnect timers keep running and can re-login even after the toggle is off. Clear timers even when not connected.</violation>
</file>
<file name="src/tray.ts">
<violation number="1" location="src/tray.ts:301">
P2: The new tray toggle calls `enableDiscord()`/`disableDiscord()` before the Discord integration is guaranteed to be initialised. If the user clicks the tray item before `initDiscordPresence()` runs, `client` is still undefined and `enable()` throws when accessing `client.isConnected`. Consider guarding these calls until init completes or making `enable/disable` no-op until the client is created.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…tionally - Add null/undefined checks for client in enable() and disable() to prevent crashes from tray interaction before Discord init - Clear reconnect timer unconditionally in disable() to prevent ghost reconnections when client is disconnected Signed-off-by: Martin Wimpress <code@wimpress.io>
Contributor
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
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/discord-presence/index.ts">
<violation number="1" location="src/integrations/discord-presence/index.ts:180">
P2: `disable()` duplicates `disconnectClient()` teardown logic, creating two disconnect paths that can drift and cause inconsistent cleanup behavior.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…uplication Refactor disable() to call disconnectClient() instead of duplicating teardown logic. This ensures both disconnect paths remain consistent and prevents maintenance drift in the future. Signed-off-by: Martin Wimpress <code@wimpress.io>
Contributor
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Requires human review: This PR involves security-related changes, including input validation and permission restrictions (Electron sandbox), which are explicitly flagged as requiring human review.
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
Addresses security findings from initial audit by implementing input validation, restricting external communication channels, resolving dependency vulnerabilities, and documenting security posture. All fixes employ principle of least privilege.
Changes
Input Validation & Protocol Hardening:
Privilege Restriction:
Dependency Security:
Documentation:
Testing
Related Issues
Addresses all findings from security audit review.