Ryan Malloy
f053762c4e
The success paths were fine; several failure paths drew a confident conclusion without checking the thing they waited for. Registry (was: any parse error killed the whole server, since load() runs in the lifespan): - quarantine an unreadable file and start empty instead of raising, so the tools that stop runaway VMs keep working when bookkeeping is damaged - skip malformed or invalidly-named records rather than failing the load; report both through list_vms as registry_warnings - read-modify-write under an exclusive flock so a second instance merges instead of clobbering, with a PID-unique temp file - drop the lifespan shutdown save, which could resurrect deleted records - version the schema and round-trip unknown record fields sandbox_destroy (the only tool that deletes files): - verify the process actually died, escalating to SIGKILL, and refuse to delete an overlay QEMU still holds open - assert the target is inside the VM state tree before rmtree - report cleanup errors instead of swallowing them; destroyed now reflects what happened Launch races: - reserve the name before the first await so two concurrent launches cannot race over one set of sockets - refuse to unlink a QMP socket that is still accepting connections - register the VM with a warning rather than orphaning it when the pidfile is unreadable but QEMU is up Guest agent and QMP: - bound every guest-agent call, not just the handshake; cap max_bytes and stop guest_file_read spinning on a zero-progress agent - serialize QMP sessions per VM (the monitor is single-client) and say "another operation holds it" instead of "the VM has likely exited" - poll liveness while waiting for SHUTDOWN so a crashed VM is reported as exited rather than as a guest ignoring ACPI - default command timeout, with a longer bound for savevm/loadvm - stricter snapshot tags; log destructive operations to stderr Adds tests/test_reliability.py covering the conditions above.
262 lines
10 KiB
Python
262 lines
10 KiB
Python
"""Failure-path tests for the conditions found in the reliability review.
|
|
|
|
Every test here corresponds to a way the server used to fail badly: a corrupt
|
|
state file taking down every tool, a destructive tool deleting outside its
|
|
tree or while QEMU still held the file, a guest agent that stops answering
|
|
mid-call, two instances clobbering each other's records.
|
|
"""
|
|
|
|
import asyncio
|
|
import json
|
|
import socket
|
|
|
|
import pytest
|
|
from fastmcp import Client
|
|
from fastmcp.exceptions import ToolError
|
|
|
|
from conftest import FakeQMPClient, result_data, write_registry
|
|
from mcqemu.config import Config
|
|
from mcqemu.models import VMRecord
|
|
from mcqemu.registry import VMRegistry
|
|
from mcqemu.server import mcp
|
|
from test_lifecycle import seeded_record
|
|
|
|
|
|
def config_for(dirs) -> Config:
|
|
return Config(state_dir=dirs.state, runtime_dir=dirs.run)
|
|
|
|
|
|
# --- registry survives damage ----------------------------------------------
|
|
|
|
|
|
async def test_corrupt_registry_does_not_kill_the_server(dirs):
|
|
"""A truncated state file must not make every tool unreachable."""
|
|
(dirs.state / "vms.json").write_text('{"broken": ')
|
|
async with Client(mcp) as client:
|
|
listing = result_data(await client.call_tool("list_vms", {}))
|
|
assert listing["vms"] == []
|
|
assert any("quarantined" in w for w in listing["registry_warnings"])
|
|
assert list(dirs.state.glob("vms.corrupt.*.json")), "bad file should be preserved"
|
|
|
|
|
|
async def test_malformed_record_is_skipped_not_fatal(dirs):
|
|
good = seeded_record(dirs, "good")
|
|
(dirs.state / "vms.json").write_text(
|
|
json.dumps({"good": good.to_dict(), "broken": {"no": "required fields"}})
|
|
)
|
|
async with Client(mcp) as client:
|
|
listing = result_data(await client.call_tool("list_vms", {}))
|
|
assert [v["name"] for v in listing["vms"]] == ["good"]
|
|
assert any("broken" in w for w in listing["registry_warnings"])
|
|
|
|
|
|
async def test_traversal_key_is_dropped_on_load(dirs):
|
|
"""A registry key that is not a valid VM name never reaches the tools."""
|
|
rec = seeded_record(dirs, "ok")
|
|
payload = {"../../escape": rec.to_dict(), "ok": rec.to_dict()}
|
|
(dirs.state / "vms.json").write_text(json.dumps(payload))
|
|
async with Client(mcp) as client:
|
|
listing = result_data(await client.call_tool("list_vms", {}))
|
|
assert [v["name"] for v in listing["vms"]] == ["ok"]
|
|
assert any("invalid VM name" in w for w in listing["registry_warnings"])
|
|
|
|
|
|
def test_unknown_schema_version_refuses_rather_than_stripping(dirs):
|
|
(dirs.state / "vms.json").write_text(json.dumps({"version": 99, "vms": {}}))
|
|
registry = VMRegistry(config_for(dirs))
|
|
registry.load()
|
|
assert registry.all() == []
|
|
assert "newer than this build" in registry.degraded
|
|
|
|
|
|
def test_unknown_record_fields_round_trip(dirs):
|
|
"""An older build must not strip a newer one's fields when it saves."""
|
|
registry = VMRegistry(config_for(dirs))
|
|
registry.load()
|
|
raw = seeded_record(dirs, "vm1").to_dict() | {"future_field": {"keep": "me"}}
|
|
(dirs.state / "vms.json").write_text(json.dumps({"vm1": raw}))
|
|
|
|
registry2 = VMRegistry(config_for(dirs))
|
|
registry2.load()
|
|
registry2.add(seeded_record(dirs, "vm2")) # triggers a full rewrite
|
|
|
|
on_disk = json.loads((dirs.state / "vms.json").read_text())["vms"]
|
|
assert on_disk["vm1"]["future_field"] == {"keep": "me"}
|
|
|
|
|
|
def test_concurrent_instances_merge_instead_of_clobbering(dirs):
|
|
"""Two mcqemu processes sharing a registry must not orphan each other."""
|
|
a, b = VMRegistry(config_for(dirs)), VMRegistry(config_for(dirs))
|
|
a.load()
|
|
b.load() # both start from an empty view
|
|
a.add(seeded_record(dirs, "from-a"))
|
|
b.add(seeded_record(dirs, "from-b")) # stale in-memory view, must still merge
|
|
|
|
fresh = VMRegistry(config_for(dirs))
|
|
fresh.load()
|
|
assert sorted(fresh.names()) == ["from-a", "from-b"]
|
|
|
|
|
|
def test_name_reservation_blocks_a_second_in_flight_launch(dirs):
|
|
registry = VMRegistry(config_for(dirs))
|
|
registry.load()
|
|
assert registry.reserve("vm1") is True
|
|
assert registry.reserve("vm1") is False
|
|
registry.release("vm1")
|
|
assert registry.reserve("vm1") is True
|
|
|
|
|
|
# --- destructive tool refuses to guess -------------------------------------
|
|
|
|
|
|
async def test_sandbox_destroy_refuses_path_outside_state_dir(dirs, all_pids_dead, monkeypatch):
|
|
"""Even with a hostile record, deletion stays inside the VM state tree."""
|
|
victim = dirs.state / "IMPORTANT"
|
|
victim.mkdir()
|
|
(victim / "precious.txt").write_text("keep me")
|
|
|
|
record = seeded_record(dirs, "sb", config={"disks": [], "sandbox_overlay": "/tmp/x.qcow2"})
|
|
write_registry(dirs, record)
|
|
# Simulate a corrupted config whose state dir escapes the vms/ root.
|
|
monkeypatch.setattr(Config, "vm_state_dir", lambda self, name: victim)
|
|
|
|
async with Client(mcp) as client:
|
|
with pytest.raises(ToolError, match="not inside"):
|
|
await client.call_tool("sandbox_destroy", {"name": "sb"})
|
|
assert (victim / "precious.txt").exists()
|
|
|
|
|
|
async def test_sandbox_destroy_refuses_when_vm_survives_kill(dirs, all_pids_alive, monkeypatch):
|
|
"""If the process will not die, its overlay must not be deleted."""
|
|
overlay = dirs.state / "vms" / "sb" / "overlay.qcow2"
|
|
overlay.parent.mkdir(parents=True)
|
|
overlay.write_bytes(b"disk")
|
|
write_registry(
|
|
dirs,
|
|
seeded_record(dirs, "sb", config={"disks": [], "sandbox_overlay": str(overlay)}),
|
|
)
|
|
monkeypatch.setattr("mcqemu.tools.sandbox.os.kill", lambda pid, sig: None)
|
|
monkeypatch.setattr("mcqemu.tools.sandbox._await_exit", _never_exits)
|
|
|
|
async with Client(mcp) as client:
|
|
with pytest.raises(ToolError, match="survived both"):
|
|
await client.call_tool("sandbox_destroy", {"name": "sb"})
|
|
assert overlay.exists(), "overlay must survive a failed kill"
|
|
async with Client(mcp) as client:
|
|
listing = result_data(await client.call_tool("list_vms", {}))
|
|
assert [v["name"] for v in listing["vms"]] == ["sb"], "record must not be dropped"
|
|
|
|
|
|
async def _never_exits(pid, seconds):
|
|
return False
|
|
|
|
|
|
async def test_sandbox_destroy_reports_cleanup_failure_honestly(dirs, all_pids_dead, monkeypatch):
|
|
overlay = dirs.state / "vms" / "sb" / "overlay.qcow2"
|
|
overlay.parent.mkdir(parents=True)
|
|
overlay.write_bytes(b"disk")
|
|
write_registry(
|
|
dirs,
|
|
seeded_record(dirs, "sb", config={"disks": [], "sandbox_overlay": str(overlay)}),
|
|
)
|
|
|
|
def boom(target, onexc=None):
|
|
onexc(None, str(target), PermissionError("read-only filesystem"))
|
|
|
|
monkeypatch.setattr("mcqemu.tools.sandbox.shutil.rmtree", boom)
|
|
async with Client(mcp) as client:
|
|
data = result_data(await client.call_tool("sandbox_destroy", {"name": "sb"}))
|
|
assert data["destroyed"] is False
|
|
assert data["cleanup_errors"], "a failed delete must be reported, not swallowed"
|
|
|
|
|
|
# --- launch does not trample a live VM -------------------------------------
|
|
|
|
|
|
async def test_launch_refuses_to_reuse_a_live_qmp_socket(dirs):
|
|
"""An unregistered but running QEMU must not lose its monitor socket."""
|
|
run_dir = dirs.run / "ghost"
|
|
run_dir.mkdir(parents=True)
|
|
sock_path = run_dir / "qmp.sock"
|
|
listener = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
|
|
listener.bind(str(sock_path))
|
|
listener.listen(1)
|
|
try:
|
|
async with Client(mcp) as client:
|
|
with pytest.raises(ToolError, match="LIVE QMP socket"):
|
|
await client.call_tool("launch_vm", {"name": "ghost", "no_net": True})
|
|
assert sock_path.exists(), "the live socket must not be unlinked"
|
|
finally:
|
|
listener.close()
|
|
|
|
|
|
# --- guest agent cannot hang the server ------------------------------------
|
|
|
|
|
|
async def test_guest_exec_times_out_when_agent_stops_answering(dirs, fake_qmp, monkeypatch):
|
|
monkeypatch.setattr("mcqemu.tools.guest.CALL_TIMEOUT", 1.0)
|
|
FakeQMPClient.responses["guest-sync"] = lambda args: args["id"]
|
|
FakeQMPClient.responses["guest-exec"] = {"pid": 7}
|
|
FakeQMPClient.responses["guest-exec-status"] = lambda args: asyncio.sleep(3600)
|
|
write_registry(dirs, seeded_record(dirs, "vm1"))
|
|
async with Client(mcp) as client:
|
|
with pytest.raises(ToolError, match="stopped responding"):
|
|
await asyncio.wait_for(
|
|
client.call_tool("guest_exec", {"name": "vm1", "command": "sleep", "timeout": 2}),
|
|
timeout=30,
|
|
)
|
|
|
|
|
|
async def test_guest_file_read_gives_up_on_a_zero_progress_agent(dirs, fake_qmp):
|
|
"""A FIFO or tty returns count=0 without EOF forever; don't spin on it."""
|
|
FakeQMPClient.responses["guest-sync"] = lambda args: args["id"]
|
|
FakeQMPClient.responses["guest-file-open"] = 3
|
|
FakeQMPClient.responses["guest-file-read"] = {"count": 0, "eof": False}
|
|
FakeQMPClient.responses["guest-file-close"] = {}
|
|
write_registry(dirs, seeded_record(dirs, "vm1"))
|
|
async with Client(mcp) as client:
|
|
data = result_data(
|
|
await asyncio.wait_for(
|
|
client.call_tool("guest_file_read", {"name": "vm1", "path": "/dev/fifo"}),
|
|
timeout=20,
|
|
)
|
|
)
|
|
assert data["bytes_read"] == 0
|
|
reads = [c for c, _ in FakeQMPClient.calls if c == "guest-file-read"]
|
|
assert len(reads) < 10, "should stop after a few empty reads, not loop"
|
|
|
|
|
|
async def test_guest_file_read_rejects_absurd_max_bytes(dirs, fake_qmp):
|
|
FakeQMPClient.responses["guest-sync"] = lambda args: args["id"]
|
|
write_registry(dirs, seeded_record(dirs, "vm1"))
|
|
async with Client(mcp) as client:
|
|
with pytest.raises(ToolError, match="max_bytes"):
|
|
await client.call_tool(
|
|
"guest_file_read", {"name": "vm1", "path": "/x", "max_bytes": 10**12}
|
|
)
|
|
|
|
|
|
# --- untrusted bytes --------------------------------------------------------
|
|
|
|
|
|
async def test_screenshot_rejects_non_png_output(dirs, fake_qmp):
|
|
def write_junk(args):
|
|
with open(args["filename"], "wb") as f:
|
|
f.write(b"not a png at all")
|
|
return {}
|
|
|
|
FakeQMPClient.responses["screendump"] = write_junk
|
|
write_registry(dirs, seeded_record(dirs, "vm1"))
|
|
async with Client(mcp) as client:
|
|
with pytest.raises(ToolError, match="valid PNG"):
|
|
await client.call_tool("vm_click", {"name": "vm1", "x": 1, "y": 1})
|
|
|
|
|
|
def test_record_name_follows_the_registry_key(dirs):
|
|
"""Tools look records up by key, so a mismatched inner name is corrected."""
|
|
rec = VMRecord(name="lies", source="spawned", qmp_socket="/tmp/q.sock")
|
|
(dirs.state / "vms.json").write_text(json.dumps({"truth": rec.to_dict()}))
|
|
registry = VMRegistry(config_for(dirs))
|
|
registry.load()
|
|
assert registry.get("truth").name == "truth"
|