Make a secret file usable on Windows, and stop two tests passing for the wrong reason

docs/android-import-handover.md

@@ -129,11 +129,19 @@ Today's importer skips the unknown directory silently. What it needs:
 
 - The new directory in `AppleZipImporterUtil.MATCHERS`. Note the identifier is **not** a UUID -
   these come from files other tools wrote - so the existing v4-UUID regex does not apply.
-- Storage for a key-list accessory. `OwnedBeacon.content` assumes a plist; `accessoryJson` is
-  already FindMy.py's own JSON and is the natural home, but the fetch path calls
-  `FindMyAccessory.from_json` specifically and a custom tag is a `FixedRollingKeyPairAccessory`.
-- **A Room migration** if the schema moves - rule 1, and there is no going back from getting that
-  wrong.
+- ~~Storage for a key-list accessory... the fetch path calls `FindMyAccessory.from_json`
+  specifically.~~ **Done.** `main.accessoryFromJson` dispatches on FindMy.py's own `type` tag, and
+  everything else the fetch path touches - `keys_between`, `get_min_index`, `get_max_index`,
+  `update_alignment`, `to_json` - is on both classes already, so nothing downstream learns which
+  kind it has.
+- ~~**A Room migration** if the schema moves.~~ **It does not move.** `OwnedBeacons.content` is
+  already nullable in schema 3, so a key-list tag is a row with `accessory_json` set and `content`
+  NULL. No new column, no migration, and rule 1 does not come into it.
+
+  The risk moved rather than vanished: the read paths must tolerate a NULL plist.
+  `BeaconRepository` already does, but `BeaconDataParser` reads
+  `getOwnedBeaconInfo().content` straight into an XML parse, and a custom tag has no naming record
+  either - its display name is the `name` in its own mapping. That is where the work is.
 
 ## 5. The app becomes an exporter too — and then most people stop needing an export
 
@@ -267,53 +275,57 @@ support thread.
 
 ---
 
