From 9448537b6b314299d17e0ec92691c71039f21fe3 Mon Sep 17 00:00:00 2001 From: null Date: Wed, 15 Jul 2026 00:58:28 -0500 Subject: [PATCH] fix(auth): make a profile write unable to unpair a couple (C-AUTH-001 hardening) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit fixed the path that reached the hazard. This removes the hazard: createUser was a whole-document set() of every field, so any caller holding a partially-loaded User wrote coupleId = null over a paired user and dropped coupleId/partnerId. The User that does this is not exotic — for ~200ms after every sign-in, users/{uid} exists with no fields on it at all. Getting a read slightly wrong should cost a no-op, not a relationship's history. createUser now merges, and UserProfileWrite omits every null or blank value, so a field is only ever written with a real value and a hollow User produces an empty map. For a document that doesn't exist yet — the only case callers actually intend — the result is byte-identical to before. Clearing a field stays the job of the targeted update* methods, where the call site says so. It also carries the same locked-placeholder require()s the targeted writers already had. plan is deliberately no longer written: User.plan defaults to "free", so merging it would silently downgrade a paying user whose document we read while it was hollow. The server owns that field (RevenueCat), reads already default a missing plan to "free", and no gate reads it — entitlements live in the server-only subdoc. Checked against firestore.rules: create requires no fields, and update's hasOnly allowlist still covers everything written. Correcting my last commit message while I'm here: it claimed CreateProfile submit calls createUser and overwrites the name. Only half true — it uses targeted merges when it sees an existing doc, and only calls createUser when it reads null. The path that actually reached the sharp edge was Google sign-in: mergeGoogleProfile decided "new account" from runCatching { getUser(uid) }.getOrNull(), which is null for a failed read as much as for a missing document — C-AUTH-001's conflation, in a place that then wrote a whole document. It also existed as two verbatim copies, in LoginViewModel and SignUpViewModel, so the same bug was there twice. Now one GoogleProfileMerger: it does nothing on a failed read, and won't seed a name over a stub (which would overwrite a returning user's real name with their Google one). Both view models drop userRepository entirely as a result. User.isSignInStub is now shared rather than private to OnboardingViewModel, since two callers need the same question answered. UserProfileWriteTest pins the rule; mutation-checked by reverting the omit rule, which fails 3 of them. Full unit suite green. Sign-in verified live on the throwaway. Co-Authored-By: Claude Opus 4.8 --- .../data/remote/FirestoreUserDataSource.kt | 42 ++++--- .../closer/data/remote/UserProfileWrite.kt | 49 ++++++++ .../main/java/app/closer/domain/model/User.kt | 12 ++ .../domain/usecase/GoogleProfileMerger.kt | 73 ++++++++++++ .../java/app/closer/ui/auth/LoginViewModel.kt | 32 +----- .../app/closer/ui/auth/SignUpViewModel.kt | 32 +----- .../data/remote/UserProfileWriteTest.kt | 105 ++++++++++++++++++ docs/Engineering_Reference_Manual.md | 5 +- 8 files changed, 276 insertions(+), 74 deletions(-) create mode 100644 app/src/main/java/app/closer/data/remote/UserProfileWrite.kt create mode 100644 app/src/main/java/app/closer/domain/usecase/GoogleProfileMerger.kt create mode 100644 app/src/test/java/app/closer/data/remote/UserProfileWriteTest.kt diff --git a/app/src/main/java/app/closer/data/remote/FirestoreUserDataSource.kt b/app/src/main/java/app/closer/data/remote/FirestoreUserDataSource.kt index 4aabebed..7297bf18 100644 --- a/app/src/main/java/app/closer/data/remote/FirestoreUserDataSource.kt +++ b/app/src/main/java/app/closer/data/remote/FirestoreUserDataSource.kt @@ -95,21 +95,35 @@ class FirestoreUserDataSource @Inject constructor( awaitClose { listener.remove() } } + /** + * Write a user's profile without ever erasing what is already there. + * + * This was a whole-document `set()` of every field, which made it destructive by construction: + * any caller holding a partially-loaded [User] — or the field-less document that exists for + * ~200ms after sign-in (C-AUTH-001) — would write `coupleId = null` over a paired user, dropping + * `coupleId`/`partnerId` and unpairing the couple, plus overwriting their name. That is far too + * sharp an edge for a function every auth path calls: getting a read slightly wrong should cost + * a no-op, not a relationship's history. + * + * So it merges, and [UserProfileWrite] omits anything null or blank — a field is only ever + * written with a real value. For a document that doesn't exist yet (the only case callers + * actually intend) the result is identical to before. Clearing a field stays the job of the + * targeted `update*` methods, which say so at the call site. + */ suspend fun createUser(user: User) { - userRef(user.id).set( - mapOf( - "email" to user.email, - "displayName" to encryptProfileField(user.displayName, user.coupleId), - "photoUrl" to user.photoUrl, - "sex" to encryptProfileField(user.sex, user.coupleId), - "partnerId" to user.partnerId, - "coupleId" to user.coupleId, - "plan" to user.plan, - "birthDate" to user.birthDate, - "createdAt" to user.createdAt, - "lastActiveAt" to user.lastActiveAt - ) - ).await() + // Same guard the targeted writers carry: never persist the "locked" UI string as a value. + require(user.displayName != FieldEncryptor.LOCKED_PLACEHOLDER) { + "Refusing to persist the locked placeholder as displayName" + } + require(user.sex != FieldEncryptor.LOCKED_PLACEHOLDER) { + "Refusing to persist the locked placeholder as sex" + } + val fields = UserProfileWrite.fields( + user, + displayName = encryptProfileField(user.displayName, user.coupleId), + sex = encryptProfileField(user.sex, user.coupleId) + ) + userRef(user.id).set(fields, SetOptions.merge()).await() } /** Age-gate DOB (O-AGE-001). Set once when a Google/legacy user has no birthDate yet. */ diff --git a/app/src/main/java/app/closer/data/remote/UserProfileWrite.kt b/app/src/main/java/app/closer/data/remote/UserProfileWrite.kt new file mode 100644 index 00000000..a62ff445 --- /dev/null +++ b/app/src/main/java/app/closer/data/remote/UserProfileWrite.kt @@ -0,0 +1,49 @@ +package app.closer.data.remote + +import app.closer.domain.model.User + +/** + * The field map [FirestoreUserDataSource.createUser] writes. + * + * Split out from the data source so the one rule that matters can be pinned by a JVM test, with no + * Firestore in the way: **a null or blank value is never written.** The map is merged into a document + * that may already hold a real, paired profile, so anything hollow in the [User] has to be left alone + * rather than written as an erasure. + * + * This exists because the write used to be a whole-document `set()`, which put every caller one bad + * read away from unpairing a couple: a partially-loaded `User` — or the field-less document that + * exists for ~200ms after sign-in (C-AUTH-001) — carries `coupleId = null`, and writing that over a + * paired user drops `coupleId`/`partnerId` and overwrites their name. Clearing a field is the job of + * the targeted `update*` methods, where the call site says so out loud. + */ +internal object UserProfileWrite { + + /** + * [displayName] and [sex] are passed in already encrypted — the data source owns the couple key. + * + * `plan` is deliberately absent: it is a billing fact the server owns (RevenueCat writes it), and + * [User.plan] defaults to `"free"`, so merging it would silently downgrade a paying user whose + * document we read while it was still hollow. Reads already treat a missing `plan` as `"free"`, + * so not writing it is the same thing for a new user and harmless for everyone else. + */ + fun fields(user: User, displayName: String, sex: String): Map = buildMap { + putIfReal("email", user.email) + putIfReal("displayName", displayName) + putIfReal("photoUrl", user.photoUrl) + putIfReal("sex", sex) + putIfReal("partnerId", user.partnerId) + putIfReal("coupleId", user.coupleId) + putIfReal("birthDate", user.birthDate) + // Epoch millis: 0 is "unset" (the hollow document), never a real timestamp. + putIfReal("createdAt", user.createdAt.takeIf { it > 0L }) + putIfReal("lastActiveAt", user.lastActiveAt.takeIf { it > 0L }) + } + + private fun MutableMap.putIfReal(key: String, value: Any?) { + when (value) { + null -> Unit + is String -> if (value.isNotBlank()) put(key, value) + else -> put(key, value) + } + } +} diff --git a/app/src/main/java/app/closer/domain/model/User.kt b/app/src/main/java/app/closer/domain/model/User.kt index 7cd7a0bc..c4eef82a 100644 --- a/app/src/main/java/app/closer/domain/model/User.kt +++ b/app/src/main/java/app/closer/domain/model/User.kt @@ -16,3 +16,15 @@ data class User( val createdAt: Long = System.currentTimeMillis(), val lastActiveAt: Long = System.currentTimeMillis() ) + +/** + * True when nothing has been written to this person's document yet — as opposed to "this person has + * no profile", which looks identical field-by-field and means the opposite (C-AUTH-001). + * + * Signing in registers an FCM token, and that merge-write materialises `users/{uid}` before the + * profile has synced: for roughly the first 200ms every read returns a document that exists and has + * no identity on it. Every path that creates a real user writes [email] and [createdAt] first, so + * their absence means "too early to tell", never "new user". Read it again rather than acting on it. + */ +val User.isSignInStub: Boolean + get() = email.isBlank() && createdAt == 0L && coupleId == null diff --git a/app/src/main/java/app/closer/domain/usecase/GoogleProfileMerger.kt b/app/src/main/java/app/closer/domain/usecase/GoogleProfileMerger.kt new file mode 100644 index 00000000..fb334471 --- /dev/null +++ b/app/src/main/java/app/closer/domain/usecase/GoogleProfileMerger.kt @@ -0,0 +1,73 @@ +package app.closer.domain.usecase + +import android.util.Log +import app.closer.domain.model.GoogleSignInResult +import app.closer.domain.model.User +import app.closer.domain.model.isSignInStub +import app.closer.domain.repository.UserRepository +import javax.inject.Inject +import javax.inject.Singleton + +/** + * Seeds a user's profile from what Google already knows (name, photo) at sign-in. + * + * Best-effort by design: it fills gaps for a new account and otherwise stays out of the way. It ran + * as identical copies inside both `LoginViewModel` and `SignUpViewModel`, which is how the same bug + * came to exist twice. + * + * The bug: it decided "this is a new account" from `runCatching { getUser(uid) }.getOrNull()`, which + * returns null both when the user genuinely has no document *and* when the read simply failed. On the + * second reading it created the profile from scratch, overwriting a real one. That is the same + * conflation as C-AUTH-001, and the same rule applies — a read you don't trust decides nothing. + */ +@Singleton +class GoogleProfileMerger @Inject constructor( + private val userRepository: UserRepository +) { + suspend fun merge(result: GoogleSignInResult) { + val uid = result.uid + if (uid.isBlank()) return + + val read = runCatching { userRepository.getUser(uid) } + if (read.isFailure) { + // Can't tell a new account from an unreadable one. Writing either way risks overwriting a + // real profile, and the only thing at stake here is a pre-filled name — so do nothing. + Log.w(TAG, "Couldn't read the profile at Google sign-in; leaving it untouched", read.exceptionOrNull()) + return + } + val existing = read.getOrNull() + + if (existing == null) { + userRepository.createUser( + User( + id = uid, + email = result.email, + displayName = result.displayName, + photoUrl = result.photoUrl, + createdAt = System.currentTimeMillis(), + lastActiveAt = System.currentTimeMillis() + ) + ) + return + } + + // A document with nothing written to it yet looks exactly like a brand-new account: every + // field blank. Filling it in from Google would overwrite the real name of a returning user + // whose profile simply hadn't synced yet. Their name is already set — leave it alone. + if (existing.isSignInStub) { + Log.w(TAG, "Profile not synced yet at Google sign-in; not seeding name/photo over it") + return + } + + if (existing.displayName.isBlank() && result.displayName.isNotBlank()) { + userRepository.updateDisplayName(uid, result.displayName) + } + if (existing.photoUrl.isBlank() && result.photoUrl.isNotBlank()) { + userRepository.updatePhotoUrl(uid, result.photoUrl) + } + } + + private companion object { + const val TAG = "GoogleProfileMerger" + } +} diff --git a/app/src/main/java/app/closer/ui/auth/LoginViewModel.kt b/app/src/main/java/app/closer/ui/auth/LoginViewModel.kt index 678142e4..f0aa2f9f 100644 --- a/app/src/main/java/app/closer/ui/auth/LoginViewModel.kt +++ b/app/src/main/java/app/closer/ui/auth/LoginViewModel.kt @@ -2,10 +2,8 @@ package app.closer.ui.auth import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import app.closer.domain.model.GoogleSignInResult -import app.closer.domain.model.User import app.closer.domain.repository.AuthRepository -import app.closer.domain.repository.UserRepository +import app.closer.domain.usecase.GoogleProfileMerger import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -26,7 +24,7 @@ data class LoginUiState( @HiltViewModel class LoginViewModel @Inject constructor( private val authRepository: AuthRepository, - private val userRepository: UserRepository + private val googleProfileMerger: GoogleProfileMerger ) : ViewModel() { private val _uiState = MutableStateFlow(LoginUiState()) @@ -51,7 +49,7 @@ class LoginViewModel @Inject constructor( viewModelScope.launch { authRepository.signInWithGoogle(idToken) .onSuccess { result -> - mergeGoogleProfile(result) + googleProfileMerger.merge(result) _uiState.update { it.copy(isLoading = false, success = true) } } .onFailure { e -> _uiState.update { it.copy(isLoading = false, error = friendlyError(e)) } } @@ -69,30 +67,6 @@ class LoginViewModel @Inject constructor( } } - private suspend fun mergeGoogleProfile(result: GoogleSignInResult) { - val uid = result.uid - if (uid.isBlank()) return - val existing = runCatching { userRepository.getUser(uid) }.getOrNull() - if (existing == null) { - userRepository.createUser( - User( - id = uid, - email = result.email, - displayName = result.displayName, - photoUrl = result.photoUrl, - createdAt = System.currentTimeMillis(), - lastActiveAt = System.currentTimeMillis() - ) - ) - } else { - if (existing.displayName.isBlank() && result.displayName.isNotBlank()) { - userRepository.updateDisplayName(uid, result.displayName) - } - if (existing.photoUrl.isBlank() && result.photoUrl.isNotBlank()) { - userRepository.updatePhotoUrl(uid, result.photoUrl) - } - } - } private fun friendlyError(e: Throwable): String = when { e.message?.contains("no user record") == true -> "No account found with that email." diff --git a/app/src/main/java/app/closer/ui/auth/SignUpViewModel.kt b/app/src/main/java/app/closer/ui/auth/SignUpViewModel.kt index 89651023..f7167124 100644 --- a/app/src/main/java/app/closer/ui/auth/SignUpViewModel.kt +++ b/app/src/main/java/app/closer/ui/auth/SignUpViewModel.kt @@ -5,10 +5,8 @@ import androidx.lifecycle.viewModelScope import app.closer.domain.AgeGate import app.closer.domain.SignupHandoff import app.closer.ui.brand.CloserCopy -import app.closer.domain.model.GoogleSignInResult -import app.closer.domain.model.User import app.closer.domain.repository.AuthRepository -import app.closer.domain.repository.UserRepository +import app.closer.domain.usecase.GoogleProfileMerger import dagger.hilt.android.lifecycle.HiltViewModel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -35,7 +33,7 @@ data class SignUpUiState( @HiltViewModel class SignUpViewModel @Inject constructor( private val authRepository: AuthRepository, - private val userRepository: UserRepository, + private val googleProfileMerger: GoogleProfileMerger, private val signupHandoff: SignupHandoff ) : ViewModel() { @@ -89,7 +87,7 @@ class SignUpViewModel @Inject constructor( viewModelScope.launch { authRepository.signInWithGoogle(idToken) .onSuccess { result -> - mergeGoogleProfile(result) + googleProfileMerger.merge(result) _uiState.update { it.copy(isLoading = false, googleSuccess = true) } } .onFailure { e -> _uiState.update { it.copy(isLoading = false, error = friendlyError(e)) } } @@ -98,30 +96,6 @@ class SignUpViewModel @Inject constructor( fun reportError(message: String) = _uiState.update { it.copy(error = message) } - private suspend fun mergeGoogleProfile(result: GoogleSignInResult) { - val uid = result.uid - if (uid.isBlank()) return - val existing = runCatching { userRepository.getUser(uid) }.getOrNull() - if (existing == null) { - userRepository.createUser( - User( - id = uid, - email = result.email, - displayName = result.displayName, - photoUrl = result.photoUrl, - createdAt = System.currentTimeMillis(), - lastActiveAt = System.currentTimeMillis() - ) - ) - } else { - if (existing.displayName.isBlank() && result.displayName.isNotBlank()) { - userRepository.updateDisplayName(uid, result.displayName) - } - if (existing.photoUrl.isBlank() && result.photoUrl.isNotBlank()) { - userRepository.updatePhotoUrl(uid, result.photoUrl) - } - } - } private fun friendlyError(e: Throwable): String = when { e.message?.contains("email address is already") == true -> "An account with this email already exists." diff --git a/app/src/test/java/app/closer/data/remote/UserProfileWriteTest.kt b/app/src/test/java/app/closer/data/remote/UserProfileWriteTest.kt new file mode 100644 index 00000000..d6346f16 --- /dev/null +++ b/app/src/test/java/app/closer/data/remote/UserProfileWriteTest.kt @@ -0,0 +1,105 @@ +package app.closer.data.remote + +import app.closer.domain.model.User +import app.closer.domain.model.isSignInStub +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * These are about one thing: a profile write must never be able to erase a couple. + * + * `createUser` used to `set()` the whole document, so a caller holding a partially-loaded [User] + * would write `coupleId = null` over a paired user and unpair them (C-AUTH-001). The read that + * produces such a User is not exotic — for ~200ms after every sign-in, `users/{uid}` exists with no + * fields on it at all — so the write itself has to be incapable of the damage. + */ +class UserProfileWriteTest { + + /** Exactly what a read returns in the first ~200ms after sign-in. */ + private val signInStub = User(id = "u1", createdAt = 0L, lastActiveAt = 0L) + + private val pairedUser = User( + id = "u1", + email = "ava@example.com", + displayName = "enc:v1:name", + photoUrl = "https://example.com/a.jpg", + sex = "enc:v1:sex", + partnerId = "ben", + coupleId = "couple1", + birthDate = 946_684_800_000L, + createdAt = 1_700_000_000_000L, + lastActiveAt = 1_700_000_001_000L + ) + + private fun fieldsOf(user: User) = + UserProfileWrite.fields(user, displayName = user.displayName, sex = user.sex) + + @Test + fun aHollowUserCannotUnpairACouple() { + val fields = fieldsOf(signInStub) + // The whole bug in one assertion: merging this must not touch the pairing. + assertFalse("coupleId must not be written from a hollow user", fields.containsKey("coupleId")) + assertFalse("partnerId must not be written from a hollow user", fields.containsKey("partnerId")) + } + + @Test + fun aHollowUserWritesNothingAtAll() { + // Not just the pairing: there is nothing real on this document, so nothing should be said + // about it. A merge of an empty map is a no-op, which is the correct outcome. + assertEquals(emptyMap(), fieldsOf(signInStub)) + } + + @Test + fun aRealUserStillWritesEveryField() { + // The non-destructive rule must not cost a genuine create anything. + val fields = fieldsOf(pairedUser) + assertEquals("ava@example.com", fields["email"]) + assertEquals("enc:v1:name", fields["displayName"]) + assertEquals("https://example.com/a.jpg", fields["photoUrl"]) + assertEquals("enc:v1:sex", fields["sex"]) + assertEquals("ben", fields["partnerId"]) + assertEquals("couple1", fields["coupleId"]) + assertEquals(946_684_800_000L, fields["birthDate"]) + assertEquals(1_700_000_000_000L, fields["createdAt"]) + assertEquals(1_700_000_001_000L, fields["lastActiveAt"]) + } + + @Test + fun blanksAreOmittedRatherThanWrittenAsErasures() { + val partial = pairedUser.copy(photoUrl = "", sex = "", birthDate = null) + val fields = fieldsOf(partial) + assertFalse("a blank photoUrl must not clear an existing one", fields.containsKey("photoUrl")) + assertFalse("a blank sex must not clear an existing one", fields.containsKey("sex")) + assertFalse("a null birthDate must not clear an existing one", fields.containsKey("birthDate")) + // ...while everything real about the same user is still written. + assertEquals("couple1", fields["coupleId"]) + } + + @Test + fun planIsNeverWritten() { + // User.plan defaults to "free", so writing it would downgrade a paying user whose document + // we happened to read while it was hollow. The server owns this field. + assertFalse(fieldsOf(pairedUser).containsKey("plan")) + assertFalse(fieldsOf(pairedUser.copy(plan = "premium")).containsKey("plan")) + } + + @Test + fun unsetTimestampsAreNotWrittenAsZero() { + // 0 is "unset", and writing it would date a real account to 1970. + val fields = fieldsOf(pairedUser.copy(createdAt = 0L, lastActiveAt = 0L)) + assertFalse(fields.containsKey("createdAt")) + assertFalse(fields.containsKey("lastActiveAt")) + } + + @Test + fun theStubIsRecognisedAndARealUserIsNot() { + assertTrue("the post-sign-in document must read as a stub", signInStub.isSignInStub) + assertFalse("a paired user must never read as a stub", pairedUser.isSignInStub) + // A genuinely new account is NOT a stub: every create writes email + createdAt first, which + // is what keeps new users flowing to profile setup instead of being retried forever. + val brandNew = User(id = "u2", email = "new@example.com", createdAt = 1_700_000_000_000L) + assertFalse("a new account with identity on it is not a stub", brandNew.isSignInStub) + } +} diff --git a/docs/Engineering_Reference_Manual.md b/docs/Engineering_Reference_Manual.md index 930c5e0f..0960edc4 100644 --- a/docs/Engineering_Reference_Manual.md +++ b/docs/Engineering_Reference_Manual.md @@ -1481,8 +1481,9 @@ OOB code = `truncate6(SHA-256(pubkey ‖ nonce))`. ### C-AUTH-001 - a `users/{uid}` read right after sign-in returns a field-less document **Symptom**: signing in on a *new device* with a fully set-up, paired account routed to `CREATE_PROFILE` ("What should your partner call you?", over a `🔒 Couldn't unlock on this device` value) instead of `RECOVERY`. Reproduced end to end on a throwaway emulator with a real paired account. **Cause**: for roughly the first 200ms after sign-in, `getUser(uid)` returns a document that **exists with no fields at all** - no `email`, no `displayName`, no `coupleId` - and the real document only lands afterwards. Sign-in registers the FCM token, and that `SetOptions.merge()` write materialises `users/{uid}` before the profile has synced. Measured on-device: two reads at t+0ms field-less, the real document at t+215ms. `Source.SERVER` does **not** dodge it, so "read from the server" is not the fix. -**Why it was destructive**: `FirestoreUserDataSource.createUser` is a whole-document `set()`, not a merge, and `CreateProfileViewModel` submit calls it. Tapping through that mistakenly-shown screen overwrites the real `displayName` with the locked placeholder and drops `coupleId`/`partnerId`, unpairing the couple - and the user never reaches Recovery. -**Fix (R30)**: `OnboardingViewModel.resolveDestination` treats a field-less document as "ask again", not as "new user": it retries (3x, 400ms backoff) until a document with real identity on it arrives, and only that decides. A read that never succeeds routes Home (non-destructive; Home routes to Recovery by itself when the couple key is missing). `UserRepository.getUserFromServer` (`Source.SERVER`) backs the decision so an offline read throws instead of guessing from cache. +**Why it was destructive**: the user never reaches Recovery, and the screen they get instead invites them to overwrite the profile it couldn't read. `CreateProfileViewModel` submit is only *partly* protected - it uses targeted merges when it sees an existing doc (and `updateDisplayName`/`updateSex` `require` the value isn't the locked placeholder), but it calls `createUser` when it reads `null`, and `createUser` was a whole-document `set()` of every field. Any caller holding a partially-loaded `User` therefore wrote `coupleId = null` over a paired user, dropping `coupleId`/`partnerId`. The Google path reached exactly that: `mergeGoogleProfile` decided "new account" from `runCatching { getUser(uid) }.getOrNull()`, which is `null` for a *failed read* as much as for a missing doc. +**Fix (R30)**: `OnboardingViewModel.resolveDestination` treats a field-less document as "ask again", not as "new user": it retries (3x, 400ms backoff) until a document with real identity on it arrives, and only that decides. A read that never succeeds routes Home (non-destructive; Home routes to Recovery by itself when the couple key is missing). `UserRepository.getUserFromServer` (`Source.SERVER`) backs the decision so an offline read throws instead of guessing from cache. `User.isSignInStub` is the shared predicate. +**Hardening (R30)**: the primitive can no longer do the damage. `createUser` merges, and `UserProfileWrite` omits every null/blank value, so a hollow `User` produces an empty map - a no-op - instead of an erasure; it also carries the placeholder `require`s, and deliberately never writes `plan` (client default `"free"` would downgrade a paying user; the server owns it, reads default it). `mergeGoogleProfile` was duplicated verbatim in `LoginViewModel` and `SignUpViewModel` - the same bug twice - and is now one `GoogleProfileMerger` that does nothing on a failed read and won't seed over a stub. Pinned by `UserProfileWriteTest` (mutation-checked: reverting the omit rule fails 3 of them). **Re-introduction risk**: **any** one-shot `users/{uid}` read on a startup path that branches on a field being blank. `exists() == true` does not mean "populated". Blank fields this early mean "not synced yet", and `createUser`'s `set()` semantics make guessing wrong expensive. If you need "does this person have a profile", require identity (`email`/`createdAt`) to be present before believing the answer. ---