-
Notifications
You must be signed in to change notification settings - Fork 0
feat(desktop): add resumable Last.fm importer #40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+5,694
−27
Merged
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
7be700a
feat(desktop): add resumable Last.fm importer
rianjs d5360f1
fix(desktop): serialize Last.fm importer state safely
rianjs 6cbafa3
test(desktop): cover importer async failure paths
rianjs a1e4966
fix: guard Last.fm importer mutations by account
rianjs a8081a2
fix: guard Last.fm queue page selection
rianjs 3a4ddc9
fix: align Last.fm importer setup progress
rianjs 2c1019e
fix: tolerate restricted Spotify search results
rianjs 6c8898c
fix: preserve Last.fm picker selections
rianjs 76068e6
fix: preserve album state on track rematch
rianjs 9bdeff6
fix: show standalone track confidence
rianjs b67a952
fix: align importer typography
rianjs File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding
lastfm-importerto the main window's default capability grants the importerdialog:defaultandopener:default, even though this dedicated review surface does not need the main window's broad plugin grants. This widens the impact of a renderer compromise in the importer. Give it a separate capability with only its required core/window permissions and, if the attribution link requires it, the narrow opener permission rather than the dialog and opener defaults.Reply inline to this comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in d5360f1. The main
defaultcapability now covers onlymainand retains the dialog/opener permissions it uses. Addedcapabilities/lastfm-importer.jsonscoped tolastfm-importerwith core/window-title and opener permissions only—no dialog permission. Frontend typecheck/build, Rust clippy, and workspace tests pass.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks—this addresses the capability-scope concern by isolating the importer and removing its dialog permission.
Summary:
Resolved: the importer now has a dedicated, least-privilege capability without dialog access; the main window retains its existing broader permissions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-actionable in pass 2: the capability scope remains fixed in d5360f1. The importer capability is separate, has core/window-title and opener only, and has no dialog permission; no additional code change was needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved: the importer remains isolated in a dedicated least-privilege capability with no dialog permission; no further change is needed.