feat: add github action to release APK artifact#389
Conversation
|
Here's my review of PR #389: feat: add github action to release APK artifact. OverviewThis PR adds a new GitHub Actions workflow ( Issues1. Secret injection directly in shell commands (Security)Secrets are interpolated directly into # Current (risky)
- name: Create .env file
run: echo "${{ secrets.MOBILE_APP_ENV }}" > mobile-app/.env
# Recommended
- name: Create .env file
env:
MOBILE_APP_ENV: ${{ secrets.MOBILE_APP_ENV }}
run: echo "$MOBILE_APP_ENV" > mobile-app/.envSame applies to Note: The existing 2. No Rust/Cargo cachingThe Rust toolchain is set up with 4 Android targets ( - name: Cache Cargo
uses: actions/cache@v4
with:
path: |
~/.cargo/registry
~/.cargo/git
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
restore-keys: ${{ runner.os }}-cargo-3. Flutter version not pinned- name: Set up Flutter
uses: subosito/flutter-action@v2
with:
channel: "stable"
cache: trueThis uses whatever the latest stable Flutter version is at build time. Builds could break or behave differently when Flutter releases a new stable. Consider pinning a specific version ( 4. Deprecated release actions
- name: Create Release
uses: softprops/action-gh-release@v2
with:
tag_name: ${{ steps.get_version.outputs.tag }}
name: Quantus Wallet ${{ steps.get_version.outputs.version }}
files: ./artifacts/quantus_wallet_*.apkNote: The miner workflow uses the same deprecated actions, so this is an existing pattern. Could be a follow-up improvement for both workflows. 5. DRY: Duplicate version readingThe "Read version from pubspec.yaml" step is duplicated across build-android:
outputs:
version: ${{ steps.get_version.outputs.version }}
tag: ${{ steps.get_version.outputs.tag }}Then reference it in the release job as 6.
|
|
Here's my review of PR #389: feat: add github action to release APK artifact. OverviewThis PR adds The second commit ( What looks great
Issues to address1. Missing newline at end of fileThe diff ends with 2. Flutter version not pinned- name: Set up Flutter
uses: subosito/flutter-action@v2
with:
channel: "stable"
cache: trueThis uses whatever flutter-version: "3.29.2" # or whatever you're developing againstThis is a repo-wide issue (miner workflow too), but since this is a new workflow, it's a good opportunity to set the right precedent. 3. Cargo target directory not cachedThe cache covers - name: Cache Cargo
uses: actions/cache@v4
with:
path: |
~/.cargo/registry
~/.cargo/git
quantus_sdk/native/target
key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }}
restore-keys: |
${{ runner.os }}-cargo-The target directory can be large, so weigh the cache storage cost against build time savings. At minimum, worth testing once to see the impact. 4. All signing secrets exposed to every stepThe job-level - name: Decode Android keystore
if: inputs.build_mode == 'release'
env:
ANDROID_KEYSTORE_BASE64: ${{ secrets.ANDROID_KEYSTORE_BASE64 }}
run: echo "$ANDROID_KEYSTORE_BASE64" | base64 --decode > mobile-app/android/app/upload-keystore.jks
- name: Create key.properties
if: inputs.build_mode == 'release'
env:
ANDROID_STORE_PASSWORD: ${{ secrets.ANDROID_STORE_PASSWORD }}
ANDROID_KEY_ALIAS: ${{ secrets.ANDROID_KEY_ALIAS }}
ANDROID_KEY_PASSWORD: ${{ secrets.ANDROID_KEY_PASSWORD }}
run: |
cat > mobile-app/android/key.properties << EOF
storeFile=app/upload-keystore.jks
storePassword=$ANDROID_STORE_PASSWORD
keyAlias=$ANDROID_KEY_ALIAS
keyPassword=$ANDROID_KEY_PASSWORD
EOFThis is a "nice to have" — the current approach works and is simpler. But if a future step has a bug or runs untrusted code, scoped secrets reduce blast radius. Minor nits
VerdictSolid workflow that's already been improved from the first round of feedback. Items 1-2 (missing newline, unpinned Flutter) are quick fixes worth doing before merge. Items 3-4 (cargo target cache, scoped secrets) are optimizations that could be follow-ups. Good work! |
|
I would recommend doing 1, 2, and 3 rust compilation takes forever, so caching is good - although might also run out of disk space we will see Update: actually we always want to keep flutter to the latest, so yeah, not fixing flutter ... ;) |
Summary
I added github action to release APK artifact as downloadable. I have also added secrets variables needed in the workflow to the repo.