-## 6. Two smaller things
-
-**The FindMy pin is out of step with itself.** `app/build.gradle.kts` installs the
-`feat/icloud-keychain-export` branch, while `app/src/test/python/requirements.txt` pins
-`FindMy==0.9.8`. Whatever `test_pinned_versions_match_the_app_build` is checking, it is not those
-two agreeing - so the bridge tests exercise a different library than the app ships. The exporter
-solved the same problem with a lockfile that records the resolved commit; the app still needs
-`@<sha>` appended before any build that leaves the machine, which its own comment already says.
-
-**The app registers as `0FINDMYPY001`.** Nothing passes a serial to FindMy.py, so the device the
-user sees in their Apple account list is named after the library rather than after this app -
-which is what [rule 11](../AGENTS.md) exists to prevent, and not what the docs describe. FindMy.py
-now takes `serial=` on the Anisette provider and defaults CloudKit to it, so this is a one-line
-fix at the point the provider is built, plus a decision that `0PENTAGVIEWR` is the app's. The
-exporter presents `0PENTAGXPORT`, deliberately different: two installs, two entries, each
-removable without breaking the other.
-
-**And the entry has no name, which is the field a person reads first.** Nothing calls the
-`postdata` announce of
-[Stage 2 §7](./findmy-export/02-mobileme-delegate.md#7-naming-the-registered-device), so Apple
-names the row after whatever model the client claims - `MacBookPro18,3` shows up as a bare
-**"MacBookPro"**, sitting in a list of the user's real hardware with nothing to tell it apart.
-FindMy.py cannot do this at all today; the ask is written up in
-[findmy-py-device-name-request.md](./findmy-py-device-name-request.md). Target is
-**`OpenTagViewer App`**, with the exporter as `OpenTagViewer Exporter`.
-
-**Once name and serial are set, the model is only choosing an icon** - so the app should claim an
-iPhone and the desktop tool a Mac, which makes a device list readable at a glance. Two conditions
-on that:
-
-- **Do not switch the model on its own.** Today the app sends serial `"0"` and no name, so an
-  iPhone claim would produce an entry called "iPhone" with no serial, among the user's real
-  iPhones. Strictly worse than the Mac claim it has now. Model, name and serial land together or
-  not at all.
-- **All five strings move together.** Model, OS version, build, CFNetwork and Darwin describe one
-  real release - see [rule 11](../AGENTS.md) and
-  [Stage 1 §2.2](./findmy-export/01-authentication.md), which carries a worked set:
-  `iPhone15,2`, iOS `17.4`, build `21E219`, CFNetwork `1494.0.7`, Darwin `23.4.0`.
+## 6. Two smaller things — **mostly done**
+
+**~~The FindMy pin is out of step with itself.~~** Done, and then done again more widely. The app
+pins a commit in both `app/build.gradle.kts` and `app/src/test/python/requirements.txt`; the
+exporter, which tracked the branch and so drifted every time anybody pushed, now pins the same
+commit in `python/pyproject.toml`. `test_main.py::test_the_whole_repository_pins_one_findmy`
+asserts all four agree and names the file that does not.
+
+The reason it is one pin and not two: **`opentagviewer_export` is shared code with two runtimes.**
+Chaquopy packages it into the APK, the exporter runs the same files from source, and one package
+behind two library versions breaks exactly one of its consumers on the first API difference.
+
+**~~The app registers as `0FINDMYPY001`.~~** Done. A new sign-in presents `0PENTAGVIEWR`, beside
+the exporter's `0PENTAGXPORT`. **New sessions only** - an account established before this keeps
+what it was bound to, because re-identifying an existing session costs that user a sign-in and
+leaves a second entry they never asked for. See `app/src/main/python/identity.py`.
+
+**The entry has no name, and it is not going to get one.** Naming the row needs the `postdata`
+announce of [Stage 2 §7](./findmy-export/02-mobileme-delegate.md#7-naming-the-registered-device),
+which authenticates with the `com.apple.gs.idms.hb` heartbeat token. That token arrives once, in
+the same set as the PET, and an account serialised before FindMy.py started keeping it has nothing
+to announce with - so for the installed base it does not work, and making it work means those
+users signing in again. **Not worth a re-login for a label**, and
+[findmy-py-device-name-request.md](./findmy-py-device-name-request.md) is therefore not being
+pursued for the app. The serial carries the recognisability instead.
+
+**~~Once name and serial are set, the model is only choosing an icon.~~** Done, with the icon
+being the whole point: `iPhone15,2` renders as **"iPhone 14 Pro"** rather than as a bare
+"MacBookPro" among the user's real Macs. Both conditions were met -
+
+- **It did not switch on its own.** The serial landed in the same release, so the entry is an
+  iPhone *with* `0PENTAGVIEWR` on it rather than an unnamed, unserialled phone among real ones.
+- **All five strings moved together**, from the worked set in
+  [Stage 1 §2.2](./findmy-export/01-authentication.md): `iPhone15,2`, iOS `17.4`, build `21E219`,
+  CFNetwork `1494.0.7`, Darwin `23.4.0`.
+
+**And only for fresh installs.** An install that already has an ADI identity keeps the Mac it was
+provisioned as, forever - the two are one identity and Apple binds a session to it. That is
+`AdiDeviceIdentity.Hardware`, whose two profiles Python reads across the bridge rather than
+copying. [Rule 11](../AGENTS.md) has the longer version, including the trap that cost most of the
+time: FindMy.py transforms `X-Apple-I-MD-LU` and `X-Mme-Device-Id` on the way out and the Java ADI
+path does not, so passing "the same string" aligns one field and silently leaves the other as two.
 
 > **[observed] Claiming to be a phone does not make it a second factor.** A registered entry
 > presenting as an iPhone still reports *"This device cannot be used to receive Apple Account
 > verification codes"*. What decides that is omitting `ptkn`, exactly as
 > [Stage 1 §13](./findmy-export/01-authentication.md) argued - so the icon change carries no risk
 > of turning this app into a second factor for someone's Apple ID.
 
-`AdiDeviceIdentity.CLIENT_INFO`'s comment used to justify its value as the least remarkable string
-Apple sees. That reasoning is gone: an entry built to be recognised is not blending in.
+`AdiDeviceIdentity.CLIENT_INFO` no longer exists; a compile-time constant could only ever be one
+thing for everyone, which is precisely what a per-install profile cannot be.
 
 **Build the disclosure text from the account, not from constants.** §7.1 of Stage 2 requires that
 the identifiers shown to the user are the ones actually sent - the whole point being that somebody

python/exporter/device.py

@@ -103,6 +103,11 @@ def save(uid: str, devid: str, anisette: Any, path: Path | None = None) -> None:
         path.write_text(json.dumps(document, indent=2), encoding="utf-8")
         # Readable by its owner alone. It is not secret, but it names this installation to Apple
         # and there is no reason for anybody else on the machine to have it.
+        #
+        # **A no-op on Windows**, where chmod only toggles the read-only attribute and the group
+        # and other bits cannot be cleared at all. Left in rather than made conditional: it is
+        # correct where it works, harmless where it does not, and the alternative - real ACLs -
+        # needs icacls or pywin32. Worth knowing that this file is not protected there.
         path.chmod(0o600)
     except OSError as e:
         logger.warning("Could not store the device identity at %s: %s", path, e)

python/exporter/secrets.py

@@ -77,7 +77,18 @@ def _from_file(path: Path) -> str:
     # **Refused rather than warned about.** A warning on a file anybody can read is advice nobody
     # acts on, and the whole reason to prefer a file over an environment variable is that a file
     # can have permissions. One that does not is worse than the option it was chosen over.
-    if mode & (stat.S_IRGRP | stat.S_IROTH):
+    #
+    # **Except on Windows, where the bits carry no information.** `st_mode` there is synthesised
+    # by CPython - every regular file reports 0o666, and `os.chmod(path, 0o600)` does not move
+    # the group and other bits at all. So this test was true of every file on the system: it
+    # refused all of them, and told the user to run `chmod 600`, which cannot change the answer.
+    # Reading a secret from a file was simply impossible on a platform this ships a binary for.
+    #
+    # Skipped rather than approximated. Real permissions on Windows are ACLs, which `stat` cannot
+    # see, and checking them needs `icacls` or pywin32 - a large dependency for a guard that is
+    # advisory even on POSIX. Leaving it in place was not the safer option; it was the one that
+    # made the safest input route unusable and pushed people to the environment variable instead.
+    if os.name != "nt" and mode & (stat.S_IRGRP | stat.S_IROTH):
         msg = (
             f"{path} is readable by other users on this machine, so it is not a safe place for a"
             f" secret. Run: chmod 600 {path}"

python/test/test_device.py

@@ -14,6 +14,7 @@
 import pytest
 
 from exporter import device
+from test.unittestutils import skip_unless_posix_permissions
 
 ANISETTE = {"type": "aniLocal", "prov_data": "AAAA", "serial": "0PENTAGXPORT"}
 
@@ -39,6 +40,7 @@ def test_forgetting_it_leaves_nothing(self, identity):
         assert device.load(identity) is None
         assert device.forget(identity) is False
 
+    @skip_unless_posix_permissions
     def test_it_is_readable_by_its_owner_alone(self, identity):
         device.save("uid-1", "devid-1", ANISETTE, identity)
 

python/test/test_scriptable.py

@@ -16,10 +16,12 @@
 from __future__ import annotations
 
 import asyncio
+import re
 
 import pytest
 
 from exporter import cli, prompts, secrets
+from test.unittestutils import skip_unless_posix_permissions as POSIX_PERMISSIONS
 
 
 @pytest.fixture(autouse=True)
@@ -80,6 +82,7 @@ def test_leading_and_trailing_spaces_are_kept(self, secret_file):
     def test_a_second_line_is_not_part_of_the_secret(self, secret_file):
         assert secrets.read(secret_file("hunter2\n# a note\n"), "UNSET_VAR") == "hunter2"
 
+    @POSIX_PERMISSIONS
     def test_a_world_readable_file_is_refused(self, secret_file):
         # Refused rather than warned about. The reason to prefer a file over an environment
         # variable is that a file can have permissions; one that does not is worse than what it
@@ -89,16 +92,20 @@ def test_a_world_readable_file_is_refused(self, secret_file):
         with pytest.raises(secrets.SecretError, match="readable by other users"):
             secrets.read(path, "UNSET_VAR")
 
+    @POSIX_PERMISSIONS
     def test_a_group_readable_file_is_refused_too(self, secret_file):
         path = secret_file("hunter2\n", mode=0o640)
 
         with pytest.raises(secrets.SecretError, match="readable by other users"):
             secrets.read(path, "UNSET_VAR")
 
+    @POSIX_PERMISSIONS
     def test_the_advice_names_the_fix(self, secret_file):
         path = secret_file("hunter2\n", mode=0o644)
 
-        with pytest.raises(secrets.SecretError, match=f"chmod 600 {path}"):
+        # re.escape because `match` is a regex and a Windows path is full of backslashes -
+        # the second, unrelated reason this one failed there.
+        with pytest.raises(secrets.SecretError, match=re.escape(f"chmod 600 {path}")):
             secrets.read(path, "UNSET_VAR")
 
     def test_an_empty_file_is_an_error_rather_than_an_empty_password(self, secret_file):

python/test/unittestutils.py

@@ -20,3 +20,14 @@
     SYSTEM != DARWIN and SYSTEM != LINUX,
     reason="Requires Unix-like OS"
 )
+
+skip_unless_posix_permissions = pytest.mark.skipif(
+    os.name == "nt",
+    reason=(
+        "Windows has no POSIX mode bits to assert on. CPython synthesises st_mode as 0o666 for "
+        "every regular file and os.chmod cannot clear the group and other bits, so a test of "
+        "who can read a file proves nothing there - two of these used to pass for the wrong "
+        "reason, because the guard they check refused every file on the system. Real "
+        "permissions on Windows are ACLs, which stat cannot see."
+    )
+)