Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Verified Commit 838a0db4 authored by Fahim M. Choudhury's avatar Fahim M. Choudhury
Browse files

fix(update): block non-owned update execution 

Add ownership guards before enqueue and before final update execution so hidden or stale work cannot bypass policy.

Treat blocked updates as an expected policy outcome instead of a generic installation issue.
parent b7a38215

.agent/docs/fix-app-updates-with-different-stores/task-list.md

+17 −14
Original line number Diff line number Diff line
@@ -417,25 +417,25 @@ Block every update attempt that bypasses discovery, including manual actions, ba 
### Tasks



#### Task 4.1 - Add pre-enqueue update guard

- [ ] In `InstallationEnqueuer.kt`, check ownership when `isAnUpdate == true`.

- [ ] If blocked, do not enqueue work.

- [ ] Log the package and reason code.

- [x] In `InstallationEnqueuer.kt`, check ownership when `isAnUpdate == true`.

- [x] If blocked, do not enqueue work.

- [x] Log the package and reason code.



#### Task 4.2 - Add final execution guard

- [ ] In `InstallationProcessor.kt`, re-check ownership before the install step begins for update work.

- [ ] If blocked, stop gracefully.

- [ ] Do not convert this case into `INSTALLATION_ISSUE`.

- [ ] Prefer `Status.BLOCKED` if a persisted status is required.

- [x] In `InstallationProcessor.kt`, re-check ownership before the install step begins for update work.

- [x] If blocked, stop gracefully.

- [x] Do not convert this case into `INSTALLATION_ISSUE`.

- [x] Prefer `Status.BLOCKED` if a persisted status is required.



#### Task 4.3 - Keep behavior safe for stale work

- [ ] Make execution-time guard authoritative.

- [ ] Ensure a package hidden during discovery still cannot be updated by a stale worker request.

- [x] Make execution-time guard authoritative.

- [x] Ensure a package hidden during discovery still cannot be updated by a stale worker request.



#### Task 4.4 - Update tests

- [ ] Extend `InstallationEnqueuerTest.kt`.

- [ ] Extend `InstallationProcessorTest.kt`.

- [ ] Extend `UpdatesWorkerTest.kt` if worker behavior needs explicit coverage.

- [ ] Cover:

- [x] Extend `InstallationEnqueuerTest.kt`.

- [x] Extend `InstallationProcessorTest.kt`.

- [x] Extend `UpdatesWorkerTest.kt` if worker behavior needs explicit coverage.

- [x] Cover:

  - manual update blocked before enqueue

  - eligible update still enqueues

  - stale queued update blocked during processing
@@ -466,7 +466,10 @@ Body: 


### Iteration Notes



- Add notes here after the phase is done.

- `InstallationEnqueuer` now short-circuits blocked update requests before `canEnqueue(...)`, `updateAwaiting(...)`, or `InstallWorkManager.enqueueWork(...)` run.

- `InstallationProcessor` now re-checks ownership for effective update work before foreground/download/install starts, marks blocked entries as `Status.BLOCKED`, finishes cleanly, and returns success instead of converting policy blocks into installation issues.

- No `UpdatesWorker` code change was needed in this phase because the enqueue and execution guards already cover user-triggered and worker-triggered update paths.

- `InstallationEnqueuerTest` now covers manual update blocking and still verifies that eligible updates enqueue normally; `InstallationProcessorTest` now covers stale queued update blocking and confirms blocked updates do not become generic failures.



## Phase 5 - Add Legacy Migration Handling

app/src/main/java/foundation/e/apps/data/install/core/InstallationEnqueuer.kt

+22 −0
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import foundation.e.apps.data.install.core.helper.PreEnqueueChecker 
import foundation.e.apps.data.install.workmanager.InstallWorkManager

import foundation.e.apps.data.install.wrapper.AppEventDispatcher

import foundation.e.apps.data.installation.model.AppInstall

import foundation.e.apps.data.installation.port.UpdateEligibilityChecker

import foundation.e.apps.data.preference.PlayStoreAuthStore

import foundation.e.apps.domain.model.User

