mirror of
https://github.com/torlando-tech/pyxis.git
synced 2026-09-25 06:04:51 +00:00
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).
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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'));
|
||||
});
|
||||
|
||||
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user