feat(controller): OTA updater security + correctness redesign (F15, PR A)#30
Merged
Merged
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
…R A) The in-app OTA self-updater downloaded the new APK to public Downloads and installed it with no integrity check. DownloadManager reports "complete" even when the server returned an HTML/text error page, so a wrong or MITM'd response was installed as garbage (the "downloaded a text file" bug); the download receiver was also registered EXPORTED. PR A — security + functional correctness, as a layered `update` slice: - domain: UpdateCheck (version rule) and CertDigests.sameSigner — pure JVM, unit-tested (UpdateCheckTest, CertDigestsTest). - data/ApkVerifier: the downloaded APK must be signed by the SAME certificate as the running app (public certs via PackageManager — no secrets, no server change). Rejects MITM/tampered APKs and non-APK downloads (kills the text-file bug), handling API <28 (GET_SIGNATURES) vs >=28 (GET_SIGNING_CERTIFICATES). - MainActivity seam: stage the APK in the app's PRIVATE external dir (not public Downloads); only install when the DownloadManager status is SUCCESSFUL; verify the signature before install (delete + clear error on failure); handle the API 26+ "install unknown apps" permission; register the completion receiver NOT_EXPORTED. New strings (en + es); REQUEST_INSTALL_PACKAGES permission. Out of scope (follow-ups): PR B (UpdateViewModel + in-app download-progress UX) and a network-security-config to scope cleartext to the local box hosts (S18), deferred to avoid risking box connectivity.
luisguzman-adfa
force-pushed
the
feat/ota-updater-security-redesign
branch
from
899e43f to
0c69557
Compare
luisguzman-adfa
added a commit
that referenced
this pull request
Jun 23, 2026
…ress (PR B) Presentation/data slice for PR B (on top of PR A #30), behavior-neutral until wired: - domain: OtaDownloadGateway port (query progress / cancel). - data: DownloadManagerGateway (queries DownloadManager COLUMN_STATUS / bytes; remove() to cancel). - presentation: UpdateUiState (IDLE/DOWNLOADING/VERIFYING/READY/INSTALLING/ERROR + percent, with pure fromDownload mapping) + UpdateViewModel (polls ~400ms while downloading; cancel) + UpdateViewModelFactory. Unit-tested (UpdateUiStateTest + DownloadProgressTest). Next: modal progress dialog layout + MainActivity wiring (keep system notification).
luisguzman-adfa
added a commit
that referenced
this pull request
Jun 24, 2026
- Status banner: Phase 0 done + Phase 1 core-complete; Phase 2 not started (~37 raw threads); Phase 3 partial (slices land but god classes grew — DeployFragment ~3022, MainActivity ~2384); M15 done, M8/M9 open. - Phase 1 line: F15 A+B (#30/#32) + rootfs validation (#31) + integrity (#37) done. - Integrity/writer entry: marked DONE via #37 (was 'next PR'). - F4 + M7: confirmed mitigated by inspection (onPause teardown + remove-before-post). - Lint workaround: added root-cause analysis with concrete carve targets (addNewTerminalSession ~727 LOC; DeployFragment bind* methods) for D1/F1. Docs only; no code change.
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
First of two PRs redesigning the in-app OTA self-updater (tech-debt F15). The updater downloaded the APK to public Downloads and installed it with no integrity check;
DownloadManagerreports "complete" even when the server returns an HTML/text error page, so a wrong/MITM'd response got installed as garbage (the "downloaded a text file" bug). The completion receiver was also registered EXPORTED.PR A = security + functional correctness. PR B (separate) adds the
UpdateViewModel+ in-app download-progress UX.Changes (layered
org.iiab.controller.updateslice)domain/(pure JVM, unit-tested):UpdateCheck(is the server build newer?) andCertDigests.sameSigner(signer-set comparison).data/ApkVerifier: the downloaded APK must be signed by the same certificate as the running app — public certs viaPackageManager(no private key, no secret, no server change), handling API <28 (GET_SIGNATURES) vs ≥28 (GET_SIGNING_CERTIFICATES). Rejects MITM/tampered APKs and non-APK downloads (kills the text-file bug).MainActivityseam: stage the APK in the app's private external dir (not public Downloads); install only when theDownloadManagerstatus is SUCCESSFUL; verify the signature before install (delete + clear error on failure); handle the API 26+ "install unknown apps" permission; register the receiver NOT_EXPORTED. New strings (en + es);REQUEST_INSTALL_PACKAGES.How the signature check works (no secrets)
Every APK embeds its public signing certificate. We read the running app's cert and the downloaded APK's cert via
PackageManagerat runtime and compare — Android already requires an update to be signed by the same key, so we just enforce it early with a clean error. No GitHub Actions secret / private key involved.Testing
UpdateCheckTest+CertDigestsTest(pure JVM, CI gate). TheApkVerifier/MainActivityparts are Android-bound — validated by inspection + CI compile and on-device (a text/HTML response is now rejected with a clear message instead of installing garbage).Out of scope (follow-ups)
UpdateViewModel+ in-app progress bar / cancel / state UX.network-security-configto scope cleartext to the local box hosts (S18) — deferred to avoid risking box connectivity; the updater URLs are HTTPS so this PR doesn't depend on it.