import foundation.e.apps.domain.preferences.SessionRepository
@@ -40,6 +41,7 @@ class InstallationEnqueuer @Inject constructor( 
    private val sessionRepository: SessionRepository,

    private val playStoreAuthStore: PlayStoreAuthStore,

    private val appEventDispatcher: AppEventDispatcher,

    private val updateEligibilityChecker: UpdateEligibilityChecker,

) {

    suspend fun enqueue(

        appInstall: AppInstall,
@@ -53,6 +55,8 @@ class InstallationEnqueuer @Inject constructor( 
                    false

                }



                shouldBlockUpdateEnqueue(appInstall, isAnUpdate) -> false



                canEnqueue(appInstall, isAnUpdate) -> {

                    appManagerWrapper.updateAwaiting(appInstall)

                    // Enqueueing installation work is managed by InstallOrchestrator#observeDownloads().
@@ -85,6 +89,24 @@ class InstallationEnqueuer @Inject constructor( 
        }

    }



    private fun shouldBlockUpdateEnqueue(appInstall: AppInstall, isAnUpdate: Boolean): Boolean {

        if (!isAnUpdate) {

            return false

        }



        val eligibilityResult = updateEligibilityChecker.getUpdateEligibility(appInstall.packageName)

        if (eligibilityResult.isEligible) {

            return false

        }



        Timber.w(

            "Block update enqueue by ownership policy: packageName=%s reasonCode=%s",

            appInstall.packageName,

            eligibilityResult.reasonCode,

        )

        return true

    }



    private suspend fun isEnqueueingPaidAppInAnonymouseMode(

        appInstall: AppInstall,

        isSystemApp: Boolean

app/src/test/java/foundation/e/apps/installProcessor/InstallationEnqueuerTest.kt

+42 −1
Original line number Diff line number Diff line
@@ -37,8 +37,11 @@ import foundation.e.apps.data.install.core.helper.DevicePreconditions 
import foundation.e.apps.data.install.core.helper.DownloadUrlRefresher

import foundation.e.apps.data.install.core.helper.PreEnqueueChecker

import foundation.e.apps.data.install.core.InstallationEnqueuer

import foundation.e.apps.data.installation.model.InstallSourceDetails

import foundation.e.apps.data.installation.model.UpdateEligibilityResult

import foundation.e.apps.data.installation.model.InstallationSource

import foundation.e.apps.data.installation.model.InstallationType

import foundation.e.apps.data.installation.port.UpdateEligibilityChecker

import foundation.e.apps.data.install.workmanager.InstallWorkManager

import foundation.e.apps.data.installation.port.NetworkStatusChecker

import foundation.e.apps.data.installation.port.StorageSpaceChecker
@@ -81,6 +84,7 @@ class InstallationEnqueuerTest { 
    private lateinit var devicePreconditions: DevicePreconditions

    private lateinit var downloadUrlRefresher: DownloadUrlRefresher

    private lateinit var preEnqueueChecker: PreEnqueueChecker

    private lateinit var updateEligibilityChecker: UpdateEligibilityChecker

    private lateinit var enqueuer: InstallationEnqueuer



    @Before
@@ -97,8 +101,10 @@ class InstallationEnqueuerTest { 
        storageSpaceChecker = mockk(relaxed = true)

        networkStatusChecker = mockk(relaxed = true)

        appManager = mockk(relaxed = true)

        updateEligibilityChecker = mockk(relaxed = true)

        coEvery { sessionRepository.awaitUser() } returns User.NO_GOOGLE

        coEvery { playStoreAuthStore.awaitAuthData() } returns null

        every { updateEligibilityChecker.getUpdateEligibility(any()) } returns eligibleUpdateResult()

        downloadUrlRefresher = DownloadUrlRefresher(

            applicationRepository,

            appInstallRepository,
@@ -125,7 +131,8 @@ class InstallationEnqueuerTest { 
            appManagerWrapper,

            sessionRepository,

            playStoreAuthStore,

            appEventDispatcher

            appEventDispatcher,

            updateEligibilityChecker,

        )

    }
@@ -233,6 +240,27 @@ class InstallationEnqueuerTest { 
        }

    }



    @Test

    fun enqueue_blocksManualUpdateBeforeEnqueueWhenOwnershipCheckFails() = runTest {

        val appInstall = createPwaInstall()



        mockkObject(InstallWorkManager)

        try {

            every {

                updateEligibilityChecker.getUpdateEligibility(appInstall.packageName)

            } returns blockedUpdateResult(UpdateEligibilityResult.BLOCK_OTHER_UPDATE_OWNER)



            val result = enqueuer.enqueue(appInstall, isAnUpdate = true)



            assertFalse(result)

            coVerify(exactly = 0) { appManagerWrapper.addDownload(any()) }

            coVerify(exactly = 0) { appManagerWrapper.updateAwaiting(appInstall) }

            verify(exactly = 0) { InstallWorkManager.enqueueWork(any(), any(), any()) }

        } finally {

            unmockkObject(InstallWorkManager)

        }

    }



    @Test

    fun enqueue_enqueuesUpdateWorkWhenUpdateAndChecksPass() = runTest {

        val appInstall = createPwaInstall()
@@ -249,6 +277,7 @@ class InstallationEnqueuerTest { 
            val result = enqueuer.enqueue(appInstall, isAnUpdate = true)



            assertTrue(result)

            verify(exactly = 1) { updateEligibilityChecker.getUpdateEligibility(appInstall.packageName) }

            coVerify { appManagerWrapper.updateAwaiting(appInstall) }

            verify(exactly = 1) { InstallWorkManager.enqueueWork(context, appInstall, true) }

        } finally {
@@ -494,4 +523,16 @@ class InstallationEnqueuerTest { 
        packageName = "com.example.app",

        isFree = isFree

    )



    private fun eligibleUpdateResult() = UpdateEligibilityResult(

        isEligible = true,

        reasonCode = UpdateEligibilityResult.ALLOW_INSTALLER_OF_RECORD,

        installSourceDetails = InstallSourceDetails(readSucceeded = true),

    )



    private fun blockedUpdateResult(reasonCode: String) = UpdateEligibilityResult(

        isEligible = false,

        reasonCode = reasonCode,

        installSourceDetails = InstallSourceDetails(readSucceeded = true),

    )

}

app/src/test/java/foundation/e/apps/installProcessor/InstallationProcessorTest.kt

+54 −1
Original line number Diff line number Diff line
@@ -27,8 +27,12 @@ import foundation.e.apps.data.install.download.DownloadManagerUtils 
import foundation.e.apps.data.installation.model.AppInstall

import foundation.e.apps.data.installation.core.InstallationProcessor

import foundation.e.apps.data.install.core.helper.InstallationCompletionHandler

import foundation.e.apps.data.installation.model.InstallSourceDetails

import foundation.e.apps.data.installation.model.UpdateEligibilityResult

import foundation.e.apps.data.installation.port.UpdateEligibilityChecker

import foundation.e.apps.domain.model.install.Status

import foundation.e.apps.util.MainCoroutineRule

import io.mockk.every

import io.mockk.mockk

import io.mockk.verify

import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -55,6 +59,7 @@ class InstallationProcessorTest { 
    private lateinit var fakeFusedManagerRepository: FakeAppManagerWrapper

    private lateinit var downloadManagerUtils: DownloadManagerUtils

    private lateinit var installationCompletionHandler: InstallationCompletionHandler

    private lateinit var updateEligibilityChecker: UpdateEligibilityChecker

    private lateinit var workRunner: InstallationProcessor

    private lateinit var context: Context
@@ -74,11 +79,14 @@ class InstallationProcessorTest { 
            FakeAppManagerWrapper(fakeFusedDownloadDAO, context, fakeFusedManager, fakeFDroidRepository)

        downloadManagerUtils = mockk(relaxed = true)

        installationCompletionHandler = mockk(relaxed = true)

        updateEligibilityChecker = mockk(relaxed = true)

        every { updateEligibilityChecker.getUpdateEligibility(any()) } returns eligibleUpdateResult()

        workRunner = InstallationProcessor(

            appInstallRepository,

            fakeFusedManagerRepository,

            downloadManagerUtils,

            installationCompletionHandler

            installationCompletionHandler,

            updateEligibilityChecker,

        )

    }
@@ -203,6 +211,39 @@ class InstallationProcessorTest { 
        verify { downloadManagerUtils.updateDownloadStatus(232L) }

    }



    @Test

    fun processInstall_blocksStaleQueuedUpdateDuringProcessing() = runTest {

        val fusedDownload = initTest()

        fakeFusedManagerRepository.isAppInstalled = true

        every {

            updateEligibilityChecker.getUpdateEligibility(fusedDownload.packageName)

        } returns blockedUpdateResult(UpdateEligibilityResult.BLOCK_OTHER_UPDATE_OWNER)



        val result = workRunner.processInstall(fusedDownload.id, true) {

            // _ignored_

        }

        val finalFusedDownload = fakeFusedDownloadDAO.getDownloadById(fusedDownload.id)



        assertTrue(result.isSuccess)

        assertEquals(Status.BLOCKED, finalFusedDownload?.status)

    }



    @Test

    fun processInstall_blockedUpdateDoesNotBecomeInstallationIssue() = runTest {

        val fusedDownload = initTest()

        fakeFusedManagerRepository.isAppInstalled = true

        every {

            updateEligibilityChecker.getUpdateEligibility(fusedDownload.packageName)

        } returns blockedUpdateResult(UpdateEligibilityResult.BLOCK_OTHER_INSTALLER)



        workRunner.processInstall(fusedDownload.id, true) {

            // _ignored_

        }

        val finalFusedDownload = fakeFusedDownloadDAO.getDownloadById(fusedDownload.id)



        assertEquals(Status.BLOCKED, finalFusedDownload?.status)

    }



    private suspend fun initTest(

        packageName: String? = null,

        downloadUrlList: MutableList<String>? = null
@@ -228,4 +269,16 @@ class InstallationProcessorTest { 
        downloadURLList = downloadUrlList ?: mutableListOf("apk1", "apk2"),

        packageName = packageName ?: "com.unit.test"

    )



    private fun eligibleUpdateResult() = UpdateEligibilityResult(

        isEligible = true,

        reasonCode = UpdateEligibilityResult.ALLOW_INSTALLER_OF_RECORD,

        installSourceDetails = InstallSourceDetails(readSucceeded = true),

    )



    private fun blockedUpdateResult(reasonCode: String) = UpdateEligibilityResult(

        isEligible = false,

        reasonCode = reasonCode,

        installSourceDetails = InstallSourceDetails(readSucceeded = true),

    )

}

data/src/main/java/foundation/e/apps/data/installation/core/InstallationProcessor.kt

+23 −0
Original line number Diff line number Diff line
@@ -5,6 +5,7 @@ import foundation.e.apps.data.installation.model.InstallationResult 
import foundation.e.apps.data.installation.port.InstallationAppManager

import foundation.e.apps.data.installation.port.InstallationCompletionNotifier

import foundation.e.apps.data.installation.port.InstallationDownloadStatusUpdater

import foundation.e.apps.data.installation.port.UpdateEligibilityChecker

import foundation.e.apps.data.installation.repository.AppInstallRepository

import foundation.e.apps.domain.model.install.Status

import kotlinx.coroutines.CancellationException
@@ -18,6 +19,7 @@ class InstallationProcessor @Inject constructor( 
    private val installationAppManager: InstallationAppManager,

    private val installationDownloadStatusUpdater: InstallationDownloadStatusUpdater,

    private val installationCompletionNotifier: InstallationCompletionNotifier,

    private val updateEligibilityChecker: UpdateEligibilityChecker,

) {

    @Suppress("ReturnCount")

    @OptIn(DelicateCoroutinesApi::class)
@@ -54,6 +56,10 @@ class InstallationProcessor @Inject constructor( 
            val isUpdateWork =

                isItUpdateWork && installationAppManager.isFusedDownloadInstalled(appInstall)



            if (isUpdateWork && shouldBlockUpdateExecution(appInstall)) {

                return Result.success(InstallationResult.OK)

            }



            if (areFilesDownloadedButNotInstalled(appInstall)) {

                Timber.i("===> Downloaded But not installed ${appInstall.name}")

                installationAppManager.updateDownloadStatus(appInstall, Status.INSTALLING)
@@ -79,6 +85,23 @@ class InstallationProcessor @Inject constructor( 
        }

    }



    private suspend fun shouldBlockUpdateExecution(appInstall: AppInstall): Boolean {

        val eligibilityResult = updateEligibilityChecker.getUpdateEligibility(appInstall.packageName)

        if (eligibilityResult.isEligible) {

            return false

        }



        Timber.w(

            "Block update execution by ownership policy: packageName=%s reasonCode=%s",

            appInstall.packageName,

            eligibilityResult.reasonCode,

        )

        appInstall.status = Status.BLOCKED

        installationAppManager.updateDownloadStatus(appInstall, Status.BLOCKED)

        finishInstallation(appInstall, true)

        return true

    }



    @OptIn(DelicateCoroutinesApi::class)

    private fun checkDownloadingState(appInstall: AppInstall) {

        if (appInstall.status == Status.DOWNLOADING) {