diff --git a/BUILDING.md b/BUILDING.md index 0c5be556fe..5e7aaf68af 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -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. --- diff --git a/amethyst/build.gradle.kts b/amethyst/build.gradle.kts index 45681f09b1..1f432394be 100644 --- a/amethyst/build.gradle.kts +++ b/amethyst/build.gradle.kts @@ -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//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>() + val problems = mutableListOf>() - 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 ?: "" - appendLine(" ./tools/arti-build/build-arti.sh --target=$triple") + val rebuild = libs[libName]?.invoke(abi, triple) ?: "" + 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 diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/QrCodeScanner.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/QrCodeScanner.kt index 355c84bbce..be5a115e2c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/QrCodeScanner.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/QrCodeScanner.kt @@ -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 } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrImageImport.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrImageImport.kt index 4e9d376766..83141ac20c 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrImageImport.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrImageImport.kt @@ -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 } diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrScannerState.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrScannerState.kt index fec05646b0..e868474895 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrScannerState.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrScannerState.kt @@ -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 } /** diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScanOutcomeSheet.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScanOutcomeSheet.kt index 4043dd3e78..5d210c288a 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScanOutcomeSheet.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScanOutcomeSheet.kt @@ -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) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScannedPayload.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScannedPayload.kt index 24dbc8a2f7..498ec86884 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScannedPayload.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScannedPayload.kt @@ -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) diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/StructuredAppendAccumulator.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/StructuredAppendAccumulator.kt index 86d6a845d8..26a0c8cc77 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/StructuredAppendAccumulator.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/StructuredAppendAccumulator.kt @@ -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() } diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrScannerStateTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrScannerStateTest.kt new file mode 100644 index 0000000000..dd8ac61c84 --- /dev/null +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/QrScannerStateTest.kt @@ -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") +} diff --git a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScannedPayloadTest.kt b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScannedPayloadTest.kt index a97bc38f09..b9fa0f3bd1 100644 --- a/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScannedPayloadTest.kt +++ b/amethyst/src/test/java/com/vitorpamplona/amethyst/ui/screen/loggedIn/qrcode/scanner/ScannedPayloadTest.kt @@ -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) diff --git a/tools/arti-build/README.md b/tools/arti-build/README.md index 9d9987a064..bf29ccabb1 100644 --- a/tools/arti-build/README.md +++ b/tools/arti-build/README.md @@ -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`. diff --git a/tools/zxing-cpp-build/ANDROID_NDK_VERSION b/tools/zxing-cpp-build/ANDROID_NDK_VERSION deleted file mode 100644 index e7aa49e341..0000000000 --- a/tools/zxing-cpp-build/ANDROID_NDK_VERSION +++ /dev/null @@ -1 +0,0 @@ -30.0.16248370 diff --git a/tools/zxing-cpp-build/README.md b/tools/zxing-cpp-build/README.md index 68fea2ad38..48d1426725 100644 --- a/tools/zxing-cpp-build/README.md +++ b/tools/zxing-cpp-build/README.md @@ -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 | diff --git a/tools/zxing-cpp-build/build-zxingcpp.sh b/tools/zxing-cpp-build/build-zxingcpp.sh index a1b253ab14..50b45f9df6 100755 --- a/tools/zxing-cpp-build/build-zxingcpp.sh +++ b/tools/zxing-cpp-build/build-zxingcpp.sh @@ -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