Skip to content

fix: harden desktop security and repair async state races - #11

Closed
heyitstarek wants to merge 8 commits into
tryliner:mainfrom
heyitstarek:fix/reliability-security-review
Closed

heyitstarek wants to merge 8 commits into
tryliner:mainfrom
heyitstarek:fix/reliability-security-review

Conversation

@heyitstarek

Copy link
Copy Markdown

Summary

Repairs desktop security and request lifetimes that could leak credentials, restore signed-out sessions, show another account's library, or overwrite newer playback/search/import state.

- HTTP fallback, weakened renderer isolation, renderer-local session secrets
+ HTTPS-only account traffic, sandbox/web security, exact-document IPC, protected native sessions
- stale async work can commit after logout/cancel/track change
+ account/operation guards and resource cleanup
- update files/state can be left inconsistent on malformed or failed downloads
+ bounded signed downloads, safe archive paths, atomic activation and recovery

Commits separate renderer/session security, API/cache behavior, account views, playback/lyrics, frontend lifetimes, and maintenance. No product features, backend changes, request-signing changes, or license changes.

Evidence

  • Before: baseline passed 278 tests, but new deferred-response regressions reproduced stale session/library/lyrics/import/search/cover state; native probes exposed disabled isolation and unsafe cross-origin behavior.
    After: 428 tests across 66 files pass, TypeScript and the production build pass, and the isolated Windows Electron security smoke passes..
  • Native Windows Electron 44 probes exercise the built main/overlay, sandboxing, web security, blocked hostile navigation/frames/objects, IPC sender checks, media/artwork compatibility, and synthetic-token safeStorage save/load/clear. No real account or production backend is used.
  • Pinned pnpm 10.34.7 frozen install and workflow validation pass. Dependency audit changed from 17 high / 16 moderate / 7 low to one moderate optional build-chain sprintf-js advisory with no patched release; the disputed http-cache-semantics advisory is not claimed as independently remediated.
  • CI validates branch pushes and PRs; release builds wait for test/typecheck/build and Windows native smoke gates. Build commands explicitly disable implicit publishing.

Merge Danger

Door: Two-way for code; reverting to a version without protected-session support requires signing in again after migration removes legacy plaintext tokens.

Blast Radius: Desktop

Cross-cutting native/session changes need a coordinated desktop and frontend release. Existing signed OTA bundles remain upstream-controlled and can replace frontend code; this PR does not change that update policy. Linux without a recognized OS secret store uses memory-only sessions. Native Linux/macOS runtime, packaged installers, live playback, server retention and music-source rights have not been verified.

@ProdunVladyslav

Copy link
Copy Markdown
Contributor

Thank you, this is a serious piece of work, and several of these issues are real on main. At 83 files and +6k lines, though, mixing security, races, dependency overrides and CI, it’s not something we can review or roll back safely in one go, and it no longer merges cleanly. So I’m closing it, and I’d really like the fixes to come back as small separate PRs, roughly in this order:

sandbox: true and webSecurity: true for the main and overlay windows, plus the smoke test
a will-navigate guard, IPC sender checks, and narrowing the access-control-allow-origin: * override
the shell:open-* handlers: only open directories, never arbitrary paths
bundle-manager hardening: version validation, path containment, size cap, redirect: "error"
auth tokens moved to safeStorage. Please open an issue first, since this forces everyone to sign in again.
the race and stale-cache fixes, one PR per feature

Please leave the CI workflows and the pnpm-workspace overrides out for now. Rebase each one on the current main. 1–4 are the highest priority, and I’ll review them quickly.

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.

2 participants