Restore release signing via CI-injected keystore (our own key)#48
Merged
Conversation
…when absent The inherited release.keystore was removed from the repo (gitignored). Wire CI to recreate it from a new KEYSTORE_B64 secret (decoded to release.keystore before assembleOssRelease in build.yml/release.yml), and make Helpers.kt only enable release signing when the keystore file exists AND a non-blank password is set, so debug / PR / keyless builds skip signing instead of failing on the missing file. Signing verified end-to-end locally with a freshly generated 4096-bit RSA key (CN=NekoBox, OU=hawkff): assembleOssRelease produces an APK signed with our key (apksigner cert SHA-256 matches the keystore). GitHub secrets KEYSTORE_B64/ KEYSTORE_PASS/ALIAS_NAME/ALIAS_PASS set on the repo.
📝 WalkthroughWalkthroughThe PR introduces conditional release signing: ChangesKeystore Secret Provisioning and Signing Guard
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@buildSrc/src/main/kotlin/Helpers.kt`:
- Line 127: The canSign variable on line 127 only validates keystorePwd and
releaseKeystore existence, but the signing configuration block unconditionally
uses alias and pwd variables which may be null or blank, causing cryptic Gradle
failures. Update the canSign predicate to also check that alias and pwd are not
null or blank, ensuring all required signing credentials are validated before
proceeding with the signing configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea1696af-9a71-4fbc-b347-3c5d8e6fe522
📒 Files selected for processing (4)
.github/workflows/build.yml.github/workflows/release.yml.gitignorebuildSrc/src/main/kotlin/Helpers.kt
…ord) Per Greptile: canSign only checked the keystore file + store password, so if ALIAS_NAME/ALIAS_PASS were absent while the others were present, Gradle would build a signing config with null alias/password and fail at packaging with an opaque error. Require all of keystore-exists + storePass + alias + keyPass non-blank before enabling release signing; otherwise skip cleanly.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Restores release signing after the inherited
release.keystorewas removed from the repo(PR #44), using a CI-secret-injected keystore and our own freshly generated key.
Background
release.keystore(inherited from the upstream fork) was removed from HEAD and gitignored, butnothing recreated it at build time — so
assembleOssRelease(build.yml / release.yml) would failat packaging on the missing file. PR CI stayed green only because it builds debug. We're now
maintaining our own fork, so this uses a new key we generated.
Changes
buildSrc/Helpers.kt— only enable release signing when the keystore file exists AND anon-blank password is provided. Debug / PR / keyless builds skip signing cleanly instead of
failing on the missing file (empty-string CI secrets count as absent).
.github/workflows/build.yml,release.yml— decoderelease.keystorefrom a newKEYSTORE_B64secret beforeassembleOssRelease(base64 -d), guarded so it's a no-op whenthe secret is unset.
.gitignore— explicitly ignorebuildSrc/build(it's sometimes a symlink to APFS /tmp asthe exFAT AppleDouble build-failure workaround; the
build/rule doesn't match a symlink).Key + secrets (handled out of band)
CN=NekoBox, OU=hawkff, 30y validity, stored locally at~/.nekobox-signing/(mode 600, never committed).KEYSTORE_B64,KEYSTORE_PASS,ALIAS_NAME,ALIAS_PASS.Testing
assembleOssReleaseproducesNekoBox-…-arm64-v8a.apksigned with our key —apksigner verifyshowsCN=NekoBox, OU=hawkffand cert SHA-25613a113a3…5865add, matching the keystore fingerprint.Caveat (separate, pre-existing — not in this PR)
assembleOssReleasecurrently also fails lint on a pre-existingMissingDefaultResourceerror (
dracula_backgroundinvalues-night/colors.xml, from the Dracula theme #38) — unrelatedto signing. That should be fixed separately before a real release build, or it will block
build.yml/release.yml regardless of signing.
Greptile Summary
This PR restores release APK signing by decoding the keystore from a
KEYSTORE_B64CI secret at build time and tightening the Gradle signing guard to require all four credentials to be present and non-blank before wiring up a signing config.buildSrc/Helpers.kt: replaces the loosekeystorePwd != nullcheck with acanSignguard that verifies the keystore file exists and thatKEYSTORE_PASS,ALIAS_NAME, andALIAS_PASSare all non-blank — addressing the null-alias crash flagged in the prior review.build.yml/release.yml: adds a guardedbase64 -dstep to materializerelease.keystorefromKEYSTORE_B64before the Gradle release task, with a no-op when the secret is absent; also corrects the${{ secrets.ALIAS_PASS }}spacing inconsistency from the previous code..gitignore: explicitly ignoresbuildSrc/buildto cover the macOS exFAT symlink edge-case where the genericbuild/rule does not match a symlink.Confidence Score: 5/5
Safe to merge — the signing guard is correct and complete, the keystore decode is a no-op when the secret is absent, and all issues from prior review rounds have been resolved.
All four credentials are now validated before a signing config is created, preventing the null-alias build failure. The base64 -d decode is properly guarded by a non-empty check, so PR/debug builds without the secret continue to work. No new defects were introduced.
No files require special attention.
Important Files Changed
keystorePwd != nullguard with a four-conditioncanSigncheck (file exists, password, alias, and alias-password all non-blank), fixing both the missing-file crash and the null alias issue flagged in prior review.base64 -ddecode step forKEYSTORE_B64before the release Gradle task; also fixes the missing space in${{ secrets.ALIAS_PASS }}from the old code.buildSrc/buildentry to handle the macOS exFAT symlink edge-case where the generic trailing-slashbuild/rule does not match a symlink.Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant GH as GitHub Actions participant Shell as Runner Shell participant Gradle as Gradle / Helpers.kt participant APK as APK Signer GH->>Shell: Expand secrets (KEYSTORE_B64, KEYSTORE_PASS, ALIAS_NAME, ALIAS_PASS) Shell->>Shell: if [ -n "$KEYSTORE_B64" ] alt secret present Shell->>Shell: "echo base64 | base64 -d > release.keystore" else secret absent Shell->>Shell: skip (no-op) end Shell->>Gradle: assembleOssRelease Gradle->>Gradle: "canSign = file.exists() && !pwd.isNullOrBlank() && !alias.isNullOrBlank() && !aliasPwd.isNullOrBlank()" alt "canSign = true" Gradle->>Gradle: create signingConfig("release") Gradle->>APK: sign APK with release key else "canSign = false" Gradle->>APK: produce unsigned APK end APK-->>GH: upload artifact%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant GH as GitHub Actions participant Shell as Runner Shell participant Gradle as Gradle / Helpers.kt participant APK as APK Signer GH->>Shell: Expand secrets (KEYSTORE_B64, KEYSTORE_PASS, ALIAS_NAME, ALIAS_PASS) Shell->>Shell: if [ -n "$KEYSTORE_B64" ] alt secret present Shell->>Shell: "echo base64 | base64 -d > release.keystore" else secret absent Shell->>Shell: skip (no-op) end Shell->>Gradle: assembleOssRelease Gradle->>Gradle: "canSign = file.exists() && !pwd.isNullOrBlank() && !alias.isNullOrBlank() && !aliasPwd.isNullOrBlank()" alt "canSign = true" Gradle->>Gradle: create signingConfig("release") Gradle->>APK: sign APK with release key else "canSign = false" Gradle->>APK: produce unsigned APK end APK-->>GH: upload artifactReviews (3): Last reviewed commit: "chore(ci): normalize spacing in ALIAS_PA..." | Re-trigger Greptile