Merge pull request #4147 from vitorpamplona/claude/qr-reader-improvements-ukfl60

fix(qr): audit follow-ups — keep key material off the screen and out of the log, guard the QR native library
2026-10-05 19:28:25 +00:00 · 2026-09-18 18:28:50 -04:00
parent c05f0c75c2 18cac054c3
commit 29738585d3
14 changed files with 370 additions and 102 deletions
@@ -89,8 +89,8 @@ cd amethyst

 ## Generated & vendored artifacts

-Two build inputs are **generated by tools but committed to the repo**, so a
-normal build or release does **not** run either — Gradle just consumes the
+Three build inputs are **generated by tools but committed to the repo**, so a
+normal build or release does **not** run any of them — Gradle just consumes the
 checked-in output. You only regenerate them under the specific conditions below,
 and each has its own guide:

@@ -98,34 +98,44 @@ and each has its own guide:
 |---|---|---|---|
 | **Material Symbols subset font** | `commonsUI/src/commonMain/composeResources/font/material_symbols_outlined.ttf` | You add/remove a `MaterialSymbol("\uXXXX")` codepoint in `MaterialSymbols.kt`, or bump the upstream font | [`tools/material-symbols-subset/README.md`](tools/material-symbols-subset/README.md) — run `./tools/material-symbols-subset/subset.sh` |
 | **Arti (Tor) native libs** | `amethyst/src/main/jniLibs/*.so` | You update the pinned Arti version, change the JNI wrapper, or want to reproduce the binaries | [`tools/arti-build/README.md`](tools/arti-build/README.md) |
+| **zxing-cpp (QR decoder) native libs** | `amethyst/src/main/jniLibs/*/libzxingcpp_android.so` | You bump `ZXING_CPP_VERSION`, change the JNI wrapper, or want to reproduce the binaries | [`tools/zxing-cpp-build/README.md`](tools/zxing-cpp-build/README.md) |

 > **Material Symbols is mandatory after icon changes.** The bundled font is a
 > ~210-glyph subset; a new codepoint that isn't in it renders as tofu (□) at
 > runtime. Regenerate and commit the `.ttf` alongside the `MaterialSymbols.kt`
 > change. Reusing an existing codepoint needs no regeneration.

-Both tools have their own prerequisites (`fonttools`/`brotli` for the font; a
-Rust toolchain + the exact Android NDK revision pinned in
-`tools/arti-build/ANDROID_NDK_VERSION` for Arti) documented in their READMEs —
-they are **not** required to build Amethyst from the committed sources.
+Each tool has its own prerequisites (`fonttools`/`brotli` for the font; a Rust
+toolchain for Arti; `cmake` + `ninja` for zxing-cpp; and, for both native
+builds, the exact Android NDK revision pinned in
+`tools/arti-build/ANDROID_NDK_VERSION`) documented in their READMEs — none of
+them are required to build Amethyst from the committed sources.

-The NDK half of that Arti pin does reach the ordinary Android build, though.
-AGP strips every native library it packages with the NDK's `llvm-strip`, so
-`:amethyst` sets `ndkVersion` from `ANDROID_NDK_VERSION` — one revision for the
-libraries we build and the ones we merge from dependencies. `libarti_android.so`
-is then excluded from that strip step (`packaging.jniLibs.keepDebugSymbols`):
-the Cargo release profile already stripped it, and llvm-strip would only rewrite
-its `.comment` stamps, so skipping the pass costs no size and lets the `.so`
-inside an APK be compared byte-for-byte against the committed, independently
-reproducible one. The Rust toolchain stays irrelevant either way; Studio/AGP
-fetches the pinned NDK on demand, or pre-install it with
+That NDK pin is a single file for the whole repo, not a copy per tool:
+`build-arti.sh`, `build-zxingcpp.sh` and `:amethyst`'s `ndkVersion` all read it,
+so bumping it moves every native build and the packaging toolchain together and
+they cannot drift apart. (It lives under `tools/arti-build/` for history; it is
+not Arti's alone.)
+
+The NDK half of that pin reaches the ordinary Android build. AGP strips every
+native library it packages with the NDK's `llvm-strip`, so `:amethyst` sets
+`ndkVersion` from `ANDROID_NDK_VERSION` — one revision for the libraries we
+build and the ones we merge from dependencies. Both of our own libraries are
+then excluded from that strip step (`packaging.jniLibs.keepDebugSymbols`):
+they are already stripped by the pinned toolchain, so skipping the pass costs
+no size and lets the `.so` inside an APK be compared byte-for-byte against the
+committed, independently reproducible one. The Rust toolchain stays irrelevant
+either way; Studio/AGP fetches the pinned NDK on demand, or pre-install it with
 `sdkmanager "ndk;$(cat tools/arti-build/ANDROID_NDK_VERSION)"`.

-> **One Arti library per ABI split.** The APK is split four ways (`arm64-v8a`,
-> `x86_64`, `armeabi-v7a`, `x86`) and every split needs its own
-> `libarti_android.so`; a split without one installs and runs with Tor silently
-> unavailable. The `verifyArtiAbis` Gradle task fails the build if the two lists
-> drift, and names the `build-arti.sh --target=…` to run.
+> **Every ABI split needs its own copy of each library.** The APK is split four
+> ways (`arm64-v8a`, `x86_64`, `armeabi-v7a`, `x86`). A split missing
+> `libarti_android.so` installs and runs with Tor silently unavailable; one
+> missing `libzxingcpp_android.so` installs with the QR scanner broken. The
+> `verifyNativeAbis` Gradle task fails the build if a library's ABI list drifts
+> from the split list, and names the `build-arti.sh --target=…` /
+> `build-zxingcpp.sh --abi …` to run — and rejects a file that is not an ELF
+> built for that architecture, which a missing-file check would pass.

 ---

@@ -67,14 +67,27 @@ afterEvaluate {
    }
 }

