A test run must not publish a credential (ios#62, Android half)

The Kotlin counterpart of the iOS fix, and Android's version of the bug was
quieter rather than absent.

Android was already ahead on the part iOS got wrong: every live suite is gated
behind `assumeTrue`, and `LiveTarget.refuseProduction` refuses a production
host outright. What it still had was the credential itself.
`LiveTwoProductsTest` and `LiveShapesTest` both carried
`const val PASSPHRASE = "a-long-enough-passphrase"` and used it to create
accounts on accounts.flamenet.io whenever somebody set FN_LIVE_MANAGED=1.

🔴 The gate stops it running by accident. It does not stop the password being
written down. Every account those runs ever created can be signed into by
anybody who can read this repository, and "they were throwaway accounts" does
not change that — they are real accounts on a real service and they persist
until somebody deletes them.

Both now read it from the environment through `LiveTarget.passphrase()` with no
fallback, and both gates assume on it being non-empty, so opting in without
configuring one reports SKIPPED rather than signing up with a published value.

And a guard, matching the Swift one: TestSafetyTest fails the build if a suite
that reaches a live relay carries a credential literal, or cannot announce a
skip. Proven by putting the literal back and watching it go red.

⚠️ The guard had to be narrowed twice before it was worth keeping, and both
narrowings are recorded in it. Keying on the `Live` name prefix flagged
LiveTarget.kt (the helper) and LiveTargetTest.kt (a unit test of the helper).
Keying on the string "http" then flagged four suites that sign up against a
MockWebServer — which is exactly the correct way to test sign-up. The
discriminator is MockWebServer: a file that stands up a mock cannot reach
anybody. It now matches the nine real live suites and nothing else, and a test
asserts the predicate matches at least five, so the checks cannot pass by
guarding an empty set.

PARITY.md gains a Test safety section. Full JVM suite green.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-09-04 13:29:38 -04:00
co-authored by Claude Opus 5
parent 22189bea93
commit 775b6e62d0
5 changed files with 266 additions and 8 deletions
+27
View File
@@ -150,6 +150,33 @@ emulator.
---
## Test safety
| Feature | iOS | Android | Note |
|---|:--:|:--:|---|
| Live suites cannot run by accident | ✅ | ✅ | Android was already ahead here — every live suite gated on `assumeTrue`, and `LiveTarget.refuseProduction` refuses a production host outright |
| No credential written into the source | ✅ | ✅ | both fixed together, ios#62 |
| A guard that fails the build if either regresses | ✅ | ✅ | `Tests/UITestSafetyTests.swift`, `client/.../TestSafetyTest.kt` |
🔴 **iOS was creating real accounts on production on every plain test run**, with
a literal password from a public repository; Android's equivalent suites were
properly gated but still carried
`const val PASSPHRASE = "a-long-enough-passphrase"` and used it to sign up on
`accounts.flamenet.io` whenever somebody opted in. The gate stops it running by
accident. It does not stop the password being written down.
Both now read the passphrase from the environment with **no fallback**, and an
unconfigured run reports SKIPPED rather than signing up with a value anybody can
read. `FN_LIVE_PASSPHRASE` on Android, `TEST_RUNNER_FN_DEMO_PASSPHRASE` on iOS.
⚠️ Both guards are source scans, and both had to be narrowed before they were
worth keeping — the Swift one flagged an email default because "password"
appeared in a trailing comment, the Kotlin one flagged four suites that sign up
against a `MockWebServer`, which is the correct way to test sign-up. A guard
with standing false positives is one somebody switches off.
---
## The 2 September revert
Twenty-five Android merge requests landed that evening and `aebf41e`
@@ -47,11 +47,18 @@ class LiveShapesTest {
private fun needFleet() {
LiveTarget.refuseProduction(System.getenv("FN_LIVE_FLEET"))
assumeTrue(System.getenv("FN_LIVE_FLEET") != null, "set FN_LIVE_FLEET to run against the fleet")
assumeTrue(PASSPHRASE.isNotEmpty(), "set FN_LIVE_PASSPHRASE")
}
private fun needManaged() = assumeTrue(
System.getenv("FN_LIVE_MANAGED") == "1",
"set FN_LIVE_MANAGED=1 — these create real accounts on accounts.flamenet.io")
private fun needManaged() {
assumeTrue(
System.getenv("FN_LIVE_MANAGED") == "1",
"set FN_LIVE_MANAGED=1 — these create real accounts on accounts.flamenet.io")
assumeTrue(
PASSPHRASE.isNotEmpty(),
"set FN_LIVE_PASSPHRASE — these create real accounts and will not use " +
"a passphrase written into the repository")
}
private val local = "https://relay1.flamenet.io"
private val sharedSelfHosted = "https://relay2.flamenet.io"
@@ -388,6 +395,10 @@ class LiveShapesTest {
}
private companion object {
const val PASSPHRASE = "a-long-enough-passphrase"
/**
* 🔴 Was `"a-long-enough-passphrase"`, a literal in a source file used
* to create real accounts. See [LiveTarget.passphrase].
*/
val PASSPHRASE: String get() = LiveTarget.passphrase()
}
}
@@ -77,4 +77,26 @@ object LiveTarget {
)
}
}
/**
* The passphrase a live suite signs up with.
*
* 🔴 **From the environment, with no fallback, and that is the whole
* point.** Both live suites carried
* `const val PASSPHRASE = "a-long-enough-passphrase"` and used it to create
* accounts on `accounts.flamenet.io` whenever somebody set
* `FN_LIVE_MANAGED=1`. A credential written into a source file is a
* published password: every account those runs ever made can be signed into
* by anybody who can read this repository, and nothing about the accounts
* being throwaway changes that — they are real accounts on a real service,
* and they persist.
*
* ⚠️ Empty rather than throwing, so the caller turns it into an announced
* skip. A live suite that cannot run must report SKIPPED and not fail: a
* red suite for a missing variable teaches people to ignore red.
*
* The iOS side had the same class of bug twice (ios#62) — a literal
* fallback used against production. This is the Kotlin half of that fix.
*/
fun passphrase(): String = System.getenv("FN_LIVE_PASSPHRASE").orEmpty()
}
@@ -26,13 +26,20 @@ class LiveTwoProductsTest {
private val filesHost = "https://files.flamenet.io"
private val localRelay = "https://relay1.flamenet.io"
private fun needManaged() = assumeTrue(
System.getenv("FN_LIVE_MANAGED") == "1",
"set FN_LIVE_MANAGED=1 — this creates a real account")
private fun needManaged() {
assumeTrue(
System.getenv("FN_LIVE_MANAGED") == "1",
"set FN_LIVE_MANAGED=1 — this creates a real account")
assumeTrue(
PASSPHRASE.isNotEmpty(),
"set FN_LIVE_PASSPHRASE — this creates a real account and will not " +
"use a passphrase written into the repository")
}
private fun needFleet() {
LiveTarget.refuseProduction(System.getenv("FN_LIVE_FLEET"))
assumeTrue(System.getenv("FN_LIVE_FLEET") != null, "set FN_LIVE_FLEET")
assumeTrue(PASSPHRASE.isNotEmpty(), "set FN_LIVE_PASSPHRASE")
}
/**
@@ -114,6 +121,10 @@ class LiveTwoProductsTest {
}
private companion object {
const val PASSPHRASE = "a-long-enough-passphrase"
/**
* 🔴 Was `"a-long-enough-passphrase"`, a literal in a source file used
* to create real accounts. See [LiveTarget.passphrase].
*/
val PASSPHRASE: String get() = LiveTarget.passphrase()
}
}
@@ -0,0 +1,187 @@
package io.flamenet.client
import java.io.File
import kotlin.test.Test
import kotlin.test.assertTrue
/**
* A test run must not publish a credential (the Kotlin half of ios#62).
*
* 🔴 **The iOS side had this bug twice, and Android had a quieter version of
* it.** Every live suite here is properly gated behind `assumeTrue` — better
* than iOS managed — but two of them carried
* `const val PASSPHRASE = "a-long-enough-passphrase"` and used it to create
* accounts on `accounts.flamenet.io` whenever somebody set `FN_LIVE_MANAGED=1`.
*
* The gate stops it running by accident. It does not stop the password being
* written down. Every account those runs ever created can be signed into by
* anybody who can read this repository, and "they were throwaway accounts"
* does not change it — they are real accounts on a real service and they
* persist until somebody deletes them.
*
* ⚠️ **A source scan, and coarse on purpose.** It cannot prove a suite is safe;
* it proves the specific thing that went wrong cannot come back silently.
* That is worth having even though it is not a proof, and it costs a few
* milliseconds in the JVM suite rather than a device.
*/
class TestSafetyTest {
private fun testSources(): List<File> {
// From `client/build/classes/...` back to the repository root, then to
// every Kotlin test source under it. Walking from the working directory
// rather than a compiled resource, so this sees the tree as written.
val root = generateSequence(File(".").absoluteFile) { it.parentFile }
.firstOrNull { File(it, "settings.gradle.kts").exists() || File(it, "PARITY.md").exists() }
?: File(".").absoluteFile
return root.walkTopDown()
.onEnter { it.name != "build" && it.name != ".git" }
.filter { it.isFile && it.extension == "kt" }
.filter { it.path.contains("/test/") || it.path.contains("/androidTest/") }
.toList()
}
/**
* Lines that are code rather than prose.
*
* ⚠️ Trailing comments go too. The Swift version of this guard flagged an
* *email* default because the word "password" appeared in a comment at the
* end of the line — and a guard that cries wolf is a guard somebody
* switches off.
*/
private fun codeLines(body: String): List<String> =
body.lineSequence()
.filterNot {
val t = it.trimStart()
t.startsWith("//") || t.startsWith("*") || t.startsWith("/*")
}
.map { line ->
var i = 0
while (true) {
val at = line.indexOf("//", i)
if (at < 0) return@map line
if (at == 0 || line[at - 1] != ':') return@map line.substring(0, at)
i = at + 2
}
@Suppress("UNREACHABLE_CODE") line
}
.toList()
/**
* Whether this file can actually reach a relay somebody else runs.
*
* ⚠️ **`MockWebServer` is the discriminator, and getting this wrong made
* the guard useless twice.** Keying on the `Live` name prefix flagged the
* guard helper and its own unit test; keying on the string "http" flagged
* four suites that sign up against a local mock, which is exactly the
* correct way to test sign-up. A file that stands up a mock server cannot
* reach anybody, whatever else it contains.
*
* What is left is: it signs up or signs in, it does NOT mock the server,
* and it names a real host or takes one from the environment.
*/
private fun reachesALiveRelay(code: List<String>): Boolean {
val reaching = listOf("signUp(", "signIn(", "register(")
if (reaching.none { needle -> code.any { it.contains(needle) } }) return false
if (code.any { it.contains("MockWebServer") }) return false
return code.any { it.contains("flamenet.io") || it.contains("System.getenv") }
}
/**
* 🔴 **No credential may be a literal in a suite that reaches a relay.**
*
* The rule is scoped to files that actually sign up or sign in, because a
* purely local unit test using `passphrase = "pass"` against an in-memory
* store publishes nothing and is not the problem.
*/
@Test
fun `no live suite carries a credential literal`() {
val offenders = mutableListOf<String>()
for (file in testSources()) {
val code = codeLines(file.readText())
if (!reachesALiveRelay(code)) continue
for (line in code) {
val looksLikeCredential = line.contains("PASSPHRASE") ||
line.contains("passphrase", ignoreCase = true) ||
line.contains("password", ignoreCase = true)
if (!looksLikeCredential) continue
// `val X = "…"` or `const val X = "…"` — an assignment to a
// non-empty literal. A getter that reads the environment has no
// `= "` on it and is the shape this wants people to use.
val assigned = Regex("""\bva[lr]\s+\w+\s*(:\s*String\s*)?=\s*"([^"]+)"""")
.find(line)?.groupValues?.getOrNull(2)
if (assigned != null) {
offenders += "${file.name}: $assigned"
}
}
}
assertTrue(
offenders.isEmpty(),
"""
A suite that signs up or signs in carries a credential written into
the source: $offenders
This repository is readable by everybody who can read it, so that
string is a published password — and these suites create REAL
accounts on a real service. Being gated behind an environment
variable stops it running by accident; it does not stop the password
being written down.
Read it from the environment instead, with no fallback:
LiveTarget.passphrase(), and an assumeTrue on it being non-empty so
an unconfigured run reports SKIPPED rather than signing up with a
value anybody can read.
""".trimIndent(),
)
}
/**
* 🔴 **Every suite that reaches a live relay must be able to decline.**
*
* `assumeTrue` reports SKIPPED, which is the truth. A bare `return` makes a
* test that did nothing report PASSED, and a suite of those is
* indistinguishable from a suite that works — the point `LiveShapesTest`
* already makes in prose and this makes checkable.
*/
@Test
fun `every live suite can announce a skip`() {
// ⚠️ Scoped to what a file DOES, not to what it is called — see
// `reachesALiveRelay`.
val offenders = testSources()
.filter { reachesALiveRelay(codeLines(it.readText())) }
.filterNot { codeLines(it.readText()).any { line -> line.contains("assumeTrue") } }
.map { it.name }
assertTrue(
offenders.isEmpty(),
"these live suites cannot skip and will reach a real relay on any " +
"plain `./gradlew test`: $offenders",
)
}
/**
* ⚠️ A scan that finds nothing passes. This proves it is reading the real
* tree — a moved directory would otherwise turn both checks above into
* no-ops reporting success.
*/
@Test
fun `the scan is looking at the real test sources`() {
val found = testSources()
assertTrue(found.size > 15, "found ${found.size} test sources, too few to be this repository")
assertTrue(found.any { it.name == "LiveShapesTest.kt" }, "LiveShapesTest.kt was not found")
assertTrue(
found.any { codeLines(it.readText()).any { line -> line.contains("assumeTrue") } },
"the scan cannot see into the files it is reading",
)
// 🔴 And the live-relay predicate must actually match something. If it
// matched nothing, both checks above would pass by looking at an empty
// set — the exact way a guard reports success while guarding nothing.
val live = found.filter { reachesALiveRelay(codeLines(it.readText())) }
assertTrue(
live.size >= 5,
"the live-relay predicate matched ${live.size} suites, so the checks " +
"above are guarding almost nothing: ${live.map { it.name }}",
)
}
}