Always lock wizard bundles, and let a dead CloudKit token reach the sign-in screen

.github/workflows/build-debug.yml

@@ -78,6 +78,32 @@ jobs:
         run: |
           echo "MAPS_API_KEY=${{ secrets.MAPS_API_KEY || 'maps_key_default_value' }}" > secrets.properties
 
+      # **So every debug APK this repository publishes is signed by the same key.**
+      #
+      # A runner has no ~/.android/debug.keystore, so AGP generates one per run. That makes each
+      # debug APK unupgradeable over the last (INSTALL_FAILED_UPDATE_INCOMPATIBLE, and with
+      # allowBackup false an uninstall destroys the tester's imported beacons), and it makes the
+      # Maps SHA-1 restriction impossible to satisfy, which is why maps rendered blank here and
+      # not locally.
+      #
+      # Absent secret is not fatal: a fork without it falls back to the generated key and still
+      # builds, which is the behaviour a fork wants.
+      #
+      # Passed through `env` rather than interpolated into the script: the `secrets` context is
+      # not available in a step-level `if` at all, and a `${{ }}` inside a run block puts the
+      # value on the command line, where a shell trace or an injected newline can expose it.
+      - name: Write the shared debug keystore
+        env:
+          DEBUG_KEYSTORE_BASE64: ${{ secrets.DEBUG_KEYSTORE_BASE64 }}
+        run: |
+          if [ -z "$DEBUG_KEYSTORE_BASE64" ]; then
+            echo "No DEBUG_KEYSTORE_BASE64 secret; AGP will generate a debug key for this run."
+            echo "APKs from this run cannot be installed over ones from another."
+            exit 0
+          fi
+          printf '%s' "$DEBUG_KEYSTORE_BASE64" | base64 -d > app/debug-keystore.jks
+          echo "Debug keystore written, $(wc -c < app/debug-keystore.jks) bytes"
+
       # KVM is required for a hardware-accelerated emulator; without it the run times out.
       - name: Enable KVM
         run: |
@@ -197,6 +223,32 @@ jobs:
         run: |
           echo "MAPS_API_KEY=${{ secrets.MAPS_API_KEY || 'maps_key_default_value' }}" > secrets.properties
 
+      # **So every debug APK this repository publishes is signed by the same key.**
+      #
+      # A runner has no ~/.android/debug.keystore, so AGP generates one per run. That makes each
+      # debug APK unupgradeable over the last (INSTALL_FAILED_UPDATE_INCOMPATIBLE, and with
+      # allowBackup false an uninstall destroys the tester's imported beacons), and it makes the
+      # Maps SHA-1 restriction impossible to satisfy, which is why maps rendered blank here and
+      # not locally.
+      #
+      # Absent secret is not fatal: a fork without it falls back to the generated key and still
+      # builds, which is the behaviour a fork wants.
+      #
+      # Passed through `env` rather than interpolated into the script: the `secrets` context is
+      # not available in a step-level `if` at all, and a `${{ }}` inside a run block puts the
+      # value on the command line, where a shell trace or an injected newline can expose it.
+      - name: Write the shared debug keystore
+        env:
+          DEBUG_KEYSTORE_BASE64: ${{ secrets.DEBUG_KEYSTORE_BASE64 }}
+        run: |
+          if [ -z "$DEBUG_KEYSTORE_BASE64" ]; then
+            echo "No DEBUG_KEYSTORE_BASE64 secret; AGP will generate a debug key for this run."
+            echo "APKs from this run cannot be installed over ones from another."
+            exit 0
+          fi
+          printf '%s' "$DEBUG_KEYSTORE_BASE64" | base64 -d > app/debug-keystore.jks
+          echo "Debug keystore written, $(wc -c < app/debug-keystore.jks) bytes"
+
       # No stub wheel to validate any more: it is generated from app/stubs/unicorn/ by
       # generateUnicornStubWheel during the build, rather than being checked in.
 

AGENTS.md

@@ -163,8 +163,20 @@ message about the zip rather than about a code. Publish an exporter that locks b
 that app is out, and every bundle written that day is unopenable by whoever receives it, and the
 recipient is the one person in that transaction who chose none of it and can fix none of it.
 