-// Every ABI we split the APK for, and therefore every ABI that needs its own
-// libarti_android.so under src/main/jniLibs/ (see tools/arti-build/). The two
-// lists drifted once: the splits shipped four ABIs while Arti was built for two,
-// so the armeabi-v7a and x86 APKs installed and ran with the dependencies' native
-// libraries all present (secp256k1's JNI ships every ABI) and Tor alone dead for
-// the life of the install. `verifyArtiAbis` below keeps them in step.
+// Every ABI we split the APK for, and therefore every ABI that needs its own copy of each
+// library we build and commit ourselves under src/main/jniLibs/. The lists drifted once: the
+// splits shipped four ABIs while Arti was built for two, so the armeabi-v7a and x86 APKs
+// installed and ran with the dependencies' native libraries all present (secp256k1's JNI ships
+// every ABI) and Tor alone dead for the life of the install. `verifyNativeAbis` below keeps
+// them in step.
 val shippedAbis = listOf("x86", "x86_64", "arm64-v8a", "armeabi-v7a")

+// The libraries that guard covers, and how to rebuild one when it is missing. Both are built
+// from source by tools/ rather than pulled prebuilt, so both can go missing the same way — and
+// a QR scanner that cannot load is as silently broken on that install as a dead Tor.
+val committedNativeLibs =
+    mapOf(
+        "libarti_android.so" to { abi: String, triple: String ->
+            "./tools/arti-build/build-arti.sh --target=$triple"
+        },
+        "libzxingcpp_android.so" to { abi: String, _: String ->
+            "./tools/zxing-cpp-build/build-zxingcpp.sh --abi $abi"
+        },
+    )
+
 android {
    namespace = "com.vitorpamplona.amethyst"
    compileSdk =
@@ -92,10 +105,14 @@ android {
    // libraries") — three different APKs from the same source, which is
    // exactly what F-Droid's rebuild verification cannot have.
    //
-    // Read straight from the Arti pin rather than copied into the version
-    // catalog: the two can then never drift, and bumping ANDROID_NDK_VERSION
-    // (which also means rebuilding the .so) moves the packaging toolchain with
-    // it. See tools/arti-build/README.md → "Reproducible builds".
+    // Read straight from the pin rather than copied into the version catalog:
+    // the two can then never drift, and bumping ANDROID_NDK_VERSION (which also
+    // means rebuilding the .so files) moves the packaging toolchain with it.
+    // That one file is the repo's only NDK pin -- tools/arti-build/build-arti.sh
+    // and tools/zxing-cpp-build/build-zxingcpp.sh read it too, so every
+    // committed .so is produced and stripped by the same revision. It lives
+    // under tools/arti-build for history; it is not Arti's alone. See
+    // tools/arti-build/README.md → "Reproducible builds".
    ndkVersion =
        providers
            .fileContents(layout.settingsDirectory.file("tools/arti-build/ANDROID_NDK_VERSION"))
@@ -341,6 +358,17 @@ android {
            // symbols to drop) and makes that comparison exact. Dependency .so files
            // are still stripped, with the NDK pinned by ndkVersion above.
            keepDebugSymbols += "**/libarti_android.so"
+
+            // Same guarantee for the QR decoder, for a different reason. Unlike Arti's, this
+            // library is *not* currently rewritten by AGP's pass -- verified by running the
+            // pinned NDK's `llvm-strip --strip-unneeded` over the committed file and getting
+            // identical bytes -- because tools/zxing-cpp-build strips it with that very same
+            // llvm-strip, which makes a second pass idempotent. That idempotence is a property
+            // of one NDK revision, though, and reading it back from the APK should not depend
+            // on a strip pass staying a no-op across bumps. Excluding it makes
+            // `unzip -p app.apk lib/<abi>/libzxingcpp_android.so | sha256sum` match
+            // src/main/jniLibs by construction, at no size cost.
+            keepDebugSymbols += "**/libzxingcpp_android.so"
        }
    }

@@ -381,20 +409,20 @@ android {
 // only surfaces at System.loadLibrary time on a user's device — where
 // TorManager's flow swallows the UnsatisfiedLinkError and leaves the status Off
 // forever. Checked at build time instead, against the same list the splits use.
-val verifyArtiAbis =
-    tasks.register("verifyArtiAbis") {
+val verifyNativeAbis =
+    tasks.register("verifyNativeAbis") {
        group = "verification"
-        description = "Checks that every ABI in the APK splits has a libarti_android.so for that architecture."
+        description = "Checks that every ABI in the APK splits has each committed native library, built for that architecture."

        val jniLibs = file("src/main/jniLibs")
        val abis = shippedAbis
-        // Per ABI: the Rust target triple (so a failure names the exact build
-        // command) and the ELF identity the library must have — 32/64-bit class
-        // (header byte 4) and e_machine (bytes 18-19, little-endian on every
-        // Android ABI we ship). Existence alone is not enough: a truncated file,
-        // an empty placeholder, or arm64's .so copied into x86/ all load as
-        // nothing on device, which is the same silent dead Tor this task exists
-        // to prevent — and unlike a missing file, those look fine in git.
+        val libs = committedNativeLibs
+        // Per ABI: the Rust target triple (so an Arti failure names the exact build command) and
+        // the ELF identity every library must have — 32/64-bit class (header byte 4) and
+        // e_machine (bytes 18-19, little-endian on every Android ABI we ship). Existence alone is
+        // not enough: a truncated file, an empty placeholder, or arm64's .so copied into x86/ all
+        // load as nothing on device, which is the same silent failure this task exists to prevent
+        // — and unlike a missing file, those look fine in git.
        val expected =
            mapOf(
                "arm64-v8a" to Triple("aarch64-linux-android", 2, 0xB7),
@@ -405,48 +433,51 @@ val verifyArtiAbis =

        doLast {
            val bitness = mapOf(1 to "32-bit", 2 to "64-bit")
-            val problems = mutableListOf<Pair<String, String>>()
+            val problems = mutableListOf<Triple<String, String, String>>()

-            abis.forEach { abi ->
-                val lib = File(jniLibs, "$abi/libarti_android.so")
-                val want = expected[abi]
-                val header = ByteArray(20)
-                val read = if (lib.isFile) lib.inputStream().use { it.read(header) } else -1
+            libs.keys.forEach { libName ->
+                abis.forEach { abi ->
+                    val lib = File(jniLibs, "$abi/$libName")
+                    val want = expected[abi]
+                    val header = ByteArray(20)
+                    val read = if (lib.isFile) lib.inputStream().use { it.read(header) } else -1

-                val problem =
-                    when {
-                        !lib.isFile -> "no libarti_android.so"
-                        want == null -> "no expected ELF identity recorded for this ABI"
-                        read < header.size ||
-                            header[0] != 0x7F.toByte() ||
-                            header[1] != 'E'.code.toByte() ||
-                            header[2] != 'L'.code.toByte() ||
-                            header[3] != 'F'.code.toByte() -> "not an ELF file (truncated or corrupt)"
-                        header[4].toInt() != want.second ->
-                            "${bitness[header[4].toInt()] ?: "unknown-class"} ELF, expected ${bitness[want.second]}"
-                        else -> {
-                            val machine = (header[18].toInt() and 0xFF) or ((header[19].toInt() and 0xFF) shl 8)
-                            if (machine != want.third) {
-                                "built for ELF machine 0x%02x, expected 0x%02x".format(machine, want.third)
-                            } else {
-                                null
+                    val problem =
+                        when {
+                            !lib.isFile -> "no $libName"
+                            want == null -> "no expected ELF identity recorded for this ABI"
+                            read < header.size ||
+                                header[0] != 0x7F.toByte() ||
+                                header[1] != 'E'.code.toByte() ||
+                                header[2] != 'L'.code.toByte() ||
+                                header[3] != 'F'.code.toByte() -> "not an ELF file (truncated or corrupt)"
+                            header[4].toInt() != want.second ->
+                                "${bitness[header[4].toInt()] ?: "unknown-class"} ELF, expected ${bitness[want.second]}"
+                            else -> {
+                                val machine = (header[18].toInt() and 0xFF) or ((header[19].toInt() and 0xFF) shl 8)
+                                if (machine != want.third) {
+                                    "built for ELF machine 0x%02x, expected 0x%02x".format(machine, want.third)
+                                } else {
+                                    null
+                                }
                            }
                        }
-                    }

-                if (problem != null) problems += abi to problem
+                    if (problem != null) problems += Triple(libName, abi, problem)
+                }
            }

            if (problems.isNotEmpty()) {
                throw GradleException(
                    buildString {
-                        appendLine("libarti_android.so is missing or wrong for ${problems.size} ABI split(s):")
-                        problems.forEach { (abi, problem) -> appendLine("    $abi: $problem") }
-                        appendLine("Those APK splits would install with Tor permanently unavailable.")
-                        appendLine("Rebuild them (tools/arti-build/README.md):")
-                        problems.forEach { (abi, _) ->
+                        appendLine("Committed native libraries are missing or wrong for ${problems.size} (library, ABI split) pair(s):")
+                        problems.forEach { (libName, abi, problem) -> appendLine("    $libName / $abi: $problem") }
+                        appendLine("Those APK splits would install with that library permanently unavailable.")
+                        appendLine("Rebuild them:")
+                        problems.forEach { (libName, abi, _) ->
                            val triple = expected[abi]?.first ?: "<add the Rust target for $abi>"
-                            appendLine("    ./tools/arti-build/build-arti.sh --target=$triple")
+                            val rebuild = libs[libName]?.invoke(abi, triple) ?: "<no rebuild command recorded for $libName>"
+                            appendLine("    $rebuild")
                        }
                        append("…or drop the ABI from `shippedAbis` in amethyst/build.gradle.kts.")
                    },
@@ -455,7 +486,9 @@ val verifyArtiAbis =
        }
    }

-tasks.named("preBuild") { dependsOn(verifyArtiAbis) }
+tasks.named("preBuild") {
+    dependsOn(verifyNativeAbis)
+}

 // androidx.appfunctions-compiler runs in a per-module mode by default,
 // emitting only the dispatcher Kotlin code. The aggregator that builds
@@ -74,7 +74,14 @@ private fun routeFor(
        uriToRoute(uri, accountViewModel.account)
    } catch (e: Throwable) {
        if (e is CancellationException) throw e
-        Log.e("NIP19 Scanner", "Error parsing $contents", e)
+        // The payload itself never reaches the log. A QR code is as likely to hold an nsec, a
+        // wallet-connect secret or a Cashu token as a profile link, and logcat is readable over
+        // adb and swept up by device bug reports — the same material ScannedPayload.containsSecret
+        // exists to keep off the screen two files away. The classification and the length say
+        // enough to debug a routing failure; classifying again here is wrapped because this is
+        // the branch for a payload that already made something throw.
+        val kind = runCatching { classifyScannedPayload(contents)::class.simpleName }.getOrNull() ?: "unclassifiable"
+        Log.e("NIP19 Scanner", "Could not route a scanned $kind payload of ${contents.length} chars", e)
        // A QR code can hold anything at all. Never let one throw.
        null
    }
@@ -28,6 +28,7 @@ import android.net.Uri
 import androidx.core.graphics.scale
 import com.vitorpamplona.quartz.utils.Log
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ensureActive
 import kotlinx.coroutines.withContext
 import kotlin.math.max

@@ -70,12 +71,19 @@ object QrImageImport {
                    if (sampleSizeFor(longestEdge, FIRST_PASS_MAX_EDGE) != 1) add(1)
                }

+            // Checked between passes because a pass itself is one long blocking call into JNI
+            // and cannot be interrupted. The expensive pass is the full-resolution retry: a
+            // modern phone photo is 50-108 MP, so closing the scanner while one is running
+            // otherwise leaves several hundred megabytes and a thorough decode grinding away on
+            // an IO thread for a result nobody is waiting for any more.
            for (sampleSize in sampleSizes) {
+                ensureActive()
                val found = decodeAt(context, uri, sampleSize, upscale = false, decoder)
                if (found.isNotEmpty()) return@withContext found
            }

            if (longestEdge <= SMALL_IMAGE_EDGE) {
+                ensureActive()
                val found = decodeAt(context, uri, sampleSize = 1, upscale = true, decoder)
                if (found.isNotEmpty()) return@withContext found
            }
@@ -23,6 +23,7 @@ package com.vitorpamplona.amethyst.ui.screen.loggedIn.qrcode.scanner
 import androidx.compose.runtime.Stable
 import androidx.compose.runtime.getValue
 import androidx.compose.runtime.mutableFloatStateOf
+import androidx.compose.runtime.mutableLongStateOf
 import androidx.compose.runtime.mutableStateOf
 import androidx.compose.runtime.setValue

@@ -80,7 +81,7 @@ class QrScannerState {
    private var darkSinceMs = 0L

    /** Milliseconds since anything at all was decoded — what auto-zoom watches. */
-    var msSinceLastDetection by mutableStateOf(0L)
+    var msSinceLastDetection by mutableLongStateOf(0L)
        private set

    private var lastDetectionMs = 0L
@@ -107,6 +108,10 @@ class QrScannerState {
        frame = scan.frame
        updateDarkness(scan.brightness, nowMs)

+        // Checked on every frame, not just on empty ones: an abandoned half-capture is abandoned
+        // whether or not the camera is busy reading something else.
+        if (sequence.dropIfStale(nowMs)) sequenceProgress = null
+
        // Seed the clock on the first frame. Left at zero, the very first empty frame would read
        // as "nothing decoded since the epoch" and send auto-zoom hunting before the user has had
        // a chance to aim.
@@ -123,12 +128,6 @@ class QrScannerState {
        if (found.isEmpty()) {
            candidates = emptyList()
            msSinceLastDetection = nowMs - lastDetectionMs
-            // Drop a half-captured multi-part code once its parts stop arriving, or its
-            // "Captured 1 of 3 parts" hint sticks on screen forever and hides every other hint.
-            if (sequenceProgress != null && msSinceLastDetection > StructuredAppendAccumulator.DEFAULT_TIMEOUT_MS) {
-                sequence.reset()
-                sequenceProgress = null
-            }
            return null
        }

@@ -154,7 +153,14 @@ class QrScannerState {
        nowMs: Long,
    ): String? {
        candidates = emptyList()
-        return accept(result.text, nowMs)
+        // Deliberately bypasses the dedupe window. That window exists to stop ONE code decoding
+        // thirty times a second from firing the caller thirty times; a tap is one decision by a
+        // person, and swallowing it makes the highlight a target that can be tapped with nothing
+        // happening. The latch is still armed, so the camera frames that follow -- the tapped
+        // code is very probably still in view -- do not fire it again.
+        lastSubmittedText = result.text
+        lastSubmittedAt = nowMs
+        return result.text
    }

    /**
@@ -143,7 +143,7 @@ private fun explain(payload: ScannedPayload): String =
        // sends them somewhere that cannot accept it.
        is ScannedPayload.Bunker -> stringRes(Res.string.qr_scanner_kind_bunker)
        is ScannedPayload.NostrConnect -> stringRes(Res.string.qr_scanner_kind_signer)
-        is ScannedPayload.EncryptedKey -> stringRes(Res.string.qr_scanner_kind_nsec)
+        is ScannedPayload.PrivateKey -> stringRes(Res.string.qr_scanner_kind_nsec)
        is ScannedPayload.Lightning -> stringRes(Res.string.qr_scanner_kind_lightning)
        is ScannedPayload.Cashu -> stringRes(Res.string.qr_scanner_kind_cashu)
        is ScannedPayload.Web -> stringRes(Res.string.qr_scanner_kind_web)
@@ -107,14 +107,17 @@ sealed interface ScannedPayload {
    ) : ScannedPayload

    /**
-     * Key material we can recognise but not decode — chiefly an `ncryptsec`, which
-     * `Nip19Parser` lists in its regex but has no branch to parse, so it would otherwise land in
-     * [Unknown] and be echoed to the screen.
+     * Key material we can recognise but not decode. Two things land here:
     *
-     * Deliberately classified by *prefix*, not by parse success: whether a payload is dangerous
-     * to display cannot depend on whether we happen to be able to read it.
+     * - an `ncryptsec`, which `Nip19Parser` lists in its regex but has no branch to parse;
+     * - an `nsec` the parser rejected — truncated by a half-finished copy, or transcribed with a
+     *   typo into whatever generated the code. It is still a private key, and a damaged one
+     *   still shows all but a few of its characters.
+     *
+     * Both are classified by *prefix*, not by parse success: whether a payload is dangerous to
+     * display cannot depend on whether we happen to be able to read it.
     */
-    data class EncryptedKey(
+    data class PrivateKey(
        override val raw: String,
    ) : ScannedPayload {
        override val containsSecret get() = true
@@ -160,12 +163,18 @@ fun classifyScannedPayload(text: String): ScannedPayload {
    // Before the NIP-19 scan, and by prefix rather than by parse: an ncryptsec cannot be decoded
    // here, so waiting to find out what it is would mean deciding it is harmless.
    if (lower.startsWith("ncryptsec1") || lower.startsWith("nostr:ncryptsec1")) {
-        return ScannedPayload.EncryptedKey(raw)
+        return ScannedPayload.PrivateKey(raw)
    }
    if (LIGHTNING_PREFIXES.any { lower.startsWith(it) }) return ScannedPayload.Lightning(raw)

    Nip19Parser.uriToRoute(raw)?.let { return ScannedPayload.Nostr(raw, it.entity) }

+    // An nsec the parser would not take. The parse is tried first so a well-formed one still
+    // becomes a [ScannedPayload.Nostr] and keeps the routing that logging in by scanning one
+    // depends on — but a damaged one must not fall through to [ScannedPayload.Unknown], where
+    // the sheet prints the payload on screen with a Copy button next to it.
+    if (lower.startsWith("nsec1") || lower.startsWith("nostr:nsec1")) return ScannedPayload.PrivateKey(raw)
+
    if (HEX_64.matches(raw)) {
        val npub = runCatching { NPub.create(raw.lowercase()) }.getOrNull()
        if (npub != null) return ScannedPayload.HexPubKey(raw, npub)
@@ -86,9 +86,26 @@ class StructuredAppendAccumulator(
        return joined.toString()
    }

+    /**
+     * Drops a half-captured sequence whose parts stopped arriving, and says whether it did.
+     *
+     * Kept here, against this accumulator's own last-update clock, because that is the only clock
+     * that measures the right thing. The caller cannot substitute "nothing has been decoded at
+     * all": walking away from a half-scanned poster and pointing the camera at an ordinary code
+     * keeps decoding something on every frame, so that clock never advances and the abandoned
+     * sequence is never dropped.
+     */
+    fun dropIfStale(nowMs: Long): Boolean {
+        if (expected == 0) return false
+        if (nowMs - lastUpdateMs <= timeoutMs) return false
+        reset()
+        return true
+    }
+
    fun reset() {
        sequenceId = null
        expected = 0
+        lastUpdateMs = 0
        parts.clear()
    }

@@ -0,0 +1,147 @@
+/*
+ * Copyright (c) 2025 Vitor Pamplona
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of
+ * this software and associated documentation files (the "Software"), to deal in
+ * the Software without restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the
+ * Software, and to permit persons to whom the Software is furnished to do so,
+ * subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS
+ * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR
+ * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
+ * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+package com.vitorpamplona.amethyst.ui.screen.loggedIn.qrcode.scanner
+
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+class QrScannerStateTest {
+    private val frame = ScanFrame(720, 1280)
+
+    private fun result(
+        text: String,
+        sequenceId: String? = null,
+        sequenceIndex: Int = -1,
+        sequenceSize: Int = -1,
+    ) = ScanResult(
+        text = text,
+        bounds = null,
+        sequenceId = sequenceId,
+        sequenceIndex = sequenceIndex,
+        sequenceSize = sequenceSize,
+    )
+
+    private fun scan(
+        vararg results: ScanResult,
+        brightness: Float = 1f,
+    ) = FrameScan(results.toList(), frame, brightness)
+
+    // ---- the tap path ----
+
+    @Test
+    fun `a tapped candidate is accepted even right after the same code was submitted`() {
+        val state = QrScannerState()
+
+        // Alone in frame, so it is auto-accepted.
+        assertEquals("npub1aaa", state.onFrame(scan(result("npub1aaa")), 1_000L))
+
+        // The caller could not use it; the user dismisses the sheet.
+        state.onRejected(classified())
+        state.dismissRejection(1_100L)
+
+        // A second code enters the frame, so nothing is auto-accepted any more...
+        assertNull(state.onFrame(scan(result("npub1aaa"), result("npub1bbb")), 1_200L))
+
+        // ...and the user taps the first one deliberately, inside the dedupe window.
+        assertEquals("npub1aaa", state.onCandidateTapped(result("npub1aaa"), 1_300L))
+    }
+
+    // ---- multi-part sequences ----
+
+    @Test
+    fun `a half-captured sequence is dropped once an unrelated code is being scanned`() {
+        val state = QrScannerState()
+
+        assertNull(state.onFrame(scan(result("part0", "seq", 0, 3)), 1_000L))
+        assertEquals(1 to 3, state.sequenceProgress)
+
+        // The user gives up and scans an ordinary code instead, for well past the timeout.
+        var now = 2_000L
+        repeat(5) {
+            state.onFrame(scan(result("npub1zzz")), now)
+            now += StructuredAppendAccumulator.DEFAULT_TIMEOUT_MS / 2
+        }
+
+        assertNull("the abandoned sequence hint is still on screen", state.sequenceProgress)
+    }
+
+    @Test
+    fun `a part claiming an index outside its own size never joins into the payload`() {
+        val state = QrScannerState()
+
+        // Two parts arrive for a 2-part sequence, but the second claims index 7. The count is
+        // satisfied while index 1 is still missing, and splicing "a" with a part that does not
+        // belong at that position would hand the caller a corrupt payload.
+        assertNull(state.onFrame(scan(result("a", "seq", 0, 2)), 1_000L))
+        assertNull(state.onFrame(scan(result("b", "seq", 7, 2)), 1_100L))
+
+        // The genuine part 1 completes it, and the stray index is not spliced in.
+        assertEquals("ab!", state.onFrame(scan(result("b!", "seq", 1, 2)), 1_200L))
+    }
+
+    // ---- the rules that already work, pinned so they keep working ----
+
+    @Test
+    fun `one code alone in frame is accepted, and not again while it is held there`() {
+        val state = QrScannerState()
+
+        assertEquals("npub1aaa", state.onFrame(scan(result("npub1aaa")), 1_000L))
+        assertNull(state.onFrame(scan(result("npub1aaa")), 1_100L))
+        assertNull(state.onFrame(scan(result("npub1aaa")), 1_000L + QrScannerState.DEDUPE_MS - 1))
+        assertEquals("npub1aaa", state.onFrame(scan(result("npub1aaa")), 1_000L + QrScannerState.DEDUPE_MS))
+    }
+
+    @Test
+    fun `several codes in frame are drawn but none is chosen`() {
+        val state = QrScannerState()
+
+        assertNull(state.onFrame(scan(result("npub1aaa"), result("npub1bbb")), 1_000L))
+        assertEquals(2, state.candidates.size)
+    }
+
+    @Test
+    fun `nothing is decided while the cannot-open sheet is up`() {
+        val state = QrScannerState()
+
+        state.onRejected(classified())
+        assertNull(state.onFrame(scan(result("npub1bbb")), 1_000L))
+        assertTrue(state.candidates.isEmpty())
+    }
+
+    @Test
+    fun `the torch is offered only after the scene has been dark for a while`() {
+        val state = QrScannerState()
+
+        state.onFrame(scan(brightness = 0.05f), 1_000L)
+        assertFalse(state.isDark)
+
+        state.onFrame(scan(brightness = 0.05f), 1_000L + QrScannerState.DARK_DWELL_MS)
+        assertTrue(state.isDark)
+
+        state.onFrame(scan(brightness = 0.9f), 2_500L)
+        assertFalse(state.isDark)
+    }
+
+    private fun classified() = classifyScannedPayload("not something this screen takes")
+}
@@ -144,6 +144,27 @@ class ScannedPayloadTest {
        assertFalse(classifyScannedPayload("just some text").containsSecret)
    }

+    @Test
+    fun `an nsec is secret in every shape a QR code can carry it`() {
+        // Bech32 is case-insensitive, and a QR encoder that wants alphanumeric mode -- half the
+        // bytes of byte mode, so a noticeably smaller and easier-to-scan code -- must uppercase
+        // its payload. An uppercased nsec is therefore not a corner case; it is what a
+        // size-conscious generator produces.
+        assertTrue("an uppercase nsec must never be echoed", classifyScannedPayload(NSEC_FIXTURE.uppercase()).containsSecret)
+        assertTrue(classifyScannedPayload("nostr:$NSEC_FIXTURE").containsSecret)
+        assertTrue(classifyScannedPayload("NOSTR:${NSEC_FIXTURE.uppercase()}").containsSecret)
+    }
+
+    @Test
+    fun `an nsec too damaged to parse is still secret`() {
+        // Same principle the ncryptsec branch already follows: whether a payload is dangerous to
+        // put on screen cannot depend on whether we happen to be able to read it. A half-copied
+        // key pasted from the clipboard, or one transcribed with a typo into a QR generator,
+        // still shows most of its characters.
+        assertTrue(classifyScannedPayload(NSEC_FIXTURE.dropLast(4)).containsSecret)
+        assertTrue(classifyScannedPayload(NSEC_FIXTURE.dropLast(1) + "q").containsSecret)
+    }
+
    @Test
    fun `raw is always the trimmed input`() {
        assertEquals(npub, classifyScannedPayload("\n $npub \t").raw)
@@ -196,8 +196,9 @@ The ABI list therefore lives in three places that must agree: `splits.abi` in
 `amethyst/build.gradle.kts`, `targets` in `rust-toolchain.toml`, and `TARGETS`
 in `build-arti.sh`. (`verify-reproducible.sh` has no copy of its own — it asks
 `build-arti.sh --print-abis`, so it can never hash a different set than the one
-it just rebuilt.) The `verifyArtiAbis` Gradle task, wired into `preBuild`, fails
-the build when an ABI split has no `libarti_android.so` **or** has one that is
+it just rebuilt.) The `verifyNativeAbis` Gradle task, wired into `preBuild`, fails
+the build when an ABI split is missing any committed native library
+(`libarti_android.so`, `libzxingcpp_android.so`) **or** has one that is
 not an ELF of that architecture — a truncated file or arm64's library copied
 into `x86/` loads as nothing on device, exactly like a missing one, and unlike a
 missing one it looks fine in `git status`.
@@ -1 +0,0 @@
-30.0.16248370
@@ -30,8 +30,10 @@ verify the binaries, bump the zxing-cpp version, or change the build flags.
 ```

 Prerequisites: `git`, `cmake`, `ninja`, and the exact NDK revision in
-[`ANDROID_NDK_VERSION`](ANDROID_NDK_VERSION) — which is deliberately the same revision
-`tools/arti-build` pins, so one NDK install serves both native builds.
+[`tools/arti-build/ANDROID_NDK_VERSION`](../arti-build/ANDROID_NDK_VERSION). That is the
+repo's single NDK pin, not a copy: `build-zxingcpp.sh`, `build-arti.sh` and `:amethyst`'s
+`ndkVersion` all read that one file, so one NDK install serves every native build and the
+three can never drift apart.

 ## Reproducible builds

@@ -39,7 +41,7 @@ Five things have to be fixed, and each is:

 | Source of non-determinism | Pinned by |
 |---|---|
-| compiler + linker version | [`ANDROID_NDK_VERSION`](ANDROID_NDK_VERSION); `build-zxingcpp.sh` refuses any other revision |
+| compiler + linker version | [`tools/arti-build/ANDROID_NDK_VERSION`](../arti-build/ANDROID_NDK_VERSION); `build-zxingcpp.sh` refuses any other revision |
 | upstream source | [`ZXING_CPP_VERSION`](ZXING_CPP_VERSION), cloned at that tag and nothing else |
 | absolute paths baked into `__FILE__`, assertions, debug records | `-ffile-prefix-map` / `-fdebug-prefix-map` in [`repro-env.sh`](repro-env.sh) |
 | timestamps | `SOURCE_DATE_EPOCH`, derived from the pinned tag's commit rather than from build time; `__DATE__`/`__TIME__` redacted |
@@ -13,15 +13,22 @@
 #   ./build-zxingcpp.sh --out DIR        # write .so somewhere else (verify uses this)
 #
 # Prerequisites: git, cmake, ninja, and the exact NDK revision in
-# ANDROID_NDK_VERSION. Any other revision is refused — it would change the
-# output bytes, which is the whole point.
+# tools/arti-build/ANDROID_NDK_VERSION. Any other revision is refused — it
+# would change the output bytes, which is the whole point.
 set -euo pipefail

 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)"

 ZXING_VERSION="$(tr -d '[:space:]' < "$SCRIPT_DIR/ZXING_CPP_VERSION")"
-NDK_VERSION="$(tr -d '[:space:]' < "$SCRIPT_DIR/ANDROID_NDK_VERSION")"
+# One pin for the whole repo, deliberately not a copy of our own. :amethyst
+# already reads this same file for `ndkVersion` (the NDK that strips whatever
+# lands in src/main/jniLibs), so a second copy here could only ever be a way
+# to disagree with the toolchain that packages the .so we produce — the exact
+# byte-level drift the pin exists to prevent. Bumping it rebuilds both
+# libarti_android.so and libzxingcpp_android.so, which is correct: they must
+# be built and stripped by one NDK.
+NDK_VERSION="$(tr -d '[:space:]' < "$PROJECT_ROOT/tools/arti-build/ANDROID_NDK_VERSION")"

 # Canonical build path. Codegen and link ordering can key on the real build
 # directory even with path remapping in place, so everyone builds here or
@@ -33,9 +40,10 @@ OUTPUT_DIR="$PROJECT_ROOT/amethyst/src/main/jniLibs"
 LIB_NAME="libzxingcpp_android.so"
 MIN_SDK_VERSION=26

-# Every ABI the app splits on. Unlike Tor — an optional feature that ships on
-# two ABIs — a QR scanner that does not load is a broken core feature, and the
-# decoder this replaced was pure Java and worked everywhere.
+# Every ABI the app splits on. A QR scanner that does not load is a broken core
+# feature — the decoder this replaced was pure Java and worked everywhere — so
+# every split gets one, and :amethyst's verifyNativeAbis fails the build if one
+# is missing or built for the wrong architecture.
 ABIS=(arm64-v8a armeabi-v7a x86 x86_64)

 while [ $# -gt 0 ]; do