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