fix: harden the Arti build gates and drop initialize()'s sentinel

Follow-up to the toolchain update, from an audit of that diff.

Build script:
- The NDK pin rejected machines that have the pinned revision installed.
  An exported ANDROID_NDK_HOME or ANDROID_NDK_ROOT short-circuited the
  search and then failed the revision check, and GitHub runners export both
  at their own bundled NDK. Every candidate is now checked against its own
  source.properties and a mismatch moves on, so the build fails only when
  the pinned revision is genuinely absent, listing what it found instead.
- verify_jni_symbols printed missing exports and exited 0, so a library that
  would throw UnsatisfiedLinkError on every call could ship. It now fails the
  build, and checks only the ABIs this run built.
- Both post-build checks now use the pinned NDK's own llvm-readelf and
  llvm-nm. The stamp check silently skipped on macOS, which has no readelf,
  and Apple's nm cannot read ELF at all, so the symbol check would have
  reported every symbol missing there.
- The stamp check read its note through `readelf | grep -q`, the same
  SIGPIPE-plus-pipefail shape this branch removed from the symbol check.
- $HOME is expanded with a default, so `set -u` no longer aborts before the
  "NDK not found" message in an environment without HOME.

verify-reproducible.sh hashed every .so under jniLibs, so --release, which
rebuilds arm64 only, hashed the untouched x86_64 library identically in both
runs and reported the whole tree reproducible and matching the commit. It now
hashes and diffs only the ABIs the run builds, and prints which those are.

lib.rs:
- initialize() signalled "already initialized" out of the JNI closure as an
  empty string, re-tested after it. A destroy() landing in between would let
  the empty string through as the data directory, which resolves to relative
  state/ and cache/ paths against the process working directory. The check
  now reads the whole Option outside the closure and no sentinel exists.
- Corrected the comments claiming the error policy keeps a panic from
  crossing extern "C". It does not: the policy's panic arm runs through
  catch_unwind, which catches nothing under this crate's panic = "abort"
  profile. The Err arm, which is what the code relies on, is unaffected.

README: the troubleshooting section still told readers to install cargo-ndk
unpinned and to export ANDROID_NDK_HOME at an arbitrary revision, which was
the exact way to trip the old gate.

Verified: two clean builds byte-for-byte identical, both ABIs stamped r30,
JNI exports present, 16 KiB alignment kept. JVM tier-3 smoke test green, and
a scratch harness drove getVersion, setLogCallback, initialize, a second
initialize on a live client (the reuse path the sentinel used to carry) and
destroy over real JNI.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011cSuXeu4bUTNRAUCZJcLLW

tools/arti-build/Cargo.toml

@@ -18,9 +18,10 @@ arti-client = { version = "0.46", default-features = false, features = [
 ] }
 tor-rtcompat = { version = "0.46", default-features = false, features = ["tokio", "rustls"] }
 # Direct dep on rustls so we can install the `ring` crypto provider ourselves —
-# arti-v2.3.0's tor-rtcompat no longer installs one implicitly. `ring` matches
+# since arti-v2.3.0 tor-rtcompat no longer installs one implicitly. `ring`
-# what arti-v2.2.0 effectively used and avoids the Android build pain of
+# matches what arti-v2.2.0 effectively used and avoids the Android build pain
-# aws-lc-rs (which became Arti's default in 2.3.0).
+# of aws-lc-rs (Arti's default since 2.3.0), which `default-features = false`
+# keeps out of the build entirely.
 rustls = { version = "0.23", default-features = false, features = ["ring", "std"] }
 jni = "0.22"
 tokio = { version = "1", features = ["rt-multi-thread", "net", "io-util", "time", "macros"] }

tools/arti-build/README.md

@@ -41,8 +41,9 @@ committed binary wasn't tampered with. **Five** things have to be fixed:
 > `~/Android/Sdk/ndk/*/`, so the committed libraries were produced by r25b while
 > this file told everyone to install r27 — two verifiers could both follow the
 > README and get different, equally "correct" results. `build-arti.sh` now
-> resolves the pinned revision by name, re-reads `source.properties` to confirm
+> reads each candidate's `source.properties` and keeps looking until it finds
-> it, and re-checks the `.note.android.ident` stamp of every `.so` it produced.
+> the pinned revision, then re-checks the `.note.android.ident` stamp of every
+> `.so` it produced.
 >
 > [`CARGO_NDK_VERSION`](CARGO_NDK_VERSION) records the `cargo-ndk` release the
 > pinned output was verified with. `cargo-ndk` only wraps the NDK, so a mismatch
