From be8efe01cc2980171bf2d949f6f96043d782fa33 Mon Sep 17 00:00:00 2001 From: Thomas Kroll Date: Fri, 13 Nov 2026 14:03:21 +0100 Subject: [PATCH] feat: offer updates as a browser download instead of installing them inside the app The GitHub build keeps the update check (latest release, pre-release switch, Update available banner and sheet) but no longer installs anything itself. Download sends the browser to the release's APK (the release page when the release has no APK), a separate link opens the release page, and the sheet shows the file's SHA-256 when GitHub lists one. The browser is only sent to HTTPS links under this repository's GitHub releases. REQUEST_INSTALL_PACKAGES, the PackageInstaller session, the status receiver, the install-permission flow, the in-app APK download and their tests are removed, and the main manifest strips the permission from every flavor and library; a unit test fails if it comes back. Why: on 2026-11-13 Google Play Protect blocked every GitHub build that could install packages, and the CI build of this change (same version and key, no permission) was rated "This app seems safe" on the same phone, see ADR-0046 (Accepted). The store flavor behaves as before. README and website explain the Play Protect dialogs and how to verify a download; ADR-0008, ADR-0034 and ADR-0040 point to ADR-0046. Closes #306, closes #307 Co-Authored-By: Claude Sonnet 5.5 --- CHANGELOG.md | 19 + CONTRIBUTING.md | 2 +- HANDOFF.md | 11 +- README.md | 20 +- app/src/direct/AndroidManifest.xml | 8 - app/src/main/AndroidManifest.xml | 24 +- .../nofuturekid/nova/data/model/UpdateInfo.kt | 49 +- .../nova/data/update/ApkInstaller.kt | 49 -- .../nova/data/update/InstallStatusReceiver.kt | 105 ----- .../nova/data/update/UpdateController.kt | 135 ------ .../nova/data/update/UpdateInstaller.kt | 151 ------ .../nova/data/update/UpdateRepository.kt | 91 ++-- .../nofuturekid/nova/di/UpdateModule.kt | 25 - .../nova/ui/screens/main/MainScreen.kt | 17 +- .../nova/ui/screens/main/MainViewModel.kt | 31 +- .../ui/screens/settings/SettingsScreen.kt | 45 +- .../nova/ui/screens/update/UpdateBanner.kt | 2 +- .../nova/ui/screens/update/UpdateDialog.kt | 169 +++---- .../nova/NoInstallPermissionTest.kt | 51 ++ .../repository/TransientErrorToleranceTest.kt | 2 +- .../nova/data/update/UpdateControllerTest.kt | 436 ------------------ .../nova/data/update/UpdateRepositoryTest.kt | 159 +++++++ .../ui/screens/main/StaleCrossServerTest.kt | 3 +- .../0008-in-app-updater-packageinstaller.md | 6 +- .../0034-update-and-data-at-rest-hardening.md | 6 +- .../0040-fdroid-play-distribution-flavors.md | 6 +- docs/adr/0046-update-via-browser-download.md | 65 +++ docs/adr/README.md | 7 +- docs/privacy/POLICY.md | 15 +- site/index.html | 75 ++- site/style.css | 1 + 31 files changed, 612 insertions(+), 1173 deletions(-) delete mode 100644 app/src/direct/AndroidManifest.xml delete mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/update/ApkInstaller.kt delete mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/update/InstallStatusReceiver.kt delete mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateController.kt delete mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateInstaller.kt delete mode 100644 app/src/main/kotlin/io/github/nofuturekid/nova/di/UpdateModule.kt create mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/NoInstallPermissionTest.kt delete mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/data/update/UpdateControllerTest.kt create mode 100644 app/src/test/kotlin/io/github/nofuturekid/nova/data/update/UpdateRepositoryTest.kt create mode 100644 docs/adr/0046-update-via-browser-download.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 00b0cf12..d93c8d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,25 @@ release version + date and opens a fresh `[Unreleased]`. ## [Unreleased] +### Changed +- **Updates are now downloaded in your browser instead of being installed inside + NOVA** — NOVA still tells you when a new version is out and shows its release + notes, the file size and its SHA-256. "Download" now lets your browser download + the new APK; you install it by opening the downloaded file, and Android may ask + you once to allow your browser or file manager to install apps. NOVA no longer + asks for, or holds, the permission to install apps itself. Why: on a real phone + Google Play Protect stopped every current NOVA build with "Harmful app blocked" + (you could only continue through "More details > Install anyway"), while a build + of this version without that permission was rated "This app seems safe". Play + Protect may still offer to scan each new version before it installs; that is + expected. Versions up to 0.1.46 still install updates themselves, so the update + to this version can show the warning one last time (#306, #307). + +### Added +- **The Play Protect notice is documented** — the README and the website now + explain what Google Play Protect shows when you install or update NOVA, why, and + how to check a download (SHA-256 and signing certificate). + ## [0.1.46] - 2026-11-13 Adding a self-signed server now asks about its certificate only once, and the Overview shows dashes instead of zeros when it has no data. The 0.1.46 beta confirmed this on a real phone: the trust question appears once, the Overview shows dashes while offline, Uptime is a duration and disk rows have no dangling separator. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 77a0cd2d..3ddec84f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -99,7 +99,7 @@ Trailing digit is required (`v0.2.0-beta` without a number is rejected by the re | New end-to-end feature touching install + UI flow | Beta-first required (ADR-0006). | | Anything affecting how the app talks to the Unraid server | Beta-first required (ADR-0006). | -When in doubt: beta-first. The in-app updater (ADR-0008) means betas reach the dev's device with no extra friction. +When in doubt: beta-first. The update notice (ADR-0046) announces each beta to the dev's device and sends the browser to the APK, so betas reach it with little friction. ### Cleaning up old releases diff --git a/HANDOFF.md b/HANDOFF.md index 0485bd0c..45c09df0 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -97,7 +97,7 @@ _Toolchain raised in the 0.1.28 cycle — see ADR-0023 (AGP-9) / ADR-0024 - Coil **3.6.3** for AsyncImage - okhttp **5.5.1**, kotlinx.coroutines **1.11.0**, serialization **1.13.0** - Renovate (gradle + github-actions, monthly, no auto-merge; `renovate.json`); Dependabot only for security alerts and security updates (ADR-0025) -- Distribution flavors `direct` (GitHub APK; declares `REQUEST_INSTALL_PACKAGES`, in-app updater enabled) and `store` (the F-Droid build; updater disabled at runtime via `BuildConfig.HAS_UPDATER = false`) — ADR-0040 +- Distribution flavors `direct` (GitHub APK; update check enabled, updates are downloaded in the browser, ADR-0046) and `store` (the F-Droid build; updater disabled at runtime via `BuildConfig.HAS_UPDATER = false`) — ADR-0040 - Tests: JUnit 4; Robolectric 4.17 + Roborazzi 1.78 only for the store screenshots (`-PnovaScreenshots`); Gradle wrapper pinned by `distributionSha256Sum` ## Layout @@ -121,9 +121,8 @@ app/src/main/ │ ├── repository/ UnraidRepository (domain streams: polling + subscriptionOrPoll, mutations via runAction, logs), │ │ ServerRepository (CRUD + active server), SettingsRepository, │ │ DomainState, Nudge (refresh nudges), ResponseClassifier - │ └── update/ UpdateRepository, UpdateInstaller, UpdateController, ApkInstaller, InstallStatusReceiver - │ (compiled into both flavors; disabled at runtime when BuildConfig.HAS_UPDATER is false) - ├── di/ AppModule (empty, constructor injection), UpdateModule + │ └── update/ UpdateRepository (compiled into both flavors; disabled at runtime when BuildConfig.HAS_UPDATER is false) + ├── di/ AppModule (empty, constructor injection) └── ui/ ├── UnraidApp.kt ├── theme/ Color, Density, Dimens, Shapes, Alpha, Type, Theme, ThemeMode (CompositionLocals) @@ -232,7 +231,7 @@ mapScalar("URL", "kotlin.String") mapScalar("Port", "kotlin.Int") ``` -JSON-scalar decision rationale (incl. why-not-String): [ADR-0007](docs/adr/0007-apollo-json-scalar-anyadapter.md). In-app updater architecture: [ADR-0008](docs/adr/0008-in-app-updater-packageinstaller.md). +JSON-scalar decision rationale (incl. why-not-String): [ADR-0007](docs/adr/0007-apollo-json-scalar-anyadapter.md). Update notice and browser download: [ADR-0046](docs/adr/0046-update-via-browser-download.md) (the earlier in-app installer, [ADR-0008](docs/adr/0008-in-app-updater-packageinstaller.md), is superseded in part). ### Critical gotchas (don't relearn the hard way) @@ -244,7 +243,7 @@ JSON-scalar decision rationale (incl. why-not-String): [ADR-0007](docs/adr/0007- - **KSP2 + Hilt 2.59.2** (ADR-0020/0023): the old `ksp.useKSP2=false` / Hilt-2.52 pin is gone. Hilt requires Kotlin ≥ 2.1.10 — it is coupled to the toolchain (ADR-0023 rollback note: Hilt can't be reverted in isolation). - **`DockerContainer.mounts` is `JSON` (single scalar) NOT `[JSON!]`** — the JSON content itself is an encoded array of mount objects. Easy to mistype based on intuition; v0.1.11 made the snapshot fail server-wide because of this exact wrong type. Parse via `parseMountsArray` in `GraphQlMapper.kt`. - **JSON scalar → kotlin.Any via Apollo's `AnyAdapter`** — the Unraid GraphQL `JSON` scalar serialises as inline JSON values (objects/arrays/primitives), not pre-stringified text. Custom `kotlin.String` adapters crash. We map it to `kotlin.Any` with `com.apollographql.apollo.api.AnyAdapter` (public, built-in). Don't try to write your own with `reader.readAny()` — that's `@ApolloInternal`. -- **In-app updater** lives under `data/update/`: `UpdateRepository` polls `https://api.github.com/repos/hellwand/nova/releases` (installed versions up to 0.1.42 still carry the old `nofuturekid/nova` name and reach it through GitHub's redirect; no auth, 60 req/h rate limit fine for on-start checks), `UpdateInstaller` downloads APK to `cacheDir/updates/` and uses `PackageInstaller` for the install handshake, `InstallStatusReceiver` catches the session callback via a `BroadcastReceiver` declared in the manifest. The `REQUEST_INSTALL_PACKAGES` permission is declared in the `direct` flavor manifest only (`app/src/direct/AndroidManifest.xml`); the updater code itself is compiled into both flavors and disabled at runtime when `BuildConfig.HAS_UPDATER` is false (`store`). On first install Android prompts the user to whitelist us at `Settings.ACTION_MANAGE_UNKNOWN_APP_SOURCES`. The Settings screen exposes the "Include pre-releases" toggle + manual "Check now". Banner on MainScreen dismisses per-tag via `dismissedUpdateTag` in DataStore. +- **Update check** lives under `data/update/`: `UpdateRepository` polls `https://api.github.com/repos/hellwand/nova/releases` (installed versions up to 0.1.42 still carry the old `nofuturekid/nova` name and reach it through GitHub's redirect; no auth, 60 req/h rate limit fine for on-start checks), `UpdateDialog` only sends the browser to the release's APK (or the release page when it has no APK) with `ACTION_VIEW`; the app downloads and installs nothing itself and declares no `REQUEST_INSTALL_PACKAGES` in any flavor (ADR-0046). The check is compiled into both flavors and disabled at runtime when `BuildConfig.HAS_UPDATER` is false (`store`). The Settings screen exposes the "Include pre-releases" toggle + manual "Check now". Banner on MainScreen dismisses per-tag via `dismissedUpdateTag` in DataStore. - Cleartext HTTP requires `android:usesCleartextTraffic="true"` in the manifest. LAN access against `http://192.168.x.x` needs this. The base-config in `network_security_config.xml` permits cleartext; the GitHub hosts (`api.github.com`, `github.com`, `objects.githubusercontent.com`) are pinned to HTTPS. - **CI `test` job (ADR-0036) can transiently fail at Gradle _configuration_** with a plugin-not-resolvable error (e.g. `com.google.devtools.ksp` not found) on a plugin-portal/registry blip — it fetches plugins fresh under PR `cache-read-only`. **Re-run the job; it is not a code defect** (see ADR-0036 "Known consequence — transient plugin-fetch in the `test` job" note; PR #163 2026-05-19). diff --git a/README.md b/README.md index 3093b83c..99b8e261 100644 --- a/README.md +++ b/README.md @@ -77,7 +77,7 @@ A few things bend to taste in **Settings**: ## Getting started 1. **Install** NOVA — either from [F-Droid](https://f-droid.org/packages/io.github.nofuturekid.nova/) (stable releases only) or as the latest APK from the [Releases page](https://github.com/hellwand/nova/releases). (For the APK, Android will ask you to allow installing from your browser/files app — that's expected for apps outside the Play Store.) - - Betas are GitHub pre-releases, so they come as an APK from the Releases page. The in-app updater keeps it current when **Include pre-releases** is switched on in Settings; Obtainium works too. + - Betas are GitHub pre-releases, so they come as an APK from the Releases page. NOVA tells you when a new version is out (switch on **Include pre-releases** in Settings for betas) and sends your browser to the APK; Obtainium works too. - The F-Droid build and the GitHub APK are signed with different keys, so switching between them needs an uninstall first (your saved servers and API keys go with the app). See [ADR-0040](./docs/adr/0040-fdroid-play-distribution-flavors.md). 2. On your Unraid server, generate an **API key**: web UI → _Settings → Management Access → API / Connect_. 3. Open the app → **Add server** and fill in: @@ -87,6 +87,22 @@ A few things bend to taste in **Settings**: - **API Key** — the one you just generated 4. Tap **Test** (next to the local and/or remote connection), then **Save**. The pill in the top bar flips between **Local** and **Remote**. +### Google Play Protect + +**Versions up to 0.1.46 (GitHub APK).** On 2026-11-13 we observed that Google Play Protect stopped every GitHub build we tried (0.1.45-beta1, 0.1.45 and 0.1.46-beta1) when it was installed or updated, on a phone with Android 9 and on one with Android 17. Two dialogs appear one after the other: + +1. **"App scan recommended"** (German: "App-Scan empfohlen") — Play Protect does not know the file yet. Android shows this for every new file that is not from Google Play. Tap **Scan app** ("App scannen"). +2. **"Harmful app blocked"** (German: "Schädliche App blockiert"), "This app may be harmful" ("Diese App könnte schädlich sein."). You can continue through **More details > Install anyway** ("Weitere Details > Trotzdem installieren"). + +This is Google's rating of the file, not a finding about what NOVA does; Google does not say why it gives it. Our observation, which Google has not confirmed: these builds could install other apps (that is how the old in-app updater worked), and a build of the same version without that ability was rated "This app seems safe" ("Diese App ist anscheinend sicher") by the same scan on the same phone. NOVA is open source (GPL v3), and you can check what you installed: + +- **SHA-256 of the file:** the release page lists it next to the APK; compare it with `sha256sum NOVA-v….apk`. +- **Signing certificate:** `apksigner verify --print-certs NOVA-v….apk` should show the SHA-256 digest `9f86baa17c1208004d9284e99597f77276c77301b06340b10075c793152b6824`. + +**From the next version on**, NOVA no longer installs updates itself and no longer asks for the permission to install apps. **Download** in the update sheet lets your browser download the file; you open it to install. Android will still ask once to scan each new file ("App scan recommended"), because Play Protect does not know it yet; that is expected. We cannot promise that no warning appears, since Google's rating can change. + +**F-Droid** is the channel without any such prompt we have seen, but it has stable releases only, gets updates later, and is signed with a different key. Switching between the GitHub APK and F-Droid means uninstalling first, which removes your saved servers and API keys. + Requires an Unraid 7.x server with the API/Connect plugin enabled. All app features need Unraid API **4.30.0 or newer**; the live network speed needs **4.35.0 or newer** (API versions, not Unraid OS versions). On an older API some screens fail to load. ## Trademark attribution @@ -98,7 +114,7 @@ Unraid® is a registered trademark of Lime Technology, Inc. NOVA is an independe - **Full policy:** - **Your API key stays on your device.** It's encrypted at rest using Android Keystore via Google's Tink library, and explicitly excluded from cloud backup / device-transfer. - **No analytics, no telemetry, no tracking services.** The app talks to your Unraid server's API; in the GitHub (`direct`) build it also asks GitHub's API for app updates (at app start, when you open Settings, or on "Check now"); and it loads container icons from the URLs your own server supplies (often GitHub or a CDN). -- **Update downloads** go directly from GitHub Releases to your device over HTTPS. The downloaded APK is verified by byte-size match and constant-time SHA-256 against the published release asset before any install attempt; mismatch fails closed. See [ADR-0034](./docs/adr/0034-update-and-data-at-rest-hardening.md) for the rationale. +- **Update downloads** are made by your browser, not by NOVA: the update sheet opens the release's APK on GitHub, shows its SHA-256 so you can compare it with the downloaded file, and the app itself downloads and installs nothing. See [ADR-0046](./docs/adr/0046-update-via-browser-download.md). - **Source is the spec.** Audit, fork, run yourself — GPL v3. ## Contributing & internals diff --git a/app/src/direct/AndroidManifest.xml b/app/src/direct/AndroidManifest.xml deleted file mode 100644 index a2b4cd73..00000000 --- a/app/src/direct/AndroidManifest.xml +++ /dev/null @@ -1,8 +0,0 @@ - - - - - diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 4980bed6..3c27b3f4 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -1,11 +1,18 @@ - + - + + - - - - - - - diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/model/UpdateInfo.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/model/UpdateInfo.kt index 99ade0c8..a523f840 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/model/UpdateInfo.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/model/UpdateInfo.kt @@ -5,19 +5,44 @@ data class UpdateInfo( val version: String, // e.g. "0.1.16" (no leading v) val tag: String, // e.g. "v0.1.16" val releaseUrl: String, // browser URL of the GitHub Release page - val downloadUrl: String, // direct APK asset URL + /** The release's APK file; null if the release has no APK attached. */ + val apk: ApkAsset? = null, + val isPrerelease: Boolean, + val releaseNotes: String, + /** Release publish time as epoch millis; null if GitHub didn't supply it. */ + val publishedAtEpochMs: Long? = null, +) { + /** + * What "Download" opens in the browser: the APK file itself, or the + * release page when the release has no APK attached (ADR-0046). The + * app never downloads or installs the file itself. + */ + val downloadUrl: String get() = apk?.url ?: releaseUrl +} + +/** The APK file of a GitHub release, as listed by the Releases API. */ +data class ApkAsset( + val url: String, // browser_download_url of the asset val sizeBytes: Long, /** * GitHub asset content digest, e.g. "sha256:". Null on older - * assets GitHub did not digest (pre-digest releases). When null the - * install path falls back to the size check only — see ADR-0034. + * assets GitHub did not digest (pre-digest releases). */ val digest: String? = null, - val isPrerelease: Boolean, - val releaseNotes: String, - /** Release publish time as epoch millis; null if GitHub didn't supply it. */ - val publishedAtEpochMs: Long? = null, -) +) { + /** + * The file's SHA-256 as 64 lower-case hex digits, to show next to the + * download so the user can compare it; null if GitHub gave no digest + * or one that is not a SHA-256. + */ + val sha256: String? + get() = digest + ?.takeIf { it.startsWith("sha256:", ignoreCase = true) } + ?.substring("sha256:".length) + ?.trim() + ?.lowercase() + ?.takeIf { Regex("[0-9a-f]{64}").matches(it) } +} sealed interface UpdateState { /** Initial value or while a request is in flight. */ @@ -29,11 +54,3 @@ sealed interface UpdateState { /** Check failed (network, parse, rate-limit). */ data class Error(val message: String) : UpdateState } - -sealed interface InstallState { - data object Idle : InstallState - data class Downloading(val progress: Float) : InstallState - data object Installing : InstallState - data class NeedsPermission(val intent: android.content.Intent) : InstallState - data class Failed(val message: String) : InstallState -} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/ApkInstaller.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/ApkInstaller.kt deleted file mode 100644 index 3e690c04..00000000 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/ApkInstaller.kt +++ /dev/null @@ -1,49 +0,0 @@ -package io.github.nofuturekid.nova.data.update - -import android.content.Intent -import java.io.File - -/** - * The APK download + system-install seam used by [UpdateController]. - * - * Extracted so the controller's install state machine can be unit-tested - * without Android's `PackageManager` / `PackageInstaller` (ADR-0030 D2). - * The sole production implementation is [UpdateInstaller]; bound in - * `UpdateModule`. - */ -interface ApkInstaller { - /** - * Download [url] to a cache file, reporting 0f..1f progress, then - * verify the bytes before returning (ADR-0034 #1): - * - * - the downloaded byte count must equal [expectedSizeBytes]; - * - if [expectedDigest] is non-null (`"sha256:"`) the file's - * SHA-256 must match it (constant-time compare). - * - * On any mismatch the temp file is deleted and - * [VerificationException] is thrown — the bytes are never handed to - * the system installer. A null [expectedDigest] falls back to the - * size check only (documented residual gap, ADR-0034). - */ - suspend fun download( - url: String, - expectedSizeBytes: Long, - expectedDigest: String?, - onProgress: (Float) -> Unit, - ): File - - /** Thrown by [download] when the downloaded APK fails verification. */ - class VerificationException(message: String) : RuntimeException(message) - - /** - * Hand [apk] to the system installer. Throws - * [NeedsPermissionException] if "install unknown apps" is not yet - * granted (no session is committed, so no broadcast will arrive). - */ - fun install(apk: File) - - /** Thrown by [install] when the user must grant install permission. */ - class NeedsPermissionException(val intent: Intent) : RuntimeException( - "User must enable 'install unknown apps' first.", - ) -} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/InstallStatusReceiver.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/InstallStatusReceiver.kt deleted file mode 100644 index f9acccfe..00000000 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/InstallStatusReceiver.kt +++ /dev/null @@ -1,105 +0,0 @@ -package io.github.nofuturekid.nova.data.update - -import android.content.BroadcastReceiver -import android.content.Context -import android.content.Intent -import android.content.pm.PackageInstaller -import kotlinx.coroutines.flow.MutableSharedFlow -import kotlinx.coroutines.flow.asSharedFlow - -/** - * Catches the PendingIntent fired by PackageInstaller when an install - * session resolves. The receiver lives statically so the session callback - * can post results onto a flow the ViewModel listens on. - */ -class InstallStatusReceiver : BroadcastReceiver() { - override fun onReceive(context: Context, intent: Intent) { - val status = intent.getIntExtra(PackageInstaller.EXTRA_STATUS, PackageInstaller.STATUS_FAILURE) - val message = intent.getStringExtra(PackageInstaller.EXTRA_STATUS_MESSAGE).orEmpty() - when (status) { - PackageInstaller.STATUS_PENDING_USER_ACTION -> { - // Android wants the user to confirm. Pull the chained intent - // and launch it as a new task so the ActivityResult flow can - // proceed without an Activity in the stack. - @Suppress("DEPRECATION") - val confirm = intent.getParcelableExtra(Intent.EXTRA_INTENT) - confirm?.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - if (confirm != null) { - context.startActivity(confirm) - _events.tryEmit(InstallEvent.UserConfirmShown) - } else { - _events.tryEmit(InstallEvent.Failed("Confirm intent missing")) - } - } - PackageInstaller.STATUS_SUCCESS -> _events.tryEmit(InstallEvent.Success) - else -> _events.tryEmit(InstallEvent.Failed(message.ifBlank { "Install failed (code $status)" })) - } - } - - companion object { - const val ACTION = "io.github.nofuturekid.nova.action.INSTALL_STATUS" - - // replay = 1 so a collector that subscribes *after* the - // PackageInstaller status broadcast has already fired (an in-process - // cold-start / restart race during install — #10) still receives the - // last install result instead of hanging forever on "Installing". - // extraBufferCapacity is kept so the non-suspending tryEmit() in - // onReceive() stays lossless. - // - // `var` (not `val`) only so resetForTest() can hand each test a - // pristine instance; production never reassigns this field (no prod - // code path calls resetForTest), so the single long-lived - // UpdateController collector subscribes once at init to the original - // instance and behaviour is byte-identical to a plain `val`. - // @Volatile guarantees test-thread visibility of the reassignment. - @Volatile - private var _events = MutableSharedFlow( - replay = 1, - extraBufferCapacity = 8, - ) - /** - * Read-only stream consumers (e.g. MainViewModel) listen on. A getter - * (not a cached val) so that after resetForTest() a freshly-built - * (test) collector subscribes to the current `_events` instance. In - * production `_events` is never reassigned, so this always returns the - * original instance — unchanged production behaviour. - */ - val events: kotlinx.coroutines.flow.SharedFlow - get() = _events.asSharedFlow() - - /** - * Test-only seam: emit an [InstallEvent] as if the system - * PackageInstaller broadcast had arrived. Lets UpdateController's - * broadcast-driven transitions be unit-tested without a real - * install session (ADR-0030 D2). Not used by production code; - * named `*ForTest` to mark intent (no annotation dep added). - */ - fun emitForTest(event: InstallEvent): Boolean = _events.tryEmit(event) - - /** - * Test-only seam: recreate the process-global flow so each test - * starts with a fully pristine InstallStatusReceiver — both the - * replay slot AND the extraBufferCapacity (#10) drained. - * - * Previously this called `_events.resetReplayCache()`, which clears - * ONLY the replay slot, not the 8-slot extraBufferCapacity. The - * triage-#22 concurrent test (two in-flight installUpdate calls → - * double install → multiple events into the global flow) exposed - * that buffered, uncollected emissions survived resetReplayCache() - * and leaked into the next test's freshly-constructed - * UpdateController collector → an order-dependent flaky failure. - * Recreating the SharedFlow drops both replay and buffer, fully - * isolating tests. Not used by production code (prod never resets - * this seam; see `_events`/`events` notes — behaviour unchanged). - */ - fun resetForTest() { - _events = MutableSharedFlow(replay = 1, extraBufferCapacity = 8) - } - } -} - -sealed interface InstallEvent { - data object UserConfirmShown : InstallEvent - data object Success : InstallEvent - data class Failed(val message: String) : InstallEvent -} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateController.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateController.kt deleted file mode 100644 index b927171a..00000000 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateController.kt +++ /dev/null @@ -1,135 +0,0 @@ -package io.github.nofuturekid.nova.data.update - -import android.content.ActivityNotFoundException -import android.content.Intent -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.flow.MutableStateFlow -import kotlinx.coroutines.flow.StateFlow -import kotlinx.coroutines.flow.asStateFlow -import kotlinx.coroutines.launch -import io.github.nofuturekid.nova.data.model.InstallState -import io.github.nofuturekid.nova.data.model.UpdateInfo -import javax.inject.Inject -import javax.inject.Singleton - -/** - * Process-global owner of the in-app updater's install pipeline (ADR-0012 - * revisit, actioned in ADR-0030 D2). - * - * Previously the download + PackageInstaller + permission flow + the - * [InstallStatusReceiver.events] collector were duplicated in both - * `MainViewModel` and `SettingsViewModel`, each with its own `_installState` - * and an `ownsInstall` guard so a ViewModel only reflected installs it - * itself started. - * - * There is exactly one install in flight at any time, so a single owner - * matches reality. This singleton holds the one [installState]; both - * ViewModels observe and forward it. The per-ViewModel `ownsInstall` guard - * is structurally gone: with a single shared state there is no "other" - * ViewModel to isolate from. **Intended behaviour change:** install - * progress is now reflected on both the Overview and Settings screens - * regardless of which screen started the install (documented in ADR-0012 - * and the ADR-0030 D2 acceptance line). - * - * The broadcast collector runs on a process-lifetime [scope] — a - * [SupervisorJob] on [Dispatchers.Default]. The singleton outlives any - * screen, so this is deliberately not tied to a ViewModel's lifecycle. - * Hilt uses the [@Inject][Inject] constructor (installer only); the - * second constructor lets tests drive the controller with a `TestScope`. - */ -@Singleton -class UpdateController( - private val installer: ApkInstaller, - private val scope: CoroutineScope, -) { - @Inject - constructor(installer: ApkInstaller) : this( - installer, - CoroutineScope(SupervisorJob() + Dispatchers.Default), - ) - - private val _installState = MutableStateFlow(InstallState.Idle) - val installState: StateFlow = _installState.asStateFlow() - - init { - // Mirror PackageInstaller broadcasts into the single install-state - // flow so any observing screen reacts to system confirm / success - // / failure. No `ownsInstall` guard: there is one shared state for - // the one in-flight install (ADR-0012 revisit / ADR-0030 D2). - scope.launch { - InstallStatusReceiver.events.collect { event -> - _installState.value = when (event) { - is InstallEvent.UserConfirmShown -> InstallState.Installing - is InstallEvent.Success -> InstallState.Idle - is InstallEvent.Failed -> InstallState.Failed(event.message) - } - } - } - } - - fun installUpdate(info: UpdateInfo) { - // In-flight guard (triage #22): a double-tap on Install must not - // start a second download + PackageInstaller session for one APK. - // Only a terminal state may start a run — Idle (fresh) or Failed - // (retry allowed). Any non-terminal state (Downloading / Installing - // / NeedsPermission) means an install is already in progress, so a - // second call is a no-op: ignore it rather than launch a coroutine - // that would clobber the first install's progress back to - // Downloading(0f) and commit a duplicate session. - // - // The slot is claimed atomically BEFORE launching (#241): a plain - // read-then-launch left a window in which two calls made before the - // coroutine had run both saw Idle and both started an install. - while (true) { - val state = _installState.value - if (state !is InstallState.Idle && state !is InstallState.Failed) return - if (_installState.compareAndSet(state, InstallState.Downloading(0f))) break - } - scope.launch { - try { - val apk = installer.download( - info.downloadUrl, - info.sizeBytes, - info.digest, - ) { progress -> - _installState.value = InstallState.Downloading(progress) - } - _installState.value = InstallState.Installing - try { - installer.install(apk) - } catch (e: ApkInstaller.NeedsPermissionException) { - // No session committed → no broadcast will arrive. - _installState.value = InstallState.NeedsPermission(e.intent) - } - } catch (e: Exception) { - _installState.value = InstallState.Failed(e.message ?: "Update failed") - } - } - } - - fun resetInstall() { - _installState.value = InstallState.Idle - } - - fun installFailed(message: String) { - _installState.value = InstallState.Failed(message) - } - - /** - * Opens the "install unknown apps" screen through [launch] and resets - * the install state. A ROM without that settings activity makes the - * launcher throw [ActivityNotFoundException]; that is reported through - * the install-error path instead of crashing or silently resetting to - * Idle. Shared by the Overview and Settings screens (#242). - */ - fun launchPermissionIntent(state: InstallState.NeedsPermission, launch: (Intent) -> Unit) { - try { - launch(state.intent) - resetInstall() - } catch (e: ActivityNotFoundException) { - installFailed("Can't open the install-permission screen on this device") - } - } -} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateInstaller.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateInstaller.kt deleted file mode 100644 index cd1464e5..00000000 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateInstaller.kt +++ /dev/null @@ -1,151 +0,0 @@ -package io.github.nofuturekid.nova.data.update - -import android.app.PendingIntent -import android.content.Context -import android.content.Intent -import android.content.pm.PackageInstaller -import android.net.Uri -import android.os.Build -import android.provider.Settings -import dagger.hilt.android.qualifiers.ApplicationContext -import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.withContext -import okhttp3.OkHttpClient -import okhttp3.Request -import okio.buffer -import okio.sink -import java.io.File -import java.security.MessageDigest -import java.util.concurrent.TimeUnit -import javax.inject.Inject -import javax.inject.Singleton - -/** - * Handles APK download + invocation of Android's PackageInstaller. - * - * Flow: - * 1. download(url, onProgress) → File in app cache - * 2. install(File): if user hasn't granted "install unknown apps" yet, - * throws NeedsPermissionException carrying the settings intent. - * Otherwise opens a PackageInstaller session and commits it; the - * PendingIntent fires InstallStatusReceiver which routes events - * onto InstallStatusReceiver.events. - */ -@Singleton -class UpdateInstaller @Inject constructor( - @param:ApplicationContext private val context: Context, -) : ApkInstaller { - private val http: OkHttpClient = OkHttpClient.Builder() - .connectTimeout(15, TimeUnit.SECONDS) - .readTimeout(60, TimeUnit.SECONDS) - .build() - - override suspend fun download( - url: String, - expectedSizeBytes: Long, - expectedDigest: String?, - onProgress: (Float) -> Unit, - ): File = - withContext(Dispatchers.IO) { - // Defence in depth: refuse a non-https URL even though - // UpdateRepository already screens it (ADR-0034 #1). - if (!url.startsWith("https://", ignoreCase = true)) { - throw ApkInstaller.VerificationException( - "Refusing to download update over a non-HTTPS URL", - ) - } - val req = Request.Builder().url(url).build() - http.newCall(req).execute().use { resp -> - if (!resp.isSuccessful) throw RuntimeException("HTTP ${resp.code} downloading APK") - val body = resp.body - val total = body.contentLength().coerceAtLeast(1L) - val out = updatesDir().resolve("NOVA-update.apk") - out.parentFile?.mkdirs() - if (out.exists()) out.delete() - val sha = MessageDigest.getInstance("SHA-256") - var written = 0L - body.source().use { src -> - out.sink().buffer().use { sink -> - val buffer = okio.Buffer() - while (true) { - val read = src.read(buffer, 8192L) - if (read == -1L) break - // Snapshot exactly the bytes we are about to - // persist, hash them, then write the same bytes. - val chunk = buffer.readByteArray(read) - sha.update(chunk) - sink.write(chunk) - written += read - onProgress((written.toFloat() / total).coerceIn(0f, 1f)) - } - sink.flush() - } - } - onProgress(1f) - - // ── Verify BEFORE the bytes can reach PackageInstaller ── - // Fail closed: any mismatch deletes the temp file and - // throws; install() is never reached. See ADR-0034 #1. - if (written != expectedSizeBytes) { - out.delete() - throw ApkInstaller.VerificationException( - "Update size mismatch: expected $expectedSizeBytes bytes, got $written", - ) - } - if (expectedDigest != null) { - val expectedHex = expectedDigest.substringAfter("sha256:", "") - .trim() - .lowercase() - if (expectedHex.isEmpty()) { - out.delete() - throw ApkInstaller.VerificationException( - "Update digest is not a sha256 digest: $expectedDigest", - ) - } - val actualHex = sha.digest() - .joinToString("") { "%02x".format(it.toInt() and 0xFF) } - if (!MessageDigest.isEqual( - expectedHex.toByteArray(Charsets.US_ASCII), - actualHex.toByteArray(Charsets.US_ASCII), - ) - ) { - out.delete() - throw ApkInstaller.VerificationException( - "Update SHA-256 mismatch — refusing to install", - ) - } - } - out - } - } - - override fun install(apk: File) { - val pm = context.packageManager - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && !pm.canRequestPackageInstalls()) { - throw ApkInstaller.NeedsPermissionException(unknownAppSourcesIntent()) - } - val installer = pm.packageInstaller - val params = PackageInstaller.SessionParams(PackageInstaller.SessionParams.MODE_FULL_INSTALL).apply { - setAppPackageName(context.packageName) - } - val sessionId = installer.createSession(params) - installer.openSession(sessionId).use { session -> - session.openWrite("update.apk", 0, apk.length()).use { out -> - apk.inputStream().use { input -> input.copyTo(out) } - session.fsync(out) - } - val intent = Intent(InstallStatusReceiver.ACTION).setPackage(context.packageName) - val flags = PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_MUTABLE - val pi = PendingIntent.getBroadcast(context, sessionId, intent, flags) - session.commit(pi.intentSender) - } - } - - fun unknownAppSourcesIntent(): Intent { - val uri = Uri.parse("package:${context.packageName}") - return Intent(Settings.ACTION_MANAGE_UNKNOWN_APP_SOURCES, uri) - .addFlags(Intent.FLAG_ACTIVITY_NEW_TASK) - } - - private fun updatesDir(): File = File(context.cacheDir, "updates") -} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateRepository.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateRepository.kt index a0f01373..f7c75419 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateRepository.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/data/update/UpdateRepository.kt @@ -5,6 +5,7 @@ import kotlinx.coroutines.withContext import kotlinx.serialization.Serializable import kotlinx.serialization.json.Json import io.github.nofuturekid.nova.BuildConfig +import io.github.nofuturekid.nova.data.model.ApkAsset import io.github.nofuturekid.nova.data.model.UpdateInfo import io.github.nofuturekid.nova.data.model.UpdateState import okhttp3.OkHttpClient @@ -18,7 +19,8 @@ import javax.inject.Singleton /** * Pulls release metadata from GitHub and decides whether a newer build is * available. Independent of Apollo / Unraid auth — uses a dedicated OkHttp - * client without any Unraid headers. + * client without any Unraid headers. It only reports; the user downloads + * and installs the file through the browser (ADR-0046). */ @Singleton class UpdateRepository @Inject constructor() { @@ -28,8 +30,6 @@ class UpdateRepository @Inject constructor() { .readTimeout(15, TimeUnit.SECONDS) .build() - private val json = Json { ignoreUnknownKeys = true } - suspend fun check(includePrereleases: Boolean): UpdateState = withContext(Dispatchers.IO) { val req = Request.Builder() .url("https://api.github.com/repos/$REPO/releases?per_page=20") @@ -40,43 +40,7 @@ class UpdateRepository @Inject constructor() { if (!resp.isSuccessful) { return@withContext UpdateState.Error("GitHub HTTP ${resp.code}") } - val body = resp.body.string() - val releases = json.decodeFromString>(body) - val current = parseVersion(BuildConfig.VERSION_NAME) - ?: return@withContext UpdateState.Error("Can't parse own version ${BuildConfig.VERSION_NAME}") - val match = releases.asSequence() - .filter { !it.draft } - .filter { includePrereleases || !it.prerelease } - .mapNotNull { rel -> - val v = parseVersion(rel.tagName) ?: return@mapNotNull null - val apk = rel.assets.firstOrNull { it.name.endsWith(".apk") } - ?: return@mapNotNull null - Triple(v, rel, apk) - } - .firstOrNull { (v, _, _) -> v > current } - ?: return@withContext UpdateState.UpToDate - val (_, rel, apk) = match - // Defence in depth against a MITM that rewrites the asset - // URL to plain HTTP: refuse a non-https download URL up - // front (fail closed). See ADR-0034 #1. - if (!apk.browserDownloadUrl.startsWith("https://", ignoreCase = true)) { - return@withContext UpdateState.Error( - "Refusing update: download URL is not HTTPS", - ) - } - UpdateState.Available( - UpdateInfo( - version = rel.tagName.removePrefix("v"), - tag = rel.tagName, - releaseUrl = rel.htmlUrl, - downloadUrl = apk.browserDownloadUrl, - sizeBytes = apk.size, - digest = apk.digest, - isPrerelease = rel.prerelease, - releaseNotes = rel.body.orEmpty(), - publishedAtEpochMs = parseIso8601(rel.publishedAt), - ), - ) + evaluate(resp.body.string(), includePrereleases, BuildConfig.VERSION_NAME) } } catch (e: Exception) { UpdateState.Error(e.message ?: "Network error") @@ -115,6 +79,53 @@ class UpdateRepository @Inject constructor() { companion object { private const val REPO = "hellwand/nova" + private const val RELEASES_PREFIX = "https://github.com/$REPO/releases/" + + private val json = Json { ignoreUnknownKeys = true } + + /** + * Decides from the Releases API response [body] which release, if + * any, is newer than [currentVersion]. Drafts are never offered; + * pre-releases only when [includePrereleases]. A release without + * an APK asset is still offered (its link is the release page). + */ + internal fun evaluate( + body: String, + includePrereleases: Boolean, + currentVersion: String, + ): UpdateState { + val releases = json.decodeFromString>(body) + val current = parseVersion(currentVersion) + ?: return UpdateState.Error("Can't parse own version $currentVersion") + val (_, rel) = releases.asSequence() + .filter { !it.draft } + .filter { includePrereleases || !it.prerelease } + .mapNotNull { rel -> parseVersion(rel.tagName)?.let { it to rel } } + .firstOrNull { (v, _) -> v > current } + ?: return UpdateState.UpToDate + val apk = rel.assets.firstOrNull { it.name.endsWith(".apk") } + // Defence in depth: the browser is only sent to this repository's + // own release pages and files, over HTTPS. A tampered or + // redirected API response that points elsewhere (another host, + // another repository, a non-https scheme) fails closed. + // See ADR-0034 #1 and ADR-0046. + if (!rel.htmlUrl.startsWith(RELEASES_PREFIX, ignoreCase = true) || + (apk != null && !apk.browserDownloadUrl.startsWith(RELEASES_PREFIX, ignoreCase = true)) + ) { + return UpdateState.Error("Refusing update: link is not a release link of $REPO") + } + return UpdateState.Available( + UpdateInfo( + version = rel.tagName.removePrefix("v"), + tag = rel.tagName, + releaseUrl = rel.htmlUrl, + apk = apk?.let { ApkAsset(it.browserDownloadUrl, it.size, it.digest) }, + isPrerelease = rel.prerelease, + releaseNotes = rel.body.orEmpty(), + publishedAtEpochMs = parseIso8601(rel.publishedAt), + ), + ) + } private fun parseIso8601(raw: String?): Long? { if (raw.isNullOrBlank()) return null diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/di/UpdateModule.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/di/UpdateModule.kt deleted file mode 100644 index c1d0fd01..00000000 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/di/UpdateModule.kt +++ /dev/null @@ -1,25 +0,0 @@ -package io.github.nofuturekid.nova.di - -import dagger.Binds -import dagger.Module -import dagger.hilt.InstallIn -import dagger.hilt.components.SingletonComponent -import io.github.nofuturekid.nova.data.update.ApkInstaller -import io.github.nofuturekid.nova.data.update.UpdateInstaller -import javax.inject.Singleton - -/** - * Binds the [ApkInstaller] seam to its sole production implementation - * [UpdateInstaller] (ADR-0030 D2). The interface exists so - * `UpdateController`'s install state machine is unit-testable without - * Android's `PackageInstaller`. `UpdateController` itself is a - * constructor-injected `@Singleton`, so Hilt discovers it directly — no - * provider needed here. - */ -@Module -@InstallIn(SingletonComponent::class) -abstract class UpdateModule { - @Binds - @Singleton - abstract fun bindApkInstaller(impl: UpdateInstaller): ApkInstaller -} diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainScreen.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainScreen.kt index 943018b8..08eab153 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainScreen.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainScreen.kt @@ -48,15 +48,12 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip import androidx.compose.ui.graphics.Color import androidx.compose.ui.unit.dp -import androidx.activity.compose.rememberLauncherForActivityResult -import androidx.activity.result.contract.ActivityResultContracts import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import kotlinx.coroutines.launch import io.github.nofuturekid.nova.BuildConfig import io.github.nofuturekid.nova.data.model.ArrayState import io.github.nofuturekid.nova.data.model.Container import io.github.nofuturekid.nova.data.model.ConnectionMode -import io.github.nofuturekid.nova.data.model.InstallState import io.github.nofuturekid.nova.data.model.NotifType import io.github.nofuturekid.nova.data.model.Server import io.github.nofuturekid.nova.data.model.UpdateState @@ -146,12 +143,8 @@ fun MainScreen( var refreshing by remember { mutableStateOf(false) } val updateState by vm.updateState.collectAsStateWithLifecycle() - val installState by vm.installState.collectAsStateWithLifecycle() val dismissedTag by vm.dismissedUpdateTag.collectAsStateWithLifecycle() var showUpdateDialog by remember { mutableStateOf(false) } - val permissionLauncher = rememberLauncherForActivityResult( - contract = ActivityResultContracts.StartActivityForResult(), - ) { /* nothing — user returns; if granted, they tap Install again */ } // Transient action-failure signal (triage #19, ADR-0037). A failed // user action no longer crashes the app — the resilient launcher in @@ -540,15 +533,7 @@ fun MainScreen( if (showUpdateDialog && u is UpdateState.Available) { UpdateDialog( info = u.info, - install = installState, - onInstall = { vm.installUpdate(u.info) }, - onDismiss = { - showUpdateDialog = false - if (installState is InstallState.Failed) vm.resetInstall() - }, - onGrantPermission = { state -> - vm.launchPermissionIntent(state) { permissionLauncher.launch(it) } - }, + onDismiss = { showUpdateDialog = false }, ) } } diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt index 47813498..a84137a4 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/main/MainViewModel.kt @@ -1,6 +1,5 @@ package io.github.nofuturekid.nova.ui.screens.main -import android.content.Intent import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import dagger.hilt.android.lifecycle.HiltViewModel @@ -33,14 +32,12 @@ import io.github.nofuturekid.nova.data.model.DisplayThresholds import io.github.nofuturekid.nova.data.model.NotifType import io.github.nofuturekid.nova.data.model.Container import io.github.nofuturekid.nova.data.model.ContainerLiveStats -import io.github.nofuturekid.nova.data.model.InstallState import io.github.nofuturekid.nova.data.model.LiveMetrics import io.github.nofuturekid.nova.data.model.NetworkThroughput import io.github.nofuturekid.nova.data.model.Notifications import io.github.nofuturekid.nova.data.model.Server import io.github.nofuturekid.nova.data.model.ServerInfo import io.github.nofuturekid.nova.data.model.Temperature -import io.github.nofuturekid.nova.data.model.UpdateInfo import io.github.nofuturekid.nova.data.model.UpdateState import io.github.nofuturekid.nova.data.model.Vm import io.github.nofuturekid.nova.data.repository.ActionFailedException @@ -48,7 +45,6 @@ import io.github.nofuturekid.nova.data.repository.DomainState import io.github.nofuturekid.nova.data.repository.ServerRepository import io.github.nofuturekid.nova.data.repository.SettingsRepository import io.github.nofuturekid.nova.data.repository.UnraidRepository -import io.github.nofuturekid.nova.data.update.UpdateController import io.github.nofuturekid.nova.data.update.UpdateRepository import javax.inject.Inject @@ -68,7 +64,6 @@ class MainViewModel @Inject constructor( private val unraid: UnraidRepository, private val settings: SettingsRepository, private val updates: UpdateRepository, - private val updateController: UpdateController, ) : ViewModel() { // ── Resilient action seam (triage #19, ADR-0037) ────────────────── @@ -274,19 +269,12 @@ class MainViewModel @Inject constructor( private val _updatingContainerIds = MutableStateFlow>(emptySet()) val updatingContainerIds: StateFlow> = _updatingContainerIds.asStateFlow() - // ── App-updater plumbing (unchanged) ────────────────────────────── + // ── Update check (the app only reports; the user downloads and + // installs the file in the browser, ADR-0046) ─────────────────── private val _updateState = MutableStateFlow(UpdateState.Checking) val updateState: StateFlow = _updateState.asStateFlow() - // Install pipeline is owned by the @Singleton UpdateController - // (ADR-0012 revisit / ADR-0030 D2). This ViewModel forwards the one - // shared install state; the public API to the composables is - // unchanged. There is intentionally no `ownsInstall` guard any more: - // a single shared state means both screens reflect the one in-flight - // install regardless of which screen started it. - val installState: StateFlow = updateController.installState - val dismissedUpdateTag: StateFlow = settings.dismissedUpdateTag .stateIn(viewModelScope, SharingStarted.WhileSubscribed(5_000), null) @@ -343,21 +331,6 @@ class MainViewModel @Inject constructor( settings.setDismissedUpdateTag(tag) } - fun installUpdate(info: UpdateInfo) { - if (!BuildConfig.HAS_UPDATER) return - updateController.installUpdate(info) - } - - fun resetInstall() { - if (!BuildConfig.HAS_UPDATER) return - updateController.resetInstall() - } - - fun launchPermissionIntent(state: InstallState.NeedsPermission, launch: (Intent) -> Unit) { - if (!BuildConfig.HAS_UPDATER) return - updateController.launchPermissionIntent(state, launch) - } - fun toggleConnection() = viewModelScope.launch { servers.setConnectionMode( if (ui.value.connectionMode == ConnectionMode.Local) ConnectionMode.Remote else ConnectionMode.Local diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/settings/SettingsScreen.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/settings/SettingsScreen.kt index de88748e..432b44e7 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/settings/SettingsScreen.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/settings/SettingsScreen.kt @@ -2,8 +2,6 @@ package io.github.nofuturekid.nova.ui.screens.settings import android.content.Intent import android.net.Uri -import androidx.activity.compose.rememberLauncherForActivityResult -import androidx.activity.result.contract.ActivityResultContracts import androidx.compose.foundation.background import androidx.compose.foundation.border import androidx.compose.foundation.clickable @@ -55,11 +53,8 @@ import kotlinx.coroutines.launch import io.github.nofuturekid.nova.BuildConfig import io.github.nofuturekid.nova.data.local.LayoutMode import io.github.nofuturekid.nova.data.model.AppSettings -import io.github.nofuturekid.nova.data.model.InstallState -import io.github.nofuturekid.nova.data.model.UpdateInfo import io.github.nofuturekid.nova.data.model.UpdateState import io.github.nofuturekid.nova.data.repository.SettingsRepository -import io.github.nofuturekid.nova.data.update.UpdateController import io.github.nofuturekid.nova.data.update.UpdateRepository import io.github.nofuturekid.nova.ui.components.BtnVariant import io.github.nofuturekid.nova.ui.components.Pill @@ -93,7 +88,6 @@ data class SettingsUi( class SettingsViewModel @Inject constructor( private val repo: SettingsRepository, private val updates: UpdateRepository, - private val updateController: UpdateController, ) : ViewModel() { private val layouts = combine( repo.dockerView, repo.vmsView, repo.arrayView, @@ -117,14 +111,6 @@ class SettingsViewModel @Inject constructor( private val _checkState = MutableStateFlow(UpdateState.Checking) val checkState: StateFlow = _checkState.asStateFlow() - // Install pipeline is owned by the @Singleton UpdateController - // (ADR-0012 revisit / ADR-0030 D2). This ViewModel forwards the one - // shared install state; the public API to the composables is - // unchanged. The old per-ViewModel `ownsInstall` guard is gone: with - // a single shared state both screens reflect the one in-flight - // install regardless of which screen started it (intended change). - val installState: StateFlow = updateController.installState - init { // Refresh the snapshot when the user opens Settings so they see the // current update state instead of a stale 'Up to date'. @@ -151,21 +137,6 @@ class SettingsViewModel @Inject constructor( _checkState.value = updates.check(state.value.includePrereleases) repo.setLastUpdateCheck(System.currentTimeMillis()) } - - fun installUpdate(info: UpdateInfo) { - if (!BuildConfig.HAS_UPDATER) return - updateController.installUpdate(info) - } - - fun resetInstall() { - if (!BuildConfig.HAS_UPDATER) return - updateController.resetInstall() - } - - fun launchPermissionIntent(state: InstallState.NeedsPermission, launch: (Intent) -> Unit) { - if (!BuildConfig.HAS_UPDATER) return - updateController.launchPermissionIntent(state, launch) - } } @Composable @@ -179,11 +150,7 @@ fun SettingsScreen( val t = UnraidTheme.colors val ui by vm.state.collectAsState() val checkState by vm.checkState.collectAsState() - val installState by vm.installState.collectAsState() var showUpdateDialog by remember { mutableStateOf(false) } - val permissionLauncher = rememberLauncherForActivityResult( - contract = ActivityResultContracts.StartActivityForResult(), - ) { /* user returns from system settings; they'll re-tap Install */ } Column( modifier = Modifier @@ -344,7 +311,7 @@ fun SettingsScreen( if (cs is UpdateState.Available) { UnraidButton( onClick = { showUpdateDialog = true }, - label = "Install v${cs.info.version}", + label = "Update to v${cs.info.version}", variant = BtnVariant.Filled, ) } @@ -491,15 +458,7 @@ fun SettingsScreen( if (showUpdateDialog && cs2 is UpdateState.Available) { UpdateDialog( info = cs2.info, - install = installState, - onInstall = { vm.installUpdate(cs2.info) }, - onDismiss = { - showUpdateDialog = false - if (installState is InstallState.Failed) vm.resetInstall() - }, - onGrantPermission = { state -> - vm.launchPermissionIntent(state) { permissionLauncher.launch(it) } - }, + onDismiss = { showUpdateDialog = false }, ) } } diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/update/UpdateBanner.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/update/UpdateBanner.kt index 4150d239..5a0fd269 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/update/UpdateBanner.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/update/UpdateBanner.kt @@ -59,7 +59,7 @@ fun UpdateBanner( } Spacer(Modifier.height(2.dp)) Text( - text = "v${info.version} · tap to install", + text = "v${info.version} · tap for details", color = t.muted, style = MaterialTheme.typography.labelSmall, ) diff --git a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/update/UpdateDialog.kt b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/update/UpdateDialog.kt index 2dc6eae2..77240327 100644 --- a/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/update/UpdateDialog.kt +++ b/app/src/main/kotlin/io/github/nofuturekid/nova/ui/screens/update/UpdateDialog.kt @@ -1,5 +1,8 @@ package io.github.nofuturekid.nova.ui.screens.update +import android.content.ActivityNotFoundException +import android.content.Context +import android.content.Intent import androidx.compose.foundation.background import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box @@ -13,6 +16,7 @@ import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.width import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape +import androidx.compose.foundation.text.selection.SelectionContainer import androidx.compose.foundation.verticalScroll import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.MaterialTheme @@ -20,17 +24,21 @@ import androidx.compose.material3.ModalBottomSheet import androidx.compose.material3.Text import androidx.compose.material3.rememberModalBottomSheetState import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier import androidx.compose.ui.draw.clip -import androidx.compose.ui.graphics.Color +import androidx.compose.ui.platform.LocalContext import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp +import androidx.core.net.toUri import com.mikepenz.markdown.m3.Markdown import com.mikepenz.markdown.m3.markdownColor import com.mikepenz.markdown.m3.markdownTypography import io.github.nofuturekid.nova.BuildConfig -import io.github.nofuturekid.nova.data.model.InstallState import io.github.nofuturekid.nova.data.model.UpdateInfo import io.github.nofuturekid.nova.ui.components.BtnVariant import io.github.nofuturekid.nova.ui.components.Pill @@ -38,7 +46,6 @@ import io.github.nofuturekid.nova.ui.components.Tone import io.github.nofuturekid.nova.ui.components.UC import io.github.nofuturekid.nova.ui.components.UnraidButton import io.github.nofuturekid.nova.ui.components.UnraidIconButton -import io.github.nofuturekid.nova.ui.components.UnraidProgress import io.github.nofuturekid.nova.ui.theme.JetBrainsMono import io.github.nofuturekid.nova.ui.theme.UnraidAlpha import io.github.nofuturekid.nova.ui.theme.UnraidTheme @@ -47,12 +54,14 @@ import io.github.nofuturekid.nova.ui.theme.UnraidTheme @Composable fun UpdateDialog( info: UpdateInfo, - install: InstallState, - onInstall: () -> Unit, onDismiss: () -> Unit, - onGrantPermission: (InstallState.NeedsPermission) -> Unit, ) { val t = UnraidTheme.colors + val context = LocalContext.current + var linkError by remember { mutableStateOf(false) } + fun open(url: String) { + linkError = !openInBrowser(context, url) + } val sheetState = rememberModalBottomSheetState(skipPartiallyExpanded = true) ModalBottomSheet( onDismissRequest = onDismiss, @@ -89,7 +98,8 @@ fun UpdateDialog( } Spacer(Modifier.height(4.dp)) Text( - text = "v${BuildConfig.VERSION_NAME} → v${info.version} · ${formatBytes(info.sizeBytes)}", + text = "v${BuildConfig.VERSION_NAME} → v${info.version}" + + (info.apk?.let { " · ${formatBytes(it.sizeBytes)}" } ?: ""), color = t.muted, style = MaterialTheme.typography.bodySmall.copy(fontFamily = JetBrainsMono), ) @@ -147,87 +157,90 @@ fun UpdateDialog( Spacer(Modifier.height(18.dp)) - // Status / progress / actions - when (install) { - is InstallState.Idle -> { - Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - Spacer(Modifier.weight(1f)) - UnraidButton( - onClick = onDismiss, - label = "Later", - variant = BtnVariant.Text, - tone = Tone.Neutral, - ) - UnraidButton( - onClick = onInstall, - label = "Install", - variant = BtnVariant.Filled, - ) - } - } - is InstallState.Downloading -> { - Text( - text = "Downloading… ${(install.progress * 100).toInt()}%", - color = t.muted, - style = MaterialTheme.typography.bodySmall, - ) - Spacer(Modifier.height(8.dp)) - UnraidProgress(install.progress, color = t.accent, height = 6.dp) - } - is InstallState.Installing -> { - Text( - text = "Waiting for Android install confirm…", - color = t.muted, - style = MaterialTheme.typography.bodySmall, - ) - } - is InstallState.NeedsPermission -> { - Text( - text = "Android needs you to allow this app to install other apps. Tap below to open the system settings.", - color = t.warn, - style = MaterialTheme.typography.bodySmall, - ) - Spacer(Modifier.height(10.dp)) - Row { - Spacer(Modifier.weight(1f)) - UnraidButton( - onClick = { onGrantPermission(install) }, - label = "Open settings", - variant = BtnVariant.Filled, - tone = Tone.Warn, - ) - } - } - is InstallState.Failed -> { + // NOVA does not install anything itself (ADR-0046): the file is + // downloaded and opened by the browser and Android's installer. + val apk = info.apk + Text( + text = if (apk != null) { + "NOVA does not install updates itself. Download lets your browser download " + + "the file; open the downloaded file to install it. Android may ask you to " + + "allow your browser or file manager to install apps, and Play Protect may " + + "ask to scan the new version first." + } else { + "This release has no APK file attached. Open the release page to see " + + "what is available." + }, + color = t.muted, + style = MaterialTheme.typography.bodySmall, + ) + apk?.sha256?.let { sha -> + Spacer(Modifier.height(10.dp)) + Text( + text = "SHA-256 of the file", + color = t.muted, + style = MaterialTheme.typography.labelSmall.copy(letterSpacing = 1.3.sp), + ) + Spacer(Modifier.height(4.dp)) + SelectionContainer { Text( - text = "Install failed: ${install.message}", - color = t.danger, - style = MaterialTheme.typography.bodySmall, + text = sha, + color = t.text, + style = MaterialTheme.typography.bodySmall.copy(fontFamily = JetBrainsMono), ) - Spacer(Modifier.height(10.dp)) - Row(horizontalArrangement = Arrangement.spacedBy(8.dp)) { - Spacer(Modifier.weight(1f)) - UnraidButton( - onClick = onDismiss, - label = "Close", - variant = BtnVariant.Text, - tone = Tone.Neutral, - ) - UnraidButton( - onClick = onInstall, - label = "Retry", - variant = BtnVariant.Tonal, - tone = Tone.Danger, - ) - } } } + if (linkError) { + Spacer(Modifier.height(10.dp)) + Text( + text = "No app on this phone can open the link.", + color = t.danger, + style = MaterialTheme.typography.bodySmall, + ) + } + + Spacer(Modifier.height(14.dp)) + // "Release page" sits on its own row: three buttons side by side + // do not fit a 360 dp screen at large font scales. + if (apk != null) { + UnraidButton( + onClick = { open(info.releaseUrl) }, + label = "Release page", + variant = BtnVariant.Text, + ) + } + Row( + verticalAlignment = Alignment.CenterVertically, + horizontalArrangement = Arrangement.spacedBy(8.dp), + ) { + Spacer(Modifier.weight(1f)) + UnraidButton( + onClick = onDismiss, + label = "Later", + variant = BtnVariant.Text, + tone = Tone.Neutral, + ) + UnraidButton( + onClick = { open(info.downloadUrl) }, + label = if (apk != null) "Download" else "Release page", + variant = BtnVariant.Filled, + ) + } Spacer(Modifier.height(20.dp)) } } } +/** Sends [url] to the browser; false if no app on the phone can open it. */ +private fun openInBrowser(context: Context, url: String): Boolean = try { + context.startActivity( + Intent(Intent.ACTION_VIEW, url.toUri()).addFlags(Intent.FLAG_ACTIVITY_NEW_TASK), + ) + true +} catch (_: ActivityNotFoundException) { + false +} + private fun Modifier.heightConstrainedAtMost(max: androidx.compose.ui.unit.Dp): Modifier = this.heightIn(max = max) diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/NoInstallPermissionTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/NoInstallPermissionTest.kt new file mode 100644 index 00000000..4d852c5b --- /dev/null +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/NoInstallPermissionTest.kt @@ -0,0 +1,51 @@ +package io.github.nofuturekid.nova + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import java.io.File + +/** + * WHY: Google Play Protect stopped every NOVA build that declared + * REQUEST_INSTALL_PACKAGES ("Harmful app blocked") and rated the same build + * without it "This app seems safe" (ADR-0046). The permission must not come + * back unnoticed, neither through a flavor manifest nor by dropping the + * remove marker that strips it from library manifests. + * + * Gradle runs unit tests with the module dir as working dir. + */ +class NoInstallPermissionTest { + + private val permission = "android.permission.REQUEST_INSTALL_PACKAGES" + + private fun manifests(): List { + val src = File("src") + assertTrue("src not found from ${File(".").absolutePath}", src.isDirectory) + return src.listFiles { f -> f.isDirectory }.orEmpty() + .map { File(it, "AndroidManifest.xml") } + .filter { it.isFile } + } + + private fun declaring(text: String) = + Regex("""]*>""").findAll(text).map { it.value } + .filter { it.contains(permission) }.toList() + + @Test + fun noSourceManifestDeclaresTheInstallPermission() { + val offenders = manifests().filter { m -> + declaring(m.readText()).any { !it.contains("tools:node=\"remove\"") } + } + assertEquals("these manifests declare $permission: $offenders", emptyList(), offenders) + } + + @Test + fun mainManifestStripsTheInstallPermissionFromEveryFlavorAndLibrary() { + val main = File("src/main/AndroidManifest.xml") + assertTrue("main manifest not found", main.isFile) + val entries = declaring(main.readText()) + assertTrue( + "main manifest must carry ", + entries.any { it.contains("tools:node=\"remove\"") }, + ) + } +} diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/TransientErrorToleranceTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/TransientErrorToleranceTest.kt index 33988a96..086920c7 100644 --- a/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/TransientErrorToleranceTest.kt +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/repository/TransientErrorToleranceTest.kt @@ -36,7 +36,7 @@ import org.junit.Test * failure #1; widen/narrow `TRANSIENT_ERROR_TOLERANCE` and the failure#2 / * failure#3 assertions break. * - * Collector style mirrors UpdateControllerTest / StaleCrossServerTest + * Collector style mirrors StaleCrossServerTest * (launch into a list, driven on a virtual scheduler). Because the poll loop * is infinite, the scheduler is stepped one poll interval at a time via * advanceTimeBy + runCurrent (advanceUntilIdle would never return). JUnit + diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/update/UpdateControllerTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/update/UpdateControllerTest.kt deleted file mode 100644 index 61024f41..00000000 --- a/app/src/test/kotlin/io/github/nofuturekid/nova/data/update/UpdateControllerTest.kt +++ /dev/null @@ -1,436 +0,0 @@ -package io.github.nofuturekid.nova.data.update - -import android.content.ActivityNotFoundException -import android.content.Intent -import kotlinx.coroutines.CompletableDeferred -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.test.StandardTestDispatcher -import kotlinx.coroutines.test.TestScope -import kotlinx.coroutines.test.UnconfinedTestDispatcher -import kotlinx.coroutines.test.runTest -import io.github.nofuturekid.nova.data.model.InstallState -import io.github.nofuturekid.nova.data.model.UpdateInfo -import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue -import org.junit.Before -import org.junit.Test -import java.io.File -import java.util.concurrent.atomic.AtomicInteger - -/** - * Unit tests for [UpdateController] — the @Singleton install pipeline - * extracted in ADR-0030 D2 (ADR-0012 revisit). - * - * These prove the install state machine and the single-owner semantics. - * Each test states WHY it matters (Rule 9): if a transition or the - * single-shared-state behaviour regresses, the corresponding test fails — - * not just a smoke check. - */ -@OptIn(ExperimentalCoroutinesApi::class) -class UpdateControllerTest { - - @Before - fun resetReceiver() { InstallStatusReceiver.resetForTest() } - - private val info = UpdateInfo( - version = "1.2.3", - tag = "v1.2.3", - releaseUrl = "https://example/r", - downloadUrl = "https://example/app.apk", - sizeBytes = 1L, - isPrerelease = false, - releaseNotes = "", - ) - - /** - * Fake install seam. Records the install call and lets each test pick - * the download / install outcome. Defaults to a successful download + - * a no-op install (the real terminal state then arrives via the - * InstallStatusReceiver broadcast, mirroring production). - */ - private class FakeInstaller( - val onDownload: (suspend (String, (Float) -> Unit) -> File) = { _, p -> - p(0.5f); p(1f); File("downloaded.apk") - }, - val onInstall: (File) -> Unit = {}, - ) : ApkInstaller { - var installedFile: File? = null - override suspend fun download( - url: String, - expectedSizeBytes: Long, - expectedDigest: String?, - onProgress: (Float) -> Unit, - ): File = onDownload(url, onProgress) - override fun install(apk: File) { - installedFile = apk - onInstall(apk) - } - } - - /** - * WHY: the happy path must walk Downloading(progress) → Installing, - * then a system Success broadcast must land the controller back at - * Idle. If installUpdate stopped emitting Downloading/Installing, or - * if the broadcast collector stopped mapping Success→Idle, this fails. - */ - @Test - fun downloadingThenInstallingThenSuccess() = runTest { - val scope = TestScope(UnconfinedTestDispatcher(testScheduler)) - val controller = UpdateController(FakeInstaller(), scope) - - assertEquals(InstallState.Idle, controller.installState.value) - - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - - // download() reported up to 1f then install() ran with no - // NeedsPermission → state rests at Installing awaiting the - // system broadcast. - assertEquals(InstallState.Installing, controller.installState.value) - - InstallStatusReceiver.emitForTest(InstallEvent.UserConfirmShown) - scope.testScheduler.advanceUntilIdle() - assertEquals(InstallState.Installing, controller.installState.value) - - InstallStatusReceiver.emitForTest(InstallEvent.Success) - scope.testScheduler.advanceUntilIdle() - assertEquals(InstallState.Idle, controller.installState.value) - } - - /** - * WHY: a failed system install must surface InstallState.Failed with - * the broadcast's message so the dialog shows the error. Regression - * guard for the Failed branch of the events collector. - */ - @Test - fun systemBroadcastFailureSurfacesFailedState() = runTest { - val scope = TestScope(UnconfinedTestDispatcher(testScheduler)) - val controller = UpdateController(FakeInstaller(), scope) - - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - - InstallStatusReceiver.emitForTest(InstallEvent.Failed("disk full")) - scope.testScheduler.advanceUntilIdle() - - val s = controller.installState.value - assertTrue(s is InstallState.Failed) - assertEquals("disk full", (s as InstallState.Failed).message) - } - - /** - * WHY: when "install unknown apps" is not granted, install() throws - * NeedsPermissionException and NO session is committed, so no - * broadcast will ever arrive. The controller must move to - * NeedsPermission (carrying the settings intent) rather than hang on - * Installing forever. - */ - @Test - fun needsPermissionPathYieldsNeedsPermissionState() = runTest { - val intent = Intent("test.settings") - val scope = TestScope(UnconfinedTestDispatcher(testScheduler)) - val controller = UpdateController( - FakeInstaller(onInstall = { - throw ApkInstaller.NeedsPermissionException(intent) - }), - scope, - ) - - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - - val s = controller.installState.value - assertTrue(s is InstallState.NeedsPermission) - assertEquals(intent, (s as InstallState.NeedsPermission).intent) - } - - /** - * WHY: a thrown download error must terminate as Failed (not leave the - * UI stuck on Downloading), and resetInstall() must clear it back to - * Idle so the user can retry. Guards both the catch branch and reset. - */ - @Test - fun downloadFailureThenResetReturnsToIdle() = runTest { - val scope = TestScope(UnconfinedTestDispatcher(testScheduler)) - val controller = UpdateController( - FakeInstaller(onDownload = { _, _ -> throw RuntimeException("network down") }), - scope, - ) - - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - - val failed = controller.installState.value - assertTrue(failed is InstallState.Failed) - assertEquals("network down", (failed as InstallState.Failed).message) - - controller.resetInstall() - assertEquals(InstallState.Idle, controller.installState.value) - } - - /** - * WHY: this is the ADR-0012 revisit / ADR-0030 D2 behaviour change. - * There is ONE shared install state. A single controller instance - * observed by two callers reflects the same in-flight install to - * both, with no per-observer `ownsInstall` isolation. If someone - * reintroduced per-observer filtering, the second observer would not - * see an install it didn't start and this assertion would fail. - */ - @Test - fun singleSharedStateIsVisibleToEveryObserver() = runTest { - val scope = TestScope(UnconfinedTestDispatcher(testScheduler)) - val controller = UpdateController(FakeInstaller(), scope) - - // Two independent "screens" read the one controller. - val overviewState = controller.installState - val settingsState = controller.installState - - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - InstallStatusReceiver.emitForTest(InstallEvent.UserConfirmShown) - scope.testScheduler.advanceUntilIdle() - - // Same instance, same value — both screens reflect the install - // regardless of which "started" it. - assertEquals(InstallState.Installing, overviewState.value) - assertEquals(InstallState.Installing, settingsState.value) - assertEquals(overviewState, settingsState) - } - - /** - * Re-entrancy / double-tap (triage #22) — FIXED CONTRACT. - * - * GUARDED BEHAVIOUR: [UpdateController.installUpdate] now drops a - * second call while an install is in flight. The guard at the top of - * installUpdate only proceeds when `_installState` is terminal — - * Idle (fresh) or Failed (retry) — and returns without launching when - * it is non-terminal (Downloading / Installing / NeedsPermission). - * A user double-tap on Install therefore produces exactly ONE - * download and ONE PackageInstaller session, and the first install's - * progress is NOT clobbered back to Downloading(0f). - * - * This test gates download() on a per-call latch so every step is - * deterministic: hold invoke #1 mid-download at a known progress, - * fire the double-tap, prove it was a no-op, release #1 to finish, - * then prove a fresh installUpdate after a terminal state still - * starts a new run (retry must keep working — the guard must not - * permanently wedge the pipeline). - * - * DOC-NOTE (Rule 9): this is the regression sentinel for triage #22. - * If the in-flight guard regresses (e.g. someone removes the early - * return or makes it launch unconditionally again), the double-tap - * starts a second download → `downloads`/`installs` go to 2 and the - * clobber assertion flips → this test fails. Its failure is the - * intended signal that the #22 guard was lost. - */ - @Test - fun secondInvokeWhileInFlightIsGuarded_noOp() = runTest { - val scope = TestScope(UnconfinedTestDispatcher(testScheduler)) - val downloads = AtomicInteger(0) - val installs = AtomicInteger(0) - // download() parks on a per-call gate so the in-flight window is - // deterministic: hold invoke #1 mid-download at progress 0.5, - // fire the double-tap while it is parked, then release. - val gate1 = CompletableDeferred() - val gate2 = CompletableDeferred() - - val installer = object : ApkInstaller { - override suspend fun download( - url: String, - expectedSizeBytes: Long, - expectedDigest: String?, - onProgress: (Float) -> Unit, - ): File { - val n = downloads.incrementAndGet() - onProgress(0.5f) // advance off Downloading(0f) so a clobber is visible - (if (n == 1) gate1 else gate2).await() // park this call - onProgress(1f) - return File("app-$n.apk") - } - override fun install(apk: File) { installs.incrementAndGet() } - } - val controller = UpdateController(installer, scope) - - // Invoke #1: enters download() and parks on gate1 at progress 0.5. - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - // WHY: proves invoke #1 is genuinely in flight (one download - // started, parked) and state advanced past Downloading(0f) — the - // precondition for a true re-entrant double-tap, not a serial one. - assertEquals(1, downloads.get()) - assertEquals(0, installs.get()) - assertEquals( - "invoke #1 is mid-download at 0.5 before the double-tap", - InstallState.Downloading(0.5f), - controller.installState.value, - ) - - // Invoke #2 — the double-tap — while #1 is still parked on gate1. - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - // WHY: GUARDED. installState is non-terminal (Downloading), so the - // second call returns immediately without launching: NO second - // download, NO extra install, and crucially the first install's - // progress is NOT reset to Downloading(0f). This is the core - // triage #22 fix — one user intent → one session. - assertEquals( - "double-tap must NOT start a second download (in-flight guard)", - 1, - downloads.get(), - ) - assertEquals(0, installs.get()) - assertEquals( - "double-tap must NOT clobber invoke #1's progress", - InstallState.Downloading(0.5f), - controller.installState.value, - ) - - // Release invoke #1; let its single pipeline run to completion. - gate1.complete(Unit) - scope.testScheduler.advanceUntilIdle() - // WHY: exactly one download and one install ran for the two taps — - // the regression sentinel for triage #22. If the guard regresses - // these become 2 and the test fails (intended signal). - assertEquals( - "guarded double-tap downloads once (triage #22 fix)", - 1, - downloads.get(), - ) - assertEquals( - "guarded double-tap installs once (triage #22 fix)", - 1, - installs.get(), - ) - // WHY: install() returned with no NeedsPermission / broadcast yet, - // so the controller rests at Installing awaiting the system - // broadcast — recoverable, not wedged. - assertEquals(InstallState.Installing, controller.installState.value) - - // RETRY MUST STILL WORK: drive the controller to a terminal state - // (system failure → Failed) and prove a fresh installUpdate from a - // terminal state DOES start a new run. The guard must gate only - // in-flight re-entrancy, never permanently wedge the pipeline. - InstallStatusReceiver.emitForTest(InstallEvent.Failed("system rejected")) - scope.testScheduler.advanceUntilIdle() - assertTrue( - "precondition: controller is in a terminal Failed state", - controller.installState.value is InstallState.Failed, - ) - - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - // WHY: from a terminal state (Failed) the guard lets the call - // through, so a brand-new download starts. Proves retry-after-fail - // still works — the guard is in-flight-only, not a one-shot latch. - assertEquals( - "installUpdate from a terminal Failed state starts a fresh run (retry works)", - 2, - downloads.get(), - ) - gate2.complete(Unit) - scope.testScheduler.advanceUntilIdle() - assertEquals( - "the retried run installs (now exactly 2 total: 1 guarded + 1 retry)", - 2, - installs.get(), - ) - } - - /** - * WHY (#241): the guard used to read the state and only set - * Downloading inside the launched coroutine, so two calls made before - * the dispatcher ran both saw Idle and both started a download plus a - * PackageInstaller session. With a StandardTestDispatcher nothing runs - * until the scheduler advances, which is exactly that window. If the - * slot is not claimed synchronously, downloads/installs become 2 and - * this fails. - */ - @Test - fun twoCallsBeforeTheDispatcherAdvancesStartOneInstall() = runTest { - val scope = TestScope(StandardTestDispatcher(testScheduler)) - val downloads = AtomicInteger(0) - val installs = AtomicInteger(0) - val installer = object : ApkInstaller { - override suspend fun download( - url: String, - expectedSizeBytes: Long, - expectedDigest: String?, - onProgress: (Float) -> Unit, - ): File { - downloads.incrementAndGet() - return File("app.apk") - } - override fun install(apk: File) { installs.incrementAndGet() } - } - val controller = UpdateController(installer, scope) - - controller.installUpdate(info) - controller.installUpdate(info) // double-tap before anything ran - assertEquals( - "the slot is claimed synchronously, before the coroutine runs", - InstallState.Downloading(0f), - controller.installState.value, - ) - scope.testScheduler.advanceUntilIdle() - - assertEquals("one download for two taps", 1, downloads.get()) - assertEquals("one install session for two taps", 1, installs.get()) - - // Retry after Failed must still work: the claim is not a one-shot latch. - InstallStatusReceiver.emitForTest(InstallEvent.Failed("system rejected")) - scope.testScheduler.advanceUntilIdle() - assertTrue(controller.installState.value is InstallState.Failed) - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - assertEquals("retry after Failed starts a new run", 2, downloads.get()) - } - - /** - * WHY (#242): on a ROM without the unknown-app-sources settings - * activity the launcher throws ActivityNotFoundException. The user must - * see the install-error message, not a crash and not a silent reset to - * Idle. If the guard is removed the exception escapes (test errors); if - * the failure is swallowed into Idle the state assertion fails. - */ - @Test - fun missingSettingsActivityIsReportedAsInstallFailure() = runTest { - val scope = TestScope(UnconfinedTestDispatcher(testScheduler)) - val controller = UpdateController(FakeInstaller(), scope) - val needsPermission = InstallState.NeedsPermission(Intent("test.settings")) - - controller.launchPermissionIntent(needsPermission) { - throw ActivityNotFoundException("no settings activity") - } - - val s = controller.installState.value - assertTrue(s is InstallState.Failed) - assertEquals( - "Can't open the install-permission screen on this device", - (s as InstallState.Failed).message, - ) - } - - /** - * WHY (#242): the normal path must hand the intent to the launcher and - * clear the NeedsPermission state, otherwise the dialog stays up after - * the user came back from the settings screen. - */ - @Test - fun launchedPermissionIntentResetsInstallState() = runTest { - val scope = TestScope(UnconfinedTestDispatcher(testScheduler)) - val intent = Intent("test.settings") - val controller = UpdateController( - FakeInstaller(onInstall = { throw ApkInstaller.NeedsPermissionException(intent) }), - scope, - ) - controller.installUpdate(info) - scope.testScheduler.advanceUntilIdle() - val needsPermission = controller.installState.value as InstallState.NeedsPermission - - var launched: Intent? = null - controller.launchPermissionIntent(needsPermission) { launched = it } - - assertEquals(intent, launched) - assertEquals(InstallState.Idle, controller.installState.value) - } -} diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/data/update/UpdateRepositoryTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/data/update/UpdateRepositoryTest.kt new file mode 100644 index 00000000..507755a4 --- /dev/null +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/data/update/UpdateRepositoryTest.kt @@ -0,0 +1,159 @@ +package io.github.nofuturekid.nova.data.update + +import io.github.nofuturekid.nova.data.model.ApkAsset +import io.github.nofuturekid.nova.data.model.UpdateInfo +import io.github.nofuturekid.nova.data.model.UpdateState +import io.github.nofuturekid.nova.data.update.UpdateRepository.Companion.evaluate +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * The update check only announces a release; the user downloads and installs + * it in the browser (ADR-0046). These tests pin what the app sends the + * browser to, because that is all that is left of the updater and a wrong + * link is the one way it can still fail the user. + */ +class UpdateRepositoryTest { + + private val sha = "a".repeat(64) + + private fun release( + tag: String, + prerelease: Boolean = false, + draft: Boolean = false, + assets: String = apkAsset(tag), + ) = """ + {"tag_name":"$tag","draft":$draft,"prerelease":$prerelease, + "html_url":"https://github.com/hellwand/nova/releases/tag/$tag", + "body":"notes","published_at":"2026-11-13T10:00:00Z","assets":[$assets]} + """.trimIndent() + + private fun apkAsset(tag: String, digest: String? = "sha256:$sha") = """ + {"name":"NOVA-$tag.apk","size":4096, + "browser_download_url":"https://github.com/hellwand/nova/releases/download/$tag/NOVA-$tag.apk" + ${if (digest != null) ""","digest":"$digest"""" else ""}} + """.trimIndent() + + private fun json(vararg releases: String) = releases.joinToString(",", "[", "]") + + private fun available(state: UpdateState): UpdateInfo { + assertTrue("expected Available, was $state", state is UpdateState.Available) + return (state as UpdateState.Available).info + } + + /** + * WHY: the browser must land on the APK itself so one tap starts the + * download; the page link is only the fallback. + */ + @Test + fun releaseWithApkOpensTheApkUrl() { + val info = available(evaluate(json(release("v0.1.47")), false, "0.1.46")) + assertEquals( + "https://github.com/hellwand/nova/releases/download/v0.1.47/NOVA-v0.1.47.apk", + info.downloadUrl, + ) + assertEquals("https://github.com/hellwand/nova/releases/tag/v0.1.47", info.releaseUrl) + assertEquals(4096L, info.apk?.sizeBytes) + } + + /** + * WHY: a newer release must still be announced when its APK is missing + * (it used to be skipped silently, hiding the update); the user then + * gets the release page instead of a dead download. + */ + @Test + fun releaseWithoutApkOpensTheReleasePage() { + val info = available(evaluate(json(release("v0.1.47", assets = "")), false, "0.1.46")) + assertNull(info.apk) + assertEquals("https://github.com/hellwand/nova/releases/tag/v0.1.47", info.downloadUrl) + } + + @Test + fun nonApkAssetsAreIgnored() { + val txt = """{"name":"SHA256SUMS.txt","size":10,"browser_download_url":"https://github.com/x/SHA256SUMS.txt"}""" + val info = available(evaluate(json(release("v0.1.47", assets = txt)), false, "0.1.46")) + assertNull(info.apk) + } + + /** + * WHY: stable users must not be offered betas; the switch in Settings is + * the only way to opt in. This was true before the browser change and + * must stay true. + */ + @Test + fun prereleaseIsOfferedOnlyWhenIncluded() { + val body = json(release("v0.1.47-beta1", prerelease = true)) + assertEquals(UpdateState.UpToDate, evaluate(body, false, "0.1.46")) + assertEquals("0.1.47-beta1", available(evaluate(body, true, "0.1.46")).version) + } + + @Test + fun draftsAreNeverOffered() { + val body = json(release("v0.1.47", draft = true)) + assertEquals(UpdateState.UpToDate, evaluate(body, true, "0.1.46")) + } + + @Test + fun sameOrOlderVersionIsUpToDate() { + val body = json(release("v0.1.46"), release("v0.1.45")) + assertEquals(UpdateState.UpToDate, evaluate(body, false, "0.1.46")) + } + + /** + * WHY: ADR-0034 #1 fail-closed. A link rewritten to plain HTTP must not + * reach the browser, whether it is the APK or the release page. + */ + @Test + fun nonHttpsLinkIsRefused() { + val http = """{"name":"a.apk","size":1,"browser_download_url":"http://github.com/hellwand/nova/releases/download/v0.1.47/a.apk"}""" + assertTrue(evaluate(json(release("v0.1.47", assets = http)), false, "0.1.46") is UpdateState.Error) + val httpPage = release("v0.1.47").replace("https://github.com/hellwand/nova/releases/tag", "http://github.com/hellwand/nova/releases/tag") + assertTrue(evaluate(json(httpPage), false, "0.1.46") is UpdateState.Error) + } + + /** + * WHY: the app no longer compares the file with a digest, so the only + * thing left to protect the user is where the browser is sent. A + * tampered API response must not be able to open an https page on + * another host or in another repository. + */ + @Test + fun linkOutsideThisRepositorysReleasesIsRefused() { + val otherHostApk = """{"name":"a.apk","size":1,"browser_download_url":"https://evil.example/NOVA.apk"}""" + assertTrue(evaluate(json(release("v0.1.47", assets = otherHostApk)), false, "0.1.46") is UpdateState.Error) + val otherRepoApk = """{"name":"a.apk","size":1,"browser_download_url":"https://github.com/someone/else/releases/download/v0.1.47/a.apk"}""" + assertTrue(evaluate(json(release("v0.1.47", assets = otherRepoApk)), false, "0.1.46") is UpdateState.Error) + val lookalikeHost = """{"name":"a.apk","size":1,"browser_download_url":"https://github.com.evil.example/hellwand/nova/releases/download/v0.1.47/a.apk"}""" + assertTrue(evaluate(json(release("v0.1.47", assets = lookalikeHost)), false, "0.1.46") is UpdateState.Error) + val otherHostPage = release("v0.1.47").replace("https://github.com/hellwand/nova/releases/tag", "https://evil.example/releases/tag") + assertTrue(evaluate(json(otherHostPage), false, "0.1.46") is UpdateState.Error) + val otherRepoPage = release("v0.1.47").replace("hellwand/nova/releases/tag", "someone/else/releases/tag") + assertTrue(evaluate(json(otherRepoPage), false, "0.1.46") is UpdateState.Error) + } + + /** + * WHY: the shown checksum is what the user compares with the downloaded + * file, so only a real SHA-256 may be shown; anything else is omitted + * rather than displayed as if it were verifiable. + */ + @Test + fun checksumIsShownOnlyForAValidSha256Digest() { + fun sha(digest: String?) = ApkAsset("https://x/a.apk", 1, digest).sha256 + assertEquals(sha, sha("sha256:$sha")) + assertEquals(sha, sha("SHA256:${sha.uppercase()}")) + assertNull(sha(null)) + assertNull(sha("sha512:$sha")) + assertNull(sha("sha256:abc")) + assertNull(sha("sha256:${"z".repeat(64)}")) + } + + @Test + fun digestFromTheApiReachesTheUpdateInfo() { + val info = available(evaluate(json(release("v0.1.47")), false, "0.1.46")) + assertEquals(sha, info.apk?.sha256) + val noDigest = release("v0.1.47", assets = apkAsset("v0.1.47", digest = null)) + assertNull(available(evaluate(json(noDigest), false, "0.1.46")).apk?.sha256) + } +} diff --git a/app/src/test/kotlin/io/github/nofuturekid/nova/ui/screens/main/StaleCrossServerTest.kt b/app/src/test/kotlin/io/github/nofuturekid/nova/ui/screens/main/StaleCrossServerTest.kt index e4ff9262..71bcf4e3 100644 --- a/app/src/test/kotlin/io/github/nofuturekid/nova/ui/screens/main/StaleCrossServerTest.kt +++ b/app/src/test/kotlin/io/github/nofuturekid/nova/ui/screens/main/StaleCrossServerTest.kt @@ -45,8 +45,7 @@ import org.junit.Test * asserted to STILL leak server A's Content under server B — so this suite * genuinely fails if the guard is removed or the identity key regresses. * - * Collector style mirrors UpdateControllerTest (launch into a list + - * advanceUntilIdle); JUnit + kotlinx-coroutines-test only, no new deps. + * Collector style: launch into a list + advanceUntilIdle; JUnit + kotlinx-coroutines-test only, no new deps. */ @OptIn(ExperimentalCoroutinesApi::class) class StaleCrossServerTest { diff --git a/docs/adr/0008-in-app-updater-packageinstaller.md b/docs/adr/0008-in-app-updater-packageinstaller.md index 55e8a706..caf09f22 100644 --- a/docs/adr/0008-in-app-updater-packageinstaller.md +++ b/docs/adr/0008-in-app-updater-packageinstaller.md @@ -1,6 +1,6 @@ # ADR-0008: In-app updater via `REQUEST_INSTALL_PACKAGES` + `PackageInstaller` -- **Status**: Accepted +- **Status**: Superseded in part by [ADR-0046](./0046-update-via-browser-download.md) (the install part, 2026-11-13) - **Date**: 2026-05-14 - **Tags**: ui, security, release @@ -55,3 +55,7 @@ Auto-check fires once on app launch. Manual "Check now" available in Settings. N - `app/src/main/kotlin/.../ui/screens/update/UpdateBanner.kt`, `UpdateDialog.kt` - ADR-0005 — pre-release tag convention used by the updater's filter. - ADR-0012 — Settings entry-point (added later). + +## Amendment 2026-11-13 — the install part is superseded by ADR-0046 + +[ADR-0046](./0046-update-via-browser-download.md) supersedes the permission (`REQUEST_INSTALL_PACKAGES`), `UpdateInstaller`, `InstallStatusReceiver`, the permission flow and the in-app APK download described above: Google Play Protect blocked every build that could install packages, and the build without that ability was rated safe. What stays from this ADR is the update _check_ (`UpdateRepository`, the "Include pre-releases" filter, banner and sheet); "Install" became "Download", which sends the browser to the release APK. The text above stays as written. diff --git a/docs/adr/0034-update-and-data-at-rest-hardening.md b/docs/adr/0034-update-and-data-at-rest-hardening.md index fad63719..7c939099 100644 --- a/docs/adr/0034-update-and-data-at-rest-hardening.md +++ b/docs/adr/0034-update-and-data-at-rest-hardening.md @@ -1,6 +1,6 @@ # ADR-0034: Verify the update APK and harden data-at-rest / transport -- **Status**: Accepted +- **Status**: Accepted; #1 (verification of the downloaded APK) replaced by [ADR-0046](./0046-update-via-browser-download.md) (2026-11-13) - **Date**: 2026-05-19 - **Tags**: security, ci/release, data @@ -133,3 +133,7 @@ URLs is verified by static review of the backup/transfer rules, not on-device. Tier-3 (ADR-0027) is satisfied for the testable surface; the latent paths stay CI/review-covered until a post-beta6 self-update exercises them. + +## Amendment 2026-11-13 — #1 replaced by ADR-0046 + +The app no longer downloads or installs the update, so the size and SHA-256 verification of #1 is gone. [ADR-0046](./0046-update-via-browser-download.md) replaces it: the update sheet shows the checksum for the user to compare, and the HTTPS rule is kept and tightened to links under this repository's GitHub releases. The other items of this ADR are unchanged. The text above stays as written. diff --git a/docs/adr/0040-fdroid-play-distribution-flavors.md b/docs/adr/0040-fdroid-play-distribution-flavors.md index 32eac409..a7d02d89 100644 --- a/docs/adr/0040-fdroid-play-distribution-flavors.md +++ b/docs/adr/0040-fdroid-play-distribution-flavors.md @@ -1,6 +1,6 @@ # ADR-0040: F-Droid + Play distribution via two Gradle product flavors -- **Status**: Accepted +- **Status**: Accepted; superseded in part by [ADR-0046](./0046-update-via-browser-download.md) (`REQUEST_INSTALL_PACKAGES` in `direct`, 2026-11-13) - **Date**: 2026-05-21 - **Tags**: release, build, distribution, legal @@ -99,3 +99,7 @@ The original sourceSet-split sketch is preserved above for posterity. If reprodu ## Amendment 2026-11-10 — both flavors are signed in CI The Signing paragraph says the `store` flavor is "unsigned in CI output". `app/build.gradle.kts` signs both release flavors with the release keystore when it is present, so CI's `assembleRelease` output for `store` is signed too. This is harmless: the `store` APK is not uploaded (only `direct` is), and F-Droid / Play re-sign it with their own keys regardless. + +## Amendment 2026-11-13 — no install permission in `direct` any more + +[ADR-0046](./0046-update-via-browser-download.md) supersedes the parts that say the `direct` flavor declares `REQUEST_INSTALL_PACKAGES` and carries the in-app installer: `app/src/direct/AndroidManifest.xml` is gone and no flavor declares the permission. The `direct`/`store` split, `BuildConfig.HAS_UPDATER` (the update _check_ stays `direct`-only) and the signing-key consequences are unchanged. The text above stays as written. diff --git a/docs/adr/0046-update-via-browser-download.md b/docs/adr/0046-update-via-browser-download.md new file mode 100644 index 00000000..dcb8ec71 --- /dev/null +++ b/docs/adr/0046-update-via-browser-download.md @@ -0,0 +1,65 @@ +# ADR-0046: Offer updates of the GitHub build as a browser download, without an in-app installer + +- **Status**: Accepted (2026-11-13) +- **Date**: 2026-11-13 +- **Tags**: ui, security, release, distribution + +## Context + +The `direct` flavor (the APK on the GitHub Releases page) has installed its own updates since the in-app updater landed (ADR-0008): it declares `REQUEST_INSTALL_PACKAGES`, downloads the APK itself and hands it to a `PackageInstaller` session. ADR-0040 moved the permission into the `direct` flavor only; the F-Droid build (`store` flavor) has neither the permission nor the installer entry point. + +**What was observed (2026-11-13).** Google Play Protect stopped every current GitHub build of NOVA when it was installed or updated: v0.1.45-beta1, v0.1.45 and v0.1.46-beta1, each a new file. It happened on a real Android 9 phone, and a maintainer's Pixel on Android 17 was stopped as well. Play Protect first showed "App scan recommended"; after the user tapped "Scan app" it answered "Harmful app blocked – This app may be harmful" without naming a reason. The only way past it is "More details > Install anyway". The F-Droid build never showed any warning. Issues #306 and #307 describe what the user sees. + +**What is proven.** From the phone's system log and Google's documentation: + +- The warning came from Google's servers, not from the phone's own check: the on-device analysis of the APK found nothing; Google's server-side lookup then asked for a scan, and the cloud scan returned the "potentially unwanted / generic malware" verdict. The user then continued and the file was uploaded to Google. +- The first dialog ("App scan recommended") is documented behaviour for an app Play Protect has never seen: every GitHub release is a new file that is not on Google Play, so this prompt is expected for every release, whatever else is true. +- Nothing in the APK that could trigger a verdict changed between 0.1.44 and 0.1.45 (manifest and code strings differ only in version and the live-stream watchdog texts). +- Every GitHub build since the updater landed (2026-05-14) has been able to install packages. +- The verdict wording is not one of the categories Google documents, so Google's documentation does not say why it was given. + +**What the A/B on the phone showed (2026-11-13).** The official CI build of this pull request is the same version (0.1.46-beta1) with the same signing key, but without `REQUEST_INSTALL_PACKAGES` and without the installer code (its manifest declares only `INTERNET` and `ACCESS_NETWORK_STATE`). It went through the same Google cloud scan 35 minutes later, on the same phone, and was rated "This app seems safe"; the system installer then also says the app needs no special access. The new flow was tried on the phone as well: the banner and the Settings button, the sheet with notes, size and SHA-256, Download (opens the release APK in the browser) and Release page (opens the release page) all work. + +This shows the verdict changed when the permission and the installer code were removed. It does not prove they are the only factor: one phone, one day, one Play Store version, and Google's classifier can change. The "App scan recommended" step itself remains for every new file. + +**Supporting, not proof for NOVA.** Another open-source app with a built-in updater (OpenAmpere) received the identical verdict and found in an A/B test on a Google Play emulator image that the permission alone was enough (with the permission renamed and nothing else changed, the build was rated "looks safe"); its release without the updater installs without a warning. Google's Play policy also names apps that update themselves outside Google Play as what the permission is restricted for. Why the warning appeared only now is open; other projects report the same verdict since late October 2026, which may point to a change on Google's side. + +## Decision + +Remove the ability to install packages from the `direct` flavor, and keep only the update _notice_. + +- Keep the update check: latest release, the "Include pre-releases" switch, the "Update available" banner and sheet with version, size and release notes. +- Replace "Install" with **Download**, which opens the release's APK URL in the browser (`ACTION_VIEW`), and a second, smaller **Release page** link. A release without an APK asset is still announced; its single action opens the release page. +- The sheet says that NOVA does not install updates itself, that the browser downloads the file and the user opens it to install, and that Android may ask to allow the browser or file manager to install apps and Play Protect may ask to scan the new version first. It shows the SHA-256 that GitHub lists for the asset (when the API provides one) so the user can compare it. +- Remove `REQUEST_INSTALL_PACKAGES` and guard its absence: the main manifest carries a `tools:node="remove"` entry so no flavor and no library can bring it back, and a unit test fails if a manifest declares it or the entry is dropped. Also remove (the now empty `app/src/direct/AndroidManifest.xml`), the `PackageInstaller` session, the status receiver, the permission request flow, the in-app APK download with its cache handling, and their tests and texts. No feature flag: the capability is gone from the code, not hidden. +- The `store` flavor is unchanged in behaviour (`BuildConfig.HAS_UPDATER` stays `false`, no update UI, no GitHub requests). Because the installer classes lived in `main`, they also disappear from the `store` binary, where they were dormant. + +This supersedes the install part of ADR-0008 (permission, installer, receiver, permission flow) and the `direct`-flavor permission in ADR-0040, and replaces the verification in ADR-0034 #1 (the app no longer handles the file; it can only show the checksum). What remains of ADR-0034 #1 is the HTTPS rule, tightened: the browser is only sent to HTTPS links under `https://github.com/hellwand/nova/releases/`, anything else is an error. + +## Consequences + +- **Positive.** The build without the permission was rated safe where the build with it was blocked, so new GitHub releases should no longer be rated harmful, and the in-app message that users cannot act on (#307) and the permission round trip (#306) disappear with the code that produced them. The `direct` flavor gets smaller and the two flavors differ less. The app can no longer be made to install anything. +- **Negative / trade-offs.** + - No one-tap update any more: download in the browser, open the file, confirm, and possibly allow the browser or file manager to install apps once. + - The app no longer verifies the downloaded file; it can only show the checksum. The file is still protected by HTTPS, by the link check above and by Android refusing an update signed with a different key. + - Users of versions up to 0.1.46 still have the old updater. Their update to the first release with this change is installed by the old code and may show the Play Protect warning one last time. + - If Google rates a later build as harmful again, the one-tap update was given up for nothing. The "App scan recommended" prompt for a never-seen file stays either way, and users may mistake it for a failed fix; the sheet, the README and the website say it is expected. +- **Trigger to revisit.** Revisit if a release built from this code is flagged "harmful" again (then the cause is elsewhere and the updater with a better failure text from #307 is worth keeping), or if Google documents a way to register sideloaded apps or NOVA is distributed through a store that updates it (then the in-app installer or a direct link can be reconsidered). + +## Alternatives considered + +- **Keep the in-app updater and show a better message (#307).** Cheapest, and fine if the lead is wrong. Rejected, because users still hit the "harmful" verdict first and the message only explains how to click through it; it stays the fallback if a release of this code is flagged again. +- **Appeal to Google.** The form asks for a public VirusTotal upload, is answered with no reply, and peers report that the verdict is computed again for every new file, so a new release is flagged again. Not rejected forever, but not a fix. +- **Recommend F-Droid only.** The F-Droid build has neither the permission nor the installer and never showed a warning, but it has stable releases only, a different signing key (switching means uninstalling and losing the saved servers), and F-Droid builds of other apps have been flagged for other reasons. Remains available as a hint next to the browser download (README and website). +- **Hand the file to the system installer with `ACTION_INSTALL_PACKAGE` instead of a session.** Android requires the same permission for that intent, so it would keep the suspected trigger with a worse flow. +- **Download with `DownloadManager` and open the file from the download notification.** Needs no permission, but is more code, differs between phone makers, and NOVA cannot check what the user opens. A possible refinement of this decision later. +- **Change TLS handling, cleartext, shrinking, package name or signing key.** No evidence that any of them matters (the F-Droid build contains the same code and is not flagged), and a package rename would make NOVA a new app for every user. + +## References + +- Issues #306 and #307 (what the user sees on a blocked in-app update). +- ADR-0008 (in-app updater), ADR-0034 (update integrity), ADR-0040 (distribution flavors). +- Google, Play Protect client protections: +- Google, warning guidance for developers: +- Google, Play policy on `REQUEST_INSTALL_PACKAGES`: +- OpenAmpere, same verdict and A/B test: diff --git a/docs/adr/README.md b/docs/adr/README.md index 61038588..fbdfb909 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -42,7 +42,7 @@ ADRs live under `docs/` so the CI docs-only bypass applies — pure-ADR PRs comp | [0005](./0005-prerelease-tag-convention.md) | Pre-release tag convention `-beta1` / `-rc1` (trailing digit required) | Accepted | | [0006](./0006-pragmatic-beta-first-release-policy.md) | Pragmatic risk-categorised beta-first release policy | Accepted | | [0007](./0007-apollo-json-scalar-anyadapter.md) | Apollo JSON scalar via built-in `AnyAdapter` | Accepted | -| [0008](./0008-in-app-updater-packageinstaller.md) | In-app updater via `REQUEST_INSTALL_PACKAGES` + `PackageInstaller` | Accepted | +| [0008](./0008-in-app-updater-packageinstaller.md) | In-app updater via `REQUEST_INSTALL_PACKAGES` + `PackageInstaller` | Superseded in part by ADR-0046 (install part) | | [0009](./0009-docs-only-ci-bypass.md) | Docs-only CI bypass via path diff + step gating | Superseded by ADR-0029 | | [0010](./0010-release-cleanup-keeps-tags.md) | Release cleanup keeps tags (never `--cleanup-tag`) | Accepted (release-notes content amended by [ADR-0031](./0031-curated-changelog.md); the tag-keeping + compare-link rationale here is unchanged) | | [0011](./0011-cancel-in-progress-only-on-pr-branches.md) | `cancel-in-progress` only on PR branches, not main | Mooted 2026-05-17 (superseded by ADR-0014) | @@ -68,15 +68,16 @@ ADRs live under `docs/` so the CI docs-only bypass applies — pure-ADR PRs comp | [0031](./0031-curated-changelog.md) | Curated CHANGELOG.md drives release notes | Accepted | | [0032](./0032-notifications-actions.md) | In-app notification actions (archive / unread / delete) | Accepted | | [0033](./0033-github-pages-preview.md) | Publish the interactive UI prototype to GitHub Pages | Accepted | -| [0034](./0034-update-and-data-at-rest-hardening.md) | Verify the update APK; harden data-at-rest / transport | Accepted | +| [0034](./0034-update-and-data-at-rest-hardening.md) | Verify the update APK; harden data-at-rest / transport | Accepted (#1 replaced by ADR-0046) | | [0035](./0035-apikey-store-decrypt-failure-semantics.md) | Distinguish absent vs. undecryptable stored API keys (amends ADR-0024) | Accepted | | [0036](./0036-ci-governance-tests-cache-codeql.md) | Run unit tests in CI (blocking); basic Gradle cache; CodeQL v4 + default-branch push | Accepted | | [0037](./0037-resilient-action-launchers.md) | Resilient action launchers — failed user actions surface, never crash | Accepted | | [0038](./0038-lime-trademark-compliance-rename-pending.md) | Lime Technology trademark compliance — rename required (pending name) | Accepted | | [0039](./0039-rename-to-nova.md) | Rename app to NOVA for Lime trademark compliance | Accepted | -| [0040](./0040-fdroid-play-distribution-flavors.md) | F-Droid + Play distribution via two Gradle product flavors | Accepted | +| [0040](./0040-fdroid-play-distribution-flavors.md) | F-Droid + Play distribution via two Gradle product flavors | Accepted (`REQUEST_INSTALL_PACKAGES` in `direct` superseded by ADR-0046) | | [0041](./0041-host-ssl-entry-and-self-signed-local-trust.md) | Host+SSL connection entry and self-signed local-cert trust (TOFU) | Accepted | | [0042](./0042-graphql-subscription-network-pilot.md) | GraphQL subscription transport — network throughput pilot | Accepted | | [0043](./0043-graphql-subscription-metrics-docker-rollout.md) | Roll the GraphQL subscription transport out to CPU, memory, temperature, and docker stats | Accepted | | [0044](./0044-container-detail-live-stats-overlay.md) | Surface live CPU/Mem/Net/Disk in the container detail sheet from the docker stats overlay | Accepted | | [0045](./0045-array-disk-display.md) | Array disk display — sleeping disks, per-disk thresholds, errors | Accepted (updated beta10 — 2026-05-30) | +| [0046](./0046-update-via-browser-download.md) | Offer updates of the GitHub build as a browser download, without an in-app installer | Accepted | diff --git a/docs/privacy/POLICY.md b/docs/privacy/POLICY.md index 4e84662f..f0670a88 100644 --- a/docs/privacy/POLICY.md +++ b/docs/privacy/POLICY.md @@ -18,12 +18,12 @@ NOVA accesses the following on your device: NOVA initiates network requests to: - **Your own Unraid server**, at the addresses you configured, using the Unraid API/Connect plugin's GraphQL endpoint. All app functionality runs through this. -- **GitHub** (`api.github.com`, `objects.githubusercontent.com`), only when: - - The app starts (a check for app updates runs once at launch), you open Settings, or you press "Check now" - - You install an in-app update (the APK is downloaded from a GitHub Release asset) +- **GitHub** (`api.github.com`), only when the app starts (a check for app updates runs once at launch), you open Settings, or you press "Check now". These requests appear in GitHub's standard server logs with no NOVA-specific identifier beyond the default `User-Agent` header. *(`direct` flavor only — see "Per-flavor differences" below.)* + When you tap "Download" on an available update, the app makes no request itself: it asks your browser to open the release's APK (or its release page) on GitHub. That download is made by your browser, under the browser's own headers and privacy terms. + - **Container icon hosts** (both flavors). The Docker tab and container details show each container's icon. The icon URL is supplied by your own Unraid server (from the container's template), and is usually hosted by a third party chosen by the template author — for example `raw.githubusercontent.com`, `cdn.jsdelivr.net` or `github.com`. The app fetches these URLs directly, so those hosts see your IP address and the standard request headers. NOVA never adds credentials or identifiers to these requests, and decides neither which hosts are contacted nor which icons exist: that comes from the templates on your server. The app does **not** send data to any other service. No analytics, no telemetry, no crash reporting, no advertising SDKs, no third-party authentication providers. @@ -47,18 +47,19 @@ NOVA declares these Android permissions (visible in the app's permission list un - **`INTERNET`** — required to reach your Unraid server, container icon hosts and (`direct` flavor) GitHub - **`ACCESS_NETWORK_STATE`** — lets the app check whether a network is available -- **`REQUEST_INSTALL_PACKAGES`** *(`direct` flavor only — GitHub-distributed APK)* — required for the in-app updater to install a newer APK. The F-Droid and Play Store flavors of NOVA do not declare this permission. + +NOVA does not declare `REQUEST_INSTALL_PACKAGES` in any flavor: it never installs apps itself. Updates of the GitHub APK are downloaded and opened by your browser and Android's installer. (Versions up to 0.1.46 did declare it, in the GitHub APK only.) ## Per-flavor differences NOVA ships in two flavors that differ only in their distribution channel: -- **`direct`** — installed from the [GitHub Releases page](https://github.com/hellwand/nova/releases). Includes the in-app updater (talks to `api.github.com` and downloads APKs from GitHub Releases over HTTPS). -- **`store`** — installed from F-Droid (and, in the future, the Google Play Store). The in-app updater is absent. App updates are managed by your store of choice. No GitHub traffic for app updates or update checks. Container icons are still fetched from the URLs your server provides (see "Data the app sends"). +- **`direct`** — installed from the [GitHub Releases page](https://github.com/hellwand/nova/releases). Checks for updates (talks to `api.github.com`) and offers the new APK as a browser download from GitHub Releases; it does not install updates itself. +- **`store`** — installed from F-Droid (and, in the future, the Google Play Store). The update check is absent. App updates are managed by your store of choice. No GitHub traffic for app updates or update checks. Container icons are still fetched from the URLs your server provides (see "Data the app sends"). ## Update integrity (`direct` flavor only) -In-app updates verify the downloaded APK against a SHA-256 hash and byte-size match published in the GitHub release asset metadata, over HTTPS. Mismatch fails closed — nothing is installed. See [ADR-0034](https://github.com/hellwand/nova/blob/main/docs/adr/0034-update-and-data-at-rest-hardening.md) for the rationale. +The app does not download or install updates, so it cannot verify the file. The update sheet shows the SHA-256 that GitHub lists for the APK (when GitHub provides one) so you can compare it with the downloaded file. The update check runs over HTTPS and refuses any link that is not an HTTPS release link of the NOVA repository on GitHub. See [ADR-0034](https://github.com/hellwand/nova/blob/main/docs/adr/0034-update-and-data-at-rest-hardening.md) and [ADR-0046](https://github.com/hellwand/nova/blob/main/docs/adr/0046-update-via-browser-download.md). ## Your rights diff --git a/site/index.html b/site/index.html index 4b8fc223..e1931ee4 100644 --- a/site/index.html +++ b/site/index.html @@ -248,7 +248,8 @@

F-Droid

GitHub

Download the APK from the latest release. This build has an - in-app updater that checks GitHub, and an optional + update check that tells you about new releases and sends your + browser to the file, and an optional “Include pre-releases” setting if you want betas. Android asks you to allow installs from your browser or files app, which is expected for apps from outside a store. @@ -272,6 +273,73 @@

GitHub

servers again.

+

Google Play Protect

+

+ Versions up to 0.1.46 (GitHub build). On + 2026-11-13 we observed that Google Play Protect stopped every + GitHub build we tried (0.1.45-beta1, 0.1.45 and 0.1.46-beta1) when + it was installed or updated, on a phone with Android 9 and on one + with Android 17. Two dialogs appear one after the other: +

+
    +
  • + “App scan recommended” (German: + “App-Scan empfohlen”). Play Protect does not know the + file yet. Android shows this for every new file that is not from + Google Play. Tap Scan app (“App scannen”). +
    • +
  • + “Harmful app blocked” (German: + “Schädliche App blockiert”), with “This + app may be harmful” (“Diese App könnte + schädlich sein.”). You can continue through + More details > Install anyway (“Weitere Details + > Trotzdem installieren”). +
    • +
+

+ This is Google’s rating of the file, not a finding about what + NOVA does, and Google does not say why it gives it. What we + observed, and Google has not confirmed: these builds could install + other apps (that is how the old in-app updater worked), and a build + of the same version without that ability was rated “This app + seems safe” (“Diese App ist anscheinend sicher”) + by the same scan on the same phone. NOVA is open source, and you + can check what you downloaded: +

+
    +
  • + SHA-256 of the file: the release page lists it + next to the APK. Compare it with the output of + sha256sum NOVA-v….apk. +
    • +
  • + Signing certificate: + apksigner verify --print-certs NOVA-v….apk + should show this SHA-256 digest: + 9f86baa17c1208004d9284e99597f77276c77301b06340b10075c793152b6824 +
    • +
+

+ From the next version on, NOVA no longer installs + updates itself and no longer asks for the permission to install + apps. Download in the update sheet lets your browser + download the file, and you open it to install. Android will still + ask once to scan each new file (“App scan recommended”) + because Play Protect does not know it yet; that is expected. We + cannot promise that no warning ever appears, since Google’s + rating can change. +

+

+ F-Droid is the channel without any such prompt we + have seen. It has stable releases only, gets updates later, and is + signed with a different key, so switching between it and the GitHub + build means uninstalling first. That removes your saved servers + and API keys. +

+

Requirements

  • Android 8.0 or newer.
    • @@ -361,8 +429,9 @@

    Update check (GitHub build)

    The GitHub build contacts api.github.com when the app starts, when you open Settings and when you press - “Check now”. An in-app update is downloaded from a - GitHub release. The F-Droid build makes no such requests. + “Check now”. A new version is downloaded by your + browser from the GitHub release page, not by NOVA. The F-Droid + build makes no such requests.

  • diff --git a/site/style.css b/site/style.css index 8ae19609..b634c2af 100644 --- a/site/style.css +++ b/site/style.css @@ -56,6 +56,7 @@ h3 { margin: 0 0 6px; font-size: 1.05rem; line-height: 1.3; } .lead { margin: 0 auto 24px; max-width: 60ch; font-size: 1.15rem; color: var(--text-2); } .sub { margin: 0 0 24px; max-width: 70ch; color: var(--text-2); } .cards + .sub, .install + .sub { margin: 24px 0 0; } +.plain + .sub { margin-top: 24px; } .cta { display: flex; gap: 12px; justify-content: center; flex-wrap: wrap; margin: 0 0 12px; } .note { margin: 0; font-size: 14px; color: var(--text-2); } .btn { display: inline-block; padding: 10px 20px; border: 1px solid var(--border); border-radius: 8px; background: var(--surface); color: var(--text); font-weight: 500; text-decoration: none; }