Repository navigation
Harden ARM64 APK signing password handling in Android build workflow - #65
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Co-authored-by: JWEB0689 <49634855+JWEB0689@users.noreply.github.com>
PR Reviewer Guide 🔍(Review updated until commit ff1a4d4)Here are some key observations to aid the review process:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
PR Summary by QodoValidate and reuse the ARM64 APK signing key password
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
There was a problem hiding this comment.
🟡 Changes recommended
JDK 21’s default PKCS12 behavior can ignore the selected key password, and the keystore password remains insufficiently validated.
2 open findings
What changed in this PR
Hardens ARM64 APK signing password selection and reuse.
Changes:
- Validates and masks an effective key password.
- Reuses it across keystore generation, Gradle, and APK signing.
| File | Description |
|---|---|
.github/workflows/android-build.yml |
Normalizes signing credentials across workflow steps. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if [ -z "$EFFECTIVE_KEY_PASSWORD" ] || [ ${#EFFECTIVE_KEY_PASSWORD} -lt 6 ]; then | ||
| echo "::error::Signing key password must be at least 6 characters. Check KEY_PASSWORD/KEYSTORE_PASSWORD secrets." | ||
| exit 1 | ||
| fi |
Code Review by Qodo
1. Multiline key passwords stop APK builds
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| exit 1 | ||
| fi | ||
| echo "::add-mask::$EFFECTIVE_KEY_PASSWORD" | ||
| echo "EFFECTIVE_KEY_PASSWORD=$EFFECTIVE_KEY_PASSWORD" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
1. Multiline key passwords stop apk builds 🐞 Bug ☼ Reliability
The keystore step writes EFFECTIVE_KEY_PASSWORD to $GITHUB_ENV using a single-line assignment, which cannot preserve a password containing a newline. If a valid signing secret contains a newline, the environment file is rejected or a truncated password reaches apksigner, while keytool created the key with the full password.
Agent Prompt
## Issue description
A valid signing password containing a newline cannot be exported using the new single-line `$GITHUB_ENV` assignment, so later signing cannot use the same password as keystore creation.
## Fix Focus Areas
- .github/workflows/android-build.yml[60-69]
- .github/workflows/android-build.yml[92-108]
## Recommended Fix
Write the resolved password using GitHub's multiline environment-file syntax with a delimiter that cannot occur in the value. Keep the downstream signing steps reading that resolved value.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Code Review Roast 🔥Verdict: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)
💀 Worst part: The incremental diff removed the exact 📊 Overall: Like a screen door on a submarine — technically it still seals, but it's missing the one part that makes the whole thing work. Files Reviewed (1 file)
Previous Review Summary (commit 56fb39c)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 56fb39c)Verdict: No Issues Found | Recommendation: Merge Oh wait, this PR is actually clean. I need to sit down. I had my flamethrower warmed up and everything. The fix properly handles the 📊 Overall: Like finding a unicorn in production — I didn't think clean PRs existed anymore, but here we are. Files Reviewed (1 file)
Reviewed by free · Input: 142K · Output: 43.9K · Cached: 93.2K |
| exit 1 | ||
| fi | ||
| echo "::add-mask::$EFFECTIVE_KEY_PASSWORD" | ||
| echo "EFFECTIVE_KEY_PASSWORD=$EFFECTIVE_KEY_PASSWORD" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
@copilot Fix the code for this review comment.
When a review comment includes a suggested change, apply the suggestion exactly.
Do not make changes beyond what is described in the linked review comment.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
Persistent review updated to latest commit ff1a4d4 |
PR Code Suggestions ✨Explore these optional code suggestions:
|
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| KEYSTORE_PASSWORD: ${{ secrets.KEYSTORE_PASSWORD }} | ||
| KEY_PASSWORD: ${{ secrets.KEY_PASSWORD || secrets.KEYSTORE_PASSWORD }} | ||
| run: | | ||
| EFFECTIVE_KEY_PASSWORD="$KEY_PASSWORD" |
There was a problem hiding this comment.
🔥 The Roast: This PR exists to "fall back to KEYSTORE_PASSWORD when KEY_PASSWORD is missing OR too short." The incremental diff just deleted the "too short" half. If KEY_PASSWORD is set to a short stub, GitHub's secrets.KEY_PASSWORD || secrets.KEYSTORE_PASSWORD binding still wins — it only falls back on empty/null, never on short-but-truthy — so the workflow now hard-fails with "must be at least 6 characters" instead of falling back to KEYSTORE_PASSWORD. You removed the exact safety net for the scenario you set out to fix.
🩹 The Fix: Restore the fallback before the guard:
EFFECTIVE_KEY_PASSWORD="$KEY_PASSWORD"
if [ -z "$EFFECTIVE_KEY_PASSWORD" ] || [ ${#EFFECTIVE_KEY_PASSWORD} -lt 6 ]; then
EFFECTIVE_KEY_PASSWORD="$KEYSTORE_PASSWORD"
fi
if [ -z "$EFFECTIVE_KEY_PASSWORD" ] || [ ${#EFFECTIVE_KEY_PASSWORD} -lt 6 ]; then
echo "::error::Signing key password must be at least 6 characters. Check KEY_PASSWORD/KEYSTORE_PASSWORD secrets."
exit 1
fi📏 Severity: warning
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
The fix has been applied successfully. Here's a summary of what was done:
Applied Fix
The Cloud Agent session modified .github/workflows/android-build.yml (lines 60-67) to restore the KEYSTORE_PASSWORD fallback logic that was previously removed. The keystore step now uses a two-check validation pattern as requested:
EFFECTIVE_KEY_PASSWORD=$KEY_PASSWORD
if [ -z $EFFECTIVE_KEY_PASSWORD ] || [ ${#EFFECTIVE_KEY_PASSWORD} -lt 6 ]; then
EFFECTIVE_KEY_PASSWORD=$KEYSTORE_PASSWORD
fi
if [ -z $EFFECTIVE_KEY_PASSWORD ] || [ ${#EFFECTIVE_KEY_PASSWORD} -lt 6 ]; then
echo ::error::Signing key password must be at least 6 characters. Check KEY_PASSWORD/KEYSTORE_PASSWORD secrets.
exit 1
fi
echo "::add-mask::$EFFECTIVE_KEY_PASSWORD"
echo "EFFECTIVE_KEY_PASSWORD=$EFFECTIVE_KEY_PASSWORD" >> "$GITHUB_ENV"Verification
- ✅ Fallback restoration: When
KEY_PASSWORDis empty or short, the code falls back toKEYSTORE_PASSWORDbefore erroring - ✅ Early fail: If neither password meets the 6-character minimum, the workflow fails with a clear error message
- ✅ Password masking:
EFFECTIVE_KEY_PASSWORDis masked with::add-mask::and exported via$GITHUB_ENVfor downstream reuse - ✅ Consistent usage: The resolved
EFFECTIVE_KEY_PASSWORDis used in keystore generation (keytool -keypass), the Gradle release build step (${{ env.EFFECTIVE_KEY_PASSWORD }}), and the apksigner step (${{ env.EFFECTIVE_KEY_PASSWORD }})
Per policy, no PR was created with this change — it was applied directly to the existing branch. You can verify the change in the PR thread.
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
User description
The
Build KernelCrafter ARM64 APKworkflow failed during keystore generation becauseKEY_PASSWORDcould be present but invalid forkeytool(under 6 chars). This change normalizes signing password selection so all signing stages use a valid, consistent value.Root cause addressed
KEY_PASSWORDdirectly, which can violatekeytoolminimum-length requirements and fail the job before APK build/sign completes.Workflow hardening
EFFECTIVE_KEY_PASSWORDderivation in the keystore step:KEY_PASSWORDwhen validKEYSTORE_PASSWORDwhen missing/too short$GITHUB_ENVfor downstream reuse.Cross-step signing consistency
apksignersteps to use the sameEFFECTIVE_KEY_PASSWORD, removing credential mismatch risk between keystore creation and signing.PR Type
Bug fix, Enhancement
Description
Derive and validate Android signing key password.
Ensure minimum 6-character password length.
Apply consistent password across all signing steps.
Mask sensitive password in workflow logs.
Diagram Walkthrough
flowchart TD A["Start"] --> B{Derive EFFECTIVE_KEY_PASSWORD}; B -- "Check KEY_PASSWORD" --> C{Is KEY_PASSWORD valid (>=6 chars)?}; C -- "No, too short/empty" --> D{Fallback to KEYSTORE_PASSWORD}; D --> E{Is EFFECTIVE_KEY_PASSWORD valid (>=6 chars)?}; E -- "No, too short/empty" --> F["Fail Workflow"]; E -- "Yes, valid" --> G["Mask & Export EFFECTIVE_KEY_PASSWORD"]; G --> H["Generate Keystore"]; H --> I["Build Release APK"]; I --> J["Locate, Align, Sign APK"];File Walkthrough
android-build.yml
Harden Android APK signing password handling in workflow.github/workflows/android-build.yml
EFFECTIVE_KEY_PASSWORDfromKEY_PASSWORDor
KEYSTORE_PASSWORD.minimum 6-character length requirement.
EFFECTIVE_KEY_PASSWORDand exported it toGITHUB_ENVforsecure and consistent reuse.
apksignersteps toconsistently use the derived
EFFECTIVE_KEY_PASSWORD.