@@ -106,9 +107,13 @@ repo is checked out.
    sdkmanager "ndk;$(cat ANDROID_NDK_VERSION)"
    ```
    `build-arti.sh` finds it automatically under `$ANDROID_HOME/ndk/`,
-   `~/Android/Sdk/ndk/`, `~/Library/Android/sdk/ndk/` or
+   `$ANDROID_SDK_ROOT/ndk/`, `~/Android/Sdk/ndk/`, `~/Library/Android/sdk/ndk/`
-   `/usr/local/lib/android/sdk/ndk/`. Set `ANDROID_NDK_HOME` only if yours
+   or `/usr/local/lib/android/sdk/ndk/`. `ANDROID_NDK_HOME` and
-   lives somewhere else — it is version-checked either way.
+   `ANDROID_NDK_ROOT` are tried first when set, but they are only hints: every
+   candidate is checked against its own `source.properties`, and one at the
+   wrong revision is reported and skipped rather than failing the build. CI
+   images (GitHub runners among them) export both at a bundled NDK that is not
+   ours.
 
 ## Building
 
@@ -168,9 +173,12 @@ readelf -p .note.android.ident amethyst/src/main/jniLibs/arm64-v8a/libarti_andro
 readelf -p .comment amethyst/src/main/jniLibs/arm64-v8a/libarti_android.so
 ```
 
-For the pinned toolchain that prints `r30` / `16248370`, clang 21.0.0 and the
+For the pinned toolchain the first command prints `r30` and build `16248370`.
-`rustc` version from `rust-toolchain.toml`. `build-arti.sh` runs the first check
+The second prints the `rustc` version from `rust-toolchain.toml`, the NDK's
-itself after every build.
+`clang` and `LLD`, and a second, different clang string that comes from the
+prebuilt runtime objects the NDK links in — two clang lines there is normal.
+`build-arti.sh` runs the first check itself after every build, using the NDK's
+own `llvm-readelf` so it works the same on macOS.
 
 ## Directory structure
 
@@ -285,13 +293,18 @@ panic = "abort"       # No unwinding (smaller binary)
 
 ### `cargo-ndk` not found
 ```bash
-cargo install cargo-ndk
+cargo install cargo-ndk --version "$(cat CARGO_NDK_VERSION)" --locked
 ```
 
-### NDK not found
+### NDK not found, or "wrong revision"
+Install the pinned revision — the build refuses any other, and the error lists
+every directory it looked at and what it found there:
 ```bash
-export ANDROID_NDK_HOME="$HOME/Android/Sdk/ndk/<version>"
+sdkmanager "ndk;$(cat ANDROID_NDK_VERSION)"
 ```
