From 56d65b7b4cf2911da1dce0cf0605fde0fed211d6 Mon Sep 17 00:00:00 2001 From: adsamcik Date: Tue, 21 Jul 2026 12:18:35 +0200 Subject: [PATCH] fix(tracker): harden ActivityWatcherService foreground start against platform rejection ActivityWatcherService.startForegroundCompat had no exception handling around startForeground() at all, unlike TrackerService's already-hardened equivalent (tryStartForeground). This service can be started from background contexts (e.g. after activity-recognition transitions), so an Android 12+ ForegroundServiceStartNotAllowedException or a SecurityException on the declared foreground-service type would crash it with an uncaught exception. Extracts the tolerant-start logic into two top-level, directly testable functions mirroring TrackerService's pattern: isForegroundServiceStartRestriction (pure class-name/SDK check) and startForegroundTolerant (try/catch + Reporter logging). onCreate() now stops the service cleanly via stopSelf() instead of continuing setup when the foreground declaration is rejected. Supersedes PR #221 (closed, stale/conflicting): dev/v10 already contains a far more thorough version of that PR's core request (foreground-service type derived from active tracking config/tier rather than permission possession, with dedicated tests) via TrackerService.foregroundServiceTypeCandidates / resolveInitialPolicyTier. Service.onTimeout(int, int) (Android 15+) was verified inapplicable here -- it only fires for dataSync/mediaProcessing foreground-service types, not location/health/specialUse -- so it was dropped from scope rather than added as dead code. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 48e20dfd-a1bb-4266-bba5-2646e115c5cf --- .../tracker/service/ActivityWatcherService.kt | 69 +++++++++++++++---- ...IsForegroundServiceStartRestrictionTest.kt | 42 +++++++++++ .../service/StartForegroundTolerantTest.kt | 42 +++++++++++ 3 files changed, 141 insertions(+), 12 deletions(-) create mode 100644 tracker/engine/src/test/java/com/adsamcik/tracker/tracker/service/IsForegroundServiceStartRestrictionTest.kt create mode 100644 tracker/engine/src/test/java/com/adsamcik/tracker/tracker/service/StartForegroundTolerantTest.kt diff --git a/tracker/engine/src/main/java/com/adsamcik/tracker/tracker/service/ActivityWatcherService.kt b/tracker/engine/src/main/java/com/adsamcik/tracker/tracker/service/ActivityWatcherService.kt index 3313f472b..c192586b3 100644 --- a/tracker/engine/src/main/java/com/adsamcik/tracker/tracker/service/ActivityWatcherService.kt +++ b/tracker/engine/src/main/java/com/adsamcik/tracker/tracker/service/ActivityWatcherService.kt @@ -10,6 +10,7 @@ import android.os.Build import androidx.core.app.NotificationCompat import com.adsamcik.tracker.activity.R import com.adsamcik.tracker.activity.api.ActivityRequestManager +import com.adsamcik.tracker.logger.Reporter import com.adsamcik.tracker.shared.base.Time import com.adsamcik.tracker.shared.base.data.ActivityInfo import com.adsamcik.tracker.shared.base.extension.notificationManager @@ -48,7 +49,11 @@ class ActivityWatcherService : CoreService() { activityWatcherController.attachService(this) activityInfo = activityRequestManager.lastActivity.toLegacyActivityInfo() - startForegroundCompat(updateNotification()) + if (!startForegroundCompat(updateNotification())) { + Reporter.w(TAG, "Unable to start in foreground; stopping") + stopSelf() + return + } notificationManager = (this as Context).notificationManager @@ -78,18 +83,19 @@ class ActivityWatcherService : CoreService() { return START_REDELIVER_INTENT } - private fun startForegroundCompat(notification: Notification) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { - // SPECIAL_USE foreground service type is available from Android 14. - startForeground( - NOTIFICATION_ID, - notification, - ServiceInfo.FOREGROUND_SERVICE_TYPE_SPECIAL_USE, - ) - } else { - startForeground(NOTIFICATION_ID, notification) + private fun startForegroundCompat(notification: Notification): Boolean = + startForegroundTolerant(sdkInt = Build.VERSION.SDK_INT, tag = TAG) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { + // SPECIAL_USE foreground service type is available from Android 14. + startForeground( + NOTIFICATION_ID, + notification, + ServiceInfo.FOREGROUND_SERVICE_TYPE_SPECIAL_USE, + ) + } else { + startForeground(NOTIFICATION_ID, notification) + } } - } private fun updateNotification(): Notification { val intent = packageManager.getLaunchIntentForPackage(packageName) @@ -132,5 +138,44 @@ class ActivityWatcherService : CoreService() { companion object { private const val NOTIFICATION_ID = -568465 + private const val TAG = "ActivityWatcherService" } } + +/** + * Runs [start] and reports whether the platform's foreground-service start restrictions blocked + * it, mirroring [TrackerService]'s `tryStartForeground`. + * + * On Android 12+ ([Build.VERSION_CODES.S]) a background-restricted start throws + * `ForegroundServiceStartNotAllowedException`, checked by class name to avoid an API-31 type + * reference. A [SecurityException] can also surface if the declared foreground-service type is no + * longer permitted. Both are tolerated rejections; every other exception propagates. + * + * @return `true` when [start] completed without a tolerated rejection. + */ +internal inline fun startForegroundTolerant( + sdkInt: Int, + tag: String, + start: () -> Unit, +): Boolean = try { + start() + true +} catch (exception: SecurityException) { + Reporter.w(tag, "Foreground start rejected: ${exception.message}") + false +} catch (@Suppress("TooGenericExceptionCaught") exception: RuntimeException) { + if (!isForegroundServiceStartRestriction(sdkInt, exception::class.java.name)) throw exception + Reporter.w(tag, "Foreground start not allowed from background: ${exception.message}") + false +} + +/** + * True when [exceptionClassName] identifies `android.app.ForegroundServiceStartNotAllowedException` + * on an SDK where that platform exception can actually be thrown ([Build.VERSION_CODES.S]+). + * + * Matching by name (rather than `is ForegroundServiceStartNotAllowedException`) avoids referencing + * an API-31 type directly, since this codebase's `minSdk` predates it. + */ +internal fun isForegroundServiceStartRestriction(sdkInt: Int, exceptionClassName: String): Boolean = + sdkInt >= Build.VERSION_CODES.S && + exceptionClassName == "android.app.ForegroundServiceStartNotAllowedException" diff --git a/tracker/engine/src/test/java/com/adsamcik/tracker/tracker/service/IsForegroundServiceStartRestrictionTest.kt b/tracker/engine/src/test/java/com/adsamcik/tracker/tracker/service/IsForegroundServiceStartRestrictionTest.kt new file mode 100644 index 000000000..11fb7a7cd --- /dev/null +++ b/tracker/engine/src/test/java/com/adsamcik/tracker/tracker/service/IsForegroundServiceStartRestrictionTest.kt @@ -0,0 +1,42 @@ +package com.adsamcik.tracker.tracker.service + +import android.os.Build +import io.kotest.matchers.shouldBe +import org.junit.jupiter.api.Test + +class IsForegroundServiceStartRestrictionTest { + + @Test + fun `matching class name on API 31+ is a restriction`() { + isForegroundServiceStartRestriction( + sdkInt = Build.VERSION_CODES.S, + exceptionClassName = "android.app.ForegroundServiceStartNotAllowedException", + ) shouldBe true + } + + @Test + fun `matching class name above API 31 is a restriction`() { + isForegroundServiceStartRestriction( + sdkInt = Build.VERSION_CODES.S + 5, + exceptionClassName = "android.app.ForegroundServiceStartNotAllowedException", + ) shouldBe true + } + + @Test + fun `matching class name below API 31 is not a restriction`() { + // The platform exception cannot be thrown before API 31, so a coincidentally matching + // class name from some other RuntimeException must not be swallowed. + isForegroundServiceStartRestriction( + sdkInt = Build.VERSION_CODES.R, + exceptionClassName = "android.app.ForegroundServiceStartNotAllowedException", + ) shouldBe false + } + + @Test + fun `unrelated class name on API 31+ is not a restriction`() { + isForegroundServiceStartRestriction( + sdkInt = Build.VERSION_CODES.S, + exceptionClassName = "java.lang.IllegalStateException", + ) shouldBe false + } +} diff --git a/tracker/engine/src/test/java/com/adsamcik/tracker/tracker/service/StartForegroundTolerantTest.kt b/tracker/engine/src/test/java/com/adsamcik/tracker/tracker/service/StartForegroundTolerantTest.kt new file mode 100644 index 000000000..156ff6c31 --- /dev/null +++ b/tracker/engine/src/test/java/com/adsamcik/tracker/tracker/service/StartForegroundTolerantTest.kt @@ -0,0 +1,42 @@ +package com.adsamcik.tracker.tracker.service + +import android.os.Build +import io.kotest.matchers.shouldBe +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config + +/** + * [startForegroundTolerant] delegates its "is this a background-start restriction" decision to + * [isForegroundServiceStartRestriction] (covered exhaustively in + * [IsForegroundServiceStartRestrictionTest]); this class instead covers its own control flow — + * success, [SecurityException] tolerance, and propagation of anything else — using + * Robolectric because the failure paths call through to [Reporter], which logs via + * `android.util.Log`. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [28]) +class StartForegroundTolerantTest { + + @Test + fun `start that succeeds returns true`() { + startForegroundTolerant(sdkInt = Build.VERSION_CODES.S, tag = "test") { + // no-op: represents a successful startForeground call + } shouldBe true + } + + @Test + fun `SecurityException is tolerated and returns false`() { + startForegroundTolerant(sdkInt = Build.VERSION_CODES.S, tag = "test") { + throw SecurityException("declared foreground-service type no longer permitted") + } shouldBe false + } + + @Test(expected = IllegalStateException::class) + fun `unrelated RuntimeException propagates unchanged`() { + startForegroundTolerant(sdkInt = Build.VERSION_CODES.S, tag = "test") { + throw IllegalStateException("unrelated failure") + } + } +}