-**The wizard's lock is defaulted on**, in `wizard.py`'s `lock_bundle`, and
-`test_wizard_bundle_locking.py` asserts that.
+**The wizard has no lock switch at all**, and that is stronger than a default. `_write_it` calls
+`generate_passcode()` unconditionally, so no path from that window produces an unlocked bundle;
+`test_wizard_bundle_locking.py` asserts the control's *absence*, by attribute and by walking the
+widgets, rather than asserting a value somebody can flip back.
+
+It was a ticked checkbox, and a ticked checkbox is one idle click from an unlocked zip holding
+key material that cannot be revoked. That click gets made: a user sent @parawanderer their tags
+in an unlocked bundle. **Do not restore it** — the person the lock protects is precisely the
+person who would untick it to make a message go away.
+
+The escape hatch is `exporter.cli`'s `--no-password`, and its being CLI-only is the design rather
+than an omission. Somebody who found a flag and typed it has chosen an unlocked bundle; somebody
+clicking through a window has not, and offering both the same control treats those as one
+decision.
 
 **It was flipped on before app 1.1.0 was published, which is not what the paragraph above
 describes, and the exception is worth understanding rather than copying.** The ordering exists to

CONTRIBUTING.md

@@ -115,6 +115,46 @@ than replacing it. **Never uninstall a production install to force an install**
 `allowBackup` is false, so the beacons and location history are gone for good, and getting
 them back means redoing the macOS export.
 
+### The shared debug keystore
+
+**Android signs debug builds with `~/.android/debug.keystore`, and a CI runner has no such file,
+so it generates one per run.** Two things follow, and both were met before this was fixed:
+
+- **A debug APK cannot be installed over one from a different machine or a different CI run.**
+  Same `applicationId`, different signing key, so the install fails with
+  `INSTALL_FAILED_UPDATE_INCOMPATIBLE`. The only way forward is an uninstall, and an uninstall
+  destroys that device's imported beacons and location history.
+- **Google Maps renders blank.** A Maps key is restricted by package name *and* signing SHA-1,
+  and a SHA-1 that changes every build cannot be whitelisted at all. It looks exactly like a
+  build with no API key in it.
+
+So CI writes one fixed keystore from the `DEBUG_KEYSTORE_BASE64` repository secret, and
+`app/build.gradle.kts` uses `app/debug-keystore.jks` when that file is present. Its SHA-1 is
+whitelisted against the Maps key, so **CI debug builds render maps and upgrade in place**.
+
+To make your local builds interchangeable with CI's, put the same keystore at
+`app/debug-keystore.jks`:
+
+```bash
+gh secret list -R parawanderer/OpenTagViewer     # confirms it exists; secrets cannot be read back
+# Ask a maintainer for the file, then:
+ls -l app/debug-keystore.jks
+keytool -list -v -keystore app/debug-keystore.jks -storepass android -alias androiddebugkey \
+  | grep SHA1
+```
+
+`*.jks` is gitignored, and the passwords are Android's well-known debug constants
+(`android` / `androiddebugkey`) deliberately: the key proves nothing and guards nothing, and
+giving it real secrets would only add something else to supply before the project builds.
+
+**Without the file, everything still builds** — Gradle logs a line saying so and falls back to
+the generated key. That is the right behaviour for a fork, and it is also why a missing keystore
+does not announce itself as an error when maps later come up blank.
+
+**Changing the debug key means one more uninstall, once.** Anything already installed was signed
+with the old per-run key, so the first build after this lands still refuses to install over it.
+After that they upgrade in place.
+
 ---
 
 ## Testing

app/build.gradle.kts

@@ -186,6 +186,46 @@ android {
             keyAlias = System.getenv("KEY_ALIAS")
             keyPassword = System.getenv("KEY_PASSWORD")
         }