+Point `ANDROID_NDK_HOME` at it only if it lives outside the standard SDK
+layouts; an `ANDROID_NDK_HOME` left over from another project is skipped, not
+fatal.
 
 ### Rust targets not installed
 ```bash

tools/arti-build/build-arti.sh

@@ -85,6 +85,29 @@ print_info()    { echo -e "${YELLOW}→ $1${NC}"; }
 # Prerequisites
 # ============================================================================
 
+# Pkg.Revision of an NDK install, or empty if the directory is not one.
+ndk_revision() {
+    sed -n 's/^Pkg\.Revision *= *//p' "$1/source.properties" 2>/dev/null | tr -d '[:space:]' || true
+}
+
+# Path to an ELF tool, preferring the pinned NDK's own llvm-* copy. The NDK
+# ships them on every platform, which keeps the post-build checks working on
+# macOS: there is no readelf in the Xcode command line tools, and Apple's nm
+# cannot read ELF at all, so the checks would otherwise skip or report every
+# symbol missing on exactly the machines most likely to have the wrong NDK.
+ndk_tool() {
+    local name="$1" candidate
+    for candidate in "${ANDROID_NDK_HOME:-}"/toolchains/llvm/prebuilt/*/bin/"llvm-$name"; do
+        if [ -x "$candidate" ]; then
+            echo "$candidate"
+            return 0
+        fi
+    done
+    command -v "$name" 2>/dev/null && return 0
+    command -v "g$name" 2>/dev/null && return 0
+    return 1
+}
+
 check_prerequisites() {
     print_header "Checking prerequisites"
 
@@ -102,43 +125,48 @@ check_prerequisites() {
         print_success "cargo-ndk: $CARGO_NDK_VERSION"
     fi
 
-    # An explicit ANDROID_NDK_HOME wins (it is verified below like any other);
+    # Find the pinned revision wherever it lives, checking each candidate's own
-    # otherwise look for the pinned revision by name in the usual SDK layouts.
+    # source.properties and moving on when it does not match. An exported
-    # Deliberately no wildcard: picking "some NDK" is what let the committed
+    # ANDROID_NDK_HOME / ANDROID_NDK_ROOT is only a hint: CI images (GitHub
-    # binaries be built with r25b while the docs asked for r27.
+    # runners export both) and IDE installs routinely point them at a bundled
-    if [ -z "${ANDROID_NDK_HOME:-}" ]; then
+    # NDK that is not ours, and failing outright there would reject a machine
-        for candidate in \
+    # that has the pinned revision installed right next to it. No wildcard
-            "${ANDROID_NDK_ROOT:-}" \
+    # anywhere: picking "some NDK" is what let the committed binaries be built
-            "${ANDROID_HOME:-}/ndk/$NDK_VERSION" \
+    # with r25b while the docs asked for r27.
-            "${ANDROID_SDK_ROOT:-}/ndk/$NDK_VERSION" \
+    local candidate revision found_ndk="" rejected=""
-            "$HOME/Android/Sdk/ndk/$NDK_VERSION" \
+    for candidate in \
-            "$HOME/Library/Android/sdk/ndk/$NDK_VERSION" \
+        "${ANDROID_NDK_HOME:-}" \
-            "/usr/local/lib/android/sdk/ndk/$NDK_VERSION"; do
+        "${ANDROID_NDK_ROOT:-}" \
-            [ -n "$candidate" ] || continue
+        "${ANDROID_HOME:-}/ndk/$NDK_VERSION" \
-            if [ -d "$candidate" ]; then
+        "${ANDROID_SDK_ROOT:-}/ndk/$NDK_VERSION" \
-                export ANDROID_NDK_HOME="${candidate%/}"
+        "${HOME:-}/Android/Sdk/ndk/$NDK_VERSION" \
-                break
+        "${HOME:-}/Library/Android/sdk/ndk/$NDK_VERSION" \
-            fi
+        "/usr/local/lib/android/sdk/ndk/$NDK_VERSION"; do
-        done
+        [ -n "$candidate" ] || continue
-    fi
+        [ -d "$candidate" ] || continue
 
-    if [ -z "${ANDROID_NDK_HOME:-}" ]; then
+        revision="$(ndk_revision "$candidate")"
-        print_error "Android NDK $NDK_VERSION not found (and ANDROID_NDK_HOME is unset)"
+        if [ "$revision" = "$NDK_VERSION" ]; then
+            found_ndk="${candidate%/}"
+            break
+        fi
+        rejected="${rejected}    ${candidate%/} is ${revision:-not an NDK}"$'\n'
+    done
+
+    if [ -z "$found_ndk" ]; then
+        print_error "Android NDK $NDK_VERSION not found"
+        echo "  It is pinned because another revision produces a .so that does not"
+        echo "  match the committed one (tools/arti-build/ANDROID_NDK_VERSION)."
+        if [ -n "$rejected" ]; then
+            echo "  Looked at, wrong revision:"
+            printf '%s' "$rejected"
+        fi
         echo "  Install it:  sdkmanager \"ndk;$NDK_VERSION\""
         echo "  Or point ANDROID_NDK_HOME at an existing $NDK_VERSION install."
         exit 1
     fi
 
-    local found_ndk
+    export ANDROID_NDK_HOME="$found_ndk"
-    found_ndk="$(sed -n 's/^Pkg\.Revision *= *//p' "$ANDROID_NDK_HOME/source.properties" 2>/dev/null | tr -d '[:space:]' || true)"
-    if [ "$found_ndk" != "$NDK_VERSION" ]; then
-        print_error "NDK revision mismatch — this build would not reproduce the shipped .so"
-        echo "  Pinned:   $NDK_VERSION (tools/arti-build/ANDROID_NDK_VERSION)"
-        echo "  Found:    ${found_ndk:-unknown} at $ANDROID_NDK_HOME"
-        echo "  Install:  sdkmanager \"ndk;$NDK_VERSION\""
-        exit 1
-    fi
-
     print_success "NDK: $ANDROID_NDK_HOME ($NDK_VERSION)"
 
     for target in "${TARGETS[@]}"; do
@@ -279,11 +307,20 @@ verify_jni_symbols() {
         "Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_destroy"
     )
 
-    for arch_dir in "$OUTPUT_DIR"/*/; do
+    local nm_bin
-        local lib="$arch_dir$LIB_NAME"
+    nm_bin="$(ndk_tool nm || true)"
+    if [ -z "$nm_bin" ]; then
+        print_error "no nm found (looked in the NDK and on PATH) — cannot verify the JNI exports"
+        exit 1
+    fi
+
+    local failed=0
+    for target in "${TARGETS[@]}"; do
+        local arch
+        arch="$(abi_dir_for "$target")"
+        local lib="$OUTPUT_DIR/$arch/$LIB_NAME"
         [ -f "$lib" ] || continue
 
-        local arch=$(basename "$arch_dir")
         local missing=0
 
         # Read the dynamic symbol table once, into a variable. Piping nm into
@@ -292,7 +329,7 @@ verify_jni_symbols() {
         # reports the pipeline as failed — so every symbol that IS exported gets
         # reported as missing. (Reproducible on any build, old or new.)
         local syms
-        syms="$(nm -D "$lib" 2>/dev/null || true)"
+        syms="$("$nm_bin" -D "$lib" 2>/dev/null || true)"
 
         for sym in "${expected_symbols[@]}"; do
             if [[ "$syms" != *"$sym"* ]]; then
@@ -303,16 +340,27 @@ verify_jni_symbols() {
 
         if [ "$missing" -eq 0 ]; then
             print_success "$arch: All JNI symbols present"
+        else
+            failed=1
         fi
     done
+
+    # Hard failure: a library missing these exports still loads, and then every
+    # ArtiNative call throws UnsatisfiedLinkError at runtime instead.
+    if [ "$failed" -ne 0 ]; then
+        print_error "JNI exports missing — refusing to leave this .so in jniLibs"
+        exit 1
+    fi
 }
 
 verify_ndk_stamp() {
     print_header "Verifying NDK stamp"
 
-    if ! command -v readelf >/dev/null 2>&1; then
+    local readelf_bin
-        print_info "readelf not found — skipping (install binutils to enable this check)"
+    readelf_bin="$(ndk_tool readelf || true)"
-        return 0
+    if [ -z "$readelf_bin" ]; then
+        print_error "no readelf found (looked in the NDK and on PATH) — cannot verify the NDK stamp"
+        exit 1
     fi
 
     # Every NDK-linked shared object carries .note.android.ident, which records
@@ -326,11 +374,17 @@ verify_ndk_stamp() {
         local lib="$OUTPUT_DIR/$arch/$LIB_NAME"
         [ -f "$lib" ] || continue
 
-        if readelf -p .note.android.ident "$lib" 2>/dev/null | grep -qw "$NDK_BUILD_NUMBER"; then
+        # Read the note once into a variable: `readelf | grep -q` would let grep
+        # exit first, kill readelf with SIGPIPE, and fail the pipeline under
+        # `set -o pipefail` — the same trap that made the symbol check above
+        # report every exported symbol as missing.
+        local note
+        note="$("$readelf_bin" -p .note.android.ident "$lib" 2>/dev/null || true)"
+        if grep -qw "$NDK_BUILD_NUMBER" <<< "$note"; then
             print_success "$arch: built by NDK $NDK_VERSION"
         else
             print_error "$arch: not stamped with NDK build $NDK_BUILD_NUMBER — wrong toolchain?"
-            readelf -p .note.android.ident "$lib" 2>/dev/null || true
+            printf '%s\n' "$note"
             exit 1
         fi
     done

tools/arti-build/src/lib.rs

@@ -86,8 +86,13 @@ pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_getVersion<'
 ) -> JString<'caller> {
     // jni 0.22: the raw environment pointer is FFI-only (`EnvUnowned`); JNI calls
     // need the `Env` that `with_env` borrows for the closure. `resolve` maps an
-    // error to the policy — here a Java RuntimeException plus a null return —
+    // `Err` to the policy — here a Java RuntimeException plus a null return —
-    // instead of unwinding out of `extern "C"`, which aborts the process.
+    // rather than losing it.
+    //
+    // Only the `Err` half is live: the policy's panic half runs through
+    // `catch_unwind`, which catches nothing under this crate's
+    // `panic = "abort"` release profile, so a panic in here still takes the
+    // process down exactly as it did before the migration.
     env.with_env(|env| -> JniResult<JString<'caller>> {
         if JAVA_VM.lock().unwrap().is_none() {
             if let Ok(vm) = env.get_java_vm() {
@@ -134,10 +139,16 @@ pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_initialize<'
     // Everything JNI-owned is read inside this closure; the rest of the function
     // is pure Rust that blocks on Tokio, which must not hold an `Env`.
     //
-    // The policy only applies to a panic or an `Err` returned here, and `None`
+    // `None` carries a failed read, because it is `Option::default()` and so is
-    // is `Option::default()`, so both of those land on the same `-1` the old
+    // also what the policy yields for an `Err`. Both end at the same `-1` the
-    // `Err` arm returned. A jint policy default would have been `0`, which this
+    // old `Err` arm returned. Resolving to `jint` directly would have defaulted
-    // API reports as success.
+    // to `0`, the value this API reports as success.
+    //
+    // The already-initialized check deliberately stays *outside* the closure,
+    // against the whole `Option`: threading it through as a sentinel value
+    // would leave that sentinel to be re-tested after the closure, and a
+    // concurrent destroy() landing in between would let it through as the data
+    // directory.
     let data_dir_str: Option<String> = env
         .with_env(|env| -> JniResult<Option<String>> {
             if JAVA_VM.lock().unwrap().is_none() {
@@ -146,12 +157,6 @@ pub extern "C" fn Java_com_vitorpamplona_amethyst_ui_tor_ArtiNative_initialize<'
                 }
             }
 
-            // Already initialized — the caller-visible "reuse" path still has to
-            // run below, so signal it with an empty string rather than here.
-            if ARTI_CLIENT.lock().unwrap().is_some() {
-                return Ok(Some(String::new()));
-            }
-
             Ok(match data_dir.try_to_string(env) {
                 Ok(s) => Some(s),
                 Err(e) => {

tools/arti-build/verify-reproducible.sh

@@ -28,15 +28,25 @@ sha256() {
     if command -v sha256sum >/dev/null 2>&1; then sha256sum "$@"; else shasum -a 256 "$@"; fi
 }
 
-# sha256 of every built .so, keyed by ABI dir (relative paths → stable keys).
+# Only the ABIs this run actually rebuilds. Hashing everything under jniLibs/
-# `find | sort` (plain text sort) is portable across GNU and BSD userlands.
+# (what `find` used to do) made `--release` look like it had verified the
+# x86_64 library: that build never touches it, so the untouched file hashed
+# identically in both runs and the script reported the whole tree reproducible
+# and matching the commit.
+ABIS="arm64-v8a x86_64"
+for arg in ${PASSTHRU[@]+"${PASSTHRU[@]}"}; do
+    [ "$arg" = "--release" ] && ABIS="arm64-v8a"
+done
+
+# sha256 of each built .so, keyed by ABI dir (relative paths → stable keys).
 hashes() {
-    ( cd "$JNILIBS" && find . -name libarti_android.so | sort | while IFS= read -r f; do
+    ( cd "$JNILIBS" && for abi in $ABIS; do
-        sha256 "$f"
+        [ -f "$abi/libarti_android.so" ] && sha256 "$abi/libarti_android.so"
     done )
 }
 
 echo "### Reproducibility check for libarti_android.so"
+echo "### ABIs: $ABIS"
 echo "### Canonical build path: ${ARTI_REPRO_DIR:-/tmp/amethyst-arti-build}"
 echo
 
@@ -61,11 +71,17 @@ fi
 # Informational: is the binary committed in git already the reproducible one?
 echo
 echo "### vs. the committed binaries:"
-if git -C "$PROJECT_ROOT" diff --quiet -- amethyst/src/main/jniLibs/; then
+BUILT_PATHS=""
+for abi in $ABIS; do
+    BUILT_PATHS="$BUILT_PATHS amethyst/src/main/jniLibs/$abi/libarti_android.so"
+done
+
+# shellcheck disable=SC2086 # BUILT_PATHS is a deliberate multi-path list
+if git -C "$PROJECT_ROOT" diff --quiet -- $BUILT_PATHS; then
     echo "✓ The reproducible build matches what's committed — the shipped .so is verifiable as-is."
 else
     echo "⚠ The reproducible build differs from the committed .so (e.g. the committed one"
     echo "  predates this toolchain). Commit the rebuilt binaries so the shipped artifact"
     echo "  is itself a reproducible build:"
-    echo "      git -C \"$PROJECT_ROOT\" add amethyst/src/main/jniLibs && git commit"
+    echo "      git -C \"$PROJECT_ROOT\" add$BUILT_PATHS && git commit"
 fi