diff --git a/AGENTS.md b/AGENTS.md index cd026ad0..7ad17052 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -130,12 +130,23 @@ So releasing is two steps, in this order: **And the app's release goes out before the exporter's, whenever the exporter's changes what a bundle is.** They are separate releases with separate tags, which makes them look independent; -they are not. Exporter 1.4.0 locks bundles by default, and an app older than 1.1.0 cannot decrypt -one at all — it fails with a message about the zip rather than about a code. Publish the exporter -first 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. - -Nothing enforces this — `release_version.py` checks a tag against a version, not one release +they are not. A locked bundle cannot be opened by an app older than 1.1.0 at all — it fails with a +message about the zip rather than about a code. Publish an exporter that locks by default before +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. + +**So the wizard's lock is currently defaulted off**, in `wizard.py`'s `lock_bundle`, and +`test_wizard_bundle_locking.py` asserts that. Flip it in the same change that raises the minimum +app version, not before and not separately. + +**A gate on the format must not hold back unrelated fixes.** This nearly happened: the fix for +[#140](https://github.com/parawanderer/OpenTagViewer/issues/140) — where every share comes back +unreadable and the export fails outright — sat unreleased behind this ordering because it happened +to be on the same `main` as the locking change. Three people reported the same bug in the meantime. +If a fix does not touch what a bundle *is*, it is not what this rule is about, and shipping it +should not wait on an app release. + +Nothing enforces any of this — `release_version.py` checks a tag against a version, not one release against another — so it is a thing to remember, which is why it is written here. `scripts/release_version.py --kind exporter --tag ` enforces it, and runs in `test-release-version` diff --git a/python/exporter/wizard.py b/python/exporter/wizard.py index feaddea8..2d3221fc 100644 --- a/python/exporter/wizard.py +++ b/python/exporter/wizard.py @@ -245,16 +245,21 @@ def _build(self) -> None: self.confirm_button = ttk.Button(buttons, text="Export…", command=self._export, state="disabled") self.confirm_button.grid(row=0, column=4) - # **On by default, and the default is the whole point.** A bundle holds key material that - # cannot be revoked - the only way to withdraw an exported accessory is to unpair it - and - # it then travels through a mail account or a chat app and outlives the conversation by + # **Off until the app that can open one is released, then on.** A bundle holds key material + # that cannot be revoked - the only way to withdraw an exported accessory is to unpair it - + # and it then travels through a mail account or a chat app and outlives the conversation by # years, sitting in a backup long after everyone has forgotten it is there. Whoever most - # needs the lock is whoever would never go looking for a checkbox to turn it on. + # needs the lock is whoever would never go looking for a checkbox to turn it on, so on is + # where this belongs and where it is going. # - # The opt-out exists for one real case, the same one behind the CLI's --no-password: an - # app older than 1.1.0 cannot decrypt anything at all, so a recipient running one cannot - # open a locked bundle, and they did not choose the exporter's version. - self.lock_bundle = tk.BooleanVar(value=True) + # It is off today for one reason: **no released app can decrypt a locked bundle.** Anything + # older than 1.1.0 fails with a message about the zip rather than about a code, and the + # person who meets that failure is the recipient - who chose neither the exporter nor its + # version, and can do nothing about either. + # + # So the ordering is app first, exporter second - see AGENTS.md rule 9. Flip this to True in + # the same change that raises the minimum, not before, and not separately. + self.lock_bundle = tk.BooleanVar(value=False) ttk.Checkbutton( buttons, text="Lock with a code", diff --git a/python/test/test_wizard_locks_by_default.py b/python/test/test_wizard_bundle_locking.py similarity index 85% rename from python/test/test_wizard_locks_by_default.py rename to python/test/test_wizard_bundle_locking.py index c5dc491e..9ae92269 100644 --- a/python/test/test_wizard_locks_by_default.py +++ b/python/test/test_wizard_bundle_locking.py @@ -1,14 +1,16 @@ """ -The wizard locks the bundles it writes, and shows the code once. +The wizard can lock the bundles it writes, and shows the code once. -**This default was blocked, not missing.** ``wizard.py`` hard-coded ``password=None`` with a -comment saying so: before the Android app carried zip4j it could not decrypt anything at all, so -a locked bundle was a file nobody's installed app could open - and the people worst affected were -recipients, who did not choose the exporter's version and could not fix it from their side. +**The default is off, and it is off for a reason that will expire.** A locked bundle can only be +opened by app 1.1.0 or newer; anything older fails with a message about the zip rather than about +a code. The person who meets that failure is the recipient, who chose neither the exporter nor its +version - so until 1.1.0 is *released*, not merely built, locking by default would break exports +for the one party who can do nothing about it. -App 1.1.0 reads them. So the default flips, and these tests exist because nothing caught the old -behaviour either: no test asserted ``password=None``, so the flip would have gone unnoticed in -both directions. +**These tests assert the default in both directions on purpose.** Nothing caught the previous flip +in either direction, so the value has changed twice with no test noticing. When app 1.1.0 ships and +the default goes back on, `test_the_checkbox_starts_unticked` is the test that should fail and be +inverted - deliberately, in the same change, rather than discovered later. The code is the part with a permanent cost. It is not stored anywhere and cannot be recovered, so a bundle written without the user being shown its code is a bundle nobody can ever open. @@ -63,12 +65,17 @@ def write(window, bundle, path, *, locked: bool): class TestTheDefault: """ - A bundle holds key material that cannot be revoked, and travels through other people's - infrastructure. Whoever most needs the lock is whoever would never find a checkbox for it. + Off until an app that can open one is released - see the module docstring. + + A bundle holds key material that cannot be revoked and travels through other people's + infrastructure, so on is where this belongs eventually. It is not there yet. """ - def test_the_checkbox_starts_ticked(self, window): - assert window.lock_bundle.get() is True + def test_the_checkbox_starts_unticked(self, window): + assert window.lock_bundle.get() is False, ( + "locking by default writes bundles that no released app can open. Flip this only in" + " the change that raises the minimum app version - see AGENTS.md rule 9" + ) def test_a_bundle_is_written_with_a_code(self, window, bundle, tmp_path): write_zip, _shown, _info, _error, _closed = write(