+
+        // **A debug key that is the same key every time, when one is supplied.**
+        //
+        // Without this, AGP signs debug builds with `~/.android/debug.keystore`, which a fresh
+        // CI runner *generates on the spot*. Two consequences, both of which bit:
+        //
+        // 1. Every CI debug APK is signed by a different key, so installing a newer one over an
+        //    older one fails with INSTALL_FAILED_UPDATE_INCOMPATIBLE. The only way forward is to
+        //    uninstall, and `allowBackup` is false, so that permanently destroys the imported
+        //    beacons and location history on that device. Testing successive builds meant
+        //    wiping the app every time.
+        // 2. A Google Maps key is restricted by package name *and* signing SHA-1, so a key whose
+        //    SHA-1 changes per build cannot be whitelisted at all. Maps rendered blank in every
+        //    CI debug build, which reads as the API key being missing from the build.
+        //
+        // Supplied through the environment rather than committed: it is a low-value key, but a
+        // signing key in a public repository is a bad habit to start, and `.gitignore` covers
+        // the filename. CI writes it from the DEBUG_KEYSTORE_BASE64 secret; see
+        // CONTRIBUTING.md for using the same one locally, which is what makes a locally built
+        // APK and a CI one interchangeable on the same device.
+        //
+        // **Falls back to AGP's default when absent**, so a clone with no keystore still builds.
+        // The passwords are Android's well-known debug constants on purpose: this key proves
+        // nothing and guards nothing, and inventing secrets for it would only mean another thing
+        // that has to be supplied before the project compiles.
+        getByName("debug") {
+            val supplied = file(System.getenv("DEBUG_KEYSTORE_FILE") ?: "debug-keystore.jks")
+            if (supplied.exists()) {
+                storeFile = supplied
+                storePassword = "android"
+                keyAlias = "androiddebugkey"
+                keyPassword = "android"
+            } else {
+                logger.lifecycle(
+                    "No debug keystore at ${supplied.path}; using the default one. Debug APKs " +
+                        "from this build will not match CI's, so installing one over the other " +
+                        "needs an uninstall. See CONTRIBUTING.md."
+                )
+            }
+        }
     }
 
     buildTypes {

app/src/androidTest/java/dev/wander/android/opentagviewer/ui/compat/WindowPaddingUtilTest.java

@@ -0,0 +1,158 @@
+package dev.wander.android.opentagviewer.ui.compat;
+
+import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
+import static org.junit.Assert.assertEquals;
+
+import android.content.Context;
+import android.view.View;
+import android.widget.FrameLayout;
+
+import androidx.core.graphics.Insets;
+import androidx.core.view.ViewCompat;
+import androidx.core.view.WindowInsetsCompat;
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/**
+ * Where the system-bar insets land.
+ *
+ * <p><b>This is a screenshot bug that no screenshot test catches</b>, because the insets a test
+ * device reports are not the ones that break it: a gesture-navigation phone has a bottom inset of
+ * a few dp, and three-button navigation has around 48. So the values are dispatched here rather
+ * than waited for, which is also the only way to assert what happens on a *second* delivery.
+ *
+ * <p>Both failures being pinned have shipped. Padding applied twice grew the gap on every
+ * rotation; padding the root of a screen with a bottom sheet left the sheet stopping short of the
+ * screen edge, showing a band of the activity's background beneath it and clipping the last row
+ * of the list.
+ */
+@RunWith(AndroidJUnit4.class)
+public class WindowPaddingUtilTest {
+
+    private static final int STATUS_BAR = 60;
+    private static final int NAV_BAR = 48;
+
+    private Context context;
+
+    @Before
+    public void setUp() {
+        this.context = getInstrumentation().getTargetContext();
+    }
+
+    private static WindowInsetsCompat systemBars() {
+        return new WindowInsetsCompat.Builder()
+                .setInsets(
+                        WindowInsetsCompat.Type.systemBars(),
+                        Insets.of(0, STATUS_BAR, 0, NAV_BAR))
+                .build();
+    }
+
+    /** Insets arrive on their own schedule, so a test has to hand them over itself. */
+    private static void deliverInsetsTo(final View view) {
+        ViewCompat.dispatchApplyWindowInsets(view, systemBars());
+    }
+
+    @Test
+    public void awholeScreenIsKeptClearOfBothBars() {
+        final View screen = new FrameLayout(this.context);
+
+        WindowPaddingUtil.insetForSystemBars(screen);
+        deliverInsetsTo(screen);
+
+        assertEquals("the status bar would cover the heading", STATUS_BAR, screen.getPaddingTop());
+        assertEquals("a button here would be behind the navigation bar",
+                NAV_BAR, screen.getPaddingBottom());
+    }
+
+    /**
+     * <b>And the padding a layout already asked for is kept rather than replaced.</b>
+     */
+    @Test
+    public void bitaddsToThePaddingTheLayoutAlreadyHad() {
+        final View screen = new FrameLayout(this.context);
+        screen.setPadding(0, 7, 0, 11);
+
+        WindowPaddingUtil.insetForSystemBars(screen);
+        deliverInsetsTo(screen);
+
+        assertEquals(STATUS_BAR + 7, screen.getPaddingTop());
+        assertEquals(NAV_BAR + 11, screen.getPaddingBottom());
+    }
+
+    /**
+     * <b>Delivered twice, the gap does not double.</b>
+     *
+     * <p>Insets arrive more than once - a rotation, a keyboard, switching to three-button
+     * navigation - and reading the view's current padding inside the listener would add to a
+     * value that already includes the last delivery.
+     */
+    @Test
+    public void ctwodeliveriesDoNotStack() {
+        final View screen = new FrameLayout(this.context);
+        screen.setPadding(0, 7, 0, 11);
+
+        WindowPaddingUtil.insetForSystemBars(screen);
+        deliverInsetsTo(screen);
+        deliverInsetsTo(screen);
+
+        assertEquals(STATUS_BAR + 7, screen.getPaddingTop());
+        assertEquals(NAV_BAR + 11, screen.getPaddingBottom());
+    }
+
+    /**
+     * <b>The paired form puts the bottom on the content and leaves the root's alone.</b>
+     *
+     * <p>The history screen's sheet has to reach the bottom of the display. Padding its root
+     * instead shortened everything inside it, so the sheet stopped above the navigation bar with
+     * the activity's background showing beneath it - a white band under a grey sheet - and the
+     * last row of the list clipped by the same gap.
+     */
+    @Test
+    public void dthepairedFormGivesTheBottomToTheContent() {
+        final View root = new FrameLayout(this.context);
+        final View list = new FrameLayout(this.context);
+
+        WindowPaddingUtil.insetForSystemBars(root, list);
+        deliverInsetsTo(root);
+
+        assertEquals("the root still clears the status bar", STATUS_BAR, root.getPaddingTop());
+        assertEquals("the root must reach the bottom edge, or the sheet stops short",
+                0, root.getPaddingBottom());
+        assertEquals("the list has to clear the navigation bar itself",
+                NAV_BAR, list.getPaddingBottom());
+    }
+
+    /** The content's own padding survives, and the root's does too. */
+    @Test
+    public void ethepairedFormKeepsBothViewsOwnPadding() {
+        final View root = new FrameLayout(this.context);
+        final View list = new FrameLayout(this.context);
+        root.setPadding(0, 3, 0, 5);
+        list.setPadding(0, 0, 0, 9);
+
+        WindowPaddingUtil.insetForSystemBars(root, list);
+        deliverInsetsTo(root);
+
+        assertEquals(STATUS_BAR + 3, root.getPaddingTop());
+        assertEquals("the root's own bottom padding is not the navigation bar's, and stays",
+                5, root.getPaddingBottom());
+        assertEquals(NAV_BAR + 9, list.getPaddingBottom());
+    }
+
+    /** And it does not stack on a second delivery either. */
+    @Test
+    public void fthepairedFormDoesNotStackOnTheContent() {
+        final View root = new FrameLayout(this.context);
+        final View list = new FrameLayout(this.context);
+        list.setPadding(0, 0, 0, 9);
+
+        WindowPaddingUtil.insetForSystemBars(root, list);
+        deliverInsetsTo(root);
+        deliverInsetsTo(root);
+
+        assertEquals(NAV_BAR + 9, list.getPaddingBottom());
+    }
+}