From 4456f351acb3226dd27562fcd75c0f4e68fbc75a Mon Sep 17 00:00:00 2001 From: null Date: Wed, 19 Aug 2026 22:00:06 -0500 Subject: [PATCH] feat: the privacy promise appears in Settings, from one string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §4 requires the promise in onboarding, in Settings, and on a public privacy page. It was made once, during onboarding, before the user had entered a single date — which makes it a marketing line. Repeated above the controls that act on that data, it is a statement somebody can hold the product to. One copy, in strings.xml, read by both screens. A second literal is how two versions of a promise come to exist, which is the failure DOC_TRUST_MAP.md exists to prevent, here in code rather than in prose. There is NO Privacy Policy row. §4 wants one and no hosted page exists, and a policy link that 404s is worse than no link — which is also the convention SettingsScreen already states: a row for something unbuilt is absent, not disabled. The issue's verify line allows exactly this. Three tests, and the second is the one that matters. The promise must say we never SELL the data, and must NOT have been strengthened into claims the app cannot keep — no third party, never shared, end-to-end — because Play Billing and an ad SDK eventually will process something, and a promise the implementation cannot keep is worse than a narrower one that holds. The third scans Kotlin for a re-introduced literal, with comments stripped first per GUARDS.md §2, or the KDoc explaining the rule would fail it. Proved: replacing the resource lookup with the literal fails exactly one test. ## Two defects found on the way, both pre-existing **No Robolectric test in :app could read a string resource.** core/database and core/data have carried unitTests.isIncludeAndroidResources since they were written; app never did. So the module owning almost all of the user-facing copy was the one module whose copy could not be tested, and every getString() threw NotFoundException with an id that had resolved perfectly well. **checkPermissions read manifests that do not ship.** Turning the above on made AGP write merged_manifest/debugUnitTest/, the guard walked the whole tree, and the build failed on REORDER_TASKS — a test-runner permission no user ever sees. The tempting fix is to allowlist it, which would then permit it in the real manifest too and quietly undo the guard. It now reads only debug and release, and refuses to pass unless it read BOTH: checking debug while release went unread is the failure that matters, since the Play listing and the Data Safety form describe the release manifest. That is strictly stricter than before, and proved twice — a forbidden permission in the app manifest still fails it, and a missing release manifest now fails it where it used to pass. GUARDS.md §8 gains a third prove-guard edge, found while proving the above: a FAIL_PATTERN matching nothing gives the same "caught it, and only it" verdict as one matching exactly once, because the script only refuses on more than one. The empty "what failed" block is the tell. closes #37 Co-Authored-By: Claude Opus 5 (1M context) --- app/build.gradle.kts | 10 ++ .../feature/onboarding/OnboardingScreen.kt | 16 +-- .../period/feature/settings/SettingsScreen.kt | 38 ++++++ app/src/main/res/values/strings.xml | 19 +++ .../feature/settings/PrivacyPromiseTest.kt | 111 ++++++++++++++++++ build.gradle.kts | 34 +++++- docs/architecture/GUARDS.md | 14 +++ 7 files changed, 228 insertions(+), 14 deletions(-) create mode 100644 app/src/test/kotlin/dev/privacyllc/period/feature/settings/PrivacyPromiseTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b0e7f30..2bb0b4f 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -40,6 +40,16 @@ android { } } + // Without this, Robolectric in this module cannot read a single string + // resource — every getString() throws Resources$NotFoundException with an id + // that resolved perfectly well. `core/database` and `core/data` have carried + // it since they were written; `app` never did, which meant the module that + // owns almost all of the user-facing copy was the one module whose copy + // could not be tested. Found while making §4's privacy promise checkable. + testOptions { + unitTests.isIncludeAndroidResources = true + } + buildFeatures { compose = true } diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/onboarding/OnboardingScreen.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/onboarding/OnboardingScreen.kt index 1c00ff2..b9c13ca 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/onboarding/OnboardingScreen.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/onboarding/OnboardingScreen.kt @@ -36,10 +36,12 @@ import androidx.compose.ui.semantics.Role import androidx.compose.ui.Modifier import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.Dp +import androidx.compose.ui.res.stringResource import androidx.compose.ui.unit.dp import androidx.hilt.lifecycle.viewmodel.compose.hiltViewModel import androidx.lifecycle.compose.collectAsStateWithLifecycle import dev.privacyllc.period.core.datastore.NotificationPrivacy +import dev.privacyllc.period.R import dev.privacyllc.period.designsystem.art.ForecastIllustration import dev.privacyllc.period.designsystem.art.LastPeriodIllustration import dev.privacyllc.period.designsystem.art.LearningIllustration @@ -280,17 +282,15 @@ private fun PreviousHistory(state: OnboardingUiState, viewModel: OnboardingViewM @Composable private fun PrivacyPromise(onNext: () -> Unit) { // §4 requires this promise here, in Settings, and on the public privacy - // page. The wording is deliberate: we never SELL your data. It does not - // claim no third party ever processes anything, because Play Billing and an - // ad SDK will, and a promise the implementation cannot keep is worse than a - // narrower one it can. + // page. It is a string resource rather than a literal for exactly that + // reason: Settings shows the same words, and two literals is how two + // versions of a promise come to exist. The wording itself, and why it is + // deliberately narrow, is documented beside the resource. StepBody( artHeight = 280.dp, art = { PrivacyIllustration(size = it) }, - title = "Your cycle belongs to you.", - body = "We will never sell your personal or health data.\n\n" + - "Your period history and fertility information are private. We don't sell them " + - "to advertisers, data brokers, or third parties.", + title = stringResource(R.string.privacy_promise_title), + body = stringResource(R.string.privacy_promise_body), ) { Button(onClick = onNext, modifier = Modifier.fillMaxWidth()) { Text("Continue") } } diff --git a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/SettingsScreen.kt b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/SettingsScreen.kt index 2584176..9951e4f 100644 --- a/app/src/main/kotlin/dev/privacyllc/period/feature/settings/SettingsScreen.kt +++ b/app/src/main/kotlin/dev/privacyllc/period/feature/settings/SettingsScreen.kt @@ -76,6 +76,7 @@ fun SettingsScreen( Spacer(Modifier.height(8.dp)) SectionHeader("Privacy & Security") + PrivacyPromise() SettingsRow( title = "App lock", subtitle = "Ask for a PIN before the app opens", @@ -192,6 +193,43 @@ private fun versionName(): String { } } +/** + * §4's promise, where it can be held against the product. + * + * Onboarding already shows this, once, before the user has entered a single + * date — which makes it a marketing line. Repeated here, immediately above the + * controls that act on the data, it is a statement somebody can check the app + * against: the promise and the Delete button on the same screen. + * + * Shown rather than folded into an `ExpandableRow`. Everything else in About is + * an explanation the user opens when they want it; this is the one thing the + * screen should say without being asked. + * + * Both words come from `strings.xml` and so does onboarding's copy. Two literals + * is how two versions of a promise come to exist, which is exactly the failure + * `DOC_TRUST_MAP.md` exists to prevent — in code rather than in prose. + * + * **There is no Privacy Policy row.** §4 wants one and no hosted page exists + * yet, and a policy link that 404s is worse than no link at all. It follows the + * convention this file already states: a row for something that does not exist + * is absent, not disabled. + */ +@Composable +private fun PrivacyPromise() { + Column(Modifier.padding(vertical = 8.dp)) { + Text( + stringResource(R.string.privacy_promise_title), + style = MaterialTheme.typography.bodyLarge, + ) + Spacer(Modifier.height(4.dp)) + Text( + stringResource(R.string.privacy_promise_body), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } +} + @Composable private fun SectionHeader(text: String) { Spacer(Modifier.height(16.dp)) diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 5df008b..c0440cd 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -29,5 +29,24 @@ Notifications deliberately do NOT carry it: a long sentence about fertility on a lock screen is the leak the discreet copy exists to avoid. --> + + Your cycle belongs to you. + We will never sell your personal or health data.\n\nYour period history and fertility information are private. We don\'t sell them to advertisers, data brokers, or third parties. + Fertility and ovulation dates are estimates based on cycle history and are not intended to be used as contraception or as a medical diagnosis. diff --git a/app/src/test/kotlin/dev/privacyllc/period/feature/settings/PrivacyPromiseTest.kt b/app/src/test/kotlin/dev/privacyllc/period/feature/settings/PrivacyPromiseTest.kt new file mode 100644 index 0000000..57187a0 --- /dev/null +++ b/app/src/test/kotlin/dev/privacyllc/period/feature/settings/PrivacyPromiseTest.kt @@ -0,0 +1,111 @@ +package dev.privacyllc.period.feature.settings + +import androidx.test.core.app.ApplicationProvider +import dev.privacyllc.period.R +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import java.io.File + +/** + * §4's promise: said in two places, written in one. + * + * Onboarding shows it before any data is entered; Settings shows it beside the + * controls that act on that data. Those are different jobs and both are + * required — but a second literal is how two versions of a promise come to + * exist, which is the failure `DOC_TRUST_MAP.md` exists to prevent, here in + * code rather than in prose. + * + * So one test reads the resource and one scans for a copy of it. + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34]) +class PrivacyPromiseTest { + + private val context = ApplicationProvider.getApplicationContext() + + private val title: String get() = context.getString(R.string.privacy_promise_title) + private val body: String get() = context.getString(R.string.privacy_promise_body) + + @Test fun `the promise exists and says the thing it is for`() { + assertTrue("the title is empty", title.isNotBlank()) + assertTrue( + "the promise must say what it promises: that the data is never sold", + body.contains("never sell", ignoreCase = true), + ) + } + + /** + * The wording is deliberately narrower than it could be, and that is the + * part most likely to be "improved" by somebody who has not read why. + * + * It promises we never **sell** the data. It does not claim no third party + * ever processes anything, because Play Billing and an ad SDK eventually + * will — and a promise the implementation cannot keep is worse than a + * narrower one that holds. `SECURITY.md` records the same wording and the + * same reason. + */ + @Test fun `the promise does not claim more than the app can keep`() { + val overclaims = listOf( + "no third party", + "never shared", + "nobody else", + "no one else", + "never leaves your phone", + "fully encrypted", + "end-to-end", + ) + val found = overclaims.filter { body.contains(it, ignoreCase = true) } + assertEquals( + "the promise has been strengthened past what the app can deliver — " + + "see the comment beside the string resource", + emptyList(), + found, + ) + } + + /** + * Nobody has re-introduced a literal copy. + * + * Scans Kotlin source rather than checking the two known screens, because + * the failure is a *third* surface pasting the sentence in. The distinctive + * clause is used rather than the whole paragraph so that a reflowed string + * still matches. + */ + @Test fun `the promise appears as a literal in no Kotlin source`() { + val distinctive = "never sell your personal or health data" + + val sources = File("src/main/kotlin").walkTopDown() + .filter { it.isFile && it.extension == "kt" } + .toList() + + // A scan that reads nothing is not a pass — the same refusal the Gradle + // guards make, for the same reason. + assertTrue( + "no Kotlin sources were scanned, so nothing was checked — has the " + + "module layout moved?", + sources.size > 10, + ) + + // Comments stripped first — GUARDS.md §2. A KDoc explaining why the + // promise lives in one place would otherwise contain the sentence it + // forbids, so the clearest possible comment would fail the test and the + // obvious fix would be to delete the explanation. + fun codeOf(text: String): String = + text.replace(Regex("""/\*.*?\*/""", RegexOption.DOT_MATCHES_ALL), "") + .lines().joinToString("\n") { it.substringBefore("//") } + + val offenders = sources.filter { codeOf(it.readText()).contains(distinctive, ignoreCase = true) } + .map { it.name } + + assertEquals( + "the promise is written out in Kotlin as well as in strings.xml, so " + + "there are now two copies to keep in step", + emptyList(), + offenders, + ) + } +} diff --git a/build.gradle.kts b/build.gradle.kts index d2ec34c..58a7f3a 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -277,13 +277,35 @@ tasks.register("checkPermissions") { // `merged_manifests` (plural) tree that nothing here regenerates, and // reading it meant a stale file from an earlier build failing the check // — a guard that cries wolf gets switched off. - val files = intermediates.resolve("merged_manifest").walkTopDown() - .filter { it.isFile && it.name == "AndroidManifest.xml" } - .toList() - if (files.isEmpty()) { - // Never a silent pass: no manifest means the check did not run. + // + // And only the SHIPPING variants. AGP writes a merged manifest for test + // variants too, under `merged_manifest/debugUnitTest/`, which appears + // the moment a module turns on `unitTests.isIncludeAndroidResources`. + // That manifest carries the test runner's own permissions — REORDER_TASKS + // among them — none of which reach a user. Reading it failed the build + // over a permission that does not ship, and the tempting fix is to add + // it to `allowedPermissions`, which would then permit it in the real + // manifest as well and quietly undo the guard. + val shippingVariants = setOf("debug", "release") + val variantDirs = intermediates.resolve("merged_manifest").listFiles() + ?.filter { it.isDirectory && it.name in shippingVariants } + .orEmpty() + + val files = variantDirs.flatMap { dir -> + dir.walkTopDown().filter { it.isFile && it.name == "AndroidManifest.xml" }.toList() + } + + // Never a silent pass, and it has to be stricter than "found something". + // Checking debug while release quietly went unread is the failure that + // matters here: the Play listing and the Data Safety form describe the + // release manifest, so a guard that only ever saw debug would be green + // over the one that ships. + val seen = variantDirs.map { it.name }.toSet() + if (files.isEmpty() || seen != shippingVariants) { throw GradleException( - "no merged manifest found, so no permission was checked. Build :app first.", + "expected a merged manifest for each of $shippingVariants but read " + + "${seen.ifEmpty { "none" }}, so the permission set was not checked. " + + "This is not a pass — build :app first.", ) } diff --git a/docs/architecture/GUARDS.md b/docs/architecture/GUARDS.md index bc21cee..9b943db 100644 --- a/docs/architecture/GUARDS.md +++ b/docs/architecture/GUARDS.md @@ -180,6 +180,20 @@ Two rules follow, and the first is the general one: The uncomfortable part: the documented proofs had been exiting 3 rather than 0 since they were written. Nobody had run them and read the last line. +A third edge, found the same way. When the fallback counts log lines, a +`PROVE_GUARD_FAIL_PATTERN` that matches **nothing** produces the same verdict as +one matching exactly once — "the guard caught it, and only it" — because the +script refuses on `COUNT > 1` and treats zero as fine. The redness itself was +real, so the conclusion happened to be right; the *"and only it"* half was +unverified. It surfaced as an empty `--- what failed ---` block above a +confident summary, from a pattern written `Forbidden` against output that says +`FORBIDDEN`. + +So the rule in this section is literal. An empty block under a green verdict is +the tool telling you it counted nothing, and the answer is to fix the pattern +and run it again — or to break the thing by hand and read the failure, which +takes a minute and cannot mislead. + ## 9. Some guards cannot be driven by `prove-guard.sh`, and must still be proved `scripts/prove-guard.sh` breaks a guard's target by **replacing a string inside a