diff --git a/docs/flasher/js/map-installer.js b/docs/flasher/js/map-installer.js index 519eed1f..efae25ca 100644 --- a/docs/flasher/js/map-installer.js +++ b/docs/flasher/js/map-installer.js @@ -100,13 +100,22 @@ async function readInstallMarker(pyxis, name = INSTALL_MARKER_NAME) { catch (error) { if (error?.name === 'NotFoundError') return null; throw error; } return new TextDecoder('utf-8').decode(new Uint8Array(await (await handle.getFile()).arrayBuffer())); } +export {readInstallMarker}; // Respect the other producers' marker files. A fresh cross marker (the // CLI builder's, or the legacy shared one) means a live cross-producer // installer: refuse. A stale or malformed one is an abandoned install; // reclaim it only when reclaim is true (at acquire). Fresh cross // markers are never touched. -async function checkCrossInstallers(pyxis, reclaim) { +// +// Reclaim is conditional: the file is re-read immediately before +// deletion and a token that became fresh after the original check (the +// owner renewed while we were deciding) aborts the reclaim instead of +// deleting a live claim. A sub-millisecond renewal can still land +// between that final read and the removeEntry; the owner's commit-time +// verification requires its own marker, so such an overlap aborts on +// its side instead of proceeding. +export async function checkCrossInstallers(pyxis, reclaim) { for (const name of CROSS_INSTALL_MARKERS) { const raw = await readInstallMarker(pyxis, name); if (raw === null) continue; @@ -114,7 +123,14 @@ async function checkCrossInstallers(pyxis, reclaim) { if (parsed && installMarkerIsFresh(parsed.epochMs)) { fail('Another map installer is already running on this card; wait for it to finish and retry'); } - if (reclaim) { try { await pyxis.removeEntry(name); } catch {} } + if (reclaim) { + const confirm = await readInstallMarker(pyxis, name); + const confirmParsed = confirm !== null ? parseInstallMarker(confirm) : null; + if (confirmParsed && installMarkerIsFresh(confirmParsed.epochMs)) { + fail('Another map installer is already running on this card; wait for it to finish and retry'); + } + try { await pyxis.removeEntry(name); } catch {} + } } } @@ -201,16 +217,29 @@ export async function renewInstallMarker(pyxis, owner) { // already-published pack is kept; a retry re-derives from the new state and // converges (a pack no record names is device-harmless: the firmware only // reads packs enumerated in the active selection). -async function verifyActivationState(pyxis, mapSets, styleName, markerToken, state) { +export async function verifyActivationState(pyxis, mapSets, styleName, markerToken, state) { // Cross producers: a fresh marker in the CLI/legacy files means the // other installer started after we acquired. await checkCrossInstallers(pyxis, false); const marker = await readInstallMarker(pyxis); - if (marker !== null) { - // Compare by owner: our own marker may carry a renewed (newer) epoch. + if (markerToken !== null) { + // Our own marker is REQUIRED: a missing file means the claim was + // reclaimed from under us (a cross-reclaimer raced our last + // renewal) and the activation would proceed alongside the new + // holder. Abort instead. + const ours = parseInstallMarker(markerToken); + if (marker === null) { + fail('The install claim was reclaimed during installation; wait for the other installer to finish and retry'); + } const parsed = parseInstallMarker(marker); - const ours = markerToken !== null ? parseInstallMarker(markerToken) : null; - if (parsed && installMarkerIsFresh(parsed.epochMs) && (ours === null || parsed.owner !== ours.owner)) { + if (!parsed || parsed.owner !== ours.owner) { + fail('The map-install marker was reclaimed during installation; wait for the other installer to finish and retry'); + } + } else if (marker !== null) { + // No token was acquired (resume path without a marker): only a + // FRESH marker in our own file is a live foreign claim. + const parsed = parseInstallMarker(marker); + if (parsed && installMarkerIsFresh(parsed.epochMs)) { fail('Another map installer is running on this card; wait for it to finish and retry'); } } diff --git a/tests/tools/test_build_map_pack.py b/tests/tools/test_build_map_pack.py index c319dea0..272374b4 100644 --- a/tests/tools/test_build_map_pack.py +++ b/tests/tools/test_build_map_pack.py @@ -10,6 +10,7 @@ from pathlib import Path import struct import subprocess import sys +import time import zlib import pytest @@ -1480,3 +1481,82 @@ def test_fresh_cross_marker_during_install_aborts_at_renewal( assert "web-racer" in marker_path.read_text() # Our own claim is released even on the abort. assert not (sd / "pyxis-map/.pyxis-installing-cli").exists() + + +def test_cross_reclaim_refuses_when_owner_renews_between_check_and_delete( + tmp_path: Path) -> None: + # Round 12 (Greptile): reclamation is check-then-act. If the + # cross-installer renews while we are deciding, the delete would + # clobber a live claim. The conditional re-read must see the fresh + # token and abort instead of deleting. + tool = load_tool() + sd = tmp_path / "sd" + sd.mkdir() + (sd / "pyxis-map").mkdir() + marker = sd / "pyxis-map/.pyxis-installing-web" + marker.write_text(f"PYXI 1 web-owner {int(time.time() * 1000) - 16 * 60 * 1000}\n") + + real_read = tool._read_selection_at + + def racing_read(parent_fd, name, _calls=[0]): + if name == ".pyxis-installing-web": + _calls[0] += 1 + if _calls[0] == 2: + marker.write_text(f"PYXI 1 web-owner {int(time.time() * 1000)}\n") + return real_read(parent_fd, name) + + tool._read_selection_at = racing_read + try: + pyxis_fd = os.open(str(sd / "pyxis-map"), os.O_RDONLY | os.O_DIRECTORY) + except OSError: + pyxis_fd = os.open(str(sd / "pyxis-map"), os.O_RDONLY) + try: + with pytest.raises(tool.PackError, match="another map installer is already running"): + tool._check_cross_installers(pyxis_fd, reclaim=True) + assert marker.exists() + assert tool._marker_is_fresh( + tool._parse_marker(marker.read_bytes())[1]) + finally: + os.close(pyxis_fd) + tool._read_selection_at = real_read + + +def test_commit_abort_when_own_marker_was_reclaimed( + tmp_path: Path) -> None: + # Round 12 (Greptile): a missing own marker at commit time means our + # claim was reclaimed from under us. Verification must abort, not + # proceed with the activation writes. + tool = load_tool() + sd = tmp_path / "sd" + sd.mkdir() + (sd / "pyxis-map").mkdir() + (sd / "pyxis-map/map-sets").mkdir() + token = "PYXI 1 cli-self 12345" + pyxis_fd = os.open(str(sd / "pyxis-map"), os.O_RDONLY | os.O_DIRECTORY) + map_sets_fd = os.open(str(sd / "pyxis-map/map-sets"), os.O_RDONLY | os.O_DIRECTORY) + try: + # No marker file exists at all: the claim was reclaimed. + with pytest.raises(tool.PackError, match="reclaimed during installation"): + tool._verify_activation_state_at( + pyxis_fd, map_sets_fd, + slot_raw=[None, None], style_raw=None, + style_name="osm-bright.pmas", marker_token=token) + # A foreign owner in our own file: also an abort. + (sd / "pyxis-map/.pyxis-installing-cli").write_text(token.replace("cli-self", "cli-other") + "\n") + with pytest.raises(tool.PackError, match="reclaimed during installation"): + tool._verify_activation_state_at( + pyxis_fd, map_sets_fd, + slot_raw=[None, None], style_raw=None, + style_name="osm-bright.pmas", marker_token=token) + # Our own marker (any epoch: renewals advance it): passes the + # marker check; the slot/style re-reads against identical state + # still pass. + (sd / "pyxis-map/.pyxis-installing-cli").write_text( + f"PYXI 1 cli-self {int(time.time() * 1000)}\n") + tool._verify_activation_state_at( + pyxis_fd, map_sets_fd, + slot_raw=[None, None], style_raw=None, + style_name="osm-bright.pmas", marker_token=token) + finally: + os.close(pyxis_fd) + os.close(map_sets_fd) diff --git a/tests/web/test_map_installer.mjs b/tests/web/test_map_installer.mjs index 46fa334e..cfede97d 100644 --- a/tests/web/test_map_installer.mjs +++ b/tests/web/test_map_installer.mjs @@ -939,3 +939,76 @@ test('a fresh cross marker during installation aborts at renewal', async () => { assert.equal(pyxis.children.has('.pyxis-installing-cli'), true); assert.equal(pyxis.children.has(MARKER), false, 'our marker released on abort'); }); + + +// --- Round 12 (Greptile): the cross-marker reclaim is a check-then-act, +// and the owner can renew in the window. Reclaim must therefore be +// conditional: a token that became fresh between the original check and +// the deletion aborts the reclaim (the claim is live again), and the +// owner's commit-time verification must REQUIRE its own marker -- a +// missing claim is an abort, not a pass. + +import {checkCrossInstallers, readInstallMarker, verifyActivationState} from '../../docs/flasher/js/map-installer.js'; + +test('reclaim aborts when the owner renews between check and delete', async () => { + const root = new MemoryDirectoryHandle('sd'); + const pyxis = await root.getDirectoryHandle('pyxis-map', {create: true}); + const staleToken = `PYXI 1 cli-owner ${Date.now() - 16 * 60 * 1000}`; + const freshToken = `PYXI 1 cli-owner ${Date.now()}`; + const writeMarker = async text => { + // Overwrite semantics (the seam simulates the owner's renewal + // rewrite): no exclusive, since the file already holds the stale + // token. + const handle = await pyxis.getFileHandle('.pyxis-installing-cli', {create: true}); + const writable = await handle.createWritable({keepExistingData: false}); + await writable.write(new TextEncoder().encode(text)); + await writable.close(); + }; + await writeMarker(staleToken); + // Deterministic seam: each marker read goes through + // pyxis.getFileHandle. The first call is the original check (stale + // token); the owner's renewal lands before the second call (the + // conditional re-read), which must see the fresh token and abort + // instead of deleting. + let readCount = 0; + const realGet = pyxis.getFileHandle.bind(pyxis); + pyxis.getFileHandle = async (name, options) => { + // Count plain reads only: the marker writes (create:true) are the + // test's own seam writes, not reclaim reads. + if (name === '.pyxis-installing-cli' && !options?.create) { + readCount += 1; + if (readCount === 2) await writeMarker(freshToken); + } + return realGet(name, options); + }; + const realRemove = pyxis.removeEntry.bind(pyxis); + let removed = false; + pyxis.removeEntry = async name => { removed = true; return realRemove(name); }; + try { + await assert.rejects(checkCrossInstallers(pyxis, true), MapInstallerError, 'renewal must abort the reclaim'); + assert.equal(removed, false, 'the renewed live claim must not be deleted'); + const surviving = await readInstallMarker(pyxis, '.pyxis-installing-cli'); + assert.equal(surviving, freshToken, 'the renewed token survives the reclaim attempt'); + } finally { + pyxis.removeEntry = realRemove; + pyxis.getFileHandle = realGet; + } +}); + +test('commit-time verification aborts when the own marker is missing', async () => { + const root = new MemoryDirectoryHandle('sd'); + const pyxis = await root.getDirectoryHandle('pyxis-map', {create: true}); + const mapSets = await pyxis.getDirectoryHandle('map-sets', {create: true}); + const owner = `web-${Date.now().toString(16)}`; + const token = `PYXI 1 ${owner} ${Date.now()}`; + // The claim existed at acquire time but a cross-reclaimer deleted it + // after our last renewal: verify must abort, not proceed. + const handle = await pyxis.getFileHandle(MARKER, {create: true, exclusive: true}); + const writable = await handle.createWritable({keepExistingData: false}); + await writable.write(new TextEncoder().encode(token)); + await writable.close(); + await pyxis.removeEntry(MARKER); + await assert.rejects( + verifyActivationState(pyxis, mapSets, 'osm-bright.pmas', token, {slotBytes: [null, null], styleBytes: null}), + MapInstallerError, 'a reclaimed claim must abort the commit'); +}); diff --git a/tools/maps/build_map_pack.py b/tools/maps/build_map_pack.py index c6c6e887..cd381aca 100644 --- a/tools/maps/build_map_pack.py +++ b/tools/maps/build_map_pack.py @@ -1410,6 +1410,14 @@ def _check_cross_installers(pyxis_fd: int, *, reclaim: bool) -> None: malformed one is an abandoned install; reclaim it only when the caller says so (at acquire, where claiming is the intent). Fresh foreign markers are never touched. + + Reclaim is conditional: the file is re-read immediately before + deletion and a token that became fresh after the original check + (the owner renewed while we were deciding) aborts the reclaim + instead of deleting a live claim. A sub-microsecond renewal can + still land between that final read and the unlink; the owner's + commit-time verification requires its own marker, so such an + overlap aborts on its side instead of proceeding. """ for name in _CROSS_INSTALL_MARKERS: raw = _read_selection_at(pyxis_fd, name) @@ -1420,6 +1428,11 @@ def _check_cross_installers(pyxis_fd: int, *, reclaim: bool) -> None: raise PackError("another map installer is already running on " "this card; wait for it to finish and retry") if reclaim: + confirm = _read_selection_at(pyxis_fd, name) + confirm_parsed = _parse_marker(confirm) if confirm is not None else None + if confirm_parsed is not None and _marker_is_fresh(confirm_parsed[1]): + raise PackError("another map installer is already running on " + "this card; wait for it to finish and retry") _unlink_quietly_at(pyxis_fd, name) @@ -1563,22 +1576,34 @@ def _verify_activation_state_at(pyxis_fd: int, map_sets_fd: int, *, is kept; a retry re-derives from the new state and converges (the pack is device-harmless while it is not named by any record: the firmware only reads packs enumerated in the active selection). + + The own marker is REQUIRED when this install acquired one: a missing + own file means the claim was reclaimed from under us (a stale + cross-reclaimer raced our last renewal) and the activation would + proceed alongside the new holder. Abort instead. """ # Cross producers: a fresh marker in the web/legacy files means the # other installer started (or is mid-install) after we acquired. _check_cross_installers(pyxis_fd, reclaim=False) marker_raw = _read_selection_at(pyxis_fd, _INSTALL_MARKER_NAME) - if marker_raw is not None: - # Compare by owner, not the full token: we renew the marker's - # epoch during publication (heartbeat), so our own marker may - # carry a newer epoch than the one acquired at install start. - own_owner = None - if marker_token is not None: - parsed_own = _parse_marker(marker_token.encode("ascii")) - own_owner = parsed_own[0] if parsed_own is not None else None + if marker_token is not None: + parsed_own = _parse_marker(marker_token.encode("ascii")) + own_owner = parsed_own[0] if parsed_own is not None else None + if marker_raw is None: + raise PackError("the install claim was reclaimed during " + "installation; wait for the other installer " + "to finish and retry") parsed = _parse_marker(marker_raw) - if parsed is not None and _marker_is_fresh(parsed[1]) \ - and parsed[0] != own_owner: + if parsed is None or parsed[0] != own_owner: + raise PackError("the map-install marker was reclaimed during " + "installation; wait for the other installer " + "to finish and retry") + elif marker_raw is not None: + # No token was acquired (resume path without a marker): only a + # FRESH marker in our own file is a live foreign claim; a + # stale one is an abandoned install we are resuming. + parsed = _parse_marker(marker_raw) + if parsed is not None and _marker_is_fresh(parsed[1]): raise PackError("another map installer is running on this card; " "wait for it to finish and retry") for index, slot_name in enumerate(("active-pack.0", "active-pack.1")):