feat: integrate enhanced login features from submodule and add QR/cookie UI#105
feat: integrate enhanced login features from submodule and add QR/cookie UI#105Zhenghao-Wen wants to merge 45 commits into2697a:feature/new-uifrom
Conversation
main.dart, login_page.dart
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds multi-platform GitHub Actions for Flutter builds, updates branding to "Bujuan", migrates bujuan_music_api to a git dependency, refactors login with phone/QR/cookie flows, removes the lyrics provider, and loads a stored cookie at startup. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as App UI
participant Local as SharedPreferences
participant API as BujuanMusicManager (API)
participant QRsrv as QR Auth Server
UI->>Local: check saved manual_cookie
Local-->>UI: manual_cookie (if present)
UI->>API: setCookie(manual_cookie)
API-->>UI: auth validation (user info)
UI->>API: request QR key
API-->>UI: qr_key + qr_image_url
UI->>UI: display QR image
loop poll
UI->>QRsrv: poll qr status (qr_key)
QRsrv-->>UI: status (pending/confirmed/expired)
end
QRsrv-->>API: on-confirm provide cookie/token
API-->>UI: user info on success
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pubspec.yaml (1)
30-57:⚠️ Potential issue | 🔴 CriticalAdd
shared_preferencesdependency — code imports are unresolved.The codebase imports
shared_preferencesin at least two files (lib/main.dartandlib/pages/login/login_page.dart), but the dependency is missing frompubspec.yaml. This will cause import resolution failures and prevent the app from building or running.Fix
nb_utils: ^7.1.8 flutter_lyric: ^3.0.2 + shared_preferences: ^2.0.0 qr_flutter: ^4.1.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pubspec.yaml` around lines 30 - 57, The project is missing the shared_preferences dependency referenced by imports in lib/main.dart and lib/pages/login/login_page.dart; add a shared_preferences entry under dependencies in pubspec.yaml (e.g., shared_preferences: ^2.0.15), save the file and run flutter pub get to fetch the package so the unresolved imports in the files (imports of shared_preferences) are resolved.
🧹 Nitpick comments (4)
.github/workflows/web.yml (1)
22-24: Glob-based pub-cache wipe is brittle and partially redundant withcache: true.
rm -rf ~/.pub-cache/git/bujuan_music_api-*runs after flutter-action has already restored the pub cache. While the ordering is correct (cache restore → wipe specific dir → pub get), the globbujuan_music_api-*could inadvertently match unrelated packages that share the same prefix. A more targeted approach pins the exact cache key:- - name: Clear submodule cache - run: rm -rf ~/.pub-cache/git/bujuan_music_api-* + - name: Clear bujuan_music_api pub cache + run: | + find ~/.pub-cache/git -maxdepth 1 -name 'bujuan_music_api-*' -type d -exec rm -rf {} + || trueAlternatively, disable
cache: trueon the flutter-action step for this workflow since you want a fresh fetch every run.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/web.yml around lines 22 - 24, The glob-based wipe in the "Clear submodule cache" step (rm -rf ~/.pub-cache/git/bujuan_music_api-*) is brittle and may match unrelated packages; either replace this step with a targeted removal that uses the exact cache key for the bujuan_music_api repo, or disable the flutter-action cache by setting its cache: false so the action does a fresh fetch each run; update the workflow to reference the specific unique cache key or toggle the flutter-action's cache option (the flutter-action step and the "Clear submodule cache" rm command are the symbols to change).lib/pages/play/provider.dart (1)
5-5: Untracked TODO — consider opening an issue.The TODO comment doesn't link to any issue tracker entry, making it easy to lose. If the lyrics feature restoration is planned, opening a GitHub issue for it would ensure it's tracked.
Would you like me to draft the issue description for restoring the
getMediaLyricprovider andparseLrchelper?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/pages/play/provider.dart` at line 5, The TODO comment "恢复歌词 provider" is untracked—open a GitHub issue to track restoring the getMediaLyric provider and parseLrc helper, then replace the inline TODO in lib/pages/play/provider.dart with a short TODO that references the created issue number/URL; the issue should include a description of desired behavior, acceptance criteria (unit tests for parseLrc, integration with getMediaLyric, sample LRC parsing scenarios), and any migration notes so future readers can follow the work.lib/pages/home/provider.dart (1)
14-23: Remove duplicaterecommendSongs()call innewAlbum.Lines 14 and 16 invoke the same endpoint with identical parameters, causing unnecessary API latency. Both responses are cast to
RecommendSongEntityat lines 30 and 31, then both extract.data?.dailySongs.Eliminate line 16's
recommendSongFuture, remove it fromFuture.wait(), and reuse the first response for bothsongEntityandrecommendSongEntity.💡 Suggested refactor
var recommendResourceFuture = BujuanMusicManager().recommendResource(); var songsFuture = BujuanMusicManager().recommendSongs(); var topArtistFuture = BujuanMusicManager().topArtist(limit: 10); - var recommendSongFuture = BujuanMusicManager().recommendSongs(); var list = await Future.wait([ recommendResourceFuture, topArtistFuture, songsFuture, - recommendSongFuture, ]); return compute(_buildHomeData, list); } ... var songEntity = list[2] as RecommendSongEntity; - var recommendSongEntity = list[3] as RecommendSongEntity; + var recommendSongEntity = songEntity;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/pages/home/provider.dart` around lines 14 - 23, The code calls BujuanMusicManager().recommendSongs() twice (variables recommendSongFuture and songsFuture) causing duplicate API requests; remove the duplicate recommendSongFuture declaration and drop it from the Future.wait([...]) array, then after awaiting 'list' reuse the songsFuture result when populating both songEntity and recommendSongEntity (i.e. assign recommendSongEntity = songEntity or pull .data?.dailySongs from the songsFuture response) so only one recommendSongs() call is made; update any references to recommendSongFuture accordingly.android/app/build.gradle (1)
41-53: Gate signing config assignment when environment variables are missing.The current code assigns
storePassword,keyAlias, andkeyPasswordfrom environment variables without fallbacks. When these variables are unset (typical in local development), AGP will fail the release build with "missing required property" errors. The suggested approach of conditionally assigning thesigningConfigonly when all required properties are available allows unsigned release builds locally, rather than failing the build.💡 Gate signing config by env availability
+ def hasReleaseSigning = + System.getenv("KEYSTORE_PATH") && + System.getenv("KEYSTORE_PASSWORD") && + System.getenv("KEY_ALIAS") && + System.getenv("KEY_PASSWORD") + buildTypes { release { - signingConfig signingConfigs.release + if (hasReleaseSigning) { + signingConfig signingConfigs.release + } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@android/app/build.gradle` around lines 41 - 53, The release signing currently sets storePassword, keyAlias, and keyPassword directly from env vars (storePassword, keyAlias, keyPassword) which causes AGP to fail when they are unset; update the buildTypes.release block to conditionally assign signingConfig signingConfigs.release only when all required env variables (KEYSTORE_PATH/KEYSTORE_PASSWORD/KEY_ALIAS/KEY_PASSWORD) are present (e.g., check System.getenv(...) != null for each) so local/CI environments without these vars can build unsigned releases, leaving signingConfigs.release unchanged otherwise; reference the existing signingConfigs.release and the env var names when implementing the guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build_new.yml:
- Around line 42-45: Remove exporting sensitive secrets to the job-wide
environment: only set KEYSTORE_PATH in GITHUB_ENV and stop writing
KEYSTORE_PASSWORD, KEY_ALIAS, and KEY_PASSWORD there. Instead, keep
KEYSTORE_PASSWORD, KEY_ALIAS, and KEY_PASSWORD as step-level secrets consumed
directly by the Build APKs step (use the step's env or with: secrets.* so they
aren't exposed to downstream steps). Update the workflow to reference
KEYSTORE_PATH from GITHUB_ENV and pass the other three values from secrets only
where needed (e.g., the build/sign step) to prevent job-wide exposure.
In @.github/workflows/web.yml:
- Around line 29-36: The workflow currently only warns when .g.dart files are
missing in the submodule ("Verify generated files in submodule") which lets the
subsequent "Build Web" step run and fail; update the job to either (A) run
flutter pub run build_runner build (or flutter pub get then build_runner) in the
workspace of the git dependency before the "Build Web" step to generate missing
*.g.dart files, or (B) change the verify step to exit non-zero when the ls check
fails so the workflow stops early; target the verify step name and the
subsequent "Build Web" step when implementing the fix so generation runs or the
job fails before building.
In `@flutter`:
- Line 1: The flutter git submodule is incomplete (no .gitmodules) and pinned to
commit 582a0e7c5581dc0ca5f7bfd8662bb8db6f59d536 while pubspec.yaml depends on
bujuan_music_api via ref: login_test, creating dual-dependency drift; fix by
either fully configuring the submodule (add a .gitmodules entry for the flutter
submodule and run git submodule init/update) or remove the submodule entry from
the index if you don't intend to keep it, then ensure bujuan_music_api is merged
to main and update pubspec.yaml to pin the dependency to the explicit commit SHA
that matches the submodule (replace ref: login_test with commit:
582a0e7c5581dc0ca5f7bfd8662bb8db6f59d536) and, if retaining the submodule,
update the submodule pointer to that same SHA so both resolution paths reference
the identical commit.
In `@lib/main.dart`:
- Around line 47-54: The _loadManualCookie function currently reads a plaintext
auth cookie from SharedPreferences and passes it to
BujuanMusicManager().setCookie; replace this with secure storage by switching
from SharedPreferences to flutter_secure_storage (or platform keychain/keystore)
for both read and write paths so credentials aren't stored in plaintext. Update
_loadManualCookie to read using FlutterSecureStorage (same key 'manual_cookie'),
and also update the write location in login_page.dart where the cookie is saved
to use the same secure API; ensure null/empty checks remain and that
BujuanMusicManager().setCookie is called only after retrieving the secure value.
In `@lib/pages/home/provider.dart`:
- Around line 30-31: The code force-casts results of
BujuanMusicManager.recommendSongs() to non-nullable RecommendSongEntity
(variables songEntity and recommendSongEntity and other similar casts at the
indicated locations), which can throw when the method returns null; change those
casts to nullable types (RecommendSongEntity?) and update usages to use
null-aware operators (?., ?? or early-null checks) or guard returns so you
safely handle null responses from recommendSongs() instead of force-casting to
non-nullable.
In `@lib/pages/login/login_page.dart`:
- Around line 476-477: Update the misleading button label in the login_page
widget: replace the Text('Get an SMS QR code') string with a clear action like
Text('Send SMS verification code') (or 'Send SMS code') in the button widget so
the UI reflects that the action sends an SMS verification code rather than
providing a QR code.
- Around line 71-90: Several async callbacks in the login page (notably
_sendCode, _resendCode, _loginWithCode, _loginWithCookie and the Timer.periodic
loop inside _initQrLogin) perform UI/context work after awaits without verifying
the widget is still mounted; add guarded mounted checks immediately after every
await and before any call to setState(), _showTopSnackBar(), Navigator calls or
other context-dependent operations (e.g., in _sendCode after the
BujuanMusicManager().sendSmsCode await, in every await inside the Timer.periodic
callback, and before setting _isSendingCode, _lastSentPhone, showing
sheets/snackbars, or mutating state), and ensure the Timer.periodic loop cancels
itself or returns early if mounted is false to avoid lifecycle exceptions.
- Around line 232-289: The periodic QR check can overlap because the
Timer.periodic callback is async; add an in-flight guard (e.g., a private bool
_isQrChecking) and check it at the top of the Timer.periodic callback used to
poll in _qrTimer and return immediately if true, then set _isQrChecking = true
before calling BujuanMusicManager().qrCodeLogin and ensure you clear it in a
finally block (set to false) after the await/processing so no concurrent
qrCodeLogin calls happen; keep the existing expiry handling (timer.cancel /
state updates) but ensure the guard is cleared on all paths including errors.
- Around line 257-258: Replace plaintext SharedPreferences usage for the
'manual_cookie' key with encrypted FlutterSecureStorage: where you currently
call SharedPreferences.getInstance() and prefs.setString('manual_cookie',
result.cookie!), instantiate a FlutterSecureStorage (e.g., final storage =
FlutterSecureStorage()) and use storage.write(key: 'manual_cookie', value:
result.cookie) and storage.read(key: 'manual_cookie') for reads; update all
occurrences referenced (the spots using SharedPreferences, prefs, setString and
the 'manual_cookie' key at the other locations) and add the import for
flutter_secure_storage plus appropriate async/error handling around read/write
operations.
In `@lib/pages/play/provider.dart`:
- Around line 2-3: Remove the dangling part directive and unused import: delete
the line containing part 'provider.g.dart' and the import
'package:riverpod_annotation/riverpod_annotation.dart' from provider.dart (or
re-add them only if you restore the `@riverpod-annotated` lyrics provider and its
generated file); this eliminates the compile-time reference to the deleted
provider.g.dart and removes the unused riverpod_annotation import.
In `@pubspec.yaml`:
- Around line 41-43: The git dependency entry referencing bujuan_music_api
currently uses a mutable branch ref ('ref: login_test') which makes builds
non-reproducible; update the pubspec.yaml git dependency (the block with "git:
url: https://github.com/Zhenghao-Wen/bujuan_music_api.git" and "ref:
login_test") to use the exact commit SHA (or a released tag) instead of the
branch name, then run pub get to refresh lockfiles and commit the updated
lockfile so CI uses the pinned commit.
---
Outside diff comments:
In `@pubspec.yaml`:
- Around line 30-57: The project is missing the shared_preferences dependency
referenced by imports in lib/main.dart and lib/pages/login/login_page.dart; add
a shared_preferences entry under dependencies in pubspec.yaml (e.g.,
shared_preferences: ^2.0.15), save the file and run flutter pub get to fetch the
package so the unresolved imports in the files (imports of shared_preferences)
are resolved.
---
Nitpick comments:
In @.github/workflows/web.yml:
- Around line 22-24: The glob-based wipe in the "Clear submodule cache" step (rm
-rf ~/.pub-cache/git/bujuan_music_api-*) is brittle and may match unrelated
packages; either replace this step with a targeted removal that uses the exact
cache key for the bujuan_music_api repo, or disable the flutter-action cache by
setting its cache: false so the action does a fresh fetch each run; update the
workflow to reference the specific unique cache key or toggle the
flutter-action's cache option (the flutter-action step and the "Clear submodule
cache" rm command are the symbols to change).
In `@android/app/build.gradle`:
- Around line 41-53: The release signing currently sets storePassword, keyAlias,
and keyPassword directly from env vars (storePassword, keyAlias, keyPassword)
which causes AGP to fail when they are unset; update the buildTypes.release
block to conditionally assign signingConfig signingConfigs.release only when all
required env variables (KEYSTORE_PATH/KEYSTORE_PASSWORD/KEY_ALIAS/KEY_PASSWORD)
are present (e.g., check System.getenv(...) != null for each) so local/CI
environments without these vars can build unsigned releases, leaving
signingConfigs.release unchanged otherwise; reference the existing
signingConfigs.release and the env var names when implementing the guard.
In `@lib/pages/home/provider.dart`:
- Around line 14-23: The code calls BujuanMusicManager().recommendSongs() twice
(variables recommendSongFuture and songsFuture) causing duplicate API requests;
remove the duplicate recommendSongFuture declaration and drop it from the
Future.wait([...]) array, then after awaiting 'list' reuse the songsFuture
result when populating both songEntity and recommendSongEntity (i.e. assign
recommendSongEntity = songEntity or pull .data?.dailySongs from the songsFuture
response) so only one recommendSongs() call is made; update any references to
recommendSongFuture accordingly.
In `@lib/pages/play/provider.dart`:
- Line 5: The TODO comment "恢复歌词 provider" is untracked—open a GitHub issue to
track restoring the getMediaLyric provider and parseLrc helper, then replace the
inline TODO in lib/pages/play/provider.dart with a short TODO that references
the created issue number/URL; the issue should include a description of desired
behavior, acceptance criteria (unit tests for parseLrc, integration with
getMediaLyric, sample LRC parsing scenarios), and any migration notes so future
readers can follow the work.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (27)
android/app/src/main/res/mipmap-hdpi/ic_launcher.pngis excluded by!**/*.pngandroid/app/src/main/res/mipmap-mdpi/ic_launcher.pngis excluded by!**/*.pngandroid/app/src/main/res/mipmap-xhdpi/ic_launcher.pngis excluded by!**/*.pngandroid/app/src/main/res/mipmap-xxhdpi/ic_launcher.pngis excluded by!**/*.pngandroid/app/src/main/res/mipmap-xxxhdpi/ic_launcher.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-1024x1024@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-20x20@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-29x29@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-40x40@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-60x60@3x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@1x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-76x76@2x.pngis excluded by!**/*.pngios/Runner/Assets.xcassets/AppIcon.appiconset/Icon-App-83.5x83.5@2x.pngis excluded by!**/*.pngpubspec.lockis excluded by!**/*.lockweb/favicon.pngis excluded by!**/*.pngweb/icons/Icon-192.pngis excluded by!**/*.pngweb/icons/Icon-512.pngis excluded by!**/*.pngweb/icons/Icon-maskable-192.pngis excluded by!**/*.pngweb/icons/Icon-maskable-512.pngis excluded by!**/*.pngwindows/runner/resources/app_icon.icois excluded by!**/*.ico
📒 Files selected for processing (19)
.github/workflows/build_new.yml.github/workflows/web.ymlCHANGELOG.mdandroid/app/build.gradleandroid/app/src/main/AndroidManifest.xmlbujuanbuildflutterios/Runner/Info.plistlib/main.dartlib/pages/home/provider.dartlib/pages/home/provider.g.dartlib/pages/login/login_page.dartlib/pages/main/provider.g.dartlib/pages/play/provider.dartlib/pages/play/provider.g.dartlib/pages/playlist/provider.g.dartmacos/Flutter/GeneratedPluginRegistrant.swiftplugin/bujuan_music_apipubspec.yaml
💤 Files with no reviewable changes (3)
- lib/pages/play/provider.g.dart
- plugin/bujuan_music_api
- macos/Flutter/GeneratedPluginRegistrant.swift
| echo "KEYSTORE_PATH=${{ runner.temp }}/keystore.jks" >> $GITHUB_ENV | ||
| echo "KEYSTORE_PASSWORD=${{ secrets.ANDROID_KEYSTORE_PASSWORD }}" >> $GITHUB_ENV | ||
| echo "KEY_ALIAS=${{ secrets.ANDROID_KEY_ALIAS }}" >> $GITHUB_ENV | ||
| echo "KEY_PASSWORD=${{ secrets.ANDROID_KEY_PASSWORD }}" >> $GITHUB_ENV |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Actions GITHUB_ENV environment variable scope availability subsequent steps
💡 Result:
Writing NAME=VALUE to the file pointed to by GITHUB_ENV sets (or updates) an environment variable for all subsequent steps in the same job. It is not available earlier steps, and the step that writes to GITHUB_ENV does not see the new value (within that same step). [1]
So this works:
- name: Set var
run: echo "MY_VAR=hello" >> "$GITHUB_ENV"
- name: Use var (later step)
run: echo "$MY_VAR"But if you need to use the value within the same step, assign it to a normal shell variable in that step as well.
Also, GITHUB_ENV is for within a job; to pass values between jobs, use job outputs instead. [2]
Sources
[1] GitHub Docs — Workflow commands: “Setting an environment variable” (GITHUB_ENV subsequent-step behavior) (docs.github.com)
[2] GitHub Docs — “Store information in variables” (passing values between steps vs jobs) (docs.github.com)
Citations:
- 1: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions
- 2: https://docs.github.com/en/actions/learn-github-actions/variables
🏁 Script executed:
# Find and examine the workflow file
find . -name "build_new.yml" -type fRepository: 2697a/bujuan
Length of output: 89
🏁 Script executed:
cat -n ./.github/workflows/build_new.ymlRepository: 2697a/bujuan
Length of output: 9756
Avoid exporting signing secrets through GITHUB_ENV.
Lines 43–45 write sensitive values into the job-wide environment, making them available to all downstream steps. Since the Build APKs step directly accesses secrets and subsequent steps don't need these values, keep only the keystore path in global scope.
💡 Limit global env to keystore path only
- name: Decode Keystore
run: |
echo "${{ secrets.ANDROID_KEYSTORE_BASE64 }}" | base64 -d > ${{ runner.temp }}/keystore.jks
echo "KEYSTORE_PATH=${{ runner.temp }}/keystore.jks" >> $GITHUB_ENV
- echo "KEYSTORE_PASSWORD=${{ secrets.ANDROID_KEYSTORE_PASSWORD }}" >> $GITHUB_ENV
- echo "KEY_ALIAS=${{ secrets.ANDROID_KEY_ALIAS }}" >> $GITHUB_ENV
- echo "KEY_PASSWORD=${{ secrets.ANDROID_KEY_PASSWORD }}" >> $GITHUB_ENV🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build_new.yml around lines 42 - 45, Remove exporting
sensitive secrets to the job-wide environment: only set KEYSTORE_PATH in
GITHUB_ENV and stop writing KEYSTORE_PASSWORD, KEY_ALIAS, and KEY_PASSWORD
there. Instead, keep KEYSTORE_PASSWORD, KEY_ALIAS, and KEY_PASSWORD as
step-level secrets consumed directly by the Build APKs step (use the step's env
or with: secrets.* so they aren't exposed to downstream steps). Update the
workflow to reference KEYSTORE_PATH from GITHUB_ENV and pass the other three
values from secrets only where needed (e.g., the build/sign step) to prevent
job-wide exposure.
| # 验证子模块中是否包含必要的 .g.dart 文件 | ||
| - name: Verify generated files in submodule | ||
| run: | | ||
| echo "Checking for .g.dart files in submodule..." | ||
| ls -la ~/.pub-cache/git/bujuan_music_api-*/lib/api/song/entity/*.g.dart || echo "::warning::No .g.dart files found in submodule" | ||
|
|
||
| - name: Build Web | ||
| run: flutter build web --release |
There was a problem hiding this comment.
Missing build_runner step — build will fail if .g.dart files are absent from the dependency.
The "Verify generated files" step emits a ::warning:: but does not fail, so the workflow proceeds to flutter build web --release even when no .g.dart files are present. If the login_test branch of bujuan_music_api does not pre-commit generated files, the Flutter build will fail with unresolved types from json_serializable/riverpod_generator.
Either:
- Pre-commit all
.g.dartfiles into thebujuan_music_apigit dependency, or - Add a
build_runnerstep and promote the verify step to a hard failure:
⚙️ Proposed fix
+ - name: Generate code
+ run: flutter pub run build_runner build --delete-conflicting-outputs
+
- name: Verify generated files in submodule
run: |
echo "Checking for .g.dart files in submodule..."
- ls -la ~/.pub-cache/git/bujuan_music_api-*/lib/api/song/entity/*.g.dart || echo "::warning::No .g.dart files found in submodule"
+ ls -la ~/.pub-cache/git/bujuan_music_api-*/lib/api/song/entity/*.g.dart
- name: Build Web
run: flutter build web --release📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # 验证子模块中是否包含必要的 .g.dart 文件 | |
| - name: Verify generated files in submodule | |
| run: | | |
| echo "Checking for .g.dart files in submodule..." | |
| ls -la ~/.pub-cache/git/bujuan_music_api-*/lib/api/song/entity/*.g.dart || echo "::warning::No .g.dart files found in submodule" | |
| - name: Build Web | |
| run: flutter build web --release | |
| # 验证子模块中是否包含必要的 .g.dart 文件 | |
| - name: Generate code | |
| run: flutter pub run build_runner build --delete-conflicting-outputs | |
| - name: Verify generated files in submodule | |
| run: | | |
| echo "Checking for .g.dart files in submodule..." | |
| ls -la ~/.pub-cache/git/bujuan_music_api-*/lib/api/song/entity/*.g.dart | |
| - name: Build Web | |
| run: flutter build web --release |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/web.yml around lines 29 - 36, The workflow currently only
warns when .g.dart files are missing in the submodule ("Verify generated files
in submodule") which lets the subsequent "Build Web" step run and fail; update
the job to either (A) run flutter pub run build_runner build (or flutter pub get
then build_runner) in the workspace of the git dependency before the "Build Web"
step to generate missing *.g.dart files, or (B) change the verify step to exit
non-zero when the ls check fails so the workflow stops early; target the verify
step name and the subsequent "Build Web" step when implementing the fix so
generation runs or the job fails before building.
| @@ -0,0 +1 @@ | |||
| Subproject commit 582a0e7c5581dc0ca5f7bfd8662bb8db6f59d536 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Step 1: Check if flutter file exists and list its contents
echo "=== Checking flutter submodule file ==="
if [ -f "flutter" ]; then
cat -n flutter
echo ""
echo "=== File details ==="
file flutter
else
echo "flutter file not found"
fi
echo ""
echo "=== Checking if flutter is a git submodule ==="
git config --file .gitmodules --get-regexp path | grep flutter
echo ""
echo "=== Git submodules configuration ==="
if [ -f ".gitmodules" ]; then
cat -n .gitmodules
fiRepository: 2697a/bujuan
Length of output: 204
🏁 Script executed:
#!/bin/bash
# Step 2: Search for pubspec.yaml and check bujuan_music_api dependency
echo "=== Locating pubspec.yaml ==="
fd pubspec.yaml
echo ""
echo "=== Checking pubspec.yaml for bujuan_music_api dependency ==="
fd pubspec.yaml -x cat -n {} \;Repository: 2697a/bujuan
Length of output: 5670
🏁 Script executed:
#!/bin/bash
# Check for flutter directory and its contents
echo "=== Checking for flutter directory ==="
if [ -d "flutter" ]; then
echo "flutter directory found"
ls -la flutter/
else
echo "flutter directory not found"
fi
echo ""
echo "=== Checking git status for flutter-related entries ==="
git ls-files | grep -i flutter
echo ""
echo "=== Checking .gitmodules history ==="
git log --oneline -n 20 -- .gitmodules 2>/dev/null || echo "No history available"
echo ""
echo "=== Checking for any recent deletions of flutter ==="
git status --shortRepository: 2697a/bujuan
Length of output: 1060
🏁 Script executed:
#!/bin/bash
# Check git history to see if flutter submodule was ever committed
echo "=== Checking git log for flutter-related changes ==="
git log --all --oneline --name-status | grep -i flutter | head -20Repository: 2697a/bujuan
Length of output: 875
🏁 Script executed:
#!/bin/bash
echo "=== Contents of flutter directory ==="
ls -la flutter/
echo ""
echo "=== File count in flutter directory ==="
find flutter -type f | wc -l
echo ""
echo "=== Is flutter a git submodule? ==="
git config --file .gitmodules --get-regexp ^ || echo "No submodule configuration found"Repository: 2697a/bujuan
Length of output: 317
🏁 Script executed:
#!/bin/bash
# Check git cat-file for the flutter entry to see what it actually is
echo "=== Git object details for flutter ==="
git ls-files -s flutter
echo ""
echo "=== Checking if flutter is tracked as submodule ==="
git config --file .gitmodules --listRepository: 2697a/bujuan
Length of output: 299
Submodule pinned to an unmerged feature-branch commit with incomplete configuration — resolve before merging.
The submodule flutter is tracked in git's index (mode 160000) at commit 582a0e7c5581dc0ca5f7bfd8662bb8db6f59d536, which resides on the login_test feature branch. However, the .gitmodules configuration file is missing, leaving the submodule in a broken/incomplete state. The flutter directory exists but is uninitialized.
Additionally, there is a dual-dependency problem creating divergence risk:
- The git submodule is pinned to
582a0e7 pubspec.yamlreferenceshttps://github.com/Zhenghao-Wen/bujuan_music_api.gitwithref: login_test(a mutable branch name)
If the login_test branch advances past 582a0e7, these two dependency resolution paths will resolve different versions of the API, causing build or runtime inconsistencies.
Before merging:
- Complete the submodule configuration by adding proper
.gitmodulesentry, or remove the incomplete submodule entry entirely. - Ensure the companion
bujuan_music_apiPR is merged to its main branch. - Pin
pubspec.yaml's bujuan_music_api dependency to an explicit commit SHA instead of the mutableref: login_test. - Update the submodule pointer (if retained) to match the pinned commit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flutter` at line 1, The flutter git submodule is incomplete (no .gitmodules)
and pinned to commit 582a0e7c5581dc0ca5f7bfd8662bb8db6f59d536 while pubspec.yaml
depends on bujuan_music_api via ref: login_test, creating dual-dependency drift;
fix by either fully configuring the submodule (add a .gitmodules entry for the
flutter submodule and run git submodule init/update) or remove the submodule
entry from the index if you don't intend to keep it, then ensure
bujuan_music_api is merged to main and update pubspec.yaml to pin the dependency
to the explicit commit SHA that matches the submodule (replace ref: login_test
with commit: 582a0e7c5581dc0ca5f7bfd8662bb8db6f59d536) and, if retaining the
submodule, update the submodule pointer to that same SHA so both resolution
paths reference the identical commit.
| Future<void> _loadManualCookie() async { | ||
| final prefs = await SharedPreferences.getInstance(); | ||
| final cookie = prefs.getString('manual_cookie'); | ||
| if (cookie != null && cookie.isNotEmpty) { | ||
| BujuanMusicManager().setCookie(cookie); | ||
| debugPrint('Loaded manual cookie from storage'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Sensitive credential stored in plaintext SharedPreferences — consider encryption.
The manual_cookie value is a raw session cookie (auth credential) written and read from SharedPreferences in plaintext. On rooted Android devices or with ADB backup enabled, this key-value pair is trivially extractable.
Consider using flutter_secure_storage to store the cookie under the OS keychain/keystore instead of SharedPreferences. This would require updating both the write path in login_page.dart and the read path here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/main.dart` around lines 47 - 54, The _loadManualCookie function currently
reads a plaintext auth cookie from SharedPreferences and passes it to
BujuanMusicManager().setCookie; replace this with secure storage by switching
from SharedPreferences to flutter_secure_storage (or platform keychain/keystore)
for both read and write paths so credentials aren't stored in plaintext. Update
_loadManualCookie to read using FlutterSecureStorage (same key 'manual_cookie'),
and also update the write location in login_page.dart where the cookie is saved
to use the same secure API; ensure null/empty checks remain and that
BujuanMusicManager().setCookie is called only after retrieving the secure value.
| var songEntity = list[2] as RecommendSongEntity; // 改用旧实体 | ||
| var recommendSongEntity = list[3] as RecommendSongEntity; // 改用旧实体 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the provider.dart file
find . -type f -name "provider.dart" | grep -E "home|pages"Repository: 2697a/bujuan
Length of output: 277
🏁 Script executed:
# Also check the structure to understand the codebase
git ls-files | head -20Repository: 2697a/bujuan
Length of output: 777
🏁 Script executed:
# Read the provider.dart file to see the context
cat -n ./lib/pages/home/provider.dartRepository: 2697a/bujuan
Length of output: 3310
🏁 Script executed:
# Search for BujuanMusicManager and recommendSongs() definition
rg "BujuanMusicManager|recommendSongs" --type dart -A 3 -B 1Repository: 2697a/bujuan
Length of output: 10763
🏁 Script executed:
# Search for the BujuanMusicManager class definition
rg "class BujuanMusicManager" --type dart -A 20Repository: 2697a/bujuan
Length of output: 38
🏁 Script executed:
# Search for recommendSongs method signature in the codebase
rg "recommendSongs\(\)" --type dart -B 2 -A 5Repository: 2697a/bujuan
Length of output: 1292
🏁 Script executed:
# Check if bujuan_music_api is a package in this repo or external
find . -type f -name "pubspec.yaml" | head -5Repository: 2697a/bujuan
Length of output: 70
🏁 Script executed:
# Check pubspec.yaml to see bujuan_music_api dependency
cat ./pubspec.yaml | grep -A 10 -B 2 "bujuan_music_api"Repository: 2697a/bujuan
Length of output: 434
🌐 Web query:
site:github.com/Zhenghao-Wen/bujuan_music_api recommendSongs return type
💡 Result:
I couldn’t determine the recommendSongs return type from the web UI, because:
- The repo exists and is public, but GitHub isn’t exposing the Dart source listing/content in the pages I can access here. (github.com)
- GitHub’s code search for
repo:Zhenghao-Wen/bujuan_music_api recommendSongsis blocked unless you’re signed in, so I can’t locate the symbol that way either. (github.com)
How to get it quickly on your side:
git clone https://github.com/Zhenghao-Wen/bujuan_music_api.git
cd bujuan_music_api
git checkout login_test
rg "recommendSongs" -n lib
# then open the matching .dart file and read the function signatureCitations:
- 1: https://github.com/Zhenghao-Wen/bujuan_music_api
- 2: https://github.com/Zhenghao-Wen/bujuan_music_api/search?q=recommendSongs&type=code
🏁 Script executed:
# Clone the bujuan_music_api repo and check the recommendSongs signature
cd /tmp && git clone https://github.com/Zhenghao-Wen/bujuan_music_api.git
cd bujuan_music_api
git checkout login_test
rg "recommendSongs" -n lib --type dartRepository: 2697a/bujuan
Length of output: 563
Handle nullable recommendSongs() responses before casting.
BujuanMusicManager.recommendSongs() returns Future<RecommendSongEntity?>, but lines 30–31 force-cast to non-nullable RecommendSongEntity, causing a runtime crash if null is returned. Line 60 correctly uses nullable access (?.), establishing the inconsistency.
Change casts to nullable and propagate with null-safe operators:
Suggested fix
- var songEntity = list[2] as RecommendSongEntity;
- var recommendSongEntity = list[3] as RecommendSongEntity;
+ var songEntity = list[2] as RecommendSongEntity?;
+ var recommendSongEntity = list[3] as RecommendSongEntity?;
- var songs = songEntity.data?.dailySongs ?? [];
+ var songs = songEntity?.data?.dailySongs ?? [];
...
- var recommend = recommendSongEntity.data?.dailySongs ?? [];
+ var recommend = recommendSongEntity?.data?.dailySongs ?? [];Also applies to: Lines 33, 48
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/pages/home/provider.dart` around lines 30 - 31, The code force-casts
results of BujuanMusicManager.recommendSongs() to non-nullable
RecommendSongEntity (variables songEntity and recommendSongEntity and other
similar casts at the indicated locations), which can throw when the method
returns null; change those casts to nullable types (RecommendSongEntity?) and
update usages to use null-aware operators (?., ?? or early-null checks) or guard
returns so you safely handle null responses from recommendSongs() instead of
force-casting to non-nullable.
| _qrTimer = Timer.periodic(const Duration(seconds: 2), (timer) async { | ||
| if (_qrKey == null) return; | ||
| try { | ||
| final result = await BujuanMusicManager().qrCodeLogin(key: _qrKey!); | ||
| if (result == null) return; | ||
|
|
||
| switch (result.code) { | ||
| case 800: | ||
| _showTopSnackBar('QR code expired, please refresh'); | ||
| timer.cancel(); | ||
| setState(() { | ||
| _isQrMode = false; | ||
| _qrKey = null; | ||
| }); | ||
| break; | ||
| case 801: | ||
| case 802: | ||
| // waiting | ||
| break; | ||
| case 803: | ||
| timer.cancel(); | ||
| if (result.cookie != null && result.cookie!.isNotEmpty) { | ||
| // 保存到内存 | ||
| BujuanMusicManager().setCookie(result.cookie!); | ||
| // 保存到 SharedPreferences | ||
| final prefs = await SharedPreferences.getInstance(); | ||
| await prefs.setString('manual_cookie', result.cookie!); | ||
| try { | ||
| final userInfo = await BujuanMusicManager().userInfo(); | ||
| if (userInfo?.profile != null) { | ||
| await setValue(AppConfig.userInfoKey, userInfo!.profile!.toJson()); | ||
| if (mounted) { | ||
| Navigator.popUntil(context, (route) => route.isFirst); | ||
| context.replace(AppRouter.home); | ||
| } | ||
| } else { | ||
| _showTopSnackBar('Failed to get user info with this cookie'); | ||
| await prefs.remove('manual_cookie'); | ||
| BujuanMusicManager().clearCookie(); | ||
| } | ||
| } catch (e) { | ||
| _showTopSnackBar('Error fetching user info: $e'); | ||
| await prefs.remove('manual_cookie'); | ||
| BujuanMusicManager().clearCookie(); | ||
| } | ||
| } else { | ||
| _showTopSnackBar('No cookie received, please try manual input'); | ||
| } | ||
| break; | ||
| default: | ||
| if (result.message != null) { | ||
| _showTopSnackBar(result.message!); | ||
| } | ||
| } | ||
| } catch (e) { | ||
| print('QR check error: $e'); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Dart Timer.periodic async callback wait for future completion before next tick
💡 Result:
Timer.periodic cannot “wait” for an async callback to finish before scheduling the next tick—the callback type is void Function(Timer) (not Future), so if you start async work inside it, ticks can overlap. (api.flutter.dev)
Option A (recommended): don’t use Timer.periodic; use an async loop
Future<void> poll(Duration period) async {
while (true) {
final sw = Stopwatch()..start();
await doWork(); // waits for completion
final remaining = period - sw.elapsed;
if (remaining > Duration.zero) await Future.delayed(remaining);
}
}This guarantees no overlap because await pauses until the Future completes. (dart.dev)
Option B: self-rescheduling one-shot Timer (no overlap, Timer-based)
Timer? _t;
bool _running = true;
void start(Duration period) {
void schedule() {
_t = Timer(period, () async {
await doWork();
if (_running) schedule();
});
}
schedule();
}
void stop() { _running = false; _t?.cancel(); }Option C: keep Timer.periodic, but prevent re-entry (skips ticks while busy)
bool busy = false;
Timer.periodic(period, (_) async {
if (busy) return;
busy = true;
try { await doWork(); } finally { busy = false; }
});Citations:
- 1: https://api.flutter.dev/flutter/dart-async/Timer-class.html?utm_source=openai
- 2: https://dart.dev/libraries/async/async-await?utm_source=openai
Prevent overlapping QR poll requests.
The Timer.periodic callback is async; because Timer.periodic does not wait for async work to complete before the next tick, new ticks can start before the previous qrCodeLogin finishes, causing concurrent polls and duplicate state transitions.
💡 Add an in-flight guard for polling
+ bool _qrPollInFlight = false;
...
_qrTimer = Timer.periodic(const Duration(seconds: 2), (timer) async {
+ if (_qrPollInFlight) return;
+ _qrPollInFlight = true;
if (_qrKey == null) return;
try {
final result = await BujuanMusicManager().qrCodeLogin(key: _qrKey!);
...
} catch (e) {
print('QR check error: $e');
+ } finally {
+ _qrPollInFlight = false;
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _qrTimer = Timer.periodic(const Duration(seconds: 2), (timer) async { | |
| if (_qrKey == null) return; | |
| try { | |
| final result = await BujuanMusicManager().qrCodeLogin(key: _qrKey!); | |
| if (result == null) return; | |
| switch (result.code) { | |
| case 800: | |
| _showTopSnackBar('QR code expired, please refresh'); | |
| timer.cancel(); | |
| setState(() { | |
| _isQrMode = false; | |
| _qrKey = null; | |
| }); | |
| break; | |
| case 801: | |
| case 802: | |
| // waiting | |
| break; | |
| case 803: | |
| timer.cancel(); | |
| if (result.cookie != null && result.cookie!.isNotEmpty) { | |
| // 保存到内存 | |
| BujuanMusicManager().setCookie(result.cookie!); | |
| // 保存到 SharedPreferences | |
| final prefs = await SharedPreferences.getInstance(); | |
| await prefs.setString('manual_cookie', result.cookie!); | |
| try { | |
| final userInfo = await BujuanMusicManager().userInfo(); | |
| if (userInfo?.profile != null) { | |
| await setValue(AppConfig.userInfoKey, userInfo!.profile!.toJson()); | |
| if (mounted) { | |
| Navigator.popUntil(context, (route) => route.isFirst); | |
| context.replace(AppRouter.home); | |
| } | |
| } else { | |
| _showTopSnackBar('Failed to get user info with this cookie'); | |
| await prefs.remove('manual_cookie'); | |
| BujuanMusicManager().clearCookie(); | |
| } | |
| } catch (e) { | |
| _showTopSnackBar('Error fetching user info: $e'); | |
| await prefs.remove('manual_cookie'); | |
| BujuanMusicManager().clearCookie(); | |
| } | |
| } else { | |
| _showTopSnackBar('No cookie received, please try manual input'); | |
| } | |
| break; | |
| default: | |
| if (result.message != null) { | |
| _showTopSnackBar(result.message!); | |
| } | |
| } | |
| } catch (e) { | |
| print('QR check error: $e'); | |
| } | |
| }); | |
| _qrTimer = Timer.periodic(const Duration(seconds: 2), (timer) async { | |
| if (_qrPollInFlight) return; | |
| _qrPollInFlight = true; | |
| if (_qrKey == null) return; | |
| try { | |
| final result = await BujuanMusicManager().qrCodeLogin(key: _qrKey!); | |
| if (result == null) return; | |
| switch (result.code) { | |
| case 800: | |
| _showTopSnackBar('QR code expired, please refresh'); | |
| timer.cancel(); | |
| setState(() { | |
| _isQrMode = false; | |
| _qrKey = null; | |
| }); | |
| break; | |
| case 801: | |
| case 802: | |
| // waiting | |
| break; | |
| case 803: | |
| timer.cancel(); | |
| if (result.cookie != null && result.cookie!.isNotEmpty) { | |
| // 保存到内存 | |
| BujuanMusicManager().setCookie(result.cookie!); | |
| // 保存到 SharedPreferences | |
| final prefs = await SharedPreferences.getInstance(); | |
| await prefs.setString('manual_cookie', result.cookie!); | |
| try { | |
| final userInfo = await BujuanMusicManager().userInfo(); | |
| if (userInfo?.profile != null) { | |
| await setValue(AppConfig.userInfoKey, userInfo!.profile!.toJson()); | |
| if (mounted) { | |
| Navigator.popUntil(context, (route) => route.isFirst); | |
| context.replace(AppRouter.home); | |
| } | |
| } else { | |
| _showTopSnackBar('Failed to get user info with this cookie'); | |
| await prefs.remove('manual_cookie'); | |
| BujuanMusicManager().clearCookie(); | |
| } | |
| } catch (e) { | |
| _showTopSnackBar('Error fetching user info: $e'); | |
| await prefs.remove('manual_cookie'); | |
| BujuanMusicManager().clearCookie(); | |
| } | |
| } else { | |
| _showTopSnackBar('No cookie received, please try manual input'); | |
| } | |
| break; | |
| default: | |
| if (result.message != null) { | |
| _showTopSnackBar(result.message!); | |
| } | |
| } | |
| } catch (e) { | |
| print('QR check error: $e'); | |
| } finally { | |
| _qrPollInFlight = false; | |
| } | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/pages/login/login_page.dart` around lines 232 - 289, The periodic QR
check can overlap because the Timer.periodic callback is async; add an in-flight
guard (e.g., a private bool _isQrChecking) and check it at the top of the
Timer.periodic callback used to poll in _qrTimer and return immediately if true,
then set _isQrChecking = true before calling BujuanMusicManager().qrCodeLogin
and ensure you clear it in a finally block (set to false) after the
await/processing so no concurrent qrCodeLogin calls happen; keep the existing
expiry handling (timer.cancel / state updates) but ensure the guard is cleared
on all paths including errors.
| final prefs = await SharedPreferences.getInstance(); | ||
| await prefs.setString('manual_cookie', result.cookie!); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is Flutter shared_preferences encrypted by default on Android/iOS recommended for storing auth tokens cookies security
💡 Result:
No. Flutter shared_preferences is not encrypted by default on either platform—it’s just a wrapper over Android SharedPreferences and iOS NSUserDefaults (meant for simple app settings), and the plugin explicitly says it “must not be used for storing critical data.” [1]
Should you store auth tokens / session cookies in shared_preferences?
Not recommended. Anything stored there can be extracted on compromised devices (e.g., rooted/jailbroken, backups, device-level forensics). OWASP explicitly treats storing sensitive material in SharedPreferences as a “not recommended” storage choice compared to KeyStore/Keychain-backed approaches. [2]
Recommended approach for tokens on Android/iOS (Flutter)
Use platform secure storage (Keychain/KeyStore). In Flutter, the common choice is flutter_secure_storage, which stores secrets using iOS Keychain and encrypted shared preferences backed by Android Keystore (via Tink). [3]
Rule of thumb
shared_preferences: UI settings, feature flags, non-sensitive prefs.flutter_secure_storage(Keychain/KeyStore): access/refresh tokens, session secrets, sensitive identifiers. [1][3]
Sources: shared_preferences package docs [1], OWASP Mobile AppSec Testing Guide (storage/key handling) [2], flutter_secure_storage package docs [3].
🏁 Script executed:
# Find and check lines around 389-390 in the login_page.dart file
grep -n "remove\|setString\|manual_cookie" lib/pages/login/login_page.dart | head -20Repository: 2697a/bujuan
Length of output: 405
Replace SharedPreferences with FlutterSecureStorage for storing authentication cookies.
shared_preferences stores data in plaintext (not encrypted by default on Android or iOS) and is explicitly unsuitable for storing sensitive credentials like auth cookies. Use flutter_secure_storage instead, which encrypts data via Android Keystore and iOS Keychain.
This applies to lines 258, 269, 274 (and 390, 400, 406 for the same manual_cookie key).
Replace all cookie persistence calls
-import 'package:shared_preferences/shared_preferences.dart';
+import 'package:flutter_secure_storage/flutter_secure_storage.dart';
...
+static const _secureStorage = FlutterSecureStorage();
...
- final prefs = await SharedPreferences.getInstance();
- await prefs.setString('manual_cookie', result.cookie!);
+ await _secureStorage.write(key: 'manual_cookie', value: result.cookie!);
...
- await prefs.remove('manual_cookie');
+ await _secureStorage.delete(key: 'manual_cookie');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/pages/login/login_page.dart` around lines 257 - 258, Replace plaintext
SharedPreferences usage for the 'manual_cookie' key with encrypted
FlutterSecureStorage: where you currently call SharedPreferences.getInstance()
and prefs.setString('manual_cookie', result.cookie!), instantiate a
FlutterSecureStorage (e.g., final storage = FlutterSecureStorage()) and use
storage.write(key: 'manual_cookie', value: result.cookie) and storage.read(key:
'manual_cookie') for reads; update all occurrences referenced (the spots using
SharedPreferences, prefs, setString and the 'manual_cookie' key at the other
locations) and add the import for flutter_secure_storage plus appropriate
async/error handling around read/write operations.
| : const Text('Get an SMS QR code'), | ||
| ), |
There was a problem hiding this comment.
Fix misleading button copy.
Line 476 says “Get an SMS QR code”, but this action sends an SMS verification code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/pages/login/login_page.dart` around lines 476 - 477, Update the
misleading button label in the login_page widget: replace the Text('Get an SMS
QR code') string with a clear action like Text('Send SMS verification code') (or
'Send SMS code') in the button widget so the UI reflects that the action sends
an SMS verification code rather than providing a QR code.
| import 'package:riverpod_annotation/riverpod_annotation.dart'; | ||
|
|
||
| import '../../widgets/lyric/mode/lyric_line.dart'; | ||
|
|
||
| part 'provider.g.dart'; |
There was a problem hiding this comment.
Dangling part directive will cause a compile-time error.
provider.g.dart was deleted as part of this PR, but the part 'provider.g.dart' directive still references it. Dart requires the part file to exist at compile time, so this will break the build. The riverpod_annotation import is also unused since no @riverpod annotations remain.
Remove both until the lyrics provider is restored:
🐛 Proposed fix
-import 'package:riverpod_annotation/riverpod_annotation.dart';
-part 'provider.g.dart';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import 'package:riverpod_annotation/riverpod_annotation.dart'; | |
| import '../../widgets/lyric/mode/lyric_line.dart'; | |
| part 'provider.g.dart'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/pages/play/provider.dart` around lines 2 - 3, Remove the dangling part
directive and unused import: delete the line containing part 'provider.g.dart'
and the import 'package:riverpod_annotation/riverpod_annotation.dart' from
provider.dart (or re-add them only if you restore the `@riverpod-annotated` lyrics
provider and its generated file); this eliminates the compile-time reference to
the deleted provider.g.dart and removes the unused riverpod_annotation import.
| git: | ||
| url: https://github.com/Zhenghao-Wen/bujuan_music_api.git | ||
| ref: login_test |
There was a problem hiding this comment.
Pin bujuan_music_api to a commit SHA, not a branch name.
ref: login_test tracks the tip of a mutable branch. Two builds at different times (or on different machines) can silently resolve to different commits, making builds non-reproducible and CI results unreliable. Once the companion bujuan_music_api PR is merged, replace the branch ref with the exact commit SHA (or a tag):
♻️ Proposed fix
bujuan_music_api:
git:
url: https://github.com/Zhenghao-Wen/bujuan_music_api.git
- ref: login_test
+ ref: <commit-sha-after-companion-pr-merge>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@pubspec.yaml` around lines 41 - 43, The git dependency entry referencing
bujuan_music_api currently uses a mutable branch ref ('ref: login_test') which
makes builds non-reproducible; update the pubspec.yaml git dependency (the block
with "git: url: https://github.com/Zhenghao-Wen/bujuan_music_api.git" and "ref:
login_test") to use the exact commit SHA (or a released tag) instead of the
branch name, then run pub get to refresh lockfiles and commit the updated
lockfile so CI uses the pinned commit.
2 participants
Description
This PR adapts the app to use the improved
bujuan_music_apisubmodule (see companion PR in that repo). Key changes:bujuan_music_apito the latest commit that includes:messagefield inLoginEntityfor better error feedback.qr_flutter).LoginEntity.message.SharedPreferencesto persist cookies from QR/manual login across app restarts.#1ED760) consistently (e.g., dialog borders and cursor).BujuanMusicManager.setCookie().shared_preferences: ^2.0.0inpubspec.yaml(was previously commented out).Testing
Notes
bujuan_music_api. It is recommended to merge that first, then update the submodule reference here to the merged commit.Please review. Thanks!
Actually, this is my first pull request and my fork may no longer be maintained.
Dont' forget check my PR of the submodule (bujuan_music_api)!
Summary by CodeRabbit
New Features
Bug Fixes
Chores