Carry minted credentials to the capability probe as a value

On a first provision of an Atomic site, the My Site screen showed a false
"Unable to connect to your site" banner. `ensureAuth` minted an application
password, but `detectCapabilities` re-read the `SiteModel` fresh and a concurrent
whole-row site write (`insertOrUpdateSite` uses `UpdateAllExceptId`) had clobbered
the just-encrypted credential columns before that read (#22905). With no
credentials present, the authenticated direct-host probe was skipped, the
unauthenticated discovery failed against the private host, and the probe reported
the site unreachable.

`ensureAuth` now returns the credentials it obtained in an internal `AuthResult`,
and `detectCapabilities` overlays that immutable value onto its own
coroutine-local `SiteModel` copy. The stages still read fresh and write only their
own column — no `SiteModel` is shared or mutated across the parallel
detect/`recoverXmlRpc` branches, so there's no race.

Verified on-device against a private Atomic site: the authenticated direct-host
probe runs and the banner is gone.

Author: jkmassel

WordPress/src/main/java/org/wordpress/android/repositories/SiteProvisioningSource.kt

@@ -138,45 +138,55 @@ class SiteProvisioningSource @Inject constructor(
     private fun shouldRun(siteLocalId: Int): Boolean =
         jobs[siteLocalId]?.isActive != true && siteLocalId !in ready
 
-    private suspend fun runPipeline(siteLocalId: Int): SiteReadiness =
-        when (val auth = ensureAuth(siteLocalId)) {
+    private suspend fun runPipeline(siteLocalId: Int): SiteReadiness {
+        val auth = ensureAuth(siteLocalId)
+        return when (auth.state) {
             SiteAuthState.Provisioned, SiteAuthState.NotApplicable -> coroutineScope {
-                // Post-auth, the REST-capability chain and the XML-RPC recovery are independent —
-                // each reads the site fresh and writes only its own column — so run them in
-                // parallel. recoverRestUrlIfNeeded precedes detectCapabilities within its branch because
-                // the probe needs the recovered REST root.
-                val capabilities = async { recoverRestUrlIfNeeded(siteLocalId); detectCapabilities(siteLocalId) }
+                // Post-auth, the REST-capability chain and the XML-RPC recovery are independent — each
+                // reads the site fresh and writes only its own column — so run them in parallel.
+                // recoverRestUrlIfNeeded precedes detectCapabilities within its branch because the probe
+                // needs the recovered REST root. The mint's credentials reach the probe as an immutable
+                // value (auth.credentials), never via a shared mutated SiteModel — the fresh-read columns
+                // can't be trusted to carry them (transient, and clobberable by a concurrent write, #22905).
+                val capabilities = async {
+                    recoverRestUrlIfNeeded(siteLocalId)
+                    detectCapabilities(siteLocalId, auth.credentials)
+                }
                 val xmlRpc = async { recoverXmlRpcIfNeeded(siteLocalId) }
                 xmlRpc.await()
                 capabilities.await()
             }
-            else -> SiteReadiness.NeedsAuth(auth)
+            else -> SiteReadiness.NeedsAuth(auth.state)
         }
+    }
 
     /**
-     * Stage 1 — ensure the site has working application-password credentials.
-     * Validates stored creds with Basic auth against the direct host; on a
-     * confirmed rejection wipes them and mints fresh ones via the FluxC Jetpack
-     * tunnel. The mint persists the credentials, so later stages read them back.
+     * Stage 1 — ensure the site has working application-password credentials, and
+     * return them. Validates stored creds with Basic auth against the direct host;
+     * on a confirmed rejection wipes them and mints fresh ones via the FluxC Jetpack
+     * tunnel. The credentials are returned in the [AuthResult] so [detectCapabilities]
+     * can authenticate with them without trusting a fresh re-read — the plain columns
+     * are transient and a concurrent whole-row site write can clobber the encrypted
+     * ones mid-run (#22905).
      */
     // Each return is a distinct auth outcome (missing site, valid, transient, minted, failed);
     // collapsing to one return would thread a result through nested branches and read worse.
     @Suppress("ReturnCount")
-    private suspend fun ensureAuth(siteLocalId: Int): SiteAuthState {
+    private suspend fun ensureAuth(siteLocalId: Int): AuthResult {
         val site = siteStore.getSiteByLocalId(siteLocalId)
-            ?: return SiteAuthState.Unprovisionable(hadCredentials = false)
+            ?: return AuthResult(SiteAuthState.Unprovisionable(hadCredentials = false))
         // WP.com Simple sites are fully proxied and OAuth-bearer-authed — no application password
         // applies (the mint returns NotSupported). Capability detection works through the proxy, so
         // treat them as ready instead of blocking detection behind a mint that can never run.
-        if (site.isWPComSimpleSite) return SiteAuthState.NotApplicable
+        if (site.isWPComSimpleSite) return AuthResult(SiteAuthState.NotApplicable)
         val hadCredentials = !applicationPasswordLoginHelper.siteHasBadCredentials(site)
         if (hadCredentials) {
             when (applicationPasswordValidator.validate(site)) {
                 ApplicationPasswordValidator.Outcome.Valid ->
-                    return SiteAuthState.Provisioned
+                    return AuthResult(SiteAuthState.Provisioned, site.provisionedCredentials())
                 ApplicationPasswordValidator.Outcome.NetworkUnavailable -> {
                     appLogWrapper.d(AppLog.T.MAIN, "A_P: Validation network error for ${site.url}")
-                    return SiteAuthState.Provisioning
+                    return AuthResult(SiteAuthState.Provisioning)
                 }
                 ApplicationPasswordValidator.Outcome.Invalid -> {
                     appLogWrapper.d(AppLog.T.MAIN, "A_P: Stored creds invalid for ${site.url}, clearing")
@@ -185,23 +195,24 @@ class SiteProvisioningSource @Inject constructor(
                 }
             }
         }
+        // createApplicationPassword mutates this local `site` with the freshly minted plain credentials.
         val createResult = siteStore.createApplicationPassword(site)
         if (!createResult.isError && createResult.credentials != null) {
             wpApiClientProvider.clearSelfHostedClient(site.id)
             appLogWrapper.d(AppLog.T.MAIN, "A_P: Headless mint succeeded for ${site.url}")
-            return SiteAuthState.Provisioned
+            return AuthResult(SiteAuthState.Provisioned, site.provisionedCredentials())
         }
         appLogWrapper.d(
             AppLog.T.MAIN,
             "A_P: Headless mint failed for ${site.url} (notSupported=${createResult.error?.notSupported})"
         )
-        return SiteAuthState.Unprovisionable(hadCredentials = hadCredentials)
+        return AuthResult(SiteAuthState.Unprovisionable(hadCredentials = hadCredentials))
     }
 
     /**
      * Stage 2a — recover the REST API root for Atomic sites minted through the
      * Jetpack tunnel (which never runs discovery and leaves `wpApiRestUrl` null).
-     * Persists the one column; the capability probe re-reads it.
+     * Persists the one column; the capability probe (sequenced after it) re-reads it.
      */
     private suspend fun recoverRestUrlIfNeeded(siteLocalId: Int) {
         val site = siteStore.getSiteByLocalId(siteLocalId) ?: return
@@ -229,12 +240,25 @@ class SiteProvisioningSource @Inject constructor(
 
     /**
      * Stage 3 — probe the REST API for editor-capability support and persist it.
-     * Reached only once auth is [SiteAuthState.Provisioned], so credentials are
-     * guaranteed present: a failure here is a real transport problem, not a
-     * pending mint.
+     * Reached only once auth is [SiteAuthState.Provisioned] / [SiteAuthState.NotApplicable].
+     * [credentials] (from [ensureAuth]) are overlaid onto this run-local copy so the
+     * authenticated direct-host probe can run even when the fresh read doesn't reflect
+     * the mint; a failure here is then a real transport problem, not a pending mint.
      */
-    private suspend fun detectCapabilities(siteLocalId: Int): SiteReadiness {
+    private suspend fun detectCapabilities(
+        siteLocalId: Int,
+        credentials: ProvisionedCredentials?,
+    ): SiteReadiness {
         val site = siteStore.getSiteByLocalId(siteLocalId) ?: return SiteReadiness.Unreachable
+        // Overlay the credentials ensureAuth obtained. The fresh read can't be trusted to carry them:
+        // the plain columns are transient (never persisted), and a concurrent whole-row site write
+        // during My Site load can clobber the encrypted ones before this read (#22905). This copy is
+        // local to this coroutine — not shared with the parallel XML-RPC stage — so the overlay is
+        // race-free.
+        credentials?.let {
+            if (site.apiRestUsernamePlain.isNullOrEmpty()) site.apiRestUsernamePlain = it.username
+            if (site.apiRestPasswordPlain.isNullOrEmpty()) site.apiRestPasswordPlain = it.password
+        }
         val ok = editorSettingsRepository.fetchEditorCapabilitiesForSite(site)
         val hasCache = editorSettingsRepository.hasCachedCapabilities(site)
         return when {
@@ -245,6 +269,26 @@ class SiteProvisioningSource @Inject constructor(
     }
 }
 
+/**
+ * Snapshot of the application-password credentials [SiteProvisioningSource.ensureAuth] obtained,
+ * handed forward to the capability probe as an immutable value rather than via a shared, mutated
+ * [SiteModel] — so the parallel stages never race on it.
+ */
+private data class ProvisionedCredentials(val username: String, val password: String)
+
+private fun SiteModel.provisionedCredentials(): ProvisionedCredentials? {
+    val user = apiRestUsernamePlain
+    val pass = apiRestPasswordPlain
+    return if (!user.isNullOrEmpty() && !pass.isNullOrEmpty()) ProvisionedCredentials(user, pass) else null
+}
+
+/**
+ * [SiteProvisioningSource.ensureAuth]'s outcome: the public [SiteAuthState] plus, when provisioned,
+ * the credentials to hand to the capability probe. Internal so the credentials never leak into the
+ * UI-facing [SiteReadiness] / [SiteAuthState].
+ */
+private data class AuthResult(val state: SiteAuthState, val credentials: ProvisionedCredentials? = null)
+
 /**
  * Whether a site's application password is usable. Owned by [SiteProvisioningSource];
  * rendered by the application-password card.

WordPress/src/test/java/org/wordpress/android/repositories/SiteProvisioningSourceTest.kt

@@ -7,6 +7,7 @@ import org.junit.Before
 import org.junit.Test
 import org.mockito.Mock
 import org.mockito.kotlin.any
+import org.mockito.kotlin.argThat
 import org.mockito.kotlin.eq
 import org.mockito.kotlin.never
 import org.mockito.kotlin.times
@@ -91,6 +92,14 @@ class SiteProvisioningSourceTest : BaseUnitTest(StandardTestDispatcher()) {
         if (!ok) whenever(editorSettingsRepository.hasCachedCapabilities(any())).thenReturn(cached)
     }
 
+    private fun provisionableSiteCopy() = SiteModel().apply {
+        id = TEST_SITE_LOCAL_ID
+        url = "https://test.example.com"
+        // Non-null REST root + XML-RPC url so both recovery branches short-circuit.
+        wpApiRestUrl = "https://test.example.com/wp-json"
+        xmlRpcUrl = "https://test.example.com/xmlrpc.php"
+    }
+
     // region auth stage
 
     @Test
@@ -174,6 +183,33 @@ class SiteProvisioningSourceTest : BaseUnitTest(StandardTestDispatcher()) {
         verify(siteStore, never()).createApplicationPassword(any())
     }
 
+    @Test
+    fun `carries minted credentials to the probe even when the re-read lacks them`() = test {
+        // ensureAuth reads one copy and the mint sets credentials on it; detectCapabilities re-reads a
+        // SEPARATE copy that — simulating a concurrent whole-row clobber (#22905) — carries none. The
+        // probe must still receive a credentialed site, because the pipeline hands the mint's creds
+        // forward as an immutable value rather than relying on the re-read or a shared, race-prone model.
+        val authCopy = provisionableSiteCopy()
+        val credentiallessReread = provisionableSiteCopy()
+        whenever(siteStore.getSiteByLocalId(TEST_SITE_LOCAL_ID)).thenReturn(authCopy, credentiallessReread)
+        stubHasStoredCredentials(false)
+        whenever(siteStore.createApplicationPassword(any())).thenAnswer { invocation ->
+            // The real createApplicationPassword sets the plain credentials on the passed site.
+            (invocation.arguments[0] as SiteModel).apply {
+                apiRestUsernamePlain = "user"
+                apiRestPasswordPlain = "pass"
+            }
+            OnApplicationPasswordCreated(authCopy, ApplicationPasswordCredentials("user", "pass", uuid = "u"))
+        }
+        stubCapabilityProbe(ok = true)
+
+        source.await(site)
+
+        verify(editorSettingsRepository).fetchEditorCapabilitiesForSite(
+            argThat { apiRestUsernamePlain == "user" && apiRestPasswordPlain == "pass" }
+        )
+    }
+
     // endregion
 
     // region recovery + capability stages