ci: publish regtest apk on pr builds

[ovitrif]

Closes #1240

This PR:

1. Uploads the regtest debug universal APK built by the existing CI job as a 30-day workflow artifact whenever the build and unit tests pass, on every non-draft PR push and every push to master
2. Posts one sticky comment on the PR with the artifact link, built commit SHA, and run link, updated in place on later pushes

Description

• The CI build step already produces assembleDevDebug with the real google-services.json from GOOGLE_SERVICES_JSON_BASE64, so the artifact receives Firebase notifications like the E2E builds.
• Artifact name: bitkit-dev-debug-apk_<run_number>, only the *-universal.apk file is uploaded.
• The comment step is skipped for fork PRs, where the token cannot write comments.
• The build job stays read-only; a separate apk-comment job holds pull-requests: write and posts the comment from the build job's artifact URL output, so PR-controlled Gradle code never sees a writable token.

Preview

N/A — no user-visible changes.

QA Notes

Manual Tests

• 1. (after merge) Open a PR with a code change → CI run: bitkit-dev-debug-apk_<run> artifact appears and one Regtest APK comment links to it.
• 2. (after merge) Push again to the same PR: the same comment updates with the new SHA and artifact link, no second comment.
• 3. (after merge) Push a commit with a failing unit test: no APK artifact and no comment update, the unit test report is still uploaded.
• 4. (after merge) Merge to master → CI run for the merge commit: artifact present, no comment attempted.
• 5. (after merge) Install the artifact on a regtest device → receive a push notification: it arrives.

Automated Checks

• actionlint .github/workflows/ci.yml passes locally.
• Workflow behavior can only be verified on PRs opened after this merges (after merge).

🤖 Generated with Claude Code

[github-actions]

Regtest APK

