From 9591d8074ca450509b44debb8d182b7bcd7dc4ae Mon Sep 17 00:00:00 2001 From: Torlando <281092095+torlando-agent[bot]@users.noreply.github.com> Date: Sat, 29 Aug 2026 19:17:08 +0000 Subject: [PATCH] fix: make install-marker renewal ownership-checked Greptile round 9 remediation: The round-8 heartbeat rewrote the marker unconditionally. If installer A stalled on a single tile operation for longer than the marker TTL, installer B could legitimately reclaim the expired marker, and A's next heartbeat then overwrote B's live claim -- stealing the card out from under a running install. The renewal now reads the marker before rewriting and only proceeds when it still carries OUR owner (the same owner comparison used by release and commit-time verification). A reclaimed, deleted, or corrupt marker means the claim is void: the install aborts with a clear "wait for the other installer to finish and retry" error, leaving the foreign claim intact. The partially published pack stays device-harmless (no record names it) and the user retries once the other installer finishes. - CLI: _renew_install_marker() checks owner before rewriting; PackError aborts the install (the existing cleanup paths remove the staged temp pack and release the marker as a no-op). - Flasher: renewInstallMarker() checks owner before rewriting; fail() aborts the install (the existing cleanup paths remove the owned pack and release the marker as a no-op). - Tests: CLI 88 (renewal refuses to steal a reclaimed/missing/ corrupt claim; same-owner renewal still advances the epoch); flasher 26 (same contract). --- docs/flasher/js/map-installer.js | 28 +++++++++++++------- tests/tools/test_build_map_pack.py | 41 +++++++++++++++++++++++++++++ tests/web/test_map_installer.mjs | 40 ++++++++++++++++++++++++++++ tools/maps/build_map_pack.py | 42 +++++++++++++++++++++--------- 4 files changed, 129 insertions(+), 22 deletions(-) diff --git a/docs/flasher/js/map-installer.js b/docs/flasher/js/map-installer.js index 46cd0b38..fc75ef2a 100644 --- a/docs/flasher/js/map-installer.js +++ b/docs/flasher/js/map-installer.js @@ -133,16 +133,26 @@ async function releaseInstallMarker(pyxis, token) { } // Heartbeat: renew our marker's epoch during long publications (a full -// world pack on slow SD storage can outlive the TTL). Best effort; the -// commit-time revalidation is the backstop. +// world pack on slow SD storage can outlive the TTL). +// +// The renewal is ownership-checked, never a blind overwrite: if the +// marker no longer carries OUR owner (it aged out and another producer +// legitimately reclaimed it, was deleted, or is corrupt), our claim is +// void and the install aborts. Overwriting a foreign claim would steal +// the card mid-install; the (possibly partially published) pack stays +// device-harmless and the user retries after the other installer +// finishes. export async function renewInstallMarker(pyxis, owner) { - try { - const token = `PYXI 1 ${owner} ${Date.now()}`; - const handle = await pyxis.getFileHandle(INSTALL_MARKER_NAME); - const writable = await handle.createWritable({keepExistingData: false}); - try { await writable.write(textEncoder.encode(token)); await writable.close(); } - catch (error) { try { await writable.abort(); } catch {} throw error; } - } catch {} + const current = await readInstallMarker(pyxis); + const parsed = current ? parseInstallMarker(current) : null; + if (!parsed || parsed.owner !== owner) { + fail('The map-install marker was reclaimed during installation; wait for the other installer to finish and retry'); + } + const token = `PYXI 1 ${owner} ${Date.now()}`; + const handle = await pyxis.getFileHandle(INSTALL_MARKER_NAME); + const writable = await handle.createWritable({keepExistingData: false}); + try { await writable.write(textEncoder.encode(token)); await writable.close(); } + catch (error) { try { await writable.abort(); } catch {} throw error; } } // Commit-time revalidation: the slot/style records were derived by diff --git a/tests/tools/test_build_map_pack.py b/tests/tools/test_build_map_pack.py index a1133be9..6b72a4d8 100644 --- a/tests/tools/test_build_map_pack.py +++ b/tests/tools/test_build_map_pack.py @@ -1362,3 +1362,44 @@ def test_long_publication_marker_stays_live_past_ttl( assert not (sd / "pyxis-map/.pyxis-installing").exists() # Sanity on the TTL constant the boundary above used. assert ttl_ms == 900 * 1000 + + +def test_marker_renewal_refuses_to_steal_reclaimed_claim( + tmp_path: Path) -> None: + # Round 10 (Greptile): a blind heartbeat would let a stalled installer + # steal back a marker that a second producer LEGITIMATELY reclaimed + # after our TTL expired. The renewal must be ownership-checked: + # foreign owner -> abort (PackError), the foreign claim is left + # intact for the other installer to finish. + tool = load_tool() + card = tmp_path / "sd" + card.mkdir() + pyxis = card / "pyxis-map" + pyxis.mkdir() + pyxis_fd = os.open(str(pyxis), os.O_RDONLY | os.O_DIRECTORY) + try: + # Producer B reclaims after our (producer A) marker aged out. + (pyxis / ".pyxis-installing").write_text( + f"PYXI 1 cli-other {int(__import__('time').time() * 1000)}\n") + # Renewing as a DIFFERENT owner must abort... + with pytest.raises(tool.PackError, match="reclaimed during installation"): + tool._renew_install_marker(pyxis_fd, "cli-us") + # ...and must leave B's claim untouched. + raw = (pyxis / ".pyxis-installing").read_text().strip().split(" ") + assert raw[2] == "cli-other" + # Renewing as the SAME owner is allowed and advances the epoch. + before = int(raw[3]) + tool._renew_install_marker(pyxis_fd, "cli-other") + after = (pyxis / ".pyxis-installing").read_text().strip().split(" ") + assert int(after[3]) >= before + # A missing or corrupt marker is also refused (claim void), never + # overwritten. + (pyxis / ".pyxis-installing").unlink() + with pytest.raises(tool.PackError, match="reclaimed during installation"): + tool._renew_install_marker(pyxis_fd, "cli-other") + assert not (pyxis / ".pyxis-installing").exists() + (pyxis / ".pyxis-installing").write_text("garbage\n") + with pytest.raises(tool.PackError, match="reclaimed during installation"): + tool._renew_install_marker(pyxis_fd, "cli-other") + finally: + os.close(pyxis_fd) diff --git a/tests/web/test_map_installer.mjs b/tests/web/test_map_installer.mjs index a1ff7e62..37593db3 100644 --- a/tests/web/test_map_installer.mjs +++ b/tests/web/test_map_installer.mjs @@ -859,3 +859,43 @@ test('renewing a live marker advances the epoch but keeps the owner', async () = assert.ok(parsed.epochMs >= Date.now() - 1000, 'renewal refreshes the epoch'); assert.ok(installMarkerIsFresh(parsed.epochMs), 'a renewed marker is live again'); }); + + +// --- Round 10 (Greptile): a heartbeat must never steal back a +// legitimately reclaimed marker. If our marker aged out and another +// producer reclaimed it, the renewal aborts and leaves the foreign +// claim intact; the install then fails and the user retries after the +// other installer finishes. +test('marker renewal refuses to steal a reclaimed claim', async () => { + const root = new MemoryDirectoryHandle('sd'); + const pyxis = await root.getDirectoryHandle('pyxis-map', {create: true}); + const writeMarker = async text => { + const writable = await (await pyxis.getFileHandle('.pyxis-installing', {create: true})).createWritable({keepExistingData: false}); + await writable.write(new TextEncoder().encode(text)); + await writable.close(); + }; + const readMarker = async () => { + let handle = null; try { handle = await pyxis.getFileHandle('.pyxis-installing'); } catch { return null; } + const file = await handle.getFile(); + return new TextDecoder('utf-8').decode(new Uint8Array(await file.arrayBuffer())); + }; + // Producer B reclaims after producer A's marker aged out. + await writeMarker(`PYXI 1 cli-other ${Date.now()}`); + // A's heartbeat must abort without touching B's claim. + await assert.rejects( + () => renewInstallMarker(pyxis, 'web-ours'), + error => error.message.includes('reclaimed during installation'), + ); + assert.equal(parseInstallMarker(await readMarker()).owner, 'cli-other'); + // Renewing as the same owner is fine. + await renewInstallMarker(pyxis, 'cli-other'); + const renewed = parseInstallMarker(await readMarker()); + assert.equal(renewed.owner, 'cli-other'); + assert.ok(installMarkerIsFresh(renewed.epochMs)); + // Missing or corrupt marker: refused, never overwritten. + await pyxis.removeEntry('.pyxis-installing'); + await assert.rejects(() => renewInstallMarker(pyxis, 'cli-other')); + assert.equal(await readMarker(), null); + await writeMarker('garbage'); + await assert.rejects(() => renewInstallMarker(pyxis, 'cli-other')); +}); diff --git a/tools/maps/build_map_pack.py b/tools/maps/build_map_pack.py index e40cf6d9..16f2d165 100644 --- a/tools/maps/build_map_pack.py +++ b/tools/maps/build_map_pack.py @@ -1443,21 +1443,37 @@ def _renew_install_marker(pyxis_fd: int, owner: str) -> None: and a second producer could reclaim our live claim. The rewrite keeps our owner identity and advances the epoch; the release path and the commit-time check compare the owner, so renewed markers are - still recognized as ours. Best effort: a failed renewal is retried - on the next tile and the commit-time marker check is the backstop. + still recognized as ours. + + The renewal is ownership-checked, never a blind overwrite: if the + marker no longer carries OUR owner -- it aged out and another + producer legitimately reclaimed it, was deleted, or is corrupt -- + our claim is void. Overwriting a foreign claim would steal the + card mid-install, so the install aborts instead; the (possibly + partially published) pack stays device-harmless and the user + retries after the other installer finishes. A transient I/O error + on the marker read/write also aborts: an unverifiable claim is no + safer than a lost one. """ + current = _read_selection_at(pyxis_fd, _INSTALL_MARKER_NAME) + if current is None: + raise PackError("the map-install marker was reclaimed during " + "installation; wait for the other installer to " + "finish and retry") + parsed = _parse_marker(current) + if parsed is None or parsed[0] != owner: + raise PackError("the map-install marker was reclaimed during " + "installation; wait for the other installer to " + "finish and retry") + token = _marker_token(owner) + descriptor = os.open(_INSTALL_MARKER_NAME, + os.O_WRONLY | os.O_TRUNC | os.O_CLOEXEC, + dir_fd=pyxis_fd) try: - token = _marker_token(owner) - descriptor = os.open(_INSTALL_MARKER_NAME, - os.O_WRONLY | os.O_TRUNC | os.O_CLOEXEC, - dir_fd=pyxis_fd) - try: - _write_all(descriptor, token.encode("ascii"), _INSTALL_MARKER_NAME) - os.fsync(descriptor) - finally: - os.close(descriptor) - except OSError: - pass + _write_all(descriptor, token.encode("ascii"), _INSTALL_MARKER_NAME) + os.fsync(descriptor) + finally: + os.close(descriptor) def _release_install_marker(pyxis_fd: int, token: str) -> None: