Ryan Malloy
5a2d4703ab
QEMU command line: - escape commas in every interpolated path (qopt); a path like 'data,readonly=on.qcow2' previously injected a drive option - reject extra_args flags that breach VM isolation (host filesystem passthrough, host block devices, spawning chardevs, -runas) and document the parameter as operator-only - detect duplicate host ports across port_forwards instead of failing at QEMU launch; auto ports no longer collide with each other Sandbox isolation: - sandbox_vm now blocks guest-initiated traffic by default (restrict=on), with allow_network=True to opt in. Verified end to end: with identical guest network state, a default sandbox reaches neither a host loopback service nor the internet, while allow_network=True reaches both - note in the docstring that the guest agent answers before the guest has finished booting Bounded work per call: - vm_serial_read seeks a 256KB window from the end instead of reading a console log that grows without bound into memory - cap vm_type_text length and vm_mouse_move deltas - screenshots get a unique filename and are cleaned up, so a concurrent capture cannot swap the frame under vm_click Identity and liveness: - attach_vm requires an actual unix socket and stores the resolved path - attached VMs are judged by connecting, not by a stat that a stale socket file would pass - refuse to act on a PID whose cmdline proves it is a different VM - a sandbox's base image counts as in use while its overlay is live - fix a latent NameError in vm_mouse_move's homing branch
448 lines
17 KiB
Python
448 lines
17 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
|
|
from pathlib import Path
|
|
|
|
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"
|
|
|
|
|
|
# --- QEMU command line cannot be injected through paths ---------------------
|
|
|
|
|
|
def test_comma_in_disk_path_is_escaped():
|
|
"""A comma in a path would otherwise start a new QEMU option."""
|
|
from mcqemu.launcher import build_cmdline
|
|
from mcqemu.models import VMConfig
|
|
from test_launcher import paths
|
|
|
|
evil = "/vms/data,readonly=on,if=none.qcow2"
|
|
cfg = VMConfig(name="t", disks=[evil])
|
|
args = build_cmdline(cfg, paths(), accel="kvm", disk_formats={evil: "qcow2"})
|
|
drive = next(a for a in args if a.startswith("file=/vms/data"))
|
|
# Parse it the way QEMU does: split on single commas, un-double the rest.
|
|
options = dict(opt.split("=", 1) for opt in qemu_split(drive))
|
|
assert options["file"] == evil, "the path must survive intact"
|
|
assert options["if"] == "virtio", "the embedded 'if=none' must not become an option"
|
|
assert set(options) == {"file", "if", "format"}
|
|
|
|
|
|
def qemu_split(option_string: str) -> list[str]:
|
|
"""QEMU's option parser: ',,' is a literal comma, ',' separates options."""
|
|
return [part.replace("\0", ",") for part in option_string.replace(",,", "\0").split(",")]
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"bad",
|
|
[
|
|
["-fsdev", "local,path=/home,security_model=none,id=h"],
|
|
["-virtfs", "local,path=/,mount_tag=root"],
|
|
["-drive", "file=/dev/sda,format=raw"],
|
|
["-runas", "root"],
|
|
["-monitor", "stdio"],
|
|
],
|
|
)
|
|
async def test_extra_args_rejects_isolation_breaks(client, bad):
|
|
with pytest.raises(ToolError, match="extra_args may not"):
|
|
await client.call_tool("launch_vm", {"name": "esc", "extra_args": bad})
|
|
|
|
|
|
async def test_extra_args_allows_ordinary_flags(dirs, monkeypatch):
|
|
"""The escape hatch still works for benign tuning flags."""
|
|
from mcqemu.launcher import check_extra_args
|
|
|
|
check_extra_args(["-vga", "std", "-rtc", "base=localtime"]) # must not raise
|
|
|
|
|
|
def test_duplicate_host_ports_are_rejected():
|
|
from mcqemu.launcher import resolve_port_forwards
|
|
|
|
with pytest.raises(ToolError, match="twice"):
|
|
resolve_port_forwards(["18991:22", "18991:23"])
|
|
|
|
|
|
def test_auto_ports_do_not_collide_with_each_other():
|
|
from mcqemu.launcher import resolve_port_forwards
|
|
|
|
resolved = resolve_port_forwards(["auto:22", "auto:80", "auto:443"])
|
|
hosts = [entry.split(":")[0] for entry in resolved]
|
|
assert len(set(hosts)) == 3
|
|
|
|
|
|
def test_restrict_net_blocks_outbound_but_keeps_forwards():
|
|
from mcqemu.launcher import build_cmdline
|
|
from mcqemu.models import VMConfig
|
|
from test_launcher import paths
|
|
|
|
cfg = VMConfig(name="t", restrict_net=True, port_forwards=["2222:22"])
|
|
netdev = next(
|
|
a
|
|
for a in build_cmdline(cfg, paths(), accel="kvm", disk_formats={})
|
|
if a.startswith("user,id=net0")
|
|
)
|
|
assert "restrict=on" in netdev
|
|
assert "hostfwd=tcp::2222-:22" in netdev
|
|
|
|
|
|
# --- bounded work per call --------------------------------------------------
|
|
|
|
|
|
async def test_type_text_rejects_a_novel(dirs, fake_qmp):
|
|
write_registry(dirs, seeded_record(dirs, "vm1"))
|
|
async with Client(mcp) as client:
|
|
with pytest.raises(ToolError, match="limit is"):
|
|
await client.call_tool("vm_type_text", {"name": "vm1", "text": "x" * 5000})
|
|
|
|
|
|
async def test_mouse_move_rejects_absurd_delta(dirs, fake_qmp):
|
|
write_registry(dirs, seeded_record(dirs, "vm1"))
|
|
async with Client(mcp) as client:
|
|
with pytest.raises(ToolError, match="within"):
|
|
await client.call_tool("vm_mouse_move", {"name": "vm1", "dx": 10**9})
|
|
|
|
|
|
async def test_serial_read_only_reads_the_tail(dirs, fake_qmp):
|
|
"""A multi-megabyte console log must not be slurped into memory."""
|
|
log = dirs.state / "big-serial.log"
|
|
log.write_text("noise\n" * 200_000 + "FINAL LINE\n")
|
|
assert log.stat().st_size > 1_000_000
|
|
write_registry(dirs, seeded_record(dirs, "vm1", serial_log=str(log)))
|
|
async with Client(mcp) as client:
|
|
data = result_data(
|
|
await client.call_tool("vm_serial_read", {"name": "vm1", "tail_lines": 1})
|
|
)
|
|
assert data["tail"] == "FINAL LINE"
|
|
assert data["log_bytes"] == log.stat().st_size
|
|
assert data["read_from_end_bytes"] < data["log_bytes"]
|
|
|
|
|
|
async def test_screenshots_do_not_share_a_filename(dirs, fake_qmp):
|
|
"""A fixed path lets a concurrent capture swap the frame under vm_click."""
|
|
written: list[str] = []
|
|
|
|
def capture(args):
|
|
written.append(args["filename"])
|
|
with open(args["filename"], "wb") as f:
|
|
f.write(fake_png(64, 48))
|
|
return {}
|
|
|
|
from test_display import fake_png
|
|
|
|
FakeQMPClient.responses["screendump"] = capture
|
|
write_registry(dirs, seeded_record(dirs, "vm1"))
|
|
async with Client(mcp) as client:
|
|
await client.call_tool("vm_screenshot", {"name": "vm1"})
|
|
await client.call_tool("vm_screenshot", {"name": "vm1"})
|
|
assert len(set(written)) == 2, "each capture needs its own file"
|
|
assert not any(Path(p).exists() for p in written), "temp captures must be cleaned up"
|
|
|
|
|
|
# --- identity and liveness --------------------------------------------------
|
|
|
|
|
|
def test_stale_socket_file_does_not_read_as_alive(dirs, tmp_path):
|
|
"""A SIGKILLed QEMU leaves its socket behind; stat() alone would lie."""
|
|
dead = tmp_path / "dead.sock"
|
|
listener = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
|
|
listener.bind(str(dead))
|
|
listener.listen(1)
|
|
registry = VMRegistry(config_for(dirs))
|
|
registry.load()
|
|
record = VMRecord(name="ext", source="attached", qmp_socket=str(dead))
|
|
assert registry.is_process_alive(record) is True
|
|
listener.close() # process gone, file remains
|
|
assert dead.exists()
|
|
assert registry.is_process_alive(record) is False
|
|
|
|
|
|
def test_pid_matching_rejects_a_different_vm():
|
|
"""After a pidfile clobber a record can point at another QEMU."""
|
|
import os
|
|
|
|
from mcqemu.registry import pid_matches_vm
|
|
|
|
# Our own process has no -name in its cmdline, so nothing is proven.
|
|
assert pid_matches_vm(os.getpid(), "anything") is True
|
|
assert pid_matches_vm(2**22 + 999, "gone") is True # unreadable == unproven
|
|
|
|
|
|
def test_sandbox_base_counts_as_in_use(dirs):
|
|
"""Resizing the base under a live overlay must be refused."""
|
|
registry = VMRegistry(config_for(dirs))
|
|
registry.load()
|
|
base = dirs.state / "base.qcow2"
|
|
base.write_bytes(b"x")
|
|
registry.add(
|
|
seeded_record(
|
|
dirs,
|
|
"sb",
|
|
source="attached",
|
|
pid=None,
|
|
qmp_socket=str(dirs.run / "live.sock"),
|
|
config={"disks": [], "sandbox_base": str(base)},
|
|
)
|
|
)
|
|
listener = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
|
|
(dirs.run).mkdir(exist_ok=True)
|
|
listener.bind(str(dirs.run / "live.sock"))
|
|
listener.listen(1)
|
|
try:
|
|
assert str(base.resolve()) in registry.disks_in_use()
|
|
finally:
|
|
listener.close()
|