Built from c89e9f7 (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

[greptile-apps]

Confidence Score: 3/5

This PR is not safe to merge until pull-request write access is isolated from PR-controlled Gradle execution.

Findings

1.   Writable Token Reaches PR Code ▶
2.  Sticky Lookup Is Truncated ▶

Summary

• Uploads the devDebug universal APK with 30-day retention.
• Creates or updates a marker-based APK comment for non-fork pull requests.
• Adds pull-request write permission to the existing build job.
• The write permission currently reaches PR-controlled Gradle execution and should be isolated before merging.

Diagram

sequenceDiagram
    participant PR as Pull request
    participant Build as Build job
    participant Gradle as PR-controlled Gradle code
    participant Artifacts as GitHub Artifacts
    participant API as GitHub PR API
    PR->>Build: Trigger workflow
    Build->>Gradle: assembleDevDebug with writable token
    Gradle-->>Build: Universal APK
    Build->>Artifacts: Upload APK
    Build->>API: Create or update sticky comment
    Build->>Gradle: Run unit tests

Loading

[jvsena42]

Could be triggered by a GH CI action to only spend resources when intended

[ovitrif]

Could be triggered by a GH CI action to only spend resources when intended

That's what I was prompting for, but the simpler path is hooking into the existing CI workflow: it already builds assembleDevDebug on every PR push, so this adds no build, only a 139 MB artifact upload (free on public repos) and a few-second comment job. A manual trigger would bring back the "someone has to dispatch it" step this PR removes.

[jvsena42]

Could be triggered by a GH CI action to only spend resources when intended

That's what I was prompting for, but the simpler path is hooking into the existing CI workflow: it already builds assembleDevDebug on every PR push, so this adds no build, only a 139 MB artifact upload (free on public repos) and a few-second comment job. A manual trigger would bring back the "someone has to dispatch it" step this PR removes.

Got it, so the resource is already used anyway

[jvsena42]

The security shape of this is right, and I checked the parts that usually go wrong on a publish-artifact workflow:

• Trigger is pull_request, not pull_request_target (:6), so fork PRs get a read-only token and no secrets — no fork privilege-escalation path.
• The privileged apk-comment job runs no contributor code: no actions/checkout, an inline github-script body, gated on head.repo.fork == false (:83), and HEAD_SHA/ARTIFACT_URL passed via env: and read through process.env rather than template-interpolated — so no script injection.
• The artifact really is regtest: assembleDevDebug → dev flavor → BuildConfig.NETWORK = "REGTEST" (app/build.gradle.kts:87, :333-337), .dev suffix, "Bitkit Regtest" name. No mainnet variant can reach it.
• No signing-key exposure — debug uses the checked-in debug.keystore with public credentials; the release keystore is a separate config this job never touches. GOOGLE_SERVICES_JSON_BASE64 in a published devDebug APK is pre-existing (e2e.yml:69, :86-89 already does it on every non-draft PR), not introduced here. CHATWOOT_API isn't consumed by any Gradle or Kotlin source, so it isn't baked in.

Requesting changes on one thing that will land after merge rather than here — details inline.

requested re-review; former changes addressed

[jvsena42]

Reviewed as a supply-chain surface, since this publishes a downloadable artifact built from PR code. No HIGH, no MEDIUM. One LOW inline, and it's arguably out of scope — your call.

The things that would have been serious, and why they aren't:

• Trigger is pull_request (:6, types [opened, synchronize, reopened, ready_for_review]). Not pull_request_target, no workflow_run. The new apk-comment job adds no trigger of its own and additionally gates on github.event_name == 'pull_request' (:86). No untrusted-code-with-secrets path.
• Fork PRs cannot reach secrets. Repo is public, so under pull_request from a fork secrets.* resolve empty and GITHUB_TOKEN is read-only. Concretely: GOOGLE_SERVICES_JSON_BASE64 (:54) empty → the decode step's if [ -z ... ]; then ... exit 0 (:47-50) takes the placeholder path instead of failing; CHATWOOT_API (:61) empty. And apk-comment is hard-gated on head.repo.fork == false (:86), so the pull-requests: write job never runs for a fork at all. Nothing in the diff makes a fork PR newly hang or fail.
• Permissions narrow rather than widen. There is no workflow-level permissions: block, so adding job-level blocks can only reduce. build gets contents: read + packages: read (:24-26); apk-comment gets only pull-requests: write (:89-90) — no contents, and it runs no actions/checkout, so it never materialises PR code. The greptile P1 (writable token reaching PR-controlled Gradle) is genuinely resolved by the split: the token handed to Gradle at :60 now lives in a job whose ceiling is read-only. Run 34413156506 at head 8688216 shows pull-requests: write alone is sufficient for paginate(listComments) + createComment.
• No untrusted interpolation. Grepped ${{ across the whole github-script body (:99-120) — zero hits. The only two interpolations in the new job are in env: (:96, :97) and read back as process.env.*, never template-expanded into shell or script context. Neither is attacker-controlled: artifact-url is emitted by actions/upload-artifact itself (PR-authored Gradle can only write to its own step's $GITHUB_OUTPUT under id build), and head.sha is 40 hex chars. No pull_request.title/body/head.ref/user.login anywhere in the file. The new job has no run: block at all.
• The artifact really is regtest — assembleDevDebug → dev flavour → .dev applicationId suffix, BuildConfig.NETWORK = "REGTEST". Debug signing uses the checked-in debug.keystore with public credentials; the release keystore is a separate config this job never references. No mainnet variant is reachable from this workflow.
• Artifact hygiene. bitkit-dev-debug-apk_${{ github.run_number }} (:79) is unique per run, so concurrent PRs can't collide, and it's distinct from unit_test_report_... and e2e.yml's bitkit_e2e.apk. retention-days: 30 (:81) matches the "expires in 30 days" text at :108.
• Skip paths are correct. Draft → build skipped → null artifact-url → apk-comment skipped. Build fails → upload skipped → empty output → no comment. Push to master → blocked by the event check.

Not raised, because they're settled or produce no wrong outcome: the sticky-comment staleness question (the body carries head sha + run link, which reads as the intended mitigation); if: always() at job level surviving a cancel-in-progress cancellation (traced it — the cancelled run can only comment when its build already succeeded, and the superseding run's comment lands after, so the final PR state is still the newest commit; !cancelled() would be tidier but changes no outcome). if-no-files-found: error and packages: read are both the outcomes of my earlier threads and are exactly the shape I asked for.

[jvsena42]

Fix confirmed at f85f88f4, and you went further than I asked — I think correctly.

I'd suggested only adding if: always() to Upload test report so a red test run still yields a report. You did that (:68, now always() && steps.build.outcome == 'success') and dropped the condition from Upload APK, so it falls back to the implicit success() and no APK is published from a run whose tests failed. That's the better call: the asymmetry I flagged was "the wrong artifact survives a red run", and withholding the APK entirely resolves it more cleanly than publishing both.

Checked the knock-on effects:

• Test report on a red run: build succeeds → tests fail → job status is failure → always() keeps the step alive, and steps.build.outcome == 'success' still holds, so app/build/reports/tests/testDevDebugUnitTest/ uploads. If the build fails there are no tests to report, and the guard correctly skips. That's the case I raised.
• apk-comment losing always() (:86) is consistent, not a regression. artifact-url is only non-empty when Upload APK ran, which now requires the job to have succeeded — so needs.build.outputs.artifact-url != '' alone can never be true on a failed build. Dropping always() also removes the cancellation edge I'd noted as cosmetic last round, where a cancelled run could still comment. Now it can't.
• Draft and fork paths unchanged: draft → build skipped → null output → job skipped; fork still hard-gated on head.repo.fork == false.

Nothing else in the two commits. That was my only finding on this PR — clean from my side.

[jvsena42]

Re-checked at b7a3e1a6. The only delta since I confirmed the fix is the master merge — git diff f85f88f4 b7a3e1a6 -- .github/workflows/ci.yml is empty, so the reviewed content is byte-identical and git show --cc shows no conflict resolution in that file.

The PR's own delta against master is still just the +56-line APK job. Nothing to re-review; my previous confirmation stands.

One note, not a finding: this branch merged a master that predates #1216, so git diff origin/master b7a3e1a6 currently shows the Paykit issuer-interop files as deletions. That's just being behind, not a removal — mergeable=MERGEABLE, so it'll resolve on the next sync or at merge time. Flagging only so the diff view doesn't alarm anyone reading it.

[jvsena42]

Re-checked at 820e73c2. Master merge only — git diff b7a3e1a6 820e73c2 -- .github/workflows/ci.yml is empty, and git show --cc 820e73c2 has no combined-diff lines, so nothing was resolved by hand.

The PR's own delta against master is now exactly .github/workflows/ci.yml | 56 ++++ and nothing else — the Paykit files that showed as deletions last time are gone from the diff, as expected once this branch caught up with #1216. That note is resolved.

Nothing to re-review; my confirmation stands.

[piotr-iohk]

Just to note every e2e run already has staging regtest apk as artifact, see: e.g. https://github.com/synonymdev/bitkit-android/actions/runs/34602699398 (Artifacts: bitkit-e2e-apk-regtest_4563)